Fix creation of $XDG_RUNTIME_DIR and $SNAP_USER_DATA

spread-tests/main/user-xdg-runtime-dir-created/task.yaml

@@ -2,20 +2,39 @@ summary: Ensure that XDG_RUNTIME_DIR directory is created by snap-confine
 # This is blacklisted on debian because debian doesn't use apparmor yet
 systems: [-debian-8]
 details: |
-    This test checks that XDG_RUNTIME_DIR is actually created
+    This test checks that XDG_RUNTIME_DIR is actually created and is owned by
+    the correct user.
 prepare: |
     echo "Having installed the snapd-hacker-toolbelt snap"
     snap install snapd-hacker-toolbelt
-    echo "Having removed the XDG_RUNTIME_DIR directory"
-    rm -rf "/run/user/`id -u`/snapd-hacker-toolbelt"
+    echo "And having created a test user with a home directory"
+    useradd test
+    mkdir /home/test
+    chown test.test /home/test
 execute: |
-    cd /
-    echo "FIXME: export XDG_RUNTIME_DIR for now until snapd does it"
-    export XDG_RUNTIME_DIR="/run/user/`id -u`/snapd-hacker-toolbelt"
-    echo "We can now run snapd-hacker-toolbelt.busybox true"
-    /snap/bin/snapd-hacker-toolbelt.busybox true
+    echo "We can set a per-snap XDG_RUNTIME_DIR (until snapd does this)"
+    export XDG_RUNTIME_DIR="/run/user/$(id -u test)/snap.snapd-hacker-toolbelt"
+    echo "We can now run snapd-hacker-toolbelt.busybox true as the test user"
+    su -l -c "XDG_RUNTIME_DIR=$XDG_RUNTIME_DIR /snap/bin/snapd-hacker-toolbelt.busybox true" test
     echo "And see that the XDG_RUNTIME_DIR directory was created"
-    test -d /run/user/`id -u`/snapd-hacker-toolbelt
+    [ -d "$XDG_RUNTIME_DIR" ]
+    echo "And is owned by the right user and group"
+    [ "$(stat -c '%U.%G' /)" = root.root ]
+    [ "$(stat -c '%U.%G' /run)" = root.root ]
+    [ "$(stat -c '%U.%G' /run/user)" = root.root ]
+    [ "$(stat -c '%U.%G' /run/user/$(id -u test))" = test.test ]
+    [ "$(stat -c '%U.%G' /run/user/$(id -u test)/snap.snapd-hacker-toolbelt)" = test.test ]
+    echo "And has the right permissions"
+    [ "$(stat -c '%a' /)" = 755 ]
+    [ "$(stat -c '%a' /run)" = 755 ]
+    [ "$(stat -c '%a' /run/user)" = 755 ]
+    [ "$(stat -c '%a' /run/user/$(id -u test))" = 700 ]
+    [ "$(stat -c '%a' /run/user/$(id -u test)/snap.snapd-hacker-toolbelt)" = 755 ]
+    # Sanity check when we remove the XDG_RUNTIME_DIR override above
+    [ "$(stat -c '%a' $XDG_RUNTIME_DIR)" = 755 ]
+    [ "$(stat -c '%U.%G' $XDG_RUNTIME_DIR)" = test.test ]
 restore: |
+    rm -rf "/home/test"
+    rm -rf "/run/user/$(id -u test)"
+    userdel test
     snap remove snapd-hacker-toolbelt
-    rm -rf "/run/user/`id -u`/snapd-hacker-toolbelt"

src/ns-support.c

