Re: [Openvpn-devel] [PATCH] Require at least 20MB of mlock()-able memory if --mlock is used.
Am 07.03.21 um 19:44 schrieb Gert Doering: > Hi, > > On Sun, Mar 07, 2021 at 01:36:03PM -0500, Selva Nair wrote: >>> "I'm not sure", TBH. rlimit handling in unix is a bit of an unknown >>> territory for me. >>> >>> What I understand is that root can *increment* the rlimit at will, but >>> I'd assume that the rlimit value "in existance right now" (specifically, >>> the soft limit) applies to root processes as well. Sort of a voluntary >>> protection against processes running away. >> >> On modern linux kernels (since some 2.6.x..) RLIMIT_MEMLOCK applies only to >> unprivileged processes -- privileged processes allowed to lock "unlimited" >> amount of memory as documented in man mlock. We updated the man page based >> on that sometime ago. > > Indeed, "man mlock" says something about "privileged processes" on Linux > (it doesn't say that on FreeBSD). > >> We could also consider using setrlimit to increase the limit before >> dropping privileges. > > That's another possible angle... just up soft+hard to "something" > (how much would that be? :-) ) and log the fact. > > David, Arne, any opinion on this? Where do we want to go? Looking at this feature from today's perspective, it feels like one of OpenVPN's boutique features. Was probably useful at some point but doesn't really make much sense today anymore. Esepcially with what is written in the manpage. Today you rather would use full disk encryption or disable swapping rather than to rely on OpenVPN's --mlock. That being said I am against your patch, I am just wondering if that is a feature we need to keep at all. Arne ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Prefer TLS libraries TLS PRF function, fix OpenVPN in FIPS mode
After some discussion on IRC today, it turns out that I was holding my test rig wrong. As in: breakage occurs in the combination of "mbed TLS 2.25.0, TLS, TCP and --dev tap", but it does not actually matter whether this patch is applied or not - 2.5.1 breaks as well. Arne's test found the commit in mbedTLS between 2.24 and 2.25 that introduces the breakage (360e2c41d8211e43), but this does not really explain anything - but it's fairly clear "not something broken in our code", or at least "not recently". As discussed on IRC, I have fixed what whitespace uncrustify complained about ("if(", function return types on the preceding line, #endif with comment) This all said, I now do actually understand what the patch does (and it looks good) and I think I have all versions of old/new mbedtls and old/new openssl covered, client and server. All pass, except for mbedtls 2.25.0 + tcp + tap. Your patch has been applied to the master branch. commit 06f6cf3ff850f2930bf4a864ae9898407e94ffb9 Author: Arne Schwabe Date: Fri Mar 5 15:13:52 2021 +0100 Prefer TLS libraries TLS PRF function, fix OpenVPN in FIPS mode Signed-off-by: Arne Schwabe Acked-by: Antonio Quartulli Message-Id: <20210305141352.21847-1-a...@rfc2549.org> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21612.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH applied] Re: Prefer TLS libraries TLS PRF function, fix OpenVPN in FIPS mode
Hi, On Mon, Mar 08, 2021 at 11:53:09AM +0100, Gert Doering wrote: > After some discussion on IRC today, it turns out that I was holding > my test rig wrong. As in: breakage occurs in the combination of > "mbed TLS 2.25.0, TLS, TCP and --dev tap", but it does not actually > matter whether this patch is applied or not - 2.5.1 breaks as well. Arne found the trigger. It is neither related to TCP nor to "--dev tap", but it needs a server side with "OpenSSL 1.1.1" *and* a client side with "mbedTLS 2.25" to trigger this - both ends need to negotiate curve25591, and then mbedTLS will crash in the debug print function. https://github.com/ARMmbed/mbedtls/issues/4208 My current test rigs do not test this combination, except for this particular test case (tcp+tap towards --inetd server), so we were chasing red herrings for a while... I need to think long and hard now how to add meaningful tests with the new test matrix openssl 1.0.2u <-> openssl 1.1.1 mbedTLS "oldish" <-> mbedTLS "very new to the test sets, without making the already-long server side test (40 minutes) go totally out of bounds... gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany g...@greenie.muc.de signature.asc Description: PGP signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] Fix EVP_PKEY_CTX_... compilation with LibreSSL
Commit 06f6cf3ff850f29 introduced use of newer OpenSSL functions for the TLS 1.0-1.1 PRF, to make OpenVPN work with FIPS-enabled OpenSSL. LibreSSL masquerades as "very new OpenSSL" but does not have these functions (or at least not on the OpenBSD system tested), so compilationg breaks. Add a "but not if LibreSSL" check to the OpenSSL version check, as we do in other places. Signed-off-by: Gert Doering --- src/openvpn/crypto_openssl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c index 7aaa6624..49698e4b 100644 --- a/src/openvpn/crypto_openssl.c +++ b/src/openvpn/crypto_openssl.c @@ -51,7 +51,7 @@ #include #include -#if OPENSSL_VERSION_NUMBER >= 0x1010L +#if (OPENSSL_VERSION_NUMBER >= 0x1010L) && !defined(LIBRESSL_VERSION_NUMBER) #include #endif @@ -1128,7 +1128,7 @@ engine_load_key(const char *file, SSL_CTX *ctx) #endif /* if HAVE_OPENSSL_ENGINE */ } -#if OPENSSL_VERSION_NUMBER >= 0x1010L +#if (OPENSSL_VERSION_NUMBER >= 0x1010L) && !defined(LIBRESSL_VERSION_NUMBER) bool ssl_tls1_PRF(const uint8_t *seed, int seed_len, const uint8_t *secret, int secret_len, uint8_t *output, int output_len) -- 2.21.0 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Fix EVP_PKEY_CTX_... compilation with LibreSSL
Am 08.03.21 um 12:44 schrieb Gert Doering: > Commit 06f6cf3ff850f29 introduced use of newer OpenSSL functions > for the TLS 1.0-1.1 PRF, to make OpenVPN work with FIPS-enabled OpenSSL. > > LibreSSL masquerades as "very new OpenSSL" but does not have these > functions (or at least not on the OpenBSD system tested), so compilationg > breaks. > > Add a "but not if LibreSSL" check to the OpenSSL version check, as we > do in other places. > I have to say that I am not really surprised by this but at least currently while we still support OpenSSL 1.0.2 it is not very intrusive. Acked-By: Arne Schwabe ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Fix EVP_PKEY_CTX_... compilation with LibreSSL
Patch has been applied to the master branch. commit 4cf01c8e4381403998341aa32f79f4bf24c7ccb1 Author: Gert Doering Date: Mon Mar 8 12:44:05 2021 +0100 Fix EVP_PKEY_CTX_... compilation with LibreSSL Signed-off-by: Gert Doering Acked-by: Arne Schwabe Message-Id: <20210308114405.19066-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21628.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Require at least 20MB of mlock()-able memory if --mlock is used.
On 08/03/2021 08:06, Arne Schwabe wrote: Looking at this feature from today's perspective, it feels like one of OpenVPN's boutique features. Was probably useful at some point but doesn't really make much sense today anymore. Esepcially with what is written in the manpage. Today you rather would use full disk encryption or disable swapping rather than to rely on OpenVPN's --mlock. That being said I am against your patch, I am just wondering if that is a feature we need to keep at all. Having all openvpn data remain permanently in memory also offers a (small) performance boost. Your alternative offers would impact performance and be system wide. Therefore, I for one disagree. -- ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] Avoid a crash in mbed TLS 2.25 with --verb < 8
mbed TLS 2.25 has a nasty bug that the print function for Montgomery style EC curves (Curve25519 and Curve448) does segfault. See also the issue reported here: https://github.com/ARMmbed/mbedtls/issues/4208 We request always debug level 3 from mbed TLS but filter out any debug output of level 3 unless verb 8 or higher is set. This commeit sets the debug level to 2 to avoid this problem by makeing mbed TLS not generatin the problematic debug output. For the affected version to still use --verb 8 with mbed TLS 2.25 is to restrict the EC groups to ones that do not crash the print function like with '--tls-groups secp521r1:secp384r1:secp256r1'. This patch has no patch on user-visible behaviour on unaffected mbed TLS versions. Signed-off-by: Arne Schwabe --- src/openvpn/options.c | 6 ++ src/openvpn/ssl_common.h | 1 + src/openvpn/ssl_mbedtls.c | 11 ++- 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 0eb049d8..6d908e15 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -5883,6 +5883,12 @@ add_option(struct options *options, { VERIFY_PERMISSION(OPT_P_MESSAGES); options->verbosity = positive_atoi(p[1]); +if (options->verbosity > 7) +{ +/* We pass this flag to the SSL library to avoid a bug + * in mbed TLS 2.5.0 with high log level */ +options->ssl_flags |= SSLF_TLS_DEBUG_ENABLED; +} #if !defined(ENABLE_DEBUG) && !defined(ENABLE_SMALL) /* Warn when a debug verbosity is supplied when built without debug support */ if (options->verbosity >= 7) diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h index bbb8135d..f821c654 100644 --- a/src/openvpn/ssl_common.h +++ b/src/openvpn/ssl_common.h @@ -350,6 +350,7 @@ struct tls_options #define SSLF_TLS_VERSION_MIN_MASK 0xF /* (uses bit positions 6 to 9) */ #define SSLF_TLS_VERSION_MAX_SHIFT10 #define SSLF_TLS_VERSION_MAX_MASK 0xF /* (uses bit positions 10 to 13) */ +#define SSLF_TLS_DEBUG_ENABLED(1<<14) unsigned int ssl_flags; #ifdef ENABLE_MANAGEMENT diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index b30b6b9d..a7b6c2c6 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -1058,7 +1058,16 @@ key_state_ssl_init(struct key_state_ssl *ks_ssl, mbedtls_ssl_config_defaults(ks_ssl->ssl_config, ssl_ctx->endpoint, MBEDTLS_SSL_TRANSPORT_STREAM, MBEDTLS_SSL_PRESET_DEFAULT); #ifdef MBEDTLS_DEBUG_C -mbedtls_debug_set_threshold(3); +/* This works around a crash in mbed TLS 2.25 if Curve25591 is selected + * for DH (https://github.com/ARMmbed/mbedtls/issues/4208)*/ +if (session->opt->ssl_flags & SSLF_TLS_DEBUG_ENABLED) +{ +mbedtls_debug_set_threshold(3); +} +else +{ +mbedtls_debug_set_threshold(2); +} #endif mbedtls_ssl_conf_dbg(ks_ssl->ssl_config, my_debug, NULL); mbedtls_ssl_conf_rng(ks_ssl->ssl_config, mbedtls_ctr_drbg_random, -- 2.30.1 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Require at least 20MB of mlock()-able memory if --mlock is used.
On 07/03/2021 22:28, Gert Doering wrote: Hi, On Sun, Mar 07, 2021 at 02:20:32PM -0500, Selva Nair wrote: That's another possible angle... just up soft+hard to "something" (how much would that be? :-) ) and log the fact. Rereading my comment on Trac #1059 I recall testing this and concluding 100MB enough for clients. On modern machines that's a low amount of memory --- not allowing swapout of 100MB should be acceptable. For servers, I think there is no reliable limit that we could come up with. This is interesting. My machines, including servers, have a way lower memory usage - but I'm not using EC. Here's a linux server with 3 peers and a client (arm32): USER PID %CPU %MEMVSZ RSS TTY STAT START TIME COMMAND root 14569 0.0 0.5 5704 2980 ?Ss2020 12:31 openvpn root 21020 0.1 0.5 5568 3040 ?Ss2020 454:45 openvpn Here's a FreeBSD server that serves 5 clients right now, but has peaks up to 150 concurrent clients (amd64) USER PID %CPU %MEMVSZRSS TT STAT STARTED TIME COMMAND root 50230.3 0.1 30924 23172 - Ss 20Dec201945:07.12 /usr/local/sbin/openvpn --cd /usr/local/etc/openvpn --daemon ... and another two Linux server instances... USER PID %CPU %MEMVSZ RSS TTY STAT START TIME COMMAND root 1138 2.1 1.3 26732 10628 ?Ss2019 21678:18 /usr/sbin/openvpn root 2897 0.8 0.9 29548 7080 ?Ss2019 8596:15 /usr/sbin/openvpn the last 3 see a high number of clients during work time, and quite some churn (especially due to iOS/android connecting and disconnecting frequently), but long-term memory usage is not high. So there must be some massive transient usage due to EC at reconnection time... FWIW A couple of my CentOS 8.3 servers with 2 client connections running 24/7. * Server 1 USER PID %CPU %MEMVSZ RSS TTY STAT START TIME COMMAND openvpn 3437979 0.0 0.1 76360 2924 ?Ss Feb25 2:19 /usr/sbin/openvpn * Server 2 USER PID %CPU %MEMVSZ RSS TTY STAT START TIME COMMAND openvpn 2805670 0.0 0.8 76624 6896 ?Ss Feb25 7:08 /usr/sbin/openvpn The start date is from when openvpn-2.5.1 got released and updated. OpenSSL 1.1.1g-12 is used on both hosts. -- kind regards, David Sommerseth OpenVPN Inc OpenPGP_signature Description: OpenPGP digital signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel