[lxc-devel] [PATCH] Fix unprivileged containers started by root

Stéphane Graber stgraber at ubuntu.com
Wed Feb 26 21:34:17 UTC 2014


On Wed, Feb 26, 2014 at 03:55:56PM -0500, Dwight Engen wrote:
> On Wed, 26 Feb 2014 14:55:57 -0500
> Stéphane Graber <stgraber at ubuntu.com> wrote:
> 
> > Hi Dwight,
> > 
> > Serge suggested that you may have been relying on the previous
> > behavior, will that change be a problem for you?
> > 
> > Do you think this is reasonable?
> > 
> > Stéphane
> 
> Hey Stéphane, yeah I was running unprivileged containers and using the
> lxc writing the mapping functionality. Up till this change we could do
> manual management of uid ranges and not *have* to have new[ug]idmap. At
> least I hadn't hit the other two cases where it was needed (running a
> template in a mapped userns and chown_mapped_root()). I do get that it
> would be nice to just rely on the presence of new[ug]id map, especially
> to avoid range overlap.
> 
> RHEL7/OL7/Fedora rawhide still only ship shadow 4.1.x so that what is a
> bit of a problem right now. Not that its hard to build a 4.2 shadow
> rpm, I've got one locally and I think Mike was building one too. Do we
> know where other distros are as far as updating to the newest shadow? I
> guess it just comes down to if we want to say that your distro must
> have the new shadow if you want to use userns containers.

Ubuntu will have it starting with 14.04. I also have some backports of
it working as far down as 12.04.

Debian has been updating their packaging branch and I hope for a shadow
upload there in the near future.


I guess the question is whether we expect distros that ship a > 3.12 to
still ship shadow 4.1. To me it seems like forcing the distro's hands a
bit and requiring the new shadow will make things much easier for us as
well as prevent some mistakes on our users' part.

Otherwise I'd totally see a scenario where someone on a system with
shadow 4.2 will create a bunch of users not realizing that the newly
granted ranges happen to overlap with existing containers that were
created by root. That user is then able to strace/kill/read the
filesystem of any existing such container...

