Skip to content

Commit 7cd75a1

Browse files
mergify[bot]MiguelCompany
mergify[bot]
and
MiguelCompany
authored
Use correct algorithm strings on PermissionsToken and IdentityToken (#5450) (#5485)
* 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> * Change default to keep current functionality Signed-off-by: Miguel Company <miguelcompany@eprosima.com> --------- Signed-off-by: Miguel Company <miguelcompany@eprosima.com> Co-authored-by: Miguel Company <miguelcompany@eprosima.com>
1 parent 4f65262 commit 7cd75a1

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
@@ -57,6 +57,67 @@ using namespace eprosima::fastrtps;
5757
using namespace eprosima::fastrtps::rtps;
5858
using namespace eprosima::fastrtps::rtps::security;
5959

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

764825
static bool generate_permissions_token(
765-
AccessPermissionsHandle& handle)
826+
AccessPermissionsHandle& handle,
827+
bool transmit_legacy_algorithms)
766828
{
767829
Property property;
768830
PermissionsToken& token = handle->permissions_token_;
@@ -774,7 +836,7 @@ static bool generate_permissions_token(
774836
token.properties().push_back(std::move(property));
775837

776838
property.name("dds.perm_ca.algo");
777-
property.value() = handle->algo;
839+
property.value() = convert_to_token_algo(handle->algo, transmit_legacy_algorithms);
778840
property.propagate(true);
779841
token.properties().push_back(std::move(property));
780842

@@ -834,6 +896,13 @@ PermissionsHandle* Permissions::validate_local_permissions(
834896
return nullptr;
835897
}
836898

899+
bool transmit_legacy_algorithms = true;
900+
std::string* legacy = PropertyPolicyHelper::find_property(access_properties, "transmit_algorithms_as_legacy");
901+
if (legacy != nullptr)
902+
{
903+
transmit_legacy_algorithms = (*legacy == "true");
904+
}
905+
837906
std::string* permissions_ca = PropertyPolicyHelper::find_property(access_properties, "permissions_ca");
838907

839908
if (permissions_ca == nullptr)
@@ -876,7 +945,7 @@ PermissionsHandle* Permissions::validate_local_permissions(
876945
// Check subject name.
877946
if (check_subject_name(identity, *ah, domain_id, rules, permissions_data, exception))
878947
{
879-
if (generate_permissions_token(*ah))
948+
if (generate_permissions_token(*ah, transmit_legacy_algorithms))
880949
{
881950
if (generate_credentials_token(*ah, *permissions, exception))
882951
{
@@ -1010,7 +1079,8 @@ PermissionsHandle* Permissions::validate_remote_permissions(
10101079

10111080
if (algo != nullptr)
10121081
{
1013-
if (algo->compare(lph->algo) != 0)
1082+
std::string used_algo = parse_token_algo(*algo);
1083+
if (used_algo.compare(lph->algo) != 0)
10141084
{
10151085
exception = _SecurityException_("Remote participant PermissionsCA algorithm differs from local");
10161086
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
@@ -893,6 +893,67 @@ static bool generate_challenge(
893893
return returnedValue;
894894
}
895895

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

9651026
static bool generate_identity_token(
966-
PKIIdentityHandle& handle)
1027+
PKIIdentityHandle& handle,
1028+
bool transmit_legacy_algorithms)
9671029
{
9681030
Property property;
9691031
IdentityToken& token = handle->identity_token_;
@@ -975,7 +1037,7 @@ static bool generate_identity_token(
9751037
token.properties().push_back(std::move(property));
9761038

9771039
property.name("dds.cert.algo");
978-
property.value() = handle->sign_alg_;
1040+
property.value() = convert_to_token_algo(handle->sign_alg_, transmit_legacy_algorithms);
9791041
property.propagate(true);
9801042
token.properties().push_back(std::move(property));
9811043

@@ -985,7 +1047,7 @@ static bool generate_identity_token(
9851047
token.properties().push_back(std::move(property));
9861048

9871049
property.name("dds.ca.algo");
988-
property.value() = handle->algo;
1050+
property.value() = convert_to_token_algo(handle->algo, transmit_legacy_algorithms);
9891051
property.propagate(true);
9901052
token.properties().push_back(std::move(property));
9911053

@@ -1012,6 +1074,13 @@ ValidationResult_t PKIDH::validate_local_identity(
10121074
return ValidationResult_t::VALIDATION_FAILED;
10131075
}
10141076

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

10171086
if (identity_ca == nullptr)
@@ -1157,7 +1226,7 @@ ValidationResult_t PKIDH::validate_local_identity(
11571226
adjusted_participant_key, exception))
11581227
{
11591228
// Generate IdentityToken.
1160-
if (generate_identity_token(*ih))
1229+
if (generate_identity_token(*ih, transmit_legacy_algorithms))
11611230
{
11621231
(*ih)->participant_key_ = adjusted_participant_key;
11631232
*local_identity_handle = ih;
@@ -1210,7 +1279,7 @@ ValidationResult_t PKIDH::validate_remote_identity(
12101279

12111280
(*rih)->sn = ca_sn ? *ca_sn : "";
12121281
(*rih)->cert_sn_ = ""; // cert_sn ? *cert_sn : "";
1213-
(*rih)->algo = cert_algo ? *cert_algo : "";
1282+
(*rih)->algo = cert_algo ? parse_token_algo(*cert_algo) : "";
12141283
(*rih)->participant_key_ = remote_participant_key;
12151284
*remote_identity_handle = rih;
12161285

‎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
@@ -4545,6 +4545,70 @@ TEST(Security, openssl_correctly_finishes)
45454545
std::exit(0);
45464546
}
45474547

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

0 commit comments

Comments
 (0)