commit bc0010b3d149df00406b82c37eb59874d8525af4 Author: Daniel P. Berrange Date: Wed Nov 11 12:07:00 2009 +0000 Fix save and restore with non-privileged guests and SELinux When running qemu:///system instance, libvirtd runs as root, but QEMU may optionally be configured to run non-root. When then saving a guest to a state file, the file is initially created as root, and thus QEMU cannot write to it. It is also missing labelling required to allow access via SELinux. * src/qemu_driver.c: Set ownership on save image before running migrate command in virDomainSave impl. Call out to security driver to set save image labelling * src/security.h: Add driver APIs for setting and restoring saved state file labelling * src/security_selinux.c: Implement saved state file labelling for SELinux diff --git a/src/security.h b/src/security.h index fde2978..5514962 100644 --- a/src/security.h +++ b/src/security.h @@ -42,6 +42,11 @@ typedef int (*virSecurityDomainRestoreHostdevLabel) (virConnectPtr conn, typedef int (*virSecurityDomainSetHostdevLabel) (virConnectPtr conn, virDomainObjPtr vm, virDomainHostdevDefPtr dev); +typedef int (*virSecurityDomainSetSavedStateLabel) (virConnectPtr conn, + virDomainObjPtr vm, + const char *savefile); +typedef int (*virSecurityDomainRestoreSavedStateLabel) (virConnectPtr conn, + const char *savefile); typedef int (*virSecurityDomainGenLabel) (virConnectPtr conn, virDomainObjPtr sec); typedef int (*virSecurityDomainReserveLabel) (virConnectPtr conn, @@ -71,6 +76,8 @@ struct _virSecurityDriver { virSecurityDomainRestoreLabel domainRestoreSecurityLabel; virSecurityDomainRestoreHostdevLabel domainRestoreSecurityHostdevLabel; virSecurityDomainSetHostdevLabel domainSetSecurityHostdevLabel; + virSecurityDomainSetSavedStateLabel domainSetSavedStateLabel; + virSecurityDomainRestoreSavedStateLabel domainRestoreSavedStateLabel; /* * This is internally managed driver state and should only be accessed diff --git a/src/security_selinux.c b/src/security_selinux.c index 0e31077..bd838e6 100644 --- a/src/security_selinux.c +++ b/src/security_selinux.c @@ -523,6 +523,7 @@ done: return ret; } + static int SELinuxRestoreSecurityPCILabel(virConnectPtr conn, pciDevice *dev ATTRIBUTE_UNUSED, @@ -623,6 +624,26 @@ SELinuxRestoreSecurityLabel(virConnectPtr conn, return rc; } + +static int +SELinuxSetSavedStateLabel(virConnectPtr conn, + virDomainObjPtr vm, + const char *savefile) +{ + const virSecurityLabelDefPtr secdef = &vm->def->seclabel; + + return SELinuxSetFilecon(conn, savefile, secdef->imagelabel); +} + + +static int +SELinuxRestoreSavedStateLabel(virConnectPtr conn, + const char *savefile) +{ + return SELinuxRestoreSecurityFileLabel(conn, savefile); +} + + static int SELinuxSecurityVerify(virConnectPtr conn, virDomainDefPtr def) { @@ -692,4 +713,6 @@ virSecurityDriver virSELinuxSecurityDriver = { .domainSetSecurityLabel = SELinuxSetSecurityLabel, .domainSetSecurityHostdevLabel = SELinuxSetSecurityHostdevLabel, .domainRestoreSecurityHostdevLabel = SELinuxRestoreSecurityHostdevLabel, + .domainSetSavedStateLabel = SELinuxSetSavedStateLabel, + .domainRestoreSavedStateLabel = SELinuxRestoreSavedStateLabel, }; diff -rup libvirt-0.7.1/src/qemu_driver.c new/src/qemu_driver.c --- libvirt-0.7.1/src/qemu_driver.c 2010-05-17 16:28:38.243890000 -0400 +++ new/src/qemu_driver.c 2010-05-17 16:36:28.035091000 -0400 @@ -3907,6 +3907,20 @@ static int qemudDomainSave(virDomainPtr } fd = -1; + if (driver->privileged && + chown(path, driver->user, driver->group) < 0) { + virReportSystemError(NULL, errno, + _("unable to set ownership of '%s' to user %d:%d"), + path, driver->user, driver->group); + goto cleanup; + } + + if (driver->securityDriver && + driver->securityDriver->domainSetSavedStateLabel && + driver->securityDriver->domainSetSavedStateLabel(dom->conn, vm, path) == -1) + goto cleanup; + + /* Migrate to file */ safe_path = qemudEscapeShellArg(path); if (!safe_path) { @@ -3956,6 +3970,20 @@ static int qemudDomainSave(virDomainPtr goto cleanup; } + if (driver->privileged && + chown(path, 0, 0) < 0) { + virReportSystemError(NULL, errno, + _("unable to set ownership of '%s' to user %d:%d"), + path, 0, 0); + goto cleanup; + } + + if (driver->securityDriver && + driver->securityDriver->domainRestoreSavedStateLabel && + driver->securityDriver->domainRestoreSavedStateLabel(dom->conn, path) == -1) + VIR_WARN("failed to restore save state label on %s", path); + + /* Shut it down */ qemudShutdownVMDaemon(dom->conn, driver, vm); event = virDomainEventNewFromObj(vm, diff -rup libvirt-0.7.1/src/qemu_driver.c new/src/qemu_driver.c --- libvirt-0.7.1/src/qemu_driver.c 2010-05-17 17:55:34.000000000 -0400 +++ new/src/qemu_driver.c 2010-05-18 11:45:29.903145000 -0400 @@ -4028,7 +4028,7 @@ static int qemudDomainSave(virDomainPtr if (driver->securityDriver && driver->securityDriver->domainRestoreSavedStateLabel && - driver->securityDriver->domainRestoreSavedStateLabel(dom->conn, path) == -1) + driver->securityDriver->domainRestoreSavedStateLabel(dom->conn, vm, path) == -1) VIR_WARN("failed to restore save state label on %s", path); @@ -4616,6 +4616,11 @@ static int qemudDomainRestore(virConnect } def = NULL; + if (driver->securityDriver && + driver->securityDriver->domainSetSavedStateLabelRO && + driver->securityDriver->domainSetSavedStateLabelRO(conn, vm, path) == -1) + goto cleanup; + if (header.version == 2) { const char *intermediate_argv[3] = { NULL, "-dc", NULL }; const char *prog = qemudSaveCompressionTypeToString(header.compressed); @@ -4651,11 +4656,6 @@ static int qemudDomainRestore(virConnect close(fd); fd = -1; if (ret < 0) { - if (!vm->persistent) { - virDomainRemoveInactive(&driver->domains, - vm); - vm = NULL; - } goto cleanup; } @@ -4677,6 +4677,19 @@ static int qemudDomainRestore(virConnect ret = 0; cleanup: + if (ret < 0) { + if (!vm->persistent) { + virDomainRemoveInactive(&driver->domains, + vm); + vm = NULL; + } + } + + if (driver->securityDriver && + driver->securityDriver->domainRestoreSavedStateLabel && + driver->securityDriver->domainRestoreSavedStateLabel(conn, vm, path) == -1) + VIR_WARN("Unable to restore labelling on %s", path); + virDomainDefFree(def); VIR_FREE(xml); if (fd != -1) diff -rup libvirt-0.7.1/src/security.h new/src/security.h --- libvirt-0.7.1/src/security.h 2010-05-17 17:55:34.000000000 -0400 +++ new/src/security.h 2010-05-18 11:41:27.703746000 -0400 @@ -44,7 +44,11 @@ typedef int (*virSecurityDomainSetHostde typedef int (*virSecurityDomainSetSavedStateLabel) (virConnectPtr conn, virDomainObjPtr vm, const char *savefile); +typedef int (*virSecurityDomainSetSavedStateLabelRO) (virConnectPtr conn, + virDomainObjPtr vm, + const char *savefile); typedef int (*virSecurityDomainRestoreSavedStateLabel) (virConnectPtr conn, + virDomainObjPtr vm, const char *savefile); typedef int (*virSecurityDomainGenLabel) (virConnectPtr conn, virDomainObjPtr sec); @@ -76,6 +80,7 @@ struct _virSecurityDriver { virSecurityDomainRestoreHostdevLabel domainRestoreSecurityHostdevLabel; virSecurityDomainSetHostdevLabel domainSetSecurityHostdevLabel; virSecurityDomainSetSavedStateLabel domainSetSavedStateLabel; + virSecurityDomainSetSavedStateLabelRO domainSetSavedStateLabelRO; virSecurityDomainRestoreSavedStateLabel domainRestoreSavedStateLabel; /* diff -rup libvirt-0.7.1/src/security_selinux.c new/src/security_selinux.c --- libvirt-0.7.1/src/security_selinux.c 2010-05-17 17:55:34.000000000 -0400 +++ new/src/security_selinux.c 2010-05-18 11:49:24.542106000 -0400 @@ -364,12 +364,20 @@ SELinuxRestoreSecurityFileLabel(virConne goto err; } - if (stat(newpath, &buf) != 0) + if (stat(newpath, &buf) != 0) { + virReportSystemError(conn, err, + _("cannot stat %s"), newpath); goto err; + } - if (matchpathcon(newpath, buf.st_mode, &fcon) == 0) { - rc = SELinuxSetFilecon(conn, newpath, fcon); + if (matchpathcon(newpath, buf.st_mode, &fcon) != 0) { + virReportSystemError(conn, err, + _("failed to determine default context for %s"), newpath); + goto err; } + + rc = SELinuxSetFilecon(conn, newpath, fcon); + err: VIR_FREE(fcon); VIR_FREE(newpath); @@ -632,7 +640,17 @@ SELinuxSetSavedStateLabel(virConnectPtr static int +SELinuxSetSavedStateLabelRO(virConnectPtr conn, + virDomainObjPtr vm ATTRIBUTE_UNUSED, + const char *savefile) +{ + return SELinuxSetFilecon(conn, savefile, default_content_context); +} + + +static int SELinuxRestoreSavedStateLabel(virConnectPtr conn, + virDomainObjPtr vm ATTRIBUTE_UNUSED, const char *savefile) { return SELinuxRestoreSecurityFileLabel(conn, savefile); @@ -709,5 +727,6 @@ virSecurityDriver virSELinuxSecurityDriv .domainSetSecurityHostdevLabel = SELinuxSetSecurityHostdevLabel, .domainRestoreSecurityHostdevLabel = SELinuxRestoreSecurityHostdevLabel, .domainSetSavedStateLabel = SELinuxSetSavedStateLabel, + .domainSetSavedStateLabelRO = SELinuxSetSavedStateLabelRO, .domainRestoreSavedStateLabel = SELinuxRestoreSavedStateLabel, };