New property to select preferred key agreement algorithm (#5413)

* Refs #19921. Implement selection of key agreement. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #19921. Change default to ECDH. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #19921. Add unit test. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #19921. Factor out duplicated publisher code on BB test. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #19921. Factor out duplicated subscriber code on BB test. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #19921. Add new parameter to BB test. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #19921. Apply new parameter on publisher properties. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #19921. Apply new parameter on subscriber properties. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #19921. Improve emplace_back calls. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #19921. Uncrustify. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #22280. Use `DH` alias instead of `RSA`. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #22280. Add new property to communication tests XML profiles. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #22280. Fix unit test. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #22280. Configure key agreement on BB test depending on process id. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #19921. Add `AUTO` value to new option. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #19921. Add `AUTO` value to blackbox test. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #22280. Remove unused lambda capture. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #22280. Fix failing blackbox tests. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Update versions.md Signed-off-by: Miguel Company <miguelcompany@eprosima.com> --------- Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
eProsima · Nov 28, 2024 · 8a99a07 · 8a99a07
1 parent b418c8a
commit 8a99a07
Show file tree

Hide file tree

Showing 12 changed files with 396 additions and 923 deletions.
diff --git a/src/cpp/security/authentication/PKIDH.cpp b/src/cpp/security/authentication/PKIDH.cpp
@@ -55,6 +55,7 @@
 
 #include <cassert>
 #include <algorithm>
+#include <utility>
 
 #define S1(x) #x
 #define S2(x) S1(x)
@@ -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;
+        }
+    }
+
     PKIIdentityHandle* ih = &PKIIdentityHandle::narrow(*get_identity_handle(exception));
 
     (*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(
     {
         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);
@@ -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(),
@@ -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(),

diff --git a/test/blackbox/common/BlackboxTestsSecurity.cpp b/test/blackbox/common/BlackboxTestsSecurity.cpp
diff --git a/test/dds/communication/security/secure_msg_crypto_besteffort_pub_profile.xml b/test/dds/communication/security/secure_msg_crypto_besteffort_pub_profile.xml
@@ -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>

diff --git a/test/dds/communication/security/secure_msg_submsg_crypto_besteffort_pub_profile.xml b/test/dds/communication/security/secure_msg_submsg_crypto_besteffort_pub_profile.xml
@@ -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>

diff --git a/test/dds/communication/security/secure_msg_submsg_crypto_besteffort_sub_profile.xml b/test/dds/communication/security/secure_msg_submsg_crypto_besteffort_sub_profile.xml
@@ -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>

diff --git a/test/dds/communication/security/secure_submsg_crypto_besteffort_pub_profile.xml b/test/dds/communication/security/secure_submsg_crypto_besteffort_pub_profile.xml
@@ -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>

diff --git a/test/dds/communication/security/secure_submsg_crypto_besteffort_sub_profile.xml b/test/dds/communication/security/secure_submsg_crypto_besteffort_sub_profile.xml
@@ -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>

diff --git a/test/dds/communication/security/simple_secure_besteffort_pub_profile.xml b/test/dds/communication/security/simple_secure_besteffort_pub_profile.xml
@@ -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>

diff --git a/test/dds/communication/security/simple_secure_besteffort_sub_profile.xml b/test/dds/communication/security/simple_secure_besteffort_sub_profile.xml
@@ -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>

diff --git a/test/unittest/security/authentication/AuthenticationPluginTests.hpp b/test/unittest/security/authentication/AuthenticationPluginTests.hpp
@@ -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,

diff --git a/test/unittest/security/authentication/BuiltinPKIDHTests.cpp b/test/unittest/security/authentication/BuiltinPKIDHTests.cpp
@@ -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;

diff --git a/versions.md b/versions.md
@@ -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
 --------------