[libvirt] [PATCH RFC 2/3] security_manager: Introduce {Save, Load}Status

In order to keep security driver's private data for future libvirtd restart, we must copy the behaviour of other drivers and create and load state XML file. This requires that the security driver is able to produce its internal state in XML form and parse it back then. These routines are called whenever security manager API is called, because the manager can't know if the driver's state has changed or not. The driver can decide to not update the XML in which case saveStatus method should return 0 and set @buf argument to NULL. The production of internal state XML can be enforced too. The saveStatus method obtains @force argument set to true in which case it should produce XML even though state hasn't change since last run. This can be handy during migration when transferring the internal state to the destination. The state XML file is kept in the state directory of parent hypervisor driver. --- src/lxc/lxc_controller.c | 2 +- src/lxc/lxc_driver.c | 1 + src/qemu/qemu_driver.c | 3 + src/security/security_driver.h | 9 +++ src/security/security_manager.c | 161 ++++++++++++++++++++++++++++++++++++++- src/security/security_manager.h | 2 + tests/seclabeltest.c | 2 +- tests/securityselinuxlabeltest.c | 3 +- tests/securityselinuxtest.c | 3 +- 9 files changed, 180 insertions(+), 6 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 15aa334..fe50c2a 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1639,7 +1639,7 @@ int main(int argc, char *argv[]) ctrl->handshakeFd = handshakeFd; if (!(ctrl->securityManager = virSecurityManagerNew(securityDriver, - LXC_DRIVER_NAME, + LXC_DRIVER_NAME, NULL, false, false, false))) goto cleanup; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index f136df2..9da2bd9 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1387,6 +1387,7 @@ lxcSecurityInit(virLXCDriverPtr driver) VIR_INFO("lxcSecurityInit %s", driver->securityDriverName); virSecurityManagerPtr mgr = virSecurityManagerNew(driver->securityDriverName, LXC_DRIVER_NAME, + driver->stateDir, false, driver->securityDefaultConfined, driver->securityRequireConfined); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1e96915..3b28a84 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -320,6 +320,7 @@ qemuSecurityInit(virQEMUDriverPtr driver) while (names && *names) { if (!(mgr = virSecurityManagerNew(*names, QEMU_DRIVER_NAME, + cfg->stateDir, cfg->allowDiskFormatProbing, cfg->securityDefaultConfined, cfg->securityRequireConfined))) @@ -337,6 +338,7 @@ qemuSecurityInit(virQEMUDriverPtr driver) } else { if (!(mgr = virSecurityManagerNew(NULL, QEMU_DRIVER_NAME, + cfg->stateDir, cfg->allowDiskFormatProbing, cfg->securityDefaultConfined, cfg->securityRequireConfined))) @@ -348,6 +350,7 @@ qemuSecurityInit(virQEMUDriverPtr driver) if (cfg->privileged) { if (!(mgr = virSecurityManagerNewDAC(QEMU_DRIVER_NAME, + cfg->stateDir, cfg->user, cfg->group, cfg->allowDiskFormatProbing, diff --git a/src/security/security_driver.h b/src/security/security_driver.h index d54f754..ee255a0 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -112,6 +112,12 @@ typedef int (*virSecurityDomainSetHugepages) (virSecurityManagerPtr mgr, virDomainDefPtr def, const char *path); +typedef int (*virSecurityDriverSaveStatus) (virSecurityManagerPtr mgr, + virBufferPtr *buf, + bool force); +typedef int (*virSecurityDriverLoadStatus) (virSecurityManagerPtr mgr, + xmlDocPtr xml); + struct _virSecurityDriver { size_t privateDataLen; const char *name; @@ -122,6 +128,9 @@ struct _virSecurityDriver { virSecurityDriverGetModel getModel; virSecurityDriverGetDOI getDOI; + virSecurityDriverSaveStatus saveStatus; + virSecurityDriverLoadStatus loadStatus; + virSecurityDomainSecurityVerify domainSecurityVerify; virSecurityDomainSetImageLabel domainSetSecurityImageLabel; diff --git a/src/security/security_manager.c b/src/security/security_manager.c index c621366..521901a 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -42,6 +42,7 @@ struct _virSecurityManager { bool defaultConfined; bool requireConfined; const char *virtDriver; + char *stateDir; void *privateData; }; @@ -62,8 +63,109 @@ static int virSecurityManagerOnceInit(void) VIR_ONCE_GLOBAL_INIT(virSecurityManager); +static char * +virSecurityManagerGetStatusFile(const char *stateDir, + const char *drvName) +{ + char *ret = NULL; + + if (virAsprintf(&ret, "%s/%s.xml", stateDir, drvName) < 0) { + virReportOOMError(); + return NULL; + } + + return ret; +} + +static int +virSecurityManagerSaveStatusXML(virSecurityManagerPtr mgr, + const char *xml) +{ + int ret = -1; + char *statusFile = NULL; + + if (!(statusFile = virSecurityManagerGetStatusFile(mgr->stateDir, + mgr->drv->name))) + return ret; + + if (virFileMakePath(mgr->stateDir) < 0) { + virReportSystemError(errno, + _("cannot create status directory '%s'"), + mgr->stateDir); + goto cleanup; + } + + VIR_DEBUG("Saving state file %s for %s security driver", + statusFile, mgr->drv->name); + + ret = virXMLSaveFile(statusFile, NULL, NULL, xml); +cleanup: + VIR_FREE(statusFile); + return ret; +} + +static int +virSecurityManagerSaveStatus(virSecurityManagerPtr mgr) +{ + int ret = -1; + char *xml = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; + virBufferPtr bufptr = &buf; + + if (!mgr->drv->saveStatus) + return 0; + + ret = mgr->drv->saveStatus(mgr, &bufptr, false); + + xml = virBufferContentAndReset(bufptr); + + if ((ret < 0) || !xml) + goto cleanup; + + if ((ret = virSecurityManagerSaveStatusXML(mgr, xml)) < 0) + goto cleanup; + + ret = 0; + +cleanup: + VIR_FREE(xml); + return ret; +} + +static int +virSecurityManagerLoadStatus(virSecurityManagerPtr mgr) +{ + int ret = -1; + char *statusFile = NULL; + xmlDocPtr xml; + + if (!mgr->drv->loadStatus) + return 0; + + if (!(statusFile = virSecurityManagerGetStatusFile(mgr->stateDir, + mgr->drv->name))) + return ret; + + if (!virFileExists(statusFile)) { + VIR_FREE(statusFile); + return 0; + } + + VIR_DEBUG("Loading state file %s for %s security driver", + statusFile, mgr->drv->name); + + if ((xml = virXMLParseFile(statusFile))) { + ret = mgr->drv->loadStatus(mgr, xml); + xmlFreeDoc(xml); + } + + VIR_FREE(statusFile); + return ret; +} + static virSecurityManagerPtr virSecurityManagerNewDriver(virSecurityDriverPtr drv, const char *virtDriver, + const char *stateDir, bool allowDiskFormatProbing, bool defaultConfined, bool requireConfined) @@ -74,9 +176,9 @@ static virSecurityManagerPtr virSecurityManagerNewDriver(virSecurityDriverPtr dr if (virSecurityManagerInitialize() < 0) return NULL; - VIR_DEBUG("drv=%p (%s) virtDriver=%s allowDiskFormatProbing=%d " + VIR_DEBUG("drv=%p (%s) virtDriver=%s stateDir=%s allowDiskFormatProbing=%d " "defaultConfined=%d requireConfined=%d", - drv, drv->name, virtDriver, + drv, drv->name, virtDriver, stateDir, allowDiskFormatProbing, defaultConfined, requireConfined); @@ -97,11 +199,22 @@ static virSecurityManagerPtr virSecurityManagerNewDriver(virSecurityDriverPtr dr mgr->virtDriver = virtDriver; mgr->privateData = privateData; + if (stateDir && + (virAsprintf(&mgr->stateDir, "%s/security", stateDir) < 0)) { + virObjectUnref(mgr); + return NULL; + } + if (drv->open(mgr) < 0) { virObjectUnref(mgr); return NULL; } + if (virSecurityManagerLoadStatus(mgr) < 0) { + virObjectUnref(mgr); + return NULL; + } + return mgr; } @@ -110,6 +223,7 @@ virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary) virSecurityManagerPtr mgr = virSecurityManagerNewDriver(&virSecurityDriverStack, virSecurityManagerGetDriver(primary), + NULL, virSecurityManagerGetAllowDiskFormatProbing(primary), virSecurityManagerGetDefaultConfined(primary), virSecurityManagerGetRequireConfined(primary)); @@ -131,6 +245,7 @@ int virSecurityManagerStackAddNested(virSecurityManagerPtr stack, } virSecurityManagerPtr virSecurityManagerNewDAC(const char *virtDriver, + const char *stateDir, uid_t user, gid_t group, bool allowDiskFormatProbing, @@ -141,6 +256,7 @@ virSecurityManagerPtr virSecurityManagerNewDAC(const char *virtDriver, virSecurityManagerPtr mgr = virSecurityManagerNewDriver(&virSecurityDriverDAC, virtDriver, + stateDir, allowDiskFormatProbing, defaultConfined, requireConfined); @@ -157,6 +273,7 @@ virSecurityManagerPtr virSecurityManagerNewDAC(const char *virtDriver, virSecurityManagerPtr virSecurityManagerNew(const char *name, const char *virtDriver, + const char *stateDir, bool allowDiskFormatProbing, bool defaultConfined, bool requireConfined) @@ -187,6 +304,7 @@ virSecurityManagerPtr virSecurityManagerNew(const char *name, return virSecurityManagerNewDriver(drv, virtDriver, + stateDir, allowDiskFormatProbing, defaultConfined, requireConfined); @@ -225,6 +343,7 @@ static void virSecurityManagerDispose(void *obj) if (mgr->drv->close) mgr->drv->close(mgr); VIR_FREE(mgr->privateData); + VIR_FREE(mgr->stateDir); } const char * @@ -286,6 +405,8 @@ int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr, int ret; virObjectLock(mgr); ret = mgr->drv->domainRestoreSecurityImageLabel(mgr, vm, disk); + if (!ret) + ret = virSecurityManagerSaveStatus(mgr); virObjectUnlock(mgr); return ret; } @@ -301,6 +422,8 @@ int virSecurityManagerSetDaemonSocketLabel(virSecurityManagerPtr mgr, int ret; virObjectLock(mgr); ret = mgr->drv->domainSetSecurityDaemonSocketLabel(mgr, vm); + if (!ret) + ret = virSecurityManagerSaveStatus(mgr); virObjectUnlock(mgr); return ret; } @@ -316,6 +439,8 @@ int virSecurityManagerSetSocketLabel(virSecurityManagerPtr mgr, int ret; virObjectLock(mgr); ret = mgr->drv->domainSetSecuritySocketLabel(mgr, vm); + if (!ret) + ret = virSecurityManagerSaveStatus(mgr); virObjectUnlock(mgr); return ret; } @@ -331,6 +456,8 @@ int virSecurityManagerClearSocketLabel(virSecurityManagerPtr mgr, int ret; virObjectLock(mgr); ret = mgr->drv->domainClearSecuritySocketLabel(mgr, vm); + if (!ret) + ret = virSecurityManagerSaveStatus(mgr); virObjectUnlock(mgr); return ret; } @@ -347,6 +474,8 @@ int virSecurityManagerSetImageLabel(virSecurityManagerPtr mgr, int ret; virObjectLock(mgr); ret = mgr->drv->domainSetSecurityImageLabel(mgr, vm, disk); + if (!ret) + ret = virSecurityManagerSaveStatus(mgr); virObjectUnlock(mgr); return ret; } @@ -364,6 +493,8 @@ int virSecurityManagerRestoreHostdevLabel(virSecurityManagerPtr mgr, int ret; virObjectLock(mgr); ret = mgr->drv->domainRestoreSecurityHostdevLabel(mgr, vm, dev, vroot); + if (!ret) + ret = virSecurityManagerSaveStatus(mgr); virObjectUnlock(mgr); return ret; } @@ -381,6 +512,8 @@ int virSecurityManagerSetHostdevLabel(virSecurityManagerPtr mgr, int ret; virObjectLock(mgr); ret = mgr->drv->domainSetSecurityHostdevLabel(mgr, vm, dev, vroot); + if (!ret) + ret = virSecurityManagerSaveStatus(mgr); virObjectUnlock(mgr); return ret; } @@ -397,6 +530,8 @@ int virSecurityManagerSetSavedStateLabel(virSecurityManagerPtr mgr, int ret; virObjectLock(mgr); ret = mgr->drv->domainSetSavedStateLabel(mgr, vm, savefile); + if (!ret) + ret = virSecurityManagerSaveStatus(mgr); virObjectUnlock(mgr); return ret; } @@ -413,6 +548,8 @@ int virSecurityManagerRestoreSavedStateLabel(virSecurityManagerPtr mgr, int ret; virObjectLock(mgr); ret = mgr->drv->domainRestoreSavedStateLabel(mgr, vm, savefile); + if (!ret) + ret = virSecurityManagerSaveStatus(mgr); virObjectUnlock(mgr); return ret; } @@ -471,6 +608,8 @@ int virSecurityManagerGenLabel(virSecurityManagerPtr mgr, } cleanup: + if ((rc >= 0) && (virSecurityManagerSaveStatus(mgr) < 0)) + rc = -1; virObjectUnlock(mgr); VIR_FREE(sec_managers); return rc; @@ -484,6 +623,8 @@ int virSecurityManagerReserveLabel(virSecurityManagerPtr mgr, int ret; virObjectLock(mgr); ret = mgr->drv->domainReserveSecurityLabel(mgr, vm, pid); + if (!ret) + ret = virSecurityManagerSaveStatus(mgr); virObjectUnlock(mgr); return ret; } @@ -499,6 +640,8 @@ int virSecurityManagerReleaseLabel(virSecurityManagerPtr mgr, int ret; virObjectLock(mgr); ret = mgr->drv->domainReleaseSecurityLabel(mgr, vm); + if (!ret) + ret = virSecurityManagerSaveStatus(mgr); virObjectUnlock(mgr); return ret; } @@ -515,6 +658,8 @@ int virSecurityManagerSetAllLabel(virSecurityManagerPtr mgr, int ret; virObjectLock(mgr); ret = mgr->drv->domainSetSecurityAllLabel(mgr, vm, stdin_path); + if (!ret) + ret = virSecurityManagerSaveStatus(mgr); virObjectUnlock(mgr); return ret; } @@ -531,6 +676,8 @@ int virSecurityManagerRestoreAllLabel(virSecurityManagerPtr mgr, int ret; virObjectLock(mgr); ret = mgr->drv->domainRestoreSecurityAllLabel(mgr, vm, migrated); + if (!ret) + ret = virSecurityManagerSaveStatus(mgr); virObjectUnlock(mgr); return ret; } @@ -548,6 +695,8 @@ int virSecurityManagerGetProcessLabel(virSecurityManagerPtr mgr, int ret; virObjectLock(mgr); ret = mgr->drv->domainGetSecurityProcessLabel(mgr, vm, pid, sec); + if (!ret) + ret = virSecurityManagerSaveStatus(mgr); virObjectUnlock(mgr); return ret; } @@ -563,6 +712,8 @@ int virSecurityManagerSetProcessLabel(virSecurityManagerPtr mgr, int ret; virObjectLock(mgr); ret = mgr->drv->domainSetSecurityProcessLabel(mgr, vm); + if (!ret) + ret = virSecurityManagerSaveStatus(mgr); virObjectUnlock(mgr); return ret; } @@ -618,6 +769,8 @@ int virSecurityManagerSetImageFDLabel(virSecurityManagerPtr mgr, int ret; virObjectLock(mgr); ret = mgr->drv->domainSetSecurityImageFDLabel(mgr, vm, fd); + if (!ret) + ret = virSecurityManagerSaveStatus(mgr); virObjectUnlock(mgr); return ret; } @@ -634,6 +787,8 @@ int virSecurityManagerSetTapFDLabel(virSecurityManagerPtr mgr, int ret; virObjectLock(mgr); ret = mgr->drv->domainSetSecurityTapFDLabel(mgr, vm, fd); + if (!ret) + ret = virSecurityManagerSaveStatus(mgr); virObjectUnlock(mgr); return ret; } @@ -684,6 +839,8 @@ int virSecurityManagerSetHugepages(virSecurityManagerPtr mgr, int ret; virObjectLock(mgr); ret = mgr->drv->domainSetSecurityHugepages(mgr, vm, path); + if (!ret) + ret = virSecurityManagerSaveStatus(mgr); virObjectUnlock(mgr); return ret; } diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 711b354..bc87589 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -31,6 +31,7 @@ typedef virSecurityManager *virSecurityManagerPtr; virSecurityManagerPtr virSecurityManagerNew(const char *name, const char *virtDriver, + const char *stateDir, bool allowDiskFormatProbing, bool defaultConfined, bool requireConfined); @@ -40,6 +41,7 @@ int virSecurityManagerStackAddNested(virSecurityManagerPtr stack, virSecurityManagerPtr nested); virSecurityManagerPtr virSecurityManagerNewDAC(const char *virtDriver, + const char *stateDir, uid_t user, gid_t group, bool allowDiskFormatProbing, diff --git a/tests/seclabeltest.c b/tests/seclabeltest.c index cd34b6b..f0eed63 100644 --- a/tests/seclabeltest.c +++ b/tests/seclabeltest.c @@ -17,7 +17,7 @@ main(int argc ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED) if (virThreadInitialize() < 0) return EXIT_FAILURE; - mgr = virSecurityManagerNew(NULL, "QEMU", false, true, false); + mgr = virSecurityManagerNew(NULL, "QEMU", NULL, false, true, false); if (mgr == NULL) { fprintf(stderr, "Failed to start security driver"); return EXIT_FAILURE; diff --git a/tests/securityselinuxlabeltest.c b/tests/securityselinuxlabeltest.c index 2454772..cc44e7d 100644 --- a/tests/securityselinuxlabeltest.c +++ b/tests/securityselinuxlabeltest.c @@ -314,7 +314,8 @@ mymain(void) { int ret = 0; - if (!(mgr = virSecurityManagerNew("selinux", "QEMU", false, true, false))) { + if (!(mgr = virSecurityManagerNew("selinux", "QEMU", NULL, + false, true, false))) { virErrorPtr err = virGetLastError(); if (err->code == VIR_ERR_CONFIG_UNSUPPORTED) return EXIT_AM_SKIP; diff --git a/tests/securityselinuxtest.c b/tests/securityselinuxtest.c index ba00010..a1f7ddb 100644 --- a/tests/securityselinuxtest.c +++ b/tests/securityselinuxtest.c @@ -271,7 +271,8 @@ mymain(void) int ret = 0; virSecurityManagerPtr mgr; - if (!(mgr = virSecurityManagerNew("selinux", "QEMU", false, true, false))) { + if (!(mgr = virSecurityManagerNew("selinux", "QEMU", NULL, + false, true, false))) { virErrorPtr err = virGetLastError(); if (err->code == VIR_ERR_CONFIG_UNSUPPORTED) return EXIT_AM_SKIP; -- 1.8.1.4