[lxc-devel] [lxc/master] start: fix execute and improve setgroups() calls
brauner on Github
lxc-bot at linuxcontainers.org
Mon Jan 2 14:28:48 UTC 2017
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 1112 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20170102/bad78632/attachment.bin>
-------------- next part --------------
From ff7eea05b3be4df393dcc7e8d6f17a2fcc2ab260 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Mon, 2 Jan 2017 15:12:10 +0100
Subject: [PATCH 1/2] utils: add uid, gid, group convenience wrappers
This commit adds lxc_switch_uid_gid() which allows to switch the uid and gid of
a process via setuid() and setgid() and lxc_setgroups() which allows to set
groups via setgroups(). The main advantage is that they nicely log the switches
they perform.
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/utils.c | 30 ++++++++++++++++++++++++++++++
src/lxc/utils.h | 4 ++++
2 files changed, 34 insertions(+)
diff --git a/src/lxc/utils.c b/src/lxc/utils.c
index 7c2098e..ff4ef1b 100644
--- a/src/lxc/utils.c
+++ b/src/lxc/utils.c
@@ -27,6 +27,7 @@
#include <dirent.h>
#include <errno.h>
#include <fcntl.h>
+#include <grp.h>
#include <libgen.h>
#include <stddef.h>
#include <stdio.h>
@@ -2048,3 +2049,32 @@ int lxc_safe_long(const char *numstr, long int *converted)
*converted = sli;
return 0;
}
+
+int lxc_switch_uid_gid(uid_t uid, gid_t gid)
+{
+ if (setgid(gid) < 0) {
+ SYSERROR("Failed to switch to gid %d.", gid);
+ return -errno;
+ }
+ NOTICE("Switched to gid %d.", gid);
+
+ if (setuid(uid) < 0) {
+ SYSERROR("Failed to switch to uid %d.", uid);
+ return -errno;
+ }
+ NOTICE("Switched to uid %d.", uid);
+
+ return 0;
+}
+
+/* Simple covenience function which enables uniform logging. */
+int lxc_setgroups(int size, gid_t list[])
+{
+ if (setgroups(0, NULL) < 0) {
+ SYSERROR("Failed to setgroups().");
+ return -errno;
+ }
+ NOTICE("Dropped additional groups.");
+
+ return 0;
+}
diff --git a/src/lxc/utils.h b/src/lxc/utils.h
index b7dcd5d..2b56905 100644
--- a/src/lxc/utils.h
+++ b/src/lxc/utils.h
@@ -327,4 +327,8 @@ int lxc_safe_uint(const char *numstr, unsigned int *converted);
int lxc_safe_int(const char *numstr, int *converted);
int lxc_safe_long(const char *numstr, long int *converted);
+/* Switch to a new uid and gid. */
+int lxc_switch_uid_gid(uid_t uid, gid_t gid);
+int lxc_setgroups(int size, gid_t list[]);
+
#endif /* __LXC_UTILS_H */
From 4cb425dca342f22025e57a854eaad1c566588d5a Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Mon, 2 Jan 2017 15:14:22 +0100
Subject: [PATCH 2/2] 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 at ubuntu.com>
---
src/lxc/start.c | 57 +++++++++++++++++++++++++++++++++++----------------------
1 file changed, 35 insertions(+), 22 deletions(-)
diff --git a/src/lxc/start.c b/src/lxc/start.c
index 71206e0..40d422c 100644
--- 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
More information about the lxc-devel
mailing list