conf: move seclabel validation into post-parse phase

Currently the disk and chardev seclabels are validated immediately at
the time their data is parsed. This forces the parser to fill in the
top level secmodel at time of parsing which is an undesirable thing.
This validation conceptually should be done in the post-parse phase
instead.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index cedbe687acba321ccd67f71a0b2d01b1577c1e93..ee048e81c04429f03d189f0134b4655e90246ce5 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5944,8 +5944,42 @@ virDomainDiskAddressDiskBusCompatibility(virDomainDiskBus bus,
 
 
 static int
-virDomainDiskDefValidate(const virDomainDiskDef *disk)
+virSecurityDeviceLabelDefValidateXML(virSecurityDeviceLabelDefPtr *seclabels,
+                                     size_t nseclabels,
+                                     virSecurityLabelDefPtr *vmSeclabels,
+                                     size_t nvmSeclabels)
+{
+    virSecurityDeviceLabelDefPtr seclabel;
+    size_t i;
+    size_t j;
+
+    for (i = 0; i < nseclabels; i++) {
+        seclabel = seclabels[i];
+
+        /* find the security label that it's being overridden */
+        for (j = 0; j < nvmSeclabels; j++) {
+            if (STRNEQ_NULLABLE(vmSeclabels[j]->model, seclabel->model))
+                continue;
+
+            if (!vmSeclabels[j]->relabel) {
+                virReportError(VIR_ERR_XML_ERROR, "%s",
+                               _("label overrides require relabeling to be "
+                                 "enabled at the domain level"));
+                return -1;
+            }
+        }
+    }
+
+    return 0;
+}
+
+
+static int
+virDomainDiskDefValidate(const virDomainDef *def,
+                         const virDomainDiskDef *disk)
 {
+    virStorageSourcePtr next;
+
     /* Validate LUN configuration */
     if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) {
         /* volumes haven't been translated at this point, so accept them */
@@ -5999,6 +6033,14 @@ virDomainDiskDefValidate(const virDomainDiskDef *disk)
         return -1;
     }
 
+    for (next = disk->src; next; next = next->backingStore) {
+        if (virSecurityDeviceLabelDefValidateXML(next->seclabels,
+                                                 next->nseclabels,
+                                                 def->seclabels,
+                                                 def->nseclabels) < 0)
+            return -1;
+    }
+
     return 0;
 }
 
@@ -6022,10 +6064,11 @@ virDomainDefHasUSB(const virDomainDef *def)
 
 
 static int
-virDomainChrSourceDefValidate(const virDomainChrSourceDef *def,
-                              const virDomainChrDef *chr_def)
+virDomainChrSourceDefValidate(const virDomainChrSourceDef *src_def,
+                              const virDomainChrDef *chr_def,
+                              const virDomainDef *def)
 {
-    switch ((virDomainChrType) def->type) {
+    switch ((virDomainChrType) src_def->type) {
     case VIR_DOMAIN_CHR_TYPE_NULL:
     case VIR_DOMAIN_CHR_TYPE_PTY:
     case VIR_DOMAIN_CHR_TYPE_VC:
@@ -6037,7 +6080,7 @@ virDomainChrSourceDefValidate(const virDomainChrSourceDef *def,
     case VIR_DOMAIN_CHR_TYPE_FILE:
     case VIR_DOMAIN_CHR_TYPE_DEV:
     case VIR_DOMAIN_CHR_TYPE_PIPE:
-        if (!def->data.file.path) {
+        if (!src_def->data.file.path) {
             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                            _("Missing source path attribute for char device"));
             return -1;
@@ -6045,13 +6088,13 @@ virDomainChrSourceDefValidate(const virDomainChrSourceDef *def,
         break;
 
     case VIR_DOMAIN_CHR_TYPE_NMDM:
-        if (!def->data.nmdm.master) {
+        if (!src_def->data.nmdm.master) {
             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                            _("Missing master path attribute for nmdm device"));
             return -1;
         }
 
-        if (!def->data.nmdm.slave) {
+        if (!src_def->data.nmdm.slave) {
             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                            _("Missing slave path attribute for nmdm device"));
             return -1;
@@ -6059,19 +6102,19 @@ virDomainChrSourceDefValidate(const virDomainChrSourceDef *def,
         break;
 
     case VIR_DOMAIN_CHR_TYPE_TCP:
-        if (!def->data.tcp.host) {
+        if (!src_def->data.tcp.host) {
             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                            _("Missing source host attribute for char device"));
             return -1;
         }
 
-        if (!def->data.tcp.service) {
+        if (!src_def->data.tcp.service) {
             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                            _("Missing source service attribute for char device"));
             return -1;
         }
 
-        if (def->data.tcp.listen && def->data.tcp.reconnect.enabled) {
+        if (src_def->data.tcp.listen && src_def->data.tcp.reconnect.enabled) {
             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                            _("chardev reconnect is possible only for connect mode"));
             return -1;
@@ -6079,7 +6122,7 @@ virDomainChrSourceDefValidate(const virDomainChrSourceDef *def,
         break;
 
     case VIR_DOMAIN_CHR_TYPE_UDP:
-        if (!def->data.udp.connectService) {
+        if (!src_def->data.udp.connectService) {
             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                            _("Missing source service attribute for char device"));
             return -1;
@@ -6090,7 +6133,7 @@ virDomainChrSourceDefValidate(const virDomainChrSourceDef *def,
         /* The source path can be auto generated for certain specific
          * types of channels, but in most cases we should report an
          * error if the user didn't provide it */
-        if (!def->data.nix.path &&
+        if (!src_def->data.nix.path &&
             !(chr_def &&
               chr_def->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL &&
               (chr_def->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN ||
@@ -6100,7 +6143,7 @@ virDomainChrSourceDefValidate(const virDomainChrSourceDef *def,
             return -1;
         }
 
-        if (def->data.nix.listen && def->data.nix.reconnect.enabled) {
+        if (src_def->data.nix.listen && src_def->data.nix.reconnect.enabled) {
             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                            _("chardev reconnect is possible only for connect mode"));
             return -1;
@@ -6108,13 +6151,13 @@ virDomainChrSourceDefValidate(const virDomainChrSourceDef *def,
         break;
 
     case VIR_DOMAIN_CHR_TYPE_SPICEPORT:
-        if (!def->data.spiceport.channel) {
+        if (!src_def->data.spiceport.channel) {
             virReportError(VIR_ERR_XML_ERROR, "%s",
                            _("Missing source channel attribute for char device"));
             return -1;
         }
-        if (strspn(def->data.spiceport.channel,
-                   SERIAL_CHANNEL_NAME_CHARS) < strlen(def->data.spiceport.channel)) {
+        if (strspn(src_def->data.spiceport.channel,
+                   SERIAL_CHANNEL_NAME_CHARS) < strlen(src_def->data.spiceport.channel)) {
             virReportError(VIR_ERR_INVALID_ARG, "%s",
                            _("Invalid character in source channel for char device"));
             return -1;
@@ -6122,6 +6165,12 @@ virDomainChrSourceDefValidate(const virDomainChrSourceDef *def,
         break;
     }
 
+    if (virSecurityDeviceLabelDefValidateXML(src_def->seclabels,
+                                             src_def->nseclabels,
+                                             def->seclabels,
+                                             def->nseclabels) < 0)
+        return -1;
+
     return 0;
 }
 
@@ -6138,7 +6187,7 @@ virDomainRedirdevDefValidate(const virDomainDef *def,
         return -1;
     }
 
-    return virDomainChrSourceDefValidate(redirdev->source, NULL);
+    return virDomainChrSourceDefValidate(redirdev->source, NULL, def);
 }
 
 
@@ -6261,27 +6310,30 @@ virDomainControllerDefValidate(const virDomainControllerDef *controller)
 
 
 static int
-virDomainChrDefValidate(const virDomainChrDef *chr)
+virDomainChrDefValidate(const virDomainChrDef *chr,
+                        const virDomainDef *def)
 {
-    return virDomainChrSourceDefValidate(chr->source, chr);
+    return virDomainChrSourceDefValidate(chr->source, chr, def);
 }
 
 
 static int
-virDomainSmartcardDefValidate(const virDomainSmartcardDef *smartcard)
+virDomainSmartcardDefValidate(const virDomainSmartcardDef *smartcard,
+                              const virDomainDef *def)
 {
     if (smartcard->type == VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH)
-        return virDomainChrSourceDefValidate(smartcard->data.passthru, NULL);
+        return virDomainChrSourceDefValidate(smartcard->data.passthru, NULL, def);
 
     return 0;
 }
 
 
 static int
-virDomainRNGDefValidate(const virDomainRNGDef *rng)
+virDomainRNGDefValidate(const virDomainRNGDef *rng,
+                        const virDomainDef *def)
 {
     if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD)
-        return virDomainChrSourceDefValidate(rng->source.chardev, NULL);
+        return virDomainChrSourceDefValidate(rng->source.chardev, NULL, def);
 
     return 0;
 }
@@ -6478,7 +6530,7 @@ virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev,
 {
     switch ((virDomainDeviceType) dev->type) {
     case VIR_DOMAIN_DEVICE_DISK:
-        return virDomainDiskDefValidate(dev->data.disk);
+        return virDomainDiskDefValidate(def, dev->data.disk);
 
     case VIR_DOMAIN_DEVICE_REDIRDEV:
         return virDomainRedirdevDefValidate(def, dev->data.redirdev);
@@ -6490,13 +6542,13 @@ virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev,
         return virDomainControllerDefValidate(dev->data.controller);
 
     case VIR_DOMAIN_DEVICE_CHR:
-        return virDomainChrDefValidate(dev->data.chr);
+        return virDomainChrDefValidate(dev->data.chr, def);
 
     case VIR_DOMAIN_DEVICE_SMARTCARD:
-        return virDomainSmartcardDefValidate(dev->data.smartcard);
+        return virDomainSmartcardDefValidate(dev->data.smartcard, def);
 
     case VIR_DOMAIN_DEVICE_RNG:
-        return virDomainRNGDefValidate(dev->data.rng);
+        return virDomainRNGDefValidate(dev->data.rng, def);
 
     case VIR_DOMAIN_DEVICE_HOSTDEV:
         return virDomainHostdevDefValidate(dev->data.hostdev);
@@ -9052,37 +9104,6 @@ virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn,
 }
 
 
-static int
-virSecurityDeviceLabelDefValidateXML(virSecurityDeviceLabelDefPtr *seclabels,
-                                     size_t nseclabels,
-                                     virSecurityLabelDefPtr *vmSeclabels,
-                                     size_t nvmSeclabels)
-{
-    virSecurityDeviceLabelDefPtr seclabel;
-    size_t i;
-    size_t j;
-
-    for (i = 0; i < nseclabels; i++) {
-        seclabel = seclabels[i];
-
-        /* find the security label that it's being overridden */
-        for (j = 0; j < nvmSeclabels; j++) {
-            if (STRNEQ_NULLABLE(vmSeclabels[j]->model, seclabel->model))
-                continue;
-
-            if (!vmSeclabels[j]->relabel) {
-                virReportError(VIR_ERR_XML_ERROR, "%s",
-                               _("label overrides require relabeling to be "
-                                 "enabled at the domain level"));
-                return -1;
-            }
-        }
-    }
-
-    return 0;
-}
-
-
 /* Parse the XML definition for a lease
  */
 static virDomainLeaseDefPtr
@@ -9711,9 +9732,7 @@ virDomainDiskSourceDefParseAuthValidate(const virStorageSource *src)
 
 
 static int
-virDomainDiskDefParseValidate(const virDomainDiskDef *def,
-                              virSecurityLabelDefPtr *vmSeclabels,
-                              size_t nvmSeclabels)
+virDomainDiskDefParseValidate(const virDomainDiskDef *def)
 
 {
     virStorageSourcePtr next;
@@ -9804,12 +9823,6 @@ virDomainDiskDefParseValidate(const virDomainDiskDef *def,
                 return -1;
             }
         }
-
-        if (virSecurityDeviceLabelDefValidateXML(next->seclabels,
-                                                 next->nseclabels,
-                                                 vmSeclabels,
-                                                 nvmSeclabels) < 0)
-            return -1;
     }
 
     return 0;
@@ -9966,8 +9979,6 @@ static virDomainDiskDefPtr
 virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
                          xmlNodePtr node,
                          xmlXPathContextPtr ctxt,
-                         virSecurityLabelDefPtr* vmSeclabels,
-                         int nvmSeclabels,
                          unsigned int flags)
 {
     virDomainDiskDefPtr def;
@@ -10370,7 +10381,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
         virDomainDiskDefParsePrivateData(ctxt, def, xmlopt) < 0)
         goto error;
 
-    if (virDomainDiskDefParseValidate(def, vmSeclabels, nvmSeclabels) < 0)
+    if (virDomainDiskDefParseValidate(def) < 0)
         goto error;
 
  cleanup:
@@ -12660,9 +12671,7 @@ static int
 virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
                               xmlNodePtr cur, unsigned int flags,
                               virDomainChrDefPtr chr_def,
-                              xmlXPathContextPtr ctxt,
-                              virSecurityLabelDefPtr* vmSeclabels,
-                              int nvmSeclabels)
+                              xmlXPathContextPtr ctxt)
 {
     bool logParsed = false;
     bool protocolParsed = false;
@@ -12745,11 +12754,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
                 if (virSecurityDeviceLabelDefParseXML(&def->seclabels,
                                                       &def->nseclabels,
                                                       ctxt,
-                                                      flags) < 0 ||
-                    virSecurityDeviceLabelDefValidateXML(def->seclabels,
-                                                         def->nseclabels,
-                                                         vmSeclabels,
-                                                         nvmSeclabels) < 0) {
+                                                      flags) < 0) {
                     ctxt->node = saved_node;
                     goto error;
                 }
@@ -12886,8 +12891,6 @@ static virDomainChrDefPtr
 virDomainChrDefParseXML(virDomainXMLOptionPtr xmlopt,
                         xmlXPathContextPtr ctxt,
                         xmlNodePtr node,
-                        virSecurityLabelDefPtr* vmSeclabels,
-                        int nvmSeclabels,
                         unsigned int flags)
 {
     xmlNodePtr cur;
@@ -12934,7 +12937,7 @@ virDomainChrDefParseXML(virDomainXMLOptionPtr xmlopt,
         goto error;
 
     if (virDomainChrSourceDefParseXML(def->source, node->children, flags, def,
-                                      ctxt, vmSeclabels, nvmSeclabels) < 0)
+                                      ctxt) < 0)
         goto error;
 
     if (def->source->type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) {
@@ -13065,7 +13068,7 @@ virDomainSmartcardDefParseXML(virDomainXMLOptionPtr xmlopt,
 
         cur = node->children;
         if (virDomainChrSourceDefParseXML(def->data.passthru, cur, flags,
-                                          NULL, ctxt, NULL, 0) < 0)
+                                          NULL, ctxt) < 0)
             goto error;
 
         if (def->data.passthru->type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) {
@@ -14740,7 +14743,7 @@ virDomainRNGDefParseXML(virDomainXMLOptionPtr xmlopt,
 
         if (virDomainChrSourceDefParseXML(def->source.chardev,
                                           backends[0]->children, flags,
-                                          NULL, ctxt, NULL, 0) < 0)
+                                          NULL, ctxt) < 0)
             goto error;
         break;
 
@@ -15761,7 +15764,7 @@ virDomainRedirdevDefParseXML(virDomainXMLOptionPtr xmlopt,
     /* boot gets parsed in virDomainDeviceInfoParseXML
      * source gets parsed in virDomainChrSourceDefParseXML */
     if (virDomainChrSourceDefParseXML(def->source, cur, flags,
-                                      NULL, ctxt, NULL, 0) < 0)
+                                      NULL, ctxt) < 0)
         goto error;
 
     if (def->source->type == VIR_DOMAIN_CHR_TYPE_SPICEVMC)
@@ -16461,8 +16464,6 @@ virDomainDeviceDefParse(const char *xmlStr,
     switch ((virDomainDeviceType) dev->type) {
     case VIR_DOMAIN_DEVICE_DISK:
         if (!(dev->data.disk = virDomainDiskDefParseXML(xmlopt, node, ctxt,
-                                                        def->seclabels,
-                                                        def->nseclabels,
                                                         flags)))
             return NULL;
         break;
@@ -16532,8 +16533,6 @@ virDomainDeviceDefParse(const char *xmlStr,
         if (!(dev->data.chr = virDomainChrDefParseXML(xmlopt,
                                                       ctxt,
                                                       node,
-                                                      def->seclabels,
-                                                      def->nseclabels,
                                                       flags)))
             return NULL;
         break;
@@ -16600,14 +16599,11 @@ virDomainDeviceDefParse(const char *xmlStr,
 
 virDomainDiskDefPtr
 virDomainDiskDefParse(const char *xmlStr,
-                      const virDomainDef *def,
                       virDomainXMLOptionPtr xmlopt,
                       unsigned int flags)
 {
     g_autoptr(xmlDoc) xml = NULL;
     g_autoptr(xmlXPathContext) ctxt = NULL;
-    virSecurityLabelDefPtr *seclabels = NULL;
-    size_t nseclabels = 0;
 
     if (!(xml = virXMLParseStringCtxt(xmlStr, _("(disk_definition)"), &ctxt)))
         return NULL;
@@ -16619,13 +16615,7 @@ virDomainDiskDefParse(const char *xmlStr,
         return NULL;
     }
 
-    if (def) {
-        seclabels = def->seclabels;
-        nseclabels = def->nseclabels;
-    }
-
-    return virDomainDiskDefParseXML(xmlopt, ctxt->node, ctxt,
-                                    seclabels, nseclabels, flags);
+    return virDomainDiskDefParseXML(xmlopt, ctxt->node, ctxt, flags);
 }
 
 
@@ -20815,8 +20805,6 @@ virDomainDefParseXML(xmlDocPtr xml,
         virDomainDiskDefPtr disk = virDomainDiskDefParseXML(xmlopt,
                                                             nodes[i],
                                                             ctxt,
-                                                            def->seclabels,
-                                                            def->nseclabels,
                                                             flags);
         if (!disk)
             goto error;
@@ -20967,8 +20955,6 @@ virDomainDefParseXML(xmlDocPtr xml,
         virDomainChrDefPtr chr = virDomainChrDefParseXML(xmlopt,
                                                          ctxt,
                                                          nodes[i],
-                                                         def->seclabels,
-                                                         def->nseclabels,
                                                          flags);
         if (!chr)
             goto error;
@@ -20995,8 +20981,6 @@ virDomainDefParseXML(xmlDocPtr xml,
         virDomainChrDefPtr chr = virDomainChrDefParseXML(xmlopt,
                                                          ctxt,
                                                          nodes[i],
-                                                         def->seclabels,
-                                                         def->nseclabels,
                                                          flags);
         if (!chr)
             goto error;
@@ -21025,8 +21009,6 @@ virDomainDefParseXML(xmlDocPtr xml,
         virDomainChrDefPtr chr = virDomainChrDefParseXML(xmlopt,
                                                          ctxt,
                                                          nodes[i],
-                                                         def->seclabels,
-                                                         def->nseclabels,
                                                          flags);
         if (!chr)
             goto error;
@@ -21045,8 +21027,6 @@ virDomainDefParseXML(xmlDocPtr xml,
         virDomainChrDefPtr chr = virDomainChrDefParseXML(xmlopt,
                                                          ctxt,
                                                          nodes[i],
-                                                         def->seclabels,
-                                                         def->nseclabels,
                                                          flags);
         if (!chr)
             goto error;

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 5a149792f3ee7437f342d0fc7124b0b85963f0b6..55554fe68140f043b12a1527b2e975ad595cac60 100644 (file)
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3047,7 +3047,6 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(const char *xmlStr,
                                               void *parseOpaque,
                                               unsigned int flags);
 virDomainDiskDefPtr virDomainDiskDefParse(const char *xmlStr,
-                                          const virDomainDef *def,
                                           virDomainXMLOptionPtr xmlopt,
                                           unsigned int flags);
 virDomainDefPtr virDomainDefParseString(const char *xmlStr,

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index cd62f8753230040a7252e0d083448c31ffee4afc..dce106f84ab1b5a04a7d1e09e52f3b6d06d640e8 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18477,7 +18477,7 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *disk, const char *destxml,
         }
     }
 
-    if (!(diskdef = virDomainDiskDefParse(destxml, vm->def, driver->xmlopt,
+    if (!(diskdef = virDomainDiskDefParse(destxml, driver->xmlopt,
                                           VIR_DOMAIN_DEF_PARSE_INACTIVE |
                                           VIR_DOMAIN_DEF_PARSE_DISK_SOURCE)))
         goto cleanup;

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index fcdbcefb5d3f1e5742c9eb0e415f3f498c715454..2c170548ec5dfd50a2f9100e6e054aa14e3fbf9d 100644 (file)
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -200,7 +200,7 @@ testQemuDiskXMLToProps(const void *opaque)
         goto cleanup;
 
     /* qemu stores node names in the status XML portion */
-    if (!(disk = virDomainDiskDefParse(xmlstr, NULL, data->driver->xmlopt,
+    if (!(disk = virDomainDiskDefParse(xmlstr, data->driver->xmlopt,
                                        VIR_DOMAIN_DEF_PARSE_STATUS)))
         goto cleanup;