-
Notifications
You must be signed in to change notification settings - Fork 569
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
settings in default.profile and disable-common.inc that break AppImages #3249
Comments
Keep in mind that disabling almost all options makes using firejail at all questionable. |
That's a good point. Maybe the AppImage runtime could be tweaked to need fewer privileges? Alas, I'm just a user and don't know if that would be doable without crippling functionality. |
it's not really about appimage runtime but about apps. Having one sandbox fits all results in almost no sandbox because one app needs this, other app needs that and so on. To overcome this we need to differentiate sandbox depending on called app but I don't know how feasible is this with appimages. |
I understand what you're saying, but that's not what I was suggesting. I was suggesting a generic profile that allows what all AppImages need in order to mount the squashfs payload via fuse--let's call it the generic "AppImage loading" phase (all AppImages need to be allowed to call fusermount, for example). After that, sure--we are in app-specific territory where we can't predict what privileges are needed. That's a separate problem that perhaps could be solved by documentation in appimagehub that lets users of a specific AppImage know what additional (if any) application-specific privileges are needed beyond what's in the generic AppImage profile. |
@bdantas I just saw #2690 again and that reminded me that we have support for using a special blacklist ${PATH}/fusermount See #2690 (comment). |
Ok, so I created a barebones AppImage for testing. It does nothing but run this script:
The AppImage was created using the latest stable release (version 12) of AppRun-x86_64 and appimagetool-x86_64.AppImage available here: https://github.com/AppImage/AppImageKit/releases Here is the AppImage if you'd like to inspect it: http://files.dantas.airpost.net/public/Dummy-x86_64.AppImage The installed firejail is version 0.9.62 which seems to include AppImage support:
I'm running the AppImage in a minimalistic 64-bit Tiny Core Linux machine (linux kernel 5.4.3) with this command in a terminal emulator:
As was the case with the two AppImages in my original post, Dummy-x86_64.AppImage is only able to echo hello in the terminal if I comment-out these lines in default.profile like this:
and also comment-out these lines in disable-common.inc:
If I remove the pound sign from any of the lines above, the test AppImage fails to run. If I have this in disable-common.inc:
Then the test AppImage also fails to run. Conclusions:
|
P.S. I just tested the same Dummy-x86_64.AppImage in a totally different environment (Devuan ASCII with firejail version 0.9.44.8). Results were almost identical to the above. The only differences where these two:
|
@bdantas Thank you for doing extensive testing. I've made similar tests using your Dummy-x86_64.AppImage referenced above and can confirm (most of) your observations.
As per man firejail the conditional '?HAS_APPIMAGE:' only works when it encounters '--appimage' as argument to the firejail command. At least as far as I can see in your responses throughout this issue that argument hasn't been used, which would explain it not doing anything positive in this context. In fact, using the '--appimage' on CLI (or it's counterpart 'appimage' in a profile file) is the recommended use of firejail with AppImages. That being said, I can reproduce, so marking this as a bug. Strangely enough |
|
@bdantas As an workaround to get an default-appimage.profile.
|
Although, as (very) temporary workaround, the contribution of @rusty-snake is technically sound and valid - besides being well intended I'm sure - it is crucial to again remind AppImage users to what @Vincent43 already stated above: crippling firejail in such a way is an excersise in futility. Personally I've always wondered why firejail supports them in the first place and feel tempted to argue for dropping support, like we did for snap and flatpak. Just my 2 cents. |
Appimages historically worked with |
After commenting out proposed options in generic Appimage profile there would be no need to add any new privs because all of them would be already allowed for every appimage. Just some binaries and configs will be blacklisted and pid namespace would be used but any app would be able to escalate to full root access if it use some working 0day. Even with current default profile appimages sandboxing is inferior in comparison with generic apps that use whitelisting but if it's true that current appimage runtime is incompatible with core firejail sandbox features then still calling appimages sandboxed will only get false sense security for users. I agree with comment above that in such case official appimage support should be dropped. |
@Vincent43 I tested the AppImage (yubioath-desktop) in the link you provided. Like all the other AppImages I've tried or created myself, it only works in firejail if I use --noprofile or if I cripple default.profile as shown in my previous posts. I sandboxed some additional applications (non-AppImages) in Tiny Core Linux and it turns out that anytime disable-common.inc is sourced by a profile, the application doesn't work unless I comment-out the line that blacklists nc. So the nc issue is a general firejail issue in Tiny Core Linux and is not specific to AppImages. Please let the AppImage developers weigh in (especially @probonopd) before you guys make any decisions about official AppImage support in firejail. I'm just a user and know nothing about the internals of AppImage's runtime binary. Perhaps decreasing privilege requirements of the runtime would not be difficult. If that turns out to be the case, the only special consideration the AppImage loading phase would need from firejail is for fusermount to be allowed (via --appimage argument and the special ?HAS_APPIMAGE line in disable-common.inc). |
So we need to figure out if this is appimage issue, Tiny Core Linux issue or some specific issue to the way you create appimages. On my system I don't even have |
I think the solution for the issue around |
I think the first order of business is for me to move all testing to a more traditional distro. I have Devuan on a separate partition on my personal machine--it is a more traditional distro than TCL and uses GNU coreutils rather than BusyBox. I'm going to compile firejail 0.9.62 in Devuan right now. I'm highly motivated to get to the bottom of this. |
Ok, I'm ready to roll:
I will do a few tests and will report back. |
@Vincent43 now I think we are on the same page. I made no changes whatsoever to the files in /usr/local/etc/firejail. Here are my results:
So it seems Dummy and Firefox have the same issue, which does not affect yubioath. The Firefox AppImage is the 72.0.1 version available here: https://www.appimagehub.com/p/1331794/ |
It seems the issue with Dummy and Firefox has to do with permissions to run bash or AppRun:
|
Thanks, @rusty-snake. So I tried changing AppRun ownership to root:root and permissions to 755, no luck. Also tried 775, no luck. EDIT: Ooooh, I see. The permisisons issue is on the /run/firejail/appimage/.appimage-XXXX/ directory, not on the AppRun binary. Gosh, how do I get the right permissions? I don't see an obvious solution in that thread. |
firejail/src/firejail/appimage.c Lines 98 to 123 in dc12ce6
|
@rusty-snake pardon my ignorance, but I don't understand. Is this part of firejail's source code the problem or the solution? If the solution, are you saying I should use latest git instead of 0.9.62? At any rate, it seems that I've beaten this to death. There turned out to be multiple pseudo-issues and just one real issue that's already known. Sorry for the noise. |
What if you change firejail/src/firejail/appimage.c Line 102 in dc12ce6
0700 to 0755 ?
|
Nothing |
I tested two completely different AppImages (Firefox.AppImage, which I downloaded from appimagehub.com, and Xscreensaver.AppImage, which I created from scratch myself) on Tiny Core Linux 64-bit with firejail 0.9.62.
The only way to get these two AppImages to run with firejail is to comment-out the following lines in default.profile:
and also comment-out these lines in disable-common.inc:
I discovered the above after much trial and error.
Perhaps firejail could learn to detect AppImages and use a different profile (e.g., default-appimage.profile) and include file (e.g., disable-common-appimage.inc) that do not contain the offending lines above?
@probonopd might also be interested in this information.
The text was updated successfully, but these errors were encountered: