[Qemu-devel] [PATCH 1/2] Do not generate spurious error for -runas root

[Qemu-devel] [PATCH 1/2] Do not generate spurious error for -runas root

[Chris Webb]

change_process_uid() checks that privileges have been successfully dropped by
verifying that setuid(0) fails. However, if qemu is explicitly -runas root,
the setuid(0) would correctly succeed, so we skip the test in that case.

Signed-off-by: Chris Webb <chris@arachsys.com>
---
 os-posix.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index dc4a6bb..1b2061a 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -209,7 +209,7 @@ static void change_process_uid(void)
             fprintf(stderr, "Failed to setuid(%d)\n", user_pwd->pw_uid);
             exit(1);
         }
-        if (setuid(0) != -1) {
+        if (user_pwd->pw_uid != 0 && setuid(0) != -1) {
             fprintf(stderr, "Dropping privileges failed\n");
             exit(1);
         }
-- 
1.7.5.4

[Qemu-devel] [PATCH 2/2] Allow -runas to be specified as UID:GID as well as USERNAME

[Chris Webb]

This allows qemu to drop privileges to a dynamically allocated, anonymous UID
and GID without needing a temporary /etc/passwd entry for that UID. The
UID:GID format is very standard, being (for example) the syntax used by
chown(1) for numeric IDs.

Signed-off-by: Chris Webb <chris@arachsys.com>
---
 os-posix.c |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index 1b2061a..66f5018 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -179,6 +179,15 @@ void os_parse_cmd_args(int index, const char *optarg)
     case QEMU_OPTION_runas:
         user_pwd = getpwnam(optarg);
         if (!user_pwd) {
+            long uid, gid, tail;
+            if (sscanf(optarg, "%ld:%ld%ln", &uid, &gid, &tail) >= 2
+                  && !optarg[tail]) {
+                user_pwd = g_malloc0(sizeof(user_pwd));
+                user_pwd->pw_uid = uid;
+                user_pwd->pw_gid = gid;
+            }
+        }
+        if (!user_pwd) {
             fprintf(stderr, "User \"%s\" doesn't exist\n", optarg);
             exit(1);
         }
@@ -200,7 +209,12 @@ 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) {
+        if (!user_pwd->pw_name) {
+            if (setgroups(0, NULL) < 0) {
+                fprintf(stderr, "Failed to setgroups(0, NULL)\n");
+                exit(1);
+            }
+        } else 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);
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH 2/2] Allow -runas to be specified as UID:GID as well as USERNAME

[Avi Kivity]

On 11/24/2011 06:29 PM, Chris Webb wrote:
> This allows qemu to drop privileges to a dynamically allocated, anonymous UID
> and GID without needing a temporary /etc/passwd entry for that UID. The
> UID:GID format is very standard, being (for example) the syntax used by
> chown(1) for numeric IDs.

> @@ -179,6 +179,15 @@ void os_parse_cmd_args(int index, const char *optarg)
>      case QEMU_OPTION_runas:
>          user_pwd = getpwnam(optarg);
>          if (!user_pwd) {
> +            long uid, gid, tail;
> +            if (sscanf(optarg, "%ld:%ld%ln", &uid, &gid, &tail) >= 2
> +                  && !optarg[tail]) {
> +                user_pwd = g_malloc0(sizeof(user_pwd));

g_new0() please.  user_pwd is never freed, not that it matters ,uch.

> +                user_pwd->pw_uid = uid;
> +                user_pwd->pw_gid = gid;
> +            }
> +        }
> +        if (!user_pwd) {
>              fprintf(stderr, "User \"%s\" doesn't exist\n", optarg);
>              exit(1);
>          }

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 2/2] Allow -runas to be specified as UID:GID as well as USERNAME

[Avi Kivity]

On 11/24/2011 06:46 PM, Avi Kivity wrote:
> On 11/24/2011 06:29 PM, Chris Webb wrote:
> > This allows qemu to drop privileges to a dynamically allocated, anonymous UID
> > and GID without needing a temporary /etc/passwd entry for that UID. The
> > UID:GID format is very standard, being (for example) the syntax used by
> > chown(1) for numeric IDs.
>
> > @@ -179,6 +179,15 @@ void os_parse_cmd_args(int index, const char *optarg)
> >      case QEMU_OPTION_runas:
> >          user_pwd = getpwnam(optarg);
> >          if (!user_pwd) {
> > +            long uid, gid, tail;
> > +            if (sscanf(optarg, "%ld:%ld%ln", &uid, &gid, &tail) >= 2
> > +                  && !optarg[tail]) {
> > +                user_pwd = g_malloc0(sizeof(user_pwd));
>
> g_new0() please.

In fact this is a bug - you allocate a pointer instead of a struct passwd.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 2/2] Allow -runas to be specified as UID:GID as well as USERNAME

[Chris Webb]

Avi Kivity <avi@redhat.com> writes:

> On 11/24/2011 06:29 PM, Chris Webb wrote:
> > This allows qemu to drop privileges to a dynamically allocated, anonymous UID
> > and GID without needing a temporary /etc/passwd entry for that UID. The
> > UID:GID format is very standard, being (for example) the syntax used by
> > chown(1) for numeric IDs.
> 
> > @@ -179,6 +179,15 @@ void os_parse_cmd_args(int index, const char *optarg)
> >      case QEMU_OPTION_runas:
> >          user_pwd = getpwnam(optarg);
> >          if (!user_pwd) {
> > +            long uid, gid, tail;
> > +            if (sscanf(optarg, "%ld:%ld%ln", &uid, &gid, &tail) >= 2
> > +                  && !optarg[tail]) {
> > +                user_pwd = g_malloc0(sizeof(user_pwd));
> 
> g_new0() please.  user_pwd is never freed, not that it matters ,uch.

Okay, will re-spin; thanks!

Cheers,

Chris.