bedrocklinux / bedrocklinux-userland

This tracks development for the things such as scripts and (defaults for) config files for Bedrock Linux
https://bedrocklinux.org
GNU General Public License v2.0
602 stars 65 forks source link

Incompatibilities with Firejail #219

Closed n0toose closed 3 years ago

n0toose commented 3 years ago

Hi, I was experimenting with the aspect of security on Bedrock Linux and tried out sandboxing. Firejail tends to fail with some applications due to permission issues.

Here's an example:

$ firejail --x11=xephyr --net=wlan0 openbox 

*** Starting xephyr server: "Xephyr" "-ac" "-br" "-noreset" "-screen" "800x600" "-title" "firejail x11 sandbox" ":361" ***

*** Attaching to Xephyr display 361 ***

Reading profile /etc/firejail/openbox.profile
Reading profile /etc/firejail/globals.local
Reading profile /etc/firejail/disable-common.inc
Parent pid 561816, child pid 561822

Interface        MAC                IP               Mask             Status
lo                                  127.0.0.1        255.0.0.0        UP    
eth0-561816      de:ad:be:ef:00:00  [Redacted]    [Redacted]  UP    
Default gateway [Redacted]

Warning: /sbin directory link was not blacklisted
Warning: /usr/sbin directory link was not blacklisted
Warning: cleaning all supplementary groups
Warning: Cannot confine the application using AppArmor.
Maybe firejail-default AppArmor profile is not loaded into the kernel.
As root, run "aa-enforce firejail-default" to load it.
Child process initialized in 1755.03 ms
bouncer: could not execute
    /bedrock/bin/strat
due to: execv:
: Operation not permitted

Parent is shutting down, bye...
n0toose commented 3 years ago

When invoking commands directly (e.g. /usr/bin/chromium instead of chromium), it seems to work properly.

cptpcrd commented 3 years ago

AFAIK this is expected behavior. strat needs to run with elevated permissions so it can chroot() to the other stratum, and Firejail (correctly) blocks this for security reasons.

Everything should still work within the same stratum, however. If you run strat -r <strat> firejail ..., that should fix the permission errors within the stratum.

n0toose commented 3 years ago

Is it on the maintainers of Firejail to have Firejail specifically look into the /usr/bin folder of the current strat, rather than looking at the /bedrock folder as indicated by the PATH environment variable?

cptpcrd commented 3 years ago

Is it on the maintainers of Firejail to have Firejail specifically look into the /usr/bin folder of the current strat, rather than looking at the /bedrock folder as indicated by the PATH environment variable?

I would probably say no, but I'm not a maintainer; if you want more official answers then you need to talk to @paradigm.

  1. You could make firejail "just work" by adding firejail to the restrict list in bedrock.conf.
  2. It may be possible to make bouncer autodetect that it's running in firejail and skip running strat, though I'm not sure if this edge case is worth the changes.
n0toose commented 3 years ago

@cptpcrd If that's the case and if firejail won't even work otherwise anyways (and giving a sandbox the pass for some crazy chroot capabilities - is that right? - doesn't sound like a splendid idea security-wise), is it worth updating the default configuration file for that specific purpose? (May I open a PR if that's the case?)

cptpcrd commented 3 years ago

(and giving a sandbox the pass for some crazy chroot capabilities - is that right? - doesn't sound like a splendid idea security-wise)

The details would need some hammering out, but making bouncer autodetect it's inside firejail and skip running strat wouldn't bypass the sandbox. It would actually have about the same effect as restricting firejail (so that would probably be a better idea).

Allowing bouncer/strat to break out of the sandbox is an entirely different matter, and you may actually be able to do this simply by passing --caps.keep=sys_chroot to firejail. However, I don't necessarily recommend doing that because it will reduce your sandbox's security and may allow escapes.

is it worth updating the default configuration file for that specific purpose? (May I open a PR if that's the case?)

In my opinion adding firejail to the restrict list in bedrock.conf would be the best way to resolve this. However, I'm not a maintainer; I've only made small contributions to the project.

n0toose commented 3 years ago

I'll test it out and open a PR afterwards if everything seems good to go.

paradigm commented 3 years ago

I'm in complete agreement with cptpcrd's analysis here. The issue appears to be a combination of:

Most users who use firejail on Bedrock will likely want to change on of those two things. Having Bedrock automatically weaken firejail's security by default is a Very Bad Idea, and so if Bedrock is going to pick on of the two possibilities to support out-of-the-box it has to be hiding cross-stratum binaries from firejail.

I think we need to do a few things things here:

  1. Restrict firejail by default.
  2. Document the need to potentially install multiple firejail binaries and use strat to specify the one that corresponds to the binary the user wishes to run restricted, if the user wishes to have firejail block CAP_SYS_CHROOT as it does by default.
  3. Document the possibility of unrestricting firejail and either running it with --caps.keep=sys_chroot or configuring /etc/firejail/default.profile to use caps.keep sys_chroot to allow cross-stratum binaries (at the expense of potential abuse of CAP_SYS_CHROOT).

panos, you're more than welcome to make PRs for any/all of these work items if you'd like. If you don't want to, I'll be happy to make the changes myself when time allows.

You'll likely want to touch these places for (1):

For both of those locations, keep alphabetical order when you insert the new firejail entry.

For (2) and (3), the change should be in

likely by adding a Firejail row under the miscellaneous features section which links to more detail in a details section

n0toose commented 3 years ago

Will work on it later today! Thanks for all the pointers, @paradigm!