start: fix execute and improve setgroups() calls

lxc_execute() and lxc-execute where broken when a user tried to switch to a
non-root uid/gid. This prevented necessary setup operations like mounting the
rootfs which require root in the user namespace. This commit separates
switching to root in the user namespace from switching to the requested uid/gid
by lxc_execute().
This should be safe: Once we switched to root in the user namespace via
setuid() and then switch to a non-root uid/gid in the user namespace for
lxc_execute() via setuid() we cannot regain root privileges again. So we can
only make us safer (Unless I forget about some very intricate user namespace
nonsense; which is not as unlikely as I try to make it sound.).

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>

diff --git a/src/lxc/start.c b/src/lxc/start.c
index 71206e036893ba2bf45d4714e97b10c42447f717..40d422ce822a573d88d99a3815901ea21ff115ed 100644 (file)
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -769,32 +769,17 @@ static int do_start(void *data)
                goto out_warn_father;
 
        /* If we are in a new user namespace, become root there to have
-        * privilege over our namespace. When using lxc-execute we default to
-        * root, but this can be overriden using the lxc.init_uid and
-        * lxc.init_gid configuration options.
+        * privilege over our namespace.
         */
        if (!lxc_list_empty(&handler->conf->id_map)) {
-               gid_t new_gid = 0;
-               if (handler->conf->is_execute && handler->conf->init_gid)
-                       new_gid = handler->conf->init_gid;
-
-               uid_t new_uid = 0;
-               if (handler->conf->is_execute && handler->conf->init_uid)
-                       new_uid = handler->conf->init_uid;
-
-               NOTICE("Switching to uid=%d and gid=%d in new user namespace.", new_uid, new_gid);
-               if (setgid(new_gid)) {
-                       SYSERROR("Failed to setgid().");
+               if (lxc_switch_uid_gid(0, 0) < 0)
                        goto out_warn_father;
-               }
-               if (setuid(new_uid)) {
-                       SYSERROR("Failed to setuid().");
-                       goto out_warn_father;
-               }
-               if (setgroups(0, NULL)) {
-                       SYSERROR("Failed to setgroups().");
+
+               /* Drop groups only after we switched to a valid gid in the new
+                * user namespace.
+                */
+               if (lxc_setgroups(0, NULL) < 0)
                        goto out_warn_father;
-               }
        }
 
        if (access(handler->lxcpath, X_OK)) {
@@ -900,6 +885,34 @@ 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) {
+               uid_t new_uid = 0;
+               gid_t new_gid = 0;
+
+               if (handler->conf->init_uid > 0)
+                       new_uid = handler->conf->init_uid;
+
+               if (handler->conf->init_gid > 0)
+                       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.
+                */
+               bool can_setgroups = ((getuid() == 0) && (getgid() == 0));
+               if (lxc_list_empty(&handler->conf->id_map) && can_setgroups) {
+                       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