public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* handling of supplemental groups with userns
@ 2015-09-22 16:21 Mike Frysinger
       [not found] ` <87d1xafdk8.fsf@x220.int.ebiederm.org>
  0 siblings, 1 reply; 3+ messages in thread
From: Mike Frysinger @ 2015-09-22 16:21 UTC (permalink / raw)
  To: containers; +Cc: ebiederm, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 2069 bytes --]

is it possible to map in supplemental groups in a userns when the user
lacks setgid/etc... capabilities in the parent ns ?  it doesn't seem
like it's currently possible, but is there a reason to not enable it ?

basically i have a build tool that i want to isolate a bit, but it
requires access to some of my supplemental groups.  if i map just
my effective uid/gid, the build will fail when it tries to use the
chown/chgrp commands (gets back EINVAL).

my scenario boils down to:
 - normal unprivileged user (uid=8282)
 - member of multiple groups (gid=100, getgroups={100,16,250,...})
 - create a new userns (to get access to other ns like mount/pid)
   but still have access to existing groups where i'm root
 - use various features that require caps (new pidns/mntns/etc...)
 - create another userns and map back non-root users/groups
i.e. i switch from 8282 to 0, do what i need, then switch back to 8282.

i've attached a simple test program to show the issue.  it can map the
current uid/gid fine, but fails to do so with a supplemental group.

my reading of kernel/user_namespace.c shows that it's not possible:
(1) if gid_map has any entries, map_write bails early with EPERM
(2) if gid_map is empty, then writing a single entry (e.g. "0 100 1")
    works, but then a 2nd write runs into (1)
(3) if gid_map is empty, then writing multiple entries (e.g.
    "0 100 1\n250 250 1\n") fails in new_idmap_permitted -- the first
    check is skipped (since new_map->nr_extents is 2), and the user
    does not have caps in the parent ns

i'm aware UID_GID_MAP_MAX_EXTENTS is low, so it's not even possible if i
had the caps to map all the existing supplemental groups, which is why i
only want to map the few critical groups.  but assuming this use case is
one we want to support, maybe it makes sense to add a knob to map all of
the user's supplemental groups ?

in the mean time, a "quick" fix might be to change new_idmap_permitted
to walk all the extents, and if all the ranges are set to 1, check the
supplemental groups in addition to the current egid ?
-mike

[-- Attachment #1.2: test.c --]
[-- Type: text/x-c, Size: 2280 bytes --]

/* To compile:
 *  gcc test.c
 * To run:
 *  ./a.out [1|2|3|4|5]
 *  ./a.out -1 "string to write to gid_map"
 */

#define _GNU_SOURCE
#include <assert.h>
#include <err.h>
#include <sched.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/types.h>

int main(int argc, char *argv[])
{
	int ret;

	FILE *fp;

	int uid = getuid();
	int gid = getgid();
	int nuid = 0;
	int ngid = 0;
	int suppgid;

	gid_t *groups;
	size_t i, gsize;

	int group_mode;
	switch (argc) {
	case 0:
	case 1:
		group_mode = 1;
	case 2:
		group_mode = atoi(argv[1]);
	case 3:
		group_mode = -1;
	}

	/* Find one supplemental group */
	gsize = getgroups(0, NULL);
	assert(gsize != 0);
	groups = malloc(sizeof(*groups) * gsize);
	ret = getgroups(gsize, groups);
	assert(ret == gsize);
	for (i = 0; i < gsize; ++i)
		if (groups[i] != gid) {
			suppgid = groups[i];
			break;
		}
	if (i == gsize)
		errx(1, "could not find a supplemental group to test");
	free(groups);

	/* Create new userns */
	assert(unshare(CLONE_NEWUSER) == 0);

	/* Map the current uid */
	fp = fopen("/proc/self/uid_map", "we");
	assert(fp);
	setbuf(fp, NULL);
	fprintf(fp, "%i %i 1", nuid, uid);
	fclose(fp);

	/* Disable setgroups() */
	fp = fopen("/proc/self/setgroups", "we");
	assert(fp);
	setbuf(fp, NULL);
	fputs("deny", fp);
	fclose(fp);

	/* Map the various gids */
	fp = fopen("/proc/self/gid_map", "we");
	assert(fp);
	setbuf(fp, NULL);
	switch (group_mode) {
	case 1:	/* This works */
		fprintf(fp, "%i %i 1\n", ngid, gid);
		break;
	case 2:	/* This fails */
		fprintf(fp, "%i %i 1\n", ngid, gid);
		fprintf(fp, "%i %i 1\n", suppgid, suppgid);
		break;
	case 3:	/* This fails */
		fprintf(fp, "%i %i 1\n%i %i 1\n", ngid, gid, suppgid, suppgid);
		break;
	case 4:	/* This fails */
		fprintf(fp, "%i %i 1\n", suppgid, suppgid);
		break;
	case 5:	/* This fails */
		fprintf(fp, "0 0 10000\n");
		break;
	case -1:
		fprintf(fp, argv[2]);
		break;
	}
	fclose(fp);

	/* Validate */
	printf("uid:%i gid:%i\n", getuid(), getgid());
	gsize = getgroups(0, NULL);
	assert(gsize != 0);
	groups = malloc(sizeof(*groups) * gsize);
	ret = getgroups(gsize, groups);
	assert(ret == gsize);
	printf("groups: ");
	for (i = 0; i < gsize; ++i)
		printf("%i ", groups[i]);
	printf("\n");
	free(groups);
}

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: handling of supplemental groups with userns
       [not found] ` <87d1xafdk8.fsf@x220.int.ebiederm.org>
