-
Notifications
You must be signed in to change notification settings - Fork 237
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
"--new-session" underadvertised and CVE-2017-5226 still a thing in 2023 by default? #555
Comments
bubblewrap is more of a tool for building sandboxes than a sandbox itself. All of its options map fairly directly to lower-level kernel features, and it's up to the authors of larger frameworks that use bubblewrap (like Flatpak) to provide a coherent security model using those features, which makes CVE-2017-5226 really more like a Flatpak vulnerability than a bubblewrap vulnerability. Because bwrap needs to be setuid on some systems, and has historically been setuid, any new code that we add to it has to weigh up the benefits of that new code path against the additional security exposure of having that code in a setuid executable, which is a good reason to avoid compiling seccomp filters in bubblewrap itself, making #150 unappealing. Also, because bubblewrap is a toolkit for building sandboxes more than a sandbox, it isn't in a great position to assess whether the options it is given are "safe" or "unsafe" (particularly tricky when some use-cases for it don't even intend to be a security boundary). If you have concrete suggestions for things that can be done better, please open merge requests - but if those merge requests add significant code to the security-sensitive critical path, or if they break backwards compatibility, then we won't necessarily be able to merge them. |
Hi @smcv , thanks for your reply! I understand that bubblewrap is useful to build sandboxes, but the visible marketing — in particular the GitHub repository main description "Unprivileged sandboxing tool" — are in some conflict with what you say about mostly setuid and not being an end user tool, not sure how subjective my reading is there. People building sandboxes on top of bubblewrap need a realistic chance to get it right and it was only luck that I heard about Best, Sebastian |
It is an unprivileged sandboxing tool - that's accurate. That's not the same thing as being a complete, ready-to-use sandbox with its own coherent security model. It is not my intention to be doing any marketing (and please see the license text for details of the extent to which there is no warranty). Where would you have expected to find details of the care required when constructing bubblewrap command-lines?
Honestly, I don't see bubblewrap as a standalone thing, more of an implementation detail of higher-level layers like Flatpak, libgnome-desktop and sandwine. It is possible to construct sandboxes with it, which might or might not comply with your security policy - we can't know whether they do or not, because we don't know your security policy (and the way you tell bubblewrap your security policy is by constructing its command-line arguments, hopefully carefully). |
I've published GHSA-7gfv-rvfx-h87x to clarify the handling of CVE-2017-5226. |
…model bubblewrap can provide a robust security boundary that severely limits functionality, or it can provide full functionality without any attempt at being a security boundary, or anything in between those extremes. If a caller of bubblewrap chooses inappropriate command-line arguments for their desired security model, then bubblewrap will not provide the security model they are aiming for, but this is not a bubblewrap vulnerability. Apparently this isn't clear to everyone, so try to clarify. The one place where bubblewrap *does* define some sort of security policy for itself is when it's setuid root, in which case it's responsible for preventing users from carrying out privilege escalation attacks like CVE-2020-5291. Resolves: containers#555 Signed-off-by: Simon McVittie <[email protected]>
Hi @smcv I had a chance to digest on this a bit more and to play with flatpak a bit to see its invocation of bubblewrap… Regarding marketing I should have been more precise, sorry. I understand that technically "Unprivileged sandboxing tool" is not wrong, but I would argue that's too easy to read as "end user sandboxing tool" while you seem to imply it's a lot closer to "plumbing-only sandboxing tool for senior sandbox developers only". Wouldn't it be great to resolve some of that ambiguity for more realistic user expectations? Regarding GHSA-7gfv-rvfx-h87x I think clarification is great but name CVE-2017-5226 is already hard-glued to bubblewrap to the Internet so I think this may create more confusion then less. Maybe just requesting a new CVE from Mitre with similar metadata would be better? Maybe I do not yet understand your idea fully. |
Do you have any better/clearer suggestions? |
@smcv it's a tough one! Maybe "Low-level process sandbox, USE WITH CARE"? The key ideas are:
What do you think? |
…5226 Without it, it is possible to escape the sandbox via TIOCSTI ioctls on the session PTY. Related: containers/bubblewrap#555 Related: containers/bubblewrap#142 Related: https://news.ycombinator.com/item?id=30825088 Signed-off-by: Ariadne Conill <[email protected]>
…-5226) Bubblewrap has an under-documented option which helps to protect against abuse of TIOCSTI ioctls against the session PTY to escape the build sandbox, the --new-session option. Related: containers/bubblewrap#555 Related: containers/bubblewrap#142 Related: https://news.ycombinator.com/item?id=30825088 Signed-off-by: Ariadne Conill <[email protected]>
…model bubblewrap can provide a robust security boundary that severely limits functionality, or it can provide full functionality without any attempt at being a security boundary, or anything in between those extremes. If a caller of bubblewrap chooses inappropriate command-line arguments for their desired security model, then bubblewrap will not provide the security model they are aiming for, but this is not a bubblewrap vulnerability. Apparently this isn't clear to everyone, so try to clarify. The one place where bubblewrap *does* define some sort of security policy for itself is when it's setuid root, in which case it's responsible for preventing users from carrying out privilege escalation attacks like CVE-2020-5291. Resolves: #555 Signed-off-by: Simon McVittie <[email protected]>
@alexlarsson I would like to note that the issue is now marked as closed while merging #564 did nothing to improve on the |
I can open this and then we can close it when the manpage adds more verbiage, but its not a default we can change without breaking existing users. |
Unless TIOCSTI is strictly needed, it should likely be disabled by setting You can also disable it at runtime with
|
@WhyNotHugo a lot has happened in this space since I first created this ticket and it is clear by now that |
Hi!
Thanks for making bubblewrap and sharing it as Software Libre! 🙏
Someone pointed out the importance of
--new-session
on Hacker News and I'm in debt to them for speaking up about it both personally and with sandwine. They go on saying:I checked the main readme for mentions of
--new-session
and found no matches and checked the help output and it doesn't mention security implecations. The man page has something but why check the man page if--help
seemed to answer the questions you new you had. So there really was no place other than Hacker News educating me prior to first usage and I could not even heard of--new-session
still, realistically.After seeing the CVE-2017-5226 demo from #142 (thanks!) work on my own terminal (scary!), including stealth mode with echo off, I agree that
--new-session
needs more user attention and/or become default. Maybe it needs a counter-part--same-session
also so that--new-session
can become the default at some point in the future if #150 is not being implemented — the latest reply there is off 2017. The warning idea from #162 — also of 2017 — will help with user education (which is good) but would come too late if bubblewrap is run behind the scenes rather than manually by the user.Any chance for motion in that direction?
Thanks and best, Sebastian
The text was updated successfully, but these errors were encountered: