Skip to content

Commit

Permalink
tree-wide: improve setgroups() dropping
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Christian Brauner committed Mar 2, 2020
1 parent 8ecd7f2 commit bb39982
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 26 deletions.
6 changes: 3 additions & 3 deletions src/lxc/attach.c
Expand Up @@ -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);
Expand All @@ -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;
Expand Down
14 changes: 6 additions & 8 deletions src/lxc/cgroups/cgfsng.c
Expand Up @@ -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,
Expand All @@ -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);
}

Expand Down Expand Up @@ -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,
Expand All @@ -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;
Expand Down
5 changes: 2 additions & 3 deletions src/lxc/lxccontainer.c
Expand Up @@ -3618,6 +3618,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;
Expand All @@ -3628,9 +3630,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;

Expand Down
12 changes: 6 additions & 6 deletions src/lxc/start.c
Expand Up @@ -1181,16 +1181,16 @@ 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.
*/
if (!lxc_setgroups(0, NULL) &&
(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)
Expand Down Expand Up @@ -1424,9 +1424,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.
Expand All @@ -1438,6 +1435,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");
Expand Down
5 changes: 2 additions & 3 deletions src/lxc/storage/btrfs.c
Expand Up @@ -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;
Expand Down
5 changes: 2 additions & 3 deletions src/lxc/storage/storage_utils.c
Expand Up @@ -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;
Expand Down

0 comments on commit bb39982

Please sign in to comment.