Skip to content

Commit d831eb1

Browse files
Mario-DLMiguelCompany
authored andcommitted
[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
1 parent 28552ce commit d831eb1

File tree

4 files changed

+63
-15
lines changed

4 files changed

+63
-15
lines changed

src/cpp/rtps/RTPSDomainImpl.hpp

+11
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,15 @@
3131

3232
#include <utils/SystemInfo.hpp>
3333

34+
<<<<<<< HEAD
3435
#include <utils/shared_memory/BoostAtExitRegistry.hpp>
36+
=======
37+
#if HAVE_SECURITY
38+
#include <security/OpenSSLInit.hpp>
39+
#endif // HAVE_SECURITY
40+
41+
#include <fastdds/xtypes/type_representation/TypeObjectRegistry.hpp>
42+
>>>>>>> 44310c4f8 ([22024] Improve `OpenSSL` lifecycle handling (#5384))
3543

3644
namespace eprosima {
3745
namespace fastrtps {
@@ -237,6 +245,9 @@ class RTPSDomainImpl
237245
std::shared_ptr<eprosima::detail::BoostAtExitRegistry> boost_singleton_handler_ { eprosima::detail::
238246
BoostAtExitRegistry::
239247
get_instance() };
248+
#if HAVE_SECURITY
249+
std::shared_ptr<security::OpenSSLInit> openssl_singleton_handler_{ security::OpenSSLInit::get_instance() };
250+
#endif // HAVE_SECURITY
240251

241252
std::mutex m_mutex;
242253

src/cpp/rtps/security/SecurityManager.cpp

-1
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,6 @@ SecurityManager::SecurityManager(
106106
participant->getRTPSParticipantAttributes().allocation.data_limits})
107107
{
108108
assert(participant != nullptr);
109-
static OpenSSLInit openssl_init;
110109
}
111110

112111
SecurityManager::~SecurityManager()

src/cpp/security/OpenSSLInit.hpp

+25-14
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,19 @@
1+
// Copyright 2024 Proyectos y Sistemas de Mantenimiento SL (eProsima).
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
#include <memory>
16+
117
#include <openssl/evp.h>
218
#include <openssl/engine.h>
319
#include <openssl/rand.h>
@@ -14,24 +30,19 @@ class OpenSSLInit
1430

1531
OpenSSLInit()
1632
{
17-
#if OPENSSL_VERSION_NUMBER < 0x10100000L
18-
OpenSSL_add_all_algorithms();
19-
#endif // if OPENSSL_VERSION_NUMBER < 0x10100000L
33+
uint64_t opts = OPENSSL_INIT_NO_ATEXIT;
34+
OPENSSL_init_crypto(opts, NULL);
2035
}
2136

2237
~OpenSSLInit()
2338
{
24-
#if OPENSSL_VERSION_NUMBER < 0x10000000L
25-
ERR_remove_state(0);
26-
ENGINE_cleanup();
27-
#elif OPENSSL_VERSION_NUMBER < 0x10100000L
28-
ERR_remove_thread_state(NULL);
29-
ENGINE_cleanup();
30-
#endif // if OPENSSL_VERSION_NUMBER < 0x10000000L
31-
RAND_cleanup();
32-
CRYPTO_cleanup_all_ex_data();
33-
ERR_free_strings();
34-
EVP_cleanup();
39+
OPENSSL_cleanup();
40+
}
41+
42+
static std::shared_ptr<OpenSSLInit> get_instance()
43+
{
44+
static auto instance = std::make_shared<OpenSSLInit>();
45+
return instance;
3546
}
3647

3748
};

test/blackbox/common/BlackboxTestsSecurity.cpp

+27
Original file line numberDiff line numberDiff line change
@@ -5189,6 +5189,33 @@ TEST(Security, participant_stateless_secure_writer_pool_change_is_removed_upon_p
51895189
EXPECT_EQ(0u, n_logs);
51905190
}
51915191

5192+
// Regression test for Redmine issue #22024
5193+
// OpenSSL assertion is not thrown when the library is abruptly finished.
5194+
TEST(Security, openssl_correctly_finishes)
5195+
{
5196+
// Create
5197+
PubSubWriter<HelloWorldPubSubType> writer("HelloWorldTopic_openssl_is_correctly_finished");
5198+
PubSubReader<HelloWorldPubSubType> reader("HelloWorldTopic_openssl_is_correctly_finished");
5199+
5200+
const std::string governance_file("governance_helloworld_all_enable.smime");
5201+
const std::string permissions_file("permissions_helloworld.smime");
5202+
5203+
CommonPermissionsConfigure(reader, writer, governance_file, permissions_file);
5204+
5205+
reader.init();
5206+
writer.init();
5207+
5208+
ASSERT_TRUE(reader.isInitialized());
5209+
ASSERT_TRUE(writer.isInitialized());
5210+
5211+
std::this_thread::sleep_for(std::chrono::seconds(2));
5212+
5213+
// Here we force the atexit function from openssl to be abruptly called
5214+
// i.e in a disordered way
5215+
// If OpenSSL is not correctly finished, a SIGSEGV will be thrown
5216+
std::exit(0);
5217+
}
5218+
51925219
void blackbox_security_init()
51935220
{
51945221
certs_path = std::getenv("CERTS_PATH");

0 commit comments

Comments
 (0)