@ 2015-09-22 21:52   ` Mike Frysinger
  2015-09-29  3:06     ` Mike Frysinger
  0 siblings, 1 reply; 3+ messages in thread
From: Mike Frysinger @ 2015-09-22 21:52 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: linux-kernel, containers

[-- Attachment #1: Type: text/plain, Size: 2814 bytes --]

On 22 Sep 2015 14:40, Eric W. Biederman wrote:
> Mike Frysinger writes:
> > is it possible to map in supplemental groups in a userns when the user
> > lacks setgid/etc... capabilities in the parent ns ?  it doesn't seem
> > like it's currently possible, but is there a reason to not enable it ?
> 
> In your unprivileged use scenario, you won't be able to drop
> your supplementary groups so why do you need them mapped?
> 
> > basically i have a build tool that i want to isolate a bit, but it
> > requires access to some of my supplemental groups.  if i map just
> > my effective uid/gid, the build will fail when it tries to use the
> > chown/chgrp commands (gets back EINVAL).
> 
> Yes.  That really isn't valid as you are dropping groups.  Peculiarly
> enough dropping groups can be a security issue as in some permission
> checks not being a member of a group can give you enhanced access to
> files and directories.

i don't want to drop groups ... i want the exact opposite actually :).
ideally, `id` would have the same output before/after.  instead, i get
65534 for all the supplemental groups.  these commands work before i
create a new userns and i want them to keep working afterwards:
	chgrp 100 foo
	chgrp 250 foo
instead, only the first works (since that's my effective gid) and the
second fails (since i'm in that via a supplemental group).

> So to do something like what you want, you need a setuid helper (something
> like newuidmap or newgidmap) to verify that what you are doing is ok
> by local policy.

i know i can get it ahead of time if i have the caps apriori, but that's
not what i want to require.  if i had those, then i would generally be
able to simply create the namespaces directly and not bother with userns
in the first place :).

> > my scenario boils down to:
> >  - normal unprivileged user (uid=8282)
> >  - member of multiple groups (gid=100, getgroups={100,16,250,...})
> >  - create a new userns (to get access to other ns like mount/pid)
> >    but still have access to existing groups where i'm root
> >  - use various features that require caps (new pidns/mntns/etc...)
> >  - create another userns and map back non-root users/groups
> > i.e. i switch from 8282 to 0, do what i need, then switch back to 8282.
> 
> [snip]
> 
> > in the mean time, a "quick" fix might be to change new_idmap_permitted
> > to walk all the extents, and if all the ranges are set to 1, check the
> > supplemental groups in addition to the current egid ?
> 
> That allows dropping groups that you could not drop normally and so we
> can't allow it, by default.

if setgroups is set to deny, then it's not possible for me to drop any
groups, and therefore allowing me to map supplemental groups wouldn't be
a problem right ?
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: handling of supplemental groups with userns
  2015-09-22 21:52   ` Mike Frysinger
@ 2015-09-29  3:06     ` Mike Frysinger
  0 siblings, 0 replies; 3+ messages in thread
From: Mike Frysinger @ 2015-09-29  3:06 UTC (permalink / raw)
  To: Eric W. Biederman, linux-kernel, containers

[-- Attachment #1: Type: text/plain, Size: 1675 bytes --]

On 22 Sep 2015 17:52, Mike Frysinger wrote:
> On 22 Sep 2015 14:40, Eric W. Biederman wrote:
> > Mike Frysinger writes:
> > > in the mean time, a "quick" fix might be to change new_idmap_permitted
> > > to walk all the extents, and if all the ranges are set to 1, check the
> > > supplemental groups in addition to the current egid ?
> > 
> > That allows dropping groups that you could not drop normally and so we
> > can't allow it, by default.
> 
> if setgroups is set to deny, then it's not possible for me to drop any
> groups, and therefore allowing me to map supplemental groups wouldn't be
> a problem right ?

so this does open up a permission people do not have today by themselves.
since there's no requirement that the current gid be in the supplemental
groups, they could drop a single group by changing to a supp one.

for example, if it looked like:
	getuid() = 1000
	getgid() = 1000
	getgroups() = {100, 250}

then if 1000/100/250 were mapped in, they could do:
	setgid(100)
	getgid() = 100
	getgroups() = {100, 250}
and thus group 1000 has been dropped.

the reason this doesn't happen today is that writes to gid_map is permitted
only if the writer has CAP_SETGID in the parent userns and the gid has been
mapped there.  normal users don't start out with that cap, and any setuid
helper has to make sure to drop caps & setuid/setgid before execing the next
layer (i.e. do the standard stuff).

so the only way to permit this behavior is if the knobs were extended and we
had a parallel USERNS_SETGID_ALLOWED.  or we updated setgid to check the bit
USERNS_SETGROUPS_ALLOWED since it seems in spirit to cover that.
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-09-29  3:06 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-22 16:21 handling of supplemental groups with userns Mike Frysinger
     [not found] ` <87d1xafdk8.fsf@x220.int.ebiederm.org>
2015-09-22 21:52   ` Mike Frysinger
2015-09-29  3:06     ` Mike Frysinger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox