Use DCO on Windows by default

On startup, check following conditions:

 - ovpn-dco-win driver is installed. Perform this check
   by trying to open adapter by symbolic name.

 - options are compatible with dco. Same checks as on
   Linux and FreeBSD. In addition, check that --mode server
   is not used and --windows-driver is not set to tap-windows6/wintun.

If both checks are passed, use DCO.

Move options_postprocess_mutate_invariant() call
below since it depends on selected windows driver.

dco_check_option() has side effect on Windows -
if dco is not used, it might complain "cipher chachapoly
not supported by dco, disabling dco" if chachapoly
support is missing system-wide. To not to see this,
check dco options only if dco is enabled. This means
moving dco_enabled() from dco_check_startup_option()
to one level above. We do similar thing in
multi_connection_established() before checking ccd options.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20220915104028.188-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25221.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index fb27114b76499c16fe56ca94893142ba27361648..0e11c58a948403d9b6b53aceb016e6cfeb3fd58b 100644 (file)
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -257,11 +257,11 @@ dco_check_option_ce(const struct connection_entry *ce, int msglevel)
 bool
 dco_check_startup_option(int msglevel, const struct options *o)
 {
-    /* check if DCO was already disabled by the user or if no dev name was
-     * specified at all. In the latter case, later logic will most likely stop
-     * OpenVPN, so no need to print any message here.
+    /* check if no dev name was specified at all. In the case,
+     * later logic will most likely stop OpenVPN, so no need to
+     * print any message here.
      */
-    if (!dco_enabled(o) || !o->dev)
+    if (!o->dev)
     {
         return false;
     }
@@ -294,8 +294,15 @@ dco_check_startup_option(int msglevel, const struct options *o)
 #if defined(_WIN32)
     if (o->mode == MODE_SERVER)
     {
-        msg(msglevel, "Only client and p2p data channel offload is supported "
-            "with ovpn-dco.");
+        msg(msglevel, "--mode server is set. Disabling Data Channel Offload");
+        return false;
+    }
+
+    if ((o->windows_driver == WINDOWS_DRIVER_WINTUN)
+        || (o->windows_driver == WINDOWS_DRIVER_TAP_WINDOWS6))
+    {
+        msg(msglevel, "--windows-driver is set to '%s'. Disabling Data Channel Offload",
+            print_windows_driver(o->windows_driver));
         return false;
     }
 

diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c
index a20308668910dab9e2f5e72443d485734b951be7..d0fe206fe1402b01686a8638a899573699ca6946 100644 (file)
--- a/src/openvpn/dco_win.c
+++ b/src/openvpn/dco_win.c
@@ -360,7 +360,28 @@ dco_swap_keys(dco_context_t *dco, unsigned int peer_id)
 bool
 dco_available(int msglevel)
 {
-    return true;
+    /* try to open device by symbolic name */
+    HANDLE h = CreateFile("\\\\.\\ovpn-dco", GENERIC_READ | GENERIC_WRITE,
+                          0, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_SYSTEM | FILE_FLAG_OVERLAPPED, NULL);
+
+    if (h != INVALID_HANDLE_VALUE)
+    {
+        CloseHandle(h);
+        return true;
+    }
+
+    DWORD err = GetLastError();
+    if (err == ERROR_ACCESS_DENIED)
+    {
+        /* this likely means that device exists but is already in use,
+         * don't bail out since later we try to open all existing dco
+         * devices and then bail out if all devices are in use
+         */
+        return true;
+    }
+
+    msg(msglevel, "Note: ovpn-dco-win driver is missing, disabling data channel offload.");
+    return false;
 }
 
 int

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 765695dafa50762a1940d8b1d41166e0a306c44e..d7954ebc755e5c6f0ebd298c5b03492715eaca2b 100644 (file)
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -183,7 +183,7 @@ static const char usage_message[] =
     "                  does not begin with \"tun\" or \"tap\".\n"
     "--dev-node node : Explicitly set the device node rather than using\n"
     "                  /dev/net/tun, /dev/tun, /dev/tap, etc.\n"
-#if defined(ENABLE_DCO) && (defined(TARGET_LINUX) || defined(TARGET_FREEBSD))
+#if defined(ENABLE_DCO)
     "--disable-dco   : Do not attempt using Data Channel Offload.\n"
 #endif
     "--lladdr hw     : Set the link layer address of the tap device.\n"
@@ -851,7 +851,7 @@ init_options(struct options *o, const bool init_gc)
     o->tuntap_options.dhcp_masq_offset = 0;     /* use network address as internal DHCP server address */
     o->route_method = ROUTE_METHOD_ADAPTIVE;
     o->block_outside_dns = false;
-    o->windows_driver = WINDOWS_DRIVER_TAP_WINDOWS6;
+    o->windows_driver = WINDOWS_DRIVER_UNSPECIFIED;
 #endif
     o->vlan_accept = VLAN_ALL;
     o->vlan_pvid = 1;
@@ -903,6 +903,10 @@ init_options(struct options *o, const bool init_gc)
     }
 #endif /* _WIN32 */
     o->allow_recursive_routing = false;
+
+#ifndef ENABLE_DCO
+    o->tuntap_options.disable_dco = true;
+#endif /* ENABLE_DCO */
 }
 
 void
@@ -1797,7 +1801,7 @@ show_settings(const struct options *o)
     SHOW_STR(dev);
     SHOW_STR(dev_type);
     SHOW_STR(dev_node);
-#if defined(ENABLE_DCO) && (defined(TARGET_LINUX) || defined(TARGET_FREEBSD))
+#if defined(ENABLE_DCO)
     SHOW_BOOL(tuntap_options.disable_dco);
 #endif
     SHOW_STR(lladdr);
