netblue30 / firejail

Linux namespaces and seccomp-bpf sandbox
https://firejail.wordpress.com
GNU General Public License v2.0
5.56k stars 557 forks source link

More restrictive profiles when run with --appimage #2256

Open crass opened 5 years ago

crass commented 5 years ago

I started thinking about this because I noticed that the firefox profile creates a whitelisted home while the digikam profile does not. At first I was surprised that the digikam profile did not whitelist home, but then considered that collections of photos could be in arbitrary directories of home or /media. So it does make sense.

The logic still applies to digikam's appimages, however there is less trust in the appimage binary because its not downloaded securely or verified normally, as the digikam installed through the package manager would be. So perhaps we don't want to by default be giving appimages access to home or /media.

So I'm thinking that running an appimage should conditionally disable-mnt and whitelisting or making readonly the home directory. Ideally, these directives would be put in disable-common.profile. However, I don't see how that would be done with whitelisting because the directories under home that need to be whitelisted varies depending on the profile. For instance, digikam would want ${PICTURES} whitelisted, but amarok would want ${MUSIC}. Setting home to readonly would be better, but suffer the same issues with write, eg. digikam wanting to write to its database in ${PICTURES}.

Am I missing something? Does anyone have an idea on doing this by only changing a few lines in disable-common.profile? Or does every profile potentially need to be updated? Also what are the thoughts on this proposal? Are there reasons that this shouldn't be done? I don't think it should be a problem, since the user can just add the appropriate --whitelist=... to get access to the needed directories.

netblue30 commented 5 years ago

I think we need to enhance the conditional support in the profile parser. We already recognize HAS-APPIMAGE:

?HAS_APPIMAGE:   whitelist   ${HOME}/special/appimage/dir

We need to expand it to handle include command, so we can bring in lots of other commands from a different profile file in case appimage is enabled.

?HAS_APPIMAGE:   include filename.profile

We also need support to negate the condition, something like

?!HAS_APPIMAGE: include filename.profile
crass commented 5 years ago

Yep, I agree with those enhancements. However, I don't see how they would solve the issue. Conditional includes are just a convenience and don't allow any more functionality. Negative conditions do add more functionality, but not seeing how that helps here.

I think something like a "convert noblacklist to whitelist" command (or behavior when using --appimage) would do what I want for the digikam case. This seems potentially confusing (would need to be well documented and noted in logs) because noblacklist will behave as whitelist. Perhaps another command weak_whitelist to be used instead of noblacklist in the digikam profile would be better, where weak_whitelist acts as noblacklist unless another option, say strengthen_weak_whitelist, is applied, which would make weak_whitelist behave as whitelist.

This feels like the right direction, but still not quite right.

rusty-snake commented 5 years ago

@crass you can do in globals.local

?HAS_APPIMAGE: disable-mnt
?HAS_APPIMAGE: private ${HOME}/appimage_home

"convert noblacklist to whitelist"

this will break many apps, but if you want you can do this with sed, awk & Co. profile before:

noblacklist ~/here

profile after:

noblacklist ~/here
?HAS_APPIMAGE: whitelist ~/here
?HAS_APPIMAGE: include whitelist-common.inc