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

Firejail breaks Brave browser default sandboxing #2944

Closed
corecontingency opened this issue Sep 6, 2019 · 13 comments
Closed

Firejail breaks Brave browser default sandboxing #2944

corecontingency opened this issue Sep 6, 2019 · 13 comments
Labels
bug Something isn't working

Comments

@corecontingency
Copy link
Contributor

corecontingency commented Sep 6, 2019

Running brave with firejail has this error:

gzip: /proc/config.gz: Permission denied
User namespaces are not detected as enabled on your system, Brave will run with the sandbox disabled

Brave opens fine, and runs fine, but the sandbox does not work. Image is below.

brave_no_sandbox_a

It seems like brave checks if the kernel has user namespaces enabled by checking /proc/config.gz. Unfortunately, this file cannot be read from the firejail sandbox, so brave assumes user namespaces are disabled (even when they are actually enabled), so it runs with the --no-sandbox flag.

I believe this issue can be fixed if /proc/config.gz is made readable. Unfortunately, even if using a completely blank profile, this file still cannot be read. I have tried passing

noblacklist /proc/config.gz
read-only /proc/config.gz
whitelist /proc/config.gz

but the first two do nothing, and the last says it is an invalid whitelist, and firejail won't run.

How do I make this file accessible from the sandbox?

@rusty-snake
Copy link
Collaborator

Unfortunately, even if using a completely blank profile

A blank profile (aka --noprofile) can't help here because the blacklist is hardcoded:

disable_file(BLACKLIST_FILE, "/proc/config.gz");

@rusty-snake
Copy link
Collaborator

@corecontingency
still an issue?

@corecontingency
Copy link
Contributor Author

corecontingency commented Dec 16, 2019

Yes. Just replicated this exact same bug in a Manjaro VM, using the firejail-git and brave-bin packages from the AUR.

I think that this might be fixed if Brave was able to read /proc/config.gz from the sandbox, but I don't know how hard removing the hardcoded blacklist would be. What is the reasoning behind blacklisting /proc/config.gz? I understand it contains information about the configuration options when the kernel was compiled. Could allowing a program to read this file lead to a sandbox escape, or is it just a case of "better safe than sorry"?

I am not sure how much effort should be put into this, considering chromium has a sandbox already, and the issues that firejail is having with electron based apps.

@glitsj16
Copy link
Collaborator

@corecontingency I've made a tentative patch that you can download and integrate into the firejail-git PKGBUILD you mentioned earlier. The patch enables using --noblacklist /proc/config.gz on the command line / noblacklist /proc/config.gz in a brave.local file.

BEWARE: although I personally don't see much issues in making /proc/config.gz accessable that way, it's just a dirty workaround for now. Would be nice of you could test it and report back though. We'll see what more experienced collaborators think of patching firejail this way later on.

@corecontingency
Copy link
Contributor Author

It works! After compiling with the patch, I added noblacklist /proc/config.gz to the profile. Shown below is what chrome://sandbox shows in brave running under firejail:

brave_fixed

Now for the discussion about how much danger this adds to the sandbox...

@glitsj16
Copy link
Collaborator

@corecontingency Thanks for testing/confirming the patch works.

I made a PR in #3087 awaiting review. Let's move any discussion about the patch to the PR.

@rusty-snake
Copy link
Collaborator

/proc/config.gz does not exists on all distros (for example fedora). what happens if we hide it?

@corecontingency
Copy link
Contributor Author

corecontingency commented Dec 17, 2019

That might work too. I imagine that if it cannot find /proc/config.gz it checks the config file in /boot/. Possibly it is getting confused on Manjaro/Arch and not checking /boot/ because /proc/config.gz is present in the sandbox, just unreadable?

Wait, nvm, /boot/ is unreadable entirely in firejail.

@glitsj16
Copy link
Collaborator

@corecontingency I've been doing some more research on Brave and I might have found a way to have our cake and eat it too. Translation: it might not be needed to make any changes to firejail after all and still be able to keeo Brave's native sandbox intact. It all boils down to bypassing the (sandbox) checks in /usr/bin/brave on Arch Linux and derivatives. This issue doesn't exist on Debian/Ubuntu and hopefully other Linux distributions.

Would you be willing to do another test to confirm/deny whether my logic holds up? If so, make sure you don't have firejail patched with the tentative stuff I provided above and run the below from a terminal:

$ firejail --ignore=quiet /usr/lib/brave-bin/brave

For me this starts Brave without any major hickups, showing the exact same (mostly) green sandbox status cfr. the image in your earlier post. This would keep the firejail sandbox as tight as possible, whatever the state of /proc/config.gz, which is the better option here IMHO. Feel free to disagree, shoot holes in my argumentation etcetera. We really need to fix Brave somehow, that's the only goal here... TIA.

@corecontingency
Copy link
Contributor Author

corecontingency commented Dec 18, 2019

Seems like this works, good idea! Used a fresh Manjaro vm with AUR packages firejail-git and brave-bin (all defaults) installed. The browser seems to work fine too, it saves your tabs, addons, and settings.

Downside of this method is that it might be harder for the end-user. I don't think firecfg could use a symlink in /usr/local/bin to translate a call of brave from the desktop file to firejail /usr/lib/brave-bin/brave. If this is true, after noticing the sandbox is broken in brave, users would have to head to /etc/firejail/brave.profile, and (presumably) read a comment saying to make a copy of the brave desktop file, put it in ~/.local/share/applications/, and replace the line Exec=brave %U with Exec=firejail /usr/lib/brave-bin/brave %U in the desktop file if running Arch/Manjaro.

This isn't too much of a hassle, but something to keep in mind.

@Vincent43
Copy link
Collaborator

@glitsj16 this approach will be hard to propagate to users as we can't really mandate how they launch brave. I still believe my proposition from #3087 (comment) will work better.

@glitsj16
Copy link
Collaborator

@corecontingency Thanks for your response and analysis. After further deliberation it is indeed much better to go with @Vincent43's suggestions. I've updated the relevant PR accordingly.

@glitsj16 glitsj16 added the bug Something isn't working label Dec 19, 2019
@Vincent43
Copy link
Collaborator

This should be fixed by 8199725

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants