Merge remote-tracking branch 'upstream/master' into dont-kill-me

cmd/snap-confine/mount-support-nvidia.c

@@ -39,9 +39,10 @@
 // note: if the parent dir changes to something other than
 // the current /var/lib/snapd/lib then sc_mkdir_and_mount_and_bind
 // and sc_mkdir_and_mount_and_bind need updating.
-#define SC_LIBGL_DIR   "/var/lib/snapd/lib/gl"
-#define SC_LIBGL32_DIR "/var/lib/snapd/lib/gl32"
-#define SC_VULKAN_DIR  "/var/lib/snapd/lib/vulkan"
+#define SC_LIB "/var/lib/snapd/lib"
+#define SC_LIBGL_DIR   SC_LIB "/gl"
+#define SC_LIBGL32_DIR SC_LIB "/gl32"
+#define SC_VULKAN_DIR  SC_LIB "/vulkan"
 
 #define SC_VULKAN_SOURCE_DIR "/usr/share/vulkan"
 
@@ -222,6 +223,10 @@ static void sc_mkdir_and_mount_and_glob_files(const char *rootfs_dir,
 	if (res != 0 && errno != EEXIST) {
 		die("cannot create tmpfs target %s", libgl_dir);
 	}
+	if (res == 0 && (chown(libgl_dir, 0, 0) < 0)) {
+		// Adjust the ownership only if we created the directory.
+		die("cannot change ownership of %s", libgl_dir);
+	}
 
 	debug("mounting tmpfs at %s", libgl_dir);
 	if (mount("none", libgl_dir, "tmpfs", MS_NODEV | MS_NOEXEC, NULL) != 0) {
@@ -361,6 +366,10 @@ static void sc_mkdir_and_mount_and_bind(const char *rootfs_dir,
 	if (res != 0 && errno != EEXIST) {
 		die("cannot create directory %s", dst);
 	}
+	if (res == 0 && (chown(dst, 0, 0) < 0)) {
+		// Adjust the ownership only if we created the directory.
+		die("cannot change ownership of %s", dst);
+	}
 	// Bind mount the binary nvidia driver into $tgt_dir (i.e. /var/lib/snapd/lib/gl).
 	debug("bind mounting nvidia driver %s -> %s", src, dst);
 	if (mount(src, dst, NULL, MS_BIND, NULL) != 0) {
@@ -398,6 +407,15 @@ void sc_mount_nvidia_driver(const char *rootfs_dir)
 	if (access(SC_NVIDIA_DRIVER_VERSION_FILE, F_OK) != 0) {
 		return;
 	}
+
+	int res = mkdir(SC_LIB, 0755);
+	if (res != 0 && errno != EEXIST) {
+		die("cannot create " SC_LIB);
+	}
+	if (res == 0 && (chown(SC_LIB, 0, 0) < 0)) {
+		// Adjust the ownership only if we created the directory.
+		die("cannot change ownership of " SC_LIB);
+	}
 #ifdef NVIDIA_MULTIARCH
 	sc_mount_nvidia_driver_multiarch(rootfs_dir);
 #endif				// ifdef NVIDIA_MULTIARCH

cmd/snap-confine/snap-confine.apparmor.in

@@ -512,6 +512,8 @@
 
         # Needed to perform mount/unmounts.
         capability sys_admin,
+        # Needed for mimic construction.
+        capability chown,
 
         # Allow freezing and thawing the per-snap cgroup freezers
         /sys/fs/cgroup/freezer/snap.*/freezer.state rw,
@@ -550,22 +552,36 @@
         /tmp/.snap/{,**} rw,
         # Allow mounting/unmounting any part of $SNAP over to a temporary place
         # in /tmp/.snap/ during the preparation of a writable mimic.
-        mount options=(bind, rw) /snap/*/** -> /tmp/.snap/**,
-        # Allow mounting tmpfs over the original $SNAP/** directory.
-        mount fstype=tmpfs options=(rw) tmpfs -> /snap/*/**,
+        # FIXME: update this with per-snap snap-update-ns profiles
+        mount options=(bind, rw) /** -> /tmp/.snap/**,
+        # Allow mounting tmpfs over the original read-only directory.
+        # FIXME: update this with per-snap snap-update-ns profiles
+        mount fstype=tmpfs options=(rw) tmpfs -> /**,
         # Allow bind mounting anything from the temporary place in /tmp/.snap/
         # back to $SNAP/** (to re-construct the data that was there before).
-        mount options=(bind, rw) /tmp/.snap/** -> /snap/*/**,
+        # FIXME: update this with per-snap snap-update-ns profiles
+        mount options=(bind, rw) /tmp/.snap/** -> /**,
         # Allow unmounting the temporary directory in /tmp once it is no longer
         # necessary.
         umount /tmp/.snap/**,
 
-        # Allow creating missing directories under $SNAP once the writable
-        # mimic is ready.
+        # Allow creating missing directories anywhere under the root directory
+        # (but not in the root directory itself) where they need to be created
+        # as a mount point for layouts or for content sharing. This is a
+        # superset of other cases so they are removed
+        # FIXME: update this with per-snap snap-update-ns profiles
         / r,
-        /snap/ r,
-        /snap/{,**} r,
-        /snap/*/** w,
+        /** r,
+        /*/** w,
+
+        # Allow layouts to bind mount *from* $SNAP, $SNAP_DATA and $SNAP_COMMON
+        # *to* anywhere under the root directory. This is safe because the
+        # mounts happen inside an isolated mount namespace (but see below).
+        mount options=(bind) /snap/*/** -> /*/**,
+        mount options=(bind) /var/snap/*/** -> /*/**,
+        # As an exception, don't allow bind mounts to /media which has special
+        # sharing and propagates mount events outside of the snap namespace.
+        audit deny mount -> /media,
 
         # Allow the content interface to bind fonts from the host filesystem
         mount options=(ro bind) /var/lib/snapd/hostfs/usr/share/fonts/ -> /snap/*/*/**,
@@ -575,11 +591,7 @@
         mount options=(ro bind) /var/lib/snapd/hostfs/var/cache/fontconfig/ -> /var/cache/fontconfig/,
 
         # Allow unmounts matching possible mounts listed above.
-        umount /snap/*/*/**,
-        umount /var/snap/*/**,
-        umount /usr/share/fonts,
-        umount /usr/local/share/fonts,
-        umount /var/cache/fontconfig,
+        umount /*/**,
 
         # But we don't want anyone to touch /snap/bin
         audit deny mount /snap/bin/** -> /**,

cmd/snap-update-ns/change.go

@@ -102,6 +102,8 @@ func (c *Change) createPath(path string, pokeHoles bool) ([]*Change, error) {
 			// performed the hole poking and thus additional changes must be nil.
 			_, err = c.createPath(path, false)
 		}
+	} else if err != nil {
+		err = fmt.Errorf("cannot create path %q: %s", path, err)
 	}
 	return changes, err
 }
@@ -126,11 +128,11 @@ func (c *Change) ensureTarget() ([]*Change, error) {
 		switch kind {
 		case "":
 			if !fi.Mode().IsDir() {
-				err = fmt.Errorf("cannot use %q for mounting: not a directory", path)
+				err = fmt.Errorf("cannot use %q as mount point: not a directory", path)
 			}
 		case "file":
 			if !fi.Mode().IsRegular() {
-				err = fmt.Errorf("cannot use %q for mounting: not a regular file", path)
+				err = fmt.Errorf("cannot use %q as mount point: not a regular file", path)
 			}
 		case "symlink":
 			// When we want to create a symlink we just need the empty
@@ -139,9 +141,6 @@ func (c *Change) ensureTarget() ([]*Change, error) {
 		}
 	} else if os.IsNotExist(err) {
 		changes, err = c.createPath(path, true)
-		if err != nil {
-			err = fmt.Errorf("cannot create path %q: %s", path, err)
-		}
 	} else {
 		// If we cannot inspect the element let's just bail out.
 		err = fmt.Errorf("cannot inspect %q: %v", path, err)
@@ -168,18 +167,15 @@ func (c *Change) ensureSource() error {
 		switch kind {
 		case "":
 			if !fi.Mode().IsDir() {
-				err = fmt.Errorf("cannot use %q for mounting: not a directory", path)
+				err = fmt.Errorf("cannot use %q as bind-mount source: not a directory", path)
 			}
 		case "file":
 			if !fi.Mode().IsRegular() {
-				err = fmt.Errorf("cannot use %q for mounting: not a regular file", path)
+				err = fmt.Errorf("cannot use %q as bind-mount source: not a regular file", path)
 			}
 		}
 	} else if os.IsNotExist(err) {
 		_, err = c.createPath(path, false)
-		if err != nil {
-			err = fmt.Errorf("cannot create path %q: %s", path, err)
-		}
 	} else {
 		// If we cannot inspect the element let's just bail out.
 		err = fmt.Errorf("cannot inspect %q: %v", path, err)