Improve documentation for ROS 2 security #1662

SidFaber · 2021-06-07T14:55:54Z

Apologies for the length of this PR but I felt the first cut at a section of security tutorials should all go at once. This adds content currently in the sros2 github repository, adds a "concepts" page and some adds additional content as tutorials:

Update "Concepts" with a paragraph on ROS 2 Security
Add a new "About ROS 2 Security" page under "Concepts" to give a security overview, introduce security enclaves, summarize the six security files, and mention important security environment variables
Add a new "Tutorials" section titled "Security" and remove the current link to the sros2 repo under the "Demos" section
Migrate existing content in the sros2 github repo as new security tutorials "Introducing ROS 2 Security", "Security Across Two Machines" and "Security Access Controls"
Add "Understanding the ROS 2 Security Keystore" tutorial to explain details about the security files
Add "Examine Network Traffic" security tutorial to demonstrate inspection of raw network traffic
Update "Security Access Controls" content to show manual editing of a policy file to restrict enclave permissions
Update "Security Across Two Machines" content to use security standard alice and bob names for clarity rather than feather2 and oldschool
Combine Linux, MacOS and Windows content as tabs in one tutorial rather than as separate documents
Reference other tutorials where possible rather than including content in these docs (e.g., ROS installation, changing middleware, etc.)
Add quiz questions to "Introducing" and "Keystore" tutorials to improve learning

Include content currently in the sros2 github repository, a "concepts" page and add additional content as tutorials: - Update "Concepts" with a paragraph on ROS 2 Security - Add a new "About ROS 2 Security" page under "Concepts" to give a security overview, introduce security enclaves, summarize the size security files, and mention important security environment variables - Add a new "Tutorials" section titled "Security" and remove the current link to content in the sros2 github repository - Migrate existing content in the sros2 github repository the security tutorials "Introducing ROS 2 Security", "Security Across Two Machines" and "Security Access Controls"; combine Linux, MacOS and Windows content as tabs - Reference other tutorials where possible rather than including content here (e.g., ROS installation, changing middleware, etc.) - Add "Understanding the ROS 2 Security Keystore" tutorial discussing details about the security files - Add "Examine Network Traffic" security tutorial to demonstrate inspection of raw network traffic - Update "Security Access Controls" content to show manual editing of a policy file to restrict enclave permissions - Update "Security Across Two Machines" content to use security standard Alice and Bob names for clarity, rather than feather2 and oldschool - Add questions to "Introducing" and "Keystore" tutorials Signed-off-by: Sid Faber <[email protected]>

SidFaber · 2021-06-14T13:01:59Z

@ros-security-wg @ruffsl @mikaelarguedas any chance I can get some eyes on this? Thank you!!

ros-discourse · 2021-06-21T21:14:50Z

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-2021-06-17/21017/1

vmayoral

I liked it quite a bit @SidFaber! Well done.

I left a few suggestions and comments below.

vmayoral · 2021-06-25T13:13:53Z

source/Concepts.rst

+Security
+^^^^^^^^
+
+ROS 2 includes the ability to secure communications among nodes within the ROS 2 graph.


Suggested change

ROS 2 includes the ability to secure communications among nodes within the ROS 2 graph.

ROS 2 includes the ability to secure communications among nodes within the ROS 2 computational graph.

Though in the remainder of the content we can just omit it, let's make a clear distinction first between the ROS 2 computational graph and the ROS 2 data layer graph.

Updated with e80f5e1

vmayoral · 2021-06-25T13:20:31Z

source/Concepts.rst

+^^^^^^^^
+
+ROS 2 includes the ability to secure communications among nodes within the ROS 2 graph.
+Similar to discovery, security happens through the underlying ROS 2 middleware.


Suggested change

Similar to discovery, security happens through the underlying ROS 2 middleware.

Similar to discovery, security happens through the underlying ROS 2 middleware (provided it has support for the corresponding security plugins).

Newcomers won't know that there're different middleware implementations, with different levels of support for the plugins. We should specify.

Also, my feeling in here is that we should probably say a bit more about other middlewares (and not just assume DDS will be the only option, since there're already use cases were this changes depending on the application/domain).

Updated with e80f5e1

vmayoral · 2021-06-25T13:42:04Z

source/Concepts/About-Security.rst

+The identity and permissions certificates also have associate private key files.
+Add new enclaves to the domain by signing their Certificate Signing Request (CSR) with the identity certificate's private key.
+Similarly, grant permissions for a new enclave by signing a permissions XML document with the permission certificate's private key.
+


An example in here would be awesome.

I tried to leave this really lightweight, and leave all the examples in the tutorials. If you feel this should link to something else in this PR, leave me a suggestion and I'll add it in.

source/Concepts/About-Security.rst

source/Tutorials/Security/Examine-Traffic.rst

vmayoral · 2021-06-25T14:05:31Z

source/Tutorials/Security/Introducing-ros2-security.rst

+
+Take the Quiz!
+--------------
+


This is pretty cool 😄 !

Clever! I like this, too.

source/Tutorials/Security/Introducing-ros2-security.rst

mikaelarguedas

thanks Sid, this is great!

Is there a sister PR to retire the doc from the sros2 repo and refer to this set of tutorials instead ?

Dropped a couple inline remarks

mikaelarguedas · 2021-06-26T07:14:32Z

source/Tutorials/Security/Introducing-ros2-security.rst

+The ``sros2`` package provides the tools and instructions to use ROS2 on top of DDS-Security.
+The security features have been tested across platforms (Linux, macOS, and Windows) as well as across different languages (C++ and Python).
+
+Although we are designing SROS2 to work with any secure middleware, at the moment we are testing with RTI Connext Secure 5.3.1 and eProsima's Fast-RTPS 1.6.0.


We may need to update this. DDS implementation versions vary depending on the ROS 2 distro used, and I believe CycloneDDS is now tested at the same level as other implementations

Changed this wording to be somewhat more generic & distribution independent with e80f5e1.

mikaelarguedas · 2021-06-26T07:17:41Z

source/Tutorials/Security/Introducing-ros2-security.rst

+
+    If you don't have OpenSSL installed, please follow :ref:`these instructions <windows-install-binary-installing-prerequisites>`
+
+Fast-RTPS requires an additional CMake flag to build the security plugins, so the colcon invocation needs to be modified to pass:


To be updated ? All active ROS 2 versions now use Fast-DDS

Changed Fast-RTPS to Fast-DDS in e80f5e1. This was just a lift of the material from ths sros2 repo, is there more to be changed?

mikaelarguedas · 2021-06-26T07:18:49Z

source/Tutorials/Security/Introducing-ros2-security.rst

+Selecting an alternate middleware
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+If you choose not to use the default middleware implementation, be sure to `change your DDS implementation <../../Installation/DDS-Implementations>` before proceeding.


do these need a :ref: or a trailing underscore to render properly ?

@clalancette tagging you on this, more of a policy issue here. This link works on my local build, but to add a :ref: would mean a change to the DDS-Implementations page, even though it's not quite related to this PR.

Would you prefer adding the :ref:?

I think this is far enough along that we can come through and amend these smaller issues in a subsequent small PR.

I agree with @kscottz: amendments can be in smaller, future PRs.

mikaelarguedas · 2021-06-26T07:20:29Z

source/Tutorials/Security/Introducing-ros2-security.rst

+    .. code-block:: bash
+
+      cd ~/sros2_demo
+      ros2 security create_keystore demo_keystore


not sure how the tab system works, but is there a way for this type of line working across all platforms to not be duplicated n times ?

I believe there's a type of #include directive but figured it was overkill for what we're doing here?

mikaelarguedas · 2021-06-26T07:21:33Z

source/Tutorials/Security/Introducing-ros2-security.rst

+    .. code-block:: bash
+
+      ros2 security create_enclave demo_keystore /talker_listener/talker
+      ros2 security create_enclave demo_keystore /talker_listener/listener


same here, do we need to duplicate all commands 3 times if they're identical ?

Same comment as above.

mikaelarguedas · 2021-06-26T07:45:09Z

source/Tutorials/Security/Examine-Traffic.rst

+.. code-block:: bash
+
+  sudo apt update
+  sudo apt install tcpdump


Linux-only tutorial ?

Yes. Packet capture in Windows is a bit different, and the installation is quite different. If you feel this should be branded as a "Linux only" tutorial, just lmk.

I think it would be good to explicitly say that this is a Linux-only tutorial at the top. The same for any other tutorials that this is true for. This can be done, however, in a smaller PR.

mikaelarguedas · 2021-06-26T07:46:04Z

source/Tutorials/Security/Examine-Traffic.rst

+.. code-block:: bash
+
+  # In terminal 1:
+  source ~/.bashrc_ros2


what is the content of this file ?

As above, leftover from RosDevDays. Removed in e80f5e1.

mikaelarguedas · 2021-06-26T07:47:42Z

source/Tutorials/Security/Examine-Traffic.rst

+^^^^^^^^^^^^^^^^^
+
+Stop both the talker and the listener nodes.
+Enable encryption for both by setting the security environment variables and launch them again.


same remark about the use of "launch"

Updated in e80f5e1.

mikaelarguedas · 2021-06-26T07:50:18Z

source/Tutorials/Security/The-Keystore.rst

+
+  source ~/.bashrc_ros2
+
+  export ROS_SECURITY_KEYSTORE=~/sros2_demo/demo_keys


is there a way to share snippets between tutorials ? this environment setup would benefit from being referenced instead of explicitely written every time (getting the snippet with the tab for each platform would be awesome)

Discussed above. It's doable, but I don't believe it's worth the effort.

Also if you're asking if we should have the reader do some sort of shortcut--I actually intentionally keep adding this in to drive home the idea of the environment variables that really matter.

mikaelarguedas · 2021-06-26T07:50:26Z

source/Tutorials/Security/Examine-Traffic.rst

+.. code-block:: bash
+
+  # In terminal 1:
+  export ROS_SECURITY_KEYSTORE=~/sros2_demo/demo_keys


is there a way to share snippets between tutorials ? this environment setup would benefit from being referenced instead of explicitely written every time (getting the snippet with the tab for each platform would be awesome)

As above...

vmayoral · 2021-07-13T16:13:54Z

@clalancette can we please get some help to move this forward? @SidFaber proposal is quite valuable (and needed) IMHO.

Happy to rebase this and include all suggestions if @SidFaber or @clalancette can’t process reviews.

Signed-off-by: Sid Faber <[email protected]>

SidFaber · 2021-07-13T23:46:05Z

@vmayoral @mikaelarguedas @clalancette thanks a bunch for moving this forward, sorry I've been out of pocket for the last few weeks. Really appreciate the great feedback, please let me know anything you'd like to iterate on.

ros-discourse · 2021-07-21T09:50:33Z

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-2021-07-15/21453/2

source/Concepts/About-Security.rst

kscottz

This is great and we should get it merged pronto. I don't think the outstanding issues are necessarily Sid's responsibility and we can address them in subsequent small PRs.

I say we merge it in. @clalancette Is that okay with you?

kscottz · 2021-07-21T22:44:47Z

Also, Thanks @SidFaber this is great and we really appreciate it.

audrow

This looks good to me. I'm happy to get this moving.

In the future, it'd be much easier to review if this was broken into a few smaller PRs.

source/Tutorials.rst

source/Concepts/About-Security.rst

audrow · 2021-08-27T18:56:19Z

source/Tutorials/Security/Examine-Traffic.rst

+.. code-block:: bash
+
+  sudo apt update
+  sudo apt install tcpdump


I think it would be good to explicitly say that this is a Linux-only tutorial at the top. The same for any other tutorials that this is true for. This can be done, however, in a smaller PR.

audrow · 2021-08-27T18:58:52Z

source/Tutorials/Security/Introducing-ros2-security.rst

+Selecting an alternate middleware
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+If you choose not to use the default middleware implementation, be sure to `change your DDS implementation <../../Installation/DDS-Implementations>` before proceeding.


I agree with @kscottz: amendments can be in smaller, future PRs.

audrow · 2021-08-27T19:00:19Z

source/Tutorials/Security/Introducing-ros2-security.rst

+
+Take the Quiz!
+--------------
+


Clever! I like this, too.

Signed-off-by: Audrow Nash <[email protected]>

audrow · 2021-08-27T19:08:32Z

Thanks for the PR, @SidFaber - this was huge!

vmayoral reviewed Jun 25, 2021

View reviewed changes

mikaelarguedas reviewed Jun 26, 2021

View reviewed changes

Formatting and grammar updates

e80f5e1

Signed-off-by: Sid Faber <[email protected]>

kscottz reviewed Jul 21, 2021

View reviewed changes

source/Concepts/About-Security.rst Outdated Show resolved Hide resolved

kscottz approved these changes Jul 21, 2021

View reviewed changes

clalancette self-assigned this Aug 5, 2021

audrow approved these changes Aug 27, 2021

View reviewed changes

Minor formatting and wording changes

7929aed

Signed-off-by: Audrow Nash <[email protected]>

audrow merged commit f880ebd into ros2:rolling Aug 27, 2021

	ROS 2 includes the ability to secure communications among nodes within the ROS 2 graph.
	ROS 2 includes the ability to secure communications among nodes within the ROS 2 computational graph.

	Similar to discovery, security happens through the underlying ROS 2 middleware.
	Similar to discovery, security happens through the underlying ROS 2 middleware (provided it has support for the corresponding security plugins).


		If you don't have OpenSSL installed, please follow :ref:`these instructions <windows-install-binary-installing-prerequisites>`

		Fast-RTPS requires an additional CMake flag to build the security plugins, so the colcon invocation needs to be modified to pass:


		source ~/.bashrc_ros2

		export ROS_SECURITY_KEYSTORE=~/sros2_demo/demo_keys


		Take the Quiz!
		--------------


		Take the Quiz!
		--------------

Improve documentation for ROS 2 security #1662

Improve documentation for ROS 2 security #1662

Conversation

SidFaber commented Jun 7, 2021

SidFaber commented Jun 14, 2021

ros-discourse commented Jun 21, 2021

vmayoral left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikaelarguedas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

audrow Aug 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vmayoral commented Jul 13, 2021 • edited Loading

SidFaber commented Jul 13, 2021

ros-discourse commented Jul 21, 2021

kscottz left a comment

Choose a reason for hiding this comment

kscottz commented Jul 21, 2021

audrow left a comment

Choose a reason for hiding this comment

audrow Aug 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

audrow commented Aug 27, 2021

audrow Aug 27, 2021 •

edited

Loading

vmayoral commented Jul 13, 2021 •

edited

Loading

audrow Aug 27, 2021 •

edited

Loading