diff mbox series

[Openvpn-devel] dco: perform pull options check only if we pulled any option

Message ID 20220805150837.8169-1-a@unstable.cc
State Accepted
Headers
Series [Openvpn-devel] dco: perform pull options check only if we pulled any option |

Commit Message

Antonio Quartulli Aug. 5, 2022, 5:08 a.m. UTC 
  The do_deferred_options() function is invoked also on the server side in
order to process all negotiated bits.

However, in this case we should not perform any pull options check, as
it's required only on the client side.

Move check within the "if (options.pull)" block to ensure we perform the
check only when required.

Reported-By: Gert Doering <gert@greenie.muc.de>
Signed-off-by: Antonio Quartulli <a@unstable.cc>
---
 src/openvpn/init.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

Comments

Gert Doering Aug. 6, 2022, 12:33 a.m. UTC | #1 
Acked-by: Gert Doering <gert@greenie.muc.de>

We really want to check "pushed options", so we should do this when
we actually have --pull in our config :-)

Stared at code (ok), and tested on DCO build with DCO kernel, with the
three scenarios

  - client, normal operation -> works
  - client, with "--pull-filter ignore peer-id" (-> triggers message)
  - server, connecting clients -> annoying message is gone now

Your patch has been applied to the master branch.

commit f9ef554a5bda5c354e59261f9dbf6519e2815388
Author: Antonio Quartulli
Date:   Fri Aug 5 17:08:37 2022 +0200

     dco: perform pull options check only if we pulled any option

     Signed-off-by: Antonio Quartulli <a@unstable.cc>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20220805150837.8169-1-a@unstable.cc>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24824.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 2e7544de..b6705921 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2402,22 +2402,23 @@  do_deferred_options(struct context *c, const unsigned int found)
         c->c2.tls_multi->peer_id = c->options.peer_id;
     }
 
-    /* process (potentially pushed) crypto options */
+    /* process (potentially) pushed options */
     if (c->options.pull)
     {
         if (!check_pull_client_ncp(c, found))
         {
             return false;
         }
-    }
 
-    /* Check if pushed options are compatible with DCO, if enabled */
-    if (dco_enabled(&c->options)
-        && !dco_check_pull_options(D_PUSH_ERRORS, &c->options))
-    {
-        msg(D_PUSH_ERRORS, "OPTIONS ERROR: pushed options are incompatible with "
-            "data channel offload. Use --disable-dco to connect to this server");
-        return false;
+        /* Check if pushed options are compatible with DCO, if enabled */
+        if (dco_enabled(&c->options)
+            && !dco_check_pull_options(D_PUSH_ERRORS, &c->options))
+        {
+            msg(D_PUSH_ERRORS, "OPTIONS ERROR: pushed options are incompatible "
+                "with data channel offload. Use --disable-dco to connect to "
+                "this server");
+            return false;
+        }
     }
 
     return true;