[PATCH v4] run QEMU as non-root

[PATCH v4] run QEMU as non-root

[Stefano Stabellini]

Try to use "xen-qemudepriv-$domname" first, then
"xen-qemudepriv-domid$domid", finally "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@eu.citrix.com>

---
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 |   34 +++++++++++++++++++++++++
 tools/libxl/libxl_dm.c         |   54 ++++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_internal.h   |    4 +++
 4 files changed, 99 insertions(+)
 create mode 100644 docs/misc/qemu-deprivilege.txt

diff --git a/INSTALL b/INSTALL
index a0f2e7b..3286a5f 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 regular user.
+
+
 History of options
 ==================
 
diff --git a/docs/misc/qemu-deprivilege.txt b/docs/misc/qemu-deprivilege.txt
new file mode 100644
index 0000000..db61aaf
--- /dev/null
+++ b/docs/misc/qemu-deprivilege.txt
@@ -0,0 +1,34 @@
+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-$domname"
+Where $domname is the name of the domain being created.
+To use this, you just need to create a user with the appropriate name
+for each domain. For example, if your virtual machine is named "windows":
+
+adduser --system xen-qemuuser-windows
+
+
+2) a user named "xen-qemuuser-domid$domid", 
+Where $domid is the domid of the domain being created.
+This requires the reservation of 65536 uids from xen-qemuuser-domid0
+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 in '' $(seq 0 65335)
+do
+    adduser --system xen-qemuuser-domid$i
+done
+
+
+3) 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
+
+
+4) root
+As a last resort, libxl will start QEMU as root.
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 0c6408d..79a9a22 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,32 @@ 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 +467,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 +907,31 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
         default:
             break;
         }
