Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[22280] New property to select preferred key agreement algorithm #5413

Merged
merged 19 commits into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
d282b83
Refs #19921. Implement selection of key agreement.
MiguelCompany Nov 21, 2024
b239812
Refs #19921. Change default to ECDH.
MiguelCompany Nov 21, 2024
8444ffe
Refs #19921. Add unit test.
MiguelCompany Nov 21, 2024
c28420e
Refs #19921. Factor out duplicated publisher code on BB test.
MiguelCompany Nov 21, 2024
e6920a5
Refs #19921. Factor out duplicated subscriber code on BB test.
MiguelCompany Nov 21, 2024
7e65dae
Refs #19921. Add new parameter to BB test.
MiguelCompany Nov 21, 2024
7ca65d4
Refs #19921. Apply new parameter on publisher properties.
MiguelCompany Nov 21, 2024
5d5c8e6
Refs #19921. Apply new parameter on subscriber properties.
MiguelCompany Nov 21, 2024
7ab48ed
Refs #19921. Improve emplace_back calls.
MiguelCompany Nov 21, 2024
79b130a
Refs #19921. Uncrustify.
MiguelCompany Nov 21, 2024
1c52291
Refs #22280. Use `DH` alias instead of `RSA`.
MiguelCompany Nov 22, 2024
708fa0d
Refs #22280. Add new property to communication tests XML profiles.
MiguelCompany Nov 22, 2024
0d965a0
Refs #22280. Fix unit test.
MiguelCompany Nov 26, 2024
5af17b3
Refs #22280. Configure key agreement on BB test depending on process id.
MiguelCompany Nov 27, 2024
c2b6d0d
Refs #19921. Add `AUTO` value to new option.
MiguelCompany Nov 27, 2024
363d383
Refs #19921. Add `AUTO` value to blackbox test.
MiguelCompany Nov 27, 2024
3b71837
Refs #22280. Remove unused lambda capture.
MiguelCompany Nov 27, 2024
ba47491
Refs #22280. Fix failing blackbox tests.
MiguelCompany Nov 28, 2024
24ac7c9
Update versions.md
MiguelCompany Nov 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 46 additions & 2 deletions src/cpp/security/authentication/PKIDH.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@

#include <cassert>
#include <algorithm>
#include <utility>

#define S1(x) #x
#define S2(x) S1(x)
Expand Down Expand Up @@ -1051,6 +1052,37 @@ ValidationResult_t PKIDH::validate_local_identity(
password = &empty_password;
}

std::string key_agreement_algorithm = "AUTO";
std::string* key_agreement_property =
PropertyPolicyHelper::find_property(auth_properties, "preferred_key_agreement");
if (nullptr != key_agreement_property)
{
const std::pair<std::string, std::string> key_agreement_allowed_values[] = {
{DH_2048_256, DH_2048_256},
{ECDH_prime256v1, ECDH_prime256v1},
{"ECDH", ECDH_prime256v1},
{"DH", DH_2048_256},
{"AUTO", "AUTO"}
};

key_agreement_algorithm = "";
for (const auto& allowed_value : key_agreement_allowed_values)
{
if (key_agreement_property->compare(allowed_value.first) == 0)
{
key_agreement_algorithm = allowed_value.second;
break;
}
}

if (key_agreement_algorithm.empty())
{
exception = _SecurityException_("Invalid key agreement algorithm '" + *key_agreement_property + "'");
EMERGENCY_SECURITY_LOGGING("PKIDH", exception.what());
return ValidationResult_t::VALIDATION_FAILED;
}
}

Mario-DL marked this conversation as resolved.
Show resolved Hide resolved
PKIIdentityHandle* ih = &PKIIdentityHandle::narrow(*get_identity_handle(exception));

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

if (key_agreement_algorithm == "AUTO")
{
if ((*ih)->algo == RSA_SHA256)
{
key_agreement_algorithm = DH_2048_256;
}
else
{
key_agreement_algorithm = ECDH_prime256v1;
}
}

(*ih)->kagree_alg_ = key_agreement_algorithm;

if (identity_crl != nullptr)
{
X509_CRL* crl = load_crl(*identity_crl, exception);
Expand Down Expand Up @@ -1266,7 +1312,6 @@ ValidationResult_t PKIDH::begin_handshake_request(
bproperty.propagate(true);
(*handshake_handle_aux)->handshake_message_.binary_properties().push_back(std::move(bproperty));

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

// TODO(Ricardo) Only support right now DH+MODP-2048-256
// c.kagree_algo.
bproperty.name("c.kagree_algo");
bproperty.value().assign((*handshake_handle_aux)->kagree_alg_.begin(),
Expand Down
1,172 changes: 251 additions & 921 deletions test/blackbox/common/BlackboxTestsSecurity.cpp

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@
<name>dds.sec.auth.builtin.PKI-DH.private_key</name>
<value>file://mainpubkey.pem</value>
</property>
<property>
<name>dds.sec.auth.builtin.PKI-DH.preferred_key_agreement</name>
<value>ECDH</value>
</property>
<!-- Activate DDS:Crypto:AES-GCM-GMAC plugin -->
<property>
<name>dds.sec.crypto.plugin</name>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@
<name>dds.sec.auth.builtin.PKI-DH.private_key</name>
<value>file://mainpubkey.pem</value>
</property>
<property>
<name>dds.sec.auth.builtin.PKI-DH.preferred_key_agreement</name>
<value>DH</value>
</property>
<!-- Activate DDS:Crypto:AES-GCM-GMAC plugin -->
<property>
<name>dds.sec.crypto.plugin</name>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@
<name>dds.sec.auth.builtin.PKI-DH.private_key</name>
<value>file://mainsubkey.pem</value>
</property>
<property>
<name>dds.sec.auth.builtin.PKI-DH.preferred_key_agreement</name>
<value>ECDH</value>
</property>
<!-- Activate DDS:Crypto:AES-GCM-GMAC plugin -->
<property>
<name>dds.sec.crypto.plugin</name>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@
<name>dds.sec.auth.builtin.PKI-DH.private_key</name>
<value>file://mainpubkey.pem</value>
</property>
<property>
<name>dds.sec.auth.builtin.PKI-DH.preferred_key_agreement</name>
<value>ECDH</value>
</property>
<!-- Activate DDS:Crypto:AES-GCM-GMAC plugin -->
<property>
<name>dds.sec.crypto.plugin</name>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@
<name>dds.sec.auth.builtin.PKI-DH.private_key</name>
<value>file://mainsubkey.pem</value>
</property>
<property>
<name>dds.sec.auth.builtin.PKI-DH.preferred_key_agreement</name>
<value>DH</value>
</property>
<!-- Activate DDS:Crypto:AES-GCM-GMAC plugin -->
<property>
<name>dds.sec.crypto.plugin</name>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@
<name>dds.sec.auth.builtin.PKI-DH.private_key</name>
<value>file://mainpubkey.pem</value>
</property>
<property>
<name>dds.sec.auth.builtin.PKI-DH.preferred_key_agreement</name>
<value>ECDH</value>
</property>
<!-- Activate Access:Permissions plugin -->
<property>
<name>dds.sec.access.plugin</name>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@
<name>dds.sec.auth.builtin.PKI-DH.private_key</name>
<value>file://mainsubkey.pem</value>
</property>
<property>
<name>dds.sec.auth.builtin.PKI-DH.preferred_key_agreement</name>
<value>DH</value>
</property>
<!-- Activate Access:Permissions plugin -->
<property>
<name>dds.sec.access.plugin</name>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ TEST_F(AuthenticationPluginTest, handshake_process_ok)
ValidationResult_t::VALIDATION_FAILED;

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

result = plugin.validate_local_identity(&local_identity_handle1,
adjusted_participant_key1,
Expand Down
69 changes: 69 additions & 0 deletions test/unittest/security/authentication/BuiltinPKIDHTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,75 @@ void AuthenticationPluginTest::check_shared_secrets(
ASSERT_TRUE(*sharedsecret_1 == *sharedsecret_2);
}

TEST_F(AuthenticationPluginTest, validate_local_identity_kagree_algo)
{
const std::string correct_values[] =
{
"DH",
"ECDH",
"DH+MODP-2048-256",
"ECDH+prime256v1-CEUM"
};

const std::string wrong_values[] =
{
"RSA+MODP-2048-256",
"ECDH+MODP-2048-256",
"RSA",
"ECDH+prime256v1",
"unknown",
""
};

auto test_fn = [this](
const std::string& alg,
ValidationResult_t expected_result) -> void
{
IdentityHandle* local_identity_handle = nullptr;
GUID_t adjusted_participant_key;
uint32_t domain_id = 0;
RTPSParticipantAttributes participant_attr;
GUID_t candidate_participant_key;
SecurityException exception;
ValidationResult_t result = ValidationResult_t::VALIDATION_FAILED;

fill_candidate_participant_key(candidate_participant_key);
participant_attr.properties = get_valid_policy();
participant_attr.properties.properties().emplace_back(
Property("dds.sec.auth.builtin.PKI-DH.preferred_key_agreement", alg));
result = plugin.validate_local_identity(&local_identity_handle,
adjusted_participant_key,
domain_id,
participant_attr,
candidate_participant_key,
exception);

ASSERT_TRUE(result == expected_result);
if (ValidationResult_t::VALIDATION_OK == result)
{
ASSERT_TRUE(local_identity_handle != nullptr);
check_local_identity_handle(*local_identity_handle);
ASSERT_TRUE(adjusted_participant_key != GUID_t::unknown());
ASSERT_TRUE(plugin.return_identity_handle(local_identity_handle, exception));
}
else
{
ASSERT_TRUE(local_identity_handle == nullptr);
ASSERT_TRUE(adjusted_participant_key == GUID_t::unknown());
}
};

for (const std::string& value : correct_values)
{
test_fn(value, ValidationResult_t::VALIDATION_OK);
}

for (const std::string& value : wrong_values)
{
test_fn(value, ValidationResult_t::VALIDATION_FAILED);
}
}

TEST_F(AuthenticationPluginTest, validate_local_identity_validation_ok_with_pwd)
{
IdentityHandle* local_identity_handle = nullptr;
Expand Down
1 change: 1 addition & 0 deletions versions.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Forthcoming
* Added ``Extended Incompatible QoS`` feature for monitor service.
* Machine UUID added to the Data(p) to check Participants in same host, instead of using GUIDs.
* Windows ci example testing infrastructure and `hello world` example.
* New property to configure the preferred key agreement algorithm.

Version v3.1.0
--------------
Expand Down