On Fri, Oct 30, 2026 at 12:04:56AM -0400, Laine Stump wrote:
On 10/26/26 8:18 AM, Daniel P. Berrangé via Devel wrote:
From: Daniel P. Berrangé <berrange@redhat.com>
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é <berrange@redhat.com> --- 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.
This is validating the name part of the TXT record, which IIUC, still has to comply with the naming rules for DNS records. Only the value is free-form unrestricted octets. I did, however, create a bug. A "_" is permitted in the TXT record name and required in a SRV record. With regards, Daniel -- |: https://berrange.com ~~ https://hachyderm.io/@berrange :| |: https://libvirt.org ~~ https://entangle-photo.org :| |: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|