[Xen-devel] [PATCH 5/9] libxl: Do root checks once in libxl__domain_get_device_model_uid

[George Dunlap <george.dunlap@xxxxxxxxxx>]

At the moment, we check for equivalence to literal "root" before
deciding whether to add the `runas` command-line option to QEMU.  This
is unsatisfactory for several reasons.

First, just because the string doesn't match "root" doesn't mean the
final uid won't end up being zero; in particular, the range_base
calculations may end up producing "0:NNN", which would be root in any
case.

Secondly, it's almost certainly a configuration error if the resulting
uid ends up to be zero; rather than silently do what was specified but
probably not intended, throw an error.

To fix this, check for root once in
libxl__domain_get_device_model_uid.  If the result is root, return an
error; if appropriate, set the user.

After that, assume that the presence of state->dm_runas implies that a
`runas` argument should be constructed.

Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxx>
---
CC: Ian Jackson <ian.jackson@xxxxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
---
 tools/libxl/libxl_dm.c | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 30038eb4e9..3cc6bc0f1d 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -129,8 +129,18 @@ static int libxl__domain_get_device_model_uid(libxl__gc 
*gc,
         return 0;
     
     user = b_info->device_model_user;
-    if (user)
-        goto end_search;
+    if (user) {
+        ret = userlookup_helper_getpwnam(gc, user, &user_pwbuf, &user_base);
+        if (ret < 0)
+            return ret;
+        if (!ret) {
+            LOGD(ERROR, guest_domid,
+                 "Couldn't find device_model_user %s",
+                 user);
+            return -EINVAL;
+        }
+        goto root_check;
+    }
 
     if (!libxl_defbool_val(b_info->dm_restrict)) {
         LOGD(DEBUG, guest_domid,
@@ -156,6 +166,12 @@ static int libxl__domain_get_device_model_uid(libxl__gc 
*gc,
                  guest_domid, user_clash->pw_name);
             return ERROR_FAIL;
         }
+
+        if (intended_uid == 0) {
+            LOGD(ERROR, guest_domid, "intended_uid is 0 (root)!");
+            return ERROR_INVAL;
+        }
+        
         LOGD(DEBUG, guest_domid, "using uid %ld", (long)intended_uid);
         user = GCSPRINTF("%ld:%ld", (long)intended_uid,
                          (long)user_base->pw_gid);
@@ -163,19 +179,26 @@ static int libxl__domain_get_device_model_uid(libxl__gc 
*gc,
     }
     
     user = LIBXL_QEMU_USER_SHARED;
-    ret = userlookup_helper_getpwnam(gc, user, &user_pwbuf, 0);
+    ret = userlookup_helper_getpwnam(gc, user, &user_pwbuf, &user_base);
     if (ret < 0)
         return ret;
     if (ret > 0) {
         LOGD(WARN, guest_domid, "Could not find user %s, falling back to %s",
              LIBXL_QEMU_USER_RANGE_BASE, LIBXL_QEMU_USER_SHARED);
-        goto end_search;
+        goto root_check;
     }
     
     LOGD(ERROR, guest_domid,
          "Could not find user %s or range base pseudo-user %s, cannot 
restrict",
          LIBXL_QEMU_USER_SHARED, LIBXL_QEMU_USER_RANGE_BASE);
     return ERROR_INVAL;
+
+root_check:
+    /* Make sure that the user doesn't map to root. */
+    if (user_base->pw_uid == 0) {
+        LOGD(ERROR, guest_domid, "User %s maps to uid 0 (root)!", user);
+        return ERROR_INVAL;
+    }
     
 end_search:
     state->dm_runas = user;
@@ -1752,7 +1775,7 @@ static int libxl__build_device_model_args_new(libxl__gc 
*gc,
             break;
         }
 
-        if (state->dm_runas && strcmp(state->dm_runas, "root")) {
+        if (state->dm_runas) {
             flexarray_append(dm_args, "-runas");
             flexarray_append(dm_args, state->dm_runas);
         }
-- 
2.19.1


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