From dffcf2d14840f5843f3d1ba6ad60beb13bb44e1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Dom=C3=ADnguez=20L=C3=B3pez?= <116071334+Mario-DL@users.noreply.github.com> Date: Thu, 14 Nov 2024 11:03:19 +0100 Subject: [PATCH] [22024] Improve `OpenSSL` lifecycle handling (#5384) * Refs #22024: Add BB test Signed-off-by: Mario Dominguez * Refs #22024: Make OpenSSLInit Mayers singleton Signed-off-by: Mario Dominguez * Refs #22024: Fix: Do not register atexit in OPenSSL. Instead, Comply with OpenSSL initialization and destruction. Signed-off-by: Mario Dominguez * Refs #22024: Do not reference OpenSSLInit if security features are no present Signed-off-by: Mario Dominguez --------- Signed-off-by: Mario Dominguez (cherry picked from commit 44310c4f8efabfacc55d14604d25175d8f33070a) --- src/cpp/rtps/RTPSDomainImpl.hpp | 7 ++++ src/cpp/rtps/security/SecurityManager.cpp | 1 - src/cpp/security/OpenSSLInit.hpp | 39 ++++++++++++------- .../blackbox/common/BlackboxTestsSecurity.cpp | 27 +++++++++++++ 4 files changed, 59 insertions(+), 15 deletions(-) diff --git a/src/cpp/rtps/RTPSDomainImpl.hpp b/src/cpp/rtps/RTPSDomainImpl.hpp index 0a5fe87b650..ca48b880cad 100644 --- a/src/cpp/rtps/RTPSDomainImpl.hpp +++ b/src/cpp/rtps/RTPSDomainImpl.hpp @@ -35,6 +35,10 @@ #include #include +#if HAVE_SECURITY +#include +#endif // HAVE_SECURITY + #include namespace eprosima { @@ -285,6 +289,9 @@ class RTPSDomainImpl std::shared_ptr boost_singleton_handler_ { eprosima::detail:: BoostAtExitRegistry:: get_instance() }; +#if HAVE_SECURITY + std::shared_ptr openssl_singleton_handler_{ security::OpenSSLInit::get_instance() }; +#endif // HAVE_SECURITY std::mutex m_mutex; diff --git a/src/cpp/rtps/security/SecurityManager.cpp b/src/cpp/rtps/security/SecurityManager.cpp index 1ee994464e6..a302d47fae2 100644 --- a/src/cpp/rtps/security/SecurityManager.cpp +++ b/src/cpp/rtps/security/SecurityManager.cpp @@ -106,7 +106,6 @@ SecurityManager::SecurityManager( participant->get_attributes().allocation.data_limits}) { assert(participant != nullptr); - static OpenSSLInit openssl_init; } SecurityManager::~SecurityManager() diff --git a/src/cpp/security/OpenSSLInit.hpp b/src/cpp/security/OpenSSLInit.hpp index fab1e0aba3d..d204fe132bf 100644 --- a/src/cpp/security/OpenSSLInit.hpp +++ b/src/cpp/security/OpenSSLInit.hpp @@ -1,3 +1,19 @@ +// Copyright 2024 Proyectos y Sistemas de Mantenimiento SL (eProsima). +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include + #include #include #include @@ -14,24 +30,19 @@ class OpenSSLInit OpenSSLInit() { -#if OPENSSL_VERSION_NUMBER < 0x10100000L - OpenSSL_add_all_algorithms(); -#endif // if OPENSSL_VERSION_NUMBER < 0x10100000L + uint64_t opts = OPENSSL_INIT_NO_ATEXIT; + OPENSSL_init_crypto(opts, NULL); } ~OpenSSLInit() { -#if OPENSSL_VERSION_NUMBER < 0x10000000L - ERR_remove_state(0); - ENGINE_cleanup(); -#elif OPENSSL_VERSION_NUMBER < 0x10100000L - ERR_remove_thread_state(NULL); - ENGINE_cleanup(); -#endif // if OPENSSL_VERSION_NUMBER < 0x10000000L - RAND_cleanup(); - CRYPTO_cleanup_all_ex_data(); - ERR_free_strings(); - EVP_cleanup(); + OPENSSL_cleanup(); + } + + static std::shared_ptr get_instance() + { + static auto instance = std::make_shared(); + return instance; } }; diff --git a/test/blackbox/common/BlackboxTestsSecurity.cpp b/test/blackbox/common/BlackboxTestsSecurity.cpp index 83e331628d8..05e1d60fea3 100644 --- a/test/blackbox/common/BlackboxTestsSecurity.cpp +++ b/test/blackbox/common/BlackboxTestsSecurity.cpp @@ -5191,6 +5191,33 @@ TEST(Security, participant_stateless_secure_writer_pool_change_is_removed_upon_p EXPECT_EQ(0u, n_logs); } +// Regression test for Redmine issue #22024 +// OpenSSL assertion is not thrown when the library is abruptly finished. +TEST(Security, openssl_correctly_finishes) +{ + // Create + PubSubWriter writer("HelloWorldTopic_openssl_is_correctly_finished"); + PubSubReader reader("HelloWorldTopic_openssl_is_correctly_finished"); + + const std::string governance_file("governance_helloworld_all_enable.smime"); + const std::string permissions_file("permissions_helloworld.smime"); + + CommonPermissionsConfigure(reader, writer, governance_file, permissions_file); + + reader.init(); + writer.init(); + + ASSERT_TRUE(reader.isInitialized()); + ASSERT_TRUE(writer.isInitialized()); + + std::this_thread::sleep_for(std::chrono::seconds(2)); + + // Here we force the atexit function from openssl to be abruptly called + // i.e in a disordered way + // If OpenSSL is not correctly finished, a SIGSEGV will be thrown + std::exit(0); +} + void blackbox_security_init() { certs_path = std::getenv("CERTS_PATH");