Make waiting on auth an explicit state in the context state machine

Previously we relied on checking tls_authentication_status to check
wether to determine if the context auth state is actually valid or not.
This patch eliminates that check by introducing waiting on the
authentication as extra state in the context auth, state machine.

The simplification and reorganization of the state machine in this
and the previous patches also eliminates a number of corner cases,
including the specific one that lead to CVE-2020-15078.

Patch v3: Fix ccd config from management being ignored
Patch v4: Fix race condition, we need to accept the config from
          management if we are in CAS_WAITING_AUTH or earlier states
	  and not just in CAS_WAITING_AUTH state

CVE: 2020-15078

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210604143938.779193-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22491.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 52b177c1b080222d049dbde46af8fb9cc7b29855..024b3042771a1bed40af6f87551a46ced9fa2f62 100644 (file)
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2596,11 +2596,6 @@ static const multi_client_connect_handler client_connect_handlers[] = {
 static void
 multi_connection_established(struct multi_context *m, struct multi_instance *mi)
 {
-    if (tls_authentication_status(mi->context.c2.tls_multi) != TLS_AUTHENTICATION_SUCCEEDED)
-    {
-        return;
-    }
-
     /* We are only called for the CAS_PENDING_x states, so we
      * can ignore other states here */
     bool from_deferred = (mi->context.c2.tls_multi->multi_state != CAS_PENDING);
@@ -3970,7 +3965,7 @@ management_client_auth(void *arg,
         {
             if (auth)
             {
-                if (is_cas_pending(mi->context.c2.tls_multi->multi_state))
+                if (mi->context.c2.tls_multi->multi_state <= CAS_WAITING_AUTH)
                 {
                     set_cc_config(mi, cc_config);
                     cc_config_owned = false;

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 3fd2cefb55fbec5d688860cfcd069b26f7f3c88b..3288628ceee08982cfe7db03a3406b621692984e 100644 (file)
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -2810,7 +2810,7 @@ tls_process(struct tls_multi *multi,
                     if (session->opt->mode == MODE_SERVER)
                     {
                         /* On a server we continue with running connect scripts next */
-                        multi->multi_state = CAS_PENDING;
+                        multi->multi_state = CAS_WAITING_AUTH;
                     }
                     else
                     {
@@ -3136,6 +3136,13 @@ tls_multi_process(struct tls_multi *multi,
 
     enum tls_auth_status tas = tls_authentication_status(multi);
 
+    /* If we have successfully authenticated and are still waiting for the authentication to finish
+     * move the state machine for the multi context forward */
+    if (multi->multi_state == CAS_WAITING_AUTH && tas == TLS_AUTHENTICATION_SUCCEEDED)
+    {
+        multi->multi_state = CAS_PENDING;
+    }
+
     /*
      * If lame duck session expires, kill it.
      */

diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index ded86050dd0d3cd5b755ee0330e00c75d5c4b2b6..33ddaac119895d26920404186e36599436998f88 100644 (file)
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -511,6 +511,7 @@ struct tls_session
  * connect scripts/plugins */
 enum multi_status {
     CAS_NOT_CONNECTED,
+    CAS_WAITING_AUTH,               /**< TLS connection established but deferred auth not finished */
     CAS_PENDING,
     CAS_PENDING_DEFERRED,
     CAS_PENDING_DEFERRED_PARTIAL,   /**< at least handler succeeded, no result yet*/