[lxc-devel] [PATCH] Fix unprivileged containers started by root
Serge Hallyn
serge.hallyn at ubuntu.com
Thu Feb 27 15:18:17 UTC 2014
Quoting Stéphane Graber (stgraber at ubuntu.com):
> 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)
So I guess we should keep this path. Perhaps under #if !HAVE_NEWUIDMAP?
However, we have room for improvement by having lxc_usernsexec use
lxc_map_ids(), so we at least only have to do this in one place.
Then create_run_template() will automatically do the right thing
because it is calling lxc-usernsexec which will now work for root.
> -{
> - 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))) {
I think we should drop the geteuid() check. Instead we should ensure
that before we get to thsi point, if geteuid() != 0 id_map had better
not be empty. Otherwise we're in a bad condition anyway.
> 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)) {
Like here :)
> 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
>
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
More information about the lxc-devel
mailing list