[PATCH v2 4/6] tools/dm_restrict: Unshare mount and IPC namespaces on Linux

[George Dunlap <george.dunlap@citrix.com>]

QEMU running under Xen doesn't need mount or IPC functionality.
Create and enter separate namespaces for each of these before
executing QEMU, so that in the event that other restrictions fail, the
process won't be able to even name system mount points or exsting
non-file-based IPC descriptors to attempt to attack them.

Unsharing is something a process can only do to itself (it would
seem); so add an os-specific "dm_preexec_restrict()" hook just before
we exec() the device model.

Also add checks to depriv-process-checker.sh to verify that dm is
running in a new namespace (or at least, a different one than the
caller).

Suggested-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---

CC: Ian Jackson <ian.jackson@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Anthony Perard <anthony.perard@citrix.com>
---
 docs/designs/qemu-deprivilege.md             | 12 ++++++------
 tools/libxl/libxl_dm.c                       |  2 ++
 tools/libxl/libxl_freebsd.c                  |  5 +++++
 tools/libxl/libxl_internal.h                 |  7 +++++++
 tools/libxl/libxl_linux.c                    | 14 ++++++++++++++
 tools/libxl/libxl_netbsd.c                   |  5 +++++
 tools/tests/depriv/depriv-process-checker.sh | 18 ++++++++++++++++++
 7 files changed, 57 insertions(+), 6 deletions(-)

diff --git a/docs/designs/qemu-deprivilege.md b/docs/designs/qemu-deprivilege.md
index df5bb07d7c..58d5df6072 100644
--- a/docs/designs/qemu-deprivilege.md
+++ b/docs/designs/qemu-deprivilege.md
@@ -75,12 +75,6 @@ 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.
@@ -102,6 +96,12 @@ call:
 
 '''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.
+
 ### Basic RLIMITs
 
 '''Description''': A number of limits on the resources that a given
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index df6fb64bee..6733514370 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2391,6 +2391,8 @@ retry_transaction:
         goto out_close;
     if (!rc) { /* inner child */
         setsid();
+        if (libxl_defbool_val(b_info->dm_restrict))
+            libxl__local_dm_preexec_restrict(gc, logfile_w);
         libxl__exec(gc, null, logfile_w, logfile_w, dm, args, envs);
     }
 
diff --git a/tools/libxl/libxl_freebsd.c b/tools/libxl/libxl_freebsd.c
index 6442ccec72..aed2a2b115 100644
--- a/tools/libxl/libxl_freebsd.c
+++ b/tools/libxl/libxl_freebsd.c
@@ -245,3 +245,8 @@ int libxl__pci_topology_init(libxl__gc *gc,
 {
     return ERROR_NI;
 }
+
+void libxl__local_dm_preexec_restrict(libxl__gc *gc, int stderrfd)
+{
+    return;
+}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 802382c704..6e5ff977a6 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3782,6 +3782,13 @@ struct libxl__dm_spawn_state {
 
 _hidden void libxl__spawn_local_dm(libxl__egc *egc, libxl__dm_spawn_state*);
 
+/* 
+ * Called after forking but before executing the local devicemodel.
+ * Must call _exit(-1) on failure, printing an error message to
+ * stderrfd if available.
+ */
+_hidden void libxl__local_dm_preexec_restrict(libxl__gc *gc, int stderrfd);
+
 /* Stubdom device models. */
 
 typedef struct {
diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c
index 6ef0abc693..2eeac8df9a 100644
--- a/tools/libxl/libxl_linux.c
+++ b/tools/libxl/libxl_linux.c
@@ -307,6 +307,20 @@ int libxl__pci_topology_init(libxl__gc *gc,
     return err;
 }
 
+void libxl__local_dm_preexec_restrict(libxl__gc *gc, int stderrfd)
+{
+    int rc;
+
+    /* Unshare mount and IPC namespaces.  These are unused by QEMU. */
+    rc = unshare(CLONE_NEWNS | CLONE_NEWIPC);
+    if (rc < 0) {
+        char *msg = GCSPRINTF("libxl: Mount and IPC namespace unfailed with error %d\n",
+                              errno);
+        write(stderrfd, msg, strlen(msg));
+        _exit(-1);
+    }
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/libxl_netbsd.c b/tools/libxl/libxl_netbsd.c
index 2edfb00641..dce3f1fdce 100644
--- a/tools/libxl/libxl_netbsd.c
+++ b/tools/libxl/libxl_netbsd.c
@@ -124,3 +124,8 @@ int libxl__pci_topology_init(libxl__gc *gc,
 {
     return ERROR_NI;
 }
+
+void libxl__local_dm_preexec_restrict(libxl__gc *gc, int stderrfd)
+{
+    return;
+}
diff --git a/tools/tests/depriv/depriv-process-checker.sh b/tools/tests/depriv/depriv-process-checker.sh
index cb240fd0ed..7dc2573799 100755
--- a/tools/tests/depriv/depriv-process-checker.sh
+++ b/tools/tests/depriv/depriv-process-checker.sh
@@ -81,6 +81,24 @@ else
     echo "SKIPPED (XEN_RUN_DIR undefined)"
 fi
 
+# TEST: Namespace unsharing
+#
+# Read /proc/<dmpid>/ns/<namespace> and make sure it's not equal to
+# the current processes' value
+for nsname in ipc mnt; do
+    echo -n "Unshare namespace $nsname: "
+    dmns=$(readlink /proc/$dmpid/ns/$nsname)
+    myns=$(readlink /proc/self/ns/$nsname)
+
+    if [[ "$dmns" == "$myns" ]] ; then
+	echo "FAILED"
+	failed="true"
+    else
+	echo "PASSED"
+    fi
+done
+
+
 if $failed ; then
     exit 1
 else
-- 
2.18.0


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