[Openvpn-devel,v3] Extract handle_connection_attempt from multi_get_create_instance_udp

Message ID 20260628130500.26086-1-gert@greenie.muc.de
State New
Headers
Series [Openvpn-devel,v3] Extract handle_connection_attempt from multi_get_create_instance_udp |

Commit Message

Gert Doering June 28, 2026, 1:04 p.m. UTC
  From: Arne Schwabe <arne@rfc2549.org>

The multi_get_create_instance_udp is quite large. This factors out the
one branch that handles and creates new connection attempts.

Change-Id: I6a032465e66b49ab0ce8b1a84ead8d9acef918de
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1720
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1720
This mail reflects revision 3 of this Change.

Acked-by according to Gerrit (reflected above):
Frank Lichtenheld <frank@lichtenheld.com>
  

Comments

Gert Doering June 28, 2026, 2:56 p.m. UTC | #1
Basically, this is "just split out a large chunk of code to a separate
function" (trivial enough) but "git show" is not smart enough here to
see that ("--color-moved=zebra -w" *should* but isn't?!) - anyway, stared
at the code for a bit, and ran t_server tests.

As this is "refactoring" and not acutely needed for any of the pending
bug fixes, I've only applied it to master.

Your patch has been applied to the master branch.

commit 1572ee902f1d1ad75c1ad621ddcdc8bc80a5d522
Author: Arne Schwabe
Date:   Sun Jun 28 15:04:55 2026 +0200

     Extract handle_connection_attempt from multi_get_create_instance_udp

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1720
     Message-Id: <20260628130500.26086-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg37336.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
  

Patch

diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index b1de446..7d4fee1 100644
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -194,12 +194,81 @@ 
     return false;
 }
 
-/*
+/**
+ * Handles a packet if no existing session exists for this incoming packet.
+ *
+ * It will either send a reply with a hmac cookie if this is the first
+ * packet of a three-way handshake or create a multi_instance if it is a
+ * packet that completes the three-way handshake.
+ */
+static struct multi_instance *
+handle_connection_attempt(struct multi_context *m,
+                          struct link_socket *sock,
+                          struct mroute_addr *real,
+                          const uint64_t hv,
+                          struct hash_bucket *bucket)
+{
+    struct hash *hash = m->hash;
+    struct tls_pre_decrypt_state state = { 0 };
+    struct multi_instance *mi = NULL;
+    struct gc_arena gc = gc_new();
+
+    if (m->deferred_shutdown_signal.signal_received)
+    {
+        msg(D_MULTI_ERRORS,
+            "MULTI: Connection attempt from %s ignored while server is "
+            "shutting down",
+            mroute_addr_print(real, &gc));
+    }
+    else if (do_pre_decrypt_check(m, &state, *real, sock))
+    {
+        /* This is an unknown session but with valid tls-auth/tls-crypt
+         * (or no auth at all).  If this is the initial packet of a
+         * session, we just send a reply with a HMAC session id and
+         * do not generate a session slot */
+
+        if (frequency_limit_event_allowed(m->new_connection_limiter))
+        {
+            /* a successful three-way handshake only counts against
+             * connect-freq but not against connect-freq-initial */
+            reflect_filter_rate_limit_decrease(m->initial_rate_limiter);
+
+            mi = multi_create_instance(m, real, sock);
+            if (mi)
+            {
+                hash_add_fast(hash, bucket, &mi->real, hv, mi);
+                mi->did_real_hash = true;
+                multi_assign_peer_id(m, mi);
+
+                /* If we have a session id already, ensure that the
+                 * state is using the same */
+                if (session_id_defined(&state.server_session_id)
+                    && 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_INITIAL];
+                    session_skip_to_pre_start(session, &state, &m->top.c2.from);
+                }
+            }
+        }
+        else
+        {
+            msg(D_MULTI_ERRORS,
+                "MULTI: Connection from %s would exceed new connection frequency limit as controlled by --connect-freq",
+                mroute_addr_print(real, &gc));
+        }
+    }
+    free_tls_pre_decrypt_state(&state);
+    gc_free(&gc);
+    return mi;
+}
+
+/**
  * Get a client instance based on real address.  If
  * the instance doesn't exist, create it while
  * maintaining real address hash table atomicity.
  */
-
 struct multi_instance *
 multi_get_create_instance_udp(struct multi_context *m, bool *floated, struct link_socket *sock)
 {
@@ -258,54 +327,7 @@ 
         /* we have no existing multi instance for this connection */
         if (!mi)
         {
-            struct tls_pre_decrypt_state state = { 0 };
-            if (m->deferred_shutdown_signal.signal_received)
-            {
-                msg(D_MULTI_ERRORS,
-                    "MULTI: Connection attempt from %s ignored while server is "
-                    "shutting down",
-                    mroute_addr_print(&real, &gc));
-            }
-            else if (do_pre_decrypt_check(m, &state, real, sock))
-            {
-                /* This is an unknown session but with valid tls-auth/tls-crypt
-                 * (or no auth at all).  If this is the initial packet of a
-                 * session, we just send a reply with a HMAC session id and
-                 * do not generate a session slot */
-
-                if (frequency_limit_event_allowed(m->new_connection_limiter))
-                {
-                    /* a successful three-way handshake only counts against
-                     * connect-freq but not against connect-freq-initial */
-                    reflect_filter_rate_limit_decrease(m->initial_rate_limiter);
-
-                    mi = multi_create_instance(m, &real, sock);
-                    if (mi)
-                    {
-                        hash_add_fast(hash, bucket, &mi->real, hv, mi);
-                        mi->did_real_hash = true;
-                        multi_assign_peer_id(m, mi);
-
-                        /* If we have a session id already, ensure that the
-                         * state is using the same */
-                        if (session_id_defined(&state.server_session_id)
-                            && 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_INITIAL];
-                            session_skip_to_pre_start(session, &state, &m->top.c2.from);
-                        }
-                    }
-                }
-                else
-                {
-                    msg(D_MULTI_ERRORS,
-                        "MULTI: Connection from %s would exceed new connection frequency limit as controlled by --connect-freq",
-                        mroute_addr_print(&real, &gc));
-                }
-            }
-            free_tls_pre_decrypt_state(&state);
+            mi = handle_connection_attempt(m, sock, &real, hv, bucket);
         }
 
 #ifdef ENABLE_DEBUG