diff mbox series

[Openvpn-devel,v2] Extract session_move_active into its own function

Message ID 20220426132324.76517-2-arne@rfc2549.org
State Accepted
Headers show
Series [Openvpn-devel,v2] Extract session_move_active into its own function | expand

Commit Message

Arne Schwabe April 26, 2022, 3:23 a.m. UTC 
This makes the tls_process function smaller and easier to understand and
this state easier to understand in its own function.
---
 src/openvpn/ssl.c | 92 ++++++++++++++++++++++++++---------------------
 1 file changed, 52 insertions(+), 40 deletions(-)

Comments

Gert Doering April 26, 2022, 3:52 a.m. UTC | #1 
Acked-by: Gert Doering <gert@greenie.muc.de>

This is the 2nd half of "patch 13 v2".  It has a semi-ACK from Frank,
and now a proper commit message as well :-)

Verified "it's only moved code" by hand (indent change), and lightly
tested.

Whitespace error has been fixed by git, so the "whitespace fix" hunk coming
up won't apply - but I can handle that :-)

Your patch has been applied to the master branch.

commit 4dbe508ab6d011d57f4acc71f3c0330deb852206
Author: Arne Schwabe
Date:   Tue Apr 26 15:23:24 2022 +0200

     Extract session_move_active into its own function

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


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 6c6648afa..c90113044 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -2431,6 +2431,54 @@  session_move_pre_start(const struct tls_session *session,
     return true;
 
 }
+
+/**
+ * Moves the key to state to S_ACTIVE and also advances the multi_state state
+ * machine if this is the initial connection. 
+ */
+static void
+session_move_active(struct tls_multi *multi, struct tls_session *session,
+                    struct link_socket_info *to_link_socket_info,
+                    struct key_state *ks)
+{
+    dmsg(D_TLS_DEBUG_MED, "STATE S_ACTIVE");
+
+    ks->established = now;
+    if (check_debug_level(D_HANDSHAKE))
+    {
+        print_details(&ks->ks_ssl, "Control Channel:");
+    }
+    ks->state = S_ACTIVE;
+    /* Cancel negotiation timeout */
+    ks->must_negotiate = 0;
+    INCR_SUCCESS;
+
+    /* Set outgoing address for data channel packets */
+    link_socket_set_outgoing_addr(to_link_socket_info, &ks->remote_addr,
+                                  session->common_name, session->opt->es);
+
+    /* Check if we need to advance the tls_multi state machine */
+    if (multi->multi_state == CAS_NOT_CONNECTED)
+    {
+        if (session->opt->mode == MODE_SERVER)
+        {
+            /* On a server we continue with running connect scripts next */
+            multi->multi_state = CAS_WAITING_AUTH;
+        }
+        else
+        {
+            /* Skip the connect script related states */
+            multi->multi_state = CAS_WAITING_OPTIONS_IMPORT;
+        }
+    }
+
+    /* Flush any payload packets that were buffered before our state transitioned to S_ACTIVE */
+    flush_payload_buffer(ks);
+
+#ifdef MEASURE_TLS_HANDSHAKE_STATS
+    show_tls_performance_stats();
+#endif
+}
 /*
  * This is the primary routine for processing TLS stuff inside the
  * the main event loop.  When this routine exits
@@ -2545,47 +2593,11 @@  tls_process(struct tls_multi *multi,
 
         /* Wait for ACK */
         if (((ks->state == S_GOT_KEY && !session->opt->server)
-             || (ks->state == S_SENT_KEY && session->opt->server)))
+             || (ks->state == S_SENT_KEY && session->opt->server))
+             && no_pending_reliable_packets(ks))
         {
-            if (no_pending_reliable_packets(ks))
-            {
-                ks->established = now;
-                dmsg(D_TLS_DEBUG_MED, "STATE S_ACTIVE");
-                if (check_debug_level(D_HANDSHAKE))
-                {
-                    print_details(&ks->ks_ssl, "Control Channel:");
-                }
-                state_change = true;
-                ks->state = S_ACTIVE;
-                /* Cancel negotiation timeout */
-                ks->must_negotiate = 0;
-                INCR_SUCCESS;
-
-                /* Set outgoing address for data channel packets */
-                link_socket_set_outgoing_addr(to_link_socket_info, &ks->remote_addr, session->common_name, session->opt->es);
-
-                /* Check if we need to advance the tls_multi state machine */
-                if (multi->multi_state == CAS_NOT_CONNECTED)
-                {
-                    if (session->opt->mode == MODE_SERVER)
-                    {
-                        /* On a server we continue with running connect scripts next */
-                        multi->multi_state = CAS_WAITING_AUTH;
-                    }
-                    else
-                    {
-                        /* Skip the connect script related states */
-                        multi->multi_state = CAS_WAITING_OPTIONS_IMPORT;
-                    }
-                }
-
-                /* Flush any payload packets that were buffered before our state transitioned to S_ACTIVE */
-                flush_payload_buffer(ks);
-
-#ifdef MEASURE_TLS_HANDSHAKE_STATS
-                show_tls_performance_stats();
-#endif
-            }
+            session_move_active(multi, session, to_link_socket_info, ks);
+            state_change = true;
         }
 
         /* Reliable buffer to outgoing TCP/UDP (send up to CONTROL_SEND_ACK_MAX ACKs