diff mbox series

[Openvpn-devel,v104] dco-win: check for incompatible options

Message ID 20220819065250.222590-1-a@unstable.cc
State Accepted
Headers show
Series [Openvpn-devel,v104] dco-win: check for incompatible options | expand

Commit Message

Antonio Quartulli Aug. 18, 2022, 8:52 p.m. UTC 
At the moment dco-win doesn't support --persist-tun and --server,
so check for these options at startup time.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Signed-off-by: Lev Stipakov <lev@openvpn.net>
---

Changes from v103:
* fix ifdef condition (use || instead of &&) in options.c

Changes from v102:
* remove platform defined log level and make check_options_ calls on
  Windows explicit and document why.

Changes from v101:
* rebased
* remove call to dco_check_option_ from verify() and reuse invocation
  that was already implemented for linux/freebsd in mutate_ce()
* hide log level to use in case of option check failure inside
  dco_win/linux/freebsd.h

Changes from v100:
* improved commit title/message
---
 src/openvpn/dco.c     | 17 +++++++++++++++--
 src/openvpn/options.c |  8 +++++++-
 2 files changed, 22 insertions(+), 3 deletions(-)

Comments

Gert Doering Aug. 18, 2022, 11:07 p.m. UTC | #1 
Acked-by: Gert Doering <gert@greenie.muc.de>

"This one is better" :-)

Test built via GHA for MSVC, tested on Linux non-DCO / DCO client and 
DCO server.  This time, neither "non DCO" nor "DCO build" exhibited 
any surprises (as expected).

Not tested actual Windows build, but the checks match the commit
message (and the discussions on what works and what doesn't).

Your patch has been applied to the master branch.

commit 8c3b7c11d1212a6521e84a1d423abe75b974741e
Author: Antonio Quartulli
Date:   Fri Aug 19 08:52:50 2022 +0200

     dco-win: check for incompatible options

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


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index 08c6fcf7..4190747a 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -225,7 +225,20 @@  dco_update_keys(dco_context_t *dco, struct tls_multi *multi)
 bool
 dco_check_startup_option_conflict(int msglevel, const struct options *o)
 {
-#if defined(TARGET_LINUX)
+#if defined(_WIN32)
+    if (o->mode == MODE_SERVER)
+    {
+        msg(msglevel, "Only client and p2p data channel offload is supported "
+            "with ovpn-dco-win.");
+        return false;
+    }
+
+    if (o->persist_tun)
+    {
+        msg(msglevel, "--persist-tun is not supported with ovpn-dco-win.");
+        return false;
+    }
+#elif defined(TARGET_LINUX)
     /* if the device name is fixed, we need to check if an interface with this
      * name already exists. IF it does, it must be a DCO interface, otherwise
      * DCO has to be disabled in order to continue.
@@ -250,7 +263,7 @@  dco_check_startup_option_conflict(int msglevel, const struct options *o)
                 strerror(-ret), ret);
         }
     }
-#endif /* if defined(TARGET_LINUX) */
+#endif /* if defined(_WIN32) */
 
 #if defined(HAVE_LIBCAPNG)
     /* DCO can't operate without CAP_NET_ADMIN. To retain it when switching user
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 2415c1a8..2b0bb20c 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3669,10 +3669,16 @@  options_postprocess_mutate(struct options *o, struct env_set *es)
             "incompatible with each other.");
     }
 
-    /* check if any option should force disabling DCO */
 #if defined(TARGET_LINUX) || defined(TARGET_FREEBSD)
+    /* check if any option should force disabling DCO */
     o->tuntap_options.disable_dco = !dco_check_option_conflict(D_DCO, o)
                                     || !dco_check_startup_option_conflict(D_DCO, o);
+#elif defined(_WIN32)
+    /* in Windows we have no 'fallback to non-DCO' strategy, so if a conflicting
+     * option is found, we simply bail out by means of M_USAGE
+     */
+    dco_check_option_conflict(M_USAGE, o);
+    dco_check_startup_option_conflict(M_USAGE, o);
 #endif
 
     if (dco_enabled(o) && o->dev_node)