]> git.ipfire.org Git - thirdparty/openvpn.git/commitdiff
Introduce connection state for reconnecting peer in p2p
authorArne Schwabe <arne@rfc2549.org>
Wed, 30 Nov 2022 16:57:05 +0000 (17:57 +0100)
committerGert Doering <gert@greenie.muc.de>
Wed, 30 Nov 2022 19:21:25 +0000 (20:21 +0100)
We introduce this state to make the reconnecting of a client more
obvious and what is called again instead of making it implicit. The
new state CAS_RECONNECT_PENDING is between CAS_WAITING_OPTIONS_IMPORT and
CAS_CONNECT_DONE as we need to redo some of the steps of the connection
setup, so this new state is going a "half step" back in the state machine.

We also do no longer generate data channel keys for untrusted session. This
is done for clarity but also to allow them being generated after the
session has become actually active.

These changes allow a reconnect in p2p mode with DCO to work as the initial
reconnect working.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221130165705.159610-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25595.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
src/openvpn/forward.c
src/openvpn/init.c
src/openvpn/ssl.c
src/openvpn/ssl.h
src/openvpn/ssl_common.h

index 3b5b04074055d2e3b40f85c2a586ef1c623a93ac..37340aef53efee8798b1212cb4ece8d20553eb9d 100644 (file)
@@ -174,7 +174,14 @@ check_tls(struct context *c)
         const int tmp_status = tls_multi_process
                                    (c->c2.tls_multi, &c->c2.to_link, &c->c2.to_link_addr,
                                    get_link_socket_info(c), &wakeup);
-        if (tmp_status == TLSMP_ACTIVE)
+
+        if (tmp_status == TLSMP_RECONNECT)
+        {
+            event_timeout_init(&c->c2.wait_for_connect, 1, now);
+            reset_coarse_timers(c);
+        }
+
+        if (tmp_status == TLSMP_ACTIVE || tmp_status == TLSMP_RECONNECT)
         {
             update_time();
             interval_action(&c->c2.tmp_int);
@@ -196,9 +203,15 @@ check_tls(struct context *c)
 
     interval_schedule_wakeup(&c->c2.tmp_int, &wakeup);
 
-    /* Our current code has no good hooks in the TLS machinery to update
+    /*
+     * Our current code has no good hooks in the TLS machinery to update
      * DCO keys. So we check the key status after the whole TLS machinery
      * has been completed and potentially update them
+     *
+     * We have a hidden state transition from secondary to primary key based
+     * on ks->auth_deferred_expire that DCO needs to check that the normal
+     * TLS state engine does not check. So we call the \c check_dco_key_status
+     * function even if tmp_status does not indicate that something has changed.
      */
     check_dco_key_status(c);
 
@@ -302,7 +315,6 @@ check_push_request(struct context *c)
 static void
 check_connection_established(struct context *c)
 {
-
     if (connection_established(c))
     {
         /* if --pull was specified, send a push request to server */
@@ -337,7 +349,6 @@ check_connection_established(struct context *c)
 
         event_timeout_clear(&c->c2.wait_for_connect);
     }
-
 }
 
 bool
index 0e476977574c852f44e4af79a176815b16120a6c..eb7b4b8053f7be83f2848e61401b1269b8a7cdb3 100644 (file)
@@ -2219,7 +2219,14 @@ do_up(struct context *c, bool pulled_options, unsigned int option_types_found)
                 }
             }
         }
+    }
 
+    /* This part needs to be run in p2p mode (without pull) when the client
+     * reconnects to setup various things (like DCO and NCP cipher) that
+     * might have changed from the previous connection.
+     */
+    if (!c->c2.do_up_ran || (c->c2.tls_multi && c->c2.tls_multi->multi_state == CAS_RECONNECT_PENDING))
+    {
         if (c->mode == MODE_POINT_TO_POINT)
         {
             /* ovpn-dco requires adding the peer now, before any option can be set,
index 818100c23bd1f0b01b7fd67971af426313b0d6d3..9e5480528a4138da020a486fd9ae8c8882d4971b 100644 (file)
@@ -3249,29 +3249,29 @@ tls_multi_process(struct tls_multi *multi,
 
     if (multi->multi_state >= CAS_CONNECT_DONE)
     {
-        for (int i = 0; i < TM_SIZE; ++i)
-        {
-            struct tls_session *session = &multi->session[i];
-            struct key_state *ks = &session->key[KS_PRIMARY];
+        /* Only generate keys for the TM_ACTIVE session. We defer generating
+         * keys for TM_UNTRUSTED until we actually trust it.
+         * For TM_LAME_DUCK it makes no sense to generate new keys. */
+        struct tls_session *session = &multi->session[TM_ACTIVE];
+        struct key_state *ks = &session->key[KS_PRIMARY];
 
-            if (ks->state == S_ACTIVE && ks->authenticated == KS_AUTH_TRUE)
+        if (ks->state == S_ACTIVE && ks->authenticated == KS_AUTH_TRUE)
+        {
+            /* Session is now fully authenticated.
+            * tls_session_generate_data_channel_keys will move ks->state
+            * from S_ACTIVE to S_GENERATED_KEYS */
+            if (!tls_session_generate_data_channel_keys(multi, session))
             {
-                /* Session is now fully authenticated.
-                * tls_session_generate_data_channel_keys will move ks->state
-                * from S_ACTIVE to S_GENERATED_KEYS */
-                if (!tls_session_generate_data_channel_keys(multi, session))
-                {
-                    msg(D_TLS_ERRORS, "TLS Error: generate_key_expansion failed");
-                    ks->authenticated = KS_AUTH_FALSE;
-                    ks->state = S_ERROR;
-                }
+                msg(D_TLS_ERRORS, "TLS Error: generate_key_expansion failed");
+                ks->authenticated = KS_AUTH_FALSE;
+                ks->state = S_ERROR;
+            }
 
-                /* Update auth token on the client if needed on renegotiation
-                 * (key id !=0) */
-                if (session->key[KS_PRIMARY].key_id != 0)
-                {
-                    resend_auth_token_renegotiation(multi, session);
-                }
+            /* Update auth token on the client if needed on renegotiation
+             * (key id !=0) */
+            if (session->key[KS_PRIMARY].key_id != 0)
+            {
+                resend_auth_token_renegotiation(multi, session);
             }
         }
     }
@@ -3304,6 +3304,12 @@ tls_multi_process(struct tls_multi *multi,
         move_session(multi, TM_ACTIVE, TM_UNTRUSTED, true);
         msg(D_TLS_DEBUG_LOW, "TLS: tls_multi_process: untrusted session promoted to %strusted",
             tas == TLS_AUTHENTICATION_SUCCEEDED ? "" : "semi-");
+
+        if (multi->multi_state == CAS_CONNECT_DONE)
+        {
+            multi->multi_state = CAS_RECONNECT_PENDING;
+            active = TLSMP_RECONNECT;
+        }
     }
 
     /*
index 646ec581afa85ff1a8619f340f239d17c5ec8fc1..55c672d449a977d78adafa33d0272dd6476cea56 100644 (file)
@@ -212,6 +212,7 @@ void tls_multi_free(struct tls_multi *multi, bool clear);
 #define TLSMP_INACTIVE 0
 #define TLSMP_ACTIVE   1
 #define TLSMP_KILL     2
+#define TLSMP_RECONNECT 3
 
 /*
  * Called by the top-level event loop.
index e967970ddaedad81f2940d1c38bc83d78903fb89..978a9fca0ecbdc9289e78ec72cce3d60fc08f67c 100644 (file)
@@ -551,6 +551,10 @@ enum multi_status {
     CAS_PENDING_DEFERRED_PARTIAL,   /**< at least handler succeeded but another is still pending */
     CAS_FAILED,                     /**< Option import failed or explicitly denied the client */
     CAS_WAITING_OPTIONS_IMPORT,     /**< client with pull or p2p waiting for first time options import */
+    CAS_RECONNECT_PENDING,          /**< session has already successful established (CAS_CONNECT_DONE)
+                                     * but has a reconnect and needs to redo some initialisation, this state is
+                                     * similar CAS_WAITING_OPTIONS_IMPORT but skips a few things. The normal connection
+                                     * skips this step. */
     CAS_CONNECT_DONE,
 };