Skip to content
This repository was archived by the owner on Oct 23, 2025. It is now read-only.

REP-2006: ROS 2 Security Vulnerability Disclosure Policy.#262

Merged
clalancette merged 15 commits intomasterfrom
rep-2006
May 28, 2020
Merged

REP-2006: ROS 2 Security Vulnerability Disclosure Policy.#262
clalancette merged 15 commits intomasterfrom
rep-2006

Conversation

@clalancette
Copy link
Copy Markdown
Contributor

This PR adds in REP-2006, which is the Security Vulnerability Disclosure Policy for ROS 2. The original proposal was drafted by the ROS 2 Security Working Group, with some further edits by myself and @SidFaber .

We've decided to open this as a REP (rather than just documentation) since it affects all of the ROS 2 Common Packages in REP-2005. Thus we would like to give the community a chance to weigh in on this policy.

@chapulina @ros2/security_working_group @thomas-moulard FYI.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@thomas-moulard
Copy link
Copy Markdown

This LGTM, thanks for pushing on this!

Copy link
Copy Markdown
Contributor

@mikaelarguedas mikaelarguedas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad to see this work turn into a REP!


The Vulnerability Disclosure Program outlined here covers all code within the ROS 2 Common Packages as defined in REP 2005.

The ROS Security Working Group may also be able to assist you with reporting vulnerabilities which are not in scope of this policy but which are related to ROS or more generally to robotics.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be done by the same entity / group of people as the people processing the reports?

If not, how would one contact the working group for these not-covered-by-this-rep vulnerabilities?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be done by the same entity / group of people as the people processing the reports?

By "done", do you mean helping to contact the relevant companies, or doing the engineering work?

If not, how would one contact the working group for these not-covered-by-this-rep vulnerabilities?

I believe the intention here is to say "if you find a vulnerability in a robot/software not directly related to this REP, we may know the people who are responsible and can direct you there". I would think the reporting process would be the same as outlined below, its just that the ROS 2 security group wouldn't take any direct action other than redirecting.

But I didn't write this. @SidFaber, any feedback here?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reporting to happen through the security@ email address; the security WG's best communication vehicle is our matrix chat room. The person/people triaging incoming email to that address should also be included in the Security WG matrix chat room, and simply call to the chat room for any support needed.

So if the triage person is not able to quickly handle the report, they should call out for help in matrix.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reporting to happen through the security@ email address

This part of the REP is discussing reporting things that are not in scope. In that sense, do we really want them still reporting to security@? I'd say the reporter should contact the WG on their own (in matrix or via our google group) for help on issues outside the scope of the VDP, or security@ could get swamped with out of scope issues.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now this doc only gives the reader a single entry point which is the security@ email. If we want reports elsewhere, we need to add another entry point. The google group is private, and I think we want to keep the matrix room private?

Agreed the security@ email could get spammed, but it should only be with security related issues. I would imagine OR would manage this similar to abuse@ and postmater@ email aliases.

Copy link
Copy Markdown
Contributor

@kyrofa kyrofa May 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want reports elsewhere, we need to add another entry point. The google group is private, and I think we want to keep the matrix room private?

Neither are private, nor should they be. That's one of the reasons we don't want the WG to be driving vuln handling-- we can't handle embargo in a public group, and we want the security WG to be completely open to the public.

We could add a link to https://github.com/ros-security/community, which contains the security WG communications channels (I notice now that we're missing the google group email though, I'll add that).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Security WG has always seemed very open to discussing security issues, but looking at it now I don't see a public facing, easily accessible "contact us" landing place. I've added an agenda item to discuss public interaction at the next WG meeting and would propose leaving this as-is for the time being. Once the WG settles on an approach for public interaction, we can open a PR on how to interact with the Security WG.

Copy link
Copy Markdown
Contributor Author

@clalancette clalancette May 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've changed this even more down below. @SidFaber @kyrofa is there more to be done for this particular item, or can we resolve this conversation?

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@Blast545
Copy link
Copy Markdown

+1, it's great to see this!

@clalancette
Copy link
Copy Markdown
Contributor Author

At this point, I think the outstanding things to answer here are:

  1. Do we need the verbiage I added in REP-2005? I could go either way with that.
  2. We should start collecting some of the "process" information into a document somewhere. That will obviously evolve with time, but a starting document that has some of what we discussed above would be a good thing to have. @SidFaber to put this somewhere.
  3. Is there anything more we need to discuss about REP-2006: ROS 2 Security Vulnerability Disclosure Policy. #262 (comment) ? @kyrofa has a concern that we don't want all security-related issues for all robots, so maybe we should tighten up that language to just say "ROS-related"?
  4. Reviews and approvals.

Are there any other outstanding items here?

Copy link
Copy Markdown
Contributor

@vmayoral vmayoral left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty happy to this got into a REP. After last week's discussion I've done a more careful review of the current content and reflected some concerns that I believe should be addressed.

clalancette and others added 4 commits May 18, 2020 08:56
Co-authored-by: Víctor Mayoral Vilches <v.mayoralv@gmail.com>
Co-authored-by: Sid Faber <56845980+SidFaber@users.noreply.github.com>
Co-authored-by: Víctor Mayoral Vilches <v.mayoralv@gmail.com>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette
Copy link
Copy Markdown
Contributor Author

All right, with all of the latest fixes, I think the only unresolved conversation here is around the point of contact for the working groups. @kyrofa @SidFaber can you look into resolving that and propose changes to the documents?

I don't think there are other open comments here. @ros2/security_working_group any further thoughts? Otherwise I'd look to get some final reviews and approvals here. Thanks to everyone who has been involved in the conversation so far.

@vmayoral
Copy link
Copy Markdown
Contributor

Sent a last minor comment, other than that, looks pretty good IMHO.

Copy link
Copy Markdown
Contributor

@SidFaber SidFaber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good with this as it stands, I believe all the open items have been addressed.

Co-authored-by: Víctor Mayoral Vilches <v.mayoralv@gmail.com>
Copy link
Copy Markdown
Contributor

@mikaelarguedas mikaelarguedas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great!

@ros2/security_working_group any further thoughts?

AFAIK tagging @ros2/security_working_group doesn't have any effect on this org. It seems that almost all people involved in the original draft gave feedback here, tagging @ros-security-wg @ruffsl @artivis to be sure they get a chance to give feedback.

Otherwise I'd look to get some final reviews and approvals here.

It looks like this draft reached enough consensus to rope in the wider ROS community by sending an RFC on Discourse, WDYT?

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette
Copy link
Copy Markdown
Contributor Author

It looks like this draft reached enough consensus to rope in the wider ROS community by sending an RFC on Discourse, WDYT?

Sure, I'll post something there.

@ros-discourse
Copy link
Copy Markdown

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

https://discourse.ros.org/t/ros-2-security-vulnerability-disclosure-policy/14242/1

This was referenced May 22, 2020
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette
Copy link
Copy Markdown
Contributor Author

3\. A REP in the ROS universe/community is absolute "acceptable". But a pentester might not be able to easily find this information inside this REP, as he might not be familiar with the community and REPs. After reading the page from hackerone it might be a good practice to add a `SECURITY.md` file to the covered packages listed in REP 2005. Content of such a file would be minimal and link to this REP 2006. Such standards might also help increase visibility for the effort to become more security aware.

As you might have seen with all of the PRs I just linked here, there is now (or will be once we merge those PRs) text in the QUALITY_DECLARATION.md saying where to find the Vulnerability Disclosure Policy. I know it isn't the same thing as SECURITY.md, but I also am not at all sure that either is an industry standard.

Copy link
Copy Markdown

@ablasdel ablasdel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great. It is going to be amazing to have an official security policy for the Foxy launch!
Impressive work!

@ros-discourse
Copy link
Copy Markdown

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

https://discourse.ros.org/t/ros-2-security-working-group-meeting-05-26-2020/14299/1

Copy link
Copy Markdown

@hidmic hidmic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@clalancette checked. Not much more than a proof-read.

clalancette and others added 2 commits May 26, 2020 17:26
Co-authored-by: Michel Hidalgo <michel@ekumenlabs.com>
Co-authored-by: Michel Hidalgo <michel@ekumenlabs.com>
@clalancette
Copy link
Copy Markdown
Contributor Author

All right, there have been no comments for 2 days here, and no substantial changes in 6 days. The only open issues really revolve around internal processes, which will be done separately. Finally, there is also approval from a number of people, so I'm going to go ahead and merge this. If there are any changes we wish to make, we can do it in follow-up PRs.

Thanks to everyone who contributed to the discussion here!

@clalancette clalancette merged commit c368ff2 into master May 28, 2020
@clalancette clalancette deleted the rep-2006 branch May 28, 2020 14:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.