[lxc-devel] [PATCH 3/3] lxc-attach: User namespaces: Use init's user & group id when attaching

[Serge Hallyn]

Quoting Christian Seiler (christian at iwakd.de):
> When attaching to a container with a user namespace, try to detect the
> user and group ids of init via /proc and attach as that same user. Only
> if that is unsuccessful, fall back to (0, 0).
> ---
>  src/lxc/attach.c     |   53 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/lxc/attach.h     |    2 ++
>  src/lxc/lxc_attach.c |   15 ++++++++++----
>  3 files changed, 66 insertions(+), 4 deletions(-)
> 
> diff --git a/src/lxc/attach.c b/src/lxc/attach.c
> index 88356e1..399ec36 100644
> --- a/src/lxc/attach.c
> +++ b/src/lxc/attach.c
> @@ -479,3 +479,56 @@ struct passwd *lxc_attach_getpwuid(uid_t uid)
>  		exit(-1);
>  	}
>  }
> +
> +int lxc_attach_get_init_uidgid(uid_t* init_uid, gid_t* init_gid)
> +{
> +	FILE *proc_file;
> +	char proc_fn[MAXPATHLEN];
> +	char *line = NULL;
> +	size_t line_bufsz = 0;
> +	int ret;
> +	long value = -1;
> +	uid_t uid = (uid_t)-1;
> +	gid_t gid = (gid_t)-1;
> +
> +	/* read capabilities */

What does this comment mean?

> +	snprintf(proc_fn, MAXPATHLEN, "/proc/%d/status", 1);
> +
> +	proc_file = fopen(proc_fn, "r");
> +	if (!proc_file)
> +		return -1;
> +
> +	while (getline(&line, &line_bufsz, proc_file) != -1) {
> +		/* format is: real, effective, saved set user, fs
> +		 * we only care about real uid
> +		 */
> +		ret = sscanf(line, "Uid: %ld", &value);
> +		if (ret != EOF && ret > 0) {
> +			uid = (uid_t) value;
> +		} else {
> +			ret = sscanf(line, "Gid: %ld", &value);
> +			if (ret != EOF && ret > 0)
> +				gid = (gid_t) value;
> +		}
> +		if (uid != (uid_t)-1 && gid != (gid_t)-1)
> +			break;
> +	}
> +
> +	fclose(proc_file);
> +	free(line);
> +
> +	/* only override arguments if we found something */
> +	if (uid != (uid_t)-1)
> +		*init_uid = uid;
> +	if (gid != (gid_t)-1)
> +		*init_gid = gid;
> +
> +	/* TODO: we should also parse supplementary groups and use
> +	 * setgroups() to set them */
> +
> +	/* at least some entries were not found, we return error */
> +	if (uid == (uid_t)-1 || gid == (gid_t)-1)
> +		return -1;
> +
> +	return 0;
> +}
> diff --git a/src/lxc/attach.h b/src/lxc/attach.h
> index 90e693a..1e9b87e 100644
> --- a/src/lxc/attach.h
> +++ b/src/lxc/attach.h
> @@ -41,4 +41,6 @@ extern int lxc_attach_drop_privs(struct lxc_proc_context_info *ctx);
>  struct passwd;
>  extern struct passwd *lxc_attach_getpwuid(uid_t uid);
>  
> +extern int lxc_attach_get_init_uidgid(uid_t* init_uid, gid_t* init_gid);
> +
>  #endif
> diff --git a/src/lxc/lxc_attach.c b/src/lxc/lxc_attach.c
> index 9c86ffe..cdc1601 100644
> --- a/src/lxc/lxc_attach.c
> +++ b/src/lxc/lxc_attach.c
> @@ -417,13 +417,20 @@ int main(int argc, char *argv[])
>  		lxc_sync_fini(handler);
>  
>  		if (namespace_flags & CLONE_NEWUSER) {
> -			/* XXX FIXME this should get the uid of the container init and setuid to that */
> -			/* XXX FIXME or perhaps try to map in the lxc-attach caller's uid? */
> -			if (setgid(0)) {
> +			uid_t init_uid = 0;
> +			gid_t init_gid = 0;
> +
> +			/* ignore errors, we will fall back to root in that case
> +			 * (/proc was not mounted etc.)
> +			 */

It's fine if you want to do that, but then please have
lxc_attach_get_init_uidgid() return void, drop this comment
and drop the cast below :)

But also...  you don't actually re-try with init_gid/init_uid of 0.
If lxc_attach_get_init_uidgid() set one of those to -1, then you'll
just fail here.

> +			(void) lxc_attach_get_init_uidgid(&init_uid, &init_gid);
> +
> +			/* try to set the uid/gid combination */
> +			if (setgid(init_gid)) {
>  				SYSERROR("switching to container gid");
>  				return -1;
>  			}
> -			if (setuid(0)) {
> +			if (setuid(init_uid)) {
>  				SYSERROR("switching to container uid");
>  				return -1;
>  			}
> -- 
> 1.7.10.4
>