Snapd vs upstream kernel vs apparmor

mwhudson · August 15, 2017, 10:11pm

Hi,

There is a debian bug (https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=851473) that says that snapd basically does not work at all when apparmour is enabled in debian. I was going to say that the latest 2.27.1-1 upload does not fix this but it seems that maybe it did, I’m quite confused. In any case, Debian is probably going to enable apparmor by default for Buster (yay!!) so we should have a plan to make sure that snapd works with apparmor enabled on a kernel without all the ubuntu patches. There are two aspects to this I guess:

making sure that snapd functions at all when apparmor is enabled but apparmor perhaps misses some features (this is what the debian bug above is about)
actually using the limited apparmor features to enforce as much confinement as possible

Have people done some proper thinking about this already? (@jdstrand?). I don’t really have any idea how to test 2. above currently.

(I get the impression that all the apparmor features may in fact be upstream by the time buster is released but in the mean time…)

jdstrand · August 16, 2017, 1:06pm

The plan is to have everything upstream, but I don’t know what kernel Buster will use.

I just tested this in an up to date sid with 4.12 kernel. I did:

$ sudo apt-get install snapd
$ sudo snap install hello-world
$ /snap/bin/hello-world.evil
Hello Evil World!
This example demonstrates the app confinement
You should see a permission denied error next
If you see this line the confinement is not working correctly, please file a bug

That tested snapd with apparmor disabled. To enable apparmor, adjust /etc/default/grub to have:

GRUB_CMDLINE_LINUX_DEFAULT="quiet apparmor=1 security=apparmor"

then do:

$ sudo update-grub
$ sudo reboot
<login again>
$ sudo aa-status
$ sudo aa-status
apparmor module is loaded.
0 profiles are loaded.
0 profiles are in enforce mode.
0 profiles are in complain mode.
0 processes have profiles defined.
0 processes are in enforce mode.
0 processes are in complain mode.
0 processes are unconfined but have a profile defined.
$ snap --version
snap    2.27.1-1
snapd   2.27.1-1
series  16
debian  unknown
kernel  4.12.0-1-amd64
$ hello-world.evil 
Hello Evil World!
This example demonstrates the app confinement
You should see a permission denied error next
If you see this line the confinement is not working correctly, please file a bug

I found this curious-- the snap-confine profile was not loaded. I found that ‘apt-get install snapd’ installs an empty profile in /etc/apparmor.d/usr.lib.snapd.snap-confine.real

$ ls -l /etc/apparmor.d/usr.lib.snapd.snap-confine.real
-rw-r--r-- 1 root root 0 Aug 14 04:53 /etc/apparmor.d/usr.lib.snapd.snap-confine.real

If I generate the profile from git checkout release/2.27 (I used ./configure --prefix=/usr --libexecdir=/usr/lib/snapd --enable-nvidia-ubuntu --enable-static-libcap --enable-static-libapparmor --enable-static-libseccomp) and copy it over to the Debian machine:

$ sudo cp ./snap-confine.apparmor /etc/apparmor.d/usr.lib.snapd.snap-confine.real 
$ sudo apparmor_parser -r /etc/apparmor.d/usr.lib.snapd.snap-confine.real 
$ sudo aa-status
apparmor module is loaded.
2 profiles are loaded.
2 profiles are in enforce mode.
   /usr/lib/snapd/snap-confine
   /usr/lib/snapd/snap-confine//mount-namespace-capture-helper
0 profiles are in complain mode.
0 processes have profiles defined.
0 processes are in enforce mode.
0 processes are in complain mode.
0 processes are unconfined but have a profile defined.
$ sudo snap install hello-world
...
$ /snap/bin/hello-world.evil
/usr/lib/snapd/snap-confine: error while loading shared libraries: libcap.so.2: cannot open shared object file: No such file or directory
$ grep DEN /var/log/syslog
Aug 16 07:22:48 debian-sid-amd64 kernel: audit: type=1400 audit(1502886168.383:7): apparmor="DENIED" operation="open" profile="/usr/lib/snapd/snap-confine" name="/lib/x86_64-linux-gnu/libcap.so.2.25" pid=2627 comm="snap-confine" requested_mask="r" denied_mask="r" fsuid=0 ouid=0

Ok, so snap-confine is confined, but it is missing a rule needed to run. Note that snapd didn’t load the hello-world profiles:

