diff mbox series

[Openvpn-devel] Fix regression, reinstate LibreSSL support.

Message ID 20190818111811.8853-2-matthias.andree@gmx.de
State Accepted
Headers show
Series [Openvpn-devel] Fix regression, reinstate LibreSSL support. | expand

Commit Message

Matthias Andree Aug. 18, 2019, 1:18 a.m. UTC 
OpenVPN 2.4.6 could be compiled with LibreSSL, 2.4.7 cannot.  This was broken
since 9de7fe0a "Add support for tls-ciphersuites for TLS 1.3".

This patch avoids using TLS 1.3 directly, be it that OpenSSL was compiled
without TLS 1.3 support, or LibreSSL was used.

This patch was based on an OpenBSD patch by
Jeremie Courreges-Anglas <jca@openbsd.org>, see
https://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/net/openvpn/patches/patch-src_openvpn_ssl_openssl_c
but was revised to be more obvious and check actual feature macros,
do not rely on current LibreSSL implementation details alone.

Franco Fichtner reports that OPNsense has been a long-time user
of LibreSSL without reported breakage, see also:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=238382#c10

Signed-off-by: Matthias Andree <matthias.andree@gmx.de>
---
 src/openvpn/ssl_openssl.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

--
2.21.0

Comments

Arne Schwabe Aug. 18, 2019, 2:11 a.m. UTC | #1 
Am 18.08.19 um 13:18 schrieb Matthias Andree:
> OpenVPN 2.4.6 could be compiled with LibreSSL, 2.4.7 cannot.  This was broken
> since 9de7fe0a "Add support for tls-ciphersuites for TLS 1.3".
> 
> This patch avoids using TLS 1.3 directly, be it that OpenSSL was compiled
> without TLS 1.3 support, or LibreSSL was used.

Test looks good. I don't like the additonal ifdefs but yeah seem no way
around them.

The ifdefs don't break OpenSSL. But I have not tested LibreSSL.


Acked-By: Arne Schwabe <arne@rfc2549.org>
Matthias Andree Sept. 14, 2019, 11:38 p.m. UTC | #2 
Am 18.08.19 um 14:11 schrieb Arne Schwabe:
> Am 18.08.19 um 13:18 schrieb Matthias Andree:
>> OpenVPN 2.4.6 could be compiled with LibreSSL, 2.4.7 cannot.  This was broken
>> since 9de7fe0a "Add support for tls-ciphersuites for TLS 1.3".
>>
>> This patch avoids using TLS 1.3 directly, be it that OpenSSL was compiled
>> without TLS 1.3 support, or LibreSSL was used.
> Test looks good. I don't like the additonal ifdefs but yeah seem no way
> around them.
>
> The ifdefs don't break OpenSSL. But I have not tested LibreSSL.
>
>
> Acked-By: Arne Schwabe <arne@rfc2549.org>

So, four weeks later, what is the commit status of this change?

I don't yet see it in Git and no relevant messages here.
Gert Doering Sept. 18, 2019, 2:01 a.m. UTC | #3 
Your patch has been applied to the release/2.4 branch.

Sorry for the delay.  Vacation, and too many distractions.

Lightly tested on an OpenSSL 1.1, a mbedTLS build and an LibreSSL 2.7.2
on OpenBSD 6.3 - with OpenSSL and mbedTLS, it builds and passes all
tests.  

With LibreSSL 2.7.2, it fails due to

./../../openvpn.git/src/openvpn/ssl_openssl.c:1873: undefined reference to `SSL_get1_supported_ciphers'

which looks like this:

#if (OPENSSL_VERSION_NUMBER < 0x1010000fL)
    STACK_OF(SSL_CIPHER) *sk = SSL_get_ciphers(ssl);
#else
    STACK_OF(SSL_CIPHER) *sk = SSL_get1_supported_ciphers(ssl);   
#endif

this is code which has been in release/2.4 for quite some time (part of
the TLS 1.3 support, commit e8467c864, "--show-tls" enhancements) - so 
if it doesn't break for you, I assume that the call was added to more
recent LibreSSL versions.


commit c8823f7b09c3a6a2101b54763ac2ef146d372328 (release/2.4)
Author: Matthias Andree
Date:   Sun Aug 18 13:18:11 2019 +0200

     Fix regression, reinstate LibreSSL support.

     Signed-off-by: Matthias Andree <matthias.andree@gmx.de>
     Acked-by: Arne Schwabe <arne@rfc2549.org>
     Message-Id: <20190818111811.8853-2-matthias.andree@gmx.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18790.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
Matthias Andree Sept. 21, 2019, 2:07 a.m. UTC | #4 
Am 18.09.19 um 14:01 schrieb Gert Doering:
> Your patch has been applied to the release/2.4 branch.
>
> Sorry for the delay.  Vacation, and too many distractions.
>
> Lightly tested on an OpenSSL 1.1, a mbedTLS build and an LibreSSL 2.7.2
> on OpenBSD 6.3 - with OpenSSL and mbedTLS, it builds and passes all
> tests.  
>
> With LibreSSL 2.7.2, it fails due to
>
> ./../../openvpn.git/src/openvpn/ssl_openssl.c:1873: undefined reference to `SSL_get1_supported_ciphers'
>
> which looks like this:
>
> #if (OPENSSL_VERSION_NUMBER < 0x1010000fL)
>     STACK_OF(SSL_CIPHER) *sk = SSL_get_ciphers(ssl);
> #else
>     STACK_OF(SSL_CIPHER) *sk = SSL_get1_supported_ciphers(ssl);   
> #endif
>
> this is code which has been in release/2.4 for quite some time (part of
> the TLS 1.3 support, commit e8467c864, "--show-tls" enhancements) - so 
> if it doesn't break for you, I assume that the call was added to more
> recent LibreSSL versions.

I was testing against LibreSSL 2.9.2, the oldest for FreeBSD, and this
particular call is listed in the OpenBSD 6.5 changelog here:
https://www.openbsd.org/plus65.html "Provided SSL_get_client_ciphers()
and SSL_get1_supported_ciphers() (part of the OpenSSL 1.1 API)." But I
haven't figured out when or where this was added to LibreSSL releases.

It really looks to me that there isn't a strategy for LibreSSL, but I'll
not backport things to old LibreSSL version, the answer should be
"upgrade or else leave it to your packager/distributor".
diff mbox series

Patch

diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index a78dae99..293bb192 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -206,7 +206,7 @@  info_callback(INFO_CALLBACK_SSL_CONST SSL *s, int where, int ret)
 int
 tls_version_max(void)
 {
-#if defined(TLS1_3_VERSION)
+#if defined(TLS1_3_VERSION) && !defined(OPENSSL_NO_TLS1_3)
     return TLS_VER_1_3;
 #elif defined(TLS1_2_VERSION) || defined(SSL_OP_NO_TLSv1_2)
     return TLS_VER_1_2;
@@ -233,7 +233,7 @@  openssl_tls_version(int ver)
     {
         return TLS1_2_VERSION;
     }
-#if defined(TLS1_3_VERSION)
+#if defined(TLS1_3_VERSION) && !defined(OPENSSL_NO_TLS1_3)
     else if (ver == TLS_VER_1_3)
     {
         return TLS1_3_VERSION;
@@ -459,8 +459,8 @@  tls_ctx_restrict_ciphers_tls13(struct tls_root_ctx *ctx, const char *ciphers)
         return;
     }

-#if (OPENSSL_VERSION_NUMBER < 0x1010100fL)
-        crypto_msg(M_WARN, "Not compiled with OpenSSL 1.1.1 or higher. "
+#if (OPENSSL_VERSION_NUMBER < 0x1010100fL) || !defined(TLS1_3_VERSION) || defined(OPENSSL_NO_TLS1_3)
+        crypto_msg(M_WARN, "Not compiled with OpenSSL 1.1.1 or higher, or without TLS 1.3 support. "
                        "Ignoring TLS 1.3 only tls-ciphersuites '%s' setting.",
                         ciphers);
 #else
@@ -1846,7 +1846,7 @@  show_available_tls_ciphers_list(const char *cipher_list,
         crypto_msg(M_FATAL, "Cannot create SSL_CTX object");
     }

-#if (OPENSSL_VERSION_NUMBER >= 0x1010100fL)
+#if (OPENSSL_VERSION_NUMBER >= 0x1010100fL) && defined(TLS1_3_VERSION) && !defined(OPENSSL_NO_TLS1_3)
     if (tls13)
     {
         SSL_CTX_set_min_proto_version(tls_ctx.ctx, TLS1_3_VERSION);