Move QEMU monitor socket in /var/lib/libvirt/qemu

Separate the guest created QEMU monitor socket location
from the libvirtd create XML / PID data files, to improve
security separation when running QEMU non-root

* libvirt.spec.in: Leave /var/run/libvirt/qemu as root:root
* src/qemu_conf.h: Add libDir and cacheDir directory paths
* src/qemu_driver.c: Move QEMU monitor socket from
  stateDir to libDir to avoid making security critical directory
  accessible to QEMU guests.
* src/util.c: Delay running hook till after damonizing to
  ensure pidfile is still written before changing UID/GID

libvirt.spec.in

@@ -505,7 +505,7 @@ fi
 %dir %attr(0700, root, root) %{_localstatedir}/cache/libvirt/
 
 %if %{with_qemu}
-%dir %attr(0700, %{qemu_user}, %{qemu_group}) %{_localstatedir}/run/libvirt/qemu/
+%dir %attr(0700, root, root) %{_localstatedir}/run/libvirt/qemu/
 %dir %attr(0700, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/
 %dir %attr(0700, %{qemu_user}, %{qemu_group}) %{_localstatedir}/cache/libvirt/qemu/
 %endif

src/qemu_conf.h

@@ -90,10 +90,16 @@ struct qemud_driver {
     virDomainObjList domains;
 
     brControl *brctl;
+    /* These four directories are ones libvirtd uses (so must be root:root
+     * to avoid security risk from QEMU processes */
     char *configDir;
     char *autostartDir;
     char *logDir;
     char *stateDir;
+    /* These two directories are ones QEMU processes use (so must match
+     * the QEMU user/group */
+    char *libDir;
+    char *cacheDir;
     unsigned int vncTLS : 1;
     unsigned int vncTLSx509verify : 1;
     unsigned int vncSASL : 1;

src/qemu_driver.c

@@ -71,9 +71,6 @@
 
 #define VIR_FROM_THIS VIR_FROM_QEMU
 
-/* For storing short-lived temporary files. */
-#define TEMPDIR LOCAL_STATE_DIR "/cache/libvirt/qemu"
-
 #define QEMU_CMD_PROMPT "\n(qemu) "
 #define QEMU_PASSWD_PROMPT "Password: "
 
@@ -490,6 +487,14 @@ qemudStartup(int privileged) {
         if (virAsprintf(&qemu_driver->stateDir,
                       "%s/run/libvirt/qemu", LOCAL_STATE_DIR) == -1)
             goto out_of_memory;
+
+        if (virAsprintf(&qemu_driver->libDir,
+                      "%s/lib/libvirt/qemu", LOCAL_STATE_DIR) == -1)
+            goto out_of_memory;
+
+        if (virAsprintf(&qemu_driver->cacheDir,
+                      "%s/cache/libvirt/qemu", LOCAL_STATE_DIR) == -1)
+            goto out_of_memory;
     } else {
         uid_t uid = geteuid();
         char *userdir = virGetUserDirectory(NULL, uid);
@@ -510,6 +515,10 @@ qemudStartup(int privileged) {
 
         if (virAsprintf(&qemu_driver->stateDir, "%s/qemu/run", base) == -1)
             goto out_of_memory;
+        if (virAsprintf(&qemu_driver->libDir, "%s/qemu/lib", base) == -1)
+            goto out_of_memory;
+        if (virAsprintf(&qemu_driver->cacheDir, "%s/qemu/cache", base) == -1)
+            goto out_of_memory;
     }
 
     if (virFileMakePath(qemu_driver->stateDir) < 0) {
@@ -518,6 +527,18 @@ qemudStartup(int privileged) {
                   qemu_driver->stateDir, virStrerror(errno, ebuf, sizeof ebuf));
         goto error;
     }
+    if (virFileMakePath(qemu_driver->libDir) < 0) {
+        char ebuf[1024];
+        VIR_ERROR(_("Failed to create lib dir '%s': %s\n"),
+                  qemu_driver->libDir, virStrerror(errno, ebuf, sizeof ebuf));
+        goto error;
+    }
+    if (virFileMakePath(qemu_driver->cacheDir) < 0) {
+        char ebuf[1024];
+        VIR_ERROR(_("Failed to create cache dir '%s': %s\n"),
+                  qemu_driver->cacheDir, virStrerror(errno, ebuf, sizeof ebuf));
+        goto error;
+    }
 
     /* Configuration paths are either ~/.libvirt/qemu/... (session) or
      * /etc/libvirt/qemu/... (system).
@@ -712,6 +733,8 @@ qemudShutdown(void) {
     VIR_FREE(qemu_driver->configDir);
     VIR_FREE(qemu_driver->autostartDir);
     VIR_FREE(qemu_driver->stateDir);
+    VIR_FREE(qemu_driver->libDir);
+    VIR_FREE(qemu_driver->cacheDir);
     VIR_FREE(qemu_driver->vncTLSx509certdir);
     VIR_FREE(qemu_driver->vncListen);
     VIR_FREE(qemu_driver->vncPassword);
@@ -1988,7 +2011,7 @@ qemuPrepareMonitorChr(virConnectPtr conn,
     monitor_chr->data.nix.listen = 1;
 
     if (virAsprintf(&monitor_chr->data.nix.path, "%s/%s.monitor",
-                    driver->stateDir, vm) < 0) {
+                    driver->libDir, vm) < 0) {
         virReportOOMError(conn);
         return -1;
     }
@@ -6648,7 +6671,7 @@ qemudDomainMemoryPeek (virDomainPtr dom,
     struct qemud_driver *driver = dom->conn->privateData;
     virDomainObjPtr vm;
     char cmd[256], *info = NULL;
-    char tmp[] = TEMPDIR "/qemu.mem.XXXXXX";
+    char *tmp = NULL;
     int fd = -1, ret = -1;
 
     qemuDriverLock(driver);
@@ -6675,6 +6698,11 @@ qemudDomainMemoryPeek (virDomainPtr dom,
         goto cleanup;
     }
 
+    if (virAsprintf(&tmp, driver->cacheDir,  "/qemu.mem.XXXXXX") < 0) {
+        virReportOOMError(dom->conn);
+        goto cleanup;
+    }
+
     /* Create a temporary filename. */
     if ((fd = mkstemp (tmp)) == -1) {
         virReportSystemError (dom->conn, errno,
@@ -6708,6 +6736,7 @@ qemudDomainMemoryPeek (virDomainPtr dom,
     ret = 0;
 
 cleanup:
+    VIR_FREE(tmp);
     VIR_FREE(info);
     if (fd >= 0) close (fd);
     unlink (tmp);

src/util.c

@@ -511,10 +511,6 @@ __virExec(virConnectPtr conn,
         childerr != childout)
         close(childerr);
 
-    if (hook)
-        if ((hook)(data) != 0)
-            _exit(1);
-
     /* Daemonize as late as possible, so the parent process can detect
      * the above errors with wait* */
     if (flags & VIR_EXEC_DAEMON) {
@@ -551,6 +547,10 @@ __virExec(virConnectPtr conn,
         }
     }
 
+    if (hook)
+        if ((hook)(data) != 0)
+            _exit(1);
+
     /* The steps above may need todo something privileged, so
      * we delay clearing capabilities until the last minute */
     if ((flags & VIR_EXEC_CLEAR_CAPS) &&