From dbfd1f3b8e3961ffdc0828d584a582c054ffef36 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Tue, 14 Jan 2025 10:32:36 +0100 Subject: [PATCH] Log any errors before cancel_init() (backport #5530) (#5546) * 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) * 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 --------- Signed-off-by: Eugenio Collado Co-authored-by: Santiago <95138114+Santti4go@users.noreply.github.com> Co-authored-by: EugenioCollado <121509066+EugenioCollado@users.noreply.github.com> --- src/cpp/rtps/security/SecurityManager.cpp | 9 +++- .../security/SecurityInitializationTests.cpp | 42 +++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/cpp/rtps/security/SecurityManager.cpp b/src/cpp/rtps/security/SecurityManager.cpp index 73de7106165..581b961b1dd 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 + EPROSIMA_LOG_ERROR(SECURITY, "Error while configuring security plugin."); + if (0 != strlen(exception.what())) + { + EPROSIMA_LOG_ERROR(SECURITY, exception.what()); + } + cancel_init(); return false; } 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); +} +