Ensures all params are ready before invoking dco_set_peer()

In UDP case, on a p2mp server, dco_set_peer() is currently called
at the wrong time since the mssfix param is calculated later on in
tls_session_update_crypto_params_do_work().

Move the dco_set_peer() inside tls_session_update_crypto_params_do_work(),
and remove p2p_set_dco_keepalive() to avoid calling dco_set_peer()
twice on the client side.

This way, we'll ensure that all crypto and frame params are properly
initialized and if an update occurs DCO will be notified.

Change-Id: Ic8538e734dba53cd43fead3961e4401c8037e079
Signed-off-by: Gianmarco De Gregori <gianmarco@mandelbit.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20240906145745.67596-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29086.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 4f63346800eb0ada11dbe06749dc8a640a7cddb5..dd56961c1a38b02caec174ca15af69b538cb4e7d 100644 (file)
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2178,27 +2178,6 @@ options_hash_changed_or_zero(const struct sha256_digest *a,
            || !memcmp(a, &zero, sizeof(struct sha256_digest));
 }
 
-static bool
-p2p_set_dco_keepalive(struct context *c)
-{
-    if (dco_enabled(&c->options)
-        && (c->options.ping_send_timeout || c->c2.frame.mss_fix))
-    {
-        int ret = dco_set_peer(&c->c1.tuntap->dco,
-                               c->c2.tls_multi->dco_peer_id,
-                               c->options.ping_send_timeout,
-                               c->options.ping_rec_timeout,
-                               c->c2.frame.mss_fix);
-        if (ret < 0)
-        {
-            msg(D_DCO, "Cannot set parameters for DCO peer (id=%u): %s",
-                c->c2.tls_multi->dco_peer_id, strerror(-ret));
-            return false;
-        }
-    }
-    return true;
-}
-
 /**
  * Helper function for tls_print_deferred_options_results
  * Adds the ", " delimitor if there already some data in the
@@ -2363,7 +2342,8 @@ do_deferred_options_part2(struct context *c)
     if (!tls_session_update_crypto_params(c->c2.tls_multi, session,
                                           &c->options, &c->c2.frame,
                                           frame_fragment,
-                                          get_link_socket_info(c)))
+                                          get_link_socket_info(c),
+                                          &c->c1.tuntap->dco))
     {
         msg(D_TLS_ERRORS, "OPTIONS ERROR: failed to import crypto options");
         return false;
@@ -2472,12 +2452,6 @@ do_up(struct context *c, bool pulled_options, unsigned int option_types_found)
             }
         }
 
-        if (c->mode == MODE_POINT_TO_POINT && !p2p_set_dco_keepalive(c))
-        {
-            msg(D_TLS_ERRORS, "ERROR: Failed to apply DCO keepalive or MSS fix parameters");
-            return false;
-        }
-
         if (c->c2.did_open_tun)
         {
             c->c1.pulled_options_digest_save = c->c2.pulled_options_digest;
@@ -2582,7 +2556,8 @@ do_deferred_p2p_ncp(struct context *c)
 
     if (!tls_session_update_crypto_params(c->c2.tls_multi, session, &c->options,
                                           &c->c2.frame, frame_fragment,
-                                          get_link_socket_info(c)))
+                                          get_link_socket_info(c),
+                                          &c->c1.tuntap->dco))
     {
         msg(D_TLS_ERRORS, "ERROR: failed to set crypto cipher");
         return false;

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 03177bb8bb1b9a68870412fa0a64638e67cba8f8..0509911d63d6ab4350a6103b359b68a1d1eadcf2 100644 (file)
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2364,21 +2364,6 @@ multi_client_setup_dco_initial(struct multi_context *m,
         return false;
     }
 
-    if (mi->context.options.ping_send_timeout || mi->context.c2.frame.mss_fix)
-    {
-        ret = dco_set_peer(&mi->context.c1.tuntap->dco,
-                           mi->context.c2.tls_multi->dco_peer_id,
-                           mi->context.options.ping_send_timeout,
-                           mi->context.options.ping_rec_timeout,
-                           mi->context.c2.frame.mss_fix);
-        if (ret < 0)
-        {
-            msg(D_DCO, "Cannot set DCO peer parameters for %s (id=%u): %s",
-                multi_instance_string(mi, false, gc),
-                mi->context.c2.tls_multi->dco_peer_id, strerror(-ret));
-            return false;
-        }
-    }
     return true;
 }
 
@@ -2398,7 +2383,8 @@ multi_client_generate_tls_keys(struct context *c)
     struct tls_session *session = &c->c2.tls_multi->session[TM_ACTIVE];
     if (!tls_session_update_crypto_params(c->c2.tls_multi, session, &c->options,
                                           &c->c2.frame, frame_fragment,
-                                          get_link_socket_info(c)))
+                                          get_link_socket_info(c),
+                                          &c->c1.tuntap->dco))
     {
         msg(D_TLS_ERRORS, "TLS Error: initializing data channel failed");
         register_signal(c->sig, SIGUSR1, "process-push-msg-failed");

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 8c72d0b328c213f3efd1621eb2a451cd7c19b34b..e2be6140022539bbc318a09cbc4aac9c3a065af6 100644 (file)
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1591,7 +1591,8 @@ tls_session_update_crypto_params_do_work(struct tls_multi *multi,
                                          struct options *options,
                                          struct frame *frame,
                                          struct frame *frame_fragment,
-                                         struct link_socket_info *lsi)
+                                         struct link_socket_info *lsi,
+                                         dco_context_t *dco)
 {
     if (session->key[KS_PRIMARY].crypto_options.key_ctx_bi.initialized)
     {
@@ -1638,6 +1639,25 @@ tls_session_update_crypto_params_do_work(struct tls_multi *multi,
             return false;
         }
     }
+
+    if (dco_enabled(options))
+    {
+        /* dco_set_peer() must be called if either keepalive or
+         * mssfix are set to update in-kernel config */
+        if (options->ping_send_timeout || frame->mss_fix)
+        {
+            int ret = dco_set_peer(dco, multi->dco_peer_id,
+                                   options->ping_send_timeout,
+                                   options->ping_rec_timeout,
+                                   frame->mss_fix);
+            if (ret < 0)
+            {
+                msg(D_DCO, "Cannot set DCO peer parameters for peer (id=%u): %s",
+                    multi->dco_peer_id, strerror(-ret));
+                return false;
+            }
+        }
+    }
     return tls_session_generate_data_channel_keys(multi, session);
 }
 
