[22024] Improve OpenSSL lifecycle handling (#5384)

* Refs #22024: Add BB test Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> * Refs #22024: Make OpenSSLInit Mayers singleton Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> * Refs #22024: Fix: Do not register atexit in OPenSSL. Instead, Comply with OpenSSL initialization and destruction. Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> * Refs #22024: Do not reference OpenSSLInit if security features are no present Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> --------- Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> (cherry picked from commit 44310c4) # Conflicts: # src/cpp/rtps/RTPSDomainImpl.hpp
eProsima · Nov 14, 2024 · b4b1064 · b4b1064
1 parent 1e7099f
commit b4b1064
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 15 deletions.
diff --git a/src/cpp/rtps/RTPSDomainImpl.hpp b/src/cpp/rtps/RTPSDomainImpl.hpp
@@ -31,7 +31,15 @@
 
 #include <utils/SystemInfo.hpp>
 
+<<<<<<< HEAD
 #include <utils/shared_memory/BoostAtExitRegistry.hpp>
+=======
+#if HAVE_SECURITY
+#include <security/OpenSSLInit.hpp>
+#endif // HAVE_SECURITY
+
+#include <fastdds/xtypes/type_representation/TypeObjectRegistry.hpp>
+>>>>>>> 44310c4f8 ([22024] Improve `OpenSSL` lifecycle handling (#5384))
 
 namespace eprosima {
 namespace fastrtps {
@@ -249,6 +257,9 @@ class RTPSDomainImpl
     std::shared_ptr<eprosima::detail::BoostAtExitRegistry> boost_singleton_handler_ { eprosima::detail::
                                                                                               BoostAtExitRegistry::
                                                                                               get_instance() };
+#if HAVE_SECURITY
+    std::shared_ptr<security::OpenSSLInit> 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
@@ -94,7 +94,6 @@ SecurityManager::SecurityManager(
     participant->getRTPSParticipantAttributes().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
@@ -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 <memory>
+
 #include <openssl/evp.h>
 #include <openssl/engine.h>
 #include <openssl/rand.h>
@@ -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<OpenSSLInit> get_instance()
+    {
+        static auto instance = std::make_shared<OpenSSLInit>();
+        return instance;
     }
 
 };

diff --git 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<HelloWorldPubSubType> writer("HelloWorldTopic_openssl_is_correctly_finished");
+    PubSubReader<HelloWorldPubSubType> 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");