Comment 9 for bug 1791711

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

I took another look at this for hardening the snap-confine profile and reconfirmed that even adding a rule like so:

# pivot_root mediation in AppArmor is not complete. See LP: #1791711
#pivot_root,
pivot_root oldroot=/tmp/snap.rootfs_*/var/lib/snapd/hostfs/ /tmp/snap.rootfs_*/,

is not sufficient and still allows bypassing the profile since a user could modify the reproducer to use:
...
  if (mount("/tmp/snap.rootfs_BAD", "/tmp/snap.rootfs_BAD", NULL, MS_BIND, NULL)) err(1, "mount");
  int pvr = syscall(__NR_pivot_root, "/tmp/snap.rootfs_BAD", "/tmp/snap.rootfs_BAD/var/lib/snapd/hostfs");
  if (pvr == 0)
    printf("pivot_root success\n");
  else
    perror("pivot_root");
  try_open("/var/lib/snapd/hostfs/etc/shadow");
...

then do:
$ mkdir -p /tmp/snap.rootfs_BAD/var/lib/snapd/hostfs
$ sudo gcc -shared -o /usr/lib/x86_64-linux-gnu/libc.so-preload preload_p.c -fPIC && sudo chmod u+s /usr/lib/x86_64-linux-gnu/libc.so-preload && LD_PRELOAD=libc.so-preload /usr/lib/snapd/snap-confine
constructor running from /usr/lib/snapd/snap-confine
fail: /etc/shadow (Permission denied)
pivot_root success
success: /var/lib/snapd/hostfs/etc/shadow
read got: "root:!:17"

and therefore subvert the profile.

That said, a normal user isn't going to be able to create a setuid library in a root-owned path which allows LD_PRELOAD to work with the setuid snap-confine. Therefore subverting the profile in this exact manner would need to be combined with another vulnerability on the system. I realize the POC is meant to demonstrate what can happen when snap-confine is fully under attacker control, and while I look forward to when we have full pivot_root mediation in apparmor so we can address this bug, I still consider the profile a useful hardening measure since even if snap-confine were under full attacker control, it is still limited by the profile. As such, to add a little more hardening, I'm considering adding this:

# pivot_root mediation in AppArmor is not complete. See LP: #1791711. However,
# we can mediate the new_root and put_old to be what we expect, and then deny
# directory creation within old_root to prevent trivial pivoting into a
# whitelisted path
audit deny /tmp/snap.rootfs_*/{var/,var/lib/,var/lib/snapd/,var/lib/snapd/hostfs/} w,
pivot_root oldroot=/tmp/snap.rootfs_*/var/lib/snapd/hostfs/ /tmp/snap.rootfs_*/,

This doesn't address the modified POC since, again, a user could create /tmp/snap.rootfs_BAD/var/lib/snapd/hostfs, but it would at least prevent snap-confine itself from being able to create the old_root. Note, the explicit deny is just to ensure a rule isn't added that would undo the hardening.