[Xen-devel] [PATCH v2 3/6] tools/dm_restrict: Ask QEMU to chroot

[George Dunlap <george.dunlap@xxxxxxxxxx>]

When dm_restrict is enabled, ask QEMU to chroot into an empty directory.

* Create /var/run/qemu/root-domid (deleting the old one if it's there)
* Pass the -chroot option to QEMU

Rather than running `rm -rf` on the directory before creating it
(since there is no library function to do this), simply rmdir the
directory, relying on the fact that the previous QEMU instance, if
properly restcirted, shouldn't have been able to write anything
anyway.

Also test that this worked in depriv-process-checker.sh.  This
requires XEN_RUN_DIR to be set in the environment so we know where to
look.

Suggested-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxx>
---
Not sure the best way to get XEN_RUN_DIR; having configure process this
file seems like a bit overkill.

CC: Ian Jackson <ian.jackson@xxxxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Anthony Perard <anthony.perard@xxxxxxxxxx>
---
 docs/designs/qemu-deprivilege.md             | 12 +++---
 tools/libxl/libxl_dm.c                       | 39 +++++++++++++++++++-
 tools/tests/depriv/depriv-process-checker.sh | 17 +++++++++
 3 files changed, 61 insertions(+), 7 deletions(-)

diff --git a/docs/designs/qemu-deprivilege.md b/docs/designs/qemu-deprivilege.md
index 1e731c16aa..df5bb07d7c 100644
--- a/docs/designs/qemu-deprivilege.md
+++ b/docs/designs/qemu-deprivilege.md
@@ -58,12 +58,6 @@ FIXME: Double-check the correctness of the above
 
 '''Testing status''': Tested
 
-# Restrictions / improvements still to do
-
-This lists potential restrictions still to do.  It is meant to be
-listed in order of ease of implementation, with low-hanging fruit
-first.
-
 ## Chroot
 
 '''Description''': Qemu runs in its own chroot, such that even if it
@@ -81,6 +75,12 @@ Then adds the following to the qemu command-line:
        
 '''Tested''': Not tested
 
+## Restrictions / improvements still to do
+
+This lists potential restrictions still to do.  It is meant to be
+listed in order of ease of implementation, with low-hanging fruit
+first.
+
 ## Namespaces for unused functionality (Linux only)
 
 '''Description''': Enter QEMU into its own mount & IPC namespaces.
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index abd31ee6f2..df6fb64bee 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1410,9 +1410,46 @@ static int libxl__build_device_model_args_new(libxl__gc 
*gc,
         }
     }
 
-    if (libxl_defbool_val(b_info->dm_restrict))
+    if (libxl_defbool_val(b_info->dm_restrict)) {
+        char * chroot_dir = GCSPRINTF("%s/qemu-root-%d",
+                                      libxl__run_dir_path(), guest_domid);
+        
         flexarray_append(dm_args, "-xen-domid-restrict");
 
+        /* 
+         * Run QEMU in a chroot at /var/run/xen/root-%d
+         *
+         * There is no library function to do the equivalent of `rm
+         * -rf`.  However deprivileged QEMU in theory shouldn't be
+         * able to write any files anyway, as the chroot would be
+         * owned by root, but it would be running as an unprivileged
+         * process.  So in theory, old chroots should always be empty.
+         * 
+         * rmdir the directory before attempting to create
+         * it; if it returns anything other than ENOENT, fail domain
+         * creation.
+         */
+        if (rmdir(chroot_dir) != 0
+            && errno != ENOENT) {
+            LOGED(ERROR, guest_domid,
+                  "failed to remove existing chroot dir %s", chroot_dir);
+            return ERROR_FAIL;
+        }
+        
+        for (;;) {
+            if (!mkdir(chroot_dir, 0000))
+                break;
+            if (errno == EINTR) continue;
+            LOGED(ERROR, guest_domid,
+                  "failed to create chroot dir %s", chroot_dir);
+            return ERROR_FAIL;
+        }
+
+        // Add "-chroot [dir]" to command-line
+        flexarray_append(dm_args, "-chroot");
+        flexarray_append(dm_args, chroot_dir);
+    }
+
     if (state->saved_state) {
         /* This file descriptor is meant to be used by QEMU */
         *dm_state_fd = open(state->saved_state, O_RDONLY);
diff --git a/tools/tests/depriv/depriv-process-checker.sh 
b/tools/tests/depriv/depriv-process-checker.sh
index 4aa58e7760..cb240fd0ed 100755
--- a/tools/tests/depriv/depriv-process-checker.sh
+++ b/tools/tests/depriv/depriv-process-checker.sh
@@ -64,6 +64,23 @@ else
 fi
 echo $result
 
+# TEST: chroot
+#
+# Read /proc/<dmpid>/root to see if it's correct.
+echo -n "Chroot: "
+if [[ -n "$XEN_RUN_DIR" ]] ; then
+    tgt_chroot=$XEN_RUN_DIR/qemu-root-$domid
+    root=$(readlink /proc/$dmpid/root)
+    if [[ "$root" != "$tgt_chroot" ]] ; then
+       echo "FAILED"
+       failed="true"
+    else
+       echo "PASSED"
+    fi
+else
+    echo "SKIPPED (XEN_RUN_DIR undefined)"
+fi
+
 if $failed ; then
     exit 1
 else
-- 
2.18.0


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