Skip to content

Commit

Permalink
Security directory lookup improvements (#332)
Browse files Browse the repository at this point in the history 
* Changing security directory lookup to a prefix match rather than exact match.

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* Changing security directory lookup to a prefix match rather than exact match.

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* Changing security directory lookup to a prefix match rather than exact match.

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* Changing security directory lookup to a prefix match rather than exact match.

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* Adding security_directory module and moving rcl_get_secure_root function to it. Adding tests.

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* Adding security_directory module and moving rcl_get_secure_root function to it. Adding tests.

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* Adding security_directory module and moving rcl_get_secure_root function to it. Adding tests.

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* Adding security_directory module and moving rcl_get_secure_root function to it. Adding tests.

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* Adding security_directory module and moving rcl_get_secure_root function to it. Adding tests.

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* Changing security directory prefix matching to be optional. Improving error messages around security directory lookup.

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* Fixing get_best_matching_directory so that it always fetches the next file inside the loop.

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* make pr ready for ros2cli security feature (#1)

* update docs about possibility of rcl_take no taking (#356)

* update rcl_wait doc with respect to subs and possibility of failing takes

* add a note about possible failing takes in rcl_take docs

* 0.6.2

* Set rmw_wait timeout using ros timers too (#357)

* 0.6.3

* Avoid timer period being set to 0 (#359)

* Fix logic that moves goal handles when one expires (#360)

* Fix error from uncrustify v0.68 (#364)

* Ensure that context instance id storage is aligned correctly (#365)

* Ensure that context instance id storage is aligned correctly

* Make alignment compatible with MSVC

* Namespace alignment macro with RCL_

* [rcl] Guard against bad allocation calling rcl_arguments_copy() (#367)

* [rcl] Add test for copying arguments struct with no arguments

* Override allocate function in test to reveal bug

* [rcl] Only allocate arrays if there are things to copy in rcl_argument_copy()

Also guard against freeing invalid pointers if rcl_argument_copy() fails.

* Remove uncessary guard against NULL pointer

* linter, styles, uncrustify fixes

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* Update rcl/include/rcl/security_directory.h

Co-Authored-By: AAlon <avishayalon@gmail.com>
Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* Adding line break in docstring

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* Removing duplicate doc string

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* Removing tinydir from the source tree, instead using the ROS package tinydir_vendor.

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* Removing tinydir

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* Reformatting license notice as per linter template.

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* Update test_security_directory.cpp

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* Changing input to putenv to be a global, statically allocated buffer.

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* test_security_directory - Using a larger buffer for env string manipulations.

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* Copy environment variable to allocated string so it is not clobbered by next lookup

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* Address review comments

fix security directory exact match comment and unset env vars before tests

Signed-off-by: Emerson Knapp <eknapp@amazon.com>

* Remove strncpy

Signed-off-by: Emerson Knapp <eknapp@amazon.com>
  • Loading branch information
@AAlon @jacobperron
AAlon authored and jacobperron committed Feb 21, 2019
1 parent 4710751 commit b0a124d
Show file tree
Hide file tree
Showing 8 changed files with 499 additions and 88 deletions.
3 changes: 3 additions & 0 deletions rcl/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ find_package(rcutils REQUIRED)
find_package(rmw REQUIRED)
find_package(rmw_implementation REQUIRED)
find_package(rosidl_generator_c REQUIRED)
find_package(tinydir_vendor REQUIRED)

include_directories(include)

Expand Down Expand Up @@ -54,6 +55,7 @@ set(${PROJECT_NAME}_sources
src/rcl/timer.c
src/rcl/validate_topic_name.c
src/rcl/wait.c
src/rcl/security_directory.c
)

add_library(${PROJECT_NAME} ${${PROJECT_NAME}_sources})
Expand All @@ -65,6 +67,7 @@ ament_target_dependencies(${PROJECT_NAME}
"rcutils"
"rosidl_generator_c"
${RCL_LOGGING_IMPL}
"tinydir_vendor"
)

# Causes the visibility macros to use dllexport rather than dllimport,
Expand Down
66 changes: 66 additions & 0 deletions rcl/include/rcl/security_directory.h
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// Copyright 2018 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.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

#ifndef RCL__SECURITY_DIRECTORY_H_
#define RCL__SECURITY_DIRECTORY_H_

#ifdef __cplusplus
extern "C"
{
#endif

#include "rcl/allocator.h"
#include "rcl/visibility_control.h"

#ifndef ROS_SECURITY_NODE_DIRECTORY_VAR_NAME
#define ROS_SECURITY_NODE_DIRECTORY_VAR_NAME "ROS_SECURITY_NODE_DIRECTORY"
#endif

#ifndef ROS_SECURITY_ROOT_DIRECTORY_VAR_NAME
#define ROS_SECURITY_ROOT_DIRECTORY_VAR_NAME "ROS_SECURITY_ROOT_DIRECTORY"
#endif

#ifndef ROS_SECURITY_LOOKUP_TYPE_VAR_NAME
#define ROS_SECURITY_LOOKUP_TYPE_VAR_NAME "ROS_SECURITY_LOOKUP_TYPE"
#endif

/// Return the secure root directory associated with a node given its validated name and namespace.
/**
* E.g. for a node named "c" in namespace "/a/b", the secure root path will be
* "a/b/c", where the delimiter "/" is native for target file system (e.g. "\\" for _WIN32).
* If no exact match is found for the node name, a best match would be used instead
* (by performing longest-prefix matching).
*
* However, this expansion can be overridden by setting the secure node directory environment
* variable, allowing users to explicitly specify the exact secure root directory to be utilized.
* Such an override is useful for where the FQN of a node is non-deterministic before runtime,
* or when testing and using additional tools that may not otherwise be easily provisioned.
*
* \param[in] node_name validated node name (a single token)
* \param[in] node_namespace validated, absolute namespace (starting with "/")
* \param[in] allocator the allocator to use for allocation
* \returns machine specific (absolute) node secure root path or NULL on failure
*/
RCL_PUBLIC
const char * rcl_get_secure_root(
const char * node_name,
const char * node_namespace,
const rcl_allocator_t * allocator
);

#ifdef __cplusplus
}
#endif

#endif // RCL__SECURITY_DIRECTORY_H_
2 changes: 2 additions & 0 deletions rcl/package.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@
<build_depend>rcl_interfaces</build_depend>
<build_depend>rcutils</build_depend>
<build_depend>rosidl_generator_c</build_depend>
<build_depend>tinydir_vendor</build_depend>

<build_export_depend>rcl_interfaces</build_export_depend>
<build_export_depend>rcutils</build_export_depend>
<build_export_depend>rosidl_generator_c</build_export_depend>
<build_export_depend>tinydir_vendor</build_export_depend>

<exec_depend>rcl_interfaces</exec_depend>
<exec_depend>ament_cmake</exec_depend>
Expand Down
91 changes: 3 additions & 88 deletions rcl/src/rcl/node.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ extern "C"
#include "rcl/logging_rosout.h"
#include "rcl/rcl.h"
#include "rcl/remap.h"
#include "rcl/security_directory.h"
#include "rcutils/filesystem.h"
#include "rcutils/find.h"
#include "rcutils/format_string.h"
Expand All @@ -46,9 +47,8 @@ extern "C"

#include "./common.h"
#include "./context_impl.h"
#include "tinydir/tinydir.h"

#define ROS_SECURITY_NODE_DIRECTORY_VAR_NAME "ROS_SECURITY_NODE_DIRECTORY"
#define ROS_SECURITY_ROOT_DIRECTORY_VAR_NAME "ROS_SECURITY_ROOT_DIRECTORY"
#define ROS_SECURITY_STRATEGY_VAR_NAME "ROS_SECURITY_STRATEGY"
#define ROS_SECURITY_ENABLE_VAR_NAME "ROS_SECURITY_ENABLE"

Expand Down Expand Up @@ -101,86 +101,6 @@ const char * rcl_create_node_logger_name(
return node_logger_name;
}

/// Return the secure root directory associated with a node given its validated name and namespace.
/**
* E.g. for a node named "c" in namespace "/a/b", the secure root path will be
* "a/b/c", where the delimiter "/" is native for target file system (e.g. "\\" for _WIN32).
* However, this expansion can be overridden by setting the secure node directory environment
* variable, allowing users to explicitly specify the exact secure root directory to be utilized.
* Such an override is useful for where the FQN of a node is non-deterministic before runtime,
* or when testing and using additional tools that may not otherwise not be easily provisioned.
*
* \param[in] node_name validated node name (a single token)
* \param[in] node_namespace validated, absolute namespace (starting with "/")
* \param[in] allocator the allocator to use for allocation
* \returns machine specific (absolute) node secure root path or NULL on failure
*/
const char * rcl_get_secure_root(
const char * node_name,
const char * node_namespace,
const rcl_allocator_t * allocator)
{
bool ros_secure_node_override = true;
const char * ros_secure_root_env = NULL;
if (NULL == node_name) {
return NULL;
}
if (rcutils_get_env(ROS_SECURITY_NODE_DIRECTORY_VAR_NAME, &ros_secure_root_env)) {
return NULL;
}
if (!ros_secure_root_env) {
return NULL;
}
size_t ros_secure_root_size = strlen(ros_secure_root_env);
if (!ros_secure_root_size) {
// check root directory if node directory environment variable is empty
if (rcutils_get_env(ROS_SECURITY_ROOT_DIRECTORY_VAR_NAME, &ros_secure_root_env)) {
return NULL;
}
if (!ros_secure_root_env) {
return NULL;
}
ros_secure_root_size = strlen(ros_secure_root_env);
if (!ros_secure_root_size) {
return NULL; // environment variable was empty
} else {
ros_secure_node_override = false;
}
}
char * node_secure_root = NULL;
if (ros_secure_node_override) {
node_secure_root =
(char *)allocator->allocate(ros_secure_root_size + 1, allocator->state);
memcpy(node_secure_root, ros_secure_root_env, ros_secure_root_size + 1);
// TODO(ros2team): This make an assumption on the value and length of the root namespace.
// This should likely come from another (rcl/rmw?) function for reuse.
// If the namespace is the root namespace ("/"), the secure root is just the node name.
} else if (strlen(node_namespace) == 1) {
node_secure_root = rcutils_join_path(ros_secure_root_env, node_name, *allocator);
} else {
char * node_fqn = NULL;
char * node_root_path = NULL;
// Combine node namespace with node name
// TODO(ros2team): remove the hard-coded value of the root namespace.
node_fqn = rcutils_format_string(*allocator, "%s%s%s", node_namespace, "/", node_name);
// Get native path, ignore the leading forward slash.
// TODO(ros2team): remove the hard-coded length, use the length of the root namespace instead.
node_root_path = rcutils_to_native_path(node_fqn + 1, *allocator);
node_secure_root = rcutils_join_path(ros_secure_root_env, node_root_path, *allocator);
allocator->deallocate(node_fqn, allocator->state);
allocator->deallocate(node_root_path, allocator->state);
}
// Check node_secure_root is not NULL before checking directory
if (NULL == node_secure_root) {
allocator->deallocate(node_secure_root, allocator->state);
return NULL;
} else if (!rcutils_is_directory(node_secure_root)) {
allocator->deallocate(node_secure_root, allocator->state);
return NULL;
}
return node_secure_root;
}

rcl_node_t
rcl_get_zero_initialized_node()
{
Expand Down Expand Up @@ -385,15 +305,10 @@ rcl_node_init(
// File discovery magic here
const char * node_secure_root = rcl_get_secure_root(name, local_namespace_, allocator);
if (node_secure_root) {
RCUTILS_LOG_INFO_NAMED(ROS_PACKAGE_NAME, "Found security directory: %s", node_secure_root);
node_security_options.security_root_path = node_secure_root;
} else {
if (RMW_SECURITY_ENFORCEMENT_ENFORCE == node_security_options.enforce_security) {
RCL_SET_ERROR_MSG(
"SECURITY ERROR: unable to find a folder matching the node name in the "
RCUTILS_STRINGIFY(ROS_SECURITY_NODE_DIRECTORY_VAR_NAME)
" or "
RCUTILS_STRINGIFY(ROS_SECURITY_ROOT_DIRECTORY_VAR_NAME)
" directories while the requested security strategy requires it");
ret = RCL_RET_ERROR;
goto cleanup;
}
Expand Down
Loading

0 comments on commit b0a124d

Please sign in to comment.