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

feature request: alternative to user namespace sandbox on Linux #3420

Closed
diracdeltas opened this issue Feb 19, 2019 · 23 comments
Closed

feature request: alternative to user namespace sandbox on Linux #3420

diracdeltas opened this issue Feb 19, 2019 · 23 comments
Labels
feature-request OS/Linux packaging Making Brave natively available to install on a variety of platforms, app stores, and repositories. priority/P5 Not scheduled. Don't anticipate work on this any time soon. security

Comments

@diracdeltas
Copy link
Member

(follow up to #1986 since that issue got closed by my PR to change the Linux sandboxing error message)

Problem:

  1. On Linux, Brave generally does not run unless either the --no-sandbox flag is passed or user namespaces are enabled.
  2. Previously, the default error message told users to try running with --no-sandbox. In Add more helpful message for running on Debian brave-core#1204, I changed it to tell users they probably need user namespaces enabled. The rationale was that running with any sandbox, even a subpar one, is better than running without any sandbox.
  3. Users often complain to us that we should not be encouraging enabling of userns because it's an insecure/poorly-designed feature. Citations include https://lists.archlinux.org/pipermail/arch-general/2017-February/043066.html and https://wiki.archlinux.org/index.php/security#Sandboxing_applications.

The main argument from https://lists.archlinux.org/pipermail/arch-general/2017-February/043066.html is that userns has so many vulnerabilities to the point of being useless, and hence is security theater; in other words we should assume any code that runs with some privileges in a user namespace can run with those privileges on the host itself.

However, if that were true, running Brave in a userns sandbox would be equivalent to running Brave without a sandbox. Does userns actively make security worse? (Other than perhaps luring people into a false sense of security.)

Some potential options:

  1. In the error message (Add more helpful message for running on Debian brave-core#1204), mention that userns is considered harmful by some and the other option is to just run without the sandbox. I'm not a big fan of this since it's a complicated and not-necessarily-actionable message for non-technical users.
  2. Ship the Chromium setuid sandbox which is supposedly being deprecated soon or was already deprecated, also citing security reasons: https://bugs.chromium.org/p/chromium/issues/detail?id=312380
  3. Write our own sandbox :(
@Jacalz
Copy link
Contributor

Jacalz commented Feb 19, 2019

The issue is somewhat solved for the snap application since snap apps use sandboxing as long as they are set up properly. But as you say this is a security flaw for non snap apps.

This should still be concidered pretty important though due to the fact that a lot of people doesn’t use snap. It has flaws like being slow, bad font and theme integration and a couple other issues...

@diracdeltas
Copy link
Member Author

@posix4e how does sandboxing on snap work?

@posix4e
Copy link
Contributor

posix4e commented Feb 19, 2019

"The issue is somewhat solved for the snap application since snap apps use sandboxing as long as they are set up properly. But as you say this is a security flaw for non snap apps." -> Snap also uses userns for security and we now have proper isolation in the snap. That being said, although I am the package maintainer for snap, I'm still concerned about the runtime performance.

Dropping UserNS support is a fantasy:
Along with docker, the linux kernel, chromium, firejail, seccomp bpf and apparmor all use it. I fear that having a setuid root binary on brave is a big step backwards, but if a user wants to submit a patch backporting it from chromium I'd be happy to help, but I am not willing to put my name behind the idea.

I have never gotten a direct answer on why the LD_LIBRARY_PATH attacks wouldn't allow any library, installed and run by a setuid binary to have full root access on the system. I believe this is why chromium is dropping setuid

@Mikaela
Copy link

Mikaela commented Feb 20, 2019

I think this is the documentation for snap sandboxing: https://docs.snapcraft.io/snap-confinement/6233

However the brave snap recommends people not use it and instead get the official package. (Sorry, only after posting this I read the previous comment well enough to notice that posix4e is the Snap package maintainer :))

Flatpak (#1000) would also have sandboxing documentation & reference and they would allow you to host the repository by yourself while in Snap users can only use a single repository at a time.

@posix4e
Copy link
Contributor

posix4e commented Feb 20, 2019

@Mikaela Let's be a bit more nuiansced. It is particularly interesting to know why we don't recommend the snap. This is because it's not a part of the normal brave build and security reviews (@diracdeltas will have more) not because of a specific limitation of snapcraft vs flatpack. That being said if you are interested in helping with a flatpak, please email because i was going to start on one

@jacobc-eth
Copy link

Thank you for creating this issue @diracdeltas

Of your three potential solutions, I think that re-enabling the Chromium solution is a good stop gap. If Chromium really will deprecate its sandbox, tthat will recreate these same issues for many Linux users, and the Chromium project will have a great deal more resources to resolve it at that time.

That said, I think Brave having a sandbox that is superior to what Chromium and other browsers provide is actually a powerful additional unique value proposition to the Brave community who tend to be very serious about security.

Option 1 would be really wrong, imho.

@jacobc-eth
Copy link

It has been two months, with Linux user's security still degraded. Is there any update around this issue? @diracdeltas

@cribari
Copy link

cribari commented Apr 25, 2019

I agree with @jacobcantele .

@Brave-Matt
Copy link

+1 https://www.reddit.com/r/brave_browser/comments/bnew3b/brave_run_with_sandbox_disabled/

@Brave-Matt
Copy link

@tildelowengrimm tildelowengrimm added the packaging Making Brave natively available to install on a variety of platforms, app stores, and repositories. label Jun 29, 2019
@tildelowengrimm tildelowengrimm added the priority/P5 Not scheduled. Don't anticipate work on this any time soon. label Aug 6, 2019
@jonathonf
Copy link

it's an insecure/poorly-designed feature. Citations include

You refer to a mailing list post from 2017. What are the/any/some current security concerns with the feature?

@madaidan
Copy link

@Jacalz

The issue is somewhat solved for the snap application since snap apps use sandboxing as long as they are set up properly.

No, it isn't. The snap sandbox isn't even comparable to chromium's.

@posix4e

Dropping UserNS support is a fantasy:
Along with docker,

Obviously docker needs it for unprivileged containers.

the linux kernel,

The linux kernel does not need it. Entire user namespace support for the kernel can easily be dropped just by setting CONFIG_USER_NS=n

chromium,

That's what this issue is trying to solve.

firejail,

An insecure sandbox with far too large attack surface is not a reason to decrease kernel security for everyone.

Besides, firejail doesn't need user namespaces either.

netblue30/firejail#425 (comment)

netblue30/firejail#9 (comment)

seccomp bpf

seccomp-bpf filters syscalls. It has absolutely nothing to do with user namespaces.

and apparmor all use it.

apparmor also has nothing to do with user namespaces.

I fear that having a setuid root binary on brave is a big step backwards,

Exposing a whole lot of kernel code to unprivileged users is an even bigger step backwards.

@Mikaela

Flatpak (#1000) would also have sandboxing documentation & reference

Flatpak isn't comparable to chromium's either. Flatpak has a completely broken sandbox. It mounts /home as read-write for most apps meaning to escape you just need to run echo evil_malware >> .bashrc. I believe snap does the same

@jonathonf

What are the/any/some current security concerns with the feature?

It still exposes huge attack surface. Create an unprivileged user namespace with a net namespace and now you have access to the entirety of netfilter code. That mailing list post is still true. You can even ask Micay himself if you want.

@jonathonf
Copy link

jonathonf commented Oct 12, 2019

It still exposes huge attack surface. Create an unprivileged user namespace with a net namespace and now you have access to the entirety of netfilter code. That mailing list post is still true. You can even ask Micay himself if you want.

This is fine, but that discussion doesn't actually provide any examples, and a response (https://lists.archlinux.org/pipermail/arch-general/2017-February/043076.html) contradicts all of those claims. Arch have even enabled user namespaces in recent kernels...

If someone could show a current vulnerability (ideally with some sort of PoC) then it would make an argument against user namespaces much easier. Referring to historical issues which have since been fixed (or were hypothetical in the first place) isn't an argument against user namespaces, but an argument for testing and bug fixing.

@madaidan
Copy link

This is fine, but that discussion doesn't actually provide any examples,

Many examples were provided in the linked tickets.

and a response (https://lists.archlinux.org/pipermail/arch-general/2017-February/043076.html) contradicts all of those claims.

That response is by someone who has no idea what they're talking about and was already responded to by Micay.

https://lists.archlinux.org/pipermail/arch-general/2017-February/043078.html

Read the whole discussion. Stop cherry picking.

Arch have even enabled user namespaces in recent kernels...

It had unprivileged user namespaces disabled by default with a kernel patch for years until very recently.

If someone could show a current vulnerability (ideally with some sort of PoC) then it would make an argument against user namespaces much easier. Referring to historical issues which have since been fixed (or were hypothetical in the first place) isn't an argument against user namespaces, but an argument for testing and bug fixing.

Here are just a few examples:

https://nvd.nist.gov/vuln/detail/CVE-2019-11360

https://www.cvedetails.com/cve/CVE-2018-14646/

https://www.cvedetails.com/cve/CVE-2018-18955/

https://www.cvedetails.com/cve/CVE-2018-6559/

https://www.cvedetails.com/cve/CVE-2018-16884/

@jacobc-eth
Copy link

This issue has caused a lot of damage to the reputation of Brave in the F/OSS community (look at the issues linking to this one listed up above or the dozens of other threads across various different Linux communities that have removed Brave from their repositories). That alone should be sufficient incentive to take this issue seriously. This issue has now been open for 9 months, without any security fix even being worked on. Please, please take this issue seriously.

@posix4e
Copy link
Contributor

posix4e commented Nov 14, 2019 via email

@madaidan
Copy link

madaidan commented Nov 14, 2019

Can you point out the communities which don't support
user namespaces?

Debian, one of the most popular linux distros disables unprivileged user namespaces by default.

The Arch Linux linux-hardened kernel disables unprivileged user namespaces.

The KSPP recommends to disable them along with tons of other security experts so you can bet many security focused Linux users will be following their advice.

And many more.

Namely the concern I had was around dynamic library loading in a setuid environment before any of the rendering threads launch.

AT_SECURE is set on setuid/setgid binaries on Linux which prevents that from being exploited entirely.

There has not been a single known privilege escalation exploit in chromium's suid sandbox.

@jacobc-eth
Copy link

In addition to those threads about why the user name spaces are disabled in multiple kernel distributions, here are some of the issues and forum threads I know of that discuss this issue in detail:

Privacy Tools removes Brave
Manjaro removes Brave from repo
Original thread where Brave is added to Manjaro, turns hostile

@fmarier
Copy link
Member

fmarier commented Dec 18, 2019

As of Brave 1.2, users without user namespaces on their systems will be able to make use of the SUID sandbox. I have confirmed that the fix proposed in #6247 works.

This is what brave://sandbox shows in that case:
Screenshot from 2019-12-12 16-24-57

unless kernel.yama.ptrace_scope is off:
Screenshot from 2019-12-12 16-24-38

daradib added a commit to ocf/puppet that referenced this issue Jul 14, 2020
Only allow ptrace from a parent process to its children or via
CAP_SYS_PTRACE.

To verify sandbox status for Brave, Chrome, Firefox see
brave://sandbox, chrome://sandbox, about:support, respectively.

Also describe disadvantages of enabling unprivileged user namespaces.
Distributions like Debian currently disable unprivileged user namespaces
by default to decrease the kernel attack surface for local privilege
escalation. See Debian bug #898446. If kept disabled, Brave 1.2+ and
Chrome will still enforce namespace sandboxing via their setuid-root
helper executable. See brave/brave-browser#3420 and
brave/brave-browser#6247. Firefox does not include a setuid-root binary,
however, so unprivileged user namespaces are useful to have for
defence-in-depth, but not critical. See
<https://www.morbo.org/2018/05/linux-sandboxing-improvements-in_10.html>.
daradib added a commit to ocf/puppet that referenced this issue Jul 14, 2020
Only allow ptrace from a parent process to its children or via
CAP_SYS_PTRACE.

To verify sandbox status for Brave, Chrome, Firefox see
brave://sandbox, chrome://sandbox, about:support, respectively.

Also describe disadvantages of enabling unprivileged user namespaces.
Distributions like Debian currently disable unprivileged user namespaces
by default to decrease the kernel attack surface for local privilege
escalation. See Debian bug #898446. If kept disabled, Brave 1.2+ and
Chrome will still enforce namespace sandboxing via their setuid-root
helper executable. See brave/brave-browser#3420 and
brave/brave-browser#6247. Firefox does not include a setuid-root binary,
however, so unprivileged user namespaces are useful to have for
defence-in-depth, but not critical. See
<https://www.morbo.org/2018/05/linux-sandboxing-improvements-in_10.html>.
@gabrc52
Copy link

gabrc52 commented Oct 2, 2020

As of Brave 1.2, users without user namespaces on their systems will be able to make use of the SUID sandbox. I have confirmed that the fix proposed in #6247 works.

So, would it be better to use this? Doesn't this give root privileges to the sandbox?

@madaidan
Copy link

madaidan commented Oct 4, 2020

Doesn't this give root privileges to the sandbox?

Root is only used to setup the sandbox. Privileges are dropped after entering it.

@mixedCase
Copy link

@diracdeltas Issue's good to close now that the SUID sandbox fallback is operational, no?

@wknapik
Copy link
Contributor

wknapik commented Aug 16, 2023

This looks outdated and no longer applicable, closing. Please reopen if necessary, or open a new issue.

@wknapik wknapik closed this as completed Aug 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request OS/Linux packaging Making Brave natively available to install on a variety of platforms, app stores, and repositories. priority/P5 Not scheduled. Don't anticipate work on this any time soon. security
Projects
None yet
Development

No branches or pull requests