-
Notifications
You must be signed in to change notification settings - Fork 162
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
+499
−88
Merged
Changes from 23 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 df041a4
Adding security_directory module and moving rcl_get_secure_root funct…
AAlon 813f39b
Changing security directory prefix matching to be optional. Improving…
AAlon f313279
Fixing get_best_matching_directory so that it always fetches the next…
AAlon a411455
Adding security_directory module and moving rcl_get_secure_root funct…
AAlon f3924b7
Changing security directory lookup to a prefix match rather than exac…
AAlon de72335
Adding security_directory module and moving rcl_get_secure_root funct…
AAlon 4df23d3
Changing security directory lookup to a prefix match rather than exac…
AAlon 0ceb0d2
Adding security_directory module and moving rcl_get_secure_root funct…
AAlon cea9fc7
Changing security directory lookup to a prefix match rather than exac…
AAlon 435ad08
Adding security_directory module and moving rcl_get_secure_root funct…
AAlon fd0fbda
make pr ready for ros2cli security feature (#1)
xabxx 01bb136
Update rcl/include/rcl/security_directory.h
jacobperron 9c02cc8
Adding line break in docstring
AAlon 814d62a
Removing duplicate doc string
AAlon 3cdfb0a
Removing tinydir from the source tree, instead using the ROS package …
AAlon 74fddf0
Removing tinydir
AAlon b2a511c
Reformatting license notice as per linter template.
AAlon baa53f3
Update test_security_directory.cpp
AAlon a788d8c
Changing input to putenv to be a global, statically allocated buffer.
AAlon 373845a
test_security_directory - Using a larger buffer for env string manipu…
AAlon c34e140
Copy environment variable to allocated string so it is not clobbered …
2c27443
Address review comments
5f791d4
Remove strncpy
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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_ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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" | ||
|
@@ -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" | ||
|
||
|
@@ -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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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() | ||
{ | ||
|
@@ -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; | ||
} | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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.