[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH 1/3] 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   |    2 +-
 src/qemu_conf.h   |    6 ++++++
 src/qemu_driver.c |   39 ++++++++++++++++++++++++++++++++++-----
 src/util.c        |    8 ++++----
 4 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 067b835..71d2896 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -503,7 +503,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 a126dac..35529cc 100644
--- a/src/qemu_conf.h
+++ b/src/qemu_conf.h
@@ -88,10 +88,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 eb22940..c561c06 100644
--- 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: "
 
@@ -477,6 +474,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);
@@ -497,6 +502,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) {
@@ -505,6 +514,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).
@@ -668,6 +689,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);
@@ -1942,7 +1965,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;
     }
@@ -6540,7 +6563,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);
@@ -6567,6 +6590,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,
@@ -6600,6 +6628,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 0d4f3fa..ab296d8 100644
--- a/src/util.c
+++ b/src/util.c
@@ -509,10 +509,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) {
@@ -549,6 +545,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) &&
-- 
1.6.2.5


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]