Currently, if we label a file to match qemu process DAC label, we do not store the original owner anywhere. So when relabeling back, the only option we have is to relabel to root:root which is obviously wrong. However, bare remembering is not enough. We need to keep track of how many times we labeled a file so only the last restore chown()-s file back to the original owner. In order to not pollute domain XML, this info is kept in driver's private data in a hash table with path being key and pair <oldLabel, refcount> being value. --- src/security/security_dac.c | 351 ++++++++++++++++++++++++++++++++++------- src/security/security_driver.h | 3 + 2 files changed, 296 insertions(+), 58 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 0b274b7..4b8f0a2 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -42,8 +42,121 @@ struct _virSecurityDACData { uid_t user; gid_t group; bool dynamicOwnership; + virHashTablePtr oldOwners; /* to hold pairs <path, virOldLabelPtr> */ }; +struct _virOldLabel { + char *owner; + int refCount; +}; + +static void virOldLabelFree(virOldLabelPtr label) +{ + if (!label) + return; + + VIR_FREE(label->owner); + VIR_FREE(label); +} + +static void +hashDataFree(void *payload, const void *name ATTRIBUTE_UNUSED) +{ + virOldLabelFree(payload); +} + +/** + * virSecurityDACRememberLabel: + * @priv: private DAC driver data + * @path: path which is about to be relabelled + * + * Store the original DAC label of @path. + * Returns: number of references of @path, or -1 on error + */ +static int +virSecurityDACRememberLabel(virSecurityDACDataPtr priv, + const char *path) +{ + struct stat sb; + virOldLabelPtr oldLabel = NULL; + char *user = NULL, *group = NULL; + + oldLabel = virHashLookup(priv->oldOwners, path); + + if (oldLabel) { + /* just increment ref counter */ + oldLabel->refCount++; + goto cleanup; + } + + if (stat(path, &sb) < 0) { + virReportSystemError(errno, _("Unable to stat %s"), path); + goto cleanup; + } + + user = virGetUserName(sb.st_uid); + group = virGetGroupName(sb.st_gid); + if ((!user && (virAsprintf(&user, "+%ld", (long) sb.st_uid) < 0)) || + (!group && (virAsprintf(&group, "+%ld", (long) sb.st_gid) < 0)) || + (VIR_ALLOC(oldLabel) < 0) || + (virAsprintf(&oldLabel->owner, "%s:%s", user, group) < 0)) { + virReportOOMError(); + VIR_FREE(oldLabel); + goto cleanup; + } + + oldLabel->refCount = 1; + if (virHashUpdateEntry(priv->oldOwners, path, oldLabel) < 0) { + virOldLabelFree(oldLabel); + oldLabel = NULL; + } + +cleanup: + VIR_FREE(user); + VIR_FREE(group); + return oldLabel ? oldLabel->refCount : -1; +} + +static int parseIds(const char *label, uid_t *uidPtr, gid_t *gidPtr); + +/** + * virSecurityDACGetRememberedLabel: + * @priv: private DAC driver data + * @path: path which we want to restore label on + * @user: original owner of @path + * @group: original owner of @path + * + * Decrements reference counter on @path. If this was the last + * reference, we need to restore the original label, in which + * case @user and @group are updated. + * Returns: the number of references of @path + * 0 if we need to restore the label + * -1 on error + */ +static int +virSecurityDACGetRememberedLabel(virSecurityDACDataPtr priv, + const char *path, + uid_t *user, + gid_t *group) +{ + int ret = -1; + virOldLabelPtr oldLabel = NULL; + + oldLabel = virHashLookup(priv->oldOwners, path); + if (!oldLabel) + goto cleanup; + + ret = --oldLabel->refCount; + + if (!ret) { + ret = parseIds(oldLabel->owner, user, group); + virHashRemoveEntry(priv->oldOwners, path); + } + +cleanup: + return ret; +} + void virSecurityDACSetUser(virSecurityManagerPtr mgr, uid_t user) { @@ -242,14 +355,20 @@ virSecurityDACProbe(const char *virtDriver ATTRIBUTE_UNUSED) } static int -virSecurityDACOpen(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +virSecurityDACOpen(virSecurityManagerPtr mgr) { - return 0; + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + + priv->oldOwners = virHashCreate(0, hashDataFree); + return priv->oldOwners ? 0 : -1; } static int -virSecurityDACClose(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +virSecurityDACClose(virSecurityManagerPtr mgr) { + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + + virHashFree(priv->oldOwners); return 0; } @@ -306,7 +425,9 @@ virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid) } static int -virSecurityDACRestoreSecurityFileLabel(const char *path) +virSecurityDACRestoreSecurityFileLabel(const char *path, + uid_t user, + gid_t group) { struct stat buf; int rc = -1; @@ -323,8 +444,7 @@ virSecurityDACRestoreSecurityFileLabel(const char *path) if (stat(newpath, &buf) != 0) goto err; - /* XXX record previous ownership */ - rc = virSecurityDACSetOwnership(newpath, 0, 0); + rc = virSecurityDACSetOwnership(newpath, user, group); err: VIR_FREE(newpath); @@ -345,7 +465,8 @@ virSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, uid_t user; gid_t group; - if (virSecurityDACGetImageIds(def, priv, &user, &group)) + if (virSecurityDACGetImageIds(def, priv, &user, &group) || + (virSecurityDACRememberLabel(priv, path) < 0)) return -1; return virSecurityDACSetOwnership(path, user, group); @@ -382,26 +503,14 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, virDomainDiskDefPtr disk, int migrated) { + int ret; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + uid_t user = 0; + gid_t group = 0; - if (!priv->dynamicOwnership) - return 0; - - if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) - return 0; - - /* Don't restore labels on readoly/shared disks, because - * other VMs may still be accessing these - * Alternatively we could iterate over all running - * domains and try to figure out if it is in use, but - * this would not work for clustered filesystems, since - * we can't see running VMs using the file on other nodes - * Safest bet is thus to skip the restore step. - */ - if (disk->readonly || disk->shared) - return 0; - - if (!disk->src) + if (!priv->dynamicOwnership || + (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) || + !disk->src) return 0; /* If we have a shared FS & doing migrated, we must not @@ -411,7 +520,7 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, */ if (migrated) { int rc = virStorageFileIsSharedFS(disk->src); - if (rc < 0) + if (rc > 0) return -1; if (rc == 1) { VIR_DEBUG("Skipping image label restore on %s because FS is shared", @@ -420,7 +529,15 @@ virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, } } - return virSecurityDACRestoreSecurityFileLabel(disk->src); + ret = virSecurityDACGetRememberedLabel(priv, disk->src, &user, &group); + if (ret < 0) + return -1; + else if (ret > 0) { + VIR_DEBUG("Skipping image label restore on %s " + "because the image is still in use", disk->src); + return 0; + } + return virSecurityDACRestoreSecurityFileLabel(disk->src, user, group); } @@ -445,7 +562,8 @@ virSecurityDACSetSecurityPCILabel(virPCIDevicePtr dev ATTRIBUTE_UNUSED, uid_t user; gid_t group; - if (virSecurityDACGetIds(def, priv, &user, &group)) + if (virSecurityDACGetIds(def, priv, &user, &group) || + (virSecurityDACRememberLabel(priv, file) < 0)) return -1; return virSecurityDACSetOwnership(file, user, group); @@ -464,7 +582,8 @@ virSecurityDACSetSecurityUSBLabel(virUSBDevicePtr dev ATTRIBUTE_UNUSED, uid_t user; gid_t group; - if (virSecurityDACGetIds(def, priv, &user, &group)) + if (virSecurityDACGetIds(def, priv, &user, &group) || + (virSecurityDACRememberLabel(priv, file) < 0)) return -1; return virSecurityDACSetOwnership(file, user, group); @@ -536,18 +655,46 @@ done: static int virSecurityDACRestoreSecurityPCILabel(virPCIDevicePtr dev ATTRIBUTE_UNUSED, const char *file, - void *opaque ATTRIBUTE_UNUSED) + void *opaque) { - return virSecurityDACRestoreSecurityFileLabel(file); + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(opaque); + uid_t user = 0; + gid_t group = 0; + int ret; + + ret = virSecurityDACGetRememberedLabel(priv, file, &user, &group); + if (ret < 0) + return -1; + else if (ret > 0) { + VIR_DEBUG("Skipping security label restore on %s " + "because the PCI device is still in use", file); + return 0; + } + + return virSecurityDACRestoreSecurityFileLabel(file, user, group); } static int virSecurityDACRestoreSecurityUSBLabel(virUSBDevicePtr dev ATTRIBUTE_UNUSED, const char *file, - void *opaque ATTRIBUTE_UNUSED) + void *opaque) { - return virSecurityDACRestoreSecurityFileLabel(file); + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(opaque); + uid_t user = 0; + gid_t group = 0; + int ret; + + ret = virSecurityDACGetRememberedLabel(priv, file, &user, &group); + if (ret < 0) + return -1; + else if (ret > 0) { + VIR_DEBUG("Skipping security label restore on %s " + "because the USB device is still in use", file); + return 0; + } + + return virSecurityDACRestoreSecurityFileLabel(file, user, group); } @@ -630,6 +777,8 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, switch (dev->type) { case VIR_DOMAIN_CHR_TYPE_DEV: case VIR_DOMAIN_CHR_TYPE_FILE: + if (virSecurityDACRememberLabel(priv, dev->data.file.path) < 0) + goto cleanup; ret = virSecurityDACSetOwnership(dev->data.file.path, user, group); break; @@ -637,16 +786,22 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, if ((virAsprintf(&in, "%s.in", dev->data.file.path) < 0) || (virAsprintf(&out, "%s.out", dev->data.file.path) < 0)) { virReportOOMError(); - goto done; + goto cleanup; } if (virFileExists(in) && virFileExists(out)) { + if ((virSecurityDACRememberLabel(priv, in) < 0) || + (virSecurityDACRememberLabel(priv, out) < 0)) + goto cleanup; if ((virSecurityDACSetOwnership(in, user, group) < 0) || (virSecurityDACSetOwnership(out, user, group) < 0)) { - goto done; + goto cleanup; } - } else if (virSecurityDACSetOwnership(dev->data.file.path, - user, group) < 0) { - goto done; + } else { + if (virSecurityDACRememberLabel(priv, dev->data.file.path) < 0) + goto cleanup; + if (virSecurityDACSetOwnership(dev->data.file.path, + user, group) < 0) + goto cleanup; } ret = 0; break; @@ -656,38 +811,85 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, break; } -done: +cleanup: VIR_FREE(in); VIR_FREE(out); return ret; } static int -virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, +virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr, virDomainChrSourceDefPtr dev) { + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); char *in = NULL, *out = NULL; int ret = -1; + uid_t user = 0; + gid_t group = 0; switch (dev->type) { case VIR_DOMAIN_CHR_TYPE_DEV: case VIR_DOMAIN_CHR_TYPE_FILE: - ret = virSecurityDACRestoreSecurityFileLabel(dev->data.file.path); + ret = virSecurityDACGetRememberedLabel(priv, dev->data.file.path, + &user, &group); + if (ret < 0) + return -1; + else if (ret > 0) { + VIR_DEBUG("Skipping security label restore on %s " + "because the chardev device is still in use", + dev->data.file.path); + return 0; + } + + ret = virSecurityDACRestoreSecurityFileLabel(dev->data.file.path, + user, group); break; case VIR_DOMAIN_CHR_TYPE_PIPE: if ((virAsprintf(&out, "%s.out", dev->data.file.path) < 0) || (virAsprintf(&in, "%s.in", dev->data.file.path) < 0)) { virReportOOMError(); - goto done; + goto cleanup; } if (virFileExists(in) && virFileExists(out)) { - if ((virSecurityDACRestoreSecurityFileLabel(out) < 0) || - (virSecurityDACRestoreSecurityFileLabel(in) < 0)) { - goto done; + ret = virSecurityDACGetRememberedLabel(priv, in, &user, &group); + if (ret < 0) + goto cleanup; + else if (ret > 0) { + VIR_DEBUG("Skipping security label restore on %s " + "because the chardev device is still in use", in); + goto cleanup; + } + ret = virSecurityDACGetRememberedLabel(priv, out, &user, &group); + if (ret < 0) + goto cleanup; + else if (ret > 0) { + VIR_DEBUG("Skipping security label restore on %s " + "because the chardev device is still in use", out); + goto cleanup; + } + if ((virSecurityDACRestoreSecurityFileLabel(out, user, group) < 0) || + (virSecurityDACRestoreSecurityFileLabel(in, user, group) < 0)) { + ret = -1; + goto cleanup; + } + } else { + ret = virSecurityDACGetRememberedLabel(priv, dev->data.file.path, + &user, &group); + if (ret < 0) + goto cleanup; + else if (ret > 0) { + VIR_DEBUG("Skipping security label restore on %s " + "because the chardev device is still in use", + dev->data.file.path); + goto cleanup; + } + + if (virSecurityDACRestoreSecurityFileLabel(dev->data.file.path, + user, group) < 0) { + ret = -1; + goto cleanup; } - } else if (virSecurityDACRestoreSecurityFileLabel(dev->data.file.path) < 0) { - goto done; } ret = 0; break; @@ -697,7 +899,7 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, break; } -done: +cleanup: VIR_FREE(in); VIR_FREE(out); return ret; @@ -723,6 +925,8 @@ virSecurityDACRestoreSecurityAllLabel(virSecurityManagerPtr mgr, virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); int i; int rc = 0; + uid_t user = 0; + gid_t group = 0; if (!priv->dynamicOwnership) return 0; @@ -752,13 +956,29 @@ virSecurityDACRestoreSecurityAllLabel(virSecurityManagerPtr mgr, mgr) < 0) rc = -1; - if (def->os.kernel && - virSecurityDACRestoreSecurityFileLabel(def->os.kernel) < 0) - rc = -1; + if (def->os.kernel) { + i = virSecurityDACGetRememberedLabel(priv, def->os.kernel, &user, &group); + if (i < 0) + rc = -1; + else if (i > 0) + VIR_DEBUG("Skipping security label restore on %s " + "because the kernel image is still in use", def->os.kernel); + else if (virSecurityDACRestoreSecurityFileLabel(def->os.kernel, + user, group) < 0) + rc = -1; + } - if (def->os.initrd && - virSecurityDACRestoreSecurityFileLabel(def->os.initrd) < 0) - rc = -1; + if (def->os.initrd) { + i = virSecurityDACGetRememberedLabel(priv, def->os.initrd, &user, &group); + if (i < 0) + rc = -1; + else if (i > 0) + VIR_DEBUG("Skipping security label restore on %s " + "because the initrd image is still in use", def->os.initrd); + else if (virSecurityDACRestoreSecurityFileLabel(def->os.initrd, + user, group) < 0) + rc = -1; + } return rc; } @@ -815,11 +1035,13 @@ virSecurityDACSetSecurityAllLabel(virSecurityManagerPtr mgr, return -1; if (def->os.kernel && - virSecurityDACSetOwnership(def->os.kernel, user, group) < 0) + ((virSecurityDACRememberLabel(priv, def->os.kernel) < 0) || + (virSecurityDACSetOwnership(def->os.kernel, user, group) < 0))) return -1; if (def->os.initrd && - virSecurityDACSetOwnership(def->os.initrd, user, group) < 0) + ((virSecurityDACRememberLabel(priv, def->os.initrd) < 0) || + (virSecurityDACSetOwnership(def->os.initrd, user, group) < 0))) return -1; return 0; @@ -835,7 +1057,8 @@ virSecurityDACSetSavedStateLabel(virSecurityManagerPtr mgr, gid_t group; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); - if (virSecurityDACGetImageIds(def, priv, &user, &group)) + if (virSecurityDACGetImageIds(def, priv, &user, &group) || + (virSecurityDACRememberLabel(priv, savefile) < 0)) return -1; return virSecurityDACSetOwnership(savefile, user, group); @@ -848,11 +1071,23 @@ virSecurityDACRestoreSavedStateLabel(virSecurityManagerPtr mgr, const char *savefile) { virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + uid_t user = 0; + gid_t group = 0; + int ret; if (!priv->dynamicOwnership) return 0; - return virSecurityDACRestoreSecurityFileLabel(savefile); + ret = virSecurityDACGetRememberedLabel(priv, savefile, &user, &group); + if (ret < 0) + return -1; + else if (ret > 0) { + VIR_DEBUG("Skipping security label restore on %s " + "because the saved state file is still in use", savefile); + return 0; + } + + return virSecurityDACRestoreSecurityFileLabel(savefile, user, group); } diff --git a/src/security/security_driver.h b/src/security/security_driver.h index cc401e1..d54f754 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -40,6 +40,9 @@ typedef enum { typedef struct _virSecurityDriver virSecurityDriver; typedef virSecurityDriver *virSecurityDriverPtr; +typedef struct _virOldLabel virOldLabel; +typedef virOldLabel *virOldLabelPtr; + typedef virSecurityDriverStatus (*virSecurityDriverProbe) (const char *virtDriver); typedef int (*virSecurityDriverOpen) (virSecurityManagerPtr mgr); typedef int (*virSecurityDriverClose) (virSecurityManagerPtr mgr); -- 1.8.1.4

On 27.02.2013 01:23, Eric Blake wrote: I haven't take look at that. My idea is; store userName:groupName whenever possible; When one of them is not accessible, use +NNN instead. The rationale is - whenever a user or a gropu changes its ID, we will follow it. For instance, a file X has owner A:B (1:1) but libvirt chowns to C:D. Meanwhile A's ID is changed from 1 to 2. So when relabeling we should chown to 2:1 instead of 1:1 as A's ID is 2 not 1. That means I have to do parsing and all the virAsprintf magic. However, maybe this is not what we want and I should really remember just numeric values of IDs which has nice tradeoff - much simpler code. This in fact is thread safe since the security manager from layer just above locks itself before calling any security driver's method. So there's not locking visible here, as we rely on inherited sec driver's lock. But agreed that turning this into virAtomic does no harm. Moreover, we don't need to care for it if we ever drop the upper layer lock. No. Will reorder. Aaah. Okay. Yeah, my intent was to keep things consistent with current libvirt behavior. Yep. I don't know how it got there. Yes. That is what the next two patches do. Moreover, the design may look slightly like an overkill for now, but it is just for easier implementation for migrating the internal state of security driver. Michal

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