From 182a80b9221ab3ce2f90e08852ef4333de64fd3f Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Tue, 25 Aug 2009 16:49:09 +0100 Subject: [PATCH] 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 abf44bec55..76eb7b972e 100644 --- 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 f41c2233be..9fa455975a 100644 --- 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 099cbe9c28..5ad434f4c3 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: " @@ -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 282f7d9188..2529837436 100644 --- 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) && -- GitLab