[Interoprability] Key agreement and digital signature identifiers verification.

[lexamor]

Is there an already existing issue for this?

• [X] I have searched the existing issues

Expected behavior

Algorithms identifier should be verified correctly in some reliable way.

Current behavior

Verification fails if the incoming identifier/payload contains an embedded null-terminator.

Steps to reproduce

1. Configure security (for the both fastdds&cyclone client apps).
2. Launch them

*fastdds client should be the initiator of the handshake since the issue is only reproducible for reply parsing. During the request parsing strcmp function is used.

Fast DDS version/commit

2.9.1

Platform/Architecture

Ubuntu Focal 20.04 amd64

Transport layer

UDPv4

Additional context

On the FastDDS side during the handshake response processing for the verification of the identifiers of the used digital signature algorithm and the key agreement algorithm the std::string compare method is used. But in case the incoming payload contains an embedded null-terminated symbol at the end the verification fails.

// Check signature algorithm
std::string s_dsign_algo(dsign_algo->begin(), dsign_algo->end());
if (s_dsign_algo.compare(RSA_SHA256) != 0 &&
        s_dsign_algo.compare(ECDSA_SHA256) != 0)
{
    ...
    return ValidationResult_t::VALIDATION_FAILED
}

// Check key agreement algorithm
std::string s_kagree_algo(kagree_algo->begin(), kagree_algo->end());
if (s_kagree_algo.compare(handshake_handle->kagree_alg_) != 0)
if (strcmp(handshake_handle->kagree_alg_.c_str(), s_kagree_algo.c_str()) != 0)
{
    ...
    return ValidationResult_t::VALIDATION_FAILED
}

CycloneDDS implementation:

DDS_Security_BinaryProperty_set_by_string(&tokens[tokidx++], DDS_AUTHTOKEN_PROP_C_DSIGN_ALGO, get_dsign_algo(localIdent->dsignAlgoKind));
DDS_Security_BinaryProperty_set_by_string(&tokens[tokidx++], DDS_AUTHTOKEN_PROP_C_KAGREE_ALGO, get_kagree_algo(localIdent->kagreeAlgoKind));

... 
DDS_Security_BinaryProperty_set_by_string(
     DDS_Security_BinaryProperty_t *bp,
     const char *name,
     const char *data)
{
...
    length = (uint32_t) strlen(data) + 1;
    DDS_Security_BinaryProperty_set_by_value(bp, name, (unsigned char *)data, length);
}`

Possible patch to resolve the issue:

diff --git a/src/cpp/security/authentication/PKIDH.cpp b/src/cpp/security/authentication/PKIDH.cpp
index 5bb7c4a81..6d8d010d1 100644
--- a/src/cpp/security/authentication/PKIDH.cpp
+++ b/src/cpp/security/authentication/PKIDH.cpp
@@ -1919,8 +1919,8 @@ ValidationResult_t PKIDH::process_handshake_request(
 
     // Check signature algorithm
     std::string s_dsign_algo(dsign_algo->begin(), dsign_algo->end());
-    if (s_dsign_algo.compare(RSA_SHA256) != 0 &&
-            s_dsign_algo.compare(ECDSA_SHA256) != 0)
+    if (strcmp(RSA_SHA256, s_dsign_algo.c_str()) != 0 &&
+            strcmp(ECDSA_SHA256, s_dsign_algo.c_str()) != 0)
     {
         WARNING_SECURITY_LOGGING("PKIDH", std::string("Not supported signature algorithm (") + s_dsign_algo + ")");
         return ValidationResult_t::VALIDATION_FAILED;
@@ -1939,7 +1939,7 @@ ValidationResult_t PKIDH::process_handshake_request(
 
     // Check key agreement algorithm
     std::string s_kagree_algo(kagree_algo->begin(), kagree_algo->end());
-    if (s_kagree_algo.compare(handshake_handle->kagree_alg_) != 0)
+    if (strcmp(handshake_handle->kagree_alg_.c_str(), s_kagree_algo.c_str()) != 0)
     {
         WARNING_SECURITY_LOGGING("PKIDH", std::string("Invalid key agreement algorithm. Received ") +
                 s_kagree_algo + ", expected " + handshake_handle->kagree_alg_);

XML configuration file

No response

Relevant log output

Not supported signature algorithm 
...
Invalid key agreement algorithm. Received

Network traffic capture

No response