BAMF_DESKTOP_FILE_HINT not set in correct place for unity7

[Jamie Strandboge]

Occasionally when I pin items to the Unity7 launcher, the BAMF code (as I'm told) incorrectly matches to /snap/app/revision/.... This is a security issue because the Exec= line points to /snap/app/revision/... which bypasses snap run (/snap/bin/...) and therefore snap-confine.

I'm told by Marcus (aka, 3v1n0 aka Trevinho) that this is because BAMF_DESKTOP_FILE_HINT is not exported by snap env and instead only injected in the desktop file that is created in /var/lib/snapd/desktop/applications upon snap install. This means that the wrong Exec= (ie, where it points to the binary) may occur in two places:

1. when launching /snap/bin/... from the command line
2. when something in /var/lib/snapd/desktop/applications/*.desktop doesn't match properly

In both cases, the initial launch is fine, but pinning the icon to the launcher results in the wrong entry in the Exec= line and launching from this pinned launcher entry after is unconfined. You can check by doing:

1. launch application from the dash
2. run sudo aa-status and see if it is launched under confinement
3. pin the icon that is in the launcher
4. close the application, then launch from the pinned icon
5. run sudo aa-status and see if it is launched under confinement

This doesn't happen all the time. For example, vlc seems to work fine both from the command line and from launching via a pinned launcher entry. chrome-test on the other hand doesn't seem to work with either.

Related https://github.com/snapcore/snapd/pull/1580 -- puts BAMF_DESKTOP_FILE_HINT in the desktop file instead of in the environment, but Marco requested that this change (https://github.com/snapcore/snapd/pull/1580#issuecomment-234546220).

https://trello.com/c/xP1hN3BF/152-improve-desktop-file-support-by-adding-a-new-bamf-desktop-file-hint-environment-hint also discussed this issue, but the card is archived and therefore it won't be worked on.

I'm having trouble finding a simple reproducer (other than chrome-test) but am told by Marco that the BAMF matching will always work if BAMF_DESKTOP_FILE_HINT in the process' environment always points to the desktop file in /var/lib/snapd/desktop/applications. I will continue to look for a simple reproducer.

[Marco Trevisan (Treviño) (3v1n0)]

We also should protect BAMF from creating .desktop files for such SNAPs pointing to the actual binary instead that to the wrapper script.

[Jamie Strandboge (jdstrand)]

Here is one that results in a broken launcher:

$ snap install glade-attente

$ glade-attente.glade

pin to launcher

close glade

try to launch glade-attente.glade

The resulting desktop entry in ~/.local/share/applications/glade.desktop is 'Exec=glade'

Notice that the desktop file name (glade.desktop) and the Exec= don't match glade-attente.glade

[Jamie Strandboge (jdstrand)]

(glade is also not found in the dash)

[Jamie Strandboge (jdstrand)]

Here is one that results in launching outside of confinement:

$ sudo snap install chrome-test --edge

Launch from the dash

Pin the non-pulsating icon to the dash

The resulting desktop entry in ~/.local/share/applications/google-chrome.desktop is 'Exec=/snap/chrome-test/16/opt/google/chrome/chrome --no-default-browser-check --no-first-run --user-data-dir=/home/jamie/snap/chrome-test/common'

Notice that the desktop file name (google-chrome.desktop) and the Exec= don't match 'chrome-test'

[Jamie Strandboge (jdstrand)]

Interestingly, installing 'snap install gnome-easytag --devmode --edge' and pinning it to the launcher results in nothing created for it in ~/.local/share/applications but it launches under confinement (as seen with aa-status).

[Jamie Strandboge (jdstrand)]

But if I launch easytag from the command line and pin the resulting icon, then ~/.local/share/applications/easytag.desktop is created with Exec=easytag (ie, both the desktop file name and Exec is wrong).

[Jamie Strandboge (jdstrand)]

$ sudo snap install trusthtables

It doesn't show up in the dash even though /var/lib/snapd/desktop/applications/trusthtables_truthtables.desktop exists

launching from command line:
$ truthtables
/snap/truthtables/3/bin/qt5-launch: 74: exec: truthtables: not found

(something obviously wrong with the snap)

[Jamie Strandboge (jdstrand)]

claswsmail results in even different behavior:

sudo snap install claws-mail-moon127 --devmode --edge

launch from dash, launched via snap-confine (see aa-status)

pin non-pulsating to launcher, launches outside of snap run (see aa-status) with no desktop file in ~/.local/share/applications

[Jamie Strandboge (jdstrand)]

At this point it is hard to pin point what the problem is as there is different behavior depending on the snap where:

- sometimes things show up in the dash, other times not
- sometimes we can pin the launcher and have it launch under snap-confine and sometimes not
- sometimes we can launch from the command line, pin it to the launcher and launch it under snap-confine from pinned icon and sometimes not
- sometimes desktop files show up in ~/.local/share/applications, sometimes not

What they all have in common is that /var/lib/snapd/desktop/applications/*.desktop files are created. Unity and snapd must work together for predictable application launching both from a usability and security standpoint.

[Jamie Strandboge (jdstrand)]

I'm marking this as High for now since it appears that a malicious snap should be able to craft a desktop file such that it will launch outside of confinement.

[Michael Vogt (mvo)]

Right now the way desktop files work is that you put them in meta/gui/*.desktop and they will get installed into /var/lib/snapd/desktop/applications.

When running a snap I could introspect that dir and look if snap.command is part of the /var/lib/snapd/desktop/application/*.desktop and if so, set BAMF_DESKTOP_FILE_HINT.

However what about the case when two desktop files reference the same command with different parameters? Eg: "game.desktop:Exec=game" and "game-leveleditor.desktop":Exec=game --level-edior" (or libreoffice --writer which is a more real-live example).

*If* doing a naive matching as outlined above helps I'm happy to do it.

[John Lenton (chipaca)]

The way we're currently doing it is by setting the environment from the desktop file, and that works OK for things launched from that desktop file (right?).

So if when running a snap we find that env key not set, I think we need to set BAMF_DESKTOP_FILE_HINT to *something*. Otherwise things (bamf in this instance, but I'm sure other desktops do something similar) can and will pick up the unwrapped binary, in cases where the app is actually a wrapper shell script.

So, I think what we do today in snappy is OK, in that we copy the snap-provided desktop files over, but it's not enough: we need to be a bit smarter, and ensure that every app that requests unity7 (and x11, methinks) has a desktop file, either generating it ourselves or denying the interface when missing, and we need to set that env key to point to something ourselves if it isn't set already.

If a snap provides multiple desktop files per app, and the env key isn't provided, I think just matching the shortest exec line would work. If the app needs more control they can provide multiple apps that map to the same binary.

If an app is launched that requests one of these interfaces and we can't match it with a desktop file, we should bail. This'll cover corner cases related to weird upgrade paths, amongst other things.

To recap, I think:

* `snap install` and friends should start creating desktop files when needed and missing (iow, if snap run won't be able to find it)
* `snap run` figures out if the thing should have BAMF_DESKTOP_FILE_HINT, find the right desktop file, and set it to that; bailing if missing (if there's a way to know we were launched not-from-command-line, pop up a dialog?)
* `snap-confine` should bail if an interface that we know needs this doesn't have it

[Michael Vogt (mvo)]

As discussed on telegram we agreed that `snap run` should do basic matching when not run from a desktop file. However we should be aware that this matching will not 100% accurate because desktop files may use the same binary but different commandline arguments.

[Jamie Strandboge (jdstrand)]

Is there any more progress on this?

[Jamie Strandboge (jdstrand)]

Actually, there is https://bugs.launchpad.net/bamf/+bug/1747802 which is fixed. I checked the code and this should be resolved. Marking as fixed.