[PATCH v6] run QEMU as non-root

[PATCH v6] run QEMU as non-root

[Stefano Stabellini]

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.

Expose a device_model_user setting in libxl_domain_build_info, so that
opinionated callers, such as libvirt, can set any user they like.

QEMU is going to setuid and setgid to the user ID and the group ID of
the specified user, soon after initialization, before starting to deal
with any guest IO.

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 v6:
- add device_model_user to libxl_domain_build_info
- improve doc
- improve wording in commit message

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 |   31 ++++++++++++++++++++++
 tools/libxl/libxl.h            |    6 +++++
 tools/libxl/libxl_dm.c         |   57 ++++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_internal.h   |    4 +++
 tools/libxl/libxl_types.idl    |    1 +
 6 files changed, 106 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..12eb104
--- /dev/null
+++ b/docs/misc/qemu-deprivilege.txt
@@ -0,0 +1,31 @@
+For security reasons, libxl tries to pass a non-root username to QEMU as
+argument. During initialization QEMU calls setuid and setgid with the
+user ID and the group ID of the user passed as argument.
+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 --no-create-home --system xen-qemuuser-domid$i
+done
+
+You might want to consider passing --group to adduser to create a new
+group for each new user.
+
+
+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 --no-create-home --system xen-qemuuser-shared
+
+
+3) root
+As a last resort, libxl will start QEMU as root.
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index efc0617..3f4283f 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -192,6 +192,12 @@
  * is not present, instead of ERROR_INVAL.
  */
 #define LIBXL_HAVE_ERROR_DOMAIN_NOTFOUND 1
