[PATCH] [RFC] run QEMU as non-root

[PATCH] [RFC] run QEMU as non-root

[Stefano Stabellini]

Run QEMU as non-root. Starting from uid 6000, the chosen uid is
base+domid. If the uid doesn't exist, try just 6000. This is less
secure: ideally we don't want different domains having their QEMUs
running with the same uid. Finally if uid 6000 doesn't exist either,
fall back to running QEMU as root.

The uids need to be manually created by the user or, more likely, by the
xen package maintainer.

To actually secure QEMU when running in Dom0, we need at least to
deprivilege the privcmd and xenstore interfaces, this is just the first
step in that direction.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 tools/libxl/libxl_dm.c       |   17 +++++++++++++++++
 tools/libxl/libxl_internal.h |    2 ++
 2 files changed, 19 insertions(+)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 0c6408d..942c5df 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -19,6 +19,8 @@
 
 #include "libxl_internal.h"
 #include <xen/hvm/e820.h>
+#include <sys/types.h>
+#include <pwd.h>
 
 static const char *libxl_tapif_script(libxl__gc *gc)
 {
@@ -439,6 +441,7 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
     int i, connection, devid;
     uint64_t ram_size;
     const char *path, *chardev;
+    struct passwd *user = NULL;
 
     dm_args = flexarray_make(gc, 16, 1);
 
@@ -878,6 +881,20 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
         default:
             break;
         }
