Improve OpenSSL lifecycle handling (#5384) (#5405)

* [22024] Improve `OpenSSL` lifecycle handling (#5384) * Refs #22024: Add BB test Signed-off-by: Mario Dominguez <[email protected]> * Refs #22024: Make OpenSSLInit Mayers singleton Signed-off-by: Mario Dominguez <[email protected]> * Refs #22024: Fix: Do not register atexit in OPenSSL. Instead, Comply with OpenSSL initialization and destruction. Signed-off-by: Mario Dominguez <[email protected]> * Refs #22024: Do not reference OpenSSLInit if security features are no present Signed-off-by: Mario Dominguez <[email protected]> --------- Signed-off-by: Mario Dominguez <[email protected]> (cherry picked from commit 44310c4) # Conflicts: # src/cpp/rtps/RTPSDomainImpl.hpp * Solve conflicts Signed-off-by: Mario Dominguez <[email protected]> * Apply Miguels suggestion Signed-off-by: Mario Dominguez <[email protected]> --------- Signed-off-by: Mario Dominguez <[email protected]> Co-authored-by: Mario Domínguez López <[email protected]> Co-authored-by: Mario Dominguez <[email protected]>
eProsima · Nov 20, 2024 · 725e8ab · 725e8ab
1 parent 7f54166
commit 725e8ab
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 20 deletions.
diff --git a/src/cpp/rtps/RTPSDomainImpl.hpp b/src/cpp/rtps/RTPSDomainImpl.hpp
@@ -29,9 +29,12 @@
 #include <fastrtps/rtps/RTPSDomain.h>
 #include <fastrtps/rtps/writer/RTPSWriter.h>
 
+#include <utils/shared_memory/BoostAtExitRegistry.hpp>
 #include <utils/SystemInfo.hpp>
 
-#include <utils/shared_memory/BoostAtExitRegistry.hpp>
+#if HAVE_SECURITY
+#include <security/OpenSSLInit.hpp>
+#endif // HAVE_SECURITY
 
 namespace eprosima {
 namespace fastrtps {
@@ -249,6 +252,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,7 +1,20 @@
-#include <openssl/evp.h>
-#include <openssl/engine.h>
-#include <openssl/rand.h>
-#include <openssl/err.h>
+// 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/crypto.h>
 
 namespace eprosima {
 namespace fastrtps {
@@ -14,24 +27,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");