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 <christian.brauner@ubuntu.com>

diff --git a/src/lxc/start.c b/src/lxc/start.c
index 5d4596adfd9b88745542f0fa48ec89854a1cc493..da822cdc641d39898819521155fa70b089cbbf44 100644 (file)
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -1027,33 +1027,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
@@ -1109,6 +1082,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.
         */