From 432a8f221ab55da6fbeadd34cc43a4819aff9fad Mon Sep 17 00:00:00 2001 From: Geod24 Date: Thu, 12 May 2022 09:36:16 +0200 Subject: [PATCH 1/5] openssl: Work around deimos/openssl change in 2.0.2+1.1.1.h As explained in the comment, a breaking change happened while fixing a bug. --- tls/vibe/stream/openssl.d | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/tls/vibe/stream/openssl.d b/tls/vibe/stream/openssl.d index 88c219a25f..75f2d72c0a 100644 --- a/tls/vibe/stream/openssl.d +++ b/tls/vibe/stream/openssl.d @@ -198,6 +198,14 @@ static if (OPENSSL_VERSION.startsWith("1.1")) { } } +// Deimos had an incorrect translation for this define prior to 2.0.2+1.1.0h +// See https://github.com/D-Programming-Deimos/openssl/issues/63#issuecomment-840266138 +static if (!is(typeof(GEN_DNS))) +{ + private enum GEN_DNS = GENERAL_NAME.GEN_DNS; + private enum GEN_IPADD = GENERAL_NAME.GEN_IPADD; +} + private int SSL_set_tlsext_host_name(ssl_st* s, const(char)* c) @trusted { return cast(int) SSL_ctrl(s, SSL_CTRL_SET_TLSEXT_HOSTNAME, TLSEXT_NAMETYPE_host_name, cast(void*)c); } @@ -286,7 +294,7 @@ final class OpenSSLStream : TLSStream { readPeerCertInfo(); auto result = () @trusted { return SSL_get_verify_result(m_tls); } (); if (result == X509_V_OK && (ctx.peerValidationMode & TLSPeerValidationMode.checkPeer)) { - if (!verifyCertName(m_peerCertificate, GENERAL_NAME.GEN_DNS, vdata.peerName)) { + if (!verifyCertName(m_peerCertificate, GEN_DNS, vdata.peerName)) { version(Windows) import core.sys.windows.winsock2; else import core.sys.posix.netinet.in_; @@ -305,7 +313,7 @@ final class OpenSSLStream : TLSStream { break; } - if (!verifyCertName(m_peerCertificate, GENERAL_NAME.GEN_IPADD, () @trusted { return addr[0 .. addrlen]; } ())) { + if (!verifyCertName(m_peerCertificate, GEN_IPADD, () @trusted { return addr[0 .. addrlen]; } ())) { logDiagnostic("Error validating TLS peer address"); result = X509_V_ERR_APPLICATION_VERIFICATION; } @@ -1160,12 +1168,12 @@ private bool verifyCertName(X509* cert, int field, in char[] value, bool allow_w int cnid; int alt_type; final switch (field) { - case GENERAL_NAME.GEN_DNS: + case GEN_DNS: cnid = NID_commonName; alt_type = V_ASN1_IA5STRING; str_match = allow_wildcards ? (in s) => matchWildcard(value, s) : (in s) => s.icmp(value) == 0; break; - case GENERAL_NAME.GEN_IPADD: + case GEN_IPADD: cnid = 0; alt_type = V_ASN1_OCTET_STRING; str_match = (in s) => s == value; @@ -1178,7 +1186,7 @@ private bool verifyCertName(X509* cert, int field, in char[] value, bool allow_w foreach (i; 0 .. sk_GENERAL_NAME_num(gens)) { auto gen = sk_GENERAL_NAME_value(gens, i); if (gen.type != field) continue; - ASN1_STRING *cstr = field == GENERAL_NAME.GEN_DNS ? gen.d.dNSName : gen.d.iPAddress; + ASN1_STRING *cstr = field == GEN_DNS ? gen.d.dNSName : gen.d.iPAddress; if (check_value(cstr, alt_type)) return true; } if (!cnid) return false; From dc693646b0002f8d1992284cbf509d1526fa0bb7 Mon Sep 17 00:00:00 2001 From: Geod24 Date: Thu, 12 May 2022 11:18:08 +0200 Subject: [PATCH 2/5] [trivial] openssl: separate module decl / version / import more visibly --- tls/vibe/stream/openssl.d | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tls/vibe/stream/openssl.d b/tls/vibe/stream/openssl.d index 75f2d72c0a..b27f3fd9b2 100644 --- a/tls/vibe/stream/openssl.d +++ b/tls/vibe/stream/openssl.d @@ -6,7 +6,9 @@ Authors: Sönke Ludwig */ module vibe.stream.openssl; + version(Have_openssl): + import vibe.core.log; import vibe.core.net; import vibe.core.stream; From e53b865d2d5982372011c656a99a5be27aaccd23 Mon Sep 17 00:00:00 2001 From: Geod24 Date: Thu, 12 May 2022 11:31:57 +0200 Subject: [PATCH 3/5] openssl: Warn when version detection fails OpenSSL issues are not rare, and are an ongoing source of pain. Auto-detection has massively improved since this module was originally written, hence the soft fallback should almost never been hit. If it is, it's better to inform the user. --- tls/vibe/stream/openssl.d | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tls/vibe/stream/openssl.d b/tls/vibe/stream/openssl.d index b27f3fd9b2..f14a4a9919 100644 --- a/tls/vibe/stream/openssl.d +++ b/tls/vibe/stream/openssl.d @@ -51,8 +51,12 @@ else static if (__traits(compiles, {import openssl_version; })) mixin("public import openssl_version : OPENSSL_VERSION;"); else - // try 1.1.0 as softfallback if old other means failed + { + pragma(msg, __MODULE__, ": Could not detect OpenSSL version - Assuming OpenSSL 1.1.0"); + pragma(msg, "If you're using dub, make sure pkg-config is installed. ", + "You can also define the version explicitly (see documentation), e.g. VibeUseOpenSSL11"); enum OPENSSL_VERSION = "1.1.0"; + } } version (VibePragmaLib) { From f059fc343396bc7e3b5ab00d693485c7c5e7cff3 Mon Sep 17 00:00:00 2001 From: Geod24 Date: Thu, 12 May 2022 11:47:27 +0200 Subject: [PATCH 4/5] openssl: Start to use the newer API instead of the older one As explained in the comment, the current approach is to define a 1.0.x API in terms of a 1.1.x API. This now breaks again because the same 1.0.x API would need to be also defined in terms of the 3.0.x API. The diff in this commit starts turning things around, by using a piece of the 3.0.x (and 1.1.x) API in the OpenSSL code and just aliasing the bindings for 1.0.x --- tls/vibe/stream/openssl.d | 41 +++++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/tls/vibe/stream/openssl.d b/tls/vibe/stream/openssl.d index f14a4a9919..6acca2e189 100644 --- a/tls/vibe/stream/openssl.d +++ b/tls/vibe/stream/openssl.d @@ -71,12 +71,6 @@ enum haveALPN = OPENSSL_VERSION_NUMBER >= 0x10200000 || alpn_forced; // openssl/1.1.0 hack: provides a 1.0.x API in terms of the 1.1.x API static if (OPENSSL_VERSION.startsWith("1.1")) { - extern(C) const(SSL_METHOD)* TLS_client_method(); - alias SSLv23_client_method = TLS_client_method; - - extern(C) const(SSL_METHOD)* TLS_server_method(); - alias SSLv23_server_method = TLS_server_method; - // this does nothing in > openssl 1.1.0 void SSL_load_error_strings() {} @@ -174,7 +168,24 @@ static if (OPENSSL_VERSION.startsWith("1.1")) { c_ulong SSL_CTX_set_options(SSL_CTX *ctx, c_ulong op); } -} else { +} + +// OpenSSL version 1.1.0 is the successor to v1.0.2h and was released +// 2016-08-25 (https://www.openssl.org/news/changelog.html). +// It might still be in use for ESM users of Ubuntu 16.04 and lower, +// which will expire in 2026. +// The OpenSSL Deimos binding *must* be the right one for our version, although +// users might need to explicitly change `dub.selections.json` / dub.{json,sdl}. +// However, we cannot use the bindings directly in Vibe.d code, as it would require +// us to version some calls multiple times. Instead, we should attempt to use the +// API of the latest supported version, and provide wrapper for older versions. +// The original approach we took was the other way around, but it means that +// every release might add more work (e.g. OpenSSL 3.0.0 release). +static if (OPENSSL_VERSION.startsWith("1.0")) +{ + private alias TLS_client_method = SSLv23_client_method; + private alias TLS_server_method = SSLv23_server_method; + private void BIO_set_init(BIO* b, int init_) @safe nothrow { b.init_ = 1; } @@ -638,22 +649,22 @@ final class OpenSSLContext : TLSContext { final switch (kind) { case TLSContextKind.client: final switch (ver) { - case TLSVersion.any: method = SSLv23_client_method(); veroptions |= SSL_OP_NO_SSLv3; break; + case TLSVersion.any: method = TLS_client_method(); veroptions |= SSL_OP_NO_SSLv3; break; case TLSVersion.ssl3: throw new Exception("SSLv3 is not supported anymore"); - case TLSVersion.tls1: method = SSLv23_client_method(); veroptions |= SSL_OP_NO_SSLv3|SSL_OP_NO_TLSv1_1|SSL_OP_NO_TLSv1_2; maxver = TLS1_VERSION; break; - case TLSVersion.tls1_1: method = SSLv23_client_method(); veroptions |= SSL_OP_NO_SSLv3|SSL_OP_NO_TLSv1|SSL_OP_NO_TLSv1_2; minver = TLS1_1_VERSION; maxver = TLS1_1_VERSION; break; - case TLSVersion.tls1_2: method = SSLv23_client_method(); veroptions |= SSL_OP_NO_SSLv3|SSL_OP_NO_TLSv1|SSL_OP_NO_TLSv1_1; minver = TLS1_2_VERSION; break; + case TLSVersion.tls1: method = TLS_client_method(); veroptions |= SSL_OP_NO_SSLv3|SSL_OP_NO_TLSv1_1|SSL_OP_NO_TLSv1_2; maxver = TLS1_VERSION; break; + case TLSVersion.tls1_1: method = TLS_client_method(); veroptions |= SSL_OP_NO_SSLv3|SSL_OP_NO_TLSv1|SSL_OP_NO_TLSv1_2; minver = TLS1_1_VERSION; maxver = TLS1_1_VERSION; break; + case TLSVersion.tls1_2: method = TLS_client_method(); veroptions |= SSL_OP_NO_SSLv3|SSL_OP_NO_TLSv1|SSL_OP_NO_TLSv1_1; minver = TLS1_2_VERSION; break; case TLSVersion.dtls1: method = DTLSv1_client_method(); minver = DTLS1_VERSION; maxver = DTLS1_VERSION; break; } break; case TLSContextKind.server: case TLSContextKind.serverSNI: final switch (ver) { - case TLSVersion.any: method = SSLv23_server_method(); veroptions |= SSL_OP_NO_SSLv3; break; + case TLSVersion.any: method = TLS_server_method(); veroptions |= SSL_OP_NO_SSLv3; break; case TLSVersion.ssl3: throw new Exception("SSLv3 is not supported anymore"); - case TLSVersion.tls1: method = SSLv23_server_method(); veroptions |= SSL_OP_NO_SSLv3|SSL_OP_NO_TLSv1_1|SSL_OP_NO_TLSv1_2; maxver = TLS1_VERSION; break; - case TLSVersion.tls1_1: method = SSLv23_server_method(); veroptions |= SSL_OP_NO_SSLv3|SSL_OP_NO_TLSv1|SSL_OP_NO_TLSv1_2; minver = TLS1_1_VERSION; maxver = TLS1_1_VERSION; break; - case TLSVersion.tls1_2: method = SSLv23_server_method(); veroptions |= SSL_OP_NO_SSLv3|SSL_OP_NO_TLSv1|SSL_OP_NO_TLSv1_1; minver = TLS1_2_VERSION; break; + case TLSVersion.tls1: method = TLS_server_method(); veroptions |= SSL_OP_NO_SSLv3|SSL_OP_NO_TLSv1_1|SSL_OP_NO_TLSv1_2; maxver = TLS1_VERSION; break; + case TLSVersion.tls1_1: method = TLS_server_method(); veroptions |= SSL_OP_NO_SSLv3|SSL_OP_NO_TLSv1|SSL_OP_NO_TLSv1_2; minver = TLS1_1_VERSION; maxver = TLS1_1_VERSION; break; + case TLSVersion.tls1_2: method = TLS_server_method(); veroptions |= SSL_OP_NO_SSLv3|SSL_OP_NO_TLSv1|SSL_OP_NO_TLSv1_1; minver = TLS1_2_VERSION; break; case TLSVersion.dtls1: method = DTLSv1_server_method(); minver = DTLS1_VERSION; maxver = DTLS1_VERSION; break; } options |= SSL_OP_CIPHER_SERVER_PREFERENCE; From c07e73e6f767ee449268ecd72aaf5fd600128e1b Mon Sep 17 00:00:00 2001 From: Geod24 Date: Thu, 12 May 2022 12:17:19 +0200 Subject: [PATCH 5/5] openssl: Remove init code for 1.1.0 and later As explained in the comments, the init code is no longer necessary. --- tls/vibe/stream/openssl.d | 107 +++++++++++++++++--------------------- 1 file changed, 48 insertions(+), 59 deletions(-) diff --git a/tls/vibe/stream/openssl.d b/tls/vibe/stream/openssl.d index 6acca2e189..6a034480b0 100644 --- a/tls/vibe/stream/openssl.d +++ b/tls/vibe/stream/openssl.d @@ -71,27 +71,6 @@ enum haveALPN = OPENSSL_VERSION_NUMBER >= 0x10200000 || alpn_forced; // openssl/1.1.0 hack: provides a 1.0.x API in terms of the 1.1.x API static if (OPENSSL_VERSION.startsWith("1.1")) { - // this does nothing in > openssl 1.1.0 - void SSL_load_error_strings() {} - - extern(C) int OPENSSL_init_ssl(ulong opts, const void* settings); - - // # define SSL_library_init() OPENSSL_init_ssl(0, NULL) - int SSL_library_init() { - return OPENSSL_init_ssl(0, null); - } - - //# define CRYPTO_num_locks() (1) - int CRYPTO_num_locks() { - return 1; - } - - void CRYPTO_set_id_callback(T)(T t) { - } - - void CRYPTO_set_locking_callback(T)(T t) { - } - // #define SSL_get_ex_new_index(l, p, newf, dupf, freef) \ // CRYPTO_get_ex_new_index(CRYPTO_EX_INDEX_SSL, l, p, newf, dupf, freef) @@ -213,6 +192,34 @@ static if (OPENSSL_VERSION.startsWith("1.0")) private void BIO_set_flags(BIO *b, int flags) @safe nothrow { b.flags |= flags; } + + // The need for calling `CRYPTO_set_id_callback` / `CRYPTO_set_locking_callback` + // was removed in OpenSSL 1.1.0, which are the only users of those callbacks + // and mutexes. + private __gshared InterruptibleTaskMutex[] g_cryptoMutexes; + + private extern(C) c_ulong onCryptoGetThreadID() nothrow @safe + { + try { + return cast(c_ulong)(cast(size_t)() @trusted { return cast(void*)Thread.getThis(); } () * 0x35d2c57); + } catch (Exception e) { + logWarn("OpenSSL: failed to get current thread ID: %s", e.msg); + return 0; + } + } + + private extern(C) void onCryptoLock(int mode, int n, const(char)* file, int line) nothrow @safe + { + try { + enforce(n >= 0 && n < () @trusted { return g_cryptoMutexes; } ().length, "Mutex index out of range."); + auto mutex = () @trusted { return g_cryptoMutexes[n]; } (); + assert(mutex !is null); + if (mode & CRYPTO_LOCK) mutex.lock(); + else mutex.unlock(); + } catch (Exception e) { + logWarn("OpenSSL: failed to lock/unlock mutex: %s", e.msg); + } + } } // Deimos had an incorrect translation for this define prior to 2.0.2+1.1.0h @@ -1135,30 +1142,35 @@ alias SSLState = ssl_st*; /**************************************************************************************************/ private { - __gshared InterruptibleTaskMutex[] g_cryptoMutexes; __gshared int gs_verifyDataIndex; } shared static this() { - logDebug("Initializing OpenSSL..."); - SSL_load_error_strings(); - SSL_library_init(); + static if (OPENSSL_VERSION.startsWith("1.0")) + { + logDebug("Initializing OpenSSL..."); + // Not required as of OpenSSL 1.1.0, see: + // https://wiki.openssl.org/index.php/Library_Initialization + SSL_load_error_strings(); + SSL_library_init(); + + g_cryptoMutexes.length = CRYPTO_num_locks(); + // TODO: investigate if a normal Mutex is enough - not sure if BIO is called in a locked state + foreach (i; 0 .. g_cryptoMutexes.length) + g_cryptoMutexes[i] = new InterruptibleTaskMutex; + foreach (ref m; g_cryptoMutexes) { + assert(m !is null); + } - g_cryptoMutexes.length = CRYPTO_num_locks(); - // TODO: investigate if a normal Mutex is enough - not sure if BIO is called in a locked state - foreach (i; 0 .. g_cryptoMutexes.length) - g_cryptoMutexes[i] = new InterruptibleTaskMutex; - foreach (ref m; g_cryptoMutexes) { - assert(m !is null); + // Those two were removed in v1.1.0, see: + // https://github.com/openssl/openssl/issues/1260 + CRYPTO_set_id_callback(&onCryptoGetThreadID); + CRYPTO_set_locking_callback(&onCryptoLock); + logDebug("... done."); } - CRYPTO_set_id_callback(&onCryptoGetThreadID); - CRYPTO_set_locking_callback(&onCryptoLock); - enforce(RAND_poll(), "Fatal: failed to initialize random number generator entropy (RAND_poll)."); - logDebug("... done."); - gs_verifyDataIndex = SSL_get_ex_new_index(0, cast(void*)"VerifyData".ptr, null, null, null); } @@ -1312,29 +1324,6 @@ private nothrow @safe extern(C) return 0; } - c_ulong onCryptoGetThreadID() - { - try { - return cast(c_ulong)(cast(size_t)() @trusted { return cast(void*)Thread.getThis(); } () * 0x35d2c57); - } catch (Exception e) { - logWarn("OpenSSL: failed to get current thread ID: %s", e.msg); - return 0; - } - } - - void onCryptoLock(int mode, int n, const(char)* file, int line) - { - try { - enforce(n >= 0 && n < () @trusted { return g_cryptoMutexes; } ().length, "Mutex index out of range."); - auto mutex = () @trusted { return g_cryptoMutexes[n]; } (); - assert(mutex !is null); - if (mode & CRYPTO_LOCK) mutex.lock(); - else mutex.unlock(); - } catch (Exception e) { - logWarn("OpenSSL: failed to lock/unlock mutex: %s", e.msg); - } - } - int onBioNew(BIO *b) nothrow { BIO_set_init(b, 0);