+
+        user = libxl__sprintf(gc, "%s-%s", LIBXL_QEMU_USER_PREFIX, c_info->name);
+        if (libxl__dm_runas_helper(gc, user) > 0)
+            goto end_search;
+
+        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-%s or user %s (+domid %d), falling back to %s",
+                    LIBXL_QEMU_USER_PREFIX, c_info->name, 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
 
 /*
-- 
1.7.10.4

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

[Ian Campbell]

On Mon, 2015-06-01 at 17:46 +0100, Stefano Stabellini wrote:
> Try to use "xen-qemudepriv-$domname" first, then
> "xen-qemudepriv-domid$domid", finally "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@eu.citrix.com>
> 
> ---
> 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 |   34 +++++++++++++++++++++++++
>  tools/libxl/libxl_dm.c         |   54 ++++++++++++++++++++++++++++++++++++++++
>  tools/libxl/libxl_internal.h   |    4 +++
>  4 files changed, 99 insertions(+)
>  create mode 100644 docs/misc/qemu-deprivilege.txt
> 
> diff --git a/INSTALL b/INSTALL
> index a0f2e7b..3286a5f 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 regular user.

I'd be tempted to say something like "as a dedicated user" or something,
"regular user" sounds like I should be running it as my normal login
user or something.



> +
> +
>  History of options
>  ==================
>  
> diff --git a/docs/misc/qemu-deprivilege.txt b/docs/misc/qemu-deprivilege.txt
> new file mode 100644
> index 0000000..db61aaf
> --- /dev/null
> +++ b/docs/misc/qemu-deprivilege.txt
> @@ -0,0 +1,34 @@
> +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-$domname"
> +Where $domname is the name of the domain being created.
> +To use this, you just need to create a user with the appropriate name
> +for each domain. For example, if your virtual machine is named "windows":
> +
> +adduser --system xen-qemuuser-windows

I just had an annoying thought: On migration (with xl at least) the
receive side appends "-incoming" to the domain name for the duration and
then renames ones everything is done, which is suddenly not going to
match here :-(

Since this is managed at the xl level I don't see any way libxl could
cope without being told separately the domain's "real" name.

I think this reduces the utility of this option quite substantially (or
at least presents an enormous caveat), perhaps to the point where we may
as well not bother to offer it?

> +
> +
> +2) a user named "xen-qemuuser-domid$domid", 
> +Where $domid is the domid of the domain being created.
> +This requires the reservation of 65536 uids from xen-qemuuser-domid0

Is xen-qemuuser-domid0 actually useful?

> +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 in '' $(seq 0 65335)

This creates a user with no $i on it too, which I think is a left over
from the previous scheme and no longer useful.

> +do
> +    adduser --system xen-qemuuser-domid$i
> +done
> +
> +
> +3) 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
> +
> +
> +4) root
> +As a last resort, libxl will start QEMU as root.
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 0c6408d..79a9a22 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,32 @@ 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);

This line is slightly too long. Wrapping buf_size would suffice.

> +        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 +467,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 +907,31 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>          default:
>              break;
>          }
> +
> +        user = libxl__sprintf(gc, "%s-%s", LIBXL_QEMU_USER_PREFIX, c_info->name);
> +        if (libxl__dm_runas_helper(gc, user) > 0)
> +            goto end_search;
> +
> +        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-%s or user %s (+domid %d), falling back to %s",

This message ("+domid %d)" seems to refer to the old way of doing
things.

> +                    LIBXL_QEMU_USER_PREFIX, c_info->name, 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
>  
>  /*

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

[Stefano Stabellini]

On Thu, 25 Jun 2015, Ian Campbell wrote:
> On Mon, 2015-06-01 at 17:46 +0100, Stefano Stabellini wrote:
> > Try to use "xen-qemudepriv-$domname" first, then
> > "xen-qemudepriv-domid$domid", finally "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@eu.citrix.com>
> > 
> > ---
> > 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 |   34 +++++++++++++++++++++++++
> >  tools/libxl/libxl_dm.c         |   54 ++++++++++++++++++++++++++++++++++++++++
> >  tools/libxl/libxl_internal.h   |    4 +++
> >  4 files changed, 99 insertions(+)
> >  create mode 100644 docs/misc/qemu-deprivilege.txt
> > 
> > diff --git a/INSTALL b/INSTALL
> > index a0f2e7b..3286a5f 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 regular user.
> 
> I'd be tempted to say something like "as a dedicated user" or something,
> "regular user" sounds like I should be running it as my normal login
> user or something.

OK


> > +
> > +
> >  History of options
> >  ==================
> >  
> > diff --git a/docs/misc/qemu-deprivilege.txt b/docs/misc/qemu-deprivilege.txt
> > new file mode 100644
> > index 0000000..db61aaf
> > --- /dev/null
> > +++ b/docs/misc/qemu-deprivilege.txt
> > @@ -0,0 +1,34 @@
> > +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-$domname"
> > +Where $domname is the name of the domain being created.
> > +To use this, you just need to create a user with the appropriate name
> > +for each domain. For example, if your virtual machine is named "windows":
> > +
> > +adduser --system xen-qemuuser-windows
> 
> I just had an annoying thought: On migration (with xl at least) the
> receive side appends "-incoming" to the domain name for the duration and
> then renames ones everything is done, which is suddenly not going to
> match here :-(
> 
> Since this is managed at the xl level I don't see any way libxl could
> cope without being told separately the domain's "real" name.
> 
> I think this reduces the utility of this option quite substantially (or
> at least presents an enormous caveat), perhaps to the point where we may
> as well not bother to offer it?

I think we can detect the "--incoming" and remove it. The only problem
is that "--incoming" is actually set by xl (xl_cmdimpl.c), not libxl.
Could we break the abstraction layer just this time and in libxl_dm.c
have something like:


     if ((strlen(c_info->name) > strlen("--incoming")) &&
         (!strcmp(c_info->name + strlen(c_info->name) - strlen("--incoming"), "--incoming"))) {
         name = libxl__strdup(gc, c_info->name);
         name[strlen(c_info->name) - strlen("--incoming")] = '\0';
     } else {
         name = c_info->name;
     }
     user = libxl__sprintf(gc, "%s-%s", LIBXL_QEMU_USER_PREFIX, name);


This is ugly but it was only meant to demonstrate the idea.


> > +
> > +
> > +2) a user named "xen-qemuuser-domid$domid", 
> > +Where $domid is the domid of the domain being created.
> > +This requires the reservation of 65536 uids from xen-qemuuser-domid0
> 
> Is xen-qemuuser-domid0 actually useful?

All right, it can be 1-65535 inclusive.


> > +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 in '' $(seq 0 65335)
> 
> This creates a user with no $i on it too, which I think is a left over
> from the previous scheme and no longer useful.
 
You are right. I'll turn it into the more bash-like:

for ((i=1; i<65336; i++))
do


> > +do
> > +    adduser --system xen-qemuuser-domid$i
> > +done
> > +
> > +
> > +3) 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
> > +
> > +
> > +4) root
> > +As a last resort, libxl will start QEMU as root.
> > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > index 0c6408d..79a9a22 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,32 @@ 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);
> 
> This line is slightly too long. Wrapping buf_size would suffice.

OK


> > +        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 +467,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 +907,31 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> >          default:
> >              break;
> >          }
> > +
> > +        user = libxl__sprintf(gc, "%s-%s", LIBXL_QEMU_USER_PREFIX, c_info->name);
> > +        if (libxl__dm_runas_helper(gc, user) > 0)
> > +            goto end_search;
> > +
> > +        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-%s or user %s (+domid %d), falling back to %s",
> 
> This message ("+domid %d)" seems to refer to the old way of doing
> things.

OK


> > +                    LIBXL_QEMU_USER_PREFIX, c_info->name, 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
> >  
> >  /*
> 
> 

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

[Ian Campbell]

On Mon, 2015-06-29 at 18:06 +0100, Stefano Stabellini wrote:
> On Thu, 25 Jun 2015, Ian Campbell wrote:
> > On Mon, 2015-06-01 at 17:46 +0100, Stefano Stabellini wrote:
> > > Try to use "xen-qemudepriv-$domname" first, then
> > > "xen-qemudepriv-domid$domid", finally "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@eu.citrix.com>
> > > 
> > > ---
> > > 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 |   34 +++++++++++++++++++++++++
> > >  tools/libxl/libxl_dm.c         |   54 ++++++++++++++++++++++++++++++++++++++++
> > >  tools/libxl/libxl_internal.h   |    4 +++
> > >  4 files changed, 99 insertions(+)
> > >  create mode 100644 docs/misc/qemu-deprivilege.txt
> > > 
> > > diff --git a/INSTALL b/INSTALL
> > > index a0f2e7b..3286a5f 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 regular user.
> > 
> > I'd be tempted to say something like "as a dedicated user" or something,
> > "regular user" sounds like I should be running it as my normal login
> > user or something.
> 
> OK
> 
> 
> > > +
> > > +
> > >  History of options
> > >  ==================
> > >  
> > > diff --git a/docs/misc/qemu-deprivilege.txt b/docs/misc/qemu-deprivilege.txt
> > > new file mode 100644
> > > index 0000000..db61aaf
> > > --- /dev/null
> > > +++ b/docs/misc/qemu-deprivilege.txt
> > > @@ -0,0 +1,34 @@
> > > +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-$domname"
> > > +Where $domname is the name of the domain being created.
> > > +To use this, you just need to create a user with the appropriate name
> > > +for each domain. For example, if your virtual machine is named "windows":
> > > +
> > > +adduser --system xen-qemuuser-windows
> > 
> > I just had an annoying thought: On migration (with xl at least) the
> > receive side appends "-incoming" to the domain name for the duration and
> > then renames ones everything is done, which is suddenly not going to
> > match here :-(
> > 
> > Since this is managed at the xl level I don't see any way libxl could
> > cope without being told separately the domain's "real" name.
> > 
> > I think this reduces the utility of this option quite substantially (or
> > at least presents an enormous caveat), perhaps to the point where we may
> > as well not bother to offer it?
> 
> I think we can detect the "--incoming" and remove it. The only problem
> is that "--incoming" is actually set by xl (xl_cmdimpl.c), not libxl.
> Could we break the abstraction layer just this time and in libxl_dm.c
> have something like:
> 
> 
>      if ((strlen(c_info->name) > strlen("--incoming")) &&
>          (!strcmp(c_info->name + strlen(c_info->name) - strlen("--incoming"), "--incoming"))) {
>          name = libxl__strdup(gc, c_info->name);
>          name[strlen(c_info->name) - strlen("--incoming")] = '\0';
>      } else {
>          name = c_info->name;
>      }
>      user = libxl__sprintf(gc, "%s-%s", LIBXL_QEMU_USER_PREFIX, name);
> 
> 
> This is ugly but it was only meant to demonstrate the idea.

It's not brilliant :-(

At the risk of going off into the weeds, is there any mileage in
exposing some sort of name suffix in the libxl API which can be used by
e.g. xl to handle this stuff? Alternatively some sort of "realname"
field perhaps? Neither very nice, but both better than the layering
violation I think.

> > > +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 in '' $(seq 0 65335)
> > 
> > This creates a user with no $i on it too, which I think is a left over
> > from the previous scheme and no longer useful.
>  
> You are right. I'll turn it into the more bash-like:
> 
> for ((i=1; i<65336; i++))
> do

I just meant to remove the unnecessary '' (and adjust the starting point
for seq).

Ian.

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

[Stefano Stabellini]

On Tue, 30 Jun 2015, Ian Campbell wrote:
> On Mon, 2015-06-29 at 18:06 +0100, Stefano Stabellini wrote:
> > On Thu, 25 Jun 2015, Ian Campbell wrote:
> > > On Mon, 2015-06-01 at 17:46 +0100, Stefano Stabellini wrote:
> > > > Try to use "xen-qemudepriv-$domname" first, then
> > > > "xen-qemudepriv-domid$domid", finally "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@eu.citrix.com>
> > > > 
> > > > ---
> > > > 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 |   34 +++++++++++++++++++++++++
> > > >  tools/libxl/libxl_dm.c         |   54 ++++++++++++++++++++++++++++++++++++++++
> > > >  tools/libxl/libxl_internal.h   |    4 +++
> > > >  4 files changed, 99 insertions(+)
> > > >  create mode 100644 docs/misc/qemu-deprivilege.txt
> > > > 
> > > > diff --git a/INSTALL b/INSTALL
> > > > index a0f2e7b..3286a5f 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 regular user.
> > > 
> > > I'd be tempted to say something like "as a dedicated user" or something,
> > > "regular user" sounds like I should be running it as my normal login
> > > user or something.
> > 
> > OK
> > 
> > 
> > > > +
> > > > +
> > > >  History of options
> > > >  ==================
> > > >  
> > > > diff --git a/docs/misc/qemu-deprivilege.txt b/docs/misc/qemu-deprivilege.txt
> > > > new file mode 100644
> > > > index 0000000..db61aaf
> > > > --- /dev/null
> > > > +++ b/docs/misc/qemu-deprivilege.txt
> > > > @@ -0,0 +1,34 @@
> > > > +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-$domname"
> > > > +Where $domname is the name of the domain being created.
> > > > +To use this, you just need to create a user with the appropriate name
> > > > +for each domain. For example, if your virtual machine is named "windows":
> > > > +
> > > > +adduser --system xen-qemuuser-windows
> > > 
> > > I just had an annoying thought: On migration (with xl at least) the
> > > receive side appends "-incoming" to the domain name for the duration and
> > > then renames ones everything is done, which is suddenly not going to
> > > match here :-(
> > > 
> > > Since this is managed at the xl level I don't see any way libxl could
> > > cope without being told separately the domain's "real" name.
> > > 
> > > I think this reduces the utility of this option quite substantially (or
> > > at least presents an enormous caveat), perhaps to the point where we may
> > > as well not bother to offer it?
> > 
> > I think we can detect the "--incoming" and remove it. The only problem
> > is that "--incoming" is actually set by xl (xl_cmdimpl.c), not libxl.
> > Could we break the abstraction layer just this time and in libxl_dm.c
> > have something like:
> > 
> > 
> >      if ((strlen(c_info->name) > strlen("--incoming")) &&
> >          (!strcmp(c_info->name + strlen(c_info->name) - strlen("--incoming"), "--incoming"))) {
> >          name = libxl__strdup(gc, c_info->name);
> >          name[strlen(c_info->name) - strlen("--incoming")] = '\0';
> >      } else {
> >          name = c_info->name;
> >      }
> >      user = libxl__sprintf(gc, "%s-%s", LIBXL_QEMU_USER_PREFIX, name);
> > 
> > 
> > This is ugly but it was only meant to demonstrate the idea.
> 
> It's not brilliant :-(
> 
> At the risk of going off into the weeds, is there any mileage in
> exposing some sort of name suffix in the libxl API which can be used by
> e.g. xl to handle this stuff? Alternatively some sort of "realname"
> field perhaps? Neither very nice, but both better than the layering
> violation I think.

I guess I could add a constant to libxl.h and use it in both libxl and
xl?

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

[Stefano Stabellini]

On Tue, 30 Jun 2015, Ian Campbell wrote:
# 
> On Tue, 2015-06-30 at 11:46 +0100, Stefano Stabellini wrote:
> > I guess I could add a constant to libxl.h and use it in both libxl and
> > xl?
> 
> Yuk. I'd be more inclined to just drop the domain-name-specific user
> functionality and leave just the per-domid one and catch all fallback.
 
OK

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

[Ian Campbell]

On Tue, 2015-06-30 at 11:46 +0100, Stefano Stabellini wrote:
> I guess I could add a constant to libxl.h and use it in both libxl and
> xl?

Yuk. I'd be more inclined to just drop the domain-name-specific user
functionality and leave just the per-domid one and catch all fallback.

Ian.