Move context_auth from context_2 to tls_multi and name it multi_state

context_2 and tls_multi have the same life cycle for TLS connections
but so this move does not affect behaviour of the variable.

OpenVPN TLS multi code has a grown a lot more complex and code that
handles multi objects needs to know the state that the object is in.
Since not all code has access to the context_2 struct, the code that
does not have access is often not checking the state directly but
checks other parts of multi that have been affected from a state
change.

This patch also renames it to multi_state as this variable represents
the multi state machine status rather than just the state of the connect
authentication (more upcoming patches will move other states
into this variable).

Patch V2: also rename context_auth to multi_state, explain a bit why this
          change is done.
Patch V3: Add comments for c2->multi NULL check forwarding. Fix compile
          with ENABLE_ASYNC_PUSH.

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

diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index 29b52b8dde87804799e87942156a298058a77fa8..df96d04805111a9ed7c539b4aa4c226b40317cde 100644 (file)
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -533,9 +533,10 @@ encrypt_sign(struct context *c, bool comp_frag)
 
     /*
      * Drop non-TLS outgoing packet if client-connect script/plugin
-     * has not yet succeeded.
+     * has not yet succeeded. In non-TLS tls_multi mode is not defined
+     * and we always pass packets.
      */
-    if (c->c2.context_auth != CAS_SUCCEEDED)
+    if (c->c2.tls_multi && c->c2.tls_multi->multi_state != CAS_SUCCEEDED)
     {
         c->c2.buf.len = 0;
     }
@@ -967,9 +968,10 @@ process_incoming_link_part1(struct context *c, struct link_socket_info *lsi, boo
 
         /*
          * Drop non-TLS packet if client-connect script/plugin and cipher selection
-         * has not yet succeeded.
+         * has not yet succeeded. In non-TLS mode tls_multi is not defined
+         * and we always pass packets.
          */
-        if (c->c2.context_auth != CAS_SUCCEEDED)
+        if (c->c2.tls_multi && c->c2.tls_multi->multi_state != CAS_SUCCEEDED)
         {
             c->c2.buf.len = 0;
         }

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 7c9500f3eee3b395878e5f5920af45009ae0e801..4a769fecd6265b8d82f751cb481356c2c8b30987 100644 (file)
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -674,7 +674,7 @@ multi_close_instance(struct multi_context *m,
 #ifdef ENABLE_MANAGEMENT
     set_cc_config(mi, NULL);
 #endif
-    if (mi->context.c2.context_auth == CAS_SUCCEEDED)
+    if (mi->context.c2.tls_multi->multi_state == CAS_SUCCEEDED)
     {
         multi_client_disconnect_script(mi);
     }
@@ -775,7 +775,7 @@ multi_create_instance(struct multi_context *m, const struct mroute_addr *real)
         goto err;
     }
 
-    mi->context.c2.context_auth = CAS_PENDING;
+    mi->context.c2.tls_multi->multi_state = CAS_PENDING;
 
     if (hash_n_elements(m->hash) >= m->max_clients)
     {
@@ -2405,18 +2405,18 @@ multi_client_connect_late_setup(struct multi_context *m,
     mi->reporting_addr_ipv6 = mi->context.c2.push_ifconfig_ipv6_local;
 
     /* set context-level authentication flag */
-    mi->context.c2.context_auth = CAS_SUCCEEDED;
+    mi->context.c2.tls_multi->multi_state = CAS_SUCCEEDED;
 
     /* authentication complete, calculate dynamic client specific options */
     if (!multi_client_set_protocol_options(&mi->context))
     {
-        mi->context.c2.context_auth = CAS_FAILED;
+        mi->context.c2.tls_multi->multi_state = CAS_FAILED;
     }
     /* Generate data channel keys only if setting protocol options
      * has not failed */
     else if (!multi_client_generate_tls_keys(&mi->context))
     {
-        mi->context.c2.context_auth = CAS_FAILED;
+        mi->context.c2.tls_multi->multi_state = CAS_FAILED;
     }
 
     /* send push reply if ready */
@@ -2601,7 +2601,7 @@ multi_connection_established(struct multi_context *m, struct multi_instance *mi)
 
     /* We are only called for the CAS_PENDING_x states, so we
      * can ignore other states here */
-    bool from_deferred = (mi->context.c2.context_auth != CAS_PENDING);
+    bool from_deferred = (mi->context.c2.tls_multi->multi_state != CAS_PENDING);
 
     int *cur_handler_index = &mi->client_connect_defer_state.cur_handler_index;
     unsigned int *option_types_found =
@@ -2613,7 +2613,7 @@ multi_connection_established(struct multi_context *m, struct multi_instance *mi)
         *cur_handler_index = 0;
         *option_types_found = 0;
         /* Initially we have no handler that has returned a result */
-        mi->context.c2.context_auth = CAS_PENDING_DEFERRED;
+        mi->context.c2.tls_multi->multi_state = CAS_PENDING_DEFERRED;
 
         multi_client_connect_early_setup(m, mi);
     }
@@ -2636,7 +2636,7 @@ multi_connection_established(struct multi_context *m, struct multi_instance *mi)
                  * Remember that we already had at least one handler
                  * returning a result should we go to into deferred state
                  */
-                mi->context.c2.context_auth = CAS_PENDING_DEFERRED_PARTIAL;
+                mi->context.c2.tls_multi->multi_state = CAS_PENDING_DEFERRED_PARTIAL;
                 break;
 
             case CC_RET_SKIPPED:
@@ -2688,12 +2688,12 @@ multi_connection_established(struct multi_context *m, struct multi_instance *mi)
     {
         /* run the disconnect script if we had a connect script that
          * did not fail */
-        if (mi->context.c2.context_auth == CAS_PENDING_DEFERRED_PARTIAL)
+        if (mi->context.c2.tls_multi->multi_state == CAS_PENDING_DEFERRED_PARTIAL)
         {
             multi_client_disconnect_script(mi);
         }
 
-        mi->context.c2.context_auth = CAS_FAILED;
+        mi->context.c2.tls_multi->multi_state = CAS_FAILED;
     }
 
     /* increment number of current authenticated clients */
@@ -2996,13 +2996,13 @@ multi_process_post(struct multi_context *m, struct multi_instance *mi, const uns
         {
             /* connection is "established" when SSL/TLS key negotiation succeeds
              * and (if specified) auth user/pass succeeds */
-            if (is_cas_pending(mi->context.c2.context_auth)
+            if (is_cas_pending(mi->context.c2.tls_multi->multi_state)
                 && CONNECTION_ESTABLISHED(&mi->context))
             {
                 multi_connection_established(m, mi);
             }
 #if defined(ENABLE_ASYNC_PUSH)
-            if (is_cas_pending(mi->context.c2.context_auth)
+            if (is_cas_pending(mi->context.c2.tls_multi->multi_state)
                 && mi->client_connect_defer_state.deferred_ret_file)
             {
                 add_inotify_file_watch(m, mi, m->top.c2.inotify_fd,
@@ -3962,7 +3962,7 @@ management_client_auth(void *arg,
         {
             if (auth)
             {
-                if (is_cas_pending(mi->context.c2.context_auth))
+                if (is_cas_pending(mi->context.c2.tls_multi->multi_state))
                 {
                     set_cc_config(mi, cc_config);
                     cc_config_owned = false;

diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h
index 22d2447f6ee09f2194b6cc5cb5fad45abfccfa11..a15ff08d16e79bb15030c7e232a13c498fed481e 100644 (file)
--- a/src/openvpn/openvpn.h
+++ b/src/openvpn/openvpn.h
@@ -205,17 +205,6 @@ struct context_1
 };
 
 
-/* client authentication state, CAS_SUCCEEDED must be 0 since
- * non multi code path still checks this variable but does not initialise it
- * so the code depends on zero initialisation */
-enum client_connect_status {
-    CAS_SUCCEEDED=0,
-    CAS_PENDING,
-    CAS_PENDING_DEFERRED,
-    CAS_PENDING_DEFERRED_PARTIAL,   /**< at least handler succeeded, no result yet*/
-    CAS_FAILED,
-};
-
 static inline bool
 is_cas_pending(enum client_connect_status cas)
 {
@@ -444,9 +433,6 @@ struct context_2
     int push_ifconfig_ipv6_netbits;
     struct in6_addr push_ifconfig_ipv6_remote;
 
-
-    enum client_connect_status context_auth;
-
     struct event_timeout push_request_interval;
     time_t push_request_timeout;
 

diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index 47a67e5031a082044979fe1095c48b632d970336..15a9141eec090e69af3eaed88ea00dc874487316 100644 (file)
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -852,14 +852,14 @@ process_incoming_push_request(struct context *c)
 {
     int ret = PUSH_MSG_ERROR;
 
-    if ((c->c2.tls_multi && tls_authentication_status(c->c2.tls_multi, 0) == TLS_AUTHENTICATION_FAILED)
-        || c->c2.context_auth == CAS_FAILED)
+    if (tls_authentication_status(c->c2.tls_multi, 0) == TLS_AUTHENTICATION_FAILED
+        || c->c2.tls_multi->multi_state == CAS_FAILED)
     {
         const char *client_reason = tls_client_reason(c->c2.tls_multi);
         send_auth_failed(c, client_reason);
         ret = PUSH_MSG_AUTH_FAILURE;
     }
-    else if (c->c2.context_auth == CAS_SUCCEEDED)
+    else if (c->c2.tls_multi->multi_state == CAS_SUCCEEDED)
     {
         time_t now;
 

diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index b2fa44e56c3da065432b3cf378d3058cd10e08c5..5b24623d47e8b90422545cfc9dc45b2e544f7449 100644 (file)
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -488,6 +488,19 @@ struct tls_session
  */
 #define KEY_SCAN_SIZE 3
 
+
+/* client authentication state, CAS_SUCCEEDED must be 0 since
+ * non multi code path still checks this variable but does not initialise it
+ * so the code depends on zero initialisation */
+enum client_connect_status {
+    CAS_SUCCEEDED=0,
+    CAS_PENDING,
+    CAS_PENDING_DEFERRED,
+    CAS_PENDING_DEFERRED_PARTIAL,   /**< at least handler succeeded, no result yet*/
+    CAS_FAILED,
+};
+
+
 /**
  * Security parameter state for a single VPN tunnel.
  * @ingroup control_processor
@@ -523,6 +536,7 @@ struct tls_multi
 
     int n_sessions;             /**< Number of sessions negotiated thus
                                  *   far. */
+    enum client_connect_status multi_state;
 
     /*
      * Number of errors.