From bee45b11c61d354c19b741f7375a3296a2b72ea6 Mon Sep 17 00:00:00 2001 From: Erik Boasson Date: Fri, 22 Mar 2024 15:28:43 +0100 Subject: [PATCH] Drop support for OpenSSL pre-1.1.1 OpenSSL versions older than 1.1.1 have all been dead for over 4 years. I don't see why an Cyclone would have to continue supporting the bad practice of not updating EOL'd security sensitive libraries full of known vulnerabilities. Of course nobody should be using OpenSSL 1.1.1 anymore (it has been EOL'd about half a year ago), but I know there are still plenty of systems in the field that rely on it and even the CI on Azure gets the latest Linux images with it pre-installed. Signed-off-by: Erik Boasson --- README.md | 2 +- docs/dev/dds_security_effort.md | 4 +- src/core/ddsi/src/ddsi_ssl.c | 79 ------------------- .../authentication/src/auth_utils.c | 16 +--- .../tests/common/src/handshake_helper.c | 21 +---- .../src/get_xxx_sec_attributes_utests.c | 4 - .../src/register_local_datareader_utests.c | 4 - .../src/register_local_datawriter_utests.c | 4 - .../src/register_local_participant_utests.c | 4 - ...egister_matched_remote_datareader_utests.c | 4 - ...egister_matched_remote_datawriter_utests.c | 4 - ...gister_matched_remote_participant_utests.c | 4 - .../validate_begin_handshake_reply_utests.c | 8 -- .../include/dds/security/openssl_support.h | 35 -------- src/security/openssl/src/openssl_support.c | 76 ------------------ 15 files changed, 4 insertions(+), 265 deletions(-) diff --git a/README.md b/README.md index 7f08fae3e9..808566ded4 100644 --- a/README.md +++ b/README.md @@ -114,7 +114,7 @@ In order to build Cyclone DDS you need a Linux, Mac or Windows 10 machine (or, w * C compiler (most commonly GCC on Linux, Visual Studio on Windows, Xcode on macOS); * Optionally GIT version control system; * [CMake](https://cmake.org/download/), version 3.16 or later; - * Optionally [OpenSSL](https://www.openssl.org/), preferably version 1.1; + * Optionally [OpenSSL](https://www.openssl.org/), we recommend a fully patched and supported version but 1.1.1 will still work; * Optionally [Eclipse Iceoryx](https://iceoryx.io) version 2.0 for shared memory and zero-copy support; * Optionally [Bison](https://www.gnu.org/software/bison/) parser generator. A cached source is checked into the repository. diff --git a/docs/dev/dds_security_effort.md b/docs/dev/dds_security_effort.md index 5f150838c1..265363a448 100644 --- a/docs/dev/dds_security_effort.md +++ b/docs/dev/dds_security_effort.md @@ -137,9 +137,7 @@ No major changes between the DDS Security plugins in OpenSplice and Cyclone are expected. The DDS Security plugins require OpenSSL. Cyclone DDS already uses OpenSSL. -However, it expects (or at least it's preferred to have) version 1.1 or newer, -while the OpenSplice Security plugins are build against 1.0.2. There are some -API changes between the two versions. This will take some porting effort. +We recommend a fully patched and supported version but 1.1.1 will still work. The build system should be ported from makefiles to cmake files. diff --git a/src/core/ddsi/src/ddsi_ssl.c b/src/core/ddsi/src/ddsi_ssl.c index 26c75f9970..e43f8e53fb 100644 --- a/src/core/ddsi/src/ddsi_ssl.c +++ b/src/core/ddsi/src/ddsi_ssl.c @@ -129,52 +129,6 @@ static ssize_t ddsi_ssl_write (SSL *ssl, const void *buf, size_t len, dds_return return sent; } -/* Standard OpenSSL init and thread support routines. See O'Reilly. */ -#if OPENSSL_VERSION_NUMBER < 0x10100000L -static unsigned long ddsi_ssl_id (void) -{ - return (unsigned long) ddsrt_gettid (); -} - -typedef struct CRYPTO_dynlock_value { - ddsrt_mutex_t m_mutex; -} CRYPTO_dynlock_value; - -static CRYPTO_dynlock_value *ddsi_ssl_locks = NULL; - -static void ddsi_ssl_dynlock_lock (int mode, CRYPTO_dynlock_value *lock, const char *file, int line) -{ - (void) file; - (void) line; - if (mode & CRYPTO_LOCK) - ddsrt_mutex_lock (&lock->m_mutex); - else - ddsrt_mutex_unlock (&lock->m_mutex); -} - -static void ddsi_ssl_lock (int mode, int n, const char *file, int line) -{ - ddsi_ssl_dynlock_lock (mode, &ddsi_ssl_locks[n], file, line); -} - -static CRYPTO_dynlock_value *ddsi_ssl_dynlock_create (const char *file, int line) -{ - (void) file; - (void) line; - CRYPTO_dynlock_value *val = ddsrt_malloc (sizeof (*val)); - ddsrt_mutex_init (&val->m_mutex); - return val; -} - -static void ddsi_ssl_dynlock_destroy (CRYPTO_dynlock_value *lock, const char *file, int line) -{ - (void) file; - (void) line; - ddsrt_mutex_destroy (&lock->m_mutex); - ddsrt_free (lock); -} -#endif - static int ddsi_ssl_password (char *buf, int num, int rwflag, void *udata) { size_t cnt; @@ -361,48 +315,15 @@ static bool ddsi_ssl_init (struct ddsi_domaingv *gv) SSL_load_error_strings (); SSL_library_init (); OpenSSL_add_all_algorithms (); - -#if OPENSSL_VERSION_NUMBER < 0x10100000L - { - const int locks = CRYPTO_num_locks (); - assert (locks >= 0); - ddsi_ssl_locks = ddsrt_malloc (sizeof (CRYPTO_dynlock_value) * (size_t) locks); - for (int i = 0; i < locks; i++) - ddsrt_mutex_init (&ddsi_ssl_locks[i].m_mutex); - } -#endif - /* Leave these in place: OpenSSL 1.1 defines them as no-op macros that not even reference the symbol, - therefore leaving them in means we get compile time errors if we the library expects the callbacks - to be defined and we somehow failed to detect that previously */ - CRYPTO_set_id_callback (ddsi_ssl_id); - CRYPTO_set_locking_callback (ddsi_ssl_lock); - CRYPTO_set_dynlock_create_callback (ddsi_ssl_dynlock_create); - CRYPTO_set_dynlock_lock_callback (ddsi_ssl_dynlock_lock); - CRYPTO_set_dynlock_destroy_callback (ddsi_ssl_dynlock_destroy); ddsi_ssl_ctx = ddsi_ssl_ctx_init (gv); - return (ddsi_ssl_ctx != NULL); } static void ddsi_ssl_fini (void) { SSL_CTX_free (ddsi_ssl_ctx); - CRYPTO_set_id_callback (0); - CRYPTO_set_locking_callback (0); - CRYPTO_set_dynlock_create_callback (0); - CRYPTO_set_dynlock_lock_callback (0); - CRYPTO_set_dynlock_destroy_callback (0); ERR_free_strings (); EVP_cleanup (); - -#if OPENSSL_VERSION_NUMBER < 0x10100000L - { - const int locks = CRYPTO_num_locks (); - for (int i = 0; i < locks; i++) - ddsrt_mutex_destroy (&ddsi_ssl_locks[i].m_mutex); - ddsrt_free (ddsi_ssl_locks); - } -#endif } void ddsi_ssl_config_plugin (struct ddsi_ssl_plugins *plugin) diff --git a/src/security/builtin_plugins/authentication/src/auth_utils.c b/src/security/builtin_plugins/authentication/src/auth_utils.c index 388f58fff6..6b71c13e73 100644 --- a/src/security/builtin_plugins/authentication/src/auth_utils.c +++ b/src/security/builtin_plugins/authentication/src/auth_utils.c @@ -887,23 +887,9 @@ DDS_Security_ValidationResult_t generate_dh_keys(EVP_PKEY **dhkey, Authenticatio static const BIGNUM *dh_get_public_key(DH *dhkey) { -#ifdef AUTH_INCLUDE_DH_ACCESSORS const BIGNUM *pubkey, *privkey; DH_get0_key(dhkey, &pubkey, &privkey); return pubkey; -#else - return dhkey->pub_key; -#endif -} - -static int dh_set_public_key(DH *dhkey, BIGNUM *pubkey) -{ -#ifdef AUTH_INCLUDE_DH_ACCESSORS - return DH_set0_key(dhkey, pubkey, NULL); -#else - dhkey->pub_key = pubkey; -#endif - return 1; } static DDS_Security_ValidationResult_t dh_public_key_to_oct_modp(EVP_PKEY *pkey, unsigned char **buffer, uint32_t *length, DDS_Security_SecurityException *ex) @@ -1014,7 +1000,7 @@ static DDS_Security_ValidationResult_t dh_oct_to_public_key_modp(EVP_PKEY **pkey } dhkey = DH_get_2048_256(); - if (dh_set_public_key(dhkey, pubkey) == 0) + if (DH_set0_key(dhkey, pubkey, NULL) == 0) { DDS_Security_Exception_set_with_openssl_error(ex, DDS_AUTH_PLUGIN_CONTEXT, DDS_SECURITY_ERR_UNDEFINED_CODE, DDS_SECURITY_VALIDATION_FAILED, "Failed to set DH public key: "); goto fail_get_pubkey; diff --git a/src/security/builtin_plugins/tests/common/src/handshake_helper.c b/src/security/builtin_plugins/tests/common/src/handshake_helper.c index 8e4ee733a9..1ba43b8b5e 100644 --- a/src/security/builtin_plugins/tests/common/src/handshake_helper.c +++ b/src/security/builtin_plugins/tests/common/src/handshake_helper.c @@ -50,26 +50,9 @@ static const BIGNUM * dh_get_public_key( DH *dhkey) { -#ifdef AUTH_INCLUDE_DH_ACCESSORS const BIGNUM *pubkey, *privkey; DH_get0_key(dhkey, &pubkey, &privkey); return pubkey; -#else - return dhkey->pub_key; -#endif -} - -static int -dh_set_public_key( - DH *dhkey, - BIGNUM *pubkey) -{ -#ifdef AUTH_INCLUDE_DH_ACCESSORS - return DH_set0_key(dhkey, pubkey, NULL); -#else - dhkey->pub_key = pubkey; -#endif - return 1; } ASN1_INTEGER * @@ -208,8 +191,7 @@ modp_data_to_pubkey( goto fail_dhkey; } - dh_set_public_key(dhkey,bn); - + DH_set0_key(dhkey, bn, NULL); if (!(pkey = EVP_PKEY_new())) { char *msg = get_openssl_error_message_for_test(); printf("Failed to allocate pkey: %s", msg); @@ -688,7 +670,6 @@ create_dh_key_ecdh( #endif - /* for DEBUG purposes */ void print_binary_test( char* name, unsigned char *value, uint32_t size){ uint32_t i; diff --git a/src/security/builtin_plugins/tests/get_xxx_sec_attributes/src/get_xxx_sec_attributes_utests.c b/src/security/builtin_plugins/tests/get_xxx_sec_attributes/src/get_xxx_sec_attributes_utests.c index d4053dfadd..80316008aa 100644 --- a/src/security/builtin_plugins/tests/get_xxx_sec_attributes/src/get_xxx_sec_attributes_utests.c +++ b/src/security/builtin_plugins/tests/get_xxx_sec_attributes/src/get_xxx_sec_attributes_utests.c @@ -23,10 +23,6 @@ #include "common/src/loader.h" #include "config_env.h" -#if OPENSLL_VERSION_NUMBER >= 0x10002000L -#define AUTH_INCLUDE_EC -#endif - static const char *RELATIVE_PATH_TO_ETC_DIR = "/get_xxx_sec_attributes/etc/"; static const char *IDENTITY_CERTIFICATE = diff --git a/src/security/builtin_plugins/tests/register_local_datareader/src/register_local_datareader_utests.c b/src/security/builtin_plugins/tests/register_local_datareader/src/register_local_datareader_utests.c index 6eebf2c4cc..b6b218354e 100644 --- a/src/security/builtin_plugins/tests/register_local_datareader/src/register_local_datareader_utests.c +++ b/src/security/builtin_plugins/tests/register_local_datareader/src/register_local_datareader_utests.c @@ -25,10 +25,6 @@ #include "common/src/crypto_helper.h" #include "crypto_objects.h" -#if OPENSLL_VERSION_NUMBER >= 0x10002000L -#define AUTH_INCLUDE_EC -#endif - #define TEST_SHARED_SECRET_SIZE 32 static struct plugins_hdl *plugins = NULL; diff --git a/src/security/builtin_plugins/tests/register_local_datawriter/src/register_local_datawriter_utests.c b/src/security/builtin_plugins/tests/register_local_datawriter/src/register_local_datawriter_utests.c index 63d0cc2eab..5e4d8ec63f 100644 --- a/src/security/builtin_plugins/tests/register_local_datawriter/src/register_local_datawriter_utests.c +++ b/src/security/builtin_plugins/tests/register_local_datawriter/src/register_local_datawriter_utests.c @@ -25,10 +25,6 @@ #include "common/src/crypto_helper.h" #include "crypto_objects.h" -#if OPENSLL_VERSION_NUMBER >= 0x10002000L -#define AUTH_INCLUDE_EC -#endif - #define TEST_SHARED_SECRET_SIZE 32 static struct plugins_hdl *plugins = NULL; diff --git a/src/security/builtin_plugins/tests/register_local_participant/src/register_local_participant_utests.c b/src/security/builtin_plugins/tests/register_local_participant/src/register_local_participant_utests.c index 8dfd71f47a..50f847ae24 100644 --- a/src/security/builtin_plugins/tests/register_local_participant/src/register_local_participant_utests.c +++ b/src/security/builtin_plugins/tests/register_local_participant/src/register_local_participant_utests.c @@ -24,10 +24,6 @@ #include "common/src/loader.h" #include "crypto_objects.h" -#if OPENSLL_VERSION_NUMBER >= 0x10002000L -#define AUTH_INCLUDE_EC -#endif - static struct plugins_hdl *plugins = NULL; static dds_security_cryptography *crypto = NULL; diff --git a/src/security/builtin_plugins/tests/register_matched_remote_datareader/src/register_matched_remote_datareader_utests.c b/src/security/builtin_plugins/tests/register_matched_remote_datareader/src/register_matched_remote_datareader_utests.c index 3b2bdaa895..d170f187f5 100644 --- a/src/security/builtin_plugins/tests/register_matched_remote_datareader/src/register_matched_remote_datareader_utests.c +++ b/src/security/builtin_plugins/tests/register_matched_remote_datareader/src/register_matched_remote_datareader_utests.c @@ -25,10 +25,6 @@ #include "common/src/crypto_helper.h" #include "crypto_objects.h" -#if OPENSLL_VERSION_NUMBER >= 0x10002000L -#define AUTH_INCLUDE_EC -#endif - #define TEST_SHARED_SECRET_SIZE 32 static struct plugins_hdl *plugins = NULL; diff --git a/src/security/builtin_plugins/tests/register_matched_remote_datawriter/src/register_matched_remote_datawriter_utests.c b/src/security/builtin_plugins/tests/register_matched_remote_datawriter/src/register_matched_remote_datawriter_utests.c index eded0ccd5d..9ff2c04d87 100644 --- a/src/security/builtin_plugins/tests/register_matched_remote_datawriter/src/register_matched_remote_datawriter_utests.c +++ b/src/security/builtin_plugins/tests/register_matched_remote_datawriter/src/register_matched_remote_datawriter_utests.c @@ -25,10 +25,6 @@ #include "common/src/crypto_helper.h" #include "crypto_objects.h" -#if OPENSLL_VERSION_NUMBER >= 0x10002000L -#define AUTH_INCLUDE_EC -#endif - #define TEST_SHARED_SECRET_SIZE 32 static struct plugins_hdl *plugins = NULL; diff --git a/src/security/builtin_plugins/tests/register_matched_remote_participant/src/register_matched_remote_participant_utests.c b/src/security/builtin_plugins/tests/register_matched_remote_participant/src/register_matched_remote_participant_utests.c index 60f4bd1144..b9faf9a6e9 100644 --- a/src/security/builtin_plugins/tests/register_matched_remote_participant/src/register_matched_remote_participant_utests.c +++ b/src/security/builtin_plugins/tests/register_matched_remote_participant/src/register_matched_remote_participant_utests.c @@ -22,10 +22,6 @@ #include "common/src/loader.h" #include "crypto_objects.h" -#if OPENSLL_VERSION_NUMBER >= 0x10002000L -#define AUTH_INCLUDE_EC -#endif - #define TEST_SHARED_SECRET_SIZE 32 static struct plugins_hdl *plugins = NULL; diff --git a/src/security/builtin_plugins/tests/validate_begin_handshake_reply/src/validate_begin_handshake_reply_utests.c b/src/security/builtin_plugins/tests/validate_begin_handshake_reply/src/validate_begin_handshake_reply_utests.c index 138c960e6e..92f26f3396 100644 --- a/src/security/builtin_plugins/tests/validate_begin_handshake_reply/src/validate_begin_handshake_reply_utests.c +++ b/src/security/builtin_plugins/tests/validate_begin_handshake_reply/src/validate_begin_handshake_reply_utests.c @@ -333,15 +333,7 @@ static DDS_Security_GUID_t remote_participant_guid2; static bool future_challenge_done = false; -#if OPENSSL_VERSION_NUMBER >= 0x1000200fL -#define AUTH_INCLUDE_EC #include -#if OPENSSL_VERSION_NUMBER >= 0x10100000L -#define AUTH_INCLUDE_DH_ACCESSORS -#endif -#else -#error "version not found" -#endif static void serializer_participant_data( diff --git a/src/security/openssl/include/dds/security/openssl_support.h b/src/security/openssl/include/dds/security/openssl_support.h index 352c16bb9f..ceff5dce88 100644 --- a/src/security/openssl/include/dds/security/openssl_support.h +++ b/src/security/openssl/include/dds/security/openssl_support.h @@ -13,26 +13,6 @@ #include "dds/security/dds_security_api_types.h" -/* There's OpenSSL 1.1.x and there's OpenSSL 1.0.2 and the difference is like - night and day: 1.1.0 deprecated all the initialization and cleanup routines - and so any library can link with OpenSSL and use it safely without breaking - the application code or some other library in the same process. - - OpenSSL 1.0.2h deprecated the cleanup functions such as EVP_cleanup because - calling the initialisation functions multiple times was survivable, but an - premature invocation of the cleanup functions deadly. It still has the per- - thread error state that one ought to clean up, but that firstly requires - keeping track of which threads make OpenSSL calls, and secondly we do - perform OpenSSL calls on the applications main-thread and so cleaning up - might interfere with the application code. - - Compatibility with 1.0.2 exists merely as a courtesy to those who insist on - using it with that problematic piece of code. We only initialise it, and we - don't clean up thread state. If Cyclone DDS is the only part of the process - that uses OpenSSL, it should be ok (just some some minor leaks at the end), - if the application code or another library also uses it, it'll probably be - fine too. */ - #ifdef _WIN32 /* WinSock2 must be included before openssl 1.0.2 headers otherwise winsock will be used */ #include @@ -47,19 +27,10 @@ #include #include -#if OPENSSL_VERSION_NUMBER >= 0x1000200fL -#define AUTH_INCLUDE_EC #include -#if OPENSSL_VERSION_NUMBER >= 0x10100000L -#define AUTH_INCLUDE_DH_ACCESSORS -#endif #if OPENSSL_VERSION_NUMBER >= 0x30000000L #include #endif -#else -#error "OpenSSL version is not supported" -#endif - #include #include #include @@ -73,12 +44,6 @@ void dds_openssl_init (void); -#if OPENSSL_VERSION_NUMBER < 0x10100000L -/* 1.1.0 has it as a supported API. 1.0.2 has it in practice and since that has been - obsolete for ages, chances are that we can safely use it */ -struct tm *OPENSSL_gmtime(const time_t *timer, struct tm *result); -#endif - void DDS_Security_Exception_set_with_openssl_error (DDS_Security_SecurityException *ex, const char *context, int code, int minor_code, const char *error_area) ddsrt_nonnull_all; diff --git a/src/security/openssl/src/openssl_support.c b/src/security/openssl/src/openssl_support.c index a72ddceac6..2ab2d5ced6 100644 --- a/src/security/openssl/src/openssl_support.c +++ b/src/security/openssl/src/openssl_support.c @@ -18,86 +18,10 @@ #include "dds/security/core/dds_security_utils.h" #include "dds/security/openssl_support.h" -#if OPENSSL_VERSION_NUMBER < 0x10100000L -static unsigned long ssl_id (void) -{ - return (unsigned long) ddsrt_gettid (); -} - -typedef struct CRYPTO_dynlock_value { - ddsrt_mutex_t m_mutex; -} CRYPTO_dynlock_value; - -CRYPTO_dynlock_value *dds_openssl102_ssl_locks = NULL; - -static void ssl_dynlock_lock (int mode, CRYPTO_dynlock_value *lock, const char *file, int line) -{ - (void) file; - (void) line; - if (mode & CRYPTO_LOCK) - ddsrt_mutex_lock (&lock->m_mutex); - else - ddsrt_mutex_unlock (&lock->m_mutex); -} - -static void ssl_lock (int mode, int n, const char *file, int line) -{ - ssl_dynlock_lock (mode, &dds_openssl102_ssl_locks[n], file, line); -} - -static CRYPTO_dynlock_value *ssl_dynlock_create (const char *file, int line) -{ - (void) file; - (void) line; - CRYPTO_dynlock_value *val = ddsrt_malloc (sizeof (*val)); - ddsrt_mutex_init (&val->m_mutex); - return val; -} - -static void ssl_dynlock_destroy (CRYPTO_dynlock_value *lock, const char *file, int line) -{ - (void) file; - (void) line; - ddsrt_mutex_destroy (&lock->m_mutex); - ddsrt_free (lock); -} - -void dds_openssl_init (void) -{ - // This is terribly fragile and broken-by-design, but with OpenSSL sometimes - // linked dynamically and sometimes linked statically, with Windows and Unix - // in the mix, this appears to be the compromise that makes it work reliably - // enough ... - if (CRYPTO_get_id_callback () == 0) - { - CRYPTO_set_id_callback (ssl_id); - CRYPTO_set_locking_callback (ssl_lock); - CRYPTO_set_dynlock_create_callback (ssl_dynlock_create); - CRYPTO_set_dynlock_lock_callback (ssl_dynlock_lock); - CRYPTO_set_dynlock_destroy_callback (ssl_dynlock_destroy); - - if (dds_openssl102_ssl_locks == NULL) - { - const int locks = CRYPTO_num_locks (); - assert (locks >= 0); - dds_openssl102_ssl_locks = ddsrt_malloc (sizeof (CRYPTO_dynlock_value) * (size_t) locks); - for (int i = 0; i < locks; i++) - ddsrt_mutex_init (&dds_openssl102_ssl_locks[i].m_mutex); - } - - OpenSSL_add_all_algorithms (); - OpenSSL_add_all_ciphers (); - OpenSSL_add_all_digests (); - ERR_load_BIO_strings (); - ERR_load_crypto_strings (); - } -} -#else void dds_openssl_init (void) { // nothing needed for OpenSSL 1.1.0 and later } -#endif void DDS_Security_Exception_set_with_openssl_error (DDS_Security_SecurityException *ex, const char *context, int code, int minor_code, const char *error_area) {