From 21bfd1e9b9dacaca0c92eb14d62e4f643464fc31 Mon Sep 17 00:00:00 2001
From: Pavel Hrdina <phrdina@redhat.com>
Date: Fri, 18 Aug 2017 17:14:34 +0200
Subject: [PATCH] conf: error out for multiple source elements while parsing
 chardev

Currently we accept and correctly parse this chardev XML:

  ...
  <channel type='tcp'>
    <source mode='connect'/>
    <source mode='bind' host='localhost'/>
    <source service='4567'/>
    <target type='virtio' name='test'/>
  </channel>
  ...

The parsed formatted XML is:

  ...
  <channel type='tcp'>
    <source mode='connect' host='localhost' service='4567'/>
    <target type='virtio' name='test'/>
  </channel>
  ...

That behavior is super wrong and should not be allowed.  If you notice
the current parse takes the first found attribute and uses that value,
so for example from the "<source mode='bind' host='localhost'/>" only
the "host" attribute is used.  It works the same way for all possible
attributes that we are able to parse for source element.

This patch enforces providing only one source element for all character
devices, only for UDP type we allow to provide two source elements
since you can specify both modes.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 src/conf/domain_conf.c                        | 17 ++++++++++++
 .../generic-chardev-tcp-multiple-source.xml   | 26 +++++++++++++++++++
 .../generic-chardev-udp-multiple-source.xml   | 26 +++++++++++++++++++
 tests/genericxml2xmltest.c                    |  4 +++
 4 files changed, 73 insertions(+)
 create mode 100644 tests/genericxml2xmlindata/generic-chardev-tcp-multiple-source.xml
 create mode 100644 tests/genericxml2xmlindata/generic-chardev-udp-multiple-source.xml

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d77983069a..1c3034db05 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10925,12 +10925,29 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
     char *append = NULL;
     char *haveTLS = NULL;
     char *tlsFromConfig = NULL;
+    int sourceParsed = 0;
 
     for (; cur; cur = cur->next) {
         if (cur->type != XML_ELEMENT_NODE)
             continue;
 
         if (virXMLNodeNameEqual(cur, "source")) {
+            /* Parse only the first source element since only one is used
+             * for chardev devices, the only exception is UDP type, where
+             * user can specify two source elements. */
+            if (sourceParsed >= 1 && def->type != VIR_DOMAIN_CHR_TYPE_UDP) {
+                virReportError(VIR_ERR_XML_ERROR, "%s",
+                               _("only one source element is allowed for "
+                                 "character device"));
+                goto error;
+            } else if (sourceParsed >= 2) {
+                virReportError(VIR_ERR_XML_ERROR, "%s",
+                               _("only two source elements are allowed for "
+                                 "character device"));
+                goto error;
+            }
+            sourceParsed++;
+
             if (!mode)
                 mode = virXMLPropString(cur, "mode");
             if (!haveTLS)
diff --git a/tests/genericxml2xmlindata/generic-chardev-tcp-multiple-source.xml b/tests/genericxml2xmlindata/generic-chardev-tcp-multiple-source.xml
new file mode 100644
index 0000000000..bb8592aa4f
--- /dev/null
+++ b/tests/genericxml2xmlindata/generic-chardev-tcp-multiple-source.xml
@@ -0,0 +1,26 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-i686</emulator>
+    <controller type='virtio-serial' index='0'/>
+    <channel type='tcp'>
+      <source mode='bind' host='localhost' service='5678'/>
+      <source mode='connect' host='localhost' service='5678'/>
+      <protocol type='raw'/>
+      <target type='virtio' name='test1'/>
+    </channel>
+    <memballoon model='none'/>
+  </devices>
+</domain>
diff --git a/tests/genericxml2xmlindata/generic-chardev-udp-multiple-source.xml b/tests/genericxml2xmlindata/generic-chardev-udp-multiple-source.xml
new file mode 100644
index 0000000000..2b87a1bfaa
--- /dev/null
+++ b/tests/genericxml2xmlindata/generic-chardev-udp-multiple-source.xml
@@ -0,0 +1,26 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-i686</emulator>
+    <controller type='virtio-serial' index='0'/>
+    <channel type='udp'>
+      <source mode='connect' host='localhost' service='5678'/>
+      <source mode='bind' host='localhost' service='4567'/>
+      <source mode='connect' host='localhost' service='3456'/>
+      <target type='virtio' name='test1'/>
+    </channel>
+    <memballoon model='none'/>
+  </devices>
+</domain>
diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c
index 7dc137ed16..03913a68c9 100644
--- a/tests/genericxml2xmltest.c
+++ b/tests/genericxml2xmltest.c
@@ -110,9 +110,13 @@ mymain(void)
                  TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE);
     DO_TEST_FULL("chardev-tcp-missing-service", 0, false,
                  TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE);
+    DO_TEST_FULL("chardev-tcp-multiple-source", 0, false,
+                 TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE);
     DO_TEST_DIFFERENT("chardev-udp");
     DO_TEST_FULL("chardev-udp-missing-connect-service", 0, false,
                  TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE);
+    DO_TEST_FULL("chardev-udp-multiple-source", 0, false,
+                 TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE);
     DO_TEST_DIFFERENT("chardev-unix");
     DO_TEST_FULL("chardev-unix-smartcard-missing-path", 0, false,
                  TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE);