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

[Laine Stump]

On 12/10/2013 04:45 PM, Daniel P. Berrange wrote:
> So considering your example XML config above we're debating the
> behaviour of the following 5 possible DNS requests
>
>  - myguest
>  - myguest.example.com
>  - notmyguest
>  - notmyguest.example.com
>  - google.com
>
> Originally
>
>  - myguest -> dnsmasq
>  - myguest.example.com -> dnsmasq
>  - notmyguest -> forwarded
>  - notmyguest.example.com -> forwarded
>  - google.com -> forwarded
>
> With the current GIT
>
>  - myguest -> dnsmasq
>  - myguest.example.com -> dnsmasq
>  - notmyguest -> error
>  - notmyguest.example.com -> error
>  - google.com -> forwarded
>
> With your proposal
>
>  - myguest -> dnsmasq
>  - myguest.example.com -> dnsmasq
>  - notmyguest -> error
>  - notmyguest.example.com -> fowarded
>  - google.com -> forwarded

Nicely organized and diagrammed!

This is really a case of accepting a patch without performing adequate
due diligence on the ramifications of the change in behavior. The reason
behind adding the patch seemed valid, but it had a wider effect than
intended.


> IMHO I tend to think that the original behaviour should not have been
> changed and is the right default. If we desired either of the other
> behaviours we should have a config option for them to force returning
> of errors instead of forwarding. A simple boolean wouldn't suffice
> since there are 3 possible valid behaviours here - so we'd need an
> enum attribute

The first time I encountered this particular blowback of the original
patch (it had also included --filterwin2k, which itself caused problems
and was reverted), my response was to add the

  <dns forwardPlainNames='yes'/>

attribute - that restores (tries to anyway) the original behavior, but
not by default. (My thinking was that the intent of the original patch
was desirable, since it had been ACKed and pushed (and besides,
forwarding of non-qualified names really is frowned on, at least by root
dns servers). Too bad we didn't have this discussion back then :-/
That's the problem of having such a high volume of mail on the list -
there's never enough time for everyone to read it all, so some
discussions just never happen.

So is your suggestion that we add to this patch another patch which
changes the default for "forwardPlainNames" to "no"?

(I can also see how this could/should functionally fit together with a
list of domains/dns servers, which we *also* "kind of" support via the
<forwarder> element (which unfortunately only does things halfway - it
allows specifying the IP address of a dns server, but not what domain it
should be used for; strange, seeing that the dnsmasq option it is
encapsulating does exactly that). I'm thinking there should be another
patch which extends each <forwarder> to have an optional "domain"
attribute to limit which domains a particular dns server is used for)