search

ros-infrastructure / rep

ROS Enhancement Proposals
http://www.ros.org/reps/rep-0000.html
149 stars 136 forks source link

[REP-2015] ROS 2 DDS Security PKCS#11 Support #375

Open Mario-DL opened 1 year ago

Mario-DL commented 1 year ago

This PR introduces a new REP-2015 proposal based on https://github.com/ros2/design/pull/332 concerning the ROS2 DDS Security PKCS#11 Support.

MiguelCompany commented 1 year ago

@fujitatomoya @clalancette Any chance this could be in soon? It is currently being discussed on the Security WG.

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-4-20-2023/31087/1

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/announcing-rep-2015-ros-2-dds-security-pkcs-11-support/31357/1

kscottz commented 1 year ago

Please stylize the name as "ROS 2" not "ROS2" as mentioned in the ROS style guide.

MiguelCompany commented 7 months ago

The main difference is that the certificates and, more importantly, their private keys will be stored in a hardware security module (HSM), i.e. an external device. Operations with the private keys will be executed inside the HSM, so they will never be in the computer's memory.

MichaelOrlov commented 7 months ago

@MiguelCompany Thanks for the explanation. Now it has become clear and does make sense.

clalancette commented 7 months ago

I think overall the addition of this feature to SROS 2 makes sense. I have two ovearching comments here:

  1. While I know that this functionality is coming from the DDS side of things, I think this REP is too DDS-specific. It is fine to reference the DDS Security specification as an inspiration, but I think the rest of it should be DDS-agnostic. In particular, how would a non-DDS RMW implement this?
  2. Using the file extension as the way to distinguish between a file and a URI isn't great, in my opinion. I'd much rather see us explicitly pass some kind of metadata (an enum or something) through the layers so that it is clear what kind of data we are passing.
ros-discourse commented 6 months ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-meeting-minutes-2024-02-15/36221/1

MiguelCompany commented 6 months ago
  1. I think this REP is too DDS-specific

Yes and no. Regarding PKCS#11 URIs, they are defined in RFC 7512 so I would not consider them DDS-specific.

The original SROS 2 design article by @kyrofa is DDS-specific.

There has never been a REP on SROS 2, we opened this one because we were told to here. Perhaps the original authors of the design documents @kyrofa, @ruffsl, and @mikaelarguedas could come up with a proper REP on ROS 2 security.

Regarding how would a non-DDS RMW implement this, the interfaces are designed so that all the RMW receives is a string with the security enclave. How that string translates into specific security artifacts is not part of the RMW interface.

I think this also answers point 2. I should also point out that this was discussed in the security WG, and it was the best solution we found meeting the following requirements:

The contents of this REP might need a refactor (perhaps even a full rework to convert it into a proper ROS 2 security REP), but I don't think that should block the corresponding implementation PRs: