diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 5f081936e4..56c4a09d32 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -924,6 +924,21 @@ virNetworkDNSHostDefParseXML(const char *networkName, return -1; } +/* This includes all characters used in the names of current + * /etc/services and /etc/protocols files (on Fedora 20), except ".", + * which we can't allow because it would conflict with the use of "." + * as a field separator in the SRV record, there appears to be no way + * to escape it in, and the protocols/services that use "." in the + * name are obscure and unlikely to be used anyway. + */ +#define PROTOCOL_CHARS \ + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" \ + "-+/" + +#define SERVICE_CHARS \ + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" \ + "_-+/*" + static int virNetworkDNSSrvDefParseXML(const char *networkName, xmlNodePtr node, @@ -931,80 +946,108 @@ virNetworkDNSSrvDefParseXML(const char *networkName, virNetworkDNSSrvDefPtr def, bool partialOkay) { + int ret; + xmlNodePtr save_ctxt = ctxt->node; + + ctxt->node = node; + if (!(def->service = virXMLPropString(node, "service")) && !partialOkay) { virReportError(VIR_ERR_XML_DETAIL, - _("Missing required service attribute in DNS SRV record " - "of network %s"), networkName); + _("missing required service attribute in DNS SRV record " + "of network '%s'"), networkName); goto error; } - if (def->service && strlen(def->service) > DNS_RECORD_LENGTH_SRV) { - virReportError(VIR_ERR_XML_DETAIL, - _("Service name '%s' in network %s is too long, limit is %d bytes"), - def->service, networkName, DNS_RECORD_LENGTH_SRV); - goto error; + if (def->service) { + if (strlen(def->service) > DNS_RECORD_LENGTH_SRV) { + virReportError(VIR_ERR_XML_DETAIL, + _("service attribute '%s' in network '%s' is too long, " + "limit is %d bytes"), + def->service, networkName, DNS_RECORD_LENGTH_SRV); + goto error; + } + if (strspn(def->service, SERVICE_CHARS) < strlen(def->service)) { + virReportError(VIR_ERR_XML_DETAIL, + _("invalid character in service attribute '%s' " + "in DNS SRV record of network '%s'"), + def->service, networkName); + goto error; + } } if (!(def->protocol = virXMLPropString(node, "protocol")) && !partialOkay) { virReportError(VIR_ERR_XML_DETAIL, - _("Missing required protocol attribute " - "in dns srv record '%s' of network %s"), + _("missing required protocol attribute " + "in DNS SRV record '%s' of network '%s'"), def->service, networkName); goto error; } - - /* Check whether protocol value is the supported one */ - if (def->protocol && STRNEQ(def->protocol, "tcp") && - (STRNEQ(def->protocol, "udp"))) { + if (def->protocol && + strspn(def->protocol, PROTOCOL_CHARS) < strlen(def->protocol)) { virReportError(VIR_ERR_XML_DETAIL, - _("Invalid protocol attribute value '%s' " - "in DNS SRV record of network %s"), + _("invalid character in protocol attribute '%s' " + "in DNS SRV record of network '%s'"), def->protocol, networkName); goto error; } /* Following attributes are optional */ - if ((def->target = virXMLPropString(node, "target")) && - (def->domain = virXMLPropString(node, "domain"))) { - xmlNodePtr save_ctxt = ctxt->node; + def->domain = virXMLPropString(node, "domain"); + def->target = virXMLPropString(node, "target"); - ctxt->node = node; - if (virXPathUInt("string(./@port)", ctxt, &def->port) < 0 || - def->port > 65535) { - virReportError(VIR_ERR_XML_DETAIL, - _("Missing or invalid port attribute " - "in network %s"), networkName); - goto error; - } - - if (virXPathUInt("string(./@priority)", ctxt, &def->priority) < 0 || - def->priority > 65535) { - virReportError(VIR_ERR_XML_DETAIL, - _("Missing or invalid priority attribute " - "in network %s"), networkName); - goto error; - } - - if (virXPathUInt("string(./@weight)", ctxt, &def->weight) < 0 || - def->weight > 65535) { - virReportError(VIR_ERR_XML_DETAIL, - _("Missing or invalid weight attribute " - "in network %s"), networkName); - goto error; - } - - ctxt->node = save_ctxt; - } - - if (!(def->service || def->protocol)) { + ret = virXPathUInt("string(./@port)", ctxt, &def->port); + if (ret >= 0 && !def->target) { virReportError(VIR_ERR_XML_DETAIL, - _("Missing required service attribute or protocol " - "in DNS SRV record of network %s"), networkName); + _("DNS SRV port attribute not permitted without " + "target for service '%s' in network '%s'"), + def->service, networkName); goto error; } + if (ret == -2 || (ret >= 0 && (def->port < 1 || def->port > 65535))) { + virReportError(VIR_ERR_XML_DETAIL, + _("invalid DNS SRV port attribute " + "for service '%s' in network '%s'"), + def->service, networkName); + goto error; + } + + ret = virXPathUInt("string(./@priority)", ctxt, &def->priority); + if (ret >= 0 && !def->target) { + virReportError(VIR_ERR_XML_DETAIL, + _("DNS SRV priority attribute not permitted without " + "target for service '%s' in network '%s'"), + def->service, networkName); + goto error; + } + if (ret == -2 || (ret >= 0 && def->priority > 65535)) { + virReportError(VIR_ERR_XML_DETAIL, + _("Invalid DNS SRV priority attribute " + "for service '%s' in network '%s'"), + def->service, networkName); + goto error; + } + + ret = virXPathUInt("string(./@weight)", ctxt, &def->weight); + if (ret >= 0 && !def->target) { + virReportError(VIR_ERR_XML_DETAIL, + _("DNS SRV weight attribute not permitted without " + "target for service '%s' in network '%s'"), + def->service, networkName); + goto error; + } + if (ret == -2 || (ret >= 0 && def->weight > 65535)) { + virReportError(VIR_ERR_XML_DETAIL, + _("invalid DNS SRV weight attribute " + "for service '%s' in network '%s'"), + def->service, networkName); + goto error; + } + + ctxt->node = save_ctxt; return 0; error: virNetworkDNSSrvDefClear(def); + ctxt->node = save_ctxt; return -1; } diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 46c0cb0ec5..fbcc8244db 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -740,10 +740,6 @@ networkDnsmasqConfContents(virNetworkObjPtr network, int r, ret = -1; int nbleases = 0; size_t i; - char *record = NULL; - char *recordPort = NULL; - char *recordWeight = NULL; - char *recordPriority = NULL; virNetworkDNSDefPtr dns = &network->def->dns; virNetworkIpDefPtr tmpipdef, ipdef, ipv4def, ipv6def; bool ipv6SLAAC; @@ -884,33 +880,57 @@ networkDnsmasqConfContents(virNetworkObjPtr network, } for (i = 0; i < dns->nsrvs; i++) { - if (dns->srvs[i].service && dns->srvs[i].protocol) { - if (dns->srvs[i].port && - virAsprintf(&recordPort, "%d", dns->srvs[i].port) < 0) - goto cleanup; - if (dns->srvs[i].priority && - virAsprintf(&recordPriority, "%d", dns->srvs[i].priority) < 0) - goto cleanup; - if (dns->srvs[i].weight && - virAsprintf(&recordWeight, "%d", dns->srvs[i].weight) < 0) - goto cleanup; - - if (virAsprintf(&record, "%s.%s.%s,%s,%s,%s,%s", - dns->srvs[i].service, - dns->srvs[i].protocol, - dns->srvs[i].domain ? dns->srvs[i].domain : "", - dns->srvs[i].target ? dns->srvs[i].target : "", - recordPort ? recordPort : "", - recordPriority ? recordPriority : "", - recordWeight ? recordWeight : "") < 0) - goto cleanup; - - virBufferAsprintf(&configbuf, "srv-host=%s\n", record); - VIR_FREE(record); - VIR_FREE(recordPort); - VIR_FREE(recordWeight); - VIR_FREE(recordPriority); + /* service/protocol are required, and should have been validated + * by the parser. + */ + if (!dns->srvs[i].service) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Missing required 'service' " + "attribute in SRV record of network '%s'"), + network->def->name); + goto cleanup; } + if (!dns->srvs[i].protocol) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Missing required 'service' " + "attribute in SRV record of network '%s'"), + network->def->name); + goto cleanup; + } + /* RFC2782 requires that service and protocol be preceded by + * an underscore. + */ + virBufferAsprintf(&configbuf, "srv-host=_%s._%s", + dns->srvs[i].service, dns->srvs[i].protocol); + + /* domain is optional - it defaults to the domain of this network */ + if (dns->srvs[i].domain) + virBufferAsprintf(&configbuf, ".%s", dns->srvs[i].domain); + + /* If target is empty or ".", that means "the service is + * decidedly not available at this domain" (RFC2782). In that + * case, any port, priority, or weight is irrelevant. + */ + if (dns->srvs[i].target && STRNEQ(dns->srvs[i].target, ".")) { + + virBufferAsprintf(&configbuf, ",%s", dns->srvs[i].target); + /* port, priority, and weight are optional, but are + * identified by their position in the line. If an item is + * unspecified, but something later in the line *is* + * specified, we need to give the default value for the + * unspecified item. (According to the dnsmasq manpage, + * the default for port is 1). + */ + if (dns->srvs[i].port || + dns->srvs[i].priority || dns->srvs[i].weight) + virBufferAsprintf(&configbuf, ",%d", + dns->srvs[i].port ? dns->srvs[i].port : 1); + if (dns->srvs[i].priority || dns->srvs[i].weight) + virBufferAsprintf(&configbuf, ",%d", dns->srvs[i].priority); + if (dns->srvs[i].weight) + virBufferAsprintf(&configbuf, ",%d", dns->srvs[i].weight); + } + virBufferAddLit(&configbuf, "\n"); } /* Find the first dhcp for both IPv4 and IPv6 */ @@ -1086,10 +1106,6 @@ networkDnsmasqConfContents(virNetworkObjPtr network, cleanup: virBufferFreeAndReset(&configbuf); - VIR_FREE(record); - VIR_FREE(recordPort); - VIR_FREE(recordWeight); - VIR_FREE(recordPriority); return ret; } diff --git a/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf b/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf index ce4dd6f5c4..e60411ba25 100644 --- a/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf +++ b/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf @@ -12,7 +12,7 @@ listen-address=192.168.123.1 listen-address=fc00:db8:ac10:fe01::1 listen-address=fc00:db8:ac10:fd01::1 listen-address=10.24.10.1 -srv-host=name.tcp.,,,, +srv-host=_name._tcp dhcp-range=192.168.122.2,192.168.122.254 dhcp-no-override dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases diff --git a/tests/networkxml2confdata/nat-network-dns-srv-record.conf b/tests/networkxml2confdata/nat-network-dns-srv-record.conf index b47cbe7d32..16e7dca506 100644 --- a/tests/networkxml2confdata/nat-network-dns-srv-record.conf +++ b/tests/networkxml2confdata/nat-network-dns-srv-record.conf @@ -8,7 +8,13 @@ strict-order except-interface=lo bind-dynamic interface=virbr0 -srv-host=name.tcp.test-domain-name,.,1024,10,10 +srv-host=_name._tcp.test-domain-name.com,test.example.com,1111,11,111 +srv-host=_name2._udp,test2.example.com,2222,22,222 +srv-host=_name3._tcp.test3.com,test3.example.com,3333,33 +srv-host=_name4._tcp.test4.com,test4.example.com,4444 +srv-host=_name5._udp,test5.example.com,1,55,555 +srv-host=_name6._tcp.test6.com,test6.example.com,6666,0,666 +srv-host=_name7._tcp.test7.com,test7.example.com,1,0,777 dhcp-range=192.168.122.2,192.168.122.254 dhcp-no-override dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases diff --git a/tests/networkxml2confdata/nat-network-dns-srv-record.xml b/tests/networkxml2confdata/nat-network-dns-srv-record.xml index 3dd19e6698..d01b331fb1 100644 --- a/tests/networkxml2confdata/nat-network-dns-srv-record.xml +++ b/tests/networkxml2confdata/nat-network-dns-srv-record.xml @@ -6,7 +6,13 @@ - + + + + + + +