browser-support[allow-sandbox=true] should not grant access to /var/lib/snapd/desktop/applications

[James Henstridge]

Access to /var/lib/snapd/desktop/applications interferes with proper functioning of GLib's portal support.

The problem is that g_app_info_launch_default_for_uri() only calls out to the portal if it can't find a .desktop file that can handle the mime type. By making the .desktop files under /var/lib/snapd readable to the sandboxed app, it will try to launch them directly in preference to delegating the task to xdg-desktop-portal. This of course fails, since strictly confined snap applications can not invoke other snap applications.

The AppArmor rules in snapd are prefaced with a comment saying "Combined, the risks outweigh the benefits of closing this information leak for the small number of snaps allowed to use allow-sandbox: true". I think we need to revisit those risks, since it is causing actual harm.

[Jamie Strandboge (jdstrand)]

Currently the desktop-legacy and unity7 interfaces allow:

/var/lib/snapd/desktop/applications/ r,
/var/lib/snapd/desktop/applications/mimeinfo.cache r,
/var/lib/snapd/desktop/applications/@{SNAP_INSTANCE_NAME}_*.desktop r,

Due to how apparmor works, deny rules take precedence over allow rules.

A) Adding:

  deny /var/lib/snapd/desktop/applications/ r,

would block all of that for desktop-legacy and unity7.

B) Using this instead:

  deny /var/lib/snapd/desktop/applications/*.desktop r,

would block @{SNAP_INSTANCE_NAME}_*.desktop rule for desktop-legacy and unity7. It would also leave the information leak open (since can read the dir).

C) is like B but we also replace the @{SNAP_INSTANCE_NAME}_*.desktop rule in desktop-legacy and unity7 with the explicit deny rule. This might cause a regression for anything expecting to read its own desktop file.

D) We could simply remove the rules from browser-support, but that would mean we'd have noisy denials in the logs (as happens now for anything that tries to read these files (like anything that doesn't plugs browser-support[allow-sandbox=true]).

E) We could create a utility function in the interfaces code that dynamically generates the deny rule and the allow rule based on @{SNAP_INSTANCE_NAME}. Eg

  spec.AddSnippet(specific_desktop_applications(<snap instance name>))

where it yields for foo.desktop:

  /var/lib/snapd/desktop/applications/ r,
  /var/lib/snapd/desktop/applications/@{SNAP_INSTANCE_NAME}_*.desktop r,
  deny /var/lib/snapd/desktop/applications/@{HOME}/[^f]*.desktop r,
  deny /var/lib/snapd/desktop/applications/@{HOME}/f[^o]*.desktop r,
  deny /var/lib/snapd/desktop/applications/@{HOME}/fo[^o]*.desktop r,
  deny /var/lib/snapd/desktop/applications/@{HOME}/foo[^_.]*.desktop r,

(we can clean up /usr/share/applications a bit as well to only allow access to mimeapps.list, snap-handle-link.desktop and xdg-open.desktop).

We can use this in browser-support, desktop and desktop-legacy which allows all the expected access, eliminates the noise and should be low risk of regression. Info leak remains.

I don't care for A, B or C since they seem regression prone. A and C remove the info leak, yes, but this bug isn't about the info leak and there are other leaks in the code base (we should try to not introduce more info leaks to the code base, but removing them must be done with care and in a way so as not to break existing applications).

The options are then D and E IME. D can be done quickly and will introduce some noise, but we can perhaps get that into 2.44.1. E would be in a subsequent PR in 2.45.

[Jamie Strandboge (jdstrand)]

FYI, I implemented https://github.com/snapcore/snapd/pull/8301 for option 'D'. As it stands, I think there is too much for 2.44.1. Please comment in that PR on the approach. If people are in agreement it is a good approach, I can implement 'E' on top of it.

[James Henstridge (jamesh)]

For what it is worth, the access rules in unity7 and desktop_legacy are probably unnecessary too.

In desktop-legacy, it is prefixed with the following comment:

    # Support BAMF_DESKTOP_FILE_HINT by allowing reading our desktop files
    # parallel-installs: this leaks read access to desktop files owned by keyed
    # instances of @{SNAP_NAME} to @{SNAP_NAME} snap

That doesn't quite match up with how bamf actually works. The function that uses the environment variable is here:

    https://git.launchpad.net/bamf/tree/src/bamf-matcher.c#n560

As one of the methods for discovering the desktop file associated with a process, bamf-daemon will read the process's /proc/$pid/environ looking for the BAMF_DESKTOP_FILE_HINT variable. The target process is not expected to read the environment variable, let alone the file it references. And bamf-daemon itself is going to be running unconfined so it doesn't care about the AppArmor rules either.

Doing a code search on Github (https://github.com/search?q=BAMF_DESKTOP_FILE_HINT&type=Code) did show up some KDE use of the environment variable, but it is used in pretty much the same way as bamf itself does:

    https://github.com/KDE/plasma-workspace/blob/master/libnotificationmanager/utils.cpp#L48
    https://github.com/KDE/plasma-workspace/blob/master/libtaskmanager/tasktools.cpp#L506

The search also showed up some uses in Go code, but that was all in snapd plus its forked repositories. The vast majority of the hits were in desktop files setting the environment variable, or data that had clearly been derived from desktop files.

So choice (A) might not be such a bad idea (or even deny /var/lib/snapd/desktop entirely).

[Jamie Strandboge (jdstrand)]

I tracked it down to unity messaging menu and xdg-mime requirements. I updated the PR accordingly to remove the incorrect BAMF info (thanks!) with more info in the PR for you to consider.

[Jamie Strandboge (jdstrand)]

A simple reproducer for this is, on a focal system:
1. snap install firefox --candidate # has the xdg-portal support
2. snap install eog # has support for image/tiff, which firefox does not
3. copy a tiff file into the user's home
4. navigate firefox to ~/some.tiff

Without the fix, will see:
env: '/snap/bin/eog': Permission denied

with the following denial:

audit(1591112420.394:85): apparmor="DENIED" operation="exec" profile="snap.firefox.firefox" name="/snap/snapd/7264/usr/bin/snap" pid=5568 comm="env" requested_mask="x" denied_mask="x" fsuid=1000 ouid=0

With it, it works.

[Jamie Strandboge (jdstrand)]

This was fixed a long time ago and is confirmed fixed in the current release of snapd.