On 07/05/2011 05:51 AM, Daniel P. Berrange wrote:

From: "Daniel P. Berrange"<berrange@redhat.com>

When no<seclabel> is present in the XML, the virDomainSeclabelDef struct is left as all zeros. Unfortunately, this means it gets setup as type=dynamic, with relabel=no, which is an illegal combination.

Change the 'bool relabel' attribute in virDomainSeclabelDef to the inverse 'bool norelabel' so that the default initialization is sensible

* src/conf/domain_conf.c, src/conf/domain_conf.h, src/security/security_apparmor.c, src/security/security_selinux.c: Replace 'relabel' with 'norelabel'

ACK. (although it's a bit confusing that the name in the XML and the name in the C struct are opposites of each other :-) Note that even after applying this patch, if you have any domains that were started with the erroneous code in place, you will have to manually edit the state file in /var/run/libvirt/qemu/${domainname}.xml and remove the <seclabel> section at the end. If you don't do this, libvirt will still fail to reconnect to the domain when it's restarted. As I understand, this is all fallout of the following commit, which was pushed *after* 0.9.3, so only a few people will be affected: commit 6321fd979851a29da974823dddd6b2a4a7e43f83 Author: Daniel P. Berrange <berrange@redhat.com> Date: Fri Jun 24 10:21:33 2011 +0100 Allow for resource relabelling with static labels Add a new attribute to the <seclabel> XML to allow resource relabelling to be enabled with static label usage. <seclabel model='selinux' type='static' relabel='yes'> <label>system_u:system_r:svirt_t:s0:c392,c662</label> </seclabel>

--- src/conf/domain_conf.c | 18 +++++++++--------- src/conf/domain_conf.h | 2 +- src/security/security_apparmor.c | 8 ++++---- src/security/security_selinux.c | 20 ++++++++++---------- 4 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 109a947..b74ab4d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5076,25 +5076,25 @@ virSecurityLabelDefParseXML(const virDomainDefPtr def, VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p != NULL) { if (STREQ(p, "yes")) { - def->seclabel.relabel = true; + def->seclabel.norelabel = false; } else if (STREQ(p, "no")) { - def->seclabel.relabel = false; + def->seclabel.norelabel = true; } else { virDomainReportError(VIR_ERR_XML_ERROR, _("invalid security relabel value %s"), p); goto error; } if (def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC&& - !def->seclabel.relabel) { + def->seclabel.norelabel) { virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("dynamic label type must use resource relabeling")); goto error; } } else { if (def->seclabel.type == VIR_DOMAIN_SECLABEL_STATIC) - def->seclabel.relabel = false; + def->seclabel.norelabel = true; else - def->seclabel.relabel = true; + def->seclabel.norelabel = false; }

