From f73b7f92e8cf5b93ab52760be43b2d3245c8807e Mon Sep 17 00:00:00 2001 From: Alex Murray Date: Mon, 19 Sep 2022 13:50:36 +0930 Subject: [PATCH] [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: Name 0018-cve-2022-3328-2.patch --- cmd/snap-confine/mount-support-test.c | 40 ----- cmd/snap-confine/mount-support.c | 166 +++++++----------- cmd/snap-confine/snap-confine.apparmor.in | 8 +- cmd/snap-update-ns/system.go | 6 +- cmd/snap-update-ns/system_test.go | 8 +- interfaces/builtin/x11.go | 6 +- interfaces/builtin/x11_test.go | 4 +- tests/lib/reset.sh | 4 +- tests/main/cgroup-tracking/task.yaml | 6 +- .../main/interfaces-x11-unix-socket/task.yaml | 2 +- tests/main/security-private-tmp/task.yaml | 2 +- tests/main/snap-confine-tmp-mount/task.yaml | 37 +--- .../task.yaml | 6 +- 13 files changed, 98 insertions(+), 197 deletions(-) diff --git a/cmd/snap-confine/mount-support-test.c b/cmd/snap-confine/mount-support-test.c index 9f2d78af..0580d433 100644 --- a/cmd/snap-confine/mount-support-test.c +++ b/cmd/snap-confine/mount-support-test.c @@ -92,50 +92,10 @@ static void test_is_subdir(void) g_assert_false(is_subdir("/", "")); } -static void test_must_mkdir_and_open_with_perms(void) -{ - // make a directory with some contents and check we can - // must_mkdir_and_open_with_perms() to get control of it - GError *error = NULL; - GStatBuf st; - gchar *test_dir = g_dir_make_tmp("test-mkdir-XXXXXX", &error); - g_assert_no_error(error); - g_assert_nonnull(test_dir); - g_assert_cmpint(chmod(test_dir, 0755), ==, 0); - g_assert_true(g_file_test - (test_dir, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_DIR)); - g_assert_cmpint(g_stat(test_dir, &st), ==, 0); - g_assert_true(st.st_uid == getuid()); - g_assert_true(st.st_gid == getgid()); - g_assert_true(st.st_mode == (S_IFDIR | 0755)); - - gchar *test_subdir = g_build_filename(test_dir, "foo", NULL); - g_assert_cmpint(g_mkdir_with_parents(test_dir, 0755), ==, 0); - g_file_test(test_subdir, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_DIR); - - // take over dir - int fd = - must_mkdir_and_open_with_perms(test_dir, getuid(), getgid(), 0700); - // check can unlink dir itself with no contents successfully and it - // still exists - g_assert_cmpint(fd, >=, 0); - g_assert_false(g_file_test - (test_subdir, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_DIR)); - g_assert_true(g_file_test - (test_dir, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_DIR)); - g_assert_cmpint(g_stat(test_dir, &st), ==, 0); - g_assert_true(st.st_uid == getuid()); - g_assert_true(st.st_gid == getgid()); - g_assert_true(st.st_mode == (S_IFDIR | 0700)); - close(fd); -} - static void __attribute__((constructor)) init(void) { g_test_add_func("/mount/get_nextpath/typical", test_get_nextpath__typical); g_test_add_func("/mount/get_nextpath/weird", test_get_nextpath__weird); g_test_add_func("/mount/is_subdir", test_is_subdir); - g_test_add_func("/mount/must_mkdir_and_open_with_perms", - test_must_mkdir_and_open_with_perms); } diff --git a/cmd/snap-confine/mount-support.c b/cmd/snap-confine/mount-support.c index d6ec6746..3265e4a6 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 @@ -49,97 +50,13 @@ #include "mount-support-nvidia.h" #define MAX_BUF 1000 +#define SNAP_PRIVATE_TMP_ROOT_DIR "/tmp/snap-private-tmp" static void sc_detach_views_of_writable(sc_distro distro, bool normal_mode); -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) { // Create a 0700 base directory. This is the "base" directory that is // protected from other users. This directory name is NOT randomly @@ -162,37 +79,80 @@ static void setup_private_mount(const char *snap_name) // Because the directories are reused across invocations by distinct users // and because the directories are trivially guessable, each invocation // unconditionally chowns/chmods them to appropriate values. - char base_dir[MAX_BUF] = { 0 }; + 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; - sc_must_snprintf(base_dir, sizeof(base_dir), "/tmp/snap.%s", snap_name); - sc_must_snprintf(tmp_dir, sizeof(tmp_dir), "%s/tmp", base_dir); - /* 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. */ + /* 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. */ sc_identity old = sc_set_effective_identity(sc_root_group_identity()); - // Create /tmp/snap.$SNAP_NAME/ 0700 root.root. - base_dir_fd = must_mkdir_and_open_with_perms(base_dir, 0, 0, 0700); - // Create /tmp/snap.$SNAP_NAME/tmp 01777 root.root Ignore EEXIST since we + + // /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", + SNAP_PRIVATE_TMP_ROOT_DIR, base); + } + // Create /tmp/$PRIVATE/snap.$SNAP_NAME/tmp 01777 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", base_dir); + die("cannot create private tmp directory %s/tmp", base); } (void)sc_set_effective_identity(old); 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_dir); + die("cannot open private tmp directory %s/tmp", base); } - if (fchown(tmp_dir_fd, 0, 0) < 0) { - die("cannot chown private tmp directory %s/tmp to root.root", - base_dir); + if (fstat(tmp_dir_fd, &st) < 0) { + die("cannot stat %s/%s/tmp", SNAP_PRIVATE_TMP_ROOT_DIR, base); } - if (fchmod(tmp_dir_fd, 01777) < 0) { - die("cannot chmod private tmp directory %s/tmp to 01777", - base_dir); + if (st.st_uid != 0 || st.st_gid != 0 || st.st_mode != (S_IFDIR | 01777)) { + die("%s/%s/tmp has unexpected ownership / permissions", + SNAP_PRIVATE_TMP_ROOT_DIR, base); } + // 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); sc_do_mount(tmp_dir, "/tmp", NULL, MS_BIND, NULL); sc_do_mount("none", "/tmp", NULL, MS_PRIVATE, NULL); } @@ -765,7 +725,7 @@ void sc_populate_mount_ns(struct sc_apparmor *apparmor, int snap_update_ns_fd, } // TODO: rename this and fold it into bootstrap - setup_private_mount(inv->snap_instance); + setup_private_tmp(inv->snap_instance); // set up private /dev/pts // TODO: fold this into bootstrap setup_private_pts(); diff --git a/cmd/snap-confine/snap-confine.apparmor.in b/cmd/snap-confine/snap-confine.apparmor.in index e9c224ee..0536bc2b 100644 --- a/cmd/snap-confine/snap-confine.apparmor.in +++ b/cmd/snap-confine/snap-confine.apparmor.in @@ -142,6 +142,7 @@ mount options=(rw rshared) -> /var/lib/snapd/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_*/, @@ -290,10 +291,11 @@ # set up snap-specific private /tmp dir capability chown, /tmp/ rw, - /tmp/snap.*/ rw, - /tmp/snap.*/tmp/ rw, + /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 diff --git a/cmd/snap-update-ns/system.go b/cmd/snap-update-ns/system.go index 73b1a323..83b6f305 100644 --- a/cmd/snap-update-ns/system.go +++ b/cmd/snap-update-ns/system.go @@ -79,12 +79,12 @@ func (upCtx *SystemProfileUpdateContext) Assumptions() *Assumptions { // the slot-side snap, as there is no mechanism to convey this information. // As such, provide write access to all of /tmp. as.AddUnrestrictedPaths("/var/lib/snapd/hostfs/tmp") - as.AddModeHint("/var/lib/snapd/hostfs/tmp/snap.*", 0700) - as.AddModeHint("/var/lib/snapd/hostfs/tmp/snap.*/tmp", 1777) + as.AddModeHint("/var/lib/snapd/hostfs/tmp/snap-private-tmp/snap.*", 0700) + as.AddModeHint("/var/lib/snapd/hostfs/tmp/snap-private-tmp/snap.*/tmp", 1777) // This is to ensure that unprivileged users can create the socket. This // permission only matters if the plug-side app constructs its mount // namespace before the slot-side app is launched. - as.AddModeHint("/var/lib/snapd/hostfs/tmp/snap.*/tmp/.X11-unix", 1777) + as.AddModeHint("/var/lib/snapd/hostfs/tmp/snap-private-tmp/snap.*/tmp/.X11-unix", 1777) return as } diff --git a/cmd/snap-update-ns/system_test.go b/cmd/snap-update-ns/system_test.go index bb8b93ac..8bb92c51 100644 --- a/cmd/snap-update-ns/system_test.go +++ b/cmd/snap-update-ns/system_test.go @@ -56,10 +56,10 @@ func (s *systemSuite) TestAssumptions(c *C) { c.Check(as.ModeForPath("/stuff"), Equals, os.FileMode(0755)) c.Check(as.ModeForPath("/tmp"), Equals, os.FileMode(0755)) c.Check(as.ModeForPath("/var/lib/snapd/hostfs/tmp"), Equals, os.FileMode(0755)) - c.Check(as.ModeForPath("/var/lib/snapd/hostfs/tmp/snap.x11-server"), Equals, os.FileMode(0700)) - c.Check(as.ModeForPath("/var/lib/snapd/hostfs/tmp/snap.x11-server/tmp"), Equals, os.FileMode(1777)) - c.Check(as.ModeForPath("/var/lib/snapd/hostfs/tmp/snap.x11-server/foo"), Equals, os.FileMode(0755)) - c.Check(as.ModeForPath("/var/lib/snapd/hostfs/tmp/snap.x11-server/tmp/.X11-unix"), Equals, os.FileMode(1777)) + c.Check(as.ModeForPath("/var/lib/snapd/hostfs/tmp/snap-private-tmp/snap.x11-server"), Equals, os.FileMode(0700)) + c.Check(as.ModeForPath("/var/lib/snapd/hostfs/tmp/snap-private-tmp/snap.x11-server/tmp"), Equals, os.FileMode(1777)) + c.Check(as.ModeForPath("/var/lib/snapd/hostfs/tmp/snap-private-tmp/snap.x11-server/foo"), Equals, os.FileMode(0755)) + c.Check(as.ModeForPath("/var/lib/snapd/hostfs/tmp/snap-private-tmp/snap.x11-server/tmp/.X11-unix"), Equals, os.FileMode(1777)) // Instances can, in addition, access /snap/$SNAP_INSTANCE_NAME upCtx = update.NewSystemProfileUpdateContext("foo_instance", false) diff --git a/interfaces/builtin/x11.go b/interfaces/builtin/x11.go index 4a9314fe..5f46a3b8 100644 --- a/interfaces/builtin/x11.go +++ b/interfaces/builtin/x11.go @@ -187,7 +187,7 @@ func (iface *x11Interface) MountConnectedPlug(spec *mount.Specification, plug *i } slotSnapName := slot.Snap().InstanceName() return spec.AddMountEntry(osutil.MountEntry{ - Name: fmt.Sprintf("/var/lib/snapd/hostfs/tmp/snap.%s/tmp/.X11-unix", slotSnapName), + Name: fmt.Sprintf("/var/lib/snapd/hostfs/tmp/snap-private-tmp/snap.%s/tmp/.X11-unix", slotSnapName), Dir: "/tmp/.X11-unix", Options: []string{"bind", "ro"}, }) @@ -214,8 +214,8 @@ func (iface *x11Interface) AppArmorConnectedPlug(spec *apparmor.Specification, p slotSnapName := slot.Snap().InstanceName() spec.AddUpdateNS(fmt.Sprintf(` /tmp/.X11-unix/ rw, - /var/lib/snapd/hostfs/tmp/snap.%s/tmp/.X11-unix/ rw, - mount options=(rw, bind) /var/lib/snapd/hostfs/tmp/snap.%s/tmp/.X11-unix/ -> /tmp/.X11-unix/, + /var/lib/snapd/hostfs/tmp/snap-private-tmp/snap.%s/tmp/.X11-unix/ rw, + mount options=(rw, bind) /var/lib/snapd/hostfs/tmp/snap-private-tmp/snap.%s/tmp/.X11-unix/ -> /tmp/.X11-unix/, mount options=(ro, remount, bind) -> /tmp/.X11-unix/, mount options=(rslave) -> /tmp/.X11-unix/, umount /tmp/.X11-unix/, diff --git a/interfaces/builtin/x11_test.go b/interfaces/builtin/x11_test.go index 452a6866..f62bb641 100644 --- a/interfaces/builtin/x11_test.go +++ b/interfaces/builtin/x11_test.go @@ -119,7 +119,7 @@ func (s *X11InterfaceSuite) TestMountSpec(c *C) { spec = &mount.Specification{} c.Assert(spec.AddConnectedPlug(s.iface, s.plug, s.coreSlot), IsNil) c.Assert(spec.MountEntries(), DeepEquals, []osutil.MountEntry{{ - Name: "/var/lib/snapd/hostfs/tmp/snap.x11/tmp/.X11-unix", + Name: "/var/lib/snapd/hostfs/tmp/snap-private-tmp/snap.x11/tmp/.X11-unix", Dir: "/tmp/.X11-unix", Options: []string{"bind", "ro"}, }}) @@ -155,7 +155,7 @@ func (s *X11InterfaceSuite) TestAppArmorSpec(c *C) { c.Assert(spec.SecurityTags(), DeepEquals, []string{"snap.consumer.app"}) c.Assert(spec.SnippetForTag("snap.consumer.app"), testutil.Contains, "fontconfig") c.Assert(spec.UpdateNS(), HasLen, 1) - c.Assert(spec.UpdateNS()[0], testutil.Contains, `mount options=(rw, bind) /var/lib/snapd/hostfs/tmp/snap.x11/tmp/.X11-unix/ -> /tmp/.X11-unix/,`) + c.Assert(spec.UpdateNS()[0], testutil.Contains, `mount options=(rw, bind) /var/lib/snapd/hostfs/tmp/snap-private-tmp/snap.x11/tmp/.X11-unix/ -> /tmp/.X11-unix/,`) // Slot side connection permissions spec = &apparmor.Specification{} diff --git a/tests/lib/reset.sh b/tests/lib/reset.sh index 440afd78..e3b05bbe 100755 --- a/tests/lib/reset.sh +++ b/tests/lib/reset.sh @@ -46,7 +46,7 @@ reset_classic() { ls -lR "$SNAP_MOUNT_DIR"/ /var/snap/ exit 1 fi - rm -rf /tmp/snap.* + rm -rf /tmp/snap-private-tmp case "$SPREAD_SYSTEM" in fedora-*|centos-*) @@ -158,7 +158,7 @@ reset_all_snap() { systemctl stop snapd.service snapd.socket restore_snapd_state rm -rf /root/.snap - rm -rf /tmp/snap.* + rm -rf /tmp/snap-private-tmp/snap.* if [ "$1" != "--keep-stopped" ]; then systemctl start snapd.service snapd.socket fi diff --git a/tests/main/cgroup-tracking/task.yaml b/tests/main/cgroup-tracking/task.yaml index d9d18594..afb0a436 100644 --- a/tests/main/cgroup-tracking/task.yaml +++ b/tests/main/cgroup-tracking/task.yaml @@ -116,7 +116,7 @@ execute: | trap "pkill sleep || true" EXIT echo "Ensure that snap-confine has finished its task and that the snap process" echo "is active. Note that we don't want to wait forever either." - retry -n 30 test -e /tmp/snap.test-snapd-tracking/tmp/1.stamp + retry -n 30 test -e /tmp/snap-private-tmp/snap.test-snapd-tracking/tmp/1.stamp pid1_sleep=$(cat /tmp/1.pid) echo "During startup snap-run has asked systemd to move the process to a" @@ -135,7 +135,7 @@ execute: | #shellcheck disable=SC2016 tests.session -p /tmp/2.pid -u "$USER" exec snap run test-snapd-tracking.sh -c 'touch /tmp/2.stamp && exec sleep 2m' & session2_pid=$! - retry -n 30 test -e /tmp/snap.test-snapd-tracking/tmp/2.stamp + retry -n 30 test -e /tmp/snap-private-tmp/snap.test-snapd-tracking/tmp/2.stamp pid2_sleep=$(cat /tmp/2.pid) pid2_tracking_cg_path="$(grep -E "^$base_cg_id:" < "/proc/$pid2_sleep/cgroup" | cut -d : -f 3)" echo "$pid2_tracking_cg_path" | MATCH '.*/snap\.test-snapd-tracking\.sh\.[0-9a-f-]+\.scope' @@ -158,7 +158,7 @@ execute: | #shellcheck disable=SC2016 tests.session -p /tmp/3.pid -u "$USER" exec snap run test-snapd-tracking.sh -c 'touch /tmp/3.stamp && sleep 1m' & session3_pid=$! - retry -n 30 test -e /tmp/snap.test-snapd-tracking/tmp/3.stamp + retry -n 30 test -e /tmp/snap-private-tmp/snap.test-snapd-tracking/tmp/3.stamp pid3_sh=$(cat /tmp/3.pid) pid3_tracking_cg_path="$(grep -E "^$base_cg_id:" < "/proc/$pid3_sh/cgroup" | cut -d : -f 3)" MATCH "$pid3_sh" < "${base_cg_path}${pid3_tracking_cg_path}/cgroup.procs" diff --git a/tests/main/interfaces-x11-unix-socket/task.yaml b/tests/main/interfaces-x11-unix-socket/task.yaml index 01395540..e397db13 100644 --- a/tests/main/interfaces-x11-unix-socket/task.yaml +++ b/tests/main/interfaces-x11-unix-socket/task.yaml @@ -25,7 +25,7 @@ execute: | echo "The snaps can communicate via the unix domain socket in /tmp" x11-server & - retry -n 4 --wait 0.5 test -e /tmp/snap.x11-server/tmp/.X11-unix/X0 + retry -n 4 --wait 0.5 test -e /tmp/snap-private-tmp/snap.x11-server/tmp/.X11-unix/X0 x11-client | MATCH "Hello from xserver" echo "The client cannot remove the unix domain sockets shared with it" diff --git a/tests/main/security-private-tmp/task.yaml b/tests/main/security-private-tmp/task.yaml index a9361f0b..7d87dfbd 100644 --- a/tests/main/security-private-tmp/task.yaml +++ b/tests/main/security-private-tmp/task.yaml @@ -18,7 +18,7 @@ prepare: | snap install --dangerous not-test-snapd-sh_1.0_all.snap restore: | - rm -rf "$SNAP_INSTALL_DIR" /tmp/foo /tmp/snap.not-test-snapd-sh /tmp/snap.test-snapd-sh/ + rm -rf "$SNAP_INSTALL_DIR" /tmp/foo /tmp/snap-private-tmp/snap.not-test-snapd-sh /tmp/snap-private-tmp/snap.test-snapd-sh/ execute: | #shellcheck source=tests/lib/dirs.sh diff --git a/tests/main/snap-confine-tmp-mount/task.yaml b/tests/main/snap-confine-tmp-mount/task.yaml index 97d51016..6ca421f6 100644 --- a/tests/main/snap-confine-tmp-mount/task.yaml +++ b/tests/main/snap-confine-tmp-mount/task.yaml @@ -1,10 +1,8 @@ -summary: ensure snap-confine controls private mount namespace +summary: ensure snap-confine correctly setups up the private tmp mount namespace details: | - Ensure that when creating the private mount namespace for a snap that - if it already exists but is not owned by root then any existing - contents within the private mount directory is first removed before the - mount is created. + Ensure that when creating the private tmp mount namespace for a snap that it + is done so under /tmp/snap-private-tmp/snap.SNAP_NAME which is owned by root # ubuntu-14.04: the test sets up a user session, which requires more recent systemd systems: [-ubuntu-14.04-*] @@ -19,23 +17,6 @@ restore: | tests.session -u test restore execute: | - rm -rf /tmp/snap.test-snapd-sh - # create /tmp/snap.test-snapd-sh as a regular user - tests.session -u test exec sh -c "mkdir /tmp/snap.test-snapd-sh" - test_umask=$(tests.session -u test exec sh -c "umask") - # check permissions are as expected - expected=$(printf "%o" $((0777-test_umask))) - stat -c "%U %G %a" /tmp/snap.test-snapd-sh | MATCH "test test $expected" - # and place other contents there - tests.session -u test exec sh -c "mkdir /tmp/snap.test-snapd-sh/tmp" - tests.session -u test exec sh -c "touch /tmp/snap.test-snapd-sh/tmp/foo" - stat -c "%U %G %a" /tmp/snap.test-snapd-sh/tmp | MATCH "test test $expected" - expected=$(printf "%o" $((0666-test_umask))) - stat -c "%U %G %a" /tmp/snap.test-snapd-sh/tmp/foo | MATCH "test test $expected" - - # then execute snap-confine - this should take over our imposter base - # dir but execute id successfully - snap-confine outputs to stderr and - # id will output to stdout so capture each separately SNAP_CONFINE=$(os.paths libexec-dir)/snapd/snap-confine if os.query is-core; then # on Ubuntu Core we need to use the correct path to ensure it is @@ -43,17 +24,15 @@ execute: | # snap SNAP_CONFINE=$(aa-status | grep "snap-confine$") fi + # execute snap-confine as a standard user - this should create a private tmp + # dir as /tmp/snap-private-tmp/snap.test-snapd-sh/ - snap-confine outputs to + # stderr and id will output to stdout so capture each separately tests.session -u test exec sh -c "env -i SNAPD_DEBUG=1 SNAP_INSTANCE_NAME=test-snapd-sh $SNAP_CONFINE --base core snap.test-snapd-sh.sh /bin/bash -c id 1>/tmp/snap-confine-stdout.log 2>/tmp/snap-confine-stderr.log" tests.cleanup defer rm -f /tmp/snap-confine-stdout.log /tmp/snap-confine-stderr.log - stat -c "%U %G %a" /tmp/snap.test-snapd-sh | MATCH "root root 700" + stat -c "%U %G %a" /tmp/snap-private-tmp/snap.test-snapd-sh | MATCH "root root 700" - # contents should have been removed and tmp dir recreated with root - # ownership but foo file should have been removed - stat -c "%U %G %a" /tmp/snap.test-snapd-sh/tmp | MATCH "root root 1777" - [ -f /tmp/snap.test-snapd-sh/tmp/foo ] && exit 1 - # actual dir should be owned by root now - stat -c "%U %G %a" /tmp/snap.test-snapd-sh | MATCH "root root 700" + stat -c "%U %G %a" /tmp/snap-private-tmp/snap.test-snapd-sh/tmp | MATCH "root root 1777" # and snap-confine should ensure the target binary is executed as the test user MATCH "uid=12345\(test\) gid=12345\(test\)" /tmp/snap-confine-stdout.log diff --git a/tests/main/snap-confine-undesired-mode-group/task.yaml b/tests/main/snap-confine-undesired-mode-group/task.yaml index d4e94634..2c69d752 100644 --- a/tests/main/snap-confine-undesired-mode-group/task.yaml +++ b/tests/main/snap-confine-undesired-mode-group/task.yaml @@ -16,7 +16,7 @@ execute: | # trees. Such files may indicate that parts of code invomed from # snap-confine (which includes snap-update-ns and snap-discard-ns) ran as # the group of the calling user and did not manage that properly. - for dname in /run/snapd /sys/fs/cgroup /tmp/snap.*; do + for dname in /run/snapd /sys/fs/cgroup /tmp/snap-private-tmp/snap.*; do # Filter out cgroups that are expected to be owned by the test user. # Since we are a looking at sysfs, which is modified asynchronously, # ignore errors of the kind where readdir and stat race with a @@ -28,7 +28,7 @@ execute: | # - symbolic links, those are always 777 # - the file cgroup.event_control which is ugo+w for some reason # - the per-snap tmp directory as it is meant to be world-writable - find "$dname" -ignore_readdir_race ! -type s ! -type l ! -name cgroup.event_control ! -path '/tmp/snap.*/tmp' -perm /o+w >> world-writable.txt + find "$dname" -ignore_readdir_race ! -type s ! -type l ! -name cgroup.event_control ! -path '/tmp/snap-private-tmp/snap.*/tmp' -perm /o+w >> world-writable.txt done # The test fails if any such file is detected @@ -53,4 +53,4 @@ restore: | snap remove test-snapd-app rm -f test-snapd-app_1.0_all.snap rm -f wrong-*.txt - rm -rf /tmp/snap.test-snapd-app + rm -rf /tmp/snap-private-tmp/snap.test-snapd-app -- 2.30.2