$ sudo aa-status
apparmor module is loaded.
2 profiles are loaded.
2 profiles are in enforce mode.
   /usr/lib/snapd/snap-confine
   /usr/lib/snapd/snap-confine//mount-namespace-capture-helper
0 profiles are in complain mode.
0 processes have profiles defined.
0 processes are in enforce mode.
0 processes are in complain mode.
0 processes are unconfined but have a profile defined.

If I add this rule to the snap-confine profile:

/lib/@{multiarch}/libcap.so* mr,

Then reload the profile and try hello-world.evil, I can reproduce the Debian bug:

$ sudo apparmor_parser -r /etc/apparmor.d/usr.lib.snapd.snap-confine.real
$ /snap/bin/hello-world.evil 
execv failed: Permission denied
$ sudo grep DEN /var/log/syslog | tail -1
Aug 16 07:25:17 debian-sid-amd64 kernel: [ 1351.246752] audit: type=1400 audit(1502886317.157:10): apparmor="DENIED" operation="exec" profile="/usr/lib/snapd/snap-confine" name="/usr/lib/snapd/snap-exec" pid=2665 comm="snap-confine" requested_mask="x" denied_mask="x" fsuid=1000 ouid=0

Ok, if I build 2.27 like we do on Ubuntu, and copy it all over to the sid machine, then snapd still won’t load the hello-world profiles:

$ sudo aa-status
apparmor module is loaded.
2 profiles are loaded.
2 profiles are in enforce mode.
   /usr/lib/snapd/snap-confine
   /usr/lib/snapd/snap-confine//mount-namespace-capture-helper
0 profiles are in complain mode.
0 processes have profiles defined.
0 processes are in enforce mode.
0 processes are in complain mode.
0 processes are unconfined but have a profile defined.

This is because snapd forces devmode (see requiredApparmorFeatures in release/release.go). If I booted a kernel with all the required features, I would expect things to start to work.

I believe there are several bugs:

the snap-confine profile is empty
the snap-confine profile is missing a rule for libcap (on Debian we aren’t using --enable-static-libcap like in Ubuntu)
the snap-exec denial has to do with the fact that snapd is built with --disable-apparmor. snap-confine is not performing a profile transition to hello-world.evil, so snap-confine continues to stay in its profile where snap-exec is not allowed
snapd-design: when booting a kernel without the required apparmor feature set, the system is put in devmode and apparmor profiles are not loaded

Detangling this would require:

ship a proper apparmor profile for snap-confine
use --enable-static-libcap
conditionally add a rule to snap-confine’s profile: /usr/lib/snapd/snap-exec uxr,

With the above, one should be able to freely enable/disable apparmor via kernel command line, and when booting a kernel with the supported feature set, the profiles will be generated. Note that point ‘3’ needs a little design since snapd will be updating snap-confine’s profile dynamically.

Once all that is done, we could revisit the required apparmor feature set, but note this is an explicit snapd design decision that went through extensive review with @niemeyer, @mvo, @zyga-snapd and myself.

jdstrand · August 16, 2017, 1:49pm

I had an idea for how to do this in this topic Snaps and NFS /home - #9 by jdstrand

mwhudson · August 16, 2017, 9:57pm

Point 1 is explained by this bit of the rules file I think:

        # Rename the apparmor profile, see dh_apparmor call above for an explanation.
ifeq ($(shell dpkg-vendor --query Vendor),Ubuntu)
        mv $(CURDIR)/debian/tmp/etc/apparmor.d/usr.lib.snapd.snap-confine $(CURDIR)/debian/tmp/etc/apparmor.d/usr.lib.snapd.snap-confine.real
else
        # HACK: As we want to share the packaging bits between Debian and Ubuntu
        # we have to get an empty file in place zwhich satisfies the item in
        # the snapd.install file.
        install -d $(CURDIR)/debian/tmp/etc/apparmor.d
        touch $(CURDIR)/debian/tmp/etc/apparmor.d/usr.lib.snapd.snap-confine.real
endif

Point 2. is similarly explained by this:

