-
-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
Chromium is not adequately sandboxed on Linux 3.4 #895
Comments
You need 3.8 or newer kernel as you can see, you need user namespaces
|
I think the setuid bit should be set so that the SUID sandbox works in < 3.8 |
Although you are probably right, I would not like to run a web browser with setuid root :) I think the easiest would be to use NixOS' The only "clean" way to both install chromium and add it to setuidPrograms, would be to create a full nixos module just for chromium.
Which sounds a bit like overkill. |
Just |
I agree. But I'm afraid this is impossible with nix. So you would need to patch chromium to point it to a I think it's easier just to warn users of older kernels that chromium can work more safely on 3.8+. |
Well, we patch the sandbox path for chromium 30 directly, as it is more or less hardcoded. What I somewhat hoped is that we could have USER_NS enabled by default in kernel 3.8. Unfortunately, it's not possible because of XFS not supporting user namespaces yet. There are a few patchsets sent to the LKML that address this which are not yet in mainline, even by 3.11. An alternative to this, we could patch the userns sandbox to call the setuid wrapper, in case Another thing thing that bothers me:
I think that's at least a first step if we have that in the standard kernel, what do you think? |
I don't fully understand the XFS part. What happens if you turn on USER_NS? Will it break XFS? Or just work for everyone, except for XFS users? I mean I'm pretty sure vfat will not be supporting user namespaces either :) Seccomp is probably not very invasive, so I'm all for adding it to the standard kernel. The sandbox path hacking to get setuid going sounds like a lot of work, but if you need it for chromium 30 anyway, I guess there's no reason not to. Of course by pointing to paths outside the store (for setuid) it will break for non-nixos users. But I'm not sure any of them use nixpkgs chromium anyway. |
I used chromium from nixpkgs before i had nixos installed.
|
Well, if you have XFS enabled or as module in the kernel config, you simply cannot activate The hackery on the sandbox path is just for the userns-sandbox, which obviously shouldn't be setuid. But my appreach would be that the userns-sandbox just attempts to start the SUID sandbox wrapper if the cloning should fail. We could also getenv() something like edit: A downside of the wrapper approach would be that we need to have the sandbox in the system profile. |
Okay, just had a detailled looked into it and this needs changing the patches or alternatively adding another patch in order to be fixed properly. I'm going to split up the source into Chromium and the sandbox, so that both can be built separately and people don't need to build the full Chromium derivation on a nixos-rebuild. This means, that the fix for it will take some while (my weekend will be pretty busy), but I'll do my best to find time to fix it, whenever possible before the weekend. |
@aszlig Are you still working on this? |
yep |
Okay, just found out that in version 26, the USER_NS sandbox is going to be implemented upstream, so I'm not going to refactor this any further for 14.04, because we already have USER_NS available in NixOS 14.04 by default. I'm just not sure what we're going to do for non-NixOS distros that are using kernels < 3.8, because the SUID sandbox is tied to the Chromium version and also has to be referred by it (also with a back-reference to the Chromium Zygote host). There is still the error mentioned in the text of the issue, so people running older kernels should be aware of it and if this is needed for 14.04, we can backport the corresponding changes from Chromium 26 to implement the fallback. |
Closed, because user namespaces work with recent kernels and they're going to be included upstream someday[TM] as well. If someone still optionally wants to have the setuid-root sandbox, please open a new pull-request with an implementation. |
Not much has changed since, right? (#10809) |
@vcunat: Changed in terms of supporting the setuid sandbox on non-NixOS? |
I personally would be happy to see this fixed. While I do use NixOS at home, I use CentOS at work since that's the "standard" Linux distribution there. Nix packages aren't supposed to be tightly coupled to NixOS, are they? |
Yes, without
There certainly are aspects that cause problems on non-NixOS. Even for our glibc the support for 2.6.32 kernel wasn't self-evident (b5a605c) but that was possible to fix easily. This issue might not be. |
As one of those users, yes, I'd be happy enough to run it without sandboxing when I'm on CentOS 6. |
For our own implementation of the user namespaces sandbox (added in c06c636), the browser reverted to no sandboxing if no support for user namespaces was available. Since there is an upstream implementation (https://crbug.com/312380), the behavior is to bail out if sandboxing isn't available. And IMHO this is a good thing, because people who deliberately want to run without sandboxing can still do it with If someone wants to do that, I'm fine with merging it, but just running silently without sandboxing if initialization fails is not an option. |
Oh, I didn't know it's enough to pass a command-line argument... |
@aszlig I'm having a bit of difficulty following exactly what you are saying since I don't follow the sandboxing concerns closely on either the Chromium side or the Nix side, but it sounds to me like you're saying, in summary, that Chromium is broken on non-Nix distributions with older kernels like CentOS. It's good to know about the command-line argument, which I will try. However, I don't quite understand what you mean by "adding a way to have an external sandbox binary that can be made setuid"? What is an external sandbox, what would this achieve, and do you have any pointers on how to get started on it? |
@a1russell: External because it would otherwise require to have setuid binaries in the Nix store. The Chromium setuid sandbox is basically an external binary which is setting up the chroot, seccomp, namespaces, whatnot and is spawned by the zygote process. So if we really really really want that SUID sandbox, we'd still need to add either a commandline flag/env variable to point to the sandbox binary, or set it to a static location outside of the store. |
@aszlig I think I understand now. Is there precedent for installing binaries outside the Nix store? Or perhaps, in order to install to something like $XDG_DATA_HOME, we install on first run via a wrapper script around Chromium. Just brainstorming... |
When I start chromium, I see this warning:
[57340:57340:0901/111157:ERROR:zygote_host_impl_linux.cc(174)] User namespace sandbox failed to start, running without sandbox! You need at least kernel 3.8.0 with CONFIG_USER_NS enabled in order to use the sandbox without setuid bit.
chrome://sandbox reports:
I have tried kernels
and
cc @aszlig
The text was updated successfully, but these errors were encountered: