ros2/rmw_connext

[rmw_connext_cpp] Cannot create secure nodes with current usage of partitions

mikaelarguedas opened this issue · 2 comments

Found that out when investigating ros2/sros2#32 (comment)

As all our nodes now start with parameters enabled. They create a set of services topics by default:

get_parametersReply
get_parametersRequest
get_parameter_typesReply
get_parameter_typesRequest
list_parametersReply
list_parametersRequest
describe_parametersReply
describe_parametersRequest
set_parametersReply
set_parametersRequest

These topics are using the partitions using the previx defined in the design doc and the node name (rq/<NODE_NAME> for requests and rr/<NODE_NAME> for replies)

If we define the access policies to match this, e.g.

          <partitions>
            <partition>rq/talker</partition>
          </partitions>
          <topics>
            <topic>get_parametersRequest</topic>
          </topics>

The node creation fails:

RTI_Security_AccessControl_check_create_datawriter:endpoint not allowed: no rule found; default DENY
DDS_DomainParticipantTrustPlugins_getLocalDataWriterSecurityState:!security function check_create_datawriter
DDS_DataWriter_create_presentation_writerI:ERROR: Failed to get local datawriter security state
DDS_DataWriter_createI:!create PRESPsWriter
DDS_Publisher_create_datawriter_disabledI:!create DataWriter
DDSDataWriter_impl::createI:!create writer
initialize:!create DataWriter
connext::details::EntityUntypedImpl::initialize:!failed (see previous errors)

>>> [rcutils|error_handling.c:155] rcutils_set_error_state()
This error state is being overwritten:

  'C++ exception during construction of Requester, at /home/mikael/work/ros2/current_ws/build_debug_isolated/rcl_interfaces/rosidl_typesupport_connext_cpp/rcl_interfaces/srv/dds_connext/get_parameters__type_support.cpp:98'

with this new error message:

  'failed to create requester, at /home/mikael/work/ros2/current_ws/src/ros2/rmw_connext/rmw_connext_cpp/src/rmw_client.cpp:139'

rcutils_reset_error() should be called after error handling to avoid this.
<<<
terminate called after throwing an instance of 'rclcpp::exceptions::RCLError'
  what():  could not create client: failed to create requester, at /home/mikael/work/ros2/current_ws/src/ros2/rmw_connext/rmw_connext_cpp/src/rmw_client.cpp:139, at /home/mikael/work/ros2/current_ws/src/ros2/rcl/rcl/src/rcl/client.c:174

This is due to that fact that the partition is set after the requester is created. So the requester has an empty partition when the access rules are being checked.
requester creation here:

requester = callbacks->create_requester(
participant, service_str, &datareader_qos, &datawriter_qos,
reinterpret_cast<void **>(&response_datareader),
reinterpret_cast<void **>(&request_datawriter),
&rmw_allocate);

partition set here:

dds_publisher->set_qos(publisher_qos);

This should be fixed as soon as we get rid of the use of partitions for topic namespacing, at that point we should remove the whitelist for empty partitions here:
https://github.com/ros2/sros2/blob/69ee5b691604cebc8af822db359bba7c67a9df7d/sros2/api/__init__.py#L348-L349

@Karsten1987 @rohitsalem @ruffsl FYI

Thanks @mikaelarguedas for revisiting this and tracking the source of this issue. For others interested, I shared a living jupyter notebook a while back the demonstrates the issue:
https://github.com/ruffsl/PPAC_ROS2/blob/3fffb70aade982d485c528ffce09be0a52b9d515/create_profile_from_discovery.ipynb

I'll go back and clarify the conclusions given @mikaelarguedas report here soon.

Closing this as partitions are not used anymore: #285