Re: [libvirt] [PATCH 1/7] Update internal data structures to support multiple security label definitions

--- src/conf/capabilities.c | 17 ++- src/conf/capabilities.h | 6 +- src/conf/domain_audit.c | 14 ++- src/conf/domain_conf.c | 310 +++++++++++++++++++++++++++++++++++------------ src/conf/domain_conf.h | 18 +++- 5 files changed, 272 insertions(+), 93 deletions(-)

diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 542bf03..7e786c5 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 421030d..9a0092d 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 653657b..ac296d5 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -619,6 +619,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))) { @@ -631,11 +632,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 9def71d..91ffb6f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -852,12 +852,13 @@ 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); + VIR_FREE(def); } @@ -866,6 +867,7 @@ virSecurityDeviceLabelDefFree(virSecurityDeviceLabelDefPtr def) { if (!def) return; + VIR_FREE(def->model); VIR_FREE(def->label); VIR_FREE(def); } @@ -951,7 +953,9 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def) virStorageEncryptionFree(def->encryption); virDomainDeviceInfoClear(&def->info); - virSecurityDeviceLabelDefFree(def->seclabel); + 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]); @@ -1617,7 +1621,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); @@ -3071,10 +3077,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, { char *p;

- if (virXPathNode("./seclabel", ctxt) == NULL) - return 0; - - p = virXPathStringLimit("string(./seclabel/@type)", + p = virXPathStringLimit("string(./@type)", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p == NULL) { def->type = VIR_DOMAIN_SECLABEL_DYNAMIC; @@ -3088,7 +3091,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, } } - p = virXPathStringLimit("string(./seclabel/@relabel)", + p = virXPathStringLimit("string(./@relabel)", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p != NULL) { if (STREQ(p, "yes")) { @@ -3105,13 +3108,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 { @@ -3128,7 +3133,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(./label[1])", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p == NULL) { virDomainReportError(VIR_ERR_XML_ERROR, @@ -3143,7 +3148,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(./imagelabel[1])", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p == NULL) { virDomainReportError(VIR_ERR_XML_ERROR, @@ -3155,93 +3160,166 @@ 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(./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/@model)", - VIR_SECURITY_MODEL_BUFLEN-1, ctxt); - if (p == NULL) { - virDomainReportError(VIR_ERR_XML_ERROR, - "%s", _("missing security model")); - goto error; - } - def->model = p; + /* TODO: check */ + /* Always parse model */ + p = virXPathStringLimit("string(./@model)", + VIR_SECURITY_MODEL_BUFLEN-1, ctxt); + if (p == NULL) { + virDomainReportError(VIR_ERR_XML_ERROR, + "%s", _("missing security model")); + goto error; } + def->model = p; return 0; error: - virSecurityLabelDefClear(def); return -1; } - 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", 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; } + for (i = 0; i < n; i++) { + if (VIR_ALLOC(def->seclabels[i]) < 0) { + virReportOOMError(); + goto error; + } + }

+ + /* Parse each "seclabel" tag */ + for (i = 0; i < n; i++) { + ctxt->node = list[i]; + if (virSecurityLabelDefParseXML(def->seclabels[i], ctxt, flags))

+ goto error; + } + def->nseclabels = n; + ctxt->node = saved_node; + return 0; - if (VIR_ALLOC(*def) < 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/@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) { + // TODO primary ? + // vmDef = ? } 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; + } + } + VIR_FREE(model); } - VIR_FREE(p); - } else { - (*def)->norelabel = false; - } - p = virXPathStringLimit("string(./seclabel/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(./seclabel/label[1])", + 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; } @@ -3325,7 +3403,8 @@ virDomainDiskDefParseXML(virCapsPtr caps, xmlNodePtr node, xmlXPathContextPtr ctxt, virBitmapPtr bootMap, - virSecurityLabelDefPtr vmSeclabel, + virSecurityLabelDefPtr* vmSeclabels, + int nvmSeclabels, unsigned int flags) { virDomainDiskDefPtr def; @@ -3663,9 +3742,8 @@ virDomainDiskDefParseXML(virCapsPtr caps, if (sourceNode) { xmlNodePtr saved_node = ctxt->node; ctxt->node = sourceNode; - if (virSecurityDeviceLabelDefParseXML(&def->seclabel, - vmSeclabel, - ctxt) < 0) + if (virSecurityDeviceLabelDefParseXML(def, vmSeclabels, + nvmSeclabels, ctxt) < 0) goto error; ctxt->node = saved_node; } @@ -7011,7 +7089,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->seclabel, flags))) + NULL, def->seclabels, + def->nseclabels, + flags))) goto error; } else if (xmlStrEqual(node->name, BAD_CAST "lease")) { dev->type = VIR_DOMAIN_DEVICE_LEASE; @@ -7939,7 +8019,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 (virSecurityLabelDefParseXML(&def->seclabel, ctxt, flags) == -1) + if (virSecurityLabelDefsParseXML(def, ctxt, flags) == -1) goto error; /* Extract domain memory */ @@ -8538,7 +8618,8 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, nodes[i], ctxt, bootMap, - &def->seclabel, + def->seclabels, + def->nseclabels, flags); if (!disk) goto error; @@ -10843,6 +10924,8 @@ virSecurityLabelDefFormat(virBufferPtr buf, virSecurityLabelDefPtr def) virBufferAsprintf(buf, " relabel='%s'", def->norelabel ? "no" : "yes"); +VIR_DEBUG("FMT %s: %s %s %s", def->model, def->label, def->imagelabel, def->baselabel); // TODO remove + if (def->label || def->imagelabel || def->baselabel) { virBufferAddLit(buf, ">\n"); @@ -10911,6 +10994,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) { @@ -11006,10 +11090,11 @@ virDomainDiskDefFormat(virBufferPtr buf, if (def->startupPolicy) virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); - if (def->seclabel) { + if (def->nseclabels) { virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 8); - virSecurityDeviceLabelDefFormat(buf, def->seclabel); + for (n = 0; n < def->nseclabels; n++) + virSecurityDeviceLabelDefFormat(buf, def->seclabels[n]); virBufferAdjustIndent(buf, -8); virBufferAddLit(buf, " </source>\n"); } else { @@ -11019,10 +11104,11 @@ virDomainDiskDefFormat(virBufferPtr buf, case VIR_DOMAIN_DISK_TYPE_BLOCK: virBufferEscapeString(buf, " <source dev='%s'", def->src); - if (def->seclabel) { + if (def->nseclabels) { virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 8); - virSecurityDeviceLabelDefFormat(buf, def->seclabel); + for (n = 0; n < def->nseclabels; n++) + virSecurityDeviceLabelDefFormat(buf, def->seclabels[n]); virBufferAdjustIndent(buf, -8); virBufferAddLit(buf, " </source>\n"); } else { @@ -13020,7 +13106,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAddLit(buf, " </devices>\n"); virBufferAdjustIndent(buf, 2); - virSecurityLabelDefFormat(buf, &def->seclabel); + for (n = 0; n < def->nseclabels; n++) + virSecurityLabelDefFormat(buf, def->seclabels[n]); virBufferAdjustIndent(buf, -2); if (def->namespaceData && def->ns.format) { @@ -15217,3 +15304,66 @@ 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 4c56902..49a7120 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -309,6 +309,7 @@ struct _virSecurityLabelDef { typedef struct _virSecurityDeviceLabelDef virSecurityDeviceLabelDef; typedef virSecurityDeviceLabelDef *virSecurityDeviceLabelDefPtr; struct _virSecurityDeviceLabelDef { + char *model; char *label; /* image label string */ bool norelabel; }; @@ -553,7 +554,6 @@ struct _virDomainDiskDef { int device; int bus; char *src; - virSecurityDeviceLabelDefPtr seclabel;