From: Daniel P. Berrangé <[email protected]> The current network XML parser only validates that a DHCP hostname starts with a-Z character or a digit, or that a TXT record name doesn't contain space or comma. This is way too weak, and misses validation from other parts of the schema.
This can allow invalid characters that result in invalid syntax in dnsmasq config files. Use the new hostname validation API to check all places that accept a hostname or domain name as a hardening task to prevent accidental creation of malformed dnsmasq cnofig files. Fixes: https://gitlab.com/libvirt/libvirt/-/work_items/929 Signed-off-by: Daniel P. Berrangé <[email protected]> --- src/conf/network_conf.c | 31 +++++++++---------- .../nat-network-bad-hostname.xml | 22 +++++++++++++ .../nat-network-dhcp-hosts-bad.xml | 21 +++++++++++++ .../nat-network-dns-hosts-bad.xml | 26 ++++++++++++++++ tests/networkxmlconftest.c | 3 ++ tests/virschematest.c | 3 ++ 6 files changed, 90 insertions(+), 16 deletions(-) create mode 100644 tests/networkxmlconfdata/nat-network-bad-hostname.xml create mode 100644 tests/networkxmlconfdata/nat-network-dhcp-hosts-bad.xml create mode 100644 tests/networkxmlconfdata/nat-network-dns-hosts-bad.xml diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 8eba1321d6..2fec4d2b91 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -35,6 +35,7 @@ #include "viruuid.h" #include "virbuffer.h" #include "virstring.h" +#include "virutil.h" #define VIR_FROM_THIS VIR_FROM_NETWORK @@ -533,10 +534,7 @@ virNetworkDHCPHostDefParseXML(const char *networkName, } name = virXMLPropString(node, "name"); - if (name && !(g_ascii_isalpha(name[0]) || g_ascii_isdigit(name[0]))) { - virReportError(VIR_ERR_XML_ERROR, - _("Cannot use host name '%1$s' in network '%2$s'"), - name, networkName); + if (name && virHostnameValidate(name, true) < 0) { return -1; } @@ -664,10 +662,7 @@ virNetworkDNSHostDefParseXML(const char *networkName, if (!(hostnames[i] = virXMLNodeContentString(g_ptr_array_index(hostnameNodes, i)))) return -1; - if (*hostnames[i] == '\0') { - virReportError(VIR_ERR_XML_DETAIL, - _("Missing hostname in network '%1$s' DNS HOST record"), - networkName); + if (virHostnameValidate(hostnames[i], true) < 0) { return -1; } } @@ -792,8 +787,11 @@ virNetworkDNSSrvDefParseXML(const char *networkName, def->domain = virXMLPropString(node, "domain"); def->target = virXMLPropString(node, "target"); - if (virNetworkDNSDefCheckLineBreaks("SRV", "domain", def->domain) < 0 || - virNetworkDNSDefCheckLineBreaks("SRV", "target", def->target) < 0) + if (def->domain && + virHostnameValidate(def->domain, true) < 0) + goto error; + if (def->target && STRNEQ(def->target, ".") && + virHostnameValidate(def->target, true) < 0) goto error; ret = virXPathUInt("string(./@port)", ctxt, &def->port); @@ -852,18 +850,13 @@ virNetworkDNSTxtDefParseXML(const char *networkName, virNetworkDNSTxtDef *def, bool partialOkay) { - const char *bad = " ,"; - if (!(def->name = virXMLPropString(node, "name"))) { virReportError(VIR_ERR_XML_DETAIL, _("missing required name attribute in DNS TXT record of network %1$s"), networkName); goto error; } - if (strcspn(def->name, bad) != strlen(def->name)) { - virReportError(VIR_ERR_XML_DETAIL, - _("prohibited character in DNS TXT record name '%1$s' of network %2$s"), - def->name, networkName); + if (virHostnameValidate(def->name, true) < 0) { goto error; } if (!(def->value = virXMLPropString(node, "value")) && !partialOkay) { @@ -958,6 +951,9 @@ virNetworkDNSDefParseXML(const char *networkName, _("Invalid forwarder element, must contain at least one of addr or domain")); return -1; } + if (def->forwarders[i].domain && + virHostnameValidate(def->forwarders[i].domain, true) < 0) + return -1; def->nfwds++; } } @@ -1621,6 +1617,9 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt, /* Parse network domain information */ domain_node = virXPathNode("./domain[1]", ctxt); def->domain = virXMLPropString(domain_node, "name"); + if (def->domain && + virHostnameValidate(def->domain, true) < 0) + return NULL; if (virXMLPropTristateBool(domain_node, "localOnly", VIR_XML_PROP_NONE, &def->domainLocalOnly) < 0) diff --git a/tests/networkxmlconfdata/nat-network-bad-hostname.xml b/tests/networkxmlconfdata/nat-network-bad-hostname.xml new file mode 100644 index 0000000000..24f823f0e6 --- /dev/null +++ b/tests/networkxmlconfdata/nat-network-bad-hostname.xml @@ -0,0 +1,22 @@ +<network> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <bridge name="virbr0"/> + <forward mode="nat" dev="eth1"/> + <domain name="example.com fish"/> + <ip address="192.168.122.1" netmask="255.255.255.0"> + <dhcp> + <range start="192.168.122.2" end="192.168.122.254"/> + <host mac="00:16:3e:77:e2:ed" name="a.example.com" ip="192.168.122.10"/> + <host mac="00:16:3e:3e:a9:1a" name="b.example.com" ip="192.168.122.11"/> + </dhcp> + </ip> + <ip family="ipv4" address="192.168.123.1" netmask="255.255.255.0"> + </ip> + <ip family="ipv6" address="2001:db8:ac10:fe01::1" prefix="64"> + </ip> + <ip family="ipv6" address="2001:db8:ac10:fd01::1" prefix="64"> + </ip> + <ip family="ipv4" address="10.24.10.1"> + </ip> +</network> diff --git a/tests/networkxmlconfdata/nat-network-dhcp-hosts-bad.xml b/tests/networkxmlconfdata/nat-network-dhcp-hosts-bad.xml new file mode 100644 index 0000000000..9d9711c7bf --- /dev/null +++ b/tests/networkxmlconfdata/nat-network-dhcp-hosts-bad.xml @@ -0,0 +1,21 @@ +<network> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <bridge name="virbr0"/> + <forward mode="nat" dev="eth1"/> + <ip address="192.168.122.1" netmask="255.255.255.0"> + <dhcp> + <range start="192.168.122.2" end="192.168.122.254"/> + <host mac="00:16:3e:77:e2:ed" name="a.example.com fish" ip="192.168.122.10"/> + <host mac="00:16:3e:3e:a9:1a" name="b.example.com" ip="192.168.122.11"/> + </dhcp> + </ip> + <ip family="ipv4" address="192.168.123.1" netmask="255.255.255.0"> + </ip> + <ip family="ipv6" address="2001:db8:ac10:fe01::1" prefix="64"> + </ip> + <ip family="ipv6" address="2001:db8:ac10:fd01::1" prefix="64"> + </ip> + <ip family="ipv4" address="10.24.10.1"> + </ip> +</network> diff --git a/tests/networkxmlconfdata/nat-network-dns-hosts-bad.xml b/tests/networkxmlconfdata/nat-network-dns-hosts-bad.xml new file mode 100644 index 0000000000..b5f43b21dc --- /dev/null +++ b/tests/networkxmlconfdata/nat-network-dns-hosts-bad.xml @@ -0,0 +1,26 @@ +<network> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9c</uuid> + <forward dev='eth0' mode='nat'/> + <bridge name='virbr0' stp='on' delay='0'/> + <domain name="example.com"/> + <dns forwardPlainNames='no'> + <host ip='192.168.122.122'> + <hostname>pudding fish</hostname> + </host> + <host ip='192.168.122.1'> + <hostname>host</hostname> + <hostname>gateway</hostname> + </host> + <host ip='192.168.122.2'> + <hostname>shared</hostname> + <hostname>names</hostname> + </host> + <host ip='fd8f:1391:3a82:150::c0a8:9603'> + <hostname>shared</hostname> + <hostname>names</hostname> + </host> + </dns> + <ip address='192.168.122.1' netmask='255.255.255.0'> + </ip> +</network> diff --git a/tests/networkxmlconftest.c b/tests/networkxmlconftest.c index c0d3886a69..eb60c19604 100644 --- a/tests/networkxmlconftest.c +++ b/tests/networkxmlconftest.c @@ -319,6 +319,7 @@ mymain(void) DO_TEST("open-network"); DO_TEST_PARSE_ERROR("open-network-with-forward-dev"); DO_TEST("nat-network"); + DO_TEST_PARSE_ERROR("nat-network-bad-hostname"); DO_TEST("netboot-network"); DO_TEST("netboot-proxy-network"); DO_TEST("netboot-tftp"); @@ -330,6 +331,8 @@ mymain(void) DO_TEST("nat-network-dns-srv-records"); DO_TEST("nat-network-dns-srv-record-minimal"); DO_TEST("nat-network-dns-hosts"); + DO_TEST_PARSE_ERROR("nat-network-dns-hosts-bad"); + DO_TEST_PARSE_ERROR("nat-network-dhcp-hosts-bad"); DO_TEST("nat-network-dns-forward-plain"); DO_TEST("nat-network-dns-forwarders"); DO_TEST("nat-network-dns-forwarder-no-resolv"); diff --git a/tests/virschematest.c b/tests/virschematest.c index fabe45b65d..e1fa82aa97 100644 --- a/tests/virschematest.c +++ b/tests/virschematest.c @@ -306,6 +306,9 @@ static const char *skip_networkxmlconfdata[] = { "nat-network-dns-srv-record-domain-newline.xml", "nat-network-dns-srv-record-target-newline.xml", "nat-network-dns-txt-recor-record-newline.xml", + "nat-network-bad-hostname.xml", + "nat-network-dhcp-hosts-bad.xml", + "nat-network-dns-hosts-bad.xml", NULL, }; -- 2.56.0
