| Message ID | 1310203327-27069-1-git-send-email-stefanha@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Sat, Jul 9, 2011 at 10:22 AM, Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> wrote: > Andrew Griffiths reports that -runas does not set supplementary group > IDs. This means that gid 0 (root) is not dropped when switching to an > unprivileged user. > > Add an initgroups(3) call to use the -runas user's /etc/groups > membership to update the supplementary group IDs. > > Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> > --- > Note this needs compile testing on various POSIX host platforms. Tested on > Linux. Should work on BSD and Solaris. initgroups(3) is SVr4/BSD but not in > POSIX. > > os-posix.c | 6 ++++++ > 1 files changed, 6 insertions(+), 0 deletions(-) Are you happy with this patch? Bumping because security-related. Regarding portability, Linux, BSD, Solaris, and Mac OS X all provide initgroups(3). I think we're good. Stefan
* Stefan Hajnoczi (stefanha@linux.vnet.ibm.com) wrote: > @@ -199,6 +200,11 @@ static void change_process_uid(void) > fprintf(stderr, "Failed to setgid(%d)\n", user_pwd->pw_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); > + } Does initgroups need access to /etc/group? How does this combine w/ -chroot? Added bonus...this will fail when the initial user is not privileged _and_ is the same user as -runas user (probably not what a user intended, but would've worked before). Something like: [doh@laptop qemu]$ qemu -runas doh
* Chris Wright (chrisw@sous-sol.org) wrote: > * Stefan Hajnoczi (stefanha@linux.vnet.ibm.com) wrote: > > @@ -199,6 +200,11 @@ static void change_process_uid(void) > > fprintf(stderr, "Failed to setgid(%d)\n", user_pwd->pw_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); > > + } > > Does initgroups need access to /etc/group? How does this combine w/ > -chroot? Tested this on Linux, and w/out /etc/group it simply fails to add any supplementary groups (doesn't fail completely, just fails safely). Appears similar from solaris manpages. Given that... Acked-by: Chris Wright <chrisw@sous-sol.org>
Thanks, applied. On Sat, Jul 9, 2011 at 12:22 PM, Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> wrote: > Andrew Griffiths reports that -runas does not set supplementary group > IDs. This means that gid 0 (root) is not dropped when switching to an > unprivileged user. > > Add an initgroups(3) call to use the -runas user's /etc/groups > membership to update the supplementary group IDs. > > Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> > --- > Note this needs compile testing on various POSIX host platforms. Tested on > Linux. Should work on BSD and Solaris. initgroups(3) is SVr4/BSD but not in > POSIX. > > os-posix.c | 6 ++++++ > 1 files changed, 6 insertions(+), 0 deletions(-) > > diff --git a/os-posix.c b/os-posix.c > index 7dfb278..6f8d488 100644 > --- a/os-posix.c > +++ b/os-posix.c > @@ -31,6 +31,7 @@ > /*needed for MAP_POPULATE before including qemu-options.h */ > #include <sys/mman.h> > #include <pwd.h> > +#include <grp.h> > #include <libgen.h> > > /* Needed early for CONFIG_BSD etc. */ > @@ -199,6 +200,11 @@ static void change_process_uid(void) > fprintf(stderr, "Failed to setgid(%d)\n", user_pwd->pw_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 (setuid(user_pwd->pw_uid) < 0) { > fprintf(stderr, "Failed to setuid(%d)\n", user_pwd->pw_uid); > exit(1); > -- > 1.7.5.4 > > >
diff --git a/os-posix.c b/os-posix.c index 7dfb278..6f8d488 100644 --- a/os-posix.c +++ b/os-posix.c @@ -31,6 +31,7 @@ /*needed for MAP_POPULATE before including qemu-options.h */ #include <sys/mman.h> #include <pwd.h> +#include <grp.h> #include <libgen.h> /* Needed early for CONFIG_BSD etc. */ @@ -199,6 +200,11 @@ static void change_process_uid(void) fprintf(stderr, "Failed to setgid(%d)\n", user_pwd->pw_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 (setuid(user_pwd->pw_uid) < 0) { fprintf(stderr, "Failed to setuid(%d)\n", user_pwd->pw_uid); exit(1);
Andrew Griffiths reports that -runas does not set supplementary group IDs. This means that gid 0 (root) is not dropped when switching to an unprivileged user. Add an initgroups(3) call to use the -runas user's /etc/groups membership to update the supplementary group IDs. Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> --- Note this needs compile testing on various POSIX host platforms. Tested on Linux. Should work on BSD and Solaris. initgroups(3) is SVr4/BSD but not in POSIX. os-posix.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)