/* Only parse label, if using static labels, or @@ -5114,7 +5114,7 @@ virSecurityLabelDefParseXML(const virDomainDefPtr def, }

/* Only parse imagelabel, if requested live XML with relabeling */ - if (def->seclabel.relabel&& + if (!def->seclabel.norelabel&& !(flags& VIR_DOMAIN_XML_INACTIVE)) { p = virXPathStringLimit("string(./seclabel/imagelabel[1])", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); @@ -9890,15 +9890,15 @@ char *virDomainDefFormat(virDomainDefPtr def, (flags& VIR_DOMAIN_XML_INACTIVE)) { virBufferAsprintf(&buf, "<seclabel type='%s' model='%s' relabel='%s'/>\n", sectype, def->seclabel.model, - def->seclabel.relabel ? "yes" : "no"); + def->seclabel.norelabel ? "no" : "yes"); } else { virBufferAsprintf(&buf, "<seclabel type='%s' model='%s' relabel='%s'>\n", sectype, def->seclabel.model, - def->seclabel.relabel ? "yes" : "no"); + def->seclabel.norelabel ? "no" : "yes"); if (def->seclabel.label) virBufferEscapeString(&buf, "<label>%s</label>\n", def->seclabel.label); - if (def->seclabel.relabel&& def->seclabel.imagelabel) + if (!def->seclabel.norelabel&& def->seclabel.imagelabel) virBufferEscapeString(&buf, "<imagelabel>%s</imagelabel>\n", def->seclabel.imagelabel); if (def->seclabel.baselabel&& diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 258289a..44d5bcd 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -960,7 +960,7 @@ struct _virSecurityLabelDef { char *imagelabel; /* security image label string */ char *baselabel; /* base name of label string */ int type; /* virDomainSeclabelType */ - bool relabel; + bool norelabel; };

enum virDomainTimerNameType { diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 98995a8..76c6e3d 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -265,7 +265,7 @@ reload_profile(virSecurityManagerPtr mgr, int rc = -1; char *profile_name = NULL;

- if (!secdef->relabel) + if (secdef->norelabel) return 0;

if ((profile_name = get_profile_name(vm)) == NULL) @@ -610,7 +610,7 @@ AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr, int rc = -1; char *profile_name;

- if (!secdef->relabel) + if (secdef->norelabel) return 0;

if (!disk->src || disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) @@ -682,7 +682,7 @@ AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr, struct SDPDOP *ptr; int ret = -1;

- if (!secdef->relabel) + if (secdef->norelabel) return 0;

if (dev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) @@ -741,7 +741,7 @@ AppArmorRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr,

{ const virSecurityLabelDefPtr secdef =&vm->def->seclabel; - if (!secdef->relabel) + if (secdef->norelabel) return 0;

return reload_profile(mgr, vm, NULL, false); diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 44e770e..50e1978 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -537,7 +537,7 @@ SELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, { const virSecurityLabelDefPtr secdef =&vm->def->seclabel;

- if (!secdef->relabel) + if (secdef->norelabel) return 0;

/* Don't restore labels on readoly/shared disks, because @@ -621,7 +621,7 @@ SELinuxSetSecurityImageLabel(virSecurityManagerPtr mgr, const virSecurityLabelDefPtr secdef =&vm->def->seclabel; bool allowDiskFormatProbing = virSecurityManagerGetAllowDiskFormatProbing(mgr);

- if (!secdef->relabel) + if (secdef->norelabel) return 0;

return virDomainDiskDefForeachPath(disk, @@ -661,7 +661,7 @@ SELinuxSetSecurityHostdevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, const virSecurityLabelDefPtr secdef =&vm->def->seclabel; int ret = -1;

- if (!secdef->relabel) + if (secdef->norelabel) return 0;

if (dev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) @@ -730,7 +730,7 @@ SELinuxRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, const virSecurityLabelDefPtr secdef =&vm->def->seclabel; int ret = -1;

- if (!secdef->relabel) + if (secdef->norelabel) return 0;

if (dev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) @@ -784,7 +784,7 @@ SELinuxSetSecurityChardevLabel(virDomainObjPtr vm, char *in = NULL, *out = NULL; int ret = -1;

- if (!secdef->relabel) + if (secdef->norelabel) return 0;

switch (dev->type) { @@ -830,7 +830,7 @@ SELinuxRestoreSecurityChardevLabel(virDomainObjPtr vm, char *in = NULL, *out = NULL; int ret = -1;

- if (!secdef->relabel) + if (secdef->norelabel) return 0;

switch (dev->type) { @@ -918,7 +918,7 @@ SELinuxRestoreSecurityAllLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,

VIR_DEBUG("Restoring security label on %s", vm->def->name);

- if (!secdef->relabel) + if (secdef->norelabel) return 0;

for (i = 0 ; i< vm->def->nhostdevs ; i++) { @@ -989,7 +989,7 @@ SELinuxSetSavedStateLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, { const virSecurityLabelDefPtr secdef =&vm->def->seclabel;

- if (!secdef->relabel) + if (secdef->norelabel) return 0;

return SELinuxSetFilecon(savefile, secdef->imagelabel); @@ -1003,7 +1003,7 @@ SELinuxRestoreSavedStateLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, { const virSecurityLabelDefPtr secdef =&vm->def->seclabel;

- if (!secdef->relabel) + if (secdef->norelabel) return 0;

return SELinuxRestoreSecurityFileLabel(savefile); @@ -1218,7 +1218,7 @@ SELinuxSetSecurityAllLabel(virSecurityManagerPtr mgr, const virSecurityLabelDefPtr secdef =&vm->def->seclabel; int i;

- if (!secdef->relabel) + if (secdef->norelabel) return 0;

for (i = 0 ; i< vm->def->ndisks ; i++) {