Deprecated security methods (#77)

Signed-off-by: Alejandro Hernandez Cordero <[email protected]>

Author: ahcorde

rmw_dds_common/CMakeLists.txt

@@ -16,12 +16,14 @@ find_package(ament_cmake REQUIRED)
 find_package(rcpputils REQUIRED)
 find_package(rcutils REQUIRED)
 find_package(rmw REQUIRED)
+find_package(rmw_security_common REQUIRED)
 find_package(rosidl_default_generators REQUIRED)
 find_package(rosidl_runtime_c REQUIRED)
 
 ament_add_default_options()
 ament_export_dependencies(ament_cmake_core)
 ament_export_dependencies(rcutils)
+ament_export_dependencies(rmw_security_common)
 ament_export_dependencies(rmw)
 
 rosidl_generate_interfaces(
@@ -43,6 +45,7 @@ set_target_properties(${PROJECT_NAME}_library
   PROPERTIES OUTPUT_NAME ${PROJECT_NAME})
 target_link_libraries(${PROJECT_NAME}_library PUBLIC
   rcutils::rcutils
+  rmw_security_common::rmw_security_common_library
   rmw::rmw)
 target_link_libraries(${PROJECT_NAME}_library PRIVATE
   rcpputils::rcpputils

rmw_dds_common/include/rmw_dds_common/security.hpp

@@ -47,6 +47,7 @@ namespace rmw_dds_common
  * \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.
  */
+[[deprecated("This function is now available in rmw_security_common")]]
 RMW_DDS_COMMON_PUBLIC
 bool get_security_files(
   const std::string & prefix, const std::string & secure_root,
@@ -77,6 +78,7 @@ bool get_security_files(
  * \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.
  */
+[[deprecated("This function is now available in rmw_security_common")]]
 RMW_DDS_COMMON_PUBLIC
 bool get_security_files(
   bool supports_pkcs11,

rmw_dds_common/package.xml

@@ -20,6 +20,7 @@
   <depend>rcutils</depend>
   <depend>rcpputils</depend>
   <depend>rmw</depend>
+  <depend>rmw_security_common</depend>
   <depend>rosidl_runtime_c</depend>
   <depend>rosidl_runtime_cpp</depend>
 

rmw_dds_common/src/security.cpp

@@ -21,57 +21,38 @@
 #include <vector>
 
 #include "rmw_dds_common/security.hpp"
+#include "rmw_security_common/security.hpp"
+#include "rmw/error_handling.h"
 
-namespace rmw_dds_common
-{
+#include "rcpputils/scope_exit.hpp"
 
-// 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;
-}
+#include "rcutils/allocator.h"
+#include "rcutils/types/string_map.h"
 
-// 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)
+namespace rmw_dds_common
 {
-  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<std::string, std::string> & result)
 {
+#if !defined(_WIN32)
+# pragma GCC diagnostic push
+# pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+# ifdef __clang__
+#  pragma clang diagnostic push
+#  pragma clang diagnostic ignored "-Wdeprecated-declarations"
+# endif
+#else  // !defined(_WIN32)
+# pragma warning(push)
+# pragma warning(disable: 4996)
+#endif
   return get_security_files(false, prefix, secure_root, result);
+#if !defined(_WIN32)
+# pragma GCC diagnostic pop
+#else  // !defined(_WIN32)
+# pragma warning(pop)
+#endif
 }
 
 bool get_security_files(
@@ -80,68 +61,40 @@ bool get_security_files(
   const std::string & secure_root,
   std::unordered_map<std::string, std::string> & result)
 {
-  using std::placeholders::_1;
-  using std::placeholders::_2;
-  using std::placeholders::_3;
-  using std::placeholders::_4;
-  using security_file_processor =
-    std::function<bool (bool, const std::string &, const std::filesystem::path &, std::string &)>;
-  using processor_vector =
-    std::vector<std::pair<std::string, security_file_processor>>;
-
-  // 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<std::string, processor_vector> 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)}}},
-  };
+  rcutils_allocator_t allocator = rcutils_get_default_allocator();
+  rcutils_string_map_t security_files = rcutils_get_zero_initialized_string_map();
+  rcutils_ret_t ret = rcutils_string_map_init(&security_files, 0, allocator);
+
+  auto scope_exit_ws = rcpputils::make_scope_exit(
+    [&security_files]()
+    {
+      rcutils_ret_t ret = rcutils_string_map_fini(&security_files);
+      if (ret != RMW_RET_OK) {
+        RMW_SET_ERROR_MSG("error cleaning string map memory");
+      }
+    });
 
-  const std::unordered_map<std::string, std::string> optional_files{
-    {"CRL", "crl.pem"},
-  };
+  if (ret != RCUTILS_RET_OK) {
+    RMW_SET_ERROR_MSG("error initializin map");
+    return false;
+  }
 
-  for (const std::pair<const std::string,
-    std::vector<std::pair<std::string, security_file_processor>>> & 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] = attribute_value;
+  ret = get_security_files_support_pkcs(
+    supports_pkcs11, prefix.c_str(), secure_root.c_str(), &security_files);
+  if (ret != RCUTILS_RET_OK) {
+    RMW_SET_ERROR_MSG("error calling get_security_files_support_pkcs");
+    return false;
   }
 
-  for (const std::pair<const std::string, std::string> & el : optional_files) {
-    std::filesystem::path full_path(secure_root);
-    full_path /= el.second;
-    if (std::filesystem::is_regular_file(full_path)) {
-      result[el.first] = prefix + full_path.generic_string();
+  const char * key = rcutils_string_map_get_next_key(&security_files, NULL);
+  while (key != NULL) {
+    const char * value = rcutils_string_map_get(&security_files, key);
+    if (NULL == value) {
+      RMW_SET_ERROR_MSG("unable to get value for known key, should not happen");
+      return false;
     }
+    result[key] = value;
+    key = rcutils_string_map_get_next_key(&security_files, key);
   }
 
   return true;

rmw_dds_common/test/test_security.cpp

@@ -49,6 +49,18 @@ static void write_test_pkcs11_content(const std::array<std::string, N> & pkcs11_
 
 class test_security : public ::testing::TestWithParam<bool> {};
 
+#if !defined(_WIN32)
+# pragma GCC diagnostic push
+# pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+# ifdef __clang__
+#  pragma clang diagnostic push
+#  pragma clang diagnostic ignored "-Wdeprecated-declarations"
+# endif
+#else  // !defined(_WIN32)
+# pragma warning(push)
+# pragma warning(disable: 4996)
+#endif
+
 TEST_P(test_security, files_exist_no_prefix)
 {
   std::filesystem::path dir = std::filesystem::path("./test_folder");
@@ -403,3 +415,9 @@ INSTANTIATE_TEST_SUITE_P(
   [](const testing::TestParamInfo<bool> & info) {
     return info.param ? "with_pkcs11_support" : "with_no_pkcs11_support";
   });
+
+#if !defined(_WIN32)
+# pragma GCC diagnostic pop
+#else  // !defined(_WIN32)
+# pragma warning(pop)
+#endif