Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add docs about security contact and threat model #20529

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JanZerebecki
Copy link

I have linked to the good blog post as a starting point.

That specific file name in the Git root is now used by various tools.

For future reference what a threat model documentation looks like, a few links from my bookmarks are:

I have linked to the good blog post as a starting point.

That specific file name in the Git root is now used by various tools.

For future reference what a threat model documentation looks like,
a few links from my bookmarks are:
- https://github.com/TryQuiet/quiet/wiki/Threat-Model
- https://docs.openprivacy.ca/cwtch-security-handbook/risk.html
- https://code.briarproject.org/briar/briar/-/wikis/threat-model
The post is not exactly clear on this, but there could be a security boundary
between cockpit-ws running on one host and another host where cockpit-bridge is
spawned via ssh, where if the second host was infected with malware it should
be prevented from harming the first.
Copy link
Author

Choose a reason for hiding this comment

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

I'm not satisfied with this last sentence, because it doesn't give a decision. But I'm not in a situation to suggest one way or another. Nor have I reviewed the security.

Any suggestion if this should say it currently is or is not treated as a security boundary?

@mmartinv I think we talked about this?

Copy link
Member

Choose a reason for hiding this comment

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

With directly logging into a remote host from the login page, there is a strong security boundary, exactly the same as with ssh remotehost. I.e. a malicious remotehost can't influence the source host.

However, this completely breaks down when using the "Add new host" functionality. This is somewhere between "extremely hard" and "impossible" to fix, so we are now considering to entirely remove that functionality. See https://issues.redhat.com/browse/COCKPIT-870 for details. Until we figure out how to do that without breaking existing customers, the text here should point out this distinction.

JanZerebecki added a commit to JanZerebecki/cockpit-project.github.io that referenced this pull request May 28, 2024
martinpitt pushed a commit to martinpitt/cockpit-project.github.io that referenced this pull request May 29, 2024
Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

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

Thanks, and sorry for the very late reply! I'm afraid we've been completely swamped recently. This is just a sketch for now, and will take a few hours of writing and refinement to become actually useful. There's a Red Hat internal document about cockpit's threat model and in-depth analysis. If you want to work on this further, I can ask the authors if it can be shared with you.

@@ -0,0 +1,14 @@
# Reporting Security Issues
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer this to go into README.md, so that it gets rendered in the default github project view. It also creates clutter in the project root dir. It does seem fairly common, though -- which tools look at this? If this is about .well-known/security.txt, that could just point to an anchor in README or a file in doc/.

Until there is a more concise explanation here about the threat model (what it
defends against) and assurance case (why it is secure), one should be able to
get the necessary informaion from this blog post:
https://cockpit-project.org/blog/is-cockpit-secure.html
Copy link
Member

Choose a reason for hiding this comment

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

This paragraph is a placeholder, and the blog post is rather dated -- the screenshots don't match current versions any more, and the video is gone. The text is still current, though. Perhaps we should copy the text here then? But that's still not the format of a threat model, "just" a description.

The post is not exactly clear on this, but there could be a security boundary
between cockpit-ws running on one host and another host where cockpit-bridge is
spawned via ssh, where if the second host was infected with malware it should
be prevented from harming the first.
Copy link
Member

Choose a reason for hiding this comment

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

With directly logging into a remote host from the login page, there is a strong security boundary, exactly the same as with ssh remotehost. I.e. a malicious remotehost can't influence the source host.

However, this completely breaks down when using the "Add new host" functionality. This is somewhere between "extremely hard" and "impossible" to fix, so we are now considering to entirely remove that functionality. See https://issues.redhat.com/browse/COCKPIT-870 for details. Until we figure out how to do that without breaking existing customers, the text here should point out this distinction.

@@ -0,0 +1,9 @@
Until there is a more concise explanation here about the threat model (what it
Copy link
Member

Choose a reason for hiding this comment

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

Does this really have to be a separate file? Both SECURITY.md and threat-model.md are tiny, this feels like over-splitting.

@martinpitt martinpitt marked this pull request as draft June 24, 2024 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants