[Openvpn-devel] [PATCH applied] Re: add and send IV_PROTO_DNS_OPTION_V2 flag
I have not tested this beyond "does it compile". My understanding is that this is to align openvpn 2.x and 3.x in regards to "if this bit is set, the client understands the new variants in `--dns`" and since the "new code" is only in master, so is this patch. Your patch has been applied to the master branch. commit 8991f0d5c6c06d1e42919d1d6a0813ca1c46f8a1 (master) Author: Heiko Hund Date: Thu Jul 25 13:22:48 2024 +0200 add and send IV_PROTO_DNS_OPTION_V2 flag Signed-off-by: Heiko Hund Acked-by: Arne Schwabe Message-Id: <20240725112248.21075-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28970.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v1] add and send IV_PROTO_DNS_OPTION_V2 flag
From: Heiko Hund Incompatible changes to the --dns server address and --dns server exclude-domains options were introduced after the code for handling them was released. Add and send a new IV_PROTO flag, so servers which act on the flags set can differentiate between clients which have implemented --dns and those which just support the new option. This enables them to decide which variant of options to send to the client. Change-Id: I975057c20c1457ef88111f8d142ca3fd2039d5ff Signed-off-by: Heiko Hund Acked-by: Arne Schwabe --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/680 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Arne Schwabe diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index e0e9591..14c38cf 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -1900,8 +1900,8 @@ /* support for P_DATA_V2 */ int iv_proto = IV_PROTO_DATA_V2; -/* support for the --dns option */ -iv_proto |= IV_PROTO_DNS_OPTION; +/* support for the latest --dns option */ +iv_proto |= IV_PROTO_DNS_OPTION_V2; /* support for exit notify via control channel */ iv_proto |= IV_PROTO_CC_EXIT_NOTIFY; diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h index 1a45048..6c2bfc3 100644 --- a/src/openvpn/ssl.h +++ b/src/openvpn/ssl.h @@ -94,7 +94,7 @@ * result. */ #define IV_PROTO_NCP_P2P (1<<5) -/** Supports the --dns option introduced in version 2.6 */ +/** Supports the --dns option introduced in version 2.6. Not sent anymore. */ #define IV_PROTO_DNS_OPTION (1<<6) /** Support for explicit exit notify via control channel @@ -107,6 +107,9 @@ /** Support to dynamic tls-crypt (renegotiation with TLS-EKM derived tls-crypt key) */ #define IV_PROTO_DYN_TLS_CRYPT (1<<9) +/** Supports the --dns option after all the incompatible changes */ +#define IV_PROTO_DNS_OPTION_V2 (1<<11) + /* Default field in X509 to be username */ #define X509_USERNAME_FIELD_DEFAULT "CN" ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Add Ubuntu 24.04 runner to Github Actions
Your patch has been applied to the master branch. I tried to apply it to release/2.6 as well, but the mbedtls builds fail (on 24.04) with error messages like this: crypto_backend.h:352:6: note: previous declaration of `cipher_ctx_init' with type `void(cipher_ctx_t *, const uint8_t *, const char *, int)' {aka `void(mbedtls_cipher_context_t *, const unsigned char *, const char *, int)'} so it seems we need some mbedtls compat backport first, or stop building 2.6 + 24.04 + mbedtls. Details: https://github.com/cron2/openvpn/actions/runs/10041826490/job/27750800767 (looking more closely, 24.04 + ossl 3 fails as well, with "configure: error: PKCS11 enabled but libpkcs11-helper is missing") commit 856065b2eb37286c389550593472bf180bc5be9d (master) Author: Arne Schwabe Date: Fri Jul 19 15:11:41 2024 +0200 Add Ubuntu 24.04 runner to Github Actions Signed-off-by: Arne Schwabe Acked-by: Frank Lichtenheld Message-Id: <20240719131141.75324-1-fr...@lichtenheld.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28942.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Fix missing spaces in various messages
Went through the individual messages, all very welcome fixes. Test compiled, just to be sure no quote or anything escaped. Your patch has been applied to the master and release/2.6 branch (bugfix). commit 824fe9ce497bd26a9609abb7324427e906ead6a4 (master) commit 02346806adafd3c656f018a7a1b3fb2c585a1cea (release/2.6) Author: Frank Lichtenheld Date: Mon Jul 22 14:10:34 2024 +0200 Fix missing spaces in various messages Signed-off-by: Frank Lichtenheld Acked-by: Gert Doering Message-Id: <20240722121034.10816-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28950.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v1] Fix missing spaces in various messages
From: Frank Lichtenheld These result from broken up literals where it is easy to miss the missing space. Change-Id: Ic27d84c74c1dd6ff7973ca6966d186f475c67e21 Signed-off-by: Frank Lichtenheld Acked-by: Gert Doering --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/679 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c index 78243b1..7f0d53d 100644 --- a/src/openvpn/dco.c +++ b/src/openvpn/dco.c @@ -185,7 +185,7 @@ } else { -msg(D_DCO_DEBUG, "Swapping primary and secondary keys to" +msg(D_DCO_DEBUG, "Swapping primary and secondary keys to " "primary-id=%d secondary-id=(to be deleted)", primary->key_id); } diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 162b23e..03177bb 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -1804,7 +1804,7 @@ } else if (dco_enabled(o)) { -msg(M_INFO, "Client does not support DATA_V2. Data channel offloaing " +msg(M_INFO, "Client does not support DATA_V2. Data channel offloading " "requires DATA_V2. Dropping client."); auth_set_client_reason(tls_multi, "Data channel negotiation " "failed (missing DATA_V2)"); @@ -1815,7 +1815,7 @@ * not accept our pushed ciphers */ if (proto & IV_PROTO_NCP_P2P) { -msg(M_WARN, "Note: peer reports running in P2P mode (no --pull/--client" +msg(M_WARN, "Note: peer reports running in P2P mode (no --pull/--client " "option). It will not negotiate ciphers with this server. " "Expect this connection to fail."); } @@ -2027,7 +2027,7 @@ /* Not EOF but other error -> fall through to error state */ default: /* We received an unknown/unexpected value. Assume failure. */ -msg(M_WARN, "WARNING: Unknown/unexpected value in deferred" +msg(M_WARN, "WARNING: Unknown/unexpected value in deferred " "client-connect resultfile"); ret = CC_RET_FAILED; } @@ -2427,7 +2427,7 @@ */ if (!mi->context.c2.push_ifconfig_defined) { -msg(D_MULTI_ERRORS, "MULTI: no dynamic or static remote" +msg(D_MULTI_ERRORS, "MULTI: no dynamic or static remote " "--ifconfig address is available for %s", multi_instance_string(mi, false, )); } @@ -2443,7 +2443,7 @@ print_in_addr_t(mi->context.options.push_ifconfig_constraint_netmask, 0, ); /* JYFIXME -- this should cause the connection to fail */ -msg(D_MULTI_ERRORS, "MULTI ERROR: primary virtual IP for %s (%s)" +msg(D_MULTI_ERRORS, "MULTI ERROR: primary virtual IP for %s (%s) " "violates tunnel network/netmask constraint (%s/%s)", multi_instance_string(mi, false, ), print_in_addr_t(mi->context.c2.push_ifconfig_local, 0, ), @@ -2492,7 +2492,7 @@ } else if (mi->context.options.iroutes) { -msg(D_MULTI_ERRORS, "MULTI: --iroute options rejected for %s -- iroute" +msg(D_MULTI_ERRORS, "MULTI: --iroute options rejected for %s -- iroute " "only works with tun-style tunnels", multi_instance_string(mi, false, )); } diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 64e67aa..ba9b05e 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -7033,7 +7033,7 @@ } else if (streq(p[0], "max-routes") && !p[2]) { -msg(M_WARN, "DEPRECATED OPTION: --max-routes option ignored." +msg(M_WARN, "DEPRECATED OPTION: --max-routes option ignored. " "The number of routes is unlimited as of OpenVPN 2.4. " "This option will be removed in a future version, " "please remove it from your configuration."); @@ -9328,7 +9328,7 @@ s++; } msg(M_WARN, "DEPRECATED FEATURE: automatically upcased the " -"--x509-username-field parameter to '%s'; please update your" +"--x509-username-field parameter to '%s'; please update your " "configuration", p[j]); } } diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c index 934ff8d..0b0e2c3 100644 --- a
[Openvpn-devel] [PATCH applied] Re: configure: Switch to C11 by default
As discussed before, this breaks OpenBSD 6.8 builds (and, generally, other systems with very old compilers). Not sure the benefit of getting rid of a few hoops for anonymous unions is worth it, but then, it's not like people on those systems couldn't just update... Your patch has been applied to the master branch. commit 37b696a207548df88fe65aa130fe6d522e7ce920 (master) Author: Frank Lichtenheld Date: Wed Jul 10 18:03:06 2024 +0200 configure: Switch to C11 by default Signed-off-by: Frank Lichtenheld Acked-by: Arne Schwabe Message-Id: <20240710160306.190351-1-fr...@lichtenheld.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28916.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Allow trailing \r and \n in control channel message
Stared at code, reviewed, and fed to my test case where the server replies with "AUTH_FAIL,you stink\r\n" - which wasn't working with the previous code (and was overlooked during testing), now works. 2024-07-17 20:54:32 AUTH: Received control message: AUTH_FAILED,you stink Also, unit tests, yay :-) Your patch has been applied to the master and release/2.6 branch (backport will be applied to 2.5). commit be31325e1dfdffbb152374985c2ae7b6644e3519 (master) commit 343573990135023d855d151fcd9248e5c26d9f8b (release/2.6) Author: Arne Schwabe Date: Wed Jul 10 16:06:23 2024 +0200 Allow trailing \r and \n in control channel message Signed-off-by: Arne Schwabe Acked-by: Frank Lichtenheld Message-Id: <20240710140623.172829-1-fr...@lichtenheld.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28910.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Allow trailing \r and \n in control channel message
Acked-by: Gert Doering Thanks for the backport. Verified that it's the same change (without the unit test, and having extract_command_buffer() in forward.c), and that it gets the job done :-) Your patch has been applied to the release/2.5 branch. commit dddb87f126a6e87e61de830a9efe0bc257a71e2b (release/2.5) Author: Arne Schwabe Date: Thu Jul 11 13:30:22 2024 +0200 Allow trailing \r and \n in control channel message Signed-off-by: Arne Schwabe Signed-off-by: Frank Lichtenheld Acked-by: Gert Doering Message-Id: <2024073022.52076-1-fr...@lichtenheld.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28923.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v2] configure: Switch to C11 by default
Hi, On Wed, Jul 10, 2024 at 06:03:06PM +0200, Frank Lichtenheld wrote: > Mostly so we can use anonymous structs without jumping through > hoops or relying on unofficial support. > > Change-Id: I72934e747d1ad68a7e3675afbeb1b63df7941186 > Signed-off-by: Frank Lichtenheld > Acked-by: Arne Schwabe > --- > > This change was reviewed on Gerrit and approved by at least one > developer. I request to merge it to master. This breaks all OpenBSD 6.8 builds. Are you intentionally ignoring this ("this OS is out of support, we'll discontinue this buildbot"? If yes, we might want to point out that this change has fallout on older "build environments" - not sure what is problematic here, C compiler or system headers, or libc... gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany g...@greenie.muc.de signature.asc Description: PGP signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: t_server_null: multiple improvements and fixes
Thanks for the good work on this - now all the more exotic platforms (with stubborn sysadmins) pass "the basic tests", and we can move onwards to more creative setups, and towards better diagnostic output. Tested on the buildbots (via gerrit). Your patch has been applied to the master branch. commit f8f477139804b06183b515a529c982f524547d18 Author: Samuli Seppänen Date: Thu Jul 4 15:33:36 2024 +0200 t_server_null: multiple improvements and fixes Signed-off-by: Samuli Seppänen Acked-by: Frank Lichtenheld Message-Id: <2024070417.26595-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28871.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v5] t_server_null: multiple improvements and fixes
From: Samuli Seppänen - exit after a timeout if unable to kill servers - use sudo or equivalent only for server stop/start - use /bin/sh directly instead of through /usr/bin/env - simplify sudo call in the sample rc file - remove misleading and outdated documentation - make it work on OpenBSD 7.5 - make it work on NetBSD 10.0 - make server logs readable by normal users Change-Id: I2cce8ad4e0d262e1404ab1eb6ff673d8590b6b3a Signed-off-by: Samuli Seppänen Acked-by: Frank Lichtenheld --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/669 This mail reflects revision 5 of this Change. Acked-by according to Gerrit (reflected above): Frank Lichtenheld diff --git a/doc/t_server_null.rst b/doc/t_server_null.rst index e3a098a..5fe9080 100644 --- a/doc/t_server_null.rst +++ b/doc/t_server_null.rst @@ -43,6 +43,12 @@ * run as root * a privilege escalation tool (sudo, doas, su) and the permission to become root +If you use "doas" you should enable nopass feature in */etc/doas.conf*. For +example to allow users in the *wheel* group to run commands without a password +prompt:: + +permit nopass keepenv :wheel + Technical implementation @@ -73,13 +79,6 @@ * Waits until servers have launched. Then launch all clients, wait for them to exit and then check test results by parsing the client log files. Each client kills itself after some delay using an "--up" script. -Note that "make check" moves on once *t_server_null_client.sh* has exited. At -that point *t_server_null_server.sh* is still running, because it exists only -after waiting a few seconds for more client connections to potentially appear. -This is a feature and not a bug, but means that launching "make check" runs too -quickly might cause test failures or unexpected behavior such as leftover -OpenVPN server processes. - Configuration - diff --git a/tests/t_server_null.rc-sample b/tests/t_server_null.rc-sample index 28c3773..98d7869 100644 --- a/tests/t_server_null.rc-sample +++ b/tests/t_server_null.rc-sample @@ -1,6 +1,5 @@ # Uncomment to run tests with sudo -#SUDO_EXEC=`which sudo` -#RUN_SUDO="${SUDO_EXEC} -E" +#RUN_SUDO="sudo -E" TEST_RUN_LIST="1 2 3 10 11" diff --git a/tests/t_server_null.sh b/tests/t_server_null.sh index 0e53ba4..7627edf 100755 --- a/tests/t_server_null.sh +++ b/tests/t_server_null.sh @@ -1,4 +1,4 @@ -#!/usr/bin/env sh +#!/bin/sh # TSERVER_NULL_SKIP_RC="${TSERVER_NULL_SKIP_RC:-77}" @@ -57,12 +57,7 @@ srcdir="${srcdir:-.}" -if [ -z "${RUN_SUDO}" ]; then -"${srcdir}/t_server_null_server.sh" & -else -$RUN_SUDO "${srcdir}/t_server_null_server.sh" & -fi - +"${srcdir}/t_server_null_server.sh" & "${srcdir}/t_server_null_client.sh" retval=$? diff --git a/tests/t_server_null_client.sh b/tests/t_server_null_client.sh index 8890007..e7dd332 100755 --- a/tests/t_server_null_client.sh +++ b/tests/t_server_null_client.sh @@ -1,4 +1,4 @@ -#!/usr/bin/env sh +#!/bin/sh launch_client() { test_name=$1 @@ -76,19 +76,22 @@ count=0 server_max_wait=15 while [ $count -lt $server_max_wait ]; do -server_pids="" -server_count=$(set|grep 'SERVER_NAME_'|wc -l) +servers_up=0 +server_count=$(echo $TEST_SERVER_LIST|wc -w) # We need to trim single-quotes because some shells return quoted values # and some don't. Using "set -o posix" which would resolve this problem is # not supported in all shells. +# +# While inactive server configurations may get checked they won't increase +# the active server count as the processes won't be running. for i in `set|grep 'SERVER_NAME_'|cut -d "=" -f 2|tr -d "[\']"`; do server_pid=$(cat $i.pid 2> /dev/null) -server_pids="${server_pids} ${server_pid}" +if ps -p $server_pid > /dev/null 2>&1; then +servers_up=$(( $servers_up + 1 )) +fi done -servers_up=$(ps -p $server_pids 2>/dev/null|sed '1d'|wc -l) - echo "OpenVPN test servers up: ${servers_up}/${server_count}" if [ $servers_up -ge $server_count ]; then @@ -101,6 +104,7 @@ if [ $count -eq $server_max_wait ]; then retval=1 +exit $retval fi done diff --git a/tests/t_server_null_default.rc b/tests/t_server_null_default.rc index 63b6bcd..825bb52 100755 --- a/tests/t_server_null_default.rc +++ b/tests/t_server_null_default.rc @@ -24,7 +24,7 @@ MAX_CLIENTS="10" CLIENT_MATCH="Test-Client" SERVER_EXEC="${top_builddir}/src/openvpn/openvpn" -SERVER_BASE_OPTS="--daemon --local 127.0.0.1 --dev tun --topology subnet --server 10.29.41.0 255.255.255.0 --max-clients $MAX_CLIENTS --persist-tun --verb 3" +SERVER_BASE_OPTS="--daemon --local 127.0.0.1 --dev tun --topology subnet --max-clients $MAX_CLIENTS --persist-tun --verb 3" SERVER_CIPHER_OPTS="" SERVER_CERT_OPTS="--ca ${CA} --dh ${DH} --cert ${SERVER_CERT} --key
[Openvpn-devel] [PATCH applied] Re: mbedtls: Warn if --tls-version-min is too low
Thanks for that. This fixes my server test rig, which sets --tls-version-min to accept connections from very old clients - it will now (still) fail old clients that can not do TLS 1.2 (namely, OpenVPN 2.2(!) - 2.3 and up are fine), but it will not fail "everything else" as the current code did. Your patch has been applied to the master branch. commit c535fa7afe45937bbc7dda435b2b05e57f7ecd53 (master) Author: Max Fillinger Date: Wed Jul 3 19:41:58 2024 +0200 mbedtls: Warn if --tls-version-min is too low Signed-off-by: Max Fillinger Acked-by: Arne Schwabe Message-Id: <20240703174158.7137-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28865.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v1] mbedtls: Warn if --tls-version-min is too low
From: Max Fillinger Recent versions of mbedtls only support TLS 1.2. When the minimum version is set to TLS 1.0 or 1.1, log a warning and use 1.2 as the actual minimum version. Change-Id: Ibc641388d8016533c94dfef3618376f6dfa91f4e Signed-off-by: Max Fillinger Acked-by: Arne Schwabe --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/684 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Arne Schwabe diff --git a/src/openvpn/options.c b/src/openvpn/options.c index dbe1425..64e67aa 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -8942,6 +8942,15 @@ msg(msglevel, "unknown tls-version-min parameter: %s", p[1]); goto err; } + +#ifdef ENABLE_CRYPTO_MBEDTLS +if (ver < TLS_VER_1_2) +{ +msg(M_WARN, "--tls-version-min %s is not supported by mbedtls, using 1.2", p[1]); +ver = TLS_VER_1_2; +} +#endif + options->ssl_flags &= ~(SSLF_TLS_VERSION_MIN_MASK << SSLF_TLS_VERSION_MIN_SHIFT); options->ssl_flags |= (ver << SSLF_TLS_VERSION_MIN_SHIFT); ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: configure: Try to detect LZO with pkg-config
Detailed test reports are in the gerrit notes... so not repeating them here :-) - but finally, we have something that just detects LZO "wherever it is", as long as a pkg-config entry is there - the need to have LZO_CFLAGS/LZO_LIBS on all the *BSDs has irked me since 2.1 or so... (thanks!) Sanity-tested the 2.6 branch via GHA. Your patch has been applied to the master and release/2.6 branch (no real code change, maintenance). commit 0ea51261d096b54281287bbd2a6899041c4dbd43 (master) commit 3c43b016e9767df74909ea5644399e8872e38c97 (release/2.6) Author: Frank Lichtenheld Date: Wed Jun 26 18:19:21 2024 +0200 configure: Try to detect LZO with pkg-config Signed-off-by: Frank Lichtenheld Acked-by: Gert Doering Message-Id: <20240626161921.179301-1-fr...@lichtenheld.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28848.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Http-proxy: fix bug preventing proxy credentials caching
The original ACK came from Frank, I'm just ACKing the change v8 -> v10 (rebased, and add (void) to function declaration / prototypes where missing). I do not have a test setup to properly test this right now - I do have proxies, but they do not need auth, and especially the "cache on reconnect" needs a different client-side driving framework to verify client behaviour (volunteers for a t_client.sh-alike using the management interface?). But I am told that Frank and Gianmarco have tested this very extensively, and it does not break any of the things I *can* test (basic auth-user-pass and token). So in it goes :-) Your patch has been applied to the master and release/2.6 branch (bugfix). commit 3cfd6f961d5c92bec283ac3616e1633b4e16760c (master) commit ad0c2c078ea505436b19255ebfbc8365044c5953 (release/2.6) Author: Gianmarco De Gregori Date: Sun Jun 23 22:05:51 2024 +0200 Http-proxy: fix bug preventing proxy credentials caching Signed-off-by: Gianmarco De Gregori Acked-by: Gert Doering Acked-by: Frank Lichtenheld Message-Id: <20240623200551.20092-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28835.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v10] Http-proxy: fix bug preventing proxy credentials caching
From: Gianmarco De Gregori Caching proxy credentials was not working due to the lack of handling already defined creds in get_user_pass(), which prevented the caching from working properly. Fix this issue by getting the value of c->first_time, that indicates if we're at the first iteration of the main loop and use it as second argument of the get_user_pass_http(). Otherwise, on SIGUSR1 or SIGHUP upon instance context restart credentials would be erased every time. The nocache member has been added to the struct http_proxy_options and also a getter method to retrieve that option from ssl has been added, by doing this we're able to erase previous queried user credentials to ensure correct operation. Fixes: Trac #1187 Signed-off-by: Gianmarco De Gregori Acked-by: Gert Doering Change-Id: Ia3e06c0832c4ca0ab868c845279fb71c01a1a78a --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/523 This mail reflects revision 10 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering diff --git a/doc/man-sections/generic-options.rst b/doc/man-sections/generic-options.rst index eb9cf28..ba9376b 100644 --- a/doc/man-sections/generic-options.rst +++ b/doc/man-sections/generic-options.rst @@ -19,9 +19,6 @@ When using ``--auth-nocache`` in combination with a user/password file and ``--chroot`` or ``--daemon``, make sure to use an absolute path. - This directive does not affect the ``--http-proxy`` username/password. - It is always cached. - --cd dir Change directory to ``dir`` prior to reading any files such as configuration files, key files, scripts, etc. ``dir`` should be an diff --git a/src/openvpn/init.c b/src/openvpn/init.c index b081b2f..a49e563 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -691,6 +691,8 @@ if (c->options.ce.http_proxy_options) { +c->options.ce.http_proxy_options->first_time = c->first_time; + /* Possible HTTP proxy user/pass input */ c->c1.http_proxy = http_proxy_new(c->options.ce.http_proxy_options); if (c->c1.http_proxy) diff --git a/src/openvpn/options.c b/src/openvpn/options.c index f2c7536..dbe1425 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -1650,6 +1650,7 @@ SHOW_STR(auth_file); SHOW_STR(auth_file_up); SHOW_BOOL(inline_creds); +SHOW_BOOL(nocache); SHOW_STR(http_version); SHOW_STR(user_agent); for (i = 0; i < MAX_CUSTOM_HTTP_HEADER && o->custom_headers[i].name; i++) @@ -3151,6 +3152,11 @@ ce->flags |= CE_DISABLED; } +if (ce->http_proxy_options) +{ +ce->http_proxy_options->nocache = ssl_get_auth_nocache(); +} + /* our socks code is not fully IPv6 enabled yet (TCP works, UDP not) * so fall back to IPv4-only (trac #1221) */ diff --git a/src/openvpn/proxy.c b/src/openvpn/proxy.c index ba3d87c..5de0da4 100644 --- a/src/openvpn/proxy.c +++ b/src/openvpn/proxy.c @@ -276,7 +276,7 @@ { auth_file = p->options.auth_file_up; } -if (p->queried_creds) +if (p->queried_creds && !static_proxy_user_pass.nocache) { flags |= GET_USER_PASS_PREVIOUS_CREDS_FAILED; } @@ -288,9 +288,14 @@ auth_file, UP_TYPE_PROXY, flags); -p->queried_creds = true; -p->up = static_proxy_user_pass; +static_proxy_user_pass.nocache = p->options.nocache; } + +/* + * Using cached credentials + */ +p->queried_creds = true; +p->up = static_proxy_user_pass; } #if 0 @@ -542,7 +547,7 @@ * we know whether we need any. */ if (p->auth_method == HTTP_AUTH_BASIC || p->auth_method == HTTP_AUTH_NTLM2) { -get_user_pass_http(p, true); +get_user_pass_http(p, p->options.first_time); } #if !NTLM @@ -656,6 +661,11 @@ || p->auth_method == HTTP_AUTH_NTLM2) { get_user_pass_http(p, false); + +if (p->up.nocache) +{ +clear_user_pass_http(); +} } /* are we being called again after getting the digest server nonce in the previous transaction? */ @@ -1036,13 +1046,6 @@ } goto error; } - -/* clear state */ -if (p->options.auth_retry) -{ -clear_user_pass_http(); -} -store_proxy_authenticate(p, NULL); } /* check return code, success = 200 */ diff --git a/src/openvpn/proxy.h b/src/openvpn/proxy.h index a502c9d..d9e598c 100644 --- a/src/openvpn/proxy.h +++ b/src/openvpn/proxy.h @@ -57,6 +57,8 @@ const char *user_agent; struct http_custom_header custom_headers[MAX_CUSTOM_HTTP_HEADER]; bool inline_creds; /* auth_file_up is
[Openvpn-devel] [PATCH applied] Re: configure: Add -Wstrict-prototypes and -Wold-style-definition
"Because it makes sense" - we try to keep our style consistent and clean, and this will ensure we'll notice new commits with func() instead of func(void) prototypes. Plus fixing the inconsistencies we already had. The buildbots and GH say "this works on all supported platforms", so in it goes. Your patch has been applied to the master branch. commit 56355924b4945ec808500b18c714c111387697f9 Author: Frank Lichtenheld Date: Thu Jun 20 16:42:30 2024 +0200 configure: Add -Wstrict-prototypes and -Wold-style-definition Signed-off-by: Frank Lichtenheld Acked-by: Gert Doering Message-Id: <20240620144230.19586-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28823.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v4] configure: Add -Wstrict-prototypes and -Wold-style-definition
From: Frank Lichtenheld These are not covered by -Wall (nor -Wextra) but we want to enforce them. Change-Id: I6e08920e4cf4762b9f14a7461a29fa77df15255c Signed-off-by: Frank Lichtenheld Acked-by: Gert Doering --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/667 This mail reflects revision 4 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering diff --git a/configure.ac b/configure.ac index 2e5ab6a..c01ad09 100644 --- a/configure.ac +++ b/configure.ac @@ -1408,6 +1408,8 @@ ) ACL_CHECK_ADD_COMPILE_FLAGS([-Wno-stringop-truncation]) +ACL_CHECK_ADD_COMPILE_FLAGS([-Wstrict-prototypes]) +ACL_CHECK_ADD_COMPILE_FLAGS([-Wold-style-definition]) ACL_CHECK_ADD_COMPILE_FLAGS([-Wall]) if test "${enable_pedantic}" = "yes"; then diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h index 50ebb35..035474f 100644 --- a/src/openvpn/dco.h +++ b/src/openvpn/dco.h @@ -247,7 +247,7 @@ * * @return list of colon-separated ciphers */ -const char *dco_get_supported_ciphers(); +const char *dco_get_supported_ciphers(void); #else /* if defined(ENABLE_DCO) */ @@ -375,7 +375,7 @@ } static inline const char * -dco_get_supported_ciphers() +dco_get_supported_ciphers(void) { return ""; } diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c index 7c8b29c..9a90f5c 100644 --- a/src/openvpn/dco_freebsd.c +++ b/src/openvpn/dco_freebsd.c @@ -773,7 +773,7 @@ } const char * -dco_get_supported_ciphers() +dco_get_supported_ciphers(void) { return "none:AES-256-GCM:AES-192-GCM:AES-128-GCM:CHACHA20-POLY1305"; } diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c index b2584b9..277cd64 100644 --- a/src/openvpn/dco_linux.c +++ b/src/openvpn/dco_linux.c @@ -1053,7 +1053,7 @@ } const char * -dco_get_supported_ciphers() +dco_get_supported_ciphers(void) { return "AES-128-GCM:AES-256-GCM:AES-192-GCM:CHACHA20-POLY1305"; } diff --git a/src/openvpn/pkcs11.h b/src/openvpn/pkcs11.h index 3caedc0..772fa4e 100644 --- a/src/openvpn/pkcs11.h +++ b/src/openvpn/pkcs11.h @@ -35,7 +35,7 @@ ); void -pkcs11_terminate(); +pkcs11_terminate(void); bool pkcs11_addProvider( @@ -46,10 +46,10 @@ ); int -pkcs11_logout(); +pkcs11_logout(void); int -pkcs11_management_id_count(); +pkcs11_management_id_count(void); bool pkcs11_management_id_get( diff --git a/src/openvpn/sig.c b/src/openvpn/sig.c index cfbd942..8323f0d 100644 --- a/src/openvpn/sig.c +++ b/src/openvpn/sig.c @@ -448,7 +448,7 @@ } void -halt_low_priority_signals() +halt_low_priority_signals(void) { #ifndef _WIN32 struct sigaction sa; diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 2054eb4..17078c9 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -277,7 +277,7 @@ #endif void -enable_auth_user_pass() +enable_auth_user_pass(void) { auth_user_pass_enabled = true; } diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h index 98e59e8..0e2a43f 100644 --- a/src/openvpn/ssl.h +++ b/src/openvpn/ssl.h @@ -381,7 +381,7 @@ void pem_password_setup(const char *auth_file); /* Enables the use of user/password authentication */ -void enable_auth_user_pass(); +void enable_auth_user_pass(void); /* * Setup authentication username and password. If auth_file is given, use the diff --git a/src/openvpn/xkey_helper.c b/src/openvpn/xkey_helper.c index 283c95d..b68fb43 100644 --- a/src/openvpn/xkey_helper.c +++ b/src/openvpn/xkey_helper.c @@ -49,7 +49,7 @@ XKEY_EXTERNAL_SIGN_fn xkey_management_sign; static void -print_openssl_errors() +print_openssl_errors(void) { unsigned long e; while ((e = ERR_get_error())) diff --git a/src/openvpn/xkey_provider.c b/src/openvpn/xkey_provider.c index f5fc956..964d2eb 100644 --- a/src/openvpn/xkey_provider.c +++ b/src/openvpn/xkey_provider.c @@ -155,7 +155,7 @@ keymgmt_import_helper(XKEY_KEYDATA *key, const OSSL_PARAM params[]); static XKEY_KEYDATA * -keydata_new() +keydata_new(void) { xkey_dmsg(D_XKEY, "entry"); diff --git a/tests/unit_tests/openvpn/test_common.h b/tests/unit_tests/openvpn/test_common.h index f219e93..52503c6 100644 --- a/tests/unit_tests/openvpn/test_common.h +++ b/tests/unit_tests/openvpn/test_common.h @@ -33,7 +33,7 @@ * methods */ static inline void -openvpn_unit_test_setup() +openvpn_unit_test_setup(void) { assert_int_equal(setvbuf(stdout, NULL, _IONBF, BUFSIZ), 0); assert_int_equal(setvbuf(stderr, NULL, _IONBF, BUFSIZ), 0); diff --git a/tests/unit_tests/openvpn/test_pkcs11.c b/tests/unit_tests/openvpn/test_pkcs11.c index 84ebb29..6d283a2 100644 --- a/tests/unit_tests/openvpn/test_pkcs11.c +++ b/tests/unit_tests/openvpn/test_pkcs11.c @@ -134,7 +134,7 @@ /* Fill-in certs[] array */ void -init_cert_data() +init_cert_data(void) { struct test_cert certs_local[] = {
[Openvpn-devel] [PATCH v2] configure: Add -Wstrict-prototypes and -Wold-style-definition
From: Frank Lichtenheld These are not covered by -Wall (nor -Wextra) but we want to enforce them. Change-Id: I6e08920e4cf4762b9f14a7461a29fa77df15255c Signed-off-by: Frank Lichtenheld Acked-by: Gert Doering --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/667 This mail reflects revision 2 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering diff --git a/configure.ac b/configure.ac index 2e5ab6a..c01ad09 100644 --- a/configure.ac +++ b/configure.ac @@ -1408,6 +1408,8 @@ ) ACL_CHECK_ADD_COMPILE_FLAGS([-Wno-stringop-truncation]) +ACL_CHECK_ADD_COMPILE_FLAGS([-Wstrict-prototypes]) +ACL_CHECK_ADD_COMPILE_FLAGS([-Wold-style-definition]) ACL_CHECK_ADD_COMPILE_FLAGS([-Wall]) if test "${enable_pedantic}" = "yes"; then diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h index 50ebb35..035474f 100644 --- a/src/openvpn/dco.h +++ b/src/openvpn/dco.h @@ -247,7 +247,7 @@ * * @return list of colon-separated ciphers */ -const char *dco_get_supported_ciphers(); +const char *dco_get_supported_ciphers(void); #else /* if defined(ENABLE_DCO) */ @@ -375,7 +375,7 @@ } static inline const char * -dco_get_supported_ciphers() +dco_get_supported_ciphers(void) { return ""; } diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c index 7c8b29c..9a90f5c 100644 --- a/src/openvpn/dco_freebsd.c +++ b/src/openvpn/dco_freebsd.c @@ -773,7 +773,7 @@ } const char * -dco_get_supported_ciphers() +dco_get_supported_ciphers(void) { return "none:AES-256-GCM:AES-192-GCM:AES-128-GCM:CHACHA20-POLY1305"; } diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c index b2584b9..277cd64 100644 --- a/src/openvpn/dco_linux.c +++ b/src/openvpn/dco_linux.c @@ -1053,7 +1053,7 @@ } const char * -dco_get_supported_ciphers() +dco_get_supported_ciphers(void) { return "AES-128-GCM:AES-256-GCM:AES-192-GCM:CHACHA20-POLY1305"; } diff --git a/src/openvpn/sig.c b/src/openvpn/sig.c index cfbd942..8323f0d 100644 --- a/src/openvpn/sig.c +++ b/src/openvpn/sig.c @@ -448,7 +448,7 @@ } void -halt_low_priority_signals() +halt_low_priority_signals(void) { #ifndef _WIN32 struct sigaction sa; diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 2054eb4..17078c9 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -277,7 +277,7 @@ #endif void -enable_auth_user_pass() +enable_auth_user_pass(void) { auth_user_pass_enabled = true; } diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h index 98e59e8..0e2a43f 100644 --- a/src/openvpn/ssl.h +++ b/src/openvpn/ssl.h @@ -381,7 +381,7 @@ void pem_password_setup(const char *auth_file); /* Enables the use of user/password authentication */ -void enable_auth_user_pass(); +void enable_auth_user_pass(void); /* * Setup authentication username and password. If auth_file is given, use the diff --git a/src/openvpn/xkey_helper.c b/src/openvpn/xkey_helper.c index 283c95d..b68fb43 100644 --- a/src/openvpn/xkey_helper.c +++ b/src/openvpn/xkey_helper.c @@ -49,7 +49,7 @@ XKEY_EXTERNAL_SIGN_fn xkey_management_sign; static void -print_openssl_errors() +print_openssl_errors(void) { unsigned long e; while ((e = ERR_get_error())) diff --git a/src/openvpn/xkey_provider.c b/src/openvpn/xkey_provider.c index f5fc956..964d2eb 100644 --- a/src/openvpn/xkey_provider.c +++ b/src/openvpn/xkey_provider.c @@ -155,7 +155,7 @@ keymgmt_import_helper(XKEY_KEYDATA *key, const OSSL_PARAM params[]); static XKEY_KEYDATA * -keydata_new() +keydata_new(void) { xkey_dmsg(D_XKEY, "entry"); diff --git a/tests/unit_tests/openvpn/test_common.h b/tests/unit_tests/openvpn/test_common.h index f219e93..52503c6 100644 --- a/tests/unit_tests/openvpn/test_common.h +++ b/tests/unit_tests/openvpn/test_common.h @@ -33,7 +33,7 @@ * methods */ static inline void -openvpn_unit_test_setup() +openvpn_unit_test_setup(void) { assert_int_equal(setvbuf(stdout, NULL, _IONBF, BUFSIZ), 0); assert_int_equal(setvbuf(stderr, NULL, _IONBF, BUFSIZ), 0); diff --git a/tests/unit_tests/openvpn/test_provider.c b/tests/unit_tests/openvpn/test_provider.c index 934b2d3..cfe9ac3 100644 --- a/tests/unit_tests/openvpn/test_provider.c +++ b/tests/unit_tests/openvpn/test_provider.c @@ -119,7 +119,7 @@ } static void -init_test() +init_test(void) { openvpn_unit_test_setup(); prov[0] = OSSL_PROVIDER_load(NULL, "default"); @@ -135,7 +135,7 @@ } static void -uninit_test() +uninit_test(void) { for (size_t i = 0; i < _countof(prov); i++) { diff --git a/tests/unit_tests/openvpn/test_ssl.c b/tests/unit_tests/openvpn/test_ssl.c index a9a3137..5da5b1c 100644 --- a/tests/unit_tests/openvpn/test_ssl.c +++ b/tests/unit_tests/openvpn/test_ssl.c @@ -81,7 +81,7 @@ "-EN
[Openvpn-devel] [PATCH applied] Re: t_server_null.sh: Fix failure case
Yeah, obvious in hindsight :-) Your patch has been applied to the master branch. commit c9f29e35cd475f18c34aa96eb5fad452210404f9 Author: Frank Lichtenheld Date: Thu Jun 20 12:37:48 2024 +0200 t_server_null.sh: Fix failure case Signed-off-by: Frank Lichtenheld Acked-by: Samuli Seppänen Message-Id: <20240620103749.7923-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28815.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v1] t_server_null.sh: Fix failure case
From: Frank Lichtenheld The changes for POSIX shell compatibility and parallel make compatibility broke actually failing the test when a subtest fails. Change-Id: I35f7cf84e035bc793d6f0f59e46edf1a2efe0391 Signed-off-by: Frank Lichtenheld Acked-by: Samuli Seppänen --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/668 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Samuli Seppänen diff --git a/tests/t_server_null.sh b/tests/t_server_null.sh index cfca5ee..0e53ba4 100755 --- a/tests/t_server_null.sh +++ b/tests/t_server_null.sh @@ -64,9 +64,12 @@ fi "${srcdir}/t_server_null_client.sh" +retval=$? # When running make jobs in parallel ("make -j check") we need to ensure # that this script does not exit before all --dev null servers are dead and # their network interfaces are gone. Otherwise t_client.sh will fail because # pre and post ifconfig output does not match. wait + +exit $retval diff --git a/tests/t_server_null_client.sh b/tests/t_server_null_client.sh index 5d5542b..8890007 100755 --- a/tests/t_server_null_client.sh +++ b/tests/t_server_null_client.sh @@ -130,7 +130,7 @@ eval test_name=\"\$TEST_NAME_$SUF\" eval should_pass=\"\$SHOULD_PASS_$SUF\" -(get_client_test_result "${test_name}" "${should_pass}") +get_client_test_result "${test_name}" "${should_pass}" done exit $retval ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: interactive.c: Improve access control for gui<->service pipe
Indeed, the patch differs only in a single line, related to the snprintf() cleanup (which is only in master). Thanks, Lev, and Selva. Stared at code differences (fine) and test compiled via GHA and local ubuntu/mingw - all good. Your patch has been applied to the master branch. commit babf312ee0486e50ff1f7db5b544afc72ff7c922 Author: Lev Stipakov Date: Wed Jun 19 17:46:08 2024 +0300 interactive.c: Improve access control for gui<->service pipe Signed-off-by: Lev Stipakov Acked-by: Selva Nair Message-Id: <20240619144629.1718-2-...@openvpn.net> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28808.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: interactive.c: Improve access control for gui<->service pipe
This is another "developed in secrecy on the security@ mailing list" patch, because it has security implications. It affects windows builds, where it is possible to have two different processes provide a pipe with the same name (e!), and a connecting client will might not end up at the interactive service but at "some random process". This is not a major issue in itself, but the GUI sends a "user credentials token" (so openvpn.exe can be run as "normal user" later on) and this can be abused by a malicious process to get access to the user running openvpn-gui.exe - now, it's a somewhat theoretical attack (malicious software having sufficient privileges to do use a user token, but not having either "that user access" or "system privs" to start with) - but it's worth fixing. So, just stay calm, don't panic, and upgrade to 2.6.11 ;-) I have not tested this beyond "does it compile?" on a local ubuntu/mingw build and on GHA. Lev, Selva and Heiko did all the grunt work on coming up with a solution and testing the patch. Your patch has been applied to the release/2.6 branch. A rebase to master is in the works (this conflicted with the snprintf() cleanup patch, which is "only in master" and was merged right after *this* was developed and tested). Backport to release/2.5 is not fully straightforward either - there have been a number of fixes to interactive.c, and not all of them have been backported. OTOH, we do not intend to provide 2.5.x windows binaries ever again (and said so at 2.5.10 release), so now is the time to upgrade your windows clients to 2.6.x commit 51301eb6c233c284270e3f4ed0c7f5781f2b5c62 (release/2.6) Author: Lev Stipakov Date: Wed Jun 19 16:44:23 2024 +0300 interactive.c: Improve access control for gui<->service pipe Signed-off-by: Lev Stipakov Acked-by: Selva Nair Message-Id: <20240619134451.222-1-...@openvpn.net> URL: https://www.mail-archive.com/search?l=mid=20240619134451.222-1-...@openvpn.net Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH applied] Re: mbedtls: Remove support for old TLS versions
Hi, On Wed, Jun 19, 2024 at 01:38:46PM +, Maximilian Fillinger wrote: > I *think* I reproduced the problem you're encountering. > > If I put > > setenv opt tls-version-min 1.0 > > in the server config, then *every* connection attempt will trigger a fatal > error in the server. Doesn't matter what TLS versions the client supports. > > If I put that option into the client config, the client will exit with an > error during startup. > > It's not clear to me what the expected behavior is when tls-version-min is an > unsupported version, but if it's an error, it should happen during start-up. I would argue for - we log "minimum supported version is 1.2" and go on or - we log "minimum supported version is 1.2" and exit both is acceptable. It will break people's setups in different ways, though... the first will pretend all is well, and older clients can no longer connect, while the second one will break everything, so making it more obvious. gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany g...@greenie.muc.de signature.asc Description: PGP signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Properly handle null bytes and invalid characters in control messages
I have tested this with lots of well-behaved peers - namely, client against 2.3/2.4/2.5 servers, and (master) server against 2.2-master clients. All works :-) (I did not test with a malicious endpoint). Also, it has unit tests ;-) Your patch has been applied to the master, release/2.6 and release/2.5 branch. The 2.6 pullup pulls in "other unit tests" that have been master-only so far. Which is a bit of an annoyance, but having the UTs is good, they *pass* on 2.6, and it's easier on me than trying to only bring in this new test. On the 2.5 pullup I have left out the unit tests, and had to amend the code lightly to remove the EXIT and AUTH_PENDING message handling. release/2.4 is considered end of maintenance. commit 414f428fa29694090ec4c46b10a8aba419c85659 (master) commit 90e7a858e5594d9a019ad2b4ac6154124986291a (release/2.6) commit d4921ba22f5ae4537d808986743a228617c86328 (release/2.5) Author: Arne Schwabe Date: Mon May 27 15:02:41 2024 +0200 Properly handle null bytes and invalid characters in control messages Signed-off-by: Arne Schwabe Acked-by: Antonio Quartulli Message-Id: <20240619103004.56460-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28791.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3] Properly handle null bytes and invalid characters in control messages
Hi, On Wed, Jun 19, 2024 at 12:30:04PM +0200, Gert Doering wrote: > From: Arne Schwabe > > This makes OpenVPN more picky in accepting control message in two aspects: > - Characters are checked in the whole buffer and not until the first > NUL byte > - if the message contains invalid characters, we no longer continue > evaluating a fixed up version of the message but rather stop > processing it completely. [..] > CVE: 2024-5594 So, for the record - this patch was discussed and reviewed "in private" on the secur...@openvpn.net list, because it was seen as having security implications. 2.6.11 release will happen today or tomorrow, so I'm posting this patch to the public list now for transparency, and will proceed with testing and merging. The security impact is fairly moderate - namely, a malicious peer can send (hand crafted) control channel messages that mess up logging(!) on the other side. No breach of crypto, leak of information, or remote code execution. But still an annoyance to be fixed and properly documented. Thanks, Reynir, for reading our sources with so much passion for details :-) gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany g...@greenie.muc.de signature.asc Description: PGP signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v3] Properly handle null bytes and invalid characters in control messages
From: Arne Schwabe This makes OpenVPN more picky in accepting control message in two aspects: - Characters are checked in the whole buffer and not until the first NUL byte - if the message contains invalid characters, we no longer continue evaluating a fixed up version of the message but rather stop processing it completely. Previously it was possible to get invalid characters to end up in log files or on a terminal. This also prepares the logic a bit in the direction of having a proper framing of control messages separated by null bytes instead of relying on the TLS framing for that. All OpenVPN implementations write the 0 bytes between control commands. This patch also include several improvement suggestion from Reynir (thanks!). CVE: 2024-5594 Reported-By: Reynir Bj�rnsson Change-Id: I0d926f910637dabc89bf5fa919dc6beef1eb46d9 Signed-off-by: Arne Schwabe Acked-by: Antonio Quartulli --- src/openvpn/buffer.c | 17 src/openvpn/buffer.h | 12 +++ src/openvpn/forward.c | 123 - tests/unit_tests/openvpn/test_buffer.c | 24 + 4 files changed, 132 insertions(+), 44 deletions(-) diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c index 3a8069c56..abe6a9c89 100644 --- a/src/openvpn/buffer.c +++ b/src/openvpn/buffer.c @@ -1087,6 +1087,23 @@ string_mod(char *str, const unsigned int inclusive, const unsigned int exclusive return ret; } +bool +string_check_buf(struct buffer *buf, const unsigned int inclusive, const unsigned int exclusive) +{ +ASSERT(buf); + +for (int i = 0; i < BLEN(buf); i++) +{ +char c = BSTR(buf)[i]; + +if (!char_inc_exc(c, inclusive, exclusive)) +{ +return false; +} +} +return true; +} + const char * string_mod_const(const char *str, const unsigned int inclusive, diff --git a/src/openvpn/buffer.h b/src/openvpn/buffer.h index 27c319952..8a40010bc 100644 --- a/src/openvpn/buffer.h +++ b/src/openvpn/buffer.h @@ -943,6 +943,18 @@ bool string_class(const char *str, const unsigned int inclusive, const unsigned */ bool string_mod(char *str, const unsigned int inclusive, const unsigned int exclusive, const char replace); + +/** + * Check a buffer if it only consists of allowed characters. + * + * @param buf The buffer to be checked. + * @param inclusive The character classes that are allowed. + * @param exclusive Character classes that are not allowed even if they are also in inclusive. + * @return True if the string consists only of allowed characters, false otherwise. + */ +bool +string_check_buf(struct buffer *buf, const unsigned int inclusive, const unsigned int exclusive); + /** * Returns a copy of a string with certain classes of characters of it replaced with a specified * character. diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index 01165b2ed..ef35bc619 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -230,6 +230,51 @@ check_tls(struct context *c) } } +static void +parse_incoming_control_channel_command(struct context *c, struct buffer *buf) +{ +if (buf_string_match_head_str(buf, "AUTH_FAILED")) +{ +receive_auth_failed(c, buf); +} +else if (buf_string_match_head_str(buf, "PUSH_")) +{ +incoming_push_message(c, buf); +} +else if (buf_string_match_head_str(buf, "RESTART")) +{ +server_pushed_signal(c, buf, true, 7); +} +else if (buf_string_match_head_str(buf, "HALT")) +{ +server_pushed_signal(c, buf, false, 4); +} +else if (buf_string_match_head_str(buf, "INFO_PRE")) +{ +server_pushed_info(c, buf, 8); +} +else if (buf_string_match_head_str(buf, "INFO")) +{ +server_pushed_info(c, buf, 4); +} +else if (buf_string_match_head_str(buf, "CR_RESPONSE")) +{ +receive_cr_response(c, buf); +} +else if (buf_string_match_head_str(buf, "AUTH_PENDING")) +{ +receive_auth_pending(c, buf); +} +else if (buf_string_match_head_str(buf, "EXIT")) +{ +receive_exit_message(c); +} +else +{ +msg(D_PUSH_ERRORS, "WARNING: Received unknown control message: %s", BSTR(buf)); +} +} + /* * Handle incoming configuration * messages on the control channel. @@ -245,51 +290,41 @@ check_incoming_control_channel(struct context *c) struct buffer buf = alloc_buf_gc(len, ); if (tls_rec_payload(c->c2.tls_multi, )) { -/* force null termination of message */ -buf_null_terminate(); - -/* enforce character class restrictions */ -string_mod(BSTR(), CC_PRINT, CC_CRLF, 0); -if (buf_string_match_head_str(, "AUTH_FAILED")) +while (BLEN() > 1) { -receive_auth_failed(c, ); -} -else if (buf_string_match_head_str(, "PUSH_")) -{ -incoming_push_message(c, ); -} -
[Openvpn-devel] [PATCH applied] Re: Implement server_poll_timeout for socks
Took me long enough, but now it's in :-) - thanks, and thanks ValdikSS for reporting test success. I've run this on the server side test bed (which will not excercise SOCKS paths, but to verify that "nothing unrelated got hit") and on the client, with a few SOCKs proxy tests. These are all "fast proxies" so do not excercise the new timeout handling - and I did not find time to build something with dummynet to make it actually test on a "really slow proxy". Testing using an unreachable IP didn't yield proper results (because the TCP connect is still goverened by the "base" timeout), so you need something which answers TCP/1080, and then "is slow"... Testing using a "dead" netcat 'socks server' ("nc -k -l 1080") leads to - old code: give up after 5 seconds ("TCP port read timeout"), always - new code: respect --connect-timeout (shorter or longer than 5s) Your patch has been applied to the master and release/2.6 branch (I consider it a bugfix, and while the code touches a few places, it only really changes very localized socks.c code). commit b3a68b85a729628ca8b97f9f0c2813f795289cfc (master) commit 94bfb712366ece1ca3605d18e99580f482f0232b (release/2.6) Author: 5andr0 Date: Fri Mar 15 17:20:11 2024 +0100 Implement server_poll_timeout for socks Signed-off-by: 5andr0 Acked-by: Frank Lichtenheld Message-Id: <20240315162011.1661139-1-fr...@lichtenheld.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28408.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH applied] Re: mbedtls: Remove support for old TLS versions
Hi, this breaks *all* client connects on my server testbed. No matter if 2.2 or 2.5 client, when building with mbedtls (2.28.7), the resulting binary refuses ALL incoming connection with Jun 19 10:21:44 gentoo tap-udp-p2mp[1723]: 2001:608:0:814::f000:16 tls_version_to_ssl_version: invalid or unsupported TLS version 1 Jun 19 10:21:44 gentoo tap-tcp-p2p[1770]: tls_version_to_ssl_version: invalid or unsupported TLS version 1 Jun 19 10:21:59 gentoo tun-tcp-p2mp[1708]: tls_version_to_ssl_version: invalid or unsupported TLS version 1 Jun 19 10:22:32 gentoo tun-udp-p2mp[1713]: 194.97.140.21:49229 tls_version_to_ssl_version: invalid or unsupported TLS version 2 Jun 19 10:23:05 gentoo tun-udp-p2mp-topology-subnet[1718]: 194.97.140.21:45789 tls_version_to_ssl_version: invalid or unsupported TLS version 1 Jun 19 10:24:11 gentoo tun-udp-p2mp-fragment[1746]: 194.97.140.21:14517 tls_version_to_ssl_version: invalid or unsupported TLS version 1 Jun 19 10:44:49 gentoo tun-udp-p2mp-112-mask[1741]: 194.97.140.21:42810 tls_version_to_ssl_version: invalid or unsupported TLS version 1 so my guess would be that on mbedTLS builds that *do* support 1.1/1.2, incoming client connects with 1.1/1.2 cause "something to get upset" in the TLS version printer. Sorry for not testing this more thoroughly before merging. gert On Tue, Jun 18, 2024 at 06:30:05PM +0200, Gert Doering wrote: > Mildly tested via GHA builds. > > Not sure we want this in release/2.6 - I tend to "not", because it might > break someone's (non-recommended) setup... > > Your patch has been applied to the master branch. > > commit 013c119af96bc57c41e04e4a8f64b5d80e2e9ba6 > Author: Max Fillinger > Date: Tue Jun 18 14:02:19 2024 +0200 > > mbedtls: Remove support for old TLS versions > > Signed-off-by: Max Fillinger > Acked-by: Arne Schwabe > Message-Id: <20240618120219.5053-1-g...@greenie.muc.de> > URL: > https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28773.html > Signed-off-by: Gert Doering > > > -- > kind regards, > > Gert Doering > > > > ___ > Openvpn-devel mailing list > Openvpn-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/openvpn-devel > -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany g...@greenie.muc.de signature.asc Description: PGP signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Add t_server_null test suite
I have not tested this yet, just looked at a few details - we've discussed using @SHELL@ to get a known-working shell, which is more likely to succeed than assuming "#!/usr/bin/env" will exist, or that the "sh" it finds will be useful (Solaris, I'm looking at you...). Also, documenting that scripts do not properly get waited for does not make it a feature... But anyway. Let's move onwards, and improve iteratively. Your patch has been applied to the master branch. commit 06c7ce5d1fc3b17e0da731d22002e58b9e2d4994 Author: Samuli Seppänen Date: Thu Jun 13 10:14:22 2024 +0200 Add t_server_null test suite Signed-off-by: Samuli Seppänen Acked-by: Frank Lichtenheld Message-Id: <20240613081422.139493-1-fr...@lichtenheld.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28750.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Remove experimental denotation for --fast-io
Documentation is good :-) - not tested besides a local build ("are all the double quotes still in the right spot"). Your patch has been applied to the master and release/2.6 branch (docs). commit f6ee77d1f6149cf8f8982998aee6d433f58be507 (master) commit d5c4c643f36637987d830494b407f2855c5e3fea (release/2.6) Author: Frank Lichtenheld Date: Tue Jun 18 14:01:56 2024 +0200 Remove experimental denotation for --fast-io Signed-off-by: Frank Lichtenheld Acked-by: Arne Schwabe Message-Id: <20240618120156.4836-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28772.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Fix MBEDTLS_DEPRECATED_REMOVED build errors
Mildly tested via GHA compilation. Not sure what option I need to really trigger an "it would break without that patch" scenario so not really tested that much. Your patch has been applied to the master branch. commit 8eb397de3656402872f9c9584c6f703b87b50762 Author: rein.vanbaaren Date: Tue Jun 18 14:01:26 2024 +0200 Fix MBEDTLS_DEPRECATED_REMOVED build errors Signed-off-by: Max Fillinger Acked-by: Arne Schwabe Message-Id: <20240618120127.4564-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28771.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: mbedtls: Remove support for old TLS versions
Mildly tested via GHA builds. Not sure we want this in release/2.6 - I tend to "not", because it might break someone's (non-recommended) setup... Your patch has been applied to the master branch. commit 013c119af96bc57c41e04e4a8f64b5d80e2e9ba6 Author: Max Fillinger Date: Tue Jun 18 14:02:19 2024 +0200 mbedtls: Remove support for old TLS versions Signed-off-by: Max Fillinger Acked-by: Arne Schwabe Message-Id: <20240618120219.5053-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28773.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2] mbedtls: Remove support for old TLS versions
From: Max Fillinger Recent versions of mbedtls have dropped support for TLS 1.0 and 1.1. Rather than checking which versions are supported, drop support for everything before 1.2. Change-Id: Ia3883a26ac26df6bbb5353fb074a2e0f814737be Signed-off-by: Max Fillinger Acked-by: Arne Schwabe --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/682 This mail reflects revision 2 of this Change. Acked-by according to Gerrit (reflected above): Arne Schwabe diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index a68588e..ec9ec13 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -1040,12 +1040,8 @@ { #if defined(MBEDTLS_SSL_PROTO_TLS1_2) return TLS_VER_1_2; -#elif defined(MBEDTLS_SSL_PROTO_TLS1_1) -return TLS_VER_1_1; -#elif defined(MBEDTLS_SSL_PROTO_TLS1) -return TLS_VER_1_0; #else /* defined(MBEDTLS_SSL_PROTO_TLS1_2) */ -#error "mbedtls is compiled without support for TLS 1.0, 1.1 and 1.2." +#error "mbedtls is compiled without support for TLS 1.2." #endif /* defined(MBEDTLS_SSL_PROTO_TLS1_2) */ } @@ -1067,20 +1063,6 @@ switch (tls_ver) { -#if defined(MBEDTLS_SSL_PROTO_TLS1) -case TLS_VER_1_0: -*major = MBEDTLS_SSL_MAJOR_VERSION_3; -*minor = MBEDTLS_SSL_MINOR_VERSION_1; -break; -#endif - -#if defined(MBEDTLS_SSL_PROTO_TLS1_1) -case TLS_VER_1_1: -*major = MBEDTLS_SSL_MAJOR_VERSION_3; -*minor = MBEDTLS_SSL_MINOR_VERSION_2; -break; -#endif - #if defined(MBEDTLS_SSL_PROTO_TLS1_2) case TLS_VER_1_2: *major = MBEDTLS_SSL_MAJOR_VERSION_3; ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v1] Remove "experimental" denotation for --fast-io
From: Frank Lichtenheld This option is very old (from SVN days) and has been used by Access Server for many years. I don't think it makes sense to claim that it is "experimental" at this point. Change-Id: I913bb70c5e527e78e7cdb43110e23a8944f35a22 Signed-off-by: Frank Lichtenheld Acked-by: Arne Schwabe --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/664 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Arne Schwabe diff --git a/doc/man-sections/generic-options.rst b/doc/man-sections/generic-options.rst index f8a0f48..eb9cf28 100644 --- a/doc/man-sections/generic-options.rst +++ b/doc/man-sections/generic-options.rst @@ -215,7 +215,7 @@ are supported by OpenSSL. --fast-io - (Experimental) Optimize TUN/TAP/UDP I/O writes by avoiding a call to + Optimize TUN/TAP/UDP I/O writes by avoiding a call to poll/epoll/select prior to the write operation. The purpose of such a call would normally be to block until the device or socket is ready to accept the write. Such blocking is unnecessary on some platforms which diff --git a/src/openvpn/options.c b/src/openvpn/options.c index abcde89..f2c7536 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -268,7 +268,7 @@ #if ENABLE_IP_PKTINFO "--multihome : Configure a multi-homed UDP server.\n" #endif -"--fast-io : (experimental) Optimize TUN/TAP/UDP writes.\n" +"--fast-io : Optimize TUN/TAP/UDP writes.\n" "--remap-usr1 s : On SIGUSR1 signals, remap signal (s='SIGHUP' or 'SIGTERM').\n" "--persist-tun : Keep tun/tap device open across SIGUSR1 or --ping-restart.\n" "--persist-remote-ip : Keep remote IP address across SIGUSR1 or --ping-restart.\n" ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v4] Fix MBEDTLS_DEPRECATED_REMOVED build errors
From: rein.vanbaaren This commit allows compiling OpenVPN with recent versions of mbed TLS if MBEDTLS_DEPRECATED_REMOVED is defined. Change-Id: If96c2ebd2af16b18ed34820e8c0531547e2076d9 Signed-off-by: Max Fillinger Acked-by: Arne Schwabe --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/681 This mail reflects revision 4 of this Change. Acked-by according to Gerrit (reflected above): Arne Schwabe diff --git a/src/openvpn/mbedtls_compat.h b/src/openvpn/mbedtls_compat.h index d742b54..8559c2e 100644 --- a/src/openvpn/mbedtls_compat.h +++ b/src/openvpn/mbedtls_compat.h @@ -40,6 +40,7 @@ #include #include #include +#include #include #include #include @@ -51,6 +52,12 @@ #include #endif +#if MBEDTLS_VERSION_NUMBER >= 0x0300 +typedef uint16_t mbedtls_compat_group_id; +#else +typedef mbedtls_ecp_group_id mbedtls_compat_group_id; +#endif + static inline void mbedtls_compat_psa_crypto_init(void) { @@ -64,6 +71,16 @@ #endif /* HAVE_MBEDTLS_PSA_CRYPTO_H && defined(MBEDTLS_PSA_CRYPTO_C) */ } +static inline mbedtls_compat_group_id +mbedtls_compat_get_group_id(const mbedtls_ecp_curve_info *curve_info) +{ +#if MBEDTLS_VERSION_NUMBER >= 0x0300 +return curve_info->tls_id; +#else +return curve_info->grp_id; +#endif +} + /* * In older versions of mbedtls, mbedtls_ctr_drbg_update() did not return an * error code, and it was deprecated in favor of mbedtls_ctr_drbg_update_ret() @@ -124,6 +141,34 @@ } #if MBEDTLS_VERSION_NUMBER < 0x03020100 +typedef enum { +MBEDTLS_SSL_VERSION_UNKNOWN, /*!< Context not in use or version not yet negotiated. */ +MBEDTLS_SSL_VERSION_TLS1_2 = 0x0303, /*!< (D)TLS 1.2 */ +MBEDTLS_SSL_VERSION_TLS1_3 = 0x0304, /*!< (D)TLS 1.3 */ +} mbedtls_ssl_protocol_version; + +static inline void +mbedtls_ssl_conf_min_tls_version(mbedtls_ssl_config *conf, mbedtls_ssl_protocol_version tls_version) +{ +int major = (tls_version >> 8) & 0xff; +int minor = tls_version & 0xff; +mbedtls_ssl_conf_min_version(conf, major, minor); +} + +static inline void +mbedtls_ssl_conf_max_tls_version(mbedtls_ssl_config *conf, mbedtls_ssl_protocol_version tls_version) +{ +int major = (tls_version >> 8) & 0xff; +int minor = tls_version & 0xff; +mbedtls_ssl_conf_max_version(conf, major, minor); +} + +static inline void +mbedtls_ssl_conf_groups(mbedtls_ssl_config *conf, mbedtls_compat_group_id *groups) +{ +mbedtls_ssl_conf_curves(conf, groups); +} + static inline size_t mbedtls_cipher_info_get_block_size(const mbedtls_cipher_info_t *cipher) { diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index ec9ec13..bb88da9 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -402,7 +402,7 @@ /* Get number of groups and allocate an array in ctx */ int groups_count = get_num_elements(groups, ':'); -ALLOC_ARRAY_CLEAR(ctx->groups, mbedtls_ecp_group_id, groups_count + 1) +ALLOC_ARRAY_CLEAR(ctx->groups, mbedtls_compat_group_id, groups_count + 1) /* Parse allowed ciphers, getting IDs */ int i = 0; @@ -419,11 +419,15 @@ } else { -ctx->groups[i] = ci->grp_id; +ctx->groups[i] = mbedtls_compat_get_group_id(ci); i++; } } -ctx->groups[i] = MBEDTLS_ECP_DP_NONE; + +/* Recent mbedtls versions state that the list of groups must be terminated + * with 0. Older versions state that it must be terminated with MBEDTLS_ECP_DP_NONE + * which is also 0, so this works either way. */ +ctx->groups[i] = 0; gc_free(); } @@ -1046,33 +1050,30 @@ } /** - * Convert an OpenVPN tls-version variable to mbed TLS format (i.e. a major and - * minor ssl version number). + * Convert an OpenVPN tls-version variable to mbed TLS format * * @param tls_ver The tls-version variable to convert. - * @param major Returns the TLS major version in mbed TLS format. - * Must be a valid pointer. - * @param minor Returns the TLS minor version in mbed TLS format. - * Must be a valid pointer. + * + * @return Translated mbedTLS SSL version from OpenVPN TLS version. */ -static void -tls_version_to_major_minor(int tls_ver, int *major, int *minor) +mbedtls_ssl_protocol_version +tls_version_to_ssl_version(int tls_ver) { -ASSERT(major); -ASSERT(minor); - switch (tls_ver) { #if defined(MBEDTLS_SSL_PROTO_TLS1_2) case TLS_VER_1_2: -*major = MBEDTLS_SSL_MAJOR_VERSION_3; -*minor = MBEDTLS_SSL_MINOR_VERSION_3; -break; +return MBEDTLS_SSL_VERSION_TLS1_2; +#endif + +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) +case TLS_VER_1_3: +return MBEDTLS_SSL_VERSION_TLS1_3; #endif default: msg(M_FATAL, "%s: invalid or unsupported TLS version %d",
Re: [Openvpn-devel] windows client tests needed
Hi, if you think this is a useful security enhancement, and would like to have it in a "short term" 2.6.x release, we need test results... please! gert On Thu, Jun 06, 2024 at 02:23:33PM +0200, Gert Doering wrote: > Hi, > > we have new code in master that helps with the "TunnelCrack" and > "TunnelVision" attacks, that is, packets intended to go into the > VPN being leaked away by means of a malicious DHCP server (= routing > points outside the tunnel, so packets never hit OpenVPN). > > We used to have > > block-outside-dns > > to prevent Windows from doing DNS lookups "around the VPN" - the main > intent of this was "make sure split DNS works", but a side effect has > also been "avoid DNS leaks". > > Heiko has now extended this code to be able to "block everything not > going into the VPN". To activate this, you need > > redirect-gateway def1 block-local > > in your config ("block-local" is the keyword, but without "def1" you > end up with a split-tunnel and "nothing else is allowed", which is rarely > a really good combination). > > Repeat: if "redirect-gateway block-local" is active, NO packets leave > via LAN/WiFi/... interfaces, except those sourced by the openvpn.exe > process. This is important for maximum privacy, especially if you > roam into a network with an untrusted DHCP server. > > > Now - this code has been merged into "git master", and installers > are here: > >https://github.com/OpenVPN/openvpn-build/actions/runs/9391365526?pr=641 > > (bottom of the page, "Artifacts", .zip files with a .msi inside). > > > I want to have this in 2.6 as well, as it's sort of important for certain > classes of users (and also VPN providers, offering this as a service) - but > I do not feel it has been tested enough yet. > > So: PLEASE test these windows installers, in all 3 variants > > 1. > 2. block-outside-dns > (DNS is blocked, everything else not routed into the VPN tunnel - like > "your local printer" etc - still works) > 3. redirect-gateway def1 block-local > (ONLY VPN works) > > and report back to us. > > gert > > -- > "If was one thing all people took for granted, was conviction that if you > feed honest figures into a computer, honest figures come out. Never doubted > it myself till I met a computer with a sense of humor." > Robert A. Heinlein, The Moon is a Harsh Mistress > > Gert Doering - Munich, Germany g...@greenie.muc.de > ___ > Openvpn-devel mailing list > Openvpn-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/openvpn-devel -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany g...@greenie.muc.de signature.asc Description: PGP signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH applied] Re: Implement Windows CA template match for Crypto-API selector
Hi, On Thu, Jun 06, 2024 at 01:33:24PM +0200, Gert Doering wrote: > As instructed I have removed the "and fallback requested" part > from the comment where "fallback" was removed from the code. > > Your patch has been applied to the master branch. > > commit 13ee7f902f18e27b981f8e440facd2e6515c6c83 (master) ... and to release/2.6, as this functionality is seen as useful, and the change is very non-intrusive. So it will be part of 2.6.11 "release planned in about 2 weeks". commit dfbe11ac1842df400327be22951d0ba373534254 (release/2.6) Author: Heiko Wundram Date: Thu Jun 6 12:34:41 2024 +0200 Implement Windows CA template match for Crypto-API selector Compile-tested via GHA. gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany g...@greenie.muc.de signature.asc Description: PGP signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] windows client tests needed
Hi, On Thu, Jun 06, 2024 at 05:23:31PM +0400, Dmitry Melekhov wrote: > redirect-gateway def1 block-local > > also apply block-outside-dns ? "everything" includes DNS, so, yes. gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany g...@greenie.muc.de signature.asc Description: PGP signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] windows client tests needed
Hi, On Thu, Jun 06, 2024 at 02:23:33PM +0200, Gert Doering wrote: > Now - this code has been merged into "git master", and installers > are here: > >https://github.com/OpenVPN/openvpn-build/actions/runs/9391365526?pr=641 > > (bottom of the page, "Artifacts", .zip files with a .msi inside). ... or (freshly repared) .msi with timestamp directly here: https://build.openvpn.net/downloads/snapshots/github-actions/openvpn2/ (2024060T*-13ee7f90 or younger) The filenames on that page are "build time - commit ID", and the script did not work correctly for a month - so the 9d92221e commit "built today" is actually a 4 week old commit... gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany g...@greenie.muc.de signature.asc Description: PGP signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] windows client tests needed
Hi, we have new code in master that helps with the "TunnelCrack" and "TunnelVision" attacks, that is, packets intended to go into the VPN being leaked away by means of a malicious DHCP server (= routing points outside the tunnel, so packets never hit OpenVPN). We used to have block-outside-dns to prevent Windows from doing DNS lookups "around the VPN" - the main intent of this was "make sure split DNS works", but a side effect has also been "avoid DNS leaks". Heiko has now extended this code to be able to "block everything not going into the VPN". To activate this, you need redirect-gateway def1 block-local in your config ("block-local" is the keyword, but without "def1" you end up with a split-tunnel and "nothing else is allowed", which is rarely a really good combination). Repeat: if "redirect-gateway block-local" is active, NO packets leave via LAN/WiFi/... interfaces, except those sourced by the openvpn.exe process. This is important for maximum privacy, especially if you roam into a network with an untrusted DHCP server. Now - this code has been merged into "git master", and installers are here: https://github.com/OpenVPN/openvpn-build/actions/runs/9391365526?pr=641 (bottom of the page, "Artifacts", .zip files with a .msi inside). I want to have this in 2.6 as well, as it's sort of important for certain classes of users (and also VPN providers, offering this as a service) - but I do not feel it has been tested enough yet. So: PLEASE test these windows installers, in all 3 variants 1. 2. block-outside-dns (DNS is blocked, everything else not routed into the VPN tunnel - like "your local printer" etc - still works) 3. redirect-gateway def1 block-local (ONLY VPN works) and report back to us. gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany g...@greenie.muc.de signature.asc Description: PGP signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Implement Windows CA template match for Crypto-API selector
Thanks, Selva, for the review. I have not tested this beyond "does it make GH actions happy", "does it build on the local mingw setup" (it does) and "does the code look generally safe wrt memory etc" (it does). As instructed I have removed the "and fallback requested" part from the comment where "fallback" was removed from the code. Your patch has been applied to the master branch. commit 13ee7f902f18e27b981f8e440facd2e6515c6c83 (master) Author: Heiko Wundram Date: Thu Jun 6 12:34:41 2024 +0200 Implement Windows CA template match for Crypto-API selector Signed-off-by: Heiko Wundram Signed-off-by: Hannes Domani Acked-by: Selva Nair Message-Id: <20240606103441.26598-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28726.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2] Implement Windows CA template match for Crypto-API selector
From: Heiko Wundram 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: Ia2c3e4c5c83ee1618c43b489dbe811de5351 Signed-off-by: Heiko Wundram Signed-off-by: Hannes Domani --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/621 This mail reflects revision 2 of this Change. Acked-by according to Gerrit (reflected above): diff --git a/doc/man-sections/windows-options.rst b/doc/man-sections/windows-options.rst index e87291f..1955869 100644 --- a/doc/man-sections/windows-options.rst +++ b/doc/man-sections/windows-options.rst @@ -55,6 +55,13 @@ 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 f7e5b67..0cf8e2b 100644 --- a/src/openvpn/cryptoapi.c +++ b/src/openvpn/cryptoapi.c @@ -178,6 +178,87 @@ 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 and fallback requested */ +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, ); + +/* check for V2 extension (Windows 2003+) */ +ext = CertFindExtension(szOID_CERTIFICATE_TEMPLATE, info->cExtension, info->rgExtension); +if (ext) +{ +pvext = decode_object(, X509_CERTIFICATE_TEMPLATE, >Value, 0, ); +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(); +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(); +return true; +} +} +} + +/* no extension found, exit */ +gc_free(); +return false; +} + static const CERT_CONTEXT * find_certificate_in_store(const
[Openvpn-devel] [PATCH applied] Re: Windows: enforce 'block-local' with WFP filters
Acked-by: Gert Doering Lev has tested it and confirms that it works, I have stared long and hard at v5 of the patch, and the diffs v5->v6. The main difference v5->v6 is to make it error clean (add a "= 0") *and* that it also blocks DNS going out of the loopback interface - so if there's magic DNS proxy software using 127.0.0.1:53, the old "block-outside-dns" code would have not blocked it, but this new code *will*. Somewhat unrelated featurette, but same code path, and same intent "avoid VPN leaks of any kind, DNS or data". This is really nice functionality to ward of all "TunnelCrack" or "TunnelVision" style attacks on Windows clients, by extending the existing "redirect-gateway block-local" config option to add WFP firewall filters to really *really* block everything not going into the tunnel. I would have wished for two patches ("one for the renaming, one for the functional changes") but I already postponed this for too long... In addition to Lev's tests I have compile-tested on GHA and a local MinGW build. No surprises there. I intend to apply this to release/2.6 as well as master, but want to have more test coverage first. So I'll send mails around to solicit test coverage as soon as I have a git master snapshot installer to test. Your patch has been applied to the master branch. commit bf887c95e46c6892ac1f68be5559525f8d975530 (master) Author: Heiko Hund Date: Wed Jun 5 14:38:56 2024 +0200 Windows: enforce 'block-local' with WFP filters Signed-off-by: Heiko Hund Acked-by: Lev Stipakov Acked-by: Gert Doering Message-Id: <20240605123856.26267-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/search?l=mid=20240605123856.26267-1-g...@greenie.muc.de Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v6] Windows: enforce 'block-local' with WFP filters
From: Heiko Hund In an attempt to better defend against the TunnelCrack attacks, enforce that no traffic can pass to anything else than the VPN interface when the 'block-local' flags is given with either --redirect-gateway or --redirect-private. Reuse much of the existing --block-outside-dns code, but make it more general, so that it can also block any traffic, not just port 53. Uses the Windows Filtering Platform for enforcement in addition to the routes redirecting the networks into the tunnel. Change-Id: Ic9bf797bfc7e2d471998a84cb0f071db3e4832ba Signed-off-by: Heiko Hund Acked-by: Gert Doering --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/489 This mail reflects revision 6 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering diff --git a/CMakeLists.txt b/CMakeLists.txt index f8b37a9..096837d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -369,8 +369,6 @@ src/openvpn/base64.c src/openvpn/base64.h src/openvpn/basic.h -src/openvpn/block_dns.h -src/openvpn/block_dns.c src/openvpn/buffer.c src/openvpn/buffer.h src/openvpn/circ_list.h @@ -550,6 +548,8 @@ src/openvpn/ssl_util.h src/openvpn/vlan.c src/openvpn/vlan.h +src/openvpn/wfp_block.c +src/openvpn/wfp_block.h src/openvpn/win32.c src/openvpn/win32-util.c src/openvpn/win32.h diff --git a/doc/man-sections/vpn-network-options.rst b/doc/man-sections/vpn-network-options.rst index 98b4971..84d4273 100644 --- a/doc/man-sections/vpn-network-options.rst +++ b/doc/man-sections/vpn-network-options.rst @@ -352,6 +352,10 @@ Block access to local LAN when the tunnel is active, except for the LAN gateway itself. This is accomplished by routing the local LAN (except for the LAN gateway address) into the tunnel. + On Windows WFP filters are added in addition to the routes which + block access to resources not routed through the VPN adapter. + Push this flag to protect against TunnelCrack type of attacks + (see: https://tunnelcrack.mathyvanhoef.com/). :code:`ipv6` Redirect IPv6 routing into the tunnel. This works similar to diff --git a/include/openvpn-msg.h b/include/openvpn-msg.h index 657eb5e..7a99335 100644 --- a/include/openvpn-msg.h +++ b/include/openvpn-msg.h @@ -24,6 +24,9 @@ #ifndef OPENVPN_MSG_H_ #define OPENVPN_MSG_H_ +#include +#include + typedef enum { msg_acknowledgement, msg_add_address, @@ -35,8 +38,8 @@ msg_add_nbt_cfg, msg_del_nbt_cfg, msg_flush_neighbors, -msg_add_block_dns, -msg_del_block_dns, +msg_add_wfp_block, +msg_del_wfp_block, msg_register_dns, msg_enable_dhcp, msg_register_ring_buffers, @@ -61,6 +64,11 @@ char name[256]; } interface_t; +typedef enum { +wfp_block_local = 1<<0, +wfp_block_dns = 1<<1 +} wfp_block_flags_t; + typedef struct { message_header_t header; short family; @@ -120,8 +128,9 @@ typedef struct { message_header_t header; +wfp_block_flags_t flags; interface_t iface; -} block_dns_message_t; +} wfp_block_message_t; typedef struct { message_header_t header; diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am index 7ceec0c..56cce9d 100644 --- a/src/openvpn/Makefile.am +++ b/src/openvpn/Makefile.am @@ -156,6 +156,6 @@ $(OPTIONAL_DL_LIBS) \ $(OPTIONAL_INOTIFY_LIBS) if WIN32 -openvpn_SOURCES += openvpn_win32_resources.rc block_dns.c block_dns.h ring_buffer.h +openvpn_SOURCES += openvpn_win32_resources.rc wfp_block.c wfp_block.h ring_buffer.h openvpn_LDADD += -lgdi32 -lws2_32 -lwininet -lcrypt32 -liphlpapi -lwinmm -lfwpuclnt -lrpcrt4 -lncrypt -lsetupapi -lbcrypt endif diff --git a/src/openvpn/block_dns.c b/src/openvpn/block_dns.c deleted file mode 100644 index 5f4925f..000 --- a/src/openvpn/block_dns.c +++ /dev/null @@ -1,423 +0,0 @@ -/* - * OpenVPN -- An application to securely tunnel IP networks - * over a single TCP/UDP port, with support for SSL/TLS-based - * session authentication and key exchange, - * packet encryption, packet authentication, and - * packet compression. - * - * Copyright (C) 2002-2024 OpenVPN Inc - *2015-2016 - *2016 Selva Nair - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 - * as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; i
[Openvpn-devel] [PATCH applied] Re: test_user_pass: Fix building with --enable-systemd
Your patch has been applied to the master branch (this unit test is not in release/2.6, so nothing to fix there). Not tested myself as I do not have anything with systemd, but it does not break "non-systemd tests", as confirmed by gerrit/all the buildbots. commit 7dfff75659e6c06abe500f5b8716d9712aa41bcc (master) Author: Frank Lichtenheld Date: Wed Jun 5 13:10:12 2024 +0200 test_user_pass: Fix building with --enable-systemd Signed-off-by: Frank Lichtenheld Acked-by: Gert Doering Message-Id: <20240605111012.3023-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28708.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: LZO: do not use lzoutils.h macros
Lightly tested via local builds and GHA. This is basically a no-op, as lzoutils.h contains #define lzo_malloc(a) (malloc(a)) #define lzo_free(a) (free(a)) .. and nothing more complicated. Having lzoutil.h out of the way paves the way for "make lzo2.pc pkg-config checks work" :-) Your patch has been applied to the master and release/2.6 branch (I see this as "long-term compat" for building). commit d601237976323b5d8f6ac65c27ccc510563ad75f (master) commit 1ae753e4240434abda0a33aed07b289fa9c6ee79 (release/2.6) Author: Frank Lichtenheld Date: Tue Jun 4 23:17:08 2024 +0200 LZO: do not use lzoutils.h macros Signed-off-by: Frank Lichtenheld Acked-by: Arne Schwabe Message-Id: <20240604211708.32315-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28705.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v1] test_user_pass: Fix building with --enable-systemd
From: Frank Lichtenheld Need to make sure that ENABLE_SYSTEMD is really disabled. Change-Id: Ic33c210f06e173a450534aa0969c57f140086655 Signed-off-by: Frank Lichtenheld Acked-by: Gert Doering --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/641 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering diff --git a/tests/unit_tests/openvpn/test_user_pass.c b/tests/unit_tests/openvpn/test_user_pass.c index ac84654..b43e655 100644 --- a/tests/unit_tests/openvpn/test_user_pass.c +++ b/tests/unit_tests/openvpn/test_user_pass.c @@ -27,6 +27,8 @@ #endif #undef ENABLE_SYSTEMD +/* avoid redefining ENABLE_SYSTEMD in misc.c */ +#undef HAVE_CONFIG_H #include "syshead.h" #include "manage.h" ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2] LZO: do not use lzoutils.h macros
From: Frank Lichtenheld 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 --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/642 This mail reflects revision 2 of this Change. Acked-by according to Gerrit (reflected above): Arne Schwabe diff --git a/config.h.cmake.in b/config.h.cmake.in index 2cdfdcc..effca2a 100644 --- a/config.h.cmake.in +++ b/config.h.cmake.in @@ -184,9 +184,6 @@ /* 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 d39cf7d..7f0105b 100644 --- a/configure.ac +++ b/configure.ac @@ -1174,15 +1174,6 @@ 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], , [AC_CHECK_HEADERS( diff --git a/src/openvpn/lzo.c b/src/openvpn/lzo.c index b313284..bab2d78 100644 --- a/src/openvpn/lzo.c +++ b/src/openvpn/lzo.c @@ -107,14 +107,14 @@ { 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 e951b49..62d73a1 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) ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Allow to set ifmode for existing DCO interfaces in FreeBSD
Acked-by: Gert Doering Thanks for the patch, and the explanation on IRC. This is FreeBSD/DCO specific, and makes the case ifconfig ovpn3 create openvpn --dev ovpn3 --dev-type tun --topology subnet ... work correctly (without it, p2p ifconfig claims to work but ipv6 route addition fails as well). I have stared at the code (looks good) and tested on my FreeBSD 14/DCO test VM with and without the patch, adding relevant test cases to the t_client.rc setup. With the patch, nothing breaks ;-) and the newly considered cases work. There is a slight catch - if the interface (ovpn3) has already been brought "up", the dco_set_ifmode() call fails with 2024-06-02 14:54:06 dco_set_ifmode: failed to set ifmode=8002: Device busy (errno=16) .. so it will not work properly in all cases. (Also, t_client.sh is not really suited for this sort of test today, as the "compare pre/post ifconfig state" check fails on interface flags - need to think more how to make this work) The patch got badly whitespace mangled in transit (empty lines got turned into extra indent in the following line, and thus). As I said on IRC, I'm happy to fix that for occasional submitters, but generally recommend "git send-email" or pushing to gerrit to avoid outlook induced garbage. Your patch has been applied to the master and release/2.6 branch (highly localized, and arguably a bugfix). commit 82036c17c45d45c3fe8725f64b33720cb9c94dad (master) commit 2f2ff186564c3999efaf48d734df95471ac22d84 (release/2.6) Author: Franco Fichtner Date: Tue May 28 17:42:52 2024 + Allow to set ifmode for existing DCO interfaces in FreeBSD 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 -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Allow the TLS session to send out TLS alerts
Took me a bit to find a setup where this made a difference, but with CA / fingerprint validation errors on the client, I saw tlsv1 alert unknown ca on the server, and with a "tls-version-min/max" mismatch (server requiring 1.1, client capped to 1.0) I saw a proper error on the client 2024-06-01 22:18:57 Received fatal SSL alert: protocol version 2024-06-01 22:18:57 OpenSSL: error:0A00042E:SSL routines::tlsv1 alert protocol version:SSL alert number 70 2024-06-01 22:18:57 TLS_ERROR: BIO read tls_read_plaintext error .. while "without the patch" (older server) it would just sit there and timeout, never hearing anything back from the server. Haven't tried variants more likely to occur in practice (especially "client cert expired") but expect this to be a useful addition to troubleshooting. An older client connecting to a patched server will receive the alerts perfectly fine (as expected, no code changes in those paths), but obviously won't send them *out* - even though visible in the client log. 2024-06-01 22:25:25 Sent fatal SSL alert: unknown CA For completeness, sent through the server side threadmill (testing with the full set of 2.2 to master clients), and also through GHA - all works. Great :-) Your patch has been applied to the master branch. commit fbe3b49b373ea8e81aaa31a383258403a3bfcd07 Author: Arne Schwabe Date: Mon Apr 8 14:49:33 2024 +0200 Allow the TLS session to send out TLS alerts Signed-off-by: Arne Schwabe Acked-by: Frank Lichtenheld Message-Id: <20240408124933.243991-1-fr...@lichtenheld.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28540.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Only schedule_exit() once
Thanks :-) - this is mostly the same as v2, with a whitespace fix, and remodeling receive_exit_message() to avoid -Werror failures when comping without ENABLE_MANAGEMENT ("unused variable"). Tested on GHA, buildbots and the server testbed. Your patch has been applied to the master and release/2.6 branch. The CC EEN functionality is not part of 2.5, so the "clients can use this to get not dropped off" attack does not exist - and the patch is not applicable. commit 55bb3260c12bae33b6a8eac73cbb6972f8517411 (master) commit 65fb67cd6c320a426567b2922c4282fb8738ba3f (release/2.6) Author: Reynir Björnsson Date: Thu May 16 13:58:08 2024 +0200 Only schedule_exit() once Signed-off-by: Reynir Björnsson Acked-by: Arne Schwabe Message-Id: <20240516120434.23499-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28679.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v3] Only schedule_exit() once
From: Reynir Björnsson 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 --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/555 This mail reflects revision 3 of this Change. Acked-by according to Gerrit (reflected above): Arne Schwabe diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index 8d10f25..01165b2 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -514,17 +514,24 @@ } /* - * 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(>c2.scheduled_exit)) +{ +return false; +} tls_set_single_session(c->c2.tls_multi); update_time(); reset_coarse_timers(c); event_timeout_init(>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 6fb5a18..422c591 100644 --- a/src/openvpn/forward.h +++ b/src/openvpn/forward.h @@ -303,7 +303,7 @@ 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 1b406b9..17e7e80 100644 --- a/src/openvpn/push.c +++ b/src/openvpn/push.c @@ -204,7 +204,11 @@ * */ 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 @@ void send_auth_failed(struct context *c, const char *client_reason) { -if (event_timeout_defined(>c2.scheduled_exit)) +if (!schedule_exit(c)) { msg(D_TLS_DEBUG, "exit already scheduled for context"); return; @@ -401,8 +405,6 @@ 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 @@ 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); } ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Remove custom TLS 1.0 PRF implementation only used by LibreSSL/wolfSSL
Tested on the buildbots and on GHA, so does not break mbedtls/openssl builds & self-tests anywhere. Will break compat of LibreSSL/WolFSSL builds with pre-2.6 peers that can not do TLS EKM. Yes. Your patch has been applied to the master branch. commit 763b35f652b1913ddd01e6c548b3e6a57076ba42 Author: Arne Schwabe Date: Wed May 15 12:01:15 2024 +0200 Remove custom TLS 1.0 PRF implementation only used by LibreSSL/wolfSSL Signed-off-by: Arne Schwabe Acked-by: Gert Doering Message-Id: <20240515100115.11056-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28672.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v5] Remove custom TLS 1.0 PRF implementation only used by LibreSSL/wolfSSL
From: Arne Schwabe After the removal of the OpenSSL 1.0.2 support, LibreSSL/wolfSSL are the only libraries that still needs the custom implementation. Since our LibreSSL/wolfSSL support is always best effort, we can afford to limit LibreSSL support in this way. If they want to support this, they should expose the functionality as well. Change-Id: I5bfa3630ad4dff2807705658bc877c4a429a39ce Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/560 This mail reflects revision 5 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c index 0473fad..fbd38f3 100644 --- a/src/openvpn/crypto_openssl.c +++ b/src/openvpn/crypto_openssl.c @@ -1397,7 +1397,7 @@ return ret; } -#elif !defined(LIBRESSL_VERSION_NUMBER) +#elif !defined(LIBRESSL_VERSION_NUMBER) && !defined(ENABLE_CRYPTO_WOLFSSL) bool ssl_tls1_PRF(const uint8_t *seed, int seed_len, const uint8_t *secret, int secret_len, uint8_t *output, int output_len) @@ -1444,183 +1444,14 @@ return ret; } #else /* if defined(LIBRESSL_VERSION_NUMBER) */ -/* - * Generate the hash required by for the \c tls1_PRF function. - * - * We cannot use our normal hmac_* function as they do not work - * in a FIPS environment (no MD5 allowed, which we need). Instead - * we need to directly use the EVP_MD_* API with the special - * EVP_MD_CTX_FLAG_NON_FIPS_ALLOW flag. - * - * The function below is adapted from OpenSSL 1.0.2t - * - * @param md_kt Message digest to use - * @param sec Secret to base the hash on - * @param sec_len Length of the secret - * @param seed Seed to hash - * @param seed_len Length of the seed - * @param out Output buffer - * @param olen Length of the output buffer - */ -static -bool -tls1_P_hash(const EVP_MD *md, const unsigned char *sec, -int sec_len, const void *seed, int seed_len, -unsigned char *out, int olen) -{ -int chunk; -size_t j; -EVP_MD_CTX *ctx, *ctx_tmp, *ctx_init; -EVP_PKEY *mac_key; -unsigned char A1[EVP_MAX_MD_SIZE]; -size_t A1_len = EVP_MAX_MD_SIZE; -int ret = false; - -chunk = EVP_MD_size(md); -OPENSSL_assert(chunk >= 0); - -ctx = md_ctx_new(); -ctx_tmp = md_ctx_new(); -ctx_init = md_ctx_new(); -EVP_MD_CTX_set_flags(ctx_init, EVP_MD_CTX_FLAG_NON_FIPS_ALLOW); -mac_key = EVP_PKEY_new_mac_key(EVP_PKEY_HMAC, NULL, sec, sec_len); -if (!mac_key) -{ -goto err; -} -if (!EVP_DigestSignInit(ctx_init, NULL, md, NULL, mac_key)) -{ -goto err; -} -if (!EVP_MD_CTX_copy_ex(ctx, ctx_init)) -{ -goto err; -} -if (!EVP_DigestSignUpdate(ctx, seed, seed_len)) -{ -goto err; -} -if (!EVP_DigestSignFinal(ctx, A1, _len)) -{ -goto err; -} - -for (;; ) -{ -/* Reinit mac contexts */ -if (!EVP_MD_CTX_copy_ex(ctx, ctx_init)) -{ -goto err; -} -if (!EVP_DigestSignUpdate(ctx, A1, A1_len)) -{ -goto err; -} -if (olen > chunk && !EVP_MD_CTX_copy_ex(ctx_tmp, ctx)) -{ -goto err; -} -if (!EVP_DigestSignUpdate(ctx, seed, seed_len)) -{ -goto err; -} - -if (olen > chunk) -{ -j = olen; -if (!EVP_DigestSignFinal(ctx, out, )) -{ -goto err; -} -out += j; -olen -= j; -/* calc the next A1 value */ -if (!EVP_DigestSignFinal(ctx_tmp, A1, _len)) -{ -goto err; -} -} -else -{ -A1_len = EVP_MAX_MD_SIZE; -/* last one */ -if (!EVP_DigestSignFinal(ctx, A1, _len)) -{ -goto err; -} -memcpy(out, A1, olen); -break; -} -} -ret = true; -err: -EVP_PKEY_free(mac_key); -EVP_MD_CTX_free(ctx); -EVP_MD_CTX_free(ctx_tmp); -EVP_MD_CTX_free(ctx_init); -OPENSSL_cleanse(A1, sizeof(A1)); -return ret; -} - -/* - * Use the TLS PRF function for generating data channel keys. - * This code is based on the OpenSSL library. - * - * TLS generates keys as such: - * - * master_secret[48] = PRF(pre_master_secret[48], "master secret", - * ClientHello.random[32] + ServerHello.random[32]) - * - * key_block[] = PRF(SecurityParameters.master_secret[48], - * "key expansion", - * SecurityParameters.server_random[32] + - * SecurityParameters.client_random[32]); - * - * Notes: - * - * (1) key_block contains
[Openvpn-devel] [PATCH applied] Re: Remove OpenSSL 1.0.2 support
Looks good, tested on the buildbots (all pass, except those that do not have anything more recent than 1.0.2 - and those will no longer be considered for building and testing "master") and on GHA. Your patch has been applied to the master branch. commit 51f80db910eb48e720ce106b5b9b5ec96d8e0e23 Author: Arne Schwabe Date: Tue May 14 16:15:50 2024 +0200 Remove OpenSSL 1.0.2 support Signed-off-by: Arne Schwabe Acked-by: Gert Doering Message-Id: <20240514141550.17544-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28665.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v9] Remove OpenSSL 1.0.2 support
From: Arne Schwabe With Centos 7/Red Hat Enterprise Linux 7 being EOL this June, the last distributions that still support OpenSSL 1.0.2 are finally EOL. This means we no longer need to support OpenSSL 1.0.2 Change-Id: I90875311a4e4c403e77e30b609c1878cbd45 Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/559 This mail reflects revision 9 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering diff --git a/Changes.rst b/Changes.rst index fa0fb22..4bc3bb3 100644 --- a/Changes.rst +++ b/Changes.rst @@ -33,6 +33,10 @@ ``--topology net30`` to the config should fix the problem. By default ``--topology`` is pushed from server to client. +OpenSSL 1.0.2 support +Support for building with OpenSSL 1.0.2 has been removed. The minimum +supported OpenSSL version is now 1.1.0. + Overview of changes in 2.6 == diff --git a/INSTALL b/INSTALL index a63bab6..6007338 100644 --- a/INSTALL +++ b/INSTALL @@ -66,7 +66,7 @@ (1) TUN and/or TAP driver to allow user-space programs to control a virtual point-to-point IP or Ethernet device. See TUN/TAP Driver References section below for more info. - (2a) OpenSSL library, necessary for encryption, version 1.0.2 or higher + (2a) OpenSSL library, necessary for encryption, version 1.1.0 or higher required, available from http://www.openssl.org/ or (2b) mbed TLS library, an alternative for encryption, version 2.0 or higher diff --git a/configure.ac b/configure.ac index ce8b2b0..965ed1a 100644 --- a/configure.ac +++ b/configure.ac @@ -888,7 +888,7 @@ # if the user did not explicitly specify flags, try to autodetect PKG_CHECK_MODULES( [OPENSSL], - [openssl >= 1.0.2], + [openssl >= 1.1.0], [have_openssl="yes"], [AC_MSG_WARN([OpenSSL not found by pkg-config ${pkg_config_found}])] # If this fails, we will do another test next ) @@ -903,7 +903,7 @@ # If pkgconfig check failed or OPENSSL_CFLAGS/OPENSSL_LIBS env vars # are used, check the version directly in the OpenSSL include file if test "${have_openssl}" != "yes"; then - AC_MSG_CHECKING([additionally if OpenSSL is available and version >= 1.0.2]) + AC_MSG_CHECKING([additionally if OpenSSL is available and version >= 1.1.0]) AC_COMPILE_IFELSE( [AC_LANG_PROGRAM( [[ @@ -911,7 +911,7 @@ ]], [[ /* Version encoding: MNNFFPPS - see opensslv.h for details */ -#if OPENSSL_VERSION_NUMBER < 0x10002000L +#if OPENSSL_VERSION_NUMBER < 0x1010L #error OpenSSL too old #endif ]] @@ -981,7 +981,7 @@ [AC_MSG_ERROR([OpenSSL check for AES-256-GCM support failed])] ) - # All supported OpenSSL version (>= 1.0.2) + # All supported OpenSSL version (>= 1.1.0) # have this feature have_export_keying_material="yes" diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c index 1649ab7..0473fad 100644 --- a/src/openvpn/crypto_openssl.c +++ b/src/openvpn/crypto_openssl.c @@ -49,7 +49,7 @@ #include #include -#if (OPENSSL_VERSION_NUMBER >= 0x1010L) && !defined(LIBRESSL_VERSION_NUMBER) +#if !defined(LIBRESSL_VERSION_NUMBER) #include #endif #if OPENSSL_VERSION_NUMBER >= 0x3000L @@ -193,11 +193,7 @@ void crypto_init_lib(void) { -#if (OPENSSL_VERSION_NUMBER >= 0x1010L) OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CONFIG, NULL); -#else -OPENSSL_config(NULL); -#endif /* * If you build the OpenSSL library and OpenVPN with * CRYPTO_MDEBUG, you will get a listing of OpenSSL @@ -1401,7 +1397,7 @@ return ret; } -#elif (OPENSSL_VERSION_NUMBER >= 0x1010L) && !defined(LIBRESSL_VERSION_NUMBER) +#elif !defined(LIBRESSL_VERSION_NUMBER) bool ssl_tls1_PRF(const uint8_t *seed, int seed_len, const uint8_t *secret, int secret_len, uint8_t *output, int output_len) @@ -1447,7 +1443,7 @@ EVP_PKEY_CTX_free(pctx); return ret; } -#else /* if OPENSSL_VERSION_NUMBER >= 0x1010L */ +#else /* if defined(LIBRESSL_VERSION_NUMBER) */ /* * Generate the hash required by for the \c tls1_PRF function. * @@ -1626,5 +1622,5 @@ gc_free(); return ret; } -#endif /* if OPENSSL_VERSION_NUMBER >= 0x1010L */ +#endif /* if LIBRESSL_VERSION_NUMBER */ #endif /* ENABLE_CRYPTO_OPENSSL */ diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h index c9fa
[Openvpn-devel] [PATCH applied] Re: Workaround issue in LibreSSL crashing when enumerating digests/ciphers
This is one of many not-so-good approaches to the problem, but LibreSSL upstream says "can not backport to OpenBSD 7.5" and since we just got a new and shiny OpenBSD 7.5 buildbot, everything fails without this patch... we can remove it from master as soon as OpenBSD 7.6 is there (+ the buildbot upgraded). Tested on FreeBSD with OpenSSL 1.1 & 3.0, and OpenBSD 7.5, of course :) Your patch has been applied to the master and release/2.6 branch. commit b3a271b11723cbe520ad4ce6b4b0459de57ade06 (master) commit 8aed156be81a3bdd3098bfed5e8f95662d06633c (release/2.6) Author: Arne Schwabe Date: Thu May 9 00:05:40 2024 +0200 Workaround issue in LibreSSL crashing when enumerating digests/ciphers Signed-off-by: Arne Schwabe Acked-by: Gert Doering Message-Id: <20240508220540.12554-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28649.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Support OpenBSD with cmake
I know nothing about cmake, but Frank does, so not much for me to do :-) Your patch has been applied to the master branch. commit d5ba4acc297a6041bb45f7aa1c9a99b37b7d5e44 Author: Arne Schwabe Date: Thu May 9 00:05:12 2024 +0200 Support OpenBSD with cmake Signed-off-by: Arne Schwabe Acked-by: Frank Lichtenheld Message-Id: <20240508220512.12362-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28648.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v1] Workaround issue in LibreSSL crashing when enumerating digests/ciphers
From: Arne Schwabe 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. Change-Id: Ia08a9697d0ff41721fb0acf17ccb4cfa23cb3934 Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/586 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c index 61c6518..1649ab7 100644 --- a/src/openvpn/crypto_openssl.c +++ b/src/openvpn/crypto_openssl.c @@ -387,7 +387,19 @@ #else for (int nid = 0; nid < 1; ++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, _list); @@ -441,7 +453,19 @@ #else for (int nid = 0; nid < 1; ++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 @@ -449,7 +473,7 @@ print_digest((EVP_MD *)digest, NULL); } } -#endif +#endif /* if OPENSSL_VERSION_NUMBER >= 0x3000L */ printf("\n"); } ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v1] Support OpenBSD with cmake
From: Arne Schwabe Change-Id: I85d4d27333773e8df109e42b1fa56ccf57994e57 Signed-off-by: Arne Schwabe Acked-by: Frank Lichtenheld --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/585 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Frank Lichtenheld diff --git a/CMakeLists.txt b/CMakeLists.txt index 3127611..f8b37a9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -136,6 +136,8 @@ set(TARGET_FREEBSD YES) set(ENABLE_DCO YES) link_libraries(-lnv) +elseif (${CMAKE_SYSTEM_NAME} STREQUAL "OpenBSD") +set(TARGET_OPENBSD YES) elseif (${CMAKE_SYSTEM_NAME} STREQUAL "SunOS") set(TARGET_SOLARIS YES) set(HAVE_SYS_SOCKIO_H 1) @@ -169,7 +171,7 @@ check_symbol_exists(setgid unistd.h HAVE_SETGID) check_symbol_exists(setuid unistd.h HAVE_SETUID) check_symbol_exists(setsid unistd.h HAVE_SETSID) -check_symbol_exists(getpeereid unistd.h HAVE_GETPEEREID) +check_symbol_exists(getpeereid "unistd.h;sys/socket.h" HAVE_GETPEEREID) check_symbol_exists(epoll_create sys/epoll.h HAVE_EPOLL_CREATE) ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: configure: update old copy of pkg.m4
Arne and the buildbots say "this is fine", so, in it goes :-) Your patch has been applied to the master branch. commit bccb22ab44d7e5a60bece286c9daf8b676f2b7c3 (master) Author: Frank Lichtenheld Date: Mon May 6 18:04:07 2024 +0200 configure: update old copy of pkg.m4 Signed-off-by: Frank Lichtenheld Acked-by: Arne Schwabe Message-Id: <20240506160413.7189-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28631.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2] configure: update old copy of pkg.m4
From: Frank Lichtenheld If we copy this code, let's at least make sure we update it every decade ;) I also considered removing it. However, then autoconf can't be run on systems without pkg-config installed anymore. While that is very unusual, didn't see a good reason to break that. Change-Id: I34e96a225446693f401549d86d872c02427ef7d5 Signed-off-by: Frank Lichtenheld Acked-by: Arne Schwabe --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/558 This mail reflects revision 2 of this Change. Acked-by according to Gerrit (reflected above): Arne Schwabe diff --git a/m4/pkg.m4 b/m4/pkg.m4 index cca47a7..13a8890 100644 --- a/m4/pkg.m4 +++ b/m4/pkg.m4 @@ -1,29 +1,60 @@ -# pkg.m4 - Macros to locate and utilise pkg-config.-*- Autoconf -*- -# serial 1 (pkg-config-0.24) -# -# Copyright © 2004 Scott James Remnant . -# -# This program is free software; you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 2 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, but -# WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU -# General Public License for more details. -# -# You should have received a copy of the GNU General Public License along -# with this program; if not, write to the Free Software Foundation, Inc., -# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. -# -# As a special exception to the GNU General Public License, if you -# distribute this file as part of a program that contains a -# configuration script generated by Autoconf, you may include it under -# the same distribution terms that you use for the rest of that program. +# pkg.m4 - Macros to locate and utilise pkg-config. -*- Autoconf -*- +# serial 12 (pkg-config-0.29.2) -# PKG_PROG_PKG_CONFIG([MIN-VERSION]) -# -- +dnl Copyright © 2004 Scott James Remnant . +dnl Copyright © 2012-2015 Dan Nicholson +dnl +dnl This program is free software; you can redistribute it and/or modify +dnl it under the terms of the GNU General Public License as published by +dnl the Free Software Foundation; either version 2 of the License, or +dnl (at your option) any later version. +dnl +dnl This program is distributed in the hope that it will be useful, but +dnl WITHOUT ANY WARRANTY; without even the implied warranty of +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +dnl General Public License for more details. +dnl +dnl You should have received a copy of the GNU General Public License +dnl along with this program; if not, write to the Free Software +dnl Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA +dnl 02111-1307, USA. +dnl +dnl As a special exception to the GNU General Public License, if you +dnl distribute this file as part of a program that contains a +dnl configuration script generated by Autoconf, you may include it under +dnl the same distribution terms that you use for the rest of that +dnl program. + +dnl PKG_PREREQ(MIN-VERSION) +dnl --- +dnl Since: 0.29 +dnl +dnl Verify that the version of the pkg-config macros are at least +dnl MIN-VERSION. Unlike PKG_PROG_PKG_CONFIG, which checks the user's +dnl installed version of pkg-config, this checks the developer's version +dnl of pkg.m4 when generating configure. +dnl +dnl To ensure that this macro is defined, also add: +dnl m4_ifndef([PKG_PREREQ], +dnl [m4_fatal([must install pkg-config 0.29 or later before running autoconf/autogen])]) +dnl +dnl See the "Since" comment for each macro you use to see what version +dnl of the macros you require. +m4_defun([PKG_PREREQ], +[m4_define([PKG_MACROS_VERSION], [0.29.2]) +m4_if(m4_version_compare(PKG_MACROS_VERSION, [$1]), -1, +[m4_fatal([pkg.m4 version $1 or higher is required but ]PKG_MACROS_VERSION[ found])]) +])dnl PKG_PREREQ + +dnl PKG_PROG_PKG_CONFIG([MIN-VERSION]) +dnl -- +dnl Since: 0.16 +dnl +dnl Search for the pkg-config tool and set the PKG_CONFIG variable to +dnl first found in the path. Checks that the version of pkg-config found +dnl is at least MIN-VERSION. If MIN-VERSION is not specified, 0.9.0 is +dnl used since that's the first version where most current features of +dnl pkg-config existed. AC_DEFUN([PKG_PROG_PKG_CONFIG], [m4_pattern_forbid([^_?PKG_[A-Z_]+$]) m4_pattern_allow([^PKG_CONFIG(_(PATH|LIBDIR|SYSROOT_DIR|ALLOW_SYSTEM_(CFLAGS|LIBS)))?$]) @@ -45,18 +76,19 @@ PKG_CONFIG="" fi fi[]dnl -])# PKG_PROG_PKG_CONFIG +])dnl PKG_PROG_PKG_CONFIG -# PKG_CHECK_EXISTS(MODULES, [ACTION-IF-FOUND], [ACTION-IF-NOT-FOUND]) -# -# Check to see whether a particular set of modules exists. Similar -# to
[Openvpn-devel] [PATCH applied] Re: Only run coverity scan in OpenVPN/OpenVPN repository
Your patch has been applied to the master and release/2.6 branch. commit 815df21d389bf70dbe98cb89f2c60b6e6e816faa (master) commit 56fc48e87decfa16a15ab0293853c473bf56703f (release/2.6) Author: Arne Schwabe Date: Mon May 6 17:58:31 2024 +0200 Only run coverity scan in OpenVPN/OpenVPN repository Signed-off-by: Arne Schwabe Acked-by: Frank Lichtenheld Message-Id: <20240506155831.3524-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28627.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v1] Only run coverity scan in OpenVPN/OpenVPN repository
From: Arne Schwabe 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 --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/583 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Frank Lichtenheld diff --git a/.github/workflows/coverity-scan.yml b/.github/workflows/coverity-scan.yml index e289746..37b8102 100644 --- a/.github/workflows/coverity-scan.yml +++ b/.github/workflows/coverity-scan.yml @@ -6,6 +6,9 @@ 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 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Repeat the unknown command in errors from management interface
Not tested, but trivial enough, and the actual format of the ERROR: message is not guaranteed so GUIs shouldn't be surprised if it changes. Your patch has been applied to the master branch. commit b90a6e56250ccb18b4913bb115e5dcf4905dbfb1 Author: Arne Schwabe Date: Mon May 6 16:23:03 2024 +0200 Repeat the unknown command in errors from management interface Signed-off-by: Arne Schwabe Acked-by: Frank Lichtenheld Message-Id: <20240506142303.13198-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28621.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Remove openvpn_snprintf and similar functions
Stared a bit at the difference of v2 and v5 (windows pipe cast plus clang pragma). All the rest is identical to v2, and trivial in itself ("git show -w -I snprintf"). Self test passes, and compilation / GHA also passes. I have adapted the "We know that the results truncated" comment (added "will be") - I think that was in v2 already, or in Antonio's comment on v2, but it's a comment, so no good in asking for a v6. Your patch has been applied to the master branch. commit 130548fe4d23afac5d2948d4e5ee164eef635cfd Author: Arne Schwabe Date: Mon May 6 12:27:10 2024 +0200 Remove openvpn_snprintf and similar functions Signed-off-by: Arne Schwabe Acked-by: Frank Lichtenheld Message-Id: <20240506102710.8976-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28617.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v1] Repeat the unknown command in errors from management interface
From: Arne Schwabe This help pinpointing errors in logs from my app Change-Id: Ie2b62bc95371daf7e1eb58e0323835f169399910 Signed-off-by: Arne Schwabe Acked-by: Frank Lichtenheld --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/584 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Frank Lichtenheld diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c index 43c5507..415ee7a 100644 --- a/src/openvpn/manage.c +++ b/src/openvpn/manage.c @@ -1663,7 +1663,7 @@ #endif else { -msg(M_CLIENT, "ERROR: unknown command, enter 'help' for more options"); +msg(M_CLIENT, "ERROR: unknown command [%s], enter 'help' for more options", p[0]); } done: ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v5] Remove openvpn_snprintf and similar functions
From: Arne Schwabe Old Microsoft versions did strange behaviour but according to the newly added unit test and https://stackoverflow.com/questions/7706936/is-snprintf-always-null-terminating this is now standard conforming and we can use the normal snprintf method. Microsoft own documentation to swprintf also says you nowadays need to define _CRT_NON_CONFORMING_SWPRINTFS to get to non-standard behaviour. Change-Id: I07096977e3b562bcb5d2c6f11673a4175b8e12ac Signed-off-by: Arne Schwabe Acked-by: Frank Lichtenheld --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/547 This mail reflects revision 5 of this Change. Acked-by according to Gerrit (reflected above): Frank Lichtenheld diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c index 66fd63f..3a8069c 100644 --- a/src/openvpn/buffer.c +++ b/src/openvpn/buffer.c @@ -279,32 +279,6 @@ return ret; } - -/* - * This is necessary due to certain buggy implementations of snprintf, - * that don't guarantee null termination for size > 0. - * - * Return false on overflow. - * - * This functionality is duplicated in src/openvpnserv/common.c - * Any modifications here should be done to the other place as well. - */ - -bool -openvpn_snprintf(char *str, size_t size, const char *format, ...) -{ -va_list arglist; -int len = -1; -if (size > 0) -{ -va_start(arglist, format); -len = vsnprintf(str, size, format, arglist); -va_end(arglist); -str[size - 1] = 0; -} -return (len >= 0 && len < size); -} - /* * write a string to the end of a buffer that was * truncated by buf_printf diff --git a/src/openvpn/buffer.h b/src/openvpn/buffer.h index 7c2f75a..27c3199 100644 --- a/src/openvpn/buffer.h +++ b/src/openvpn/buffer.h @@ -448,19 +448,6 @@ */ bool buf_puts(struct buffer *buf, const char *str); -/* - * Like snprintf but guarantees null termination for size > 0 - */ -bool openvpn_snprintf(char *str, size_t size, const char *format, ...) -#ifdef __GNUC__ -#if __USE_MINGW_ANSI_STDIO -__attribute__ ((format(gnu_printf, 3, 4))) -#else -__attribute__ ((format(__printf__, 3, 4))) -#endif -#endif -; - /* * remove/add trailing characters diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c index 5d05cc4..207f145 100644 --- a/src/openvpn/crypto.c +++ b/src/openvpn/crypto.c @@ -874,11 +874,11 @@ key_direction_state_init(, key_direction); -openvpn_snprintf(log_prefix, sizeof(log_prefix), "Outgoing %s", name); +snprintf(log_prefix, sizeof(log_prefix), "Outgoing %s", name); init_key_ctx(>encrypt, >keys[kds.out_key], kt, OPENVPN_OP_ENCRYPT, log_prefix); -openvpn_snprintf(log_prefix, sizeof(log_prefix), "Incoming %s", name); +snprintf(log_prefix, sizeof(log_prefix), "Incoming %s", name); init_key_ctx(>decrypt, >keys[kds.in_key], kt, OPENVPN_OP_DECRYPT, log_prefix); diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c index c230292..32511f0 100644 --- a/src/openvpn/crypto_mbedtls.c +++ b/src/openvpn/crypto_mbedtls.c @@ -128,7 +128,7 @@ { char prefix[256]; -if (!openvpn_snprintf(prefix, sizeof(prefix), "%s:%d", func, line)) +if (!snprintf(prefix, sizeof(prefix), "%s:%d", func, line)) { return mbed_log_err(flags, errval, func); } @@ -239,11 +239,11 @@ char header[1000+1] = { 0 }; char footer[1000+1] = { 0 }; -if (!openvpn_snprintf(header, sizeof(header), "-BEGIN %s-\n", name)) +if (!snprintf(header, sizeof(header), "-BEGIN %s-\n", name)) { return false; } -if (!openvpn_snprintf(footer, sizeof(footer), "-END %s-\n", name)) +if (!snprintf(footer, sizeof(footer), "-END %s-\n", name)) { return false; } @@ -278,11 +278,11 @@ char header[1000+1] = { 0 }; char footer[1000+1] = { 0 }; -if (!openvpn_snprintf(header, sizeof(header), "-BEGIN %s-", name)) +if (!snprintf(header, sizeof(header), "-BEGIN %s-", name)) { return false; } -if (!openvpn_snprintf(footer, sizeof(footer), "-END %s-", name)) +if (!snprintf(footer, sizeof(footer), "-END %s-", name)) { return false; } diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c index 7de3991..0539ca5 100644 --- a/src/openvpn/dns.c +++ b/src/openvpn/dns.c @@ -349,11 +349,11 @@ if (j < 0) { -name_ok = openvpn_snprintf(name, sizeof(name), format, i); +name_ok = snprintf(name, sizeof(name), format, i); } else { -name_ok = openvpn_snprintf(name, sizeof(name), format, i, j); +name_ok = snprintf(name, sizeof(name), format, i, j); } if (!name_ok) diff --git a/src/openvpn/env_set.c b/src/openvpn/env_set.c index b13d01e..81ab59e 100644 --- a/src/openvpn/env_set.c +++
[Openvpn-devel] [PATCH applied] Re: Fix 'binary or' vs 'boolean or' related to server_bridge_proxy_dhcp
Code looks good, old code happened to work but was "logically wrong", so this is a reasonable fix. As it's not really a *bug* I've kept it to master. Tested on the server test rig, for good measure, though that one doesn't actually have a --server-bridge config - the TAP server test uses VLANs and scripts to do IP-and-VLAN assignments... meh. Your patch has been applied to the master branch. commit 9d92221eb4e773cae913752af6d70082ae305fe8 Author: Frank Lichtenheld Date: Thu May 2 11:53:22 2024 +0200 Fix 'binary or' vs 'boolean or' related to server_bridge_proxy_dhcp Signed-off-by: Frank Lichtenheld Acked-by: Gert Doering Message-Id: <20240502095322.9433-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28601.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Replace macos11 with macos14 in github runners
Your patch has been applied to the master and release/2.6 branch. (release/2.6 grows macos13 testing as a side effect of the merge conflict - tested, all green) commit 02f0845be7e54e8676e73621e424b6a1540b88b5 (master) commit 18520e5a25a983b616762e6082da8436d0933411 (release/2.6) Author: Arne Schwabe Date: Thu May 2 14:22:31 2024 +0200 Replace macos11 with macos14 in github runners Signed-off-by: Arne Schwabe Acked-by: Frank Lichtenheld Message-Id: <20240502122231.672-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/search?l=mid=20240502122231.672-1-g...@greenie.muc.de Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v1] Replace macos11 with macos14 in github runners
From: Arne Schwabe 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 --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/582 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Frank Lichtenheld diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index f771f5a..d7c3ecd 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -202,8 +202,16 @@ matrix: ssllib: [ openssl11, openssl3, libressl] build: [ normal, asan ] -os: [macos-11, macos-12, macos-13] +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,undefined -fno-sanitize-recover=all -fno-optimize-sibling-calls -fsanitize-address-use-after-scope -fno-omit-frame-pointer -g -O1" ldflags: -fsanitize=address,undefined -fno-sanitize-recover=all @@ -228,8 +236,10 @@ 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 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v1] Fix "binary or" vs "boolean or" related to server_bridge_proxy_dhcp
From: Frank Lichtenheld Both values are boolean so there is no reason to use "|" and it just confuses the reader whether there is something more going on here. Change-Id: Ie61fa6a78875ecbaa9d3d8e7a50603d77c9ce09e Signed-off-by: Frank Lichtenheld Acked-by: Gert Doering --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/553 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering diff --git a/src/openvpn/helper.c b/src/openvpn/helper.c index 1bab84c..24dee97 100644 --- a/src/openvpn/helper.c +++ b/src/openvpn/helper.c @@ -427,7 +427,7 @@ * if !nogw: * push "route-gateway dhcp" */ -else if (o->server_bridge_defined | o->server_bridge_proxy_dhcp) +else if (o->server_bridge_defined || o->server_bridge_proxy_dhcp) { if (o->client) { diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 02205e7..e67f10e 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -3565,7 +3565,7 @@ msg(M_WARN, "WARNING: using --pull/--client and --ifconfig together is probably not what you want"); } -if (o->server_bridge_defined | o->server_bridge_proxy_dhcp) +if (o->server_bridge_defined || o->server_bridge_proxy_dhcp) { msg(M_WARN, "NOTE: when bridging your LAN adapter with the TAP adapter, note that the new bridge adapter will often take on its own IP address that is different from what the LAN adapter was previously set to"); } ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Use topology default of subnet only for server mode
This looks good, and fixes my "reference" test case (p2p, no --server but --tls-server / --secret, --ifconfig 10.2.3.4 10.2.3.5) The twisty maze of passages surrounding o->mode is... interesting, but helper_setdefault_topology() solves this in a reasonable way, I think (the call inside helper_client_server() could have just set it, if undef, as "we're server, and dev tun" is already known - but this way it's "one common code"). I *do* wonder why options.c -> "server" doesn't just set o->mode = MODE_SERVER, but that's propably lost in the mists of time... Your patch has been applied to the master branch. commit 066fcdba9741319fa38cbe40c1761c49727d3f9a (master) Author: Frank Lichtenheld Date: Wed May 1 14:42:54 2024 +0200 Use topology default of subnet only for server mode Signed-off-by: Frank Lichtenheld Acked-by: Arne Schwabe Message-Id: <20240501124254.29114-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28592.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Add missing EVP_KDF_CTX_free in ssl_tls1_PRF
Mildly tested (GHA and local t_client tests which *should* excercise this code path with the libraries + peers involved). Your patch has been applied to the master branch. Not applied to release/2.6 as the code lacking this free() is master-only. commit d4eb413181d1c414b854d0829f00cda5ad1e293d (master) Author: Arne Schwabe Date: Wed May 1 14:18:19 2024 +0200 Add missing EVP_KDF_CTX_free in ssl_tls1_PRF Signed-off-by: Arne Schwabe Acked-by: Frank Lichtenheld Message-Id: <20240501121819.12805-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28591.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v1] Use topology default of "subnet" only for server mode
From: Frank Lichtenheld The setting of --topology changes the syntax of --ifconfig. So changing the default of --topology breaks all existing configs that use --ifconfig but not --topology. For P2P setups that is probably a signification percentage. For server setups the percentage is hopefully lower since --ifconfig is implicitly set by --server. Also more people might have set their topology explicitly since it makes a much bigger difference. Clients will usually get the topology and the IP config pushed by the server. So we decided to not switch the default for everyone to not affect P2P setups. What we care about is to change the default for --mode server, so we only do that now. For people using --server this should be transparent except for a pool reset. Change-Id: Iefd209c0856ef395ab74055496130de00b86ead0 Signed-off-by: Frank Lichtenheld Acked-by: Arne Schwabe --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/554 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Arne Schwabe diff --git a/Changes.rst b/Changes.rst index b2278ab..fa0fb22 100644 --- a/Changes.rst +++ b/Changes.rst @@ -23,11 +23,12 @@ ``persist-key`` option has been enabled by default. All the keys will be kept in memory across restart. -Default for ``--topology`` changed to ``subnet`` -Previous releases used ``net30`` as default. This only affects -configs with ``--dev tun`` and only IPv4. Note that this -changes the semantics of ``--ifconfig``, so if you have manual -settings for that in your config but not set ``--topology`` +Default for ``--topology`` changed to ``subnet`` for ``--mode server`` +Previous releases always used ``net30`` as default. This only affects +configs with ``--mode server`` or ``--server`` (the latter implies the +former), and ``--dev tun``, and only if IPv4 is enabled. +Note that this changes the semantics of ``--ifconfig``, so if you have +manual settings for that in your config but not set ``--topology`` your config might fail to parse with the new version. Just adding ``--topology net30`` to the config should fix the problem. By default ``--topology`` is pushed from server to client. diff --git a/src/openvpn/helper.c b/src/openvpn/helper.c index 1bab84c..5681718 100644 --- a/src/openvpn/helper.c +++ b/src/openvpn/helper.c @@ -137,6 +137,32 @@ } +/** + * Set --topology default depending on --mode + */ +void +helper_setdefault_topology(struct options *o) +{ +if (o->topology != TOP_UNDEF) +{ +return; +} +int dev = dev_type_enum(o->dev, o->dev_type); +if (dev != DEV_TYPE_TUN) +{ +return; +} +if (o->mode == MODE_SERVER) +{ +o->topology = TOP_SUBNET; +} +else +{ +o->topology = TOP_NET30; +} +} + + /* * Process server, server-bridge, and client helper * directives after the parameters themselves have been @@ -151,7 +177,6 @@ * Get tun/tap/null device type */ const int dev = dev_type_enum(o->dev, o->dev_type); -const int topology = o->topology; /* * @@ -177,11 +202,11 @@ if (o->server_flags & SF_NOPOOL) { -msg( M_USAGE, "--server-ipv6 is incompatible with 'nopool' option" ); +msg(M_USAGE, "--server-ipv6 is incompatible with 'nopool' option"); } if (o->ifconfig_ipv6_pool_defined) { -msg( M_USAGE, "--server-ipv6 already defines an ifconfig-ipv6-pool, so you can't also specify --ifconfig-pool explicitly"); +msg(M_USAGE, "--server-ipv6 already defines an ifconfig-ipv6-pool, so you can't also specify --ifconfig-pool explicitly"); } o->mode = MODE_SERVER; @@ -207,7 +232,7 @@ o->server_netbits_ipv6 < 112 ? 0x1000 : 2); o->ifconfig_ipv6_pool_netbits = o->server_netbits_ipv6; -push_option( o, "tun-ipv6", M_USAGE ); +push_option(o, "tun-ipv6", M_USAGE); } /* @@ -305,8 +330,10 @@ o->mode = MODE_SERVER; o->tls_server = true; +/* Need to know topology now */ +helper_setdefault_topology(o); -if (topology == TOP_NET30 || topology == TOP_P2P) +if (o->topology == TOP_NET30 || o->topology == TOP_P2P) { o->ifconfig_local = print_in_addr_t(o->server_network + 1, 0, >gc); o->ifconfig_remote_netmask = print_in_addr_t(o->server_network + 2, 0, >gc); @@ -324,12 +351,12 @@ { push_option(o, print_opt_route(o->server_network, o->server_netmask, >gc), M_USAGE); } -else if (topology == TOP_NET30) +else if (o->topology == TOP_NET30) {
[Openvpn-devel] [PATCH v1] Add missing EVP_KDF_CTX_free in ssl_tls1_PRF
From: Arne Schwabe This is just missing in the function. Found by clang+ASAN. Change-Id: I5d70198f6adbee8add619ee8a0bd6b5b1f61e506 Signed-off-by: Arne Schwabe Acked-by: Frank Lichtenheld --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/581 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Frank Lichtenheld diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c index b2c4eb6..61c6518 100644 --- a/src/openvpn/crypto_openssl.c +++ b/src/openvpn/crypto_openssl.c @@ -1372,6 +1372,7 @@ err: ret = false; out: +EVP_KDF_CTX_free(kctx); EVP_KDF_free(kdf); return ret; ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2] Remove openvpn_snprintf and similar functions
From: Arne Schwabe Old Microsoft versions did strange behaviour but according to the newly added unit test and https://stackoverflow.com/questions/7706936/is-snprintf-always-null-terminating this is now standard conforming and we can use the normal snprintf method. Microsoft own documentation to swprintf also says you nowadays need to define _CRT_NON_CONFORMING_SWPRINTFS to get to non-standard behaviour. Change-Id: I07096977e3b562bcb5d2c6f11673a4175b8e12ac Signed-off-by: Arne Schwabe Acked-by: Antonio Quartulli --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/547 This mail reflects revision 2 of this Change. Acked-by according to Gerrit (reflected above): Antonio Quartulli diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c index 66fd63f..3a8069c 100644 --- a/src/openvpn/buffer.c +++ b/src/openvpn/buffer.c @@ -279,32 +279,6 @@ return ret; } - -/* - * This is necessary due to certain buggy implementations of snprintf, - * that don't guarantee null termination for size > 0. - * - * Return false on overflow. - * - * This functionality is duplicated in src/openvpnserv/common.c - * Any modifications here should be done to the other place as well. - */ - -bool -openvpn_snprintf(char *str, size_t size, const char *format, ...) -{ -va_list arglist; -int len = -1; -if (size > 0) -{ -va_start(arglist, format); -len = vsnprintf(str, size, format, arglist); -va_end(arglist); -str[size - 1] = 0; -} -return (len >= 0 && len < size); -} - /* * write a string to the end of a buffer that was * truncated by buf_printf diff --git a/src/openvpn/buffer.h b/src/openvpn/buffer.h index 7c2f75a..27c3199 100644 --- a/src/openvpn/buffer.h +++ b/src/openvpn/buffer.h @@ -448,19 +448,6 @@ */ bool buf_puts(struct buffer *buf, const char *str); -/* - * Like snprintf but guarantees null termination for size > 0 - */ -bool openvpn_snprintf(char *str, size_t size, const char *format, ...) -#ifdef __GNUC__ -#if __USE_MINGW_ANSI_STDIO -__attribute__ ((format(gnu_printf, 3, 4))) -#else -__attribute__ ((format(__printf__, 3, 4))) -#endif -#endif -; - /* * remove/add trailing characters diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c index 5d05cc4..207f145 100644 --- a/src/openvpn/crypto.c +++ b/src/openvpn/crypto.c @@ -874,11 +874,11 @@ key_direction_state_init(, key_direction); -openvpn_snprintf(log_prefix, sizeof(log_prefix), "Outgoing %s", name); +snprintf(log_prefix, sizeof(log_prefix), "Outgoing %s", name); init_key_ctx(>encrypt, >keys[kds.out_key], kt, OPENVPN_OP_ENCRYPT, log_prefix); -openvpn_snprintf(log_prefix, sizeof(log_prefix), "Incoming %s", name); +snprintf(log_prefix, sizeof(log_prefix), "Incoming %s", name); init_key_ctx(>decrypt, >keys[kds.in_key], kt, OPENVPN_OP_DECRYPT, log_prefix); diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c index 1a39752..c806719 100644 --- a/src/openvpn/crypto_mbedtls.c +++ b/src/openvpn/crypto_mbedtls.c @@ -128,7 +128,7 @@ { char prefix[256]; -if (!openvpn_snprintf(prefix, sizeof(prefix), "%s:%d", func, line)) +if (!snprintf(prefix, sizeof(prefix), "%s:%d", func, line)) { return mbed_log_err(flags, errval, func); } @@ -239,11 +239,11 @@ char header[1000+1] = { 0 }; char footer[1000+1] = { 0 }; -if (!openvpn_snprintf(header, sizeof(header), "-BEGIN %s-\n", name)) +if (!snprintf(header, sizeof(header), "-BEGIN %s-\n", name)) { return false; } -if (!openvpn_snprintf(footer, sizeof(footer), "-END %s-\n", name)) +if (!snprintf(footer, sizeof(footer), "-END %s-\n", name)) { return false; } @@ -278,11 +278,11 @@ char header[1000+1] = { 0 }; char footer[1000+1] = { 0 }; -if (!openvpn_snprintf(header, sizeof(header), "-BEGIN %s-", name)) +if (!snprintf(header, sizeof(header), "-BEGIN %s-", name)) { return false; } -if (!openvpn_snprintf(footer, sizeof(footer), "-END %s-", name)) +if (!snprintf(footer, sizeof(footer), "-END %s-", name)) { return false; } diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c index 7de3991..0539ca5 100644 --- a/src/openvpn/dns.c +++ b/src/openvpn/dns.c @@ -349,11 +349,11 @@ if (j < 0) { -name_ok = openvpn_snprintf(name, sizeof(name), format, i); +name_ok = snprintf(name, sizeof(name), format, i); } else { -name_ok = openvpn_snprintf(name, sizeof(name), format, i, j); +name_ok = snprintf(name, sizeof(name), format, i, j); } if (!name_ok) diff --git a/src/openvpn/env_set.c b/src/openvpn/env_set.c index b13d01e..81ab59e 100644 --- a/src/openvpn/env_set.c +++
[Openvpn-devel] [PATCH applied] Re: Change default of topology to subnet
Sorry for being so slow in merging this. I had to adjust my testbeds to "not explode" and shied away from doing so :-) (all the server test instances without "topology" defaulted to net30, and just applying this patch would recreate the ip-pool-persist files, breaking t_client EXPECT_IFCONFIG4_... settings). So. Whoever reads this - *THIS CAN BE DISRUPTIVE*. But it's the right way forward, given that DCO (on the server) will only work with "--topology subnet", and it also saves on IPv4 address usage for the pools... and clients have been compatible with "subnet" across all platforms since at least OpenVPN 2.2, so no excuses. NOTE: for --server setups, this will still work, just changing the way the pool is split, thus assigning new IP addresses to clients, and invalidating the --ip-pool-persist file. NOTE2: For p2p setups (no --server, just --tls-server/--tls-client or even --secret) it will break the setup hard, as those usually use "--ifconfig ip1 ip2" and "ip2" will now be parsed as a netmask, with surprising consequences. See GH #529. Your patch has been applied to the master branch. commit 32e6586687a548174b88b64fe54bfae6c74d4c19 Author: Frank Lichtenheld Date: Fri Dec 1 12:20:22 2023 +0100 Change default of topology to subnet Signed-off-by: Frank Lichtenheld Acked-by: Arne Schwabe Message-Id: <20231201112022.15337-1-fr...@lichtenheld.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg27627.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: forked-test-driver: Show test output always
Tested this in various build environments, including Ubuntu/MinGW, and GHA, and things still work & test "as before". Combined with the previous commit, we get useful (for me) "I can see what make check is doing all the time" behaviour back. I'm still not convinced that this is a better way forward than just using the old test-driver and possibly tweaking Makefiles a bit, but "this does work for me", and automake upstream seems to think it's the way to go... thanks, Frank, for doing the actual work :-) Your patch has been applied to the master branch. commit e2ff9161e1b1b3e8c83bf01e3c488e0601834c0c Author: Frank Lichtenheld Date: Thu Jan 25 12:01:22 2024 +0100 forked-test-driver: Show test output always Signed-off-by: Frank Lichtenheld Acked-by: Arne Schwabe Message-Id: <20240125110122.16257-1-fr...@lichtenheld.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28133.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: tests: fork default automake test-driver
Tested this in various build environments, including Ubuntu/MinGW, and GHA, and things still work & test "as before". Combined with the next commit, we get useful (for me) "I can see what make check is doing all the time" behaviour back. Your patch has been applied to the master branch. commit aea6e9aa854b454f98671382eb751662dd9e1e03 Author: Frank Lichtenheld Date: Thu Jan 25 12:00:36 2024 +0100 tests: fork default automake test-driver Signed-off-by: Frank Lichtenheld Acked-by: Arne Schwabe Message-Id: <20240125110036.16070-1-fr...@lichtenheld.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28132.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Remove/combine redundant call of EVP_CipherInit before EVP_CipherInit_Ex
Looking at the OpenSSL implementation for the EVP_CipherInit*() functions makes it really clear that this is, basically, dead code. Of course the "kt" init needs to go to the second call now, because otherwise it will bomb with OpenSSL 3.x ("no default cipher"). Tested with various OpenSSL (and LibreSSL) versions across the buildbot and GHA test bed. Your patch has been applied to the master branch. (Not applied to release/2.6 since it's not actually fixing something, just cleaning up old code - typical "master" material) commit e81e3eb1a4322148b06f353eaa22b0a803fd74f4 Author: Arne Schwabe Date: Tue Apr 2 15:49:09 2024 +0200 Remove/combine redundant call of EVP_CipherInit before EVP_CipherInit_Ex Signed-off-by: Arne Schwabe Acked-by: Gert Doering Message-Id: <20240402134909.6340-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28523.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v3] Remove/combine redundant call of EVP_CipherInit before EVP_CipherInit_Ex
From: Arne Schwabe EVP_CipherInit basically is the same EVP_CipherInit_ex except that it in some instances it resets/inits the ctx parameter first. We already call EVP_CIPHER_CTX_reset to reset/init the ctx before. Also ensure that EVP_CipherInit_Ex gets the cipher to actually be able to initialise the context. OpenSSL 1.0.2: https://github.com/openssl/openssl/blob/OpenSSL_1_0_2-stable/crypto/evp/evp_enc.c#L94 EVP_CipherInit calls first EVP_CIPHER_CTX_init and then EVP_CipherInit_ex Our openssl_compat.h has for these older OpenSSL versions OpenSSL 3.0: https://github.com/openssl/openssl/blob/openssl-3.2/crypto/evp/evp_enc.c#L450 basically the same as 1.0.2. Just that method names have been changed. Change-Id: I911e25949a8647b567fd4178683534d4404ab469 Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/552 This mail reflects revision 3 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c index bfc5e37..b2c4eb6 100644 --- a/src/openvpn/crypto_openssl.c +++ b/src/openvpn/crypto_openssl.c @@ -846,11 +846,7 @@ evp_cipher_type *kt = cipher_get(ciphername); EVP_CIPHER_CTX_reset(ctx); -if (!EVP_CipherInit(ctx, kt, NULL, NULL, enc)) -{ -crypto_msg(M_FATAL, "EVP cipher init #1"); -} -if (!EVP_CipherInit_ex(ctx, NULL, NULL, key, NULL, enc)) +if (!EVP_CipherInit_ex(ctx, kt, NULL, key, NULL, enc)) { crypto_msg(M_FATAL, "EVP cipher init #2"); } ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2] Remove redundant call of EVP_CipherInit before EVP_CipherInit_Ex
From: Arne Schwabe EVP_CipherInit basically is the same EVP_CipherInit_ex except that it in some instances it resets/inits the ctx parameter first. We already call EVP_CIPHER_CTX_reset to reset/init the ctx before so this call does not do anything useful. OpenSSL 1.0.2: https://github.com/openssl/openssl/blob/OpenSSL_1_0_2-stable/crypto/evp/evp_enc.c#L94 EVP_CipherInit calls first EVP_CIPHER_CTX_init and then EVP_CipherInit_ex Our openssl_compat.h has for these older OpenSSL versions OpenSSL 3.0: https://github.com/openssl/openssl/blob/openssl-3.2/crypto/evp/evp_enc.c#L450 basically the same as 1.0.2. Just that method names have been changed. Change-Id: I911e25949a8647b567fd4178683534d4404ab469 Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/552 This mail reflects revision 2 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c index bfc5e37..13dfa8c 100644 --- a/src/openvpn/crypto_openssl.c +++ b/src/openvpn/crypto_openssl.c @@ -846,10 +846,6 @@ evp_cipher_type *kt = cipher_get(ciphername); EVP_CIPHER_CTX_reset(ctx); -if (!EVP_CipherInit(ctx, kt, NULL, NULL, enc)) -{ -crypto_msg(M_FATAL, "EVP cipher init #1"); -} if (!EVP_CipherInit_ex(ctx, NULL, NULL, key, NULL, enc)) { crypto_msg(M_FATAL, "EVP cipher init #2"); ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Match ifdef for get_sigtype function with if ifdef of caller
ACK, verified that it fixes the GHA build fails we've seen since the recent LibreSSL upgrade on the MacOS builders. Your patch has been applied to the master branch. Not applicable to release/2.6 as the offending code is not in there. commit ff402c7c2fbc49ff6d352ebdc3cdc4c27c2bbcbb (master) Author: Arne Schwabe Date: Tue Apr 2 08:36:46 2024 +0200 Match ifdef for get_sigtype function with if ifdef of caller Signed-off-by: Arne Schwabe Acked-by: Gert Doering Message-Id: <20240402063646.25490-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28512.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v1] Match ifdef for get_sigtype function with if ifdef of caller
From: Arne Schwabe These two ifdef needs to be the same otherwise the compiler will break with a undefined function. Change-Id: I5b14bf90bb07935f0bb84373ec4e62352752c03f Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/551 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c index 6f29c3d..a158617 100644 --- a/src/openvpn/ssl_openssl.c +++ b/src/openvpn/ssl_openssl.c @@ -2166,7 +2166,8 @@ EVP_PKEY_free(pkey); } -#if !defined(LIBRESSL_VERSION_NUMBER) && OPENSSL_VERSION_NUMBER >= 0x101fL +#if (!defined(LIBRESSL_VERSION_NUMBER) && OPENSSL_VERSION_NUMBER >= 0x101fL) \ +|| (defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER >= 0x309fL) /** * Translate an OpenSSL NID into a more human readable name * @param nid ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: crypto_backend: fix type of enc parameter
Lightly tested on a FreeBSD/MbedTLS 2.28.7, resulting code still works :-) Your patch has been applied to the master branch. commit 4d907bf46a470ccbd2940b9ecb64d6502d9d86bf Author: Frank Lichtenheld Date: Wed Mar 27 17:26:21 2024 +0100 crypto_backend: fix type of enc parameter Signed-off-by: Frank Lichtenheld Acked-by: Arne Schwabe Message-Id: <20240327162621.1792414-1-fr...@lichtenheld.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28498.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: misc.c: remove unused code
Your patch has been applied to the master and release/2.6 branch. (Trivial and obvious removal of dead code, passes all tests, and having the code align can be helpful) commit 4c71e816031f564f834df695b3fa717ea22720d2 (master) commit f50c67707ed033040c93a6b5d4efbbd2c0933459 (release/2.6) Author: Lev Stipakov Date: Fri Mar 29 11:37:39 2024 +0100 misc.c: remove unused code Signed-off-by: Lev Stipakov Acked-by: Frank Lichtenheld Message-Id: <20240329103739.28254-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28503.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v1] misc.c: remove unused code
From: Lev Stipakov 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 --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/550 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Frank Lichtenheld diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c index 3ff0857..598fbae 100644 --- a/src/openvpn/misc.c +++ b/src/openvpn/misc.c @@ -491,19 +491,15 @@ } 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 cb3bf68..963f3e6 100644 --- a/src/openvpn/misc.h +++ b/src/openvpn/misc.h @@ -152,26 +152,18 @@ 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 tkauth-token userpass to set * @param token token to use as password for the auth-token * * @noteall 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 7895a37..7c49451 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -340,7 +340,7 @@ void ssl_set_auth_token(const char *token) { -set_auth_token(_user_pass, _token, token); +set_auth_token(_token, token); } void ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: script-options.rst: Update ifconfig_* variables
Acked-by: Gert Doering We're so bad at times at updating documentation... verified that the newly documented options exist and do what it says. Confusing code... Your patch has been applied to the master and release/2.6 branch (doc). commit a94226cdc8ed037a6763675aa47e6c821983f174 (master) commit ea0d9c70a44e3d871136f68bddb0befc299dd692 (release/2.6) Author: Frank Lichtenheld Date: Thu Mar 21 17:16:23 2024 +0100 script-options.rst: Update ifconfig_* variables Signed-off-by: Frank Lichtenheld Acked-by: Gert Doering Message-Id: <20240321161623.2794161-1-fr...@lichtenheld.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28438.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Add bracket in fingerprint message and do not warn about missing verification
Added the Github reference to #516 Your patch has been applied to the master and release/2.6 branch (bugfix). commit 4b95656536be1f402a55ef5dffe140fa78e7eb51 (master) commit e36359aa7e5193ad002768e90ae660896a5a0fa6 (release/2.6) Author: Arne Schwabe Date: Tue Mar 26 11:38:53 2024 +0100 Add bracket in fingerprint message and do not warn about missing verification Signed-off-by: Arne Schwabe Acked-by: Frank Lichtenheld Message-Id: <20240326103853.494572-1-fr...@lichtenheld.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28474.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Fix snprintf/swnprintf related compiler warnings
Lightly stared at code and ran client side tests (that excercise proxy). Your patch has been applied to the master branch. commit 6889d9e2f1458272ded4c035df40378ace3d7395 (master) Author: Arne Schwabe Date: Tue Mar 26 11:41:01 2024 +0100 Fix snprintf/swnprintf related compiler warnings Signed-off-by: Arne Schwabe Acked-by: Frank Lichtenheld Message-Id: <20240326104101.531291-1-fr...@lichtenheld.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28475.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: phase2_tcp_server: fix Coverity issue 'Dereference after null check'
This is arguably a correct fix, though we could go a bit further in terms of refactoring and fully get rid of signal_received - if my understanding of the code is correct, it's only passed to a single function (socket_listen_accept()), which is only called from here - so "just pass on sig_info and use that" would be a bit less convoluted. But that's refactoring, so for future master... Lightly tested on the server framework. Your patch has been applied to the master and release/2.6 branch. commit e8c629fe64c67ea0a8454753be99db44df7ce53e (master) commit 5591af17694d98054da2cdf4cfd42508a8a4fb8e (release/2.6) Author: Frank Lichtenheld Date: Mon Mar 25 08:14:48 2024 +0100 phase2_tcp_server: fix Coverity issue 'Dereference after null check' Signed-off-by: Frank Lichtenheld Acked-by: Arne Schwabe Message-Id: <20240325071448.12143-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28452.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Use snprintf instead of sprintf for get_ssl_library_version
Your patch has been applied to the master and release/2.6 branch (because this is good behaviour, even if we know there can not be an overrun - today). Tested on... Linux, with "library versions: mbed TLS 2.28.7, LZO 2.10" FreeBSD, with "library versions: mbed TLS 3.5.1, LZO 2.10" commit 6a60d1bef424088df55f4d07efd45ce080fc7132 (master) commit 11ca69cfac1c6d3ed34652650688a4b3c99573b0 (release/2.6) Author: Arne Schwabe Date: Mon Mar 25 13:50:52 2024 +0100 Use snprintf instead of sprintf for get_ssl_library_version Signed-off-by: Arne Schwabe Acked-by: Frank Lichtenheld Message-Id: <20240325125052.14135-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28458.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: documentation: make section levels consistent
Your patch has been applied to the master and release/2.6 branch (doc). commit 3fdf5aa04f7b96a3b7110f75306306ac5d7ed5fd (master) commit 7993084c7f2b537e20a0a0d67385733d7d56688c (release/2.6) Author: Frank Lichtenheld Date: Mon Mar 25 08:15:20 2024 +0100 documentation: make section levels consistent Signed-off-by: Frank Lichtenheld Acked-by: Arne Schwabe Message-Id: <20240325071520.12513-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28453.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: samples: Update sample configurations
Thanks for the housekeeping... As discussed on IRC, I've added text about tls-auth being commented out to the commit message. Your patch has been applied to the master and release/2.6 branch (relevant documentation update). commit b0fc10abd06fa2307e95c8a60fa94f7ccc08d2ac (master) commit 371cc5874faf67057c9f95796195306beeba0628 (release/2.6) Author: Frank Lichtenheld Date: Mon Mar 25 08:13:20 2024 +0100 samples: Update sample configurations Signed-off-by: Frank Lichtenheld Acked-by: Arne Schwabe Message-Id: <20240325071320.11348-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28451.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v1] Use snprintf instead of sprintf for get_ssl_library_version
From: Arne Schwabe 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 --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/545 This mail reflects revision 1 of this Change. Signed-off-by line for the author was added as per our policy. Acked-by according to Gerrit (reflected above): Frank Lichtenheld diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index b44ddd5..0730d25 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -1614,7 +1614,7 @@ { 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; } ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v1] Remove openvpn_snprintf and similar functions
From: Arne Schwabe Old Microsoft versions did strange behaviour but according to the newly added unit test and https://stackoverflow.com/questions/7706936/is-snprintf-always-null-terminating this is now standard conforming and we can use the normal snprintf method. Microsoft own documentation to swprintf also says you nowadays need to define _CRT_NON_CONFORMING_SWPRINTFS to get to non-standard behaviour. Change-Id: I07096977e3b562bcb5d2c6f11673a4175b8e12ac Signed-off-by: Arne Schwabe Acked-by: Antonio Quartulli --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/547 This mail reflects revision 1 of this Change. Signed-off-by line for the author was added as per our policy. Acked-by according to Gerrit (reflected above): Antonio Quartulli diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c index 66fd63f..3a8069c 100644 --- a/src/openvpn/buffer.c +++ b/src/openvpn/buffer.c @@ -279,32 +279,6 @@ return ret; } - -/* - * This is necessary due to certain buggy implementations of snprintf, - * that don't guarantee null termination for size > 0. - * - * Return false on overflow. - * - * This functionality is duplicated in src/openvpnserv/common.c - * Any modifications here should be done to the other place as well. - */ - -bool -openvpn_snprintf(char *str, size_t size, const char *format, ...) -{ -va_list arglist; -int len = -1; -if (size > 0) -{ -va_start(arglist, format); -len = vsnprintf(str, size, format, arglist); -va_end(arglist); -str[size - 1] = 0; -} -return (len >= 0 && len < size); -} - /* * write a string to the end of a buffer that was * truncated by buf_printf diff --git a/src/openvpn/buffer.h b/src/openvpn/buffer.h index 7c2f75a..27c3199 100644 --- a/src/openvpn/buffer.h +++ b/src/openvpn/buffer.h @@ -448,19 +448,6 @@ */ bool buf_puts(struct buffer *buf, const char *str); -/* - * Like snprintf but guarantees null termination for size > 0 - */ -bool openvpn_snprintf(char *str, size_t size, const char *format, ...) -#ifdef __GNUC__ -#if __USE_MINGW_ANSI_STDIO -__attribute__ ((format(gnu_printf, 3, 4))) -#else -__attribute__ ((format(__printf__, 3, 4))) -#endif -#endif -; - /* * remove/add trailing characters diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c index 5d05cc4..207f145 100644 --- a/src/openvpn/crypto.c +++ b/src/openvpn/crypto.c @@ -874,11 +874,11 @@ key_direction_state_init(, key_direction); -openvpn_snprintf(log_prefix, sizeof(log_prefix), "Outgoing %s", name); +snprintf(log_prefix, sizeof(log_prefix), "Outgoing %s", name); init_key_ctx(>encrypt, >keys[kds.out_key], kt, OPENVPN_OP_ENCRYPT, log_prefix); -openvpn_snprintf(log_prefix, sizeof(log_prefix), "Incoming %s", name); +snprintf(log_prefix, sizeof(log_prefix), "Incoming %s", name); init_key_ctx(>decrypt, >keys[kds.in_key], kt, OPENVPN_OP_DECRYPT, log_prefix); diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c index 1a39752..c806719 100644 --- a/src/openvpn/crypto_mbedtls.c +++ b/src/openvpn/crypto_mbedtls.c @@ -128,7 +128,7 @@ { char prefix[256]; -if (!openvpn_snprintf(prefix, sizeof(prefix), "%s:%d", func, line)) +if (!snprintf(prefix, sizeof(prefix), "%s:%d", func, line)) { return mbed_log_err(flags, errval, func); } @@ -239,11 +239,11 @@ char header[1000+1] = { 0 }; char footer[1000+1] = { 0 }; -if (!openvpn_snprintf(header, sizeof(header), "-BEGIN %s-\n", name)) +if (!snprintf(header, sizeof(header), "-BEGIN %s-\n", name)) { return false; } -if (!openvpn_snprintf(footer, sizeof(footer), "-END %s-\n", name)) +if (!snprintf(footer, sizeof(footer), "-END %s-\n", name)) { return false; } @@ -278,11 +278,11 @@ char header[1000+1] = { 0 }; char footer[1000+1] = { 0 }; -if (!openvpn_snprintf(header, sizeof(header), "-BEGIN %s-", name)) +if (!snprintf(header, sizeof(header), "-BEGIN %s-", name)) { return false; } -if (!openvpn_snprintf(footer, sizeof(footer), "-END %s-", name)) +if (!snprintf(footer, sizeof(footer), "-END %s-", name)) { return false; } diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c index 7de3991..0539ca5 100644 --- a/src/openvpn/dns.c +++ b/src/openvpn/dns.c @@ -349,11 +349,11 @@ if (j < 0) { -name_ok = openvpn_snprintf(name, sizeof(name), format, i); +name_ok = snprintf(name, sizeof(name), format, i); } else { -name_ok = openvpn_snprintf(name, sizeof(name), format, i, j); +name_ok = snprintf(name, sizeof(name), format, i, j); } if (!name_ok) diff --git a/src/openvpn/env_set.c b/src/openvpn/env_set.c index
[Openvpn-devel] [PATCH v2] documentation: make section levels consistent
From: Frank Lichtenheld 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 --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/527 This mail reflects revision 2 of this Change. Acked-by according to Gerrit (reflected above): Arne Schwabe diff --git a/doc/man-sections/cipher-negotiation.rst b/doc/man-sections/cipher-negotiation.rst index 949ff86..1285e82 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 @@ ``--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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 3b26782..49385d6 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 @@ ``--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 de1662b..dfc27af 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
[Openvpn-devel] [PATCH v2] phase2_tcp_server: fix Coverity issue "Dereference after null check"
From: Frank Lichtenheld 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 --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/490 This mail reflects revision 2 of this Change. Acked-by according to Gerrit (reflected above): Arne Schwabe diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c index 480f4e5..387cb40 100644 --- a/src/openvpn/socket.c +++ b/src/openvpn/socket.c @@ -2005,7 +2005,8 @@ phase2_tcp_server(struct link_socket *sock, const char *remote_dynamic, struct signal_info *sig_info) { -volatile int *signal_received = sig_info ? _info->signal_received : NULL; +ASSERT(sig_info); +volatile int *signal_received = _info->signal_received; switch (sock->mode) { case LS_MODE_DEFAULT: @@ -2031,7 +2032,7 @@ false); if (!socket_defined(sock->sd)) { -register_signal(sig_info, SIGTERM, "socket-undefiled"); +register_signal(sig_info, SIGTERM, "socket-undefined"); return; } tcp_connection_established(>info.lsa->actual); ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel