Opened 5 years ago

Closed 4 years ago

#1159 closed Bug / Defect (fixed)

v2.4.7 fails to build with LibreSSL

Reported by: cardioid2 Owned by: Steffan Karger
Priority: major Milestone: release 2.4.8
Component: Crypto Version: OpenVPN 2.4.7 (Community Ed)
Severity: Not set (select this one, unless your'e a OpenVPN developer) Keywords:
Cc: Steffan Karger, plai

Description

ssl_openssl.c: In function ‘tls_ctx_restrict_ciphers_tls13’:
ssl_openssl.c:474:10: warning: implicit declaration of function ‘SSL_CTX_set_ciphersuites’ [-Wimplicit-function-declaration]
     if (!SSL_CTX_set_ciphersuites(ctx->ctx, openssl_ciphers))
          ^~~~~~~~~~~~~~~~~~~~~~~~
ssl_openssl.c: In function ‘show_available_tls_ciphers_list’:
ssl_openssl.c:1853:52: error: ‘TLS1_3_VERSION’ undeclared (first use in this function)
         SSL_CTX_set_min_proto_version(tls_ctx.ctx, TLS1_3_VERSION);
                                                    ^~~~~~~~~~~~~~
ssl_openssl.c:1853:52: note: each undeclared identifier is reported only once for each function it appears in
ssl_openssl.c:1874:32: warning: implicit declaration of function ‘SSL_get1_supported_ciphers’ [-Wimplicit-function-declaration]
     STACK_OF(SSL_CIPHER) *sk = SSL_get1_supported_ciphers(ssl);
                                ^~~~~~~~~~~~~~~~~~~~~~~~~~

Change History (9)

comment:1 Changed 5 years ago by cardioid2

Also two warnings:

ssl_openssl.c: In function ‘tls_ctx_restrict_ciphers_tls13’:
ssl_openssl.c:474:10: warning: implicit declaration of function ‘SSL_CTX_set_ciphersuites’ [-Wimplicit-function-declaration]
     if (!SSL_CTX_set_ciphersuites(ctx->ctx, openssl_ciphers))
          ^~~~~~~~~~~~~~~~~~~~~~~~

and

ssl_openssl.c:1874:32: warning: implicit declaration of function ‘SSL_get1_supported_ciphers’ [-Wimplicit-function-declaration]
     STACK_OF(SSL_CIPHER) *sk = SSL_get1_supported_ciphers(ssl);
ssl_openssl.c:1874:32: warning: initialization makes pointer from integer without a cast [-Wint-conversion]

comment:2 Changed 5 years ago by Gert Döring

Yes. You are using a SSL library that claims API compliance with OpenSSL >= 1.1.1, but is not providing the necessary functions to actually do so.

Please build with OpenSSL or mbedTLS.

(We were promised help for maintaining LibreSSL compatibility, which never happened in the last months since the patches for better TLS 1.3 support were committed in October last year)

We do accept patches via the normal openvpn-devel list process, but we are neither testing against LibreSSL, nor are we *caring* very much.

comment:3 Changed 5 years ago by Gert Döring

Cc: Steffan Karger plaisthos added

comment:4 Changed 5 years ago by Gert Döring

Cc: plai added; plaisthos removed

comment:5 Changed 5 years ago by cardioid2

Here is a patch. Feel free to use it.

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index c1355904..c090ea75 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -636,7 +636,9 @@ init_ssl(const struct options *options, struct tls_root_ctx *new_ctx)
     /* Since @SECLEVEL also influences loading of certificates, set the
      * cipher restrictions before loading certificates */
     tls_ctx_restrict_ciphers(new_ctx, options->cipher_list);
+#if defined(TLS1_3_VERSION)
     tls_ctx_restrict_ciphers_tls13(new_ctx, options->cipher_list_tls13);
+#endif

     if (!tls_ctx_set_options(new_ctx, options->ssl_flags))
     {
diff --git a/src/openvpn/ssl_backend.h b/src/openvpn/ssl_backend.h
index c614efa6..c83bdb1c 100644
--- a/src/openvpn/ssl_backend.h
+++ b/src/openvpn/ssl_backend.h
@@ -188,7 +188,9 @@ void tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers);
  * @param ciphers       String containing : delimited cipher names, or NULL to use
  *                                      sane defaults.
  */
+#if defined(TLS1_3_VERSION)
 void tls_ctx_restrict_ciphers_tls13(struct tls_root_ctx *ctx, const char *ciphers);
+#endif

 /**
  * Set the TLS certificate profile.  The profile defines which crypto
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index db5da65f..d846a9dc 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -423,6 +423,7 @@ tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
     }
 }

+#if defined(TLS1_3_VERSION)
 void
 convert_tls13_list_to_openssl(char *openssl_ciphers, size_t len,
                               const char *ciphers)
@@ -478,6 +479,7 @@ tls_ctx_restrict_ciphers_tls13(struct tls_root_ctx *ctx, const char *ciphers)
     }
 #endif
 }
+#endif

 void
 tls_ctx_set_cert_profile(struct tls_root_ctx *ctx, const char *profile)
@@ -1847,7 +1849,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 defined(TLS1_3_VERSION)
     if (tls13)
     {
         SSL_CTX_set_min_proto_version(tls_ctx.ctx, TLS1_3_VERSION);
@@ -1868,7 +1870,7 @@ show_available_tls_ciphers_list(const char *cipher_list,
         crypto_msg(M_FATAL, "Cannot create SSL object");
     }

-#if (OPENSSL_VERSION_NUMBER < 0x1010000fL)
+#if OPENSSL_VERSION_NUMBER < 0x1010000fL || defined(LIBRESSL_VERSION_NUMBER)
     STACK_OF(SSL_CIPHER) *sk = SSL_get_ciphers(ssl);
 #else
     STACK_OF(SSL_CIPHER) *sk = SSL_get1_supported_ciphers(ssl);

comment:6 Changed 5 years ago by tct

cc

comment:7 Changed 5 years ago by plaisthos

Biggest problem is that we have no one who really tests or works with us doing LibreSSL. There have been multiple people saying that will work with us and maintain LibreSSL but that has not materialised yet. And I am getting quite fed up with way that LibreSSL does api compatiblity. It claims to support OpenSSL 2.0.0 API when it clearly doesn't. This also means that every new feature needs an or !LIBRE_SSL. I would be okay to add FEATURE macros for libressl if they actually support new featues like TLS1.3 etc but the current way is just a pain to maintain.

Also the patch should be send to the mailing list for proper review. And the current form is wrong. (tls_ctx_restrict_ciphers_tls13 should always be defined, see the version in mbedtls.

comment:8 Changed 5 years ago by plaisthos

Also the
defined(LIBRESSL_VERSION_NUMBER) means that I have no idea of knowing when it is safe to remove the code. The idea of having the OPENSSL_VERSION macros is to remove the ifdefs when we drop support for old version but the unversioned LIBRESSL soudn like we would have to keep ancient OpenSSL APIs forever in the code to support LibreSSL

comment:9 Changed 4 years ago by Gert Döring

Component: Generic / unclassifiedCrypto
Milestone: release 2.4.8
Owner: set to Steffan Karger
Resolution: fixed
Status: newclosed
Version: OpenVPN 2.4.7 (Community Ed)

I think this is fixed in 2.4.8 -

commit c8823f7b09c3a6a2101b54763ac2ef146d372328
Author: Matthias Andree <matthias.andree@…>
Date: Sun Aug 18 13:18:11 2019 +0200

Fix regression, reinstate LibreSSL support.


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.

... closing this ticket.

Note: See TracTickets for help on using tickets.