[PATCH] nsenter: Allow selecting the uid and gid to be used in the entered userns

[PATCH] nsenter: Allow selecting the uid and gid to be used in the entered userns

[Richard Weinberger]

Using -S (--setuid) and -G (--setgid) one can select
the uid/gid which will be used in the entered user namespace.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 sys-utils/nsenter.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/sys-utils/nsenter.c b/sys-utils/nsenter.c
index 106349c..7b2255c 100644
--- a/sys-utils/nsenter.c
+++ b/sys-utils/nsenter.c
@@ -72,6 +72,8 @@ static void usage(int status)
 	fputs(_(" -n, --net   [=<file>]  enter network namespace\n"), out);
 	fputs(_(" -p, --pid   [=<file>]  enter pid namespace\n"), out);
 	fputs(_(" -U, --user  [=<file>]  enter user namespace\n"), out);
+	fputs(_(" -S, --setuid <uid>     set uid in user namespace\n"), out);
+	fputs(_(" -G, --setgid <gid>     set gid in user namespace\n"), out);
 	fputs(_(" -r, --root  [=<dir>]   set the root directory\n"), out);
 	fputs(_(" -w, --wd    [=<dir>]   set the working directory\n"), out);
 	fputs(_(" -F, --no-fork          do not fork before exec'ing <program>\n"), out);
@@ -169,6 +171,8 @@ int main(int argc, char *argv[])
 		{ "net", optional_argument, NULL, 'n' },
 		{ "pid", optional_argument, NULL, 'p' },
 		{ "user", optional_argument, NULL, 'U' },
+		{ "setuid", required_argument, NULL, 'S' },
+		{ "setgid", required_argument, NULL, 'G' },
 		{ "root", optional_argument, NULL, 'r' },
 		{ "wd", optional_argument, NULL, 'w' },
 		{ "no-fork", no_argument, NULL, 'F' },
@@ -179,6 +183,10 @@ int main(int argc, char *argv[])
 	int c, namespaces = 0;
 	bool do_rd = false, do_wd = false;
 	int do_fork = -1; /* unknown yet */
+	bool set_uid = false;
+	bool set_gid = false;
+	uid_t uid = 0;
+	gid_t gid = 0;
 
 	setlocale(LC_MESSAGES, "");
 	bindtextdomain(PACKAGE, LOCALEDIR);
