From 6c9e50cf3ae7c3a7aa5ffef3fc042da67d000d71 Mon Sep 17 00:00:00 2001 From: Dennis Potman Date: Mon, 2 Mar 2020 10:55:19 +0100 Subject: [PATCH] Fix for empty trusted CA dir Trusted CA dir in security configuration is optional, but participant creation currently fails if no or empty dir is provided. This commit fixes this issue and adds some tests for various trusted_ca_dir values. Signed-off-by: Dennis Potman --- src/ddsrt/src/filesystem/posix/filesystem.c | 7 +-- .../authentication/src/authentication.c | 10 +--- src/security/core/tests/authentication.c | 57 +++++++++++++++---- src/security/core/tests/handshake.c | 2 - 4 files changed, 53 insertions(+), 23 deletions(-) diff --git a/src/ddsrt/src/filesystem/posix/filesystem.c b/src/ddsrt/src/filesystem/posix/filesystem.c index 11b57ce..1db647c 100644 --- a/src/ddsrt/src/filesystem/posix/filesystem.c +++ b/src/ddsrt/src/filesystem/posix/filesystem.c @@ -87,14 +87,14 @@ char * ddsrt_file_normalize(const char *filepath) char *normPtr; norm = NULL; - if ((filepath != NULL) && (*filepath != '\0')) { - norm = ddsrt_malloc(strlen(filepath) + 1); + if (filepath != NULL) { + norm = ddsrt_malloc (strlen (filepath) + 1); /* replace any / or \ by DDSRT_FILESEPCHAR */ fpPtr = (char *) filepath; normPtr = norm; while (*fpPtr != '\0') { *normPtr = *fpPtr; - if ((*fpPtr == '/') || (*fpPtr == '\\')) { + if (*fpPtr == '/' || *fpPtr == '\\') { *normPtr = DDSRT_FILESEPCHAR; normPtr++; } else { @@ -106,7 +106,6 @@ char * ddsrt_file_normalize(const char *filepath) } *normPtr = '\0'; } - return norm; } diff --git a/src/security/builtin_plugins/authentication/src/authentication.c b/src/security/builtin_plugins/authentication/src/authentication.c index d6139d9..4d317d7 100644 --- a/src/security/builtin_plugins/authentication/src/authentication.c +++ b/src/security/builtin_plugins/authentication/src/authentication.c @@ -980,17 +980,13 @@ validate_local_identity( password = DDS_Security_Property_get_value(&participant_qos->property.value, PROPERTY_PASSWORD); - trusted_ca_dir = DDS_Security_Property_get_value(&participant_qos->property.value, PROPERTY_TRUSTED_CA_DIR); - - if( trusted_ca_dir ){ - result = get_trusted_ca_list(trusted_ca_dir, &(implementation->trustedCAList), ex ); - if (result != DDS_SECURITY_VALIDATION_OK) { + if (trusted_ca_dir && *trusted_ca_dir != '\0') + { + if ((result = get_trusted_ca_list (trusted_ca_dir, &(implementation->trustedCAList), ex)) != DDS_SECURITY_VALIDATION_OK) goto err_inv_trusted_ca_dir; - } } - result = load_X509_certificate(identityCaPEM, &identityCA, ex); if (result != DDS_SECURITY_VALIDATION_OK) { goto err_inv_identity_ca; diff --git a/src/security/core/tests/authentication.c b/src/security/core/tests/authentication.c index e981648..c1e022d 100644 --- a/src/security/core/tests/authentication.c +++ b/src/security/core/tests/authentication.c @@ -14,6 +14,7 @@ #include "dds/dds.h" #include "CUnit/Test.h" +#include "CUnit/Theory.h" #include "dds/version.h" #include "dds/ddsrt/cdtors.h" @@ -44,8 +45,7 @@ static const char *config = " ${TEST_IDENTITY_CERTIFICATE}" " ${TEST_IDENTITY_PRIVATE_KEY}" " ${TEST_IDENTITY_CA_CERTIFICATE}" - " testtext_Password_testtext" - " ." + " ${TRUSTED_CA_DIR:+}${TRUSTED_CA_DIR}${TRUSTED_CA_DIR:+}" " " " " " " @@ -61,6 +61,7 @@ static const char *config = #define DDS_DOMAINID1 0 #define DDS_DOMAINID2 1 +#define MAX_ADDITIONAL_CONF 255 static dds_entity_t g_domain1 = 0; static dds_entity_t g_participant1 = 0; @@ -68,7 +69,7 @@ static dds_entity_t g_participant1 = 0; static dds_entity_t g_domain2 = 0; static dds_entity_t g_participant2 = 0; -static void authentication_init(void) +static void authentication_init(bool different_ca, const char * trusted_ca_dir, bool exp_pp_fail) { struct kvp governance_vars[] = { { "ALLOW_UNAUTH_PP", "false" }, @@ -82,38 +83,74 @@ static void authentication_init(void) { "TEST_IDENTITY_CERTIFICATE", TEST_IDENTITY_CERTIFICATE }, { "TEST_IDENTITY_PRIVATE_KEY", TEST_IDENTITY_PRIVATE_KEY }, { "TEST_IDENTITY_CA_CERTIFICATE", TEST_IDENTITY_CA_CERTIFICATE }, + { "TRUSTED_CA_DIR", trusted_ca_dir }, { NULL, NULL } }; + struct kvp config_vars2[] = { { "GOVERNANCE_DATA", gov_config_signed }, { "TEST_IDENTITY_CERTIFICATE", TEST_IDENTITY2_CERTIFICATE }, { "TEST_IDENTITY_PRIVATE_KEY", TEST_IDENTITY2_PRIVATE_KEY }, { "TEST_IDENTITY_CA_CERTIFICATE", TEST_IDENTITY_CA2_CERTIFICATE }, + { "TRUSTED_CA_DIR", trusted_ca_dir }, { NULL, NULL } }; char *conf1 = ddsrt_expand_vars (config, &expand_lookup_vars_env, config_vars1); char *conf2 = ddsrt_expand_vars (config, &expand_lookup_vars_env, config_vars2); g_domain1 = dds_create_domain (DDS_DOMAINID1, conf1); - g_domain2 = dds_create_domain (DDS_DOMAINID2, conf2); + g_domain2 = dds_create_domain (DDS_DOMAINID2, different_ca ? conf2 : conf1); dds_free (conf1); dds_free (conf2); ddsrt_free (gov_config_signed); - CU_ASSERT_FATAL ((g_participant1 = dds_create_participant (DDS_DOMAINID1, NULL, NULL)) > 0); - CU_ASSERT_FATAL ((g_participant2 = dds_create_participant (DDS_DOMAINID2, NULL, NULL)) > 0); + g_participant1 = dds_create_participant (DDS_DOMAINID1, NULL, NULL); + g_participant2 = dds_create_participant (DDS_DOMAINID2, NULL, NULL); + if (exp_pp_fail) + { + CU_ASSERT_FATAL (g_participant1 <= 0); + CU_ASSERT_FATAL (g_participant2 <= 0); + } + else + { + CU_ASSERT_FATAL (g_participant1 > 0); + CU_ASSERT_FATAL (g_participant2 > 0); + } } -static void authentication_fini(void) +static void authentication_fini(bool delete_pp) { - CU_ASSERT_EQUAL_FATAL (dds_delete (g_participant1), DDS_RETCODE_OK); - CU_ASSERT_EQUAL_FATAL (dds_delete (g_participant2), DDS_RETCODE_OK); + if (delete_pp) + { + CU_ASSERT_EQUAL_FATAL (dds_delete (g_participant1), DDS_RETCODE_OK); + CU_ASSERT_EQUAL_FATAL (dds_delete (g_participant2), DDS_RETCODE_OK); + } CU_ASSERT_EQUAL_FATAL (dds_delete (g_domain1), DDS_RETCODE_OK); CU_ASSERT_EQUAL_FATAL (dds_delete (g_domain2), DDS_RETCODE_OK); } -CU_Test(ddssec_authentication, different_ca, .init = authentication_init, .fini = authentication_fini) +CU_Test(ddssec_authentication, different_ca) { + authentication_init (true, NULL, false); validate_handshake (DDS_DOMAINID1, true, NULL, true, "error: unable to get local issuer certificate"); validate_handshake (DDS_DOMAINID2, true, NULL, true, "error: unable to get local issuer certificate"); + authentication_fini (true); +} + + +CU_TheoryDataPoints(ddssec_authentication, trusted_ca_dir) = { + CU_DataPoints(const char *, "", ".", "/nonexisting", NULL), + CU_DataPoints(bool, false, false, true, false) +}; + +CU_Theory((const char * ca_dir, bool exp_fail), ddssec_authentication, trusted_ca_dir) +{ + printf("Testing custom CA dir: %s\n", ca_dir); + authentication_init (false, ca_dir, exp_fail); + if (!exp_fail) + { + validate_handshake (DDS_DOMAINID1, false, NULL, false, NULL); + validate_handshake (DDS_DOMAINID2, false, NULL, false, NULL); + } + authentication_fini (!exp_fail); } diff --git a/src/security/core/tests/handshake.c b/src/security/core/tests/handshake.c index 6de2876..2eb52fc 100644 --- a/src/security/core/tests/handshake.c +++ b/src/security/core/tests/handshake.c @@ -44,8 +44,6 @@ static const char *config = " "TEST_IDENTITY_CERTIFICATE"" " "TEST_IDENTITY_PRIVATE_KEY"" " "TEST_IDENTITY_CA_CERTIFICATE"" - " testtext_Password_testtext" - " ." " " " " " "