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

* src/qemu_driver.c: Free the vm->monitor_chr field at VM shutdown. Unlink the UNIX domain socket at VM shutdown to avoid littering FS with old sockets --- src/qemu_driver.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 0342627..b30971c 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -2239,6 +2239,13 @@ static void qemudShutdownVMDaemon(virConnectPtr conn, close(vm->monitor); vm->monitor = -1; + if (vm->monitor_chr) { + if (vm->monitor_chr->type == VIR_DOMAIN_CHR_TYPE_UNIX) + unlink(vm->monitor_chr->data.nix.path); + virDomainChrDefFree(vm->monitor_chr); + vm->monitor_chr = NULL; + } + /* shut it off for sure */ virKillProcess(vm->pid, SIGKILL); -- 1.6.2.5