@@ -186,7 +194,7 @@ int main(int argc, char *argv[])
 	atexit(close_stdout);
 
 	while ((c =
-		getopt_long(argc, argv, "hVt:m::u::i::n::p::U::r::w::F",
+		getopt_long(argc, argv, "hVt:m::u::i::n::p::U::S:G:r::w::F",
 			    longopts, NULL)) != -1) {
 		switch (c) {
 		case 'h':
@@ -234,6 +242,14 @@ int main(int argc, char *argv[])
 			else
 				namespaces |= CLONE_NEWUSER;
 			break;
+		case 'S':
+			uid = strtoul_or_err(optarg, _("failed to parse uid"));
+			set_uid = true;
+			break;
+		case 'G':
+			gid = strtoul_or_err(optarg, _("failed to parse gid"));
+			set_gid = true;
+			break;
 		case 'F':
 			do_fork = 0;
 			break;
@@ -319,5 +335,15 @@ int main(int argc, char *argv[])
 		execvp(argv[optind], argv + optind);
 		err(EXIT_FAILURE, _("failed to execute %s"), argv[optind]);
 	}
+
+	if (namespaces & CLONE_NEWUSER) {
+		if (set_uid)
+			if (setuid(uid) < 0)
+				err(EXIT_FAILURE, _("setuid failed"));
+		if (set_gid)
+			if (setgid(gid) < 0)
+				err(EXIT_FAILURE, _("setgid failed"));
+	}
+
 	exec_shell();
 }
-- 
1.8.1.4

Re: [PATCH] nsenter: Allow selecting the uid and gid to be used in the entered userns

[Eric W. Biederman]

Richard Weinberger <richard@nod.at> writes:

> Using -S (--setuid) and -G (--setgid) one can select
> the uid/gid which will be used in the entered user namespace.

There is definitely utility here.  I don't have a strong preference but
I am inclined to suggest that you remove the set_uid and set_gid
variables, and unconditionally call setuid and setgid when entering
a user namespace.

The only case where setting the uid and gid in a user namespace could be
problematic is where the uid/gid mapping has not been set.  And entering
or running a user namespace with a mapping not setup is very unlikely to
be the common practice.

Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>

>
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
>  sys-utils/nsenter.c | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/sys-utils/nsenter.c b/sys-utils/nsenter.c
> index 106349c..7b2255c 100644
> --- a/sys-utils/nsenter.c
> +++ b/sys-utils/nsenter.c
> @@ -72,6 +72,8 @@ static void usage(int status)
>  	fputs(_(" -n, --net   [=<file>]  enter network namespace\n"), out);
>  	fputs(_(" -p, --pid   [=<file>]  enter pid namespace\n"), out);
>  	fputs(_(" -U, --user  [=<file>]  enter user namespace\n"), out);
> +	fputs(_(" -S, --setuid <uid>     set uid in user namespace\n"), out);
> +	fputs(_(" -G, --setgid <gid>     set gid in user namespace\n"), out);
>  	fputs(_(" -r, --root  [=<dir>]   set the root directory\n"), out);
>  	fputs(_(" -w, --wd    [=<dir>]   set the working directory\n"), out);
>  	fputs(_(" -F, --no-fork          do not fork before exec'ing <program>\n"), out);
> @@ -169,6 +171,8 @@ int main(int argc, char *argv[])
>  		{ "net", optional_argument, NULL, 'n' },
>  		{ "pid", optional_argument, NULL, 'p' },
>  		{ "user", optional_argument, NULL, 'U' },
> +		{ "setuid", required_argument, NULL, 'S' },
> +		{ "setgid", required_argument, NULL, 'G' },
>  		{ "root", optional_argument, NULL, 'r' },
>  		{ "wd", optional_argument, NULL, 'w' },
>  		{ "no-fork", no_argument, NULL, 'F' },
> @@ -179,6 +183,10 @@ int main(int argc, char *argv[])
>  	int c, namespaces = 0;
>  	bool do_rd = false, do_wd = false;
>  	int do_fork = -1; /* unknown yet */
> +	bool set_uid = false;
> +	bool set_gid = false;
> +	uid_t uid = 0;
> +	gid_t gid = 0;
>  
>  	setlocale(LC_MESSAGES, "");
>  	bindtextdomain(PACKAGE, LOCALEDIR);
> @@ -186,7 +194,7 @@ int main(int argc, char *argv[])
>  	atexit(close_stdout);
>  
>  	while ((c =
> -		getopt_long(argc, argv, "hVt:m::u::i::n::p::U::r::w::F",
> +		getopt_long(argc, argv, "hVt:m::u::i::n::p::U::S:G:r::w::F",
>  			    longopts, NULL)) != -1) {
>  		switch (c) {
>  		case 'h':
> @@ -234,6 +242,14 @@ int main(int argc, char *argv[])
>  			else
>  				namespaces |= CLONE_NEWUSER;
>  			break;
> +		case 'S':
> +			uid = strtoul_or_err(optarg, _("failed to parse uid"));
> +			set_uid = true;
> +			break;
> +		case 'G':
> +			gid = strtoul_or_err(optarg, _("failed to parse gid"));
> +			set_gid = true;
> +			break;
>  		case 'F':
>  			do_fork = 0;
>  			break;
> @@ -319,5 +335,15 @@ int main(int argc, char *argv[])
>  		execvp(argv[optind], argv + optind);
>  		err(EXIT_FAILURE, _("failed to execute %s"), argv[optind]);
>  	}
> +
> +	if (namespaces & CLONE_NEWUSER) {
> +		if (set_uid)
> +			if (setuid(uid) < 0)
> +				err(EXIT_FAILURE, _("setuid failed"));
> +		if (set_gid)
> +			if (setgid(gid) < 0)
> +				err(EXIT_FAILURE, _("setgid failed"));
> +	}
> +
>  	exec_shell();
>  }

Re: [PATCH] nsenter: Allow selecting the uid and gid to be used in the entered userns

[Richard Weinberger]

Am 26.04.2013 03:01, schrieb Eric W. Biederman:
> Richard Weinberger <richard@nod.at> writes:
>
>> Using -S (--setuid) and -G (--setgid) one can select
>> the uid/gid which will be used in the entered user namespace.
>
> There is definitely utility here.  I don't have a strong preference but
> I am inclined to suggest that you remove the set_uid and set_gid
> variables, and unconditionally call setuid and setgid when entering
> a user namespace.

I thought about that too. But this would introduce a behavior change.
Karel, are you fine with such a change?

Thanks,
//richard

Re: [PATCH] nsenter: Allow selecting the uid and gid to be used in the entered userns

[Karel Zak]

On Thu, Apr 25, 2013 at 06:01:02PM -0700, Eric W. Biederman wrote:
> Richard Weinberger <richard@nod.at> writes:
> 
> > Using -S (--setuid) and -G (--setgid) one can select
> > the uid/gid which will be used in the entered user namespace.
> 
> There is definitely utility here.  I don't have a strong preference but
> I am inclined to suggest that you remove the set_uid and set_gid
> variables, and unconditionally call setuid and setgid when entering
> a user namespace.

 I agree.

> > --- a/sys-utils/nsenter.c
> > +++ b/sys-utils/nsenter.c
> > @@ -72,6 +72,8 @@ static void usage(int status)
> >  	fputs(_(" -n, --net   [=<file>]  enter network namespace\n"), out);
> >  	fputs(_(" -p, --pid   [=<file>]  enter pid namespace\n"), out);
> >  	fputs(_(" -U, --user  [=<file>]  enter user namespace\n"), out);
> > +	fputs(_(" -S, --setuid <uid>     set uid in user namespace\n"), out);
> > +	fputs(_(" -G, --setgid <gid>     set gid in user namespace\n"), out);

 Richard, update man page too. Please.

> > @@ -319,5 +335,15 @@ int main(int argc, char *argv[])
> >  		execvp(argv[optind], argv + optind);
> >  		err(EXIT_FAILURE, _("failed to execute %s"), argv[optind]);
> >  	}
> > +
> > +	if (namespaces & CLONE_NEWUSER) {
> > +		if (set_uid)
> > +			if (setuid(uid) < 0)
> > +				err(EXIT_FAILURE, _("setuid failed"));
> > +		if (set_gid)
> > +			if (setgid(gid) < 0)
> > +				err(EXIT_FAILURE, _("setgid failed"));
> > +	}
> > +
> >  	exec_shell();

 Why do call setuid/gid() only for shell? Would be better to move all
 this code before the block with execvp() to call setuid/gid() for
 all execvp() as well as for exec_shell()?

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH] nsenter: Allow selecting the uid and gid to be used in the entered userns

[Karel Zak]

On Thu, Apr 25, 2013 at 09:11:40PM +0200, Richard Weinberger wrote:
>  sys-utils/nsenter.c | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)

 Applied (with some changes), thanks.

    Karel
-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH] nsenter: Allow selecting the uid and gid to be used in the entered userns

[Karel Zak]

On Thu, Apr 25, 2013 at 09:11:40PM +0200, Richard Weinberger wrote:
> +	if (namespaces & CLONE_NEWUSER) {
> +		if (set_uid)
> +			if (setuid(uid) < 0)
> +				err(EXIT_FAILURE, _("setuid failed"));
> +		if (set_gid)
> +			if (setgid(gid) < 0)
> +				err(EXIT_FAILURE, _("setgid failed"));

 Ah.. we make a bug here, it has to be in reverse order (gid and then uid).

 The another question is what about supplementary groups? Do we care
 about it? If yes, we need initgroups(), otherwise it would be
 probably better to drop all by setgroups(0, NULL).
 
 I guess it's over-engineering to try to reimplement su(1) within nsenter,
 so drop the supplementary group is the right way.
 
    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com