-
-
Notifications
You must be signed in to change notification settings - Fork 567
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 can't run AppImages made with appimagetool #1032
Comments
There seems to be a bug in appimagetool. If I create an AppImage with appimagetool, it doesn't work well with firejail:
However, if I manually create an AppImage manually, then it works fine with firejail:
|
Here is an example of how to fix an AppImage from appimagehub.com in order to use it with firejail, without having to use an unnecessarily weakened sandbox. First, download the AppImage and make it executable. For this example I'm going to use Firefox 72.0.1 available here: https://www.appimagehub.com/p/1331794/ It seems the person who created this AppImage used appimagetool, because I get the "Permission denied" error:
To get past the above error, we have to re-create the AppImage without using appimagetool (for the third step below, you need the appropriate runtime from here: https://github.com/AppImage/AppImageKit/releases):
Now the "Permission denied" error goes away:
This new error is because firejail is using the default profile (/etc/firejail/default.profile or /usr/local/etc/firejail/default.profile depending on your distro), which is quite restrictive. In this specific case, the problem is that the default profile includes disable-programs.inc, which blacklists ${HOME}/.cache/mozilla and ${HOME}/.mozilla. One way to "fix" this issue by adding the
Now everything works :-) |
Now the question is, why does the manual |
I wish I did. I took a look at
Sure. Let's run
As you can see, the permissions are the same when the two AppImages are extracted. It seems the problem has to do with permissions inside the AppImage. If we could take a look inside the AppImage without actually extracting it, then perhaps we would see a difference. |
To do this, run the AppImage. While it is still running, look in |
Thank you, @shoogle. Here you go, guys:
The Dummy.AppDir directory itself as well as all the files and directories inside of it are owned by |
@bdantas, interesting find! The problem seems to be a combination of two things:
So it could potentially be solved by changing ownership or by changing permissions. |
That isn't the question you need to answer, young padawans. There is surely a version difference, we use a very specific and patched Edit: also pretty sure we call |
Easy to explain: AppImageKit/src/appimagetool.c Line 166 in 23267a1
We do this because we don't want our usernames to be leaked inside the AppImage. Now, the question is: Why does Firejail have any issues with root-owned files in an AppImage? |
Does Firejail run as root? If not then it wont be able to read inside the mount point. The mount point is In order to read (or execute) a file, you need read permission for all folders above that file in addition to read (or execute) permission for the file itself. |
Huh? Since when are AppImages only root-usable? That can't be true. |
This might be the root cause. Why is it so?
As far as I know it is a suid binary. |
@probonopd, right, but as I said before, it is the combination of being owned by root and not having read permission for anyone except the owner that matters. Sure it's fine for them to be owned by root, but only if other users have read permission too. |
Yes, but I fail to understand whether this is something that is wrong in the squashfs in the AppImage, or something that is wrong in Firejail. |
Or it could be something wrong in FUSE / runtime.c. |
Perhaps netblue30/firejail#2690 (comment)? |
I think the reasoning here is backwards. How come the random version of
No need to worry about that. The username is an abstraction for humans. The real owner of my files and directories is "1000", which my system looks up in |
It is not the version of mksquashfs, it is that we tell mksquashfs to have all files inside the squashfs owned by root. Apparently Firejail doesn't like this. |
Yes, @shoogle. The |
thanks for clarifying.
That's true for your user account on your machine, but it's not true for other users (who would be numbered 1001, 1002, etc.) and it's not true on distros like CentOS (I think) that start the numbering at something other than 1000 for the default user. So if the squashfs stores a user ID then it needs to be zero (i.e. root). That is unless there is a generic way to say "owned by current user" without giving an ID? |
Actually, that still doesn't explain everything. Imagine this:
Surely Firejail must refuse to run executables that don't have execute permission for the current user, otherwise it would reduce security rather than increase it! |
@shoogle true enough. If one needs to pick a user ID that every linux system has, I think zero (for root) is the safest bet. My point was just that probonopd's concern (other people seeing your username) does not actually happen. If I send you a tarball with files owned by bruno:bruno on my machine (my user ID is 1000 and group ID is 1000) but there is no user ID 1000 or group ID 1000 on your machine, then you'd see something like this if you ran
No "bruno" anywhere in the output. BTW, I don't know of a generic way to say "owned by current user". I'll look into it but am not optimistic. |
It is right that Firejail is setuid-root, and it uses root to mount the AppImage, but in the end it executes the binary as the user, after all privileges have been dropped. |
Ok, so the issue is that FUSE ignores file system permissions by default, whereas Firejail does not. |
To expand on this: if anyway file system permissions are ignored in FUSE mounts, could they be more generous in order to allow Firejail to execute the binaries? |
One question is if AppImage wants to care about the case when it is mounted without FUSE.
That's certainly right. As outlined in netblue30/firejail#2690 (comment) , it is not impossible to address this in Firejail, but is not simple either. |
The runtime has no influence on the mountpoint's permission; at most appimagetool. @bdantas's bug is about the runtime not respecting the existing permissions on extraction. You (dis)improved this by changing the In any case, firejail needs to implement a workaround, not we. Again: my main concern is the lack of backwards compatibility with any Let me show you why changing the pure access permissions inside the AppImage's squashfs payload is completely pointless with regards to this issue anyway: > ls -al /tmp | grep FreeCA4
[...]
drwx------ 3 root root 0 Mai 22 2019 .mount_FreeCA4W4Vxg
# from the permissions we see it should be accessible only by root, right?
# FreeCAD was started by me, though, so I have access with ls
# let's see what happens if any other user tries to access the directory
> env LC_ALL=C sudo -u nobody ls -al /tmp/.mount_FreeCA4W4Vxg/
ls: cannot access '/tmp/.mount_FreeCA4W4Vxg/': Permission denied
# FUSE resp. squashfuse prohibits access to this directory, as some
# mount option that allows all users to access the directory is both
# enabled system wide (it's a security risk!) and used on the mountpoint
# let's try another one, shall we?
# permissions:
drwxr-xr-x 3 root root 0 Mai 19 2019 .mount_OpenSC4TyTLF/
> env LC_ALL=C sudo -u nobody ls -al /tmp/.mount_OpenSC4TyTLF
ls: cannot access '/tmp/.mount_OpenSC4TyTLF*': No such file or directory
# despite being "accessible to all users", as one might put it, a
# user-started FUSE mountpoint *cannot* be accessed by anyone
# *but this user* If firejail mounted the directory as the user who started the process, it should work without having to enable insecure FUSE options or other hackery. It's a suid binary, it can do that. I have written FUSE filesystems before (e.g., for AppImageLauncher), therefore I was aware of this behavior with regards to this whole permissions and access system. I hope my quick demo illustrates that it's not as easy as it seems at a first glance (e.g., with I think firejail could fix the problem by launching the FUSE process with regular user permissions (right now it's in a section with evaluated (root) permissions. That should sort out the issue permanetly. @Vincent43 I do not claim it's necessarily easy or anything, you might have better insight there. But the conclusion that firejail have to change something is not based on opinions. Nor is it that I don't want to invest time or anything. It's purely got to do with our focus on backwards compatibility. Since firejail does not use our runtime but, well, ships its own, it's pretty clear who's in charge of the mounting behavior. As said, I think it's possible even with firejail's way of mounting. I am happy to provide feedback for a firejail fix. Please don't hesitate to contact me. |
By the way, security and privacy wise I love FUSE's (maybe not too obvious) behavior by the way. There might be various reasons why I as a user might not want third parties to be able to access the AppImages I am using. By both putting them into some protected directory I own ( This is especially handy on multi-user systems, e.g., university or school networks, enterprise environments, ... It's not P.S.: The FUSE option I was talking about is |
@smitsohu thank you for the cross-link over to firejail's tracker. @crass shows how it has to be done in firejail. @probonopd and I just had a small discussion about why we have this issue. I think it's fair to say firejail has made the assumption that contents are world readable. That assumption isn't based on the AppImage specification, I don't know why it was made. Surely our spec is incomplete there (something we intend to fix with an upcoming type 3). FUSE mounting works, though. We could implement the means to make this assumption made by firejail work, sure. But strictly speaking, our AppImages aren't violating any specification, and our tools are not the only ones which create AppImages. So even if we fix that, you'd have to reach out to all the other projects who create AppImages other than appimagetool to add this I hope the firejail team finds a way to mount as regular user through FUSE. As said, happy to provide feedback, please don't hesitate to reach out to us! |
@smitsohu I missed your question. Yes, I think we should care about this use case. But maybe not for type 2. The solution for root mounting via the kernel API is IMO to then mount AppImages with the Can you please open an issue? |
That's a FUSE mount again 😄 |
Will do. |
Well, that's my point. How FUSE works is the fact and you claim it works differently than everyone else in this topic claims it does. Your example shows that it doesn't matter if fuse dir has 700 or 755 permissions - it still can be opened by invoking user and can't be opened by other user. Why do you have such problem with 755 permissions then? |
Because this does not help with existing AppImages. In not a single one of your comments you have picked up this problem. Backwards compatibility is important to our project. You keep ignoring this point over and over, and I am not sure why... Apart from that; does firejail's mounting provide the same protection FUSE does? If I would run a (755) AppImage, could everyone just access the files? |
No, you repeatedly claimed that:
Only now, when you proved yourself wrong then you switched argument to Because this does not help with existing AppImages and claim everyone was ignoring it all the time. Sorry but if you want a productive discussion here, you (and everyone else) need to be honest. |
The runtime should never have been changing directory permissions to 700 without any notice to the user. That was clearly a bug and I'm glad it's fixed. Thank you. Ideally, appimage extraction via --appimage-extract would behave similarly to the unsquashfs utility, which extracts squashfs archives without changing any permissions. According to @probonopd, achieving this would overly complicate the runtime's source code, which is why hardwired directory permissions are used. I think hardwiring 755 is sensible and is a much better workaround (in the sense that it "works around" having to replicate unsquashfs functionality in the AppImage runtime) than using 700. My two cents about backwards compatibility and "known bad" AppImages in the wild: I'd change nothing in firejail. The ideal way to run an AppImage in firejail is to fix known-bad AppImages then use firejail's own "runtime" (C library mount), which requires fewer privileges than FUSE. The --appimage flag is what tells firejail to use its own runtime. This works:
If AppImage is known bad and user does not know how to fix the permissions issue, it is already possible to "properly" (to use TheAssassin's expression) mount AppImages in firejail using the AppImage runtime and FUSE: Simply don't use the --appimage flag. The downside is that FUSE is privilege-hungry and needs more privileges than what's granted by firejail's default profile. So this works:
|
The answer is actually yes. Looking at early versions of the code, there seems to have been a misconception that |
Here is another example that gives "permission denied" when running with Firejail.
References: |
* app-image-kit-runtime: runtime: fix double fclose() on sqfs read error 0755 permissions for directories, addresses #1032 AppImage/AppImageKit#1032 (comment) Make verbosity of --appimage-extract-and-run configurable Fixes #1002. Allow using extract-and-run via environment variable Useful for "nested" AppImage calls, e.g., linuxdeploy plugins called by linuxdeploy in a Docker environment. Refactor and improve mountpoint name generation Fix --appimage-mount mountpoint communication Right now, the buffering of stdout prevents proper reading of the mountpoint from stdout in scripts and other processes in general. This commit fixes the runtime's behavior. Print all log messages on stderr That's a lot more straightforward to users, as other tools do the same. "Special messages" like the mount dir (on --appimage-mount) etc. are still printed to stdout for easier piping and being able to differentiate between log/error messages and values the users might want to use for automation. CC #923. Runtime: Create parent dirs while extracting single file Runtime: Allow recursive extraction of partial sections of the AppImage Use SIGTERM to terminate fuse, not SIGHUP Fixes #896 Support SQUASHFS_LSYMLINK_TYPE Port over from AppImageCommunity/libappimage@1898d41 Print paths to stdout rather than stderr, closes #880 Implement absolute path calculation with proper error checks Set typical AppImage vars in extract-and-run mode src/runtime.c: preserve exit status on --appimage-extract-and-run (normal exit) SRC: runtime: exit child on execv errors instead of duplicating parent's cleanups src/runtime.c: use 127 as exit status if payload launching failed src/runtime.c: exit with error status if waitpid() failed Document mount_dir related variables Fix variable name Move temp base dir to the top to avoid redundancy Fix codacy warning Use glibc P_tmpdir macro for default temporary directory location Provide error information on failure to create temporary directory. Expand mount path for display when mounting appimage. Mount within TMPDIR when environment variable is set Switch to new libappimage repository Also compare file sizes to make existence check more robust Implement NO_CLEANUP Don't overwrite existing files Should speed up extraction on subsequent calls when using NO_CLEANUP in the future. Add note about pattern matching to help text Restore pattern matching Implement "content-aware" filenames Fix buffer size calculation C strings are annoying... Improve code style Fix error check Fix type Use nftw instead of fts I prefer fts, as it doesn't require any sort of callback and is therefore easier to implement, but it doesn't work with LFS. Improve code style Properly clean up resources Fix resource leak Fix memleak --appimage-extract-and-run mk. 1 Use return codes instead of exit calls Fix comparison Extract extraction code into separate function Also change meaning of optional parameter to override output directory. Reduce severity of symlink creation errors CC #834. Improve error message Fail more loudly. Exit on malloc fail. Be more explicit. Typo. Cleanup. Free created_inode. Handle hardlinks during extraction. Reap defunct child process Payload software that calls `wait()` or `waitpid(-1, ...)` may be confused if it is told about a defunct child process from the AppImage runtime. Fixes #812 Fix runtime build Fix runtime build Add appimage_ prefix to get_elf_size Makes the API more consistent. Refactor get_elf_size Use ssize_t to be able to add an error check (return value < 0). Before, the function just called exit(), which is undesirable when using from the library. Instead, the user of the library should be able to treat the error gracefully. Furthermore adds error checks everywhere the function is called. Load libbsd dynamically Also replaces a few occurrences of sprintf with strcpy, as sprintf was misused and yielded compiler warnings. Don't include headers if not needed Introduce CMake option for setproctitle feature Use setproctitle() with TARGET_APPIMAGE Fixes #763 Set argv0 depending on TARGET_APPIMAGE Appears to fix #743. Remove annoying debug message Make TARGET_APPIMAGE more useful Necessary for AppImageLauncher and possibly other applications to run AppImages without using the embedded runtime. Please beware that TARGET_APPIMAGE MUST be an absolute path. Don't change variable name Make whitespace consistent (#673) - Remove all trailing whitespace - Remove all spaces and tabs on otherwise empty lines - Convert tabs into spaces (file used mostly spaces before) Fix versioning inconsistencies Fix embedding of Git version number Happy New Year! Fix remaining naming issues Restructuring, mk. 1 [AppImage/AppImageKit/commit/f4f31ba086e8ebae2a73516b18] AppImage/AppImageKit@f4f31ba Signed-off-by: Tim Janik <[email protected]>
@JulianGro claims over at AppImage/appimage.github.io#2470 (comment) that "this is still a bug in AppImageKit". Maybe I am a bit slow here, but did we ever figure out what we could do on the AppImageKit/appimageool side to mitigate this? E.g., check whether the source AppDir is at least 0755 and exit otherwise with an error message? |
Well, I am being told to extract the AppImage and manually pack it with different permissions. |
This is no longer a bug in AppImageKit. The issue was fixed with this commit from March 10, 2020: The directories inside of AppImages are owned by root. To fix an AppImage that is butting heads with firejail ("Permission denied"):
Hope that helps. |
That is good to know. |
Done. So can we close this ticket then? |
@probonopd - Thanks for the new release! Yes, let's close the ticket. The issue has been extensively discussed and is now fixed in a stable release of AppImageKit. I don't think there's much left to discuss. |
EDIT: This post was too wordy. The posts below are more to-the-point.
The text was updated successfully, but these errors were encountered: