Re: [PATCH] Fix default value of security label 'relabel' attribute

Laine Stump <laine@xxxxxxxxx> · Tue, 05 Jul 2011 12:03:11 -0400

On 07/05/2011 05:51 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@xxxxxxxxxx>

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@xxxxxxxxxx>
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++) {

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list





