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

diff --git a/libvirt.spec.in b/libvirt.spec.in
index abf44bec558a339b05960cf462555149f07e3985..76eb7b972e2b162af6633b66e12388a33d44d43c 100644 (file)
--- a/libvirt.spec.in
+++ b/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

diff --git a/src/qemu_conf.h b/src/qemu_conf.h
index f41c2233be9a28744f1bbf84258e63ed62dbbabc..9fa455975a5e0928029ecc42c0aabf247241e928 100644 (file)
--- a/src/qemu_conf.h
+++ b/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;

diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 099cbe9c2892e8bc03306ff70108dd6968898fb9..5ad434f4c369d5468befe981966a6c9920785add 100644 (file)
--- a/src/qemu_driver.c
+++ b/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);

diff --git a/src/util.c b/src/util.c
index 282f7d91882145ef1d32760d46f9af0634d9a33c..2529837436a784a38977103c93146aac5f600598 100644 (file)
--- a/src/util.c
+++ b/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) &&