domain: Fix unknown flags diagnosis in virDomainGetXMLDesc

Many drivers had a comment that they did not validate the incoming
'flags' to virDomainGetXMLDesc() because they were relying on
virDomainDefFormat() to do it instead. This used to be the case
(at least since 461e0f1a and friends in 0.9.4 added unknown flag
checking in general), but regressed in commit 0ecd6851 (1.2.12),
when all of the drivers were changed to pass 'flags' through the
new helper virDomainDefFormatConvertXMLFlags(). Since this helper
silently ignores unknown flags, we need to implement flag checking
in each driver instead.

Annoyingly, this means that any new flag values added will silently
be ignored when targeting an older libvirt, rather than our usual
practice of loudly diagnosing an unsupported flag.  Add comments
in domain_conf.[ch] to remind us to be extra vigilant about the
impact when adding flags (a new flag to add data is safe if the
older server omitting the requested data doesn't break things in
the newer client; a new flag to suppress data rather than enhancing
the existing VIR_DOMAIN_XML_SECURE may form a data leak or even a
security hole).

In the qemu driver, there are multiple callers all funnelling to
qemuDomainDefFormatBufInternal(); many of them already validated
flags (and often only a subset of the full set of possible flags),
but for ease of maintenance, we can also check flags at the common
helper function.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index 912797cfcf8fc4334bc4ec26175e59e572f00cb8..3e192284cc2c2af6e4553fa9a505e5d0eced6459 100644 (file)
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -484,6 +484,8 @@ bhyveDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
     virCapsPtr caps = NULL;
     char *ret = NULL;
 
+    virCheckFlags(VIR_DOMAIN_XML_COMMON_FLAGS, NULL);
+
     if (!(vm = bhyveDomObjFromDomain(domain)))
         goto cleanup;
 

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 51eea46b4e1a21b40d46aaa9432ecee94a400b87..56c437ca0a34c9dc7f9b46bec17693d479f2f00b 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -29083,6 +29083,11 @@ virDomainDefFormatInternal(virDomainDefPtr def,
     return -1;
 }
 
