* [PATCH] nsenter: Allow selecting the uid and gid to be used in the entered userns
@ 2013-04-25 19:11 Richard Weinberger
2013-04-26 1:01 ` Eric W. Biederman
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Richard Weinberger @ 2013-04-25 19:11 UTC (permalink / raw)
To: kzak; +Cc: ebiederm, util-linux, davidlohr.bueso, 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] nsenter: Allow selecting the uid and gid to be used in the entered userns
2013-04-25 19:11 [PATCH] nsenter: Allow selecting the uid and gid to be used in the entered userns Richard Weinberger
@ 2013-04-26 1:01 ` Eric W. Biederman
2013-04-26 5:39 ` Richard Weinberger
2013-05-06 15:39 ` Karel Zak
2013-06-18 8:41 ` Karel Zak
2014-02-06 13:00 ` Karel Zak
2 siblings, 2 replies; 6+ messages in thread
From: Eric W. Biederman @ 2013-04-26 1:01 UTC (permalink / raw)
To: Richard Weinberger; +Cc: kzak, util-linux, davidlohr.bueso
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();
> }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nsenter: Allow selecting the uid and gid to be used in the entered userns
2013-04-26 1:01 ` Eric W. Biederman
@ 2013-04-26 5:39 ` Richard Weinberger
2013-05-06 15:39 ` Karel Zak
1 sibling, 0 replies; 6+ messages in thread
From: Richard Weinberger @ 2013-04-26 5:39 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: kzak, util-linux, davidlohr.bueso
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nsenter: Allow selecting the uid and gid to be used in the entered userns
2013-04-26 1:01 ` Eric W. Biederman
2013-04-26 5:39 ` Richard Weinberger
@ 2013-05-06 15:39 ` Karel Zak
1 sibling, 0 replies; 6+ messages in thread
From: Karel Zak @ 2013-05-06 15:39 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: Richard Weinberger, util-linux, davidlohr.bueso
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nsenter: Allow selecting the uid and gid to be used in the entered userns
2013-04-25 19:11 [PATCH] nsenter: Allow selecting the uid and gid to be used in the entered userns Richard Weinberger
2013-04-26 1:01 ` Eric W. Biederman
@ 2013-06-18 8:41 ` Karel Zak
2014-02-06 13:00 ` Karel Zak
2 siblings, 0 replies; 6+ messages in thread
From: Karel Zak @ 2013-06-18 8:41 UTC (permalink / raw)
To: Richard Weinberger; +Cc: ebiederm, util-linux, davidlohr.bueso
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nsenter: Allow selecting the uid and gid to be used in the entered userns
2013-04-25 19:11 [PATCH] nsenter: Allow selecting the uid and gid to be used in the entered userns Richard Weinberger
2013-04-26 1:01 ` Eric W. Biederman
2013-06-18 8:41 ` Karel Zak
@ 2014-02-06 13:00 ` Karel Zak
2 siblings, 0 replies; 6+ messages in thread
From: Karel Zak @ 2014-02-06 13:00 UTC (permalink / raw)
To: Richard Weinberger; +Cc: ebiederm, util-linux, davidlohr.bueso
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-02-06 13:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-25 19:11 [PATCH] nsenter: Allow selecting the uid and gid to be used in the entered userns Richard Weinberger
2013-04-26 1:01 ` Eric W. Biederman
2013-04-26 5:39 ` Richard Weinberger
2013-05-06 15:39 ` Karel Zak
2013-06-18 8:41 ` Karel Zak
2014-02-06 13:00 ` Karel Zak
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox