diff mbox series

[Openvpn-devel,13/28] Change FULL_SYNC macro to no_pending_reliable_packets function

Message ID 20220422142953.3805364-4-arne@rfc2549.org
State Superseded
Headers show
Series Stateless three-way handshake and control channel improvements | expand

Commit Message

Arne Schwabe April 22, 2022, 4:29 a.m. UTC 
This changes this macro to a better named inline function. This
introduces a slight whitespace problem but the next refactoring will
move the incorrectly intended block to its own function anyway.
---
 src/openvpn/ssl.c | 100 ++++++++++++++++++++++++++--------------------
 1 file changed, 57 insertions(+), 43 deletions(-)

Comments

Frank Lichtenheld April 25, 2022, 11:42 p.m. UTC | #1 
> Arne Schwabe <arne@rfc2549.org> hat am 22.04.2022 16:29 geschrieben:
> 
>  
> This changes this macro to a better named inline function. This
> introduces a slight whitespace problem but the next refactoring will
> move the incorrectly intended block to its own function anyway.

Could it be that you forgot to update the commit message when you squashed
this commit? Looking at the diff this already moves the block to its
own function?

Apart from that and a small whitespace oddity noted below the changes
look good (in the sense that they shouldn't change behavior and just
move code around).

> ---
>  src/openvpn/ssl.c | 100 ++++++++++++++++++++++++++--------------------
>  1 file changed, 57 insertions(+), 43 deletions(-)
> 
> diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
> index bad59f2a1..4ca093243 100644
> --- a/src/openvpn/ssl.c
> +++ b/src/openvpn/ssl.c
[...]
> @@ -2428,6 +2430,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. 

Spurious whitespace at the end of the line

Regards,
--
Frank Lichtenheld
diff mbox series

Patch

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index bad59f2a1..4ca093243 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1774,8 +1774,10 @@  flush_payload_buffer(struct key_state *ks)
 }
 
 /* true if no in/out acknowledgements pending */
-#define FULL_SYNC \
-    (reliable_empty(ks->send_reliable) && reliable_ack_empty(ks->rec_ack))
+static bool no_pending_reliable_packets(struct key_state *ks)
+{
+    return (reliable_empty(ks->send_reliable) && reliable_ack_empty(ks->rec_ack));
+}
 
 /*
  * Move the active key to the lame duck key and reinitialize the
@@ -2428,6 +2430,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
@@ -2518,7 +2568,7 @@  tls_process(struct tls_multi *multi,
         }
 
         /* Wait for Initial Handshake ACK */
-        if (ks->state == S_PRE_START && FULL_SYNC)
+        if (ks->state == S_PRE_START && no_pending_reliable_packets(ks))
         {
             ks->state = S_START;
             state_change = true;
@@ -2542,47 +2592,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 (FULL_SYNC)
-            {
-                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