On 10/26/26 8:18 AM, Daniel P. Berrangé via Devel wrote:
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);
I think it might be "a bridge too far" to put all these hostname
restrictions on a TXT record. According to the RFCs any octet is allowed
in TXT records (technically any octet is allowed even in A and AAAA
records, and it's up to the application retrieving the info from the DNS
server to validate it, but for those I think we can safely say that
we're an application using a DNS server and so it's okay for us to fully
validate A and AAAA records as hostnames. I don't think that's true for
TXT records.
Other than that:
Reviewed-by: Laine Stump <[email protected]>
+ 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,
};
