From: "Daniel P. Berrange" <berrange@redhat.com> Revert parsing changes: commit 302fe95ffa1bc5f1c61c0beb31a1adfbc38c668e Author: Eric Blake <eblake@redhat.com> Date: Wed Jan 4 16:01:24 2012 -0700 seclabel: fix regression in libvirtd restart commit b43432931aef92325920953ff92beabfbe5224c8 Author: Eric Blake <eblake@redhat.com> Date: Thu Dec 22 17:47:50 2011 -0700 seclabel: allow a seclabel override on a disk src These two commits changed the sec label parsing code so that the same code dealt with both the VM level sec label, and the per device label. Unfortunately, as we add more options to the VM level sec label, the logic required to use the same parsing code for the per device label becomes unintelligible. * src/conf/domain_conf.c: Remove support for parsing per device sec labels --- src/conf/domain_conf.c | 190 ++++++++++++----------------------------------- 1 files changed, 49 insertions(+), 141 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e872396..6c67cd8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1,7 +1,7 @@ /* * domain_conf.c: domain XML processing * - * Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (C) 2006-2011 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -809,15 +809,6 @@ virSecurityLabelDefClear(virSecurityLabelDefPtr def) VIR_FREE(def->baselabel); } -static void -virSecurityLabelDefFree(virSecurityLabelDefPtr def) -{ - if (!def) - return; - virSecurityLabelDefClear(def); - VIR_FREE(def); -} - void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def) { int ii; @@ -887,7 +878,6 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def) VIR_FREE(def->serial); VIR_FREE(def->src); - virSecurityLabelDefFree(def->seclabel); VIR_FREE(def->dst); VIR_FREE(def->driverName); VIR_FREE(def->driverType); @@ -2508,33 +2498,31 @@ virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def) return 0; } -/* Parse the portion of a SecurityLabel that is common to both the - * top-level <seclabel> and to a per-device override. - * default_seclabel is NULL for top-level, or points to the top-level - * when parsing an override. */ static int -virSecurityLabelDefParseXMLHelper(virSecurityLabelDefPtr def, - xmlNodePtr node, - xmlXPathContextPtr ctxt, - virSecurityLabelDefPtr default_seclabel, - unsigned int flags) +virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, + xmlXPathContextPtr ctxt, + unsigned int flags) { char *p; - xmlNodePtr save_ctxt = ctxt->node; - int ret = -1; - int type = default_seclabel ? default_seclabel->type : def->type; - ctxt->node = node; + if (virXPathNode("./seclabel", ctxt) == NULL) + return 0; - /* Can't use overrides if top-level doesn't allow relabeling. */ - if (default_seclabel && default_seclabel->norelabel) { - virDomainReportError(VIR_ERR_XML_ERROR, "%s", - _("label overrides require relabeling to be " - "enabled at the domain level")); - goto cleanup; + p = virXPathStringLimit("string(./seclabel/@type)", + VIR_SECURITY_LABEL_BUFLEN-1, ctxt); + if (p == NULL) { + virDomainReportError(VIR_ERR_XML_ERROR, + "%s", _("missing security type")); + goto error; } - - p = virXPathStringLimit("string(./@relabel)", + def->type = virDomainSeclabelTypeFromString(p); + VIR_FREE(p); + if (def->type < 0) { + virDomainReportError(VIR_ERR_XML_ERROR, + "%s", _("invalid security type")); + goto error; + } + p = virXPathStringLimit("string(./seclabel/@relabel)", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p != NULL) { if (STREQ(p, "yes")) { @@ -2545,77 +2533,38 @@ virSecurityLabelDefParseXMLHelper(virSecurityLabelDefPtr def, virDomainReportError(VIR_ERR_XML_ERROR, _("invalid security relabel value %s"), p); VIR_FREE(p); - goto cleanup; + goto error; } VIR_FREE(p); - if (!default_seclabel && - type == VIR_DOMAIN_SECLABEL_DYNAMIC && + if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC && def->norelabel) { - virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("dynamic label type must use resource " - "relabeling")); - goto cleanup; + virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("dynamic label type must use resource relabeling")); + goto error; } } else { - if (!default_seclabel && type == VIR_DOMAIN_SECLABEL_STATIC) + if (def->type == VIR_DOMAIN_SECLABEL_STATIC) def->norelabel = true; else def->norelabel = false; } /* Only parse label, if using static labels, or - * if the 'live' VM XML is requested, or if this is a device override + * if the 'live' VM XML is requested */ - if (type == VIR_DOMAIN_SECLABEL_STATIC || - !(flags & VIR_DOMAIN_XML_INACTIVE) || - (default_seclabel && !def->norelabel)) { - p = virXPathStringLimit("string(./label[1])", + if (def->type == VIR_DOMAIN_SECLABEL_STATIC || + !(flags & VIR_DOMAIN_XML_INACTIVE)) { + p = virXPathStringLimit("string(./seclabel/label[1])", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); - if (p == NULL && !(default_seclabel && def->norelabel)) { + if (p == NULL) { virDomainReportError(VIR_ERR_XML_ERROR, "%s", _("security label is missing")); - goto cleanup; + goto error; } def->label = p; } - ret = 0; -cleanup: - ctxt->node = save_ctxt; - return ret; -} - -/* Parse the top-level <seclabel>, if present. */ -static int -virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, - xmlXPathContextPtr ctxt, - unsigned int flags) -{ - char *p; - xmlNodePtr node = virXPathNode("./seclabel", ctxt); - - if (node == NULL) - return 0; - - p = virXPathStringLimit("string(./seclabel/@type)", - VIR_SECURITY_LABEL_BUFLEN-1, ctxt); - if (p == NULL) { - virDomainReportError(VIR_ERR_XML_ERROR, - "%s", _("missing security type")); - goto error; - } - def->type = virDomainSeclabelTypeFromString(p); - VIR_FREE(p); - if (def->type < 0) { - virDomainReportError(VIR_ERR_XML_ERROR, - "%s", _("invalid security type")); - goto error; - } - - if (virSecurityLabelDefParseXMLHelper(def, node, ctxt, NULL, flags) < 0) - goto error; - /* Only parse imagelabel, if requested live XML with relabeling */ if (!def->norelabel && !(flags & VIR_DOMAIN_XML_INACTIVE)) { @@ -2741,7 +2690,6 @@ virDomainDiskDefParseXML(virCapsPtr caps, xmlNodePtr node, xmlXPathContextPtr ctxt, virBitmapPtr bootMap, - virSecurityLabelDefPtr default_seclabel, unsigned int flags) { virDomainDiskDefPtr def; @@ -3051,16 +2999,6 @@ virDomainDiskDefParseXML(virCapsPtr caps, goto error; } - /* If source is present, check for an optional seclabel override. */ - if (source) { - xmlNodePtr seclabel = virXPathNode("./source/seclabel", ctxt); - if (seclabel && - (VIR_ALLOC(def->seclabel) < 0 || - virSecurityLabelDefParseXMLHelper(def->seclabel, seclabel, ctxt, - default_seclabel, flags) < 0)) - goto error; - } - if (target == NULL) { virDomainReportError(VIR_ERR_NO_TARGET, source ? "%s" : NULL, source); @@ -6408,8 +6346,7 @@ 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, flags))) goto error; } else if (xmlStrEqual(node->name, BAD_CAST "lease")) { dev->type = VIR_DOMAIN_DEVICE_LEASE; @@ -7511,7 +7448,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, nodes[i], ctxt, bootMap, - &def->seclabel, flags); if (!disk) goto error; @@ -9832,32 +9768,23 @@ virSecurityLabelDefFormat(virBufferPtr buf, virSecurityLabelDefPtr def, if (!sectype) goto cleanup; - if (def->model && - def->type == VIR_DOMAIN_SECLABEL_DYNAMIC && + if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC && !def->baselabel && (flags & VIR_DOMAIN_XML_INACTIVE)) { /* This is the default for inactive xml, so nothing to output. */ } else { - virBufferAddLit(buf, "<seclabel"); - if (def->model) - virBufferAsprintf(buf, " type='%s' model='%s'", - sectype, def->model); - virBufferAsprintf(buf, " relabel='%s'", + virBufferAsprintf(buf, "<seclabel type='%s' model='%s' relabel='%s'>\n", + sectype, def->model, def->norelabel ? "no" : "yes"); - if (def->label || def->baselabel) { - virBufferAddLit(buf, ">\n"); - virBufferEscapeString(buf, " <label>%s</label>\n", - def->label); - if (!def->norelabel) - virBufferEscapeString(buf, " <imagelabel>%s</imagelabel>\n", - def->imagelabel); - if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC) - virBufferEscapeString(buf, " <baselabel>%s</baselabel>\n", - def->baselabel); - virBufferAddLit(buf, "</seclabel>\n"); - } else { - virBufferAddLit(buf, "/>\n"); - } + virBufferEscapeString(buf, " <label>%s</label>\n", + def->label); + if (!def->norelabel) + virBufferEscapeString(buf, " <imagelabel>%s</imagelabel>\n", + def->imagelabel); + if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC) + virBufferEscapeString(buf, " <baselabel>%s</baselabel>\n", + def->baselabel); + virBufferAddLit(buf, "</seclabel>\n"); } ret = 0; cleanup: @@ -9980,36 +9907,17 @@ virDomainDiskDefFormat(virBufferPtr buf, def->startupPolicy) { switch (def->type) { case VIR_DOMAIN_DISK_TYPE_FILE: - virBufferAddLit(buf, " <source"); + virBufferAsprintf(buf," <source"); if (def->src) virBufferEscapeString(buf, " file='%s'", def->src); if (def->startupPolicy) virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); - if (def->seclabel) { - virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 8); - if (virSecurityLabelDefFormat(buf, def->seclabel, flags) < 0) - return -1; - virBufferAdjustIndent(buf, -8); - virBufferAddLit(buf, " </source>\n"); - } else { - virBufferAddLit(buf, "/>\n"); - } + virBufferAsprintf(buf, "/>\n"); break; case VIR_DOMAIN_DISK_TYPE_BLOCK: - if (def->src && def->seclabel) { - virBufferEscapeString(buf, " <source dev='%s'>\n", - def->src); - virBufferAdjustIndent(buf, 8); - if (virSecurityLabelDefFormat(buf, def->seclabel, flags) < 0) - return -1; - virBufferAdjustIndent(buf, -8); - virBufferAddLit(buf, " </source>\n"); - } else { - virBufferEscapeString(buf, " <source dev='%s'/>\n", - def->src); - } + virBufferEscapeString(buf, " <source dev='%s'/>\n", + def->src); break; case VIR_DOMAIN_DISK_TYPE_DIR: virBufferEscapeString(buf, " <source dir='%s'/>\n", -- 1.7.7.5

From: "Daniel P. Berrange" <berrange@redhat.com> This re-introduces parsing & formatting for per device seclabels. There is a new virDomainDeviceSeclabelPtr struct and corresponding APIs for parsing/formatting. --- src/conf/domain_conf.c | 132 ++++++++++++++++++++++++++++++++++++++++++++++-- src/conf/domain_conf.h | 12 ++++- 2 files changed, 139 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6c67cd8..8eacc1f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -809,6 +809,17 @@ virSecurityLabelDefClear(virSecurityLabelDefPtr def) VIR_FREE(def->baselabel); } + +static void +virSecurityDeviceLabelDefFree(virSecurityDeviceLabelDefPtr def) +{ + if (!def) + return; + VIR_FREE(def->label); + VIR_FREE(def); +} + + void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def) { int ii; @@ -887,6 +898,8 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def) virStorageEncryptionFree(def->encryption); virDomainDeviceInfoClear(&def->info); + virSecurityDeviceLabelDefFree(def->seclabel); + for (i = 0 ; i < def->nhosts ; i++) virDomainDiskHostDefFree(&def->hosts[i]); VIR_FREE(def->hosts); @@ -2609,6 +2622,67 @@ error: return -1; } + +static int +virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr *def, + virSecurityLabelDefPtr vmDef, + xmlXPathContextPtr ctxt) +{ + char *p; + + *def = NULL; + + if (virXPathNode("./seclabel", ctxt) == NULL) + 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")); + return -1; + } + + if (VIR_ALLOC(*def) < 0) { + virReportOOMError(); + return -1; + } + + 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; + } else { + virDomainReportError(VIR_ERR_XML_ERROR, + _("invalid security relabel value %s"), p); + VIR_FREE(p); + VIR_FREE(*def); + return -1; + } + VIR_FREE(p); + } else { + (*def)->norelabel = false; + } + + p = virXPathStringLimit("string(./seclabel/label[1])", + VIR_SECURITY_LABEL_BUFLEN-1, ctxt); + (*def)->label = p; + + 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; + } + + return 0; +} + + /* Parse the XML definition for a lease */ static virDomainLeaseDefPtr @@ -2690,9 +2764,11 @@ virDomainDiskDefParseXML(virCapsPtr caps, xmlNodePtr node, xmlXPathContextPtr ctxt, virBitmapPtr bootMap, + virSecurityLabelDefPtr vmSeclabel, unsigned int flags) { virDomainDiskDefPtr def; + xmlNodePtr sourceNode = NULL; xmlNodePtr cur, child; xmlNodePtr save_ctxt = ctxt->node; char *type = NULL; @@ -2748,6 +2824,8 @@ virDomainDiskDefParseXML(virCapsPtr caps, if ((source == NULL && hosts == NULL) && (xmlStrEqual(cur->name, BAD_CAST "source"))) { + sourceNode = cur; + switch (def->type) { case VIR_DOMAIN_DISK_TYPE_FILE: source = virXMLPropString(cur, "file"); @@ -2999,6 +3077,17 @@ virDomainDiskDefParseXML(virCapsPtr caps, goto error; } + /* If source is present, check for an optional seclabel override. */ + if (sourceNode) { + xmlNodePtr saved_node = ctxt->node; + ctxt->node = sourceNode; + if (virSecurityDeviceLabelDefParseXML(&def->seclabel, + vmSeclabel, + ctxt) < 0) + goto error; + ctxt->node = saved_node; + } + if (target == NULL) { virDomainReportError(VIR_ERR_NO_TARGET, source ? "%s" : NULL, source); @@ -6346,7 +6435,7 @@ 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, flags))) + NULL, &def->seclabel, flags))) goto error; } else if (xmlStrEqual(node->name, BAD_CAST "lease")) { dev->type = VIR_DOMAIN_DEVICE_LEASE; @@ -7448,6 +7537,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, nodes[i], ctxt, bootMap, + &def->seclabel, flags); if (!disk) goto error; @@ -9792,6 +9882,23 @@ cleanup: } +static void +virSecurityDeviceLabelDefFormat(virBufferPtr buf, + virSecurityDeviceLabelDefPtr def) +{ + virBufferAsprintf(buf, "<seclabel relabel='%s'", + def->norelabel ? "no" : "yes"); + if (def->label) { + virBufferAddLit(buf, ">\n"); + virBufferEscapeString(buf, " <label>%s</label>\n", + def->label); + virBufferAddLit(buf, "</seclabel>\n"); + } else { + virBufferAddLit(buf, "/>\n"); + } +} + + static int virDomainLeaseDefFormat(virBufferPtr buf, virDomainLeaseDefPtr def) @@ -9907,17 +10014,34 @@ virDomainDiskDefFormat(virBufferPtr buf, def->startupPolicy) { switch (def->type) { case VIR_DOMAIN_DISK_TYPE_FILE: - virBufferAsprintf(buf," <source"); + virBufferAddLit(buf," <source"); if (def->src) virBufferEscapeString(buf, " file='%s'", def->src); if (def->startupPolicy) virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); - virBufferAsprintf(buf, "/>\n"); + if (def->seclabel) { + virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 8); + virSecurityDeviceLabelDefFormat(buf, def->seclabel); + virBufferAdjustIndent(buf, -8); + virBufferAddLit(buf, " </source>\n"); + } else { + virBufferAddLit(buf, "/>\n"); + } break; case VIR_DOMAIN_DISK_TYPE_BLOCK: - virBufferEscapeString(buf, " <source dev='%s'/>\n", + virBufferEscapeString(buf, " <source dev='%s'", def->src); + if (def->seclabel) { + virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 8); + virSecurityDeviceLabelDefFormat(buf, def->seclabel); + virBufferAdjustIndent(buf, -8); + virBufferAddLit(buf, " </source>\n"); + } else { + virBufferAddLit(buf, "/>\n"); + } break; case VIR_DOMAIN_DISK_TYPE_DIR: virBufferEscapeString(buf, " <source dir='%s'/>\n", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3b522a9..2d31fd3 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -180,6 +180,16 @@ struct _virSecurityLabelDef { bool norelabel; }; + +/* Security configuration for domain */ +typedef struct _virSecurityDeviceLabelDef virSecurityDeviceLabelDef; +typedef virSecurityDeviceLabelDef *virSecurityDeviceLabelDefPtr; +struct _virSecurityDeviceLabelDef { + char *label; /* image label string */ + bool norelabel; +}; + + typedef struct _virDomainHostdevOrigStates virDomainHostdevOrigStates; typedef virDomainHostdevOrigStates *virDomainHostdevOrigStatesPtr; struct _virDomainHostdevOrigStates { @@ -367,7 +377,7 @@ struct _virDomainDiskDef { int device; int bus; char *src; - virSecurityLabelDefPtr seclabel; + virSecurityDeviceLabelDefPtr seclabel; char *dst; int protocol; int nhosts; -- 1.7.7.5

From: "Daniel P. Berrange" <berrange@redhat.com> Curently security labels can be of type 'dynamic' or 'static'. If no security label is given, then 'dynamic' is assumed. The current code takes advantage of this default, and avoids even saving <seclabel> elements with type='dynamic' to disk. This means if you temporarily change security driver, the guests can all still start. With the introduction of sVirt to LXC though, there needs to be a new default of 'none' to allow unconfined LXC containers. This patch introduces two new security label types - default: the host configuration decides whether to run the guest with type 'none' or 'dynamic' at guest start - none: the guest will run unconfined by security policy The 'none' label type will obviously be undesirable for some deployments, so a new qemu.conf option allows a host admin to mandate confined guests. It is also possible to turn off default confinement security_default_confined = 1|0 (default == 1) security_require_confined = 1|0 (default == 0) * src/conf/domain_conf.c, src/conf/domain_conf.h: Add new seclabel types * src/security/security_manager.c, src/security/security_manager.h: Set default sec label types * src/security/security_selinux.c: Handle 'none' seclabel type * src/qemu/qemu.conf, src/qemu/qemu_conf.c, src/qemu/qemu_conf.h, src/qemu/libvirtd_qemu.aug: New security config options * src/qemu/qemu_driver.c: Tell security driver about default config --- docs/formatdomain.html.in | 24 +++++++++---- docs/schemas/domaincommon.rng | 5 +++ po/POTFILES.in | 1 + src/conf/domain_conf.c | 70 ++++++++++++++++++++++++-------------- src/conf/domain_conf.h | 2 + src/qemu/libvirtd_qemu.aug | 2 + src/qemu/qemu.conf | 8 ++++ src/qemu/qemu_conf.c | 11 ++++++ src/qemu/qemu_conf.h | 2 + src/qemu/qemu_driver.c | 7 +++- src/security/security_manager.c | 51 +++++++++++++++++++++++++--- src/security/security_manager.h | 8 ++++- src/security/security_selinux.c | 32 ++++++++++++++---- tests/seclabeltest.c | 2 +- 14 files changed, 177 insertions(+), 48 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index dfb010d..2d4f4cf 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3484,10 +3484,11 @@ qemu-kvm -net nic,model=? /dev/null <p> The <code>seclabel</code> element allows control over the - operation of the security drivers. There are two basic - modes of operation, dynamic where libvirt automatically - generates a unique security label, or static where the - application/administrator chooses the labels. With dynamic + operation of the security drivers. There are three basic + modes of operation, 'dynamic' where libvirt automatically + generates a unique security label, 'static' where the + application/administrator chooses the labels, or 'none' + where confinement is disabled. With dynamic label generation, libvirt will always automatically relabel any resources associated with the virtual machine. With static label assignment, by default, the administrator @@ -3515,9 +3516,18 @@ qemu-kvm -net nic,model=? /dev/null <seclabel type='static' model='selinux' relabel='yes'> <label>system_u:system_r:svirt_t:s0:c392,c662</label> </seclabel> + + <seclabel type='none'/> </pre> <p> + If no 'type' attribute is provided in the input XML, then + the security driver default setting will be used, which + may be either 'none' or 'static'. If a 'baselabel' is set + but no 'type' is set, then the type is presumed to be 'dynamic' + </p> + + <p> When viewing the XML for a running guest with automatic resource relabeling active, an additional XML element, <code>imagelabel</code>, will be included. This is an @@ -3526,9 +3536,9 @@ qemu-kvm -net nic,model=? /dev/null </p> <dl> <dt><code>type</code></dt> - <dd>Either <code>static</code> or <code>dynamic</code> to determine - whether libvirt automatically generates a unique security label - or not. + <dd>Either <code>static</code>, <code>dynamic</code> or <code>none</code> + to determine whether libvirt automatically generates a unique security + label or not. </dd> <dt><code>model</code></dt> <dd>A valid security model name, matching the currently diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 4fa968d..6094643 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -116,6 +116,11 @@ </optional> </interleave> </group> + <group> + <attribute name='type'> + <value>none</value> + </attribute> + </group> </choice> </element> </define> diff --git a/po/POTFILES.in b/po/POTFILES.in index 0126320..42e80d2 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -90,6 +90,7 @@ src/secret/secret_driver.c src/security/security_apparmor.c src/security/security_dac.c src/security/security_driver.c +src/security/security_manager.c src/security/security_selinux.c src/security/virt-aa-helper.c src/storage/parthelper.c diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8eacc1f..cb6c4fd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -562,6 +562,8 @@ VIR_ENUM_IMPL(virDomainCrashedReason, VIR_DOMAIN_CRASHED_LAST, "unknown") VIR_ENUM_IMPL(virDomainSeclabel, VIR_DOMAIN_SECLABEL_LAST, + "default", + "none", "dynamic", "static") @@ -2528,13 +2530,15 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, "%s", _("missing security type")); goto error; } + def->type = virDomainSeclabelTypeFromString(p); VIR_FREE(p); - if (def->type < 0) { + if (def->type <= 0) { virDomainReportError(VIR_ERR_XML_ERROR, "%s", _("invalid security type")); goto error; } + p = virXPathStringLimit("string(./seclabel/@relabel)", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p != NULL) { @@ -2555,8 +2559,15 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, "%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")); + goto error; + } } else { - if (def->type == VIR_DOMAIN_SECLABEL_STATIC) + if (def->type == VIR_DOMAIN_SECLABEL_STATIC || + def->type == VIR_DOMAIN_SECLABEL_NONE) def->norelabel = true; else def->norelabel = false; @@ -2591,12 +2602,15 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, def->imagelabel = p; } - /* Only parse baselabel, for dynamic label */ - if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC) { + /* Only parse baselabel, for dynamic or none label types */ + if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC || + def->type == VIR_DOMAIN_SECLABEL_NONE) { p = virXPathStringLimit("string(./seclabel/baselabel[1])", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p != NULL) def->baselabel = p; + /* Forces none type to dynamic for back compat */ + def->type = VIR_DOMAIN_SECLABEL_DYNAMIC; } /* Only parse model, if static labelling, or a base @@ -9848,24 +9862,32 @@ virDomainLifecycleDefFormat(virBufferPtr buf, } -static int -virSecurityLabelDefFormat(virBufferPtr buf, virSecurityLabelDefPtr def, - unsigned int flags) +static void +virSecurityLabelDefFormat(virBufferPtr buf, virSecurityLabelDefPtr def) { const char *sectype = virDomainSeclabelTypeToString(def->type); - int ret = -1; if (!sectype) - goto cleanup; + return; + + if (def->type == VIR_DOMAIN_SECLABEL_DEFAULT) + return; + + virBufferAsprintf(buf, "<seclabel type='%s'", + sectype); + virBufferEscapeString(buf, " model='%s'", def->model); + + virBufferAsprintf(buf, " relabel='%s'", + def->norelabel ? "no" : "yes"); + + if (def->type == VIR_DOMAIN_SECLABEL_NONE) { + virBufferAddLit(buf, "/>\n"); + return; + } + + if (def->label || def->imagelabel || def->baselabel) { + virBufferAddLit(buf, ">\n"); - if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC && - !def->baselabel && - (flags & VIR_DOMAIN_XML_INACTIVE)) { - /* This is the default for inactive xml, so nothing to output. */ - } else { - virBufferAsprintf(buf, "<seclabel type='%s' model='%s' relabel='%s'>\n", - sectype, def->model, - def->norelabel ? "no" : "yes"); virBufferEscapeString(buf, " <label>%s</label>\n", def->label); if (!def->norelabel) @@ -9875,10 +9897,9 @@ virSecurityLabelDefFormat(virBufferPtr buf, virSecurityLabelDefPtr def, virBufferEscapeString(buf, " <baselabel>%s</baselabel>\n", def->baselabel); virBufferAddLit(buf, "</seclabel>\n"); + } else { + virBufferAddLit(buf, "/>\n"); } - ret = 0; -cleanup: - return ret; } @@ -11887,12 +11908,9 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAddLit(buf, " </devices>\n"); - if (def->seclabel.model) { - virBufferAdjustIndent(buf, 2); - if (virSecurityLabelDefFormat(buf, &def->seclabel, flags) < 0) - goto cleanup; - virBufferAdjustIndent(buf, -2); - } + virBufferAdjustIndent(buf, 2); + virSecurityLabelDefFormat(buf, &def->seclabel); + 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 2d31fd3..38730ab 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -162,6 +162,8 @@ struct _virDomainDeviceInfo { }; enum virDomainSeclabelType { + VIR_DOMAIN_SECLABEL_DEFAULT, + VIR_DOMAIN_SECLABEL_NONE, VIR_DOMAIN_SECLABEL_DYNAMIC, VIR_DOMAIN_SECLABEL_STATIC, diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index 078e9c4..f6cec1f 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -33,6 +33,8 @@ module Libvirtd_qemu = | bool_entry "vnc_sasl" | str_entry "vnc_sasl_dir" | str_entry "security_driver" + | bool_entry "security_default_confined" + | bool_entry "security_require_confined" | str_entry "user" | str_entry "group" | bool_entry "dynamic_ownership" diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 4ec5e6c..95428c1 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -138,6 +138,14 @@ # # security_driver = "selinux" +# If set to non-zero, then the default security labeling +# will make guests confined. If set to zero, then guests +# will be unconfined by default. Defaults to 1. +# security_default_confined = 1 + +# If set to non-zero, then attempts to create unconfined +# guests will be blocked. Defaults to 0. +# security_require_confined = 1 # The user ID for QEMU processes run by the system instance. #user = "root" diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index bc0a646..e95c7a5 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -75,6 +75,8 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, int i; /* Setup critical defaults */ + driver->securityDefaultConfined = true; + driver->securityRequireConfined = false; driver->dynamicOwnership = 1; driver->clearEmulatorCapabilities = 1; @@ -195,6 +197,15 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, } } + p = virConfGetValue (conf, "security_default_confined"); + CHECK_TYPE ("security_default_confined", VIR_CONF_LONG); + if (p) driver->securityDefaultConfined = p->l; + + p = virConfGetValue (conf, "security_require_confined"); + CHECK_TYPE ("security_require_confined", VIR_CONF_LONG); + if (p) driver->securityRequireConfined = p->l; + + p = virConfGetValue (conf, "vnc_sasl"); CHECK_TYPE ("vnc_sasl", VIR_CONF_LONG); if (p) driver->vncSASL = p->l; diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 7d79823..e85017f 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -115,6 +115,8 @@ struct qemud_driver { virDomainEventStatePtr domainEventState; char *securityDriverName; + bool securityDefaultConfined; + bool securityRequireConfined; virSecurityManagerPtr securityManager; char *saveImageFormat; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ab69dca..f44e852 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -210,7 +210,10 @@ static int qemuSecurityInit(struct qemud_driver *driver) { virSecurityManagerPtr mgr = virSecurityManagerNew(driver->securityDriverName, - driver->allowDiskFormatProbing); + driver->allowDiskFormatProbing, + driver->securityDefaultConfined, + driver->securityRequireConfined); + if (!mgr) goto error; @@ -218,6 +221,8 @@ qemuSecurityInit(struct qemud_driver *driver) virSecurityManagerPtr dac = virSecurityManagerNewDAC(driver->user, driver->group, driver->allowDiskFormatProbing, + driver->securityDefaultConfined, + driver->securityRequireConfined, driver->dynamicOwnership); if (!dac) goto error; diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 2e4956a..d0bafae 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -36,10 +36,14 @@ struct _virSecurityManager { virSecurityDriverPtr drv; bool allowDiskFormatProbing; + bool defaultConfined; + bool requireConfined; }; static virSecurityManagerPtr virSecurityManagerNewDriver(virSecurityDriverPtr drv, - bool allowDiskFormatProbing) + bool allowDiskFormatProbing, + bool defaultConfined, + bool requireConfined) { virSecurityManagerPtr mgr; @@ -50,6 +54,8 @@ static virSecurityManagerPtr virSecurityManagerNewDriver(virSecurityDriverPtr dr mgr->drv = drv; mgr->allowDiskFormatProbing = allowDiskFormatProbing; + mgr->defaultConfined = defaultConfined; + mgr->requireConfined = requireConfined; if (drv->open(mgr) < 0) { virSecurityManagerFree(mgr); @@ -64,7 +70,9 @@ virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary, { virSecurityManagerPtr mgr = virSecurityManagerNewDriver(&virSecurityDriverStack, - virSecurityManagerGetAllowDiskFormatProbing(primary)); + virSecurityManagerGetAllowDiskFormatProbing(primary), + virSecurityManagerGetDefaultConfined(primary), + virSecurityManagerGetRequireConfined(primary)); if (!mgr) return NULL; @@ -78,11 +86,15 @@ virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary, virSecurityManagerPtr virSecurityManagerNewDAC(uid_t user, gid_t group, bool allowDiskFormatProbing, + bool defaultConfined, + bool requireConfined, bool dynamicOwnership) { virSecurityManagerPtr mgr = virSecurityManagerNewDriver(&virSecurityDriverDAC, - allowDiskFormatProbing); + allowDiskFormatProbing, + defaultConfined, + requireConfined); if (!mgr) return NULL; @@ -95,13 +107,18 @@ virSecurityManagerPtr virSecurityManagerNewDAC(uid_t user, } virSecurityManagerPtr virSecurityManagerNew(const char *name, - bool allowDiskFormatProbing) + bool allowDiskFormatProbing, + bool defaultConfined, + bool requireConfined) { virSecurityDriverPtr drv = virSecurityDriverLookup(name); if (!drv) return NULL; - return virSecurityManagerNewDriver(drv, allowDiskFormatProbing); + return virSecurityManagerNewDriver(drv, + allowDiskFormatProbing, + defaultConfined, + requireConfined); } @@ -149,6 +166,16 @@ bool virSecurityManagerGetAllowDiskFormatProbing(virSecurityManagerPtr mgr) return mgr->allowDiskFormatProbing; } +bool virSecurityManagerGetDefaultConfined(virSecurityManagerPtr mgr) +{ + return mgr->defaultConfined; +} + +bool virSecurityManagerGetRequireConfined(virSecurityManagerPtr mgr) +{ + return mgr->requireConfined; +} + int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm, virDomainDiskDefPtr disk) @@ -248,6 +275,20 @@ int virSecurityManagerRestoreSavedStateLabel(virSecurityManagerPtr mgr, int virSecurityManagerGenLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm) { + if (vm->seclabel.type == VIR_DOMAIN_SECLABEL_DEFAULT) { + if (mgr->defaultConfined) + vm->seclabel.type = VIR_DOMAIN_SECLABEL_DYNAMIC; + else + vm->seclabel.type = VIR_DOMAIN_SECLABEL_NONE; + } + + if ((vm->seclabel.type == VIR_DOMAIN_SECLABEL_NONE) && + mgr->requireConfined) { + virSecurityReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Unconfined guests are not allowed on this host")); + return -1; + } + if (mgr->drv->domainGenSecurityLabel) return mgr->drv->domainGenSecurityLabel(mgr, vm); diff --git a/src/security/security_manager.h b/src/security/security_manager.h index 6731d59..32c8c3b 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -32,7 +32,9 @@ typedef struct _virSecurityManager virSecurityManager; typedef virSecurityManager *virSecurityManagerPtr; virSecurityManagerPtr virSecurityManagerNew(const char *name, - bool allowDiskFormatProbing); + bool allowDiskFormatProbing, + bool defaultConfined, + bool requireConfined); virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary, virSecurityManagerPtr secondary); @@ -40,6 +42,8 @@ virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary, virSecurityManagerPtr virSecurityManagerNewDAC(uid_t user, gid_t group, bool allowDiskFormatProbing, + bool defaultConfined, + bool requireConfined, bool dynamicOwnership); void *virSecurityManagerGetPrivateData(virSecurityManagerPtr mgr); @@ -49,6 +53,8 @@ void virSecurityManagerFree(virSecurityManagerPtr mgr); const char *virSecurityManagerGetDOI(virSecurityManagerPtr mgr); const char *virSecurityManagerGetModel(virSecurityManagerPtr mgr); bool virSecurityManagerGetAllowDiskFormatProbing(virSecurityManagerPtr mgr); +bool virSecurityManagerGetDefaultConfined(virSecurityManagerPtr mgr); +bool virSecurityManagerGetRequireConfined(virSecurityManagerPtr mgr); int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index c2dceca..7522b06 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -170,6 +170,7 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, int c1 = 0; int c2 = 0; context_t ctx = NULL; + const char *range; if ((def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC) && !def->seclabel.baselabel && @@ -200,7 +201,8 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, return rc; } - if (def->seclabel.type == VIR_DOMAIN_SECLABEL_STATIC) { + switch (def->seclabel.type) { + case VIR_DOMAIN_SECLABEL_STATIC: if (!(ctx = context_new(def->seclabel.label)) ) { virReportSystemError(errno, _("unable to allocate socket security context '%s'"), @@ -208,13 +210,15 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, return rc; } - const char *range = context_range_get(ctx); + range = context_range_get(ctx); if (!range || !(mcs = strdup(range))) { virReportOOMError(); goto cleanup; } - } else { + break; + + case VIR_DOMAIN_SECLABEL_DYNAMIC: do { c1 = virRandom(1024); c2 = virRandom(1024); @@ -246,14 +250,28 @@ SELinuxGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, _("cannot generate selinux context for %s"), mcs); goto cleanup; } - } - def->seclabel.imagelabel = SELinuxGenNewContext(default_image_context, mcs); - if (!def->seclabel.imagelabel) { + break; + + case VIR_DOMAIN_SECLABEL_NONE: + /* no op */ + break; + + default: virSecurityReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot generate selinux context for %s"), mcs); + _("unexpected security label type '%s'"), + virDomainSeclabelTypeToString(def->seclabel.type)); goto cleanup; } + if (!def->seclabel.norelabel) { + def->seclabel.imagelabel = SELinuxGenNewContext(default_image_context, mcs); + if (!def->seclabel.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))) { virReportOOMError(); diff --git a/tests/seclabeltest.c b/tests/seclabeltest.c index 5d87789..1898c3e 100644 --- a/tests/seclabeltest.c +++ b/tests/seclabeltest.c @@ -13,7 +13,7 @@ main (int argc ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED) virSecurityManagerPtr mgr; const char *doi, *model; - mgr = virSecurityManagerNew(NULL, false); + mgr = virSecurityManagerNew(NULL, false, true, true); if (mgr == NULL) { fprintf (stderr, "Failed to start security driver"); exit (-1); -- 1.7.7.5