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

[Daniel P. Berrange]

On Mon, Dec 16, 2013 at 04:41:31PM +0200, Laine Stump wrote:
> 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.

Indeed.

> > 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"?

Oh, I didn't realize we had that attribute already. Don't we need it
to be a tri-state instead of just yes/no ?

Going back to be list at the to. IIUC, the forwardPlainNames=no
toggles between

> > Originally
> >  - notmyguest -> forwarded
> >  - notmyguest.example.com -> forwarded
> >
> > With the current GIT
> >
> >  - notmyguest -> error
> >  - notmyguest.example.com -> error


But doesn't give us a way to handle this new use case:

> > With your proposal
> >
> >  - notmyguest -> error
> >  - notmyguest.example.com -> fowarded


Or am I misunderstanding ?

> (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)
> 

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|