tree-wide: improve setgroups() dropping

Drop groups before we change to userns root.

Reported-by: Teddy Reed <teddy.reed@gmail.com>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>

diff --git a/src/lxc/attach.c b/src/lxc/attach.c
index 5c50e1a1097513c0df29906a7ff12751794a7694..7b07dce440497874d31ade9f149df72e03a4471f 100644 (file)
--- a/src/lxc/attach.c
+++ b/src/lxc/attach.c
@@ -769,6 +769,9 @@ static int attach_child_main(struct attach_clone_payload *payload)
                        goto on_error;
        }
 
+       if (!lxc_setgroups(0, NULL) && errno != EPERM)
+               goto on_error;
+
        if (options->namespaces & CLONE_NEWUSER) {
                /* Check whether nsuid 0 has a mapping. */
                ns_root_uid = get_ns_uid(0);
@@ -789,9 +792,6 @@ static int attach_child_main(struct attach_clone_payload *payload)
                        goto on_error;
        }
 
-       if (!lxc_setgroups(0, NULL) && errno != EPERM)
-               goto on_error;
-
        /* Set {u,g}id. */
        if (options->uid != LXC_INVALID_UID)
                new_uid = options->uid;

diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index 9751fb761201d53caea6ec77bca0121290c8e13c..3e5345b62dd2dc82cf90e326d4e5a13b51f29950 100644 (file)
--- a/src/lxc/cgroups/cgfsng.c
+++ b/src/lxc/cgroups/cgfsng.c
@@ -1027,6 +1027,9 @@ static int cgroup_rmdir_wrapper(void *data)
        gid_t nsgid = (arg->conf->root_nsgid_map != NULL) ? 0 : arg->conf->init_gid;
        int ret;
 
+       if (!lxc_setgroups(0, NULL) && errno != EPERM)
+               return log_error_errno(-1, errno, "Failed to setgroups(0, NULL)");
+
        ret = setresgid(nsgid, nsgid, nsgid);
        if (ret < 0)
                return log_error_errno(-1, errno,
@@ -1039,10 +1042,6 @@ static int cgroup_rmdir_wrapper(void *data)
                                       "Failed to setresuid(%d, %d, %d)",
                                       (int)nsuid, (int)nsuid, (int)nsuid);
 
-       ret = setgroups(0, NULL);
-       if (ret < 0 && errno != EPERM)
-               return log_error_errno(-1, errno, "Failed to setgroups(0, NULL)");
-
        return cgroup_rmdir(arg->hierarchies, arg->container_cgroup);
 }
 
@@ -1494,6 +1493,9 @@ static int chown_cgroup_wrapper(void *data)
        uid_t nsuid = (arg->conf->root_nsuid_map != NULL) ? 0 : arg->conf->init_uid;
        gid_t nsgid = (arg->conf->root_nsgid_map != NULL) ? 0 : arg->conf->init_gid;
 
+       if (!lxc_setgroups(0, NULL) && errno != EPERM)
+               return log_error_errno(-1, errno, "Failed to setgroups(0, NULL)");
+
        ret = setresgid(nsgid, nsgid, nsgid);
        if (ret < 0)
                return log_error_errno(-1, errno,
@@ -1506,10 +1508,6 @@ static int chown_cgroup_wrapper(void *data)
                                       "Failed to setresuid(%d, %d, %d)",
                                       (int)nsuid, (int)nsuid, (int)nsuid);
 
-       ret = setgroups(0, NULL);
-       if (ret < 0 && errno != EPERM)
-               return log_error_errno(-1, errno, "Failed to setgroups(0, NULL)");
-
        destuid = get_ns_uid(arg->origuid);
        if (destuid == LXC_INVALID_UID)
                destuid = 0;

diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
index 15c5ec90311b2cbf2c07456d9607a6484c4f8fdc..55e391870acf7bf3ee32222446a96eb09f91b51e 100644 (file)
--- a/src/lxc/lxccontainer.c
+++ b/src/lxc/lxccontainer.c
@@ -3660,6 +3660,8 @@ static int clone_update_rootfs(struct clone_update_data *data)
        /* update hostname in rootfs */
        /* we're going to mount, so run in a clean namespace to simplify cleanup */
 
