From 8ccd2be5fb8855eb8b0b9a91f56f68195ef830f5 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Tue, 10 Jan 2023 08:49:16 +0100 Subject: [PATCH 1/6] Adding overload of get_security_files. Signed-off-by: Miguel Company --- .../include/rmw_dds_common/security.hpp | 32 +++++++++++++++++++ rmw_dds_common/src/security.cpp | 8 +++++ 2 files changed, 40 insertions(+) diff --git a/rmw_dds_common/include/rmw_dds_common/security.hpp b/rmw_dds_common/include/rmw_dds_common/security.hpp index 3b9f0f5..fb3b14d 100644 --- a/rmw_dds_common/include/rmw_dds_common/security.hpp +++ b/rmw_dds_common/include/rmw_dds_common/security.hpp @@ -52,6 +52,38 @@ bool get_security_files( const std::string & prefix, const std::string & secure_root, std::unordered_map & result); +/// Get the set of security files in a security enclave. +/** + * This function will look through the passed in 'secure root' + * for a set of required filenames that must be in the enclave. + * If any of the required filenames are missing, the 'result' + * will be empty and the function will return false. + * If all of the required filenames are present, then this function + * will fill in the 'result' map with a key-value pair of + * friendy name -> filename. If the prefix is not empty, then + * the prefix will be applied to the filename. + * + * The friendly names that this function will currently fill in are: + * IDENTITY_CA + * CERTIFICATE + * PRIVATE_KEY + * PERMISSIONS_CA + * GOVERNANCE + * PERMISSIONS + * + * \param[in] supports_pkcs11 Whether the RMW has support for PKCS#11 URIs. + * \param[in] prefix An optional prefix to apply to the filenames when storing them. + * \param[in] secure_root The path to the security enclave to look at. + * \param[out] result The map where the friendly name -> filename pairs are stored. + * \return `true` if all required files exist in the security enclave, `false` otherwise. + */ +RMW_DDS_COMMON_PUBLIC +bool get_security_files( + bool supports_pkcs11, + const std::string & prefix, + const std::string & secure_root, + std::unordered_map & result); + } // namespace rmw_dds_common #endif // RMW_DDS_COMMON__SECURITY_HPP_ diff --git a/rmw_dds_common/src/security.cpp b/rmw_dds_common/src/security.cpp index 701ebba..f0a70b6 100644 --- a/rmw_dds_common/src/security.cpp +++ b/rmw_dds_common/src/security.cpp @@ -25,6 +25,14 @@ namespace rmw_dds_common bool get_security_files( const std::string & prefix, const std::string & secure_root, std::unordered_map & result) +{ + return get_security_files(false, prefix, secure_root, result); +} + +bool get_security_files( + bool /* supports_pkcs11 */, + const std::string & prefix, const std::string & secure_root, + std::unordered_map & result) { const std::unordered_map required_files{ {"IDENTITY_CA", "identity_ca.cert.pem"}, From 9d717426fc2e1d23736722bd9f4eecdb0d03e39c Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Tue, 10 Jan 2023 12:40:32 +0100 Subject: [PATCH 2/6] Implement changes to support .p11 files. Signed-off-by: Miguel Company --- rmw_dds_common/src/security.cpp | 109 +++++++++++++++++++++++++++----- 1 file changed, 94 insertions(+), 15 deletions(-) diff --git a/rmw_dds_common/src/security.cpp b/rmw_dds_common/src/security.cpp index f0a70b6..683092b 100644 --- a/rmw_dds_common/src/security.cpp +++ b/rmw_dds_common/src/security.cpp @@ -13,15 +13,60 @@ // limitations under the License. #include +#include +#include #include #include #include +#include #include "rmw_dds_common/security.hpp" namespace rmw_dds_common { +// Processor for security attributes with FILE URI +static bool process_file_uri_security_file( + bool /*supports_pkcs11*/, + const std::string & prefix, + const std::filesystem::path & full_path, + std::string & result) +{ + if (!std::filesystem::is_regular_file(full_path)) { + return false; + } + result = prefix + full_path.generic_string(); + return true; +} + +// Processor for security attributes with PKCS#11 URI +static bool process_pkcs_uri_security_file( + bool supports_pkcs11, + const std::string & /*prefix*/, + const std::filesystem::path & full_path, + std::string & result) +{ + if (!supports_pkcs11) { + return false; + } + + const std::string p11_prefix("pkcs11:"); + + std::ifstream ifs(full_path); + if (!ifs.is_open()) { + return false; + } + + if (!(ifs >> result)) { + return false; + } + if (result.find(p11_prefix) != 0) { + return false; + } + + return true; +} + bool get_security_files( const std::string & prefix, const std::string & secure_root, std::unordered_map & result) @@ -30,32 +75,66 @@ bool get_security_files( } bool get_security_files( - bool /* supports_pkcs11 */, - const std::string & prefix, const std::string & secure_root, + bool supports_pkcs11, + const std::string & prefix, + const std::string & secure_root, std::unordered_map & result) { - const std::unordered_map required_files{ - {"IDENTITY_CA", "identity_ca.cert.pem"}, - {"CERTIFICATE", "cert.pem"}, - {"PRIVATE_KEY", "key.pem"}, - {"PERMISSIONS_CA", "permissions_ca.cert.pem"}, - {"GOVERNANCE", "governance.p7s"}, - {"PERMISSIONS", "permissions.p7s"}, + using std::placeholders::_1; + using std::placeholders::_2; + using std::placeholders::_3; + using std::placeholders::_4; + using security_file_processor = + std::function; + using processor_vector = + std::vector>; + + + // Key: the security attribute + // Value: ordered sequence of pairs. Each pair contains one possible file name + // for the attribute and the corresponding processor method + // Pairs are ordered by priority: the first one matching is used. + const std::unordered_map required_files{ + {"IDENTITY_CA", { + {"identity_ca.cert.p11", std::bind(process_pkcs_uri_security_file, _1, _2, _3, _4)}, + {"identity_ca.cert.pem", std::bind(process_file_uri_security_file, _1, _2, _3, _4)}}}, + {"CERTIFICATE", { + {"cert.p11", std::bind(process_pkcs_uri_security_file, _1, _2, _3, _4)}, + {"cert.pem", std::bind(process_file_uri_security_file, _1, _2, _3, _4)}}}, + {"PRIVATE_KEY", { + {"key.p11", std::bind(process_pkcs_uri_security_file, _1, _2, _3, _4)}, + {"key.pem", std::bind(process_file_uri_security_file, _1, _2, _3, _4)}}}, + {"PERMISSIONS_CA", { + {"permissions_ca.cert.p11", std::bind(process_pkcs_uri_security_file, _1, _2, _3, _4)}, + {"permissions_ca.cert.pem", std::bind(process_file_uri_security_file, _1, _2, _3, _4)}}}, + {"GOVERNANCE", { + {"governance.p7s", std::bind(process_file_uri_security_file, _1, _2, _3, _4)}}}, + {"PERMISSIONS", { + {"permissions.p7s", std::bind(process_file_uri_security_file, _1, _2, _3, _4)}}}, }; const std::unordered_map optional_files{ {"CRL", "crl.pem"}, }; - for (const std::pair & el : required_files) { - std::filesystem::path full_path(secure_root); - full_path /= el.second; - if (!std::filesystem::is_regular_file(full_path)) { + for (const std::pair>> & el : required_files) + { + std::string attribute_value; + bool processed = false; + for (auto & proc : el.second) { + std::filesystem::path full_path(secure_root); + full_path /= proc.first; + if (proc.second(supports_pkcs11, prefix, full_path, attribute_value)) { + processed = true; + break; + } + } + if (!processed) { result.clear(); return false; } - - result[el.first] = prefix + full_path.generic_string(); + result[el.first] = attribute_value; } for (const std::pair & el : optional_files) { From 5480d5bb4374cf999fda32ab853709f489102ffd Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Fri, 5 Apr 2024 10:21:37 +0200 Subject: [PATCH 3/6] Security tests converted into TEST_P Signed-off-by: Miguel Company --- rmw_dds_common/test/test_security.cpp | 31 ++++++++++++++++++++------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/rmw_dds_common/test/test_security.cpp b/rmw_dds_common/test/test_security.cpp index e1fa36c..bdff24a 100644 --- a/rmw_dds_common/test/test_security.cpp +++ b/rmw_dds_common/test/test_security.cpp @@ -22,7 +22,9 @@ #include "rmw_dds_common/security.hpp" -TEST(test_security, files_exist_no_prefix) +class test_security : public ::testing::TestWithParam {}; + +TEST_P(test_security, files_exist_no_prefix) { std::filesystem::path dir = std::filesystem::path("./test_folder"); std::filesystem::remove_all(dir); @@ -42,7 +44,8 @@ TEST(test_security, files_exist_no_prefix) } std::unordered_map security_files; - ASSERT_TRUE(rmw_dds_common::get_security_files("", dir.generic_string(), security_files)); + ASSERT_TRUE( + rmw_dds_common::get_security_files(GetParam(), "", dir.generic_string(), security_files)); EXPECT_EQ( security_files["IDENTITY_CA"], @@ -64,7 +67,7 @@ TEST(test_security, files_exist_no_prefix) std::filesystem::path("./test_folder/permissions.p7s").generic_string()); } -TEST(test_security, files_exist_with_prefix) +TEST_P(test_security, files_exist_with_prefix) { std::filesystem::path dir = std::filesystem::path("./test_folder"); std::filesystem::remove_all(dir); @@ -84,7 +87,9 @@ TEST(test_security, files_exist_with_prefix) } std::unordered_map security_files; - ASSERT_TRUE(rmw_dds_common::get_security_files("file://", dir.generic_string(), security_files)); + ASSERT_TRUE( + rmw_dds_common::get_security_files( + GetParam(), "file://", dir.generic_string(), security_files)); EXPECT_EQ( security_files["IDENTITY_CA"], @@ -106,7 +111,7 @@ TEST(test_security, files_exist_with_prefix) "file://" + std::filesystem::path("./test_folder/permissions.p7s").generic_string()); } -TEST(test_security, file_missing) +TEST_P(test_security, file_missing) { std::filesystem::path dir = std::filesystem::path("./test_folder"); std::filesystem::remove_all(dir); @@ -126,11 +131,12 @@ TEST(test_security, file_missing) } std::unordered_map security_files; - ASSERT_FALSE(rmw_dds_common::get_security_files("", dir.generic_string(), security_files)); + ASSERT_FALSE( + rmw_dds_common::get_security_files(GetParam(), "", dir.generic_string(), security_files)); ASSERT_EQ(security_files.size(), 0UL); } -TEST(test_security, optional_file_exist) +TEST_P(test_security, optional_file_exist) { std::filesystem::path dir = std::filesystem::path("./test_folder"); std::filesystem::remove_all(dir); @@ -150,7 +156,8 @@ TEST(test_security, optional_file_exist) } std::unordered_map security_files; - ASSERT_TRUE(rmw_dds_common::get_security_files("", dir.generic_string(), security_files)); + ASSERT_TRUE( + rmw_dds_common::get_security_files(GetParam(), "", dir.generic_string(), security_files)); EXPECT_EQ( security_files["IDENTITY_CA"], @@ -175,3 +182,11 @@ TEST(test_security, optional_file_exist) security_files["CRL"], std::filesystem::path("./test_folder/crl.pem").generic_string()); } + +INSTANTIATE_TEST_SUITE_P( + test_security, + test_security, + ::testing::Values(false, true), + [](const testing::TestParamInfo & info) { + return info.param ? "with_pkcs11_support" : "with_no_pkcs11_support"; + }); From 411cac5d69bba4eaf2a65ef3f5763100e59371a3 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Fri, 5 Apr 2024 11:09:02 +0200 Subject: [PATCH 4/6] Adding pkcs11 specific tests. Signed-off-by: Miguel Company --- rmw_dds_common/test/test_security.cpp | 243 ++++++++++++++++++++++++++ 1 file changed, 243 insertions(+) diff --git a/rmw_dds_common/test/test_security.cpp b/rmw_dds_common/test/test_security.cpp index bdff24a..c02b264 100644 --- a/rmw_dds_common/test/test_security.cpp +++ b/rmw_dds_common/test/test_security.cpp @@ -183,6 +183,249 @@ TEST_P(test_security, optional_file_exist) std::filesystem::path("./test_folder/crl.pem").generic_string()); } +TEST_P(test_security, wrong_pkcs11_file_ignored) +{ + std::filesystem::path dir = std::filesystem::path("./test_folder"); + std::filesystem::remove_all(dir); + EXPECT_TRUE(std::filesystem::create_directories(dir)); + EXPECT_TRUE(std::filesystem::exists(dir)); + EXPECT_TRUE(std::filesystem::is_directory(dir)); + + std::array required_files = { + "identity_ca.cert.pem", "cert.pem", "key.pem", + "permissions_ca.cert.pem", "governance.p7s", "permissions.p7s", + "identity_ca.cert.p11", "cert.p11", "key.p11", + "permissions_ca.cert.p11" + }; + for (const std::string & filename : required_files) { + std::filesystem::path full_path = dir / filename; + std::ofstream output_buffer{full_path.generic_string()}; + output_buffer << "test"; + ASSERT_TRUE(std::filesystem::exists(full_path)); + } + + std::unordered_map security_files; + ASSERT_TRUE( + rmw_dds_common::get_security_files(GetParam(), "", dir.generic_string(), security_files)); + + EXPECT_EQ( + security_files["IDENTITY_CA"], + std::filesystem::path("./test_folder/identity_ca.cert.pem").generic_string()); + EXPECT_EQ( + security_files["CERTIFICATE"], + std::filesystem::path("./test_folder/cert.pem").generic_string()); + EXPECT_EQ( + security_files["PRIVATE_KEY"], + std::filesystem::path("./test_folder/key.pem").generic_string()); + EXPECT_EQ( + security_files["PERMISSIONS_CA"], + std::filesystem::path("./test_folder/permissions_ca.cert.pem").generic_string()); + EXPECT_EQ( + security_files["GOVERNANCE"], + std::filesystem::path("./test_folder/governance.p7s").generic_string()); + EXPECT_EQ( + security_files["PERMISSIONS"], + std::filesystem::path("./test_folder/permissions.p7s").generic_string()); +} + +TEST_P(test_security, pkcs11_support_check) +{ + std::filesystem::path dir = std::filesystem::path("./test_folder"); + std::filesystem::remove_all(dir); + EXPECT_TRUE(std::filesystem::create_directories(dir)); + EXPECT_TRUE(std::filesystem::exists(dir)); + EXPECT_TRUE(std::filesystem::is_directory(dir)); + + std::array required_files = { + "identity_ca.cert.pem", "cert.pem", "key.pem", + "permissions_ca.cert.pem", "governance.p7s", "permissions.p7s" + }; + for (const std::string & filename : required_files) { + std::filesystem::path full_path = dir / filename; + std::ofstream output_buffer{full_path.generic_string()}; + output_buffer << "test"; + ASSERT_TRUE(std::filesystem::exists(full_path)); + } + + std::array pkcs11_files = { + "identity_ca.cert.p11", "cert.p11", "key.p11", + "permissions_ca.cert.p11" + }; + for (const std::string & filename : pkcs11_files) { + std::filesystem::path full_path = dir / filename; + std::ofstream output_buffer{full_path.generic_string()}; + output_buffer << "pkcs11://" << filename; + ASSERT_TRUE(std::filesystem::exists(full_path)); + } + + std::unordered_map security_files; + ASSERT_TRUE( + rmw_dds_common::get_security_files(GetParam(), "", dir.generic_string(), security_files)); + + if (GetParam()) { + EXPECT_EQ( + security_files["IDENTITY_CA"], + "pkcs11://identity_ca.cert.p11"); + EXPECT_EQ( + security_files["CERTIFICATE"], + "pkcs11://cert.p11"); + EXPECT_EQ( + security_files["PRIVATE_KEY"], + "pkcs11://key.p11"); + EXPECT_EQ( + security_files["PERMISSIONS_CA"], + "pkcs11://permissions_ca.cert.p11"); + } else { + EXPECT_EQ( + security_files["IDENTITY_CA"], + std::filesystem::path("./test_folder/identity_ca.cert.pem").generic_string()); + EXPECT_EQ( + security_files["CERTIFICATE"], + std::filesystem::path("./test_folder/cert.pem").generic_string()); + EXPECT_EQ( + security_files["PRIVATE_KEY"], + std::filesystem::path("./test_folder/key.pem").generic_string()); + EXPECT_EQ( + security_files["PERMISSIONS_CA"], + std::filesystem::path("./test_folder/permissions_ca.cert.pem").generic_string()); + } + EXPECT_EQ( + security_files["GOVERNANCE"], + std::filesystem::path("./test_folder/governance.p7s").generic_string()); + EXPECT_EQ( + security_files["PERMISSIONS"], + std::filesystem::path("./test_folder/permissions.p7s").generic_string()); +} + +TEST_P(test_security, only_pkcs11_present) +{ + std::filesystem::path dir = std::filesystem::path("./test_folder"); + std::filesystem::remove_all(dir); + EXPECT_TRUE(std::filesystem::create_directories(dir)); + EXPECT_TRUE(std::filesystem::exists(dir)); + EXPECT_TRUE(std::filesystem::is_directory(dir)); + + std::array required_files = { + "governance.p7s", "permissions.p7s" + }; + for (const std::string & filename : required_files) { + std::filesystem::path full_path = dir / filename; + std::ofstream output_buffer{full_path.generic_string()}; + output_buffer << "test"; + ASSERT_TRUE(std::filesystem::exists(full_path)); + } + + std::array pkcs11_files = { + "identity_ca.cert.p11", "cert.p11", "key.p11", + "permissions_ca.cert.p11" + }; + for (const std::string & filename : pkcs11_files) { + std::filesystem::path full_path = dir / filename; + std::ofstream output_buffer{full_path.generic_string()}; + output_buffer << "pkcs11://" << filename; + ASSERT_TRUE(std::filesystem::exists(full_path)); + } + + std::unordered_map security_files; + + if (GetParam()) { + ASSERT_TRUE( + rmw_dds_common::get_security_files(GetParam(), "", dir.generic_string(), security_files)); + EXPECT_EQ( + security_files["IDENTITY_CA"], + "pkcs11://identity_ca.cert.p11"); + EXPECT_EQ( + security_files["CERTIFICATE"], + "pkcs11://cert.p11"); + EXPECT_EQ( + security_files["PRIVATE_KEY"], + "pkcs11://key.p11"); + EXPECT_EQ( + security_files["PERMISSIONS_CA"], + "pkcs11://permissions_ca.cert.p11"); + EXPECT_EQ( + security_files["GOVERNANCE"], + std::filesystem::path("./test_folder/governance.p7s").generic_string()); + EXPECT_EQ( + security_files["PERMISSIONS"], + std::filesystem::path("./test_folder/permissions.p7s").generic_string()); + } else { + ASSERT_FALSE( + rmw_dds_common::get_security_files(GetParam(), "", dir.generic_string(), security_files)); + ASSERT_EQ(security_files.size(), 0UL); + } +} + +TEST_P(test_security, pkcs11_prefix_ignored) +{ + std::filesystem::path dir = std::filesystem::path("./test_folder"); + std::filesystem::remove_all(dir); + EXPECT_TRUE(std::filesystem::create_directories(dir)); + EXPECT_TRUE(std::filesystem::exists(dir)); + EXPECT_TRUE(std::filesystem::is_directory(dir)); + + std::array required_files = { + "identity_ca.cert.pem", "cert.pem", "key.pem", + "permissions_ca.cert.pem", "governance.p7s", "permissions.p7s" + }; + for (const std::string & filename : required_files) { + std::filesystem::path full_path = dir / filename; + std::ofstream output_buffer{full_path.generic_string()}; + output_buffer << "test"; + ASSERT_TRUE(std::filesystem::exists(full_path)); + } + + std::array pkcs11_files = { + "identity_ca.cert.p11", "cert.p11", "key.p11", + "permissions_ca.cert.p11" + }; + for (const std::string & filename : pkcs11_files) { + std::filesystem::path full_path = dir / filename; + std::ofstream output_buffer{full_path.generic_string()}; + output_buffer << "pkcs11://" << filename; + ASSERT_TRUE(std::filesystem::exists(full_path)); + } + + std::unordered_map security_files; + ASSERT_TRUE( + rmw_dds_common::get_security_files( + GetParam(), "file://", dir.generic_string(), security_files)); + + if (GetParam()) { + EXPECT_EQ( + security_files["IDENTITY_CA"], + "pkcs11://identity_ca.cert.p11"); + EXPECT_EQ( + security_files["CERTIFICATE"], + "pkcs11://cert.p11"); + EXPECT_EQ( + security_files["PRIVATE_KEY"], + "pkcs11://key.p11"); + EXPECT_EQ( + security_files["PERMISSIONS_CA"], + "pkcs11://permissions_ca.cert.p11"); + } else { + EXPECT_EQ( + security_files["IDENTITY_CA"], + "file://" + std::filesystem::path("./test_folder/identity_ca.cert.pem").generic_string()); + EXPECT_EQ( + security_files["CERTIFICATE"], + "file://" + std::filesystem::path("./test_folder/cert.pem").generic_string()); + EXPECT_EQ( + security_files["PRIVATE_KEY"], + "file://" + std::filesystem::path("./test_folder/key.pem").generic_string()); + EXPECT_EQ( + security_files["PERMISSIONS_CA"], + "file://" + std::filesystem::path("./test_folder/permissions_ca.cert.pem").generic_string()); + } + EXPECT_EQ( + security_files["GOVERNANCE"], + "file://" + std::filesystem::path("./test_folder/governance.p7s").generic_string()); + EXPECT_EQ( + security_files["PERMISSIONS"], + "file://" + std::filesystem::path("./test_folder/permissions.p7s").generic_string()); +} + INSTANTIATE_TEST_SUITE_P( test_security, test_security, From 66129b1cb5635cd2c8ee370d8121febb142bb4d2 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Fri, 5 Apr 2024 11:16:53 +0200 Subject: [PATCH 5/6] Group duplicated code Signed-off-by: Miguel Company --- rmw_dds_common/test/test_security.cpp | 101 +++++++++----------------- 1 file changed, 35 insertions(+), 66 deletions(-) diff --git a/rmw_dds_common/test/test_security.cpp b/rmw_dds_common/test/test_security.cpp index c02b264..5f58e1c 100644 --- a/rmw_dds_common/test/test_security.cpp +++ b/rmw_dds_common/test/test_security.cpp @@ -22,6 +22,30 @@ #include "rmw_dds_common/security.hpp" +// Utility to write test content on required files +template +static void write_test_content(const std::array & required_files) +{ + for (const std::string & filename : required_files) { + std::filesystem::path full_path = std::filesystem::path("./test_folder") / filename; + std::ofstream output_buffer{full_path.generic_string()}; + output_buffer << "test"; + ASSERT_TRUE(std::filesystem::exists(full_path)); + } +} + +// Utility to write pkcs11 content on required files +template +static void write_test_pkcs11_content(const std::array & pkcs11_files) +{ + for (const std::string & filename : pkcs11_files) { + std::filesystem::path full_path = std::filesystem::path("./test_folder") / filename; + std::ofstream output_buffer{full_path.generic_string()}; + output_buffer << "pkcs11://" << filename; + ASSERT_TRUE(std::filesystem::exists(full_path)); + } +} + class test_security : public ::testing::TestWithParam {}; TEST_P(test_security, files_exist_no_prefix) @@ -36,12 +60,7 @@ TEST_P(test_security, files_exist_no_prefix) "identity_ca.cert.pem", "cert.pem", "key.pem", "permissions_ca.cert.pem", "governance.p7s", "permissions.p7s" }; - for (const std::string & filename : required_files) { - std::filesystem::path full_path = dir / filename; - std::ofstream output_buffer{full_path.generic_string()}; - output_buffer << "test"; - ASSERT_TRUE(std::filesystem::exists(full_path)); - } + write_test_content(required_files); std::unordered_map security_files; ASSERT_TRUE( @@ -79,12 +98,7 @@ TEST_P(test_security, files_exist_with_prefix) "identity_ca.cert.pem", "cert.pem", "key.pem", "permissions_ca.cert.pem", "governance.p7s", "permissions.p7s" }; - for (const std::string & filename : required_files) { - std::filesystem::path full_path = dir / filename; - std::ofstream output_buffer{full_path.generic_string()}; - output_buffer << "test"; - ASSERT_TRUE(std::filesystem::exists(full_path)); - } + write_test_content(required_files); std::unordered_map security_files; ASSERT_TRUE( @@ -123,12 +137,7 @@ TEST_P(test_security, file_missing) "identity_ca.cert.pem", "cert.pem", "key.pem", "permissions_ca.cert.pem", "governance.p7s" }; - for (const std::string & filename : required_files) { - std::filesystem::path full_path = dir / filename; - std::ofstream output_buffer{full_path.generic_string()}; - output_buffer << "test"; - ASSERT_TRUE(std::filesystem::exists(full_path)); - } + write_test_content(required_files); std::unordered_map security_files; ASSERT_FALSE( @@ -148,12 +157,7 @@ TEST_P(test_security, optional_file_exist) "identity_ca.cert.pem", "cert.pem", "key.pem", "permissions_ca.cert.pem", "governance.p7s", "permissions.p7s", "crl.pem", }; - for (const std::string & filename : required_files) { - std::filesystem::path full_path = dir / filename; - std::ofstream output_buffer{full_path.generic_string()}; - output_buffer << "test"; - ASSERT_TRUE(std::filesystem::exists(full_path)); - } + write_test_content(required_files); std::unordered_map security_files; ASSERT_TRUE( @@ -197,12 +201,7 @@ TEST_P(test_security, wrong_pkcs11_file_ignored) "identity_ca.cert.p11", "cert.p11", "key.p11", "permissions_ca.cert.p11" }; - for (const std::string & filename : required_files) { - std::filesystem::path full_path = dir / filename; - std::ofstream output_buffer{full_path.generic_string()}; - output_buffer << "test"; - ASSERT_TRUE(std::filesystem::exists(full_path)); - } + write_test_content(required_files); std::unordered_map security_files; ASSERT_TRUE( @@ -240,23 +239,13 @@ TEST_P(test_security, pkcs11_support_check) "identity_ca.cert.pem", "cert.pem", "key.pem", "permissions_ca.cert.pem", "governance.p7s", "permissions.p7s" }; - for (const std::string & filename : required_files) { - std::filesystem::path full_path = dir / filename; - std::ofstream output_buffer{full_path.generic_string()}; - output_buffer << "test"; - ASSERT_TRUE(std::filesystem::exists(full_path)); - } + write_test_content(required_files); std::array pkcs11_files = { "identity_ca.cert.p11", "cert.p11", "key.p11", "permissions_ca.cert.p11" }; - for (const std::string & filename : pkcs11_files) { - std::filesystem::path full_path = dir / filename; - std::ofstream output_buffer{full_path.generic_string()}; - output_buffer << "pkcs11://" << filename; - ASSERT_TRUE(std::filesystem::exists(full_path)); - } + write_test_pkcs11_content(pkcs11_files); std::unordered_map security_files; ASSERT_TRUE( @@ -308,23 +297,13 @@ TEST_P(test_security, only_pkcs11_present) std::array required_files = { "governance.p7s", "permissions.p7s" }; - for (const std::string & filename : required_files) { - std::filesystem::path full_path = dir / filename; - std::ofstream output_buffer{full_path.generic_string()}; - output_buffer << "test"; - ASSERT_TRUE(std::filesystem::exists(full_path)); - } + write_test_content(required_files); std::array pkcs11_files = { "identity_ca.cert.p11", "cert.p11", "key.p11", "permissions_ca.cert.p11" }; - for (const std::string & filename : pkcs11_files) { - std::filesystem::path full_path = dir / filename; - std::ofstream output_buffer{full_path.generic_string()}; - output_buffer << "pkcs11://" << filename; - ASSERT_TRUE(std::filesystem::exists(full_path)); - } + write_test_pkcs11_content(pkcs11_files); std::unordered_map security_files; @@ -368,23 +347,13 @@ TEST_P(test_security, pkcs11_prefix_ignored) "identity_ca.cert.pem", "cert.pem", "key.pem", "permissions_ca.cert.pem", "governance.p7s", "permissions.p7s" }; - for (const std::string & filename : required_files) { - std::filesystem::path full_path = dir / filename; - std::ofstream output_buffer{full_path.generic_string()}; - output_buffer << "test"; - ASSERT_TRUE(std::filesystem::exists(full_path)); - } + write_test_content(required_files); std::array pkcs11_files = { "identity_ca.cert.p11", "cert.p11", "key.p11", "permissions_ca.cert.p11" }; - for (const std::string & filename : pkcs11_files) { - std::filesystem::path full_path = dir / filename; - std::ofstream output_buffer{full_path.generic_string()}; - output_buffer << "pkcs11://" << filename; - ASSERT_TRUE(std::filesystem::exists(full_path)); - } + write_test_pkcs11_content(pkcs11_files); std::unordered_map security_files; ASSERT_TRUE( From 305e4cc88ca6b8f59705c4c445d609ede74ab0fb Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Fri, 5 Apr 2024 21:04:16 +0200 Subject: [PATCH 6/6] Apply suggestions. Signed-off-by: Miguel Company --- rmw_dds_common/src/security.cpp | 1 - rmw_dds_common/test/test_security.cpp | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/rmw_dds_common/src/security.cpp b/rmw_dds_common/src/security.cpp index 683092b..c480d1a 100644 --- a/rmw_dds_common/src/security.cpp +++ b/rmw_dds_common/src/security.cpp @@ -89,7 +89,6 @@ bool get_security_files( using processor_vector = std::vector>; - // Key: the security attribute // Value: ordered sequence of pairs. Each pair contains one possible file name // for the attribute and the corresponding processor method diff --git a/rmw_dds_common/test/test_security.cpp b/rmw_dds_common/test/test_security.cpp index 5f58e1c..e6ac3a3 100644 --- a/rmw_dds_common/test/test_security.cpp +++ b/rmw_dds_common/test/test_security.cpp @@ -13,6 +13,7 @@ // limitations under the License. #include +#include #include #include #include