From bb39982059551c8c4e0108525f4d66a5d383b8d4 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Thu, 13 Feb 2020 00:16:15 +0100 Subject: [PATCH] tree-wide: improve setgroups() dropping Drop groups before we change to userns root. Reported-by: Teddy Reed Signed-off-by: Christian Brauner --- src/lxc/attach.c | 6 +++--- src/lxc/cgroups/cgfsng.c | 14 ++++++-------- src/lxc/lxccontainer.c | 5 ++--- src/lxc/start.c | 12 ++++++------ src/lxc/storage/btrfs.c | 5 ++--- src/lxc/storage/storage_utils.c | 5 ++--- 6 files changed, 21 insertions(+), 26 deletions(-) diff --git a/src/lxc/attach.c b/src/lxc/attach.c index 3ff1e5943b..ef042950ac 100644 --- 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 0b2e16513e..86b655702a 100644 --- 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 c1c4432638..eab580e22b 100644 --- a/src/lxc/lxccontainer.c +++ b/src/lxc/lxccontainer.c @@ -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; @@ -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; diff --git a/src/lxc/start.c b/src/lxc/start.c index 917788e4ad..fea4ea9fae 100644 --- a/src/lxc/start.c +++ b/src/lxc/start.c @@ -1181,9 +1181,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. */ @@ -1191,6 +1188,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) @@ -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. @@ -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"); diff --git a/src/lxc/storage/btrfs.c b/src/lxc/storage/btrfs.c index 160dc6df30..be66aef775 100644 --- 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 6132113f20..79ee62ecbd 100644 --- 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;