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

Dwight Engen dwight.engen at oracle.com
Thu Feb 27 20:42:56 UTC 2014


On Thu, 27 Feb 2014 15:05:31 -0500
Stéphane Graber <stgraber at ubuntu.com> 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.
> 
> This also updates lxc_usernsexec to use the same function as the rest
> of LXC, centralizing all the userns switch in a single function.
> 
> That function now detects the presence of newuidmap and newgidmap on
> the system, if they are present, they will be used for containers
> created as either user or root. If they're not and the user isn't
> root, an error is shown. If they're not and the user is root, LXC
> will directly set the uid_map and gid_map values.
> 
> All that should allow for a consistent experience as well as
> supporting distributions that don't yet ship newuidmap/newgidmap.
> 
> To make things simpler in the future, an helper function "on_path" is
> also introduced and used to detect the presence of newuidmap and
> newgidmap.
> 
> Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>

Thanks Stéphane, I know this was more work than v1. For the
non-new[ug]idmap case:

Tested-by: Dwight Engen <dwight.engen at oracle.com>

> ---
>  src/lxc/conf.c           |  15 ++--
>  src/lxc/lxc_usernsexec.c | 214
> ++++++++++++++---------------------------------
> src/lxc/lxccontainer.c   |   6 +- src/lxc/utils.c          |  34
> ++++++++ src/lxc/utils.h          |   1 +
>  5 files changed, 113 insertions(+), 157 deletions(-)
> 
> diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> index b6bc414..d99659a 100644
> --- a/src/lxc/conf.c
> +++ b/src/lxc/conf.c
> @@ -3164,7 +3164,12 @@ 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);
> +	int use_shadow = (on_path("newuidmap") &&
> on_path("newuidmap")); +
> +	if (!use_shadow && geteuid()) {
> +		ERROR("Missing newuidmap/newgidmap");
> +		return -1;
> +	}
>  
>  	for(type = ID_TYPE_UID; type <= ID_TYPE_GID; type++) {
>  		int left, fill;
> @@ -3175,7 +3180,7 @@ int lxc_map_ids(struct lxc_list *idmap, pid_t
> pid) return -ENOMEM;
>  		}
>  		pos = buf;
> -		if (!am_root)
> +		if (use_shadow)
>  			pos += sprintf(buf, "new%cidmap %d",
>  				type == ID_TYPE_UID ? 'u' : 'g',
>  				pid);
> @@ -3189,9 +3194,9 @@ 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 ? "" : " ",
> +					use_shadow ? " " : "",
>  					map->nsid, map->hostid,
> map->range,
> -					am_root ? "\n" : "");
> +					use_shadow ? "" : "\n");
>  			if (fill <= 0 || fill >= left)
>  				SYSERROR("snprintf failed, too many
> mappings"); pos += fill;
> @@ -3199,7 +3204,7 @@ int lxc_map_ids(struct lxc_list *idmap, pid_t
> pid) if (!had_entry)
>  			continue;
>  
> -		if (am_root) {
> +		if (!use_shadow) {
>  			ret = write_id_mapping(type, pid, buf,
> pos-buf); } else {
>  			left = 4096 - (pos - buf);
> diff --git a/src/lxc/lxc_usernsexec.c b/src/lxc/lxc_usernsexec.c
> index 2efc687..f5b7fe0 100644
> --- a/src/lxc/lxc_usernsexec.c
> +++ b/src/lxc/lxc_usernsexec.c
> @@ -41,6 +41,7 @@
>  #include <pwd.h>
>  #include <grp.h>
>  
> +#include "conf.h"
>  #include "namespace.h"
>  #include "utils.h"
>  
> @@ -131,19 +132,7 @@ static int do_child(void *vargv)
>  	return -1;
>  }
>  
> -struct id_map {
> -	char which; // b or u or g
> -	long host_id, ns_id, range;
> -	struct id_map *next;
> -};
> -
> -static struct id_map default_map = {
> -	.which = 'b',
> -	.host_id = 100000,
> -	.ns_id = 0,
> -	.range = 10000,
> -};
> -static struct id_map *active_map = &default_map;
> +static struct lxc_list active_map;
>  
>  /*
>   * given a string like "b:0:100000:10", map both uids and gids
> @@ -152,28 +141,51 @@ static struct id_map *active_map = &default_map;
>  static int parse_map(char *map)
>  {
>  	struct id_map *newmap;
> -    int ret;
> +	struct lxc_list *tmp = NULL;
> +	int ret;
> +	int i;
> +	char types[2] = {'u', 'g'};
> +	char which;
> +	long host_id, ns_id, range;
>  
>  	if (!map)
>  		return -1;
> -	newmap = malloc(sizeof(*newmap));
> -	if (!newmap)
> -		return -1;
> -	ret = sscanf(map, "%c:%ld:%ld:%ld", &newmap->which,
> &newmap->ns_id, &newmap->host_id, &newmap->range); +
> +	ret = sscanf(map, "%c:%ld:%ld:%ld", &which, &ns_id,
> &host_id, &range); if (ret != 4)
> -		goto out_free_map;
> -	if (newmap->which != 'b' && newmap->which != 'u' &&
> newmap->which != 'g')
> -		goto out_free_map;
> -	if (active_map != &default_map)
> -		newmap->next = active_map;
> -	else
> -		newmap->next = NULL;
> -	active_map = newmap;
> -	return 0;
> +		return -1;
>  
> -out_free_map:
> -	free(newmap);
> -	return -1;
> +	if (which != 'b' && which != 'u' && which != 'g')
> +		return -1;
> +
> +	for (i = 0; i < 2; i++) {
> +		if (which != types[i] && which != 'b')
> +			continue;
> +
> +		newmap = malloc(sizeof(*newmap));
> +		if (!newmap)
> +			return -1;
> +
> +		newmap->hostid = host_id;
> +		newmap->nsid = ns_id;
> +		newmap->range = range;
> +
> +		if (types[i] == 'u')
> +			newmap->idtype = ID_TYPE_UID;
> +		else
> +			newmap->idtype = ID_TYPE_GID;
> +
> +		tmp = malloc(sizeof(*tmp));
> +		if (!tmp) {
> +			free(newmap);
> +			return -1;
> +		}
> +
> +		tmp->elem = newmap;
> +		lxc_list_add_tail(&active_map, tmp);
> +	}
> +
> +	return 0;
>  }
>  
>  /*
> @@ -185,12 +197,13 @@ out_free_map:
>   * gid, because otherwise we're not sure which entries the user
>   * wanted.
>   */
> -static int read_default_map(char *fnam, char which, char *username)
> +static int read_default_map(char *fnam, int which, char *username)
>  {
>  	FILE *fin;
>  	char *line = NULL;
>  	size_t sz = 0;
>  	struct id_map *newmap;
> +	struct lxc_list *tmp = NULL;
>  	char *p1, *p2;
>  
>  	fin = fopen(fnam, "r");
> @@ -213,15 +226,21 @@ static int read_default_map(char *fnam, char
> which, char *username) free(line);
>  			return -1;
>  		}
> -		newmap->host_id = atol(p1+1);
> +		newmap->hostid = atol(p1+1);
>  		newmap->range = atol(p2+1);
> -		newmap->ns_id = 0;
> -		newmap->which = which;
> -		if (active_map != &default_map)
> -			newmap->next = active_map;
> -		else
> -			newmap->next = NULL;
> -		active_map = newmap;
> +		newmap->nsid = 0;
> +		newmap->idtype = which;
> +
> +		tmp = malloc(sizeof(*tmp));
> +		if (!tmp) {
> +			fclose(fin);
> +			free(line);
> +			free(newmap);
> +			return -1;
> +		}
> +
> +		tmp->elem = newmap;
> +		lxc_list_add_tail(&active_map, tmp);
>  		break;
>  	}
>  
> @@ -238,119 +257,13 @@ static int find_default_map(void)
>  	struct passwd *p = getpwuid(getuid());
>  	if (!p)
>  		return -1;
> -	if (read_default_map(subuidfile, 'u', p->pw_name) < 0)
> +	if (read_default_map(subuidfile, ID_TYPE_UID, p->pw_name) <
> 0) return -1;
> -	if (read_default_map(subgidfile, 'g', p->pw_name) < 0)
> +	if (read_default_map(subgidfile, ID_TYPE_GID, p->pw_name) <
> 0) return -1;
>      return 0;
>  }
>  
> -static int run_cmd(char **argv)
> -{
> -    int status;
> -	pid_t pid = fork();
> -
> -	if (pid < 0)
> -		return pid;
> -	if (pid == 0) {
> -		execvp(argv[0], argv);
> -		perror("exec failed");
> -		exit(1);
> -	}
> -	if (waitpid(pid, &status, __WALL) < 0) {
> -        perror("waitpid");
> -		return -1;
> -	}
> -
> -	return WEXITSTATUS(status);
> -}
> -
> -static int map_child_uids(int pid, struct id_map *map)
> -{
> -	char **uidargs = NULL, **gidargs = NULL;
> -	char **newuidargs = NULL, **newgidargs = NULL;
> -	int i, nuargs = 2, ngargs = 2, ret = -1;
> -	struct id_map *m;
> -
> -	uidargs = malloc(3 * sizeof(*uidargs));
> -	if (uidargs == NULL)
> -		return -1;
> -	gidargs = malloc(3 * sizeof(*gidargs));
> -	if (gidargs == NULL) {
> -		free(uidargs);
> -		return -1;
> -	}
> -	uidargs[0] = malloc(10);
> -	gidargs[0] = malloc(10);
> -	uidargs[1] = malloc(21);
> -	gidargs[1] = malloc(21);
> -	uidargs[2] = NULL;
> -	gidargs[2] = NULL;
> -	if (!uidargs[0] || !uidargs[1] || !gidargs[0] || !gidargs[1])
> -		goto out;
> -	sprintf(uidargs[0], "newuidmap");
> -	sprintf(gidargs[0], "newgidmap");
> -	sprintf(uidargs[1], "%d", pid);
> -	sprintf(gidargs[1], "%d", pid);
> -	for (m=map; m; m = m->next) {
> -		if (m->which == 'b' || m->which == 'u') {
> -			nuargs += 3;
> -			newuidargs = realloc(uidargs, (nuargs+1) *
> sizeof(*uidargs));
> -			if (!newuidargs)
> -				goto out;
> -			uidargs = newuidargs;
> -			uidargs[nuargs - 3] = malloc(21);
> -			uidargs[nuargs - 2] = malloc(21);
> -			uidargs[nuargs - 1] = malloc(21);
> -			if (!uidargs[nuargs-3] || !uidargs[nuargs-2]
> || !uidargs[nuargs-1])
> -				goto out;
> -			sprintf(uidargs[nuargs - 3], "%ld",
> m->ns_id);
> -			sprintf(uidargs[nuargs - 2], "%ld",
> m->host_id);
> -			sprintf(uidargs[nuargs - 1], "%ld",
> m->range);
> -			uidargs[nuargs] = NULL;
> -		}
> -		if (m->which == 'b' || m->which == 'g') {
> -			ngargs += 3;
> -			newgidargs = realloc(gidargs, (ngargs+1) *
> sizeof(*gidargs));
> -			if (!newgidargs)
> -				goto out;
> -			gidargs = newgidargs;
> -			gidargs[ngargs - 3] = malloc(21);
> -			gidargs[ngargs - 2] = malloc(21);
> -			gidargs[ngargs - 1] = malloc(21);
> -			if (!gidargs[ngargs-3] || !gidargs[ngargs-2]
> || !gidargs[ngargs-1])
> -				goto out;
> -			sprintf(gidargs[ngargs - 3], "%ld",
> m->ns_id);
> -			sprintf(gidargs[ngargs - 2], "%ld",
> m->host_id);
> -			sprintf(gidargs[ngargs - 1], "%ld",
> m->range);
> -			gidargs[ngargs] = NULL;
> -		}
> -	}
> -
> -	ret = -2;
> -	// exec newuidmap
> -	if (nuargs > 2 && run_cmd(uidargs) != 0) {
> -		fprintf(stderr, "Error mapping uids\n");
> -		goto out;
> -	}
> -	// exec newgidmap
> -	if (ngargs > 2 && run_cmd(gidargs) != 0) {
> -		fprintf(stderr, "Error mapping gids\n");
> -		goto out;
> -	}
> -	ret = 0;
> -
> -out:
> -	for (i=0; i<nuargs; i++)
> -		free(uidargs[i]);
> -	for (i=0; i<ngargs; i++)
> -		free(gidargs[i]);
> -	free(uidargs);
> -	free(gidargs);
> -
> -	return ret;
> -}
> -
>  int main(int argc, char *argv[])
>  {
>  	int c;
> @@ -371,6 +284,8 @@ int main(int argc, char *argv[])
>  		exit(1);
>  	}
>  
> +	lxc_list_init(&active_map);
> +
>  	while ((c = getopt(argc, argv, "m:h")) != EOF) {
>  		switch (c) {
>  			case 'm': if (parse_map(optarg))
> usage(argv[0]); break; @@ -380,7 +295,7 @@ int main(int argc, char
> *argv[]) }
>  	};
>  
> -	if (active_map == &default_map) {
> +	if (lxc_list_empty(&active_map)) {
>  		if (find_default_map()) {
>  			fprintf(stderr, "You have no allocated
> subuids or subgids\n"); exit(1);
> @@ -437,7 +352,8 @@ int main(int argc, char *argv[])
>  	}
>  
>  	buf[0] = '1';
> -	if (map_child_uids(pid, active_map)) {
> +
> +	if (lxc_map_ids(&active_map, pid)) {
>  		fprintf(stderr, "error mapping child\n");
>  		ret = 0;
>  	}
> 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; diff --git
> a/src/lxc/utils.c b/src/lxc/utils.c index da7c3b4..90bdddb 100644
> --- a/src/lxc/utils.c
> +++ b/src/lxc/utils.c
> @@ -1234,3 +1234,37 @@ int detect_shared_rootfs(void)
>  	fclose(f);
>  	return 0;
>  }
> +
> +bool on_path(char *cmd) {
> +	char *path = NULL;
> +	char *entry = NULL;
> +	char cmdpath[MAXPATHLEN];
> +	int ret;
> +
> +	path = getenv("PATH");
> +	if (!path)
> +		return false;
> +
> +	path = strdup(path);
> +	if (!path)
> +		return false;
> +
> +	entry = strtok(path, ":");
> +	while (entry) {
> +		ret = snprintf(cmdpath, MAXPATHLEN, "%s/%s", entry,
> cmd); +
> +		if (ret < 0 || ret >= MAXPATHLEN)
> +			goto next_loop;
> +
> +		if (access(cmdpath, X_OK) == 0) {
> +			free(path);
> +			return true;
> +		}
> +
> +next_loop:
> +		entry = strtok(NULL, ":");
> +	}
> +
> +	free(path);
> +	return false;
> +}
> diff --git a/src/lxc/utils.h b/src/lxc/utils.h
> index dcf0e34..978f586 100644
> --- a/src/lxc/utils.h
> +++ b/src/lxc/utils.h
> @@ -278,3 +278,4 @@ uint64_t fnv_64a_buf(void *buf, size_t len,
> uint64_t hval); #endif
>  
>  int detect_shared_rootfs(void);
> +bool on_path(char *cmd);



More information about the lxc-devel mailing list