# Currently, we enable confinement for Ubuntu only, not for derivatives,
# because derivatives may have different kernels that don't support all the
# required confinement features and we don't to mislead anyone about the
# security of the system.  Discuss a proper approach to this for downstreams
# if and when they approach us
ifeq ($(shell dpkg-vendor --query Vendor),Ubuntu)
    # On Ubuntu 16.04 we need to produce a build that can be used on wide
    # variety of systems. As such we prefer static linking over dynamic linking
    # for stability, predicability and easy of deployment. We need to link some
    # things dynamically though: udev has no stable IPC protocol between
    # libudev and udevd so we need to link with it dynamically.
    VENDOR_ARGS=--enable-nvidia-ubuntu --enable-static-libcap --enable-static-libapparmor --enable-static-libseccomp
    BUILT_USING_PACKAGES=libcap-dev libapparmor-dev libseccomp-dev
else
ifeq ($(shell dpkg-vendor --query Vendor),Debian)
    VENDOR_ARGS=--disable-apparmor --disable-seccomp
    BUILT_USING_PACKAGES=libcap-dev
else
    VENDOR_ARGS=--disable-apparmor
endif
endif

I guess this is a downstream coming to say hello – although given we set BUILT_USING_PACKAGES=libcap-dev surely the intent is that --enable-static-libcap is used on Debian too.

For 3. there is a debian delta to add that line:

Description: modify apparmor rules to work with upstream apparmor implementation
 The Ubuntu kernel contains apparmor features that have not yet gone upstream.
 The patch changes the rules to work with the upstream implementation.
 .
 NB: this does not actually work, see bugs.debian.org/851473
Author: Michael Hudson-Doyle <michael.hudson@ubuntu.com>
Origin: vendor
Forwarded: not-needed
Last-Update: 2018-08-15
---
This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
--- a/cmd/snap-confine/snap-confine.apparmor.in
+++ b/cmd/snap-confine/snap-confine.apparmor.in
@@ -387,4 +387,9 @@
 
     # Allow snap-confine to be killed
     signal (receive) peer=unconfined,
+
+    # Required when using unpatched upstream kernel
+    capability sys_ptrace,
+    # Debian compiles snap-confine without AppArmor, so allow running snaps unconfined
+    /usr/lib/snapd/snap-exec uxr,
 }

But due to the fun and games in my first snippet above this doesn’t actually do anything. And I’m not sure even this change would help once there is a newer snapd in the core snap and reexecing is happening?

So this course of action suggests itself to me:

Actually install the patched apparmor profile
Use --enable-static-libcap

I plan to upload 2.27.2 today or tomorrow so and it would be nice to get these fixes in.

(I have an unpleasant feeling there is also a bug in the apparmor profiling handling during the upgrade from the version in stretch to the version in unstable, need to test that again)

mwhudson · August 16, 2017, 10:14pm

And uh wait, why is snap-confine compiled with --disable-apparmor? Is that to make it work in the case where apparmor is not enabled at all in the kernel, i.e. snap-confine does not do dynamic detection of that?

jdstrand · August 16, 2017, 10:15pm

