[libvirt] [PATCH] network: only prevent forwarding of DNS requests for unqualified names

[Laine Stump]

In commit f386825 we began adding the option "--local=/$mydomain/" to
all dnsmasq commandlines (later changed to "local=/$mydomain/" when we
moved the options from the commandline to a conf file) with the stated
reason of preventing forwarding of DNS queries for names that weren't
fully qualified domain names ("FQDN", i.e. a name that included some
"."s and a domain name).

The original patch on the list, and discussion about it, is here:

  https://www.redhat.com/archives/libvir-list/2012-August/msg01594.html

When a domain name isn't specified (no <domain> element in the network
definition), the corresponding addition of "local=//" will prevent
forwarding of domain-less requests to the virtualization host's DNS
resolver, but if a domain *is* specified, the addition of
"local=/domain/" will prevent forwarding of any requests for names
within that domain that aren't resolvable by libvirt's dnsmasq itself.

An example of the problems this causes: let's say a network is
defined with:

   <domain name='example.com'/>
   <dhcp>
      ..
      <host mac='52:54:00:11:22:33' ip='1.2.3.4' name='myguest'/>
   </dhcp>

This results in "local=/example.com/" being added to the dnsmasq options.

If a guest requests "myguest" or "myguest.example.com", that will be
resolved by dnsmasq. If the guest asks for "www.example.com", dnsmasq
will not know the answer, but instead of forwarding it to the host, it
will return NOT FOUND to the guest. In most cases that isn't the
behavior an admin is looking for.

Really we should have been just including the option "--local=//" in
all cases, so that (unresolvable by dnsmasq) requests for names
without a domain would be treated as "local to dnsmasq" and not
forwarded, but all other requests would be forwarded. That's what this
patch does.
---

I'm debating whether there is any value at all to maintaining the
previous behavior of "don't forward unresolved requests for hosts in
the network's defined domain", or if it should just be considered
purely a bug. If so, I think it shouldn't be the default behavior, and
should be controlled by a new attribute to the <domain> element,
something like this:

  <domain name='example.com' forwardUnresolved='no'/>

(this would default to yes). Any opinions on 1) whether or not this is
needed, and 2) if so, any better name for the option? (it would be nice if it could default to 'no' or 'local-only' (which was == 0) or something else that didn't require a non-0 default or a strange double-negative name).


 src/network/bridge_driver.c                          | 12 +++++-------
 tests/networkxml2confdata/dhcp6-network.conf         |  2 +-
 tests/networkxml2confdata/nat-network-dns-hosts.conf |  2 +-
 tests/networkxml2confdata/netboot-network.conf       |  2 +-
 tests/networkxml2confdata/netboot-proxy-network.conf |  2 +-
 5 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 80c5acb..b1382e4 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -723,14 +723,12 @@ networkDnsmasqConfContents(virNetworkObjPtr network,
                           network->def->domain);
     }
 
-    if (network->def->domain || !network->def->dns.forwardPlainNames) {
-        /* need to specify local even if no domain specified, unless
-         * the config says we should forward "plain" names (i.e. not
-         * fully qualified, no '.' characters)
+    if (!network->def->dns.forwardPlainNames) {
+        /* need to specify local=// whether or not a domain is
+         * specified, unless the config says we should forward "plain"
+         * names (i.e. not fully qualified, no '.' characters)
          */
-        virBufferAsprintf(&configbuf,
-                          "local=/%s/\n",
-                          network->def->domain ? network->def->domain : "");
+        virBufferAsprintf(&configbuf, "local=//\n");
     }
 
     if (pidfile)
diff --git a/tests/networkxml2confdata/dhcp6-network.conf b/tests/networkxml2confdata/dhcp6-network.conf
index 5fde07f..24ac7fa 100644
--- a/tests/networkxml2confdata/dhcp6-network.conf
+++ b/tests/networkxml2confdata/dhcp6-network.conf
@@ -8,7 +8,7 @@ strict-order
 domain-needed
 domain=mynet
 expand-hosts
-local=/mynet/
+local=//
 except-interface=lo
 bind-dynamic
 interface=virbr0
diff --git a/tests/networkxml2confdata/nat-network-dns-hosts.conf b/tests/networkxml2confdata/nat-network-dns-hosts.conf
index 2577882..bb1d5f4 100644
--- a/tests/networkxml2confdata/nat-network-dns-hosts.conf
+++ b/tests/networkxml2confdata/nat-network-dns-hosts.conf
@@ -8,7 +8,7 @@ strict-order
 domain-needed
 domain=example.com
 expand-hosts
-local=/example.com/
+local=//
 except-interface=lo
 bind-dynamic
 interface=virbr0
diff --git a/tests/networkxml2confdata/netboot-network.conf b/tests/networkxml2confdata/netboot-network.conf
index b6f3c23..37fef40 100644
--- a/tests/networkxml2confdata/netboot-network.conf
+++ b/tests/networkxml2confdata/netboot-network.conf
@@ -8,7 +8,7 @@ strict-order
 domain-needed
 domain=example.com
 expand-hosts
-local=/example.com/
+local=//
 except-interface=lo
 bind-interfaces
 listen-address=192.168.122.1
diff --git a/tests/networkxml2confdata/netboot-proxy-network.conf b/tests/networkxml2confdata/netboot-proxy-network.conf
index 1e969fa..ae34aa0 100644
--- a/tests/networkxml2confdata/netboot-proxy-network.conf
+++ b/tests/networkxml2confdata/netboot-proxy-network.conf
@@ -8,7 +8,7 @@ strict-order
 domain-needed
 domain=example.com
 expand-hosts
-local=/example.com/
+local=//
 except-interface=lo
 bind-interfaces
 listen-address=192.168.122.1
-- 
1.8.3.1