[libvirt] [PATCH v2] Per-guest configurable user/group for QEMU processes

This is a v2 patch series that updates the libvirt's security driver mechanism to support per-guest configurable user and group for QEMU processes running together with other security drivers, such as SELinux and AppArmor. Comments and feedbacks are welcome.

This patch updates the structures that store information about each domain and each hypervisor to support multiple security labels and drivers. It also updates all the remaining code to use the new fields. --- src/conf/capabilities.c | 17 ++++-- src/conf/capabilities.h | 6 ++- src/conf/domain_audit.c | 14 +++-- src/conf/domain_conf.c | 86 +++++++++++++++++--------- src/conf/domain_conf.h | 9 ++- src/lxc/lxc_conf.c | 8 ++- src/lxc/lxc_controller.c | 8 +- src/lxc/lxc_driver.c | 35 ++++++----- src/qemu/qemu_driver.c | 15 +++-- src/qemu/qemu_process.c | 18 +++--- src/security/security_manager.c | 10 ++-- src/security/security_selinux.c | 132 +++++++++++++++++++------------------- src/test/test_driver.c | 11 ++- 13 files changed, 217 insertions(+), 152 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index cc7d018..e58e0e8 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -181,8 +181,13 @@ virCapabilitiesFree(virCapsPtr caps) { VIR_FREE(caps->host.migrateTrans); VIR_FREE(caps->host.arch); - VIR_FREE(caps->host.secModel.model); - VIR_FREE(caps->host.secModel.doi); + + for (i = 0; i < caps->host.nsecModels; i++) { + VIR_FREE(caps->host.secModels[i].model); + VIR_FREE(caps->host.secModels[i].doi); + } + VIR_FREE(caps->host.secModels); + virCPUDefFree(caps->host.cpu); VIR_FREE(caps); @@ -767,10 +772,12 @@ virCapabilitiesFormatXML(virCapsPtr caps) virBufferAddLit(&xml, " </topology>\n"); } - if (caps->host.secModel.model) { + for (i = 0; i < caps->host.nsecModels; i++) { virBufferAddLit(&xml, " <secmodel>\n"); - virBufferAsprintf(&xml, " <model>%s</model>\n", caps->host.secModel.model); - virBufferAsprintf(&xml, " <doi>%s</doi>\n", caps->host.secModel.doi); + virBufferAsprintf(&xml, " <model>%s</model>\n", + caps->host.secModels[i].model); + virBufferAsprintf(&xml, " <doi>%s</doi>\n", + caps->host.secModels[i].doi); virBufferAddLit(&xml, " </secmodel>\n"); } diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index 9568b4c..34bd8cd 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -93,6 +93,7 @@ struct _virCapsHostNUMACell { }; typedef struct _virCapsHostSecModel virCapsHostSecModel; +typedef virCapsHostSecModel *virCapsHostSecModelPtr; struct _virCapsHostSecModel { char *model; char *doi; @@ -116,7 +117,10 @@ struct _virCapsHost { size_t nnumaCell; size_t nnumaCell_max; virCapsHostNUMACellPtr *numaCell; - virCapsHostSecModel secModel; + + size_t nsecModels; + virCapsHostSecModelPtr secModels; + virCPUDefPtr cpu; unsigned char host_uuid[VIR_UUID_BUFLEN]; }; diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index cb81fa0..d224682 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -618,6 +618,7 @@ virDomainAuditSecurityLabel(virDomainObjPtr vm, bool success) char uuidstr[VIR_UUID_STRING_BUFLEN]; char *vmname; const char *virt; + int i; virUUIDFormat(vm->def->uuid, uuidstr); if (!(vmname = virAuditEncode("vm", vm->def->name))) { @@ -630,11 +631,14 @@ virDomainAuditSecurityLabel(virDomainObjPtr vm, bool success) virt = "?"; } - VIR_AUDIT(VIR_AUDIT_RECORD_MACHINE_ID, success, - "virt=%s %s uuid=%s vm-ctx=%s img-ctx=%s", - virt, vmname, uuidstr, - VIR_AUDIT_STR(vm->def->seclabel.label), - VIR_AUDIT_STR(vm->def->seclabel.imagelabel)); + for (i = 0; i < vm->def->nseclabels; i++) { + VIR_AUDIT(VIR_AUDIT_RECORD_MACHINE_ID, success, + "virt=%s %s uuid=%s vm-ctx=%s img-ctx=%s model=%s", + virt, vmname, uuidstr, + VIR_AUDIT_STR(vm->def->seclabels[i]->label), + VIR_AUDIT_STR(vm->def->seclabels[i]->imagelabel), + VIR_AUDIT_STR(vm->def->seclabels[i]->model)); + } VIR_FREE(vmname); } diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 398f630..b468174 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -857,12 +857,15 @@ virDomainGraphicsListenDefClear(virDomainGraphicsListenDefPtr def) } static void -virSecurityLabelDefClear(virSecurityLabelDefPtr def) +virSecurityLabelDefFree(virSecurityLabelDefPtr def) { - VIR_FREE(def->model); - VIR_FREE(def->label); - VIR_FREE(def->imagelabel); - VIR_FREE(def->baselabel); + if (def) { + VIR_FREE(def->model); + VIR_FREE(def->label); + VIR_FREE(def->imagelabel); + VIR_FREE(def->baselabel); + VIR_FREE(def); + } } @@ -871,6 +874,7 @@ virSecurityDeviceLabelDefFree(virSecurityDeviceLabelDefPtr def) { if (!def) return; + VIR_FREE(def->model); VIR_FREE(def->label); VIR_FREE(def); } @@ -956,7 +960,11 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def) virStorageEncryptionFree(def->encryption); virDomainDeviceInfoClear(&def->info); - virSecurityDeviceLabelDefFree(def->seclabel); + if (def->seclabels) { + for (i = 0; i < def->nseclabels; i++) + virSecurityDeviceLabelDefFree(def->seclabels[i]); + VIR_FREE(def->seclabels); + } for (i = 0 ; i < def->nhosts ; i++) virDomainDiskHostDefFree(&def->hosts[i]); @@ -1622,7 +1630,9 @@ void virDomainDefFree(virDomainDefPtr def) virDomainMemballoonDefFree(def->memballoon); - virSecurityLabelDefClear(&def->seclabel); + for (i = 0; i < def->nseclabels; i++) + virSecurityLabelDefFree(def->seclabels[i]); + VIR_FREE(def->seclabels); virCPUDefFree(def->cpu); @@ -3077,10 +3087,10 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, { char *p; - if (virXPathNode("./seclabel", ctxt) == NULL) + if (virXPathNode("./seclabel[1]", ctxt) == NULL) return 0; - p = virXPathStringLimit("string(./seclabel/@type)", + p = virXPathStringLimit("string(./seclabel[1]/@type)", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p == NULL) { def->type = VIR_DOMAIN_SECLABEL_DYNAMIC; @@ -3094,7 +3104,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, } } - p = virXPathStringLimit("string(./seclabel/@relabel)", + p = virXPathStringLimit("string(./seclabel[1]/@relabel)", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p != NULL) { if (STREQ(p, "yes")) { @@ -3134,7 +3144,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, if (def->type == VIR_DOMAIN_SECLABEL_STATIC || (!(flags & VIR_DOMAIN_XML_INACTIVE) && def->type != VIR_DOMAIN_SECLABEL_NONE)) { - p = virXPathStringLimit("string(./seclabel/label[1])", + p = virXPathStringLimit("string(./seclabel[1]/label[1])", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p == NULL) { virDomainReportError(VIR_ERR_XML_ERROR, @@ -3149,7 +3159,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, if (!def->norelabel && (!(flags & VIR_DOMAIN_XML_INACTIVE) && def->type != VIR_DOMAIN_SECLABEL_NONE)) { - p = virXPathStringLimit("string(./seclabel/imagelabel[1])", + p = virXPathStringLimit("string(./seclabel[1]/imagelabel[1])", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p == NULL) { virDomainReportError(VIR_ERR_XML_ERROR, @@ -3161,7 +3171,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, /* Only parse baselabel for dynamic label type */ if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC) { - p = virXPathStringLimit("string(./seclabel/baselabel[1])", + p = virXPathStringLimit("string(./seclabel[1]/baselabel[1])", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); def->baselabel = p; } @@ -3173,7 +3183,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, def->baselabel || (!(flags & VIR_DOMAIN_XML_INACTIVE) && def->type != VIR_DOMAIN_SECLABEL_NONE)) { - p = virXPathStringLimit("string(./seclabel/@model)", + p = virXPathStringLimit("string(./seclabel[1]/@model)", VIR_SECURITY_MODEL_BUFLEN-1, ctxt); if (p == NULL) { virDomainReportError(VIR_ERR_XML_ERROR, @@ -3186,7 +3196,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, return 0; error: - virSecurityLabelDefClear(def); + virSecurityLabelDefFree(def); return -1; } @@ -3200,7 +3210,7 @@ virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr *def, *def = NULL; - if (virXPathNode("./seclabel", ctxt) == NULL) + if (virXPathNode("./seclabel[1]", ctxt) == NULL) return 0; /* Can't use overrides if top-level doesn't allow relabeling. */ @@ -3216,7 +3226,7 @@ virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr *def, return -1; } - p = virXPathStringLimit("string(./seclabel/@relabel)", + p = virXPathStringLimit("string(./seclabel[1]/@relabel)", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p != NULL) { if (STREQ(p, "yes")) { @@ -3235,7 +3245,7 @@ virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr *def, (*def)->norelabel = false; } - p = virXPathStringLimit("string(./seclabel/label[1])", + p = virXPathStringLimit("string(./seclabel[1]/label[1])", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); (*def)->label = p; @@ -3669,10 +3679,15 @@ virDomainDiskDefParseXML(virCapsPtr caps, if (sourceNode) { xmlNodePtr saved_node = ctxt->node; ctxt->node = sourceNode; - if (virSecurityDeviceLabelDefParseXML(&def->seclabel, + if ((VIR_ALLOC(def->seclabels) < 0) || (VIR_ALLOC(def->seclabels[0]) < 0)) { + virReportOOMError(); + goto error; + } + if (virSecurityDeviceLabelDefParseXML(&def->seclabels[0], vmSeclabel, ctxt) < 0) goto error; + def->nseclabels = 1; ctxt->node = saved_node; } @@ -7115,10 +7130,16 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, goto error; } + if ((VIR_ALLOC(def->seclabels) < 0) || + (VIR_ALLOC(def->seclabels[0])) < 0 ) { + virReportOOMError(); + goto error; + } + if (xmlStrEqual(node->name, BAD_CAST "disk")) { dev->type = VIR_DOMAIN_DEVICE_DISK; if (!(dev->data.disk = virDomainDiskDefParseXML(caps, node, ctxt, - NULL, &def->seclabel, flags))) + NULL, def->seclabels[0], flags))) goto error; } else if (xmlStrEqual(node->name, BAD_CAST "lease")) { dev->type = VIR_DOMAIN_DEVICE_LEASE; @@ -8017,7 +8038,12 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of security label, done early even though we format it * late, so devices can refer to this for defaults */ - if (virSecurityLabelDefParseXML(&def->seclabel, ctxt, flags) == -1) + if ((VIR_ALLOC(def->seclabels) < 0) || (VIR_ALLOC(def->seclabels[0]) < 0)) { + virReportOOMError(); + goto error; + } + def->nseclabels = 1; + if (virSecurityLabelDefParseXML(def->seclabels[0], ctxt, flags) == -1) goto error; /* Extract domain memory */ @@ -8616,7 +8642,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, nodes[i], ctxt, bootMap, - &def->seclabel, + def->seclabels[0], flags); if (!disk) goto error; @@ -11085,10 +11111,10 @@ virDomainDiskDefFormat(virBufferPtr buf, if (def->startupPolicy) virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); - if (def->seclabel) { + if (def->seclabels && def->seclabels[0]) { virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 8); - virSecurityDeviceLabelDefFormat(buf, def->seclabel); + virSecurityDeviceLabelDefFormat(buf, def->seclabels[0]); virBufferAdjustIndent(buf, -8); virBufferAddLit(buf, " </source>\n"); } else { @@ -11098,10 +11124,10 @@ virDomainDiskDefFormat(virBufferPtr buf, case VIR_DOMAIN_DISK_TYPE_BLOCK: virBufferEscapeString(buf, " <source dev='%s'", def->src); - if (def->seclabel) { + if (def->seclabels && def->seclabels[0]) { virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 8); - virSecurityDeviceLabelDefFormat(buf, def->seclabel); + virSecurityDeviceLabelDefFormat(buf, def->seclabels[0]); virBufferAdjustIndent(buf, -8); virBufferAddLit(buf, " </source>\n"); } else { @@ -13127,9 +13153,11 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAddLit(buf, " </devices>\n"); - virBufferAdjustIndent(buf, 2); - virSecurityLabelDefFormat(buf, &def->seclabel); - virBufferAdjustIndent(buf, -2); + if (def->nseclabels && def->seclabels) { + virBufferAdjustIndent(buf, 2); + virSecurityLabelDefFormat(buf, def->seclabels[0]); + virBufferAdjustIndent(buf, -2); + } if (def->namespaceData && def->ns.format) { if ((def->ns.format)(buf, def->namespaceData) < 0) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 6c777e7..9a2189a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -310,6 +310,7 @@ struct _virSecurityLabelDef { typedef struct _virSecurityDeviceLabelDef virSecurityDeviceLabelDef; typedef virSecurityDeviceLabelDef *virSecurityDeviceLabelDefPtr; struct _virSecurityDeviceLabelDef { + char *model; char *label; /* image label string */ bool norelabel; }; @@ -554,7 +555,6 @@ struct _virDomainDiskDef { int device; int bus; char *src; - virSecurityDeviceLabelDefPtr seclabel; char *dst; int tray_status; int protocol; @@ -594,6 +594,9 @@ struct _virDomainDiskDef { virStorageEncryptionPtr encryption; bool rawio_specified; int rawio; /* no = 0, yes = 1 */ + + size_t nseclabels; + virSecurityDeviceLabelDefPtr *seclabels; }; @@ -1673,8 +1676,10 @@ struct _virDomainDef { int nhubs; virDomainHubDefPtr *hubs; + size_t nseclabels; + virSecurityLabelDefPtr *seclabels; + /* Only 1 */ - virSecurityLabelDef seclabel; virDomainWatchdogDefPtr watchdog; virDomainMemballoonDefPtr memballoon; virCPUDefPtr cpu; diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c index 72547c4..807c704 100644 --- a/src/lxc/lxc_conf.c +++ b/src/lxc/lxc_conf.c @@ -134,9 +134,13 @@ virCapsPtr lxcCapsInit(lxc_driver_t *driver) doi = virSecurityManagerGetDOI(driver->securityManager); model = virSecurityManagerGetModel(driver->securityManager); if (STRNEQ(model, "none")) { - if (!(caps->host.secModel.model = strdup(model))) + /* Allocate just the primary security driver for LXC. */ + if (VIR_ALLOC(caps->host.secModels) < 0) goto no_memory; - if (!(caps->host.secModel.doi = strdup(doi))) + caps->host.nsecModels = 1; + if (!(caps->host.secModels[0].model = strdup(model))) + goto no_memory; + if (!(caps->host.secModels[0].doi = strdup(doi))) goto no_memory; } diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 7a1ce14..fc41c74 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1689,10 +1689,10 @@ int main(int argc, char *argv[]) goto cleanup; VIR_DEBUG("Security model %s type %s label %s imagelabel %s", - NULLSTR(ctrl->def->seclabel.model), - virDomainSeclabelTypeToString(ctrl->def->seclabel.type), - NULLSTR(ctrl->def->seclabel.label), - NULLSTR(ctrl->def->seclabel.imagelabel)); + NULLSTR(ctrl->def->seclabels[0]->model), + virDomainSeclabelTypeToString(ctrl->def->seclabels[0]->type), + NULLSTR(ctrl->def->seclabels[0]->label), + NULLSTR(ctrl->def->seclabels[0]->imagelabel)); ctrl->veths = veths; ctrl->nveths = nveths; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 2d931db..af63cd0 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1489,10 +1489,12 @@ static int lxcVmTerminate(lxc_driver_t *driver, vm->def, false); virSecurityManagerReleaseLabel(driver->securityManager, vm->def); /* Clear out dynamically assigned labels */ - if (vm->def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC) { - VIR_FREE(vm->def->seclabel.model); - VIR_FREE(vm->def->seclabel.label); - VIR_FREE(vm->def->seclabel.imagelabel); + /* Manages just the primary sec driver for lxc */ + if (vm->def->nseclabels + && vm->def->seclabels[0]->type == VIR_DOMAIN_SECLABEL_DYNAMIC) { + VIR_FREE(vm->def->seclabels[0]->model); + VIR_FREE(vm->def->seclabels[0]->label); + VIR_FREE(vm->def->seclabels[0]->imagelabel); } if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) == 0) { @@ -1828,8 +1830,10 @@ static int lxcVmStart(virConnectPtr conn, /* If you are using a SecurityDriver with dynamic labelling, then generate a security label for isolation */ VIR_DEBUG("Generating domain security label (if required)"); - if (vm->def->seclabel.type == VIR_DOMAIN_SECLABEL_DEFAULT) - vm->def->seclabel.type = VIR_DOMAIN_SECLABEL_NONE; + if (vm->def->nseclabels + && vm->def->seclabels[0]->type == VIR_DOMAIN_SECLABEL_DEFAULT) { + vm->def->seclabels[0]->type = VIR_DOMAIN_SECLABEL_NONE; + } if (virSecurityManagerGenLabel(driver->securityManager, vm->def) < 0) { virDomainAuditSecurityLabel(vm, false); @@ -2034,10 +2038,11 @@ cleanup: vm->def, false); virSecurityManagerReleaseLabel(driver->securityManager, vm->def); /* Clear out dynamically assigned labels */ - if (vm->def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC) { - VIR_FREE(vm->def->seclabel.model); - VIR_FREE(vm->def->seclabel.label); - VIR_FREE(vm->def->seclabel.imagelabel); + if (vm->def->nseclabels + && vm->def->seclabels[0]->type == VIR_DOMAIN_SECLABEL_DYNAMIC) { + VIR_FREE(vm->def->seclabels[0]->model); + VIR_FREE(vm->def->seclabels[0]->label); + VIR_FREE(vm->def->seclabels[0]->imagelabel); } } for (i = 0 ; i < nttyFDs ; i++) @@ -2277,12 +2282,12 @@ static int lxcNodeGetSecurityModel(virConnectPtr conn, lxcDriverLock(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; - if (!virStrcpy(secmodel->model, driver->caps->host.secModel.model, + if (!virStrcpy(secmodel->model, driver->caps->host.secModels[0].model, VIR_SECURITY_MODEL_BUFLEN)) { lxcError(VIR_ERR_INTERNAL_ERROR, _("security model string exceeds max %d bytes"), @@ -2291,7 +2296,7 @@ static int lxcNodeGetSecurityModel(virConnectPtr conn, goto cleanup; } - if (!virStrcpy(secmodel->doi, driver->caps->host.secModel.doi, + if (!virStrcpy(secmodel->doi, driver->caps->host.secModels[0].doi, VIR_SECURITY_DOI_BUFLEN)) { lxcError(VIR_ERR_INTERNAL_ERROR, _("security DOI string exceeds max %d bytes"), diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d3988b8..a512a45 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -293,10 +293,15 @@ qemuCreateCapabilities(virCapsPtr oldcaps, doi = virSecurityManagerGetDOI(driver->securityManager); model = virSecurityManagerGetModel(driver->securityManager); + + if (VIR_ALLOC(caps->host.secModels) < 0) { + goto no_memory; + } + if (STRNEQ(model, "none")) { - if (!(caps->host.secModel.model = strdup(model))) + if (!(caps->host.secModels[0].model = strdup(model))) goto no_memory; - if (!(caps->host.secModel.doi = strdup(doi))) + if (!(caps->host.secModels[0].doi = strdup(doi))) goto no_memory; } @@ -4032,10 +4037,10 @@ static int qemudNodeGetSecurityModel(virConnectPtr conn, /* NULL indicates no driver, which we treat as * success, but simply return no data in *secmodel */ - if (driver->caps->host.secModel.model == NULL) + if (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"), @@ -4045,7 +4050,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"), diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d8169e8..e9a41df 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4079,12 +4079,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); + if (vm->def->seclabels[0]->type == VIR_DOMAIN_SECLABEL_DYNAMIC) { + if (!vm->def->seclabels[0]->baselabel) + VIR_FREE(vm->def->seclabels[0]->model); + VIR_FREE(vm->def->seclabels[0]->label); } - VIR_FREE(vm->def->seclabel.imagelabel); + VIR_FREE(vm->def->seclabels[0]->imagelabel); virDomainDefClearDeviceAliases(vm->def); if (!priv->persistentAddrs) { @@ -4227,16 +4227,16 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, goto no_memory; VIR_DEBUG("Detect security driver config"); - vm->def->seclabel.type = VIR_DOMAIN_SECLABEL_STATIC; + vm->def->seclabels[0]->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 && - !(vm->def->seclabel.model = strdup(driver->caps->host.secModel.model))) + if (driver->caps->host.secModels[0].model && + !(vm->def->seclabels[0]->model = strdup(driver->caps->host.secModels[0].model))) goto no_memory; - if (!(vm->def->seclabel.label = strdup(seclabel->label))) + if (!(vm->def->seclabels[0]->label = strdup(seclabel->label))) goto no_memory; VIR_DEBUG("Creating domain log file"); diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 8ec4d3e..5ca3201 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -308,14 +308,14 @@ int virSecurityManagerRestoreSavedStateLabel(virSecurityManagerPtr mgr, int virSecurityManagerGenLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm) { - if (vm->seclabel.type == VIR_DOMAIN_SECLABEL_DEFAULT) { + if (vm->seclabels[0]->type == VIR_DOMAIN_SECLABEL_DEFAULT) { if (mgr->defaultConfined) - vm->seclabel.type = VIR_DOMAIN_SECLABEL_DYNAMIC; + vm->seclabels[0]->type = VIR_DOMAIN_SECLABEL_DYNAMIC; else - vm->seclabel.type = VIR_DOMAIN_SECLABEL_NONE; + vm->seclabels[0]->type = VIR_DOMAIN_SECLABEL_NONE; } - if ((vm->seclabel.type == VIR_DOMAIN_SECLABEL_NONE) && + if ((vm->seclabels[0]->type == VIR_DOMAIN_SECLABEL_NONE) && mgr->requireConfined) { virSecurityReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Unconfined guests are not allowed on this host")); @@ -397,7 +397,7 @@ int virSecurityManagerSetProcessLabel(virSecurityManagerPtr mgr, int virSecurityManagerVerify(virSecurityManagerPtr mgr, virDomainDefPtr def) { - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = def->seclabels[0]; /* NULL model == dynamic labelling, with whatever driver * is active, so we can short circuit verify check to * avoid drivers de-referencing NULLs by accident diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index ffa65fb..9a708b1 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -274,43 +274,43 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr, virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr); VIR_DEBUG("SELinuxGenSecurityLabel %s", virSecurityManagerGetDriver(mgr)); - if ((def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC) && - !def->seclabel.baselabel && - def->seclabel.model) { + if ((def->seclabels[0]->type == VIR_DOMAIN_SECLABEL_DYNAMIC) && + !def->seclabels[0]->baselabel && + def->seclabels[0]->model) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("security model already defined for VM")); return rc; } - if (def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC && - def->seclabel.label) { + if (def->seclabels[0]->type == VIR_DOMAIN_SECLABEL_DYNAMIC && + def->seclabels[0]->label) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("security label already defined for VM")); return rc; } - if (def->seclabel.imagelabel) { + if (def->seclabels[0]->imagelabel) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("security image label already defined for VM")); return rc; } - if (def->seclabel.model && - STRNEQ(def->seclabel.model, SECURITY_SELINUX_NAME)) { + if (def->seclabels[0]->model && + STRNEQ(def->seclabels[0]->model, SECURITY_SELINUX_NAME)) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("security label model %s is not supported with selinux"), - def->seclabel.model); + def->seclabels[0]->model); return rc; } - VIR_DEBUG("SELinuxGenSecurityLabel %d", def->seclabel.type); + VIR_DEBUG("SELinuxGenSecurityLabel %d", def->seclabels[0]->type); - switch (def->seclabel.type) { + switch (def->seclabels[0]->type) { case VIR_DOMAIN_SECLABEL_STATIC: - if (!(ctx = context_new(def->seclabel.label)) ) { + if (!(ctx = context_new(def->seclabels[0]->label)) ) { virReportSystemError(errno, _("unable to allocate socket security context '%s'"), - def->seclabel.label); + def->seclabels[0]->label); return rc; } @@ -345,11 +345,11 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr, } } while (mcsAdd(mcs) == -1); - def->seclabel.label = - SELinuxGenNewContext(def->seclabel.baselabel ? - def->seclabel.baselabel : + def->seclabels[0]->label = + SELinuxGenNewContext(def->seclabels[0]->baselabel ? + def->seclabels[0]->baselabel : data->domain_context, mcs); - if (! def->seclabel.label) { + if (! def->seclabels[0]->label) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("cannot generate selinux context for %s"), mcs); goto cleanup; @@ -363,21 +363,21 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr, default: virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected security label type '%s'"), - virDomainSeclabelTypeToString(def->seclabel.type)); + virDomainSeclabelTypeToString(def->seclabels[0]->type)); goto cleanup; } - if (!def->seclabel.norelabel) { - def->seclabel.imagelabel = SELinuxGenNewContext(data->file_context, mcs); - if (!def->seclabel.imagelabel) { + if (!def->seclabels[0]->norelabel) { + def->seclabels[0]->imagelabel = SELinuxGenNewContext(data->file_context, mcs); + if (!def->seclabels[0]->imagelabel) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("cannot generate selinux context for %s"), mcs); goto cleanup; } } - if (!def->seclabel.model && - !(def->seclabel.model = strdup(SECURITY_SELINUX_NAME))) { + if (!def->seclabels[0]->model && + !(def->seclabels[0]->model = strdup(SECURITY_SELINUX_NAME))) { virReportOOMError(); goto cleanup; } @@ -386,12 +386,12 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr, cleanup: if (rc != 0) { - if (def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC) - VIR_FREE(def->seclabel.label); - VIR_FREE(def->seclabel.imagelabel); - if (def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC && - !def->seclabel.baselabel) - VIR_FREE(def->seclabel.model); + if (def->seclabels[0]->type == VIR_DOMAIN_SECLABEL_DYNAMIC) + VIR_FREE(def->seclabels[0]->label); + VIR_FREE(def->seclabels[0]->imagelabel); + if (def->seclabels[0]->type == VIR_DOMAIN_SECLABEL_DYNAMIC && + !def->seclabels[0]->baselabel) + VIR_FREE(def->seclabels[0]->model); } if (ctx) @@ -400,10 +400,10 @@ cleanup: VIR_FREE(mcs); VIR_DEBUG("model=%s label=%s imagelabel=%s baselabel=%s", - NULLSTR(def->seclabel.model), - NULLSTR(def->seclabel.label), - NULLSTR(def->seclabel.imagelabel), - NULLSTR(def->seclabel.baselabel)); + NULLSTR(def->seclabels[0]->model), + NULLSTR(def->seclabels[0]->label), + NULLSTR(def->seclabels[0]->imagelabel), + NULLSTR(def->seclabels[0]->baselabel)); return rc; } @@ -417,7 +417,7 @@ SELinuxReserveSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, context_t ctx = NULL; const char *mcs; - if (def->seclabel.type == VIR_DOMAIN_SECLABEL_STATIC) + if (def->seclabels[0]->type == VIR_DOMAIN_SECLABEL_STATIC) return 0; if (getpidcon(pid, &pctx) == -1) { @@ -709,9 +709,9 @@ SELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDiskDefPtr disk, int migrated) { - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = def->seclabels[0]; - if (secdef->norelabel || (disk->seclabel && disk->seclabel->norelabel)) + if (secdef->norelabel || (disk->seclabels[0] && disk->seclabels[0]->norelabel)) return 0; /* Don't restore labels on readoly/shared disks, because @@ -768,12 +768,12 @@ SELinuxSetSecurityFileLabel(virDomainDiskDefPtr disk, int ret; virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(cbdata->manager); - if (disk->seclabel && disk->seclabel->norelabel) + if (disk->seclabels[0] && disk->seclabels[0]->norelabel) return 0; - if (disk->seclabel && !disk->seclabel->norelabel && - disk->seclabel->label) { - ret = SELinuxSetFilecon(path, disk->seclabel->label); + if (disk->seclabels[0] && !disk->seclabels[0]->norelabel && + disk->seclabels[0]->label) { + ret = SELinuxSetFilecon(path, disk->seclabels[0]->label); } else if (depth == 0) { if (disk->shared) { @@ -788,14 +788,14 @@ SELinuxSetSecurityFileLabel(virDomainDiskDefPtr disk, } else { ret = SELinuxSetFileconOptional(path, data->content_context); } - if (ret == 1 && !disk->seclabel) { + if (ret == 1 && !disk->seclabels[0]) { /* If we failed to set a label, but virt_use_nfs let us * proceed anyway, then we don't need to relabel later. */ - if (VIR_ALLOC(disk->seclabel) < 0) { + if (VIR_ALLOC(disk->seclabels[0]) < 0) { virReportOOMError(); return -1; } - disk->seclabel->norelabel = true; + disk->seclabels[0]->norelabel = true; ret = 0; } return ret; @@ -808,7 +808,7 @@ SELinuxSetSecurityImageLabel(virSecurityManagerPtr mgr, { virSecuritySELinuxCallbackData cbdata; - cbdata.secdef = &def->seclabel; + cbdata.secdef = def->seclabels[0]; cbdata.manager = mgr; bool allowDiskFormatProbing = virSecurityManagerGetAllowDiskFormatProbing(mgr); @@ -839,7 +839,7 @@ SELinuxSetSecurityPCILabel(pciDevice *dev ATTRIBUTE_UNUSED, const char *file, void *opaque) { virDomainDefPtr def = opaque; - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = def->seclabels[0]; return SELinuxSetFilecon(file, secdef->imagelabel); } @@ -849,7 +849,7 @@ SELinuxSetSecurityUSBLabel(usbDevice *dev ATTRIBUTE_UNUSED, const char *file, void *opaque) { virDomainDefPtr def = opaque; - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = def->seclabels[0]; return SELinuxSetFilecon(file, secdef->imagelabel); } @@ -860,7 +860,7 @@ SELinuxSetSecurityHostdevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainHostdevDefPtr dev) { - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = def->seclabels[0]; int ret = -1; if (secdef->norelabel) @@ -929,7 +929,7 @@ SELinuxRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainHostdevDefPtr dev) { - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = def->seclabels[0]; int ret = -1; if (secdef->norelabel) @@ -982,7 +982,7 @@ SELinuxSetSecurityChardevLabel(virDomainDefPtr def, virDomainChrSourceDefPtr dev) { - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = def->seclabels[0]; char *in = NULL, *out = NULL; int ret = -1; @@ -1028,7 +1028,7 @@ SELinuxRestoreSecurityChardevLabel(virDomainDefPtr def, virDomainChrSourceDefPtr dev) { - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = def->seclabels[0]; char *in = NULL, *out = NULL; int ret = -1; @@ -1121,7 +1121,7 @@ SELinuxRestoreSecurityAllLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def, int migrated ATTRIBUTE_UNUSED) { - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = def->seclabels[0]; int i; int rc = 0; @@ -1171,7 +1171,7 @@ static int SELinuxReleaseSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def) { - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = def->seclabels[0]; if (secdef->type == VIR_DOMAIN_SECLABEL_DYNAMIC) { if (secdef->label != NULL) { @@ -1196,7 +1196,7 @@ SELinuxSetSavedStateLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def, const char *savefile) { - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = def->seclabels[0]; if (secdef->norelabel) return 0; @@ -1210,7 +1210,7 @@ SELinuxRestoreSavedStateLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def, const char *savefile) { - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = def->seclabels[0]; if (secdef->norelabel) return 0; @@ -1223,7 +1223,7 @@ static int SELinuxSecurityVerify(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def) { - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = def->seclabels[0]; if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("security label driver mismatch: " @@ -1248,10 +1248,10 @@ SELinuxSetSecurityProcessLabel(virSecurityManagerPtr mgr, virDomainDefPtr def) { /* TODO: verify DOI */ - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = def->seclabels[0]; VIR_DEBUG("SELinuxSetSecurityProcessLabel %s", secdef->label); - if (def->seclabel.label == NULL) + if (def->seclabels[0]->label == NULL) return 0; if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { @@ -1280,13 +1280,13 @@ SELinuxSetSecurityDaemonSocketLabel(virSecurityManagerPtr mgr, virDomainDefPtr def) { /* TODO: verify DOI */ - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = def->seclabels[0]; context_t execcon = NULL; context_t proccon = NULL; security_context_t scon = NULL; int rc = -1; - if (def->seclabel.label == NULL) + if (def->seclabels[0]->label == NULL) return 0; if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { @@ -1350,7 +1350,7 @@ static int SELinuxSetSecuritySocketLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm) { - const virSecurityLabelDefPtr secdef = &vm->seclabel; + const virSecurityLabelDefPtr secdef = vm->seclabels[0]; int rc = -1; if (secdef->label == NULL) @@ -1388,9 +1388,9 @@ SELinuxClearSecuritySocketLabel(virSecurityManagerPtr mgr, virDomainDefPtr def) { /* TODO: verify DOI */ - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = def->seclabels[0]; - if (def->seclabel.label == NULL) + if (def->seclabels[0]->label == NULL) return 0; if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { @@ -1467,7 +1467,7 @@ SELinuxSetSecurityAllLabel(virSecurityManagerPtr mgr, const char *stdin_path) { virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr); - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = def->seclabels[0]; int i; if (secdef->norelabel) @@ -1528,7 +1528,7 @@ SELinuxSetImageFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def, int fd) { - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = def->seclabels[0]; if (secdef->imagelabel == NULL) return 0; @@ -1538,7 +1538,7 @@ SELinuxSetImageFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, static char *genImageLabel(virSecurityManagerPtr mgr, virDomainDefPtr def) { - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = def->seclabels[0]; virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr); const char *range; context_t ctx = NULL; @@ -1575,7 +1575,7 @@ cleanup: static char *SELinuxGetSecurityMountOptions(virSecurityManagerPtr mgr, virDomainDefPtr def) { char *opts = NULL; - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = def->seclabels[0]; if (! secdef->imagelabel) secdef->imagelabel = genImageLabel(mgr,def); diff --git a/src/test/test_driver.c b/src/test/test_driver.c index b3b774d..023fd3f 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -211,12 +211,15 @@ testBuildCapabilities(virConnectPtr conn) { caps->privateDataAllocFunc = testDomainObjPrivateAlloc; caps->privateDataFreeFunc = testDomainObjPrivateFree; - caps->host.secModel.model = strdup("testSecurity"); - if (!caps->host.secModel.model) + caps->host.nsecModels = 1; + if (VIR_ALLOC_N(caps->host.secModels, caps->host.nsecModels) < 0) + goto no_memory; + caps->host.secModels[0].model = strdup("testSecurity"); + if (!caps->host.secModels[0].model) goto no_memory; - caps->host.secModel.doi = strdup(""); - if (!caps->host.secModel.doi) + caps->host.secModels[0].doi = strdup(""); + if (!caps->host.secModels[0].doi) goto no_memory; return caps; -- 1.7.1

On 18.07.2012 03:28, Marcelo Cerri wrote:
This patch updates the structures that store information about each domain and each hypervisor to support multiple security labels and drivers. It also updates all the remaining code to use the new fields. --- src/conf/capabilities.c | 17 ++++-- src/conf/capabilities.h | 6 ++- src/conf/domain_audit.c | 14 +++-- src/conf/domain_conf.c | 86 +++++++++++++++++--------- src/conf/domain_conf.h | 9 ++- src/lxc/lxc_conf.c | 8 ++- src/lxc/lxc_controller.c | 8 +- src/lxc/lxc_driver.c | 35 ++++++----- src/qemu/qemu_driver.c | 15 +++-- src/qemu/qemu_process.c | 18 +++--- src/security/security_manager.c | 10 ++-- src/security/security_selinux.c | 132 +++++++++++++++++++------------------- src/test/test_driver.c | 11 ++- 13 files changed, 217 insertions(+), 152 deletions(-)
We must update XML schema as well as we are going to allow more <model> and <doi> elements under <secmodel>. And maybe we want to add a test case. But that can be a follow up patch.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 398f630..b468174 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -857,12 +857,15 @@ virDomainGraphicsListenDefClear(virDomainGraphicsListenDefPtr def) }
static void -virSecurityLabelDefClear(virSecurityLabelDefPtr def) +virSecurityLabelDefFree(virSecurityLabelDefPtr def) { - VIR_FREE(def->model); - VIR_FREE(def->label); - VIR_FREE(def->imagelabel); - VIR_FREE(def->baselabel); + if (def) { + VIR_FREE(def->model); + VIR_FREE(def->label); + VIR_FREE(def->imagelabel); + VIR_FREE(def->baselabel); + VIR_FREE(def); + } }
We tend to write this a slightly different: { if (!def) return; VIR_FREE(def->model); ... }
@@ -3669,10 +3679,15 @@ virDomainDiskDefParseXML(virCapsPtr caps, if (sourceNode) { xmlNodePtr saved_node = ctxt->node; ctxt->node = sourceNode; - if (virSecurityDeviceLabelDefParseXML(&def->seclabel, + if ((VIR_ALLOC(def->seclabels) < 0) || (VIR_ALLOC(def->seclabels[0]) < 0)) {
Long line.
+ virReportOOMError(); + goto error; + } + if (virSecurityDeviceLabelDefParseXML(&def->seclabels[0], vmSeclabel, ctxt) < 0) goto error; + def->nseclabels = 1; ctxt->node = saved_node; }
@@ -7115,10 +7130,16 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, goto error; }
+ if ((VIR_ALLOC(def->seclabels) < 0) || + (VIR_ALLOC(def->seclabels[0])) < 0 ) { + virReportOOMError(); + goto error; + } + if (xmlStrEqual(node->name, BAD_CAST "disk")) { dev->type = VIR_DOMAIN_DEVICE_DISK; if (!(dev->data.disk = virDomainDiskDefParseXML(caps, node, ctxt, - NULL, &def->seclabel, flags))) + NULL, def->seclabels[0], flags)))
Long line.
goto error; } else if (xmlStrEqual(node->name, BAD_CAST "lease")) { dev->type = VIR_DOMAIN_DEVICE_LEASE; @@ -8017,7 +8038,12 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
/* analysis of security label, done early even though we format it * late, so devices can refer to this for defaults */ - if (virSecurityLabelDefParseXML(&def->seclabel, ctxt, flags) == -1) + if ((VIR_ALLOC(def->seclabels) < 0) || (VIR_ALLOC(def->seclabels[0]) < 0)) { + virReportOOMError(); + goto error; + }
Again, I'd split this into two lines.
+ def->nseclabels = 1; + if (virSecurityLabelDefParseXML(def->seclabels[0], ctxt, flags) == -1) goto error;
/* Extract domain memory */
I think needs to be squashed in: diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng index 06ff685..be9d295 100644 --- a/docs/schemas/capability.rng +++ b/docs/schemas/capability.rng @@ -52,12 +52,14 @@ <define name='secmodel'> <element name='secmodel'> - <element name='model'> - <text/> - </element> - <element name='doi'> - <text/> - </element> + <oneOrMore> + <element name='model'> + <text/> + </element> + <element name='doi'> + <text/> + </element> + </oneOrMore> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b468174..719efea 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -859,13 +859,14 @@ virDomainGraphicsListenDefClear(virDomainGraphicsListenDefPtr def) static void virSecurityLabelDefFree(virSecurityLabelDefPtr def) { - if (def) { - VIR_FREE(def->model); - VIR_FREE(def->label); - VIR_FREE(def->imagelabel); - VIR_FREE(def->baselabel); - VIR_FREE(def); - } + if (!def) + return; + + VIR_FREE(def->model); + VIR_FREE(def->label); + VIR_FREE(def->imagelabel); + VIR_FREE(def->baselabel); + VIR_FREE(def); } @@ -3679,7 +3680,8 @@ virDomainDiskDefParseXML(virCapsPtr caps, if (sourceNode) { xmlNodePtr saved_node = ctxt->node; ctxt->node = sourceNode; - if ((VIR_ALLOC(def->seclabels) < 0) || (VIR_ALLOC(def->seclabels[0]) < 0)) { + if ((VIR_ALLOC(def->seclabels) < 0) || + (VIR_ALLOC(def->seclabels[0]) < 0)) { virReportOOMError(); goto error; } @@ -7138,8 +7140,9 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, if (xmlStrEqual(node->name, BAD_CAST "disk")) { dev->type = VIR_DOMAIN_DEVICE_DISK; - if (!(dev->data.disk = virDomainDiskDefParseXML(caps, node, ctxt, - NULL, def->seclabels[0], flags))) + if (!(dev->data.disk = virDomainDiskDefParseXML(caps, node, ctxt, NULL, + def->seclabels[0], + flags))) goto error; } else if (xmlStrEqual(node->name, BAD_CAST "lease")) { dev->type = VIR_DOMAIN_DEVICE_LEASE; @@ -8038,7 +8041,8 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of security label, done early even though we format it * late, so devices can refer to this for defaults */ - if ((VIR_ALLOC(def->seclabels) < 0) || (VIR_ALLOC(def->seclabels[0]) < 0)) { + if ((VIR_ALLOC(def->seclabels) < 0) || + (VIR_ALLOC(def->seclabels[0]) < 0)) { virReportOOMError(); goto error; } But don't we rather want multiple <secmodel> elements than multiple <doi> and <model> inside one <secmodel>?

On 07/18/2012 10:31 AM, Michal Privoznik wrote:
On 18.07.2012 03:28, Marcelo Cerri wrote:
This patch updates the structures that store information about each domain and each hypervisor to support multiple security labels and drivers. It also updates all the remaining code to use the new fields. ---
We must update XML schema as well as we are going to allow more <model> and <doi> elements under <secmodel>. And maybe we want to add a test case. But that can be a follow up patch.
Absolutely add a testcase if you are enhancing the xml parser to accept new tags.
I think needs to be squashed in:
diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng index 06ff685..be9d295 100644 --- a/docs/schemas/capability.rng +++ b/docs/schemas/capability.rng
If you're going to touch docs/schemas, then it would also be nice to touch the counterpart docs/*.html.in. Unfortunately, I think our capability.rng is currently underdocumented in the html.
@@ -52,12 +52,14 @@
<define name='secmodel'> <element name='secmodel'> - <element name='model'> - <text/> - </element> - <element name='doi'> - <text/> - </element> + <oneOrMore> + <element name='model'> + <text/> + </element> + <element name='doi'> + <text/> + </element> + </oneOrMore> </element>
Hmm, this says that: <secmodel> <model>...</model> <doi>...</doi> <model>...</model> <doi>...</doi> </secmodel> will validate, but: <secmodel> <doi>...</doi> <model>...</model> <model>...</model> <doi>...</doi> </secmodel> will not. I think that's somewhat good (since the parameters are positional, and we insist that they come in pairs, then the pairs must be properly interleaved), but did the C code enforce that?
But don't we rather want multiple <secmodel> elements than multiple <doi> and <model> inside one <secmodel>?
Indeed - that makes the XML layout problem MUCH easier. If we add an <interleave> around the 'model' and 'doi' sub-elements, then you could write: <secmodel> <doi>...</doi> <model>...</model> </secmodel> <secmodel> <model>...</model> <doi>...</doi> </secmodel> and have an unambiguous 2-model design, without quite so much attention being placed on the exactly-one 'doi' and 'model' subelement per 'secmodel'. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Wed, Jul 18, 2012 at 11:10:10AM -0600, Eric Blake wrote:
On 07/18/2012 10:31 AM, Michal Privoznik wrote:
On 18.07.2012 03:28, Marcelo Cerri wrote:
This patch updates the structures that store information about each domain and each hypervisor to support multiple security labels and drivers. It also updates all the remaining code to use the new fields. ---
We must update XML schema as well as we are going to allow more <model> and <doi> elements under <secmodel>. And maybe we want to add a test case. But that can be a follow up patch.
Absolutely add a testcase if you are enhancing the xml parser to accept new tags.
I think needs to be squashed in:
diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng index 06ff685..be9d295 100644 --- a/docs/schemas/capability.rng +++ b/docs/schemas/capability.rng
If you're going to touch docs/schemas, then it would also be nice to touch the counterpart docs/*.html.in. Unfortunately, I think our capability.rng is currently underdocumented in the html.
@@ -52,12 +52,14 @@
<define name='secmodel'> <element name='secmodel'> - <element name='model'> - <text/> - </element> - <element name='doi'> - <text/> - </element> + <oneOrMore> + <element name='model'> + <text/> + </element> + <element name='doi'> + <text/> + </element> + </oneOrMore> </element>
Hmm, this says that:
<secmodel> <model>...</model> <doi>...</doi> <model>...</model> <doi>...</doi> </secmodel>
will validate, but:
<secmodel> <doi>...</doi> <model>...</model> <model>...</model> <doi>...</doi> </secmodel>
will not. I think that's somewhat good (since the parameters are positional, and we insist that they come in pairs, then the pairs must be properly interleaved), but did the C code enforce that?
But don't we rather want multiple <secmodel> elements than multiple <doi> and <model> inside one <secmodel>?
Indeed - that makes the XML layout problem MUCH easier. If we add an <interleave> around the 'model' and 'doi' sub-elements, then you could write:
<secmodel> <doi>...</doi> <model>...</model> </secmodel> <secmodel> <model>...</model> <doi>...</doi> </secmodel>
and have an unambiguous 2-model design, without quite so much attention being placed on the exactly-one 'doi' and 'model' subelement per 'secmodel'.
Agreed, multiple <secmodel> elements is preferrable. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

This patch updates the domain XML parser and formatter to support more than one "seclabel" element for each domain and device. The RNG schema and the tests related to this are also updated by this patch. --- docs/schemas/domaincommon.rng | 30 ++- src/conf/domain_conf.c | 339 ++++++++++++++------ src/conf/domain_conf.h | 9 + .../qemuxml2argv-seclabel-dynamic-baselabel.xml | 2 +- .../qemuxml2argv-seclabel-dynamic-override.xml | 6 +- .../qemuxml2argv-seclabel-dynamic.xml | 2 +- .../qemuxml2argv-seclabel-static.xml | 2 +- 7 files changed, 270 insertions(+), 120 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index b7562ad..72ea54e 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -55,9 +55,9 @@ <optional> <ref name="devices"/> </optional> - <optional> + <zeroOrMore> <ref name="seclabel"/> - </optional> + </zeroOrMore> <optional> <ref name='qemucmdline'/> </optional> @@ -148,18 +148,32 @@ <!-- A per-device seclabel override is more limited, either relabel=no or a <label> must be present. --> <choice> - <attribute name='relabel'> - <value>no</value> - </attribute> <group> <optional> + <attribute name='model'> + <text/> + </attribute> + </optional> + <attribute name='relabel'> + <value>no</value> + </attribute> + </group> + <group> + <optional> + <attribute name='model'> + <text/> + </attribute> + </optional> + <optional> <attribute name='relabel'> <value>yes</value> </attribute> </optional> - <element name='label'> - <text/> - </element> + <zeroOrMore> + <element name='label'> + <text/> + </element> + </zeroOrMore> </group> </choice> </element> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b468174..a63f36f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3080,17 +3080,19 @@ virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def) return 0; } -static int -virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, - xmlXPathContextPtr ctxt, +static virSecurityLabelDefPtr +virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt, unsigned int flags) { char *p; + virSecurityLabelDefPtr def = NULL; - if (virXPathNode("./seclabel[1]", ctxt) == NULL) - return 0; + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + goto error; + } - p = virXPathStringLimit("string(./seclabel[1]/@type)", + p = virXPathStringLimit("string(./@type)", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p == NULL) { def->type = VIR_DOMAIN_SECLABEL_DYNAMIC; @@ -3104,7 +3106,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, } } - p = virXPathStringLimit("string(./seclabel[1]/@relabel)", + p = virXPathStringLimit("string(./@relabel)", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p != NULL) { if (STREQ(p, "yes")) { @@ -3121,13 +3123,15 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC && def->norelabel) { virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("dynamic label type must use resource relabeling")); + "%s", _("dynamic label type must use " + "resource relabeling")); goto error; } if (def->type == VIR_DOMAIN_SECLABEL_NONE && !def->norelabel) { virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("resource relabeling is not compatible with 'none' label type")); + "%s", _("resource relabeling is not " + "compatible with 'none' label type")); goto error; } } else { @@ -3144,7 +3148,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, if (def->type == VIR_DOMAIN_SECLABEL_STATIC || (!(flags & VIR_DOMAIN_XML_INACTIVE) && def->type != VIR_DOMAIN_SECLABEL_NONE)) { - p = virXPathStringLimit("string(./seclabel[1]/label[1])", + p = virXPathStringLimit("string(./label[1])", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p == NULL) { virDomainReportError(VIR_ERR_XML_ERROR, @@ -3159,7 +3163,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, if (!def->norelabel && (!(flags & VIR_DOMAIN_XML_INACTIVE) && def->type != VIR_DOMAIN_SECLABEL_NONE)) { - p = virXPathStringLimit("string(./seclabel[1]/imagelabel[1])", + p = virXPathStringLimit("string(./imagelabel[1])", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p == NULL) { virDomainReportError(VIR_ERR_XML_ERROR, @@ -3171,93 +3175,162 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, /* Only parse baselabel for dynamic label type */ if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC) { - p = virXPathStringLimit("string(./seclabel[1]/baselabel[1])", + p = virXPathStringLimit("string(./baselabel[1])", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); def->baselabel = p; } - /* Only parse model, if static labelling, or a base - * label is set, or doing active XML - */ - if (def->type == VIR_DOMAIN_SECLABEL_STATIC || - def->baselabel || - (!(flags & VIR_DOMAIN_XML_INACTIVE) && - def->type != VIR_DOMAIN_SECLABEL_NONE)) { - p = virXPathStringLimit("string(./seclabel[1]/@model)", - VIR_SECURITY_MODEL_BUFLEN-1, ctxt); - if (p == NULL) { - virDomainReportError(VIR_ERR_XML_ERROR, - "%s", _("missing security model")); - goto error; - } - def->model = p; + /* Always parse model */ + p = virXPathStringLimit("string(./@model)", + VIR_SECURITY_MODEL_BUFLEN-1, ctxt); + if (p == NULL && def->type != VIR_DOMAIN_SECLABEL_NONE) { + virDomainReportError(VIR_ERR_XML_ERROR, + "%s", _("missing security model")); + goto error; } + def->model = p; - return 0; + return def; error: virSecurityLabelDefFree(def); - return -1; + return NULL; } - static int -virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr *def, - virSecurityLabelDefPtr vmDef, - xmlXPathContextPtr ctxt) +virSecurityLabelDefsParseXML(virDomainDefPtr def, + xmlXPathContextPtr ctxt, + unsigned int flags) { - char *p; + int i, n; + xmlNodePtr *list, saved_node; - *def = NULL; + /* Check args and save context */ + if (def == NULL || ctxt == NULL) + return 0; + saved_node = ctxt->node; - if (virXPathNode("./seclabel[1]", ctxt) == NULL) + /* Allocate a security labels based on XML */ + if ((n = virXPathNodeSet("./seclabel", ctxt, &list)) == 0) return 0; - /* Can't use overrides if top-level doesn't allow relabeling. */ - if (vmDef && vmDef->norelabel) { - virDomainReportError(VIR_ERR_XML_ERROR, "%s", - _("label overrides require relabeling to be " - "enabled at the domain level")); + if (VIR_ALLOC_N(def->seclabels, n) < 0) { + virReportOOMError(); return -1; } - if (VIR_ALLOC(*def) < 0) { + /* Parse each "seclabel" tag */ + for (i = 0; i < n; i++) { + ctxt->node = list[i]; + def->seclabels[i] = virSecurityLabelDefParseXML(ctxt, flags); + if (def->seclabels[i] == NULL) + goto error; + } + def->nseclabels = n; + ctxt->node = saved_node; + return 0; + +error: + ctxt->node = saved_node; + for (i = 0; i < n; i++) { + virSecurityLabelDefFree(def->seclabels[i]); + } + VIR_FREE(def->seclabels); + return -1; +} + +static int +virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def, + virSecurityLabelDefPtr *vmSeclabels, + int nvmSeclabels, xmlXPathContextPtr ctxt) +{ + int n, i, j; + xmlNodePtr *list; + virSecurityLabelDefPtr vmDef = NULL; + char *model, *relabel, *label; + + if (def == NULL) + return 0; + + if ((n = virXPathNodeSet("./seclabel", ctxt, &list)) == 0) + return 0; + + def->nseclabels = n; + if (VIR_ALLOC_N(def->seclabels, n) < 0) { virReportOOMError(); return -1; } + for (i = 0; i < n; i++) { + if (VIR_ALLOC(def->seclabels[i]) < 0) { + virReportOOMError(); + goto error; + } + } - p = virXPathStringLimit("string(./seclabel[1]/@relabel)", - VIR_SECURITY_LABEL_BUFLEN-1, ctxt); - if (p != NULL) { - if (STREQ(p, "yes")) { - (*def)->norelabel = false; - } else if (STREQ(p, "no")) { - (*def)->norelabel = true; + for (i = 0; i < n; i++) { + /* get model associated to this override */ + model = virXMLPropString(list[i], "model"); + if (model == NULL) { + virDomainReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid security model")); + goto error; } else { - virDomainReportError(VIR_ERR_XML_ERROR, - _("invalid security relabel value %s"), p); - VIR_FREE(p); - VIR_FREE(*def); - return -1; + /* find the security label that it's being overrided */ + for (j = 0; j < nvmSeclabels; j++) { + if (STREQ(vmSeclabels[j]->model, model)) { + vmDef = vmSeclabels[j]; + break; + } + } + def->seclabels[i]->model = model; } - VIR_FREE(p); - } else { - (*def)->norelabel = false; - } - p = virXPathStringLimit("string(./seclabel[1]/label[1])", - VIR_SECURITY_LABEL_BUFLEN-1, ctxt); - (*def)->label = p; + /* Can't use overrides if top-level doesn't allow relabeling. */ + if (vmDef && vmDef->norelabel) { + virDomainReportError(VIR_ERR_XML_ERROR, "%s", + _("label overrides require relabeling to be " + "enabled at the domain level")); + goto error; + } - if ((*def)->label && (*def)->norelabel) { - virDomainReportError(VIR_ERR_XML_ERROR, - _("Cannot specify a label if relabelling is turned off")); - VIR_FREE((*def)->label); - VIR_FREE(*def); - return -1; - } + relabel = virXMLPropString(list[i], "relabel"); + if (relabel != NULL) { + if (STREQ(relabel, "yes")) { + def->seclabels[i]->norelabel = false; + } else if (STREQ(relabel, "no")) { + def->seclabels[i]->norelabel = true; + } else { + virDomainReportError(VIR_ERR_XML_ERROR, + _("invalid security relabel value %s"), + relabel); + VIR_FREE(relabel); + goto error; + } + VIR_FREE(relabel); + } else { + def->seclabels[i]->norelabel = false; + } + ctxt->node = list[i]; + label = virXPathStringLimit("string(./label)", + VIR_SECURITY_LABEL_BUFLEN-1, ctxt); + def->seclabels[i]->label = label; + + if (label && def->seclabels[i]->norelabel) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("Cannot specify a label if relabelling is " + "turned off")); + goto error; + } + } return 0; + +error: + for (i = 0; i < n; i++) { + virSecurityDeviceLabelDefFree(def->seclabels[i]); + } + VIR_FREE(def->seclabels); + return -1; } @@ -3341,7 +3414,8 @@ virDomainDiskDefParseXML(virCapsPtr caps, xmlNodePtr node, xmlXPathContextPtr ctxt, virBitmapPtr bootMap, - virSecurityLabelDefPtr vmSeclabel, + virSecurityLabelDefPtr* vmSeclabels, + int nvmSeclabels, unsigned int flags) { virDomainDiskDefPtr def; @@ -3679,15 +3753,9 @@ virDomainDiskDefParseXML(virCapsPtr caps, if (sourceNode) { xmlNodePtr saved_node = ctxt->node; ctxt->node = sourceNode; - if ((VIR_ALLOC(def->seclabels) < 0) || (VIR_ALLOC(def->seclabels[0]) < 0)) { - virReportOOMError(); - goto error; - } - if (virSecurityDeviceLabelDefParseXML(&def->seclabels[0], - vmSeclabel, - ctxt) < 0) + if (virSecurityDeviceLabelDefParseXML(def, vmSeclabels, + nvmSeclabels, ctxt) < 0) goto error; - def->nseclabels = 1; ctxt->node = saved_node; } @@ -7130,16 +7198,12 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, goto error; } - if ((VIR_ALLOC(def->seclabels) < 0) || - (VIR_ALLOC(def->seclabels[0])) < 0 ) { - virReportOOMError(); - goto error; - } - if (xmlStrEqual(node->name, BAD_CAST "disk")) { dev->type = VIR_DOMAIN_DEVICE_DISK; if (!(dev->data.disk = virDomainDiskDefParseXML(caps, node, ctxt, - NULL, def->seclabels[0], flags))) + NULL, def->seclabels, + def->nseclabels, + flags))) goto error; } else if (xmlStrEqual(node->name, BAD_CAST "lease")) { dev->type = VIR_DOMAIN_DEVICE_LEASE; @@ -8038,12 +8102,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of security label, done early even though we format it * late, so devices can refer to this for defaults */ - if ((VIR_ALLOC(def->seclabels) < 0) || (VIR_ALLOC(def->seclabels[0]) < 0)) { - virReportOOMError(); - goto error; - } - def->nseclabels = 1; - if (virSecurityLabelDefParseXML(def->seclabels[0], ctxt, flags) == -1) + if (virSecurityLabelDefsParseXML(def, ctxt, flags) == -1) goto error; /* Extract domain memory */ @@ -8642,7 +8701,8 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, nodes[i], ctxt, bootMap, - def->seclabels[0], + def->seclabels, + def->nseclabels, flags); if (!disk) goto error; @@ -10935,16 +10995,19 @@ virSecurityLabelDefFormat(virBufferPtr buf, virSecurityLabelDefPtr def) if (def->type == VIR_DOMAIN_SECLABEL_DEFAULT) return; - virBufferAsprintf(buf, "<seclabel type='%s'", - sectype); + virBufferAsprintf(buf, "<seclabel"); + + if (def->model) { + virBufferEscapeString(buf, " model='%s'", def->model); + } + + virBufferAsprintf(buf," type='%s'", sectype); if (def->type == VIR_DOMAIN_SECLABEL_NONE) { virBufferAddLit(buf, "/>\n"); return; } - virBufferEscapeString(buf, " model='%s'", def->model); - virBufferAsprintf(buf, " relabel='%s'", def->norelabel ? "no" : "yes"); @@ -10970,8 +11033,8 @@ static void virSecurityDeviceLabelDefFormat(virBufferPtr buf, virSecurityDeviceLabelDefPtr def) { - virBufferAsprintf(buf, "<seclabel relabel='%s'", - def->norelabel ? "no" : "yes"); + virBufferAsprintf(buf, "<seclabel model='%s' relabel='%s'", + def->model, def->norelabel ? "no" : "yes"); if (def->label) { virBufferAddLit(buf, ">\n"); virBufferEscapeString(buf, " <label>%s</label>\n", @@ -11016,6 +11079,7 @@ virDomainDiskDefFormat(virBufferPtr buf, const char *copy_on_read = virDomainVirtioEventIdxTypeToString(def->copy_on_read); const char *startupPolicy = virDomainStartupPolicyTypeToString(def->startupPolicy); + int n; char uuidstr[VIR_UUID_STRING_BUFLEN]; if (!type) { @@ -11111,10 +11175,11 @@ virDomainDiskDefFormat(virBufferPtr buf, if (def->startupPolicy) virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); - if (def->seclabels && def->seclabels[0]) { + if (def->nseclabels) { virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 8); - virSecurityDeviceLabelDefFormat(buf, def->seclabels[0]); + for (n = 0; n < def->nseclabels; n++) + virSecurityDeviceLabelDefFormat(buf, def->seclabels[n]); virBufferAdjustIndent(buf, -8); virBufferAddLit(buf, " </source>\n"); } else { @@ -11124,10 +11189,11 @@ virDomainDiskDefFormat(virBufferPtr buf, case VIR_DOMAIN_DISK_TYPE_BLOCK: virBufferEscapeString(buf, " <source dev='%s'", def->src); - if (def->seclabels && def->seclabels[0]) { + if (def->nseclabels) { virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 8); - virSecurityDeviceLabelDefFormat(buf, def->seclabels[0]); + for (n = 0; n < def->nseclabels; n++) + virSecurityDeviceLabelDefFormat(buf, def->seclabels[n]); virBufferAdjustIndent(buf, -8); virBufferAddLit(buf, " </source>\n"); } else { @@ -13153,11 +13219,10 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAddLit(buf, " </devices>\n"); - if (def->nseclabels && def->seclabels) { - virBufferAdjustIndent(buf, 2); - virSecurityLabelDefFormat(buf, def->seclabels[0]); - virBufferAdjustIndent(buf, -2); - } + virBufferAdjustIndent(buf, 2); + for (n = 0; n < def->nseclabels; n++) + virSecurityLabelDefFormat(buf, def->seclabels[n]); + virBufferAdjustIndent(buf, -2); if (def->namespaceData && def->ns.format) { if ((def->ns.format)(buf, def->namespaceData) < 0) @@ -15282,3 +15347,65 @@ cleanup: VIR_FREE(xmlStr); return ret; } + +virSecurityLabelDefPtr +virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char *model) +{ + int i; + + if (def == NULL || model == NULL) + return NULL; + + for (i = 0; i < def->nseclabels; i++) { + if (def->seclabels[i]->model == NULL) + continue; + if (STREQ(def->seclabels[i]->model, model)) + return def->seclabels[i]; + } + + return virDomainDefAddSecurityLabelDef(def, model); +} + +virSecurityDeviceLabelDefPtr +virDomainDiskDefGetSecurityLabelDef(virDomainDiskDefPtr def, const char *model) +{ + int i; + + if (def == NULL) + return NULL; + + for (i = 0; i < def->nseclabels; i++) { + if (STREQ(def->seclabels[i]->model, model)) + return def->seclabels[i]; + } + return NULL; +} + +virSecurityLabelDefPtr +virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char *model) +{ + virSecurityLabelDefPtr seclabel = NULL; + + if (VIR_ALLOC(seclabel) < 0) { + virReportOOMError(); + return NULL; + } + + if (model) { + seclabel->model = strdup(model); + if (seclabel->model == NULL) { + virReportOOMError(); + virSecurityLabelDefFree(seclabel); + return NULL; + } + } + + if (VIR_EXPAND_N(def->seclabels, def->nseclabels, 1) < 0) { + virReportOOMError(); + virSecurityLabelDefFree(seclabel); + return NULL; + } + def->seclabels[def->nseclabels - 1] = seclabel; + + return seclabel; +} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 9a2189a..faa760a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2155,6 +2155,15 @@ virDomainState virDomainObjGetState(virDomainObjPtr obj, int *reason) ATTRIBUTE_NONNULL(1); +virSecurityLabelDefPtr +virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char *model); + +virSecurityDeviceLabelDefPtr +virDomainDiskDefGetSecurityLabelDef(virDomainDiskDefPtr def, const char *model); + +virSecurityLabelDefPtr +virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char *model); + typedef const char* (*virLifecycleToStringFunc)(int type); typedef int (*virLifecycleFromStringFunc)(const char *type); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-baselabel.xml b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-baselabel.xml index 98362a7..171dd47 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-baselabel.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-baselabel.xml @@ -23,7 +23,7 @@ <controller type='ide' index='0'/> <memballoon model='virtio'/> </devices> - <seclabel type='dynamic' model='selinux' relabel='yes'> + <seclabel model='selinux' type='dynamic' relabel='yes'> <baselabel>system_u:system_r:svirt_custom_t:s0</baselabel> </seclabel> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.xml b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.xml index 4de435b..769caeb 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.xml @@ -16,14 +16,14 @@ <emulator>/usr/bin/qemu</emulator> <disk type='block' device='disk'> <source dev='/dev/HostVG/QEMUGuest1'> - <seclabel relabel='no'/> + <seclabel model='selinux' relabel='no'/> </source> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='block' device='disk'> <source dev='/dev/HostVG/QEMUGuest2'> - <seclabel relabel='yes'> + <seclabel model='selinux' relabel='yes'> <label>system_u:system_r:public_content_t:s0</label> </seclabel> </source> @@ -35,7 +35,7 @@ <controller type='ide' index='0'/> <memballoon model='virtio'/> </devices> - <seclabel type='dynamic' model='selinux' relabel='yes'> + <seclabel model='selinux' type='dynamic' relabel='yes'> <baselabel>system_u:system_r:svirt_custom_t:s0</baselabel> </seclabel> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic.xml b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic.xml index 78a6b6a..36df9d4 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic.xml @@ -22,5 +22,5 @@ <controller type='ide' index='0'/> <memballoon model='virtio'/> </devices> - <seclabel type='dynamic' relabel='yes'/> + <seclabel model='selinux' type='dynamic' relabel='yes'/> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-static.xml b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-static.xml index 31d5f58..23ddef1 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-static.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-static.xml @@ -23,7 +23,7 @@ <controller type='ide' index='0'/> <memballoon model='virtio'/> </devices> - <seclabel type='static' model='selinux' relabel='no'> + <seclabel model='selinux' type='static' relabel='no'> <label>system_u:system_r:svirt_custom_t:s0:c192,c392</label> </seclabel> </domain> -- 1.7.1

On 18.07.2012 03:28, Marcelo Cerri wrote:
This patch updates the domain XML parser and formatter to support more than one "seclabel" element for each domain and device. The RNG schema and the tests related to this are also updated by this patch. --- docs/schemas/domaincommon.rng | 30 ++- src/conf/domain_conf.c | 339 ++++++++++++++------ src/conf/domain_conf.h | 9 + .../qemuxml2argv-seclabel-dynamic-baselabel.xml | 2 +- .../qemuxml2argv-seclabel-dynamic-override.xml | 6 +- .../qemuxml2argv-seclabel-dynamic.xml | 2 +- .../qemuxml2argv-seclabel-static.xml | 2 +- 7 files changed, 270 insertions(+), 120 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index b7562ad..72ea54e 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -55,9 +55,9 @@ <optional> <ref name="devices"/> </optional> - <optional> + <zeroOrMore> <ref name="seclabel"/> - </optional> + </zeroOrMore> <optional> <ref name='qemucmdline'/> </optional> @@ -148,18 +148,32 @@ <!-- A per-device seclabel override is more limited, either relabel=no or a <label> must be present. --> <choice> - <attribute name='relabel'> - <value>no</value> - </attribute> <group> <optional> + <attribute name='model'> + <text/> + </attribute> + </optional> + <attribute name='relabel'> + <value>no</value> + </attribute> + </group> + <group> + <optional> + <attribute name='model'> + <text/> + </attribute> + </optional> + <optional> <attribute name='relabel'> <value>yes</value> </attribute> </optional> - <element name='label'> - <text/> - </element> + <zeroOrMore> + <element name='label'> + <text/> + </element> + </zeroOrMore> </group> </choice> </element> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b468174..a63f36f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3080,17 +3080,19 @@ virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def) return 0; }
-static int -virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, - xmlXPathContextPtr ctxt, +static virSecurityLabelDefPtr +virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt, unsigned int flags) { char *p; + virSecurityLabelDefPtr def = NULL;
- if (virXPathNode("./seclabel[1]", ctxt) == NULL) - return 0; + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + goto error; + }
- p = virXPathStringLimit("string(./seclabel[1]/@type)", + p = virXPathStringLimit("string(./@type)", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p == NULL) { def->type = VIR_DOMAIN_SECLABEL_DYNAMIC; @@ -3104,7 +3106,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, } }
- p = virXPathStringLimit("string(./seclabel[1]/@relabel)", + p = virXPathStringLimit("string(./@relabel)", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p != NULL) { if (STREQ(p, "yes")) { @@ -3121,13 +3123,15 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC && def->norelabel) { virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("dynamic label type must use resource relabeling")); + "%s", _("dynamic label type must use " + "resource relabeling")); goto error; } if (def->type == VIR_DOMAIN_SECLABEL_NONE && !def->norelabel) { virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("resource relabeling is not compatible with 'none' label type")); + "%s", _("resource relabeling is not " + "compatible with 'none' label type")); goto error; } } else { @@ -3144,7 +3148,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, if (def->type == VIR_DOMAIN_SECLABEL_STATIC || (!(flags & VIR_DOMAIN_XML_INACTIVE) && def->type != VIR_DOMAIN_SECLABEL_NONE)) { - p = virXPathStringLimit("string(./seclabel[1]/label[1])", + p = virXPathStringLimit("string(./label[1])", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p == NULL) { virDomainReportError(VIR_ERR_XML_ERROR, @@ -3159,7 +3163,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, if (!def->norelabel && (!(flags & VIR_DOMAIN_XML_INACTIVE) && def->type != VIR_DOMAIN_SECLABEL_NONE)) { - p = virXPathStringLimit("string(./seclabel[1]/imagelabel[1])", + p = virXPathStringLimit("string(./imagelabel[1])", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p == NULL) { virDomainReportError(VIR_ERR_XML_ERROR, @@ -3171,93 +3175,162 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
/* Only parse baselabel for dynamic label type */ if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC) { - p = virXPathStringLimit("string(./seclabel[1]/baselabel[1])", + p = virXPathStringLimit("string(./baselabel[1])", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); def->baselabel = p; }
- /* Only parse model, if static labelling, or a base - * label is set, or doing active XML - */ - if (def->type == VIR_DOMAIN_SECLABEL_STATIC || - def->baselabel || - (!(flags & VIR_DOMAIN_XML_INACTIVE) && - def->type != VIR_DOMAIN_SECLABEL_NONE)) { - p = virXPathStringLimit("string(./seclabel[1]/@model)", - VIR_SECURITY_MODEL_BUFLEN-1, ctxt); - if (p == NULL) { - virDomainReportError(VIR_ERR_XML_ERROR, - "%s", _("missing security model")); - goto error; - } - def->model = p; + /* Always parse model */ + p = virXPathStringLimit("string(./@model)", + VIR_SECURITY_MODEL_BUFLEN-1, ctxt); + if (p == NULL && def->type != VIR_DOMAIN_SECLABEL_NONE) { + virDomainReportError(VIR_ERR_XML_ERROR, + "%s", _("missing security model")); + goto error; } + def->model = p;
- return 0; + return def;
error: virSecurityLabelDefFree(def); - return -1; + return NULL; }
- static int -virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr *def, - virSecurityLabelDefPtr vmDef, - xmlXPathContextPtr ctxt) +virSecurityLabelDefsParseXML(virDomainDefPtr def, + xmlXPathContextPtr ctxt, + unsigned int flags) { - char *p; + int i, n; + xmlNodePtr *list, saved_node;
- *def = NULL; + /* Check args and save context */ + if (def == NULL || ctxt == NULL) + return 0; + saved_node = ctxt->node;
- if (virXPathNode("./seclabel[1]", ctxt) == NULL) + /* Allocate a security labels based on XML */ + if ((n = virXPathNodeSet("./seclabel", ctxt, &list)) == 0) return 0;
- /* Can't use overrides if top-level doesn't allow relabeling. */ - if (vmDef && vmDef->norelabel) { - virDomainReportError(VIR_ERR_XML_ERROR, "%s", - _("label overrides require relabeling to be " - "enabled at the domain level")); + if (VIR_ALLOC_N(def->seclabels, n) < 0) { + virReportOOMError(); return -1; }
'list' must be VIR_FREE()d after use. And here^^ is just leaked.
- if (VIR_ALLOC(*def) < 0) { + /* Parse each "seclabel" tag */ + for (i = 0; i < n; i++) { + ctxt->node = list[i]; + def->seclabels[i] = virSecurityLabelDefParseXML(ctxt, flags); + if (def->seclabels[i] == NULL) + goto error; + } + def->nseclabels = n; + ctxt->node = saved_node;
VIR_FREE(list);
+ return 0; + +error: + ctxt->node = saved_node; + for (i = 0; i < n; i++) {
We can just take reverse steps. By the time we get here, 'i' represents the real position in 'def->seclabels' so we don't need to go through whole 'n'; But that's really a premature optimization.
+ virSecurityLabelDefFree(def->seclabels[i]); + } + VIR_FREE(def->seclabels);
VIR_FREE(list);
+ return -1; +} + +static int +virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def, + virSecurityLabelDefPtr *vmSeclabels, + int nvmSeclabels, xmlXPathContextPtr ctxt) +{ + int n, i, j; + xmlNodePtr *list; + virSecurityLabelDefPtr vmDef = NULL; + char *model, *relabel, *label; + + if (def == NULL) + return 0; + + if ((n = virXPathNodeSet("./seclabel", ctxt, &list)) == 0)
Again, 'list' must be VIR_FREE()d at the end.
+ return 0; + + def->nseclabels = n; + if (VIR_ALLOC_N(def->seclabels, n) < 0) { virReportOOMError(); return -1; } + for (i = 0; i < n; i++) { + if (VIR_ALLOC(def->seclabels[i]) < 0) { + virReportOOMError(); + goto error; + } + }
- p = virXPathStringLimit("string(./seclabel[1]/@relabel)", - VIR_SECURITY_LABEL_BUFLEN-1, ctxt); - if (p != NULL) { - if (STREQ(p, "yes")) { - (*def)->norelabel = false; - } else if (STREQ(p, "no")) { - (*def)->norelabel = true; + for (i = 0; i < n; i++) { + /* get model associated to this override */ + model = virXMLPropString(list[i], "model"); + if (model == NULL) { + virDomainReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid security model")); + goto error; } else { - virDomainReportError(VIR_ERR_XML_ERROR, - _("invalid security relabel value %s"), p); - VIR_FREE(p); - VIR_FREE(*def); - return -1; + /* find the security label that it's being overrided */
s/overrided/overridden/
+ for (j = 0; j < nvmSeclabels; j++) { + if (STREQ(vmSeclabels[j]->model, model)) { + vmDef = vmSeclabels[j]; + break; + } + } + def->seclabels[i]->model = model; } - VIR_FREE(p); - } else { - (*def)->norelabel = false; - }
- p = virXPathStringLimit("string(./seclabel[1]/label[1])", - VIR_SECURITY_LABEL_BUFLEN-1, ctxt); - (*def)->label = p; + /* Can't use overrides if top-level doesn't allow relabeling. */ + if (vmDef && vmDef->norelabel) { + virDomainReportError(VIR_ERR_XML_ERROR, "%s", + _("label overrides require relabeling to be " + "enabled at the domain level")); + goto error; + }
- if ((*def)->label && (*def)->norelabel) { - virDomainReportError(VIR_ERR_XML_ERROR, - _("Cannot specify a label if relabelling is turned off")); - VIR_FREE((*def)->label); - VIR_FREE(*def); - return -1; - } + relabel = virXMLPropString(list[i], "relabel"); + if (relabel != NULL) { + if (STREQ(relabel, "yes")) { + def->seclabels[i]->norelabel = false; + } else if (STREQ(relabel, "no")) { + def->seclabels[i]->norelabel = true; + } else { + virDomainReportError(VIR_ERR_XML_ERROR, + _("invalid security relabel value %s"), + relabel); + VIR_FREE(relabel); + goto error; + } + VIR_FREE(relabel); + } else { + def->seclabels[i]->norelabel = false; + }
+ ctxt->node = list[i]; + label = virXPathStringLimit("string(./label)", + VIR_SECURITY_LABEL_BUFLEN-1, ctxt); + def->seclabels[i]->label = label; + + if (label && def->seclabels[i]->norelabel) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("Cannot specify a label if relabelling is " + "turned off")); + goto error; + } + } return 0; + +error: + for (i = 0; i < n; i++) { + virSecurityDeviceLabelDefFree(def->seclabels[i]); + } + VIR_FREE(def->seclabels); + return -1; }
@@ -3341,7 +3414,8 @@ virDomainDiskDefParseXML(virCapsPtr caps, xmlNodePtr node, xmlXPathContextPtr ctxt, virBitmapPtr bootMap, - virSecurityLabelDefPtr vmSeclabel, + virSecurityLabelDefPtr* vmSeclabels, + int nvmSeclabels, unsigned int flags) { virDomainDiskDefPtr def; @@ -3679,15 +3753,9 @@ virDomainDiskDefParseXML(virCapsPtr caps, if (sourceNode) { xmlNodePtr saved_node = ctxt->node; ctxt->node = sourceNode; - if ((VIR_ALLOC(def->seclabels) < 0) || (VIR_ALLOC(def->seclabels[0]) < 0)) { - virReportOOMError(); - goto error; - } - if (virSecurityDeviceLabelDefParseXML(&def->seclabels[0], - vmSeclabel, - ctxt) < 0) + if (virSecurityDeviceLabelDefParseXML(def, vmSeclabels, + nvmSeclabels, ctxt) < 0) goto error; - def->nseclabels = 1; ctxt->node = saved_node; }
@@ -7130,16 +7198,12 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, goto error; }
- if ((VIR_ALLOC(def->seclabels) < 0) || - (VIR_ALLOC(def->seclabels[0])) < 0 ) { - virReportOOMError(); - goto error; - } - if (xmlStrEqual(node->name, BAD_CAST "disk")) { dev->type = VIR_DOMAIN_DEVICE_DISK; if (!(dev->data.disk = virDomainDiskDefParseXML(caps, node, ctxt, - NULL, def->seclabels[0], flags))) + NULL, def->seclabels, + def->nseclabels, + flags))) goto error; } else if (xmlStrEqual(node->name, BAD_CAST "lease")) { dev->type = VIR_DOMAIN_DEVICE_LEASE; @@ -8038,12 +8102,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
/* analysis of security label, done early even though we format it * late, so devices can refer to this for defaults */ - if ((VIR_ALLOC(def->seclabels) < 0) || (VIR_ALLOC(def->seclabels[0]) < 0)) { - virReportOOMError(); - goto error; - } - def->nseclabels = 1; - if (virSecurityLabelDefParseXML(def->seclabels[0], ctxt, flags) == -1) + if (virSecurityLabelDefsParseXML(def, ctxt, flags) == -1) goto error;
/* Extract domain memory */ @@ -8642,7 +8701,8 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, nodes[i], ctxt, bootMap, - def->seclabels[0], + def->seclabels, + def->nseclabels, flags); if (!disk) goto error; @@ -10935,16 +10995,19 @@ virSecurityLabelDefFormat(virBufferPtr buf, virSecurityLabelDefPtr def) if (def->type == VIR_DOMAIN_SECLABEL_DEFAULT) return;
- virBufferAsprintf(buf, "<seclabel type='%s'", - sectype); + virBufferAsprintf(buf, "<seclabel"); + + if (def->model) { + virBufferEscapeString(buf, " model='%s'", def->model); + } + + virBufferAsprintf(buf," type='%s'", sectype);
If you haven't swapped 'model' and 'type' attributes you wouldn't need this [1]. Therefore I suggest keeps things in the order they were.
if (def->type == VIR_DOMAIN_SECLABEL_NONE) { virBufferAddLit(buf, "/>\n"); return; }
- virBufferEscapeString(buf, " model='%s'", def->model); - virBufferAsprintf(buf, " relabel='%s'", def->norelabel ? "no" : "yes");
@@ -10970,8 +11033,8 @@ static void virSecurityDeviceLabelDefFormat(virBufferPtr buf, virSecurityDeviceLabelDefPtr def) { - virBufferAsprintf(buf, "<seclabel relabel='%s'", - def->norelabel ? "no" : "yes"); + virBufferAsprintf(buf, "<seclabel model='%s' relabel='%s'", + def->model, def->norelabel ? "no" : "yes"); if (def->label) { virBufferAddLit(buf, ">\n"); virBufferEscapeString(buf, " <label>%s</label>\n", @@ -11016,6 +11079,7 @@ virDomainDiskDefFormat(virBufferPtr buf, const char *copy_on_read = virDomainVirtioEventIdxTypeToString(def->copy_on_read); const char *startupPolicy = virDomainStartupPolicyTypeToString(def->startupPolicy);
+ int n; char uuidstr[VIR_UUID_STRING_BUFLEN];
if (!type) { @@ -11111,10 +11175,11 @@ virDomainDiskDefFormat(virBufferPtr buf, if (def->startupPolicy) virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); - if (def->seclabels && def->seclabels[0]) { + if (def->nseclabels) { virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 8); - virSecurityDeviceLabelDefFormat(buf, def->seclabels[0]); + for (n = 0; n < def->nseclabels; n++) + virSecurityDeviceLabelDefFormat(buf, def->seclabels[n]); virBufferAdjustIndent(buf, -8); virBufferAddLit(buf, " </source>\n"); } else { @@ -11124,10 +11189,11 @@ virDomainDiskDefFormat(virBufferPtr buf, case VIR_DOMAIN_DISK_TYPE_BLOCK: virBufferEscapeString(buf, " <source dev='%s'", def->src); - if (def->seclabels && def->seclabels[0]) { + if (def->nseclabels) { virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 8); - virSecurityDeviceLabelDefFormat(buf, def->seclabels[0]); + for (n = 0; n < def->nseclabels; n++) + virSecurityDeviceLabelDefFormat(buf, def->seclabels[n]); virBufferAdjustIndent(buf, -8); virBufferAddLit(buf, " </source>\n"); } else { @@ -13153,11 +13219,10 @@ virDomainDefFormatInternal(virDomainDefPtr def,
virBufferAddLit(buf, " </devices>\n");
- if (def->nseclabels && def->seclabels) { - virBufferAdjustIndent(buf, 2); - virSecurityLabelDefFormat(buf, def->seclabels[0]); - virBufferAdjustIndent(buf, -2); - } + virBufferAdjustIndent(buf, 2); + for (n = 0; n < def->nseclabels; n++) + virSecurityLabelDefFormat(buf, def->seclabels[n]); + virBufferAdjustIndent(buf, -2);
if (def->namespaceData && def->ns.format) { if ((def->ns.format)(buf, def->namespaceData) < 0) @@ -15282,3 +15347,65 @@ cleanup: VIR_FREE(xmlStr); return ret; } + +virSecurityLabelDefPtr +virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char *model) +{ + int i; + + if (def == NULL || model == NULL) + return NULL; + + for (i = 0; i < def->nseclabels; i++) { + if (def->seclabels[i]->model == NULL) + continue; + if (STREQ(def->seclabels[i]->model, model)) + return def->seclabels[i]; + } + + return virDomainDefAddSecurityLabelDef(def, model); +} + +virSecurityDeviceLabelDefPtr +virDomainDiskDefGetSecurityLabelDef(virDomainDiskDefPtr def, const char *model) +{ + int i; + + if (def == NULL) + return NULL; + + for (i = 0; i < def->nseclabels; i++) { + if (STREQ(def->seclabels[i]->model, model)) + return def->seclabels[i]; + } + return NULL; +} + +virSecurityLabelDefPtr +virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char *model) +{ + virSecurityLabelDefPtr seclabel = NULL; + + if (VIR_ALLOC(seclabel) < 0) { + virReportOOMError(); + return NULL; + } + + if (model) { + seclabel->model = strdup(model); + if (seclabel->model == NULL) { + virReportOOMError(); + virSecurityLabelDefFree(seclabel); + return NULL; + } + } + + if (VIR_EXPAND_N(def->seclabels, def->nseclabels, 1) < 0) { + virReportOOMError(); + virSecurityLabelDefFree(seclabel); + return NULL; + } + def->seclabels[def->nseclabels - 1] = seclabel; + + return seclabel; +} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 9a2189a..faa760a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2155,6 +2155,15 @@ virDomainState virDomainObjGetState(virDomainObjPtr obj, int *reason) ATTRIBUTE_NONNULL(1);
+virSecurityLabelDefPtr +virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char *model); + +virSecurityDeviceLabelDefPtr +virDomainDiskDefGetSecurityLabelDef(virDomainDiskDefPtr def, const char *model); + +virSecurityLabelDefPtr +virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char *model); +
I think these should be included in libvirt_private.syms within this patch rather than the next one.
typedef const char* (*virLifecycleToStringFunc)(int type); typedef int (*virLifecycleFromStringFunc)(const char *type);
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-baselabel.xml b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-baselabel.xml index 98362a7..171dd47 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-baselabel.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-baselabel.xml @@ -23,7 +23,7 @@ <controller type='ide' index='0'/> <memballoon model='virtio'/> </devices> - <seclabel type='dynamic' model='selinux' relabel='yes'> + <seclabel model='selinux' type='dynamic' relabel='yes'>
[1]: ^^ (here and the rest of this patch).
<baselabel>system_u:system_r:svirt_custom_t:s0</baselabel> </seclabel> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.xml b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.xml index 4de435b..769caeb 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.xml @@ -16,14 +16,14 @@ <emulator>/usr/bin/qemu</emulator> <disk type='block' device='disk'> <source dev='/dev/HostVG/QEMUGuest1'> - <seclabel relabel='no'/> + <seclabel model='selinux' relabel='no'/> </source> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='block' device='disk'> <source dev='/dev/HostVG/QEMUGuest2'> - <seclabel relabel='yes'> + <seclabel model='selinux' relabel='yes'> <label>system_u:system_r:public_content_t:s0</label> </seclabel> </source> @@ -35,7 +35,7 @@ <controller type='ide' index='0'/> <memballoon model='virtio'/> </devices> - <seclabel type='dynamic' model='selinux' relabel='yes'> + <seclabel model='selinux' type='dynamic' relabel='yes'> <baselabel>system_u:system_r:svirt_custom_t:s0</baselabel> </seclabel> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic.xml b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic.xml index 78a6b6a..36df9d4 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic.xml @@ -22,5 +22,5 @@ <controller type='ide' index='0'/> <memballoon model='virtio'/> </devices> - <seclabel type='dynamic' relabel='yes'/> + <seclabel model='selinux' type='dynamic' relabel='yes'/> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-static.xml b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-static.xml index 31d5f58..23ddef1 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-static.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-static.xml @@ -23,7 +23,7 @@ <controller type='ide' index='0'/> <memballoon model='virtio'/> </devices> - <seclabel type='static' model='selinux' relabel='no'> + <seclabel model='selinux' type='static' relabel='no'> <label>system_u:system_r:svirt_custom_t:s0:c192,c392</label> </seclabel> </domain>
Otherwise looking good. Side note: sometimes the patches are more readable when produced with --patience especially when moving blocks of code around. This is the farest I can go for today. I'll continue tomorrow. Michal

On 07/18/2012 10:31 AM, Michal Privoznik wrote:
On 18.07.2012 03:28, Marcelo Cerri wrote:
This patch updates the domain XML parser and formatter to support more than one "seclabel" element for each domain and device. The RNG schema and the tests related to this are also updated by this patch. --- docs/schemas/domaincommon.rng | 30 ++-
Missing documentation in docs/formatdomain.html.in. And unlike the capabilities.rng case, here I'm going to be a stickler and refuse to apply this without a doc patch, since we've been trying much harder to keep the domain html up-to-date. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Tue, Jul 17, 2012 at 10:28:35PM -0300, Marcelo Cerri wrote:
- /* Only parse model, if static labelling, or a base - * label is set, or doing active XML - */ - if (def->type == VIR_DOMAIN_SECLABEL_STATIC || - def->baselabel || - (!(flags & VIR_DOMAIN_XML_INACTIVE) && - def->type != VIR_DOMAIN_SECLABEL_NONE)) { - p = virXPathStringLimit("string(./seclabel[1]/@model)", - VIR_SECURITY_MODEL_BUFLEN-1, ctxt); - if (p == NULL) { - virDomainReportError(VIR_ERR_XML_ERROR, - "%s", _("missing security model")); - goto error; - } - def->model = p; + /* Always parse model */ + p = virXPathStringLimit("string(./@model)", + VIR_SECURITY_MODEL_BUFLEN-1, ctxt); + if (p == NULL && def->type != VIR_DOMAIN_SECLABEL_NONE) { + virDomainReportError(VIR_ERR_XML_ERROR, + "%s", _("missing security model")); + goto error; } + def->model = p;
This change is going to cause backwards compatibility problems. Existing apps using libvirt may not be providing any 'model', and we *must* continue to support that. So we should only be reporting an error on missing @model, in the smae scenarios we did previously, or if there is >1 <secmodel> set in the XML. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

These changes make the security drivers able to find and handle the correct security label information when more than one label is available. They also update the DAC driver to be used as an usual security driver. Please enter the commit message for your changes. Lines starting --- src/libvirt_private.syms | 3 + src/qemu/qemu_driver.c | 45 ++++-- src/qemu/qemu_process.c | 53 +++++-- src/security/security_apparmor.c | 112 ++++++++++---- src/security/security_dac.c | 320 ++++++++++++++++++++++++++++++++++---- src/security/security_manager.c | 98 +++++++++--- src/security/security_manager.h | 8 +- src/security/security_selinux.c | 249 +++++++++++++++++++++--------- src/security/security_stack.c | 237 +++++++++++++++++++--------- src/security/security_stack.h | 13 ++ 10 files changed, 872 insertions(+), 266 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 03f7f3e..758f219 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -281,6 +281,7 @@ virDomainDefClearPCIAddresses; virDomainDefFormat; virDomainDefFormatInternal; virDomainDefFree; +virDomainDefGetSecurityLabelDef; virDomainDefParseFile; virDomainDefParseNode; virDomainDefParseString; @@ -968,6 +969,7 @@ virSecurityManagerClearSocketLabel; virSecurityManagerFree; virSecurityManagerGenLabel; virSecurityManagerGetDOI; +virSecurityManagerGetNested; virSecurityManagerGetModel; virSecurityManagerGetProcessLabel; virSecurityManagerNew; @@ -987,6 +989,7 @@ virSecurityManagerSetHostdevLabel; virSecurityManagerSetProcessLabel; virSecurityManagerSetSavedStateLabel; virSecurityManagerSetSocketLabel; +virSecurityManagerStackAddNested; virSecurityManagerVerify; virSecurityManagerGetMountOptions; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a512a45..1b02f28 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -240,8 +240,8 @@ qemuSecurityInit(struct qemud_driver *driver) if (!dac) goto error; - if (!(driver->securityManager = virSecurityManagerNewStack(mgr, - dac))) { + if (!(driver->securityManager = virSecurityManagerNewStack(mgr)) || + !(virSecurityManagerStackAddNested(mgr, dac))) { virSecurityManagerFree(dac); goto error; @@ -263,7 +263,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))) { @@ -288,31 +292,38 @@ qemuCreateCapabilities(virCapsPtr oldcaps, goto err_exit; } - /* Security driver data */ - const char *doi, *model; + /* access sec drivers and create a sec model for each one */ + sec_managers = virSecurityManagerGetNested(driver->securityManager); + if (sec_managers == NULL) { + goto err_exit; + } - doi = virSecurityManagerGetDOI(driver->securityManager); - model = virSecurityManagerGetModel(driver->securityManager); + /* calculate length */ + for (i = 0; sec_managers[i]; i++) + ; + caps->host.nsecModels = i; - if (VIR_ALLOC(caps->host.secModels) < 0) { + if (VIR_ALLOC_N(caps->host.secModels, caps->host.nsecModels) < 0) goto no_memory; - } - if (STRNEQ(model, "none")) { - if (!(caps->host.secModels[0].model = strdup(model))) + 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.secModels[0].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; } @@ -4035,9 +4046,9 @@ 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.secModels[0].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.secModels[0].model; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e9a41df..7fbe412 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4079,12 +4079,12 @@ void qemuProcessStop(struct qemud_driver *driver, virSecurityManagerReleaseLabel(driver->securityManager, vm->def); /* Clear out dynamically assigned labels */ - if (vm->def->seclabels[0]->type == VIR_DOMAIN_SECLABEL_DYNAMIC) { - if (!vm->def->seclabels[0]->baselabel) - VIR_FREE(vm->def->seclabels[0]->model); - VIR_FREE(vm->def->seclabels[0]->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->seclabels[0]->imagelabel); virDomainDefClearDeviceAliases(vm->def); if (!priv->persistentAddrs) { @@ -4188,6 +4188,7 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, virDomainChrSourceDefPtr monConfig, bool monJSON) { + size_t i; char ebuf[1024]; int logfile = -1; char *timestamp; @@ -4195,6 +4196,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"); @@ -4227,17 +4231,35 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, goto no_memory; VIR_DEBUG("Detect security driver config"); - vm->def->seclabels[0]->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.secModels[0].model && - !(vm->def->seclabels[0]->model = strdup(driver->caps->host.secModels[0].model))) - goto no_memory; - if (!(vm->def->seclabels[0]->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) @@ -4359,7 +4381,6 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, } VIR_FORCE_CLOSE(logfile); - VIR_FREE(seclabel); return 0; diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 62af3c6..8639e16 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -262,9 +262,13 @@ reload_profile(virSecurityManagerPtr mgr, const char *fn, bool append) { - const virSecurityLabelDefPtr secdef = &def->seclabel; int rc = -1; char *profile_name = NULL; + const virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef( + def, SECURITY_APPARMOR_NAME); + + if (!secdef) + return rc; if (secdef->norelabel) return 0; @@ -298,7 +302,12 @@ AppArmorSetSecurityUSBLabel(usbDevice *dev ATTRIBUTE_UNUSED, virDomainDefPtr def = ptr->def; if (reload_profile(ptr->mgr, def, file, true) < 0) { - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef( + def, SECURITY_APPARMOR_NAME); + if (!secdef) { + virReportOOMError(); + return -1; + } virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("cannot update AppArmor profile " "\'%s\'"), @@ -316,7 +325,12 @@ AppArmorSetSecurityPCILabel(pciDevice *dev ATTRIBUTE_UNUSED, virDomainDefPtr def = ptr->def; if (reload_profile(ptr->mgr, def, file, true) < 0) { - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef( + def, SECURITY_APPARMOR_NAME); + if (!secdef) { + virReportOOMError(); + return -1; + } virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("cannot update AppArmor profile " "\'%s\'"), @@ -398,18 +412,23 @@ AppArmorGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, { int rc = -1; char *profile_name = NULL; + virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef(def, + SECURITY_APPARMOR_NAME); - if (def->seclabel.type == VIR_DOMAIN_SECLABEL_STATIC) + if (!secdef) + return -1; + + if (secdef->type == VIR_DOMAIN_SECLABEL_STATIC) return 0; - if (def->seclabel.baselabel) { + if (secdef->baselabel) { virSecurityReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Cannot set a base label with AppArmour")); return rc; } - if ((def->seclabel.label) || - (def->seclabel.model) || (def->seclabel.imagelabel)) { + if ((secdef->label) || + (secdef->model) || (secdef->imagelabel)) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("security label already defined for VM")); @@ -419,31 +438,31 @@ AppArmorGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, if ((profile_name = get_profile_name(def)) == NULL) return rc; - def->seclabel.label = strndup(profile_name, strlen(profile_name)); - if (!def->seclabel.label) { + secdef->label = strndup(profile_name, strlen(profile_name)); + if (!secdef->label) { virReportOOMError(); goto clean; } /* set imagelabel the same as label (but we won't use it) */ - def->seclabel.imagelabel = strndup(profile_name, - strlen(profile_name)); - if (!def->seclabel.imagelabel) { + secdef->imagelabel = strndup(profile_name, + strlen(profile_name)); + if (!secdef->imagelabel) { virReportOOMError(); goto err; } - def->seclabel.model = strdup(SECURITY_APPARMOR_NAME); - if (!def->seclabel.model) { + secdef->model = strdup(SECURITY_APPARMOR_NAME); + if (!secdef->model) { virReportOOMError(); goto err; } /* Now that we have a label, load the profile into the kernel. */ - if (load_profile(mgr, def->seclabel.label, def, NULL, false) < 0) { + if (load_profile(mgr, secdef->label, def, NULL, false) < 0) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("cannot load AppArmor profile " - "\'%s\'"), def->seclabel.label); + "\'%s\'"), secdef->label); goto err; } @@ -451,9 +470,9 @@ AppArmorGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, goto clean; err: - VIR_FREE(def->seclabel.label); - VIR_FREE(def->seclabel.imagelabel); - VIR_FREE(def->seclabel.model); + VIR_FREE(secdef->label); + VIR_FREE(secdef->imagelabel); + VIR_FREE(secdef->model); clean: VIR_FREE(profile_name); @@ -465,7 +484,12 @@ static int AppArmorSetSecurityAllLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, const char *stdin_path) { - if (def->seclabel.norelabel) + virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef(def, + SECURITY_APPARMOR_NAME); + if (!secdef) + return -1; + + if (secdef->norelabel) return 0; /* Reload the profile if stdin_path is specified. Note that @@ -518,7 +542,10 @@ static int AppArmorReleaseSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def) { - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef(def, + SECURITY_APPARMOR_NAME); + if (!secdef) + return -1; VIR_FREE(secdef->model); VIR_FREE(secdef->label); @@ -533,8 +560,12 @@ AppArmorRestoreSecurityAllLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def, int migrated ATTRIBUTE_UNUSED) { - const virSecurityLabelDefPtr secdef = &def->seclabel; int rc = 0; + const virSecurityLabelDefPtr secdef = + virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + + if (!secdef) + return -1; if (secdef->type == VIR_DOMAIN_SECLABEL_DYNAMIC) { if ((rc = remove_profile(secdef->label)) != 0) { @@ -552,9 +583,13 @@ AppArmorRestoreSecurityAllLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, static int AppArmorSetSecurityProcessLabel(virSecurityManagerPtr mgr, virDomainDefPtr def) { - const virSecurityLabelDefPtr secdef = &def->seclabel; int rc = -1; char *profile_name = NULL; + const virSecurityLabelDefPtr secdef = + virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + + if (!secdef) + return -1; if ((profile_name = get_profile_name(def)) == NULL) return rc; @@ -621,9 +656,13 @@ static int AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, virDomainDiskDefPtr disk) { - const virSecurityLabelDefPtr secdef = &def->seclabel; int rc = -1; char *profile_name; + const virSecurityLabelDefPtr secdef = + virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + + if (!secdef) + return -1; if (secdef->norelabel) return 0; @@ -666,7 +705,11 @@ static int AppArmorSecurityVerify(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def) { - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = + virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + + if (!secdef) + return -1; if (secdef->type == VIR_DOMAIN_SECLABEL_STATIC) { if (use_apparmor() < 0 || profile_status(secdef->label, 0) < 0) { @@ -694,9 +737,13 @@ AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr, virDomainHostdevDefPtr dev) { - const virSecurityLabelDefPtr secdef = &def->seclabel; struct SDPDOP *ptr; int ret = -1; + const virSecurityLabelDefPtr secdef = + virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + + if (!secdef) + return -1; if (secdef->norelabel) return 0; @@ -756,7 +803,12 @@ AppArmorRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr, virDomainHostdevDefPtr dev ATTRIBUTE_UNUSED) { - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = + virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + + if (!secdef) + return -1; + if (secdef->norelabel) return 0; @@ -789,7 +841,11 @@ AppArmorSetImageFDLabel(virSecurityManagerPtr mgr, char *proc = NULL; char *fd_path = NULL; - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = + virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + + if (!secdef) + return -1; if (secdef->imagelabel == NULL) return 0; diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 9182b39..18be55a 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -33,6 +33,7 @@ #include "storage_file.h" #define VIR_FROM_THIS VIR_FROM_SECURITY +#define SECURITY_DAC_NAME "dac" typedef struct _virSecurityDACData virSecurityDACData; typedef virSecurityDACData *virSecurityDACDataPtr; @@ -64,6 +65,132 @@ void virSecurityDACSetDynamicOwnership(virSecurityManagerPtr mgr, priv->dynamicOwnership = dynamicOwnership; } +static +int parseIds(const char *label, uid_t *uidPtr, gid_t *gidPtr) +{ + uid_t uid; + gid_t gid; + char *endptr = NULL; + + if (label == NULL) + return -1; + + if (virStrToLong_ui(label, &endptr, 10, &uid) || + endptr == NULL || *endptr != ':') { + return -1; + } + + if (virStrToLong_ui(endptr + 1, NULL, 10, &gid)) + return -1; + + if (uidPtr) + *uidPtr = uid; + if (gidPtr) + *gidPtr = gid; + return 0; +} + +static +int virSecurityDACParseIds(virDomainDefPtr def, uid_t *uidPtr, gid_t *gidPtr) +{ + uid_t uid; + gid_t gid; + virSecurityLabelDefPtr seclabel; + + if (def == NULL) + return -1; + + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); + if (seclabel == NULL) { + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, + _("security label for DAC not found")); + return -1; + } + + if (seclabel->label && parseIds(seclabel->label, &uid, &gid)) { + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to parse uid and gid for DAC " + "security driver")); + return -1; + } + + if (uidPtr) + *uidPtr = uid; + if (gidPtr) + *gidPtr = gid; + + return 0; +} + +static +int virSecurityDACGetIds(virDomainDefPtr def, virSecurityDACDataPtr priv, + uid_t *uidPtr, gid_t *gidPtr) +{ + if (virSecurityDACParseIds(def, uidPtr, gidPtr) == 0) + return 0; + + if (priv) { + if (uidPtr) + *uidPtr = priv->user; + if (gidPtr) + *gidPtr = priv->group; + return 0; + } + return -1; +} + +static +int virSecurityDACParseImageIds(virDomainDefPtr def, + uid_t *uidPtr, gid_t *gidPtr) +{ + uid_t uid; + gid_t gid; + virSecurityLabelDefPtr seclabel; + + if (def == NULL) + return -1; + + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); + if (seclabel == NULL) { + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, + _("security label for DAC not found")); + return -1; + } + + if (seclabel->imagelabel + && parseIds(seclabel->imagelabel, &uid, &gid)) { + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to parse uid and gid for DAC " + "security driver")); + return -1; + } + + if (uidPtr) + *uidPtr = uid; + if (gidPtr) + *gidPtr = gid; + + return 0; +} + +static +int virSecurityDACGetImageIds(virDomainDefPtr def, virSecurityDACDataPtr priv, + uid_t *uidPtr, gid_t *gidPtr) +{ + if (virSecurityDACParseImageIds(def, uidPtr, gidPtr) == 0) + return 0; + + if (priv) { + if (uidPtr) + *uidPtr = priv->user; + if (gidPtr) + *gidPtr = priv->group; + return 0; + } + return -1; +} + + static virSecurityDriverStatus virSecurityDACProbe(const char *virtDriver ATTRIBUTE_UNUSED) { @@ -85,7 +212,7 @@ virSecurityDACClose(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) static const char * virSecurityDACGetModel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) { - return "dac"; + return SECURITY_DAC_NAME; } static const char * virSecurityDACGetDOI(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) @@ -167,10 +294,17 @@ virSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, size_t depth ATTRIBUTE_UNUSED, void *opaque) { - virSecurityManagerPtr mgr = opaque; + void **params = opaque; + virSecurityManagerPtr mgr = params[0]; + virDomainDefPtr def = params[1]; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + uid_t user; + gid_t group; + + if (virSecurityDACGetImageIds(def, priv, &user, &group)) + return -1; - return virSecurityDACSetOwnership(path, priv->user, priv->group); + return virSecurityDACSetOwnership(path, user, group); } @@ -180,6 +314,9 @@ virSecurityDACSetSecurityImageLabel(virSecurityManagerPtr mgr, virDomainDiskDefPtr disk) { + uid_t user; + gid_t group; + void *params[2]; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); if (!priv->dynamicOwnership) @@ -188,12 +325,17 @@ virSecurityDACSetSecurityImageLabel(virSecurityManagerPtr mgr, if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) return 0; + if (virSecurityDACGetImageIds(def, priv, &user, &group)) + return -1; + + params[0] = mgr; + params[1] = def; return virDomainDiskDefForeachPath(disk, virSecurityManagerGetAllowDiskFormatProbing(mgr), false, - priv->user, priv->group, + user, group, virSecurityDACSetSecurityFileLabel, - mgr); + params); } @@ -259,10 +401,17 @@ virSecurityDACSetSecurityPCILabel(pciDevice *dev ATTRIBUTE_UNUSED, const char *file, void *opaque) { - virSecurityManagerPtr mgr = opaque; + void **params = opaque; + virSecurityManagerPtr mgr = params[0]; + virDomainDefPtr def = params[1]; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + uid_t user; + gid_t group; - return virSecurityDACSetOwnership(file, priv->user, priv->group); + if (virSecurityDACGetIds(def, priv, &user, &group)) + return -1; + + return virSecurityDACSetOwnership(file, user, group); } @@ -271,18 +420,26 @@ virSecurityDACSetSecurityUSBLabel(usbDevice *dev ATTRIBUTE_UNUSED, const char *file, void *opaque) { - virSecurityManagerPtr mgr = opaque; + void **params = opaque; + virSecurityManagerPtr mgr = params[0]; + virDomainDefPtr def = params[1]; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + uid_t user; + gid_t group; - return virSecurityDACSetOwnership(file, priv->user, priv->group); + if (virSecurityDACGetIds(def, priv, &user, &group)) + return -1; + + return virSecurityDACSetOwnership(file, user, group); } static int virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr, - virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDefPtr def, virDomainHostdevDefPtr dev) { + void *params[] = {mgr, def}; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); int ret = -1; @@ -300,7 +457,8 @@ virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr, if (!usb) goto done; - ret = usbDeviceFileIterate(usb, virSecurityDACSetSecurityUSBLabel, mgr); + ret = usbDeviceFileIterate(usb, virSecurityDACSetSecurityUSBLabel, + params); usbFreeDevice(usb); break; } @@ -314,7 +472,8 @@ virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr, if (!pci) goto done; - ret = pciDeviceFileIterate(pci, virSecurityDACSetSecurityPCILabel, mgr); + ret = pciDeviceFileIterate(pci, virSecurityDACSetSecurityPCILabel, + params); pciFreeDevice(pci); break; @@ -404,17 +563,23 @@ done: static int virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, virDomainChrSourceDefPtr dev) { virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); char *in = NULL, *out = NULL; int ret = -1; + uid_t user; + gid_t group; + + if (virSecurityDACGetIds(def, priv, &user, &group)) + return -1; switch (dev->type) { case VIR_DOMAIN_CHR_TYPE_DEV: case VIR_DOMAIN_CHR_TYPE_FILE: - ret = virSecurityDACSetOwnership(dev->data.file.path, priv->user, priv->group); + ret = virSecurityDACSetOwnership(dev->data.file.path, user, group); break; case VIR_DOMAIN_CHR_TYPE_PIPE: @@ -424,12 +589,12 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, goto done; } if (virFileExists(in) && virFileExists(out)) { - if ((virSecurityDACSetOwnership(in, priv->user, priv->group) < 0) || - (virSecurityDACSetOwnership(out, priv->user, priv->group) < 0)) { + if ((virSecurityDACSetOwnership(in, user, group) < 0) || + (virSecurityDACSetOwnership(out, user, group) < 0)) { goto done; } } else if (virSecurityDACSetOwnership(dev->data.file.path, - priv->user, priv->group) < 0) { + user, group) < 0) { goto done; } ret = 0; @@ -554,7 +719,7 @@ virSecurityDACSetChardevCallback(virDomainDefPtr def ATTRIBUTE_UNUSED, { virSecurityManagerPtr mgr = opaque; - return virSecurityDACSetChardevLabel(mgr, &dev->source); + return virSecurityDACSetChardevLabel(mgr, def, &dev->source); } @@ -565,6 +730,8 @@ virSecurityDACSetSecurityAllLabel(virSecurityManagerPtr mgr, { virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); int i; + uid_t user; + gid_t group; if (!priv->dynamicOwnership) return 0; @@ -591,16 +758,15 @@ virSecurityDACSetSecurityAllLabel(virSecurityManagerPtr mgr, mgr) < 0) return -1; + if (virSecurityDACGetImageIds(def, priv, &user, &group)) + return -1; + if (def->os.kernel && - virSecurityDACSetOwnership(def->os.kernel, - priv->user, - priv->group) < 0) + virSecurityDACSetOwnership(def->os.kernel, user, group) < 0) return -1; if (def->os.initrd && - virSecurityDACSetOwnership(def->os.initrd, - priv->user, - priv->group) < 0) + virSecurityDACSetOwnership(def->os.initrd, user, group) < 0) return -1; return 0; @@ -609,12 +775,17 @@ virSecurityDACSetSecurityAllLabel(virSecurityManagerPtr mgr, static int virSecurityDACSetSavedStateLabel(virSecurityManagerPtr mgr, - virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDefPtr def, const char *savefile) { + uid_t user; + gid_t group; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); - return virSecurityDACSetOwnership(savefile, priv->user, priv->group); + if (virSecurityDACGetImageIds(def, priv, &user, &group)) + return -1; + + return virSecurityDACSetOwnership(savefile, user, group); } @@ -636,12 +807,16 @@ static int virSecurityDACSetProcessLabel(virSecurityManagerPtr mgr, virDomainDefPtr def ATTRIBUTE_UNUSED) { + uid_t user; + gid_t group; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); - VIR_DEBUG("Dropping privileges of DEF to %u:%u", - (unsigned int) priv->user, (unsigned int) priv->group); + if (virSecurityDACGetIds(def, priv, &user, &group)) + return -1; + + VIR_DEBUG("Dropping privileges of DEF to %u:%u", user, group); - if (virSetUIDGID(priv->user, priv->group) < 0) + if (virSetUIDGID(user, group) < 0) return -1; return 0; @@ -656,9 +831,83 @@ virSecurityDACVerify(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, } static int -virSecurityDACGenLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainDefPtr def ATTRIBUTE_UNUSED) +virSecurityDACGenLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def) { + int rc = -1; + virSecurityLabelDefPtr seclabel; + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + + if (mgr == NULL) { + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("invalid security driver")); + return rc; + } + + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); + if (seclabel == NULL) { + return rc; + } + + if (seclabel->imagelabel) { + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("security image label already " + "defined for VM")); + return rc; + } + + if (seclabel->model + && STRNEQ(seclabel->model, SECURITY_DAC_NAME)) { + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, + _("security label model %s is not supported " + "with selinux"), + seclabel->model); + return rc; + } + + switch(seclabel->type) { + case VIR_DOMAIN_SECLABEL_STATIC: + if (seclabel->label == NULL) { + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, + _("missing label for static security " + "driver")); + return rc; + } + break; + case VIR_DOMAIN_SECLABEL_DYNAMIC: + if (virAsprintf(&seclabel->label, "%d:%d", priv->user, priv->group) < 0) { + virReportOOMError(); + return rc; + } + if (seclabel->label == NULL) { + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot generate dac user and group id")); + return rc; + } + break; + case VIR_DOMAIN_SECLABEL_NONE: + /* no op */ + break; + default: + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected security label type '%s'"), + virDomainSeclabelTypeToString(seclabel->type)); + return rc; + } + + if (!seclabel->norelabel) { + if (seclabel->imagelabel == NULL) { + seclabel->imagelabel = strdup(seclabel->label); + if (seclabel->imagelabel == NULL) { + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot generate dac user and group id")); + VIR_FREE(seclabel->label); + seclabel->label = NULL; + return rc; + } + } + } + return 0; } @@ -683,6 +932,15 @@ virSecurityDACGetProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, pid_t pid ATTRIBUTE_UNUSED, virSecurityLabelPtr seclabel ATTRIBUTE_UNUSED) { + virSecurityLabelDefPtr secdef = + virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); + + if (!secdef || !seclabel) + return -1; + + if (secdef->label) + strcpy(seclabel->label, secdef->label); + return 0; } @@ -724,7 +982,7 @@ static char *virSecurityDACGetMountOptions(virSecurityManagerPtr mgr ATTRIBUTE_U virSecurityDriver virSecurityDriverDAC = { .privateDataLen = sizeof(virSecurityDACData), - .name = "virDAC", + .name = SECURITY_DAC_NAME, .probe = virSecurityDACProbe, .open = virSecurityDACOpen, .close = virSecurityDACClose, diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 5ca3201..970bfd5 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -68,8 +68,7 @@ static virSecurityManagerPtr virSecurityManagerNewDriver(virSecurityDriverPtr dr return mgr; } -virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary, - virSecurityManagerPtr secondary) +virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary) { virSecurityManagerPtr mgr = virSecurityManagerNewDriver(&virSecurityDriverStack, @@ -81,12 +80,19 @@ virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary, if (!mgr) return NULL; - virSecurityStackSetPrimary(mgr, primary); - virSecurityStackSetSecondary(mgr, secondary); + virSecurityStackAddPrimary(mgr, primary); return mgr; } +int virSecurityManagerStackAddNested(virSecurityManagerPtr stack, + virSecurityManagerPtr nested) +{ + if (!STREQ("stack", stack->drv->name)) + return -1; + return virSecurityStackAddNested(stack, nested); +} + virSecurityManagerPtr virSecurityManagerNewDAC(const char *virtDriver, uid_t user, gid_t group, @@ -308,25 +314,51 @@ int virSecurityManagerRestoreSavedStateLabel(virSecurityManagerPtr mgr, int virSecurityManagerGenLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm) { - if (vm->seclabels[0]->type == VIR_DOMAIN_SECLABEL_DEFAULT) { - if (mgr->defaultConfined) - vm->seclabels[0]->type = VIR_DOMAIN_SECLABEL_DYNAMIC; - else - vm->seclabels[0]->type = VIR_DOMAIN_SECLABEL_NONE; - } + int rc = 0; + size_t i; + virSecurityManagerPtr* sec_managers = NULL; + virSecurityLabelDefPtr seclabel; - if ((vm->seclabels[0]->type == VIR_DOMAIN_SECLABEL_NONE) && - mgr->requireConfined) { - virSecurityReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Unconfined guests are not allowed on this host")); + if (mgr == NULL || mgr->drv == NULL) return -1; - } - if (mgr->drv->domainGenSecurityLabel) - return mgr->drv->domainGenSecurityLabel(mgr, vm); + if ((sec_managers = virSecurityManagerGetNested(mgr)) == NULL) + return -1; - virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); - return -1; + for (i = 0; sec_managers[i]; i++) { + seclabel = virDomainDefGetSecurityLabelDef(vm, + sec_managers[i]->drv->name); + if (seclabel == NULL) { + rc = -1; + goto cleanup; + } + + if (seclabel->type == VIR_DOMAIN_SECLABEL_DEFAULT) { + if (sec_managers[i]->defaultConfined) + seclabel->type = VIR_DOMAIN_SECLABEL_DYNAMIC; + else + seclabel->type = VIR_DOMAIN_SECLABEL_NONE; + } + + if ((seclabel->type == VIR_DOMAIN_SECLABEL_NONE) && + sec_managers[i]->requireConfined) { + virSecurityReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Unconfined guests are not allowed on this host")); + return -1; + } + + if (!sec_managers[i]->drv->domainGenSecurityLabel) { + virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + } else { + rc += sec_managers[i]->drv->domainGenSecurityLabel(sec_managers[i], vm); + if (rc) + goto cleanup; + } + } + +cleanup: + VIR_FREE(sec_managers); + return rc; } int virSecurityManagerReserveLabel(virSecurityManagerPtr mgr, @@ -397,12 +429,17 @@ int virSecurityManagerSetProcessLabel(virSecurityManagerPtr mgr, int virSecurityManagerVerify(virSecurityManagerPtr mgr, virDomainDefPtr def) { - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; + + if (mgr == NULL || mgr->drv == NULL) + return 0; + /* NULL model == dynamic labelling, with whatever driver * is active, so we can short circuit verify check to * avoid drivers de-referencing NULLs by accident */ - if (!secdef->model) + secdef = virDomainDefGetSecurityLabelDef(def, mgr->drv->name); + if (secdef == NULL || secdef->model == NULL) return 0; if (mgr->drv->domainSecurityVerify) @@ -435,3 +472,22 @@ char *virSecurityManagerGetMountOptions(virSecurityManagerPtr mgr, */ return NULL; } + +virSecurityManagerPtr* +virSecurityManagerGetNested(virSecurityManagerPtr mgr) +{ + virSecurityManagerPtr* list = NULL; + + if (STREQ("stack", mgr->drv->name)) { + return virSecurityStackGetNested(mgr); + } + + if (VIR_ALLOC_N(list, 2) < 0) { + virReportOOMError(); + return NULL; + } + + list[0] = mgr; + list[1] = NULL; + return list; +} diff --git a/src/security/security_manager.h b/src/security/security_manager.h index f0bf60d..f86b84d 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -37,8 +37,9 @@ virSecurityManagerPtr virSecurityManagerNew(const char *name, bool defaultConfined, bool requireConfined); -virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary, - virSecurityManagerPtr secondary); +virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary); +int virSecurityManagerStackAddNested(virSecurityManagerPtr stack, + virSecurityManagerPtr nested); virSecurityManagerPtr virSecurityManagerNewDAC(const char *virtDriver, uid_t user, @@ -109,4 +110,7 @@ int virSecurityManagerSetImageFDLabel(virSecurityManagerPtr mgr, int fd); char *virSecurityManagerGetMountOptions(virSecurityManagerPtr mgr, virDomainDefPtr vm); +virSecurityManagerPtr* +virSecurityManagerGetNested(virSecurityManagerPtr mgr); + #endif /* VIR_SECURITY_MANAGER_H__ */ diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 9a708b1..dc05910 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -271,46 +271,52 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr, int c2 = 0; context_t ctx = NULL; const char *range; - virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr); + virSecurityLabelDefPtr seclabel; + virSecuritySELinuxDataPtr data; - VIR_DEBUG("SELinuxGenSecurityLabel %s", virSecurityManagerGetDriver(mgr)); - if ((def->seclabels[0]->type == VIR_DOMAIN_SECLABEL_DYNAMIC) && - !def->seclabels[0]->baselabel && - def->seclabels[0]->model) { + if (mgr == NULL) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("security model already defined for VM")); + "%s", _("invalid security driver")); + return rc; + } + + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (seclabel == NULL) { return rc; } - if (def->seclabels[0]->type == VIR_DOMAIN_SECLABEL_DYNAMIC && - def->seclabels[0]->label) { + data = virSecurityManagerGetPrivateData(mgr); + + VIR_DEBUG("SELinuxGenSecurityLabel %s", virSecurityManagerGetDriver(mgr)); + if (seclabel->type == VIR_DOMAIN_SECLABEL_DYNAMIC && + seclabel->label) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("security label already defined for VM")); return rc; } - if (def->seclabels[0]->imagelabel) { + if (seclabel->imagelabel) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("security image label already defined for VM")); return rc; } - if (def->seclabels[0]->model && - STRNEQ(def->seclabels[0]->model, SECURITY_SELINUX_NAME)) { + if (seclabel->model && + STRNEQ(seclabel->model, SECURITY_SELINUX_NAME)) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("security label model %s is not supported with selinux"), - def->seclabels[0]->model); + seclabel->model); return rc; } - VIR_DEBUG("SELinuxGenSecurityLabel %d", def->seclabels[0]->type); + VIR_DEBUG("SELinuxGenSecurityLabel %d", seclabel->type); - switch (def->seclabels[0]->type) { + switch (seclabel->type) { case VIR_DOMAIN_SECLABEL_STATIC: - if (!(ctx = context_new(def->seclabels[0]->label)) ) { + if (!(ctx = context_new(seclabel->label)) ) { virReportSystemError(errno, _("unable to allocate socket security context '%s'"), - def->seclabels[0]->label); + seclabel->label); return rc; } @@ -345,11 +351,11 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr, } } while (mcsAdd(mcs) == -1); - def->seclabels[0]->label = - SELinuxGenNewContext(def->seclabels[0]->baselabel ? - def->seclabels[0]->baselabel : + seclabel->label = + SELinuxGenNewContext(seclabel->baselabel ? + seclabel->baselabel : data->domain_context, mcs); - if (! def->seclabels[0]->label) { + if (! seclabel->label) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("cannot generate selinux context for %s"), mcs); goto cleanup; @@ -363,21 +369,21 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr, default: virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected security label type '%s'"), - virDomainSeclabelTypeToString(def->seclabels[0]->type)); + virDomainSeclabelTypeToString(seclabel->type)); goto cleanup; } - if (!def->seclabels[0]->norelabel) { - def->seclabels[0]->imagelabel = SELinuxGenNewContext(data->file_context, mcs); - if (!def->seclabels[0]->imagelabel) { + if (!seclabel->norelabel) { + seclabel->imagelabel = SELinuxGenNewContext(data->domain_context, mcs); + if (!seclabel->imagelabel) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("cannot generate selinux context for %s"), mcs); goto cleanup; } } - if (!def->seclabels[0]->model && - !(def->seclabels[0]->model = strdup(SECURITY_SELINUX_NAME))) { + if (!seclabel->model && + !(seclabel->model = strdup(SECURITY_SELINUX_NAME))) { virReportOOMError(); goto cleanup; } @@ -386,12 +392,12 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr, cleanup: if (rc != 0) { - if (def->seclabels[0]->type == VIR_DOMAIN_SECLABEL_DYNAMIC) - VIR_FREE(def->seclabels[0]->label); - VIR_FREE(def->seclabels[0]->imagelabel); - if (def->seclabels[0]->type == VIR_DOMAIN_SECLABEL_DYNAMIC && - !def->seclabels[0]->baselabel) - VIR_FREE(def->seclabels[0]->model); + if (seclabel->type == VIR_DOMAIN_SECLABEL_DYNAMIC) + VIR_FREE(seclabel->label); + VIR_FREE(seclabel->imagelabel); + if (seclabel->type == VIR_DOMAIN_SECLABEL_DYNAMIC && + !seclabel->baselabel) + VIR_FREE(seclabel->model); } if (ctx) @@ -400,10 +406,10 @@ cleanup: VIR_FREE(mcs); VIR_DEBUG("model=%s label=%s imagelabel=%s baselabel=%s", - NULLSTR(def->seclabels[0]->model), - NULLSTR(def->seclabels[0]->label), - NULLSTR(def->seclabels[0]->imagelabel), - NULLSTR(def->seclabels[0]->baselabel)); + NULLSTR(seclabel->model), + NULLSTR(seclabel->label), + NULLSTR(seclabel->imagelabel), + NULLSTR(seclabel->baselabel)); return rc; } @@ -416,8 +422,14 @@ SELinuxReserveSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, security_context_t pctx; context_t ctx = NULL; const char *mcs; + virSecurityLabelDefPtr seclabel; + + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (seclabel == NULL) { + return -1; + } - if (def->seclabels[0]->type == VIR_DOMAIN_SECLABEL_STATIC) + if (seclabel->type == VIR_DOMAIN_SECLABEL_STATIC) return 0; if (getpidcon(pid, &pctx) == -1) { @@ -709,9 +721,16 @@ SELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDiskDefPtr disk, int migrated) { - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr seclabel; + virSecurityDeviceLabelDefPtr disk_seclabel; - if (secdef->norelabel || (disk->seclabels[0] && disk->seclabels[0]->norelabel)) + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (seclabel == NULL) + return -1; + + disk_seclabel = virDomainDiskDefGetSecurityLabelDef(disk, + SECURITY_SELINUX_NAME); + if (seclabel->norelabel || (disk_seclabel && disk_seclabel->norelabel)) return 0; /* Don't restore labels on readoly/shared disks, because @@ -763,17 +782,21 @@ SELinuxSetSecurityFileLabel(virDomainDiskDefPtr disk, size_t depth, void *opaque) { + int ret; + virSecurityDeviceLabelDefPtr disk_seclabel; virSecuritySELinuxCallbackDataPtr cbdata = opaque; const virSecurityLabelDefPtr secdef = cbdata->secdef; - int ret; virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(cbdata->manager); - if (disk->seclabels[0] && disk->seclabels[0]->norelabel) + disk_seclabel = virDomainDiskDefGetSecurityLabelDef(disk, + SECURITY_SELINUX_NAME); + + if (disk_seclabel && disk_seclabel->norelabel) return 0; - if (disk->seclabels[0] && !disk->seclabels[0]->norelabel && - disk->seclabels[0]->label) { - ret = SELinuxSetFilecon(path, disk->seclabels[0]->label); + if (disk_seclabel && !disk_seclabel->norelabel && + disk_seclabel->label) { + ret = SELinuxSetFilecon(path, disk_seclabel->label); } else if (depth == 0) { if (disk->shared) { @@ -788,14 +811,14 @@ SELinuxSetSecurityFileLabel(virDomainDiskDefPtr disk, } else { ret = SELinuxSetFileconOptional(path, data->content_context); } - if (ret == 1 && !disk->seclabels[0]) { + if (ret == 1 && !disk_seclabel) { /* If we failed to set a label, but virt_use_nfs let us * proceed anyway, then we don't need to relabel later. */ - if (VIR_ALLOC(disk->seclabels[0]) < 0) { + if (VIR_ALLOC(disk_seclabel) < 0) { virReportOOMError(); return -1; } - disk->seclabels[0]->norelabel = true; + disk_seclabel->norelabel = true; ret = 0; } return ret; @@ -807,11 +830,15 @@ SELinuxSetSecurityImageLabel(virSecurityManagerPtr mgr, virDomainDiskDefPtr disk) { + bool allowDiskFormatProbing; virSecuritySELinuxCallbackData cbdata; - cbdata.secdef = def->seclabels[0]; cbdata.manager = mgr; + cbdata.secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); - bool allowDiskFormatProbing = virSecurityManagerGetAllowDiskFormatProbing(mgr); + allowDiskFormatProbing = virSecurityManagerGetAllowDiskFormatProbing(mgr); + + if (cbdata.secdef == NULL) + return -1; if (cbdata.secdef->norelabel) return 0; @@ -838,9 +865,12 @@ static int SELinuxSetSecurityPCILabel(pciDevice *dev ATTRIBUTE_UNUSED, const char *file, void *opaque) { + virSecurityLabelDefPtr secdef; virDomainDefPtr def = opaque; - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1; return SELinuxSetFilecon(file, secdef->imagelabel); } @@ -848,8 +878,12 @@ static int SELinuxSetSecurityUSBLabel(usbDevice *dev ATTRIBUTE_UNUSED, const char *file, void *opaque) { + virSecurityLabelDefPtr secdef; virDomainDefPtr def = opaque; - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1; return SELinuxSetFilecon(file, secdef->imagelabel); } @@ -860,9 +894,13 @@ SELinuxSetSecurityHostdevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainHostdevDefPtr dev) { - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; int ret = -1; + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1; + if (secdef->norelabel) return 0; @@ -929,9 +967,13 @@ SELinuxRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainHostdevDefPtr dev) { - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; int ret = -1; + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1; + if (secdef->norelabel) return 0; @@ -982,10 +1024,14 @@ SELinuxSetSecurityChardevLabel(virDomainDefPtr def, virDomainChrSourceDefPtr dev) { - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; char *in = NULL, *out = NULL; int ret = -1; + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1; + if (secdef->norelabel) return 0; @@ -1028,10 +1074,14 @@ SELinuxRestoreSecurityChardevLabel(virDomainDefPtr def, virDomainChrSourceDefPtr dev) { - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; char *in = NULL, *out = NULL; int ret = -1; + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1; + if (secdef->norelabel) return 0; @@ -1121,12 +1171,16 @@ SELinuxRestoreSecurityAllLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def, int migrated ATTRIBUTE_UNUSED) { - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; int i; int rc = 0; VIR_DEBUG("Restoring security label on %s", def->name); + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1; + if (secdef->norelabel) return 0; @@ -1171,7 +1225,11 @@ static int SELinuxReleaseSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def) { - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; + + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1; if (secdef->type == VIR_DOMAIN_SECLABEL_DYNAMIC) { if (secdef->label != NULL) { @@ -1196,7 +1254,11 @@ SELinuxSetSavedStateLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def, const char *savefile) { - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; + + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1; if (secdef->norelabel) return 0; @@ -1210,7 +1272,11 @@ SELinuxRestoreSavedStateLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def, const char *savefile) { - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; + + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1; if (secdef->norelabel) return 0; @@ -1223,7 +1289,12 @@ static int SELinuxSecurityVerify(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def) { - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; + + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1; + if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("security label driver mismatch: " @@ -1248,12 +1319,16 @@ SELinuxSetSecurityProcessLabel(virSecurityManagerPtr mgr, virDomainDefPtr def) { /* TODO: verify DOI */ - const virSecurityLabelDefPtr secdef = def->seclabels[0]; - VIR_DEBUG("SELinuxSetSecurityProcessLabel %s", secdef->label); + virSecurityLabelDefPtr secdef; - if (def->seclabels[0]->label == NULL) + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1; + + if (secdef->label == NULL) return 0; + VIR_DEBUG("SELinuxSetSecurityProcessLabel %s", secdef->label); if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("security label driver mismatch: " @@ -1280,13 +1355,17 @@ SELinuxSetSecurityDaemonSocketLabel(virSecurityManagerPtr mgr, virDomainDefPtr def) { /* TODO: verify DOI */ - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; context_t execcon = NULL; context_t proccon = NULL; security_context_t scon = NULL; int rc = -1; - if (def->seclabels[0]->label == NULL) + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1; + + if (secdef->label == NULL) return 0; if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { @@ -1350,9 +1429,13 @@ static int SELinuxSetSecuritySocketLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm) { - const virSecurityLabelDefPtr secdef = vm->seclabels[0]; + virSecurityLabelDefPtr secdef; int rc = -1; + secdef = virDomainDefGetSecurityLabelDef(vm, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1; + if (secdef->label == NULL) return 0; @@ -1388,9 +1471,13 @@ SELinuxClearSecuritySocketLabel(virSecurityManagerPtr mgr, virDomainDefPtr def) { /* TODO: verify DOI */ - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; + + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1; - if (def->seclabels[0]->label == NULL) + if (secdef->label == NULL) return 0; if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { @@ -1466,9 +1553,13 @@ SELinuxSetSecurityAllLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, const char *stdin_path) { - virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr); - const virSecurityLabelDefPtr secdef = def->seclabels[0]; int i; + virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr); + virSecurityLabelDefPtr secdef; + + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1; if (secdef->norelabel) return 0; @@ -1528,7 +1619,11 @@ SELinuxSetImageFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def, int fd) { - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; + + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1; if (secdef->imagelabel == NULL) return 0; @@ -1538,13 +1633,17 @@ SELinuxSetImageFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, static char *genImageLabel(virSecurityManagerPtr mgr, virDomainDefPtr def) { - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr); const char *range; context_t ctx = NULL; char *label = NULL; const char *mcs = NULL; + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + goto cleanup; + if (secdef->label) { ctx = context_new(secdef->label); if (!ctx) { @@ -1575,7 +1674,11 @@ cleanup: static char *SELinuxGetSecurityMountOptions(virSecurityManagerPtr mgr, virDomainDefPtr def) { char *opts = NULL; - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; + + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return NULL; if (! secdef->imagelabel) secdef->imagelabel = genImageLabel(mgr,def); diff --git a/src/security/security_stack.c b/src/security/security_stack.c index 65d30d6..a7b8741 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -23,29 +23,70 @@ #include "security_stack.h" #include "virterror_internal.h" +#include "memory.h" #define VIR_FROM_THIS VIR_FROM_SECURITY typedef struct _virSecurityStackData virSecurityStackData; typedef virSecurityStackData *virSecurityStackDataPtr; +typedef struct _virSecurityStackItem virSecurityStackItem; +typedef virSecurityStackItem* virSecurityStackItemPtr; + +struct _virSecurityStackItem { + virSecurityManagerPtr securityManager; + virSecurityStackItemPtr next; +}; struct _virSecurityStackData { virSecurityManagerPtr primary; - virSecurityManagerPtr secondary; + virSecurityStackItemPtr itemsHead; }; -void virSecurityStackSetPrimary(virSecurityManagerPtr mgr, - virSecurityManagerPtr primary) +int +virSecurityStackAddPrimary(virSecurityManagerPtr mgr, + virSecurityManagerPtr primary) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + if (virSecurityStackAddNested(mgr, primary) < 0) + return -1; priv->primary = primary; + return 0; +} + +int +virSecurityStackAddNested(virSecurityManagerPtr mgr, + virSecurityManagerPtr nested) +{ + virSecurityStackItemPtr item = NULL; + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + + if (VIR_ALLOC(item) < 0) { + virReportOOMError(); + return -1; + } + item->securityManager = nested; + item->next = priv->itemsHead; + priv->itemsHead = item; + return 0; +} + +virSecurityManagerPtr +virSecurityStackGetPrimary(virSecurityManagerPtr mgr) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + return (priv->primary) ? priv->primary : priv->itemsHead->securityManager; +} + +void virSecurityStackSetPrimary(virSecurityManagerPtr mgr, + virSecurityManagerPtr primary) +{ + virSecurityStackAddPrimary(mgr, primary); } void virSecurityStackSetSecondary(virSecurityManagerPtr mgr, virSecurityManagerPtr secondary) { - virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); - priv->secondary = secondary; + virSecurityStackAddNested(mgr, secondary); } static virSecurityDriverStatus @@ -64,9 +105,14 @@ static int virSecurityStackClose(virSecurityManagerPtr mgr) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr next, item = priv->itemsHead; - virSecurityManagerFree(priv->primary); - virSecurityManagerFree(priv->secondary); + while (item) { + next = item->next; + virSecurityManagerFree(item->securityManager); + VIR_FREE(item); + item = next; + } return 0; } @@ -74,17 +120,13 @@ virSecurityStackClose(virSecurityManagerPtr mgr) static const char * virSecurityStackGetModel(virSecurityManagerPtr mgr) { - virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); - - return virSecurityManagerGetModel(priv->primary); + return virSecurityManagerGetModel(virSecurityStackGetPrimary(mgr)); } static const char * virSecurityStackGetDOI(virSecurityManagerPtr mgr) { - virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); - - return virSecurityManagerGetDOI(priv->primary); + return virSecurityManagerGetDOI(virSecurityStackGetPrimary(mgr)); } static int @@ -92,13 +134,15 @@ virSecurityStackVerify(virSecurityManagerPtr mgr, virDomainDefPtr def) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0; - if (virSecurityManagerVerify(priv->primary, def) < 0) - rc = -1; - - if (virSecurityManagerVerify(priv->secondary, def) < 0) - rc = -1; + for(; item; item = item->next) { + if (virSecurityManagerVerify(item->securityManager, def) < 0) { + rc = -1; + break; + } + } return rc; } @@ -108,12 +152,12 @@ static int virSecurityStackGenLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm) { - virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); int rc = 0; - if (virSecurityManagerGenLabel(priv->primary, vm) < 0) + if (virSecurityManagerGenLabel(virSecurityStackGetPrimary(mgr), vm) < 0) rc = -1; +// TODO #if 0 /* We don't allow secondary drivers to generate labels. * This may have to change in the future, but requires @@ -133,11 +177,12 @@ static int virSecurityStackReleaseLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm) { - virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); int rc = 0; - if (virSecurityManagerReleaseLabel(priv->primary, vm) < 0) + if (virSecurityManagerReleaseLabel(virSecurityStackGetPrimary(mgr), vm) < 0) rc = -1; + +// TODO #if 0 /* XXX See note in GenLabel */ if (virSecurityManagerReleaseLabel(priv->secondary, vm) < 0) @@ -153,11 +198,11 @@ virSecurityStackReserveLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, pid_t pid) { - virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); int rc = 0; - if (virSecurityManagerReserveLabel(priv->primary, vm, pid) < 0) + if (virSecurityManagerReserveLabel(virSecurityStackGetPrimary(mgr), vm, pid) < 0) rc = -1; +// TODO #if 0 /* XXX See note in GenLabel */ if (virSecurityManagerReserveLabel(priv->secondary, vm, pid) < 0) @@ -174,12 +219,13 @@ virSecurityStackSetSecurityImageLabel(virSecurityManagerPtr mgr, virDomainDiskDefPtr disk) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0; - if (virSecurityManagerSetImageLabel(priv->secondary, vm, disk) < 0) - rc = -1; - if (virSecurityManagerSetImageLabel(priv->primary, vm, disk) < 0) - rc = -1; + for (; item; item = item->next) { + if (virSecurityManagerSetImageLabel(item->securityManager, vm, disk) < 0) + rc = -1; + } return rc; } @@ -191,12 +237,13 @@ virSecurityStackRestoreSecurityImageLabel(virSecurityManagerPtr mgr, virDomainDiskDefPtr disk) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0; - if (virSecurityManagerRestoreImageLabel(priv->secondary, vm, disk) < 0) - rc = -1; - if (virSecurityManagerRestoreImageLabel(priv->primary, vm, disk) < 0) - rc = -1; + for (; item; item = item->next) { + if (virSecurityManagerRestoreImageLabel(item->securityManager, vm, disk) < 0) + rc = -1; + } return rc; } @@ -209,12 +256,13 @@ virSecurityStackSetSecurityHostdevLabel(virSecurityManagerPtr mgr, { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0; - if (virSecurityManagerSetHostdevLabel(priv->secondary, vm, dev) < 0) - rc = -1; - if (virSecurityManagerSetHostdevLabel(priv->primary, vm, dev) < 0) - rc = -1; + for (; item; item = item->next) { + if (virSecurityManagerSetHostdevLabel(item->securityManager, vm, dev) < 0) + rc = -1; + } return rc; } @@ -226,12 +274,13 @@ virSecurityStackRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr, virDomainHostdevDefPtr dev) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0; - if (virSecurityManagerRestoreHostdevLabel(priv->secondary, vm, dev) < 0) - rc = -1; - if (virSecurityManagerRestoreHostdevLabel(priv->primary, vm, dev) < 0) - rc = -1; + for (; item; item = item->next) { + if (virSecurityManagerRestoreHostdevLabel(item->securityManager, vm, dev) < 0) + rc = -1; + } return rc; } @@ -243,12 +292,13 @@ virSecurityStackSetSecurityAllLabel(virSecurityManagerPtr mgr, const char *stdin_path) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0; - if (virSecurityManagerSetAllLabel(priv->secondary, vm, stdin_path) < 0) - rc = -1; - if (virSecurityManagerSetAllLabel(priv->primary, vm, stdin_path) < 0) - rc = -1; + for (; item; item = item->next) { + if (virSecurityManagerSetAllLabel(item->securityManager, vm, stdin_path) < 0) + rc = -1; + } return rc; } @@ -260,12 +310,13 @@ virSecurityStackRestoreSecurityAllLabel(virSecurityManagerPtr mgr, int migrated) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0; - if (virSecurityManagerRestoreAllLabel(priv->secondary, vm, migrated) < 0) - rc = -1; - if (virSecurityManagerRestoreAllLabel(priv->primary, vm, migrated) < 0) - rc = -1; + for (; item; item = item->next) { + if (virSecurityManagerRestoreAllLabel(item->securityManager, vm, migrated) < 0) + rc = -1; + } return rc; } @@ -277,12 +328,13 @@ virSecurityStackSetSavedStateLabel(virSecurityManagerPtr mgr, const char *savefile) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0; - if (virSecurityManagerSetSavedStateLabel(priv->secondary, vm, savefile) < 0) - rc = -1; - if (virSecurityManagerSetSavedStateLabel(priv->primary, vm, savefile) < 0) - rc = -1; + for (; item; item = item->next) { + if (virSecurityManagerSetSavedStateLabel(item->securityManager, vm, savefile) < 0) + rc = -1; + } return rc; } @@ -294,12 +346,13 @@ virSecurityStackRestoreSavedStateLabel(virSecurityManagerPtr mgr, const char *savefile) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0; - if (virSecurityManagerRestoreSavedStateLabel(priv->secondary, vm, savefile) < 0) - rc = -1; - if (virSecurityManagerRestoreSavedStateLabel(priv->primary, vm, savefile) < 0) - rc = -1; + for (; item; item = item->next) { + if (virSecurityManagerRestoreSavedStateLabel(item->securityManager, vm, savefile) < 0) + rc = -1; + } return rc; } @@ -310,12 +363,13 @@ virSecurityStackSetProcessLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0; - if (virSecurityManagerSetProcessLabel(priv->secondary, vm) < 0) - rc = -1; - if (virSecurityManagerSetProcessLabel(priv->primary, vm) < 0) - rc = -1; + for (; item; item = item->next) { + if (virSecurityManagerSetProcessLabel(item->securityManager, vm) < 0) + rc = -1; + } return rc; } @@ -326,14 +380,14 @@ virSecurityStackGetProcessLabel(virSecurityManagerPtr mgr, pid_t pid, virSecurityLabelPtr seclabel) { - virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); int rc = 0; +// TODO #if 0 if (virSecurityManagerGetProcessLabel(priv->secondary, vm, pid, seclabel) < 0) rc = -1; #endif - if (virSecurityManagerGetProcessLabel(priv->primary, vm, pid, seclabel) < 0) + if (virSecurityManagerGetProcessLabel(virSecurityStackGetPrimary(mgr), vm, pid, seclabel) < 0) rc = -1; return rc; @@ -345,12 +399,13 @@ virSecurityStackSetDaemonSocketLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0; - if (virSecurityManagerSetDaemonSocketLabel(priv->secondary, vm) < 0) - rc = -1; - if (virSecurityManagerSetDaemonSocketLabel(priv->primary, vm) < 0) - rc = -1; + for (; item; item = item->next) { + if (virSecurityManagerSetDaemonSocketLabel(item->securityManager, vm) < 0) + rc = -1; + } return rc; } @@ -361,12 +416,13 @@ virSecurityStackSetSocketLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0; - if (virSecurityManagerSetSocketLabel(priv->secondary, vm) < 0) - rc = -1; - if (virSecurityManagerSetSocketLabel(priv->primary, vm) < 0) - rc = -1; + for (; item; item = item->next) { + if (virSecurityManagerSetSocketLabel(item->securityManager, vm) < 0) + rc = -1; + } return rc; } @@ -377,12 +433,13 @@ virSecurityStackClearSocketLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0; - if (virSecurityManagerClearSocketLabel(priv->secondary, vm) < 0) - rc = -1; - if (virSecurityManagerClearSocketLabel(priv->primary, vm) < 0) - rc = -1; + for (; item; item = item->next) { + if (virSecurityManagerClearSocketLabel(item->securityManager, vm) < 0) + rc = -1; + } return rc; } @@ -393,12 +450,13 @@ virSecurityStackSetImageFDLabel(virSecurityManagerPtr mgr, int fd) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0; - if (virSecurityManagerSetImageFDLabel(priv->secondary, vm, fd) < 0) - rc = -1; - if (virSecurityManagerSetImageFDLabel(priv->primary, vm, fd) < 0) - rc = -1; + for (; item; item = item->next) { + if (virSecurityManagerSetImageFDLabel(item->securityManager, vm, fd) < 0) + rc = -1; + } return rc; } @@ -408,6 +466,29 @@ static char *virSecurityStackGetMountOptions(virSecurityManagerPtr mgr ATTRIBUTE return NULL; } +virSecurityManagerPtr* +virSecurityStackGetNested(virSecurityManagerPtr mgr) +{ + virSecurityManagerPtr *list = NULL; + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item; + int len = 0, i = 0; + + for (item = priv->itemsHead; item; item = item->next) + len++; + + if (VIR_ALLOC_N(list, len + 1) < 0) { + virReportOOMError(); + return NULL; + } + + for (item = priv->itemsHead; item; item = item->next, i++) + list[i] = item->securityManager; + list[len] = NULL; + + return list; +} + virSecurityDriver virSecurityDriverStack = { .privateDataLen = sizeof(virSecurityStackData), .name = "stack", diff --git a/src/security/security_stack.h b/src/security/security_stack.h index bc83ff3..13a7a88 100644 --- a/src/security/security_stack.h +++ b/src/security/security_stack.h @@ -25,9 +25,22 @@ extern virSecurityDriver virSecurityDriverStack; + +int +virSecurityStackAddPrimary(virSecurityManagerPtr mgr, + virSecurityManagerPtr primary); +int +virSecurityStackAddNested(virSecurityManagerPtr mgr, + virSecurityManagerPtr nested); +virSecurityManagerPtr +virSecurityStackGetPrimary(virSecurityManagerPtr mgr); + void virSecurityStackSetPrimary(virSecurityManagerPtr mgr, virSecurityManagerPtr primary); void virSecurityStackSetSecondary(virSecurityManagerPtr mgr, virSecurityManagerPtr secondary); +virSecurityManagerPtr* +virSecurityStackGetNested(virSecurityManagerPtr mgr); + #endif /* __VIR_SECURITY_DAC */ -- 1.7.1

On 18.07.2012 03:28, Marcelo Cerri wrote:
These changes make the security drivers able to find and handle the correct security label information when more than one label is available. They also update the DAC driver to be used as an usual security driver.
Please enter the commit message for your changes. Lines starting --- src/libvirt_private.syms | 3 + src/qemu/qemu_driver.c | 45 ++++-- src/qemu/qemu_process.c | 53 +++++-- src/security/security_apparmor.c | 112 ++++++++++---- src/security/security_dac.c | 320 ++++++++++++++++++++++++++++++++++---- src/security/security_manager.c | 98 +++++++++--- src/security/security_manager.h | 8 +- src/security/security_selinux.c | 249 +++++++++++++++++++++--------- src/security/security_stack.c | 237 +++++++++++++++++++--------- src/security/security_stack.h | 13 ++ 10 files changed, 872 insertions(+), 266 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 03f7f3e..758f219 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -281,6 +281,7 @@ virDomainDefClearPCIAddresses; virDomainDefFormat; virDomainDefFormatInternal; virDomainDefFree; +virDomainDefGetSecurityLabelDef; virDomainDefParseFile; virDomainDefParseNode; virDomainDefParseString; @@ -968,6 +969,7 @@ virSecurityManagerClearSocketLabel; virSecurityManagerFree; virSecurityManagerGenLabel; virSecurityManagerGetDOI; +virSecurityManagerGetNested; virSecurityManagerGetModel; virSecurityManagerGetProcessLabel; virSecurityManagerNew; @@ -987,6 +989,7 @@ virSecurityManagerSetHostdevLabel; virSecurityManagerSetProcessLabel; virSecurityManagerSetSavedStateLabel; virSecurityManagerSetSocketLabel; +virSecurityManagerStackAddNested; virSecurityManagerVerify; virSecurityManagerGetMountOptions;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a512a45..1b02f28 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -240,8 +240,8 @@ qemuSecurityInit(struct qemud_driver *driver) if (!dac) goto error;
- if (!(driver->securityManager = virSecurityManagerNewStack(mgr, - dac))) { + if (!(driver->securityManager = virSecurityManagerNewStack(mgr)) || + !(virSecurityManagerStackAddNested(mgr, dac))) {
virSecurityManagerFree(dac); goto error; @@ -263,7 +263,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))) { @@ -288,31 +292,38 @@ qemuCreateCapabilities(virCapsPtr oldcaps, goto err_exit; }
- /* Security driver data */ - const char *doi, *model; + /* access sec drivers and create a sec model for each one */ + sec_managers = virSecurityManagerGetNested(driver->securityManager); + if (sec_managers == NULL) { + goto err_exit; + }
- doi = virSecurityManagerGetDOI(driver->securityManager); - model = virSecurityManagerGetModel(driver->securityManager); + /* calculate length */ + for (i = 0; sec_managers[i]; i++) + ; + caps->host.nsecModels = i;
- if (VIR_ALLOC(caps->host.secModels) < 0) { + if (VIR_ALLOC_N(caps->host.secModels, caps->host.nsecModels) < 0) goto no_memory; - }
- if (STRNEQ(model, "none")) { - if (!(caps->host.secModels[0].model = strdup(model))) + 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.secModels[0].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; } @@ -4035,9 +4046,9 @@ 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.secModels[0].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.secModels[0].model; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e9a41df..7fbe412 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4079,12 +4079,12 @@ void qemuProcessStop(struct qemud_driver *driver, virSecurityManagerReleaseLabel(driver->securityManager, vm->def);
/* Clear out dynamically assigned labels */ - if (vm->def->seclabels[0]->type == VIR_DOMAIN_SECLABEL_DYNAMIC) { - if (!vm->def->seclabels[0]->baselabel) - VIR_FREE(vm->def->seclabels[0]->model); - VIR_FREE(vm->def->seclabels[0]->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->seclabels[0]->imagelabel);
virDomainDefClearDeviceAliases(vm->def); if (!priv->persistentAddrs) { @@ -4188,6 +4188,7 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, virDomainChrSourceDefPtr monConfig, bool monJSON) { + size_t i; char ebuf[1024]; int logfile = -1; char *timestamp; @@ -4195,6 +4196,9 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, bool running = true; virDomainPausedReason reason; virSecurityLabelPtr seclabel = NULL; + virSecurityLabelDefPtr seclabeldef = NULL; + virSecurityManagerPtr* sec_managers;
sec_managers = NULL; As you need VIR_FREE(sec_managers); in the end it's better to initialize it to NULL rather than freeing a random address.
+ const char *model;
VIR_DEBUG("Beginning VM attach process");
@@ -4227,17 +4231,35 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, goto no_memory;
VIR_DEBUG("Detect security driver config"); - vm->def->seclabels[0]->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);
Don't forget to VIR_FREE() in the end.
+ if (sec_managers == NULL) { goto cleanup; - if (driver->caps->host.secModels[0].model && - !(vm->def->seclabels[0]->model = strdup(driver->caps->host.secModels[0].model))) - goto no_memory; - if (!(vm->def->seclabels[0]->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;
Probably a leftover from debugging. Besides, we don't support // style of comments, but rather old c89 style /* */
+ if (!(seclabeldef->model = strdup(model))) + goto no_memory; + + if (!(seclabeldef->label = strdup(seclabel->label))) + goto no_memory; + VIR_FREE(seclabel); + seclabel = NULL;
This is redundant. VIR_FREE() sets passed pointer to NULL.
+ }
VIR_DEBUG("Creating domain log file"); if ((logfile = qemuDomainCreateLog(driver, vm, false)) < 0) @@ -4359,7 +4381,6 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, }
VIR_FORCE_CLOSE(logfile); - VIR_FREE(seclabel);
This change is odd as it makes seclabel leak.
return 0;
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 62af3c6..8639e16 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -262,9 +262,13 @@ reload_profile(virSecurityManagerPtr mgr, const char *fn, bool append) { - const virSecurityLabelDefPtr secdef = &def->seclabel; int rc = -1; char *profile_name = NULL; + const virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef( + def, SECURITY_APPARMOR_NAME); + + if (!secdef) + return rc;
if (secdef->norelabel) return 0; @@ -298,7 +302,12 @@ AppArmorSetSecurityUSBLabel(usbDevice *dev ATTRIBUTE_UNUSED, virDomainDefPtr def = ptr->def;
if (reload_profile(ptr->mgr, def, file, true) < 0) { - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef( + def, SECURITY_APPARMOR_NAME); + if (!secdef) { + virReportOOMError(); + return -1; + } virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("cannot update AppArmor profile " "\'%s\'"), @@ -316,7 +325,12 @@ AppArmorSetSecurityPCILabel(pciDevice *dev ATTRIBUTE_UNUSED, virDomainDefPtr def = ptr->def;
if (reload_profile(ptr->mgr, def, file, true) < 0) { - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef( + def, SECURITY_APPARMOR_NAME); + if (!secdef) { + virReportOOMError(); + return -1; + } virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("cannot update AppArmor profile " "\'%s\'"), @@ -398,18 +412,23 @@ AppArmorGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, { int rc = -1; char *profile_name = NULL; + virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef(def, + SECURITY_APPARMOR_NAME);
- if (def->seclabel.type == VIR_DOMAIN_SECLABEL_STATIC) + if (!secdef) + return -1; + + if (secdef->type == VIR_DOMAIN_SECLABEL_STATIC) return 0;
- if (def->seclabel.baselabel) { + if (secdef->baselabel) { virSecurityReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Cannot set a base label with AppArmour")); return rc; }
- if ((def->seclabel.label) || - (def->seclabel.model) || (def->seclabel.imagelabel)) { + if ((secdef->label) || + (secdef->model) || (secdef->imagelabel)) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("security label already defined for VM")); @@ -419,31 +438,31 @@ AppArmorGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, if ((profile_name = get_profile_name(def)) == NULL) return rc;
- def->seclabel.label = strndup(profile_name, strlen(profile_name)); - if (!def->seclabel.label) { + secdef->label = strndup(profile_name, strlen(profile_name)); + if (!secdef->label) { virReportOOMError(); goto clean; }
/* set imagelabel the same as label (but we won't use it) */ - def->seclabel.imagelabel = strndup(profile_name, - strlen(profile_name)); - if (!def->seclabel.imagelabel) { + secdef->imagelabel = strndup(profile_name, + strlen(profile_name)); + if (!secdef->imagelabel) { virReportOOMError(); goto err; }
- def->seclabel.model = strdup(SECURITY_APPARMOR_NAME); - if (!def->seclabel.model) { + secdef->model = strdup(SECURITY_APPARMOR_NAME); + if (!secdef->model) { virReportOOMError(); goto err; }
/* Now that we have a label, load the profile into the kernel. */ - if (load_profile(mgr, def->seclabel.label, def, NULL, false) < 0) { + if (load_profile(mgr, secdef->label, def, NULL, false) < 0) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("cannot load AppArmor profile " - "\'%s\'"), def->seclabel.label); + "\'%s\'"), secdef->label); goto err; }
@@ -451,9 +470,9 @@ AppArmorGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, goto clean;
err: - VIR_FREE(def->seclabel.label); - VIR_FREE(def->seclabel.imagelabel); - VIR_FREE(def->seclabel.model); + VIR_FREE(secdef->label); + VIR_FREE(secdef->imagelabel); + VIR_FREE(secdef->model);
clean: VIR_FREE(profile_name); @@ -465,7 +484,12 @@ static int AppArmorSetSecurityAllLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, const char *stdin_path) { - if (def->seclabel.norelabel) + virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef(def, + SECURITY_APPARMOR_NAME); + if (!secdef) + return -1; + + if (secdef->norelabel) return 0;
/* Reload the profile if stdin_path is specified. Note that @@ -518,7 +542,10 @@ static int AppArmorReleaseSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def) { - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef(def, + SECURITY_APPARMOR_NAME); + if (!secdef) + return -1;
VIR_FREE(secdef->model); VIR_FREE(secdef->label); @@ -533,8 +560,12 @@ AppArmorRestoreSecurityAllLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def, int migrated ATTRIBUTE_UNUSED) { - const virSecurityLabelDefPtr secdef = &def->seclabel; int rc = 0; + const virSecurityLabelDefPtr secdef = + virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + + if (!secdef) + return -1;
if (secdef->type == VIR_DOMAIN_SECLABEL_DYNAMIC) { if ((rc = remove_profile(secdef->label)) != 0) { @@ -552,9 +583,13 @@ AppArmorRestoreSecurityAllLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, static int AppArmorSetSecurityProcessLabel(virSecurityManagerPtr mgr, virDomainDefPtr def) { - const virSecurityLabelDefPtr secdef = &def->seclabel; int rc = -1; char *profile_name = NULL; + const virSecurityLabelDefPtr secdef = + virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + + if (!secdef) + return -1;
if ((profile_name = get_profile_name(def)) == NULL) return rc; @@ -621,9 +656,13 @@ static int AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, virDomainDiskDefPtr disk) { - const virSecurityLabelDefPtr secdef = &def->seclabel; int rc = -1; char *profile_name; + const virSecurityLabelDefPtr secdef = + virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + + if (!secdef) + return -1;
if (secdef->norelabel) return 0; @@ -666,7 +705,11 @@ static int AppArmorSecurityVerify(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def) { - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = + virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + + if (!secdef) + return -1;
if (secdef->type == VIR_DOMAIN_SECLABEL_STATIC) { if (use_apparmor() < 0 || profile_status(secdef->label, 0) < 0) { @@ -694,9 +737,13 @@ AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr, virDomainHostdevDefPtr dev)
{ - const virSecurityLabelDefPtr secdef = &def->seclabel; struct SDPDOP *ptr; int ret = -1; + const virSecurityLabelDefPtr secdef = + virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + + if (!secdef) + return -1;
if (secdef->norelabel) return 0; @@ -756,7 +803,12 @@ AppArmorRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr, virDomainHostdevDefPtr dev ATTRIBUTE_UNUSED)
{ - const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = + virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + + if (!secdef) + return -1; + if (secdef->norelabel) return 0;
@@ -789,7 +841,11 @@ AppArmorSetImageFDLabel(virSecurityManagerPtr mgr, char *proc = NULL; char *fd_path = NULL;
- const virSecurityLabelDefPtr secdef = &def->seclabel; + const virSecurityLabelDefPtr secdef = + virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + + if (!secdef) + return -1;
if (secdef->imagelabel == NULL) return 0; diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 9182b39..18be55a 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -33,6 +33,7 @@ #include "storage_file.h"
#define VIR_FROM_THIS VIR_FROM_SECURITY +#define SECURITY_DAC_NAME "dac"
typedef struct _virSecurityDACData virSecurityDACData; typedef virSecurityDACData *virSecurityDACDataPtr; @@ -64,6 +65,132 @@ void virSecurityDACSetDynamicOwnership(virSecurityManagerPtr mgr, priv->dynamicOwnership = dynamicOwnership; }
+static +int parseIds(const char *label, uid_t *uidPtr, gid_t *gidPtr) +{ + uid_t uid; + gid_t gid; + char *endptr = NULL; + + if (label == NULL) + return -1; + + if (virStrToLong_ui(label, &endptr, 10, &uid) || + endptr == NULL || *endptr != ':') { + return -1; + } + + if (virStrToLong_ui(endptr + 1, NULL, 10, &gid)) + return -1; + + if (uidPtr) + *uidPtr = uid; + if (gidPtr) + *gidPtr = gid; + return 0; +} + +static +int virSecurityDACParseIds(virDomainDefPtr def, uid_t *uidPtr, gid_t *gidPtr) +{ + uid_t uid; + gid_t gid; + virSecurityLabelDefPtr seclabel; + + if (def == NULL) + return -1; + + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); + if (seclabel == NULL) { + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, + _("security label for DAC not found")); + return -1; + } + + if (seclabel->label && parseIds(seclabel->label, &uid, &gid)) { + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to parse uid and gid for DAC " + "security driver")); + return -1; + } + + if (uidPtr) + *uidPtr = uid; + if (gidPtr) + *gidPtr = gid; + + return 0; +} + +static +int virSecurityDACGetIds(virDomainDefPtr def, virSecurityDACDataPtr priv, + uid_t *uidPtr, gid_t *gidPtr) +{ + if (virSecurityDACParseIds(def, uidPtr, gidPtr) == 0) + return 0; + + if (priv) { + if (uidPtr) + *uidPtr = priv->user; + if (gidPtr) + *gidPtr = priv->group; + return 0; + } + return -1; +} + +static +int virSecurityDACParseImageIds(virDomainDefPtr def, + uid_t *uidPtr, gid_t *gidPtr) +{ + uid_t uid; + gid_t gid; + virSecurityLabelDefPtr seclabel; + + if (def == NULL) + return -1; + + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); + if (seclabel == NULL) { + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, + _("security label for DAC not found")); + return -1; + } + + if (seclabel->imagelabel + && parseIds(seclabel->imagelabel, &uid, &gid)) { + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to parse uid and gid for DAC " + "security driver")); + return -1; + } + + if (uidPtr) + *uidPtr = uid; + if (gidPtr) + *gidPtr = gid; + + return 0; +} + +static +int virSecurityDACGetImageIds(virDomainDefPtr def, virSecurityDACDataPtr priv, + uid_t *uidPtr, gid_t *gidPtr) +{ + if (virSecurityDACParseImageIds(def, uidPtr, gidPtr) == 0) + return 0; + + if (priv) { + if (uidPtr) + *uidPtr = priv->user; + if (gidPtr) + *gidPtr = priv->group; + return 0; + } + return -1; +} + + static virSecurityDriverStatus virSecurityDACProbe(const char *virtDriver ATTRIBUTE_UNUSED) { @@ -85,7 +212,7 @@ virSecurityDACClose(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED)
static const char * virSecurityDACGetModel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) { - return "dac"; + return SECURITY_DAC_NAME; }
static const char * virSecurityDACGetDOI(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) @@ -167,10 +294,17 @@ virSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, size_t depth ATTRIBUTE_UNUSED, void *opaque) { - virSecurityManagerPtr mgr = opaque; + void **params = opaque; + virSecurityManagerPtr mgr = params[0]; + virDomainDefPtr def = params[1]; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + uid_t user; + gid_t group; + + if (virSecurityDACGetImageIds(def, priv, &user, &group)) + return -1;
- return virSecurityDACSetOwnership(path, priv->user, priv->group); + return virSecurityDACSetOwnership(path, user, group); }
@@ -180,6 +314,9 @@ virSecurityDACSetSecurityImageLabel(virSecurityManagerPtr mgr, virDomainDiskDefPtr disk)
{ + uid_t user; + gid_t group; + void *params[2]; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
if (!priv->dynamicOwnership) @@ -188,12 +325,17 @@ virSecurityDACSetSecurityImageLabel(virSecurityManagerPtr mgr, if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) return 0;
+ if (virSecurityDACGetImageIds(def, priv, &user, &group)) + return -1; + + params[0] = mgr; + params[1] = def; return virDomainDiskDefForeachPath(disk, virSecurityManagerGetAllowDiskFormatProbing(mgr), false, - priv->user, priv->group, + user, group, virSecurityDACSetSecurityFileLabel, - mgr); + params); }
@@ -259,10 +401,17 @@ virSecurityDACSetSecurityPCILabel(pciDevice *dev ATTRIBUTE_UNUSED, const char *file, void *opaque) { - virSecurityManagerPtr mgr = opaque; + void **params = opaque; + virSecurityManagerPtr mgr = params[0]; + virDomainDefPtr def = params[1]; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + uid_t user; + gid_t group;
- return virSecurityDACSetOwnership(file, priv->user, priv->group); + if (virSecurityDACGetIds(def, priv, &user, &group)) + return -1; + + return virSecurityDACSetOwnership(file, user, group); }
@@ -271,18 +420,26 @@ virSecurityDACSetSecurityUSBLabel(usbDevice *dev ATTRIBUTE_UNUSED, const char *file, void *opaque) { - virSecurityManagerPtr mgr = opaque; + void **params = opaque; + virSecurityManagerPtr mgr = params[0]; + virDomainDefPtr def = params[1]; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + uid_t user; + gid_t group;
- return virSecurityDACSetOwnership(file, priv->user, priv->group); + if (virSecurityDACGetIds(def, priv, &user, &group)) + return -1; + + return virSecurityDACSetOwnership(file, user, group); }
static int virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr, - virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDefPtr def, virDomainHostdevDefPtr dev) { + void *params[] = {mgr, def}; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); int ret = -1;
@@ -300,7 +457,8 @@ virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr, if (!usb) goto done;
- ret = usbDeviceFileIterate(usb, virSecurityDACSetSecurityUSBLabel, mgr); + ret = usbDeviceFileIterate(usb, virSecurityDACSetSecurityUSBLabel, + params); usbFreeDevice(usb); break; } @@ -314,7 +472,8 @@ virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr, if (!pci) goto done;
- ret = pciDeviceFileIterate(pci, virSecurityDACSetSecurityPCILabel, mgr); + ret = pciDeviceFileIterate(pci, virSecurityDACSetSecurityPCILabel, + params); pciFreeDevice(pci);
break; @@ -404,17 +563,23 @@ done:
static int virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, virDomainChrSourceDefPtr dev)
{ virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); char *in = NULL, *out = NULL; int ret = -1; + uid_t user; + gid_t group; + + if (virSecurityDACGetIds(def, priv, &user, &group)) + return -1;
switch (dev->type) { case VIR_DOMAIN_CHR_TYPE_DEV: case VIR_DOMAIN_CHR_TYPE_FILE: - ret = virSecurityDACSetOwnership(dev->data.file.path, priv->user, priv->group); + ret = virSecurityDACSetOwnership(dev->data.file.path, user, group); break;
case VIR_DOMAIN_CHR_TYPE_PIPE: @@ -424,12 +589,12 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, goto done; } if (virFileExists(in) && virFileExists(out)) { - if ((virSecurityDACSetOwnership(in, priv->user, priv->group) < 0) || - (virSecurityDACSetOwnership(out, priv->user, priv->group) < 0)) { + if ((virSecurityDACSetOwnership(in, user, group) < 0) || + (virSecurityDACSetOwnership(out, user, group) < 0)) { goto done; } } else if (virSecurityDACSetOwnership(dev->data.file.path, - priv->user, priv->group) < 0) { + user, group) < 0) { goto done; } ret = 0; @@ -554,7 +719,7 @@ virSecurityDACSetChardevCallback(virDomainDefPtr def ATTRIBUTE_UNUSED, { virSecurityManagerPtr mgr = opaque;
- return virSecurityDACSetChardevLabel(mgr, &dev->source); + return virSecurityDACSetChardevLabel(mgr, def, &dev->source); }
@@ -565,6 +730,8 @@ virSecurityDACSetSecurityAllLabel(virSecurityManagerPtr mgr, { virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); int i; + uid_t user; + gid_t group;
if (!priv->dynamicOwnership) return 0; @@ -591,16 +758,15 @@ virSecurityDACSetSecurityAllLabel(virSecurityManagerPtr mgr, mgr) < 0) return -1;
+ if (virSecurityDACGetImageIds(def, priv, &user, &group)) + return -1; + if (def->os.kernel && - virSecurityDACSetOwnership(def->os.kernel, - priv->user, - priv->group) < 0) + virSecurityDACSetOwnership(def->os.kernel, user, group) < 0) return -1;
if (def->os.initrd && - virSecurityDACSetOwnership(def->os.initrd, - priv->user, - priv->group) < 0) + virSecurityDACSetOwnership(def->os.initrd, user, group) < 0) return -1;
return 0; @@ -609,12 +775,17 @@ virSecurityDACSetSecurityAllLabel(virSecurityManagerPtr mgr,
static int virSecurityDACSetSavedStateLabel(virSecurityManagerPtr mgr, - virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDefPtr def, const char *savefile) { + uid_t user; + gid_t group; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
- return virSecurityDACSetOwnership(savefile, priv->user, priv->group); + if (virSecurityDACGetImageIds(def, priv, &user, &group)) + return -1; + + return virSecurityDACSetOwnership(savefile, user, group); }
@@ -636,12 +807,16 @@ static int virSecurityDACSetProcessLabel(virSecurityManagerPtr mgr, virDomainDefPtr def ATTRIBUTE_UNUSED) { + uid_t user; + gid_t group; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
- VIR_DEBUG("Dropping privileges of DEF to %u:%u", - (unsigned int) priv->user, (unsigned int) priv->group); + if (virSecurityDACGetIds(def, priv, &user, &group)) + return -1; + + VIR_DEBUG("Dropping privileges of DEF to %u:%u", user, group);
- if (virSetUIDGID(priv->user, priv->group) < 0) + if (virSetUIDGID(user, group) < 0) return -1;
return 0; @@ -656,9 +831,83 @@ virSecurityDACVerify(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, }
static int -virSecurityDACGenLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainDefPtr def ATTRIBUTE_UNUSED) +virSecurityDACGenLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def) { + int rc = -1; + virSecurityLabelDefPtr seclabel; + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + + if (mgr == NULL) { + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("invalid security driver")); + return rc; + } + + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); + if (seclabel == NULL) { + return rc; + } + + if (seclabel->imagelabel) { + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("security image label already " + "defined for VM")); + return rc; + } + + if (seclabel->model + && STRNEQ(seclabel->model, SECURITY_DAC_NAME)) { + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, + _("security label model %s is not supported " + "with selinux"), + seclabel->model); + return rc; + } + + switch(seclabel->type) { + case VIR_DOMAIN_SECLABEL_STATIC: + if (seclabel->label == NULL) { + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, + _("missing label for static security " + "driver")); + return rc; + } + break; + case VIR_DOMAIN_SECLABEL_DYNAMIC: + if (virAsprintf(&seclabel->label, "%d:%d", priv->user, priv->group) < 0) { + virReportOOMError(); + return rc; + } + if (seclabel->label == NULL) { + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot generate dac user and group id")); + return rc; + } + break; + case VIR_DOMAIN_SECLABEL_NONE: + /* no op */ + break; + default: + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected security label type '%s'"), + virDomainSeclabelTypeToString(seclabel->type)); + return rc; + } + + if (!seclabel->norelabel) { + if (seclabel->imagelabel == NULL) { + seclabel->imagelabel = strdup(seclabel->label); + if (seclabel->imagelabel == NULL) { + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot generate dac user and group id")); + VIR_FREE(seclabel->label); + seclabel->label = NULL; + return rc; + } + } + } + return 0; }
@@ -683,6 +932,15 @@ virSecurityDACGetProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, pid_t pid ATTRIBUTE_UNUSED, virSecurityLabelPtr seclabel ATTRIBUTE_UNUSED) { + virSecurityLabelDefPtr secdef = + virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); + + if (!secdef || !seclabel) + return -1; + + if (secdef->label) + strcpy(seclabel->label, secdef->label); + return 0; }
@@ -724,7 +982,7 @@ static char *virSecurityDACGetMountOptions(virSecurityManagerPtr mgr ATTRIBUTE_U
virSecurityDriver virSecurityDriverDAC = { .privateDataLen = sizeof(virSecurityDACData), - .name = "virDAC", + .name = SECURITY_DAC_NAME, .probe = virSecurityDACProbe, .open = virSecurityDACOpen, .close = virSecurityDACClose, diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 5ca3201..970bfd5 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -68,8 +68,7 @@ static virSecurityManagerPtr virSecurityManagerNewDriver(virSecurityDriverPtr dr return mgr; }
-virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary, - virSecurityManagerPtr secondary) +virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary) { virSecurityManagerPtr mgr = virSecurityManagerNewDriver(&virSecurityDriverStack, @@ -81,12 +80,19 @@ virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary, if (!mgr) return NULL;
- virSecurityStackSetPrimary(mgr, primary); - virSecurityStackSetSecondary(mgr, secondary); + virSecurityStackAddPrimary(mgr, primary);
return mgr; }
+int virSecurityManagerStackAddNested(virSecurityManagerPtr stack, + virSecurityManagerPtr nested) +{ + if (!STREQ("stack", stack->drv->name)) + return -1; + return virSecurityStackAddNested(stack, nested); +} + virSecurityManagerPtr virSecurityManagerNewDAC(const char *virtDriver, uid_t user, gid_t group, @@ -308,25 +314,51 @@ int virSecurityManagerRestoreSavedStateLabel(virSecurityManagerPtr mgr, int virSecurityManagerGenLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm) { - if (vm->seclabels[0]->type == VIR_DOMAIN_SECLABEL_DEFAULT) { - if (mgr->defaultConfined) - vm->seclabels[0]->type = VIR_DOMAIN_SECLABEL_DYNAMIC; - else - vm->seclabels[0]->type = VIR_DOMAIN_SECLABEL_NONE; - } + int rc = 0; + size_t i; + virSecurityManagerPtr* sec_managers = NULL; + virSecurityLabelDefPtr seclabel;
- if ((vm->seclabels[0]->type == VIR_DOMAIN_SECLABEL_NONE) && - mgr->requireConfined) { - virSecurityReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Unconfined guests are not allowed on this host")); + if (mgr == NULL || mgr->drv == NULL) return -1; - }
- if (mgr->drv->domainGenSecurityLabel) - return mgr->drv->domainGenSecurityLabel(mgr, vm); + if ((sec_managers = virSecurityManagerGetNested(mgr)) == NULL) + return -1;
Again, sec_managers needs to be freed.
- virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); - return -1; + for (i = 0; sec_managers[i]; i++) { + seclabel = virDomainDefGetSecurityLabelDef(vm, + sec_managers[i]->drv->name); + if (seclabel == NULL) { + rc = -1; + goto cleanup; + } + + if (seclabel->type == VIR_DOMAIN_SECLABEL_DEFAULT) { + if (sec_managers[i]->defaultConfined) + seclabel->type = VIR_DOMAIN_SECLABEL_DYNAMIC; + else + seclabel->type = VIR_DOMAIN_SECLABEL_NONE; + } + + if ((seclabel->type == VIR_DOMAIN_SECLABEL_NONE) && + sec_managers[i]->requireConfined) { + virSecurityReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Unconfined guests are not allowed on this host")); + return -1; + } + + if (!sec_managers[i]->drv->domainGenSecurityLabel) { + virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + } else { + rc += sec_managers[i]->drv->domainGenSecurityLabel(sec_managers[i], vm); + if (rc) + goto cleanup; + } + } + +cleanup: + VIR_FREE(sec_managers); + return rc; }
int virSecurityManagerReserveLabel(virSecurityManagerPtr mgr, @@ -397,12 +429,17 @@ int virSecurityManagerSetProcessLabel(virSecurityManagerPtr mgr, int virSecurityManagerVerify(virSecurityManagerPtr mgr, virDomainDefPtr def) { - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; + + if (mgr == NULL || mgr->drv == NULL) + return 0; + /* NULL model == dynamic labelling, with whatever driver * is active, so we can short circuit verify check to * avoid drivers de-referencing NULLs by accident */ - if (!secdef->model) + secdef = virDomainDefGetSecurityLabelDef(def, mgr->drv->name); + if (secdef == NULL || secdef->model == NULL) return 0;
if (mgr->drv->domainSecurityVerify) @@ -435,3 +472,22 @@ char *virSecurityManagerGetMountOptions(virSecurityManagerPtr mgr, */ return NULL; } + +virSecurityManagerPtr* +virSecurityManagerGetNested(virSecurityManagerPtr mgr) +{ + virSecurityManagerPtr* list = NULL; + + if (STREQ("stack", mgr->drv->name)) { + return virSecurityStackGetNested(mgr); + } + + if (VIR_ALLOC_N(list, 2) < 0) { + virReportOOMError(); + return NULL; + } + + list[0] = mgr; + list[1] = NULL; + return list; +} diff --git a/src/security/security_manager.h b/src/security/security_manager.h index f0bf60d..f86b84d 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -37,8 +37,9 @@ virSecurityManagerPtr virSecurityManagerNew(const char *name, bool defaultConfined, bool requireConfined);
-virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary, - virSecurityManagerPtr secondary); +virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary); +int virSecurityManagerStackAddNested(virSecurityManagerPtr stack, + virSecurityManagerPtr nested);
virSecurityManagerPtr virSecurityManagerNewDAC(const char *virtDriver, uid_t user, @@ -109,4 +110,7 @@ int virSecurityManagerSetImageFDLabel(virSecurityManagerPtr mgr, int fd); char *virSecurityManagerGetMountOptions(virSecurityManagerPtr mgr, virDomainDefPtr vm); +virSecurityManagerPtr* +virSecurityManagerGetNested(virSecurityManagerPtr mgr); + #endif /* VIR_SECURITY_MANAGER_H__ */ diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 9a708b1..dc05910 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -271,46 +271,52 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr, int c2 = 0; context_t ctx = NULL; const char *range; - virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr); + virSecurityLabelDefPtr seclabel; + virSecuritySELinuxDataPtr data;
- VIR_DEBUG("SELinuxGenSecurityLabel %s", virSecurityManagerGetDriver(mgr)); - if ((def->seclabels[0]->type == VIR_DOMAIN_SECLABEL_DYNAMIC) && - !def->seclabels[0]->baselabel && - def->seclabels[0]->model) { + if (mgr == NULL) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("security model already defined for VM")); + "%s", _("invalid security driver")); + return rc; + } + + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (seclabel == NULL) { return rc; }
- if (def->seclabels[0]->type == VIR_DOMAIN_SECLABEL_DYNAMIC && - def->seclabels[0]->label) { + data = virSecurityManagerGetPrivateData(mgr); + + VIR_DEBUG("SELinuxGenSecurityLabel %s", virSecurityManagerGetDriver(mgr)); + if (seclabel->type == VIR_DOMAIN_SECLABEL_DYNAMIC && + seclabel->label) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("security label already defined for VM")); return rc; }
- if (def->seclabels[0]->imagelabel) { + if (seclabel->imagelabel) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("security image label already defined for VM")); return rc; }
- if (def->seclabels[0]->model && - STRNEQ(def->seclabels[0]->model, SECURITY_SELINUX_NAME)) { + if (seclabel->model && + STRNEQ(seclabel->model, SECURITY_SELINUX_NAME)) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("security label model %s is not supported with selinux"), - def->seclabels[0]->model); + seclabel->model); return rc; }
- VIR_DEBUG("SELinuxGenSecurityLabel %d", def->seclabels[0]->type); + VIR_DEBUG("SELinuxGenSecurityLabel %d", seclabel->type);
- switch (def->seclabels[0]->type) { + switch (seclabel->type) { case VIR_DOMAIN_SECLABEL_STATIC: - if (!(ctx = context_new(def->seclabels[0]->label)) ) { + if (!(ctx = context_new(seclabel->label)) ) { virReportSystemError(errno, _("unable to allocate socket security context '%s'"), - def->seclabels[0]->label); + seclabel->label); return rc; }
@@ -345,11 +351,11 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr, } } while (mcsAdd(mcs) == -1);
- def->seclabels[0]->label = - SELinuxGenNewContext(def->seclabels[0]->baselabel ? - def->seclabels[0]->baselabel : + seclabel->label = + SELinuxGenNewContext(seclabel->baselabel ? + seclabel->baselabel : data->domain_context, mcs); - if (! def->seclabels[0]->label) { + if (! seclabel->label) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("cannot generate selinux context for %s"), mcs); goto cleanup; @@ -363,21 +369,21 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr, default: virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected security label type '%s'"), - virDomainSeclabelTypeToString(def->seclabels[0]->type)); + virDomainSeclabelTypeToString(seclabel->type)); goto cleanup; }
- if (!def->seclabels[0]->norelabel) { - def->seclabels[0]->imagelabel = SELinuxGenNewContext(data->file_context, mcs); - if (!def->seclabels[0]->imagelabel) { + if (!seclabel->norelabel) { + seclabel->imagelabel = SELinuxGenNewContext(data->domain_context, mcs); + if (!seclabel->imagelabel) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("cannot generate selinux context for %s"), mcs); goto cleanup; } }
- if (!def->seclabels[0]->model && - !(def->seclabels[0]->model = strdup(SECURITY_SELINUX_NAME))) { + if (!seclabel->model && + !(seclabel->model = strdup(SECURITY_SELINUX_NAME))) { virReportOOMError(); goto cleanup; } @@ -386,12 +392,12 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr,
cleanup: if (rc != 0) { - if (def->seclabels[0]->type == VIR_DOMAIN_SECLABEL_DYNAMIC) - VIR_FREE(def->seclabels[0]->label); - VIR_FREE(def->seclabels[0]->imagelabel); - if (def->seclabels[0]->type == VIR_DOMAIN_SECLABEL_DYNAMIC && - !def->seclabels[0]->baselabel) - VIR_FREE(def->seclabels[0]->model); + if (seclabel->type == VIR_DOMAIN_SECLABEL_DYNAMIC) + VIR_FREE(seclabel->label); + VIR_FREE(seclabel->imagelabel); + if (seclabel->type == VIR_DOMAIN_SECLABEL_DYNAMIC && + !seclabel->baselabel) + VIR_FREE(seclabel->model); }
if (ctx) @@ -400,10 +406,10 @@ cleanup: VIR_FREE(mcs);
VIR_DEBUG("model=%s label=%s imagelabel=%s baselabel=%s", - NULLSTR(def->seclabels[0]->model), - NULLSTR(def->seclabels[0]->label), - NULLSTR(def->seclabels[0]->imagelabel), - NULLSTR(def->seclabels[0]->baselabel)); + NULLSTR(seclabel->model), + NULLSTR(seclabel->label), + NULLSTR(seclabel->imagelabel), + NULLSTR(seclabel->baselabel));
return rc; } @@ -416,8 +422,14 @@ SELinuxReserveSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, security_context_t pctx; context_t ctx = NULL; const char *mcs; + virSecurityLabelDefPtr seclabel; + + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (seclabel == NULL) { + return -1; + }
- if (def->seclabels[0]->type == VIR_DOMAIN_SECLABEL_STATIC) + if (seclabel->type == VIR_DOMAIN_SECLABEL_STATIC) return 0;
if (getpidcon(pid, &pctx) == -1) { @@ -709,9 +721,16 @@ SELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDiskDefPtr disk, int migrated) { - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr seclabel; + virSecurityDeviceLabelDefPtr disk_seclabel;
- if (secdef->norelabel || (disk->seclabels[0] && disk->seclabels[0]->norelabel)) + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (seclabel == NULL) + return -1; + + disk_seclabel = virDomainDiskDefGetSecurityLabelDef(disk, + SECURITY_SELINUX_NAME); + if (seclabel->norelabel || (disk_seclabel && disk_seclabel->norelabel)) return 0;
/* Don't restore labels on readoly/shared disks, because @@ -763,17 +782,21 @@ SELinuxSetSecurityFileLabel(virDomainDiskDefPtr disk, size_t depth, void *opaque) { + int ret; + virSecurityDeviceLabelDefPtr disk_seclabel; virSecuritySELinuxCallbackDataPtr cbdata = opaque; const virSecurityLabelDefPtr secdef = cbdata->secdef; - int ret; virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(cbdata->manager);
- if (disk->seclabels[0] && disk->seclabels[0]->norelabel) + disk_seclabel = virDomainDiskDefGetSecurityLabelDef(disk, + SECURITY_SELINUX_NAME); + + if (disk_seclabel && disk_seclabel->norelabel) return 0;
- if (disk->seclabels[0] && !disk->seclabels[0]->norelabel && - disk->seclabels[0]->label) { - ret = SELinuxSetFilecon(path, disk->seclabels[0]->label); + if (disk_seclabel && !disk_seclabel->norelabel && + disk_seclabel->label) { + ret = SELinuxSetFilecon(path, disk_seclabel->label); } else if (depth == 0) {
if (disk->shared) { @@ -788,14 +811,14 @@ SELinuxSetSecurityFileLabel(virDomainDiskDefPtr disk, } else { ret = SELinuxSetFileconOptional(path, data->content_context); } - if (ret == 1 && !disk->seclabels[0]) { + if (ret == 1 && !disk_seclabel) { /* If we failed to set a label, but virt_use_nfs let us * proceed anyway, then we don't need to relabel later. */ - if (VIR_ALLOC(disk->seclabels[0]) < 0) { + if (VIR_ALLOC(disk_seclabel) < 0) { virReportOOMError(); return -1; } - disk->seclabels[0]->norelabel = true; + disk_seclabel->norelabel = true; ret = 0; } return ret; @@ -807,11 +830,15 @@ SELinuxSetSecurityImageLabel(virSecurityManagerPtr mgr, virDomainDiskDefPtr disk)
{ + bool allowDiskFormatProbing; virSecuritySELinuxCallbackData cbdata; - cbdata.secdef = def->seclabels[0]; cbdata.manager = mgr; + cbdata.secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
- bool allowDiskFormatProbing = virSecurityManagerGetAllowDiskFormatProbing(mgr); + allowDiskFormatProbing = virSecurityManagerGetAllowDiskFormatProbing(mgr); + + if (cbdata.secdef == NULL) + return -1;
if (cbdata.secdef->norelabel) return 0; @@ -838,9 +865,12 @@ static int SELinuxSetSecurityPCILabel(pciDevice *dev ATTRIBUTE_UNUSED, const char *file, void *opaque) { + virSecurityLabelDefPtr secdef; virDomainDefPtr def = opaque; - const virSecurityLabelDefPtr secdef = def->seclabels[0];
+ secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1; return SELinuxSetFilecon(file, secdef->imagelabel); }
@@ -848,8 +878,12 @@ static int SELinuxSetSecurityUSBLabel(usbDevice *dev ATTRIBUTE_UNUSED, const char *file, void *opaque) { + virSecurityLabelDefPtr secdef; virDomainDefPtr def = opaque; - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1;
return SELinuxSetFilecon(file, secdef->imagelabel); } @@ -860,9 +894,13 @@ SELinuxSetSecurityHostdevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainHostdevDefPtr dev)
{ - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; int ret = -1;
+ secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1; + if (secdef->norelabel) return 0;
@@ -929,9 +967,13 @@ SELinuxRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainHostdevDefPtr dev)
{ - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; int ret = -1;
+ secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1; + if (secdef->norelabel) return 0;
@@ -982,10 +1024,14 @@ SELinuxSetSecurityChardevLabel(virDomainDefPtr def, virDomainChrSourceDefPtr dev)
{ - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; char *in = NULL, *out = NULL; int ret = -1;
+ secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1; + if (secdef->norelabel) return 0;
@@ -1028,10 +1074,14 @@ SELinuxRestoreSecurityChardevLabel(virDomainDefPtr def, virDomainChrSourceDefPtr dev)
{ - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; char *in = NULL, *out = NULL; int ret = -1;
+ secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1; + if (secdef->norelabel) return 0;
@@ -1121,12 +1171,16 @@ SELinuxRestoreSecurityAllLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def, int migrated ATTRIBUTE_UNUSED) { - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; int i; int rc = 0;
VIR_DEBUG("Restoring security label on %s", def->name);
+ secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1; + if (secdef->norelabel) return 0;
@@ -1171,7 +1225,11 @@ static int SELinuxReleaseSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def) { - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; + + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1;
if (secdef->type == VIR_DOMAIN_SECLABEL_DYNAMIC) { if (secdef->label != NULL) { @@ -1196,7 +1254,11 @@ SELinuxSetSavedStateLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def, const char *savefile) { - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; + + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1;
if (secdef->norelabel) return 0; @@ -1210,7 +1272,11 @@ SELinuxRestoreSavedStateLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def, const char *savefile) { - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; + + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1;
if (secdef->norelabel) return 0; @@ -1223,7 +1289,12 @@ static int SELinuxSecurityVerify(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def) { - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; + + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1; + if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("security label driver mismatch: " @@ -1248,12 +1319,16 @@ SELinuxSetSecurityProcessLabel(virSecurityManagerPtr mgr, virDomainDefPtr def) { /* TODO: verify DOI */ - const virSecurityLabelDefPtr secdef = def->seclabels[0]; - VIR_DEBUG("SELinuxSetSecurityProcessLabel %s", secdef->label); + virSecurityLabelDefPtr secdef;
- if (def->seclabels[0]->label == NULL) + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1; + + if (secdef->label == NULL) return 0;
+ VIR_DEBUG("SELinuxSetSecurityProcessLabel %s", secdef->label); if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("security label driver mismatch: " @@ -1280,13 +1355,17 @@ SELinuxSetSecurityDaemonSocketLabel(virSecurityManagerPtr mgr, virDomainDefPtr def) { /* TODO: verify DOI */ - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; context_t execcon = NULL; context_t proccon = NULL; security_context_t scon = NULL; int rc = -1;
- if (def->seclabels[0]->label == NULL) + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1; + + if (secdef->label == NULL) return 0;
if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { @@ -1350,9 +1429,13 @@ static int SELinuxSetSecuritySocketLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm) { - const virSecurityLabelDefPtr secdef = vm->seclabels[0]; + virSecurityLabelDefPtr secdef; int rc = -1;
+ secdef = virDomainDefGetSecurityLabelDef(vm, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1; + if (secdef->label == NULL) return 0;
@@ -1388,9 +1471,13 @@ SELinuxClearSecuritySocketLabel(virSecurityManagerPtr mgr, virDomainDefPtr def) { /* TODO: verify DOI */ - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; + + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1;
- if (def->seclabels[0]->label == NULL) + if (secdef->label == NULL) return 0;
if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { @@ -1466,9 +1553,13 @@ SELinuxSetSecurityAllLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, const char *stdin_path) { - virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr); - const virSecurityLabelDefPtr secdef = def->seclabels[0]; int i; + virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr); + virSecurityLabelDefPtr secdef; + + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1;
if (secdef->norelabel) return 0; @@ -1528,7 +1619,11 @@ SELinuxSetImageFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def, int fd) { - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; + + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return -1;
if (secdef->imagelabel == NULL) return 0; @@ -1538,13 +1633,17 @@ SELinuxSetImageFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
static char *genImageLabel(virSecurityManagerPtr mgr, virDomainDefPtr def) { - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr); const char *range; context_t ctx = NULL; char *label = NULL; const char *mcs = NULL;
+ secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + goto cleanup; + if (secdef->label) { ctx = context_new(secdef->label); if (!ctx) { @@ -1575,7 +1674,11 @@ cleanup: static char *SELinuxGetSecurityMountOptions(virSecurityManagerPtr mgr, virDomainDefPtr def) { char *opts = NULL; - const virSecurityLabelDefPtr secdef = def->seclabels[0]; + virSecurityLabelDefPtr secdef; + + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (secdef == NULL) + return NULL;
if (! secdef->imagelabel) secdef->imagelabel = genImageLabel(mgr,def); diff --git a/src/security/security_stack.c b/src/security/security_stack.c index 65d30d6..a7b8741 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -23,29 +23,70 @@ #include "security_stack.h"
#include "virterror_internal.h" +#include "memory.h"
#define VIR_FROM_THIS VIR_FROM_SECURITY
typedef struct _virSecurityStackData virSecurityStackData; typedef virSecurityStackData *virSecurityStackDataPtr; +typedef struct _virSecurityStackItem virSecurityStackItem; +typedef virSecurityStackItem* virSecurityStackItemPtr; + +struct _virSecurityStackItem { + virSecurityManagerPtr securityManager; + virSecurityStackItemPtr next; +};
struct _virSecurityStackData { virSecurityManagerPtr primary; - virSecurityManagerPtr secondary; + virSecurityStackItemPtr itemsHead; };
-void virSecurityStackSetPrimary(virSecurityManagerPtr mgr, - virSecurityManagerPtr primary) +int +virSecurityStackAddPrimary(virSecurityManagerPtr mgr, + virSecurityManagerPtr primary) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + if (virSecurityStackAddNested(mgr, primary) < 0) + return -1; priv->primary = primary; + return 0; +} + +int +virSecurityStackAddNested(virSecurityManagerPtr mgr, + virSecurityManagerPtr nested) +{ + virSecurityStackItemPtr item = NULL; + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + + if (VIR_ALLOC(item) < 0) { + virReportOOMError(); + return -1; + } + item->securityManager = nested; + item->next = priv->itemsHead; + priv->itemsHead = item; + return 0; +} + +virSecurityManagerPtr +virSecurityStackGetPrimary(virSecurityManagerPtr mgr) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + return (priv->primary) ? priv->primary : priv->itemsHead->securityManager; +} + +void virSecurityStackSetPrimary(virSecurityManagerPtr mgr, + virSecurityManagerPtr primary) +{ + virSecurityStackAddPrimary(mgr, primary); }
void virSecurityStackSetSecondary(virSecurityManagerPtr mgr, virSecurityManagerPtr secondary) { - virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); - priv->secondary = secondary; + virSecurityStackAddNested(mgr, secondary); }
static virSecurityDriverStatus @@ -64,9 +105,14 @@ static int virSecurityStackClose(virSecurityManagerPtr mgr) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr next, item = priv->itemsHead;
- virSecurityManagerFree(priv->primary); - virSecurityManagerFree(priv->secondary); + while (item) { + next = item->next; + virSecurityManagerFree(item->securityManager); + VIR_FREE(item); + item = next; + }
return 0; } @@ -74,17 +120,13 @@ virSecurityStackClose(virSecurityManagerPtr mgr) static const char * virSecurityStackGetModel(virSecurityManagerPtr mgr) { - virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); - - return virSecurityManagerGetModel(priv->primary); + return virSecurityManagerGetModel(virSecurityStackGetPrimary(mgr)); }
static const char * virSecurityStackGetDOI(virSecurityManagerPtr mgr) { - virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); - - return virSecurityManagerGetDOI(priv->primary); + return virSecurityManagerGetDOI(virSecurityStackGetPrimary(mgr)); }
static int @@ -92,13 +134,15 @@ virSecurityStackVerify(virSecurityManagerPtr mgr, virDomainDefPtr def) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0;
- if (virSecurityManagerVerify(priv->primary, def) < 0) - rc = -1; - - if (virSecurityManagerVerify(priv->secondary, def) < 0) - rc = -1; + for(; item; item = item->next) { + if (virSecurityManagerVerify(item->securityManager, def) < 0) { + rc = -1; + break; + } + }
return rc; } @@ -108,12 +152,12 @@ static int virSecurityStackGenLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm) { - virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); int rc = 0;
- if (virSecurityManagerGenLabel(priv->primary, vm) < 0) + if (virSecurityManagerGenLabel(virSecurityStackGetPrimary(mgr), vm) < 0) rc = -1;
+// TODO #if 0 /* We don't allow secondary drivers to generate labels. * This may have to change in the future, but requires @@ -133,11 +177,12 @@ static int virSecurityStackReleaseLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm) { - virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); int rc = 0;
- if (virSecurityManagerReleaseLabel(priv->primary, vm) < 0) + if (virSecurityManagerReleaseLabel(virSecurityStackGetPrimary(mgr), vm) < 0) rc = -1; + +// TODO #if 0 /* XXX See note in GenLabel */ if (virSecurityManagerReleaseLabel(priv->secondary, vm) < 0) @@ -153,11 +198,11 @@ virSecurityStackReserveLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, pid_t pid) { - virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); int rc = 0;
- if (virSecurityManagerReserveLabel(priv->primary, vm, pid) < 0) + if (virSecurityManagerReserveLabel(virSecurityStackGetPrimary(mgr), vm, pid) < 0) rc = -1; +// TODO #if 0 /* XXX See note in GenLabel */ if (virSecurityManagerReserveLabel(priv->secondary, vm, pid) < 0) @@ -174,12 +219,13 @@ virSecurityStackSetSecurityImageLabel(virSecurityManagerPtr mgr, virDomainDiskDefPtr disk) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0;
- if (virSecurityManagerSetImageLabel(priv->secondary, vm, disk) < 0) - rc = -1; - if (virSecurityManagerSetImageLabel(priv->primary, vm, disk) < 0) - rc = -1; + for (; item; item = item->next) { + if (virSecurityManagerSetImageLabel(item->securityManager, vm, disk) < 0) + rc = -1; + }
return rc; } @@ -191,12 +237,13 @@ virSecurityStackRestoreSecurityImageLabel(virSecurityManagerPtr mgr, virDomainDiskDefPtr disk) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0;
- if (virSecurityManagerRestoreImageLabel(priv->secondary, vm, disk) < 0) - rc = -1; - if (virSecurityManagerRestoreImageLabel(priv->primary, vm, disk) < 0) - rc = -1; + for (; item; item = item->next) { + if (virSecurityManagerRestoreImageLabel(item->securityManager, vm, disk) < 0) + rc = -1; + }
return rc; } @@ -209,12 +256,13 @@ virSecurityStackSetSecurityHostdevLabel(virSecurityManagerPtr mgr,
{ virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0;
- if (virSecurityManagerSetHostdevLabel(priv->secondary, vm, dev) < 0) - rc = -1; - if (virSecurityManagerSetHostdevLabel(priv->primary, vm, dev) < 0) - rc = -1; + for (; item; item = item->next) { + if (virSecurityManagerSetHostdevLabel(item->securityManager, vm, dev) < 0) + rc = -1; + }
return rc; } @@ -226,12 +274,13 @@ virSecurityStackRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr, virDomainHostdevDefPtr dev) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0;
- if (virSecurityManagerRestoreHostdevLabel(priv->secondary, vm, dev) < 0) - rc = -1; - if (virSecurityManagerRestoreHostdevLabel(priv->primary, vm, dev) < 0) - rc = -1; + for (; item; item = item->next) { + if (virSecurityManagerRestoreHostdevLabel(item->securityManager, vm, dev) < 0) + rc = -1; + }
return rc; } @@ -243,12 +292,13 @@ virSecurityStackSetSecurityAllLabel(virSecurityManagerPtr mgr, const char *stdin_path) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0;
- if (virSecurityManagerSetAllLabel(priv->secondary, vm, stdin_path) < 0) - rc = -1; - if (virSecurityManagerSetAllLabel(priv->primary, vm, stdin_path) < 0) - rc = -1; + for (; item; item = item->next) { + if (virSecurityManagerSetAllLabel(item->securityManager, vm, stdin_path) < 0) + rc = -1; + }
return rc; } @@ -260,12 +310,13 @@ virSecurityStackRestoreSecurityAllLabel(virSecurityManagerPtr mgr, int migrated) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0;
- if (virSecurityManagerRestoreAllLabel(priv->secondary, vm, migrated) < 0) - rc = -1; - if (virSecurityManagerRestoreAllLabel(priv->primary, vm, migrated) < 0) - rc = -1; + for (; item; item = item->next) { + if (virSecurityManagerRestoreAllLabel(item->securityManager, vm, migrated) < 0) + rc = -1; + }
return rc; } @@ -277,12 +328,13 @@ virSecurityStackSetSavedStateLabel(virSecurityManagerPtr mgr, const char *savefile) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0;
- if (virSecurityManagerSetSavedStateLabel(priv->secondary, vm, savefile) < 0) - rc = -1; - if (virSecurityManagerSetSavedStateLabel(priv->primary, vm, savefile) < 0) - rc = -1; + for (; item; item = item->next) { + if (virSecurityManagerSetSavedStateLabel(item->securityManager, vm, savefile) < 0) + rc = -1; + }
return rc; } @@ -294,12 +346,13 @@ virSecurityStackRestoreSavedStateLabel(virSecurityManagerPtr mgr, const char *savefile) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0;
- if (virSecurityManagerRestoreSavedStateLabel(priv->secondary, vm, savefile) < 0) - rc = -1; - if (virSecurityManagerRestoreSavedStateLabel(priv->primary, vm, savefile) < 0) - rc = -1; + for (; item; item = item->next) { + if (virSecurityManagerRestoreSavedStateLabel(item->securityManager, vm, savefile) < 0) + rc = -1; + }
return rc; } @@ -310,12 +363,13 @@ virSecurityStackSetProcessLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0;
- if (virSecurityManagerSetProcessLabel(priv->secondary, vm) < 0) - rc = -1; - if (virSecurityManagerSetProcessLabel(priv->primary, vm) < 0) - rc = -1; + for (; item; item = item->next) { + if (virSecurityManagerSetProcessLabel(item->securityManager, vm) < 0) + rc = -1; + }
return rc; } @@ -326,14 +380,14 @@ virSecurityStackGetProcessLabel(virSecurityManagerPtr mgr, pid_t pid, virSecurityLabelPtr seclabel) { - virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); int rc = 0;
+// TODO #if 0 if (virSecurityManagerGetProcessLabel(priv->secondary, vm, pid, seclabel) < 0) rc = -1; #endif - if (virSecurityManagerGetProcessLabel(priv->primary, vm, pid, seclabel) < 0) + if (virSecurityManagerGetProcessLabel(virSecurityStackGetPrimary(mgr), vm, pid, seclabel) < 0) rc = -1;
return rc; @@ -345,12 +399,13 @@ virSecurityStackSetDaemonSocketLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0;
- if (virSecurityManagerSetDaemonSocketLabel(priv->secondary, vm) < 0) - rc = -1; - if (virSecurityManagerSetDaemonSocketLabel(priv->primary, vm) < 0) - rc = -1; + for (; item; item = item->next) { + if (virSecurityManagerSetDaemonSocketLabel(item->securityManager, vm) < 0) + rc = -1; + }
return rc; } @@ -361,12 +416,13 @@ virSecurityStackSetSocketLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0;
- if (virSecurityManagerSetSocketLabel(priv->secondary, vm) < 0) - rc = -1; - if (virSecurityManagerSetSocketLabel(priv->primary, vm) < 0) - rc = -1; + for (; item; item = item->next) { + if (virSecurityManagerSetSocketLabel(item->securityManager, vm) < 0) + rc = -1; + }
return rc; } @@ -377,12 +433,13 @@ virSecurityStackClearSocketLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0;
- if (virSecurityManagerClearSocketLabel(priv->secondary, vm) < 0) - rc = -1; - if (virSecurityManagerClearSocketLabel(priv->primary, vm) < 0) - rc = -1; + for (; item; item = item->next) { + if (virSecurityManagerClearSocketLabel(item->securityManager, vm) < 0) + rc = -1; + }
return rc; } @@ -393,12 +450,13 @@ virSecurityStackSetImageFDLabel(virSecurityManagerPtr mgr, int fd) { virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; int rc = 0;
- if (virSecurityManagerSetImageFDLabel(priv->secondary, vm, fd) < 0) - rc = -1; - if (virSecurityManagerSetImageFDLabel(priv->primary, vm, fd) < 0) - rc = -1; + for (; item; item = item->next) { + if (virSecurityManagerSetImageFDLabel(item->securityManager, vm, fd) < 0) + rc = -1; + }
return rc; } @@ -408,6 +466,29 @@ static char *virSecurityStackGetMountOptions(virSecurityManagerPtr mgr ATTRIBUTE return NULL; }
+virSecurityManagerPtr* +virSecurityStackGetNested(virSecurityManagerPtr mgr) +{ + virSecurityManagerPtr *list = NULL; + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item; + int len = 0, i = 0; + + for (item = priv->itemsHead; item; item = item->next) + len++; + + if (VIR_ALLOC_N(list, len + 1) < 0) { + virReportOOMError(); + return NULL; + } + + for (item = priv->itemsHead; item; item = item->next, i++) + list[i] = item->securityManager; + list[len] = NULL; + + return list; +} + virSecurityDriver virSecurityDriverStack = { .privateDataLen = sizeof(virSecurityStackData), .name = "stack", diff --git a/src/security/security_stack.h b/src/security/security_stack.h index bc83ff3..13a7a88 100644 --- a/src/security/security_stack.h +++ b/src/security/security_stack.h @@ -25,9 +25,22 @@
extern virSecurityDriver virSecurityDriverStack;
+ +int +virSecurityStackAddPrimary(virSecurityManagerPtr mgr, + virSecurityManagerPtr primary); +int +virSecurityStackAddNested(virSecurityManagerPtr mgr, + virSecurityManagerPtr nested); +virSecurityManagerPtr +virSecurityStackGetPrimary(virSecurityManagerPtr mgr); + void virSecurityStackSetPrimary(virSecurityManagerPtr mgr, virSecurityManagerPtr primary); void virSecurityStackSetSecondary(virSecurityManagerPtr mgr, virSecurityManagerPtr secondary);
+virSecurityManagerPtr* +virSecurityStackGetNested(virSecurityManagerPtr mgr); + #endif /* __VIR_SECURITY_DAC */
I haven't spotted anything wrong other than outpointed. Michal

This patch replaces the key "security_driver" in QEMU config by "security_drivers", which accepts a list of default drivers. If "security_drivers" can't be found, libvirt will use "security_driver" to ensure that it will remain compatible with older version of the config file. --- src/qemu/libvirtd_qemu.aug | 2 +- src/qemu/qemu.conf | 2 +- src/qemu/qemu_conf.c | 42 ++++++++++++++- src/qemu/qemu_conf.h | 2 +- src/qemu/qemu_driver.c | 97 ++++++++++++++++++++++++++++-------- src/qemu/test_libvirtd_qemu.aug.in | 2 +- 6 files changed, 119 insertions(+), 28 deletions(-) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index 683aadb..fab97d7 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -39,7 +39,7 @@ module Libvirtd_qemu = | str_entry "spice_tls_x509_cert_dir" | str_entry "spice_password" - let security_entry = str_entry "security_driver" + let security_entry = str_entry "security_drivers" | bool_entry "security_default_confined" | bool_entry "security_require_confined" | str_entry "user" diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index ed4683c..ffb03f8 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -146,7 +146,7 @@ # leaving SELinux enabled for the host in general, then set this # to 'none' instead. # -#security_driver = "selinux" +#security_drivers = "selinux" # If set to non-zero, then the default security labeling # will make guests confined. If set to zero, then guests diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 88a04bc..6e1c608 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -192,14 +192,50 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, } } - p = virConfGetValue (conf, "security_driver"); - CHECK_TYPE ("security_driver", VIR_CONF_STRING); + p = virConfGetValue (conf, "security_drivers"); + CHECK_TYPE ("security_drivers", VIR_CONF_STRING); if (p && p->str) { - if (!(driver->securityDriverName = strdup(p->str))) { + char *it, *tok; + size_t len; + + for (len = 1, it = p->str; *it; it++) + len++; + if (VIR_ALLOC_N(driver->securityDriverNames, len + 1) < 0) { virReportOOMError(); virConfFree(conf); return -1; } + + i = 0; + tok = it = p->str; + while (1) { + if (*it == ',' || *it == '\0') { + driver->securityDriverNames[i] = strndup(tok, it - tok); + if (driver->securityDriverNames == NULL) { + virReportOOMError(); + virConfFree(conf); + return -1; + } + tok = it + 1; + i++; + } + if (*it == '\0') + break; + it++; + } + } else { + VIR_WARN("'security_drivers' config not found. Trying 'security_driver'"); + p = virConfGetValue (conf, "security_driver"); + CHECK_TYPE ("security_driver", VIR_CONF_STRING); + if (p && p->str) { + if (VIR_ALLOC_N(driver->securityDriverNames, 2) < 0 || + !(driver->securityDriverNames[0] = strdup(p->str))) { + virReportOOMError(); + virConfFree(conf); + return -1; + } + driver->securityDriverNames[1] = NULL; + } } p = virConfGetValue (conf, "security_default_confined"); diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 482e6d3..e808f4f 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -116,7 +116,7 @@ struct qemud_driver { virDomainEventStatePtr domainEventState; - char *securityDriverName; + char **securityDriverNames; bool securityDefaultConfined; bool securityRequireConfined; virSecurityManagerPtr securityManager; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1b02f28..f01566b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -220,36 +220,91 @@ 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; + char *primary; + virSecurityManagerPtr mgr, nested, stack; + if (driver->securityDriverNames == NULL) + primary = NULL; + else + primary = driver->securityDriverNames[0]; + + /* Create primary driver */ + mgr = virSecurityManagerNew(primary, + 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->securityDriverNames && driver->securityDriverNames[1])) { + stack = virSecurityManagerNewStack(mgr); + if (!stack) goto error; + mgr = stack; + } + + /* Loop through additional driver names and add a secudary driver to each + * one */ + if (driver->securityDriverNames) { + names = driver->securityDriverNames + 1; + while (names && *names) { + if (STREQ("dac", *names)) { + /* A DAC driver has specific 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)) || - !(virSecurityManagerStackAddNested(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->securityDriverNames; + while (names && *names) { + if (STREQ("dac", *names)) { + break; + } + names++; + } + /* If there is no 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: diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index 959f250..ff24a38 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -15,7 +15,7 @@ module Test_libvirtd_qemu = { "spice_tls" = "1" } { "spice_tls_x509_cert_dir" = "/etc/pki/libvirt-spice" } { "spice_password" = "XYZ12345" } -{ "security_driver" = "selinux" } +{ "security_drivers" = "selinux" } { "security_default_confined" = "1" } { "security_require_confined" = "1" } { "user" = "root" } -- 1.7.1

On 18.07.2012 03:28, Marcelo Cerri wrote:
This patch replaces the key "security_driver" in QEMU config by "security_drivers", which accepts a list of default drivers. If "security_drivers" can't be found, libvirt will use "security_driver" to ensure that it will remain compatible with older version of the config file. --- src/qemu/libvirtd_qemu.aug | 2 +- src/qemu/qemu.conf | 2 +- src/qemu/qemu_conf.c | 42 ++++++++++++++- src/qemu/qemu_conf.h | 2 +- src/qemu/qemu_driver.c | 97 ++++++++++++++++++++++++++++-------- src/qemu/test_libvirtd_qemu.aug.in | 2 +- 6 files changed, 119 insertions(+), 28 deletions(-)
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index 683aadb..fab97d7 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -39,7 +39,7 @@ module Libvirtd_qemu = | str_entry "spice_tls_x509_cert_dir" | str_entry "spice_password"
- let security_entry = str_entry "security_driver" + let security_entry = str_entry "security_drivers" | bool_entry "security_default_confined" | bool_entry "security_require_confined" | str_entry "user" diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index ed4683c..ffb03f8 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -146,7 +146,7 @@ # leaving SELinux enabled for the host in general, then set this # to 'none' instead. # -#security_driver = "selinux" +#security_drivers = "selinux"
No, we can't do that. Changing this would silently discard any values set by users. Moreover, separating values by comma - - that's yet another way for multiple values passing. Can't we just re-use: cgroup_controllers = [ "cpu", "devices", "memory", "blkio", "cpuset", "cpuacct" ] that is making these two syntactically and semantically correct: security_driver = "selinux" security_driver = [ "selinux", "apparmor", "YetAnotherSecurtyDriver" ] Having said that - the rest of the patch is pointless to review and need rework after we settle down on design. But I'll point out one obvious error anyway.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1b02f28..f01566b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -220,36 +220,91 @@ 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; + char *primary; + virSecurityManagerPtr mgr, nested, stack;
Stack may be use uninitialized ...
+ if (driver->securityDriverNames == NULL) + primary = NULL; + else + primary = driver->securityDriverNames[0]; + + /* Create primary driver */ + mgr = virSecurityManagerNew(primary, + 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->securityDriverNames && driver->securityDriverNames[1])) { + stack = virSecurityManagerNewStack(mgr); + if (!stack) goto error; + mgr = stack; + } + + /* Loop through additional driver names and add a secudary driver to each + * one */ + if (driver->securityDriverNames) { + names = driver->securityDriverNames + 1; + while (names && *names) { + if (STREQ("dac", *names)) { + /* A DAC driver has specific 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))
... here.
+ goto error; + names++; + } + }
Michal

Thist patch updates libvirt's API to allow applications to inspect the full list of security labels of a domain. --- daemon/remote.c | 62 ++++++++++++++++++++++++++++++++++++ include/libvirt/libvirt.h.in | 2 + python/generator.py | 1 + src/driver.h | 4 ++ src/libvirt.c | 47 +++++++++++++++++++++++++++ src/libvirt_public.syms | 7 ++++ src/qemu/qemu_driver.c | 71 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 46 +++++++++++++++++++++++++++ src/remote/remote_protocol.x | 18 +++++++++- src/remote_protocol-structs | 1 + 10 files changed, 257 insertions(+), 2 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 9334221..aef6cc1 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1424,6 +1424,68 @@ cleanup: } static int +remoteDispatchDomainGetSecurityLabelList(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_get_security_label_list_args *args, + remote_domain_get_security_label_list_ret *ret) +{ + virDomainPtr dom = NULL; + virSecurityLabelPtr seclabels = NULL; + int i, len, rv = -1; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(priv->conn, args->dom))) + goto cleanup; + + if ((len = virDomainGetSecurityLabelList(dom, &seclabels)) < 0) { + ret->ret = len; + ret->labels.labels_len = 0; + ret->labels.labels_val = NULL; + goto done; + } + + if (VIR_ALLOC_N(ret->labels.labels_val, len) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0; i < len; i++) { + size_t label_len = strlen(seclabels[i].label) + 1; + remote_domain_get_security_label_ret *cur = &ret->labels.labels_val[i]; + if (VIR_ALLOC_N(cur->label.label_val, label_len) < 0) { + virReportOOMError(); + goto cleanup; + } + if (virStrcpy(cur->label.label_val, seclabels[i].label, label_len) == NULL) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to copy security label")); + goto cleanup; + } + cur->label.label_len = label_len; + cur->enforcing = seclabels[i].enforcing; + } + ret->labels.labels_len = ret->ret = len; + +done: + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + if (dom) + virDomainFree(dom); + VIR_FREE(seclabels); + return rv; +} + +static int remoteDispatchNodeGetSecurityModel(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, virNetMessagePtr msg ATTRIBUTE_UNUSED, diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index e34438c..eb48b99 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1540,6 +1540,8 @@ int virDomainSetMemoryFlags (virDomainPtr domain, int virDomainGetMaxVcpus (virDomainPtr domain); int virDomainGetSecurityLabel (virDomainPtr domain, virSecurityLabelPtr seclabel); +int virDomainGetSecurityLabelList (virDomainPtr domain, + virSecurityLabelPtr* seclabels); typedef enum { VIR_DOMAIN_METADATA_DESCRIPTION = 0, /* Operate on <description> */ diff --git a/python/generator.py b/python/generator.py index 2dada6e..f3c8912 100755 --- a/python/generator.py +++ b/python/generator.py @@ -446,6 +446,7 @@ skip_function = ( 'virConnectOpenAuth', # Python C code is manually written 'virDefaultErrorFunc', # Python virErrorFuncHandler impl calls this from C 'virDomainGetSecurityLabel', # Needs investigation... + 'virDomainGetSecurityLabelList', # Needs investigation... 'virNodeGetSecurityModel', # Needs investigation... 'virConnectDomainEventRegister', # overridden in virConnect.py 'virConnectDomainEventDeregister', # overridden in virConnect.py diff --git a/src/driver.h b/src/driver.h index b3c1740..fd6a1fe 100644 --- a/src/driver.h +++ b/src/driver.h @@ -314,6 +314,9 @@ typedef int (*virDrvDomainGetSecurityLabel) (virDomainPtr domain, virSecurityLabelPtr seclabel); typedef int + (*virDrvDomainGetSecurityLabelList) (virDomainPtr domain, + virSecurityLabelPtr* seclabels); +typedef int (*virDrvNodeGetSecurityModel) (virConnectPtr conn, virSecurityModelPtr secmodel); typedef int @@ -934,6 +937,7 @@ struct _virDriver { virDrvDomainGetVcpus domainGetVcpus; virDrvDomainGetMaxVcpus domainGetMaxVcpus; virDrvDomainGetSecurityLabel domainGetSecurityLabel; + virDrvDomainGetSecurityLabelList domainGetSecurityLabelList; virDrvNodeGetSecurityModel nodeGetSecurityModel; virDrvDomainGetXMLDesc domainGetXMLDesc; virDrvConnectDomainXMLFromNative domainXMLFromNative; diff --git a/src/libvirt.c b/src/libvirt.c index df78e8a..912fd10 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -9008,6 +9008,53 @@ error: } /** + * virDomainGetSecurityLabelList: + * @domain: a domain object + * @seclabels: will be auto-allocated and filled with domains' security labels. + * Caller must free memory on return. + * + * Extract the security labels of an active domain. The 'label' field + * in the @seclabels argument will be initialized to the empty + * string if the domain is not running under a security model. + * + * Returns 0 in case of success, -1 in case of failure + */ +int +virDomainGetSecurityLabelList(virDomainPtr domain, + virSecurityLabelPtr* seclabels) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "seclabels=%p", seclabels); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (seclabels == NULL) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + + conn = domain->conn; + + if (conn->driver->domainGetSecurityLabelList) { + int ret; + ret = conn->driver->domainGetSecurityLabelList(domain, seclabels); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return -1; +} +/** * virDomainSetMetadata: * @domain: a domain object * @type: type of description, from virDomainMetadataType diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 2913a81..21b83e7 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -542,6 +542,13 @@ LIBVIRT_0.9.13 { virDomainSnapshotIsCurrent; virDomainSnapshotListAllChildren; virDomainSnapshotRef; + virDomainGetSecurityLabelList; } LIBVIRT_0.9.11; +LIBVIRT_0.9.14 { + global: + virDomainGetSecurityLabelList; +} LIBVIRT_0.9.13; + + # .... define new API here using predicted next version number .... diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f01566b..06a4e44 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4091,6 +4091,76 @@ cleanup: return ret; } +static int qemudDomainGetSecurityLabelList(virDomainPtr dom, + virSecurityLabelPtr* seclabels) +{ + struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; + virDomainObjPtr vm; + int i, ret = -1; + + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + + 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)) { + /* No seclabels */ + *seclabels = NULL; + ret = 0; + } else { + int len = 0; + virSecurityManagerPtr* mgrs = virSecurityManagerGetNested( + driver->securityManager); + if (!mgrs) + goto cleanup; + + /* Allocate seclabels array */ + for (i = 0; mgrs[i]; i++) + len++; + + if (VIR_ALLOC_N((*seclabels), len) < 0) { + virReportOOMError(); + goto cleanup; + } + memset(*seclabels, 0, sizeof(**seclabels) * len); + + /* Fill the array */ + for (i = 0; i < len; i++) { + if (virSecurityManagerGetProcessLabel(mgrs[i], vm->def, vm->pid, + &(*seclabels)[i]) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Failed to get security label")); + VIR_FREE(mgrs); + VIR_FREE(*seclabels); + goto cleanup; + } + } + ret = len; + VIR_FREE(mgrs); + } + +cleanup: + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return ret; +} static int qemudNodeGetSecurityModel(virConnectPtr conn, virSecurityModelPtr secmodel) { @@ -13296,6 +13366,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/remote/remote_driver.c b/src/remote/remote_driver.c index 3314f80..3c08ae9 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1929,6 +1929,51 @@ done: } static int +remoteDomainGetSecurityLabelList (virDomainPtr domain, virSecurityLabelPtr* seclabels) +{ + remote_domain_get_security_label_list_args args; + remote_domain_get_security_label_list_ret ret; + struct private_data *priv = domain->conn->privateData; + int i, rv = -1; + + remoteDriverLock(priv); + + make_nonnull_domain (&args.dom, domain); + memset(&ret, 0, sizeof(ret)); + + if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_SECURITY_LABEL_LIST, + (xdrproc_t) xdr_remote_domain_get_security_label_list_args, (char *)&args, + (xdrproc_t) xdr_remote_domain_get_security_label_list_ret, (char *)&ret) == -1) { + goto done; + } + + if (VIR_ALLOC_N(*seclabels, ret.labels.labels_len) < 0) + goto cleanup; + + for (i = 0; i < ret.labels.labels_len; i++) { + remote_domain_get_security_label_ret *cur = &ret.labels.labels_val[i]; + if (cur->label.label_val != NULL) { + if (strlen(cur->label.label_val) >= sizeof((*seclabels)->label)) { + remoteError(VIR_ERR_RPC, _("security label exceeds maximum: %zd"), + sizeof((*seclabels)->label) - 1); + VIR_FREE(*seclabels); + goto cleanup; + } + strcpy((*seclabels)[i].label, cur->label.label_val); + (*seclabels)[i].enforcing = cur->enforcing; + } + } + rv = ret.ret; + +cleanup: + xdr_free((xdrproc_t) xdr_remote_domain_get_security_label_list_ret, (char *)&ret); + +done: + remoteDriverUnlock(priv); + return rv; +} + +static int remoteDomainGetState(virDomainPtr domain, int *state, int *reason, @@ -5226,6 +5271,7 @@ static virDriver remote_driver = { .domainGetVcpus = remoteDomainGetVcpus, /* 0.3.0 */ .domainGetMaxVcpus = remoteDomainGetMaxVcpus, /* 0.3.0 */ .domainGetSecurityLabel = remoteDomainGetSecurityLabel, /* 0.6.1 */ + .domainGetSecurityLabelList = remoteDomainGetSecurityLabelList, /* 0.9.13 */ .nodeGetSecurityModel = remoteNodeGetSecurityModel, /* 0.6.1 */ .domainGetXMLDesc = remoteDomainGetXMLDesc, /* 0.3.0 */ .domainXMLFromNative = remoteDomainXMLFromNative, /* 0.6.4 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 8f1d9b5..27d34b1 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -169,6 +169,11 @@ const REMOTE_DOMAIN_BLOCK_PEEK_BUFFER_MAX = 1048576; const REMOTE_DOMAIN_MEMORY_PEEK_BUFFER_MAX = 1048576; /* + * Maximum length of a security label list. + */ +const REMOTE_SECURITY_LABEL_LIST_MAX=64; + +/* * Maximum length of a security model field. */ const REMOTE_SECURITY_MODEL_MAX = VIR_SECURITY_MODEL_BUFLEN; @@ -1082,6 +1087,15 @@ struct remote_domain_get_security_label_ret { int enforcing; }; +struct remote_domain_get_security_label_list_args { + remote_nonnull_domain dom; +}; + +struct remote_domain_get_security_label_list_ret { + remote_domain_get_security_label_ret labels<REMOTE_SECURITY_LABEL_LIST_MAX>; + int ret; +}; + struct remote_node_get_security_model_ret { char model<REMOTE_SECURITY_MODEL_MAX>; char doi<REMOTE_SECURITY_DOI_MAX>; @@ -2844,8 +2858,8 @@ enum remote_procedure { REMOTE_PROC_CONNECT_LIST_ALL_DOMAINS = 273, /* skipgen skipgen priority:high */ REMOTE_PROC_DOMAIN_LIST_ALL_SNAPSHOTS = 274, /* skipgen skipgen priority:high */ REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_ALL_CHILDREN = 275, /* skipgen skipgen priority:high */ - REMOTE_PROC_DOMAIN_EVENT_BALLOON_CHANGE = 276 /* autogen autogen */ - + REMOTE_PROC_DOMAIN_EVENT_BALLOON_CHANGE = 276, /* autogen autogen */ + REMOTE_PROC_DOMAIN_GET_SECURITY_LABEL_LIST = 277 /* skipgen skipgen priority:high */ /* * Notice how the entries are grouped in sets of 10 ? * Nice isn't it. Please keep it this way when adding more. diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 511284c..fc7ba73 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2251,4 +2251,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_LIST_ALL_SNAPSHOTS = 274, REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_ALL_CHILDREN = 275, REMOTE_PROC_DOMAIN_EVENT_BALLOON_CHANGE = 276, + REMOTE_PROC_DOMAIN_GET_SECURITY_LABEL_list = 277, }; -- 1.7.1

re: $SUBJ well, you are introducing whole new API, so it is not just a remote :) And maybe it's worth splitting into separate commits as advised here: http://libvirt.org/api_extension.html But from time to time we accept new API completely introduced, documented and implemented in one patch. So I am cointinuing On 18.07.2012 03:28, Marcelo Cerri wrote:
Thist patch updates libvirt's API to allow applications to inspect the full list of security labels of a domain. --- daemon/remote.c | 62 ++++++++++++++++++++++++++++++++++++ include/libvirt/libvirt.h.in | 2 + python/generator.py | 1 + src/driver.h | 4 ++ src/libvirt.c | 47 +++++++++++++++++++++++++++ src/libvirt_public.syms | 7 ++++ src/qemu/qemu_driver.c | 71 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 46 +++++++++++++++++++++++++++ src/remote/remote_protocol.x | 18 +++++++++- src/remote_protocol-structs | 1 + 10 files changed, 257 insertions(+), 2 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c index 9334221..aef6cc1 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1424,6 +1424,68 @@ cleanup: }
static int +remoteDispatchDomainGetSecurityLabelList(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_get_security_label_list_args *args, + remote_domain_get_security_label_list_ret *ret) +{ + virDomainPtr dom = NULL; + virSecurityLabelPtr seclabels = NULL; + int i, len, rv = -1; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(priv->conn, args->dom))) + goto cleanup; + + if ((len = virDomainGetSecurityLabelList(dom, &seclabels)) < 0) { + ret->ret = len; + ret->labels.labels_len = 0; + ret->labels.labels_val = NULL; + goto done; + } + + if (VIR_ALLOC_N(ret->labels.labels_val, len) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0; i < len; i++) { + size_t label_len = strlen(seclabels[i].label) + 1; + remote_domain_get_security_label_ret *cur = &ret->labels.labels_val[i]; + if (VIR_ALLOC_N(cur->label.label_val, label_len) < 0) { + virReportOOMError(); + goto cleanup; + } + if (virStrcpy(cur->label.label_val, seclabels[i].label, label_len) == NULL) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to copy security label")); + goto cleanup; + } + cur->label.label_len = label_len; + cur->enforcing = seclabels[i].enforcing; + } + ret->labels.labels_len = ret->ret = len; + +done: + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + if (dom) + virDomainFree(dom); + VIR_FREE(seclabels); + return rv; +}
Side note - since we have some APIs malloc()-ating on client side it might be worth learning RPC generator to deal with them so we don't have to implement them by hand.
+ +static int remoteDispatchNodeGetSecurityModel(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, virNetMessagePtr msg ATTRIBUTE_UNUSED, diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index e34438c..eb48b99 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1540,6 +1540,8 @@ int virDomainSetMemoryFlags (virDomainPtr domain, int virDomainGetMaxVcpus (virDomainPtr domain); int virDomainGetSecurityLabel (virDomainPtr domain, virSecurityLabelPtr seclabel); +int virDomainGetSecurityLabelList (virDomainPtr domain, + virSecurityLabelPtr* seclabels);
typedef enum { VIR_DOMAIN_METADATA_DESCRIPTION = 0, /* Operate on <description> */ diff --git a/python/generator.py b/python/generator.py index 2dada6e..f3c8912 100755 --- a/python/generator.py +++ b/python/generator.py @@ -446,6 +446,7 @@ skip_function = ( 'virConnectOpenAuth', # Python C code is manually written 'virDefaultErrorFunc', # Python virErrorFuncHandler impl calls this from C 'virDomainGetSecurityLabel', # Needs investigation... + 'virDomainGetSecurityLabelList', # Needs investigation... 'virNodeGetSecurityModel', # Needs investigation... 'virConnectDomainEventRegister', # overridden in virConnect.py 'virConnectDomainEventDeregister', # overridden in virConnect.py diff --git a/src/driver.h b/src/driver.h index b3c1740..fd6a1fe 100644 --- a/src/driver.h +++ b/src/driver.h @@ -314,6 +314,9 @@ typedef int (*virDrvDomainGetSecurityLabel) (virDomainPtr domain, virSecurityLabelPtr seclabel); typedef int + (*virDrvDomainGetSecurityLabelList) (virDomainPtr domain, + virSecurityLabelPtr* seclabels); +typedef int (*virDrvNodeGetSecurityModel) (virConnectPtr conn, virSecurityModelPtr secmodel); typedef int @@ -934,6 +937,7 @@ struct _virDriver { virDrvDomainGetVcpus domainGetVcpus; virDrvDomainGetMaxVcpus domainGetMaxVcpus; virDrvDomainGetSecurityLabel domainGetSecurityLabel; + virDrvDomainGetSecurityLabelList domainGetSecurityLabelList; virDrvNodeGetSecurityModel nodeGetSecurityModel; virDrvDomainGetXMLDesc domainGetXMLDesc; virDrvConnectDomainXMLFromNative domainXMLFromNative; diff --git a/src/libvirt.c b/src/libvirt.c index df78e8a..912fd10 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -9008,6 +9008,53 @@ error: }
/** + * virDomainGetSecurityLabelList: + * @domain: a domain object + * @seclabels: will be auto-allocated and filled with domains' security labels. + * Caller must free memory on return. + * + * Extract the security labels of an active domain. The 'label' field + * in the @seclabels argument will be initialized to the empty + * string if the domain is not running under a security model. + * + * Returns 0 in case of success, -1 in case of failure + */ +int +virDomainGetSecurityLabelList(virDomainPtr domain, + virSecurityLabelPtr* seclabels) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "seclabels=%p", seclabels); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (seclabels == NULL) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + + conn = domain->conn; + + if (conn->driver->domainGetSecurityLabelList) { + int ret; + ret = conn->driver->domainGetSecurityLabelList(domain, seclabels); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return -1; +} +/** * virDomainSetMetadata: * @domain: a domain object * @type: type of description, from virDomainMetadataType diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 2913a81..21b83e7 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -542,6 +542,13 @@ LIBVIRT_0.9.13 { virDomainSnapshotIsCurrent; virDomainSnapshotListAllChildren; virDomainSnapshotRef; + virDomainGetSecurityLabelList;
No! This API was N/A in 0.9.13 release.
} LIBVIRT_0.9.11;
+LIBVIRT_0.9.14 {
I guess Eric mentioned on the list that the next release is going to be 0.10.0 since we are adding support for new hypervisor. But you are not the first to mention 0.9.14 so we can substitute all occurrences just before release.
+ global: + virDomainGetSecurityLabelList; +} LIBVIRT_0.9.13; + + # .... define new API here using predicted next version number .... diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f01566b..06a4e44 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4091,6 +4091,76 @@ cleanup: return ret; }
+static int qemudDomainGetSecurityLabelList(virDomainPtr dom, + virSecurityLabelPtr* seclabels)
qemud prefix is ancient and kept in some internal APIs for historical reasons (when libvirtd wasn't called libvirtd but qemud). Not a show stopper, though.
+{ + struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData;
Typecasting is not necessary.
+ virDomainObjPtr vm; + int i, ret = -1; + + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + + 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)) { + /* No seclabels */ + *seclabels = NULL; + ret = 0; + } else { + int len = 0; + virSecurityManagerPtr* mgrs = virSecurityManagerGetNested( + driver->securityManager);
I wonder if we need to keep qemu driver locked through whole API even if we are dereferencing securityManager (which, however, doesn't change over time).
+ if (!mgrs) + goto cleanup; + + /* Allocate seclabels array */ + for (i = 0; mgrs[i]; i++) + len++; + + if (VIR_ALLOC_N((*seclabels), len) < 0) {
mgrs just leaked here
+ virReportOOMError(); + goto cleanup; + } + memset(*seclabels, 0, sizeof(**seclabels) * len); + + /* Fill the array */ + for (i = 0; i < len; i++) { + if (virSecurityManagerGetProcessLabel(mgrs[i], vm->def, vm->pid, + &(*seclabels)[i]) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Failed to get security label")); + VIR_FREE(mgrs); + VIR_FREE(*seclabels); + goto cleanup; + } + } + ret = len; + VIR_FREE(mgrs); + } + +cleanup: + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return ret; +} static int qemudNodeGetSecurityModel(virConnectPtr conn, virSecurityModelPtr secmodel) { @@ -13296,6 +13366,7 @@ static virDriver qemuDriver = { .domainGetVcpus = qemudDomainGetVcpus, /* 0.4.4 */ .domainGetMaxVcpus = qemudDomainGetMaxVcpus, /* 0.4.4 */ .domainGetSecurityLabel = qemudDomainGetSecurityLabel, /* 0.6.1 */ + .domainGetSecurityLabelList = qemudDomainGetSecurityLabelList, /* ? */
s/\?/0\.9\.14/
.nodeGetSecurityModel = qemudNodeGetSecurityModel, /* 0.6.1 */ .domainGetXMLDesc = qemuDomainGetXMLDesc, /* 0.2.0 */ .domainXMLFromNative = qemuDomainXMLFromNative, /* 0.6.4 */ diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 3314f80..3c08ae9 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1929,6 +1929,51 @@ done: }
static int +remoteDomainGetSecurityLabelList (virDomainPtr domain, virSecurityLabelPtr* seclabels) +{ + remote_domain_get_security_label_list_args args; + remote_domain_get_security_label_list_ret ret; + struct private_data *priv = domain->conn->privateData; + int i, rv = -1; + + remoteDriverLock(priv); + + make_nonnull_domain (&args.dom, domain); + memset(&ret, 0, sizeof(ret)); + + if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_SECURITY_LABEL_LIST, + (xdrproc_t) xdr_remote_domain_get_security_label_list_args, (char *)&args, + (xdrproc_t) xdr_remote_domain_get_security_label_list_ret, (char *)&ret) == -1) { + goto done; + } + + if (VIR_ALLOC_N(*seclabels, ret.labels.labels_len) < 0) + goto cleanup; + + for (i = 0; i < ret.labels.labels_len; i++) { + remote_domain_get_security_label_ret *cur = &ret.labels.labels_val[i]; + if (cur->label.label_val != NULL) { + if (strlen(cur->label.label_val) >= sizeof((*seclabels)->label)) { + remoteError(VIR_ERR_RPC, _("security label exceeds maximum: %zd"), + sizeof((*seclabels)->label) - 1); + VIR_FREE(*seclabels); + goto cleanup; + } + strcpy((*seclabels)[i].label, cur->label.label_val); + (*seclabels)[i].enforcing = cur->enforcing; + } + } + rv = ret.ret; + +cleanup: + xdr_free((xdrproc_t) xdr_remote_domain_get_security_label_list_ret, (char *)&ret); + +done: + remoteDriverUnlock(priv); + return rv; +} + +static int remoteDomainGetState(virDomainPtr domain, int *state, int *reason, @@ -5226,6 +5271,7 @@ static virDriver remote_driver = { .domainGetVcpus = remoteDomainGetVcpus, /* 0.3.0 */ .domainGetMaxVcpus = remoteDomainGetMaxVcpus, /* 0.3.0 */ .domainGetSecurityLabel = remoteDomainGetSecurityLabel, /* 0.6.1 */ + .domainGetSecurityLabelList = remoteDomainGetSecurityLabelList, /* 0.9.13 */
s/13/14/
.nodeGetSecurityModel = remoteNodeGetSecurityModel, /* 0.6.1 */ .domainGetXMLDesc = remoteDomainGetXMLDesc, /* 0.3.0 */ .domainXMLFromNative = remoteDomainXMLFromNative, /* 0.6.4 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 8f1d9b5..27d34b1 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -169,6 +169,11 @@ const REMOTE_DOMAIN_BLOCK_PEEK_BUFFER_MAX = 1048576; const REMOTE_DOMAIN_MEMORY_PEEK_BUFFER_MAX = 1048576;
/* + * Maximum length of a security label list. + */ +const REMOTE_SECURITY_LABEL_LIST_MAX=64; + +/* * Maximum length of a security model field. */ const REMOTE_SECURITY_MODEL_MAX = VIR_SECURITY_MODEL_BUFLEN; @@ -1082,6 +1087,15 @@ struct remote_domain_get_security_label_ret { int enforcing; };
+struct remote_domain_get_security_label_list_args { + remote_nonnull_domain dom; +}; + +struct remote_domain_get_security_label_list_ret { + remote_domain_get_security_label_ret labels<REMOTE_SECURITY_LABEL_LIST_MAX>; + int ret; +}; + struct remote_node_get_security_model_ret { char model<REMOTE_SECURITY_MODEL_MAX>; char doi<REMOTE_SECURITY_DOI_MAX>; @@ -2844,8 +2858,8 @@ enum remote_procedure { REMOTE_PROC_CONNECT_LIST_ALL_DOMAINS = 273, /* skipgen skipgen priority:high */ REMOTE_PROC_DOMAIN_LIST_ALL_SNAPSHOTS = 274, /* skipgen skipgen priority:high */ REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_ALL_CHILDREN = 275, /* skipgen skipgen priority:high */ - REMOTE_PROC_DOMAIN_EVENT_BALLOON_CHANGE = 276 /* autogen autogen */ - + REMOTE_PROC_DOMAIN_EVENT_BALLOON_CHANGE = 276, /* autogen autogen */ + REMOTE_PROC_DOMAIN_GET_SECURITY_LABEL_LIST = 277 /* skipgen skipgen priority:high */ /* * Notice how the entries are grouped in sets of 10 ? * Nice isn't it. Please keep it this way when adding more. diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 511284c..fc7ba73 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2251,4 +2251,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_LIST_ALL_SNAPSHOTS = 274, REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_ALL_CHILDREN = 275, REMOTE_PROC_DOMAIN_EVENT_BALLOON_CHANGE = 276, + REMOTE_PROC_DOMAIN_GET_SECURITY_LABEL_list = 277,
s/list/LIST/
};
participants (4)
-
Daniel P. Berrange
-
Eric Blake
-
Marcelo Cerri
-
Michal Privoznik