@@ -3611,8 +3615,6 @@ options_postprocess_mutate(struct options *o, struct env_set *es)
     options_set_backwards_compatible_options(o);
 
     options_postprocess_cipher(o);
-    options_postprocess_mutate_invariant(o);
-
     o->ncp_ciphers = mutate_ncp_cipher_list(o->ncp_ciphers, &o->gc);
     if (o->ncp_ciphers == NULL)
     {
@@ -3688,18 +3690,29 @@ options_postprocess_mutate(struct options *o, struct env_set *es)
             "incompatible with each other.");
     }
 
-#if defined(TARGET_LINUX) || defined(TARGET_FREEBSD)
-    /* check if any option should force disabling DCO */
-    o->tuntap_options.disable_dco = !dco_check_option(D_DCO, o)
-                                    || !dco_check_startup_option(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
-     */
     if (dco_enabled(o))
     {
-        dco_check_option(M_USAGE, o);
-        dco_check_startup_option(M_USAGE, o);
+        /* check if any option should force disabling DCO */
+        o->tuntap_options.disable_dco = !dco_check_option(D_DCO, o)
+                                        || !dco_check_startup_option(D_DCO, o);
+    }
+
+#ifdef _WIN32
+    if (dco_enabled(o))
+    {
+        o->windows_driver = WINDOWS_DRIVER_DCO;
+    }
+    else
+    {
+        if (o->windows_driver == WINDOWS_DRIVER_DCO)
+        {
+            msg(M_WARN, "Option --windows-driver ovpn-dco is ignored because Data Channel Offload is disabled");
+            o->windows_driver = WINDOWS_DRIVER_TAP_WINDOWS6;
+        }
+        else if (o->windows_driver == WINDOWS_DRIVER_UNSPECIFIED)
+        {
+            o->windows_driver = WINDOWS_DRIVER_TAP_WINDOWS6;
+        }
     }
 #endif
 
@@ -3710,6 +3723,9 @@ options_postprocess_mutate(struct options *o, struct env_set *es)
         o->dev_node = NULL;
     }
 
+    /* this depends on o->windows_driver, which is set above */
+    options_postprocess_mutate_invariant(o);
+
     /*
      * Save certain parms before modifying options during connect, especially
      * when using --pull
@@ -5908,9 +5924,7 @@ add_option(struct options *options,
 #endif
     else if (streq(p[0], "disable-dco"))
     {
-#if defined(TARGET_LINUX) || defined(TARGET_FREEBSD)
         options->tuntap_options.disable_dco = true;
-#endif
     }
     else if (streq(p[0], "dev-node") && p[1] && !p[2])
     {

diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 5488db31026bc5823452b36d7b864518de5f5727..b70cd42e0332724eb0d0a8baa4b2574e6b627afe 100644 (file)
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -883,13 +883,11 @@ bool key_is_external(const struct options *options);
 static inline bool
 dco_enabled(const struct options *o)
 {
-#if defined(_WIN32)
-    return o->windows_driver == WINDOWS_DRIVER_DCO;
-#elif defined(ENABLE_DCO)
+#ifdef ENABLE_DCO
     return !o->tuntap_options.disable_dco;
 #else
     return false;
-#endif /* defined(_WIN32) */
+#endif /* ENABLE_DCO */
 }
 
 #endif /* ifndef OPTIONS_H */

diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index a17ff50fea7289d8608437cf33836a2036d21003..2acd008203cfa1626b9347391663c1ea6c9c561b 100644 (file)
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -3545,25 +3545,6 @@ read_tun(struct tuntap *tt, uint8_t *buf, int len)
 
 #elif defined(_WIN32)
 
-static const char *
-print_windows_driver(enum windows_driver_type windows_driver)
-{
-    switch (windows_driver)
-    {
-        case WINDOWS_DRIVER_TAP_WINDOWS6:
-            return "tap-windows6";
-
-        case WINDOWS_DRIVER_WINTUN:
-            return "wintun";
-
-        case WINDOWS_DRIVER_DCO:
-            return "ovpn-dco";
-
-        default:
-            return "unspecified";
-    }
-}
-
 int
 tun_read_queue(struct tuntap *tt, int maxsize)
 {

diff --git a/src/openvpn/tun.h b/src/openvpn/tun.h
index 1cca1cfb75d473c27f68855b914d128f48bca516..24d5267036957e8b350ecf2c1edf80db307472eb 100644 (file)
--- a/src/openvpn/tun.h
+++ b/src/openvpn/tun.h
@@ -156,6 +156,7 @@ struct tuntap_options {
 
 struct tuntap_options {
     int dummy; /* not used */
+    bool disable_dco; /* not used, but removes the need in #ifdefs */
 };
 
 #endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
@@ -660,6 +661,25 @@ tuntap_is_dco_win_timeout(struct tuntap *tt, int status)
     return tuntap_is_dco_win(tt) && (status < 0) && (openvpn_errno() == ERROR_NETNAME_DELETED);
 }
 
+static const char *
+print_windows_driver(enum windows_driver_type windows_driver)
+{
+    switch (windows_driver)
+    {
+        case WINDOWS_DRIVER_TAP_WINDOWS6:
+            return "tap-windows6";
+
+        case WINDOWS_DRIVER_WINTUN:
+            return "wintun";
+
+        case WINDOWS_DRIVER_DCO:
+            return "ovpn-dco";
+
+        default:
+            return "unspecified";
+    }
+}
+
 #else  /* ifdef _WIN32 */
 
 static inline bool