@@ -1646,7 +1666,8 @@ tls_session_update_crypto_params(struct tls_multi *multi,
                                  struct tls_session *session,
                                  struct options *options, struct frame *frame,
                                  struct frame *frame_fragment,
-                                 struct link_socket_info *lsi)
+                                 struct link_socket_info *lsi,
+                                 dco_context_t *dco)
 {
     if (!check_session_cipher(session, options))
     {
@@ -1657,7 +1678,7 @@ tls_session_update_crypto_params(struct tls_multi *multi,
     session->opt->crypto_flags |= options->imported_protocol_flags;
 
     return tls_session_update_crypto_params_do_work(multi, session, options,
-                                                    frame, frame_fragment, lsi);
+                                                    frame, frame_fragment, lsi, dco);
 }
 
 

diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index 6c2bfc322cdda17aa1fe8613cbdd902002f9a15a..eea13234b530551f5fc614600cac2bf53d40ae34 100644 (file)
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -460,6 +460,8 @@ void tls_update_remote_addr(struct tls_multi *multi,
  * @param frame_fragment  The fragment frame options.
  * @param lsi             link socket info to adjust MTU related options
  *                        depending on the current protocol
+ * @param dco             The dco context to perform dco_set_peer()
+ *                        whenever a crypto param update occurs.
  *
  * @return true if updating succeeded or keys are already generated, false otherwise.
  */
@@ -468,7 +470,8 @@ bool tls_session_update_crypto_params(struct tls_multi *multi,
                                       struct options *options,
                                       struct frame *frame,
                                       struct frame *frame_fragment,
-                                      struct link_socket_info *lsi);
+                                      struct link_socket_info *lsi,
+                                      dco_context_t *dco);
 
 /*
  * inline functions