[PATCH 15/36] cmd/snap-confine: Prevent user-controlled race in setup_private_mount

When setting up the private mount namespace for a snap, snap-confine tries
to reuse the existing /tmp/snap.$SNAP_NAME directory if it already
exists. However, a user could create this directory before snap-confine is
executed and hence snap-confine would reuse it, along with any contents
that already existed. This could allow a user to symlink their own contents
into this directory and snap-confine would then mount that into the snap's
mount namespace. Finally this could allow an unprivileged attacker to cause
snap-confine to escape confinement by causing it to be executed under a
less restrictive AppArmor profile when this vulnerability is combined with
others. Fix this by moving the erroneous directory out of the way if it
doesn't have the expected permissions / ownership so we can re-create it
with the correct restrictive permissions.

This resolves CVE-2021-44731.

Signed-off-by: Alex Murray <alex.murray@canonical.com>
Gbp-Pq: Topic cve202144730
Gbp-Pq: Name 0015-cmd-snap-confine-Prevent-user-controlled-race-in-set.patch

diff --git a/cmd/snap-confine/mount-support-test.c b/cmd/snap-confine/mount-support-test.c
index a57016a56be3d4c65bc7253d345f0344bac8678d..df9fbc8195fcf693eb0cc41cfc437d0e7921f4e8 100644 (file)
--- a/cmd/snap-confine/mount-support-test.c
+++ b/cmd/snap-confine/mount-support-test.c
@@ -21,6 +21,7 @@
 #include "mount-support-nvidia.c"
 
 #include <glib.h>
+#include <glib/gstdio.h>
 
 static void replace_slashes_with_NUL(char *path, size_t len)
 {

diff --git a/cmd/snap-confine/mount-support.c b/cmd/snap-confine/mount-support.c
index 233d7ee32c6c1ea806c892df0236e40b4086bbb5..1fc44a7b260e96f01a89b14c4ed38e6d2dc2ac29 100644 (file)
--- a/cmd/snap-confine/mount-support.c
+++ b/cmd/snap-confine/mount-support.c
@@ -14,6 +14,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
  */
+
 #ifdef HAVE_CONFIG_H
 #include "config.h"
 #endif
@@ -58,6 +59,91 @@
  **/
 #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)
@@ -65,6 +151,7 @@ static void setup_private_mount(const char *snap_name)
        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.
@@ -73,17 +160,17 @@ static void setup_private_mount(const char *snap_name)
        // mounted for the applications to use
        sc_must_snprintf(tmpdir, sizeof(tmpdir), "/tmp/snap.%d_%s_XXXXXX", uid,
                         snap_name);
-       if (mkdtemp(tmpdir) == NULL) {
-               die("cannot create temporary directory essential for private /tmp");
-       }
+        base_dir_fd = must_mkdir_and_open_with_perms(tmpdir, 0, 0, 0700);
        // now we 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);
 
-       if (mkdir(tmpdir, 01777) != 0) {
-               die("cannot create temporary directory for private /tmp");
+       // Create /tmp/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", tmpdir);
        }
        umask(old_mask);