[PATCH v9] run QEMU as non-root

[PATCH v9] 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. Do not
fall back to root if device_model_user is set. Users can also set
device_model_user by hand in the xl domain config file.

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 v9:
- add a device_model_user option to the xl domain config file

Changes in v8:
- no need to pass the -runas option if the user requested for root
- return ERROR_FAIL from libxl__dm_runas_helper in case of errors
- return NULL from libxl__build_device_model_args_new if libxl__dm_runas_helper failed
- fix line too long
- remove setting errno
- replace retry goto loop, with a while loop
- const char * as argument to libxl__dm_runas_helper
- fix comment

Changes in v7:
- do not fall back to root if the user explicitly set
b_info->device_model_user.

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/man/xl.cfg.pod.5        |    5 ++++
 tools/libxl/libxl.h          |    5 ++++
 tools/libxl/libxl_dm.c       |   67 +++++++++++++++++++++++++++++++++++++++++-
 tools/libxl/libxl_internal.h |    5 ++++
 tools/libxl/libxl_types.idl  |    1 +
 tools/libxl/xl_cmdimpl.c     |    3 ++
 7 files changed, 92 insertions(+), 1 deletion(-)

diff --git a/INSTALL b/INSTALL
index 56e2950..b7e426c 100644
--- a/INSTALL
+++ b/INSTALL
@@ -304,6 +304,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/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index d422924..615822a 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1785,6 +1785,11 @@ Pass additional arbitrary options on the device-model command line for
 an HVM device model only. Each element in the list is passed as an
 option to the device-model.
 
+=item B<device_model_user="username">
+
+Run the device model as user "username", instead of
+xen-qemudepriv-domid$domid or xen-qemudepriv-shared or root.
+
 =back
 
 =head2 Keymaps
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index fa5aedd..108e8fa 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -204,6 +204,11 @@
  */
 #define LIBXL_HAVE_BUILDINFO_ARM_GIC_VERSION 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 89b3bb7..0f1fdcc 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)
 {
@@ -700,6 +702,36 @@ 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, const char *username)
+{
+    struct passwd pwd, *user = NULL;
+    char *buf = NULL;
+    long buf_size;
+    int ret;
+
+    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 ERROR_FAIL;
+    }
+
+    while (1) {
+        buf = libxl__realloc(gc, buf, buf_size);
+        ret = getpwnam_r(username, &pwd, buf, buf_size, &user);
+        if (ret == ERANGE) {
+            buf_size += 128;
+            continue;
+        }
+        if (ret != 0)
+            return ERROR_FAIL;
+        if (user != NULL)
+            return 1;
+        return 0;
+    }
+}
+
 static int libxl__build_device_model_args_new(libxl__gc *gc,
                                         const char *dm, int guest_domid,
                                         const libxl_domain_config *guest_config,
@@ -719,9 +751,10 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
     const char *keymap = dm_keymap(guest_config);
     char *machinearg;
     flexarray_t *dm_args, *dm_envs;
-    int i, connection, devid;
+    int i, connection, devid, ret;
     uint64_t ram_size;
     const char *path, *chardev;
+    char *user = NULL;
 
     dm_args = flexarray_make(gc, 16, 1);
     dm_envs = flexarray_make(gc, 16, 1);
@@ -1179,6 +1212,38 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
         default:
             break;
         }
+
+        if (b_info->device_model_user) {
+            user = b_info->device_model_user;
+            goto end_search;
+        }
+
+        user = libxl__sprintf(gc, "%s%d", LIBXL_QEMU_USER_BASE, guest_domid);
+        ret = libxl__dm_runas_helper(gc, user);
+        if (ret < 0)
+            return ret;
+        if (ret > 0)
+            goto end_search;
+
+        user = LIBXL_QEMU_USER_SHARED;
+        ret = libxl__dm_runas_helper(gc, user);
+        if (ret < 0)
+            return ret;
+        if (ret > 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 != NULL && strcmp(user, "root")) {
+            flexarray_append(dm_args, "-runas");
+            flexarray_append(dm_args, user);
+        }
     }
     flexarray_append(dm_args, NULL);
     *args = (char **) flexarray_contents(dm_args);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 1699f32..41e8607 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -4010,6 +4010,11 @@ void libxl__bitmap_copy_best_effort(libxl__gc *gc, libxl_bitmap *dptr,
                                     const libxl_bitmap *sptr);
 
 int libxl__count_physical_sockets(libxl__gc *gc, int *sockets);
+
+
+#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 502a148..8cd78a8 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -432,6 +432,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),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 9a2870e..1fe0be6 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -2190,6 +2190,9 @@ skip_vfb:
         xlu_cfg_replace_string(config, "device_model_stubdomain_seclabel",
                                &b_info->device_model_ssid_label, 0);
 
+    xlu_cfg_replace_string(config, "device_model_user",
+                           &b_info->device_model_user, 0);
+
 #define parse_extra_args(type)                                            \
     e = xlu_cfg_get_list_as_string_list(config, "device_model_args"#type, \
                                     &b_info->extra##type, 0);            \
-- 
1.7.10.4

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

[Ian Campbell]

On Mon, 2015-11-02 at 12:30 +0000, 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.
> 
> Expose a device_model_user setting in libxl_domain_build_info, so that
> opinionated callers, such as libvirt, can set any user they like. Do not
> fall back to root if device_model_user is set. Users can also set
> device_model_user by hand in the xl domain config file.
> 
> 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>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

(based on previous plus eyeballing only the changes from:
> 
> Changes in v9:
> - add a device_model_user option to the xl domain config file

Ian.

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

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

[Ian Campbell]

On Tue, 2015-11-03 at 16:49 +0000, Ian Campbell wrote:
> On Mon, 2015-11-02 at 12:30 +0000, 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.
> > 
> > Expose a device_model_user setting in libxl_domain_build_info, so that
> > opinionated callers, such as libvirt, can set any user they like. Do
> > not
> > fall back to root if device_model_user is set. Users can also set
> > device_model_user by hand in the xl domain config file.
> > 
> > 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>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>

There were some minor conflicts against some patches committed at the start
of October. I had fixed them up (I think) but then I noticed
that docs/misc/qemu-deprivilege.txt in my working tree wasn't actually
committed.

Since this patch refers to it, but didn't include it I checked before
acking that it was already in tree some how, but didn't realise it wasn't
actually committed (somehow, not sure how). Was it supposed to be in this
patch or was it supposed to be in some earlier patch?

In any case given something odd is clearly going on I don't want to just
commit some random version of that doc which I just found in my working
directory along with this patch. Please can you resubmit with that file
included (or in a precursor patch).

Also please check the coding style of the comment in libxl.h, the "/*"
should be by itself.

Thanks,
Ian.

> 
> (based on previous plus eyeballing only the changes from:
> >  
> > Changes in v9:
> > - add a device_model_user option to the xl domain config file
> 
> Ian.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

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

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 2603 bytes --]

On Tue, 3 Nov 2015, Ian Campbell wrote:
> On Tue, 2015-11-03 at 16:49 +0000, Ian Campbell wrote:
> > On Mon, 2015-11-02 at 12:30 +0000, 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.
> > >
> > > Expose a device_model_user setting in libxl_domain_build_info, so that
> > > opinionated callers, such as libvirt, can set any user they like. Do
> > > not
> > > fall back to root if device_model_user is set. Users can also set
> > > device_model_user by hand in the xl domain config file.
> > >
> > > 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>
> >
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
> There were some minor conflicts against some patches committed at the start
> of October. I had fixed them up (I think) but then I noticed
> that docs/misc/qemu-deprivilege.txt in my working tree wasn't actually
> committed.
>
> Since this patch refers to it, but didn't include it I checked before
> acking that it was already in tree some how, but didn't realise it wasn't
> actually committed (somehow, not sure how). Was it supposed to be in this
> patch or was it supposed to be in some earlier patch?
>
> In any case given something odd is clearly going on I don't want to just
> commit some random version of that doc which I just found in my working
> directory along with this patch. Please can you resubmit with that file
> included (or in a precursor patch).

Done, see v10


> Also please check the coding style of the comment in libxl.h, the "/*"
> should be by itself.

Sorry I forgot this change! Feel free to fix it as you commit if that's
OK for you.


> Thanks,
> Ian.
>
> >
> > (based on previous plus eyeballing only the changes from:
> > >  
> > > Changes in v9:
> > > - add a device_model_user option to the xl domain config file
> >
> > Ian.
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
> 

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

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