Fix the forward zones in the recursor #3244

Merged
merged 2 commits into from Jan 19, 2016

Projects

None yet

2 participants

@pieterlexis
Member

In the pre-DNSName era, when dns-native names were passed as strings, we
overloaded the NS-name for a forward or auth zone. e.g. an empty string
meant 'this is an auth zone' and '+203.0.113.1' meant 'forward to 203.0.113.1
with the RD bit set'. With DNSNames, this is impossible (yay!).

In this commit, the set of strings (and later DNSNames), is replaced by
a map where a DNSName is the key and the value is a pair of a
ComboAddress and a boolean.

A non-empty DNSName: This is a normal NS, recurse as usual (the pair is
ignored).

An empty DNSName and empty ComboAddress: We are auth for this zone,
check the auth store for an answer.

An empty DNSName and non-empty ComboAddress: The query must be forwarded
to the ComboAddress specified and the boolean in the pair tells us the
value of the RD bit in the query we need to send.

@pieterlexis
Member

ToDo:

  • Check if forward-zones-file still works
@ahupowerdns ahupowerdns commented on the diff Jan 15, 2016
pdns/syncres.cc
@@ -954,14 +951,14 @@ int SyncRes::doResolveAt(set<DNSName> nameservers, DNSName auth, bool flawedNSSe
bool sendRDQuery=false;
boost::optional<Netmask> ednsmask;
LWResult lwr;
- if(tns->empty()) {
+ if(tns->empty() && nameservers[*tns].first == ComboAddress() ) {
@ahupowerdns
ahupowerdns Jan 15, 2016 Member

I'm not sure if this is idiomatic. We should maybe add a test for it if it is. It looks like we currently get away with it.

@pieterlexis
pieterlexis Jan 18, 2016 Member

I added a unit test for this.

@ahupowerdns ahupowerdns and 1 other commented on an outdated diff Jan 15, 2016
pdns/syncres.hh
@@ -460,7 +460,7 @@ public:
private:
struct GetBestNSAnswer;
- int doResolveAt(set<DNSName> nameservers, DNSName auth, bool flawedNSSet, const DNSName &qname, const QType &qtype, vector<DNSRecord>&ret,
+ int doResolveAt(map<DNSName, pair<ComboAddress, bool> > nameservers, DNSName auth, bool flawedNSSet, const DNSName &qname, const QType &qtype, vector<DNSRecord>&ret,
@ahupowerdns
ahupowerdns Jan 15, 2016 Member

it is pretty odd that this was passed by value. I'm sure there was a reason for it one day, might be good to see if we can get away with a const reference now.

@pieterlexis
pieterlexis Jan 18, 2016 Member

oh wait, completely mixed up const and pass-by-X in my head :)

@ahupowerdns ahupowerdns commented on an outdated diff Jan 15, 2016
pdns/syncres.cc
@@ -626,14 +626,11 @@ DNSName SyncRes::getBestNSNamesFromCache(const DNSName &qname, const QType& qtyp
domainmap_t::const_iterator iter=getBestAuthZone(&authdomain);
if(iter!=t_sstorage->domainmap->end()) {
if( iter->second.d_servers.empty() )
- nsset.insert(DNSName()); // this gets picked up in doResolveAt, if empty it means "we are auth", otherwise it denotes a forward
+ nsset.insert(make_pair(DNSName(), make_pair(ComboAddress(), false))); // this gets picked up in doResolveAt, the empty DNSName, combined with the empty ComboAddress means 'we are auth'
@ahupowerdns
ahupowerdns Jan 15, 2016 Member

you could try {DNSName(), {ComboAddress(), false}} instead of make_pairs - might just work.

pieterlexis added some commits Jan 15, 2016
@pieterlexis pieterlexis Fix the forward zones in the recursor
In the pre-DNSName era, when dns-native names were passed as strings, we
overloaded the NS-name for a forward or auth zone. e.g. an empty string
meant 'this is an auth zone' and '+203.0.113.1' meant 'forward to 203.0.113.1
with the RD bit set'. With DNSNames, this is impossible (yay!).

In this commit, the set of strings (and later DNSNames), is replaced by
a map where a DNSName is the key and the value is a pair of a
ComboAddress and a boolean.

A non-empty DNSName: This is a normal NS, recurse as usual (the pair is
ignored).

An empty DNSName and empty ComboAddress: We are auth for this zone,
check the auth store for an answer.

An empty DNSName and non-empty ComboAddress: The query must be forwarded
to the ComboAddress specified and the boolean in the pair tells us the
value of the RD bit in the query we need to send.
88490c0
@pieterlexis pieterlexis Add empty ComboAddress equality unit-test c9ff7d5
@ahupowerdns ahupowerdns merged commit d34d3f8 into PowerDNS:master Jan 19, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@pieterlexis pieterlexis deleted the pieterlexis:4.0-forward-zones branch Jan 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment