-
Notifications
You must be signed in to change notification settings - Fork 272
Drop check for setcaps #709
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
base: main
Are you sure you want to change the base?
Conversation
Before bwrap was installed setuid (when user namespaces are not supported), it used setcap/file capabilities to acquire the privileges to create the bwrap environment. This was dropped 9 years ago in aedd613 ("Completely drop setcaps codepaths in favour of setuid"). and the check was added instead. Given that it has been 9 years, it is exceedingly unlikely that someone still has misconfigured bwrap in that particular way, so remove the check. Signed-off-by: Sebastian Wick <sebastian.wick@redhat.com>
80c8b6c to
9d13bfd
Compare
|
What's the purpose of this change? Is it to solve #380, or something else?
I hope you're right, but if you're wrong, the impact is a root security vulnerability, which makes me reluctant to do this (especially since I'm not really the main maintainer of bubblewrap - that would be @cgwalters). #676 is a maybe-safer version of this, although like I said on that PR, it does need some justification of why it's safe. |
|
This was prompted by https://gitlab.gnome.org/GNOME/glycin/-/issues/228, but I don't care too much really because I don't think that it is a valid use case. The code path only exists as a measure to prevent miss-configured bwrap binaries according to the comments and git log. If bwrap behaves as expected if it runs with certain caps is another question, but I don't think that figuring this is out is bwraps responsibility. |
Right, but there's misconfiguration and there's misconfiguration. If the result of bwrap being misconfigured is "doesn't work", then that's fine: we successfully "failed safe". But if the result of bwrap being misconfigured is "your unprivileged users can get root because you wrongly left it setcap For most binaries, it's normal for making it setuid or setcap to be a Very Bad Thing that will cause a security vulnerability, so this would be a non-issue; but my concern is that because, years ago, bubblewrap used to be documented to be safe (indeed desirable) to make setcap, users/sysadmins are going to blame us if they follow outdated documentation and the result is a system compromise. (I'm quite prepared to be overruled on this by a maintainer, if they take responsibility for dealing with any CVE fallout that results, but it is not something that I would be comfortable with approving myself.) |
True that.
Like I said, I don't care too much. We can just leave it as is, and if someone has a really good use case which needs this applied, we can still decide. |
|
I didn't dig into the original bug in detail but my intuition here is that a more secure way to achieve what they're trying to do would be to pass into the environment talk to a more privileged service accessible via a socket. It's basically DBus (more recently systemd socket activation) vs setuid. In the interim, I think they could drop the capability from the rest of the environment (i.e. whatever code runs bwrap). This all said, what we could do is switch to looking at the But...it doesn't seem worth merging as is just for this. |
Before bwrap was installed setuid (when user namespaces are not supported), it used setcap/file capabilities to acquire the privileges to create the bwrap environment. This was dropped 9 years ago in aedd613 ("Completely drop setcaps codepaths in favour of setuid"). and the check was added instead.
Given that it has been 9 years, it is exceedingly unlikely that someone still has misconfigured bwrap in that particular way, so remove the check.