linux: open source bind mount in the host

when creating a new user namespace, attempt to open the source for the bind mounts in the current context, as we might lose access to the directory as root in the user namespace. Closes: containers#871 Signed-off-by: Giuseppe Scrivano <[email protected]>
giuseppe · Feb 14, 2022 · bb5bc67 · bb5bc67
1 parent c673abb
commit bb5bc67
Show file tree

Hide file tree

Showing 4 changed files with 103 additions and 23 deletions.
diff --git a/src/libcrun/linux.c b/src/libcrun/linux.c
@@ -409,6 +409,27 @@ do_mount_setattr (const char *target, int targetfd, uint64_t clear, uint64_t set
   return 0;
 }
 
+static int
+get_bind_mount (const char *src, libcrun_error_t *err)
+{
+  cleanup_close int open_tree_fd = -1;
+  struct mount_attr_s attr = {
+    0,
+  };
+  int ret;
+
+  open_tree_fd = syscall_open_tree (-1, src,
+                                    AT_NO_AUTOMOUNT | AT_SYMLINK_NOFOLLOW | OPEN_TREE_CLOEXEC | OPEN_TREE_CLONE);
+  if (UNLIKELY (open_tree_fd < 0))
+    return crun_make_error (err, errno, "open `%s`", src);
+
+  ret = syscall_mount_setattr (open_tree_fd, "", AT_EMPTY_PATH, &attr);
+  if (UNLIKELY (ret < 0))
+    return crun_make_error (err, errno, "mount_setattr `%s`", src);
+
+  return get_and_reset (&open_tree_fd);
+}
+
 static int
 get_idmapped_mount (const char *src, pid_t pid, libcrun_error_t *err)
 {
@@ -1627,7 +1648,15 @@ do_mounts (libcrun_container_t *container, int rootfsfd, const char *rootfs, con
 
       if (def->mounts[i]->source && (flags & MS_BIND))
         {
-          is_dir = crun_dir_p (def->mounts[i]->source, false, err);
+          char proc_buf[64];
+          const char *path = def->mounts[i]->source;
+          if (mount_fds->fds[i] >= 0)
+            {
+              sprintf (proc_buf, "/proc/self/fd/%d", mount_fds->fds[i]);
+              path = proc_buf;
+            }
+
+          is_dir = crun_dir_p (path, false, err);
           if (UNLIKELY (is_dir < 0))
             return is_dir;
 
@@ -3332,11 +3361,27 @@ is_idmapped (runtime_spec_schema_defs_mount *mnt)
   return false;
 }
 
+static bool
+is_bind_mount (runtime_spec_schema_defs_mount *mnt)
+{
+  size_t i;
+
+  for (i = 0; i < mnt->options_len; i++)
+    {
+      if (strcmp (mnt->options[i], "bind") == 0)
+        return true;
+      if (strcmp (mnt->options[i], "rbind") == 0)
+        return true;
+    }
+  return false;
+}
+
 static int
 prepare_and_send_mounts (libcrun_container_t *container, pid_t pid, int sync_socket_host, libcrun_error_t *err)
 {
   runtime_spec_schema_config_schema *def = container->container_def;
   cleanup_close_map struct libcrun_fd_map *mount_fds = NULL;
+  bool has_userns = (get_private_data (container)->unshare_flags & CLONE_NEWUSER) ? true : false;
   size_t how_many = 0;
   size_t i;
   int ret;
@@ -3358,6 +3403,12 @@ prepare_and_send_mounts (libcrun_container_t *container, pid_t pid, int sync_soc
 
           mount_fds->fds[i] = fd;
         }
+      else if (has_userns && is_bind_mount (def->mounts[i]))
+        {
+          mount_fds->fds[i] = get_bind_mount (def->mounts[i]->source, err);
+          if (UNLIKELY (mount_fds->fds[i] < 0))
+            crun_error_release (err);
+        }
 
       if (mount_fds->fds[i] >= 0)
         how_many++;
@@ -3650,10 +3701,6 @@ init_container (libcrun_container_t *container, int sync_socket_container, struc
   if (UNLIKELY (ret < 0))
     return ret;
 
-  /* Move ownership.  */
-  get_private_data (container)->mount_fds = mount_fds;
-  mount_fds = NULL;
-
   return 0;
 }
 

diff --git a/src/libcrun/utils.h b/src/libcrun/utils.h
@@ -116,7 +116,7 @@ cleanup_pidp (void *p)
 }
 
 struct
-    libcrun_fd_map
+libcrun_fd_map
 {
   size_t nfds;
   int fds[];

diff --git a/tests/test_mounts.py b/tests/test_mounts.py
@@ -178,22 +178,56 @@ def test_mount_dev():
         return -1
     return 0
 
+
+def test_userns_bind_mount():
+    if is_rootless():
+        return 77
+    conf = base_config()
+    add_all_namespaces(conf, userns=True)
+
+    fullMapping = [
+        {
+            "containerID": 0,
+            "hostID": 1,
+            "size": 10
+        }
+    ]
+    conf['linux']['uidMappings'] = fullMapping
+    conf['linux']['gidMappings'] = fullMapping
+
+    bind_dir_parent = os.path.join(get_tests_root(), "bind-mount-userns")
+    bind_dir = os.path.join(bind_dir_parent, "m")
+    try:
+        os.makedirs(bind_dir)
+        mount_opt = {"destination": "/foo", "type": "bind", "source": bind_dir, "options": ["bind", "ro"]}
+        conf['mounts'].append(mount_opt)
+        os.chown(bind_dir_parent, 0, 0)
+        os.chmod(bind_dir_parent, 0o000)
+
+        conf['process']['args'] = ['/init', 'true']
+        run_and_get_output(conf, chown_rootfs_to=1)
+    finally:
+        shutil.rmtree(bind_dir)
+
+    return 0
+
 all_tests = {
-    "test-mount-ro" : test_mount_ro,
-    "test-mount-rw" : test_mount_rw,
-    "test-mount-relatime" : test_mount_relatime,
-    "test-mount-strictatime" : test_mount_strictatime,
-    "test-mount-exec" : test_mount_exec,
-    "test-mount-noexec" : test_mount_noexec,
-    "test-mount-suid" : test_mount_suid,
-    "test-mount-nosuid" : test_mount_nosuid,
-    "test-mount-sync" : test_mount_sync,
-    "test-mount-dirsync" : test_mount_dirsync,
-    "test-mount-symlink" : test_mount_symlink,
-    "test-mount-symlink-not-existing" : test_mount_symlink_not_existing,
-    "test-mount-dev" : test_mount_dev,
-    "test-mount-nodev" : test_mount_nodev,
-    "test-mount-path-with-multiple-slashes" : test_mount_path_with_multiple_slashes,
+    "mount-ro" : test_mount_ro,
+    "mount-rw" : test_mount_rw,
+    "mount-relatime" : test_mount_relatime,
+    "mount-strictatime" : test_mount_strictatime,
+    "mount-exec" : test_mount_exec,
+    "mount-noexec" : test_mount_noexec,
+    "mount-suid" : test_mount_suid,
+    "mount-nosuid" : test_mount_nosuid,
+    "mount-sync" : test_mount_sync,
+    "mount-dirsync" : test_mount_dirsync,
+    "mount-symlink" : test_mount_symlink,
+    "mount-symlink-not-existing" : test_mount_symlink_not_existing,
+    "mount-dev" : test_mount_dev,
+    "mount-nodev" : test_mount_nodev,
+    "mount-path-with-multiple-slashes" : test_mount_path_with_multiple_slashes,
+    "mount-userns-bind-mount" : test_userns_bind_mount,
 }
 
 if __name__ == "__main__":

diff --git a/tests/test_uid_gid.py b/tests/test_uid_gid.py
@@ -22,7 +22,7 @@ def test_userns_full_mapping():
     if is_rootless():
         return 77
     conf = base_config()
-    add_all_namespaces(conf)
+    add_all_namespaces(conf, userns=True)
 
     fullMapping = [
         {
@@ -45,7 +45,6 @@ def test_userns_full_mapping():
 
     return 0
 
-
 def test_uid():
     if is_rootless():
         return 77