+
+        user = getpwuid(LIBXL_QEMU_BASE_UID + guest_domid);
+        if (user == NULL) {
+            LIBXL__LOG(ctx, LIBXL__LOG_WARNING, "Could not find uid %d, falling back to %d\n",
+                    LIBXL_QEMU_BASE_UID + guest_domid, LIBXL_QEMU_BASE_UID);
+            user = getpwuid(LIBXL_QEMU_BASE_UID);
+            if (user == NULL)
+                LIBXL__LOG(ctx, LIBXL__LOG_WARNING, "Could not find uid %d, starting QEMU as root\n",
+                    LIBXL_QEMU_BASE_UID);
+        }
+        if (user) {
+            flexarray_append(dm_args, "-runas");
+            flexarray_append(dm_args, user->pw_name);
+        }
     }
     flexarray_append(dm_args, NULL);
     return (char **) flexarray_contents(dm_args);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 8eb38aa..065ff98 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3692,6 +3692,8 @@ static inline void libxl__update_config_vtpm(libxl__gc *gc,
  */
 void libxl__bitmap_copy_best_effort(libxl__gc *gc, libxl_bitmap *dptr,
                                     const libxl_bitmap *sptr);
+
+#define LIBXL_QEMU_BASE_UID (6000)
 #endif
 
 /*
-- 
1.7.10.4

Re: [PATCH] [RFC] run QEMU as non-root

[Ian Campbell]

On Thu, 2015-05-14 at 18:52 +0100, Stefano Stabellini wrote:
> Run QEMU as non-root. Starting from uid 6000, the chosen uid is
> base+domid. If the uid doesn't exist, try just 6000. This is less
> secure: ideally we don't want different domains having their QEMUs
> running with the same uid. Finally if uid 6000 doesn't exist either,
> fall back to running QEMU as root.

We can't just pick a random number like that, especially not hardcoded.

You should call getpwent_r.

IIRC what was suggested yesterday IRL was to look for, in order, users
named (prefixes TBD):

        xen-qemudepriv-$domname
        xen-qemudepriv-base (+domid)
        xen-qemudepriv-shared (all qemu in same non-root uid)

If none of those are present then the qemu should not be deprivileged.
There should probably be a nob to fiddle to allow the fallback to be to
fail to create the domain.

Then the admin/postinst can do as they prefer:

        adduser --system xen-qemudepriv-mysecuredomain
        
        for i in '' $(seq 1 65335) ; do
              adduser --system xen-qemudepriv-base$i
        done
        
        adduser --system xen-qemudepriv-shared

(and can combine the first with either the second or third as they
desire)

There needs to be a documentation update associated with this.

> The uids need to be manually created by the user or, more likely, by the
> xen package maintainer.
> 
> To actually secure QEMU when running in Dom0, we need at least to
> deprivilege the privcmd and xenstore interfaces, this is just the first
> step in that direction.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  tools/libxl/libxl_dm.c       |   17 +++++++++++++++++
>  tools/libxl/libxl_internal.h |    2 ++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 0c6408d..942c5df 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -19,6 +19,8 @@
>  
>  #include "libxl_internal.h"
>  #include <xen/hvm/e820.h>
> +#include <sys/types.h>
> +#include <pwd.h>
>  
>  static const char *libxl_tapif_script(libxl__gc *gc)
>  {
> @@ -439,6 +441,7 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>      int i, connection, devid;
>      uint64_t ram_size;
>      const char *path, *chardev;
> +    struct passwd *user = NULL;
>  
>      dm_args = flexarray_make(gc, 16, 1);
>  
> @@ -878,6 +881,20 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>          default:
>              break;
>          }
> +
> +        user = getpwuid(LIBXL_QEMU_BASE_UID + guest_domid);
> +        if (user == NULL) {
> +            LIBXL__LOG(ctx, LIBXL__LOG_WARNING, "Could not find uid %d, falling back to %d\n",
> +                    LIBXL_QEMU_BASE_UID + guest_domid, LIBXL_QEMU_BASE_UID);

LOG(WARNING, "Could not..")

And *LOG* appends \n itself.

> +            user = getpwuid(LIBXL_QEMU_BASE_UID);
> +            if (user == NULL)
> +                LIBXL__LOG(ctx, LIBXL__LOG_WARNING, "Could not find uid %d, starting QEMU as root\n",
> +                    LIBXL_QEMU_BASE_UID);
> +        }
> +        if (user) {
> +            flexarray_append(dm_args, "-runas");
> +            flexarray_append(dm_args, user->pw_name);
> +        }
>      }
>      flexarray_append(dm_args, NULL);
>      return (char **) flexarray_contents(dm_args);
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 8eb38aa..065ff98 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -3692,6 +3692,8 @@ static inline void libxl__update_config_vtpm(libxl__gc *gc,
>   */
>  void libxl__bitmap_copy_best_effort(libxl__gc *gc, libxl_bitmap *dptr,
>                                      const libxl_bitmap *sptr);
> +
> +#define LIBXL_QEMU_BASE_UID (6000)
>  #endif
>  
>  /*

Re: [PATCH] [RFC] run QEMU as non-root

[Stefano Stabellini]

On Fri, 15 May 2015, Ian Campbell wrote:
> On Thu, 2015-05-14 at 18:52 +0100, Stefano Stabellini wrote:
> > Run QEMU as non-root. Starting from uid 6000, the chosen uid is
> > base+domid. If the uid doesn't exist, try just 6000. This is less
> > secure: ideally we don't want different domains having their QEMUs
> > running with the same uid. Finally if uid 6000 doesn't exist either,
> > fall back to running QEMU as root.
> 
> We can't just pick a random number like that, especially not hardcoded.
> 
> You should call getpwent_r.

Are you suggesting to go over the full list of records in passwd? It
doesn't sound like a good idea. Did you mean getpwnam_r?


> IIRC what was suggested yesterday IRL was to look for, in order, users
> named (prefixes TBD):
> 
>         xen-qemudepriv-$domname
>         xen-qemudepriv-base (+domid)
>         xen-qemudepriv-shared (all qemu in same non-root uid)
> 
> If none of those are present then the qemu should not be deprivileged.

This is better. I'll go for this.


> There should probably be a nob to fiddle to allow the fallback to be to
> fail to create the domain.

I agree, but it is a bit too early for that.


> Then the admin/postinst can do as they prefer:
> 
>         adduser --system xen-qemudepriv-mysecuredomain
>         
>         for i in '' $(seq 1 65335) ; do
>               adduser --system xen-qemudepriv-base$i
>         done
>         
>         adduser --system xen-qemudepriv-shared
> 
> (and can combine the first with either the second or third as they
> desire)
> 
> There needs to be a documentation update associated with this.

OK


> > The uids need to be manually created by the user or, more likely, by the
> > xen package maintainer.
> > 
> > To actually secure QEMU when running in Dom0, we need at least to
> > deprivilege the privcmd and xenstore interfaces, this is just the first
> > step in that direction.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > ---
> >  tools/libxl/libxl_dm.c       |   17 +++++++++++++++++
> >  tools/libxl/libxl_internal.h |    2 ++
> >  2 files changed, 19 insertions(+)
> > 
> > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > index 0c6408d..942c5df 100644
> > --- a/tools/libxl/libxl_dm.c
> > +++ b/tools/libxl/libxl_dm.c
> > @@ -19,6 +19,8 @@
> >  
> >  #include "libxl_internal.h"
> >  #include <xen/hvm/e820.h>
> > +#include <sys/types.h>
> > +#include <pwd.h>
> >  
> >  static const char *libxl_tapif_script(libxl__gc *gc)
> >  {
> > @@ -439,6 +441,7 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> >      int i, connection, devid;
> >      uint64_t ram_size;
> >      const char *path, *chardev;
> > +    struct passwd *user = NULL;
> >  
> >      dm_args = flexarray_make(gc, 16, 1);
> >  
> > @@ -878,6 +881,20 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> >          default:
> >              break;
> >          }
> > +
> > +        user = getpwuid(LIBXL_QEMU_BASE_UID + guest_domid);
> > +        if (user == NULL) {
> > +            LIBXL__LOG(ctx, LIBXL__LOG_WARNING, "Could not find uid %d, falling back to %d\n",
> > +                    LIBXL_QEMU_BASE_UID + guest_domid, LIBXL_QEMU_BASE_UID);
> 
> LOG(WARNING, "Could not..")
> 
> And *LOG* appends \n itself.

OK


> > +            user = getpwuid(LIBXL_QEMU_BASE_UID);
> > +            if (user == NULL)
> > +                LIBXL__LOG(ctx, LIBXL__LOG_WARNING, "Could not find uid %d, starting QEMU as root\n",
> > +                    LIBXL_QEMU_BASE_UID);
> > +        }
> > +        if (user) {
> > +            flexarray_append(dm_args, "-runas");
> > +            flexarray_append(dm_args, user->pw_name);
> > +        }
> >      }
> >      flexarray_append(dm_args, NULL);
> >      return (char **) flexarray_contents(dm_args);
> > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> > index 8eb38aa..065ff98 100644
> > --- a/tools/libxl/libxl_internal.h
> > +++ b/tools/libxl/libxl_internal.h
> > @@ -3692,6 +3692,8 @@ static inline void libxl__update_config_vtpm(libxl__gc *gc,
> >   */
> >  void libxl__bitmap_copy_best_effort(libxl__gc *gc, libxl_bitmap *dptr,
> >                                      const libxl_bitmap *sptr);
> > +
> > +#define LIBXL_QEMU_BASE_UID (6000)
> >  #endif
> >  
> >  /*
> 
> 

Re: [PATCH] [RFC] run QEMU as non-root

[Ian Campbell]

On Fri, 2015-05-15 at 11:46 +0100, Stefano Stabellini wrote:
> On Fri, 15 May 2015, Ian Campbell wrote:
> > On Thu, 2015-05-14 at 18:52 +0100, Stefano Stabellini wrote:
> > > Run QEMU as non-root. Starting from uid 6000, the chosen uid is
> > > base+domid. If the uid doesn't exist, try just 6000. This is less
> > > secure: ideally we don't want different domains having their QEMUs
> > > running with the same uid. Finally if uid 6000 doesn't exist either,
> > > fall back to running QEMU as root.
> > 
> > We can't just pick a random number like that, especially not hardcoded.
> > 
> > You should call getpwent_r.
> 
> Are you suggesting to go over the full list of records in passwd? It
> doesn't sound like a good idea. Did you mean getpwnam_r?

yes, sorry.