Skip to content

Commit 971f120

Browse files
MiguelCompanyMiguelCompany
authored
Use correct algorithm strings on PermissionsToken and IdentityToken (#5450)
* 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>
1 parent 0c799f4 commit 971f120

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
@@ -59,6 +59,67 @@ namespace rtps {
5959

6060
using namespace security;
6161

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

696757
static bool generate_permissions_token(
697-
AccessPermissionsHandle& handle)
758+
AccessPermissionsHandle& handle,
759+
bool transmit_legacy_algorithms)
698760
{
699761
Property property;
700762
PermissionsToken& token = handle->permissions_token_;
@@ -706,7 +768,7 @@ static bool generate_permissions_token(
706768
token.properties().push_back(std::move(property));
707769

708770
property.name("dds.perm_ca.algo");
709-
property.value() = handle->algo;
771+
property.value() = convert_to_token_algo(handle->algo, transmit_legacy_algorithms);
710772
property.propagate(true);
711773
token.properties().push_back(std::move(property));
712774

@@ -766,6 +828,13 @@ PermissionsHandle* Permissions::validate_local_permissions(
766828
return nullptr;
767829
}
768830

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

771840
if (permissions_ca == nullptr)
@@ -808,7 +877,7 @@ PermissionsHandle* Permissions::validate_local_permissions(
808877
// Check subject name.
809878
if (check_subject_name(identity, *ah, domain_id, rules, permissions_data, exception))
810879
{
811-
if (generate_permissions_token(*ah))
880+
if (generate_permissions_token(*ah, transmit_legacy_algorithms))
812881
{
813882
if (generate_credentials_token(*ah, *permissions, exception))
814883
{
@@ -942,7 +1011,8 @@ PermissionsHandle* Permissions::validate_remote_permissions(
9421011

9431012
if (algo != nullptr)
9441013
{
945-
if (algo->compare(lph->algo) != 0)
1014+
std::string used_algo = parse_token_algo(*algo);
1015+
if (used_algo.compare(lph->algo) != 0)
9461016
{
9471017
exception = _SecurityException_("Remote participant PermissionsCA algorithm differs from local");
9481018
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
@@ -896,6 +896,67 @@ static bool generate_challenge(
896896
return returnedValue;
897897
}
898898

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

9681029
static bool generate_identity_token(
969-
PKIIdentityHandle& handle)
1030+
PKIIdentityHandle& handle,
1031+
bool transmit_legacy_algorithms)
9701032
{
9711033
Property property;
9721034
IdentityToken& token = handle->identity_token_;
@@ -978,7 +1040,7 @@ static bool generate_identity_token(
9781040
token.properties().push_back(std::move(property));
9791041

9801042
property.name("dds.cert.algo");
981-
property.value() = handle->sign_alg_;
1043+
property.value() = convert_to_token_algo(handle->sign_alg_, transmit_legacy_algorithms);
9821044
property.propagate(true);
9831045
token.properties().push_back(std::move(property));
9841046

@@ -988,7 +1050,7 @@ static bool generate_identity_token(
9881050
token.properties().push_back(std::move(property));
9891051

9901052
property.name("dds.ca.algo");
991-
property.value() = handle->algo;
1053+
property.value() = convert_to_token_algo(handle->algo, transmit_legacy_algorithms);
9921054
property.propagate(true);
9931055
token.properties().push_back(std::move(property));
9941056

@@ -1015,6 +1077,13 @@ ValidationResult_t PKIDH::validate_local_identity(
10151077
return ValidationResult_t::VALIDATION_FAILED;
10161078
}
10171079

1080+
bool transmit_legacy_algorithms = false;
1081+
std::string* legacy = PropertyPolicyHelper::find_property(auth_properties, "transmit_algorithms_as_legacy");
1082+
if (legacy != nullptr)
1083+
{
1084+
transmit_legacy_algorithms = (*legacy == "true");
1085+
}
1086+
10181087
std::string* identity_ca = PropertyPolicyHelper::find_property(auth_properties, "identity_ca");
10191088

10201089
if (identity_ca == nullptr)
@@ -1160,7 +1229,7 @@ ValidationResult_t PKIDH::validate_local_identity(
11601229
adjusted_participant_key, exception))
11611230
{
11621231
// Generate IdentityToken.
1163-
if (generate_identity_token(*ih))
1232+
if (generate_identity_token(*ih, transmit_legacy_algorithms))
11641233
{
11651234
(*ih)->participant_key_ = adjusted_participant_key;
11661235
*local_identity_handle = ih;
@@ -1213,7 +1282,7 @@ ValidationResult_t PKIDH::validate_remote_identity(
12131282

12141283
(*rih)->sn = ca_sn ? *ca_sn : "";
12151284
(*rih)->cert_sn_ = ""; // cert_sn ? *cert_sn : "";
1216-
(*rih)->algo = cert_algo ? *cert_algo : "";
1285+
(*rih)->algo = cert_algo ? parse_token_algo(*cert_algo) : "";
12171286
(*rih)->participant_key_ = remote_participant_key;
12181287
*remote_identity_handle = rih;
12191288

‎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
@@ -4548,6 +4548,70 @@ TEST(Security, openssl_correctly_finishes)
45484548
std::exit(0);
45494549
}
45504550

4551+
// Regression test for Redmine issue #19925
4552+
TEST(Security, legacy_token_algorithms_communicate)
4553+
{
4554+
auto test_run = [](bool legacy_pub, bool legacy_sub) -> void
4555+
{
4556+
// Create
4557+
PubSubWriter<HelloWorldPubSubType> writer("HelloWorldTopic_legacy_token_algorithms_communicate");
4558+
PubSubReader<HelloWorldPubSubType> reader("HelloWorldTopic_legacy_token_algorithms_communicate");
4559+
const std::string governance_file("governance_helloworld_all_enable.smime");
4560+
const std::string permissions_file("permissions_helloworld.smime");
4561+
4562+
// Configure Writer
4563+
{
4564+
PropertyPolicy extra_policy;
4565+
const char* value = legacy_pub ? "true" : "false";
4566+
auto& properties = extra_policy.properties();
4567+
properties.emplace_back(
4568+
"dds.sec.auth.builtin.PKI-DH.transmit_algorithms_as_legacy", value);
4569+
properties.emplace_back(
4570+
"dds.sec.access.builtin.Access-Permissions.transmit_algorithms_as_legacy", value);
4571+
CommonPermissionsConfigure(writer, governance_file, permissions_file, extra_policy);
4572+
}
4573+
4574+
// Configure Reader
4575+
{
4576+
PropertyPolicy extra_policy;
4577+
const char* value = legacy_sub ? "true" : "false";
4578+
auto& properties = extra_policy.properties();
4579+
properties.emplace_back(
4580+
"dds.sec.auth.builtin.PKI-DH.transmit_algorithms_as_legacy", value);
4581+
properties.emplace_back(
4582+
"dds.sec.access.builtin.Access-Permissions.transmit_algorithms_as_legacy", value);
4583+
CommonPermissionsConfigure(reader, governance_file, permissions_file, extra_policy);
4584+
}
4585+
4586+
// Initialize
4587+
reader.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS).init();
4588+
ASSERT_TRUE(reader.isInitialized());
4589+
writer.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS).init();
4590+
ASSERT_TRUE(writer.isInitialized());
4591+
4592+
// Wait for discovery
4593+
reader.waitAuthorized();
4594+
writer.waitAuthorized();
4595+
reader.wait_discovery();
4596+
writer.wait_discovery();
4597+
ASSERT_TRUE(reader.is_matched());
4598+
ASSERT_TRUE(writer.is_matched());
4599+
4600+
// Perform communication
4601+
auto data = default_helloworld_data_generator(1);
4602+
reader.startReception(data);
4603+
writer.send(data);
4604+
ASSERT_TRUE(data.empty());
4605+
reader.block_for_all();
4606+
};
4607+
4608+
// Test all possible combinations
4609+
test_run(false, false);
4610+
test_run(false, true);
4611+
test_run(true, false);
4612+
test_run(true, true);
4613+
}
4614+
45514615
void blackbox_security_init()
45524616
{
45534617
certs_path = std::getenv("CERTS_PATH");

0 commit comments

Comments
 (0)