+/* Converts VIR_DOMAIN_XML_COMMON_FLAGS into VIR_DOMAIN_DEF_FORMAT_*
+ * flags, and silently ignores any other flags.  Note that the caller
+ * should validate the set of flags it is willing to accept; see also
+ * the comment on VIR_DOMAIN_XML_COMMON_FLAGS about security
+ * considerations with adding new flags. */
 unsigned int virDomainDefFormatConvertXMLFlags(unsigned int flags)
 {
     unsigned int formatFlags = 0;

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 2bc3f879f7ad43fc869a00e86d079d3567bf776c..9bccd8bcd15cb2a79669549f2f3d195f967e4911 100644 (file)
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3110,6 +3110,15 @@ virDomainIOThreadIDDefPtr virDomainIOThreadIDAdd(virDomainDefPtr def,
                                                  unsigned int iothread_id);
 void virDomainIOThreadIDDel(virDomainDefPtr def, unsigned int iothread_id);
 
+/* When extending this list, remember that libvirt 1.2.12-5.0.0 had a
+ * bug that silently ignored unknown flags.  A new flag to add
+ * information is okay as long as clients still work when an older
+ * server omits the requested output, but a new flag to suppress
+ * information could result in a security hole when older libvirt
+ * supplies the sensitive information in spite of the flag. */
+# define VIR_DOMAIN_XML_COMMON_FLAGS \
+    (VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_INACTIVE | \
+     VIR_DOMAIN_XML_MIGRATABLE)
 unsigned int virDomainDefFormatConvertXMLFlags(unsigned int flags);
 
 char *virDomainDefFormat(virDomainDefPtr def,

diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index b1af6461554655d1e7bb7b9d2b299c2f79e7743b..379c2bae730a91c7f5cf3812e6bbf1f4a1e7243c 100644 (file)
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -2604,7 +2604,7 @@ esxDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
     virDomainDefPtr def = NULL;
     char *xml = NULL;
 
-    /* Flags checked by virDomainDefFormat */
+    virCheckFlags(VIR_DOMAIN_XML_COMMON_FLAGS, NULL);
 
     memset(&data, 0, sizeof(data));
 

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index f41cd1c939f7d3deba6f5f383d88278cb18127b3..0e2c6c55ef31a83c6d2039839a54733204473480 100644 (file)
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -754,7 +754,7 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
     Msvm_ProcessorSettingData *processorSettingData = NULL;
     Msvm_MemorySettingData *memorySettingData = NULL;
 
-    /* Flags checked by virDomainDefFormat */
+    virCheckFlags(VIR_DOMAIN_XML_COMMON_FLAGS, NULL);
 
     if (!(def = virDomainDefNew()))
         goto cleanup;

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 7981ccaf212f95951adafbefbef816151f2a4224..31b842aeee73584c9285016f0ac49a09e6492498 100644 (file)
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2621,7 +2621,7 @@ libxlDomainGetXMLDesc(virDomainPtr dom, unsigned int flags)
     virDomainDefPtr def;
     char *ret = NULL;
 
-    /* Flags checked by virDomainDefFormat */
+    virCheckFlags(VIR_DOMAIN_XML_COMMON_FLAGS, NULL);
 
     if (!(vm = libxlDomObjFromDomain(dom)))
         goto cleanup;

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index c48f6d9067ce7038faa5395542f852cf4d5b5cdd..516a6b4de3810a131d905cb453b43d768bed4752 100644 (file)
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -987,7 +987,7 @@ static char *lxcDomainGetXMLDesc(virDomainPtr dom,
     virDomainObjPtr vm;
     char *ret = NULL;
 
-    /* Flags checked by virDomainDefFormat */
+    virCheckFlags(VIR_DOMAIN_XML_COMMON_FLAGS, NULL);
 
     if (!(vm = lxcDomObjFromDomain(dom)))
         goto cleanup;

diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index 06950ce9ed944778f352bc50f3cc34984ab5c98a..39eeb2c12e7ebdd802962f41ca963f222c7f2c75 100644 (file)
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -519,7 +519,7 @@ static char *openvzDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) {
     virDomainObjPtr vm;
     char *ret = NULL;
 
-    /* Flags checked by virDomainDefFormat */
+    virCheckFlags(VIR_DOMAIN_XML_COMMON_FLAGS, NULL);
 
     if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
         return NULL;

diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index dc082b1d0846aeb11511b7331fb40fe8b550916e..e54799dbb49d76a1887d23f6502bfc779cb44437 100644 (file)
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -3214,7 +3214,7 @@ phypDomainGetXMLDesc(virDomainPtr dom, unsigned int flags)
     unsigned long long memory;
     unsigned int vcpus;
 
-    /* Flags checked by virDomainDefFormat */
+    virCheckFlags(VIR_DOMAIN_XML_COMMON_FLAGS, NULL);
 
     memset(&def, 0, sizeof(virDomainDef));
 

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index a0af56695e9cd21c26f10cb723f00212e9f91a42..bbd4a5efe8d84d89664ceaf5e0d78c99c31e0271 100644 (file)
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7725,6 +7725,8 @@ qemuDomainDefFormatBufInternal(virQEMUDriverPtr driver,
     virCapsPtr caps = NULL;
     virQEMUCapsPtr qemuCaps = NULL;
 
+    virCheckFlags(VIR_DOMAIN_XML_COMMON_FLAGS | VIR_DOMAIN_XML_UPDATE_CPU, -1);
+
     if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
         goto cleanup;
 

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5118f4ad42f465019a6de37fbd628536943f8365..2c8bcde10bafbef282d049245244152c3b7daea6 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7339,7 +7339,8 @@ static char
     virDomainObjPtr vm;
     char *ret = NULL;
 
-    /* Flags checked by virDomainDefFormat */
+    virCheckFlags(VIR_DOMAIN_XML_COMMON_FLAGS | VIR_DOMAIN_XML_UPDATE_CPU,
+                  NULL);
 
     if (!(vm = qemuDomObjFromDomain(dom)))
         goto cleanup;

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index c1faff46ff058748e5ca55d05d665eb910c329bd..cde9e3d4173fd60c6297b83ea0796bbd2e0f4620 100644 (file)
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -2628,7 +2628,7 @@ static char *testDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
     virDomainObjPtr privdom;
     char *ret = NULL;
 
-    /* Flags checked by virDomainDefFormat */
+    virCheckFlags(VIR_DOMAIN_XML_COMMON_FLAGS, NULL);
 
     if (!(privdom = testDomObjFromDomain(domain)))
         return NULL;

diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 664650f217fd4405058556c8d18cfea3e1a9f4eb..d95fe7c7ae677d68e30b9d3ed856537dbb0bbfb9 100644 (file)
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -4052,7 +4052,7 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags)
     if (!data->vboxObj)
         return ret;
 
-    /* Flags checked by virDomainDefFormat */
+    virCheckFlags(VIR_DOMAIN_XML_COMMON_FLAGS, NULL);
 
     if (openSessionForMachine(data, dom->uuid, &iid, &machine) < 0)
         goto cleanup;

diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c
index f94b3252fe750c1ea9376f15ea8ee76956b3605f..f4b0989afd4c5173a9991dfb9e5d65b4d0d8d3a4 100644 (file)
--- a/src/vmware/vmware_driver.c
+++ b/src/vmware/vmware_driver.c
@@ -932,7 +932,7 @@ vmwareDomainGetXMLDesc(virDomainPtr dom, unsigned int flags)
     virDomainObjPtr vm;
     char *ret = NULL;
 
-    /* Flags checked by virDomainDefFormat */
+    virCheckFlags(VIR_DOMAIN_XML_COMMON_FLAGS, NULL);
 
     if (!(vm = vmwareDomObjFromDomain(driver, dom->uuid)))
         return NULL;

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index b22a44d6adc4be6a58110d681a7ab8fac4fdfb42..f99ade82b657cdf3cb1df39a668a95047453af16 100644 (file)
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -724,7 +724,7 @@ vzDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
     virDomainObjPtr dom;
     char *ret = NULL;
 
-    /* Flags checked by virDomainDefFormat */
+    virCheckFlags(VIR_DOMAIN_XML_COMMON_FLAGS, NULL);
 
     if (!(dom = vzDomObjFromDomain(domain)))
         return NULL;