From 971f120a1caa54e8fb4f3f1a4f48e080e88e9c7b Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Wed, 11 Dec 2024 16:20:33 +0100 Subject: [PATCH] Use correct algorithm strings on PermissionsToken and IdentityToken (#5450) * Refs #19925. Add conversion methods for token algorithms. Signed-off-by: Miguel Company * Refs #19925. Configure legacy transmission on PKIDH. Signed-off-by: Miguel Company * Refs #19925. Configure legacy transmission on Permissions. Signed-off-by: Miguel Company * Refs #19925. Add blackbox test. Signed-off-by: Miguel Company * Refs #19925. Add doxygen documentation to new methods. Signed-off-by: Miguel Company --------- Signed-off-by: Miguel Company --- .../security/accesscontrol/Permissions.cpp | 78 +++++++++++++++++- src/cpp/security/authentication/PKIDH.cpp | 79 +++++++++++++++++-- .../authentication/PKIIdentityHandle.h | 3 + .../blackbox/common/BlackboxTestsSecurity.cpp | 64 +++++++++++++++ 4 files changed, 215 insertions(+), 9 deletions(-) diff --git a/src/cpp/security/accesscontrol/Permissions.cpp b/src/cpp/security/accesscontrol/Permissions.cpp index b73abc8f91d..55d784c3a2f 100644 --- a/src/cpp/security/accesscontrol/Permissions.cpp +++ b/src/cpp/security/accesscontrol/Permissions.cpp @@ -59,6 +59,67 @@ namespace rtps { using namespace security; +/** + * @brief Convert a signature algortihm before adding it to a PermissionsToken. + * + * This methods converts the signature algorithm to the format used in the PermissionsToken. + * Depending on the value of the use_legacy parameter, the algorithm will be converted to the legacy format or to the + * one specified in the DDS-SEC 1.1 specification. + * + * @param algorithm The algorithm to convert. + * @param use_legacy Whether to use the legacy format or not. + * + * @return The converted algorithm. + */ +static std::string convert_to_token_algo( + const std::string& algorithm, + bool use_legacy) +{ + // Leave as internal format when legacy is used + if (use_legacy) + { + return algorithm; + } + + // Convert to token format + if (algorithm == RSA_SHA256) + { + return RSA_SHA256_FOR_TOKENS; + } + else if (algorithm == ECDSA_SHA256) + { + return ECDSA_SHA256_FOR_TOKENS; + } + + return algorithm; +} + +/** + * @brief Parse a signature algorithm from a PermissionsToken. + * + * This method parses a signature algorithm from a PermissionsToken. + * It converts the algorithm to the internal (legacy) format used by the library. + * + * @param algorithm The algorithm to parse. + * + * @return The parsed algorithm. + */ +static std::string parse_token_algo( + const std::string& algorithm) +{ + // Convert to internal format, allowing both legacy and new formats + if (algorithm == RSA_SHA256_FOR_TOKENS) + { + return RSA_SHA256; + } + else if (algorithm == ECDSA_SHA256_FOR_TOKENS) + { + return ECDSA_SHA256; + } + + return algorithm; +} + static bool is_domain_in_set( const uint32_t domain_id, const Domains& domains) @@ -694,7 +755,8 @@ static bool check_subject_name( } static bool generate_permissions_token( - AccessPermissionsHandle& handle) + AccessPermissionsHandle& handle, + bool transmit_legacy_algorithms) { Property property; PermissionsToken& token = handle->permissions_token_; @@ -706,7 +768,7 @@ static bool generate_permissions_token( token.properties().push_back(std::move(property)); property.name("dds.perm_ca.algo"); - property.value() = handle->algo; + property.value() = convert_to_token_algo(handle->algo, transmit_legacy_algorithms); property.propagate(true); token.properties().push_back(std::move(property)); @@ -766,6 +828,13 @@ PermissionsHandle* Permissions::validate_local_permissions( return nullptr; } + bool transmit_legacy_algorithms = false; + std::string* legacy = PropertyPolicyHelper::find_property(access_properties, "transmit_algorithms_as_legacy"); + if (legacy != nullptr) + { + transmit_legacy_algorithms = (*legacy == "true"); + } + std::string* permissions_ca = PropertyPolicyHelper::find_property(access_properties, "permissions_ca"); if (permissions_ca == nullptr) @@ -808,7 +877,7 @@ PermissionsHandle* Permissions::validate_local_permissions( // Check subject name. if (check_subject_name(identity, *ah, domain_id, rules, permissions_data, exception)) { - if (generate_permissions_token(*ah)) + if (generate_permissions_token(*ah, transmit_legacy_algorithms)) { if (generate_credentials_token(*ah, *permissions, exception)) { @@ -942,7 +1011,8 @@ PermissionsHandle* Permissions::validate_remote_permissions( if (algo != nullptr) { - if (algo->compare(lph->algo) != 0) + std::string used_algo = parse_token_algo(*algo); + if (used_algo.compare(lph->algo) != 0) { exception = _SecurityException_("Remote participant PermissionsCA algorithm differs from local"); EMERGENCY_SECURITY_LOGGING("Permissions", exception.what()); diff --git a/src/cpp/security/authentication/PKIDH.cpp b/src/cpp/security/authentication/PKIDH.cpp index 7a09e434063..ee10ff1a4e5 100644 --- a/src/cpp/security/authentication/PKIDH.cpp +++ b/src/cpp/security/authentication/PKIDH.cpp @@ -896,6 +896,67 @@ static bool generate_challenge( return returnedValue; } +/** + * @brief Convert a signature algortihm before adding it to an IdentityToken. + * + * This methods converts the signature algorithm to the format used in the IdentityToken. + * Depending on the value of the use_legacy parameter, the algorithm will be converted to the legacy format or to the + * one specified in the DDS-SEC 1.1 specification. + * + * @param algorithm The algorithm to convert. + * @param use_legacy Whether to use the legacy format or not. + * + * @return The converted algorithm. + */ +static std::string convert_to_token_algo( + const std::string& algorithm, + bool use_legacy) +{ + // Leave as internal format when legacy is used + if (use_legacy) + { + return algorithm; + } + + // Convert to token format + if (algorithm == RSA_SHA256) + { + return RSA_SHA256_FOR_TOKENS; + } + else if (algorithm == ECDSA_SHA256) + { + return ECDSA_SHA256_FOR_TOKENS; + } + + return algorithm; +} + +/** + * @brief Parse a signature algorithm from an IdentityToken. + * + * This method parses the signature algorithm from an IdentityToken. + * It converts the algorithm to the internal (legacy) format used by the library. + * + * @param algorithm The algorithm to parse. + * + * @return The parsed algorithm. + */ +static std::string parse_token_algo( + const std::string& algorithm) +{ + // Convert to internal format, allowing both legacy and new formats + if (algorithm == RSA_SHA256_FOR_TOKENS) + { + return RSA_SHA256; + } + else if (algorithm == ECDSA_SHA256_FOR_TOKENS) + { + return ECDSA_SHA256; + } + + return algorithm; +} + std::shared_ptr PKIDH::generate_sharedsecret( EVP_PKEY* private_key, EVP_PKEY* public_key, @@ -966,7 +1027,8 @@ std::shared_ptr PKIDH::generate_sharedsecret( } static bool generate_identity_token( - PKIIdentityHandle& handle) + PKIIdentityHandle& handle, + bool transmit_legacy_algorithms) { Property property; IdentityToken& token = handle->identity_token_; @@ -978,7 +1040,7 @@ static bool generate_identity_token( token.properties().push_back(std::move(property)); property.name("dds.cert.algo"); - property.value() = handle->sign_alg_; + property.value() = convert_to_token_algo(handle->sign_alg_, transmit_legacy_algorithms); property.propagate(true); token.properties().push_back(std::move(property)); @@ -988,7 +1050,7 @@ static bool generate_identity_token( token.properties().push_back(std::move(property)); property.name("dds.ca.algo"); - property.value() = handle->algo; + property.value() = convert_to_token_algo(handle->algo, transmit_legacy_algorithms); property.propagate(true); token.properties().push_back(std::move(property)); @@ -1015,6 +1077,13 @@ ValidationResult_t PKIDH::validate_local_identity( return ValidationResult_t::VALIDATION_FAILED; } + bool transmit_legacy_algorithms = false; + std::string* legacy = PropertyPolicyHelper::find_property(auth_properties, "transmit_algorithms_as_legacy"); + if (legacy != nullptr) + { + transmit_legacy_algorithms = (*legacy == "true"); + } + std::string* identity_ca = PropertyPolicyHelper::find_property(auth_properties, "identity_ca"); if (identity_ca == nullptr) @@ -1160,7 +1229,7 @@ ValidationResult_t PKIDH::validate_local_identity( adjusted_participant_key, exception)) { // Generate IdentityToken. - if (generate_identity_token(*ih)) + if (generate_identity_token(*ih, transmit_legacy_algorithms)) { (*ih)->participant_key_ = adjusted_participant_key; *local_identity_handle = ih; @@ -1213,7 +1282,7 @@ ValidationResult_t PKIDH::validate_remote_identity( (*rih)->sn = ca_sn ? *ca_sn : ""; (*rih)->cert_sn_ = ""; // cert_sn ? *cert_sn : ""; - (*rih)->algo = cert_algo ? *cert_algo : ""; + (*rih)->algo = cert_algo ? parse_token_algo(*cert_algo) : ""; (*rih)->participant_key_ = remote_participant_key; *remote_identity_handle = rih; diff --git a/src/cpp/security/authentication/PKIIdentityHandle.h b/src/cpp/security/authentication/PKIIdentityHandle.h index 1d1c016d2a6..44c3cc4c805 100644 --- a/src/cpp/security/authentication/PKIIdentityHandle.h +++ b/src/cpp/security/authentication/PKIIdentityHandle.h @@ -33,6 +33,9 @@ namespace security { static const char* const RSA_SHA256 = "RSASSA-PSS-SHA256"; static const char* const ECDSA_SHA256 = "ECDSA-SHA256"; +static const char* const RSA_SHA256_FOR_TOKENS = "RSA-2048"; +static const char* const ECDSA_SHA256_FOR_TOKENS = "EC-prime256v1"; + static const char* const DH_2048_256 = "DH+MODP-2048-256"; static const char* const ECDH_prime256v1 = "ECDH+prime256v1-CEUM"; diff --git a/test/blackbox/common/BlackboxTestsSecurity.cpp b/test/blackbox/common/BlackboxTestsSecurity.cpp index 09d45380cc3..4b582667a03 100644 --- a/test/blackbox/common/BlackboxTestsSecurity.cpp +++ b/test/blackbox/common/BlackboxTestsSecurity.cpp @@ -4548,6 +4548,70 @@ TEST(Security, openssl_correctly_finishes) std::exit(0); } +// Regression test for Redmine issue #19925 +TEST(Security, legacy_token_algorithms_communicate) +{ + auto test_run = [](bool legacy_pub, bool legacy_sub) -> void + { + // Create + PubSubWriter writer("HelloWorldTopic_legacy_token_algorithms_communicate"); + PubSubReader reader("HelloWorldTopic_legacy_token_algorithms_communicate"); + const std::string governance_file("governance_helloworld_all_enable.smime"); + const std::string permissions_file("permissions_helloworld.smime"); + + // Configure Writer + { + PropertyPolicy extra_policy; + const char* value = legacy_pub ? "true" : "false"; + auto& properties = extra_policy.properties(); + properties.emplace_back( + "dds.sec.auth.builtin.PKI-DH.transmit_algorithms_as_legacy", value); + properties.emplace_back( + "dds.sec.access.builtin.Access-Permissions.transmit_algorithms_as_legacy", value); + CommonPermissionsConfigure(writer, governance_file, permissions_file, extra_policy); + } + + // Configure Reader + { + PropertyPolicy extra_policy; + const char* value = legacy_sub ? "true" : "false"; + auto& properties = extra_policy.properties(); + properties.emplace_back( + "dds.sec.auth.builtin.PKI-DH.transmit_algorithms_as_legacy", value); + properties.emplace_back( + "dds.sec.access.builtin.Access-Permissions.transmit_algorithms_as_legacy", value); + CommonPermissionsConfigure(reader, governance_file, permissions_file, extra_policy); + } + + // Initialize + reader.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS).init(); + ASSERT_TRUE(reader.isInitialized()); + writer.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS).init(); + ASSERT_TRUE(writer.isInitialized()); + + // Wait for discovery + reader.waitAuthorized(); + writer.waitAuthorized(); + reader.wait_discovery(); + writer.wait_discovery(); + ASSERT_TRUE(reader.is_matched()); + ASSERT_TRUE(writer.is_matched()); + + // Perform communication + auto data = default_helloworld_data_generator(1); + reader.startReception(data); + writer.send(data); + ASSERT_TRUE(data.empty()); + reader.block_for_all(); + }; + + // Test all possible combinations + test_run(false, false); + test_run(false, true); + test_run(true, false); + test_run(true, true); +} + void blackbox_security_init() { certs_path = std::getenv("CERTS_PATH");