Re: [Xen-devel] [PATCH v5] run QEMU as non-root

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Tue, 30 Jun 2015, Jim Fehlig wrote:
> On 06/30/2015 07:55 AM, Stefano Stabellini wrote:
> > Try to use "xen-qemudepriv-domid$domid" first, then
> > "xen-qemudepriv-shared" and root if everything else fails.
> > 
> > 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@xxxxxxxxxxxxx>
> > 
> > ---
> > Changes in v5:
> > - improve wording in doc
> > - fix wording in warning message
> > - fix example in doc
> > - drop xen-qemudepriv-$domname
> > 
> > Changes in v4:
> > - rename qemu-deprivilege to qemu-deprivilege.txt
> > - add a note about qemu-deprivilege.txt to INSTALL
> > - instead of xen-qemudepriv-base + $domid, try xen-qemudepriv-domid$domid
> > - introduce libxl__dm_runas_helper to make the code nicer
> > 
> > Changes in v3:
> > - clarify doc
> > - handle errno == ERANGE
> > ---
> >   INSTALL                        |    7 ++++++
> >   docs/misc/qemu-deprivilege.txt |   26 +++++++++++++++++++++
> >   tools/libxl/libxl_dm.c         |   50
> > ++++++++++++++++++++++++++++++++++++++++
> >   tools/libxl/libxl_internal.h   |    4 ++++
> >   4 files changed, 87 insertions(+)
> >   create mode 100644 docs/misc/qemu-deprivilege.txt
> > 
> > diff --git a/INSTALL b/INSTALL
> > index a0f2e7b..fe83c20 100644
> > --- a/INSTALL
> > +++ b/INSTALL
> > @@ -297,6 +297,13 @@ systemctl enable xendomains.service
> >   systemctl enable xen-watchdog.service
> >     +QEMU Deprivilege
> > +================
> > +It is recommended to run QEMU as non-root.
> > +See docs/misc/qemu-deprivilege.txt for an explanation on what you need
> > +to do at installation time to run QEMU as a dedicated user.
> > +
> > +
> >   History of options
> >   ==================
> >   diff --git a/docs/misc/qemu-deprivilege.txt
> > b/docs/misc/qemu-deprivilege.txt
> > new file mode 100644
> > index 0000000..783874b
> > --- /dev/null
> > +++ b/docs/misc/qemu-deprivilege.txt
> > @@ -0,0 +1,26 @@
> > +For security reasons, libxl tries to create QEMU as non-root.
> > +Libxl looks for the following users in this order:
> > +
> > +1) a user named "xen-qemuuser-domid$domid",
> > +Where $domid is the domid of the domain being created.
> > +This requires the reservation of 65535 uids from xen-qemuuser-domid1
> > +to xen-qemuuser-domid65535. To use this mechanism, you might want to
> > +create a large number of users at installation time. For example:
> > +
> > +for ((i=1; i<65536; i++))
> > +do
> > +    adduser --system xen-qemuuser-domid$i
> > +done
> > +
> > +
> > +2) a user named "xen-qemuuser-shared"
> > +As a fall back if both 1) and 2) fail, libxl will use a single user for
> > +all QEMU instances. The user is named xen-qemuuser-shared. This is
> > +less secure but still better than running QEMU as root. Using this is as
> > +simple as creating just one more user on your host:
> > +
> > +adduser --system xen-qemuuser-shared
> > +
> > +
> > +3) root
> > +As a last resort, libxl will start QEMU as root.
> 
> The more I think about it, the more I feel libxl is the wrong place for this
> policy. As mentioned earlier [0], libvirt allows apps to control the
> user:group policy. It is already supported by the qemu driver. It could be
> used by the libxl driver to inform libxl that the emulator (and other
> binaries?) it spawns should be in the context of the specified user:group.
> 
> Regards,
> Jim
> 
> [0] http://lists.xenproject.org/archives/html/xen-devel/2015-05/msg02139.html

Are you suggesting to expose a per-domain user:group setting that can be
passed down by libvirt? Maybe something under
libxl_domain_build_info.u.hvm?

Then I could move the code below to xl_cmdimpl.c, making it xl specific
(rather than libxl).


> > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > index 0c6408d..66d638d 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)
> >   {
> > @@ -418,6 +420,33 @@ static char *dm_spice_options(libxl__gc *gc,
> >       return opt;
> >   }
> >   +/* return 1 if the user was found, 0 if it was not, -1 on error */
> > +static int libxl__dm_runas_helper(libxl__gc *gc, char *username)
> > +{
> > +    struct passwd pwd, *user = NULL;
> > +    char *buf = NULL;
> > +    long buf_size;
> > +
> > +    buf_size = sysconf(_SC_GETPW_R_SIZE_MAX);
> > +    if (buf_size < 0) {
> > +        LOGE(ERROR, "sysconf(_SC_GETPW_R_SIZE_MAX) returned error %ld",
> > +                buf_size);
> > +        return -1;
> > +    }
> > +
> > +retry:
> > +    buf = libxl__realloc(gc, buf, buf_size);
> > +    errno = 0;
> > +    getpwnam_r(username, &pwd, buf, buf_size, &user);
> > +    if (user != NULL)
> > +        return 1;
> > +    if (errno == ERANGE) {
> > +        buf_size += 128;
> > +        goto retry;
> > +    }
> > +    return 0;
> > +}
> > +
> >   static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> >                                           const char *dm, int guest_domid,
> >                                           const libxl_domain_config
> > *guest_config,
> > @@ -439,6 +468,7 @@ static char **
> > libxl__build_device_model_args_new(libxl__gc *gc,
> >       int i, connection, devid;
> >       uint64_t ram_size;
> >       const char *path, *chardev;
> > +    char *user;
> >         dm_args = flexarray_make(gc, 16, 1);
> >   @@ -878,6 +908,26 @@ static char **
> > libxl__build_device_model_args_new(libxl__gc *gc,
> >           default:
> >               break;
> >           }
> > +
> > +        user = libxl__sprintf(gc, "%s%d", LIBXL_QEMU_USER_BASE,
> > guest_domid);
> > +        if (libxl__dm_runas_helper(gc, user) > 0)
> > +            goto end_search;
> > +
> > +        user = LIBXL_QEMU_USER_SHARED;
> > +        if (libxl__dm_runas_helper(gc, user) > 0) {
> > +            LOG(WARN, "Could not find user %s%d, falling back to %s",
> > +                    LIBXL_QEMU_USER_BASE, guest_domid,
> > LIBXL_QEMU_USER_SHARED);
> > +            goto end_search;
> > +        }
> > +
> > +        user = NULL;
> > +        LOG(WARN, "Could not find user %s, starting QEMU as root",
> > LIBXL_QEMU_USER_SHARED);
> > +
> > +end_search:
> > +        if (user) {
> > +            flexarray_append(dm_args, "-runas");
> > +            flexarray_append(dm_args, user);
> > +        }
> >       }
> >       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..7d0af40 100644
> > --- a/tools/libxl/libxl_internal.h
> > +++ b/tools/libxl/libxl_internal.h
> > @@ -3692,6 +3692,10 @@ 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_USER_PREFIX "xen-qemuuser"
> > +#define LIBXL_QEMU_USER_BASE   LIBXL_QEMU_USER_PREFIX"-domid"
> > +#define LIBXL_QEMU_USER_SHARED LIBXL_QEMU_USER_PREFIX"-shared"
> >   #endif
> >     /*
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel