Skip to content

Commit 4d793f0

Browse files
mergify[bot]MiguelCompany
mergify[bot]
and
MiguelCompany
authored
New property to select preferred key agreement algorithm (#5413) (#5442)
* New property to select preferred key agreement algorithm (#5413) * Refs #19921. Implement selection of key agreement. Signed-off-by: Miguel Company <[email protected]> * Refs #19921. Change default to ECDH. Signed-off-by: Miguel Company <[email protected]> * Refs #19921. Add unit test. Signed-off-by: Miguel Company <[email protected]> * Refs #19921. Factor out duplicated publisher code on BB test. Signed-off-by: Miguel Company <[email protected]> * Refs #19921. Factor out duplicated subscriber code on BB test. Signed-off-by: Miguel Company <[email protected]> * Refs #19921. Add new parameter to BB test. Signed-off-by: Miguel Company <[email protected]> * Refs #19921. Apply new parameter on publisher properties. Signed-off-by: Miguel Company <[email protected]> * Refs #19921. Apply new parameter on subscriber properties. Signed-off-by: Miguel Company <[email protected]> * Refs #19921. Improve emplace_back calls. Signed-off-by: Miguel Company <[email protected]> * Refs #19921. Uncrustify. Signed-off-by: Miguel Company <[email protected]> * Refs #22280. Use `DH` alias instead of `RSA`. Signed-off-by: Miguel Company <[email protected]> * Refs #22280. Add new property to communication tests XML profiles. Signed-off-by: Miguel Company <[email protected]> * Refs #22280. Fix unit test. Signed-off-by: Miguel Company <[email protected]> * Refs #22280. Configure key agreement on BB test depending on process id. Signed-off-by: Miguel Company <[email protected]> * Refs #19921. Add `AUTO` value to new option. Signed-off-by: Miguel Company <[email protected]> * Refs #19921. Add `AUTO` value to blackbox test. Signed-off-by: Miguel Company <[email protected]> * Refs #22280. Remove unused lambda capture. Signed-off-by: Miguel Company <[email protected]> * Refs #22280. Fix failing blackbox tests. Signed-off-by: Miguel Company <[email protected]> * Update versions.md Signed-off-by: Miguel Company <[email protected]> --------- Signed-off-by: Miguel Company <[email protected]> (cherry picked from commit 8a99a07) # Conflicts: # versions.md * Fix conflicts. Signed-off-by: Miguel Company <[email protected]> * Change default value to `DH`. Signed-off-by: Miguel Company <[email protected]> --------- Signed-off-by: Miguel Company <[email protected]> Co-authored-by: Miguel Company <[email protected]>
1 parent d71913b commit 4d793f0

12 files changed

+396
-923
lines changed

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

+46-2
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555

5656
#include <cassert>
5757
#include <algorithm>
58+
#include <utility>
5859

5960
#define S1(x) #x
6061
#define S2(x) S1(x)
@@ -1051,6 +1052,37 @@ ValidationResult_t PKIDH::validate_local_identity(
10511052
password = &empty_password;
10521053
}
10531054

1055+
std::string key_agreement_algorithm = DH_2048_256;
1056+
std::string* key_agreement_property =
1057+
PropertyPolicyHelper::find_property(auth_properties, "preferred_key_agreement");
1058+
if (nullptr != key_agreement_property)
1059+
{
1060+
const std::pair<std::string, std::string> key_agreement_allowed_values[] = {
1061+
{DH_2048_256, DH_2048_256},
1062+
{ECDH_prime256v1, ECDH_prime256v1},
1063+
{"ECDH", ECDH_prime256v1},
1064+
{"DH", DH_2048_256},
1065+
{"AUTO", "AUTO"}
1066+
};
1067+
1068+
key_agreement_algorithm = "";
1069+
for (const auto& allowed_value : key_agreement_allowed_values)
1070+
{
1071+
if (key_agreement_property->compare(allowed_value.first) == 0)
1072+
{
1073+
key_agreement_algorithm = allowed_value.second;
1074+
break;
1075+
}
1076+
}
1077+
1078+
if (key_agreement_algorithm.empty())
1079+
{
1080+
exception = _SecurityException_("Invalid key agreement algorithm '" + *key_agreement_property + "'");
1081+
EMERGENCY_SECURITY_LOGGING("PKIDH", exception.what());
1082+
return ValidationResult_t::VALIDATION_FAILED;
1083+
}
1084+
}
1085+
10541086
PKIIdentityHandle* ih = &PKIIdentityHandle::narrow(*get_identity_handle(exception));
10551087

10561088
(*ih)->store_ = load_identity_ca(*identity_ca, (*ih)->there_are_crls_, (*ih)->sn, (*ih)->algo,
@@ -1060,6 +1092,20 @@ ValidationResult_t PKIDH::validate_local_identity(
10601092
{
10611093
ERR_clear_error();
10621094

1095+
if (key_agreement_algorithm == "AUTO")
1096+
{
1097+
if ((*ih)->algo == RSA_SHA256)
1098+
{
1099+
key_agreement_algorithm = DH_2048_256;
1100+
}
1101+
else
1102+
{
1103+
key_agreement_algorithm = ECDH_prime256v1;
1104+
}
1105+
}
1106+
1107+
(*ih)->kagree_alg_ = key_agreement_algorithm;
1108+
10631109
if (identity_crl != nullptr)
10641110
{
10651111
X509_CRL* crl = load_crl(*identity_crl, exception);
@@ -1266,7 +1312,6 @@ ValidationResult_t PKIDH::begin_handshake_request(
12661312
bproperty.propagate(true);
12671313
(*handshake_handle_aux)->handshake_message_.binary_properties().push_back(std::move(bproperty));
12681314

1269-
// TODO(Ricardo) Only support right now DH+MODP-2048-256
12701315
// c.kagree_algo.
12711316
bproperty.name("c.kagree_algo");
12721317
bproperty.value().assign(lih->kagree_alg_.begin(),
@@ -1636,7 +1681,6 @@ ValidationResult_t PKIDH::begin_handshake_reply(
16361681
bproperty.propagate(true);
16371682
(*handshake_handle_aux)->handshake_message_.binary_properties().push_back(std::move(bproperty));
16381683

1639-
// TODO(Ricardo) Only support right now DH+MODP-2048-256
16401684
// c.kagree_algo.
16411685
bproperty.name("c.kagree_algo");
16421686
bproperty.value().assign((*handshake_handle_aux)->kagree_alg_.begin(),

‎test/blackbox/common/BlackboxTestsSecurity.cpp

+251-921
Large diffs are not rendered by default.

‎test/dds/communication/security/secure_msg_crypto_besteffort_pub_profile.xml

+4
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@
3434
<name>dds.sec.auth.builtin.PKI-DH.private_key</name>
3535
<value>file://mainpubkey.pem</value>
3636
</property>
37+
<property>
38+
<name>dds.sec.auth.builtin.PKI-DH.preferred_key_agreement</name>
39+
<value>ECDH</value>
40+
</property>
3741
<!-- Activate DDS:Crypto:AES-GCM-GMAC plugin -->
3842
<property>
3943
<name>dds.sec.crypto.plugin</name>

‎test/dds/communication/security/secure_msg_submsg_crypto_besteffort_pub_profile.xml

+4
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@
3333
<name>dds.sec.auth.builtin.PKI-DH.private_key</name>
3434
<value>file://mainpubkey.pem</value>
3535
</property>
36+
<property>
37+
<name>dds.sec.auth.builtin.PKI-DH.preferred_key_agreement</name>
38+
<value>DH</value>
39+
</property>
3640
<!-- Activate DDS:Crypto:AES-GCM-GMAC plugin -->
3741
<property>
3842
<name>dds.sec.crypto.plugin</name>

‎test/dds/communication/security/secure_msg_submsg_crypto_besteffort_sub_profile.xml

+4
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@
3333
<name>dds.sec.auth.builtin.PKI-DH.private_key</name>
3434
<value>file://mainsubkey.pem</value>
3535
</property>
36+
<property>
37+
<name>dds.sec.auth.builtin.PKI-DH.preferred_key_agreement</name>
38+
<value>ECDH</value>
39+
</property>
3640
<!-- Activate DDS:Crypto:AES-GCM-GMAC plugin -->
3741
<property>
3842
<name>dds.sec.crypto.plugin</name>

‎test/dds/communication/security/secure_submsg_crypto_besteffort_pub_profile.xml

+4
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@
3333
<name>dds.sec.auth.builtin.PKI-DH.private_key</name>
3434
<value>file://mainpubkey.pem</value>
3535
</property>
36+
<property>
37+
<name>dds.sec.auth.builtin.PKI-DH.preferred_key_agreement</name>
38+
<value>ECDH</value>
39+
</property>
3640
<!-- Activate DDS:Crypto:AES-GCM-GMAC plugin -->
3741
<property>
3842
<name>dds.sec.crypto.plugin</name>

‎test/dds/communication/security/secure_submsg_crypto_besteffort_sub_profile.xml

+4
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@
3333
<name>dds.sec.auth.builtin.PKI-DH.private_key</name>
3434
<value>file://mainsubkey.pem</value>
3535
</property>
36+
<property>
37+
<name>dds.sec.auth.builtin.PKI-DH.preferred_key_agreement</name>
38+
<value>DH</value>
39+
</property>
3640
<!-- Activate DDS:Crypto:AES-GCM-GMAC plugin -->
3741
<property>
3842
<name>dds.sec.crypto.plugin</name>

‎test/dds/communication/security/simple_secure_besteffort_pub_profile.xml

+4
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@
2626
<name>dds.sec.auth.builtin.PKI-DH.private_key</name>
2727
<value>file://mainpubkey.pem</value>
2828
</property>
29+
<property>
30+
<name>dds.sec.auth.builtin.PKI-DH.preferred_key_agreement</name>
31+
<value>ECDH</value>
32+
</property>
2933
<!-- Activate Access:Permissions plugin -->
3034
<property>
3135
<name>dds.sec.access.plugin</name>

‎test/dds/communication/security/simple_secure_besteffort_sub_profile.xml

+4
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@
2626
<name>dds.sec.auth.builtin.PKI-DH.private_key</name>
2727
<value>file://mainsubkey.pem</value>
2828
</property>
29+
<property>
30+
<name>dds.sec.auth.builtin.PKI-DH.preferred_key_agreement</name>
31+
<value>DH</value>
32+
</property>
2933
<!-- Activate Access:Permissions plugin -->
3034
<property>
3135
<name>dds.sec.access.plugin</name>

‎test/unittest/security/authentication/AuthenticationPluginTests.hpp

+1
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ TEST_F(AuthenticationPluginTest, handshake_process_ok)
165165
ValidationResult_t::VALIDATION_FAILED;
166166

167167
participant_attr.properties = get_valid_policy();
168+
participant_attr.properties.properties().emplace_back("dds.sec.auth.builtin.PKI-DH.preferred_key_agreement", "DH");
168169

169170
result = plugin.validate_local_identity(&local_identity_handle1,
170171
adjusted_participant_key1,

‎test/unittest/security/authentication/BuiltinPKIDHTests.cpp

+69
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,75 @@ void AuthenticationPluginTest::check_shared_secrets(
402402
ASSERT_TRUE(*sharedsecret_1 == *sharedsecret_2);
403403
}
404404

405+
TEST_F(AuthenticationPluginTest, validate_local_identity_kagree_algo)
406+
{
407+
const std::string correct_values[] =
408+
{
409+
"DH",
410+
"ECDH",
411+
"DH+MODP-2048-256",
412+
"ECDH+prime256v1-CEUM"
413+
};
414+
415+
const std::string wrong_values[] =
416+
{
417+
"RSA+MODP-2048-256",
418+
"ECDH+MODP-2048-256",
419+
"RSA",
420+
"ECDH+prime256v1",
421+
"unknown",
422+
""
423+
};
424+
425+
auto test_fn = [this](
426+
const std::string& alg,
427+
ValidationResult_t expected_result) -> void
428+
{
429+
IdentityHandle* local_identity_handle = nullptr;
430+
GUID_t adjusted_participant_key;
431+
uint32_t domain_id = 0;
432+
RTPSParticipantAttributes participant_attr;
433+
GUID_t candidate_participant_key;
434+
SecurityException exception;
435+
ValidationResult_t result = ValidationResult_t::VALIDATION_FAILED;
436+
437+
fill_candidate_participant_key(candidate_participant_key);
438+
participant_attr.properties = get_valid_policy();
439+
participant_attr.properties.properties().emplace_back(
440+
Property("dds.sec.auth.builtin.PKI-DH.preferred_key_agreement", alg));
441+
result = plugin.validate_local_identity(&local_identity_handle,
442+
adjusted_participant_key,
443+
domain_id,
444+
participant_attr,
445+
candidate_participant_key,
446+
exception);
447+
448+
ASSERT_TRUE(result == expected_result);
449+
if (ValidationResult_t::VALIDATION_OK == result)
450+
{
451+
ASSERT_TRUE(local_identity_handle != nullptr);
452+
check_local_identity_handle(*local_identity_handle);
453+
ASSERT_TRUE(adjusted_participant_key != GUID_t::unknown());
454+
ASSERT_TRUE(plugin.return_identity_handle(local_identity_handle, exception));
455+
}
456+
else
457+
{
458+
ASSERT_TRUE(local_identity_handle == nullptr);
459+
ASSERT_TRUE(adjusted_participant_key == GUID_t::unknown());
460+
}
461+
};
462+
463+
for (const std::string& value : correct_values)
464+
{
465+
test_fn(value, ValidationResult_t::VALIDATION_OK);
466+
}
467+
468+
for (const std::string& value : wrong_values)
469+
{
470+
test_fn(value, ValidationResult_t::VALIDATION_FAILED);
471+
}
472+
}
473+
405474
TEST_F(AuthenticationPluginTest, validate_local_identity_validation_ok_with_pwd)
406475
{
407476
IdentityHandle* local_identity_handle = nullptr;

‎versions.md

+1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
Forthcoming
22
-----------
33

4+
* New property to configure the preferred key agreement algorithm.
45

56
Version v3.1.0
67
--------------

0 commit comments

Comments
 (0)