[Xen-devel] [PATCH v3 10/11] libxl: Introduce specific username to be used as a reaper

[George Dunlap <george.dunlap@xxxxxxxxxx>]

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@xxxxxxxxxx>
---
v3:
- New in this version

CC: Ian Jackson <ian.jackson@xxxxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Anthony Perard <anthony.perard@xxxxxxxxxx>
---
 docs/features/qemu-deprivilege.pandoc |  9 ++++++
 tools/libxl/libxl_dm.c                | 40 +++++++++++++++++----------
 tools/libxl/libxl_internal.h          |  1 +
 3 files changed, 35 insertions(+), 15 deletions(-)

diff --git a/docs/features/qemu-deprivilege.pandoc 
b/docs/features/qemu-deprivilege.pandoc
index ce21a60ef7..eb05981a83 100644
--- 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 f7c4e5eb3b..731d7f3c2c 100644
--- 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,
@@ -2904,9 +2917,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 e3619daf3a..f588e63599 100644
--- 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)
 {
-- 
2.19.2


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel