From e520e59a78d5a4a7a1dcb2c246c75f8bd11fc416 Mon Sep 17 00:00:00 2001 From: Bart van Strien Date: Sat, 13 Apr 2024 14:21:01 +0200 Subject: [PATCH 1/5] Add support for OpenSSL3 to OpenSSLConnection --- src/generic/OpenSSLConnection.cpp | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/generic/OpenSSLConnection.cpp b/src/generic/OpenSSLConnection.cpp index 658f24f..adea6aa 100644 --- a/src/generic/OpenSSLConnection.cpp +++ b/src/generic/OpenSSLConnection.cpp @@ -13,9 +13,15 @@ OpenSSLConnection::SSLFuncs::SSLFuncs() valid = false; + // Try OpenSSL 3 + handle *sslhandle = OpenLibrary("libssl.so.3"); + handle *cryptohandle = OpenLibrary("libcrypto.so.3"); // Try OpenSSL 1.1 - handle *sslhandle = OpenLibrary("libssl.so.1.1"); - handle *cryptohandle = OpenLibrary("libcrypto.so.1.1"); + if (!sslhandle || !cryptohandle) + { + sslhandle = OpenLibrary("libssl.so.1.1"); + cryptohandle = OpenLibrary("libcrypto.so.1.1"); + } // Try OpenSSL 1.0 if (!sslhandle || !cryptohandle) { @@ -50,7 +56,8 @@ OpenSSLConnection::SSLFuncs::SSLFuncs() valid = valid && LoadSymbol(write, sslhandle, "SSL_write"); valid = valid && LoadSymbol(shutdown, sslhandle, "SSL_shutdown"); valid = valid && LoadSymbol(get_verify_result, sslhandle, "SSL_get_verify_result"); - valid = valid && LoadSymbol(get_peer_certificate, sslhandle, "SSL_get_peer_certificate"); + valid = valid && (LoadSymbol(get_peer_certificate, sslhandle, "SSL_get1_peer_certificate") || + LoadSymbol(get_peer_certificate, sslhandle, "SSL_get_peer_certificate")); valid = valid && (LoadSymbol(SSLv23_method, sslhandle, "SSLv23_method") || LoadSymbol(SSLv23_method, sslhandle, "TLS_method")); From 575dab8be550888946b676343992ec0a3ae6489a Mon Sep 17 00:00:00 2001 From: Bart van Strien Date: Sat, 13 Apr 2024 14:24:35 +0200 Subject: [PATCH 2/5] Fix memory leak in OpenSSLConnection I checked, but as far back as the docs for 1.1, the docs say you should call free. --- src/generic/OpenSSLConnection.cpp | 2 ++ src/generic/OpenSSLConnection.h | 1 + 2 files changed, 3 insertions(+) diff --git a/src/generic/OpenSSLConnection.cpp b/src/generic/OpenSSLConnection.cpp index adea6aa..9010a05 100644 --- a/src/generic/OpenSSLConnection.cpp +++ b/src/generic/OpenSSLConnection.cpp @@ -63,6 +63,7 @@ OpenSSLConnection::SSLFuncs::SSLFuncs() LoadSymbol(SSLv23_method, sslhandle, "TLS_method")); valid = valid && LoadSymbol(check_host, cryptohandle, "X509_check_host"); + valid = valid && LoadSymbol(X509_free, cryptohandle, "X509_free"); if (library_init) library_init(); @@ -125,6 +126,7 @@ bool OpenSSLConnection::connect(const std::string &hostname, uint16_t port) close(); return false; } + ssl.X509_free(cert); return true; } diff --git a/src/generic/OpenSSLConnection.h b/src/generic/OpenSSLConnection.h index 5be17be..2b4e288 100644 --- a/src/generic/OpenSSLConnection.h +++ b/src/generic/OpenSSLConnection.h @@ -53,6 +53,7 @@ class OpenSSLConnection : public Connection const SSL_METHOD *(*SSLv23_method)(); int (*check_host)(X509 *cert, const char *name, size_t namelen, unsigned int flags, char **peername); + void (*X509_free)(X509* cert); }; static SSLFuncs ssl; }; From 4f9674ef21acbd6d1557b152a7829c966e3835de Mon Sep 17 00:00:00 2001 From: Bart van Strien Date: Sat, 13 Apr 2024 14:31:56 +0200 Subject: [PATCH 3/5] Prefer modern functions --- src/generic/OpenSSLConnection.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/generic/OpenSSLConnection.cpp b/src/generic/OpenSSLConnection.cpp index 9010a05..f92cb9e 100644 --- a/src/generic/OpenSSLConnection.cpp +++ b/src/generic/OpenSSLConnection.cpp @@ -39,8 +39,9 @@ OpenSSLConnection::SSLFuncs::SSLFuncs() return; valid = true; - valid = valid && (LoadSymbol(library_init, sslhandle, "SSL_library_init") || - LoadSymbol(init_ssl, sslhandle, "OPENSSL_init_ssl")); + valid = valid && ( + LoadSymbol(init_ssl, sslhandle, "OPENSSL_init_ssl") || + LoadSymbol(library_init, sslhandle, "SSL_library_init")); valid = valid && LoadSymbol(CTX_new, sslhandle, "SSL_CTX_new"); valid = valid && LoadSymbol(CTX_ctrl, sslhandle, "SSL_CTX_ctrl"); @@ -59,8 +60,10 @@ OpenSSLConnection::SSLFuncs::SSLFuncs() valid = valid && (LoadSymbol(get_peer_certificate, sslhandle, "SSL_get1_peer_certificate") || LoadSymbol(get_peer_certificate, sslhandle, "SSL_get_peer_certificate")); - valid = valid && (LoadSymbol(SSLv23_method, sslhandle, "SSLv23_method") || - LoadSymbol(SSLv23_method, sslhandle, "TLS_method")); + valid = valid && ( + LoadSymbol(SSLv23_method, sslhandle, "TLS_client_method") || + LoadSymbol(SSLv23_method, sslhandle, "TLS_method") || + LoadSymbol(SSLv23_method, sslhandle, "SSLv23_method")); valid = valid && LoadSymbol(check_host, cryptohandle, "X509_check_host"); valid = valid && LoadSymbol(X509_free, cryptohandle, "X509_free"); From 003e1459a9fe13aa2f1271a472fc2ea782a8ddd7 Mon Sep 17 00:00:00 2001 From: Bart van Strien Date: Sat, 13 Apr 2024 14:34:49 +0200 Subject: [PATCH 4/5] Prefer SSL_CTX_set_options over SSL_CTX_ctrl As indicated by SSL_CTRL_OPTIONS no longer being defined, we should avoid it as much as possible. --- src/generic/OpenSSLConnection.cpp | 7 ++++++- src/generic/OpenSSLConnection.h | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/generic/OpenSSLConnection.cpp b/src/generic/OpenSSLConnection.cpp index f92cb9e..e5c2fc2 100644 --- a/src/generic/OpenSSLConnection.cpp +++ b/src/generic/OpenSSLConnection.cpp @@ -45,6 +45,8 @@ OpenSSLConnection::SSLFuncs::SSLFuncs() valid = valid && LoadSymbol(CTX_new, sslhandle, "SSL_CTX_new"); valid = valid && LoadSymbol(CTX_ctrl, sslhandle, "SSL_CTX_ctrl"); + if (valid) + LoadSymbol(CTX_set_options, sslhandle, "SSL_CTX_set_options"); valid = valid && LoadSymbol(CTX_set_verify, sslhandle, "SSL_CTX_set_verify"); valid = valid && LoadSymbol(CTX_set_default_verify_paths, sslhandle, "SSL_CTX_set_default_verify_paths"); valid = valid && LoadSymbol(CTX_free, sslhandle, "SSL_CTX_free"); @@ -87,7 +89,10 @@ OpenSSLConnection::OpenSSLConnection() if (!context) return; - ssl.CTX_ctrl(context, SSL_CTRL_OPTIONS, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3, nullptr); + if (ssl.CTX_set_options) + ssl.CTX_set_options(context, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3); + else + ssl.CTX_ctrl(context, SSL_CTRL_OPTIONS, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3, nullptr); ssl.CTX_set_verify(context, SSL_VERIFY_PEER, nullptr); ssl.CTX_set_default_verify_paths(context); } diff --git a/src/generic/OpenSSLConnection.h b/src/generic/OpenSSLConnection.h index 2b4e288..0c44a69 100644 --- a/src/generic/OpenSSLConnection.h +++ b/src/generic/OpenSSLConnection.h @@ -36,6 +36,7 @@ class OpenSSLConnection : public Connection SSL_CTX *(*CTX_new)(const SSL_METHOD *method); long (*CTX_ctrl)(SSL_CTX *ctx, int cmd, long larg, void *parg); + long (*CTX_set_options)(SSL_CTX *ctx, long options); void (*CTX_set_verify)(SSL_CTX *ctx, int mode, void *verify_callback); int (*CTX_set_default_verify_paths)(SSL_CTX *ctx); void (*CTX_free)(SSL_CTX *ctx); From 901291e8a5da8a8d21421a286e6846525f18e2bd Mon Sep 17 00:00:00 2001 From: Bart van Strien Date: Sat, 13 Apr 2024 14:43:26 +0200 Subject: [PATCH 5/5] Introduce helper for trying different OpenSSL versions So now the source just contains an in-order list, which is a lot more legible. --- src/generic/OpenSSLConnection.cpp | 47 +++++++++++++++---------------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/src/generic/OpenSSLConnection.cpp b/src/generic/OpenSSLConnection.cpp index e5c2fc2..ff5404e 100644 --- a/src/generic/OpenSSLConnection.cpp +++ b/src/generic/OpenSSLConnection.cpp @@ -7,35 +7,34 @@ // Not present in openssl 1.1 headers #define SSL_CTRL_OPTIONS 32 +static bool TryOpenLibraries(const char *sslName, LibraryLoader::handle *& sslHandle, const char *cryptoName, LibraryLoader::handle *&cryptoHandle) +{ + sslHandle = LibraryLoader::OpenLibrary(sslName); + cryptoHandle = LibraryLoader::OpenLibrary(cryptoName); + + if (sslHandle && cryptoHandle) + return true; + + if (sslHandle) + LibraryLoader::CloseLibrary(sslHandle); + if (cryptoHandle) + LibraryLoader::CloseLibrary(cryptoHandle); + return false; +} + OpenSSLConnection::SSLFuncs::SSLFuncs() { using namespace LibraryLoader; - valid = false; + handle *sslhandle = nullptr; + handle *cryptohandle = nullptr; - // Try OpenSSL 3 - handle *sslhandle = OpenLibrary("libssl.so.3"); - handle *cryptohandle = OpenLibrary("libcrypto.so.3"); - // Try OpenSSL 1.1 - if (!sslhandle || !cryptohandle) - { - sslhandle = OpenLibrary("libssl.so.1.1"); - cryptohandle = OpenLibrary("libcrypto.so.1.1"); - } - // Try OpenSSL 1.0 - if (!sslhandle || !cryptohandle) - { - sslhandle = OpenLibrary("libssl.so.1.0.0"); - cryptohandle = OpenLibrary("libcrypto.so.1.0.0"); - } - // Try OpenSSL without version - if (!sslhandle || !cryptohandle) - { - sslhandle = OpenLibrary("libssl.so"); - cryptohandle = OpenLibrary("libcrypto.so"); - } - // Give up - if (!sslhandle || !cryptohandle) + valid = TryOpenLibraries("libssl.so.3", sslhandle, "libcrypto.so.3", cryptohandle) + || TryOpenLibraries("libssl.so.1.1", sslhandle, "libcrypto.so.1.1", cryptohandle) + || TryOpenLibraries("libssl.so.1.0.0", sslhandle, "libcrypto.so.1.0.0", cryptohandle) + // Try the version-less name last, it may not be compatible or tested + || TryOpenLibraries("libssl.so", sslhandle, "libcrypto.so", cryptohandle); + if (!valid) return; valid = true;