Re: [libvirt] [PATCH 5/7] Update QEMU driver to support multiple security drivers

--- src/driver.h | 8 ++- src/qemu/qemu_conf.c | 33 ++++++++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 196 ++++++++++++++++++++++++++++++++++++++--------- src/qemu/qemu_process.c | 53 +++++++++---- 5 files changed, 236 insertions(+), 55 deletions(-)

diff --git a/src/driver.h b/src/driver.h index 03d249b..ca4927f 100644 --- a/src/driver.h +++ b/src/driver.h @@ -305,11 +305,14 @@ typedef int int maplen); typedef int (*virDrvDomainGetMaxVcpus) (virDomainPtr domain); - typedef int - (*virDrvDomainGetSecurityLabel) (virDomainPtr domain, + (*virDrvDomainGetSecurityLabel) (virDomainPtr domain, virSecurityLabelPtr seclabel); typedef int + (*virDrvDomainGetSecurityLabelList) (virDomainPtr domain, + virSecurityLabelPtr seclabels, + int nseclabels); +typedef int (*virDrvNodeGetSecurityModel) (virConnectPtr conn, virSecurityModelPtr secmodel); typedef int @@ -911,6 +914,7 @@ struct _virDriver { virDrvDomainGetVcpus domainGetVcpus; virDrvDomainGetMaxVcpus domainGetMaxVcpus; virDrvDomainGetSecurityLabel domainGetSecurityLabel; + virDrvDomainGetSecurityLabelList domainGetSecurityLabelList; virDrvNodeGetSecurityModel nodeGetSecurityModel; virDrvDomainGetXMLDesc domainGetXMLDesc; virDrvConnectDomainXMLFromNative domainXMLFromNative;

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 88a04bc..5cc2071 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -202,6 +202,39 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, } } + p = virConfGetValue (conf, "additional_security_drivers"); + CHECK_TYPE ("additional_security_driver", VIR_CONF_STRING); + if (p && p->str) { + char *it, *tok; + size_t len; + + for (len = 1, it = p->str; *it; it++) + len++; + if (VIR_ALLOC_N(driver->additionalSecurityDriverNames, len + 1) < 0) { + virReportOOMError(); + virConfFree(conf); + return -1; + }

+ + i = 0; + tok = it = p->str; + while (1) { + if (*it == ',' || *it == '\0') { + driver->additionalSecurityDriverNames[i] = strndup(tok, it - tok);

+ if (driver->additionalSecurityDriverNames == NULL) { + virReportOOMError(); + virConfFree(conf); + return -1; + } + tok = it + 1; + i++; + } + if (*it == '\0') + break; + it++; + } + } +

