Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security directory lookup improvements #332

Merged
merged 24 commits into from
Feb 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
c7810d5
Changing security directory lookup to a prefix match rather than exac…
AAlon Oct 29, 2018
df041a4
Adding security_directory module and moving rcl_get_secure_root funct…
AAlon Nov 16, 2018
813f39b
Changing security directory prefix matching to be optional. Improving…
AAlon Dec 12, 2018
f313279
Fixing get_best_matching_directory so that it always fetches the next…
AAlon Dec 12, 2018
a411455
Adding security_directory module and moving rcl_get_secure_root funct…
AAlon Nov 16, 2018
f3924b7
Changing security directory lookup to a prefix match rather than exac…
AAlon Oct 29, 2018
de72335
Adding security_directory module and moving rcl_get_secure_root funct…
AAlon Nov 16, 2018
4df23d3
Changing security directory lookup to a prefix match rather than exac…
AAlon Oct 29, 2018
0ceb0d2
Adding security_directory module and moving rcl_get_secure_root funct…
AAlon Nov 16, 2018
cea9fc7
Changing security directory lookup to a prefix match rather than exac…
AAlon Oct 29, 2018
435ad08
Adding security_directory module and moving rcl_get_secure_root funct…
AAlon Nov 16, 2018
fd0fbda
make pr ready for ros2cli security feature (#1)
xabxx Jan 8, 2019
01bb136
Update rcl/include/rcl/security_directory.h
jacobperron Jan 8, 2019
9c02cc8
Adding line break in docstring
AAlon Jan 8, 2019
814d62a
Removing duplicate doc string
AAlon Jan 8, 2019
3cdfb0a
Removing tinydir from the source tree, instead using the ROS package …
AAlon Jan 11, 2019
74fddf0
Removing tinydir
AAlon Jan 11, 2019
b2a511c
Reformatting license notice as per linter template.
AAlon Jan 11, 2019
baa53f3
Update test_security_directory.cpp
AAlon Jan 23, 2019
a788d8c
Changing input to putenv to be a global, statically allocated buffer.
AAlon Jan 24, 2019
373845a
test_security_directory - Using a larger buffer for env string manipu…
AAlon Jan 24, 2019
c34e140
Copy environment variable to allocated string so it is not clobbered …
Feb 13, 2019
2c27443
Address review comments
Feb 19, 2019
5f791d4
Remove strncpy
Feb 21, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions rcl/CMakeLists.txt
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for breaking out security related functions, and organizing them under their own source file.

)

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
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
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
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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch, line 174 was a bug.

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);
ruffsl marked this conversation as resolved.
Show resolved Hide resolved
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