> 
> > On Wed, Feb 26, 2014 at 02:53:04PM -0500, Stéphane Graber wrote:
> > > This change makes it possible to create unprivileged containers as
> > > root. They will be stored in the usual system wide location, use
> > > the usual system wide cache but will be running using a uid/gid map.
> > > 
> > > To make things consistent as well as avoid potential range
> > > conflicts, I'm removing the code that was directly writing the
> > > uid/gid ranges and instead force all unprivileged containers
> > > through newuidmap/newgidmap. This means you need to grant uid/gid
> > > ranges to root just as you would for a normal user.
> > > 
> > > bdev and network restrictions however don't apply to those
> > > containers, so you don't need to add lxc-usernic entries for root.
> > > 
> > > This essentially requires anyone wanting to run unprivileged
> > > containers to have newuidmap and newgidmap on their system, those
> > > two are provided by a very recent version of shadow.
> > > 
> > > Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
> > > ---
> > >  src/lxc/conf.c         | 56
> > > ++++++++++----------------------------------------
> > > src/lxc/lxccontainer.c |  6 +++--- 2 files changed, 14
> > > insertions(+), 48 deletions(-)
> > > 
> > > diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> > > index fc39897..2a9b672 100644
> > > --- a/src/lxc/conf.c
> > > +++ b/src/lxc/conf.c
> > > @@ -3132,32 +3132,6 @@ int lxc_assign_network(struct lxc_list
> > > *network, pid_t pid) return 0;
> > >  }
> > >  
> > > -static int write_id_mapping(enum idtype idtype, pid_t pid, const
> > > char *buf,
> > > -			    size_t buf_size)
> > > -{
> > > -	char path[PATH_MAX];
> > > -	int ret, closeret;
> > > -	FILE *f;
> > > -
> > > -	ret = snprintf(path, PATH_MAX, "/proc/%d/%cid_map", pid,
> > > idtype == ID_TYPE_UID ? 'u' : 'g');
> > > -	if (ret < 0 || ret >= PATH_MAX) {
> > > -		fprintf(stderr, "%s: path name too long\n",
> > > __func__);
> > > -		return -E2BIG;
> > > -	}
> > > -	f = fopen(path, "w");
> > > -	if (!f) {
> > > -		perror("open");
> > > -		return -EINVAL;
> > > -	}
> > > -	ret = fwrite(buf, buf_size, 1, f);
> > > -	if (ret < 0)
> > > -		SYSERROR("writing id mapping");
> > > -	closeret = fclose(f);
> > > -	if (closeret)
> > > -		SYSERROR("writing id mapping");
> > > -	return ret < 0 ? ret : closeret;
> > > -}
> > > -
> > >  int lxc_map_ids(struct lxc_list *idmap, pid_t pid)
> > >  {
> > >  	struct lxc_list *iterator;
> > > @@ -3165,7 +3139,6 @@ int lxc_map_ids(struct lxc_list *idmap, pid_t
> > > pid) int ret = 0;
> > >  	enum idtype type;
> > >  	char *buf = NULL, *pos;
> > > -	int am_root = (getuid() == 0);
> > >  
> > >  	for(type = ID_TYPE_UID; type <= ID_TYPE_GID; type++) {
> > >  		int left, fill;
> > > @@ -3176,10 +3149,9 @@ int lxc_map_ids(struct lxc_list *idmap,
> > > pid_t pid) return -ENOMEM;
> > >  		}
> > >  		pos = buf;
> > > -		if (!am_root)
> > > -			pos += sprintf(buf, "new%cidmap %d",
> > > -				type == ID_TYPE_UID ? 'u' : 'g',
> > > -				pid);
> > > +		pos += sprintf(buf, "new%cidmap %d",
> > > +			type == ID_TYPE_UID ? 'u' : 'g',
> > > +			pid);
> > >  
> > >  		lxc_list_for_each(iterator, idmap) {
> > >  			/* The kernel only takes <= 4k for writes
> > > to /proc/<nr>/[ug]id_map */ @@ -3189,10 +3161,8 @@ int
> > > lxc_map_ids(struct lxc_list *idmap, pid_t pid) 
> > >  			had_entry = 1;
> > >  			left = 4096 - (pos - buf);
> > > -			fill = snprintf(pos, left, "%s%lu %lu
> > > %lu%s",
> > > -					am_root ? "" : " ",
> > > -					map->nsid, map->hostid,
> > > map->range,
> > > -					am_root ? "\n" : "");
> > > +			fill = snprintf(pos, left, " %lu %lu %lu ",
> > > +					map->nsid, map->hostid,
> > > map->range); if (fill <= 0 || fill >= left)
> > >  				SYSERROR("snprintf failed, too
> > > many mappings"); pos += fill;
> > > @@ -3200,16 +3170,12 @@ int lxc_map_ids(struct lxc_list *idmap,
> > > pid_t pid) if (!had_entry)
> > >  			continue;
> > >  
> > > -		if (am_root) {
> > > -			ret = write_id_mapping(type, pid, buf,
> > > pos-buf);
> > > -		} else {
> > > -			left = 4096 - (pos - buf);
> > > -			fill = snprintf(pos, left, "\n");
> > > -			if (fill <= 0 || fill >= left)
> > > -				SYSERROR("snprintf failed, too
> > > many mappings");
> > > -			pos += fill;
> > > -			ret = system(buf);
> > > -		}
> > > +		left = 4096 - (pos - buf);
> > > +		fill = snprintf(pos, left, "\n");
> > > +		if (fill <= 0 || fill >= left)
> > > +			SYSERROR("snprintf failed, too many
> > > mappings");
> > > +		pos += fill;
> > > +		ret = system(buf);
> > >  
> > >  		if (ret)
> > >  			break;
> > > diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> > > index 0427791..d9aa973 100644
> > > --- a/src/lxc/lxccontainer.c
> > > +++ b/src/lxc/lxccontainer.c
> > > @@ -806,7 +806,7 @@ static struct bdev *do_bdev_create(struct
> > > lxc_container *c, const char *type, /* if we are not root, chown
> > > the rootfs dir to root in the
> > >  	 * target uidmap */
> > >  
> > > -	if (geteuid() != 0) {
> > > +	if (geteuid() != 0 || (c->lxc_conf
> > > && !lxc_list_empty(&c->lxc_conf->id_map))) { if
> > > (chown_mapped_root(bdev->dest, c->lxc_conf) < 0) { ERROR("Error
> > > chowning %s to container root", bdev->dest); bdev_put(bdev);
> > > @@ -992,7 +992,7 @@ static bool create_run_template(struct
> > > lxc_container *c, char *tpath, bool quiet
> > >  		 * and we append "--mapped-uid x", where x is the
> > > mapped uid
> > >  		 * for our geteuid()
> > >  		 */
> > > -		if (geteuid() != 0
> > > && !lxc_list_empty(&conf->id_map)) {
> > > +		if (!lxc_list_empty(&conf->id_map)) {
> > >  			int n2args = 1;
> > >  			char txtuid[20];
> > >  			char txtgid[20];
> > > @@ -1450,7 +1450,7 @@ static inline bool enter_to_ns(struct
> > > lxc_container *c) { init_pid = c->init_pid(c);
> > >  
> > >  	/* Switch to new userns */
> > > -	if (geteuid() && access("/proc/self/ns/user", F_OK) == 0) {
> > > +	if ((geteuid() != 0 || (c->lxc_conf
> > > && !lxc_list_empty(&c->lxc_conf->id_map))) &&
> > > access("/proc/self/ns/user", F_OK) == 0) { ret =
> > > snprintf(new_userns_path, MAXPATHLEN, "/proc/%d/ns/user",
> > > init_pid); if (ret < 0 || ret >= MAXPATHLEN) goto out; -- 
> > > 1.9.0
> > > 
> > 
> 

-- 
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20140226/4efaf2cc/attachment-0001.pgp>


More information about the lxc-devel mailing list