openvpn-devel Mailing List for OpenVPN

> Hi, > >> The OpenVPN community project team is proud to release OpenVPN 2.7_rc3. >> >> This is the third release candidate for the feature release 2.7.0. >> > > Can one of the developers please add this little patch? > > https://github.com/OpenVPN/openvpn/issues/834 > > It would be much appreciated. The patch is attached to this email. Would be nice if it was considered. Regards, Simon

Hi, you might have seen 2.6.17 and 2.7_rc3 releases yesterday, mentioning - Windows/Interactive Service: CVE-2025-13751	fix bug where the interactive service would error-exit in	certain error conditions instead of just logging the fact and	continuing. After the error-exit, OpenVPN connections will no	longer work until the service is restarted (or the system rebooted).	This can be triggered by any authenticated local user, and has	thus been classified as a "local denial of service" attack. as usual for bugs that we classify as "we do not want them to leak before a fixed installer is out" the fix was developed & tested & reviewed in private (Lev, Selva, Heiko). Also, for the usual maximum transparency, I'll hereby send it to the list - attached is the 2.7 patch, which is commit 6088451b616be313bd47cfed8d82d47bd6e17d93 Author: Lev Stipakov <le...@op...> Date: Mon Nov 24 12:09:23 2025 +0200 interactive.c: harden pipe handling against misbehaving clients the 2.6 patch is basically the same change, but the differences are wide character text handling (TEXT("") vs. L"") between 2.6 and 2.7 commit 29d8eccface918703b91dde230285ca869f95090 (release/2.6) Author: Lev Stipakov <le...@op...> Date: Mon Nov 24 12:09:23 2025 +0200 interactive.c: harden pipe handling against misbehaving clients and, finally, the 2.5 patch is a direct cherrypick from 2.6 commit 1f4b444e1407450a1071a633d73d2a63a0af06c2 (release/2.5) Author: Lev Stipakov <le...@op...> Date: Mon Nov 24 12:09:23 2025 +0200 interactive.c: harden pipe handling against misbehaving clients Note that we do not intend to publish new 2.5.x windows installers - but if someone is building their product on the 2.5 source tree, the fix is in. 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 ge...@gr... 

Attention is currently required from: flichtenheld, plaisthos. MaxF has posted comments on this change by MaxF. ( http://gerrit.openvpn.net/c/openvpn/+/1412?usp=email ) Change subject: Drop Mbed TLS 2.X compatibility ...................................................................... Patch Set 5: (1 comment) Patchset: PS3: > Please also remove the Ubuntu mbedTLS builds in .github/workflows/build. […] Done. I'm not familiar with github actions, so I hope this works. -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1412?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Ia4afabcb6006dc9304a4c09f824d9c7c2d4d64ad Gerrit-Change-Number: 1412 Gerrit-PatchSet: 5 Gerrit-Owner: MaxF <ma...@ma...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Comment-Date: Sat, 29 Nov 2025 13:10:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: flichtenheld <fr...@li...> 

Attention is currently required from: MaxF, flichtenheld, plaisthos. Hello flichtenheld, plaisthos, I'd like you to reexamine a change. Please visit http://gerrit.openvpn.net/c/openvpn/+/1412?usp=email to look at the new patch set (#5). The following approvals got outdated and were removed: Code-Review-1 by flichtenheld Change subject: Drop Mbed TLS 2.X compatibility ...................................................................... Drop Mbed TLS 2.X compatibility Mbed TLS 2.28 is out of support since March and adding support for Mbed TLS 4 will get ugly enough without the old compatibility code lying around too. Mbed TLS 2.28 still ships on some supported distributions (e.g. Ubuntu 24.04) but nobody is maintaining openvpn-mbedtls packages there. This commit will probably break on some test machines. Change-Id: Ia4afabcb6006dc9304a4c09f824d9c7c2d4d64ad Signed-off-by: Max Fillinger <max...@se...> --- M .github/workflows/build.yaml M CMakeLists.txt M README.mbedtls M config.h.cmake.in M configure.ac M src/openvpn/crypto_mbedtls.c M src/openvpn/mbedtls_compat.h M src/openvpn/ssl_mbedtls.c M src/openvpn/ssl_mbedtls.h M src/openvpn/ssl_verify_mbedtls.c 10 files changed, 37 insertions(+), 327 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/12/1412/5 diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 357072d..ea45740 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -136,21 +136,16 @@ fail-fast: false matrix: os: [ubuntu-22.04, ubuntu-24.04] - sslpkg: [libmbedtls-dev] - ssllib: [mbedtls] - libname: [mbed TLS] + sslpkg: [libssl-dev] + ssllib: [openssl] include: - os: ubuntu-22.04 - sslpkg: "libssl-dev" libname: OpenSSL 3.0.2 - ssllib: openssl pkcs11pkg: "libpkcs11-helper1-dev softhsm2 gnutls-bin" extraconf: --enable-pkcs11 - os: ubuntu-24.04 - sslpkg: "libssl-dev" libname: OpenSSL 3.0.13 - ssllib: openssl pkcs11pkg: "libpkcs11-helper1-dev softhsm2 gnutls-bin" extraconf: --enable-pkcs11 @@ -182,7 +177,7 @@ fail-fast: false matrix: os: [ubuntu-22.04, ubuntu-24.04] - ssllib: [mbedtls, openssl] + ssllib: [openssl] name: "clang-asan - ${{matrix.os}} - ${{matrix.ssllib}}" @@ -192,7 +187,7 @@ runs-on: ${{matrix.os}} steps: - name: Install dependencies - run: sudo apt update && sudo apt install -y liblzo2-dev libpam0g-dev liblz4-dev libcap-ng-dev libnl-genl-3-dev linux-libc-dev man2html clang libcmocka-dev python3-docutils libtool automake autoconf libmbedtls-dev + run: sudo apt update && sudo apt install -y liblzo2-dev libpam0g-dev liblz4-dev libcap-ng-dev libnl-genl-3-dev linux-libc-dev man2html clang libcmocka-dev python3-docutils libtool automake autoconf - name: Checkout OpenVPN uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 - name: autoconf diff --git a/CMakeLists.txt b/CMakeLists.txt index e812145..8a8054f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -305,10 +305,6 @@ set(CMAKE_REQUIRED_LINK_OPTIONS "-L${MBED_LIBRARY_PATH}") endif () set(CMAKE_REQUIRED_LIBRARIES "mbedtls;mbedx509;mbedcrypto") - check_symbol_exists(mbedtls_ctr_drbg_update_ret mbedtls/ctr_drbg.h HAVE_MBEDTLS_CTR_DRBG_UPDATE_RET) - check_symbol_exists(mbedtls_ssl_conf_export_keys_ext_cb mbedtls/ssl.h HAVE_MBEDTLS_SSL_CONF_EXPORT_KEYS_EXT_CB) - check_symbol_exists(mbedtls_ssl_set_export_keys_cb mbedtls/ssl.h HAVE_MBEDTLS_SSL_SET_EXPORT_KEYS_CB) - check_symbol_exists(mbedtls_ssl_tls_prf mbedtls/ssl.h HAVE_MBEDTLS_SSL_TLS_PRF) check_include_files(psa/crypto.h HAVE_PSA_CRYPTO_H) endfunction() diff --git a/README.mbedtls b/README.mbedtls index a1012e9..fb30db1 100644 --- a/README.mbedtls +++ b/README.mbedtls @@ -7,7 +7,8 @@	make	make install -This version requires mbed TLS version >= 2.0.0 or >= 3.2.1. +This version requires mbed TLS version >= 3.2.1. Versions >= 4.0.0 are not +yet supported. Support for TLS 1.3 requires an Mbed TLS version >= 3.6.4. ************************************************************************* @@ -23,12 +24,3 @@ * X.509 subject line has a different format than the OpenSSL subject line * X.509 certificate tracking - -************************************************************************* - -Mbed TLS 3 has implemented TLS 1.3, but support in OpenVPN requires the -function mbedtls_ssl_export_keying_material() which is currently not in -any released version. It is available when building mbed TLS from source -(mbedtls-3.6 or development branch). - -Without this function, only TLS 1.2 is available. diff --git a/config.h.cmake.in b/config.h.cmake.in index 1c443ab..c90d124 100644 --- a/config.h.cmake.in +++ b/config.h.cmake.in @@ -371,10 +371,6 @@ /* Availability of different mbed TLS features and APIs */ #cmakedefine HAVE_PSA_CRYPTO_H -#cmakedefine HAVE_MBEDTLS_SSL_SET_EXPORT_KEYS_CB -#cmakedefine HAVE_MBEDTLS_SSL_CONF_EXPORT_KEYS_EXT_CB -#cmakedefine HAVE_MBEDTLS_CTR_DRBG_UPDATE_RET -#cmakedefine HAVE_MBEDTLS_SSL_TLS_PRF /* Path to ifconfig tool */ #define IFCONFIG_PATH "@IFCONFIG_PATH@" diff --git a/configure.ac b/configure.ac index 44c7b65..a6f79ae 100644 --- a/configure.ac +++ b/configure.ac @@ -995,7 +995,7 @@	if test -z "${MBEDTLS_CFLAGS}" -a -z "${MBEDTLS_LIBS}"; then	# if the user did not explicitly specify flags, try to autodetect	PKG_CHECK_MODULES([MBEDTLS], -	[mbedtls >= 2.0.0 mbedx509 >= 2.0.0 mbedcrypto >= 2.0.0], +	[mbedtls >= 3.2.1 mbedx509 >= 3.2.1 mbedcrypto >= 3.2.1],	[have_mbedtls="yes"],	[LIBS="${LIBS} -lmbedtls -lmbedx509 -lmbedcrypto"]	) @@ -1020,35 +1020,17 @@ #include <mbedtls/version.h>	]],	[[ -#if MBEDTLS_VERSION_NUMBER < 0x02000000 || (MBEDTLS_VERSION_NUMBER >= 0x03000000 && MBEDTLS_VERSION_NUMBER < 0x03020100) +#if MBEDTLS_VERSION_NUMBER < 0x03020100 #error invalid version #endif	]]	)],	[AC_MSG_RESULT([ok])], -	[AC_MSG_ERROR([mbed TLS version >= 2.0.0 or >= 3.2.1 required])] +	[AC_MSG_ERROR([mbed TLS version >= 3.2.1 required])]	)	AC_CHECK_HEADERS(psa/crypto.h) -	AC_CHECK_FUNCS([mbedtls_ssl_tls_prf mbedtls_ssl_conf_export_keys_ext_cb]) - -	if test "x$ac_cv_func_mbedtls_ssl_conf_export_keys_ext_cb" != xyes; then -	AC_CHECK_FUNCS([mbedtls_ssl_set_export_keys_cb]) -	if test "x$ac_cv_func_mbedtls_ssl_set_export_keys_cb" != xyes; then -	AC_CHECK_FUNC([mbedtls_ssl_export_keying_material]) -	if test "x$ac_cv_func_mbedtls_ssl_export_keying_material" != xyes; then -	AC_MSG_ERROR(This version of mbed TLS has no support for exporting key material.) -	fi -	fi -	fi - -	AC_CHECK_FUNC( -	[mbedtls_ctr_drbg_update_ret], -	AC_DEFINE([HAVE_MBEDTLS_CTR_DRBG_UPDATE_RET], [1], - [Use mbedtls_ctr_drbg_update_ret from mbed TLS]), -	) -	CFLAGS="${saved_CFLAGS}"	LIBS="${saved_LIBS}"	AC_DEFINE([ENABLE_CRYPTO_MBEDTLS], [1], [Use mbed TLS library]) diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c index 89f0ab9..6688d48 100644 --- a/src/openvpn/crypto_mbedtls.c +++ b/src/openvpn/crypto_mbedtls.c @@ -41,7 +41,6 @@ #include "integer.h" #include "crypto_backend.h" #include "otime.h" -#include "mbedtls_compat.h" #include "misc.h" #include <mbedtls/base64.h> @@ -987,17 +986,7 @@ return diff; } -/* mbedtls-2.18.0 or newer implements tls_prf, but prf_tls1 is removed - * from recent versions, so we use our own implementation if necessary. */ -#if defined(HAVE_MBEDTLS_SSL_TLS_PRF) && defined(MBEDTLS_SSL_TLS_PRF_TLS1) -bool -ssl_tls1_PRF(const uint8_t *seed, size_t seed_len, const uint8_t *secret, size_t secret_len, - uint8_t *output, size_t output_len) -{ - return mbed_ok(mbedtls_ssl_tls_prf(MBEDTLS_SSL_TLS_PRF_TLS1, secret, secret_len, "", seed, - seed_len, output, output_len)); -} -#else /* defined(HAVE_MBEDTLS_SSL_TLS_PRF) && defined(MBEDTLS_SSL_TLS_PRF_TLS1) */ + #if defined(__GNUC__) || defined(__clang__) #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wconversion" @@ -1135,6 +1124,5 @@ #if defined(__GNUC__) || defined(__clang__) #pragma GCC diagnostic pop #endif -#endif /* HAVE_MBEDTLS_SSL_TLS_PRF && defined(MBEDTLS_SSL_TLS_PRF_TLS1) */ #endif /* ENABLE_CRYPTO_MBEDTLS */ diff --git a/src/openvpn/mbedtls_compat.h b/src/openvpn/mbedtls_compat.h index 68096c4..540f370 100644 --- a/src/openvpn/mbedtls_compat.h +++ b/src/openvpn/mbedtls_compat.h @@ -23,10 +23,8 @@ /** * @file * mbedtls compatibility stub. - * This file provide compatibility stubs for the mbedtls libraries - * prior to version 3. This version made most fields in structs private - * and requires accessor functions to be used. For earlier versions, we - * implement the accessor functions here. + * This file provides compatibility stubs to handle API differences between + * different versions of Mbed TLS. */ #ifndef MBEDTLS_COMPAT_H_ @@ -36,27 +34,10 @@ #include "errlevel.h" -#include <mbedtls/cipher.h> -#include <mbedtls/ctr_drbg.h> -#include <mbedtls/dhm.h> -#include <mbedtls/ecp.h> -#include <mbedtls/md.h> -#include <mbedtls/pem.h> -#include <mbedtls/pk.h> -#include <mbedtls/ssl.h> -#include <mbedtls/version.h> -#include <mbedtls/x509_crt.h> - #ifdef HAVE_PSA_CRYPTO_H #include <psa/crypto.h> #endif -#if MBEDTLS_VERSION_NUMBER >= 0x03000000 -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) { @@ -70,162 +51,4 @@ #endif } -static inline mbedtls_compat_group_id -mbedtls_compat_get_group_id(const mbedtls_ecp_curve_info *curve_info) -{ -#if MBEDTLS_VERSION_NUMBER >= 0x03000000 - 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() - * which does. - * - * In mbedtls 3, this function was removed and mbedtls_ctr_drbg_update() returns - * an error code. - */ -static inline int -mbedtls_compat_ctr_drbg_update(mbedtls_ctr_drbg_context *ctx, const unsigned char *additional, - size_t add_len) -{ -#if MBEDTLS_VERSION_NUMBER > 0x03000000 - return mbedtls_ctr_drbg_update(ctx, additional, add_len); -#elif defined(HAVE_MBEDTLS_CTR_DRBG_UPDATE_RET) - return mbedtls_ctr_drbg_update_ret(ctx, additional, add_len); -#else - mbedtls_ctr_drbg_update(ctx, additional, add_len); - return 0; -#endif /* HAVE_MBEDTLS_CTR_DRBG_UPDATE_RET */ -} - -static inline int -mbedtls_compat_pk_check_pair(const mbedtls_pk_context *pub, const mbedtls_pk_context *prv, - int (*f_rng)(void *, unsigned char *, size_t), void *p_rng) -{ -#if MBEDTLS_VERSION_NUMBER < 0x03020100 - return mbedtls_pk_check_pair(pub, prv); -#else - return mbedtls_pk_check_pair(pub, prv, f_rng, p_rng); -#endif /* MBEDTLS_VERSION_NUMBER < 0x03020100 */ -} - -static inline int -mbedtls_compat_pk_parse_key(mbedtls_pk_context *ctx, const unsigned char *key, size_t keylen, - const unsigned char *pwd, size_t pwdlen, - int (*f_rng)(void *, unsigned char *, size_t), void *p_rng) -{ -#if MBEDTLS_VERSION_NUMBER < 0x03020100 - return mbedtls_pk_parse_key(ctx, key, keylen, pwd, pwdlen); -#else - return mbedtls_pk_parse_key(ctx, key, keylen, pwd, pwdlen, f_rng, p_rng); -#endif -} - -static inline int -mbedtls_compat_pk_parse_keyfile(mbedtls_pk_context *ctx, const char *path, const char *password, - int (*f_rng)(void *, unsigned char *, size_t), void *p_rng) -{ -#if MBEDTLS_VERSION_NUMBER < 0x03020100 - return mbedtls_pk_parse_keyfile(ctx, path, password); -#else - return mbedtls_pk_parse_keyfile(ctx, path, password, f_rng, p_rng); -#endif -} - -#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) -{ - return (size_t)cipher->block_size; -} - -static inline size_t -mbedtls_cipher_info_get_iv_size(const mbedtls_cipher_info_t *cipher) -{ - return (size_t)cipher->iv_size; -} - -static inline size_t -mbedtls_cipher_info_get_key_bitlen(const mbedtls_cipher_info_t *cipher) -{ - return (size_t)cipher->key_bitlen; -} - -static inline mbedtls_cipher_mode_t -mbedtls_cipher_info_get_mode(const mbedtls_cipher_info_t *cipher) -{ - return cipher->mode; -} - -static inline const char * -mbedtls_cipher_info_get_name(const mbedtls_cipher_info_t *cipher) -{ - return cipher->name; -} - -static inline mbedtls_cipher_type_t -mbedtls_cipher_info_get_type(const mbedtls_cipher_info_t *cipher) -{ - return cipher->type; -} - -static inline size_t -mbedtls_dhm_get_bitlen(const mbedtls_dhm_context *ctx) -{ - return 8 * ctx->len; -} - -static inline const mbedtls_md_info_t * -mbedtls_md_info_from_ctx(const mbedtls_md_context_t *ctx) -{ - return ctx->md_info; -} - -static inline const unsigned char * -mbedtls_pem_get_buffer(const mbedtls_pem_context *ctx, size_t *buf_size) -{ - *buf_size = ctx->buflen; - return ctx->buf; -} - -static inline int -mbedtls_x509_crt_has_ext_type(const mbedtls_x509_crt *ctx, int ext_type) -{ - return ctx->ext_types & ext_type; -} -#endif /* MBEDTLS_VERSION_NUMBER < 0x03020100 */ - #endif /* MBEDTLS_COMPAT_H_ */ diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index 488f9b9..83fca78 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -49,13 +49,8 @@ #include "ssl_verify_mbedtls.h" #include <mbedtls/debug.h> #include <mbedtls/error.h> -#include <mbedtls/version.h> - -#if MBEDTLS_VERSION_NUMBER >= 0x02040000 #include <mbedtls/net_sockets.h> -#else -#include <mbedtls/net.h> -#endif +#include <mbedtls/version.h> #include <mbedtls/oid.h> #include <mbedtls/pem.h> @@ -165,50 +160,14 @@ ASSERT(NULL != ctx); return ctx->initialised; } -#ifdef MBEDTLS_SSL_KEYING_MATERIAL_EXPORT -/* mbedtls_ssl_export_keying_material does not need helper/callback methods */ -#elif defined(HAVE_MBEDTLS_SSL_CONF_EXPORT_KEYS_EXT_CB) +#if !defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT) /* - * Key export callback for older versions of mbed TLS, to be used with - * mbedtls_ssl_conf_export_keys_ext_cb(). It is called with the master - * secret, client random and server random, and the type of PRF function - * to use. - * - * Mbed TLS stores this callback in the mbedtls_ssl_config struct and it - * is used in the mbedtls_ssl_contexts set up from that config. */ -int -mbedtls_ssl_export_keys_cb(void *p_expkey, const unsigned char *ms, const unsigned char *kb, - size_t maclen, size_t keylen, size_t ivlen, - const unsigned char client_random[32], - const unsigned char server_random[32], - mbedtls_tls_prf_types tls_prf_type) -{ - struct tls_session *session = p_expkey; - struct key_state_ssl *ks_ssl = &session->key[KS_PRIMARY].ks_ssl; - struct tls_key_cache *cache = &ks_ssl->tls_key_cache; - - static_assert(sizeof(ks_ssl->ctx->session->master) == sizeof(cache->master_secret), - "master size mismatch"); - - memcpy(cache->client_server_random, client_random, 32); - memcpy(cache->client_server_random + 32, server_random, 32); - memcpy(cache->master_secret, ms, sizeof(cache->master_secret)); - cache->tls_prf_type = tls_prf_type; - - return 0; -} -#elif defined(HAVE_MBEDTLS_SSL_SET_EXPORT_KEYS_CB) -/* - * Key export callback for newer versions of mbed TLS, to be used with - * mbedtls_ssl_set_export_keys_cb(). When used with TLS 1.2, the callback - * is called with the TLS 1.2 master secret, client random, server random - * and the type of PRF to use. With TLS 1.3, it is called with several - * different keys (indicated by type), but unfortunately not the exporter - * master secret. - * - * Unlike in older versions, the callback is not stored in the - * mbedtls_ssl_config. It is placed in the mbedtls_ssl_context after it - * has been set up. */ + * If we don't have mbedtls_ssl_export_keying_material(), we use + * mbedtls_ssl_set_export_keys_cb() to obtain a copy of the TLS 1.2 + * master secret and compute the TLS-Exporter function ourselves. + * Unfortunately, with TLS 1.3, there is no alternative to + * mbedtls_ssl_export_keying_material(). + */ void mbedtls_ssl_export_keys_cb(void *p_expkey, mbedtls_ssl_key_export_type type, const unsigned char *secret, size_t secret_len, @@ -240,9 +199,7 @@ memcpy(cache->master_secret, secret, sizeof(cache->master_secret)); cache->tls_prf_type = tls_prf_type; } -#else /* ifdef MBEDTLS_SSL_KEYING_MATERIAL_EXPORT */ -#error mbedtls_ssl_conf_export_keys_ext_cb, mbedtls_ssl_set_export_keys_cb or mbedtls_ssl_export_keying_material must be available in mbed TLS -#endif /* HAVE_MBEDTLS_SSL_CONF_EXPORT_KEYS_EXT_CB */ +#endif /* !defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT) */ bool @@ -397,7 +354,7 @@ /* Get number of groups and allocate an array in ctx */ int groups_count = get_num_elements(groups, ':'); - ALLOC_ARRAY_CLEAR(ctx->groups, mbedtls_compat_group_id, groups_count + 1) + ALLOC_ARRAY_CLEAR(ctx->groups, uint16_t, groups_count + 1) /* Parse allowed ciphers, getting IDs */ int i = 0; @@ -413,7 +370,7 @@ } else { - ctx->groups[i] = mbedtls_compat_get_group_id(ci); + ctx->groups[i] = ci->tls_id; i++; } } @@ -537,29 +494,29 @@ if (priv_key_inline) { - status = mbedtls_compat_pk_parse_key(ctx->priv_key, (const unsigned char *)priv_key_file, - strlen(priv_key_file) + 1, NULL, 0, - mbedtls_ctr_drbg_random, rand_ctx_get()); + status = mbedtls_pk_parse_key(ctx->priv_key, (const unsigned char *)priv_key_file, + strlen(priv_key_file) + 1, NULL, 0, + mbedtls_ctr_drbg_random, rand_ctx_get()); if (MBEDTLS_ERR_PK_PASSWORD_REQUIRED == status) { char passbuf[512] = { 0 }; pem_password_callback(passbuf, 512, 0, NULL); - status = mbedtls_compat_pk_parse_key( + status = mbedtls_pk_parse_key( ctx->priv_key, (const unsigned char *)priv_key_file, strlen(priv_key_file) + 1, (unsigned char *)passbuf, strlen(passbuf), mbedtls_ctr_drbg_random, rand_ctx_get()); } } else { - status = mbedtls_compat_pk_parse_keyfile(ctx->priv_key, priv_key_file, NULL, - mbedtls_ctr_drbg_random, rand_ctx_get()); + status = mbedtls_pk_parse_keyfile(ctx->priv_key, priv_key_file, NULL, + mbedtls_ctr_drbg_random, rand_ctx_get()); if (MBEDTLS_ERR_PK_PASSWORD_REQUIRED == status) { char passbuf[512] = { 0 }; pem_password_callback(passbuf, 512, 0, NULL); - status = mbedtls_compat_pk_parse_keyfile(ctx->priv_key, priv_key_file, passbuf, - mbedtls_ctr_drbg_random, rand_ctx_get()); + status = mbedtls_pk_parse_keyfile(ctx->priv_key, priv_key_file, passbuf, + mbedtls_ctr_drbg_random, rand_ctx_get()); } } if (!mbed_ok(status)) @@ -575,8 +532,8 @@ return 1; } - if (!mbed_ok(mbedtls_compat_pk_check_pair(&ctx->crt_chain->pk, ctx->priv_key, - mbedtls_ctr_drbg_random, rand_ctx_get()))) + if (!mbed_ok(mbedtls_pk_check_pair(&ctx->crt_chain->pk, ctx->priv_key, + mbedtls_ctr_drbg_random, rand_ctx_get()))) { msg(M_WARN, "Private key does not match the certificate"); return 1; @@ -610,9 +567,6 @@ */ static inline int external_pkcs1_sign(void *ctx_voidptr, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng, -#if MBEDTLS_VERSION_NUMBER < 0x03020100 - int mode, -#endif mbedtls_md_type_t md_alg, unsigned int hashlen, const unsigned char *hash, unsigned char *sig) { @@ -627,13 +581,6 @@ return MBEDTLS_ERR_RSA_BAD_INPUT_DATA; } -#if MBEDTLS_VERSION_NUMBER < 0x03020100 - if (MBEDTLS_RSA_PRIVATE != mode) - { - return MBEDTLS_ERR_RSA_BAD_INPUT_DATA; - } -#endif - /* * Support a wide range of hashes. TLSv1.1 and before only need SIG_RSA_RAW, * but TLSv1.2 needs the full suite of hashes. @@ -1000,7 +947,7 @@ if (0 != memcmp(old_sha256_hash, sha256_hash, sizeof(sha256_hash))) { - if (!mbed_ok(mbedtls_compat_ctr_drbg_update(cd_ctx, sha256_hash, 32))) + if (!mbed_ok(mbedtls_ctr_drbg_update(cd_ctx, sha256_hash, 32))) { msg(M_WARN, "WARNING: failed to personalise random, could not update CTR_DRBG"); } @@ -1204,12 +1151,6 @@ mbedtls_ssl_conf_max_tls_version(ks_ssl->ssl_config, version); } -#if defined(HAVE_MBEDTLS_SSL_CONF_EXPORT_KEYS_EXT_CB) \ - && !defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT) - /* Initialize keying material exporter, old style. */ - mbedtls_ssl_conf_export_keys_ext_cb(ks_ssl->ssl_config, mbedtls_ssl_export_keys_cb, session); -#endif - /* Initialise SSL context */ ALLOC_OBJ_CLEAR(ks_ssl->ctx, mbedtls_ssl_context); mbedtls_ssl_init(ks_ssl->ctx); @@ -1219,8 +1160,8 @@ * verification. */ ASSERT(mbed_ok(mbedtls_ssl_set_hostname(ks_ssl->ctx, NULL))); -#if defined(HAVE_MBEDTLS_SSL_SET_EXPORT_KEYS_CB) && !defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT) - /* Initialize keying material exporter, new style. */ +#if !defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT) + /* Initialize the keying material exporter callback. */ mbedtls_ssl_set_export_keys_cb(ks_ssl->ctx, mbedtls_ssl_export_keys_cb, session); #endif diff --git a/src/openvpn/ssl_mbedtls.h b/src/openvpn/ssl_mbedtls.h index 0f85d96..8380a116 100644 --- a/src/openvpn/ssl_mbedtls.h +++ b/src/openvpn/ssl_mbedtls.h @@ -39,8 +39,6 @@ #include <pkcs11-helper-1.0/pkcs11h-certificate.h> #endif -#include "mbedtls_compat.h" - typedef struct _buffer_entry buffer_entry; struct _buffer_entry @@ -130,7 +128,7 @@ #endif struct external_context external_key; /**< External key context */ int *allowed_ciphers; /**< List of allowed ciphers for this connection */ - mbedtls_compat_group_id *groups; /**< List of allowed groups for this connection */ + uint16_t *groups; /**< List of allowed groups for this connection */ mbedtls_x509_crt_profile cert_profile; /**< Allowed certificate types */ }; diff --git a/src/openvpn/ssl_verify_mbedtls.c b/src/openvpn/ssl_verify_mbedtls.c index 80ef837..250c806 100644 --- a/src/openvpn/ssl_verify_mbedtls.c +++ b/src/openvpn/ssl_verify_mbedtls.c @@ -35,7 +35,6 @@ #if defined(ENABLE_CRYPTO_MBEDTLS) #include "crypto_mbedtls.h" -#include "mbedtls_compat.h" #include "ssl_verify.h" #include <mbedtls/asn1.h> #include <mbedtls/error.h> -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1412?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Ia4afabcb6006dc9304a4c09f824d9c7c2d4d64ad Gerrit-Change-Number: 1412 Gerrit-PatchSet: 5 Gerrit-Owner: MaxF <ma...@ma...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Attention: MaxF <ma...@ma...> 

Attention is currently required from: plaisthos. Hello plaisthos, I'd like you to do a code review. Please visit http://gerrit.openvpn.net/c/openvpn/+/1413?usp=email to review the following change. Change subject: Set UTF-8 as the codepage using manifest declaration ...................................................................... Set UTF-8 as the codepage using manifest declaration Works on Windows 10+. This ensures any UTF-8 string we pass to ANSI API will not get interpreted in some local code page in effect at runtime. Although we do not use any such API, OpenSSL dll we link to does (e.g., the store API used for reading certificate and key from files). OpenSSL may fix this in future versions, but this is an easy workaround that looks harmless and appropriate. Fixes failure to read certificates when filenames contain non-ascii characters reported by: Carsten Mietzsch <Car...@at...> Ref: https://learn.microsoft.com/en-us/windows/apps/design/globalizing/use-utf8-code-page Change-Id: Ic4e233c788b16a862c1fddcf314a3da661072fb5 Signed-off-by: Selva Nair <sel...@gm...> --- M src/openvpn/openvpn.manifest 1 file changed, 5 insertions(+), 0 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/13/1413/1 diff --git a/src/openvpn/openvpn.manifest b/src/openvpn/openvpn.manifest index fa5b3d7..f964152 100644 --- a/src/openvpn/openvpn.manifest +++ b/src/openvpn/openvpn.manifest @@ -14,6 +14,11 @@ <supportedOS Id="{e2011457-1546-43c5-a5fe-008deee3d3f0}"/> </application> </compatibility> + <application> + <windowsSettings> + <activeCodePage xmlns="http://schemas.microsoft.com/SMI/2019/WindowsSettings">UTF-8</activeCodePage> + </windowsSettings> + </application> <trustInfo xmlns="urn:schemas-microsoft-com:asm.v3"> <security> <requestedPrivileges> -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1413?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Ic4e233c788b16a862c1fddcf314a3da661072fb5 Gerrit-Change-Number: 1413 Gerrit-PatchSet: 1 Gerrit-Owner: selvanair <sel...@gm...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> 

Attention is currently required from: MaxF, plaisthos. flichtenheld has posted comments on this change by MaxF. ( http://gerrit.openvpn.net/c/openvpn/+/1412?usp=email ) Change subject: Drop Mbed TLS 2.X compatibility ...................................................................... Patch Set 3: Code-Review-1 (1 comment) Patchset: PS3: Please also remove the Ubuntu mbedTLS builds in .github/workflows/build.yml -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1412?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Ia4afabcb6006dc9304a4c09f824d9c7c2d4d64ad Gerrit-Change-Number: 1412 Gerrit-PatchSet: 3 Gerrit-Owner: MaxF <ma...@ma...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: MaxF <ma...@ma...> Gerrit-Comment-Date: Fri, 28 Nov 2025 20:28:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes 

The OpenVPN community project team is proud to release OpenVPN 2.7_rc3. This is the third release candidate for the feature release 2.7.0. Security fixes: * CVE-2025-13751: Windows/interactive service: fix bug where the interactive service would error-exit in certain error conditions instead of just logging the fact and continuing. After the error-exit, OpenVPN connections will no longer work until the service is restarted (or the system rebooted). This can be triggered by any authenticated local user, and has thus been classified as a "local denial of service" attack. Important bug fixes since 2.7_rc2: * Windows/Interactive Service bugfixes: many small bugfixes to registry-related DNS domain handling * Windows/Interactive Service: harden service pipe handling close a small race condition, and add restrictive ACLs * more type conversion related warnings have been fixed * --multihome behaviour regarding egress interface selection has been changed. See Changes.rst and manpage for details. * cleanup dead code in event handling code (leftover of the multisocket patch set) * add new feature, --tls-crypt-v2-max-age n. See Changes.rst and manpage for details. * improve documentation to point out the pitfalls of case-insensitive filesystems and --client-config-dir * split default gateway query logic in two: * for --redirect-gateway functionality, query for the gateway towards the actual IP address of the VPN server connecting to * for the "net_gateway" special destination for --route, and the corresponding environment variable, always query for 0.0.0.0 / :: (this will only make a difference in certain scenarios using a local proxy, or on a system with multiple interfaces, not using the "default route" for the VPN connection * see github#890) * upgrade embedded pkcs11-helper vcpkg + pkcs11-uri patch to 1.31 * CMake / autoconf cleanup wrt unused checks, outdated old-Linux checks, Windows oddities * DCO (primarily Linux): improve handling of bulk notifications from kernel (do not lose notifications, do not crash) (github#900) More details can be found in the Changes document: <https://github.com/OpenVPN/openvpn/blob/master/Changes.rst> Source code and Windows installers can be downloaded from our download page: <https://openvpn.net/community-downloads/> Packages for Debian, Ubuntu, Fedora, RHEL, and openSUSE are available in the various official Community repositories: <https://community.openvpn.net/Pages/OpenVPN%20software%20repos> Kind regards, Yuriy Darnobyt

The OpenVPN community project team is proud to release OpenVPN 2.6.17. This is a bugfix release containing one security fix. Security fixes: * CVE-2025-13751: Windows/interactive service: fix erroneous exit on error that could be used by a local Windows users to achieve a local denial-of-service Bug fixes: * Windows/interactive service: improve service pipe robustness against file access races (uuid) and access by unauthorized processes (ACL). * upgrade bundled build instruction (vcpkg and patch) for pkcs11-helper to 1.31, fixing a parser bug Windows MSI changes since 2.6.16-I001: * Built against OpenSSL 3.6.0 * Included openvpn-gui updated to 11.59.0.0	* Authorize config before opening the service pipe	* Remove dependence on pathcch.dll not in Windows 7 * Included win-dco driver updated to 2.8.0 More details can be found in the Changes document: <https://github.com/OpenVPN/openvpn/blob/release/2.6/Changes.rst> (The Changes document also contains a section with work-arounds for common problems encountered when using OpenVPN with OpenSSL 3) Source code and Windows installers can be downloaded from our download page: <https://openvpn.net/community/> Debian and Ubuntu packages are available in the official apt repositories: <https://community.openvpn.net/openvpn/wiki/OpenvpnSoftwareRepos#DebianUbuntu:UsingOpenVPNaptrepositories> On Red Hat derivatives we recommend using the Fedora Copr repository. <https://copr.fedorainfracloud.org/coprs/g/OpenVPN/openvpn-release-2.6/> Kind regards, Yuriy Darnobyt

From: Max Fillinger <max...@se...> Mbed TLS 2.28 is out of support since March and adding support for Mbed TLS 4 will get ugly enough without the old compatibility code lying around too. Mbed TLS 2.28 still ships on some supported distributions (e.g. Ubuntu 24.04) but nobody is maintaining openvpn-mbedtls packages there. This commit will probably break on some test machines. Change-Id: Ia4afabcb6006dc9304a4c09f824d9c7c2d4d64ad Signed-off-by: Max Fillinger <max...@se...> Acked-by: Frank Lichtenheld <fr...@li...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1412 --- 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/+/1412 This mail reflects revision 3 of this Change. Acked-by according to Gerrit (reflected above): Frank Lichtenheld <fr...@li...> diff --git a/CMakeLists.txt b/CMakeLists.txt index e812145..8a8054f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -305,10 +305,6 @@ set(CMAKE_REQUIRED_LINK_OPTIONS "-L${MBED_LIBRARY_PATH}") endif () set(CMAKE_REQUIRED_LIBRARIES "mbedtls;mbedx509;mbedcrypto") - check_symbol_exists(mbedtls_ctr_drbg_update_ret mbedtls/ctr_drbg.h HAVE_MBEDTLS_CTR_DRBG_UPDATE_RET) - check_symbol_exists(mbedtls_ssl_conf_export_keys_ext_cb mbedtls/ssl.h HAVE_MBEDTLS_SSL_CONF_EXPORT_KEYS_EXT_CB) - check_symbol_exists(mbedtls_ssl_set_export_keys_cb mbedtls/ssl.h HAVE_MBEDTLS_SSL_SET_EXPORT_KEYS_CB) - check_symbol_exists(mbedtls_ssl_tls_prf mbedtls/ssl.h HAVE_MBEDTLS_SSL_TLS_PRF) check_include_files(psa/crypto.h HAVE_PSA_CRYPTO_H) endfunction() diff --git a/README.mbedtls b/README.mbedtls index a1012e9..fb30db1 100644 --- a/README.mbedtls +++ b/README.mbedtls @@ -7,7 +7,8 @@	make	make install -This version requires mbed TLS version >= 2.0.0 or >= 3.2.1. +This version requires mbed TLS version >= 3.2.1. Versions >= 4.0.0 are not +yet supported. Support for TLS 1.3 requires an Mbed TLS version >= 3.6.4. ************************************************************************* @@ -23,12 +24,3 @@ * X.509 subject line has a different format than the OpenSSL subject line * X.509 certificate tracking - -************************************************************************* - -Mbed TLS 3 has implemented TLS 1.3, but support in OpenVPN requires the -function mbedtls_ssl_export_keying_material() which is currently not in -any released version. It is available when building mbed TLS from source -(mbedtls-3.6 or development branch). - -Without this function, only TLS 1.2 is available. diff --git a/config.h.cmake.in b/config.h.cmake.in index 1c443ab..c90d124 100644 --- a/config.h.cmake.in +++ b/config.h.cmake.in @@ -371,10 +371,6 @@ /* Availability of different mbed TLS features and APIs */ #cmakedefine HAVE_PSA_CRYPTO_H -#cmakedefine HAVE_MBEDTLS_SSL_SET_EXPORT_KEYS_CB -#cmakedefine HAVE_MBEDTLS_SSL_CONF_EXPORT_KEYS_EXT_CB -#cmakedefine HAVE_MBEDTLS_CTR_DRBG_UPDATE_RET -#cmakedefine HAVE_MBEDTLS_SSL_TLS_PRF /* Path to ifconfig tool */ #define IFCONFIG_PATH "@IFCONFIG_PATH@" diff --git a/configure.ac b/configure.ac index 44c7b65..a6f79ae 100644 --- a/configure.ac +++ b/configure.ac @@ -995,7 +995,7 @@	if test -z "${MBEDTLS_CFLAGS}" -a -z "${MBEDTLS_LIBS}"; then	# if the user did not explicitly specify flags, try to autodetect	PKG_CHECK_MODULES([MBEDTLS], -	[mbedtls >= 2.0.0 mbedx509 >= 2.0.0 mbedcrypto >= 2.0.0], +	[mbedtls >= 3.2.1 mbedx509 >= 3.2.1 mbedcrypto >= 3.2.1],	[have_mbedtls="yes"],	[LIBS="${LIBS} -lmbedtls -lmbedx509 -lmbedcrypto"]	) @@ -1020,35 +1020,17 @@ #include <mbedtls/version.h>	]],	[[ -#if MBEDTLS_VERSION_NUMBER < 0x02000000 || (MBEDTLS_VERSION_NUMBER >= 0x03000000 && MBEDTLS_VERSION_NUMBER < 0x03020100) +#if MBEDTLS_VERSION_NUMBER < 0x03020100 #error invalid version #endif	]]	)],	[AC_MSG_RESULT([ok])], -	[AC_MSG_ERROR([mbed TLS version >= 2.0.0 or >= 3.2.1 required])] +	[AC_MSG_ERROR([mbed TLS version >= 3.2.1 required])]	)	AC_CHECK_HEADERS(psa/crypto.h) -	AC_CHECK_FUNCS([mbedtls_ssl_tls_prf mbedtls_ssl_conf_export_keys_ext_cb]) - -	if test "x$ac_cv_func_mbedtls_ssl_conf_export_keys_ext_cb" != xyes; then -	AC_CHECK_FUNCS([mbedtls_ssl_set_export_keys_cb]) -	if test "x$ac_cv_func_mbedtls_ssl_set_export_keys_cb" != xyes; then -	AC_CHECK_FUNC([mbedtls_ssl_export_keying_material]) -	if test "x$ac_cv_func_mbedtls_ssl_export_keying_material" != xyes; then -	AC_MSG_ERROR(This version of mbed TLS has no support for exporting key material.) -	fi -	fi -	fi - -	AC_CHECK_FUNC( -	[mbedtls_ctr_drbg_update_ret], -	AC_DEFINE([HAVE_MBEDTLS_CTR_DRBG_UPDATE_RET], [1], - [Use mbedtls_ctr_drbg_update_ret from mbed TLS]), -	) -	CFLAGS="${saved_CFLAGS}"	LIBS="${saved_LIBS}"	AC_DEFINE([ENABLE_CRYPTO_MBEDTLS], [1], [Use mbed TLS library]) diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c index 89f0ab9..6688d48 100644 --- a/src/openvpn/crypto_mbedtls.c +++ b/src/openvpn/crypto_mbedtls.c @@ -41,7 +41,6 @@ #include "integer.h" #include "crypto_backend.h" #include "otime.h" -#include "mbedtls_compat.h" #include "misc.h" #include <mbedtls/base64.h> @@ -987,17 +986,7 @@ return diff; } -/* mbedtls-2.18.0 or newer implements tls_prf, but prf_tls1 is removed - * from recent versions, so we use our own implementation if necessary. */ -#if defined(HAVE_MBEDTLS_SSL_TLS_PRF) && defined(MBEDTLS_SSL_TLS_PRF_TLS1) -bool -ssl_tls1_PRF(const uint8_t *seed, size_t seed_len, const uint8_t *secret, size_t secret_len, - uint8_t *output, size_t output_len) -{ - return mbed_ok(mbedtls_ssl_tls_prf(MBEDTLS_SSL_TLS_PRF_TLS1, secret, secret_len, "", seed, - seed_len, output, output_len)); -} -#else /* defined(HAVE_MBEDTLS_SSL_TLS_PRF) && defined(MBEDTLS_SSL_TLS_PRF_TLS1) */ + #if defined(__GNUC__) || defined(__clang__) #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wconversion" @@ -1135,6 +1124,5 @@ #if defined(__GNUC__) || defined(__clang__) #pragma GCC diagnostic pop #endif -#endif /* HAVE_MBEDTLS_SSL_TLS_PRF && defined(MBEDTLS_SSL_TLS_PRF_TLS1) */ #endif /* ENABLE_CRYPTO_MBEDTLS */ diff --git a/src/openvpn/mbedtls_compat.h b/src/openvpn/mbedtls_compat.h index 68096c4..540f370 100644 --- a/src/openvpn/mbedtls_compat.h +++ b/src/openvpn/mbedtls_compat.h @@ -23,10 +23,8 @@ /** * @file * mbedtls compatibility stub. - * This file provide compatibility stubs for the mbedtls libraries - * prior to version 3. This version made most fields in structs private - * and requires accessor functions to be used. For earlier versions, we - * implement the accessor functions here. + * This file provides compatibility stubs to handle API differences between + * different versions of Mbed TLS. */ #ifndef MBEDTLS_COMPAT_H_ @@ -36,27 +34,10 @@ #include "errlevel.h" -#include <mbedtls/cipher.h> -#include <mbedtls/ctr_drbg.h> -#include <mbedtls/dhm.h> -#include <mbedtls/ecp.h> -#include <mbedtls/md.h> -#include <mbedtls/pem.h> -#include <mbedtls/pk.h> -#include <mbedtls/ssl.h> -#include <mbedtls/version.h> -#include <mbedtls/x509_crt.h> - #ifdef HAVE_PSA_CRYPTO_H #include <psa/crypto.h> #endif -#if MBEDTLS_VERSION_NUMBER >= 0x03000000 -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) { @@ -70,162 +51,4 @@ #endif } -static inline mbedtls_compat_group_id -mbedtls_compat_get_group_id(const mbedtls_ecp_curve_info *curve_info) -{ -#if MBEDTLS_VERSION_NUMBER >= 0x03000000 - 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() - * which does. - * - * In mbedtls 3, this function was removed and mbedtls_ctr_drbg_update() returns - * an error code. - */ -static inline int -mbedtls_compat_ctr_drbg_update(mbedtls_ctr_drbg_context *ctx, const unsigned char *additional, - size_t add_len) -{ -#if MBEDTLS_VERSION_NUMBER > 0x03000000 - return mbedtls_ctr_drbg_update(ctx, additional, add_len); -#elif defined(HAVE_MBEDTLS_CTR_DRBG_UPDATE_RET) - return mbedtls_ctr_drbg_update_ret(ctx, additional, add_len); -#else - mbedtls_ctr_drbg_update(ctx, additional, add_len); - return 0; -#endif /* HAVE_MBEDTLS_CTR_DRBG_UPDATE_RET */ -} - -static inline int -mbedtls_compat_pk_check_pair(const mbedtls_pk_context *pub, const mbedtls_pk_context *prv, - int (*f_rng)(void *, unsigned char *, size_t), void *p_rng) -{ -#if MBEDTLS_VERSION_NUMBER < 0x03020100 - return mbedtls_pk_check_pair(pub, prv); -#else - return mbedtls_pk_check_pair(pub, prv, f_rng, p_rng); -#endif /* MBEDTLS_VERSION_NUMBER < 0x03020100 */ -} - -static inline int -mbedtls_compat_pk_parse_key(mbedtls_pk_context *ctx, const unsigned char *key, size_t keylen, - const unsigned char *pwd, size_t pwdlen, - int (*f_rng)(void *, unsigned char *, size_t), void *p_rng) -{ -#if MBEDTLS_VERSION_NUMBER < 0x03020100 - return mbedtls_pk_parse_key(ctx, key, keylen, pwd, pwdlen); -#else - return mbedtls_pk_parse_key(ctx, key, keylen, pwd, pwdlen, f_rng, p_rng); -#endif -} - -static inline int -mbedtls_compat_pk_parse_keyfile(mbedtls_pk_context *ctx, const char *path, const char *password, - int (*f_rng)(void *, unsigned char *, size_t), void *p_rng) -{ -#if MBEDTLS_VERSION_NUMBER < 0x03020100 - return mbedtls_pk_parse_keyfile(ctx, path, password); -#else - return mbedtls_pk_parse_keyfile(ctx, path, password, f_rng, p_rng); -#endif -} - -#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) -{ - return (size_t)cipher->block_size; -} - -static inline size_t -mbedtls_cipher_info_get_iv_size(const mbedtls_cipher_info_t *cipher) -{ - return (size_t)cipher->iv_size; -} - -static inline size_t -mbedtls_cipher_info_get_key_bitlen(const mbedtls_cipher_info_t *cipher) -{ - return (size_t)cipher->key_bitlen; -} - -static inline mbedtls_cipher_mode_t -mbedtls_cipher_info_get_mode(const mbedtls_cipher_info_t *cipher) -{ - return cipher->mode; -} - -static inline const char * -mbedtls_cipher_info_get_name(const mbedtls_cipher_info_t *cipher) -{ - return cipher->name; -} - -static inline mbedtls_cipher_type_t -mbedtls_cipher_info_get_type(const mbedtls_cipher_info_t *cipher) -{ - return cipher->type; -} - -static inline size_t -mbedtls_dhm_get_bitlen(const mbedtls_dhm_context *ctx) -{ - return 8 * ctx->len; -} - -static inline const mbedtls_md_info_t * -mbedtls_md_info_from_ctx(const mbedtls_md_context_t *ctx) -{ - return ctx->md_info; -} - -static inline const unsigned char * -mbedtls_pem_get_buffer(const mbedtls_pem_context *ctx, size_t *buf_size) -{ - *buf_size = ctx->buflen; - return ctx->buf; -} - -static inline int -mbedtls_x509_crt_has_ext_type(const mbedtls_x509_crt *ctx, int ext_type) -{ - return ctx->ext_types & ext_type; -} -#endif /* MBEDTLS_VERSION_NUMBER < 0x03020100 */ - #endif /* MBEDTLS_COMPAT_H_ */ diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index 488f9b9..83fca78 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -49,13 +49,8 @@ #include "ssl_verify_mbedtls.h" #include <mbedtls/debug.h> #include <mbedtls/error.h> -#include <mbedtls/version.h> - -#if MBEDTLS_VERSION_NUMBER >= 0x02040000 #include <mbedtls/net_sockets.h> -#else -#include <mbedtls/net.h> -#endif +#include <mbedtls/version.h> #include <mbedtls/oid.h> #include <mbedtls/pem.h> @@ -165,50 +160,14 @@ ASSERT(NULL != ctx); return ctx->initialised; } -#ifdef MBEDTLS_SSL_KEYING_MATERIAL_EXPORT -/* mbedtls_ssl_export_keying_material does not need helper/callback methods */ -#elif defined(HAVE_MBEDTLS_SSL_CONF_EXPORT_KEYS_EXT_CB) +#if !defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT) /* - * Key export callback for older versions of mbed TLS, to be used with - * mbedtls_ssl_conf_export_keys_ext_cb(). It is called with the master - * secret, client random and server random, and the type of PRF function - * to use. - * - * Mbed TLS stores this callback in the mbedtls_ssl_config struct and it - * is used in the mbedtls_ssl_contexts set up from that config. */ -int -mbedtls_ssl_export_keys_cb(void *p_expkey, const unsigned char *ms, const unsigned char *kb, - size_t maclen, size_t keylen, size_t ivlen, - const unsigned char client_random[32], - const unsigned char server_random[32], - mbedtls_tls_prf_types tls_prf_type) -{ - struct tls_session *session = p_expkey; - struct key_state_ssl *ks_ssl = &session->key[KS_PRIMARY].ks_ssl; - struct tls_key_cache *cache = &ks_ssl->tls_key_cache; - - static_assert(sizeof(ks_ssl->ctx->session->master) == sizeof(cache->master_secret), - "master size mismatch"); - - memcpy(cache->client_server_random, client_random, 32); - memcpy(cache->client_server_random + 32, server_random, 32); - memcpy(cache->master_secret, ms, sizeof(cache->master_secret)); - cache->tls_prf_type = tls_prf_type; - - return 0; -} -#elif defined(HAVE_MBEDTLS_SSL_SET_EXPORT_KEYS_CB) -/* - * Key export callback for newer versions of mbed TLS, to be used with - * mbedtls_ssl_set_export_keys_cb(). When used with TLS 1.2, the callback - * is called with the TLS 1.2 master secret, client random, server random - * and the type of PRF to use. With TLS 1.3, it is called with several - * different keys (indicated by type), but unfortunately not the exporter - * master secret. - * - * Unlike in older versions, the callback is not stored in the - * mbedtls_ssl_config. It is placed in the mbedtls_ssl_context after it - * has been set up. */ + * If we don't have mbedtls_ssl_export_keying_material(), we use + * mbedtls_ssl_set_export_keys_cb() to obtain a copy of the TLS 1.2 + * master secret and compute the TLS-Exporter function ourselves. + * Unfortunately, with TLS 1.3, there is no alternative to + * mbedtls_ssl_export_keying_material(). + */ void mbedtls_ssl_export_keys_cb(void *p_expkey, mbedtls_ssl_key_export_type type, const unsigned char *secret, size_t secret_len, @@ -240,9 +199,7 @@ memcpy(cache->master_secret, secret, sizeof(cache->master_secret)); cache->tls_prf_type = tls_prf_type; } -#else /* ifdef MBEDTLS_SSL_KEYING_MATERIAL_EXPORT */ -#error mbedtls_ssl_conf_export_keys_ext_cb, mbedtls_ssl_set_export_keys_cb or mbedtls_ssl_export_keying_material must be available in mbed TLS -#endif /* HAVE_MBEDTLS_SSL_CONF_EXPORT_KEYS_EXT_CB */ +#endif /* !defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT) */ bool @@ -397,7 +354,7 @@ /* Get number of groups and allocate an array in ctx */ int groups_count = get_num_elements(groups, ':'); - ALLOC_ARRAY_CLEAR(ctx->groups, mbedtls_compat_group_id, groups_count + 1) + ALLOC_ARRAY_CLEAR(ctx->groups, uint16_t, groups_count + 1) /* Parse allowed ciphers, getting IDs */ int i = 0; @@ -413,7 +370,7 @@ } else { - ctx->groups[i] = mbedtls_compat_get_group_id(ci); + ctx->groups[i] = ci->tls_id; i++; } } @@ -537,29 +494,29 @@ if (priv_key_inline) { - status = mbedtls_compat_pk_parse_key(ctx->priv_key, (const unsigned char *)priv_key_file, - strlen(priv_key_file) + 1, NULL, 0, - mbedtls_ctr_drbg_random, rand_ctx_get()); + status = mbedtls_pk_parse_key(ctx->priv_key, (const unsigned char *)priv_key_file, + strlen(priv_key_file) + 1, NULL, 0, + mbedtls_ctr_drbg_random, rand_ctx_get()); if (MBEDTLS_ERR_PK_PASSWORD_REQUIRED == status) { char passbuf[512] = { 0 }; pem_password_callback(passbuf, 512, 0, NULL); - status = mbedtls_compat_pk_parse_key( + status = mbedtls_pk_parse_key( ctx->priv_key, (const unsigned char *)priv_key_file, strlen(priv_key_file) + 1, (unsigned char *)passbuf, strlen(passbuf), mbedtls_ctr_drbg_random, rand_ctx_get()); } } else { - status = mbedtls_compat_pk_parse_keyfile(ctx->priv_key, priv_key_file, NULL, - mbedtls_ctr_drbg_random, rand_ctx_get()); + status = mbedtls_pk_parse_keyfile(ctx->priv_key, priv_key_file, NULL, + mbedtls_ctr_drbg_random, rand_ctx_get()); if (MBEDTLS_ERR_PK_PASSWORD_REQUIRED == status) { char passbuf[512] = { 0 }; pem_password_callback(passbuf, 512, 0, NULL); - status = mbedtls_compat_pk_parse_keyfile(ctx->priv_key, priv_key_file, passbuf, - mbedtls_ctr_drbg_random, rand_ctx_get()); + status = mbedtls_pk_parse_keyfile(ctx->priv_key, priv_key_file, passbuf, + mbedtls_ctr_drbg_random, rand_ctx_get()); } } if (!mbed_ok(status)) @@ -575,8 +532,8 @@ return 1; } - if (!mbed_ok(mbedtls_compat_pk_check_pair(&ctx->crt_chain->pk, ctx->priv_key, - mbedtls_ctr_drbg_random, rand_ctx_get()))) + if (!mbed_ok(mbedtls_pk_check_pair(&ctx->crt_chain->pk, ctx->priv_key, + mbedtls_ctr_drbg_random, rand_ctx_get()))) { msg(M_WARN, "Private key does not match the certificate"); return 1; @@ -610,9 +567,6 @@ */ static inline int external_pkcs1_sign(void *ctx_voidptr, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng, -#if MBEDTLS_VERSION_NUMBER < 0x03020100 - int mode, -#endif mbedtls_md_type_t md_alg, unsigned int hashlen, const unsigned char *hash, unsigned char *sig) { @@ -627,13 +581,6 @@ return MBEDTLS_ERR_RSA_BAD_INPUT_DATA; } -#if MBEDTLS_VERSION_NUMBER < 0x03020100 - if (MBEDTLS_RSA_PRIVATE != mode) - { - return MBEDTLS_ERR_RSA_BAD_INPUT_DATA; - } -#endif - /* * Support a wide range of hashes. TLSv1.1 and before only need SIG_RSA_RAW, * but TLSv1.2 needs the full suite of hashes. @@ -1000,7 +947,7 @@ if (0 != memcmp(old_sha256_hash, sha256_hash, sizeof(sha256_hash))) { - if (!mbed_ok(mbedtls_compat_ctr_drbg_update(cd_ctx, sha256_hash, 32))) + if (!mbed_ok(mbedtls_ctr_drbg_update(cd_ctx, sha256_hash, 32))) { msg(M_WARN, "WARNING: failed to personalise random, could not update CTR_DRBG"); } @@ -1204,12 +1151,6 @@ mbedtls_ssl_conf_max_tls_version(ks_ssl->ssl_config, version); } -#if defined(HAVE_MBEDTLS_SSL_CONF_EXPORT_KEYS_EXT_CB) \ - && !defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT) - /* Initialize keying material exporter, old style. */ - mbedtls_ssl_conf_export_keys_ext_cb(ks_ssl->ssl_config, mbedtls_ssl_export_keys_cb, session); -#endif - /* Initialise SSL context */ ALLOC_OBJ_CLEAR(ks_ssl->ctx, mbedtls_ssl_context); mbedtls_ssl_init(ks_ssl->ctx); @@ -1219,8 +1160,8 @@ * verification. */ ASSERT(mbed_ok(mbedtls_ssl_set_hostname(ks_ssl->ctx, NULL))); -#if defined(HAVE_MBEDTLS_SSL_SET_EXPORT_KEYS_CB) && !defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT) - /* Initialize keying material exporter, new style. */ +#if !defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT) + /* Initialize the keying material exporter callback. */ mbedtls_ssl_set_export_keys_cb(ks_ssl->ctx, mbedtls_ssl_export_keys_cb, session); #endif diff --git a/src/openvpn/ssl_mbedtls.h b/src/openvpn/ssl_mbedtls.h index 0f85d96..8380a116 100644 --- a/src/openvpn/ssl_mbedtls.h +++ b/src/openvpn/ssl_mbedtls.h @@ -39,8 +39,6 @@ #include <pkcs11-helper-1.0/pkcs11h-certificate.h> #endif -#include "mbedtls_compat.h" - typedef struct _buffer_entry buffer_entry; struct _buffer_entry @@ -130,7 +128,7 @@ #endif struct external_context external_key; /**< External key context */ int *allowed_ciphers; /**< List of allowed ciphers for this connection */ - mbedtls_compat_group_id *groups; /**< List of allowed groups for this connection */ + uint16_t *groups; /**< List of allowed groups for this connection */ mbedtls_x509_crt_profile cert_profile; /**< Allowed certificate types */ }; diff --git a/src/openvpn/ssl_verify_mbedtls.c b/src/openvpn/ssl_verify_mbedtls.c index 80ef837..250c806 100644 --- a/src/openvpn/ssl_verify_mbedtls.c +++ b/src/openvpn/ssl_verify_mbedtls.c @@ -35,7 +35,6 @@ #if defined(ENABLE_CRYPTO_MBEDTLS) #include "crypto_mbedtls.h" -#include "mbedtls_compat.h" #include "ssl_verify.h" #include <mbedtls/asn1.h> #include <mbedtls/error.h> 

Attention is currently required from: flichtenheld, plaisthos. Hello cron2, plaisthos, I'd like you to reexamine a change. Please visit http://gerrit.openvpn.net/c/openvpn/+/1376?usp=email to look at the new patch set (#9). The change is no longer submittable: checks~ChecksSubmitRule is unsatisfied now. Change subject: tun: Change return type of write_tun/read_tun to ssize_t ...................................................................... tun: Change return type of write_tun/read_tun to ssize_t So we can directly give back the actual return type from write/read. Even if we then cast it back to int. The cast should be safe since we also specify an int as we also put an int in as length. Change-Id: I67f5bf53b80f53fd2e349f844479ed172a7b3aa1 Signed-off-by: Frank Lichtenheld <fr...@li...> --- M src/openvpn/forward.c M src/openvpn/tun.c M src/openvpn/tun.h 3 files changed, 28 insertions(+), 56 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/76/1376/9 diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index 6f1bc0c..913fb92 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -1292,11 +1292,6 @@ #endif /* if defined(ENABLE_DCO) && (defined(TARGET_LINUX) || defined(TARGET_FREEBSD)) */ } -#if defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wconversion" -#endif - /* * Output: c->c2.buf */ @@ -1323,11 +1318,11 @@ if (c->c1.tuntap->backend_driver == DRIVER_AFUNIX) { c->c2.buf.len = - read_tun_afunix(c->c1.tuntap, BPTR(&c->c2.buf), c->c2.frame.buf.payload_size); + (int)read_tun_afunix(c->c1.tuntap, BPTR(&c->c2.buf), c->c2.frame.buf.payload_size); } else { - c->c2.buf.len = read_tun(c->c1.tuntap, BPTR(&c->c2.buf), c->c2.frame.buf.payload_size); + c->c2.buf.len = (int)read_tun(c->c1.tuntap, BPTR(&c->c2.buf), c->c2.frame.buf.payload_size); } #endif /* ifdef _WIN32 */ @@ -1357,6 +1352,11 @@ check_status(c->c2.buf.len, "read from TUN/TAP", NULL, c->c1.tuntap); } +#if defined(__GNUC__) || defined(__clang__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wconversion" +#endif + /** * Drops UDP packets which OS decided to route via tun. * diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 9c04b6b..d2274e0 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -1708,8 +1708,8 @@ #include <netinet/ip.h> #include <sys/uio.h> -static inline int -header_modify_read_write_return(int len) +static inline ssize_t +header_modify_read_write_return(ssize_t len) { if (len > 0) { @@ -1721,12 +1721,7 @@ } } -#if defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wconversion" -#endif - -static int +static ssize_t write_tun_header(struct tuntap *tt, uint8_t *buf, int len) { if (tt->type == DEV_TYPE_TUN) @@ -1757,7 +1752,7 @@ } } -static int +static ssize_t read_tun_header(struct tuntap *tt, uint8_t *buf, int len) { if (tt->type == DEV_TYPE_TUN) @@ -1778,26 +1773,21 @@ } } -#if defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic pop -#endif - #if !defined(TARGET_DARWIN) -int +ssize_t write_tun(struct tuntap *tt, uint8_t *buf, int len) { return write_tun_header(tt, buf, len); } -int +ssize_t read_tun(struct tuntap *tt, uint8_t *buf, int len) { return read_tun_header(tt, buf, len); } #endif - -#endif /* defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY) || defined(TARGET_NETBSD) || if defined (TARGET_OPENBSD) || defined(TARGET_DARWIN) */ +#endif /* if defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY) || defined(TARGET_NETBSD) || defined (TARGET_OPENBSD) || defined(TARGET_DARWIN) */ bool tun_name_is_fixed(const char *dev) @@ -2054,13 +2044,13 @@ free(tt); } -int +ssize_t write_tun(struct tuntap *tt, uint8_t *buf, int len) { return write(tt->fd, buf, len); } -int +ssize_t read_tun(struct tuntap *tt, uint8_t *buf, int len) { return read(tt->fd, buf, len); @@ -2259,27 +2249,18 @@ free(tt); } -#if defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wconversion" -#endif - -int +ssize_t write_tun(struct tuntap *tt, uint8_t *buf, int len) { return write(tt->fd, buf, len); } -int +ssize_t read_tun(struct tuntap *tt, uint8_t *buf, int len) { return read(tt->fd, buf, len); } -#if defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic pop -#endif - #elif defined(TARGET_SOLARIS) #ifndef TUNNEWPPA @@ -2604,7 +2585,7 @@ argv_free(&argv); } -int +ssize_t write_tun(struct tuntap *tt, uint8_t *buf, int len) { struct strbuf sbuf; @@ -2613,7 +2594,7 @@ return putmsg(tt->fd, NULL, &sbuf, 0) >= 0 ? sbuf.len : -1; } -int +ssize_t read_tun(struct tuntap *tt, uint8_t *buf, int len) { struct strbuf sbuf; @@ -3088,11 +3069,6 @@ } } -#if defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wconversion" -#endif - void close_tun(struct tuntap *tt, openvpn_net_ctx_t *ctx) { @@ -3116,7 +3092,7 @@ gc_free(&gc); } -int +ssize_t write_tun(struct tuntap *tt, uint8_t *buf, int len) { if (tt->backend_driver == DRIVER_UTUN) @@ -3129,7 +3105,7 @@ } } -int +ssize_t read_tun(struct tuntap *tt, uint8_t *buf, int len) { if (tt->backend_driver == DRIVER_UTUN) @@ -3142,10 +3118,6 @@ } } -#if defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic pop -#endif - #elif defined(TARGET_AIX) void @@ -3268,13 +3240,13 @@ argv_free(&argv); } -int +ssize_t write_tun(struct tuntap *tt, uint8_t *buf, int len) { return write(tt->fd, buf, len); } -int +ssize_t read_tun(struct tuntap *tt, uint8_t *buf, int len) { return read(tt->fd, buf, len); @@ -6320,13 +6292,13 @@ free(tt); } -int +ssize_t write_tun(struct tuntap *tt, uint8_t *buf, int len) { return write(tt->fd, buf, len); } -int +ssize_t read_tun(struct tuntap *tt, uint8_t *buf, int len) { return read(tt->fd, buf, len); diff --git a/src/openvpn/tun.h b/src/openvpn/tun.h index 876f147..8bc876e 100644 --- a/src/openvpn/tun.h +++ b/src/openvpn/tun.h @@ -274,9 +274,9 @@ void close_tun_handle(struct tuntap *tt); -int write_tun(struct tuntap *tt, uint8_t *buf, int len); +ssize_t write_tun(struct tuntap *tt, uint8_t *buf, int len); -int read_tun(struct tuntap *tt, uint8_t *buf, int len); +ssize_t read_tun(struct tuntap *tt, uint8_t *buf, int len); #ifdef ENABLE_FEATURE_TUN_PERSIST void tuncfg(const char *dev, const char *dev_type, const char *dev_node, int persist_mode, -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1376?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I67f5bf53b80f53fd2e349f844479ed172a7b3aa1 Gerrit-Change-Number: 1376 Gerrit-PatchSet: 9 Gerrit-Owner: flichtenheld <fr...@li...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: flichtenheld <fr...@li...> 

Attention is currently required from: MaxF, plaisthos. flichtenheld has posted comments on this change by MaxF. ( http://gerrit.openvpn.net/c/openvpn/+/1412?usp=email ) Change subject: Drop Mbed TLS 2.X compatibility ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1412?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Ia4afabcb6006dc9304a4c09f824d9c7c2d4d64ad Gerrit-Change-Number: 1412 Gerrit-PatchSet: 3 Gerrit-Owner: MaxF <ma...@ma...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: MaxF <ma...@ma...> Gerrit-Comment-Date: Fri, 28 Nov 2025 16:15:03 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes 

Attention is currently required from: MaxF, plaisthos. flichtenheld has posted comments on this change by MaxF. ( http://gerrit.openvpn.net/c/openvpn/+/1412?usp=email ) Change subject: Drop Mbed TLS 2.X compatibility ...................................................................... Patch Set 3: (1 comment) Patchset: PS1: > CMake doesn't check the version. […] CMake should really use pkg-config for mbedTLS. Not sure why it doesn't. We can handle that in a separate patch. -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1412?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Ia4afabcb6006dc9304a4c09f824d9c7c2d4d64ad Gerrit-Change-Number: 1412 Gerrit-PatchSet: 3 Gerrit-Owner: MaxF <ma...@ma...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: MaxF <ma...@ma...> Gerrit-Comment-Date: Fri, 28 Nov 2025 16:03:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: flichtenheld <fr...@li...> Comment-In-Reply-To: MaxF <ma...@ma...> 

Attention is currently required from: plaisthos. flichtenheld has posted comments on this change by MaxF. ( http://gerrit.openvpn.net/c/openvpn/+/1412?usp=email ) Change subject: Drop Mbed TLS 2.X compatibility ...................................................................... Patch Set 3: -Code-Review -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1412?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Ia4afabcb6006dc9304a4c09f824d9c7c2d4d64ad Gerrit-Change-Number: 1412 Gerrit-PatchSet: 3 Gerrit-Owner: MaxF <ma...@ma...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Comment-Date: Fri, 28 Nov 2025 14:23:21 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes 

Attention is currently required from: flichtenheld, plaisthos. MaxF has posted comments on this change by MaxF. ( http://gerrit.openvpn.net/c/openvpn/+/1412?usp=email ) Change subject: Drop Mbed TLS 2.X compatibility ...................................................................... Patch Set 2: (3 comments) Patchset: PS1: > configure.ac needs more work. Presumably there should be a CMakeLists. […] CMake doesn't check the version. I think the approach is, if you have the wrong version, the build will just fail. It does a bunch of feature detection which I have now removed though. File configure.ac: http://gerrit.openvpn.net/c/openvpn/+/1412/comment/e31ce4eb_9de17423?usp=email : PS1, Line 998:	[mbedtls >= 2.0.0 mbedx509 >= 2.0.0 mbedcrypto >= 2.0.0], > Needs to be updated as well Done http://gerrit.openvpn.net/c/openvpn/+/1412/comment/c2576736_b01015cf?usp=email : PS1, Line 1036:	if test "x$ac_cv_func_mbedtls_ssl_conf_export_keys_ext_cb" != xyes; then > Now that we have the stricter version check, can we get rid of these additional tests? Thanks for pointing that out! In version 3, we always have the export_keys_cb version, this let me remove even more code! -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1412?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Ia4afabcb6006dc9304a4c09f824d9c7c2d4d64ad Gerrit-Change-Number: 1412 Gerrit-PatchSet: 2 Gerrit-Owner: MaxF <ma...@ma...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Comment-Date: Fri, 28 Nov 2025 12:59:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: flichtenheld <fr...@li...> 

Attention is currently required from: MaxF, plaisthos. Hello flichtenheld, plaisthos, I'd like you to reexamine a change. Please visit http://gerrit.openvpn.net/c/openvpn/+/1412?usp=email to look at the new patch set (#3). Change subject: Drop Mbed TLS 2.X compatibility ...................................................................... Drop Mbed TLS 2.X compatibility Mbed TLS 2.28 is out of support since March and adding support for Mbed TLS 4 will get ugly enough without the old compatibility code lying around too. Mbed TLS 2.28 still ships on some supported distributions (e.g. Ubuntu 24.04) but nobody is maintaining openvpn-mbedtls packages there. This commit will probably break on some test machines. Change-Id: Ia4afabcb6006dc9304a4c09f824d9c7c2d4d64ad Signed-off-by: Max Fillinger <max...@se...> --- M CMakeLists.txt M README.mbedtls M config.h.cmake.in M configure.ac M src/openvpn/crypto_mbedtls.c M src/openvpn/mbedtls_compat.h M src/openvpn/ssl_mbedtls.c M src/openvpn/ssl_mbedtls.h M src/openvpn/ssl_verify_mbedtls.c 9 files changed, 33 insertions(+), 318 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/12/1412/3 diff --git a/CMakeLists.txt b/CMakeLists.txt index e812145..8a8054f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -305,10 +305,6 @@ set(CMAKE_REQUIRED_LINK_OPTIONS "-L${MBED_LIBRARY_PATH}") endif () set(CMAKE_REQUIRED_LIBRARIES "mbedtls;mbedx509;mbedcrypto") - check_symbol_exists(mbedtls_ctr_drbg_update_ret mbedtls/ctr_drbg.h HAVE_MBEDTLS_CTR_DRBG_UPDATE_RET) - check_symbol_exists(mbedtls_ssl_conf_export_keys_ext_cb mbedtls/ssl.h HAVE_MBEDTLS_SSL_CONF_EXPORT_KEYS_EXT_CB) - check_symbol_exists(mbedtls_ssl_set_export_keys_cb mbedtls/ssl.h HAVE_MBEDTLS_SSL_SET_EXPORT_KEYS_CB) - check_symbol_exists(mbedtls_ssl_tls_prf mbedtls/ssl.h HAVE_MBEDTLS_SSL_TLS_PRF) check_include_files(psa/crypto.h HAVE_PSA_CRYPTO_H) endfunction() diff --git a/README.mbedtls b/README.mbedtls index a1012e9..fb30db1 100644 --- a/README.mbedtls +++ b/README.mbedtls @@ -7,7 +7,8 @@	make	make install -This version requires mbed TLS version >= 2.0.0 or >= 3.2.1. +This version requires mbed TLS version >= 3.2.1. Versions >= 4.0.0 are not +yet supported. Support for TLS 1.3 requires an Mbed TLS version >= 3.6.4. ************************************************************************* @@ -23,12 +24,3 @@ * X.509 subject line has a different format than the OpenSSL subject line * X.509 certificate tracking - -************************************************************************* - -Mbed TLS 3 has implemented TLS 1.3, but support in OpenVPN requires the -function mbedtls_ssl_export_keying_material() which is currently not in -any released version. It is available when building mbed TLS from source -(mbedtls-3.6 or development branch). - -Without this function, only TLS 1.2 is available. diff --git a/config.h.cmake.in b/config.h.cmake.in index 1c443ab..c90d124 100644 --- a/config.h.cmake.in +++ b/config.h.cmake.in @@ -371,10 +371,6 @@ /* Availability of different mbed TLS features and APIs */ #cmakedefine HAVE_PSA_CRYPTO_H -#cmakedefine HAVE_MBEDTLS_SSL_SET_EXPORT_KEYS_CB -#cmakedefine HAVE_MBEDTLS_SSL_CONF_EXPORT_KEYS_EXT_CB -#cmakedefine HAVE_MBEDTLS_CTR_DRBG_UPDATE_RET -#cmakedefine HAVE_MBEDTLS_SSL_TLS_PRF /* Path to ifconfig tool */ #define IFCONFIG_PATH "@IFCONFIG_PATH@" diff --git a/configure.ac b/configure.ac index 44c7b65..a6f79ae 100644 --- a/configure.ac +++ b/configure.ac @@ -995,7 +995,7 @@	if test -z "${MBEDTLS_CFLAGS}" -a -z "${MBEDTLS_LIBS}"; then	# if the user did not explicitly specify flags, try to autodetect	PKG_CHECK_MODULES([MBEDTLS], -	[mbedtls >= 2.0.0 mbedx509 >= 2.0.0 mbedcrypto >= 2.0.0], +	[mbedtls >= 3.2.1 mbedx509 >= 3.2.1 mbedcrypto >= 3.2.1],	[have_mbedtls="yes"],	[LIBS="${LIBS} -lmbedtls -lmbedx509 -lmbedcrypto"]	) @@ -1020,35 +1020,17 @@ #include <mbedtls/version.h>	]],	[[ -#if MBEDTLS_VERSION_NUMBER < 0x02000000 || (MBEDTLS_VERSION_NUMBER >= 0x03000000 && MBEDTLS_VERSION_NUMBER < 0x03020100) +#if MBEDTLS_VERSION_NUMBER < 0x03020100 #error invalid version #endif	]]	)],	[AC_MSG_RESULT([ok])], -	[AC_MSG_ERROR([mbed TLS version >= 2.0.0 or >= 3.2.1 required])] +	[AC_MSG_ERROR([mbed TLS version >= 3.2.1 required])]	)	AC_CHECK_HEADERS(psa/crypto.h) -	AC_CHECK_FUNCS([mbedtls_ssl_tls_prf mbedtls_ssl_conf_export_keys_ext_cb]) - -	if test "x$ac_cv_func_mbedtls_ssl_conf_export_keys_ext_cb" != xyes; then -	AC_CHECK_FUNCS([mbedtls_ssl_set_export_keys_cb]) -	if test "x$ac_cv_func_mbedtls_ssl_set_export_keys_cb" != xyes; then -	AC_CHECK_FUNC([mbedtls_ssl_export_keying_material]) -	if test "x$ac_cv_func_mbedtls_ssl_export_keying_material" != xyes; then -	AC_MSG_ERROR(This version of mbed TLS has no support for exporting key material.) -	fi -	fi -	fi - -	AC_CHECK_FUNC( -	[mbedtls_ctr_drbg_update_ret], -	AC_DEFINE([HAVE_MBEDTLS_CTR_DRBG_UPDATE_RET], [1], - [Use mbedtls_ctr_drbg_update_ret from mbed TLS]), -	) -	CFLAGS="${saved_CFLAGS}"	LIBS="${saved_LIBS}"	AC_DEFINE([ENABLE_CRYPTO_MBEDTLS], [1], [Use mbed TLS library]) diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c index 89f0ab9..6688d48 100644 --- a/src/openvpn/crypto_mbedtls.c +++ b/src/openvpn/crypto_mbedtls.c @@ -41,7 +41,6 @@ #include "integer.h" #include "crypto_backend.h" #include "otime.h" -#include "mbedtls_compat.h" #include "misc.h" #include <mbedtls/base64.h> @@ -987,17 +986,7 @@ return diff; } -/* mbedtls-2.18.0 or newer implements tls_prf, but prf_tls1 is removed - * from recent versions, so we use our own implementation if necessary. */ -#if defined(HAVE_MBEDTLS_SSL_TLS_PRF) && defined(MBEDTLS_SSL_TLS_PRF_TLS1) -bool -ssl_tls1_PRF(const uint8_t *seed, size_t seed_len, const uint8_t *secret, size_t secret_len, - uint8_t *output, size_t output_len) -{ - return mbed_ok(mbedtls_ssl_tls_prf(MBEDTLS_SSL_TLS_PRF_TLS1, secret, secret_len, "", seed, - seed_len, output, output_len)); -} -#else /* defined(HAVE_MBEDTLS_SSL_TLS_PRF) && defined(MBEDTLS_SSL_TLS_PRF_TLS1) */ + #if defined(__GNUC__) || defined(__clang__) #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wconversion" @@ -1135,6 +1124,5 @@ #if defined(__GNUC__) || defined(__clang__) #pragma GCC diagnostic pop #endif -#endif /* HAVE_MBEDTLS_SSL_TLS_PRF && defined(MBEDTLS_SSL_TLS_PRF_TLS1) */ #endif /* ENABLE_CRYPTO_MBEDTLS */ diff --git a/src/openvpn/mbedtls_compat.h b/src/openvpn/mbedtls_compat.h index 68096c4..540f370 100644 --- a/src/openvpn/mbedtls_compat.h +++ b/src/openvpn/mbedtls_compat.h @@ -23,10 +23,8 @@ /** * @file * mbedtls compatibility stub. - * This file provide compatibility stubs for the mbedtls libraries - * prior to version 3. This version made most fields in structs private - * and requires accessor functions to be used. For earlier versions, we - * implement the accessor functions here. + * This file provides compatibility stubs to handle API differences between + * different versions of Mbed TLS. */ #ifndef MBEDTLS_COMPAT_H_ @@ -36,27 +34,10 @@ #include "errlevel.h" -#include <mbedtls/cipher.h> -#include <mbedtls/ctr_drbg.h> -#include <mbedtls/dhm.h> -#include <mbedtls/ecp.h> -#include <mbedtls/md.h> -#include <mbedtls/pem.h> -#include <mbedtls/pk.h> -#include <mbedtls/ssl.h> -#include <mbedtls/version.h> -#include <mbedtls/x509_crt.h> - #ifdef HAVE_PSA_CRYPTO_H #include <psa/crypto.h> #endif -#if MBEDTLS_VERSION_NUMBER >= 0x03000000 -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) { @@ -70,162 +51,4 @@ #endif } -static inline mbedtls_compat_group_id -mbedtls_compat_get_group_id(const mbedtls_ecp_curve_info *curve_info) -{ -#if MBEDTLS_VERSION_NUMBER >= 0x03000000 - 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() - * which does. - * - * In mbedtls 3, this function was removed and mbedtls_ctr_drbg_update() returns - * an error code. - */ -static inline int -mbedtls_compat_ctr_drbg_update(mbedtls_ctr_drbg_context *ctx, const unsigned char *additional, - size_t add_len) -{ -#if MBEDTLS_VERSION_NUMBER > 0x03000000 - return mbedtls_ctr_drbg_update(ctx, additional, add_len); -#elif defined(HAVE_MBEDTLS_CTR_DRBG_UPDATE_RET) - return mbedtls_ctr_drbg_update_ret(ctx, additional, add_len); -#else - mbedtls_ctr_drbg_update(ctx, additional, add_len); - return 0; -#endif /* HAVE_MBEDTLS_CTR_DRBG_UPDATE_RET */ -} - -static inline int -mbedtls_compat_pk_check_pair(const mbedtls_pk_context *pub, const mbedtls_pk_context *prv, - int (*f_rng)(void *, unsigned char *, size_t), void *p_rng) -{ -#if MBEDTLS_VERSION_NUMBER < 0x03020100 - return mbedtls_pk_check_pair(pub, prv); -#else - return mbedtls_pk_check_pair(pub, prv, f_rng, p_rng); -#endif /* MBEDTLS_VERSION_NUMBER < 0x03020100 */ -} - -static inline int -mbedtls_compat_pk_parse_key(mbedtls_pk_context *ctx, const unsigned char *key, size_t keylen, - const unsigned char *pwd, size_t pwdlen, - int (*f_rng)(void *, unsigned char *, size_t), void *p_rng) -{ -#if MBEDTLS_VERSION_NUMBER < 0x03020100 - return mbedtls_pk_parse_key(ctx, key, keylen, pwd, pwdlen); -#else - return mbedtls_pk_parse_key(ctx, key, keylen, pwd, pwdlen, f_rng, p_rng); -#endif -} - -static inline int -mbedtls_compat_pk_parse_keyfile(mbedtls_pk_context *ctx, const char *path, const char *password, - int (*f_rng)(void *, unsigned char *, size_t), void *p_rng) -{ -#if MBEDTLS_VERSION_NUMBER < 0x03020100 - return mbedtls_pk_parse_keyfile(ctx, path, password); -#else - return mbedtls_pk_parse_keyfile(ctx, path, password, f_rng, p_rng); -#endif -} - -#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) -{ - return (size_t)cipher->block_size; -} - -static inline size_t -mbedtls_cipher_info_get_iv_size(const mbedtls_cipher_info_t *cipher) -{ - return (size_t)cipher->iv_size; -} - -static inline size_t -mbedtls_cipher_info_get_key_bitlen(const mbedtls_cipher_info_t *cipher) -{ - return (size_t)cipher->key_bitlen; -} - -static inline mbedtls_cipher_mode_t -mbedtls_cipher_info_get_mode(const mbedtls_cipher_info_t *cipher) -{ - return cipher->mode; -} - -static inline const char * -mbedtls_cipher_info_get_name(const mbedtls_cipher_info_t *cipher) -{ - return cipher->name; -} - -static inline mbedtls_cipher_type_t -mbedtls_cipher_info_get_type(const mbedtls_cipher_info_t *cipher) -{ - return cipher->type; -} - -static inline size_t -mbedtls_dhm_get_bitlen(const mbedtls_dhm_context *ctx) -{ - return 8 * ctx->len; -} - -static inline const mbedtls_md_info_t * -mbedtls_md_info_from_ctx(const mbedtls_md_context_t *ctx) -{ - return ctx->md_info; -} - -static inline const unsigned char * -mbedtls_pem_get_buffer(const mbedtls_pem_context *ctx, size_t *buf_size) -{ - *buf_size = ctx->buflen; - return ctx->buf; -} - -static inline int -mbedtls_x509_crt_has_ext_type(const mbedtls_x509_crt *ctx, int ext_type) -{ - return ctx->ext_types & ext_type; -} -#endif /* MBEDTLS_VERSION_NUMBER < 0x03020100 */ - #endif /* MBEDTLS_COMPAT_H_ */ diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index 488f9b9..83fca78 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -49,13 +49,8 @@ #include "ssl_verify_mbedtls.h" #include <mbedtls/debug.h> #include <mbedtls/error.h> -#include <mbedtls/version.h> - -#if MBEDTLS_VERSION_NUMBER >= 0x02040000 #include <mbedtls/net_sockets.h> -#else -#include <mbedtls/net.h> -#endif +#include <mbedtls/version.h> #include <mbedtls/oid.h> #include <mbedtls/pem.h> @@ -165,50 +160,14 @@ ASSERT(NULL != ctx); return ctx->initialised; } -#ifdef MBEDTLS_SSL_KEYING_MATERIAL_EXPORT -/* mbedtls_ssl_export_keying_material does not need helper/callback methods */ -#elif defined(HAVE_MBEDTLS_SSL_CONF_EXPORT_KEYS_EXT_CB) +#if !defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT) /* - * Key export callback for older versions of mbed TLS, to be used with - * mbedtls_ssl_conf_export_keys_ext_cb(). It is called with the master - * secret, client random and server random, and the type of PRF function - * to use. - * - * Mbed TLS stores this callback in the mbedtls_ssl_config struct and it - * is used in the mbedtls_ssl_contexts set up from that config. */ -int -mbedtls_ssl_export_keys_cb(void *p_expkey, const unsigned char *ms, const unsigned char *kb, - size_t maclen, size_t keylen, size_t ivlen, - const unsigned char client_random[32], - const unsigned char server_random[32], - mbedtls_tls_prf_types tls_prf_type) -{ - struct tls_session *session = p_expkey; - struct key_state_ssl *ks_ssl = &session->key[KS_PRIMARY].ks_ssl; - struct tls_key_cache *cache = &ks_ssl->tls_key_cache; - - static_assert(sizeof(ks_ssl->ctx->session->master) == sizeof(cache->master_secret), - "master size mismatch"); - - memcpy(cache->client_server_random, client_random, 32); - memcpy(cache->client_server_random + 32, server_random, 32); - memcpy(cache->master_secret, ms, sizeof(cache->master_secret)); - cache->tls_prf_type = tls_prf_type; - - return 0; -} -#elif defined(HAVE_MBEDTLS_SSL_SET_EXPORT_KEYS_CB) -/* - * Key export callback for newer versions of mbed TLS, to be used with - * mbedtls_ssl_set_export_keys_cb(). When used with TLS 1.2, the callback - * is called with the TLS 1.2 master secret, client random, server random - * and the type of PRF to use. With TLS 1.3, it is called with several - * different keys (indicated by type), but unfortunately not the exporter - * master secret. - * - * Unlike in older versions, the callback is not stored in the - * mbedtls_ssl_config. It is placed in the mbedtls_ssl_context after it - * has been set up. */ + * If we don't have mbedtls_ssl_export_keying_material(), we use + * mbedtls_ssl_set_export_keys_cb() to obtain a copy of the TLS 1.2 + * master secret and compute the TLS-Exporter function ourselves. + * Unfortunately, with TLS 1.3, there is no alternative to + * mbedtls_ssl_export_keying_material(). + */ void mbedtls_ssl_export_keys_cb(void *p_expkey, mbedtls_ssl_key_export_type type, const unsigned char *secret, size_t secret_len, @@ -240,9 +199,7 @@ memcpy(cache->master_secret, secret, sizeof(cache->master_secret)); cache->tls_prf_type = tls_prf_type; } -#else /* ifdef MBEDTLS_SSL_KEYING_MATERIAL_EXPORT */ -#error mbedtls_ssl_conf_export_keys_ext_cb, mbedtls_ssl_set_export_keys_cb or mbedtls_ssl_export_keying_material must be available in mbed TLS -#endif /* HAVE_MBEDTLS_SSL_CONF_EXPORT_KEYS_EXT_CB */ +#endif /* !defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT) */ bool @@ -397,7 +354,7 @@ /* Get number of groups and allocate an array in ctx */ int groups_count = get_num_elements(groups, ':'); - ALLOC_ARRAY_CLEAR(ctx->groups, mbedtls_compat_group_id, groups_count + 1) + ALLOC_ARRAY_CLEAR(ctx->groups, uint16_t, groups_count + 1) /* Parse allowed ciphers, getting IDs */ int i = 0; @@ -413,7 +370,7 @@ } else { - ctx->groups[i] = mbedtls_compat_get_group_id(ci); + ctx->groups[i] = ci->tls_id; i++; } } @@ -537,29 +494,29 @@ if (priv_key_inline) { - status = mbedtls_compat_pk_parse_key(ctx->priv_key, (const unsigned char *)priv_key_file, - strlen(priv_key_file) + 1, NULL, 0, - mbedtls_ctr_drbg_random, rand_ctx_get()); + status = mbedtls_pk_parse_key(ctx->priv_key, (const unsigned char *)priv_key_file, + strlen(priv_key_file) + 1, NULL, 0, + mbedtls_ctr_drbg_random, rand_ctx_get()); if (MBEDTLS_ERR_PK_PASSWORD_REQUIRED == status) { char passbuf[512] = { 0 }; pem_password_callback(passbuf, 512, 0, NULL); - status = mbedtls_compat_pk_parse_key( + status = mbedtls_pk_parse_key( ctx->priv_key, (const unsigned char *)priv_key_file, strlen(priv_key_file) + 1, (unsigned char *)passbuf, strlen(passbuf), mbedtls_ctr_drbg_random, rand_ctx_get()); } } else { - status = mbedtls_compat_pk_parse_keyfile(ctx->priv_key, priv_key_file, NULL, - mbedtls_ctr_drbg_random, rand_ctx_get()); + status = mbedtls_pk_parse_keyfile(ctx->priv_key, priv_key_file, NULL, + mbedtls_ctr_drbg_random, rand_ctx_get()); if (MBEDTLS_ERR_PK_PASSWORD_REQUIRED == status) { char passbuf[512] = { 0 }; pem_password_callback(passbuf, 512, 0, NULL); - status = mbedtls_compat_pk_parse_keyfile(ctx->priv_key, priv_key_file, passbuf, - mbedtls_ctr_drbg_random, rand_ctx_get()); + status = mbedtls_pk_parse_keyfile(ctx->priv_key, priv_key_file, passbuf, + mbedtls_ctr_drbg_random, rand_ctx_get()); } } if (!mbed_ok(status)) @@ -575,8 +532,8 @@ return 1; } - if (!mbed_ok(mbedtls_compat_pk_check_pair(&ctx->crt_chain->pk, ctx->priv_key, - mbedtls_ctr_drbg_random, rand_ctx_get()))) + if (!mbed_ok(mbedtls_pk_check_pair(&ctx->crt_chain->pk, ctx->priv_key, + mbedtls_ctr_drbg_random, rand_ctx_get()))) { msg(M_WARN, "Private key does not match the certificate"); return 1; @@ -610,9 +567,6 @@ */ static inline int external_pkcs1_sign(void *ctx_voidptr, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng, -#if MBEDTLS_VERSION_NUMBER < 0x03020100 - int mode, -#endif mbedtls_md_type_t md_alg, unsigned int hashlen, const unsigned char *hash, unsigned char *sig) { @@ -627,13 +581,6 @@ return MBEDTLS_ERR_RSA_BAD_INPUT_DATA; } -#if MBEDTLS_VERSION_NUMBER < 0x03020100 - if (MBEDTLS_RSA_PRIVATE != mode) - { - return MBEDTLS_ERR_RSA_BAD_INPUT_DATA; - } -#endif - /* * Support a wide range of hashes. TLSv1.1 and before only need SIG_RSA_RAW, * but TLSv1.2 needs the full suite of hashes. @@ -1000,7 +947,7 @@ if (0 != memcmp(old_sha256_hash, sha256_hash, sizeof(sha256_hash))) { - if (!mbed_ok(mbedtls_compat_ctr_drbg_update(cd_ctx, sha256_hash, 32))) + if (!mbed_ok(mbedtls_ctr_drbg_update(cd_ctx, sha256_hash, 32))) { msg(M_WARN, "WARNING: failed to personalise random, could not update CTR_DRBG"); } @@ -1204,12 +1151,6 @@ mbedtls_ssl_conf_max_tls_version(ks_ssl->ssl_config, version); } -#if defined(HAVE_MBEDTLS_SSL_CONF_EXPORT_KEYS_EXT_CB) \ - && !defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT) - /* Initialize keying material exporter, old style. */ - mbedtls_ssl_conf_export_keys_ext_cb(ks_ssl->ssl_config, mbedtls_ssl_export_keys_cb, session); -#endif - /* Initialise SSL context */ ALLOC_OBJ_CLEAR(ks_ssl->ctx, mbedtls_ssl_context); mbedtls_ssl_init(ks_ssl->ctx); @@ -1219,8 +1160,8 @@ * verification. */ ASSERT(mbed_ok(mbedtls_ssl_set_hostname(ks_ssl->ctx, NULL))); -#if defined(HAVE_MBEDTLS_SSL_SET_EXPORT_KEYS_CB) && !defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT) - /* Initialize keying material exporter, new style. */ +#if !defined(MBEDTLS_SSL_KEYING_MATERIAL_EXPORT) + /* Initialize the keying material exporter callback. */ mbedtls_ssl_set_export_keys_cb(ks_ssl->ctx, mbedtls_ssl_export_keys_cb, session); #endif diff --git a/src/openvpn/ssl_mbedtls.h b/src/openvpn/ssl_mbedtls.h index 0f85d96..8380a116 100644 --- a/src/openvpn/ssl_mbedtls.h +++ b/src/openvpn/ssl_mbedtls.h @@ -39,8 +39,6 @@ #include <pkcs11-helper-1.0/pkcs11h-certificate.h> #endif -#include "mbedtls_compat.h" - typedef struct _buffer_entry buffer_entry; struct _buffer_entry @@ -130,7 +128,7 @@ #endif struct external_context external_key; /**< External key context */ int *allowed_ciphers; /**< List of allowed ciphers for this connection */ - mbedtls_compat_group_id *groups; /**< List of allowed groups for this connection */ + uint16_t *groups; /**< List of allowed groups for this connection */ mbedtls_x509_crt_profile cert_profile; /**< Allowed certificate types */ }; diff --git a/src/openvpn/ssl_verify_mbedtls.c b/src/openvpn/ssl_verify_mbedtls.c index 80ef837..250c806 100644 --- a/src/openvpn/ssl_verify_mbedtls.c +++ b/src/openvpn/ssl_verify_mbedtls.c @@ -35,7 +35,6 @@ #if defined(ENABLE_CRYPTO_MBEDTLS) #include "crypto_mbedtls.h" -#include "mbedtls_compat.h" #include "ssl_verify.h" #include <mbedtls/asn1.h> #include <mbedtls/error.h> -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1412?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Ia4afabcb6006dc9304a4c09f824d9c7c2d4d64ad Gerrit-Change-Number: 1412 Gerrit-PatchSet: 3 Gerrit-Owner: MaxF <ma...@ma...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: MaxF <ma...@ma...> 

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1403?usp=email ) Change subject: dco: process messages immediately after read ...................................................................... dco: process messages immediately after read Currently, reading and processing of incoming DCO messages are decoupled: notifications are read, parsed, and the relevant information is stored in fields of dco_context_t for later processing (with the only exception being stats). This approach is problematic on Linux, since libnl does not allow reading a single netlink message at a time, which can result in loss of information when multiple notifications are available. This change adopts a read -> parse -> process paradigm. On Linux, processing is now invoked directly from within the parsing callback, which libnl calls for each received netlink packet. The other interfaces are adapted accordingly to unify the processing model across all platforms. On Linux, however, a DEL_PEER notification from the kernel triggers a GET_PEER request from userspace, which clutters the netlink communication logic and can lead to errors or even process exit when multiple simultaneous DEL_PEER notifications are received. To avoid this, introduce a lock that prevents requesting stats while we are still busy parsing other messages. Reported-by: Stefan Baranoff <ste...@tr...> Github: OpenVPN/openvpn#900 Github: OpenVPN/openvpn#918 Github: fixes OpenVPN/openvpn#919 Change-Id: Iefc251cb4483c0b9fb9d6a5207db4445cd884d52 Signed-off-by: Ralf Lici <ra...@ma...> Acked-by: Gert Doering <ge...@gr...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1403 Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg34785.html Signed-off-by: Gert Doering <ge...@gr...> --- M src/openvpn/dco.h M src/openvpn/dco_freebsd.c M src/openvpn/dco_linux.c M src/openvpn/dco_win.c M src/openvpn/forward.c M src/openvpn/forward.h M src/openvpn/multi.c M src/openvpn/multi.h M src/openvpn/multi_io.c 9 files changed, 88 insertions(+), 44 deletions(-) diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h index e5e8709..cd6e32a 100644 --- a/src/openvpn/dco.h +++ b/src/openvpn/dco.h @@ -127,12 +127,13 @@ void close_tun_dco(struct tuntap *tt, openvpn_net_ctx_t *ctx); /** - * Read data from the DCO communication channel (i.e. a control packet) + * Read and process data from the DCO communication channel + * (i.e. a control packet) * * @param dco the DCO context * @return 0 on success or a negative error code otherwise */ -int dco_do_read(dco_context_t *dco); +int dco_read_and_process(dco_context_t *dco); /** * Install a DCO in the main event loop @@ -305,7 +306,7 @@ } static inline int -dco_do_read(dco_context_t *dco) +dco_read_and_process(dco_context_t *dco) { ASSERT(false); return 0; diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c index f2a89ac..d1ad092 100644 --- a/src/openvpn/dco_freebsd.c +++ b/src/openvpn/dco_freebsd.c @@ -578,7 +578,7 @@ } int -dco_do_read(dco_context_t *dco) +dco_read_and_process(dco_context_t *dco) { struct ifdrv drv; uint8_t buf[4096]; @@ -684,11 +684,21 @@ default: msg(M_WARN, "%s: unknown kernel notification %d", __func__, type); + dco->dco_message_type = 0; break; } nvlist_destroy(nvl); + if (dco->c->mode == CM_TOP) + { + multi_process_incoming_dco(dco); + } + else + { + process_incoming_dco(dco); + } + return 0; } diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c index 0ae30b1..2664029 100644 --- a/src/openvpn/dco_linux.c +++ b/src/openvpn/dco_linux.c @@ -49,6 +49,15 @@ #include <netlink/genl/family.h> #include <netlink/genl/ctrl.h> +/* When parsing multiple DEL_PEER notifications, openvpn tries to request stats + * for each DEL_PEER message (see setenv_stats). This triggers a GET_PEER + * request-reply while we are still parsing the rest of the initial + * notifications, which can lead to NLE_BUSY or even NLE_NOMEM. + * + * This basic lock ensures we don't bite our own tail by issuing a dco_get_peer + * while still busy receiving and parsing other messages. + */ +static bool __is_locked = false; /* libnl < 3.5.0 does not set the NLA_F_NESTED on its own, therefore we * have to explicitly do it to prevent the kernel from failing upon @@ -127,7 +136,9 @@ static int ovpn_nl_recvmsgs(dco_context_t *dco, const char *prefix) { + __is_locked = true; int ret = nl_recvmsgs(dco->nl_sock, dco->nl_cb); + __is_locked = false; switch (ret) { @@ -1094,29 +1105,34 @@ * message, that stores the type-specific attributes. * * the "dco" object is then filled accordingly with the information - * retrieved from the message, so that the rest of the OpenVPN code can - * react as need be. + * retrieved from the message, so that *process_incoming_dco can react + * as need be. */ + int ret; switch (gnlh->cmd) { case OVPN_CMD_PEER_GET: { + /* return directly, there are no messages to pass to *process_incoming_dco() */ return ovpn_handle_peer(dco, attrs); } case OVPN_CMD_PEER_DEL_NTF: { - return ovpn_handle_peer_del_ntf(dco, attrs); + ret = ovpn_handle_peer_del_ntf(dco, attrs); + break; } case OVPN_CMD_PEER_FLOAT_NTF: { - return ovpn_handle_peer_float_ntf(dco, attrs); + ret = ovpn_handle_peer_float_ntf(dco, attrs); + break; } case OVPN_CMD_KEY_SWAP_NTF: { - return ovpn_handle_key_swap_ntf(dco, attrs); + ret = ovpn_handle_key_swap_ntf(dco, attrs); + break; } default: @@ -1125,11 +1141,25 @@ return NL_STOP; } + if (ret != NL_OK) + { + return ret; + } + + if (dco->c->mode == CM_TOP) + { + multi_process_incoming_dco(dco); + } + else + { + process_incoming_dco(dco); + } + return NL_OK; } int -dco_do_read(dco_context_t *dco) +dco_read_and_process(dco_context_t *dco) { msg(D_DCO_DEBUG, __func__); @@ -1141,6 +1171,12 @@ { ASSERT(dco); + if (__is_locked) + { + msg(D_DCO_DEBUG, "%s: cannot request peer stats while parsing other messages", __func__); + return 0; + } + /* peer_id == -1 means "dump all peers", but this is allowed in MP mode only. * If it happens in P2P mode it means that the DCO peer was deleted and we * can simply bail out diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c index ca5eedf..94f043f 100644 --- a/src/openvpn/dco_win.c +++ b/src/openvpn/dco_win.c @@ -690,7 +690,7 @@ } int -dco_do_read(dco_context_t *dco) +dco_read_and_process(dco_context_t *dco) { if (dco->ifmode != DCO_MODE_MP) { @@ -727,6 +727,15 @@ break; } + if (dco->c->mode == CM_TOP) + { + multi_process_incoming_dco(dco); + } + else + { + process_incoming_dco(dco); + } + return 0; } diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index bc600d6..6f1bc0c 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -1243,19 +1243,11 @@ } } -static void -process_incoming_dco(struct context *c) +void +process_incoming_dco(dco_context_t *dco) { #if defined(ENABLE_DCO) && (defined(TARGET_LINUX) || defined(TARGET_FREEBSD)) - dco_context_t *dco = &c->c1.tuntap->dco; - - dco_do_read(dco); - - /* no message for us to handle - platform specific code has logged details */ - if (dco->dco_message_type == 0) - { - return; - } + struct context *c = dco->c; /* FreeBSD currently sends us removal notifcation with the old peer-id in * p2p mode with the ping timeout reason, so ignore that one to not shoot @@ -2369,7 +2361,7 @@ { if (!IS_SIG(c)) { - process_incoming_dco(c); + dco_read_and_process(&c->c1.tuntap->dco); } } } diff --git a/src/openvpn/forward.h b/src/openvpn/forward.h index 3cd710e..06808b9 100644 --- a/src/openvpn/forward.h +++ b/src/openvpn/forward.h @@ -210,6 +210,13 @@ const struct sockaddr *float_sa); /** + * Process an incoming DCO message (from kernel space). + * + * @param dco - Pointer to the structure representing the DCO context. + */ +void process_incoming_dco(dco_context_t *dco); + +/** * Write a packet to the external network interface. * @ingroup external_multiplexer * diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 2b944667..153695c 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -3263,14 +3263,12 @@ multi_signal_instance(m, mi, SIGTERM); } -bool -multi_process_incoming_dco(struct multi_context *m) +void +multi_process_incoming_dco(dco_context_t *dco) { - dco_context_t *dco = &m->top.c1.tuntap->dco; + ASSERT(dco->c->multi); - struct multi_instance *mi = NULL; - - int ret = dco_do_read(&m->top.c1.tuntap->dco); + struct multi_context *m = dco->c->multi; int peer_id = dco->dco_message_peer_id; @@ -3279,12 +3277,12 @@ */ if (peer_id < 0) { - return ret > 0; + return; } if ((peer_id < m->max_clients) && (m->instances[peer_id])) { - mi = m->instances[peer_id]; + struct multi_instance *mi = m->instances[peer_id]; set_prefix(mi); if (dco->dco_message_type == OVPN_CMD_DEL_PEER) { @@ -3325,11 +3323,6 @@ "type %d, del_peer_reason %d", peer_id, dco->dco_message_type, dco->dco_del_peer_reason); } - - dco->dco_message_type = 0; - dco->dco_message_peer_id = -1; - dco->dco_del_peer_reason = -1; - return ret > 0; } #endif /* if defined(ENABLE_DCO) */ @@ -4462,4 +4455,4 @@ return (!ipv6_net_contains_host(&ifconfig_local, o->ifconfig_ipv6_netbits, dest)); -} \ No newline at end of file +} diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h index a62b07a..a44f9f2 100644 --- a/src/openvpn/multi.h +++ b/src/openvpn/multi.h @@ -305,13 +305,9 @@ /** * Process an incoming DCO message (from kernel space). * - * @param m - The single \c multi_context structure. - * - * @return - * - True, if the message was received correctly. - * - False, if there was an error while reading the message. + * @param dco - Pointer to the structure representing the DCO context. */ -bool multi_process_incoming_dco(struct multi_context *m); +void multi_process_incoming_dco(dco_context_t *dco); /**************************************************************************/ /** diff --git a/src/openvpn/multi_io.c b/src/openvpn/multi_io.c index fe72456..997951e 100644 --- a/src/openvpn/multi_io.c +++ b/src/openvpn/multi_io.c @@ -505,7 +505,7 @@ /* incoming data on DCO? */ else if (e->arg == MULTI_IO_DCO) { - multi_process_incoming_dco(m); + dco_read_and_process(&m->top.c1.tuntap->dco); } #endif /* signal received? */ -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1403?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Iefc251cb4483c0b9fb9d6a5207db4445cd884d52 Gerrit-Change-Number: 1403 Gerrit-PatchSet: 14 Gerrit-Owner: ralf_lici <ra...@ma...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> 

cron2 has uploaded a new patch set (#14) to the change originally created by ralf_lici. ( http://gerrit.openvpn.net/c/openvpn/+/1403?usp=email ) The following approvals got outdated and were removed: Code-Review+2 by cron2 Change subject: dco: process messages immediately after read ...................................................................... dco: process messages immediately after read Currently, reading and processing of incoming DCO messages are decoupled: notifications are read, parsed, and the relevant information is stored in fields of dco_context_t for later processing (with the only exception being stats). This approach is problematic on Linux, since libnl does not allow reading a single netlink message at a time, which can result in loss of information when multiple notifications are available. This change adopts a read -> parse -> process paradigm. On Linux, processing is now invoked directly from within the parsing callback, which libnl calls for each received netlink packet. The other interfaces are adapted accordingly to unify the processing model across all platforms. On Linux, however, a DEL_PEER notification from the kernel triggers a GET_PEER request from userspace, which clutters the netlink communication logic and can lead to errors or even process exit when multiple simultaneous DEL_PEER notifications are received. To avoid this, introduce a lock that prevents requesting stats while we are still busy parsing other messages. Reported-by: Stefan Baranoff <ste...@tr...> Github: OpenVPN/openvpn#900 Github: OpenVPN/openvpn#918 Github: fixes OpenVPN/openvpn#919 Change-Id: Iefc251cb4483c0b9fb9d6a5207db4445cd884d52 Signed-off-by: Ralf Lici <ra...@ma...> Acked-by: Gert Doering <ge...@gr...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1403 Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg34785.html Signed-off-by: Gert Doering <ge...@gr...> --- M src/openvpn/dco.h M src/openvpn/dco_freebsd.c M src/openvpn/dco_linux.c M src/openvpn/dco_win.c M src/openvpn/forward.c M src/openvpn/forward.h M src/openvpn/multi.c M src/openvpn/multi.h M src/openvpn/multi_io.c 9 files changed, 88 insertions(+), 44 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/03/1403/14 diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h index e5e8709..cd6e32a 100644 --- a/src/openvpn/dco.h +++ b/src/openvpn/dco.h @@ -127,12 +127,13 @@ void close_tun_dco(struct tuntap *tt, openvpn_net_ctx_t *ctx); /** - * Read data from the DCO communication channel (i.e. a control packet) + * Read and process data from the DCO communication channel + * (i.e. a control packet) * * @param dco the DCO context * @return 0 on success or a negative error code otherwise */ -int dco_do_read(dco_context_t *dco); +int dco_read_and_process(dco_context_t *dco); /** * Install a DCO in the main event loop @@ -305,7 +306,7 @@ } static inline int -dco_do_read(dco_context_t *dco) +dco_read_and_process(dco_context_t *dco) { ASSERT(false); return 0; diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c index f2a89ac..d1ad092 100644 --- a/src/openvpn/dco_freebsd.c +++ b/src/openvpn/dco_freebsd.c @@ -578,7 +578,7 @@ } int -dco_do_read(dco_context_t *dco) +dco_read_and_process(dco_context_t *dco) { struct ifdrv drv; uint8_t buf[4096]; @@ -684,11 +684,21 @@ default: msg(M_WARN, "%s: unknown kernel notification %d", __func__, type); + dco->dco_message_type = 0; break; } nvlist_destroy(nvl); + if (dco->c->mode == CM_TOP) + { + multi_process_incoming_dco(dco); + } + else + { + process_incoming_dco(dco); + } + return 0; } diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c index 0ae30b1..2664029 100644 --- a/src/openvpn/dco_linux.c +++ b/src/openvpn/dco_linux.c @@ -49,6 +49,15 @@ #include <netlink/genl/family.h> #include <netlink/genl/ctrl.h> +/* When parsing multiple DEL_PEER notifications, openvpn tries to request stats + * for each DEL_PEER message (see setenv_stats). This triggers a GET_PEER + * request-reply while we are still parsing the rest of the initial + * notifications, which can lead to NLE_BUSY or even NLE_NOMEM. + * + * This basic lock ensures we don't bite our own tail by issuing a dco_get_peer + * while still busy receiving and parsing other messages. + */ +static bool __is_locked = false; /* libnl < 3.5.0 does not set the NLA_F_NESTED on its own, therefore we * have to explicitly do it to prevent the kernel from failing upon @@ -127,7 +136,9 @@ static int ovpn_nl_recvmsgs(dco_context_t *dco, const char *prefix) { + __is_locked = true; int ret = nl_recvmsgs(dco->nl_sock, dco->nl_cb); + __is_locked = false; switch (ret) { @@ -1094,29 +1105,34 @@ * message, that stores the type-specific attributes. * * the "dco" object is then filled accordingly with the information - * retrieved from the message, so that the rest of the OpenVPN code can - * react as need be. + * retrieved from the message, so that *process_incoming_dco can react + * as need be. */ + int ret; switch (gnlh->cmd) { case OVPN_CMD_PEER_GET: { + /* return directly, there are no messages to pass to *process_incoming_dco() */ return ovpn_handle_peer(dco, attrs); } case OVPN_CMD_PEER_DEL_NTF: { - return ovpn_handle_peer_del_ntf(dco, attrs); + ret = ovpn_handle_peer_del_ntf(dco, attrs); + break; } case OVPN_CMD_PEER_FLOAT_NTF: { - return ovpn_handle_peer_float_ntf(dco, attrs); + ret = ovpn_handle_peer_float_ntf(dco, attrs); + break; } case OVPN_CMD_KEY_SWAP_NTF: { - return ovpn_handle_key_swap_ntf(dco, attrs); + ret = ovpn_handle_key_swap_ntf(dco, attrs); + break; } default: @@ -1125,11 +1141,25 @@ return NL_STOP; } + if (ret != NL_OK) + { + return ret; + } + + if (dco->c->mode == CM_TOP) + { + multi_process_incoming_dco(dco); + } + else + { + process_incoming_dco(dco); + } + return NL_OK; } int -dco_do_read(dco_context_t *dco) +dco_read_and_process(dco_context_t *dco) { msg(D_DCO_DEBUG, __func__); @@ -1141,6 +1171,12 @@ { ASSERT(dco); + if (__is_locked) + { + msg(D_DCO_DEBUG, "%s: cannot request peer stats while parsing other messages", __func__); + return 0; + } + /* peer_id == -1 means "dump all peers", but this is allowed in MP mode only. * If it happens in P2P mode it means that the DCO peer was deleted and we * can simply bail out diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c index ca5eedf..94f043f 100644 --- a/src/openvpn/dco_win.c +++ b/src/openvpn/dco_win.c @@ -690,7 +690,7 @@ } int -dco_do_read(dco_context_t *dco) +dco_read_and_process(dco_context_t *dco) { if (dco->ifmode != DCO_MODE_MP) { @@ -727,6 +727,15 @@ break; } + if (dco->c->mode == CM_TOP) + { + multi_process_incoming_dco(dco); + } + else + { + process_incoming_dco(dco); + } + return 0; } diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index bc600d6..6f1bc0c 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -1243,19 +1243,11 @@ } } -static void -process_incoming_dco(struct context *c) +void +process_incoming_dco(dco_context_t *dco) { #if defined(ENABLE_DCO) && (defined(TARGET_LINUX) || defined(TARGET_FREEBSD)) - dco_context_t *dco = &c->c1.tuntap->dco; - - dco_do_read(dco); - - /* no message for us to handle - platform specific code has logged details */ - if (dco->dco_message_type == 0) - { - return; - } + struct context *c = dco->c; /* FreeBSD currently sends us removal notifcation with the old peer-id in * p2p mode with the ping timeout reason, so ignore that one to not shoot @@ -2369,7 +2361,7 @@ { if (!IS_SIG(c)) { - process_incoming_dco(c); + dco_read_and_process(&c->c1.tuntap->dco); } } } diff --git a/src/openvpn/forward.h b/src/openvpn/forward.h index 3cd710e..06808b9 100644 --- a/src/openvpn/forward.h +++ b/src/openvpn/forward.h @@ -210,6 +210,13 @@ const struct sockaddr *float_sa); /** + * Process an incoming DCO message (from kernel space). + * + * @param dco - Pointer to the structure representing the DCO context. + */ +void process_incoming_dco(dco_context_t *dco); + +/** * Write a packet to the external network interface. * @ingroup external_multiplexer * diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 2b944667..153695c 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -3263,14 +3263,12 @@ multi_signal_instance(m, mi, SIGTERM); } -bool -multi_process_incoming_dco(struct multi_context *m) +void +multi_process_incoming_dco(dco_context_t *dco) { - dco_context_t *dco = &m->top.c1.tuntap->dco; + ASSERT(dco->c->multi); - struct multi_instance *mi = NULL; - - int ret = dco_do_read(&m->top.c1.tuntap->dco); + struct multi_context *m = dco->c->multi; int peer_id = dco->dco_message_peer_id; @@ -3279,12 +3277,12 @@ */ if (peer_id < 0) { - return ret > 0; + return; } if ((peer_id < m->max_clients) && (m->instances[peer_id])) { - mi = m->instances[peer_id]; + struct multi_instance *mi = m->instances[peer_id]; set_prefix(mi); if (dco->dco_message_type == OVPN_CMD_DEL_PEER) { @@ -3325,11 +3323,6 @@ "type %d, del_peer_reason %d", peer_id, dco->dco_message_type, dco->dco_del_peer_reason); } - - dco->dco_message_type = 0; - dco->dco_message_peer_id = -1; - dco->dco_del_peer_reason = -1; - return ret > 0; } #endif /* if defined(ENABLE_DCO) */ @@ -4462,4 +4455,4 @@ return (!ipv6_net_contains_host(&ifconfig_local, o->ifconfig_ipv6_netbits, dest)); -} \ No newline at end of file +} diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h index a62b07a..a44f9f2 100644 --- a/src/openvpn/multi.h +++ b/src/openvpn/multi.h @@ -305,13 +305,9 @@ /** * Process an incoming DCO message (from kernel space). * - * @param m - The single \c multi_context structure. - * - * @return - * - True, if the message was received correctly. - * - False, if there was an error while reading the message. + * @param dco - Pointer to the structure representing the DCO context. */ -bool multi_process_incoming_dco(struct multi_context *m); +void multi_process_incoming_dco(dco_context_t *dco); /**************************************************************************/ /** diff --git a/src/openvpn/multi_io.c b/src/openvpn/multi_io.c index fe72456..997951e 100644 --- a/src/openvpn/multi_io.c +++ b/src/openvpn/multi_io.c @@ -505,7 +505,7 @@ /* incoming data on DCO? */ else if (e->arg == MULTI_IO_DCO) { - multi_process_incoming_dco(m); + dco_read_and_process(&m->top.c1.tuntap->dco); } #endif /* signal received? */ -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1403?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Iefc251cb4483c0b9fb9d6a5207db4445cd884d52 Gerrit-Change-Number: 1403 Gerrit-PatchSet: 14 Gerrit-Owner: ralf_lici <ra...@ma...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> 

This was quite a bit of a journey... after staring at the code and deciding "yes this looks good" it promptly crashed on me, with most interesting error messages (connecting 100 clients simultaneously and disconnecting simultaneously as well, no EEN, so triggering 100 quasi-simultaneous PEER_DEL notifications from the kernel). After this was fixed, the Gremlins crashed it again, in an ASSERT(es)... but with v13 I can no longer break it *and* we're reasonably sure we understand what is and was happening. Tested really really hard on ubuntu 20.04 + backports, and lightly (t_client and t_server, no gremlins) on FreeBSD 14. Since the FreeBSD notification mechanism works differently, we do not expect the same sort of problems (missing / batched notifications) to materialize there. Added Github references to #900 (sbaranoff), #918, #919 (mine). The latter two are basically things that broke in earlier versions of the patch, and #918 is kept open to see if we could make the setenv_stats() <-> DCO interaction better. Your patch has been applied to the master branch. commit 7791f5358a5574d4ef1bd27e2d52300c9d98bd72 Author: Ralf Lici Date: Fri Nov 28 12:26:59 2025 +0100 dco: process messages immediately after read Signed-off-by: Ralf Lici <ra...@ma...> Acked-by: Gert Doering <ge...@gr...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1403 Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg34785.html Signed-off-by: Gert Doering <ge...@gr...> -- kind regards, Gert Doering 

From: Ralf Lici <ra...@ma...> Currently, reading and processing of incoming DCO messages are decoupled: notifications are read, parsed, and the relevant information is stored in fields of dco_context_t for later processing (with the only exception being stats). This approach is problematic on Linux, since libnl does not allow reading a single netlink message at a time, which can result in loss of information when multiple notifications are available. This change adopts a read -> parse -> process paradigm. On Linux, processing is now invoked directly from within the parsing callback, which libnl calls for each received netlink packet. The other interfaces are adapted accordingly to unify the processing model across all platforms. On Linux, however, a DEL_PEER notification from the kernel triggers a GET_PEER request from userspace, which clutters the netlink communication logic and can lead to errors or even process exit when multiple simultaneous DEL_PEER notifications are received. To avoid this, introduce a lock that prevents requesting stats while we are still busy parsing other messages. Reported-by: Stefan Baranoff <ste...@tr...> Change-Id: Iefc251cb4483c0b9fb9d6a5207db4445cd884d52 Signed-off-by: Ralf Lici <ra...@ma...> Acked-by: Gert Doering <ge...@gr...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1403 --- 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/+/1403 This mail reflects revision 13 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering <ge...@gr...> diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h index e5e8709..cd6e32a 100644 --- a/src/openvpn/dco.h +++ b/src/openvpn/dco.h @@ -127,12 +127,13 @@ void close_tun_dco(struct tuntap *tt, openvpn_net_ctx_t *ctx); /** - * Read data from the DCO communication channel (i.e. a control packet) + * Read and process data from the DCO communication channel + * (i.e. a control packet) * * @param dco the DCO context * @return 0 on success or a negative error code otherwise */ -int dco_do_read(dco_context_t *dco); +int dco_read_and_process(dco_context_t *dco); /** * Install a DCO in the main event loop @@ -305,7 +306,7 @@ } static inline int -dco_do_read(dco_context_t *dco) +dco_read_and_process(dco_context_t *dco) { ASSERT(false); return 0; diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c index f2a89ac..d1ad092 100644 --- a/src/openvpn/dco_freebsd.c +++ b/src/openvpn/dco_freebsd.c @@ -578,7 +578,7 @@ } int -dco_do_read(dco_context_t *dco) +dco_read_and_process(dco_context_t *dco) { struct ifdrv drv; uint8_t buf[4096]; @@ -684,11 +684,21 @@ default: msg(M_WARN, "%s: unknown kernel notification %d", __func__, type); + dco->dco_message_type = 0; break; } nvlist_destroy(nvl); + if (dco->c->mode == CM_TOP) + { + multi_process_incoming_dco(dco); + } + else + { + process_incoming_dco(dco); + } + return 0; } diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c index 0ae30b1..2664029 100644 --- a/src/openvpn/dco_linux.c +++ b/src/openvpn/dco_linux.c @@ -49,6 +49,15 @@ #include <netlink/genl/family.h> #include <netlink/genl/ctrl.h> +/* When parsing multiple DEL_PEER notifications, openvpn tries to request stats + * for each DEL_PEER message (see setenv_stats). This triggers a GET_PEER + * request-reply while we are still parsing the rest of the initial + * notifications, which can lead to NLE_BUSY or even NLE_NOMEM. + * + * This basic lock ensures we don't bite our own tail by issuing a dco_get_peer + * while still busy receiving and parsing other messages. + */ +static bool __is_locked = false; /* libnl < 3.5.0 does not set the NLA_F_NESTED on its own, therefore we * have to explicitly do it to prevent the kernel from failing upon @@ -127,7 +136,9 @@ static int ovpn_nl_recvmsgs(dco_context_t *dco, const char *prefix) { + __is_locked = true; int ret = nl_recvmsgs(dco->nl_sock, dco->nl_cb); + __is_locked = false; switch (ret) { @@ -1094,29 +1105,34 @@ * message, that stores the type-specific attributes. * * the "dco" object is then filled accordingly with the information - * retrieved from the message, so that the rest of the OpenVPN code can - * react as need be. + * retrieved from the message, so that *process_incoming_dco can react + * as need be. */ + int ret; switch (gnlh->cmd) { case OVPN_CMD_PEER_GET: { + /* return directly, there are no messages to pass to *process_incoming_dco() */ return ovpn_handle_peer(dco, attrs); } case OVPN_CMD_PEER_DEL_NTF: { - return ovpn_handle_peer_del_ntf(dco, attrs); + ret = ovpn_handle_peer_del_ntf(dco, attrs); + break; } case OVPN_CMD_PEER_FLOAT_NTF: { - return ovpn_handle_peer_float_ntf(dco, attrs); + ret = ovpn_handle_peer_float_ntf(dco, attrs); + break; } case OVPN_CMD_KEY_SWAP_NTF: { - return ovpn_handle_key_swap_ntf(dco, attrs); + ret = ovpn_handle_key_swap_ntf(dco, attrs); + break; } default: @@ -1125,11 +1141,25 @@ return NL_STOP; } + if (ret != NL_OK) + { + return ret; + } + + if (dco->c->mode == CM_TOP) + { + multi_process_incoming_dco(dco); + } + else + { + process_incoming_dco(dco); + } + return NL_OK; } int -dco_do_read(dco_context_t *dco) +dco_read_and_process(dco_context_t *dco) { msg(D_DCO_DEBUG, __func__); @@ -1141,6 +1171,12 @@ { ASSERT(dco); + if (__is_locked) + { + msg(D_DCO_DEBUG, "%s: cannot request peer stats while parsing other messages", __func__); + return 0; + } + /* peer_id == -1 means "dump all peers", but this is allowed in MP mode only. * If it happens in P2P mode it means that the DCO peer was deleted and we * can simply bail out diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c index ca5eedf..94f043f 100644 --- a/src/openvpn/dco_win.c +++ b/src/openvpn/dco_win.c @@ -690,7 +690,7 @@ } int -dco_do_read(dco_context_t *dco) +dco_read_and_process(dco_context_t *dco) { if (dco->ifmode != DCO_MODE_MP) { @@ -727,6 +727,15 @@ break; } + if (dco->c->mode == CM_TOP) + { + multi_process_incoming_dco(dco); + } + else + { + process_incoming_dco(dco); + } + return 0; } diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index ccb8404..1a68af4 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -1243,19 +1243,11 @@ } } -static void -process_incoming_dco(struct context *c) +void +process_incoming_dco(dco_context_t *dco) { #if defined(ENABLE_DCO) && (defined(TARGET_LINUX) || defined(TARGET_FREEBSD)) - dco_context_t *dco = &c->c1.tuntap->dco; - - dco_do_read(dco); - - /* no message for us to handle - platform specific code has logged details */ - if (dco->dco_message_type == 0) - { - return; - } + struct context *c = dco->c; /* FreeBSD currently sends us removal notifcation with the old peer-id in * p2p mode with the ping timeout reason, so ignore that one to not shoot @@ -2369,7 +2361,7 @@ { if (!IS_SIG(c)) { - process_incoming_dco(c); + dco_read_and_process(&c->c1.tuntap->dco); } } } diff --git a/src/openvpn/forward.h b/src/openvpn/forward.h index a575faf..9e9b02e 100644 --- a/src/openvpn/forward.h +++ b/src/openvpn/forward.h @@ -210,6 +210,13 @@ const struct sockaddr *float_sa); /** + * Process an incoming DCO message (from kernel space). + * + * @param dco - Pointer to the structure representing the DCO context. + */ +void process_incoming_dco(dco_context_t *dco); + +/** * Write a packet to the external network interface. * @ingroup external_multiplexer * diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 2b944667..153695c 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -3263,14 +3263,12 @@ multi_signal_instance(m, mi, SIGTERM); } -bool -multi_process_incoming_dco(struct multi_context *m) +void +multi_process_incoming_dco(dco_context_t *dco) { - dco_context_t *dco = &m->top.c1.tuntap->dco; + ASSERT(dco->c->multi); - struct multi_instance *mi = NULL; - - int ret = dco_do_read(&m->top.c1.tuntap->dco); + struct multi_context *m = dco->c->multi; int peer_id = dco->dco_message_peer_id; @@ -3279,12 +3277,12 @@ */ if (peer_id < 0) { - return ret > 0; + return; } if ((peer_id < m->max_clients) && (m->instances[peer_id])) { - mi = m->instances[peer_id]; + struct multi_instance *mi = m->instances[peer_id]; set_prefix(mi); if (dco->dco_message_type == OVPN_CMD_DEL_PEER) { @@ -3325,11 +3323,6 @@ "type %d, del_peer_reason %d", peer_id, dco->dco_message_type, dco->dco_del_peer_reason); } - - dco->dco_message_type = 0; - dco->dco_message_peer_id = -1; - dco->dco_del_peer_reason = -1; - return ret > 0; } #endif /* if defined(ENABLE_DCO) */ @@ -4462,4 +4455,4 @@ return (!ipv6_net_contains_host(&ifconfig_local, o->ifconfig_ipv6_netbits, dest)); -} \ No newline at end of file +} diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h index a62b07a..a44f9f2 100644 --- a/src/openvpn/multi.h +++ b/src/openvpn/multi.h @@ -305,13 +305,9 @@ /** * Process an incoming DCO message (from kernel space). * - * @param m - The single \c multi_context structure. - * - * @return - * - True, if the message was received correctly. - * - False, if there was an error while reading the message. + * @param dco - Pointer to the structure representing the DCO context. */ -bool multi_process_incoming_dco(struct multi_context *m); +void multi_process_incoming_dco(dco_context_t *dco); /**************************************************************************/ /** diff --git a/src/openvpn/multi_io.c b/src/openvpn/multi_io.c index fe72456..997951e 100644 --- a/src/openvpn/multi_io.c +++ b/src/openvpn/multi_io.c @@ -505,7 +505,7 @@ /* incoming data on DCO? */ else if (e->arg == MULTI_IO_DCO) { - multi_process_incoming_dco(m); + dco_read_and_process(&m->top.c1.tuntap->dco); } #endif /* signal received? */ 

Attention is currently required from: plaisthos, ralf_lici. cron2 has posted comments on this change by ralf_lici. ( http://gerrit.openvpn.net/c/openvpn/+/1403?usp=email ) Change subject: dco: process messages immediately after read ...................................................................... Patch Set 13: Code-Review+2 -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1403?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Iefc251cb4483c0b9fb9d6a5207db4445cd884d52 Gerrit-Change-Number: 1403 Gerrit-PatchSet: 13 Gerrit-Owner: ralf_lici <ra...@ma...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: ralf_lici <ra...@ma...> Gerrit-Comment-Date: Fri, 28 Nov 2025 11:26:53 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes 

Attention is currently required from: cron2, plaisthos, ralf_lici. Hello cron2, plaisthos, I'd like you to reexamine a change. Please visit http://gerrit.openvpn.net/c/openvpn/+/1403?usp=email to look at the new patch set (#13). The following approvals got outdated and were removed: Code-Review+2 by cron2 Change subject: dco: process messages immediately after read ...................................................................... dco: process messages immediately after read Currently, reading and processing of incoming DCO messages are decoupled: notifications are read, parsed, and the relevant information is stored in fields of dco_context_t for later processing (with the only exception being stats). This approach is problematic on Linux, since libnl does not allow reading a single netlink message at a time, which can result in loss of information when multiple notifications are available. This change adopts a read -> parse -> process paradigm. On Linux, processing is now invoked directly from within the parsing callback, which libnl calls for each received netlink packet. The other interfaces are adapted accordingly to unify the processing model across all platforms. On Linux, however, a DEL_PEER notification from the kernel triggers a GET_PEER request from userspace, which clutters the netlink communication logic and can lead to errors or even process exit when multiple simultaneous DEL_PEER notifications are received. To avoid this, introduce a lock that prevents requesting stats while we are still busy parsing other messages. Reported-by: Stefan Baranoff <ste...@tr...> Change-Id: Iefc251cb4483c0b9fb9d6a5207db4445cd884d52 Signed-off-by: Ralf Lici <ra...@ma...> --- M src/openvpn/dco.h M src/openvpn/dco_freebsd.c M src/openvpn/dco_linux.c M src/openvpn/dco_win.c M src/openvpn/forward.c M src/openvpn/forward.h M src/openvpn/multi.c M src/openvpn/multi.h M src/openvpn/multi_io.c 9 files changed, 88 insertions(+), 44 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/03/1403/13 diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h index e5e8709..cd6e32a 100644 --- a/src/openvpn/dco.h +++ b/src/openvpn/dco.h @@ -127,12 +127,13 @@ void close_tun_dco(struct tuntap *tt, openvpn_net_ctx_t *ctx); /** - * Read data from the DCO communication channel (i.e. a control packet) + * Read and process data from the DCO communication channel + * (i.e. a control packet) * * @param dco the DCO context * @return 0 on success or a negative error code otherwise */ -int dco_do_read(dco_context_t *dco); +int dco_read_and_process(dco_context_t *dco); /** * Install a DCO in the main event loop @@ -305,7 +306,7 @@ } static inline int -dco_do_read(dco_context_t *dco) +dco_read_and_process(dco_context_t *dco) { ASSERT(false); return 0; diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c index f2a89ac..d1ad092 100644 --- a/src/openvpn/dco_freebsd.c +++ b/src/openvpn/dco_freebsd.c @@ -578,7 +578,7 @@ } int -dco_do_read(dco_context_t *dco) +dco_read_and_process(dco_context_t *dco) { struct ifdrv drv; uint8_t buf[4096]; @@ -684,11 +684,21 @@ default: msg(M_WARN, "%s: unknown kernel notification %d", __func__, type); + dco->dco_message_type = 0; break; } nvlist_destroy(nvl); + if (dco->c->mode == CM_TOP) + { + multi_process_incoming_dco(dco); + } + else + { + process_incoming_dco(dco); + } + return 0; } diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c index 0ae30b1..2664029 100644 --- a/src/openvpn/dco_linux.c +++ b/src/openvpn/dco_linux.c @@ -49,6 +49,15 @@ #include <netlink/genl/family.h> #include <netlink/genl/ctrl.h> +/* When parsing multiple DEL_PEER notifications, openvpn tries to request stats + * for each DEL_PEER message (see setenv_stats). This triggers a GET_PEER + * request-reply while we are still parsing the rest of the initial + * notifications, which can lead to NLE_BUSY or even NLE_NOMEM. + * + * This basic lock ensures we don't bite our own tail by issuing a dco_get_peer + * while still busy receiving and parsing other messages. + */ +static bool __is_locked = false; /* libnl < 3.5.0 does not set the NLA_F_NESTED on its own, therefore we * have to explicitly do it to prevent the kernel from failing upon @@ -127,7 +136,9 @@ static int ovpn_nl_recvmsgs(dco_context_t *dco, const char *prefix) { + __is_locked = true; int ret = nl_recvmsgs(dco->nl_sock, dco->nl_cb); + __is_locked = false; switch (ret) { @@ -1094,29 +1105,34 @@ * message, that stores the type-specific attributes. * * the "dco" object is then filled accordingly with the information - * retrieved from the message, so that the rest of the OpenVPN code can - * react as need be. + * retrieved from the message, so that *process_incoming_dco can react + * as need be. */ + int ret; switch (gnlh->cmd) { case OVPN_CMD_PEER_GET: { + /* return directly, there are no messages to pass to *process_incoming_dco() */ return ovpn_handle_peer(dco, attrs); } case OVPN_CMD_PEER_DEL_NTF: { - return ovpn_handle_peer_del_ntf(dco, attrs); + ret = ovpn_handle_peer_del_ntf(dco, attrs); + break; } case OVPN_CMD_PEER_FLOAT_NTF: { - return ovpn_handle_peer_float_ntf(dco, attrs); + ret = ovpn_handle_peer_float_ntf(dco, attrs); + break; } case OVPN_CMD_KEY_SWAP_NTF: { - return ovpn_handle_key_swap_ntf(dco, attrs); + ret = ovpn_handle_key_swap_ntf(dco, attrs); + break; } default: @@ -1125,11 +1141,25 @@ return NL_STOP; } + if (ret != NL_OK) + { + return ret; + } + + if (dco->c->mode == CM_TOP) + { + multi_process_incoming_dco(dco); + } + else + { + process_incoming_dco(dco); + } + return NL_OK; } int -dco_do_read(dco_context_t *dco) +dco_read_and_process(dco_context_t *dco) { msg(D_DCO_DEBUG, __func__); @@ -1141,6 +1171,12 @@ { ASSERT(dco); + if (__is_locked) + { + msg(D_DCO_DEBUG, "%s: cannot request peer stats while parsing other messages", __func__); + return 0; + } + /* peer_id == -1 means "dump all peers", but this is allowed in MP mode only. * If it happens in P2P mode it means that the DCO peer was deleted and we * can simply bail out diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c index ca5eedf..94f043f 100644 --- a/src/openvpn/dco_win.c +++ b/src/openvpn/dco_win.c @@ -690,7 +690,7 @@ } int -dco_do_read(dco_context_t *dco) +dco_read_and_process(dco_context_t *dco) { if (dco->ifmode != DCO_MODE_MP) { @@ -727,6 +727,15 @@ break; } + if (dco->c->mode == CM_TOP) + { + multi_process_incoming_dco(dco); + } + else + { + process_incoming_dco(dco); + } + return 0; } diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index ccb8404..1a68af4 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -1243,19 +1243,11 @@ } } -static void -process_incoming_dco(struct context *c) +void +process_incoming_dco(dco_context_t *dco) { #if defined(ENABLE_DCO) && (defined(TARGET_LINUX) || defined(TARGET_FREEBSD)) - dco_context_t *dco = &c->c1.tuntap->dco; - - dco_do_read(dco); - - /* no message for us to handle - platform specific code has logged details */ - if (dco->dco_message_type == 0) - { - return; - } + struct context *c = dco->c; /* FreeBSD currently sends us removal notifcation with the old peer-id in * p2p mode with the ping timeout reason, so ignore that one to not shoot @@ -2369,7 +2361,7 @@ { if (!IS_SIG(c)) { - process_incoming_dco(c); + dco_read_and_process(&c->c1.tuntap->dco); } } } diff --git a/src/openvpn/forward.h b/src/openvpn/forward.h index a575faf..9e9b02e 100644 --- a/src/openvpn/forward.h +++ b/src/openvpn/forward.h @@ -210,6 +210,13 @@ const struct sockaddr *float_sa); /** + * Process an incoming DCO message (from kernel space). + * + * @param dco - Pointer to the structure representing the DCO context. + */ +void process_incoming_dco(dco_context_t *dco); + +/** * Write a packet to the external network interface. * @ingroup external_multiplexer * diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 2b944667..153695c 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -3263,14 +3263,12 @@ multi_signal_instance(m, mi, SIGTERM); } -bool -multi_process_incoming_dco(struct multi_context *m) +void +multi_process_incoming_dco(dco_context_t *dco) { - dco_context_t *dco = &m->top.c1.tuntap->dco; + ASSERT(dco->c->multi); - struct multi_instance *mi = NULL; - - int ret = dco_do_read(&m->top.c1.tuntap->dco); + struct multi_context *m = dco->c->multi; int peer_id = dco->dco_message_peer_id; @@ -3279,12 +3277,12 @@ */ if (peer_id < 0) { - return ret > 0; + return; } if ((peer_id < m->max_clients) && (m->instances[peer_id])) { - mi = m->instances[peer_id]; + struct multi_instance *mi = m->instances[peer_id]; set_prefix(mi); if (dco->dco_message_type == OVPN_CMD_DEL_PEER) { @@ -3325,11 +3323,6 @@ "type %d, del_peer_reason %d", peer_id, dco->dco_message_type, dco->dco_del_peer_reason); } - - dco->dco_message_type = 0; - dco->dco_message_peer_id = -1; - dco->dco_del_peer_reason = -1; - return ret > 0; } #endif /* if defined(ENABLE_DCO) */ @@ -4462,4 +4455,4 @@ return (!ipv6_net_contains_host(&ifconfig_local, o->ifconfig_ipv6_netbits, dest)); -} \ No newline at end of file +} diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h index a62b07a..a44f9f2 100644 --- a/src/openvpn/multi.h +++ b/src/openvpn/multi.h @@ -305,13 +305,9 @@ /** * Process an incoming DCO message (from kernel space). * - * @param m - The single \c multi_context structure. - * - * @return - * - True, if the message was received correctly. - * - False, if there was an error while reading the message. + * @param dco - Pointer to the structure representing the DCO context. */ -bool multi_process_incoming_dco(struct multi_context *m); +void multi_process_incoming_dco(dco_context_t *dco); /**************************************************************************/ /** diff --git a/src/openvpn/multi_io.c b/src/openvpn/multi_io.c index fe72456..997951e 100644 --- a/src/openvpn/multi_io.c +++ b/src/openvpn/multi_io.c @@ -505,7 +505,7 @@ /* incoming data on DCO? */ else if (e->arg == MULTI_IO_DCO) { - multi_process_incoming_dco(m); + dco_read_and_process(&m->top.c1.tuntap->dco); } #endif /* signal received? */ -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1403?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Iefc251cb4483c0b9fb9d6a5207db4445cd884d52 Gerrit-Change-Number: 1403 Gerrit-PatchSet: 13 Gerrit-Owner: ralf_lici <ra...@ma...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: cron2 <ge...@gr...> Gerrit-Attention: ralf_lici <ra...@ma...> 

Attention is currently required from: MaxF, plaisthos. Hello flichtenheld, plaisthos, I'd like you to reexamine a change. Please visit http://gerrit.openvpn.net/c/openvpn/+/1412?usp=email to look at the new patch set (#2). Change subject: Drop Mbed TLS 2.X compatibility ...................................................................... Drop Mbed TLS 2.X compatibility Mbed TLS 2.28 is out of support since March and adding support for Mbed TLS 4 will get ugly enough without the old compatibility code lying around too. Mbed TLS 2.28 still ships on some supported distributions (e.g. Ubuntu 24.04) but nobody is maintaining openvpn-mbedtls packages there. This commit will probably break on some test machines. Change-Id: Ia4afabcb6006dc9304a4c09f824d9c7c2d4d64ad Signed-off-by: Max Fillinger <max...@se...> --- M README.mbedtls M configure.ac M src/openvpn/crypto_mbedtls.c M src/openvpn/mbedtls_compat.h M src/openvpn/ssl_mbedtls.c M src/openvpn/ssl_mbedtls.h M src/openvpn/ssl_verify_mbedtls.c 7 files changed, 21 insertions(+), 231 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/12/1412/2 diff --git a/README.mbedtls b/README.mbedtls index a1012e9..fb30db1 100644 --- a/README.mbedtls +++ b/README.mbedtls @@ -7,7 +7,8 @@	make	make install -This version requires mbed TLS version >= 2.0.0 or >= 3.2.1. +This version requires mbed TLS version >= 3.2.1. Versions >= 4.0.0 are not +yet supported. Support for TLS 1.3 requires an Mbed TLS version >= 3.6.4. ************************************************************************* @@ -23,12 +24,3 @@ * X.509 subject line has a different format than the OpenSSL subject line * X.509 certificate tracking - -************************************************************************* - -Mbed TLS 3 has implemented TLS 1.3, but support in OpenVPN requires the -function mbedtls_ssl_export_keying_material() which is currently not in -any released version. It is available when building mbed TLS from source -(mbedtls-3.6 or development branch). - -Without this function, only TLS 1.2 is available. diff --git a/configure.ac b/configure.ac index 44c7b65..ad86031 100644 --- a/configure.ac +++ b/configure.ac @@ -1020,13 +1020,13 @@ #include <mbedtls/version.h>	]],	[[ -#if MBEDTLS_VERSION_NUMBER < 0x02000000 || (MBEDTLS_VERSION_NUMBER >= 0x03000000 && MBEDTLS_VERSION_NUMBER < 0x03020100) +#if MBEDTLS_VERSION_NUMBER < 0x03020100 #error invalid version #endif	]]	)],	[AC_MSG_RESULT([ok])], -	[AC_MSG_ERROR([mbed TLS version >= 2.0.0 or >= 3.2.1 required])] +	[AC_MSG_ERROR([mbed TLS version >= 3.2.1 required])]	)	AC_CHECK_HEADERS(psa/crypto.h) @@ -1043,12 +1043,6 @@	fi	fi -	AC_CHECK_FUNC( -	[mbedtls_ctr_drbg_update_ret], -	AC_DEFINE([HAVE_MBEDTLS_CTR_DRBG_UPDATE_RET], [1], - [Use mbedtls_ctr_drbg_update_ret from mbed TLS]), -	) -	CFLAGS="${saved_CFLAGS}"	LIBS="${saved_LIBS}"	AC_DEFINE([ENABLE_CRYPTO_MBEDTLS], [1], [Use mbed TLS library]) diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c index 89f0ab9..63c665f 100644 --- a/src/openvpn/crypto_mbedtls.c +++ b/src/openvpn/crypto_mbedtls.c @@ -41,7 +41,6 @@ #include "integer.h" #include "crypto_backend.h" #include "otime.h" -#include "mbedtls_compat.h" #include "misc.h" #include <mbedtls/base64.h> diff --git a/src/openvpn/mbedtls_compat.h b/src/openvpn/mbedtls_compat.h index 68096c4..540f370 100644 --- a/src/openvpn/mbedtls_compat.h +++ b/src/openvpn/mbedtls_compat.h @@ -23,10 +23,8 @@ /** * @file * mbedtls compatibility stub. - * This file provide compatibility stubs for the mbedtls libraries - * prior to version 3. This version made most fields in structs private - * and requires accessor functions to be used. For earlier versions, we - * implement the accessor functions here. + * This file provides compatibility stubs to handle API differences between + * different versions of Mbed TLS. */ #ifndef MBEDTLS_COMPAT_H_ @@ -36,27 +34,10 @@ #include "errlevel.h" -#include <mbedtls/cipher.h> -#include <mbedtls/ctr_drbg.h> -#include <mbedtls/dhm.h> -#include <mbedtls/ecp.h> -#include <mbedtls/md.h> -#include <mbedtls/pem.h> -#include <mbedtls/pk.h> -#include <mbedtls/ssl.h> -#include <mbedtls/version.h> -#include <mbedtls/x509_crt.h> - #ifdef HAVE_PSA_CRYPTO_H #include <psa/crypto.h> #endif -#if MBEDTLS_VERSION_NUMBER >= 0x03000000 -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) { @@ -70,162 +51,4 @@ #endif } -static inline mbedtls_compat_group_id -mbedtls_compat_get_group_id(const mbedtls_ecp_curve_info *curve_info) -{ -#if MBEDTLS_VERSION_NUMBER >= 0x03000000 - 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() - * which does. - * - * In mbedtls 3, this function was removed and mbedtls_ctr_drbg_update() returns - * an error code. - */ -static inline int -mbedtls_compat_ctr_drbg_update(mbedtls_ctr_drbg_context *ctx, const unsigned char *additional, - size_t add_len) -{ -#if MBEDTLS_VERSION_NUMBER > 0x03000000 - return mbedtls_ctr_drbg_update(ctx, additional, add_len); -#elif defined(HAVE_MBEDTLS_CTR_DRBG_UPDATE_RET) - return mbedtls_ctr_drbg_update_ret(ctx, additional, add_len); -#else - mbedtls_ctr_drbg_update(ctx, additional, add_len); - return 0; -#endif /* HAVE_MBEDTLS_CTR_DRBG_UPDATE_RET */ -} - -static inline int -mbedtls_compat_pk_check_pair(const mbedtls_pk_context *pub, const mbedtls_pk_context *prv, - int (*f_rng)(void *, unsigned char *, size_t), void *p_rng) -{ -#if MBEDTLS_VERSION_NUMBER < 0x03020100 - return mbedtls_pk_check_pair(pub, prv); -#else - return mbedtls_pk_check_pair(pub, prv, f_rng, p_rng); -#endif /* MBEDTLS_VERSION_NUMBER < 0x03020100 */ -} - -static inline int -mbedtls_compat_pk_parse_key(mbedtls_pk_context *ctx, const unsigned char *key, size_t keylen, - const unsigned char *pwd, size_t pwdlen, - int (*f_rng)(void *, unsigned char *, size_t), void *p_rng) -{ -#if MBEDTLS_VERSION_NUMBER < 0x03020100 - return mbedtls_pk_parse_key(ctx, key, keylen, pwd, pwdlen); -#else - return mbedtls_pk_parse_key(ctx, key, keylen, pwd, pwdlen, f_rng, p_rng); -#endif -} - -static inline int -mbedtls_compat_pk_parse_keyfile(mbedtls_pk_context *ctx, const char *path, const char *password, - int (*f_rng)(void *, unsigned char *, size_t), void *p_rng) -{ -#if MBEDTLS_VERSION_NUMBER < 0x03020100 - return mbedtls_pk_parse_keyfile(ctx, path, password); -#else - return mbedtls_pk_parse_keyfile(ctx, path, password, f_rng, p_rng); -#endif -} - -#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) -{ - return (size_t)cipher->block_size; -} - -static inline size_t -mbedtls_cipher_info_get_iv_size(const mbedtls_cipher_info_t *cipher) -{ - return (size_t)cipher->iv_size; -} - -static inline size_t -mbedtls_cipher_info_get_key_bitlen(const mbedtls_cipher_info_t *cipher) -{ - return (size_t)cipher->key_bitlen; -} - -static inline mbedtls_cipher_mode_t -mbedtls_cipher_info_get_mode(const mbedtls_cipher_info_t *cipher) -{ - return cipher->mode; -} - -static inline const char * -mbedtls_cipher_info_get_name(const mbedtls_cipher_info_t *cipher) -{ - return cipher->name; -} - -static inline mbedtls_cipher_type_t -mbedtls_cipher_info_get_type(const mbedtls_cipher_info_t *cipher) -{ - return cipher->type; -} - -static inline size_t -mbedtls_dhm_get_bitlen(const mbedtls_dhm_context *ctx) -{ - return 8 * ctx->len; -} - -static inline const mbedtls_md_info_t * -mbedtls_md_info_from_ctx(const mbedtls_md_context_t *ctx) -{ - return ctx->md_info; -} - -static inline const unsigned char * -mbedtls_pem_get_buffer(const mbedtls_pem_context *ctx, size_t *buf_size) -{ - *buf_size = ctx->buflen; - return ctx->buf; -} - -static inline int -mbedtls_x509_crt_has_ext_type(const mbedtls_x509_crt *ctx, int ext_type) -{ - return ctx->ext_types & ext_type; -} -#endif /* MBEDTLS_VERSION_NUMBER < 0x03020100 */ - #endif /* MBEDTLS_COMPAT_H_ */ diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index 488f9b9..3e000ef 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -49,13 +49,8 @@ #include "ssl_verify_mbedtls.h" #include <mbedtls/debug.h> #include <mbedtls/error.h> -#include <mbedtls/version.h> - -#if MBEDTLS_VERSION_NUMBER >= 0x02040000 #include <mbedtls/net_sockets.h> -#else -#include <mbedtls/net.h> -#endif +#include <mbedtls/version.h> #include <mbedtls/oid.h> #include <mbedtls/pem.h> @@ -397,7 +392,7 @@ /* Get number of groups and allocate an array in ctx */ int groups_count = get_num_elements(groups, ':'); - ALLOC_ARRAY_CLEAR(ctx->groups, mbedtls_compat_group_id, groups_count + 1) + ALLOC_ARRAY_CLEAR(ctx->groups, uint16_t, groups_count + 1) /* Parse allowed ciphers, getting IDs */ int i = 0; @@ -413,7 +408,7 @@ } else { - ctx->groups[i] = mbedtls_compat_get_group_id(ci); + ctx->groups[i] = ci->tls_id; i++; } } @@ -537,29 +532,29 @@ if (priv_key_inline) { - status = mbedtls_compat_pk_parse_key(ctx->priv_key, (const unsigned char *)priv_key_file, - strlen(priv_key_file) + 1, NULL, 0, - mbedtls_ctr_drbg_random, rand_ctx_get()); + status = mbedtls_pk_parse_key(ctx->priv_key, (const unsigned char *)priv_key_file, + strlen(priv_key_file) + 1, NULL, 0, + mbedtls_ctr_drbg_random, rand_ctx_get()); if (MBEDTLS_ERR_PK_PASSWORD_REQUIRED == status) { char passbuf[512] = { 0 }; pem_password_callback(passbuf, 512, 0, NULL); - status = mbedtls_compat_pk_parse_key( + status = mbedtls_pk_parse_key( ctx->priv_key, (const unsigned char *)priv_key_file, strlen(priv_key_file) + 1, (unsigned char *)passbuf, strlen(passbuf), mbedtls_ctr_drbg_random, rand_ctx_get()); } } else { - status = mbedtls_compat_pk_parse_keyfile(ctx->priv_key, priv_key_file, NULL, - mbedtls_ctr_drbg_random, rand_ctx_get()); + status = mbedtls_pk_parse_keyfile(ctx->priv_key, priv_key_file, NULL, + mbedtls_ctr_drbg_random, rand_ctx_get()); if (MBEDTLS_ERR_PK_PASSWORD_REQUIRED == status) { char passbuf[512] = { 0 }; pem_password_callback(passbuf, 512, 0, NULL); - status = mbedtls_compat_pk_parse_keyfile(ctx->priv_key, priv_key_file, passbuf, - mbedtls_ctr_drbg_random, rand_ctx_get()); + status = mbedtls_pk_parse_keyfile(ctx->priv_key, priv_key_file, passbuf, + mbedtls_ctr_drbg_random, rand_ctx_get()); } } if (!mbed_ok(status)) @@ -575,8 +570,8 @@ return 1; } - if (!mbed_ok(mbedtls_compat_pk_check_pair(&ctx->crt_chain->pk, ctx->priv_key, - mbedtls_ctr_drbg_random, rand_ctx_get()))) + if (!mbed_ok(mbedtls_pk_check_pair(&ctx->crt_chain->pk, ctx->priv_key, + mbedtls_ctr_drbg_random, rand_ctx_get()))) { msg(M_WARN, "Private key does not match the certificate"); return 1; @@ -610,9 +605,6 @@ */ static inline int external_pkcs1_sign(void *ctx_voidptr, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng, -#if MBEDTLS_VERSION_NUMBER < 0x03020100 - int mode, -#endif mbedtls_md_type_t md_alg, unsigned int hashlen, const unsigned char *hash, unsigned char *sig) { @@ -627,13 +619,6 @@ return MBEDTLS_ERR_RSA_BAD_INPUT_DATA; } -#if MBEDTLS_VERSION_NUMBER < 0x03020100 - if (MBEDTLS_RSA_PRIVATE != mode) - { - return MBEDTLS_ERR_RSA_BAD_INPUT_DATA; - } -#endif - /* * Support a wide range of hashes. TLSv1.1 and before only need SIG_RSA_RAW, * but TLSv1.2 needs the full suite of hashes. @@ -1000,7 +985,7 @@ if (0 != memcmp(old_sha256_hash, sha256_hash, sizeof(sha256_hash))) { - if (!mbed_ok(mbedtls_compat_ctr_drbg_update(cd_ctx, sha256_hash, 32))) + if (!mbed_ok(mbedtls_ctr_drbg_update(cd_ctx, sha256_hash, 32))) { msg(M_WARN, "WARNING: failed to personalise random, could not update CTR_DRBG"); } diff --git a/src/openvpn/ssl_mbedtls.h b/src/openvpn/ssl_mbedtls.h index 0f85d96..8380a116 100644 --- a/src/openvpn/ssl_mbedtls.h +++ b/src/openvpn/ssl_mbedtls.h @@ -39,8 +39,6 @@ #include <pkcs11-helper-1.0/pkcs11h-certificate.h> #endif -#include "mbedtls_compat.h" - typedef struct _buffer_entry buffer_entry; struct _buffer_entry @@ -130,7 +128,7 @@ #endif struct external_context external_key; /**< External key context */ int *allowed_ciphers; /**< List of allowed ciphers for this connection */ - mbedtls_compat_group_id *groups; /**< List of allowed groups for this connection */ + uint16_t *groups; /**< List of allowed groups for this connection */ mbedtls_x509_crt_profile cert_profile; /**< Allowed certificate types */ }; diff --git a/src/openvpn/ssl_verify_mbedtls.c b/src/openvpn/ssl_verify_mbedtls.c index 80ef837..250c806 100644 --- a/src/openvpn/ssl_verify_mbedtls.c +++ b/src/openvpn/ssl_verify_mbedtls.c @@ -35,7 +35,6 @@ #if defined(ENABLE_CRYPTO_MBEDTLS) #include "crypto_mbedtls.h" -#include "mbedtls_compat.h" #include "ssl_verify.h" #include <mbedtls/asn1.h> #include <mbedtls/error.h> -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1412?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Ia4afabcb6006dc9304a4c09f824d9c7c2d4d64ad Gerrit-Change-Number: 1412 Gerrit-PatchSet: 2 Gerrit-Owner: MaxF <ma...@ma...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: MaxF <ma...@ma...> 

Attention is currently required from: MaxF, plaisthos. flichtenheld has posted comments on this change by MaxF. ( http://gerrit.openvpn.net/c/openvpn/+/1412?usp=email ) Change subject: Drop Mbed TLS 2.X compatibility ...................................................................... Patch Set 1: Code-Review-2 (3 comments) Patchset: PS1: configure.ac needs more work. Presumably there should be a CMakeLists.txt patch as well? File configure.ac: http://gerrit.openvpn.net/c/openvpn/+/1412/comment/eeaca2b9_aa9059f4?usp=email : PS1, Line 998:	[mbedtls >= 2.0.0 mbedx509 >= 2.0.0 mbedcrypto >= 2.0.0], Needs to be updated as well http://gerrit.openvpn.net/c/openvpn/+/1412/comment/247cb35b_86dd2af0?usp=email : PS1, Line 1036:	if test "x$ac_cv_func_mbedtls_ssl_conf_export_keys_ext_cb" != xyes; then Now that we have the stricter version check, can we get rid of these additional tests? -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1412?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Ia4afabcb6006dc9304a4c09f824d9c7c2d4d64ad Gerrit-Change-Number: 1412 Gerrit-PatchSet: 1 Gerrit-Owner: MaxF <ma...@ma...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: MaxF <ma...@ma...> Gerrit-Comment-Date: Fri, 28 Nov 2025 11:03:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes 

From: Ralf Lici <ra...@ma...> Currently, reading and processing of incoming DCO messages are decoupled: notifications are read, parsed, and the relevant information is stored in fields of dco_context_t for later processing (with the only exception being stats). This approach is problematic on Linux, since libnl does not allow reading a single netlink message at a time, which can result in loss of information when multiple notifications are available. This change adopts a read -> parse -> process paradigm. On Linux, processing is now invoked directly from within the parsing callback, which libnl calls for each received netlink packet. The other interfaces are adapted accordingly to unify the processing model across all platforms. On Linux, however, a DEL_PEER notification from the kernel triggers a GET_PEER request from userspace, which clutters the netlink communication logic and can lead to errors or even process exit when multiple simultaneous DEL_PEER notifications are received. To avoid this, introduce a lock that prevents requesting stats while we are still busy parsing other messages. Reported-by: Stefan Baranoff <ste...@tr...> Change-Id: Iefc251cb4483c0b9fb9d6a5207db4445cd884d52 Signed-off-by: Ralf Lici <ra...@ma...> Acked-by: Gert Doering <ge...@gr...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1403 --- 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/+/1403 This mail reflects revision 12 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering <ge...@gr...> diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h index e5e8709..cd6e32a 100644 --- a/src/openvpn/dco.h +++ b/src/openvpn/dco.h @@ -127,12 +127,13 @@ void close_tun_dco(struct tuntap *tt, openvpn_net_ctx_t *ctx); /** - * Read data from the DCO communication channel (i.e. a control packet) + * Read and process data from the DCO communication channel + * (i.e. a control packet) * * @param dco the DCO context * @return 0 on success or a negative error code otherwise */ -int dco_do_read(dco_context_t *dco); +int dco_read_and_process(dco_context_t *dco); /** * Install a DCO in the main event loop @@ -305,7 +306,7 @@ } static inline int -dco_do_read(dco_context_t *dco) +dco_read_and_process(dco_context_t *dco) { ASSERT(false); return 0; diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c index f2a89ac..d1ad092 100644 --- a/src/openvpn/dco_freebsd.c +++ b/src/openvpn/dco_freebsd.c @@ -578,7 +578,7 @@ } int -dco_do_read(dco_context_t *dco) +dco_read_and_process(dco_context_t *dco) { struct ifdrv drv; uint8_t buf[4096]; @@ -684,11 +684,21 @@ default: msg(M_WARN, "%s: unknown kernel notification %d", __func__, type); + dco->dco_message_type = 0; break; } nvlist_destroy(nvl); + if (dco->c->mode == CM_TOP) + { + multi_process_incoming_dco(dco); + } + else + { + process_incoming_dco(dco); + } + return 0; } diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c index 0ae30b1..a838311 100644 --- a/src/openvpn/dco_linux.c +++ b/src/openvpn/dco_linux.c @@ -49,6 +49,15 @@ #include <netlink/genl/family.h> #include <netlink/genl/ctrl.h> +/* When parsing multiple DEL_PEER notifications, openvpn tries to request stats + * for each DEL_PEER message (see setenv_stats). This triggers a GET_PEER + * request-reply while we are still parsing the rest of the initial + * notifications, which can lead to NLE_BUSY or even NLE_NOMEM. + * + * This basic lock ensures we don't bite our own tail by issuing a dco_get_peer + * while still busy receiving and parsing other messages. + */ +static bool __is_locked = false; /* libnl < 3.5.0 does not set the NLA_F_NESTED on its own, therefore we * have to explicitly do it to prevent the kernel from failing upon @@ -1094,29 +1103,34 @@ * message, that stores the type-specific attributes. * * the "dco" object is then filled accordingly with the information - * retrieved from the message, so that the rest of the OpenVPN code can - * react as need be. + * retrieved from the message, so that *process_incoming_dco can react + * as need be. */ + int ret; switch (gnlh->cmd) { case OVPN_CMD_PEER_GET: { + /* return directly, there are no messages to pass to *process_incoming_dco() */ return ovpn_handle_peer(dco, attrs); } case OVPN_CMD_PEER_DEL_NTF: { - return ovpn_handle_peer_del_ntf(dco, attrs); + ret = ovpn_handle_peer_del_ntf(dco, attrs); + break; } case OVPN_CMD_PEER_FLOAT_NTF: { - return ovpn_handle_peer_float_ntf(dco, attrs); + ret = ovpn_handle_peer_float_ntf(dco, attrs); + break; } case OVPN_CMD_KEY_SWAP_NTF: { - return ovpn_handle_key_swap_ntf(dco, attrs); + ret = ovpn_handle_key_swap_ntf(dco, attrs); + break; } default: @@ -1125,15 +1139,33 @@ return NL_STOP; } + if (ret != NL_OK) + { + return ret; + } + + if (dco->c->mode == CM_TOP) + { + multi_process_incoming_dco(dco); + } + else + { + process_incoming_dco(dco); + } + return NL_OK; } int -dco_do_read(dco_context_t *dco) +dco_read_and_process(dco_context_t *dco) { msg(D_DCO_DEBUG, __func__); - return ovpn_nl_recvmsgs(dco, __func__); + __is_locked = true; + int ret = ovpn_nl_recvmsgs(dco, __func__); + __is_locked = false; + + return ret; } static int @@ -1141,6 +1173,12 @@ { ASSERT(dco); + if (__is_locked) + { + msg(D_DCO_DEBUG, "%s: cannot request peer stats while parsing other messages", __func__); + return 0; + } + /* peer_id == -1 means "dump all peers", but this is allowed in MP mode only. * If it happens in P2P mode it means that the DCO peer was deleted and we * can simply bail out diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c index ca5eedf..94f043f 100644 --- a/src/openvpn/dco_win.c +++ b/src/openvpn/dco_win.c @@ -690,7 +690,7 @@ } int -dco_do_read(dco_context_t *dco) +dco_read_and_process(dco_context_t *dco) { if (dco->ifmode != DCO_MODE_MP) { @@ -727,6 +727,15 @@ break; } + if (dco->c->mode == CM_TOP) + { + multi_process_incoming_dco(dco); + } + else + { + process_incoming_dco(dco); + } + return 0; } diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index ccb8404..1a68af4 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -1243,19 +1243,11 @@ } } -static void -process_incoming_dco(struct context *c) +void +process_incoming_dco(dco_context_t *dco) { #if defined(ENABLE_DCO) && (defined(TARGET_LINUX) || defined(TARGET_FREEBSD)) - dco_context_t *dco = &c->c1.tuntap->dco; - - dco_do_read(dco); - - /* no message for us to handle - platform specific code has logged details */ - if (dco->dco_message_type == 0) - { - return; - } + struct context *c = dco->c; /* FreeBSD currently sends us removal notifcation with the old peer-id in * p2p mode with the ping timeout reason, so ignore that one to not shoot @@ -2369,7 +2361,7 @@ { if (!IS_SIG(c)) { - process_incoming_dco(c); + dco_read_and_process(&c->c1.tuntap->dco); } } } diff --git a/src/openvpn/forward.h b/src/openvpn/forward.h index a575faf..9e9b02e 100644 --- a/src/openvpn/forward.h +++ b/src/openvpn/forward.h @@ -210,6 +210,13 @@ const struct sockaddr *float_sa); /** + * Process an incoming DCO message (from kernel space). + * + * @param dco - Pointer to the structure representing the DCO context. + */ +void process_incoming_dco(dco_context_t *dco); + +/** * Write a packet to the external network interface. * @ingroup external_multiplexer * diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 2b944667..153695c 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -3263,14 +3263,12 @@ multi_signal_instance(m, mi, SIGTERM); } -bool -multi_process_incoming_dco(struct multi_context *m) +void +multi_process_incoming_dco(dco_context_t *dco) { - dco_context_t *dco = &m->top.c1.tuntap->dco; + ASSERT(dco->c->multi); - struct multi_instance *mi = NULL; - - int ret = dco_do_read(&m->top.c1.tuntap->dco); + struct multi_context *m = dco->c->multi; int peer_id = dco->dco_message_peer_id; @@ -3279,12 +3277,12 @@ */ if (peer_id < 0) { - return ret > 0; + return; } if ((peer_id < m->max_clients) && (m->instances[peer_id])) { - mi = m->instances[peer_id]; + struct multi_instance *mi = m->instances[peer_id]; set_prefix(mi); if (dco->dco_message_type == OVPN_CMD_DEL_PEER) { @@ -3325,11 +3323,6 @@ "type %d, del_peer_reason %d", peer_id, dco->dco_message_type, dco->dco_del_peer_reason); } - - dco->dco_message_type = 0; - dco->dco_message_peer_id = -1; - dco->dco_del_peer_reason = -1; - return ret > 0; } #endif /* if defined(ENABLE_DCO) */ @@ -4462,4 +4455,4 @@ return (!ipv6_net_contains_host(&ifconfig_local, o->ifconfig_ipv6_netbits, dest)); -} \ No newline at end of file +} diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h index a62b07a..a44f9f2 100644 --- a/src/openvpn/multi.h +++ b/src/openvpn/multi.h @@ -305,13 +305,9 @@ /** * Process an incoming DCO message (from kernel space). * - * @param m - The single \c multi_context structure. - * - * @return - * - True, if the message was received correctly. - * - False, if there was an error while reading the message. + * @param dco - Pointer to the structure representing the DCO context. */ -bool multi_process_incoming_dco(struct multi_context *m); +void multi_process_incoming_dco(dco_context_t *dco); /**************************************************************************/ /** diff --git a/src/openvpn/multi_io.c b/src/openvpn/multi_io.c index fe72456..997951e 100644 --- a/src/openvpn/multi_io.c +++ b/src/openvpn/multi_io.c @@ -505,7 +505,7 @@ /* incoming data on DCO? */ else if (e->arg == MULTI_IO_DCO) { - multi_process_incoming_dco(m); + dco_read_and_process(&m->top.c1.tuntap->dco); } #endif /* signal received? */ 

Attention is currently required from: plaisthos, ralf_lici. cron2 has posted comments on this change by ralf_lici. ( http://gerrit.openvpn.net/c/openvpn/+/1403?usp=email ) Change subject: dco: process messages immediately after read ...................................................................... Patch Set 12: Code-Review+2 -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1403?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Iefc251cb4483c0b9fb9d6a5207db4445cd884d52 Gerrit-Change-Number: 1403 Gerrit-PatchSet: 12 Gerrit-Owner: ralf_lici <ra...@ma...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: ralf_lici <ra...@ma...> Gerrit-Comment-Date: Fri, 28 Nov 2025 10:57:53 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes