diff mbox series

[Openvpn-devel,v2] mbedtls: Remove support for old TLS versions

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

Commit Message

Gert Doering June 18, 2024, 12:02 p.m. UTC 
From: Max Fillinger <maximilian.fillinger@foxcrypto.com>

Recent versions of mbedtls have dropped support for TLS 1.0 and 1.1.
Rather than checking which versions are supported, drop support for
everything before 1.2.

Change-Id: Ia3883a26ac26df6bbb5353fb074a2e0f814737be
Signed-off-by: Max Fillinger <maximilian.fillinger@foxcrypto.com>
Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/682
This mail reflects revision 2 of this Change.

Acked-by according to Gerrit (reflected above):
Arne Schwabe <arne-openvpn@rfc2549.org>

Comments

Gert Doering June 18, 2024, 4:30 p.m. UTC | #1 
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
Gert Doering June 19, 2024, 9:44 a.m. UTC | #2 
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
>
Maximilian Fillinger June 19, 2024, 1:38 p.m. UTC | #3 
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
Gert Doering June 19, 2024, 1:45 p.m. UTC | #4 
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
Arne Schwabe June 20, 2024, 12:43 a.m. UTC | #5 
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 mbox series

Patch

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;