The reason for this is because of disable-mnt in the default profile. However, whitelisting should work as expected. The solution is not to disable disable-mnt because that's overly permissive.
This is related to #2148 in the case where firejail is installed on /media such that they should be able to benefit from the same solution.
I think having profiles in /media is a non-standard use case. Most users won't have their configurations there.
In my opinion it's better to harden the profiles for the most expected cases, and users with different setups can modify their firejail configuration.
I think you misunderstand... This is not a matter of modifying profiles. I specifically say that we __do not__ want to disable disable-mnt. The issue requires a change in the code in how firejail deals with disable-mnt and whitelist. Technically disable-mnt is a blacklisting of /mnt, /media, /run/mount, and //run/media, which happens right before any whitelisting occurs.
I think a possible solution is to not blacklist those directories, but instead mount some tiny tmpfs filesystems. This will have the effect that subdirectories of those directories will still not be accessible, but permission will not be denied when accessing them.
Okay, if I understand you correctly, you want to be able to override an existing disable-mnt.
What I meant was removing disable-mnt from the default profile and adding it where necessary.
But having the flexibility to override it would of course be better.
Yeah, actually, I want to only override a subtree of any of the disabled mounts. I'd consider only being able to whitelist one of the disabled mounts in its entirety to be insufficiently granular.
You can pass ignore=disable-mnt to override it. There is no need to alter its behavior. As you will pass your own whitelist rules anyway I think there nothing to do on our side.
As I said above, I do not want to disable disable-mnt. Let's say I do what you suggest and support my /media has /media/super-secret-usb and /media/buggy-firefox-profile... What set of whitelist rules will protect me? Now I could pass --blacklist=/media/super-secret-usb and that works for this simple example. But suppose /media has lots of directories that I want all blacklisted.... now its starts becoming a pain. What I'm suggesting is a less error-prone and more flexible solution.
Use whitelist, not blacklist. If whitelist isn't supported for /media (I'm not sure) then this is indeed something to implement. disable-mnt is for blacklisting whole media and it's appropriate for most users so we shoulsn't change it.
EDIT: whitelist does support /media, so I don't see the problem here.
@Vincent43, why don't you read an __understand__ my comment before posting. You're not making yourself look like someone whose comments should be valued. In fact, why don't you __read the code__ as I have?
Once you can explain to me why blacklist is appropriate where I mentioned it above, then you might get a better understanding of the issue.
I read your comments several times but not understand. You don't want to blacklist all /media, blacklist specific dir in /media or whitelist specific dirs in /media. What do you wan then?
As I understand it, he wants to whitelist a specific directory without whitelisting all other directories in the same mount point and without having to blacklist all non-allowed directories.
Edit: not "in the same mount point", but in any other mount points.
I'm still not getting it. Why --whitelist=/media/buggy-firefox-profile doesn't work?
I didn't check, but as far as I understand him, disable-mnt prevents that.
I didn't check, but as far as I understand him, disable-mnt prevents that.
I just checked and it does.
So basically the title should be:
Allow whitelisting paths disabled by disable-mnt.
Either whitelist needs to be changed to work with disable-mnt eg. whitelist /mnt/x
or
disable-mnt needs to support whitelisting itself eg. disable-mnt !/mnt/x
But that's what disable-mnt is supposed to do: --blacklist=/media. If user can pass his own whitelist rules then they can pass ignore=disable-mnt as well.
It's the same as someone would want to use --blacklist=~/foo --whitelist=~/foo/bar
$ firejail --noprofile --blacklist=~/foo --whitelist=~/foo/bar bash
$ ls foo
ls: cannot open directory 'foo': Permission denied
Whitelisting blacklisted dirs aren't supported in firejail in general
There is a use case where this is useful:
This is definitely an edge case and not something most users will encounter.
I mean we only really add disable-mnt to profiles that actually don't need it or are too high-risk to go without it (browsers).
Is this different than creating /home/user/super-secret and forgetting to update the profiles to blacklist it?
creating /home/user/super-secret and forgetting to update the profiles to blacklist it?
I guess you have a point. Like I said it is an edge case.
I'd consider only being able to whitelist one of the disabled mounts in its entirety to be insufficiently granular.
And going back and rereading, it seems what I said isn't even what @crass wants anyway.
Hold on. So @crass, I think there are a couple of things going on here.
I think a possible solution is to not blacklist those directories, but instead mount some tiny tmpfs filesystems. This will have the effect that subdirectories of those directories will still not be accessible, but permission will not be denied when accessing them.
That's literally how whitelist works...
You want (if I understand you correctly) to enable selective access to mounted devices without exposing other directories in /mnt. Passing --ignore=disable-mnt --whitelist=/mnt/<directory> should do the trick, shouldn't it? I just tested this using firejail --ignore=disable-mnt --whitelist=/media/camera, and as expected, _only_ /media/camera existed inside the jail (there are several other folders in there in the real /media).
All disable-mnt is doing (as you said) is blacklisting certain files and directories. If you want to enable selective access, you should be using whitelist _instead_ of blacklist (the two aren't generally compatible). Disabling disable-mnt isn't some spooky, super insecure thing _as long as you also use whitelist to restrict access_. For example, I use ignore private-dev and use whitelist /dev/<x> all the time when I have a very specific requirement for the device files I need available in the jail.
One side-effect, which we _should_ fix, is that _both_ /media and /mnt are treated with the same disable-mnt directive, which means forgetting to blacklist the other one could result in exposure. We should split it off into two different directives, disable-mnt and disable-media, to fix that.
I'd consider only being able to whitelist one of the disabled mounts in its entirety to be insufficiently granular.
You're not required to whitelist the entire mount though. You can do something like --whitelist=/media/mnt1/very/deep/subdir...
Maybe --disable-mnt could be just a front-end to the blacklist command (profile_add)? Then it would be possible to override with noblacklist just a single component (say /media) when this is necessary, while keeping everything else (/mnt, etc). On top of that you then could whitelist in /media, as outlined by @chiraag-nataraj.
Still not sure if this would help @crass.
@smitsohu I like that! More flexibility = good :slightly_smiling_face:
@crass does https://github.com/netblue30/firejail/commit/36281ef60b3fcc272e5d4d67b72d673d0028beab fix your issue?
@chiraag-nataraj @Vincent43
I'd consider only being able to whitelist one of the disabled mounts in its entirety to be insufficiently granular.
You're not required to whitelist the entire mount though. You can do something like
--whitelist=/media/mnt1/very/deep/subdir...
That is exactly what I'd like to do! Yep, and you're correct about the workings of --whitelist wrt to doing what I proposed. So this change does allow me to work around my issue. The one problem left is that you need to specify the --noblacklist=/media. This should be automatically done internally in firejail when it detects a whitelist with a path prefix of /media. Otherwise, it could still be confusing to the user (ie "I whitelisted my dir, why doesn't it work?").
@crass
As I presented in https://github.com/netblue30/firejail/issues/2149#issuecomment-428174319 this is generic firejail behavior. Changing it only for /media would be inconsistent and add more confusion. If we decide that --whitelist should automatically overcome --blacklist then it should be done for all cases.
@Vincent43 you would be correct _if_ the option was --blacklist=/media, but its not. There has already been demonstrated confusion around this (see #2188 ). The issue about why its confusing was already stated in my previous comment and by the user in #2188. Even in the documentation its not clear that --disable-mnt is equivalent to blacklisting. Contrary to your belief that what I proposed would be inconsistent, --disable-mnt is already a special case and thus any solution would be inconsistent/consistent because there's nothing for it to be consistent with. I understand your position, and it doesn't need to be repeated. However from a user perspective its wrong and causes the confusion that you admittedly want to avoid. So I maintain that this issue is not completely fixed.
I've been looking into a satisfactory solution, and hope to have a pq soon.
@crass
After recent changes the only special case in --disable-mnt is its name. Internally it works as as common --blacklist option. We could replace every instance of it with: include /etc/firejail/disable-media.inc
/etc/firejail/disable-media.inc:
blacklist /media
blacklist /run/media
blacklist /mnt
That would be consistent. Changing firejail internals to treat path with /media differently than /foo would be NOT consistent.
How you would explain to someone that they can overcome --blacklist with --whitelist=/media/user/foo in media dir but they need to pass --noblacklist=/home/user/foo and --whitelist=/home/user/foo in home dir?
@Vincent43 I know how it works, I've read the changeset that changed it. And in fact you are not completely correct about it. It does not work as you describe when disable-mnt is set to set to yes in firejail.config. You also missed that /run/mount is also blacklisted. Also, keep in mind that your idea of consistency is based on an (_inaccurate_) understanding of the code. A normal user should not be assumed to have this. What the doc says is Disable /mnt, /media, /run/mount and /run/media access. Does that mean blacklist? It certainly does not when disable-mnt is set to set to yes in firejail.config. I agree that consistency is a thing to be strived for, but your way isn't any more consistent from a user perspective.
_Think_ about it from a _user's_ perspective. Lets say is the firefox profile and executable with a profile in /media/firefox/.... The user will first try to run something like firejail firefox --profile /media/firefox/.... This will fail with firefox saying the profile could not be found. The next thing they'll want to do is --whitelist the path to see if that will _just work_. Unfortunately it won't. Then they'll look into the profiles and see nothing that disables /media specifically. They'll look more and read some docs, and discover that disable-mnt __disables__ /media. At this point they'll be at a loss _unless_ they somehow think that __disable__ means __blacklist__ and think to do --noblacklist. If you want firejail to be painless to expand adoption, why do you want them to jump through unnecessary hurdles when a simple solution already exists?
In fact, I'll reiterate, we've already seen confusion with the current behavior, so your theoretical consistency confusion is trumped by real world examples to the contrary. At a minimum the docs should be changed to cover the current behavior better. However, the user just wants things to be intuitive and work.
I propose that if --disable-mnt is specified on the command line or in a profile _and_ is set to no in firejail.config, then allow the whitelisting of paths in /media and other disable-mnt paths. This might only be odd if you specify --blacklist=/media in addition to --disable-mnt. But then you're talking about an uncommon case... who actually wants to do that?
When I run firejail with --whitelist=${HOME}/Pictures, the sandbox gets access to the Pictures subdirectory of the users $HOME __without__ needing to do a --noblacklist. This works as expected and seemingly contrary to the example you provided above. Consistency?
You also missed that /run/mount is also blacklisted. Also, keep in mind that your idea of consistency is based on an (inaccurate) understanding of the code.
Yes /run/mount is also blacklisted which doesn't change my point at all. What is inaccurate here?
What the doc says is Disable /mnt, /media, /run/mount and /run/media access. Does that mean blacklist? It certainly does not when disable-mnt is set to set to yes in firejail.config. I agree that consistency is a thing to be strived for, but your way isn't any more consistent from a user perspective
Changing docs isn't hard thing to do.
Think about it from a user's perspective. Lets say is the firefox profile and executable with a profile in /media/firefox/.... The user will first try to run something like firejail firefox --profile /media/firefox/.... This will fail with firefox saying the profile could not be found. The next thing they'll want to do is --whitelist the path to see if that will just work. Unfortunately it won't. Then they'll look into the profiles and see nothing that disables /media specifically. They'll look more and read some docs, and discover that disable-mnt disables /media. At this point they'll be at a loss unless they somehow think that disable means blacklist and think to do --noblacklist. If you want firejail to be painless to expand adoption, why do you want them to jump through unnecessary hurdles when a simple solution already exists?
I'm thinking from user perspective all the time otherwise I wouldn't care about this at all. The simple solution indeed exist and was presented by me above. Inconsistent internal changes are unnecessary hurdles.
In fact, I'll reiterate, we've already seen confusion with the current behavior, so your theoretical consistency confusion is trumped by real world examples to the contrary.
Yes, we have opened issue which you linked before and the user proposed exactly same thing as me.
At a minimum the docs should be changed to cover the current behavior better. However, the user just wants things to be intuitive and work.
Agree.
When I run firejail with --whitelist=${HOME}/Pictures, the sandbox gets access to the Pictures subdirectory of the users $HOME without needing to do a --noblacklist. This works as expected and seemingly contrary to the example you provided above. Consistency?
$ firejail --noprofile --blacklist=${HOME}/Pictures --whitelist=${HOME}/Pictures bash
$ ls Pictures/
ls: cannot open directory 'Pictures/': Permission denied
Is that consistent enough for you?
I think that we both agree about the problem but not about the solution.
IMO the best solution is either (or both):
disable-mnt in favor of /etc/firejail/disable-media.incdisable-mnt was an unicorn. This will add consistency and remove confusion. We will do global changes to all profiles anyway in couple of pending PRs)whitelist to overcome blacklist globally (not only for /media)Allow whitelist to overcome blacklist globally (not only for /media)
Well, in general it seems that users don't have problems understanding what blacklist and whitelist is, and that they are two different things and they have to take care of both.
If you mix them new problems (and feature requests) will be coming up, like: I whitelisted (blacklisted) ~.config/super, why can't I also access (blacklisted) ~/.config/super/useful?
Or conversely: I whitelisted ~/.config/super, it is nice I can see (blacklisted) ~/config/super/useful as well, but how do I block access to ~/.config/super/secret? To me this looks rather more confusing.
I vote for better documentation of disable-mnt. We could also add a comment to the profiles that in few words describes what it does. Similar to the writable-run-user option. People who care to adapt their sandboxes or conceive new ones learn by looking at the stock profiles. I think this is really a documentation issue, if anything.
Also consider that deprecating options that are widely used causes a lot of transition pain. Expect it to break most custom profiles out there (obtained by modifying the default ones). It would be disruptive and annoying.
I agreed with @smitsohu that allowing whitelist to override blacklist globally brings up a lot of hairy issues that imo are not worth it. The current behavior (that they're not compatible) is fine and (at this point) seems to be Expected Behaviorâ„¢. If we solve the uniqueness of disable-mnt (which has been solved in a recent PR/commit), it should solve this issue. I don't think this is a productive discussion at this point as the original problem seems to have been resolved, at least in master, so I'm going to close this for now.
Also consider that deprecating options that are widely used causes a lot of transition pain. Expect it to break most custom profiles out there (obtained by modifying the default ones). It would be disruptive and annoying.
Depreciation would mean that this option still works but isn't used in official profiles.
Most helpful comment
I updated
disable-mntdocumentation.https://github.com/netblue30/firejail/commit/70b227d5829db54684814ddb7c983b1bafaaaf45
https://github.com/netblue30/firejail/commit/016faf8e5bd4273153a2c2477c861221c189aa43