ros2 / design

Design documentation for ROS 2.0 effort
http://design.ros2.org/
Apache License 2.0
222 stars 194 forks source link

MARA Threat Model #228

Closed olaldiko closed 5 years ago

olaldiko commented 5 years ago

This adds an extension to the original Threat Model proposed by @thomas-moulard started on #218 .

Based on the work done by the Amazon Robotics team, we have developed a threat model for an industrial cobot, the MARA by Acutronics Robotics.

We also have some additions on attack vectors and assets in the main threat table.

The objective of this threat model extension is to create a replicable base for creating threat models for different robots on industrial environments.

Work done by @o-olalde, @XabierPB and @borkenerice and myself (@olaldiko) as part of Alias Robotics.

vmayoral commented 5 years ago

Maybe it's worth sharing that Acutronic Robotics has been funding this work for the last few months and encouraged a release aligned with @thomas-moulard's (and the rest of his colleagues).

I will go through the PR tonight soon and will try contributing with what's been our experience from "the other side". @LanderU and @ahcorde, feel free to jump in and add your views as well.

thomas-moulard commented 5 years ago

Awesome! :+1: I'll start reviewing this but before that, would you mind making sure that the new files are passing the Markdown linter: mdl.

gem install --user-install mdl / mdl ./myfile.md

I had to ignore the MD033 forbidding inline HTML but everything else seems like things which can be fixed.

olaldiko commented 5 years ago

Thanks @thomas-moulard! Sure, we'll get on it ASAP!

thomas-moulard commented 5 years ago

@olaldiko this is great, thanks for sending this! Do you think we should align and use RVSS for all of the doc instead of DREAD?

olaldiko commented 5 years ago

@olaldiko this is great, thanks for sending this! Do you think we should align and use RVSS for all of the doc instead of DREAD?

While it would be nice, in this case RVSS is not meant for measuring risks, but actual vulnerabilities. It would be the equivalent of the CVSS for robotics. However, I think that it would be interesting to evaluate each of the robots with the RSF as well to define weak points and improvements. What do you think, @thomas-moulard?

thomas-moulard commented 5 years ago

@olaldiko That SGTM, not sure if we will be able to follow-up on that for TB3 right now but it seems better to have only on standard and make it evolve as/if needed. We can explain that in another section "How to add your robot to this threat model? You should evaluate your threats using RSF, etc.". If you would like to add this section in this PR that'd be great, otherwise it can done later by either you or us.

olaldiko commented 5 years ago

Sure @thomas-moulard, that SGTM too. I will start working on the new section!

olaldiko commented 5 years ago

We have added the guidelines for adding a new robot and merged the changes from upstream. From our side it looks good for merging. We would prefer to differ the evaluation of both MARA and TB3 with the RSF to another PR in order to ease the update with other currently open PRs.

thomas-moulard commented 5 years ago

@olaldiko Let's rebase this and we can merge the doc 🚢

olaldiko commented 5 years ago

Good! @thomas-moulard, @tfoote, what would be the procedure here? Shall we rewrite history on the PR itself or the rebase is performed when merging into gh-pages branch?

thomas-moulard commented 5 years ago

I'll let @tfoote answer this one. I did rebase and pushed with git push --force-with-lease but I know not everyone agrees this is the "right thing to do".

vmayoral commented 5 years ago

@tfoote and @thomas-moulard can we merge this please?

We are preparing a few additional contributions that should ship soon after this one.

vmayoral commented 5 years ago

@olaldiko are you guys fine with it?

olaldiko commented 5 years ago

Sure! All clear from our side!

El vie., 26 abr. 2019 13:42, Víctor Mayoral Vilches < notifications@github.com> escribió:

@olaldiko https://github.com/olaldiko are you guys fine with it?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ros2/design/pull/228#issuecomment-487027792, or mute the thread https://github.com/notifications/unsubscribe-auth/AANBZKDXXMXAHN6TA6H6F63PSLTBDANCNFSM4HEGXKIQ .

thomas-moulard commented 5 years ago

@olaldiko Can you rebase the doc? I will merge it once it's done.

vmayoral commented 5 years ago

Thanks a lot for reacting @thomas-moulard, great!

@olaldiko, happy let me know if you need help rebasing the file.

olaldiko commented 5 years ago

@thomas-moulard, I think that we have successfully rebased the commits now. Please, check and let me know if any changes need to be done!

thomas-moulard commented 5 years ago

@olaldiko Could you check again? I can't merge because there are still conflicts in this PR:

Rebasing the commits of this branch on top of the base branch cannot be performed automatically due to conflicts encountered while reapplying the individual commits from the head branch.

olaldiko commented 5 years ago

@thomas-moulard , I have been doing several tests on my own fork, and I think that the best strategy here would be to squash and merge into the base branch instead of rebasing. This way the changes will show up as a single commit on the gh-pages branch. I think that this was done in #218 and others as well, as each PR shows up as a single commit on base. No conflicts arise following this strategy. Could you please check and tell me your thoughts? Thanks!

dirk-thomas commented 5 years ago

@thomas-moulard This document doesn't follow the style guide for markdown (https://index.ros.org/doc/ros2/Contributing/Developer-Guide/#markdown-restructured-text-docblocks). Please update the doc to follow the one-sentence-per-line rule. See https://github.com/ros2/design/pull/236/files#diff-60aa2b3022e7728b236f31c650141eceL3544 for a case why the current state is not desired.

thomas-moulard commented 5 years ago

Thanks Dirk for checking, I assigned to myself https://github.com/ros2/design/issues/237 and I'll do that by EOW.

thomas-moulard commented 5 years ago

https://github.com/ros2/design/pull/238 should fix style issues.