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

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)