Skip to content

Commit 0f38526

Browse files
mergify[bot]MiguelCompany
mergify[bot]
and
MiguelCompany
authored
Use correct algorithm strings on PermissionsToken and IdentityToken (#5450) (#5482)
* Refs #19925. Add conversion methods for token algorithms. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #19925. Configure legacy transmission on PKIDH. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #19925. Configure legacy transmission on Permissions. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #19925. Add blackbox test. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #19925. Add doxygen documentation to new methods. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> --------- Signed-off-by: Miguel Company <miguelcompany@eprosima.com> (cherry picked from commit 971f120) Co-authored-by: Miguel Company <miguelcompany@eprosima.com>
1 parent c8bcb69 commit 0f38526

File tree

4 files changed

+215
-9
lines changed

4 files changed

+215
-9
lines changed

‎src/cpp/security/accesscontrol/Permissions.cpp

Lines changed: 74 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,67 @@ using namespace eprosima::fastrtps;
5858
using namespace eprosima::fastrtps::rtps;
5959
using namespace eprosima::fastrtps::rtps::security;
6060

61+
/**
62+
* @brief Convert a signature algortihm before adding it to a PermissionsToken.
63+
*
64+
* This methods converts the signature algorithm to the format used in the PermissionsToken.
65+
* Depending on the value of the use_legacy parameter, the algorithm will be converted to the legacy format or to the
66+
* one specified in the DDS-SEC 1.1 specification.
67+
*
68+
* @param algorithm The algorithm to convert.
69+
* @param use_legacy Whether to use the legacy format or not.
70+
*
71+
* @return The converted algorithm.
72+
*/
73+
static std::string convert_to_token_algo(
74+
const std::string& algorithm,
75+
bool use_legacy)
76+
{
77+
// Leave as internal format when legacy is used
78+
if (use_legacy)
79+
{
80+
return algorithm;
81+
}
82+
83+
// Convert to token format
84+
if (algorithm == RSA_SHA256)
85+
{
86+
return RSA_SHA256_FOR_TOKENS;
87+
}
88+
else if (algorithm == ECDSA_SHA256)
89+
{
90+
return ECDSA_SHA256_FOR_TOKENS;
91+
}
92+
93+
return algorithm;
94+
}
95+
96+
/**
97+
* @brief Parse a signature algorithm from a PermissionsToken.
98+
*
99+
* This method parses a signature algorithm from a PermissionsToken.
100+
* It converts the algorithm to the internal (legacy) format used by the library.
101+
*
102+
* @param algorithm The algorithm to parse.
103+
*
104+
* @return The parsed algorithm.
105+
*/
106+
static std::string parse_token_algo(
107+
const std::string& algorithm)
108+
{
109+
// Convert to internal format, allowing both legacy and new formats
110+
if (algorithm == RSA_SHA256_FOR_TOKENS)
111+
{
112+
return RSA_SHA256;
113+
}
114+
else if (algorithm == ECDSA_SHA256_FOR_TOKENS)
115+
{
116+
return ECDSA_SHA256;
117+
}
118+
119+
return algorithm;
120+
}
121+
61122
static bool is_domain_in_set(
62123
const uint32_t domain_id,
63124
const Domains& domains)
@@ -693,7 +754,8 @@ static bool check_subject_name(
693754
}
694755

695756
static bool generate_permissions_token(
696-
AccessPermissionsHandle& handle)
757+
AccessPermissionsHandle& handle,
758+
bool transmit_legacy_algorithms)
697759
{
698760
Property property;
699761
PermissionsToken& token = handle->permissions_token_;
@@ -705,7 +767,7 @@ static bool generate_permissions_token(
705767
token.properties().push_back(std::move(property));
706768

707769
property.name("dds.perm_ca.algo");
708-
property.value() = handle->algo;
770+
property.value() = convert_to_token_algo(handle->algo, transmit_legacy_algorithms);
709771
property.propagate(true);
710772
token.properties().push_back(std::move(property));
711773

@@ -765,6 +827,13 @@ PermissionsHandle* Permissions::validate_local_permissions(
765827
return nullptr;
766828
}
767829

830+
bool transmit_legacy_algorithms = false;
831+
std::string* legacy = PropertyPolicyHelper::find_property(access_properties, "transmit_algorithms_as_legacy");
832+
if (legacy != nullptr)
833+
{
834+
transmit_legacy_algorithms = (*legacy == "true");
835+
}
836+
768837
std::string* permissions_ca = PropertyPolicyHelper::find_property(access_properties, "permissions_ca");
769838

770839
if (permissions_ca == nullptr)
@@ -807,7 +876,7 @@ PermissionsHandle* Permissions::validate_local_permissions(
807876
// Check subject name.
808877
if (check_subject_name(identity, *ah, domain_id, rules, permissions_data, exception))
809878
{
810-
if (generate_permissions_token(*ah))
879+
if (generate_permissions_token(*ah, transmit_legacy_algorithms))
811880
{
812881
if (generate_credentials_token(*ah, *permissions, exception))
813882
{
@@ -941,7 +1010,8 @@ PermissionsHandle* Permissions::validate_remote_permissions(
9411010

9421011
if (algo != nullptr)
9431012
{
944-
if (algo->compare(lph->algo) != 0)
1013+
std::string used_algo = parse_token_algo(*algo);
1014+
if (used_algo.compare(lph->algo) != 0)
9451015
{
9461016
exception = _SecurityException_("Remote participant PermissionsCA algorithm differs from local");
9471017
EMERGENCY_SECURITY_LOGGING("Permissions", exception.what());

‎src/cpp/security/authentication/PKIDH.cpp

Lines changed: 74 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -892,6 +892,67 @@ static bool generate_challenge(
892892
return returnedValue;
893893
}
894894

895+
/**
896+
* @brief Convert a signature algortihm before adding it to an IdentityToken.
897+
*
898+
* This methods converts the signature algorithm to the format used in the IdentityToken.
899+
* Depending on the value of the use_legacy parameter, the algorithm will be converted to the legacy format or to the
900+
* one specified in the DDS-SEC 1.1 specification.
901+
*
902+
* @param algorithm The algorithm to convert.
903+
* @param use_legacy Whether to use the legacy format or not.
904+
*
905+
* @return The converted algorithm.
906+
*/
907+
static std::string convert_to_token_algo(
908+
const std::string& algorithm,
909+
bool use_legacy)
910+
{
911+
// Leave as internal format when legacy is used
912+
if (use_legacy)
913+
{
914+
return algorithm;
915+
}
916+
917+
// Convert to token format
918+
if (algorithm == RSA_SHA256)
919+
{
920+
return RSA_SHA256_FOR_TOKENS;
921+
}
922+
else if (algorithm == ECDSA_SHA256)
923+
{
924+
return ECDSA_SHA256_FOR_TOKENS;
925+
}
926+
927+
return algorithm;
928+
}
929+
930+
/**
931+
* @brief Parse a signature algorithm from an IdentityToken.
932+
*
933+
* This method parses the signature algorithm from an IdentityToken.
934+
* It converts the algorithm to the internal (legacy) format used by the library.
935+
*
936+
* @param algorithm The algorithm to parse.
937+
*
938+
* @return The parsed algorithm.
939+
*/
940+
static std::string parse_token_algo(
941+
const std::string& algorithm)
942+
{
943+
// Convert to internal format, allowing both legacy and new formats
944+
if (algorithm == RSA_SHA256_FOR_TOKENS)
945+
{
946+
return RSA_SHA256;
947+
}
948+
else if (algorithm == ECDSA_SHA256_FOR_TOKENS)
949+
{
950+
return ECDSA_SHA256;
951+
}
952+
953+
return algorithm;
954+
}
955+
895956
std::shared_ptr<SecretHandle> PKIDH::generate_sharedsecret(
896957
EVP_PKEY* private_key,
897958
EVP_PKEY* public_key,
@@ -962,7 +1023,8 @@ std::shared_ptr<SecretHandle> PKIDH::generate_sharedsecret(
9621023
}
9631024

9641025
static bool generate_identity_token(
965-
PKIIdentityHandle& handle)
1026+
PKIIdentityHandle& handle,
1027+
bool transmit_legacy_algorithms)
9661028
{
9671029
Property property;
9681030
IdentityToken& token = handle->identity_token_;
@@ -974,7 +1036,7 @@ static bool generate_identity_token(
9741036
token.properties().push_back(std::move(property));
9751037

9761038
property.name("dds.cert.algo");
977-
property.value() = handle->sign_alg_;
1039+
property.value() = convert_to_token_algo(handle->sign_alg_, transmit_legacy_algorithms);
9781040
property.propagate(true);
9791041
token.properties().push_back(std::move(property));
9801042

@@ -984,7 +1046,7 @@ static bool generate_identity_token(
9841046
token.properties().push_back(std::move(property));
9851047

9861048
property.name("dds.ca.algo");
987-
property.value() = handle->algo;
1049+
property.value() = convert_to_token_algo(handle->algo, transmit_legacy_algorithms);
9881050
property.propagate(true);
9891051
token.properties().push_back(std::move(property));
9901052

@@ -1011,6 +1073,13 @@ ValidationResult_t PKIDH::validate_local_identity(
10111073
return ValidationResult_t::VALIDATION_FAILED;
10121074
}
10131075

1076+
bool transmit_legacy_algorithms = false;
1077+
std::string* legacy = PropertyPolicyHelper::find_property(auth_properties, "transmit_algorithms_as_legacy");
1078+
if (legacy != nullptr)
1079+
{
1080+
transmit_legacy_algorithms = (*legacy == "true");
1081+
}
1082+
10141083
std::string* identity_ca = PropertyPolicyHelper::find_property(auth_properties, "identity_ca");
10151084

10161085
if (identity_ca == nullptr)
@@ -1111,7 +1180,7 @@ ValidationResult_t PKIDH::validate_local_identity(
11111180
adjusted_participant_key, exception))
11121181
{
11131182
// Generate IdentityToken.
1114-
if (generate_identity_token(*ih))
1183+
if (generate_identity_token(*ih, transmit_legacy_algorithms))
11151184
{
11161185
(*ih)->participant_key_ = adjusted_participant_key;
11171186
*local_identity_handle = ih;
@@ -1164,7 +1233,7 @@ ValidationResult_t PKIDH::validate_remote_identity(
11641233

11651234
(*rih)->sn = ca_sn ? *ca_sn : "";
11661235
(*rih)->cert_sn_ = ""; // cert_sn ? *cert_sn : "";
1167-
(*rih)->algo = cert_algo ? *cert_algo : "";
1236+
(*rih)->algo = cert_algo ? parse_token_algo(*cert_algo) : "";
11681237
(*rih)->participant_key_ = remote_participant_key;
11691238
*remote_identity_handle = rih;
11701239

‎src/cpp/security/authentication/PKIIdentityHandle.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ namespace security {
3333
static const char* const RSA_SHA256 = "RSASSA-PSS-SHA256";
3434
static const char* const ECDSA_SHA256 = "ECDSA-SHA256";
3535

36+
static const char* const RSA_SHA256_FOR_TOKENS = "RSA-2048";
37+
static const char* const ECDSA_SHA256_FOR_TOKENS = "EC-prime256v1";
38+
3639
static const char* const DH_2048_256 = "DH+MODP-2048-256";
3740
static const char* const ECDH_prime256v1 = "ECDH+prime256v1-CEUM";
3841

‎test/blackbox/common/BlackboxTestsSecurity.cpp

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5218,6 +5218,70 @@ TEST(Security, openssl_correctly_finishes)
52185218
std::exit(0);
52195219
}
52205220

5221+
// Regression test for Redmine issue #19925
5222+
TEST(Security, legacy_token_algorithms_communicate)
5223+
{
5224+
auto test_run = [](bool legacy_pub, bool legacy_sub) -> void
5225+
{
5226+
// Create
5227+
PubSubWriter<HelloWorldPubSubType> writer("HelloWorldTopic_legacy_token_algorithms_communicate");
5228+
PubSubReader<HelloWorldPubSubType> reader("HelloWorldTopic_legacy_token_algorithms_communicate");
5229+
const std::string governance_file("governance_helloworld_all_enable.smime");
5230+
const std::string permissions_file("permissions_helloworld.smime");
5231+
5232+
// Configure Writer
5233+
{
5234+
PropertyPolicy extra_policy;
5235+
const char* value = legacy_pub ? "true" : "false";
5236+
auto& properties = extra_policy.properties();
5237+
properties.emplace_back(
5238+
"dds.sec.auth.builtin.PKI-DH.transmit_algorithms_as_legacy", value);
5239+
properties.emplace_back(
5240+
"dds.sec.access.builtin.Access-Permissions.transmit_algorithms_as_legacy", value);
5241+
CommonPermissionsConfigure(writer, governance_file, permissions_file, extra_policy);
5242+
}
5243+
5244+
// Configure Reader
5245+
{
5246+
PropertyPolicy extra_policy;
5247+
const char* value = legacy_sub ? "true" : "false";
5248+
auto& properties = extra_policy.properties();
5249+
properties.emplace_back(
5250+
"dds.sec.auth.builtin.PKI-DH.transmit_algorithms_as_legacy", value);
5251+
properties.emplace_back(
5252+
"dds.sec.access.builtin.Access-Permissions.transmit_algorithms_as_legacy", value);
5253+
CommonPermissionsConfigure(reader, governance_file, permissions_file, extra_policy);
5254+
}
5255+
5256+
// Initialize
5257+
reader.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS).init();
5258+
ASSERT_TRUE(reader.isInitialized());
5259+
writer.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS).init();
5260+
ASSERT_TRUE(writer.isInitialized());
5261+
5262+
// Wait for discovery
5263+
reader.waitAuthorized();
5264+
writer.waitAuthorized();
5265+
reader.wait_discovery();
5266+
writer.wait_discovery();
5267+
ASSERT_TRUE(reader.is_matched());
5268+
ASSERT_TRUE(writer.is_matched());
5269+
5270+
// Perform communication
5271+
auto data = default_helloworld_data_generator(1);
5272+
reader.startReception(data);
5273+
writer.send(data);
5274+
ASSERT_TRUE(data.empty());
5275+
reader.block_for_all();
5276+
};
5277+
5278+
// Test all possible combinations
5279+
test_run(false, false);
5280+
test_run(false, true);
5281+
test_run(true, false);
5282+
test_run(true, true);
5283+
}
5284+
52215285
void blackbox_security_init()
52225286
{
52235287
certs_path = std::getenv("CERTS_PATH");

0 commit comments

Comments
 (0)