From 892ca86116e04bcf2d7a655933fa84ea7e8c87ce Mon Sep 17 00:00:00 2001 From: Santiago <95138114+Santti4go@users.noreply.github.com> Date: Wed, 8 Jan 2025 04:43:11 -0300 Subject: [PATCH 1/2] Log any errors before cancel_init() (#5530) * Log any errors before cancel_init() Signed-off-by: Santti4go * Code style Signed-off-by: Santti4go * Apply suggestion Signed-off-by: Santti4go --------- Signed-off-by: Santti4go (cherry picked from commit 165d64e3f44b483b48a27efd531475c78687c840) --- src/cpp/rtps/security/SecurityManager.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/cpp/rtps/security/SecurityManager.cpp b/src/cpp/rtps/security/SecurityManager.cpp index 73de7106165..ae0586a9207 100644 --- a/src/cpp/rtps/security/SecurityManager.cpp +++ b/src/cpp/rtps/security/SecurityManager.cpp @@ -105,9 +105,9 @@ bool SecurityManager::init( ParticipantSecurityAttributes& attributes, const PropertyPolicy& participant_properties) { + SecurityException exception; try { - SecurityException exception; domain_id_ = participant_->get_domain_id(); const PropertyPolicy log_properties = PropertyPolicyHelper::get_properties_with_prefix( participant_->getRTPSParticipantAttributes().properties, @@ -383,6 +383,13 @@ bool SecurityManager::init( { if (!e) { + // Unexpected code path. Let's log any errors + logError(SECURITY, "Error while configuring security plugin.") + if (0 != strlen(exception.what())) + { + logError(SECURITY, exception.what()) + } + cancel_init(); return false; } From 1a92af2ffecc85035858963012bde8d1a5ca81c0 Mon Sep 17 00:00:00 2001 From: EugenioCollado <121509066+EugenioCollado@users.noreply.github.com> Date: Mon, 13 Jan 2025 07:30:07 +0100 Subject: [PATCH 2/2] Add test for security initialization error (#5550) * Regression tests Signed-off-by: Eugenio Collado * Update log macro Signed-off-by: Eugenio Collado * Uncrustify Signed-off-by: Eugenio Collado * Fix CI log flush Signed-off-by: Eugenio Collado --------- Signed-off-by: Eugenio Collado --- src/cpp/rtps/security/SecurityManager.cpp | 4 +- .../security/SecurityInitializationTests.cpp | 42 +++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/src/cpp/rtps/security/SecurityManager.cpp b/src/cpp/rtps/security/SecurityManager.cpp index ae0586a9207..581b961b1dd 100644 --- a/src/cpp/rtps/security/SecurityManager.cpp +++ b/src/cpp/rtps/security/SecurityManager.cpp @@ -384,10 +384,10 @@ bool SecurityManager::init( if (!e) { // Unexpected code path. Let's log any errors - logError(SECURITY, "Error while configuring security plugin.") + EPROSIMA_LOG_ERROR(SECURITY, "Error while configuring security plugin."); if (0 != strlen(exception.what())) { - logError(SECURITY, exception.what()) + EPROSIMA_LOG_ERROR(SECURITY, exception.what()); } cancel_init(); diff --git a/test/unittest/rtps/security/SecurityInitializationTests.cpp b/test/unittest/rtps/security/SecurityInitializationTests.cpp index 60dfb537fa6..74669bd3aeb 100644 --- a/test/unittest/rtps/security/SecurityInitializationTests.cpp +++ b/test/unittest/rtps/security/SecurityInitializationTests.cpp @@ -14,6 +14,8 @@ #include "SecurityTests.hpp" +#include "../../logging/mock/MockConsumer.h" + const char* const MockIdentity::class_id_ = "MockIdentityHandle"; const char* const MockHandshake::class_id_ = "MockHandshakeHandle"; const char* const SharedSecret::class_id_ = "SharedSecretHandle"; @@ -208,3 +210,43 @@ TEST_F(SecurityTest, initialization_ok) } +/* Regression test for Redmine 22545. + * + * Triggering a throw false in SecurityManager::init() should be logged properly as + * the error: "Error while configuring security plugin.". + */ +TEST_F(SecurityTest, initialization_logging_error) +{ + DefaultValue::Set(guid); + DefaultValue::Set(security_attributes_); + + EXPECT_CALL(*auth_plugin_, validate_local_identity(_, _, _, _, _, _)).Times(1). + WillOnce(DoAll(SetArgPointee<0>(&local_identity_handle_), Return(ValidationResult_t::VALIDATION_OK))); + EXPECT_CALL(crypto_plugin_->cryptokeyfactory_, + register_local_participant(Ref(local_identity_handle_), _, _, _, _)).Times(1). + WillOnce(Return(nullptr)); + + eprosima::fastdds::dds::MockConsumer* mockConsumer = new eprosima::fastdds::dds::MockConsumer(); + eprosima::fastdds::dds::Log::RegisterConsumer(std::unique_ptr(mockConsumer)); + eprosima::fastdds::dds::Log::SetVerbosity(eprosima::fastdds::dds::Log::Error); + + security_activated_ = manager_.init(security_attributes_, participant_properties_); + + // Check that the error message was logged. + // First flush the log to make sure the message is there. + eprosima::fastdds::dds::Log::Flush(); + + auto log_entries = mockConsumer->ConsumedEntries(); + ASSERT_GE(log_entries.size(), 1); + bool found = false; + for (auto entry : log_entries) + { + if (entry.message.find("Error while configuring security plugin.") != std::string::npos) + { + found = true; + break; + } + } + ASSERT_TRUE(found); +} +