From: Alex Murray Date: Mon, 19 Sep 2022 04:20:36 +0000 (+0930) Subject: [PATCH 2/4] many: Use /tmp/snap-private-tmp for per-snap private tmps X-Git-Tag: archive/raspbian/2.37.4-1+rpi1+deb10u2^2~1 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=1621f1ff27aeb9596499f034efcc5110a4c74419;p=snapd.git [PATCH 2/4] many: Use /tmp/snap-private-tmp for per-snap private tmps Backport of the following upstream patch: From fe2d2d8471665482628813934d9f19e8ca5e4a1f Mon Sep 17 00:00:00 2001 Backport of the following upstream patch: From fe2d2d8471665482628813934d9f19e8ca5e4a1f Mon Sep 17 00:00:00 2001 From: Alex Murray Date: Mon, 19 Sep 2022 13:50:36 +0930 Subject: [PATCH 2/4] many: Use /tmp/snap-private-tmp for per-snap private tmps To avoid unprivileged users being able to interfere with the creation of the private snap mount namespace, instead of creating this as /tmp/snap.$SNAP_NAME/ we can now use the systemd-tmpfiles configuration to do this for us at boot with a known fixed name (/tmp/snap-private-tmp/) and then use that as the base dir for creating per-snap private tmp mount namespaces (eg. /tmp/snap-private-tmp/snap.$SNAP_INSTANCE/tmp) etc. Signed-off-by: Alex Murray Gbp-Pq: Topic cve20223328 Gbp-Pq: Name 0017-cve-2022-3328-2.patch --- diff --git a/cmd/snap-confine/mount-support.c b/cmd/snap-confine/mount-support.c index 1fc44a7b..cda2ec17 100644 --- a/cmd/snap-confine/mount-support.c +++ b/cmd/snap-confine/mount-support.c @@ -21,6 +21,7 @@ #include "mount-support.h" +#include #include #include #include @@ -50,6 +51,7 @@ #include "mount-support-nvidia.h" #define MAX_BUF 1000 +#define SNAP_PRIVATE_TMP_ROOT_DIR "/tmp/snap-private-tmp" /*! * The void directory. @@ -59,140 +61,106 @@ **/ #define SC_VOID_DIR "/var/lib/snapd/void" -static int must_mkdir_and_open_with_perms(const char *dir, uid_t uid, gid_t gid, - mode_t mode) -{ - int retries = 10; - int fd; - - mkdir: - if (--retries == 0) { - die("lost race to create dir %s too many times", dir); - } - // Ignore EEXIST since we want to reuse and we will open with - // O_NOFOLLOW, below. - if (mkdir(dir, 0700) < 0 && errno != EEXIST) { - die("cannot create directory %s", dir); - } - fd = open(dir, O_RDONLY | O_DIRECTORY | O_CLOEXEC | O_NOFOLLOW); - if (fd < 0) { - // if is not a directory then remove it and try again - if (errno == ENOTDIR && unlink(dir) == 0) { - goto mkdir; - } - die("cannot open directory %s", dir); - } - // ensure base_dir has the expected permissions since it may have - // already existed - struct stat st; - if (fstat(fd, &st) < 0) { - die("cannot stat base directory %s", dir); - } - if (st.st_uid != uid || st.st_gid != gid - || st.st_mode != (S_IFDIR | mode)) { - unsigned char random[10] = { 0 }; - char random_dir[MAX_BUF] = { 0 }; - int offset; - size_t i; - - // base_dir isn't what we expect - create a random - // directory name and rename the existing erroneous - // base_dir to this then try recreating it again - NOTE we - // don't use mkdtemp() here since we don't want to actually - // create the directory yet as we want rename() to do that - // for us -#ifdef SYS_getrandom - // use syscall(SYS_getrandom) since getrandom() is - // not available on older glibc - if (syscall(SYS_getrandom, random, sizeof(random), 0) != - sizeof(random)) { - die("cannot get random bytes"); - } -#else - // use /dev/urandom on older systems which don't support - // SYS_getrandom - int rfd = open("/dev/urandom", O_RDONLY); - if (rfd < 0) { - die("cannot open /dev/urandom"); - } - if (read(rfd, random, sizeof(random)) != sizeof(random)) { - die("cannot get random bytes"); - } - close(rfd); -#endif - offset = - sc_must_snprintf(random_dir, sizeof(random_dir), "%s.", - dir); - for (i = 0; i < sizeof(random); i++) { - offset += - sc_must_snprintf(random_dir + offset, - sizeof(random_dir) - offset, - "%02x", (unsigned int)random[i]); - } - // try and get dir which we own by renaming it to something - // else then creating it again - - // TODO - change this to use renameat2(RENAME_EXCHANGE) - // once we can use a newer version of glibc for snapd - if (rename(dir, random_dir) < 0) { - die("cannot rename base_dir to random_dir '%s'", - random_dir); - } - close(fd); - goto mkdir; - } - return fd; -} // TODO: simplify this, after all it is just a tmpfs // TODO: fold this into bootstrap -static void setup_private_mount(const char *snap_name) +static void setup_private_tmp(const char *snap_instance) { - uid_t uid = getuid(); gid_t gid = getgid(); - char tmpdir[MAX_BUF] = { 0 }; - int base_dir_fd SC_CLEANUP(sc_cleanup_close) = -1; - - // Create a 0700 base directory, this is the base dir that is - // protected from other users. - // - // Under that basedir, we put a 1777 /tmp dir that is then bind - // mounted for the applications to use - sc_must_snprintf(tmpdir, sizeof(tmpdir), "/tmp/snap.%d_%s_XXXXXX", uid, - snap_name); - base_dir_fd = must_mkdir_and_open_with_perms(tmpdir, 0, 0, 0700); - // now we create a 1777 /tmp inside our private dir + char base[MAX_BUF] = { 0 }; + char tmp_dir[MAX_BUF] = { 0 }; + int private_tmp_root_fd SC_CLEANUP(sc_cleanup_close) = -1; + int base_dir_fd SC_CLEANUP(sc_cleanup_close) = -1; + int tmp_dir_fd SC_CLEANUP(sc_cleanup_close) = -1; + + /* Switch to root group so that mkdir and open calls below create + * filesystem elements that are not owned by the user calling into + * snap-confine. */ + if (setgid(0) < 0) { + die("cannot set group to 0"); + } + // /tmp/snap-private-tmp should have already been created by + // systemd-tmpfiles but we can try create it anyway since snapd may have + // just been installed in which case the tmpfiles conf would not have + // got executed yet + if (mkdir(SNAP_PRIVATE_TMP_ROOT_DIR, 0700) < 0 && errno != EEXIST) { + die("cannot create /tmp/snap-private-tmp"); + } + private_tmp_root_fd = open(SNAP_PRIVATE_TMP_ROOT_DIR, + O_RDONLY | O_DIRECTORY | O_CLOEXEC | + O_NOFOLLOW); + if (private_tmp_root_fd < 0) { + die("cannot open %s", SNAP_PRIVATE_TMP_ROOT_DIR); + } + struct stat st; + if (fstat(private_tmp_root_fd, &st) < 0) { + die("cannot stat %s", SNAP_PRIVATE_TMP_ROOT_DIR); + } + if (st.st_uid != 0 || st.st_gid != 0 || st.st_mode != (S_IFDIR | 0700)) { + die("%s has unexpected ownership / permissions", + SNAP_PRIVATE_TMP_ROOT_DIR); + } + // Create /tmp/snap-private-tmp/snap.$SNAP_INSTANCE_NAME/ 0700 root.root. + sc_must_snprintf(base, sizeof(base), "snap.%s", snap_instance); + if (mkdirat(private_tmp_root_fd, base, 0700) < 0 && errno != EEXIST) { + die("cannot create base directory: %s", base); + } + base_dir_fd = + openat(private_tmp_root_fd, base, + O_RDONLY | O_DIRECTORY | O_CLOEXEC | O_NOFOLLOW); + if (base_dir_fd < 0) { + die("cannot open base directory: %s", base); + } + if (fstat(base_dir_fd, &st) < 0) { + die("cannot stat %s/%s", SNAP_PRIVATE_TMP_ROOT_DIR, base); + } + if (st.st_uid != 0 || st.st_gid != 0 || st.st_mode != (S_IFDIR | 0700)) { + die("%s/%s has unexpected ownership / permissions (%u, %u, %ou)", + SNAP_PRIVATE_TMP_ROOT_DIR, base, st.st_uid, st.st_gid, st.st_mode); + } + // we will create a 1777 /tmp inside our private dir mode_t old_mask = umask(0); - char *d = sc_strdup(tmpdir); - sc_must_snprintf(tmpdir, sizeof(tmpdir), "%s/tmp", d); - free(d); - - // Create /tmp/snap.$SNAP_NAME/tmp 01777 root.root Ignore EEXIST since we + // Create /tmp/$PRIVATE/snap.$SNAP_NAME/tmp 0777 root.root Ignore EEXIST since we // want to reuse and we will open with O_NOFOLLOW, below. - if (mkdirat(base_dir_fd, "tmp", 01777) < 0 && errno != EEXIST) { - die("cannot create private tmp directory %s/tmp", tmpdir); + if (mkdirat(base_dir_fd, "tmp", 0777) < 0 && errno != EEXIST) { + die("cannot create private tmp directory %s/%s/tmp", + SNAP_PRIVATE_TMP_ROOT_DIR, base); } umask(old_mask); - // chdir to '/' since the mount won't apply to the current directory - char *pwd = get_current_dir_name(); - if (pwd == NULL) - die("cannot get current working directory"); - if (chdir("/") != 0) - die("cannot change directory to '/'"); + tmp_dir_fd = openat(base_dir_fd, "tmp", + O_RDONLY | O_DIRECTORY | O_CLOEXEC | O_NOFOLLOW); + if (tmp_dir_fd < 0) { + die("cannot open private tmp directory %s/tmp", base); + } + // set sticky bit on tmp dir after creating it to ensure it still gets + // created as root:root + if (fchmod(tmp_dir_fd, 01777) < 0) { + die("cannot set sticky bit on private tmp directory %s/%s/tmp", + SNAP_PRIVATE_TMP_ROOT_DIR, base); + } + if (fstat(tmp_dir_fd, &st) < 0) { + die("cannot stat %s/%s/tmp", SNAP_PRIVATE_TMP_ROOT_DIR, base); + } + if (st.st_uid != 0 || st.st_gid != 0 || st.st_mode != (S_IFDIR | 01777)) { + die("%s/%s/tmp has unexpected ownership / permissions (%u, %u, %ou)", + SNAP_PRIVATE_TMP_ROOT_DIR, base, st.st_uid, st.st_gid, st.st_mode); + } + // use the path to the file-descriptor in proc as the source mount point + // as this is a symlink itself to the real directory at + // /tmp/snap-private-tmp/snap.$SNAP_INSTANCE/tmp but doing it this way + // helps avoid any potential race + sc_must_snprintf(tmp_dir, sizeof(tmp_dir), + "/proc/self/fd/%d", tmp_dir_fd); // MS_BIND is there from linux 2.4 - sc_do_mount(tmpdir, "/tmp", NULL, MS_BIND, NULL); + sc_do_mount(tmp_dir, "/tmp", NULL, MS_BIND, NULL); // MS_PRIVATE needs linux > 2.6.11 sc_do_mount("none", "/tmp", NULL, MS_PRIVATE, NULL); - // do the chown after the bind mount to avoid potential shenanigans - if (chown("/tmp/", uid, gid) < 0) { - die("cannot change ownership of /tmp"); - } - // chdir to original directory - if (chdir(pwd) != 0) - die("cannot change current working directory to the original directory"); - free(pwd); + /* // restore privileges */ + if (setgid(gid) < 0) { + die("cannot restore group to %d", gid); + } } // TODO: fold this into bootstrap @@ -699,7 +667,7 @@ void sc_populate_mount_ns(struct sc_apparmor *apparmor, int snap_update_ns_fd, // set up private mounts // TODO: rename this and fold it into bootstrap - setup_private_mount(snap_name); + setup_private_tmp(snap_name); // set up private /dev/pts // TODO: fold this into bootstrap diff --git a/cmd/snap-confine/snap-confine.apparmor.in b/cmd/snap-confine/snap-confine.apparmor.in index a0940f42..148c17ce 100644 --- a/cmd/snap-confine/snap-confine.apparmor.in +++ b/cmd/snap-confine/snap-confine.apparmor.in @@ -127,6 +127,7 @@ mount options=(rw rshared) -> /snap/, # boostrapping the mount namespace + /tmp/snap.rootfs_*/ rw, mount options=(rw rshared) -> /, mount options=(rw bind) /tmp/snap.rootfs_*/ -> /tmp/snap.rootfs_*/, mount options=(rw unbindable) -> /tmp/snap.rootfs_*/, @@ -241,10 +242,11 @@ # set up snap-specific private /tmp dir capability chown, /tmp/ rw, - /tmp/snap.*/ w, - /tmp/snap.*/tmp/ w, + /tmp/snap-private-tmp/ rw, + /tmp/snap-private-tmp/snap.*/ rw, + /tmp/snap-private-tmp/snap.*/tmp/ rw, mount options=(rw private) -> /tmp/, - mount options=(rw bind) /tmp/snap.*/tmp/ -> /tmp/, + mount options=(rw bind) /tmp/snap-private-tmp/snap.*/tmp/ -> /tmp/, mount fstype=devpts options=(rw) devpts -> /dev/pts/, mount options=(rw bind) /dev/pts/ptmx -> /dev/ptmx, # for bind mounting mount options=(rw bind) /dev/pts/ptmx -> /dev/pts/ptmx, # for bind mounting under LXD