[libvirt] [PATCH v3 2/2] Add <seclabel> to character devices.
Michal Privoznik
mprivozn at redhat.com
Fri Sep 21 09:27:20 UTC 2012
On 20.09.2012 17:29, Richard W.M. Jones wrote:
> From: "Richard W.M. Jones" <rjones at redhat.com>
>
> This allows the user to control labelling of each character device
> separately (the default is to inherit from the VM).
> ---
> docs/formatdomain.html.in | 8 ++++
> src/conf/domain_conf.c | 77 +++++++++++++++++++++++++++++++++--
> src/conf/domain_conf.h | 6 +++
> src/security/security_selinux.c | 86 +++++++++++++++++++++++++++------------
> 4 files changed, 147 insertions(+), 30 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 51f897c..939312c 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -3434,6 +3434,14 @@ qemu-kvm -net nic,model=? /dev/null
> </p>
>
> <p>
> + The <code>source</code> element may contain an optional
> + <code>seclabel</code> to override the way that labelling
> + is done on the socket path. If this element is not present,
> + the <a href="#seclabel">security label is inherited from
> + the per-domain setting</a>.
> + </p>
> +
> + <p>
> Each character device element has an optional
> sub-element <code><address></code> which can tie the
> device to a
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 06b3209..7023897 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1273,6 +1273,8 @@ virDomainChrSourceDefIsEqual(const virDomainChrSourceDef *src,
>
> void virDomainChrDefFree(virDomainChrDefPtr def)
> {
> + size_t i;
> +
> if (!def)
> return;
>
> @@ -1296,6 +1298,12 @@ void virDomainChrDefFree(virDomainChrDefPtr def)
> virDomainChrSourceDefClear(&def->source);
> virDomainDeviceInfoClear(&def->info);
>
> + if (def->seclabels) {
> + for (i = 0; i < def->nseclabels; i++)
> + virSecurityDeviceLabelDefFree(def->seclabels[i]);
> + VIR_FREE(def->seclabels);
> + }
> +
> VIR_FREE(def);
> }
>
> @@ -5343,7 +5351,11 @@ error:
> * <target>, which is used by <serial> but not <smartcard>). */
> static int
> virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
> - xmlNodePtr cur, unsigned int flags)
> + xmlNodePtr cur, unsigned int flags,
> + virDomainChrDefPtr chr_def,
> + xmlXPathContextPtr ctxt,
> + virSecurityLabelDefPtr* vmSeclabels,
> + int nvmSeclabels)
> {
> char *bindHost = NULL;
> char *bindService = NULL;
> @@ -5398,6 +5410,21 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
> if (def->type == VIR_DOMAIN_CHR_TYPE_UDP)
> VIR_FREE(mode);
> }
> +
> + /* Check for an optional seclabel override in <source/>. */
> + if (chr_def) {
> + xmlNodePtr saved_node = ctxt->node;
> + ctxt->node = cur;
> + if (virSecurityDeviceLabelDefParseXML (&chr_def->seclabels,
> + &chr_def->nseclabels,
> + vmSeclabels,
> + nvmSeclabels,
> + ctxt) < 0) {
extra space; we don't type space between function name and (
> + ctxt->node = saved_node;
> + goto error;
> + }
> + ctxt->node = saved_node;
> + }
> } else if (xmlStrEqual(cur->name, BAD_CAST "protocol")) {
> if (protocol == NULL)
> protocol = virXMLPropString(cur, "type");
> @@ -5591,7 +5618,10 @@ virDomainChrDefNew(void) {
> static virDomainChrDefPtr
> virDomainChrDefParseXML(virCapsPtr caps,
> virDomainDefPtr vmdef,
> + xmlXPathContextPtr ctxt,
> xmlNodePtr node,
> + virSecurityLabelDefPtr* vmSeclabels,
> + int nvmSeclabels,
> unsigned int flags)
> {
> xmlNodePtr cur;
> @@ -5622,7 +5652,8 @@ virDomainChrDefParseXML(virCapsPtr caps,
> }
>
> cur = node->children;
> - remaining = virDomainChrSourceDefParseXML(&def->source, cur, flags);
> + remaining = virDomainChrSourceDefParseXML(&def->source, cur, flags,
> + def, ctxt, vmSeclabels, nvmSeclabels);
long line
> if (remaining < 0)
> goto error;
> if (remaining) {
> @@ -5759,7 +5790,8 @@ virDomainSmartcardDefParseXML(xmlNodePtr node,
> }
>
> cur = node->children;
> - if (virDomainChrSourceDefParseXML(&def->data.passthru, cur, flags) < 0)
> + if (virDomainChrSourceDefParseXML(&def->data.passthru, cur, flags,
> + NULL, NULL, NULL, 0) < 0)
> goto error;
>
> if (def->data.passthru.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) {
> @@ -7240,7 +7272,8 @@ virDomainRedirdevDefParseXML(const xmlNodePtr node,
> if (xmlStrEqual(cur->name, BAD_CAST "source")) {
> int remaining;
>
> - remaining = virDomainChrSourceDefParseXML(&def->source.chr, cur, flags);
> + remaining = virDomainChrSourceDefParseXML(&def->source.chr, cur, flags,
> + NULL, NULL, NULL, 0);
> if (remaining != 0)
> goto error;
> }
> @@ -9282,7 +9315,10 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
> for (i = 0 ; i < n ; i++) {
> virDomainChrDefPtr chr = virDomainChrDefParseXML(caps,
> def,
> + ctxt,
> nodes[i],
> + def->seclabels,
> + def->nseclabels,
> flags);
> if (!chr)
> goto error;
> @@ -9309,7 +9345,10 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
> for (i = 0 ; i < n ; i++) {
> virDomainChrDefPtr chr = virDomainChrDefParseXML(caps,
> def,
> + ctxt,
> nodes[i],
> + def->seclabels,
> + def->nseclabels,
> flags);
> if (!chr)
> goto error;
> @@ -9339,7 +9378,10 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
> bool create_stub = true;
> virDomainChrDefPtr chr = virDomainChrDefParseXML(caps,
> def,
> + ctxt,
> nodes[i],
> + def->seclabels,
> + def->nseclabels,
> flags);
> if (!chr)
> goto error;
> @@ -9415,7 +9457,10 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
> for (i = 0 ; i < n ; i++) {
> virDomainChrDefPtr chr = virDomainChrDefParseXML(caps,
> def,
> + ctxt,
> nodes[i],
> + def->seclabels,
> + def->nseclabels,
> flags);
> if (!chr)
> goto error;
> @@ -12359,6 +12404,7 @@ virDomainChrDefFormat(virBufferPtr buf,
> const char *targetType = virDomainChrTargetTypeToString(def->deviceType,
> def->targetType);
> bool tty_compat;
> + size_t n;
>
> int ret = 0;
>
> @@ -12438,6 +12484,14 @@ virDomainChrDefFormat(virBufferPtr buf,
> return -1;
> }
>
> + /* Security label overrides, if any. */
> + if (def->seclabels && def->nseclabels > 0) {
> + virBufferAdjustIndent(buf, 2);
> + for (n = 0; n < def->nseclabels; n++)
> + virSecurityDeviceLabelDefFormat(buf, def->seclabels[n]);
> + virBufferAdjustIndent(buf, -2);
> + }
> +
> virBufferAsprintf(buf, " </%s>\n", elementName);
>
> return ret;
> @@ -15262,6 +15316,21 @@ virDomainDiskDefGetSecurityLabelDef(virDomainDiskDefPtr def, const char *model)
> return NULL;
> }
>
> +virSecurityDeviceLabelDefPtr
> +virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const char *model)
> +{
> + int i;
> +
> + if (def == NULL)
> + return NULL;
> +
> + for (i = 0; i < def->nseclabels; i++) {
> + if (STREQ_NULLABLE(def->seclabels[i]->model, model))
> + return def->seclabels[i];
> + }
> + return NULL;
> +}
> +
> virSecurityLabelDefPtr
> virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char *model)
> {
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 510406a..84f566f 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -966,6 +966,9 @@ struct _virDomainChrDef {
> virDomainChrSourceDef source;
>
> virDomainDeviceInfo info;
> +
> + size_t nseclabels;
> + virSecurityDeviceLabelDefPtr *seclabels;
> };
>
> enum virDomainSmartcardType {
> @@ -2123,6 +2126,9 @@ virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char *model);
> virSecurityDeviceLabelDefPtr
> virDomainDiskDefGetSecurityLabelDef(virDomainDiskDefPtr def, const char *model);
>
> +virSecurityDeviceLabelDefPtr
> +virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const char *model);
> +
> virSecurityLabelDefPtr
> virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char *model);
>
> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index a7e2420..c47c872 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -1213,38 +1213,58 @@ done:
>
> static int
> virSecuritySELinuxSetSecurityChardevLabel(virDomainDefPtr def,
> - virDomainChrSourceDefPtr dev)
> + virDomainChrDefPtr dev,
> + virDomainChrSourceDefPtr dev_source)
>
> {
> - virSecurityLabelDefPtr secdef;
> + virSecurityLabelDefPtr seclabel;
> + virSecurityDeviceLabelDefPtr chr_seclabel = NULL;
> + char *imagelabel;
> char *in = NULL, *out = NULL;
> int ret = -1;
>
> - secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
> - if (secdef == NULL)
> + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
> + if (seclabel == NULL)
> return -1;
>
> - if (secdef->norelabel)
> + if (dev)
> + chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev,
> + SECURITY_SELINUX_NAME);
> +
> + if (seclabel->norelabel || (chr_seclabel && chr_seclabel->norelabel))
> return 0;
>
> - switch (dev->type) {
> + if (chr_seclabel)
> + imagelabel = chr_seclabel->label;
> + else
> + imagelabel = seclabel->imagelabel;
> +
> + switch (dev_source->type) {
> case VIR_DOMAIN_CHR_TYPE_DEV:
> case VIR_DOMAIN_CHR_TYPE_FILE:
> - ret = virSecuritySELinuxSetFilecon(dev->data.file.path, secdef->imagelabel);
> + ret = virSecuritySELinuxSetFilecon(dev_source->data.file.path, imagelabel);
> + break;
> +
> + case VIR_DOMAIN_CHR_TYPE_UNIX:
> + if (!dev_source->data.nix.listen) {
> + if (virSecuritySELinuxSetFilecon(dev_source->data.file.path, imagelabel) < 0)
again long line
> + goto done;
> + }
> + ret = 0;
> break;
>
> case VIR_DOMAIN_CHR_TYPE_PIPE:
> - if ((virAsprintf(&in, "%s.in", dev->data.file.path) < 0) ||
> - (virAsprintf(&out, "%s.out", dev->data.file.path) < 0)) {
> + if ((virAsprintf(&in, "%s.in", dev_source->data.file.path) < 0) ||
> + (virAsprintf(&out, "%s.out", dev_source->data.file.path) < 0)) {
> virReportOOMError();
> goto done;
> }
> if (virFileExists(in) && virFileExists(out)) {
> - if ((virSecuritySELinuxSetFilecon(in, secdef->imagelabel) < 0) ||
> - (virSecuritySELinuxSetFilecon(out, secdef->imagelabel) < 0)) {
> + if ((virSecuritySELinuxSetFilecon(in, imagelabel) < 0) ||
> + (virSecuritySELinuxSetFilecon(out, imagelabel) < 0)) {
> goto done;
> }
> - } else if (virSecuritySELinuxSetFilecon(dev->data.file.path, secdef->imagelabel) < 0) {
> + } else if (virSecuritySELinuxSetFilecon(dev_source->data.file.path, imagelabel) < 0) {
and again (there are more so I am stopping reporting them). Beside that
I haven't spotted anything bad with this patch. Hence ACK if you fix
coding style issues.
Michal
More information about the libvir-list
mailing list