[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