netblue30 / firejail

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

firecfg does not detect all .desktop files for cleaning #2624

Open rusty-snake opened 5 years ago

rusty-snake commented 5 years ago

Firecfg can replace DBusActivatable=true with false in .desktop files (#1574), but does not recognize all .desktop files belonging to a programm.

OS: Fedora Workstation 29 (GNOME) Firejail: 0.9.57

Example where it occurs:

Looks like no gnome- or a uppercase letter after the org.gnome. is an issue. (I don't know how firecfg scan for .desktop files).

Example where it not occurs:

~Also nautilus (org.gnome.Nautilus.desktop) is cleaned up, although it does not have a firejail profile.~

rusty-snake commented 5 years ago

IMHO a good way to fix this is that firecfg scan in all .desktop files in /usr/share/applications for the Exec line.

glitsj16 commented 5 years ago

@rusty-snake Firecfg already does that, and it handles lower- and uppercase filenames. Also, nautilus does have a profile. I don't use firecfg but for fun I installed firejail from git master in an Arch Linux systemd-nspawn container. When running sudo firecfg --debug I can't reproduce what you're seeing. Both gedit and nautilus desktop files in ~/.local/share/applications have DBusActivatable=false. Have you tried to reproduce the issues with a more recent firejail version yet?

SkewedZeppelin commented 5 years ago

@glitsj16 I can reproduce the report exactly, it is indeed a problem.

I think I saw a similar issue long ago on Arch when it was first implemented, where it would ignore certain .desktops.

Even in the original implementation it was an issue https://github.com/netblue30/firejail/issues/1574#issuecomment-331872888

rusty-snake commented 5 years ago

@rusty-snake Firecfg already does that, and it handles lower- and uppercase filenames.

As I say I don't know how firecfg does that internaly (I can't C).

Also, nautilus does have a profile.

Uhh, yes, your right.

Both gedit and nautilus desktop files in ~/.local/share/applications have DBusActivatable=false.

Example where it not occurs:

Yes for baobab, nautilus and gedit it works.

Have you tried to reproduce the issues with a more recent firejail version yet?

Not yet (later I wil do this with an git version). But I can't find an commit in https://github.com/netblue30/firejail/commits/master/src/firecfg/desktop_files.c that change there something.

glitsj16 commented 5 years ago

@rusty-snake Don't worry about it too much. @SkewedZeppelin can reproduce, so you found a bug. Nice find!

rusty-snake commented 5 years ago

@glitsj16 just to complete: reproduced with 8e5ad20.

glitsj16 commented 5 years ago

@rusty-snake I can reproduce now too (originally got the working/failing examples from your OP mixed-up as you pointed out). After some more testing I can only conclude that firecfg seems pretty broken.

There's more going wrong than the DBusActivatable issue IMHO. Epiphany doesn't have that entree in its .desktop file (at least not in Arch Linux and upstream git master). Allthough firecfg reports finding /etc/firejail/epiphany.profile and creates the symlink, it doesn't create a .desktop file in ~/.local/share/applications. Furthermore, epiphany is reported to exists in /bin (which is incorrect, it's in /usr/bin) by the Configuring symlinks ... part of the firecfg run, but isn't found (or reported as such) in the Fixing desktop files ... part.

$ sudo firecfg --debug
glitsj16 0 0 0 0
Removing all firejail symlinks:

Configuring symlinks in /usr/local/bin based on firecfg.config
...
found epiphany in directory /bin
   epiphany created
...
Fixing desktop files in /home/glitsj16/.local/share/applications
checking profile for org.gnome.baobab.desktop
found /etc/firejail/baobab.profile
found baobab in directory /bin
   org.gnome.baobab.desktop created
...
checking profile for org.gnome.Epiphany.desktop
found /etc/firejail/epiphany.profile
checking profile for geoclue-where-am-i.desktop
...

The other applications you mentioned indeed fail because they don't make it thru the checks in desktop_files.c during execution of the have_profile function. Which makes sense, there are in fact no profiles with those names (Builder, clocks, Logs, Maps). That's why org.gnome-logs.desktop works, and even org.gnome.Logs.desktop when you add Logs to firecfg.config and symlink the gnome-logs.profile to Logs in /etc/firejail.

Unrelated but nonetheless problematic (at least to me as a non-firecfg user) is that sudo firecfg --clean does NOT remove the .desktop files in ~/.local/share/applications it created. What happens if an upgrade changes the Exec=foo command? Or DBusActivatable=true is added? I'm marking this as a bug. Might attract attention from firecfg devs.

glitsj16 commented 5 years ago

@rusty-snake Just pushed a temporary fix. Lets keep this open until a proper fix is available. Thanks again for reporting!

rusty-snake commented 4 years ago

@glitsj16 If I read the desktop_file.c right, it doesn't search for Exec, it only checks the names.


Not only DBus cleaning is sometimes broken, also Exec cleaning (see #3179).

glitsj16 commented 4 years ago

Not only DBus cleaning is sometimes broken, also Exec cleaning (see #3179).

@rusty-snake It is indeed. I'm collecting info to try to fix firecfg, but it will take a few days at least. Thanks for the input :+1: .

rusty-snake commented 4 years ago

@glitsj16 I have written something in python, I have to test it and will post it tomorrow.

rusty-snake commented 4 years ago

@glitsj16 https://gist.github.com/rusty-snake/3e4b8f8555e942d2964a181d4a5f64a0

rusty-snake commented 4 years ago

Quick diff

firecfg:

firecfg.py:

glitsj16 commented 4 years ago

@rusty-snake I'll have to do some more testing but your python script seems to work fine. It's too bad that firecfg bugs haven't been getting the attention they need. Hopefully this will change soon.