Message ID | 20240618120219.5053-1-gert@greenie.muc.de |
---|---|
State | Accepted |
Headers | show |
Series | [Openvpn-devel,v2] mbedtls: Remove support for old TLS versions | expand |
Mildly tested via GHA builds. Not sure we want this in release/2.6 - I tend to "not", because it might break someone's (non-recommended) setup... Your patch has been applied to the master branch. commit 013c119af96bc57c41e04e4a8f64b5d80e2e9ba6 Author: Max Fillinger Date: Tue Jun 18 14:02:19 2024 +0200 mbedtls: Remove support for old TLS versions Signed-off-by: Max Fillinger <maximilian.fillinger@foxcrypto.com> Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org> Message-Id: <20240618120219.5053-1-gert@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28773.html Signed-off-by: Gert Doering <gert@greenie.muc.de> -- kind regards, Gert Doering
Hi, this breaks *all* client connects on my server testbed. No matter if 2.2 or 2.5 client, when building with mbedtls (2.28.7), the resulting binary refuses ALL incoming connection with Jun 19 10:21:44 gentoo tap-udp-p2mp[1723]: 2001:608:0:814::f000:16 tls_version_to_ssl_version: invalid or unsupported TLS version 1 Jun 19 10:21:44 gentoo tap-tcp-p2p[1770]: tls_version_to_ssl_version: invalid or unsupported TLS version 1 Jun 19 10:21:59 gentoo tun-tcp-p2mp[1708]: tls_version_to_ssl_version: invalid or unsupported TLS version 1 Jun 19 10:22:32 gentoo tun-udp-p2mp[1713]: 194.97.140.21:49229 tls_version_to_ssl_version: invalid or unsupported TLS version 2 Jun 19 10:23:05 gentoo tun-udp-p2mp-topology-subnet[1718]: 194.97.140.21:45789 tls_version_to_ssl_version: invalid or unsupported TLS version 1 Jun 19 10:24:11 gentoo tun-udp-p2mp-fragment[1746]: 194.97.140.21:14517 tls_version_to_ssl_version: invalid or unsupported TLS version 1 Jun 19 10:44:49 gentoo tun-udp-p2mp-112-mask[1741]: 194.97.140.21:42810 tls_version_to_ssl_version: invalid or unsupported TLS version 1 so my guess would be that on mbedTLS builds that *do* support 1.1/1.2, incoming client connects with 1.1/1.2 cause "something to get upset" in the TLS version printer. Sorry for not testing this more thoroughly before merging. gert On Tue, Jun 18, 2024 at 06:30:05PM +0200, Gert Doering wrote: > Mildly tested via GHA builds. > > Not sure we want this in release/2.6 - I tend to "not", because it might > break someone's (non-recommended) setup... > > Your patch has been applied to the master branch. > > commit 013c119af96bc57c41e04e4a8f64b5d80e2e9ba6 > Author: Max Fillinger > Date: Tue Jun 18 14:02:19 2024 +0200 > > mbedtls: Remove support for old TLS versions > > Signed-off-by: Max Fillinger <maximilian.fillinger@foxcrypto.com> > Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org> > Message-Id: <20240618120219.5053-1-gert@greenie.muc.de> > URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28773.html > Signed-off-by: Gert Doering <gert@greenie.muc.de> > > > -- > kind regards, > > Gert Doering > > > > _______________________________________________ > Openvpn-devel mailing list > Openvpn-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/openvpn-devel >
Hi, I *think* I reproduced the problem you're encountering. If I put setenv opt tls-version-min 1.0 in the server config, then *every* connection attempt will trigger a fatal error in the server. Doesn't matter what TLS versions the client supports. If I put that option into the client config, the client will exit with an error during startup. It's not clear to me what the expected behavior is when tls-version-min is an unsupported version, but if it's an error, it should happen during start-up. > -----Original Message----- > From: Gert Doering <gert@greenie.muc.de> > Sent: woensdag 19 juni 2024 11:44 > To: Maximilian Fillinger <maximilian.fillinger@foxcrypto.com> > Cc: openvpn-devel@lists.sourceforge.net > Subject: Re: [Openvpn-devel] [PATCH applied] Re: mbedtls: Remove support > for old TLS versions > > Hi, > > this breaks *all* client connects on my server testbed. No matter if > 2.2 or 2.5 client, when building with mbedtls (2.28.7), the resulting > binary refuses ALL incoming connection with > > Jun 19 10:21:44 gentoo tap-udp-p2mp[1723]: 2001:608:0:814::f000:16 > tls_version_to_ssl_version: invalid or unsupported TLS version 1 > Jun 19 10:21:44 gentoo tap-tcp-p2p[1770]: tls_version_to_ssl_version: > invalid or unsupported TLS version 1 > Jun 19 10:21:59 gentoo tun-tcp-p2mp[1708]: tls_version_to_ssl_version: > invalid or unsupported TLS version 1 > Jun 19 10:22:32 gentoo tun-udp-p2mp[1713]: 194.97.140.21:49229 > tls_version_to_ssl_version: invalid or unsupported TLS version 2 > Jun 19 10:23:05 gentoo tun-udp-p2mp-topology-subnet[1718]: > 194.97.140.21:45789 tls_version_to_ssl_version: invalid or unsupported > TLS version 1 > Jun 19 10:24:11 gentoo tun-udp-p2mp-fragment[1746]: 194.97.140.21:14517 > tls_version_to_ssl_version: invalid or unsupported TLS version 1 > Jun 19 10:44:49 gentoo tun-udp-p2mp-112-mask[1741]: 194.97.140.21:42810 > tls_version_to_ssl_version: invalid or unsupported TLS version 1 > > so my guess would be that on mbedTLS builds that *do* support 1.1/1.2, > incoming client connects with 1.1/1.2 cause "something to get upset" > in the TLS version printer. > > Sorry for not testing this more thoroughly before merging. > > gert > > > > On Tue, Jun 18, 2024 at 06:30:05PM +0200, Gert Doering wrote: > > Mildly tested via GHA builds. > > > > Not sure we want this in release/2.6 - I tend to "not", because it > might > > break someone's (non-recommended) setup... > > > > Your patch has been applied to the master branch. > > > > commit 013c119af96bc57c41e04e4a8f64b5d80e2e9ba6 > > Author: Max Fillinger > > Date: Tue Jun 18 14:02:19 2024 +0200 > > > > mbedtls: Remove support for old TLS versions > > > > Signed-off-by: Max Fillinger <maximilian.fillinger@foxcrypto.com> > > Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org> > > Message-Id: <20240618120219.5053-1-gert@greenie.muc.de> > > URL: https://www.mail-archive.com/openvpn- > devel@lists.sourceforge.net/msg28773.html > > Signed-off-by: Gert Doering <gert@greenie.muc.de> > > > > > > -- > > kind regards, > > > > Gert Doering > > > > > > > > _______________________________________________ > > Openvpn-devel mailing list > > Openvpn-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/openvpn-devel > > > > -- > "If was one thing all people took for granted, was conviction that if > you > feed honest figures into a computer, honest figures come out. Never > doubted > it myself till I met a computer with a sense of humor." > Robert A. Heinlein, The Moon is a Harsh > Mistress > > Gert Doering - Munich, Germany > gert@greenie.muc.de
Hi, On Wed, Jun 19, 2024 at 01:38:46PM +0000, Maximilian Fillinger wrote: > I *think* I reproduced the problem you're encountering. > > If I put > > setenv opt tls-version-min 1.0 > > in the server config, then *every* connection attempt will trigger a fatal error in the server. Doesn't matter what TLS versions the client supports. > > If I put that option into the client config, the client will exit with an error during startup. > > It's not clear to me what the expected behavior is when tls-version-min is an unsupported version, but if it's an error, it should happen during start-up. I would argue for - we log "minimum supported version is 1.2" and go on or - we log "minimum supported version is 1.2" and exit both is acceptable. It will break people's setups in different ways, though... the first will pretend all is well, and older clients can no longer connect, while the second one will break everything, so making it more obvious. gert
it should happen during start-up. > I would argue for > > - we log "minimum supported version is 1.2" and go on > > or > > - we log "minimum supported version is 1.2" and exit > > both is acceptable. It will break people's setups in different ways, > though... the first will pretend all is well, and older clients can no > longer connect, while the second one will break everything, so making it > more obvious. > Strong vote for going on. On many OpenSSL configuration and also distribution trying to use TLS 1.0 will already silently give you only TLS 1.2+. So no reason to behave different with mbed TLS. Arne
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index a68588e..ec9ec13 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -1040,12 +1040,8 @@ { #if defined(MBEDTLS_SSL_PROTO_TLS1_2) return TLS_VER_1_2; -#elif defined(MBEDTLS_SSL_PROTO_TLS1_1) - return TLS_VER_1_1; -#elif defined(MBEDTLS_SSL_PROTO_TLS1) - return TLS_VER_1_0; #else /* defined(MBEDTLS_SSL_PROTO_TLS1_2) */ - #error "mbedtls is compiled without support for TLS 1.0, 1.1 and 1.2." + #error "mbedtls is compiled without support for TLS 1.2." #endif /* defined(MBEDTLS_SSL_PROTO_TLS1_2) */ } @@ -1067,20 +1063,6 @@ switch (tls_ver) { -#if defined(MBEDTLS_SSL_PROTO_TLS1) - case TLS_VER_1_0: - *major = MBEDTLS_SSL_MAJOR_VERSION_3; - *minor = MBEDTLS_SSL_MINOR_VERSION_1; - break; -#endif - -#if defined(MBEDTLS_SSL_PROTO_TLS1_1) - case TLS_VER_1_1: - *major = MBEDTLS_SSL_MAJOR_VERSION_3; - *minor = MBEDTLS_SSL_MINOR_VERSION_2; - break; -#endif - #if defined(MBEDTLS_SSL_PROTO_TLS1_2) case TLS_VER_1_2: *major = MBEDTLS_SSL_MAJOR_VERSION_3;