Re: [PATCH review 00/18] userns: review of bug fixes for 3.19-rcX

[ebiederm@xmission.com (Eric W. Biederman)]

Richard Weinberger <richard@nod.at> writes:

> Am 12.12.2014 um 23:32 schrieb Eric W. Biederman:
>> 
>> The entire tree for testing is available at:
>> 	git.kernel.org:/pub/scm/linux/kernel/git/ebiederm/user-namespace.git for-testing
>> 
>> This is my queue of important bug fixes for user namespaces.  Most of
>> these changes warrant being backported.  A few are bug fixes for cases
>> where only root can trigger the issue so have not been marked for being
>> back ported to stable.
>> 
>> A few of these patches have not been posted for review preivously, so I
>> a giving the light of mailling list before I send them to Linus.  This
>> patchset has seen some testing already. 
>> 
>> Since there are small deliberate breakage of userspace in here the more
>> reviewers/testers the better.
>> 
>> Baring complictions I intend to ask Linus to pull this patchset sometime
>> early next week.
>> 
>> So far nothing broke on my libvirt-lxc test bed. :-)
>> Tested with openSUSE 13.2 and libvirt 1.2.9.
>> Tested-by: Richard Weinberger <richard@nod.at>
>
> FYI, this change set breaks util-linux's unshare(1) tool
> as an unprivileged is no longer allowed to write to
> /proc/self/gid_map.

Only the --map-root-user option.  The patch below fixes it.
I will push this upstream after I push the main change to Linus.

This probably deseres a little discussion on the util-linux list.  Most
use cases will continue to work but with setgroups disabled some things
won't work and can not be made to work without privilege.

From: "Eric W. Biederman" <ebiederm@xmission.com>
Date: Thu, 11 Dec 2014 20:05:25 -0600
Subject: [PATCH] unshare: Fix --map-root-user to work on new kernels

In rare cases droping groups with setgroups(0, NULL) is an operation
that can grant a user additional privileges.  User namespaces were
allwoing that operation to unprivileged users and that had to be
fixed.

Update unshare --map-root-user to disable the setgroups operation
before setting the gid_map.

This is needed as after the security fix gid_map is restricted to
privileged users unless setgroups has been disabled.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 include/pathnames.h |  1 +
 sys-utils/unshare.c | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/include/pathnames.h b/include/pathnames.h
index 1cc4e15e6e4f..1c53e4554268 100644
--- a/include/pathnames.h
+++ b/include/pathnames.h
@@ -92,6 +92,7 @@
 
 #define _PATH_PROC_UIDMAP	"/proc/self/uid_map"
 #define _PATH_PROC_GIDMAP	"/proc/self/gid_map"
+#define _PATH_PROC_SETGROUPS	"/proc/self/setgroups"
 
 #define _PATH_PROC_ATTR_CURRENT	"/proc/self/attr/current"
 #define _PATH_PROC_ATTR_EXEC	"/proc/self/attr/exec"
diff --git a/sys-utils/unshare.c b/sys-utils/unshare.c
index 95e4afbd055e..d409a7c936b6 100644
--- a/sys-utils/unshare.c
+++ b/sys-utils/unshare.c
@@ -39,6 +39,24 @@
 #include "pathnames.h"
 #include "all-io.h"
 
+static void disable_setgroups(void)
+{
+	const char *file = _PATH_PROC_SETGROUPS;
+	const char *deny = "deny";
+	int fd;
+
+	fd = open(file, O_WRONLY);
+	if (fd < 0) {
+		if (errno == ENOENT)
+			return;
+		 err(EXIT_FAILURE, _("cannot open %s"), file);
+	}
+
+	if (write_all(fd, deny, strlen(deny)))
+		err(EXIT_FAILURE, _("write failed %s"), file);
+	close(fd);
+}
+
 static void map_id(const char *file, uint32_t from, uint32_t to)
 {
 	char *buf;
@@ -178,6 +196,7 @@ int main(int argc, char *argv[])
 	}
 
 	if (maproot) {
+		disable_setgroups();
 		map_id(_PATH_PROC_UIDMAP, 0, real_euid);
 		map_id(_PATH_PROC_GIDMAP, 0, real_egid);
 	}
-- 
2.1.3