From 988ad01666c23d6bfdb622a2aa6cf3b7a3e8aced Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Fri, 13 Mar 2020 14:46:54 -0300 Subject: [PATCH 1/2] Update test_security tests to use security contexts Signed-off-by: Ivan Santiago Paunovic --- test_security/CMakeLists.txt | 8 +- test_security/package.xml | 1 + .../test_invalid_secure_node_creation_c.cpp | 204 ++++++++++-------- test_security/test/test_secure_publisher.cpp | 5 +- test_security/test/test_secure_subscriber.cpp | 5 +- 5 files changed, 129 insertions(+), 94 deletions(-) diff --git a/test_security/CMakeLists.txt b/test_security/CMakeLists.txt index db080a19..fb501544 100644 --- a/test_security/CMakeLists.txt +++ b/test_security/CMakeLists.txt @@ -48,6 +48,7 @@ if(BUILD_TESTING) ament_lint_auto_find_test_dependencies() find_package(launch_testing_ament_cmake REQUIRED) + find_package(osrf_testing_tools_cpp REQUIRED) # get the rmw implementations ahead of time find_package(rmw_implementation_cmake REQUIRED) @@ -80,7 +81,7 @@ if(BUILD_TESTING) ${_AMENT_EXPORT_ABSOLUTE_LIBRARIES} ${_AMENT_EXPORT_LIBRARY_TARGETS}) ament_target_dependencies(${target}${target_suffix} - "rcl") + "rcl" "osrf_testing_tools_cpp") set_tests_properties( ${target}${target_suffix} PROPERTIES REQUIRED_FILES "$" @@ -329,13 +330,14 @@ if(BUILD_TESTING) # prevents finding it repeatedly in each local scope ament_find_gtest() - set(VALID_ROS_SECURITY_ROOT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/test/test_security_files") + set(KEYSTORE_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/test/test_security_files") + set(VALID_ROS_SECURITY_ROOT_DIRECTORY "${KEYSTORE_DIRECTORY}/contexts") # generate security artifacts using sros2 find_program(PROGRAM ros2) set(node_names_list "/publisher;/subscriber;/publisher_missing_key;/publisher_invalid_cert") - set(generate_artifacts_command ${PROGRAM} security generate_artifacts -k ${VALID_ROS_SECURITY_ROOT_DIRECTORY} -n ${node_names_list}) + set(generate_artifacts_command ${PROGRAM} security generate_artifacts -k ${KEYSTORE_DIRECTORY} -c ${node_names_list}) execute_process( COMMAND ${generate_artifacts_command} RESULT_VARIABLE GENERATE_ARTIFACTS_RESULT diff --git a/test_security/package.xml b/test_security/package.xml index e2bc3542..56c2f560 100644 --- a/test_security/package.xml +++ b/test_security/package.xml @@ -20,6 +20,7 @@ launch launch_testing launch_testing_ament_cmake + osrf_testing_tools_cpp rcl rclcpp diff --git a/test_security/test/test_invalid_secure_node_creation_c.cpp b/test_security/test/test_invalid_secure_node_creation_c.cpp index ebb561fd..3a283c64 100644 --- a/test_security/test/test_invalid_secure_node_creation_c.cpp +++ b/test_security/test/test_invalid_secure_node_creation_c.cpp @@ -1,4 +1,4 @@ -// Copyright 2016-2017 Open Source Robotics Foundation, Inc. +// Copyright 2016-2020 Open Source Robotics Foundation, Inc. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -21,31 +21,22 @@ #include #include #include +#include + +#include "osrf_testing_tools_cpp/scope_exit.hpp" #include "rcl/rcl.h" #include "rcl/error_handling.h" -#include "rcutils/get_env.h" -#ifndef SCOPE_EXIT_HPP_ -#define SCOPE_EXIT_HPP_ - -template -struct ScopeExit -{ - explicit ScopeExit(Callable callable) - : callable_(callable) {} - ~ScopeExit() {callable_();} - -private: - Callable callable_; -}; +#include "rcutils/get_env.h" +#include "rcutils/strdup.h" -template -ScopeExit -make_scope_exit(Callable callable) -{ - return ScopeExit(callable); -} +#ifdef RMW_IMPLEMENTATION +# define CLASSNAME_(NAME, SUFFIX) NAME ## __ ## SUFFIX +# define CLASSNAME(NAME, SUFFIX) CLASSNAME_(NAME, SUFFIX) +#else +# define CLASSNAME(NAME, SUFFIX) NAME +#endif void custom_putenv(const char * name, const char * value) { @@ -56,92 +47,131 @@ void custom_putenv(const char * name, const char * value) #endif // _WIN32 } -#define SCOPE_EXIT(code) make_scope_exit([&]() {code;}) - -#endif // SCOPE_EXIT_HPP_ - +struct TestConfig +{ + const char * ROS_SECURITY_ROOT_DIRECTORY; + const char * ROS_SECURITY_ENABLE; + const char * ROS_SECURITY_STRATEGY; + const char * context_name; + bool should_fail_context_creation; + bool should_fail_node_creation; + const char * test_description; +}; -#ifdef RMW_IMPLEMENTATION -# define CLASSNAME_(NAME, SUFFIX) NAME ## __ ## SUFFIX -# define CLASSNAME(NAME, SUFFIX) CLASSNAME_(NAME, SUFFIX) -#else -# define CLASSNAME(NAME, SUFFIX) NAME -#endif +std::ostream & operator<<( + std::ostream & out, + const TestConfig & config) +{ + out << config.test_description; + return out; +} -class CLASSNAME (TestInvalidSecureNode, RMW_IMPLEMENTATION) : public ::testing::Test +class CLASSNAME (TestSecureNodes, RMW_IMPLEMENTATION) + : public ::testing::TestWithParam { public: rcl_context_t context; - rcl_node_t * node_ptr; - const char * previous_root_dir; + rcl_node_t node; + char * previous_root_dir; + rcutils_allocator_t allocator = rcutils_get_default_allocator(); + void SetUp() { - ASSERT_EQ(NULL, rcutils_get_env("ROS_SECURITY_ROOT_DIRECTORY", &previous_root_dir)); + const char * root_dir = nullptr; + ASSERT_EQ(nullptr, rcutils_get_env("ROS_SECURITY_ROOT_DIRECTORY", &root_dir)); + previous_root_dir = rcutils_strdup(root_dir, allocator); + ASSERT_NE(nullptr, previous_root_dir); } void TearDown() { - rcl_ret_t ret = rcl_node_fini(this->node_ptr); - delete this->node_ptr; - ret = rcl_shutdown(&context); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; custom_putenv("ROS_SECURITY_ROOT_DIRECTORY", previous_root_dir); + allocator.deallocate(previous_root_dir, allocator.state); } +}; + +using TestSecureNodes = CLASSNAME(TestSecureNodes, RMW_IMPLEMENTATION); - void test_node_creation( - const char * ROS_SECURITY_ROOT_DIRECTORY, const char * ROS_SECURITY_ENABLE, - const char * ROS_SECURITY_STRATEGY, - const char * node_name, bool should_fail_participant_creation) +TEST_P(TestSecureNodes, test_invalid_keystore) { + const auto & test_config = GetParam(); + if (test_config.ROS_SECURITY_ROOT_DIRECTORY != NULL) { + custom_putenv("ROS_SECURITY_ROOT_DIRECTORY", test_config.ROS_SECURITY_ROOT_DIRECTORY); + } + custom_putenv("ROS_SECURITY_ENABLE", test_config.ROS_SECURITY_ENABLE); + custom_putenv("ROS_SECURITY_STRATEGY", test_config.ROS_SECURITY_STRATEGY); + rcl_ret_t ret; + rcl_init_options_t init_options = rcl_get_zero_initialized_init_options(); + ret = rcl_init_options_init(&init_options, rcl_get_default_allocator()); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + context = rcl_get_zero_initialized_context(); + const char * argv[] = {"--ros-args", "--security-context", test_config.context_name}; + ret = rcl_init(sizeof(argv) / sizeof(char *), argv, &init_options, &context); + if (test_config.should_fail_context_creation) { + ASSERT_EQ(RCL_RET_ERROR, ret); + rcl_reset_error(); + return; + } + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( { - if (ROS_SECURITY_ROOT_DIRECTORY != NULL) { - custom_putenv("ROS_SECURITY_ROOT_DIRECTORY", ROS_SECURITY_ROOT_DIRECTORY); - } - custom_putenv("ROS_SECURITY_ENABLE", ROS_SECURITY_ENABLE); - custom_putenv("ROS_SECURITY_STRATEGY", ROS_SECURITY_STRATEGY); - rcl_ret_t ret; - rcl_init_options_t init_options = rcl_get_zero_initialized_init_options(); - ret = rcl_init_options_init(&init_options, rcl_get_default_allocator()); + ret = rcl_shutdown(&context); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + }); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + this->node = rcl_get_zero_initialized_node(); + rcl_node_options_t node_options = rcl_node_get_default_options(); + ret = rcl_node_init( + &this->node, "test_invalid_secure_node_creation_c", "", &context, &node_options); + if (test_config.should_fail_node_creation) { + ASSERT_EQ(RCL_RET_ERROR, ret); + rcl_reset_error(); + } else { ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - context = rcl_get_zero_initialized_context(); - ret = rcl_init(0, nullptr, &init_options, &context); + ret = rcl_node_fini(&this->node); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - this->node_ptr = new rcl_node_t; - *this->node_ptr = rcl_get_zero_initialized_node(); - // const char * name = "node_name"; - rcl_node_options_t node_options = rcl_node_get_default_options(); - ret = rcl_node_init(this->node_ptr, node_name, "", &context, &node_options); - if (should_fail_participant_creation) { - ASSERT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str; - } else { - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - } } -}; - -TEST_F(CLASSNAME(TestInvalidSecureNode, RMW_IMPLEMENTATION), test_invalid_keystore) { - test_node_creation( - "garbage", - "true", "Enforce", - "publisher", true); } -TEST_F(CLASSNAME(TestInvalidSecureNode, RMW_IMPLEMENTATION), test_unknown_identity) { - test_node_creation( +std::vector test_configs { + { + "garbage", + "true", + "Enforce", + "/publisher", + true, + true, + "test_invalid_keystore", + }, + { NULL, - "true", "Enforce", - "publisher2", true); -} - -TEST_F(CLASSNAME(TestInvalidSecureNode, RMW_IMPLEMENTATION), test_invalid_cert) { - test_node_creation( + "true", + "Enforce", + "/publisher2", + true, + true, + "test_unknown_identity", + }, + { NULL, - "true", "Enforce", - "publisher_invalid_cert", true); -} - -TEST_F(CLASSNAME(TestInvalidSecureNode, RMW_IMPLEMENTATION), test_missing_key) { - test_node_creation( + "true", + "Enforce", + "/publisher_invalid_cert", + false, + true, + "test_invalid_cert", + }, + { NULL, - "true", "Enforce", - "publisher_missing_key", true); -} + "true", + "Enforce", + "/publisher_missing_key", + false, + true, + "test_missing_key", + }, +}; + +INSTANTIATE_TEST_CASE_P( + TestInitializationFails, + TestSecureNodes, + ::testing::ValuesIn(test_configs), + ::testing::PrintToStringParamName()); diff --git a/test_security/test/test_secure_publisher.cpp b/test_security/test/test_secure_publisher.cpp index 6a5a3da4..3b5c9405 100644 --- a/test_security/test/test_secure_publisher.cpp +++ b/test_security/test/test_secure_publisher.cpp @@ -65,10 +65,11 @@ int main(int argc, char ** argv) "pass a message type\n"); return 1; } - rclcpp::init(argc, argv); + const char * args[] = {"--ros-args", "--security-context", "/publisher"}; + rclcpp::init(sizeof(args) / sizeof(char *), args); std::string message = argv[1]; std::string namespace_ = argv[2]; - std::string node_name = "publisher"; + std::string node_name = "test_secure_publisher"; std::string topic_name = "chatter"; std::shared_ptr node = nullptr; try { diff --git a/test_security/test/test_secure_subscriber.cpp b/test_security/test/test_secure_subscriber.cpp index ab9bb3bc..5d2b0945 100644 --- a/test_security/test/test_secure_subscriber.cpp +++ b/test_security/test/test_secure_subscriber.cpp @@ -117,12 +117,13 @@ int main(int argc, char ** argv) } std::string message = argv[1]; std::string namespace_ = argv[3]; - std::string node_name = "subscriber"; + std::string node_name = "test_secure_subscriber"; std::string topic_name = "chatter"; bool should_timeout = ((0 == strcmp(argv[2], "false")) || (0 == strcmp(argv[2], "0"))) ? false : true; - rclcpp::init(argc, argv); + const char * args[] = {"--ros-args", "--security-context", "/subscriber"}; + rclcpp::init(sizeof(args) / sizeof(char *), args); std::shared_ptr node = nullptr; try { node = rclcpp::Node::make_shared(node_name, namespace_); From 1623fbb60482cd15fe13f753def16b1f444a8482 Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Fri, 27 Mar 2020 11:13:27 -0300 Subject: [PATCH 2/2] Skip failing cross vendor tests after Fast-RTPS based rmw implementations are using one Participant per Context Signed-off-by: Ivan Santiago Paunovic --- test_rclcpp/CMakeLists.txt | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/test_rclcpp/CMakeLists.txt b/test_rclcpp/CMakeLists.txt index 04bc7a27..b468eea1 100644 --- a/test_rclcpp/CMakeLists.txt +++ b/test_rclcpp/CMakeLists.txt @@ -182,14 +182,29 @@ if(BUILD_TESTING) else() set(target_suffix "__${rmw_implementation1}__${rmw_implementation2}") endif() - # TODO(hidmic): enable tests once FastRTPS <=> (Opensplice|Connext) interoperation - # issues are resolved (see https://github.com/ros2/rmw_fastrtps/issues/246). - custom_launch_test_two_executables(test_node_name - node_with_name node_name_list - ARGS1 "${rmw_implementation1}" ARGS2 "node_with_name_${rmw_implementation1}" - RMW1 ${rmw_implementation1} RMW2 ${rmw_implementation2} - TIMEOUT 15 - ${SKIP_TEST}) + + set(rmw_implementation1_is_fastrtps FALSE) + set(rmw_implementation2_is_fastrtps FALSE) + if(rmw_implementation1 MATCHES "(.*)fastrtps(.*)") + set(rmw_implementation1_is_fastrtps TRUE) + endif() + if(rmw_implementation2 MATCHES "(.*)fastrtps(.*)") + set(rmw_implementation2_is_fastrtps TRUE) + endif() + + # Skipped tests between fastrtps and others, as the node names are communicated with a different mechanism. + # TODO(ivanpauno): Reenable this tests after the other implementations also use one Participant per Context. + if( + (NOT rmw_implementation1_is_fastrtps AND NOT rmw_implementation2_is_fastrtps) OR + (rmw_implementation1_is_fastrtps AND rmw_implementation2_is_fastrtps) + ) + custom_launch_test_two_executables(test_node_name + node_with_name node_name_list + ARGS1 "${rmw_implementation1}" ARGS2 "node_with_name_${rmw_implementation1}" + RMW1 ${rmw_implementation1} RMW2 ${rmw_implementation2} + TIMEOUT 15 + ${SKIP_TEST}) + endif() endmacro() macro(targets)