@@ -175,7 +175,11 @@ static bool sc_is_ns_group_dir_private()
 void sc_initialize_ns_groups()
 {
 	debug("creating namespace group directory %s", sc_ns_dir);
-	if (sc_nonfatal_mkpath(sc_ns_dir, 0755) < 0) {
+	struct sc_mkpath_opts opts = {
+		.mode = 0755,
+		.do_chown = false,
+	};
+	if (sc_nonfatal_mkpath(sc_ns_dir, &opts) < 0) {
 		die("cannot create namespace group directory %s", sc_ns_dir);
 	}
 	debug("opening namespace group directory %s", sc_ns_dir);

src/quirks.c

@@ -85,8 +85,12 @@ static void sc_quirk_mkdir_bind(const char *src_dir, const char *dest_dir,
 				unsigned flags)
 {
 	flags |= MS_BIND;
+	struct sc_mkpath_opts opts = {
+		.mode = 0755,
+		.do_chown = false,
+	};
 	debug("creating empty directory at %s", dest_dir);
-	if (sc_nonfatal_mkpath(dest_dir, 0755) < 0) {
+	if (sc_nonfatal_mkpath(dest_dir, &opts) < 0) {
 		die("cannot create empty directory at %s", dest_dir);
 	}
 	const char *flags_str = sc_mount_opt2str(flags);

src/snap-confine.apparmor.in

@@ -220,7 +220,17 @@
 
     # for creating the user XDG_RUNTIME_DIR: /run/user, /run/user/UID and
     # /run/user/UID/<name>
-    /run/user/{,[0-9]*/,[0-9]*/*/} rw,
+    / r,
+    /run r,
+    /run/user/ rw,
+    /run/user/[0-9]*/ rw,
+    /run/user/[0-9]*/*/ rw,
+    # for creating $SNAP_USER_DATA
+    / r,
+    @{HOME}/ r,
+    @{HOME}/snap/ rw,
+    @{HOME}/snap/*/ rw,
+    @{HOME}/snap/*/*/ rw,
 
     # Workaround https://launchpad.net/bugs/359338 until upstream handles
     # stacked filesystems generally.

src/snap-confine.c

@@ -113,6 +113,11 @@ int main(int argc, char **argv)
 			setup_devices_cgroup(security_tag, &udev_s);
 		snappy_udev_cleanup(&udev_s);
 
+		// Setup the XDG_RUNTIME_DIR as root (we need to write to /run here)
+		setup_user_xdg_runtime_dir(real_uid, real_gid);
+		// Ensure that the user data path exists.
+		setup_user_data(real_uid, real_gid);
+
 		// The rest does not so temporarily drop privs back to calling
 		// user (we'll permanently drop after loading seccomp)
 		if (setegid(real_gid) != 0)
@@ -125,10 +130,6 @@ int main(int argc, char **argv)
 		if (real_uid != 0 && getegid() == 0)
 			die("dropping privs did not work");
 	}
-	// Ensure that the user data path exists.
-	setup_user_data();
-	setup_user_xdg_runtime_dir();
-
 	// https://wiki.ubuntu.com/SecurityTeam/Specifications/SnappyConfinement
 #ifdef HAVE_APPARMOR
 	int rc = 0;

src/user-support.c

@@ -17,49 +17,120 @@
 #include "config.h"
 #include "user-support.h"
 
-#include <errno.h>
 #include <stdlib.h>
 #include <sys/stat.h>
+#include <unistd.h>
 
 #include "utils.h"
 
-void setup_user_data()
+void setup_user_data(uid_t uid, gid_t gid)
 {
 	const char *user_data = getenv("SNAP_USER_DATA");
-
 	if (user_data == NULL)
 		return;
-
 	// Only support absolute paths.
 	if (user_data[0] != '/') {
 		die("user data directory must be an absolute path");
 	}
-
+	// Describe how the created directory should look like.
+	int hint_SNAP_USER_DATA(int depth, const char *segment,
+				mode_t * mode_p, uid_t * uid_p, gid_t * gid_p) {
+		switch (depth) {
+		case 0:
+			// That's /
+			break;
+		case 1:
+			// that's /home
+			break;
+		case 2:
+			/// that's /home/$LOGNAME
+			break;
+		case 3:
+			// that's /home/$LOGNAME/snap
+			// NOTE: fall-through
+		case 4:
+			// that's /home/$LOGNAME/snap/$SNAP_NAME
+			// NOTE: fall-through
+		case 5:
+			// that's /home/$LOGNAME/snap/$SNAP_NAME/$SNAP_REVISION
+			*uid_p = uid;
+			*gid_p = gid;
+			*mode_p = 0755;
+			break;
+		default:
+			die("unexpected SNAP_USER_DATA segment: %s at depth %d",
+			    segment, depth);
+		}
+		return 0;
+	}
+	struct sc_mkpath_opts opts = {
+		.mode = 0755,
+		.uid = uid,
+		.gid = gid,
+		.hint_fn = hint_SNAP_USER_DATA,
+		.do_chown = true,
+	};
 	debug("creating user data directory: %s", user_data);
-	if (sc_nonfatal_mkpath(user_data, 0755) < 0) {
+	if (sc_nonfatal_mkpath(user_data, &opts) < 0) {
 		die("cannot create user data directory: %s", user_data);
 	};
 }
 
-void setup_user_xdg_runtime_dir()
+void setup_user_xdg_runtime_dir(uid_t uid, gid_t gid)
 {
+	debug("considering creating $XDG_RUNTIME_DIR for the snap session");
 	const char *xdg_runtime_dir = getenv("XDG_RUNTIME_DIR");
-
-	if (xdg_runtime_dir == NULL)
+	debug("XDG_RUNTIME_DIR is %s", xdg_runtime_dir);
+	if (xdg_runtime_dir == NULL) {
+		debug("XDG_RUNTIME_DIR is not set");
 		return;
+	}
 	// Only support absolute paths.
 	if (xdg_runtime_dir[0] != '/') {
 		die("XDG_RUNTIME_DIR must be an absolute path");
 	}
 
-	errno = 0;
+	int hint_XDG_RUNTIME_DIR(int depth, const char *segment,
+				 mode_t * mode_p, uid_t * uid_p,
+				 gid_t * gid_p) {
+		switch (depth) {
+		case 0:
+			// That's /
+			break;
+		case 1:
+			// That's /run
+			*uid_p = 0;
+			*gid_p = 0;
+			break;
+		case 2:
+			// That's /run/user
+			break;
+		case 3:
+			// That's /run/user/user/$(id -u)
+			*uid_p = uid;
+			*gid_p = gid;
+			*mode_p = 0700;
+			break;
+		case 4:
+			// That's /run/user/$(id -u)/snap.$SNAP_NAME
+			*uid_p = uid;
+			*gid_p = gid;
+			break;
+		default:
+			die("unexpected XDR_RUNTIME_DIR segment: %s at depth %d", segment, depth);
+		}
+		return 0;
+	}
+	struct sc_mkpath_opts opts = {
+		.mode = 0755,
+		.uid = uid,
+		.gid = gid,
+		.hint_fn = hint_XDG_RUNTIME_DIR,
+		.do_chown = true,
+	};
 	debug("creating user XDG_RUNTIME_DIR directory: %s", xdg_runtime_dir);
-	if (sc_nonfatal_mkpath(xdg_runtime_dir, 0755) < 0) {
+	if (sc_nonfatal_mkpath(xdg_runtime_dir, &opts) < 0) {
 		die("cannot create user XDG_RUNTIME_DIR directory: %s",
 		    xdg_runtime_dir);
 	}
-	// if successfully created the directory (ie, not EEXIST), then chmod it.
-	if (errno == 0 && chmod(xdg_runtime_dir, 0700) != 0) {
-		die("cannot change permissions of user XDG_RUNTIME_DIR directory to 0700");
-	}
 }

src/user-support.h

@@ -18,8 +18,10 @@
 #ifndef SNAP_CONFINE_USER_SUPPORT_H
 #define SNAP_CONFINE_USER_SUPPORT_H
 
-void setup_user_data();
-void setup_user_xdg_runtime_dir();
+#include <sys/types.h>
+
+void setup_user_data(uid_t uid, gid_t gid);
+void setup_user_xdg_runtime_dir(uid_t uid, gid_t gid);
 void mkpath(const char *const path);
 
 #endif

src/utils-test.c

@@ -118,20 +118,24 @@ static void _test_sc_nonfatal_mkpath(const gchar * dirname,
 	// Use sc_nonfatal_mkpath to create the directory and ensure that it worked
 	// as expected.
 	g_test_queue_destroy((GDestroyNotify) rmdir, (char *)dirname);
-	int err = sc_nonfatal_mkpath(dirname, 0755);
+	struct sc_mkpath_opts opts = {
+		.mode = 0755,
+		.do_chown = false,
+	};
+	int err = sc_nonfatal_mkpath(dirname, &opts);
 	g_assert_cmpint(err, ==, 0);
 	g_assert_cmpint(errno, ==, 0);
 	g_assert_true(g_file_test(dirname, G_FILE_TEST_EXISTS |
 				  G_FILE_TEST_IS_REGULAR));
 	// Use same function again to try to create the same directory and ensure
 	// that it didn't fail and properly retained EEXIST in errno.
-	err = sc_nonfatal_mkpath(dirname, 0755);
+	err = sc_nonfatal_mkpath(dirname, &opts);
 	g_assert_cmpint(err, ==, 0);
 	g_assert_cmpint(errno, ==, EEXIST);
 	// Now create a sub-directory of the original directory and observe the
 	// results. We should no longer see errno of EEXIST!
 	g_test_queue_destroy((GDestroyNotify) rmdir, (char *)subdirname);
-	err = sc_nonfatal_mkpath(subdirname, 0755);
+	err = sc_nonfatal_mkpath(subdirname, &opts);
 	g_assert_cmpint(err, ==, 0);
 	g_assert_cmpint(errno, ==, 0);
 }