Re: [Openvpn-devel] [PATCH] Require at least 20MB of mlock()-able memory if --mlock is used.

2021-03-08 Thread Arne Schwabe
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

2021-03-08 Thread Gert Doering
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

2021-03-08 Thread Gert Doering
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

2021-03-08 Thread 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.

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

2021-03-08 Thread Arne Schwabe
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

2021-03-08 Thread Gert Doering
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.

2021-03-08 Thread tincanteksup




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

2021-03-08 Thread Arne Schwabe
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.

2021-03-08 Thread David Sommerseth

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