Rename TM_UNTRUSTED to TM_INITIAL, always start session in TM_INITIAL rather than...

Currently we start new session in TM_ACTIVE or TM_INITIAL depending if
we already have an active session in TM_ACTIVE or not.

With this change, all session will be started in TM_INITIAL both initiated
by a peer but also session by ourselves. This simplifies state transitions
and eliminates the wacky state transition that when we have a failed
reneogitiation (and move TM_ACTIVE to TM_LAME_DUCK) that a new session of
a peer starts in TM_ACTIVE rather than TM_INITIAL

This is a squash of two mailing list patches:

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221224194253.3202231-2-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25798.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221224194253.3202231-3-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25795.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index 45815233562aa182c9f38b8ba6e046b447f7c8c8..c27c6da5bfdda94ef6c3853a567f32bbd00c8c33 100644 (file)
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -257,7 +257,7 @@ multi_get_create_instance_udp(struct multi_context *m, bool *floated)
                             && session_id_defined((&state.peer_session_id)))
                         {
                             mi->context.c2.tls_multi->n_sessions++;
-                            struct tls_session *session = &mi->context.c2.tls_multi->session[TM_ACTIVE];
+                            struct tls_session *session = &mi->context.c2.tls_multi->session[TM_INITIAL];
                             session_skip_to_pre_start(session, &state, &m->top.c2.from);
                         }
                     }

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 9e5480528a4138da020a486fd9ae8c8882d4971b..b1dc80c40d9fd27203aa9854240dc438a620a278 100644 (file)
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -890,8 +890,8 @@ session_index_name(int index)
         case TM_ACTIVE:
             return "TM_ACTIVE";
 
-        case TM_UNTRUSTED:
-            return "TM_UNTRUSTED";
+        case TM_INITIAL:
+            return "TM_INITIAL";
 
         case TM_LAME_DUCK:
             return "TM_LAME_DUCK";
@@ -1327,11 +1327,7 @@ tls_multi_init_finalize(struct tls_multi *multi, int tls_mtu)
     /* initialize the active and untrusted sessions */
 
     tls_session_init(multi, &multi->session[TM_ACTIVE]);
-
-    if (!multi->opt.single_session)
-    {
-        tls_session_init(multi, &multi->session[TM_UNTRUSTED]);
-    }
+    tls_session_init(multi, &multi->session[TM_INITIAL]);
 }
 
 /*
@@ -3173,8 +3169,11 @@ tls_multi_process(struct tls_multi *multi,
         struct key_state *ks = &session->key[KS_PRIMARY];
         struct key_state *ks_lame = &session->key[KS_LAME_DUCK];
 
-        /* set initial remote address */
-        if (i == TM_ACTIVE && ks->state == S_INITIAL
+        /* set initial remote address. This triggers connecting with that
+         * session. So we only do that if the TM_ACTIVE session is not
+         * established */
+        if (i == TM_INITIAL && ks->state == S_INITIAL
+            && get_primary_key(multi)->state <= S_INITIAL
             && link_socket_actual_defined(&to_link_socket_info->lsa->actual))
         {
             ks->remote_addr = to_link_socket_info->lsa->actual;
@@ -3221,13 +3220,14 @@ tls_multi_process(struct tls_multi *multi,
             {
                 ++multi->n_soft_errors;
 
-                if (i == TM_ACTIVE)
+                if (i == TM_ACTIVE
+                    || (i == TM_INITIAL && get_primary_key(multi)->state < S_ACTIVE))
                 {
                     error = true;
                 }
 
                 if (i == TM_ACTIVE
-                    && ks_lame->state >= S_ACTIVE
+                    && ks_lame->state >= S_GENERATED_KEYS
                     && !multi->opt.single_session)
                 {
                     move_session(multi, TM_LAME_DUCK, TM_ACTIVE, true);
@@ -3250,7 +3250,7 @@ tls_multi_process(struct tls_multi *multi,
     if (multi->multi_state >= CAS_CONNECT_DONE)
     {
         /* Only generate keys for the TM_ACTIVE session. We defer generating
-         * keys for TM_UNTRUSTED until we actually trust it.
+         * keys for TM_INITIAL 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];
@@ -3299,10 +3299,12 @@ tls_multi_process(struct tls_multi *multi,
      * verification failed.  A semi-trusted session can forward data on the
      * TLS control channel but not on the tunnel channel.
      */
-    if (TLS_AUTHENTICATED(multi, &multi->session[TM_UNTRUSTED].key[KS_PRIMARY]))
+    if (TLS_AUTHENTICATED(multi, &multi->session[TM_INITIAL].key[KS_PRIMARY]))
     {
-        move_session(multi, TM_ACTIVE, TM_UNTRUSTED, true);
-        msg(D_TLS_DEBUG_LOW, "TLS: tls_multi_process: untrusted session promoted to %strusted",
+        move_session(multi, TM_ACTIVE, TM_INITIAL, true);
+        tas = tls_authentication_status(multi);
+        msg(D_TLS_DEBUG_LOW, "TLS: tls_multi_process: initial untrusted "
+            "session promoted to %strusted",
             tas == TLS_AUTHENTICATION_SUCCEEDED ? "" : "semi-");
 
         if (multi->multi_state == CAS_CONNECT_DONE)
@@ -3633,55 +3635,8 @@ tls_pre_decrypt(struct tls_multi *multi,
 
     /*
      * Hard reset and session id does not match any session in
-     * multi->session: Possible initial packet
-     */
-    if (i == TM_SIZE && is_hard_reset_method2(op))
-    {
-        struct tls_session *session = &multi->session[TM_ACTIVE];
-        const struct key_state *ks = get_primary_key(multi);
-
-        /*
-         * If we have no session currently in progress, the initial packet will
-         * open a new session in TM_ACTIVE rather than TM_UNTRUSTED.
-         */
-        if (!session_id_defined(&ks->session_id_remote))
-        {
-            if (multi->opt.single_session && multi->n_sessions)
-            {
-                msg(D_TLS_ERRORS,
-                    "TLS Error: Cannot accept new session request from %s due to session context expire or --single-session [1]",
-                    print_link_socket_actual(from, &gc));
-                goto error;
-            }
-
-#ifdef ENABLE_MANAGEMENT
-            if (management)
-            {
-                management_set_state(management,
-                                     OPENVPN_STATE_AUTH,
-                                     NULL,
-                                     NULL,
-                                     NULL,
-                                     NULL,
-                                     NULL);
-            }
-#endif
-
-            msg(D_TLS_DEBUG_LOW,
-                "TLS: Initial packet from %s, sid=%s",
-                print_link_socket_actual(from, &gc),
-                session_id_print(&sid, &gc));
-
-            do_burst = true;
-            new_link = true;
-            i = TM_ACTIVE;
-            session->untrusted_addr = *from;
-        }
-    }
-
-    /*
-     * If we detected new session in the last if block, variable i has
-     * changed to TM_ACTIVE, so check the condition again.
+     * multi->session: Possible initial packet. New sessions always start
+     * as TM_INITIAL
      */
     if (i == TM_SIZE && is_hard_reset_method2(op))
     {
@@ -3689,16 +3644,17 @@ tls_pre_decrypt(struct tls_multi *multi,
          * No match with existing sessions,
          * probably a new session.
          */
-        struct tls_session *session = &multi->session[TM_UNTRUSTED];
+        struct tls_session *session = &multi->session[TM_INITIAL];
 
         /*
          * If --single-session, don't allow any hard-reset connection request
          * unless it the first packet of the session.
          */
-        if (multi->opt.single_session)
+        if (multi->opt.single_session && multi->n_sessions)
         {
             msg(D_TLS_ERRORS,
-                "TLS Error: Cannot accept new session request from %s due to session context expire or --single-session [2]",
+                "TLS Error: Cannot accept new session request from %s due "
+                "to session context expire or --single-session",
                 print_link_socket_actual(from, &gc));
             goto error;
         }
@@ -3709,6 +3665,19 @@ tls_pre_decrypt(struct tls_multi *multi,
             goto error;
         }
 
+#ifdef ENABLE_MANAGEMENT
+        if (management)
+        {
+            management_set_state(management,
+                                 OPENVPN_STATE_AUTH,
+                                 NULL,
+                                 NULL,
+                                 NULL,
+                                 NULL,
+                                 NULL);
+        }
+#endif
+
         /*
          * New session-initiating control packet is authenticated at this point,
          * assuming that the --tls-auth command line option was used.
@@ -3716,11 +3685,13 @@ tls_pre_decrypt(struct tls_multi *multi,
          * Without --tls-auth, we leave authentication entirely up to TLS.
          */
         msg(D_TLS_DEBUG_LOW,
-            "TLS: new session incoming connection from %s",
-            print_link_socket_actual(from, &gc));
+            "TLS: Initial packet from %s, sid=%s",
+            print_link_socket_actual(from, &gc),
+            session_id_print(&sid, &gc));
 
+        do_burst = true;
         new_link = true;
-        i = TM_UNTRUSTED;
+        i = TM_INITIAL;
         session->untrusted_addr = *from;
     }
     else
@@ -3731,7 +3702,7 @@ tls_pre_decrypt(struct tls_multi *multi,
         /*
          * Packet must belong to an existing session.
          */
-        if (i != TM_ACTIVE && i != TM_UNTRUSTED)
+        if (i != TM_ACTIVE && i != TM_INITIAL)
         {
             msg(D_TLS_ERRORS,
                 "TLS Error: Unroutable control packet received from %s (si=%d op=%s)",

diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index 55c672d449a977d78adafa33d0272dd6476cea56..bd27e57a0e44958c3cf69674c536144918ef6d4c 100644 (file)
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -159,7 +159,7 @@ struct tls_multi *tls_multi_init(struct tls_options *tls_options);
  * @ingroup control_processor
  *
  * This function initializes the \c TM_ACTIVE \c tls_session, and in
- * server mode also the \c TM_UNTRUSTED \c tls_session, associated with
+ * server mode also the \c TM_INITIAL \c tls_session, associated with
  * this \c tls_multi structure.  It also configures the control channel's
  * \c frame structure based on the data channel's \c frame given in
  * argument \a frame.

diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index 978a9fca0ecbdc9289e78ec72cce3d60fc08f67c..7d9c2460b9ee6c45c9c9a2fc6b292103e8c494c5 100644 (file)
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -512,7 +512,7 @@ struct tls_session
  *
  *  @{ */
 #define TM_ACTIVE    0          /**< Active \c tls_session. */
-#define TM_UNTRUSTED 1          /**< As yet un-trusted \c tls_session
+#define TM_INITIAL   1          /**< As yet un-trusted \c tls_session
                                  *   being negotiated. */
 #define TM_LAME_DUCK 2          /**< Old \c tls_session. */
 #define TM_SIZE      3          /**< Size of the \c tls_multi.session