From: Lev Stipakov Date: Thu, 15 Sep 2022 10:40:28 +0000 (+0300) Subject: Use DCO on Windows by default X-Git-Tag: v2.6_beta1~71 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e9a156a160bb71e03ae03ba0bf4f31ced7282a13;p=thirdparty%2Fopenvpn.git 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 Acked-by: Frank Lichtenheld 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 --- diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c index fb27114b7..0e11c58a9 100644 --- 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 a20308668..d0fe206fe 100644 --- 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 765695daf..d7954ebc7 100644 --- 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 5488db310..b70cd42e0 100644 --- 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 a17ff50fe..2acd00820 100644 --- 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 1cca1cfb7..24d526703 100644 --- 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