In Cannot run X applications on openSUSE Tumbleweed when using KDE we discussed a problem found in openSUSE where applications packaged as a snap doesn’t start in a KDE environment. Reason for this is that the necessary X11 authentication cookies are stored in a file inside /tmp on the host which isn’t shared with a snap.
Our X11 interface currently allows access to the following possible locations of the file specified in the XAUTHORITY environment variable:
# .ICEauthority files required for X authentication, per user
owner @{HOME}/.ICEauthority r,
# .Xauthority files required for X connections, per user
owner @{HOME}/.Xauthority r,
owner /{,var/}run/gdm{,3}/*/database r,
owner /{,var/}run/lightdm/authority/[0-9]* r,
owner /{,var/}run/lightdm/*/xauthority r,
owner /{,var/}run/user/*/gdm/Xauthority r,
If the XAUTHORITY environment variable points to one of these files we don’t have a problem and everything works as it should. However in some environments like KDE on openSUSE we can’t rely on this and don’t have the ability to fix the location of the file XAUTHORITY points to. Therefor we need to find another way to handle this problem more in general than only relying on a path we know and have in our AppArmor profiles.
I talked a bit with @zyga-snapd on IRC about possible solutions and what we came up with is the following:
When snap run gets executed, we parse the current XAUTHORITY environment variable and copy the file it points to into XDG_RUNTIME_DIR of the current user, more precisely at /run/user/$UID/snap.$SNAP_NAME/Xauthority. Afterwards we set XAUTHORITY to the new path. The snap application will still need to get access to that file by using the x11 interface, which will get an update to allow access the path too.
A preliminary implementation of this can be found here which I will submit as a PR once we agreed that this is the way forward.
This part is somewhat tricky. The XDG_RUNTIME_DIR is actually different (check and see) but we have an open bug where snap-confine should create an snap-run should set that variable (I think it is set already, even if snap-confine does not create that particular directory).
This part is somewhat tricky. The XDG_RUNTIME_DIR is actually different (check and see) but we have an open bug where snap-confine should create an snap-run should set that variable (I think it is set already, even if snap-confine does not create that particular directory).
Ok, to be precise: I meant XDG_RUNTIME_DIR outside of the snap environment. As long as we have what it points to available in the snap environment, it shouldn’t matter what the env variable XDG_RUNTIME_DIR is set to inside the snap env, as long as XAUTHORITY is set to an absolute path.
This path is allowed in the default profile as seen in interfaces/appamor/template.go:
owner /run/user/[0-9]*/snap.@{SNAP_NAME}/** mrwklix,
you later state:
but this is not true due to the above template rule.
I think the general ideas that we only want to allow access to the Xauthority file to snaps that plugs the x11 (or unity7) interface is good, and I also think that copying the file somewhere and setting XAUTHORITY make a lot of sense.
which is what I think we should be using instead of /run/user/[0-9]*/snap.@{SNAP_NAME}/Xauthority.
There is potential problem with the overall approach though in parsing XAUTHORITY to begin with since a malicious user could call ‘XAUTHORITY=/path/to/anything snap run …’ and whatever is in XAUTHORITY gets copied to /run/user/<uid>/Xauthority. We have to be careful here to ensure that, for example, /etc/shadow can’t be copied to /run/user/<uid>/Xauthority (eg, we don’t want to allow an unprivileged user to be able to copy a privileged file into the user’s XDG_RUNTIME_DIR with read permissions of the user, thus invalidating the intended DAC restrictions).
Here is what should be happening, in this order
ensure all of the below is happening when snap run is running as the user and not snapd
look at XAUTHORITY file and when set, create a temporary dir with 0700 permissions, then copy XAUTHORITY, its permissions and its times to this temporary dir
if /run/user/<uid>/Xauthority exists (eg, it’s out of date), compare it with /path/to/temporary/Xauthority. If same, done. Otherwise, proceed to step 4
verify the file is an xauth file. One way to do this would be to run xauth -f /path/to/temporary/Xauthority info and look for Number of entries: as > 0 (but account for translations!)
if is an xauthority file, move (with overwrite) /path/to/temporary/Xauthority to /run/user/<uid>/Xauthority
set XAUTHORITY to /run/user/<uid>/Xauthority in snap env
With the above and adding owner /run/user/[0-9]*/Xauthority r, to the x11 and unity7 abstractions, this should be robust and safe. It doesn’t guard against the user replacing arbitrary xauthority files that were created by the user, but we aren’t trying to protect the user from herself anyway, only ensuring that the /run/user/<uid>/Xauthority is a valid xauth file that the user can read.
Do we need the temporary file? These are generally tiny, and can be handled in memory. Let’s just enforce them to not be larger than a reasonable size with a read limiter.
Not strictly, no. The point of this file is to avoid race conditions with the checks on the original file. It would be fine to stat and read the file into memory and atomically write out to disk with the permissions and times. Having the temporary file does mean you can use the existing xauth tool for verification rather than reimplementing the verification bits-- but implementing the verification checks in snap run and not calling out to an external tool has its own advantages.
We probably don’t need a huge amount of verification at this point. The worst thing that can happen in this case in terms of security is the file being right.
Using xauth isn’t something we can easily do. It’s not part of the core snap. Making this a classic only thing also doesn’t work as we have and will have customers still using X11 also on Ubuntu Core. I could reimplement xauth parsing code in Go for this. What we want to avoid at all cost is having libxauth as a dependency. Any other ideas?
@jdstrand I’ve implemented what you suggested now with this commit but also added validation of the Xauthority file with a simple parsing of data inside the Xauthority file to validate it. This misses unit tests etc. and is still a bit rough. Once we have an agreement on how we want to proceed I can finalize this and propose a PR.
No, this topic is about making sure that on distributions that store Xauthority files in /tmp that the Xauthority file is available somewhere. ssh forwarding is a different issue and discussed in X11 Forwarding using SSH (ie, .Xauthority doesn’t exist in /home/user/snap/foo/current/.Xauthority).
Running through that tutorial I have to sudo ubuntu-core-vm which should run a qemu window on my X11 system. Unfortunately my xauthority magic cookie is in /tmp on the host and is as such inaccessible to the snap.
My current workaround is to copy the /tmp/xauth-1000-_0 to /tmp/snap.0_ubuntu-core-vm_VMxtMv/tmp which is quite the bodge.
Is there a better fix since this was last brought up? Should the X11 interface be extended to fix this?
i dont see such issues on 16.04 when using a Core image with my qemu-virgil package (invocation help is in “snap info qemu-virgil”) which should be similar to (but not quite the same as) ubuntu-core-vm … do you have all interfaces connected for the snap ?
the ubuntu-core-vm has many fewer plugs associated with it. Comparing to qemu-virgil I see the ubuntu-core-vm does not declare these potentially relevant plugs:
alan@KinkPad-K450:~$ lsb_release -a
No LSB modules are available.
Distributor ID: neon
Description: KDE neon LTS User Edition 5.12
Release: 16.04
Codename: xenial
