[Openvpn-devel,11/25] dco: split option parsing routines

Message ID 20220624083809.23487-12-a@unstable.cc
State Changes Requested
Headers show
Series ovpn-dco: introduce data-channel offload support | expand

Commit Message

Antonio Quartulli June 24, 2022, 8:37 a.m. UTC
DCO will try to install keys upon generating them, however, this happens
when parsing pushed cipher options (due to NCP).

For this reason we need to postpone parsing pushed cipher options to *after*
the tunnel interface has been opened, otherwise we would have no DCO netdev
object to operate on.

At the same time we split the parsing code, so that we can ensure that
the NEW_PEER call can happen after the received peer-id has been parsed
(it is required by all DCO API calls).

Signed-off-by: Antonio Quartulli <a@unstable.cc>
---
 src/openvpn/init.c  | 59 ++++++++++++++++++++++++++++-----------------
 src/openvpn/init.h  |  2 ++
 src/openvpn/multi.c |  7 ++++++
 3 files changed, 46 insertions(+), 22 deletions(-)

Comments

Arne Schwabe June 28, 2022, 2:29 p.m. UTC | #1
Am 24.06.22 um 10:37 schrieb Antonio Quartulli:
> +        if (!finish_options(c))
> +        {
> +            msg(D_TLS_ERRORS, "ERROR: Failed to finish option processing");
> +            return false;
> +        }

This error is a bit too generic for my taste. Can we make it more 
specific? Like "Failed to apply options when finalising tun setup" or 
something?

Arne

Patch

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 06911cd0..b0a4b252 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2068,14 +2068,6 @@  do_up(struct context *c, bool pulled_options, unsigned int option_types_found)
                 return false;
             }
         }
-        else if (c->mode == MODE_POINT_TO_POINT)
-        {
-            if (!do_deferred_p2p_ncp(c))
-            {
-                msg(D_TLS_ERRORS, "ERROR: Failed to apply P2P negotiated protocol options");
-                return false;
-            }
-        }
 
         /* if --up-delay specified, open tun, do ifconfig, and run up script now */
         if (c->options.up_delay || PULL_DEFINED(&c->options))
@@ -2102,6 +2094,22 @@  do_up(struct context *c, bool pulled_options, unsigned int option_types_found)
             }
         }
 
+        if (!pulled_options && c->mode == MODE_POINT_TO_POINT)
+        {
+            if (!do_deferred_p2p_ncp(c))
+            {
+                msg(D_TLS_ERRORS, "ERROR: Failed to apply P2P negotiated protocol options");
+                return false;
+            }
+        }
+
+        if (!finish_options(c))
+        {
+            msg(D_TLS_ERRORS, "ERROR: Failed to finish option processing");
+            return false;
+        }
+
+
         if (c->c2.did_open_tun)
         {
             c->c1.pulled_options_digest_save = c->c2.pulled_options_digest;
@@ -2307,23 +2315,30 @@  do_deferred_options(struct context *c, const unsigned int found)
         {
             return false;
         }
-        struct frame *frame_fragment = NULL;
+    }
+
+    return true;
+}
+
+bool
+finish_options(struct context *c)
+{
+    struct frame *frame_fragment = NULL;
 #ifdef ENABLE_FRAGMENT
-        if (c->options.ce.fragment)
-        {
-            frame_fragment = &c->c2.frame_fragment;
-        }
+    if (c->options.ce.fragment)
+    {
+        frame_fragment = &c->c2.frame_fragment;
+    }
 #endif
 
-        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)))
-        {
-            msg(D_TLS_ERRORS, "OPTIONS ERROR: failed to import crypto options");
-            return false;
-        }
+    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)))
+    {
+        msg(D_TLS_ERRORS, "OPTIONS ERROR: failed to import crypto options");
+        return false;
     }
 
     return true;
diff --git a/src/openvpn/init.h b/src/openvpn/init.h
index 2b8c2dcc..98e71d3a 100644
--- a/src/openvpn/init.h
+++ b/src/openvpn/init.h
@@ -97,6 +97,8 @@  void reset_coarse_timers(struct context *c);
 
 bool do_deferred_options(struct context *c, const unsigned int found);
 
+bool finish_options(struct context *c);
+
 void inherit_context_child(struct context *dest,
                            const struct context *src);
 
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index c72575ae..34ab90b4 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2405,6 +2405,13 @@  multi_client_connect_late_setup(struct multi_context *m,
     {
         mi->context.c2.tls_multi->multi_state = CAS_FAILED;
     }
+    /* Continue processing options only if authentication hasn't failed.
+     * Otherwise it does not make sense and we may operate on a non-configured
+     * client instance */
+    else
+    {
+        finish_options(&mi->context);
+    }
 
     /* send push reply if ready */
     if (mi->context.c2.push_request_received)