start: fix execute and improve setgroups() calls

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