+
+/* libxl_domain_build_info has device_model_user to specify the user to
+ * run the device model with. See docs/misc/qemu-deprivilege.txt.
+ */
+#define LIBXL_HAVE_DEVICE_MODEL_USER 1
+
 /*
  * libxl ABI compatibility
  *
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 0c6408d..7038e5c 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 = NULL;
 
     dm_args = flexarray_make(gc, 16, 1);
 
@@ -878,6 +908,33 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
         default:
             break;
         }
+
+        if (b_info->device_model_user) {
+            user = b_info->device_model_user;
+            if (libxl__dm_runas_helper(gc, user) <= 0)
+                user = NULL;
+            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%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
 
 /*
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 023b21e..16d5ddc 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -395,6 +395,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
     ("device_model",     string),
     ("device_model_ssidref", uint32),
     ("device_model_ssid_label", string),
+    ("device_model_user", string),
 
     # extra parameters pass directly to qemu, NULL terminated
     ("extra",            libxl_string_list),
-- 
1.7.10.4

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

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 1986 bytes --]

On Wed, 2015-07-01 at 13:50 +0100, Stefano Stabellini wrote:
> --- /dev/null
> +++ b/docs/misc/qemu-deprivilege.txt
> @@ -0,0 +1,31 @@
> +For security reasons, libxl tries to pass a non-root username to QEMU as
> +argument. During initialization QEMU calls setuid and setgid with the
> +user ID and the group ID of the user passed as argument.
> +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 --no-create-home --system xen-qemuuser-domid$i
> +done
> +
> +You might want to consider passing --group to adduser to create a new
> +group for each new user.
> +
This is, IMHO, a lot of policing, for something like libxl.

I'm saying this only now because, although always being always dubious
about it, it was Jim's comment to v5 that made me realize it properly,
and you're own reply to him in that thread, would actually be a great
alternative!

In fact, what about:
 * in libxl:
    - provide and honour (as first option) device_model_user, as you're 
      doing here;
    - if the above is not provided, check the availability of the 
      'shared' user, and use it if it's there;
    - if that is not there either, use root.
 * in xl (as you said yourself in v5's review):
    - build up the per-domain username and (if available) use
      device_model_user to pass it to libxl.

Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

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

[Stefano Stabellini]

On Wed, 1 Jul 2015, Dario Faggioli wrote:
> On Wed, 2015-07-01 at 13:50 +0100, Stefano Stabellini wrote:
> > --- /dev/null
> > +++ b/docs/misc/qemu-deprivilege.txt
> > @@ -0,0 +1,31 @@
> > +For security reasons, libxl tries to pass a non-root username to QEMU as
> > +argument. During initialization QEMU calls setuid and setgid with the
> > +user ID and the group ID of the user passed as argument.
> > +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 --no-create-home --system xen-qemuuser-domid$i
> > +done
> > +
> > +You might want to consider passing --group to adduser to create a new
> > +group for each new user.
> > +
> This is, IMHO, a lot of policing, for something like libxl.
> 
> I'm saying this only now because, although always being always dubious
> about it, it was Jim's comment to v5 that made me realize it properly,
> and you're own reply to him in that thread, would actually be a great
> alternative!
> 
> In fact, what about:
>  * in libxl:
>     - provide and honour (as first option) device_model_user, as you're 
>       doing here;
>     - if the above is not provided, check the availability of the 
>       'shared' user, and use it if it's there;
>     - if that is not there either, use root.
>  * in xl (as you said yourself in v5's review):
>     - build up the per-domain username and (if available) use
>       device_model_user to pass it to libxl.

That is of course possible and was my first suggestion in my reply.

However after speaking with IanC, I was convinced that offering a good
default security mechanism in libxl is better, so that all libxl users,
including hyper for example, get good security by default without any
need to do anything (except creating some users).

I think that libvirt is a bit of a special case here, given that it
already knows about users. I would expect most other toolstacks not to.

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

[Jim Fehlig]

On 07/01/2015 09:34 AM, Stefano Stabellini wrote:
> On Wed, 1 Jul 2015, Dario Faggioli wrote:
>> On Wed, 2015-07-01 at 13:50 +0100, Stefano Stabellini wrote:
>>> --- /dev/null
>>> +++ b/docs/misc/qemu-deprivilege.txt
>>> @@ -0,0 +1,31 @@
>>> +For security reasons, libxl tries to pass a non-root username to QEMU as
>>> +argument. During initialization QEMU calls setuid and setgid with the
>>> +user ID and the group ID of the user passed as argument.
>>> +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 --no-create-home --system xen-qemuuser-domid$i
>>> +done
>>> +
>>> +You might want to consider passing --group to adduser to create a new
>>> +group for each new user.
>>> +
>> This is, IMHO, a lot of policing, for something like libxl.
>>
>> I'm saying this only now because, although always being always dubious
>> about it, it was Jim's comment to v5 that made me realize it properly,
>> and you're own reply to him in that thread, would actually be a great
>> alternative!
>>
>> In fact, what about:
>>   * in libxl:
>>      - provide and honour (as first option) device_model_user, as you're
>>        doing here;
>>      - if the above is not provided, check the availability of the
>>        'shared' user, and use it if it's there;
>>      - if that is not there either, use root.
>>   * in xl (as you said yourself in v5's review):
>>      - build up the per-domain username and (if available) use
>>        device_model_user to pass it to libxl.
> That is of course possible and was my first suggestion in my reply.
>
> However after speaking with IanC, I was convinced that offering a good
> default security mechanism in libxl is better, so that all libxl users,
> including hyper for example, get good security by default without any
> need to do anything (except creating some users).
>
> I think that libvirt is a bit of a special case here, given that it
> already knows about users. I would expect most other toolstacks not to.

Perhaps.  But thanks for providing a way (b_info->device_model_user) for apps to 
override the libxl policy.

Regards,
Jim

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

[Ian Campbell]

On Wed, 2015-07-01 at 13:50 +0100, Stefano Stabellini wrote:

> @@ -878,6 +908,33 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>          default:
>              break;
>          }
> +
> +        if (b_info->device_model_user) {
> +            user = b_info->device_model_user;
> +            if (libxl__dm_runas_helper(gc, user) <= 0)
> +                user = NULL;

I think this case should be an error, rather than continuing and running
qemu as root.

Ian.

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

[Ian Campbell]

On Wed, 2015-07-01 at 15:03 -0600, Jim Fehlig wrote:
> Perhaps.  But thanks for providing a way (b_info->device_model_user) for apps to 
> override the libxl policy.

You mentioned in v5 that libvirt supports setting both the user and the
group and that the qemu driver supports that. How does that work?

AFAICT qemu's -runas option only takes a user and it takes that user's
primary group and uses that with no configurability. I think that's a
fine way to do things, but you implied greater configurability in
libvirt and I'm now curious...

Ian.

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

[Jim Fehlig]

On 07/09/2015 04:34 AM, Ian Campbell wrote:
> On Wed, 2015-07-01 at 15:03 -0600, Jim Fehlig wrote:
>> Perhaps.  But thanks for providing a way (b_info->device_model_user) for apps to
>> override the libxl policy.
> You mentioned in v5 that libvirt supports setting both the user and the
> group and that the qemu driver supports that. How does that work?
>
> AFAICT qemu's -runas option only takes a user and it takes that user's
> primary group and uses that with no configurability. I think that's a
> fine way to do things, but you implied greater configurability in
> libvirt and I'm now curious...

The libvirt qemu driver doesn't use qemu's -runas option. It calls 
setregid()/setreuid() in the child after fork()'ing, but before exec()'ing, qemu.

Regards,
Jim

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

[Ian Campbell]

On Thu, 2015-07-09 at 17:14 -0600, Jim Fehlig wrote:
> On 07/09/2015 04:34 AM, Ian Campbell wrote:
> > On Wed, 2015-07-01 at 15:03 -0600, Jim Fehlig wrote:
> >> Perhaps.  But thanks for providing a way (b_info->device_model_user) for apps to
> >> override the libxl policy.
> > You mentioned in v5 that libvirt supports setting both the user and the
> > group and that the qemu driver supports that. How does that work?
> >
> > AFAICT qemu's -runas option only takes a user and it takes that user's
> > primary group and uses that with no configurability. I think that's a
> > fine way to do things, but you implied greater configurability in
> > libvirt and I'm now curious...
> 
> The libvirt qemu driver doesn't use qemu's -runas option. It calls 
> setregid()/setreuid() in the child after fork()'ing, but before exec()'ing, qemu.

Makes sense, thanks.

Ian.

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

[Stefano Stabellini]

On Thu, 9 Jul 2015, Ian Campbell wrote:
> On Wed, 2015-07-01 at 13:50 +0100, Stefano Stabellini wrote:
> 
> > @@ -878,6 +908,33 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> >          default:
> >              break;
> >          }
> > +
> > +        if (b_info->device_model_user) {
> > +            user = b_info->device_model_user;
> > +            if (libxl__dm_runas_helper(gc, user) <= 0)
> > +                user = NULL;
> 
> I think this case should be an error, rather than continuing and running
> qemu as root.

I agree. I think it is best to start the guest with -runas user, exactly
as requested and if it doesn't work, we'll catch the error later when it
happens.