p = virConfGetValue (conf, "security_default_confined"); CHECK_TYPE ("security_default_confined", VIR_CONF_LONG); if (p) driver->securityDefaultConfined = p->l; diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 482e6d3..a5867b6 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -117,6 +117,7 @@ struct qemud_driver { virDomainEventStatePtr domainEventState; char *securityDriverName; + char **additionalSecurityDriverNames; bool securityDefaultConfined; bool securityRequireConfined; virSecurityManagerPtr securityManager; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index efbc421..39d9eee 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -214,36 +214,84 @@ qemuAutostartDomains(struct qemud_driver *driver) static int qemuSecurityInit(struct qemud_driver *driver) { - virSecurityManagerPtr mgr = virSecurityManagerNew(driver->securityDriverName, - QEMU_DRIVER_NAME, - driver->allowDiskFormatProbing, - driver->securityDefaultConfined, - driver->securityRequireConfined); - + char **names; + virSecurityManagerPtr mgr, nested, stack; + + /* Create primary driver */ + mgr = virSecurityManagerNew(driver->securityDriverName, + QEMU_DRIVER_NAME, + driver->allowDiskFormatProbing, + driver->securityDefaultConfined, + driver->securityRequireConfined); if (!mgr) goto error; - if (driver->privileged) { - virSecurityManagerPtr dac = virSecurityManagerNewDAC(QEMU_DRIVER_NAME, - driver->user, - driver->group, - driver->allowDiskFormatProbing, - driver->securityDefaultConfined, - driver->securityRequireConfined, - driver->dynamicOwnership); - if (!dac) + /* If a DAC driver is required or additional drivers are provived, a stack + * driver should be create to group them all */ + if (driver->privileged || driver->additionalSecurityDriverNames) { + stack = virSecurityManagerNewStack(mgr); + if (!stack) goto error; + mgr = stack; + } + + /* Loop through additional driver names and add a secudary driver to each + * one */ + if (driver->additionalSecurityDriverNames) { + names = driver->additionalSecurityDriverNames; + while (names && *names) { + if (STREQ("dac", *names)) { + /* A DAC driver has specic parameters */ + nested = virSecurityManagerNewDAC(QEMU_DRIVER_NAME, + driver->user, + driver->group, + driver->allowDiskFormatProbing, + driver->securityDefaultConfined, + driver->securityRequireConfined, + driver->dynamicOwnership); + } else { + nested = virSecurityManagerNew(*names, + QEMU_DRIVER_NAME, + driver->allowDiskFormatProbing, + driver->securityDefaultConfined, + driver->securityRequireConfined); + } + if (nested == NULL) + goto error; + if (virSecurityManagerStackAddNested(stack, nested)) + goto error; + names++; + } + } - if (!(driver->securityManager = virSecurityManagerNewStack(mgr, - dac))) { - - virSecurityManagerFree(dac); - goto error; + if (driver->privileged) { + /* When a DAC driver is required, check if there is already one in the + * additional drivers */ + names = driver->additionalSecurityDriverNames; + while (names && *names) { + if (STREQ("dac", *names)) { + break; + } + names++; + } + /* If there isn't a DAC driver, create a new one and add it to the stack + * manager */ + if (names == NULL || *names == NULL) { + nested = virSecurityManagerNewDAC(QEMU_DRIVER_NAME, + driver->user, + driver->group, + driver->allowDiskFormatProbing, + driver->securityDefaultConfined, + driver->securityRequireConfined, + driver->dynamicOwnership); + if (nested == NULL) + goto error; + if (virSecurityManagerStackAddNested(stack, nested)) + goto error; } - } else { - driver->securityManager = mgr; } + driver->securityManager = mgr; return 0; error: @@ -257,7 +305,11 @@ static virCapsPtr qemuCreateCapabilities(virCapsPtr oldcaps, struct qemud_driver *driver) { + size_t i; virCapsPtr caps; + virSecurityManagerPtr *sec_managers = NULL; + /* Security driver data */ + const char *doi, *model; /* Basic host arch / guest machine capabilities */ if (!(caps = qemuCapsInit(oldcaps))) { @@ -282,26 +334,38 @@ qemuCreateCapabilities(virCapsPtr oldcaps, goto err_exit; } - /* Security driver data */ - const char *doi, *model; + /* access sec drivers and create a sec model to each one */ + sec_managers = virSecurityManagerGetNested(driver->securityManager); + if (sec_managers == NULL) { + goto err_exit; + } + + /* calculate length */ + for (i = 0; sec_managers[i]; i++) + ; + caps->host.nsecModels = i; - doi = virSecurityManagerGetDOI(driver->securityManager); - model = virSecurityManagerGetModel(driver->securityManager); - if (STRNEQ(model, "none")) { - if (!(caps->host.secModel.model = strdup(model))) + if (VIR_ALLOC_N(caps->host.secModels, caps->host.nsecModels) < 0) + goto no_memory; + + for (i = 0; sec_managers[i]; i++) { + doi = virSecurityManagerGetDOI(sec_managers[i]); + model = virSecurityManagerGetModel(sec_managers[i]); + if (!(caps->host.secModels[i].model = strdup(model))) goto no_memory; - if (!(caps->host.secModel.doi = strdup(doi))) + if (!(caps->host.secModels[i].doi = strdup(doi))) goto no_memory; + VIR_DEBUG("Initialized caps for security driver \"%s\" with " + "DOI \"%s\"", model, doi); } - - VIR_DEBUG("Initialized caps for security driver \"%s\" with " - "DOI \"%s\"", model, doi); + VIR_FREE(sec_managers); return caps; no_memory: virReportOOMError(); err_exit: + VIR_FREE(sec_managers); virCapabilitiesFree(caps); return NULL; } @@ -3958,6 +4022,63 @@ cleanup: return ret; } +static int qemudDomainGetSecurityLabelList(virDomainPtr dom, + virSecurityLabelPtr seclabel, + int nseclabels) +{ + struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; + virDomainObjPtr vm; + int i, ret = -1; + + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); +

+ memset(seclabel, 0, sizeof(*seclabel)); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemuReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (!virDomainVirtTypeToString(vm->def->virtType)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown virt type in domain definition '%d'"), + vm->def->virtType); + goto cleanup; + } + + /* + * Check the comment in qemudDomainGetSecurityLabel function. + */ + if (virDomainObjIsActive(vm)) { + virSecurityManagerPtr* mgrs = virSecurityManagerGetNested( + driver->securityManager); + if (!mgrs) + goto cleanup; + + for (i = 0; mgrs[i] && i < nseclabels; i++) { + if (virSecurityManagerGetProcessLabel(mgrs[i], vm->def, vm->pid, + seclabel) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Failed to get security label")); + VIR_FREE(mgrs); + goto cleanup; + } + } + VIR_FREE(mgrs); + } + + ret = 0; + +cleanup: + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return ret; +} static int qemudNodeGetSecurityModel(virConnectPtr conn, virSecurityModelPtr secmodel) { @@ -3968,12 +4089,12 @@ static int qemudNodeGetSecurityModel(virConnectPtr conn, qemuDriverLock(driver); memset(secmodel, 0, sizeof(*secmodel)); - /* NULL indicates no driver, which we treat as - * success, but simply return no data in *secmodel */ - if (driver->caps->host.secModel.model == NULL) + /* We treat no driver as success, but simply return no data in *secmodel */ + if (driver->caps->host.nsecModels == 0 || + driver->caps->host.secModels[0].model == NULL) goto cleanup; - p = driver->caps->host.secModel.model; + p = driver->caps->host.secModels[0].model; if (strlen(p) >= VIR_SECURITY_MODEL_BUFLEN-1) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("security model string exceeds max %d bytes"), @@ -3983,7 +4104,7 @@ static int qemudNodeGetSecurityModel(virConnectPtr conn, } strcpy(secmodel->model, p); - p = driver->caps->host.secModel.doi; + p = driver->caps->host.secModels[0].doi; if (strlen(p) >= VIR_SECURITY_DOI_BUFLEN-1) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("security DOI string exceeds max %d bytes"), @@ -13004,6 +13125,7 @@ static virDriver qemuDriver = { .domainGetVcpus = qemudDomainGetVcpus, /* 0.4.4 */ .domainGetMaxVcpus = qemudDomainGetMaxVcpus, /* 0.4.4 */ .domainGetSecurityLabel = qemudDomainGetSecurityLabel, /* 0.6.1 */ + .domainGetSecurityLabelList = qemudDomainGetSecurityLabelList, /* ? */ .nodeGetSecurityModel = qemudNodeGetSecurityModel, /* 0.6.1 */ .domainGetXMLDesc = qemuDomainGetXMLDesc, /* 0.2.0 */ .domainXMLFromNative = qemuDomainXMLFromNative, /* 0.6.4 */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 58ba5bf..59c7e0d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3979,12 +3979,12 @@ void qemuProcessStop(struct qemud_driver *driver, virSecurityManagerReleaseLabel(driver->securityManager, vm->def); /* Clear out dynamically assigned labels */ - if (vm->def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC) { - if (!vm->def->seclabel.baselabel) - VIR_FREE(vm->def->seclabel.model); - VIR_FREE(vm->def->seclabel.label); + for (i = 0; i < vm->def->nseclabels; i++) { + if (vm->def->seclabels[i]->type == VIR_DOMAIN_SECLABEL_DYNAMIC) { + VIR_FREE(vm->def->seclabels[i]->label); + } + VIR_FREE(vm->def->seclabels[i]->imagelabel); } - VIR_FREE(vm->def->seclabel.imagelabel); virDomainDefClearDeviceAliases(vm->def); if (!priv->persistentAddrs) { @@ -4088,6 +4088,7 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, virDomainChrSourceDefPtr monConfig, bool monJSON) { + size_t i; char ebuf[1024]; int logfile = -1; char *timestamp; @@ -4095,6 +4096,9 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, bool running = true; virDomainPausedReason reason; virSecurityLabelPtr seclabel = NULL; + virSecurityLabelDefPtr seclabeldef = NULL; + virSecurityManagerPtr* sec_managers; + const char *model; VIR_DEBUG("Beginning VM attach process"); @@ -4127,17 +4131,35 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, goto no_memory; VIR_DEBUG("Detect security driver config"); - vm->def->seclabel.type = VIR_DOMAIN_SECLABEL_STATIC; - if (VIR_ALLOC(seclabel) < 0) - goto no_memory; - if (virSecurityManagerGetProcessLabel(driver->securityManager, - vm->def, vm->pid, seclabel) < 0) + sec_managers = virSecurityManagerGetNested(driver->securityManager); + if (sec_managers == NULL) { goto cleanup; - if (driver->caps->host.secModel.model && - !(vm->def->seclabel.model = strdup(driver->caps->host.secModel.model))) - goto no_memory; - if (!(vm->def->seclabel.label = strdup(seclabel->label))) - goto no_memory; + } + + for (i = 0; sec_managers[i]; i++) { + model = virSecurityManagerGetModel(sec_managers[i]); + seclabeldef = virDomainDefGetSecurityLabelDef(vm->def, model); + if (seclabeldef == NULL) { + goto cleanup; + } + seclabeldef->type = VIR_DOMAIN_SECLABEL_STATIC; + if (VIR_ALLOC(seclabel) < 0) + goto no_memory; + if (virSecurityManagerGetProcessLabel(driver->securityManager, + vm->def, vm->pid, seclabel) < 0) + goto cleanup; + + //if (driver->caps->host.secModel.model && + // !(seclabeldef.model = strdup(driver->caps->host.secModel.model))) + // goto no_memory; + if (!(seclabeldef->model = strdup(model))) + goto no_memory; + + if (!(seclabeldef->label = strdup(seclabel->label))) + goto no_memory; + VIR_FREE(seclabel); + seclabel = NULL; + } VIR_DEBUG("Creating domain log file"); if ((logfile = qemuDomainCreateLog(driver, vm, false)) < 0) @@ -4256,7 +4278,6 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, goto cleanup; VIR_FORCE_CLOSE(logfile); - VIR_FREE(seclabel); return 0;