Firejail: Allow persistent cache of appimage desktop integration prompt

Created on 9 Oct 2018  路  9Comments  路  Source: netblue30/firejail

When some appimage programs run (ie. firefox), a dialog prompts the user if they want to integrate with the desktop. I never do this, but I suspect it adds some desktop files. Regardless of the way you answer, a second dialog will ask if you want the answer to the first question remembered. This is implemented by storing files in the directory ${HOME}/.local/share/appimagekit. However, using the default firefox profile, this directory is not whitelisted so the preference is not persisted across jailed firefox executions.

I see 3 reasonable solutions:

  1. Add whitelist ${HOME}/.local/share/appimagekit/Firefox_no_desktopintegration to firefox-common.profile
  2. Add whitelist ${HOME}/.local/share/appimagekit to whitelist-common.profile
  3. Whitelist ${HOME}/.local/share/appimagekit in an appimage specific way. For instance, (__a__) hardcoding profile_add("whitelist ${HOME}/.local/share/appimagekit") when --appimage is passed. Or (__b__) having a file whitelist-appimage.inc, which gets loaded when --appimage is specified.

My preference is for __3.b__ because it's more user-configurable than __3.a__ and is appimage specific. The (extremely minor) drawback for __3__ (and __2__) is that if the appimage is malicious, they will be able to see all the responses for other appimage programs. I don't see how that could really be a security issue though.

@netblue30, what are your thoughts on this? Would a __3.b__ be acceptable?

enhancement

Most helpful comment

I'm hesitant about making the config the profile parsing more complex because of introducing parsing bugs that could allow privilege escalation.

Definitely. I just don't think having a file be auto-loaded without any way of turning it off (or changing which file is loaded) is really a great idea.

All 9 comments

IMHO 3b adds complexity to the user interface, in this regard options 2 or 3a would be better.

How does __3b__ add complexity to the user interface (by which I assume you mean command line)? __3b__ does not propose making any changes to the command line. It would add an extra whitelist-appimage.inc to the SYSCONFDIR, but that would be automatically installed. So the user would literally do nothing different than from __3a__.

The difference is that __3b__ allows other appimage specific profile options to be added later by the user, so its more flexible.

To me it seemed that you could reach your goals as well with 2 or 3a, in which case complexity in the profile infrastructure would be something to consider.

But I don't use appimages myself and indeed don't have a strong opinion. I'm quite easy to convince :smile:

2 seems like a bad idea because that directory should really only be whitelisted _if_ the application needs it (i.e. if the user is running an appimage). 3a is bad because it hardcodes things that can't easily be changed (unlike an appimage-specific whitelist file). I would vote for 3b.

On another note, what happens when you vote to integrate the appimage and the directory it writes to isn't whitelisted? We may need to add ~/.local/share/applications to the whitelist-appimage.inc file, since I believe that is where it writes its .desktop files.

@chiraag-nataraj, I haven't tested, so take it with a grain of salt. However I believe the answer to your question is that the desktop files get written to somewhere in $HOME (probably ~/.local/share/applications). Btw, ~/.local/share/applications is already whitelisted and readonly in whitelist-common.inc. I don't know that the desktop integration itself is that important because the user will likely be running the program through firejail anyway, so the .desktop files won't be of much use. I don't use .desktop files, so I'm not certain of that assertion. The main point for me was to stop having to deal with 2 annoying pops every time I start the appimage.

A more interesting extension (which might be useful to have for other reasons) is having conditional loading of includes. Something like:

if(appimage) include whitelist-appimage.inc

(the syntax is up for debate, of course). Basically, we should hopefully be able to not hardcode the loading of the file.

I'm hesitant about making the config the profile parsing more complex because of introducing parsing bugs that could allow privilege escalation. Having said that, this doesn't seem too bad.

I'm hesitant about making the config the profile parsing more complex because of introducing parsing bugs that could allow privilege escalation.

Definitely. I just don't think having a file be auto-loaded without any way of turning it off (or changing which file is loaded) is really a great idea.

@chiraag-nataraj, I had the if(conditional) syntax parsing nearly complete when it occurred to me that its kind of more complicated a syntax than it needs to be. I've settled on the extremely basic ?CONDITIONAL <profile line>. I flirted with the idea of using {CONDITIONAL} <profile line>, but that also seemed more complicated than it needed to be. I'm open to changing it to something else if we can settle on something reasonable. It does seem good enough as-is though.

Was this page helpful?
0 / 5 - 0 ratings