Re: [Xen-devel] [PATCH v5.1 7/8] os-posix: Provide new -runas : facility
Stefano Stabellini writes ("Re: [PATCH v5.1 7/8] os-posix: Provide new -runas : facility"): > CC'ing the maintainers (scripts/get_maintainer.pl is your friend) I don't know what your scripts/get_maintainer.pl does, but mine says: get_maintainer.pl: No maintainers found, printing recent contributors. get_maintainer.pl: Do not blindly cc: them on patches! Use common sense. Anthony PERARD (commit_signer:1/2=50%) Paolo Bonzini (commit_signer:1/2=50%,commit_signer:11/57=19%) Ian Jackson (commit_signer:1/2=50%) Michael Tokarev (commit_signer:12/57=21%) Eric Blake (commit_signer:10/57=18%) Thomas Huth (commit_signer:8/57=14%) Markus Armbruster (commit_signer:8/57=14%) qemu-de...@nongnu.org (open list:POSIX) I have added Paolo, Markus and Daniel Berrange to the CCs of my patch on the basis that they have commented already... Thanks, Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5.1 7/8] os-posix: Provide new -runas : facility
CC'ing the maintainers (scripts/get_maintainer.pl is your friend) On Fri, 20 Oct 2017, Ian Jackson wrote: > This allows the caller to specify a uid and gid to use, even if there > is no corresponding password entry. This will be useful in certain > Xen configurations. > > We don't support just -runas because: (i) deprivileging without > calling setgroups would be ineffective (ii) given only a uid we don't > know what gid we ought to use (since uids may eppear in multiple > passwd file entries with different gids). > > Signed-off-by: Ian Jackson > --- > v5: Use : rather than . to separate uid from gid > v4: Changed to reuse option -runas > v3: Error messages fixed. Thanks to Peter Maydell and Ross Lagerwall. > v2: Coding style fixes. > > squash! os-posix: Provide new -runas . facility > --- > os-posix.c | 64 > +++-- > qemu-options.hx | 3 ++- > 2 files changed, 55 insertions(+), 12 deletions(-) > > diff --git a/os-posix.c b/os-posix.c > index 92e9d85..f95b7bf 100644 > --- a/os-posix.c > +++ b/os-posix.c > @@ -43,6 +43,8 @@ > #endif > > static struct passwd *user_pwd; > +static uid_t user_uid = (uid_t)-1; > +static gid_t user_gid = (gid_t)-1; > static const char *chroot_dir; > static int daemonize; > static int daemon_pipe; > @@ -128,6 +130,34 @@ void os_set_proc_name(const char *s) > #endif > } > > + > +static bool os_parse_runas_uid_gid(const char *optarg) > +{ > +unsigned long lv; > +char *ep; > +uid_t got_uid; > +gid_t got_gid; > +int rc; > + > +errno = 0; > +lv = strtoul(optarg, &ep, 0); /* can't qemu_strtoul, want *ep==':' */ > +got_uid = lv; /* overflow here is ID in C99 */ > +if (errno || *ep != ':' || got_uid != lv || got_uid == (uid_t)-1) { > +return false; > +} > + > +lv = 0; > +rc = qemu_strtoul(ep + 1, 0, 0, &lv); > +got_gid = lv; /* overflow here is ID in C99 */ > +if (rc || got_gid != lv || got_gid == (gid_t)-1) { > +return false; > +} > + > +user_uid = got_uid; > +user_gid = got_gid; > +return true; > +} > + > /* > * Parse OS specific command line options. > * return 0 if option handled, -1 otherwise > @@ -145,8 +175,10 @@ void os_parse_cmd_args(int index, const char *optarg) > #endif > case QEMU_OPTION_runas: > user_pwd = getpwnam(optarg); > -if (!user_pwd) { > -fprintf(stderr, "User \"%s\" doesn't exist\n", optarg); > +if (!user_pwd && !os_parse_runas_uid_gid(optarg)) { > +fprintf(stderr, > +"User \"%s\" doesn't exist (and is not .)\n", > +optarg); > exit(1); > } > break; > @@ -166,18 +198,28 @@ void os_parse_cmd_args(int index, const char *optarg) > > static void change_process_uid(void) > { > -if (user_pwd) { > -if (setgid(user_pwd->pw_gid) < 0) { > -fprintf(stderr, "Failed to setgid(%d)\n", user_pwd->pw_gid); > +if (user_pwd || user_uid != (uid_t)-1) { > +gid_t intended_gid = user_pwd ? user_pwd->pw_gid : user_gid; > +uid_t intended_uid = user_pwd ? user_pwd->pw_uid : user_uid; > +if (setgid(intended_gid) < 0) { > +fprintf(stderr, "Failed to setgid(%d)\n", intended_gid); > exit(1); > } > -if (initgroups(user_pwd->pw_name, user_pwd->pw_gid) < 0) { > -fprintf(stderr, "Failed to initgroups(\"%s\", %d)\n", > -user_pwd->pw_name, user_pwd->pw_gid); > -exit(1); > +if (user_pwd) { > +if (initgroups(user_pwd->pw_name, user_pwd->pw_gid) < 0) { > +fprintf(stderr, "Failed to initgroups(\"%s\", %d)\n", > +user_pwd->pw_name, user_pwd->pw_gid); > +exit(1); > +} > +} else { > +if (setgroups(1, &user_gid) < 0) { > +fprintf(stderr, "Failed to setgroups(1, [%d])", > +user_gid); > +exit(1); > +} > } > -if (setuid(user_pwd->pw_uid) < 0) { > -fprintf(stderr, "Failed to setuid(%d)\n", user_pwd->pw_uid); > +if (setuid(intended_uid) < 0) { > +fprintf(stderr, "Failed to setuid(%d)\n", intended_uid); > exit(1); > } > if (setuid(0) != -1) { > diff --git a/qemu-options.hx b/qemu-options.hx > index 9f6e2ad..f707197e 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -3958,7 +3958,8 @@ ETEXI > > #ifndef _WIN32 > DEF("runas", HAS_ARG, QEMU_OPTION_runas, \ > -"-runas user change to user id user just before starting the VM\n", > +"-runas user change to user id user just before starting the VM\n" \ > +"user can be numeric uid:gid instead\n", > QEMU_ARCH_ALL) > #endif > STEXI > -- > 2.1.4 > ___ Xen-devel mailing list Xen-devel@lists.xen.
Re: [Xen-devel] [PATCH v5.1 7/8] os-posix: Provide new -runas : facility
Anthony PERARD writes ("Re: [PATCH v5.1 7/8] os-posix: Provide new -runas : facility"): > On Fri, Oct 20, 2017 at 02:38:21PM +0100, Ian Jackson wrote: > > +static bool os_parse_runas_uid_gid(const char *optarg) ... > > +errno = 0; > > +lv = strtoul(optarg, &ep, 0); /* can't qemu_strtoul, want *ep==':' */ > > Should strtoul base be 10? If that matter. If someone wants to write uids in hex then I don't see a reason to stop them... > > -if (!user_pwd) { > > -fprintf(stderr, "User \"%s\" doesn't exist\n", optarg); > > +if (!user_pwd && !os_parse_runas_uid_gid(optarg)) { > > +fprintf(stderr, > > +"User \"%s\" doesn't exist (and is not .)\n", > > The error message have not been update, I think it should be : Oops. > With the error message fix: > Reviewed-by: Anthony PERARD Thanks, Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5.1 7/8] os-posix: Provide new -runas : facility
On Fri, Oct 20, 2017 at 02:38:21PM +0100, Ian Jackson wrote: > +static bool os_parse_runas_uid_gid(const char *optarg) > +{ > +unsigned long lv; > +char *ep; > +uid_t got_uid; > +gid_t got_gid; > +int rc; > + > +errno = 0; > +lv = strtoul(optarg, &ep, 0); /* can't qemu_strtoul, want *ep==':' */ Should strtoul base be 10? If that matter. > +got_uid = lv; /* overflow here is ID in C99 */ > +if (errno || *ep != ':' || got_uid != lv || got_uid == (uid_t)-1) { > +return false; > +} > + > +lv = 0; > +rc = qemu_strtoul(ep + 1, 0, 0, &lv); > +got_gid = lv; /* overflow here is ID in C99 */ > +if (rc || got_gid != lv || got_gid == (gid_t)-1) { > +return false; > +} > + > +user_uid = got_uid; > +user_gid = got_gid; > +return true; > +} > + > /* > * Parse OS specific command line options. > * return 0 if option handled, -1 otherwise > @@ -145,8 +175,10 @@ void os_parse_cmd_args(int index, const char *optarg) > #endif > case QEMU_OPTION_runas: > user_pwd = getpwnam(optarg); > -if (!user_pwd) { > -fprintf(stderr, "User \"%s\" doesn't exist\n", optarg); > +if (!user_pwd && !os_parse_runas_uid_gid(optarg)) { > +fprintf(stderr, > +"User \"%s\" doesn't exist (and is not .)\n", The error message have not been update, I think it should be : > +optarg); > exit(1); > } > break; With the error message fix: Reviewed-by: Anthony PERARD -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v5.1 7/8] os-posix: Provide new -runas : facility
This allows the caller to specify a uid and gid to use, even if there is no corresponding password entry. This will be useful in certain Xen configurations. We don't support just -runas because: (i) deprivileging without calling setgroups would be ineffective (ii) given only a uid we don't know what gid we ought to use (since uids may eppear in multiple passwd file entries with different gids). Signed-off-by: Ian Jackson --- v5: Use : rather than . to separate uid from gid v4: Changed to reuse option -runas v3: Error messages fixed. Thanks to Peter Maydell and Ross Lagerwall. v2: Coding style fixes. squash! os-posix: Provide new -runas . facility --- os-posix.c | 64 +++-- qemu-options.hx | 3 ++- 2 files changed, 55 insertions(+), 12 deletions(-) diff --git a/os-posix.c b/os-posix.c index 92e9d85..f95b7bf 100644 --- a/os-posix.c +++ b/os-posix.c @@ -43,6 +43,8 @@ #endif static struct passwd *user_pwd; +static uid_t user_uid = (uid_t)-1; +static gid_t user_gid = (gid_t)-1; static const char *chroot_dir; static int daemonize; static int daemon_pipe; @@ -128,6 +130,34 @@ void os_set_proc_name(const char *s) #endif } + +static bool os_parse_runas_uid_gid(const char *optarg) +{ +unsigned long lv; +char *ep; +uid_t got_uid; +gid_t got_gid; +int rc; + +errno = 0; +lv = strtoul(optarg, &ep, 0); /* can't qemu_strtoul, want *ep==':' */ +got_uid = lv; /* overflow here is ID in C99 */ +if (errno || *ep != ':' || got_uid != lv || got_uid == (uid_t)-1) { +return false; +} + +lv = 0; +rc = qemu_strtoul(ep + 1, 0, 0, &lv); +got_gid = lv; /* overflow here is ID in C99 */ +if (rc || got_gid != lv || got_gid == (gid_t)-1) { +return false; +} + +user_uid = got_uid; +user_gid = got_gid; +return true; +} + /* * Parse OS specific command line options. * return 0 if option handled, -1 otherwise @@ -145,8 +175,10 @@ void os_parse_cmd_args(int index, const char *optarg) #endif case QEMU_OPTION_runas: user_pwd = getpwnam(optarg); -if (!user_pwd) { -fprintf(stderr, "User \"%s\" doesn't exist\n", optarg); +if (!user_pwd && !os_parse_runas_uid_gid(optarg)) { +fprintf(stderr, +"User \"%s\" doesn't exist (and is not .)\n", +optarg); exit(1); } break; @@ -166,18 +198,28 @@ void os_parse_cmd_args(int index, const char *optarg) static void change_process_uid(void) { -if (user_pwd) { -if (setgid(user_pwd->pw_gid) < 0) { -fprintf(stderr, "Failed to setgid(%d)\n", user_pwd->pw_gid); +if (user_pwd || user_uid != (uid_t)-1) { +gid_t intended_gid = user_pwd ? user_pwd->pw_gid : user_gid; +uid_t intended_uid = user_pwd ? user_pwd->pw_uid : user_uid; +if (setgid(intended_gid) < 0) { +fprintf(stderr, "Failed to setgid(%d)\n", intended_gid); exit(1); } -if (initgroups(user_pwd->pw_name, user_pwd->pw_gid) < 0) { -fprintf(stderr, "Failed to initgroups(\"%s\", %d)\n", -user_pwd->pw_name, user_pwd->pw_gid); -exit(1); +if (user_pwd) { +if (initgroups(user_pwd->pw_name, user_pwd->pw_gid) < 0) { +fprintf(stderr, "Failed to initgroups(\"%s\", %d)\n", +user_pwd->pw_name, user_pwd->pw_gid); +exit(1); +} +} else { +if (setgroups(1, &user_gid) < 0) { +fprintf(stderr, "Failed to setgroups(1, [%d])", +user_gid); +exit(1); +} } -if (setuid(user_pwd->pw_uid) < 0) { -fprintf(stderr, "Failed to setuid(%d)\n", user_pwd->pw_uid); +if (setuid(intended_uid) < 0) { +fprintf(stderr, "Failed to setuid(%d)\n", intended_uid); exit(1); } if (setuid(0) != -1) { diff --git a/qemu-options.hx b/qemu-options.hx index 9f6e2ad..f707197e 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -3958,7 +3958,8 @@ ETEXI #ifndef _WIN32 DEF("runas", HAS_ARG, QEMU_OPTION_runas, \ -"-runas user change to user id user just before starting the VM\n", +"-runas user change to user id user just before starting the VM\n" \ +"user can be numeric uid:gid instead\n", QEMU_ARCH_ALL) #endif STEXI -- 2.1.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel