libxl: Introduce specific username to be used as a reaper

Untrusted device models must be killed by uid rather than by pid for
safety.  To do this reliably, we need another uid, not used for any
other purpose, from which to make the kill system call.

When using xen-qemuuser-range-base, we can repurpose
xen-qemuuser-range-base itself as a UID from which to kill other
devicemodel uids (since domain ID 0 should never have a device model
associated with it).

However, we'd like people to be able to use the device_model_user
feature without also defining xen-qemuuser-range-base (which requires
the ability to 'reserve' 32k+ user IDs).

To that end, introduce the xen-qemuuser-reaper id.  When killing by
UID, first look for and use that ID if available; then fall back to
xen-qemuuser-range-base.

Document the new call in docs/features/qemu-deprivilege.pandoc.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

diff --git a/docs/features/qemu-deprivilege.pandoc b/docs/features/qemu-deprivilege.pandoc
index ce21a60ef7b207da3d204ac620ebdc402671da6a..eb05981a8360098f34986011f51258875961e624 100644 (file)
--- a/docs/features/qemu-deprivilege.pandoc
+++ b/docs/features/qemu-deprivilege.pandoc
@@ -77,12 +77,21 @@ And then in your config file, the following line:
 
     device_model_user="xen-qemuuser-c6-01"
 
+If you use this method, you should also allocate one "reaper" user to
+be used for killing device models:
+
+    adduser --system --no-create-home --group xen-qemuuser-reaper
+
 NOTE: It is important when using `device_model_user` that EACH VM HAVE
 A SEPARATE UID, and that none of these UIDs map to root.  xl will
 throw an error a uid maps to zero, but not if multiple VMs have the
 same uid.  Multiple VMs with the same device model uid will cause
 problems.
 
+It is also important that `xen-qemuuser-reaper` not have any processes
+associated with it, as they will be destroyed when deprivileged qemu
+processes are destroyed.
+
 ## Domain config changes
 
 The core domain config change is to add the following line to the

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 922fa70f11d593447d3d8eee777ab9f2b518e33b..f5322e3f450a30118ec4df3547b9093616d33a69 100644 (file)
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -268,24 +268,37 @@ static int libxl__get_reaper_uid(libxl__gc *gc, uid_t *reaper_uid)
     struct passwd *user_base, user_pwbuf;
     int rc;
 
+    rc = userlookup_helper_getpwnam(gc, LIBXL_QEMU_USER_REAPER,
+                                         &user_pwbuf, &user_base);
+    /* 
+     * Either there was an error, or we found a suitable user; stop
+     * looking
+     */
+    if (rc || user_base)
+        goto out;
+
     rc = userlookup_helper_getpwnam(gc, LIBXL_QEMU_USER_RANGE_BASE,
                                          &user_pwbuf, &user_base);
-    if (rc)
-        return rc;
+    if (rc || user_base)
+        goto out;
 
-    if (!user_base) {
-        LOG(WARN, "Couldn't find uid for reaper process");
-        return ERROR_INVAL;
-    }
-    
-    if (user_base->pw_uid == 0) {
-        LOG(ERROR, "UID for reaper process maps to root!");
-        return ERROR_INVAL;
+    LOG(WARN, "Couldn't find uid for reaper process");
+    rc = ERROR_INVAL;
+
+ out:
+    /* First check to see if the discovered user maps to root */
+    if (!rc) {
+        if (user_base->pw_uid == 0) {
+            LOG(ERROR, "UID for reaper process maps to root!");
+            rc = ERROR_INVAL;
+        }
     }
 
-    *reaper_uid = user_base->pw_uid;
+    /* If everything is OK, set reaper_uid as appropriate */
+    if (!rc)
+        *reaper_uid = user_base->pw_uid;
 
-    return 0;
+    return rc;
 }
 
 const char *libxl__domain_device_model(libxl__gc *gc,
@@ -2908,9 +2921,6 @@ static int get_reaper_lock_and_uid(libxl__destroy_devicemodel_state *ddms,
 
     /*
      * Get reaper_uid.  If we can't find such a uid, return an error.
-     *
-     * FIXME: This means that domain destruction will fail if
-     * device_model_user is set but QEMU_USER_RANGE_BASE doesn't exist.
      */
     return libxl__get_reaper_uid(gc, reaper_uid);
 }

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 8323c7924d485abed15bf8714f4226015c0b6610..67f3f6ac76e0a809793482e6137d566d4d3736e5 100644 (file)
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -4419,6 +4419,7 @@ _hidden int libxl__read_sysfs_file_contents(libxl__gc *gc,
 #define LIBXL_QEMU_USER_PREFIX "xen-qemuuser"
 #define LIBXL_QEMU_USER_SHARED LIBXL_QEMU_USER_PREFIX"-shared"
 #define LIBXL_QEMU_USER_RANGE_BASE LIBXL_QEMU_USER_PREFIX"-range-base"
+#define LIBXL_QEMU_USER_REAPER LIBXL_QEMU_USER_PREFIX"-reaper"
 
 static inline bool libxl__acpi_defbool_val(const libxl_domain_build_info *b_info)
 {