Minimal permission set for a talker and listener pair in the demo_cpp example

[ruffsl]

I'm trying to determine the minimal set of permissions for a talker and listener pair in the demo_cpp example in order to evaluate keymint's provisioning of permissions. I've checked the discovery data, and I looks like I've added everything they'd need in the permissions document, but apparently I've missed something the client library is needing, as this inevitably causes the failure in creating the DDS participant.

Bug report

Required Info:

• Operating System:
  • Ubuntu 16.04
• Installation type:
  • ros2.repos
• Version or commit hash:
  • ef1bec6
• DDS implementation:
  • RTI Connext
• Client library (if applicable):
  • rclcpp

Steps to reproduce issue

I've made a simple example that can quickly be create using keymint_tools. See the example section in the repo README for more details: https://github.com/keymint/keymint_tools#example

You can also find the resulting keystore here: https://github.com/ruffsl/keymint_ws/tree/7783f87783e27efcee80be7f6189ded1bce5869e
For example. here is the permissions file for the talker before signing:
https://github.com/ruffsl/keymint_ws/blob/7783f87783e27efcee80be7f6189ded1bce5869e/build/talker/permissions.xml

Expected behavior

From looking at the discovery data, we can collect the number of resources advertised. I'd expect collpacing this back down into the permissions files would be sufficient, but perhaps there are some requested resources from the client library to the dds vender that are not being advertised during a non-secure session, or are not captured by my DDS session monitor under secure sessions without access control.

For good measure, one can simple add this XML snippet to the beginning of the allow_rule for the grant specific to the node within the keymint package source, then rebuild the keymint package and all will work as expected. However, this use of wildcarding nullifies what we are trying to achieve here.

                <publish>
                    <topics>
                        <topic>*</topic>
                    </topics>
                    <partitions>
                        <partition>*</partition>
                    </partitions>
                </publish>
                <subscribe>
                    <topics>
                        <topic>*</topic>
                    </topics>
                    <partitions>
                        <partition>*</partition>
                    </partitions>
                </subscribe>

Actual behavior

If attempting to use the keystore files as is, you'll get this error noting the creation of the participant has failed due to the RTI_Security_AccessControl_check_create_datawriter:endpoint recognizing that the policy is insufficient for resources requested by the client library.

$ ros2 run demo_nodes_cpp talker
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/ruffsl/ws/ros2/ardent/build_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/ruffsl/ws/ros2/ardent/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/ruffsl/ws/ros2/ardent/src/ros2/rmw_connext/rmw_connext_cpp/src/rmw_client.cpp:139, at /home/ruffsl/ws/ros2/ardent/src/ros2/rcl/rcl/src/rcl/client.c:152

Additional information

Here is a record of the decoverydata captured for the talker listener cpp demo. I've provided a simple script to collapse the topics and partitions access by each participant for clarity. The resources listed are what I've incorporated into the minimal permissions keystore example linked above.

https://gist.github.com/ruffsl/d914f68342f616c361d7f44b7332e6ae

It would be nice if the error message return some context for the failed creation of the participant.
E.g. what resources were attempted before failure, and which specifically resulted in insufficient privileges.

[ruffsl]

pinging @mikaelarguedas or @codebot

[mikaelarguedas]

Yeah that's something I faced recently as well. So far SROS 2 was targetting only ROS topics and not ROS services. Now that every node starts a parameter server automatically to allow the use of external clock (clock topic) and sim time (use_sim_time parameter), the permissions need to be updated accordingly. We started discussing it here e381722 but I haven't made a PR yet to support that use case as there are a couple things I'm still nailing down.

Services also vary depending on the DDS implementation so providing a (not too lose) set of permissions for all DDS implementations may be challenging. In the case of RTI Connext we use the Service API (DDS-RPC) and leave it up to Connext to create the underlying topics, that leads to 2 surprising behaviors so far:

• Created topic names: If you look at the created topics, they are suffixed with custom strings (related to the guid of the participant), so I originally expected the rules for service topic to be more like <topic>MYSERVICENAMERequest*</topic> rather than <topic>MYSERVICENAMERequest</topic>. But after experimenting a bit it seems that the suffix is only used for CTF rules and is not impacting the advertised topic name or the rule evaluation and that the more restrictive approach works as well.
• Partition used for these topics: The other thing that seems to happen (Connext only as well AFAICT) is that some of these topics, (while reported with the right partition by the RTI tools like rtiadminconsole e.g. rr/talker ) are actually initially created with an empty partition field. I'm still looking to see if this is an issue in the way we use the Connext service API.
  Note that this will be only an issue for ROS Ardent as we will be moving away from partitions for namespacing in the future so ROS Bouncy should not face that challenge.

However, this use of wildcarding nullifies what we are trying to achieve here.

I'm surprised you couldn't get any permissions more restrictive than wildcarding everything... Simply adding empty partition to the list of allowed partitions for the service topics does the job for me (see example of permission file below).

<?xml version="1.0" encoding="UTF-8"?>
<dds xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xsi:noNamespaceSchemaLocation="http://www.omg.org/spec/DDS-SECURITY/20160303/omg_shared_ca_permissions.xsd">
  <permissions>
    <grant name="talker_policies">
      <subject_name>CN=talker</subject_name>
      <validity>
      <!--
       Format is CCYY-MM-DDThh:mm:ss[Z|(+|-)hh:mm]
                           The time zone may be specified as Z (UTC) or (+|-)hh:mm.
                           Time zones that aren't specified are considered UTC.
      -->
        <not_before>2013-10-26T00:00:00</not_before>
        <not_after>2023-10-26T22:45:30</not_after>
      </validity>
      <allow_rule>
        <domains>
          <id>42</id>
        </domains>
        <publish>
          <partitions>
            <partition>rt</partition>
          </partitions>
          <topics>
            <topic>parameter_events</topic>
          </topics>
        </publish>
        <publish>
          <partitions>
            <partition>rt</partition>
          </partitions>
          <topics>
            <topic>chatter</topic>
          </topics>
        </publish>
        <subscribe>
          <partitions>
            <partition>rt</partition>
          </partitions>
          <topics>
            <topic>clock</topic>
            <topic>parameter_events</topic>
          </topics>
        </subscribe>
        <subscribe>
          <topics>
            <topic>DCPS*</topic>
          </topics>
        </subscribe>
        <publish>
          <partitions>
            <partition></partition>
            <partition>rq/talker</partition>
          </partitions>
          <topics>
            <topic>get_parametersRequest</topic>
            <topic>get_parameter_typesRequest</topic>
            <topic>set_parametersRequest</topic>
            <topic>list_parametersRequest</topic>
            <topic>describe_parametersRequest</topic>
          </topics>
        </publish>
        <subscribe>
          <partitions>
            <partition></partition>
            <partition>rr/talker</partition>
          </partitions>
          <topics>
            <topic>get_parametersReply</topic>
            <topic>get_parameter_typesReply</topic>
            <topic>set_parametersReply</topic>
            <topic>list_parametersReply</topic>
            <topic>describe_parametersReply</topic>
          </topics>
        </subscribe>
      </allow_rule>
      <default>DENY</default>
    </grant>
  </permissions>
</dds>

---

E.g. what resources were attempted before failure, and which specifically resulted in insufficient privileges.

Agreed. Though it will need special logic within each rmw implementation and will likely require either implementing the Logging security module or at least increase the verbosity of DDS implementations loggers. What you see in the console is the content of the exception message provided by RTI Connext and ROS 2 doesn't have any additional information than what is currently displayed.

[ruffsl]

The other thing that seems to happen (Connext only as well AFAICT) is that some of these topics, (while reported with the right partition by the RTI tools like rtiadminconsole e.g. rr/talker ) are actually initially created with an empty partition field.

Simply adding empty partition to the list of allowed partitions for the service topics does the job for me (see example of permission file below).

Aw, yes! That did the trick, adding the exception for an empty partition with the service topics was the missing permission in this case. Perhaps Connext expert from RTI could explain the issue there. I was about to breakout my debugger, as I couldn't tell how the resources were being access differently than from external discovery data.

Note that this will be only an issue for ROS Ardent as we will be moving away from partitions for namespacing in the future so ROS Bouncy should not face that challenge.

🎉 Awesome! Combining the topic namespace and topic name will greatly simplify permission translations from keymint_ros packages to DDS permission files, i.e. less room for ambiguities. Presently, security profiles are quite convoluted, and results in large/verbose XML documents due to the orthogonal uses in logical circuits for policy definitions between DDS and what would been needed for Ardent. This change should also afford more straight forward compressions of collapsable rules that share common criterion.

BTW @mikaelarguedas , what do you mean by "CTF" and the rest of this statement:

But after experimenting a bit it seems that the suffix is only used for CTF rules and is not impacting the advertised topic name or the rule evaluation and that the more restrictive approach works as well.

[mikaelarguedas]

Perhaps Connext expert from RTI could explain the issue there. I was about to breakout my debugger, as I couldn't tell how the resources were being access differently than from external discovery data.

Yeah I considered contacting them but figured I'll look at how we create services first. I don't think it's an issue on our side so it may be worth contacting them. This will require a minimal pure Connext reproducible example (should be easily done by modifying their examples)

Presently, security profiles are quite convoluted, and results in large/verbose XML documents due to the orthogonal uses in logical circuits for policy definitions between DDS and what would been needed for Ardent.

Yeah, though there are still things that we assume that are not in the spec (like the Request/Reply suffix) but as all ROS 2 supported vendors comply with it that should be fine.

what do you mean by "CTF" and the rest of this statement:

Sorry typo, CFT => Content-Filtered Topics. For example if you look at one of the Reply topics in the rtiadminconsole, in the content_filter_property section you will see the topic name suffixed with the participan guid. I thought that originally the topic name itself was suffixed but maybe that's another thing that changed since a previous version.

[mikaelarguedas]

closing this as #33 and #38 got merged