From 371cc5874faf67057c9f95796195306beeba0628 Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Mon, 25 Mar 2024 08:13:20 +0100 Subject: [PATCH 01/23] samples: Update sample configurations - Remove compression settings. Not recommended anymore. - Remove old cipher setting. Replaced by data-ciphers negotiation. - Add comment how to set data-ciphers for very old clients. - Remove/reword some old comments. e.g. no need to reference OpenVPN 1.x anymore. - Mention peer-fingerprint alternative. - comment out "tls-auth" as that is not needed for a bare-bones VPN config and needs additional setup. Github: #511 Change-Id: I1a36651c0dea52259533ffc00bccb9b03bf82e26 Signed-off-by: Frank Lichtenheld Acked-by: Arne Schwabe Message-Id: <20240325071320.11348-1-gert@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28451.html Signed-off-by: Gert Doering (cherry picked from commit b0fc10abd06fa2307e95c8a60fa94f7ccc08d2ac) --- sample/sample-config-files/README | 2 + sample/sample-config-files/client.conf | 23 ++++-------- sample/sample-config-files/server.conf | 51 ++++++++++++-------------- 3 files changed, 33 insertions(+), 43 deletions(-) diff --git a/sample/sample-config-files/README b/sample/sample-config-files/README index d53ac79a1a8..1493dab6342 100644 --- a/sample/sample-config-files/README +++ b/sample/sample-config-files/README @@ -4,3 +4,5 @@ These files are part of the OpenVPN HOWTO which is located at: http://openvpn.net/howto.html + +See also the openvpn-examples man page. diff --git a/sample/sample-config-files/client.conf b/sample/sample-config-files/client.conf index 15cb1b37512..c1e4060bc0c 100644 --- a/sample/sample-config-files/client.conf +++ b/sample/sample-config-files/client.conf @@ -1,5 +1,5 @@ ############################################## -# Sample client-side OpenVPN 2.0 config file # +# Sample client-side OpenVPN 2.6 config file # # for connecting to multi-client server. # # # # This configuration can be used by multiple # @@ -103,22 +103,15 @@ key client.key # EasyRSA can do this for you. remote-cert-tls server +# Allow to connect to really old OpenVPN versions +# without AEAD support (OpenVPN 2.3.x or older) +# This adds AES-256-CBC as fallback cipher and +# keeps the modern ciphers as well. +;data-ciphers AES-256-GCM:AES-128-GCM:?CHACHA20-POLY1305:AES-256-CBC + # If a tls-auth key is used on the server # then every client must also have the key. -tls-auth ta.key 1 - -# Select a cryptographic cipher. -# If the cipher option is used on the server -# then you must also specify it here. -# Note that v2.4 client/server will automatically -# negotiate AES-256-GCM in TLS mode. -# See also the data-ciphers option in the manpage -cipher AES-256-CBC - -# Enable compression on the VPN link. -# Don't enable this unless it is also -# enabled in the server config file. -#comp-lzo +;tls-auth ta.key 1 # Set log file verbosity. verb 3 diff --git a/sample/sample-config-files/server.conf b/sample/sample-config-files/server.conf index 189fb89bf5d..84c1bfea89b 100644 --- a/sample/sample-config-files/server.conf +++ b/sample/sample-config-files/server.conf @@ -1,5 +1,5 @@ ################################################# -# Sample OpenVPN 2.0 config file for # +# Sample OpenVPN 2.6 config file for # # multi-client server. # # # # This file is for the server side # @@ -47,15 +47,15 @@ proto udp # an explicit unit number, such as tun0. # On Windows, use "dev-node" for this. # On most systems, the VPN will not function -# unless you partially or fully disable +# unless you partially or fully disable/open # the firewall for the TUN/TAP interface. ;dev tap dev tun # Windows needs the TAP-Win32 adapter name # from the Network Connections panel if you -# have more than one. On XP SP2 or higher, -# you may need to selectively disable the +# have more than one. +# You may need to selectively disable the # Windows firewall for the TAP adapter. # Non-Windows systems usually don't need this. ;dev-node MyTap @@ -66,8 +66,9 @@ dev tun # key file. The server and all clients will # use the same ca file. # -# See the "easy-rsa" directory for a series -# of scripts for generating RSA certificates +# See the "easy-rsa" project at +# https://github.com/OpenVPN/easy-rsa +# for generating RSA certificates # and private keys. Remember to use # a unique Common Name for the server # and each of the client certificates. @@ -75,6 +76,13 @@ dev tun # Any X509 key management system can be used. # OpenVPN can also use a PKCS #12 formatted key file # (see "pkcs12" directive in man page). +# +# If you do not want to maintain a CA +# and have a small number of clients +# you can also use self-signed certificates +# and use the peer-fingerprint option. +# See openvpn-examples man page for a +# configuration example. ca ca.crt cert server.crt key server.key # This file should be kept secret @@ -84,12 +92,18 @@ key server.key # This file should be kept secret # openssl dhparam -out dh2048.pem 2048 dh dh2048.pem +# Allow to connect to really old OpenVPN versions +# without AEAD support (OpenVPN 2.3.x or older) +# This adds AES-256-CBC as fallback cipher and +# keeps the modern ciphers as well. +;data-ciphers AES-256-GCM:AES-128-GCM:?CHACHA20-POLY1305:AES-256-CBC + # Network topology # Should be subnet (addressing via IP) # unless Windows clients v2.0.9 and lower have to # be supported (then net30, i.e. a /30 per client) # Defaults to net30 (not recommended) -;topology subnet +topology subnet # Configure server mode and supply a VPN subnet # for OpenVPN to draw client addresses from. @@ -218,7 +232,7 @@ ifconfig-pool-persist ipp.txt # IF YOU HAVE NOT GENERATED INDIVIDUAL # CERTIFICATE/KEY PAIRS FOR EACH CLIENT, # EACH HAVING ITS OWN UNIQUE "COMMON NAME", -# UNCOMMENT THIS LINE OUT. +# UNCOMMENT THIS LINE. ;duplicate-cn # The keepalive directive causes ping-like @@ -241,26 +255,7 @@ keepalive 10 120 # a copy of this key. # The second parameter should be '0' # on the server and '1' on the clients. -tls-auth ta.key 0 # This file is secret - -# Select a cryptographic cipher. -# This config item must be copied to -# the client config file as well. -# Note that v2.4 client/server will automatically -# negotiate AES-256-GCM in TLS mode. -# See also the ncp-cipher option in the manpage -cipher AES-256-CBC - -# Enable compression on the VPN link and push the -# option to the client (v2.4+ only, for earlier -# versions see below) -;compress lz4-v2 -;push "compress lz4-v2" - -# For compression compatible with older clients use comp-lzo -# If you enable it here, you must also -# enable it in the client config file. -;comp-lzo +;tls-auth ta.key 0 # This file is secret # The maximum number of concurrently connected # clients we want to allow. From 7993084c7f2b537e20a0a0d67385733d7d56688c Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Mon, 25 Mar 2024 08:15:20 +0100 Subject: [PATCH 02/23] documentation: make section levels consistent Previously the sections "Encryption Options" and "Data channel cipher negotiation" were on the same level as "OPTIONS", which makes no sense. Instead move them and their subsections one level down. Use ` since that was already in use in section "Virtual Routing and Forwarding". Change-Id: Ib5a7f9a978bda5ad58830e43580232660401f66d Signed-off-by: Frank Lichtenheld Acked-by: Arne Schwabe Message-Id: <20240325071520.12513-1-gert@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28453.html Signed-off-by: Gert Doering (cherry picked from commit 3fdf5aa04f7b96a3b7110f75306306ac5d7ed5fd) --- doc/man-sections/cipher-negotiation.rst | 16 ++++++++-------- doc/man-sections/encryption-options.rst | 6 +++--- doc/man-sections/pkcs11-options.rst | 2 +- doc/man-sections/renegotiation.rst | 2 +- doc/man-sections/tls-options.rst | 2 +- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/doc/man-sections/cipher-negotiation.rst b/doc/man-sections/cipher-negotiation.rst index 949ff8623ea..1285e82ac80 100644 --- a/doc/man-sections/cipher-negotiation.rst +++ b/doc/man-sections/cipher-negotiation.rst @@ -1,12 +1,12 @@ Data channel cipher negotiation -=============================== +------------------------------- OpenVPN 2.4 and higher have the capability to negotiate the data cipher that is used to encrypt data packets. This section describes the mechanism in more detail and the different backwards compatibility mechanism with older server and clients. OpenVPN 2.5 and later behaviour --------------------------------- +``````````````````````````````` When both client and server are at least running OpenVPN 2.5, that the order of the ciphers of the server's ``--data-ciphers`` is used to pick the data cipher. That means that the first cipher in that list that is also in the client's @@ -25,7 +25,7 @@ For backwards compatibility OpenVPN 2.6 and later with ``--compat-mode 2.4.x`` ``--cipher`` option to this list. OpenVPN 2.4 clients -------------------- +``````````````````` The negotiation support in OpenVPN 2.4 was the first iteration of the implementation and still had some quirks. Its main goal was "upgrade to AES-256-GCM when possible". An OpenVPN 2.4 client that is built against a crypto library that supports AES in GCM @@ -40,7 +40,7 @@ always have the `AES-256-GCM` and `AES-128-GCM` ciphers to the ``--ncp-ciphers`` options to avoid this behaviour. OpenVPN 3 clients ------------------ +````````````````` Clients based on the OpenVPN 3.x library (https://github.com/openvpn/openvpn3/) do not have a configurable ``--ncp-ciphers`` or ``--data-ciphers`` option. Newer versions by default disable legacy AES-CBC, BF-CBC, and DES-CBC ciphers. @@ -52,7 +52,7 @@ included in the server's ``--data-ciphers`` option. OpenVPN 2.3 and older clients (and clients with ``--ncp-disable``) ------------------------------------------------------------------- +`````````````````````````````````````````````````````````````````` When a client without cipher negotiation support connects to a server the cipher specified with the ``--cipher`` option in the client configuration must be included in the ``--data-ciphers`` option of the server to allow @@ -65,7 +65,7 @@ If the client is 2.3 or older and has been configured with the cipher used by the client is necessary. OpenVPN 2.4 server ------------------- +`````````````````` When a client indicates support for `AES-128-GCM` and `AES-256-GCM` (with ``IV_NCP=2``) an OpenVPN 2.4 server will send the first cipher of the ``--ncp-ciphers`` to the OpenVPN client regardless of what @@ -76,7 +76,7 @@ option is required. OpenVPN 2.5+ will only announce the ``IV_NCP=2`` flag if those ciphers are present. OpenVPN 2.3 and older servers (and servers with ``--ncp-disable``) ------------------------------------------------------------------- +`````````````````````````````````````````````````````````````````` The cipher used by the server must be included in ``--data-ciphers`` to allow the client connecting to a server without cipher negotiation support. @@ -89,7 +89,7 @@ If the server is 2.3 or older and has been configured with the cipher used by the server is necessary. Blowfish in CBC mode (BF-CBC) deprecation ------------------------------------------- +````````````````````````````````````````` The ``--cipher`` option defaulted to `BF-CBC` in OpenVPN 2.4 and older version. The default was never changed to ensure backwards compatibility. In OpenVPN 2.5 this behaviour has now been changed so that if the ``--cipher`` diff --git a/doc/man-sections/encryption-options.rst b/doc/man-sections/encryption-options.rst index abc73d90c10..a88f1457640 100644 --- a/doc/man-sections/encryption-options.rst +++ b/doc/man-sections/encryption-options.rst @@ -1,8 +1,8 @@ Encryption Options -================== +------------------ SSL Library information ------------------------ +``````````````````````` --show-ciphers (Standalone) Show all cipher algorithms to use with the ``--cipher`` @@ -32,7 +32,7 @@ SSL Library information ``--ecdh-curve`` and ``tls-groups`` options. Generating key material ------------------------ +``````````````````````` --genkey args (Standalone) Generate a key to be used of the type keytype. if keyfile diff --git a/doc/man-sections/pkcs11-options.rst b/doc/man-sections/pkcs11-options.rst index de1662b7ee7..dfc27af333b 100644 --- a/doc/man-sections/pkcs11-options.rst +++ b/doc/man-sections/pkcs11-options.rst @@ -1,5 +1,5 @@ PKCS#11 / SmartCard options ---------------------------- +``````````````````````````` --pkcs11-cert-private args Set if access to certificate object should be performed after login. diff --git a/doc/man-sections/renegotiation.rst b/doc/man-sections/renegotiation.rst index c5484404def..1e7c34002b2 100644 --- a/doc/man-sections/renegotiation.rst +++ b/doc/man-sections/renegotiation.rst @@ -1,5 +1,5 @@ Data Channel Renegotiation --------------------------- +`````````````````````````` When running OpenVPN in client/server mode, the data channel will use a separate ephemeral encryption key which is rotated at regular intervals. diff --git a/doc/man-sections/tls-options.rst b/doc/man-sections/tls-options.rst index 69961024cab..81fdfc9b737 100644 --- a/doc/man-sections/tls-options.rst +++ b/doc/man-sections/tls-options.rst @@ -1,5 +1,5 @@ TLS Mode Options ----------------- +```````````````` TLS mode is the most powerful crypto mode of OpenVPN in both security and flexibility. TLS mode works by establishing control and data From 11ca69cfac1c6d3ed34652650688a4b3c99573b0 Mon Sep 17 00:00:00 2001 From: Arne Schwabe Date: Mon, 25 Mar 2024 13:50:52 +0100 Subject: [PATCH 03/23] Use snprintf instead of sprintf for get_ssl_library_version This is avoid a warning/error (when using -Werror) under current macOS of sprintf: __deprecated_msg("This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead.") Change-Id: I3c6fd36eb9daee9244d6dc6d9f22de1c5cf9d039 Signed-off-by: Arne Schwabe Acked-by: Frank Lichtenheld Message-Id: <20240325125052.14135-1-gert@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28458.html Signed-off-by: Gert Doering (cherry picked from commit 6a60d1bef424088df55f4d07efd45ce080fc7132) --- src/openvpn/ssl_mbedtls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index 97c3cf2e6d2..08877f5eb39 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -1613,7 +1613,7 @@ get_ssl_library_version(void) { static char mbedtls_version[30]; unsigned int pv = mbedtls_version_get_number(); - sprintf( mbedtls_version, "mbed TLS %d.%d.%d", + snprintf(mbedtls_version, sizeof(mbedtls_version), "mbed TLS %d.%d.%d", (pv>>24)&0xff, (pv>>16)&0xff, (pv>>8)&0xff ); return mbedtls_version; } From 5591af17694d98054da2cdf4cfd42508a8a4fb8e Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Mon, 25 Mar 2024 08:14:48 +0100 Subject: [PATCH 04/23] phase2_tcp_server: fix Coverity issue 'Dereference after null check' As Coverity says: Either the check against null is unnecessary, or there may be a null pointer dereference. In phase2_tcp_server: Pointer is checked against null but then dereferenced anyway There is only one caller (link_socket_init_phase2) and it already has an ASSERT(sig_info). So use that here was well. v2: - fix cleanly by actually asserting that sig_info is defined Change-Id: I8ef199463d46303129a3f563fd9eace780a58b8a Signed-off-by: Frank Lichtenheld Acked-by: Arne Schwabe Message-Id: <20240325071448.12143-1-gert@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28452.html Signed-off-by: Gert Doering (cherry picked from commit e8c629fe64c67ea0a8454753be99db44df7ce53e) --- src/openvpn/socket.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c index 16cd8a09efd..f9f084a304a 100644 --- a/src/openvpn/socket.c +++ b/src/openvpn/socket.c @@ -2007,7 +2007,8 @@ static void phase2_tcp_server(struct link_socket *sock, const char *remote_dynamic, struct signal_info *sig_info) { - volatile int *signal_received = sig_info ? &sig_info->signal_received : NULL; + ASSERT(sig_info); + volatile int *signal_received = &sig_info->signal_received; switch (sock->mode) { case LS_MODE_DEFAULT: @@ -2033,7 +2034,7 @@ phase2_tcp_server(struct link_socket *sock, const char *remote_dynamic, false); if (!socket_defined(sock->sd)) { - register_signal(sig_info, SIGTERM, "socket-undefiled"); + register_signal(sig_info, SIGTERM, "socket-undefined"); return; } tcp_connection_established(&sock->info.lsa->actual); From e36359aa7e5193ad002768e90ae660896a5a0fa6 Mon Sep 17 00:00:00 2001 From: Arne Schwabe Date: Tue, 26 Mar 2024 11:38:53 +0100 Subject: [PATCH 05/23] Add bracket in fingerprint message and do not warn about missing verification Github: fixes OpenVPN/openvpn#516 Change-Id: Ia73d53002f4ba2658af18c17cce1b68f79de5781 Signed-off-by: Arne Schwabe Acked-by: Frank Lichtenheld Message-Id: <20240326103853.494572-1-frank@lichtenheld.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28474.html Signed-off-by: Gert Doering (cherry picked from commit 4b95656536be1f402a55ef5dffe140fa78e7eb51) --- src/openvpn/init.c | 3 ++- src/openvpn/ssl_verify.c | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 9f43b629dd0..7e231ec5680 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -3632,7 +3632,8 @@ do_option_warnings(struct context *c) && !o->tls_verify && o->verify_x509_type == VERIFY_X509_NONE && !(o->ns_cert_type & NS_CERT_CHECK_SERVER) - && !o->remote_cert_eku) + && !o->remote_cert_eku + && !(o->verify_hash_depth == 0 && o->verify_hash)) { msg(M_WARN, "WARNING: No server certificate verification method has been enabled. See http://openvpn.net/howto.html#mitm for more info."); } diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c index c7d7799345c..930769b7796 100644 --- a/src/openvpn/ssl_verify.c +++ b/src/openvpn/ssl_verify.c @@ -718,8 +718,8 @@ verify_cert(struct tls_session *session, openvpn_x509_cert_t *cert, int cert_dep const char *hex_fp = format_hex_ex(BPTR(&cert_fp), BLEN(&cert_fp), 0, 1, ":", &gc); msg(D_TLS_ERRORS, "TLS Error: --tls-verify/--peer-fingerprint" - "certificate hash verification failed. (got " - "fingerprint: %s", hex_fp); + "certificate hash verification failed. (got certificate " + "fingerprint: %s)", hex_fp); goto cleanup; } } From ea0d9c70a44e3d871136f68bddb0befc299dd692 Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Thu, 21 Mar 2024 17:16:23 +0100 Subject: [PATCH 06/23] script-options.rst: Update ifconfig_* variables - Remove obsolete ifconfig_broadcast. Since this was removed in 2.5.0, do not add a removal note but just completely remove it. - Add missing documentation of IPv6 variants for ifconfig_pool_* variables. Github: fixes Openvpn/openvpn#527 Change-Id: Ia8c8de6799f0291fc900628fbd06c8a414e741ca Signed-off-by: Frank Lichtenheld Acked-by: Gert Doering Message-Id: <20240321161623.2794161-1-frank@lichtenheld.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28438.html Signed-off-by: Gert Doering (cherry picked from commit a94226cdc8ed037a6763675aa47e6c821983f174) --- doc/man-sections/script-options.rst | 38 ++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/doc/man-sections/script-options.rst b/doc/man-sections/script-options.rst index e05100a27f7..0d1f9aecf02 100644 --- a/doc/man-sections/script-options.rst +++ b/doc/man-sections/script-options.rst @@ -682,13 +682,6 @@ instances. recorded to this environmental variable sequence prior to ``--up`` script execution. -:code:`ifconfig_broadcast` - The broadcast address for the virtual ethernet segment which is derived - from the ``--ifconfig`` option when ``--dev tap`` is used. Set prior to - OpenVPN calling the :code:`ifconfig` or :code:`netsh` (windows version - of ifconfig) commands which normally occurs prior to ``--up`` script - execution. - :code:`ifconfig_ipv6_local` The local VPN endpoint IPv6 address specified in the ``--ifconfig-ipv6`` option (first parameter). Set prior to OpenVPN @@ -731,30 +724,53 @@ instances. occurs prior to ``--up`` script execution. :code:`ifconfig_pool_local_ip` - The local virtual IP address for the TUN/TAP tunnel taken from an + The local virtual IPv4 address for the TUN/TAP tunnel taken from an ``--ifconfig-push`` directive if specified, or otherwise from the ifconfig pool (controlled by the ``--ifconfig-pool`` config file directive). Only set for ``--dev tun`` tunnels. This option is set on the server prior to execution of the ``--client-connect`` and ``--client-disconnect`` scripts. +:code:`ifconfig_pool_local_ip6` + The local virtual IPv6 address for the TUN/TAP tunnel taken from an + ``--ifconfig-ipv6-push`` directive if specified, or otherwise from the + ifconfig pool (controlled by the ``--ifconfig-ipv6-pool`` config file + directive). Only set for ``--dev tun`` tunnels. This option is set on + the server prior to execution of the ``--client-connect`` and + ``--client-disconnect`` scripts. + :code:`ifconfig_pool_netmask` - The virtual IP netmask for the TUN/TAP tunnel taken from an + The virtual IPv4 netmask for the TUN/TAP tunnel taken from an ``--ifconfig-push`` directive if specified, or otherwise from the ifconfig pool (controlled by the ``--ifconfig-pool`` config file directive). Only set for ``--dev tap`` tunnels. This option is set on the server prior to execution of the ``--client-connect`` and ``--client-disconnect`` scripts. +:code:`ifconfig_pool_ip6_netbits` + The virtual IPv6 prefix length for the TUN/TAP tunnel taken from an + ``--ifconfig-ipv6-push`` directive if specified, or otherwise from the + ifconfig pool (controlled by the ``--ifconfig-ipv6-pool`` config file + directive). Only set for ``--dev tap`` tunnels. This option is set on + the server prior to execution of the ``--client-connect`` and + ``--client-disconnect`` scripts. + :code:`ifconfig_pool_remote_ip` - The remote virtual IP address for the TUN/TAP tunnel taken from an + The remote virtual IPv4 address for the TUN/TAP tunnel taken from an ``--ifconfig-push`` directive if specified, or otherwise from the ifconfig pool (controlled by the ``--ifconfig-pool`` config file directive). This option is set on the server prior to execution of the ``--client-connect`` and ``--client-disconnect`` scripts. +:code:`ifconfig_pool_remote_ip6` + The remote virtual IPv6 address for the TUN/TAP tunnel taken from an + ``--ifconfig-ipv6-push`` directive if specified, or otherwise from the + ifconfig pool (controlled by the ``--ifconfig-ipv6-pool`` config file + directive). This option is set on the server prior to execution of the + ``--client-connect`` and ``--client-disconnect`` scripts. + :code:`link_mtu` - No longer passed to scripts since OpenVPN 2.6.0. Used to be the + *REMOVED* No longer passed to scripts since OpenVPN 2.6.0. Used to be the maximum packet size (not including the IP header) of tunnel data in UDP tunnel transport mode. From f50c67707ed033040c93a6b5d4efbbd2c0933459 Mon Sep 17 00:00:00 2001 From: Lev Stipakov Date: Fri, 29 Mar 2024 11:37:39 +0100 Subject: [PATCH 07/23] misc.c: remove unused code Commit 3a4fb1 "Ensure --auth-nocache is handled during renegotiation" has changed the behavior of set_auth_token(), but left unused parameter struct user_pass *up Remove this parameter and amend comments accordingly. Also remove unused function definition from misc.h. Signed-off-by: Lev Stipakov Acked-by: Frank Lichtenheld Change-Id: Ic440f2c8d46dfcb5ff41ba2f33bf28bb7286eec4 Message-Id: <20240329103739.28254-1-gert@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28503.html Signed-off-by: Gert Doering (cherry picked from commit 4c71e816031f564f834df695b3fa717ea22720d2) --- src/openvpn/misc.c | 8 ++------ src/openvpn/misc.h | 12 ++---------- src/openvpn/ssl.c | 2 +- 3 files changed, 5 insertions(+), 17 deletions(-) diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c index edc8572b703..00940efd4c7 100644 --- a/src/openvpn/misc.c +++ b/src/openvpn/misc.c @@ -491,19 +491,15 @@ purge_user_pass(struct user_pass *up, const bool force) } void -set_auth_token(struct user_pass *up, struct user_pass *tk, const char *token) +set_auth_token(struct user_pass *tk, const char *token) { - if (strlen(token)) { strncpynt(tk->password, token, USER_PASS_LEN); tk->token_defined = true; /* - * --auth-token has no username, so it needs the username - * either already set or copied from up, or later set by - * --auth-token-user - * If already set, tk is fully defined. + * If username already set, tk is fully defined. */ if (strlen(tk->username)) { diff --git a/src/openvpn/misc.h b/src/openvpn/misc.h index 24a1ca56a48..0dc08834e41 100644 --- a/src/openvpn/misc.h +++ b/src/openvpn/misc.h @@ -135,26 +135,18 @@ get_user_pass(struct user_pass *up, return get_user_pass_cr(up, auth_file, prefix, flags, NULL); } -void fail_user_pass(const char *prefix, - const unsigned int flags, - const char *reason); - void purge_user_pass(struct user_pass *up, const bool force); /** - * Sets the auth-token to token. If a username is available from - * either up or already present in tk that will be used as default - * username for the token. The method will also purge up if + * Sets the auth-token to token. The method will also purge up if * the auth-nocache option is active. * - * @param up (non Auth-token) Username/password * @param tk auth-token userpass to set * @param token token to use as password for the auth-token * * @note all parameters to this function must not be null. */ -void set_auth_token(struct user_pass *up, struct user_pass *tk, - const char *token); +void set_auth_token(struct user_pass *tk, const char *token); /** * Sets the auth-token username by base64 decoding the passed diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 6aa1051521c..162a8622de7 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -504,7 +504,7 @@ ssl_set_auth_nocache(void) void ssl_set_auth_token(const char *token) { - set_auth_token(&auth_user_pass, &auth_token, token); + set_auth_token(&auth_token, token); } void From 18520e5a25a983b616762e6082da8436d0933411 Mon Sep 17 00:00:00 2001 From: Arne Schwabe Date: Thu, 2 May 2024 14:22:31 +0200 Subject: [PATCH 08/23] Replace macos11 with macos14 in github runners Github's documentation states: macos-11 label has been deprecated and will no longer be available after 6/28/2024. Add macos14 which is nowadays supported instead. The github macos-14 runner is using the M1 platform with ARM, so this requires a bit more adjustment of paths. Change-Id: Ia70f230b2e9a78939d1875395205c8f48c4944b7 Signed-off-by: Arne Schwabe Acked-by: Frank Lichtenheld Message-Id: <20240502122231.672-1-gert@greenie.muc.de> URL: https://www.mail-archive.com/search?l=mid&q=20240502122231.672-1-gert@greenie.muc.de Signed-off-by: Gert Doering (cherry picked from commit 02f0845be7e54e8676e73621e424b6a1540b88b5) --- .github/workflows/build.yaml | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index b259212351a..be677bcceef 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -194,8 +194,16 @@ jobs: matrix: ssllib: [ openssl11, openssl3, libressl] build: [ normal, asan ] - os: [macos-11, macos-12] + os: [macos-12, macos-13, macos-14] include: + # macos14 and newer runners use ARM CPUs and homebrew uses /opt/homebrew/ + # on ARM instead of /usr/local/ + - os: macos-12 + homebrew: /usr/local/opt + - os: macos-13 + homebrew: /usr/local/opt + - os: macos-14 + homebrew: /opt/homebrew/opt - build: asan cflags: "-fsanitize=address -fno-optimize-sibling-calls -fsanitize-address-use-after-scope -fno-omit-frame-pointer -g -O1" ldflags: -fsanitize=address @@ -220,8 +228,10 @@ jobs: env: CFLAGS: ${{ matrix.cflags }} LDFLAGS: ${{ matrix.ldflags }} - OPENSSL_CFLAGS: "-I/usr/local/opt/${{matrix.libdir}}/include" - OPENSSL_LIBS: "-L/usr/local/opt/${{matrix.libdir}}/lib -lcrypto -lssl" + OPENSSL_CFLAGS: "-I${{matrix.homebrew}}/${{matrix.libdir}}/include" + OPENSSL_LIBS: "-L${{matrix.homebrew}}/${{matrix.libdir}}/lib -lcrypto -lssl" + LZO_CFLAGS: "-I${{matrix.homebrew}}/lzo/include" + LZO_LIBS: "-L${{matrix.homebrew}}/lzo/lib -llzo2" UBSAN_OPTIONS: print_stacktrace=1 steps: - name: Install dependencies From 56fc48e87decfa16a15ab0293853c473bf56703f Mon Sep 17 00:00:00 2001 From: Arne Schwabe Date: Mon, 6 May 2024 17:58:31 +0200 Subject: [PATCH 09/23] Only run coverity scan in OpenVPN/OpenVPN repository This avoids the error message triggering every night that the run failed in forked repositories Change-Id: Id95e0124d943912439c6ec6f562c0eb40d434163 Signed-off-by: Arne Schwabe Acked-by: Frank Lichtenheld Message-Id: <20240506155831.3524-1-gert@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28627.html Signed-off-by: Gert Doering (cherry picked from commit 815df21d389bf70dbe98cb89f2c60b6e6e816faa) --- .github/workflows/coverity-scan.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/coverity-scan.yml b/.github/workflows/coverity-scan.yml index e289746daa5..37b8102eb21 100644 --- a/.github/workflows/coverity-scan.yml +++ b/.github/workflows/coverity-scan.yml @@ -6,6 +6,9 @@ on: jobs: latest: + # Running coverity requires the secrets.COVERITY_SCAN_TOKEN token + # which is only available on the main repository + if: github.repository_owner == 'OpenVPN' runs-on: ubuntu-latest steps: - name: Check submission cache From 8aed156be81a3bdd3098bfed5e8f95662d06633c Mon Sep 17 00:00:00 2001 From: Arne Schwabe Date: Thu, 9 May 2024 00:05:40 +0200 Subject: [PATCH 10/23] Workaround issue in LibreSSL crashing when enumerating digests/ciphers OpenBSD/LibreSSL reimplemented EVP_get_cipherbyname/EVP_get_digestbyname and broke calling EVP_get_cipherbynid/EVP_get_digestbyname with an invalid nid in the process so that it would segfault. Workaround but doing that NULL check in OpenVPN instead of leaving it to the library. Github: see also https://github.com/libressl/openbsd/issues/150 Change-Id: Ia08a9697d0ff41721fb0acf17ccb4cfa23cb3934 Signed-off-by: Arne Schwabe Acked-by: Gert Doering Message-Id: <20240508220540.12554-1-gert@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28649.html Signed-off-by: Gert Doering (cherry picked from commit b3a271b11723cbe520ad4ce6b4b0459de57ade06) --- src/openvpn/crypto_openssl.c | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c index a773eeaec7c..fbc95ff7fbc 100644 --- a/src/openvpn/crypto_openssl.c +++ b/src/openvpn/crypto_openssl.c @@ -386,7 +386,19 @@ show_available_ciphers(void) #else for (int nid = 0; nid < 10000; ++nid) { +#if defined(LIBRESSL_VERSION_NUMBER) + /* OpenBSD/LibreSSL reimplemented EVP_get_cipherbyname and broke + * calling EVP_get_cipherbynid with an invalid nid in the process + * so that it would segfault. */ + const EVP_CIPHER *cipher = NULL; + const char *name = OBJ_nid2sn(nid); + if (name) + { + cipher = EVP_get_cipherbyname(name); + } +#else /* if defined(LIBRESSL_VERSION_NUMBER) */ const EVP_CIPHER *cipher = EVP_get_cipherbynid(nid); +#endif /* We cast the const away so we can keep the function prototype * compatible with EVP_CIPHER_do_all_provided */ collect_ciphers((EVP_CIPHER *) cipher, &cipher_list); @@ -440,7 +452,19 @@ show_available_digests(void) #else for (int nid = 0; nid < 10000; ++nid) { + /* OpenBSD/LibreSSL reimplemented EVP_get_digestbyname and broke + * calling EVP_get_digestbynid with an invalid nid in the process + * so that it would segfault. */ +#ifdef LIBRESSL_VERSION_NUMBER + const EVP_MD *digest = NULL; + const char *name = OBJ_nid2sn(nid); + if (name) + { + digest = EVP_get_digestbyname(name); + } +#else /* ifdef LIBRESSL_VERSION_NUMBER */ const EVP_MD *digest = EVP_get_digestbynid(nid); +#endif if (digest) { /* We cast the const away so we can keep the function prototype @@ -448,7 +472,7 @@ show_available_digests(void) print_digest((EVP_MD *)digest, NULL); } } -#endif +#endif /* if OPENSSL_VERSION_NUMBER >= 0x30000000L */ printf("\n"); } From 65fb67cd6c320a426567b2922c4282fb8738ba3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Reynir=20Bj=C3=B6rnsson?= Date: Thu, 16 May 2024 13:58:08 +0200 Subject: [PATCH 11/23] Only schedule_exit() once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If an exit has already been scheduled we should not schedule it again. Otherwise, the exit signal is never emitted if the peer reschedules the exit before the timeout occurs. schedule_exit() now only takes the context as argument. The signal is hard coded to SIGTERM, and the interval is read directly from the context options. Furthermore, schedule_exit() now returns a bool signifying whether an exit was scheduled; false if exit is already scheduled. The call sites are updated accordingly. A notable difference is that management is only notified *once* when an exit is scheduled - we no longer notify management on redundant exit. This patch was assigned a CVE number after already reviewed and ACKed, because it was discovered that a misbehaving client can use the (now fixed) server behaviour to avoid being disconnected by means of a managment interface "client-kill" command - the security issue here is "client can circumvent security policy set by management interface". This only affects previously authenticated clients, and only management client-kill, so normal renegotion / AUTH_FAIL ("your session ends") is not affected. CVE: 2024-28882 Change-Id: I9457f005f4ba970502e6b667d9dc4299a588d661 Signed-off-by: Reynir Björnsson Acked-by: Arne Schwabe Message-Id: <20240516120434.23499-1-gert@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28679.html Signed-off-by: Gert Doering (cherry picked from commit 55bb3260c12bae33b6a8eac73cbb6972f8517411) --- src/openvpn/forward.c | 15 +++++++++++---- src/openvpn/forward.h | 2 +- src/openvpn/push.c | 12 +++++++----- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index e9811b9c81d..29e812ffd17 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -514,17 +514,24 @@ check_server_poll_timeout(struct context *c) } /* - * Schedule a signal n_seconds from now. + * Schedule a SIGTERM signal c->options.scheduled_exit_interval seconds from now. */ -void -schedule_exit(struct context *c, const int n_seconds, const int signal) +bool +schedule_exit(struct context *c) { + const int n_seconds = c->options.scheduled_exit_interval; + /* don't reschedule if already scheduled. */ + if (event_timeout_defined(&c->c2.scheduled_exit)) + { + return false; + } tls_set_single_session(c->c2.tls_multi); update_time(); reset_coarse_timers(c); event_timeout_init(&c->c2.scheduled_exit, n_seconds, now); - c->c2.scheduled_exit_signal = signal; + c->c2.scheduled_exit_signal = SIGTERM; msg(D_SCHED_EXIT, "Delayed exit in %d seconds", n_seconds); + return true; } /* diff --git a/src/openvpn/forward.h b/src/openvpn/forward.h index 060fc374ca6..245a8029211 100644 --- a/src/openvpn/forward.h +++ b/src/openvpn/forward.h @@ -302,7 +302,7 @@ void reschedule_multi_process(struct context *c); void process_ip_header(struct context *c, unsigned int flags, struct buffer *buf); -void schedule_exit(struct context *c, const int n_seconds, const int signal); +bool schedule_exit(struct context *c); static inline struct link_socket_info * get_link_socket_info(struct context *c) diff --git a/src/openvpn/push.c b/src/openvpn/push.c index 1b406b9c531..d220eeb9744 100644 --- a/src/openvpn/push.c +++ b/src/openvpn/push.c @@ -204,7 +204,11 @@ receive_exit_message(struct context *c) * */ if (c->options.mode == MODE_SERVER) { - schedule_exit(c, c->options.scheduled_exit_interval, SIGTERM); + if (!schedule_exit(c)) + { + /* Return early when we don't need to notify management */ + return; + } } else { @@ -391,7 +395,7 @@ __attribute__ ((format(__printf__, 4, 5))) void send_auth_failed(struct context *c, const char *client_reason) { - if (event_timeout_defined(&c->c2.scheduled_exit)) + if (!schedule_exit(c)) { msg(D_TLS_DEBUG, "exit already scheduled for context"); return; @@ -401,8 +405,6 @@ send_auth_failed(struct context *c, const char *client_reason) static const char auth_failed[] = "AUTH_FAILED"; size_t len; - schedule_exit(c, c->options.scheduled_exit_interval, SIGTERM); - len = (client_reason ? strlen(client_reason)+1 : 0) + sizeof(auth_failed); if (len > PUSH_BUNDLE_SIZE) { @@ -492,7 +494,7 @@ send_auth_pending_messages(struct tls_multi *tls_multi, void send_restart(struct context *c, const char *kill_msg) { - schedule_exit(c, c->options.scheduled_exit_interval, SIGTERM); + schedule_exit(c); send_control_channel_string(c, kill_msg ? kill_msg : "RESTART", D_PUSH); } From 2f2ff186564c3999efaf48d734df95471ac22d84 Mon Sep 17 00:00:00 2001 From: Franco Fichtner Date: Tue, 28 May 2024 17:42:52 +0000 Subject: [PATCH 12/23] Allow to set ifmode for existing DCO interfaces in FreeBSD While prexisting devices work well TUN/TAP the DCO interfaces require setting the ifmode which cannot be done by FreeBSD base tooling. In peer-to-peer mode this is not a problem because that is the default mode. Subnet mode, however, will fail to be set and the resulting connection does not start: Failed to create interface ovpns2 (SIOCSIFNAME): File exists (errno=17) DCO device ovpns2 already exists, won't be destroyed at shutdown /sbin/ifconfig ovpns2 10.1.8.1/24 mtu 1500 up ifconfig: in_exec_nl(): Empty IFA_LOCAL/IFA_ADDRESS ifconfig: ioctl (SIOCAIFADDR): Invalid argument FreeBSD ifconfig failed: external program exited with error status: 1 Exiting due to fatal error Slightly restructure the code to catch the specific error condition and execute dco_set_ifmode() in this case as well. Signed-off-by: Franco Fichtner Acked-by: Gert Doering Message-Id: URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28688.html Signed-off-by: Gert Doering (cherry picked from commit 82036c17c45d45c3fe8725f64b33720cb9c94dad) --- src/openvpn/dco_freebsd.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c index 577c65f879c..7c8b29c9b48 100644 --- a/src/openvpn/dco_freebsd.c +++ b/src/openvpn/dco_freebsd.c @@ -219,6 +219,9 @@ create_interface(struct tuntap *tt, const char *dev) { ifr.ifr_data = (char *)dev; } + + snprintf(tt->dco.ifname, IFNAMSIZ, "%s", ifr.ifr_data); + ret = ioctl(tt->dco.fd, SIOCSIFNAME, &ifr); if (ret) { @@ -229,16 +232,6 @@ create_interface(struct tuntap *tt, const char *dev) return ret; } - snprintf(tt->dco.ifname, IFNAMSIZ, "%s", ifr.ifr_data); - - /* see "Interface Flags" in ifnet(9) */ - int i = IFF_POINTOPOINT | IFF_MULTICAST; - if (tt->topology == TOP_SUBNET) - { - i = IFF_BROADCAST | IFF_MULTICAST; - } - dco_set_ifmode(&tt->dco, i); - return 0; } @@ -265,7 +258,20 @@ remove_interface(struct tuntap *tt) int open_tun_dco(struct tuntap *tt, openvpn_net_ctx_t *ctx, const char *dev) { - return create_interface(tt, dev); + int ret = create_interface(tt, dev); + + if (ret >= 0 || ret == -EEXIST) + { + /* see "Interface Flags" in ifnet(9) */ + int i = IFF_POINTOPOINT | IFF_MULTICAST; + if (tt->topology == TOP_SUBNET) + { + i = IFF_BROADCAST | IFF_MULTICAST; + } + dco_set_ifmode(&tt->dco, i); + } + + return ret; } void From 1ae753e4240434abda0a33aed07b289fa9c6ee79 Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Tue, 4 Jun 2024 23:17:08 +0200 Subject: [PATCH 13/23] LZO: do not use lzoutils.h macros Instead of lzo_{free,malloc} we can just use the free and malloc as the lzoutils.h header itself suggests. Change-Id: I32ee28fde5d38d736f753c782d88a81de7fe2980 Signed-off-by: Frank Lichtenheld Acked-by: Arne Schwabe Message-Id: <20240604211708.32315-1-gert@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28705.html Signed-off-by: Gert Doering (cherry picked from commit d601237976323b5d8f6ac65c27ccc510563ad75f) --- config.h.cmake.in | 3 --- configure.ac | 9 --------- src/openvpn/lzo.c | 4 ++-- src/openvpn/lzo.h | 9 ++------- 4 files changed, 4 insertions(+), 21 deletions(-) diff --git a/config.h.cmake.in b/config.h.cmake.in index 7021310ab87..e0abdb0a1c3 100644 --- a/config.h.cmake.in +++ b/config.h.cmake.in @@ -181,9 +181,6 @@ don't. */ /* Define to 1 if you have the header file. */ #define HAVE_LZO1X_H 1 -/* Define to 1 if you have the header file. */ -#define HAVE_LZOUTIL_H 1 - /* Define to 1 if you have the `mlockall' function. */ #cmakedefine HAVE_MLOCKALL diff --git a/configure.ac b/configure.ac index 1619b434056..f56d0ccf5a7 100644 --- a/configure.ac +++ b/configure.ac @@ -1169,15 +1169,6 @@ fi if test "${have_lzo}" = "yes"; then saved_CFLAGS="${CFLAGS}" CFLAGS="${CFLAGS} ${LZO_CFLAGS}" - AC_CHECK_HEADERS( - [lzo/lzoutil.h], - , - [AC_CHECK_HEADERS( - [lzoutil.h], - , - [AC_MSG_ERROR([lzoutil.h is missing])] - )] - ) AC_CHECK_HEADERS( [lzo/lzo1x.h], , diff --git a/src/openvpn/lzo.c b/src/openvpn/lzo.c index b313284df75..bab2d7828cf 100644 --- a/src/openvpn/lzo.c +++ b/src/openvpn/lzo.c @@ -107,14 +107,14 @@ lzo_compress_init(struct compress_context *compctx) { msg(M_FATAL, "Cannot initialize LZO compression library (lzo_init() returns %d)", lzo_status); } - compctx->wu.lzo.wmem = (lzo_voidp) lzo_malloc(compctx->wu.lzo.wmem_size); + compctx->wu.lzo.wmem = (lzo_voidp) malloc(compctx->wu.lzo.wmem_size); check_malloc_return(compctx->wu.lzo.wmem); } static void lzo_compress_uninit(struct compress_context *compctx) { - lzo_free(compctx->wu.lzo.wmem); + free(compctx->wu.lzo.wmem); compctx->wu.lzo.wmem = NULL; } diff --git a/src/openvpn/lzo.h b/src/openvpn/lzo.h index e951b49bbdf..62d73a1b36b 100644 --- a/src/openvpn/lzo.h +++ b/src/openvpn/lzo.h @@ -40,16 +40,11 @@ #if defined(HAVE_LZO_CONF_H) /* The lzo.h magic gets confused and still wants * to include lzo/lzoconf.h even if our include paths - * are setup to include the paths without lzo/ include lzoconf.h to - * avoid it being include by lzoutil.h */ + * are setup to include the paths without lzo/ + */ #include #include #endif -#if defined(HAVE_LZO_LZOUTIL_H) -#include -#elif defined(HAVE_LZOUTIL_H) -#include -#endif #if defined(HAVE_LZO_LZO1X_H) #include #elif defined(HAVE_LZO1X_H) From dfbe11ac1842df400327be22951d0ba373534254 Mon Sep 17 00:00:00 2001 From: Heiko Wundram Date: Thu, 6 Jun 2024 12:34:41 +0200 Subject: [PATCH 14/23] Implement Windows CA template match for Crypto-API selector The certificate selection process for the Crypto API certificates is currently fixed to match on subject or identifier. Especially if certificates that are used for OpenVPN are managed by a Windows CA, it is appropriate to select the certificate to use by the template that it is generated from, especially on domain-joined clients which automatically acquire/renew the corresponding certificate. The attached match implements the match on TMPL: with either a template name (which is looked up through CryptFindOIDInfo) or by specifying the OID of the template directly, which then is matched against the corresponding X509 extensions specifying the template that the certificate was generated from. The logic requires to walk all certificates in the underlying store and to match the certificate extensions directly. The hook which is implemented in the certificate selection logic is generic to allow other Crypto-API certificate matches to also be implemented at some point in the future. The logic to match the certificate template is taken from the implementation in the .NET core runtime, see Pal.Windows/FindPal.cs in in the implementation of System.Security.Cryptography.X509Certificates. Change-Id: Ia2c3e4c5c83ecccce1618c43b489dbe811de5351 Signed-off-by: Heiko Wundram Signed-off-by: Hannes Domani Acked-by: Selva Nair Message-Id: <20240606103441.26598-1-gert@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28726.html Signed-off-by: Gert Doering (cherry picked from commit 13ee7f902f18e27b981f8e440facd2e6515c6c83) --- doc/man-sections/windows-options.rst | 7 ++ src/openvpn/cryptoapi.c | 101 ++++++++++++++++++++++++++- 2 files changed, 105 insertions(+), 3 deletions(-) diff --git a/doc/man-sections/windows-options.rst b/doc/man-sections/windows-options.rst index e87291f4691..19558698100 100644 --- a/doc/man-sections/windows-options.rst +++ b/doc/man-sections/windows-options.rst @@ -55,6 +55,13 @@ Windows-Specific Options cryptoapicert "ISSUER:Sample CA" + To select a certificate based on a certificate's template name or + OID of the template: + :: + + cryptoapicert "TMPL:Name of Template" + cryptoapicert "TMPL:1.3.6.1.4..." + The first non-expired certificate found in the user's store or the machine store that matches the select-string is used. diff --git a/src/openvpn/cryptoapi.c b/src/openvpn/cryptoapi.c index f7e5b674b5d..67dc382dc4e 100644 --- a/src/openvpn/cryptoapi.c +++ b/src/openvpn/cryptoapi.c @@ -178,6 +178,87 @@ parse_hexstring(const char *p, unsigned char *arr, size_t capacity) return i; } +static void * +decode_object(struct gc_arena *gc, LPCSTR struct_type, + const CRYPT_OBJID_BLOB *val, DWORD flags, DWORD *cb) +{ + /* get byte count for decoding */ + BYTE *buf; + if (!CryptDecodeObject(X509_ASN_ENCODING | PKCS_7_ASN_ENCODING, struct_type, + val->pbData, val->cbData, flags, NULL, cb)) + { + return NULL; + } + + /* do the actual decode */ + buf = gc_malloc(*cb, false, gc); + if (!CryptDecodeObject(X509_ASN_ENCODING | PKCS_7_ASN_ENCODING, struct_type, + val->pbData, val->cbData, flags, buf, cb)) + { + return NULL; + } + + return buf; +} + +static const CRYPT_OID_INFO * +find_oid(DWORD keytype, const void *key, DWORD groupid) +{ + const CRYPT_OID_INFO *info = NULL; + + /* try proper resolve, also including AD */ + info = CryptFindOIDInfo(keytype, (void *)key, groupid); + + /* fall back to all groups if not found yet */ + if (!info && groupid) + { + info = CryptFindOIDInfo(keytype, (void *)key, 0); + } + + return info; +} + +static bool +test_certificate_template(const char *cert_prop, const CERT_CONTEXT *cert_ctx) +{ + const CERT_INFO *info = cert_ctx->pCertInfo; + const CERT_EXTENSION *ext; + DWORD cbext; + void *pvext; + struct gc_arena gc = gc_new(); + const WCHAR *tmpl_name = wide_string(cert_prop, &gc); + + /* check for V2 extension (Windows 2003+) */ + ext = CertFindExtension(szOID_CERTIFICATE_TEMPLATE, info->cExtension, info->rgExtension); + if (ext) + { + pvext = decode_object(&gc, X509_CERTIFICATE_TEMPLATE, &ext->Value, 0, &cbext); + if (pvext && cbext >= sizeof(CERT_TEMPLATE_EXT)) + { + const CERT_TEMPLATE_EXT *cte = (const CERT_TEMPLATE_EXT *)pvext; + if (!stricmp(cert_prop, cte->pszObjId)) + { + /* found direct OID match with certificate property specified */ + gc_free(&gc); + return true; + } + + const CRYPT_OID_INFO *tmpl_oid = find_oid(CRYPT_OID_INFO_NAME_KEY, tmpl_name, + CRYPT_TEMPLATE_OID_GROUP_ID); + if (tmpl_oid && !stricmp(tmpl_oid->pszOID, cte->pszObjId)) + { + /* found OID match in extension against resolved key */ + gc_free(&gc); + return true; + } + } + } + + /* no extension found, exit */ + gc_free(&gc); + return false; +} + static const CERT_CONTEXT * find_certificate_in_store(const char *cert_prop, HCERTSTORE cert_store) { @@ -186,6 +267,7 @@ find_certificate_in_store(const char *cert_prop, HCERTSTORE cert_store) * SUBJ: * THUMB:, e.g. * THUMB:f6 49 24 41 01 b4 fb 44 0c ce f4 36 ae d0 c4 c9 df 7a b6 28 + * TMPL: