From c19274448ad5d6e5511559ff9d7c7b8b74eb81cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Dom=C3=ADnguez=20L=C3=B3pez?= <116071334+Mario-DL@users.noreply.github.com> Date: Fri, 1 Mar 2024 07:52:36 +0100 Subject: [PATCH 1/2] Expose Authentication Handshake Properties (#4435) * Refs #20439: Blackbox tests Signed-off-by: Mario Dominguez * Refs #20439: Add new get_property() api method in PropertyPolicy Signed-off-by: Mario Dominguez * Refs #20439: Add new PropertyParser module in Property.h Signed-off-by: Mario Dominguez * Refs #20439: Integrate AuthenticationHandshakeProperties in Security Manager Signed-off-by: Mario Dominguez * Refs #20439: Linter Signed-off-by: Mario Dominguez * Refs #20439: Fix elapsed time test comparison Signed-off-by: Mario Dominguez * Refs #20439: versions.md Signed-off-by: Mario Dominguez * Refs #20439: Review suggestions Signed-off-by: Mario Dominguez * Refs #20439: Make ints size explicit. Leave it as 32 to cover all the range of stoi() Signed-off-by: Mario Dominguez --------- Signed-off-by: Mario Dominguez (cherry picked from commit da58d84ae9a4d5b802db07f8b46a6e571e69c2dc) --- .../fastdds/rtps/attributes/PropertyPolicy.h | 191 ++++++++++-------- include/fastdds/rtps/common/Property.h | 126 ++++++++++++ src/cpp/rtps/attributes/PropertyPolicy.cpp | 50 ++++- src/cpp/rtps/security/SecurityManager.cpp | 72 ++++++- src/cpp/rtps/security/SecurityManager.h | 42 +++- test/blackbox/CMakeLists.txt | 2 + .../api/fastrtps_deprecated/PubSubReader.hpp | 2 + .../api/fastrtps_deprecated/PubSubWriter.hpp | 2 + .../blackbox/auth_handshake_props_profile.xml | 57 ++++++ .../blackbox/common/BlackboxTestsSecurity.cpp | 111 ++++++++++ versions.md | 11 +- 11 files changed, 560 insertions(+), 106 deletions(-) create mode 100644 test/blackbox/auth_handshake_props_profile.xml diff --git a/include/fastdds/rtps/attributes/PropertyPolicy.h b/include/fastdds/rtps/attributes/PropertyPolicy.h index 5150a46ebdf..92f78e9c769 100644 --- a/include/fastdds/rtps/attributes/PropertyPolicy.h +++ b/include/fastdds/rtps/attributes/PropertyPolicy.h @@ -28,93 +28,118 @@ namespace rtps { class PropertyPolicy { - public: - RTPS_DllAPI PropertyPolicy() {} - - RTPS_DllAPI PropertyPolicy(const PropertyPolicy& property_policy) : - properties_(property_policy.properties_), - binary_properties_(property_policy.binary_properties_) {} - - RTPS_DllAPI PropertyPolicy(PropertyPolicy&& property_policy) : - properties_(std::move(property_policy.properties_)), - binary_properties_(std::move(property_policy.binary_properties_)) {} - - RTPS_DllAPI PropertyPolicy& operator=(const PropertyPolicy& property_policy) - { - properties_ = property_policy.properties_; - binary_properties_ = property_policy.binary_properties_; - return *this; - } - - RTPS_DllAPI PropertyPolicy& operator=(PropertyPolicy&& property_policy) - { - properties_ = std::move(property_policy.properties_); - binary_properties_= std::move(property_policy.binary_properties_); - return *this; - } - - RTPS_DllAPI bool operator==(const PropertyPolicy& b) const - { - return (this->properties_ == b.properties_) && - (this->binary_properties_ == b.binary_properties_); - } - - //!Get properties - RTPS_DllAPI const PropertySeq& properties() const - { - return properties_; - } - - //!Set properties - RTPS_DllAPI PropertySeq& properties() - { - return properties_; - } - - //!Get binary_properties - RTPS_DllAPI const BinaryPropertySeq& binary_properties() const - { - return binary_properties_; - } - - //!Set binary_properties - RTPS_DllAPI BinaryPropertySeq& binary_properties() - { - return binary_properties_; - } - - private: - PropertySeq properties_; - - BinaryPropertySeq binary_properties_; +public: + + RTPS_DllAPI PropertyPolicy() + { + } + + RTPS_DllAPI PropertyPolicy( + const PropertyPolicy& property_policy) + : properties_(property_policy.properties_) + , binary_properties_(property_policy.binary_properties_) + { + } + + RTPS_DllAPI PropertyPolicy( + PropertyPolicy&& property_policy) + : properties_(std::move(property_policy.properties_)) + , binary_properties_(std::move(property_policy.binary_properties_)) + { + } + + RTPS_DllAPI PropertyPolicy& operator =( + const PropertyPolicy& property_policy) + { + properties_ = property_policy.properties_; + binary_properties_ = property_policy.binary_properties_; + return *this; + } + + RTPS_DllAPI PropertyPolicy& operator =( + PropertyPolicy&& property_policy) + { + properties_ = std::move(property_policy.properties_); + binary_properties_ = std::move(property_policy.binary_properties_); + return *this; + } + + RTPS_DllAPI bool operator ==( + const PropertyPolicy& b) const + { + return (this->properties_ == b.properties_) && + (this->binary_properties_ == b.binary_properties_); + } + + //!Get properties + RTPS_DllAPI const PropertySeq& properties() const + { + return properties_; + } + + //!Set properties + RTPS_DllAPI PropertySeq& properties() + { + return properties_; + } + + //!Get binary_properties + RTPS_DllAPI const BinaryPropertySeq& binary_properties() const + { + return binary_properties_; + } + + //!Set binary_properties + RTPS_DllAPI BinaryPropertySeq& binary_properties() + { + return binary_properties_; + } + +private: + + PropertySeq properties_; + + BinaryPropertySeq binary_properties_; }; class PropertyPolicyHelper { - public: - /*! - * @brief Returns only the properties whose name starts with the prefix. - * Prefix is removed in returned properties. - * @param property_policy PropertyPolicy where properties will be searched. - * @param prefix Prefix used to search properties. - * @return A copy of properties whose name starts with the prefix. - */ - RTPS_DllAPI static PropertyPolicy get_properties_with_prefix( - const PropertyPolicy& property_policy, - const std::string& prefix); - - //!Get the length of the property_policy - RTPS_DllAPI static size_t length(const PropertyPolicy& property_policy); - - //!Look for a property_policy by name - RTPS_DllAPI static std::string* find_property( - PropertyPolicy& property_policy, - const std::string& name); - - //!Retrieves a property_policy by name - RTPS_DllAPI static const std::string* find_property( - const PropertyPolicy& property_policy, - const std::string& name); +public: + + /*! + * @brief Returns only the properties whose name starts with the prefix. + * Prefix is removed in returned properties. + * @param property_policy PropertyPolicy where properties will be searched. + * @param prefix Prefix used to search properties. + * @return A copy of properties whose name starts with the prefix. + */ + RTPS_DllAPI static PropertyPolicy get_properties_with_prefix( + const PropertyPolicy& property_policy, + const std::string& prefix); + + //!Get the length of the property_policy + RTPS_DllAPI static size_t length( + const PropertyPolicy& property_policy); + + //!Look for a property_policy by name + RTPS_DllAPI static std::string* find_property( + PropertyPolicy& property_policy, + const std::string& name); + + //!Retrieves a property_policy by name + RTPS_DllAPI static const std::string* find_property( + const PropertyPolicy& property_policy, + const std::string& name); + + /** + * @brief Retrieves a property by name + * @param property_policy PropertyPolicy where the property will be searched. + * @param name Name of the property to be searched. + * @return A pointer to the property if found, nullptr otherwise. + */ + RTPS_DllAPI static const Property* get_property( + const PropertyPolicy& property_policy, + const std::string& name); }; } //namespace rtps diff --git a/include/fastdds/rtps/common/Property.h b/include/fastdds/rtps/common/Property.h index fa10981e2b9..ed1c0673069 100644 --- a/include/fastdds/rtps/common/Property.h +++ b/include/fastdds/rtps/common/Property.h @@ -18,9 +18,13 @@ #ifndef _FASTDDS_RTPS_COMMON_PROPERTYQOS_H_ #define _FASTDDS_RTPS_COMMON_PROPERTYQOS_H_ +#include +#include #include #include +#include + namespace eprosima { namespace fastrtps { namespace rtps { @@ -214,6 +218,128 @@ class PropertyHelper }; +struct PropertyParser +{ + + /** + * @brief Parse a property value as an integer + * @param property Property to parse + * @param check_upper_bound If true, check that the value is lower than upper_bound + * @param upper_bound Upper bound to check + * @param check_lower_bound If true, check that the value is greater than lower_bound + * @param lower_bound Lower bound to check + * @param exception Exception to throw if the value is not a valid integer or if it is out of bounds + * @return The parsed integer value + * + * @warning May throw an exception_t if the value is not a valid integer + * or if it is out of bounds. + */ + template + inline static int as_int( + const Property& property, + const bool& check_upper_bound, + const int& upper_bound, + const bool& check_lower_bound, + const int& lower_bound, + const exception_t& exception) + { + return parse_value( + std::function( + [](const Property& property) + { + return std::stoi(property.value()); + } + ), + property, + check_upper_bound, + upper_bound, + check_lower_bound, + lower_bound, + exception); + } + + /** + * @brief Parse a property value as a double + * @param property Property to parse + * @param check_upper_bound If true, check that the value is lower than upper_bound + * @param upper_bound Upper bound to check + * @param check_lower_bound If true, check that the value is greater than lower_bound + * @param lower_bound Lower bound to check + * @param exception Exception to throw if the value is not a valid double or if it is out of bounds + * @return The parsed double value + * + * @warning May throw an exception_t if the value is not a valid double + * or if it is out of bounds. + */ + template + inline static double as_double( + const Property& property, + const bool& check_upper_bound, + const double& upper_bound, + const bool& check_lower_bound, + const double& lower_bound, + const exception_t& exception) + { + return parse_value( + std::function( + [](const Property& property) + { + return std::stod(property.value()); + } + ), + property, + check_upper_bound, + upper_bound, + check_lower_bound, + lower_bound, + exception); + } + +private: + + template + inline static value_t parse_value( + const std::function& conversor, + const Property& property, + const bool& check_upper_bound, + const value_t& upper_bound, + const bool& check_lower_bound, + const value_t& lower_bound, + const exception_t& exception) + { + try + { + value_t converted_value = conversor(property); + + if (check_lower_bound && converted_value < lower_bound) + { + throw exception_t("Value '" + property.value() + + "' for " + property.name() + " must be greater or equal to " + + std::to_string(lower_bound)); + } + + if (check_upper_bound && converted_value > upper_bound) + { + throw exception_t("Value '" + property.value() + + "' for " + property.name() + " must be lower or equal to " + + std::to_string(upper_bound)); + } + + return converted_value; + } + catch (const std::invalid_argument&) + { + throw exception; + } + catch (const std::out_of_range&) + { + throw exception; + } + } + +}; + } //namespace eprosima } //namespace fastrtps } //namespace rtps diff --git a/src/cpp/rtps/attributes/PropertyPolicy.cpp b/src/cpp/rtps/attributes/PropertyPolicy.cpp index f3d98367baf..4f4a8e9819e 100644 --- a/src/cpp/rtps/attributes/PropertyPolicy.cpp +++ b/src/cpp/rtps/attributes/PropertyPolicy.cpp @@ -22,7 +22,9 @@ using namespace eprosima::fastrtps::rtps; -PropertyPolicy PropertyPolicyHelper::get_properties_with_prefix(const PropertyPolicy& property_policy, const std::string& prefix) +PropertyPolicy PropertyPolicyHelper::get_properties_with_prefix( + const PropertyPolicy& property_policy, + const std::string& prefix) { PropertyPolicy returned_property_policy; @@ -30,7 +32,7 @@ PropertyPolicy PropertyPolicyHelper::get_properties_with_prefix(const PropertyPo std::for_each(property_policy.properties().begin(), property_policy.properties().end(), [&returned_property_policy, &prefix](const Property& property) { - if(property.name().compare(0, prefix.size(), prefix) == 0) + if (property.name().compare(0, prefix.size(), prefix) == 0) { Property new_property(property); new_property.name().erase(0, prefix.size()); @@ -42,7 +44,7 @@ PropertyPolicy PropertyPolicyHelper::get_properties_with_prefix(const PropertyPo std::for_each(property_policy.binary_properties().begin(), property_policy.binary_properties().end(), [&returned_property_policy, &prefix](const BinaryProperty& property) { - if(property.name().compare(0, prefix.size(), prefix) == 0) + if (property.name().compare(0, prefix.size(), prefix) == 0) { BinaryProperty new_property(property); new_property.name().erase(0, prefix.size()); @@ -53,19 +55,23 @@ PropertyPolicy PropertyPolicyHelper::get_properties_with_prefix(const PropertyPo return returned_property_policy; } -size_t PropertyPolicyHelper::length(const PropertyPolicy& property_policy) +size_t PropertyPolicyHelper::length( + const PropertyPolicy& property_policy) { return property_policy.properties().size() + - property_policy.binary_properties().size(); + property_policy.binary_properties().size(); } -std::string* PropertyPolicyHelper::find_property(PropertyPolicy& property_policy, const std::string& name) +std::string* PropertyPolicyHelper::find_property( + PropertyPolicy& property_policy, + const std::string& name) { std::string* returnedValue = nullptr; - for(auto property = property_policy.properties().begin(); property != property_policy.properties().end(); ++property) + for (auto property = property_policy.properties().begin(); property != property_policy.properties().end(); + ++property) { - if(property->name().compare(name) == 0) + if (property->name().compare(name) == 0) { returnedValue = &property->value(); break; @@ -75,13 +81,16 @@ std::string* PropertyPolicyHelper::find_property(PropertyPolicy& property_policy return returnedValue; } -const std::string* PropertyPolicyHelper::find_property(const PropertyPolicy& property_policy, const std::string& name) +const std::string* PropertyPolicyHelper::find_property( + const PropertyPolicy& property_policy, + const std::string& name) { const std::string* returnedValue = nullptr; - for(auto property = property_policy.properties().begin(); property != property_policy.properties().end(); ++property) + for (auto property = property_policy.properties().begin(); property != property_policy.properties().end(); + ++property) { - if(property->name().compare(name) == 0) + if (property->name().compare(name) == 0) { returnedValue = &property->value(); break; @@ -90,3 +99,22 @@ const std::string* PropertyPolicyHelper::find_property(const PropertyPolicy& pro return returnedValue; } + +const Property* PropertyPolicyHelper::get_property( + const PropertyPolicy& property_policy, + const std::string& name) +{ + const Property* returnedValue = nullptr; + + for (auto property = property_policy.properties().begin(); property != property_policy.properties().end(); + ++property) + { + if (property->name().compare(name) == 0) + { + returnedValue = &*property; + break; + } + } + + return returnedValue; +} diff --git a/src/cpp/rtps/security/SecurityManager.cpp b/src/cpp/rtps/security/SecurityManager.cpp index 4ad1fafd943..3fce99cf785 100644 --- a/src/cpp/rtps/security/SecurityManager.cpp +++ b/src/cpp/rtps/security/SecurityManager.cpp @@ -210,6 +210,17 @@ bool SecurityManager::init( if (authentication_plugin_ != nullptr) { + // retrieve authentication properties, if any + const PropertyPolicy auth_handshake_properties = PropertyPolicyHelper::get_properties_with_prefix( + participant_->getRTPSParticipantAttributes().properties, + "dds.sec.auth.builtin.PKI-DH."); + + // if auth_handshake_properties is empty, the default values are used + if (PropertyPolicyHelper::length(auth_handshake_properties) > 0) + { + auth_handshake_props_.parse_from_property_policy(auth_handshake_properties); + } + authentication_plugin_->set_logger(logging_plugin_, exception); // Validate local participant @@ -630,7 +641,7 @@ bool SecurityManager::discovered_participant( resend_handshake_message_token(guid); return true; }, - DiscoveredParticipantInfo::INITIAL_RESEND_HANDSHAKE_MILLISECS)); // TODO (Ricardo) Configurable + static_cast(auth_handshake_props_.initial_handshake_resend_period_ms_))); IdentityHandle* remote_identity_handle = nullptr; @@ -932,6 +943,7 @@ bool SecurityManager::on_process_handshake( handshake_message_send = true; expected_sequence_number = message.message_identity().sequence_number(); remote_participant_info->change_sequence_number_ = change->sequenceNumber; + remote_participant_info->handshake_requests_sent_++; } else { @@ -996,7 +1008,8 @@ bool SecurityManager::on_process_handshake( remote_participant_info->expected_sequence_number_ = expected_sequence_number; // Avoid DoS attack by exponentially increasing event interval auto time_ms = remote_participant_info->event_->getIntervalMilliSec(); - remote_participant_info->event_->update_interval_millisec(time_ms * 2); + remote_participant_info->event_->update_interval_millisec( + time_ms * auth_handshake_props_.handshake_resend_period_gain_); remote_participant_info->event_->restart_timer(); } @@ -1538,6 +1551,7 @@ void SecurityManager::process_participant_stateless_message( if (participant_stateless_message_writer_history_->add_change(p_change)) { remote_participant_info->change_sequence_number_ = p_change->sequenceNumber; + remote_participant_info->handshake_requests_sent_++; } //TODO (Ricardo) What to do if not added? } @@ -1610,6 +1624,7 @@ void SecurityManager::process_participant_stateless_message( if (participant_stateless_message_writer_history_->add_change(p_change)) { remote_participant_info->change_sequence_number_ = p_change->sequenceNumber; + remote_participant_info->handshake_requests_sent_++; } //TODO (Ricardo) What to do if not added? } @@ -4199,7 +4214,7 @@ void SecurityManager::resend_handshake_message_token( if (remote_participant_info) { - if (remote_participant_info->handshake_requests_sent_ >= DiscoveredParticipantInfo::MAX_HANDSHAKE_REQUESTS) + if (remote_participant_info->handshake_requests_sent_ >= auth_handshake_props_.max_handshake_requests_) { if (remote_participant_info->auth_status_ != AUTHENTICATION_FAILED) { @@ -4234,7 +4249,8 @@ void SecurityManager::resend_handshake_message_token( { // Avoid DoS attack by exponentially increasing event interval auto time_ms = remote_participant_info->event_->getIntervalMilliSec(); - remote_participant_info->event_->update_interval_millisec(time_ms * 2); + remote_participant_info->event_->update_interval_millisec( + time_ms * auth_handshake_props_.handshake_resend_period_gain_); remote_participant_info->event_->restart_timer(); } } @@ -4272,6 +4288,54 @@ void SecurityManager::on_validation_failed( } } +SecurityManager::AuthenticationHandshakeProperties::AuthenticationHandshakeProperties() + : max_handshake_requests_(10) + , initial_handshake_resend_period_ms_(125) + , handshake_resend_period_gain_(1.5) +{ + +} + +void SecurityManager::AuthenticationHandshakeProperties::parse_from_property_policy( + const PropertyPolicy& auth_handshake_properties) +{ + const Property* const max_handshake_requests = + PropertyPolicyHelper::get_property(auth_handshake_properties, "max_handshake_requests"); + + if (max_handshake_requests != nullptr) + { + max_handshake_requests_ = (int32_t)PropertyParser::as_int( + *max_handshake_requests, + false, 0, + true, 1, + SecurityException("Error parsing max_handshake_requests property value.")); + } + + const Property* const initial_handshake_resend_period = + PropertyPolicyHelper::get_property(auth_handshake_properties, "initial_handshake_resend_period"); + + if (initial_handshake_resend_period != nullptr) + { + initial_handshake_resend_period_ms_ = (int32_t)PropertyParser::as_int( + *initial_handshake_resend_period, + false, 0, + true, 1, + SecurityException("Error parsing initial_handshake_resend_period property value.")); + } + + const Property* const handshake_resend_period_gain = + PropertyPolicyHelper::get_property(auth_handshake_properties, "handshake_resend_period_gain"); + + if (handshake_resend_period_gain != nullptr) + { + handshake_resend_period_gain_ = PropertyParser::as_double( + *handshake_resend_period_gain, + false, 0.0, + true, 1.0, + SecurityException("Error parsing handshake_resend_period_gain property value.")); + } +} + bool SecurityManager::DiscoveredParticipantInfo::check_guid_comes_from( Authentication* const auth_plugin, const GUID_t& adjusted, diff --git a/src/cpp/rtps/security/SecurityManager.h b/src/cpp/rtps/security/SecurityManager.h index 26eeb984c98..8572670575e 100644 --- a/src/cpp/rtps/security/SecurityManager.h +++ b/src/cpp/rtps/security/SecurityManager.h @@ -354,6 +354,30 @@ class SecurityManager AUTHENTICATION_NOT_AVAILABLE }; + struct AuthenticationHandshakeProperties + { + AuthenticationHandshakeProperties(); + ~AuthenticationHandshakeProperties() = default; + + /** + * @brief Parses the properties from a propertypolicy rule + * @param properties PropertyPolicy reference to the properties to parse + */ + void parse_from_property_policy( + const PropertyPolicy& properties); + + // Maximum number of handshake requests to be sent + // Must be greater than 0 + int32_t max_handshake_requests_; + // Initial wait time (in milliseconds) for the first handshake request resend + // Must be greater than 0 + int32_t initial_handshake_resend_period_ms_; + // Gain for the period between handshake request resends + // The initial period is multiplied by this value each time a resend is performed + // Must be greater than 1 + double handshake_resend_period_gain_; + }; + class DiscoveredParticipantInfo { struct AuthenticationInfo @@ -364,12 +388,13 @@ class SecurityManager AuthenticationInfo( AuthenticationStatus auth_status) - : identity_handle_(nullptr) + : handshake_requests_sent_(0) + , identity_handle_(nullptr) , handshake_handle_(nullptr) , auth_status_(auth_status) , expected_sequence_number_(0) , change_sequence_number_(SequenceNumber_t::unknown()) - , handshake_requests_sent_(0) + { } @@ -384,6 +409,8 @@ class SecurityManager { } + int32_t handshake_requests_sent_; + IdentityHandle* identity_handle_; HandshakeHandle* handshake_handle_; @@ -396,8 +423,6 @@ class SecurityManager EventUniquePtr event_; - uint32_t handshake_requests_sent_; - private: AuthenticationInfo( @@ -408,9 +433,6 @@ class SecurityManager typedef std::unique_ptr AuthUniquePtr; - static constexpr uint32_t INITIAL_RESEND_HANDSHAKE_MILLISECS = 125; - static constexpr uint32_t MAX_HANDSHAKE_REQUESTS = 5; - DiscoveredParticipantInfo( AuthenticationStatus auth_status, const ParticipantProxyData& participant_data) @@ -784,7 +806,13 @@ class SecurityManager Authentication* authentication_plugin_; +<<<<<<< HEAD AccessControl* access_plugin_; +======= + AuthenticationHandshakeProperties auth_handshake_props_; + + IdentityHandle* local_identity_handle_ = nullptr; +>>>>>>> da58d84ae (Expose Authentication Handshake Properties (#4435)) Cryptography* crypto_plugin_; diff --git a/test/blackbox/CMakeLists.txt b/test/blackbox/CMakeLists.txt index 5f4ae17f5fc..11d396d5686 100644 --- a/test/blackbox/CMakeLists.txt +++ b/test/blackbox/CMakeLists.txt @@ -358,6 +358,8 @@ configure_file(${CMAKE_CURRENT_SOURCE_DIR}/partitions_profile.xml ${CMAKE_CURRENT_BINARY_DIR}/partitions_profile.xml) configure_file(${CMAKE_CURRENT_SOURCE_DIR}/builtin_transports_profile.xml ${CMAKE_CURRENT_BINARY_DIR}/builtin_transports_profile.xml) +configure_file(${CMAKE_CURRENT_SOURCE_DIR}/auth_handshake_props_profile.xml + ${CMAKE_CURRENT_BINARY_DIR}/auth_handshake_props_profile.xml COPYONLY) file(COPY "${CMAKE_CURRENT_SOURCE_DIR}/datagrams" DESTINATION "${CMAKE_CURRENT_BINARY_DIR}") if(FASTRTPS_API_TESTS) diff --git a/test/blackbox/api/fastrtps_deprecated/PubSubReader.hpp b/test/blackbox/api/fastrtps_deprecated/PubSubReader.hpp index 3ec3f834d4e..9eafaab8da2 100644 --- a/test/blackbox/api/fastrtps_deprecated/PubSubReader.hpp +++ b/test/blackbox/api/fastrtps_deprecated/PubSubReader.hpp @@ -353,6 +353,8 @@ class PubSubReader eprosima::fastrtps::Domain::removeParticipant(participant_); participant_ = nullptr; } + + initialized_ = false; } std::list data_not_received() diff --git a/test/blackbox/api/fastrtps_deprecated/PubSubWriter.hpp b/test/blackbox/api/fastrtps_deprecated/PubSubWriter.hpp index 33f6e857175..cf7fbe4f3b6 100644 --- a/test/blackbox/api/fastrtps_deprecated/PubSubWriter.hpp +++ b/test/blackbox/api/fastrtps_deprecated/PubSubWriter.hpp @@ -381,6 +381,8 @@ class PubSubWriter eprosima::fastrtps::Domain::removeParticipant(participant_); participant_ = nullptr; } + + initialized_ = false; } void send( diff --git a/test/blackbox/auth_handshake_props_profile.xml b/test/blackbox/auth_handshake_props_profile.xml new file mode 100644 index 00000000000..7cb8d4b1421 --- /dev/null +++ b/test/blackbox/auth_handshake_props_profile.xml @@ -0,0 +1,57 @@ + + + + + 0 + + + + + + dds.sec.auth.plugin + builtin.PKI-DH + + + + dds.sec.auth.builtin.PKI-DH.identity_ca + file://${CERTS_PATH}/maincacert.pem + + + dds.sec.auth.builtin.PKI-DH.identity_certificate + file://${CERTS_PATH}/mainpubcert.pem + + + dds.sec.auth.builtin.PKI-DH.private_key + file://${CERTS_PATH}/mainpubkey.pem + + + + dds.sec.crypto.plugin + builtin.AES-GCM-GMAC + + + dds.sec.access.builtin.Access-Permissions.governance + file://${CERTS_PATH}/governance_helloworld_all_enable.smime + + + dds.sec.access.builtin.Access-Permissions.permissions + file://${CERTS_PATH}/permissions_helloworld.smime + + + dds.sec.auth.builtin.PKI-DH.max_handshake_requests + 10 + + + dds.sec.auth.builtin.PKI-DH.initial_handshake_resend_period + 100 + + + dds.sec.auth.builtin.PKI-DH.handshake_resend_period_gain + 1.1 + + + + + + + diff --git a/test/blackbox/common/BlackboxTestsSecurity.cpp b/test/blackbox/common/BlackboxTestsSecurity.cpp index 3d2378b5a81..2d7b812bfec 100644 --- a/test/blackbox/common/BlackboxTestsSecurity.cpp +++ b/test/blackbox/common/BlackboxTestsSecurity.cpp @@ -4817,6 +4817,117 @@ TEST_P(Security, MaliciousParticipantRemovalIgnore) reader.block_for_all(); } +TEST(Security, ValidateAuthenticationHandshakePropertiesParsing) +{ + PubSubWriter writer(TEST_TOPIC_NAME); + + PropertyPolicy property_policy; + + property_policy.properties().emplace_back(Property("dds.sec.auth.plugin", + "builtin.PKI-DH")); + property_policy.properties().emplace_back(Property("dds.sec.auth.builtin.PKI-DH.identity_ca", + "file://" + std::string(certs_path) + "/maincacert.pem")); + property_policy.properties().emplace_back(Property("dds.sec.auth.builtin.PKI-DH.identity_certificate", + "file://" + std::string(certs_path) + "/mainsubcert.pem")); + property_policy.properties().emplace_back(Property("dds.sec.auth.builtin.PKI-DH.private_key", + "file://" + std::string(certs_path) + "/mainsubkey.pem")); + property_policy.properties().emplace_back(Property("dds.sec.crypto.plugin", + "builtin.AES-GCM-GMAC")); + + // max_handshake_requests out of bounds + property_policy.properties().emplace_back(Property("dds.sec.auth.builtin.PKI-DH.max_handshake_requests", + "0")); + + writer.property_policy(property_policy).init(); + + // Writer creation should fail + ASSERT_FALSE(writer.isInitialized()); + + writer.destroy(); + + property_policy.properties().pop_back(); + + // initial_handshake_resend_period out of bounds + property_policy.properties().emplace_back(Property("dds.sec.auth.builtin.PKI-DH.initial_handshake_resend_period", + "-200")); + + writer.property_policy(property_policy).init(); + + // Writer creation should fail + ASSERT_FALSE(writer.isInitialized()); + + writer.destroy(); + + property_policy.properties().pop_back(); + + // handshake_resend_period_gain out of bounds + property_policy.properties().emplace_back(Property("dds.sec.auth.builtin.PKI-DH.handshake_resend_period_gain", + "0.5")); + + writer.property_policy(property_policy).init(); + + // Writer creation should fail + ASSERT_FALSE(writer.isInitialized()); + + writer.destroy(); + + property_policy.properties().pop_back(); + + property_policy.properties().emplace_back(Property("dds.sec.auth.builtin.PKI-DH.max_handshake_requests", + "5")); + property_policy.properties().emplace_back(Property("dds.sec.auth.builtin.PKI-DH.initial_handshake_resend_period", + "200")); + property_policy.properties().emplace_back(Property("dds.sec.auth.builtin.PKI-DH.handshake_resend_period_gain", + "1.75")); + + writer.property_policy(property_policy).init(); + + // Writer should correctly initialize + ASSERT_TRUE(writer.isInitialized()); +} + +TEST(Security, ValidateAuthenticationHandshakeProperties) +{ + // Create + PubSubReader reader(TEST_TOPIC_NAME); + PubSubWriter writer(TEST_TOPIC_NAME); + + PropertyPolicy property_policy; + std::string xml_file = "auth_handshake_props_profile.xml"; + std::string profile_name = "auth_handshake_props"; + + // Set a configuration that makes participant authentication + // to be performed quickly so that we receive handshake + // in 0.15 secs approx + writer.set_xml_filename(xml_file); + writer.set_participant_profile(profile_name); + + reader.set_xml_filename(xml_file); + reader.set_participant_profile(profile_name); + + reader.init(); + ASSERT_TRUE(reader.isInitialized()); + + writer.init(); + ASSERT_TRUE(writer.isInitialized()); + + // If the settings were correctly applied + // we expect to be authorized in less than 0.5 seconds + // In reality, this time could be 0.2 perfectly, + // but some padding is left because of the ci + // or slower platforms + std::chrono::duration max_time(500); + auto t0 = std::chrono::steady_clock::now(); + reader.waitAuthorized(); + auto auth_elapsed_time = std::chrono::duration( + std::chrono::steady_clock::now() - t0); + + // Both should be authorized + writer.waitAuthorized(); + + ASSERT_TRUE(auth_elapsed_time < max_time); +} + void blackbox_security_init() { diff --git a/versions.md b/versions.md index 9ce9b0f94a1..9fa1f854460 100644 --- a/versions.md +++ b/versions.md @@ -1,12 +1,21 @@ Forthcoming ----------- -* Define "super-client" by environment variable. +* Added authentication handshake properties. + +Version 2.13.0 +-------------- + +* Enable configuration of thread setting for all threads. +* Added monitor service feature. +* Added the possibility to define interfaces in the whitelist by interface name. +* Enable support for DataRepresentationQos to select the CDR encoding. * Added the possibility to define a listening port equal to 0 in TCP Transport * Added support for TCP to Fast DDS CLI and environment variable * Added configuration of builtin transports through DomainParticipantQos, environment variable and XML. * Added `non_blocking_send` to TCP Transport. +* Define "super-client" by environment variable. Version 2.12.0 -------------- From b6f658bb50b5ca16c62ad38e9ec3f4c812452428 Mon Sep 17 00:00:00 2001 From: elianalf <62831776+elianalf@users.noreply.github.com> Date: Tue, 12 Mar 2024 16:20:50 +0100 Subject: [PATCH 2/2] Solve conflicts Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com> --- src/cpp/rtps/security/SecurityManager.h | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/cpp/rtps/security/SecurityManager.h b/src/cpp/rtps/security/SecurityManager.h index 8572670575e..aa58cec0e24 100644 --- a/src/cpp/rtps/security/SecurityManager.h +++ b/src/cpp/rtps/security/SecurityManager.h @@ -806,13 +806,9 @@ class SecurityManager Authentication* authentication_plugin_; -<<<<<<< HEAD AccessControl* access_plugin_; -======= - AuthenticationHandshakeProperties auth_handshake_props_; - IdentityHandle* local_identity_handle_ = nullptr; ->>>>>>> da58d84ae (Expose Authentication Handshake Properties (#4435)) + AuthenticationHandshakeProperties auth_handshake_props_; Cryptography* crypto_plugin_;