That sounds good (it should, I recommended it Of course, when you do and someone boots into a kernel with apparmor enabled but not all the features, you’ll need the snap-exec rule (ie, the Debian bug will bite). If we can get the snap-exec rule fixed in snapd, then I think we can consider simply relying upon the forced devmode logic instead of having different packaging (ie, we always ship the profile, always build with static libcap) and not worry about upstreams, downstreams, derivatives, etc. snapd will add the snap-exec rule when in force devmode and won’t when in strict mode.

zyga-snapd · August 17, 2017, 9:21am

Because --disable-apparmor needs to be in sync with what snapd thinks of the current kernel. Ideally we’d have snapd tell snap-confine that such and such features need to be active but discussions on how to do that got stuck.

When they are out of sync snap-confine tries to apply a profile that doesn’t exist and fails.

mwhudson · August 17, 2017, 10:57am

It’s harder than I suggested earlier, unfortunately. With the kernel currently in buster, even if apparmor is enabled it does not have all the features snapd requires, so snapd does not generate a profile. If snap-confine has apparmor enabled, it tries to switch to a profile that doesn’t exist. If snap-confine does not have apparmor enabled, no profile for it is generated in the build… is there still value in running snap-confine under apparmor if it is not possible to switch to another profile for the snap?

The other issue is that as soon as the core snap is newer than the installed version, the snap-confine being used will have apparmor enabled and will hit the above problem, no matter what I do in the debian package.

jdstrand · August 17, 2017, 12:06pm

Ok, so I was thinking the snap-exec rule was needed when snapd detected a kernel without the full feature set, but the snap-exec rule is only needed when --disable-apparmor is used. This means that instead of dropping a file into /var/lib/snapd/apparmor/snap-confine.d for this, we can add the rule as part of profile compilation in the build (ie, snap-confine.apparmor.in -> snap-confine.apparmor). With this change, then the setuid executable is confined, even if snaps aren’t. I think this is a meaningful security improvement and could/should be done today.

As for handling the runtime kernel checks for whether to enable or disable apparmor, we could probably change this to be:

perform runtime requirements, if met, generate profiles like normal
if not met:
1. log that apparmor is enabled but the kernel doesn’t have the required features and that all snaps will not be confined with apparmor
2. generate an apparmor profile using a wide-open, non-complain mode profile (like we do for classic snaps; see classicTemplate in interfaces/apparmor/template.go)

@zyga-snapd, @mvo - what do you think? ^

mwhudson · August 17, 2017, 10:59pm

OK, so I’ve prepared a patch for this: Allow snap-confine to be confined even with --disable-apparmor by mwhudson · Pull Request #3760 · canonical/snapd · GitHub. I’ll include this with my 2.27.2 upload in a few moments. I’m also going to disable reexec on Debian for now although:

… this makes sense to me as a plan and seems a less intrusive change than the other one being discussed.

jdstrand · August 24, 2017, 12:09pm

This also came up in the context of https://bugs.launchpad.net/snappy/+bug/1687079/comments/11

@mvo, @zyga-snapd: ping on thoughts for my previous comment.

zyga-snapd · August 24, 2017, 12:13pm

I’m working on implementing the extra change to snap-confine’s profile but perhaps it is not really strictly required now (it could be retained for things like home-on-NFS.

I like the idea of open policy if apparmor is around but I worry that some trusted helpers may actually depend on the label and choose to act on the peer label assuming the peer is properly confined.

jdstrand · August 24, 2017, 12:19pm

Yes, it should be retained for home-on-NFS. I suspect there will be other things we’ll want to do here too (eg, with ecryptfs)

Re trusted helpers, that is a valid concern in general but trusted helpers always also allow connections from ‘unconfined’, which is where the conditional snap-exec ‘ux’ rule would put them. With everything getting a security label, the trusted helper is at least in a position to say ‘no, you can’t access stuff from that other label’ whereas with everything as unconfined, you can’t do that.

zyga-snapd · August 24, 2017, 12:24pm

Yeah, I agree.

Do you think we could actually confine the system meaningfully on a 4.12-4.14 kernel (assuming some patches don’t land) by taking action in the base template and on a per-interface basis?

jdstrand · August 24, 2017, 12:34pm

Depends on the definition of ‘meaningfully’. It would be incomplete, so the system could not be ‘certified/validated’ and there would be holes which is why if we did this we’d need to prominently log. You’d always have file mediation but in 4.14 for example, no ‘unix’ mediation. I don’t know what is in 4.12 vs 4.13 vs 4.14 otoh, but the farther back you go the less mediation there is. Things that could drop off would be: unix, signal, ptrace, dbus, network, and mount. IIRC, 4.12 doesn’t have any new rules (it has a ton of code to add new mediation types), 4.13 added some things and 4.14 added most everything except unix. So, if your definition of meaningfully means “at least we have file mediation”, then yes, if it is “it isn’t worth it unless we have full confinement”, then no.

@tyhicks - could you comment on this (particularly what rules show up when)?

zyga-snapd · August 24, 2017, 12:35pm

I was thinking about he case where 4.14 ships and is the next LTS kernel and we could enable apparmor support in any distribution that uses this kernel. If this makes snaps safer in general (not perfect but better than today) than I was considering doing it.

jdstrand · August 24, 2017, 12:38pm

4.14 adds, AIUI, everything except unix socket mediation. Need @tyhicks to confirm.

Note that unix socket mediation is important, so with 4.14 we’d want to prominently log snaps are running with partial confinement (ideally listing that unix is missing).

tyhicks · August 24, 2017, 2:32pm

Sure! I’ll only focus on rules and not the underlying changes, bug fixes, apparmorfs changes, etc.

4.13 added ptrace mediation. 4.14 adds signal, mount/umount/remount, pivot_root, and course-grained socket mediation. 4.15 will add UNIX socket and D-Bus mediation (which depends on UNIX socket mediation).