+       (void)lxc_setgroups(0, NULL);
+
        if (setgid(0) < 0) {
                ERROR("Failed to setgid to 0");
                return -1;
@@ -3670,9 +3672,6 @@ static int clone_update_rootfs(struct clone_update_data *data)
                return -1;
        }
 
-       if (setgroups(0, NULL) < 0)
-               WARN("Failed to clear groups");
-
        if (unshare(CLONE_NEWNS) < 0)
                return -1;
 

diff --git a/src/lxc/start.c b/src/lxc/start.c
index d70ffdf0e61d3d8c0798fa4ef16014778a0bbc87..b727e3ab9bc96d05d381399d2a684ba2378b0095 100644 (file)
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -1198,9 +1198,6 @@ static int do_start(void *data)
                if (!handler->conf->root_nsgid_map)
                        nsgid = handler->conf->init_gid;
 
-               if (!lxc_switch_uid_gid(nsuid, nsgid))
-                       goto out_warn_father;
-
                /* Drop groups only after we switched to a valid gid in the new
                 * user namespace.
                 */
@@ -1208,6 +1205,9 @@ static int do_start(void *data)
                    (handler->am_root || errno != EPERM))
                        goto out_warn_father;
 
+               if (!lxc_switch_uid_gid(nsuid, nsgid))
+                       goto out_warn_father;
+
                ret = prctl(PR_SET_DUMPABLE, prctl_arg(1), prctl_arg(0),
                            prctl_arg(0), prctl_arg(0));
                if (ret < 0)
@@ -1447,9 +1447,6 @@ static int do_start(void *data)
        if (new_gid == nsgid)
                new_gid = LXC_INVALID_GID;
 
-       if (!lxc_switch_uid_gid(new_uid, new_gid))
-               goto out_warn_father;
-
        /* If we are in a new user namespace we already dropped all groups when
         * we switched to root in the new user namespace further above. Only
         * drop groups if we can, so ensure that we have necessary privilege.
@@ -1461,6 +1458,9 @@ static int do_start(void *data)
                        if (!lxc_setgroups(0, NULL))
                                goto out_warn_father;
 
+       if (!lxc_switch_uid_gid(new_uid, new_gid))
+               goto out_warn_father;
+
        ret = lxc_ambient_caps_down();
        if (ret < 0) {
                ERROR("Failed to clear ambient capabilities");

diff --git a/src/lxc/storage/btrfs.c b/src/lxc/storage/btrfs.c
index 160dc6df30b75914998ea8e1d6624e6db3dafec3..be66aef77568977fdd888f31849d78fce3d10241 100644 (file)
--- a/src/lxc/storage/btrfs.c
+++ b/src/lxc/storage/btrfs.c
@@ -374,14 +374,13 @@ int btrfs_snapshot_wrapper(void *data)
        const char *src;
        struct rsync_data_char *arg = data;
 
+       (void)lxc_setgroups(0, NULL);
+
        if (setgid(0) < 0) {
                ERROR("Failed to setgid to 0");
                return -1;
        }
 
-       if (setgroups(0, NULL) < 0)
-               WARN("Failed to clear groups");
-
        if (setuid(0) < 0) {
                ERROR("Failed to setuid to 0");
                return -1;

diff --git a/src/lxc/storage/storage_utils.c b/src/lxc/storage/storage_utils.c
index 6132113f20ac4ede418d4c654f056655a230f999..79ee62ecbddc92bf38d9ef1eb86b8995dddea3af 100644 (file)
--- a/src/lxc/storage/storage_utils.c
+++ b/src/lxc/storage/storage_utils.c
@@ -465,14 +465,13 @@ int storage_destroy_wrapper(void *data)
 {
        struct lxc_conf *conf = data;
 
+       (void)lxc_setgroups(0, NULL);
+
        if (setgid(0) < 0) {
                SYSERROR("Failed to setgid to 0");
                return -1;
        }
 
-       if (setgroups(0, NULL) < 0)
-               SYSWARN("Failed to clear groups");
-
        if (setuid(0) < 0) {
                SYSERROR("Failed to setuid to 0");
                return -1;