From 76bdf2992c0c90f55b233dd5985d49d02c0c55a7 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Mon, 11 Sep 2017 03:16:06 +0200 Subject: [PATCH] start: switch ids at last possible instance This is technically not necessary but it is a privilege sensitive operation. Meaning if anyone wants to do something that requires privilege it should be done before the id switch. So let's move the id switch immediately before the exec so that it's called at the last possible moment. Signed-off-by: Christian Brauner --- src/lxc/start.c | 54 ++++++++++++++++++++++++------------------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/src/lxc/start.c b/src/lxc/start.c index 3ccb73311..a2fbe0d38 100644 --- a/src/lxc/start.c +++ b/src/lxc/start.c @@ -962,33 +962,6 @@ static int do_start(void *data) goto out_warn_father; } - /* The container has been setup. We can now switch to an unprivileged - * uid/gid. - */ - if (handler->conf->is_execute) { - bool have_cap_setgid; - uid_t new_uid = handler->conf->init_uid; - gid_t new_gid = handler->conf->init_gid; - - /* 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. - */ - #if HAVE_LIBCAP - have_cap_setgid = lxc_proc_cap_is_set(CAP_SETGID, CAP_EFFECTIVE); - #else - have_cap_setgid = false; - #endif - if (lxc_list_empty(&handler->conf->id_map) && have_cap_setgid) { - if (lxc_setgroups(0, NULL) < 0) - goto out_warn_father; - } - - if (lxc_switch_uid_gid(new_uid, new_gid) < 0) - goto out_warn_father; - } - /* The clearenv() and putenv() calls have been moved here to allow us to * use environment variables passed to the various hooks, such as the * start hook above. Not all of the variables like CONFIG_PATH or ROOTFS @@ -1044,6 +1017,33 @@ static int do_start(void *data) if (lxc_sync_barrier_parent(handler, LXC_SYNC_CGROUP_LIMITS)) goto out_warn_father; + /* The container has been setup. We can now switch to an unprivileged + * uid/gid. + */ + if (handler->conf->is_execute) { + bool have_cap_setgid; + uid_t new_uid = handler->conf->init_uid; + gid_t new_gid = handler->conf->init_gid; + + /* 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. + */ + #if HAVE_LIBCAP + have_cap_setgid = lxc_proc_cap_is_set(CAP_SETGID, CAP_EFFECTIVE); + #else + have_cap_setgid = false; + #endif + if (lxc_list_empty(&handler->conf->id_map) && have_cap_setgid) { + if (lxc_setgroups(0, NULL) < 0) + goto out_warn_father; + } + + if (lxc_switch_uid_gid(new_uid, new_gid) < 0) + goto out_warn_father; + } + /* After this call, we are in error because this ops should not return * as it execs. */