From fa1b87ff63cfe5da19be7b3a21e7b355ef86744f Mon Sep 17 00:00:00 2001 From: Pieter Lexis Date: Mon, 21 Mar 2016 17:23:43 +0100 Subject: [PATCH] Fix forward-zone with multiple IPs Fixes #3523 --- pdns/syncres.cc | 52 ++++++++++++++++++++++++++++--------------------- pdns/syncres.hh | 13 ++++++++++--- 2 files changed, 40 insertions(+), 25 deletions(-) diff --git a/pdns/syncres.cc b/pdns/syncres.cc index 11f3b1fff..4e3913930 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -435,7 +435,7 @@ int SyncRes::doResolve(const DNSName &qname, const QType &qtype, vector > nsset; + NsSet nsset; bool flawedNSSet=false; // the two retries allow getBestNSNamesFromCache&co to reprime the root @@ -621,7 +621,7 @@ SyncRes::domainmap_t::const_iterator SyncRes::getBestAuthZone(DNSName* qname) } /** doesn't actually do the work, leaves that to getBestNSFromCache */ -DNSName SyncRes::getBestNSNamesFromCache(const DNSName &qname, const QType& qtype, map >& nsset, bool* flawedNSSet, int depth, set&beenthere) +DNSName SyncRes::getBestNSNamesFromCache(const DNSName &qname, const QType& qtype, NsSet& nsset, bool* flawedNSSet, int depth, set&beenthere) { DNSName subdomain(qname); DNSName authdomain(qname); @@ -629,11 +629,13 @@ 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(), {ComboAddress(), false}}); // this gets picked up in doResolveAt, the empty DNSName, combined with the empty ComboAddress means 'we are auth' + // this gets picked up in doResolveAt, the empty DNSName, combined with the + // empty vector means 'we are auth for this zone' + nsset.insert({DNSName(), {{}, false}}); else { - for(auto const &server : iter->second.d_servers) { - nsset.insert({DNSName(), {server, iter->second.d_rdForward}}); // An empty DNSName, combined with a non-empty ComboAddress means 'this is a forwarded domain' - } + // Again, picked up in doResolveAt. An empty DNSName, combined with a + // non-empty vector of ComboAddresses means 'this is a forwarded domain' + nsset.insert({DNSName(), {iter->second.d_servers, iter->second.d_rdForward}}); } return authdomain; } @@ -642,7 +644,8 @@ DNSName SyncRes::getBestNSNamesFromCache(const DNSName &qname, const QType& qtyp getBestNSFromCache(subdomain, qtype, bestns, flawedNSSet, depth, beenthere); for(auto k=bestns.cbegin() ; k != bestns.cend(); ++k) { - nsset.insert({std::dynamic_pointer_cast(k->d_content)->getNS(), {ComboAddress(), false}}); // The actual resolver code will not even look at the ComboAddress or bool + // The actual resolver code will not even look at the ComboAddress or bool + nsset.insert({std::dynamic_pointer_cast(k->d_content)->getNS(), {{}, false}}); if(k==bestns.cbegin()) subdomain=k->d_name; } @@ -841,7 +844,7 @@ struct speedOrder map& d_speeds; }; -inline vector SyncRes::shuffleInSpeedOrder(map > &tnameservers, const string &prefix) +inline vector SyncRes::shuffleInSpeedOrder(NsSet &tnameservers, const string &prefix) { vector rnameservers; rnameservers.reserve(tnameservers.size()); @@ -914,7 +917,7 @@ static void addNXNSECS(vector&ret, const vector& records) } /** returns -1 in case of no results, rcode otherwise */ -int SyncRes::doResolveAt(map > &nameservers, DNSName auth, bool flawedNSSet, const DNSName &qname, const QType &qtype, +int SyncRes::doResolveAt(NsSet &nameservers, DNSName auth, bool flawedNSSet, const DNSName &qname, const QType &qtype, vector&ret, int depth, set&beenthere) { @@ -955,24 +958,30 @@ int SyncRes::doResolveAt(map > &nameservers, D bool sendRDQuery=false; boost::optional ednsmask; LWResult lwr; - if(tns->empty() && nameservers[*tns].first == ComboAddress() ) { + if(tns->empty() && nameservers[*tns].first.empty() ) { LOG(prefix<empty() ? nameservers[*tns].first.toString() : tns->toString())<< "' ("<<1+tns-rnameservers.begin()<<"/"<<(unsigned int)rnameservers.size()<<")"<empty()) { + LOG(prefix<toLogString()<< "' ("<<1+tns-rnameservers.begin()<<"/"<<(unsigned int)rnameservers.size()<<")"<dfe.getProcessingPolicy(*tns).d_kind != DNSFilterEngine::PolicyKind::NoAction) throw ImmediateServFailException("Dropped because of policy"); if(tns->empty()) { - LOG(prefix< 1) { + LOG("s"); + } + LOG(endl); - remoteIPs.push_back(nameservers[*tns].first); sendRDQuery = nameservers[*tns].second; pierceDontQuery=true; } @@ -982,13 +991,12 @@ int SyncRes::doResolveAt(map > &nameservers, D } if(remoteIPs.empty()) { - LOG(prefix<toString()<<", trying next if available"<toLogString()<<", trying next if available"<empty() ? nameservers[*tns].first.toString() : tns->toString())<<" to: "); + LOG(prefix<toLogString()<<" to: "); for(remoteIP = remoteIPs.begin(); remoteIP != remoteIPs.end(); ++remoteIP) { if(remoteIP != remoteIPs.begin()) { LOG(", "); @@ -1080,7 +1088,7 @@ int SyncRes::doResolveAt(map > &nameservers, D // if(d_timeouts + 0.5*d_throttledqueries > 6.0 && d_timeouts > 2) throw ImmediateServFailException("Too much work resolving "+qname+"|"+qtype.getName()+", timeouts: "+std::to_string(d_timeouts) +", throttles: "+std::to_string(d_throttledqueries)); if(lwr.d_rcode==RCode::ServFail || lwr.d_rcode==RCode::Refused) { - LOG(prefix<empty() ? nameservers[*tns].first.toString() : tns->toString())<<" returned a "<< (lwr.d_rcode==RCode::ServFail ? "ServFail" : "Refused") << ", trying sibling IP or NS"<toLogString()<<" ("<toString()<<") returned a "<< (lwr.d_rcode==RCode::ServFail ? "ServFail" : "Refused") << ", trying sibling IP or NS"<throttle.throttle(d_now.tv_sec,boost::make_tuple(*remoteIP, qname, qtype.getCode()),60,3); // servfail or refused continue; } @@ -1090,7 +1098,7 @@ int SyncRes::doResolveAt(map > &nameservers, D break; // this IP address worked! wasLame:; // well, it didn't - LOG(prefix<toString()<<" ("<< remoteIP->toString() <<") is lame for '"<toLogString()<<" ("<< remoteIP->toString() <<") is lame for '"<throttle.throttle(d_now.tv_sec, boost::make_tuple(*remoteIP, qname, qtype.getCode()), 60, 100); // lame } } @@ -1107,7 +1115,7 @@ int SyncRes::doResolveAt(map > &nameservers, D LOG(prefix<empty() ? nameservers[*tns].first.toString() : tns->toString())<<" ("<< remoteIP->toString() <<"), rcode="<toLogString()<<" ("<< remoteIP->toString() <<"), rcode="<sin4.sin_family==AF_INET6) @@ -1359,7 +1367,7 @@ int SyncRes::doResolveAt(map > &nameservers, D nameservers.clear(); for (auto const &nameserver : nsset) - nameservers.insert({nameserver, {ComboAddress(), false}}); + nameservers.insert({nameserver, {{}, false}}); break; } else if(!tns->empty()) { // means: not OOB, OOB == empty diff --git a/pdns/syncres.hh b/pdns/syncres.hh index cfdb53e49..62e452a5f 100644 --- a/pdns/syncres.hh +++ b/pdns/syncres.hh @@ -52,6 +52,13 @@ struct NegCacheEntry recsig_t d_dnssecProof; }; +typedef map< + DNSName, + pair< + vector, + bool + > +> NsSet; template class Throttle : public boost::noncopyable { @@ -461,7 +468,7 @@ public: private: struct GetBestNSAnswer; - int doResolveAt(map > &nameservers, DNSName auth, bool flawedNSSet, const DNSName &qname, const QType &qtype, vector&ret, + int doResolveAt(NsSet &nameservers, DNSName auth, bool flawedNSSet, const DNSName &qname, const QType &qtype, vector&ret, int depth, set&beenthere); int doResolve(const DNSName &qname, const QType &qtype, vector&ret, int depth, set& beenthere); bool doOOBResolve(const DNSName &qname, const QType &qtype, vector&ret, int depth, int &res); @@ -469,9 +476,9 @@ private: bool doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector&ret, int depth, int &res); bool doCacheCheck(const DNSName &qname, const QType &qtype, vector&ret, int depth, int &res); void getBestNSFromCache(const DNSName &qname, const QType &qtype, vector&bestns, bool* flawedNSSet, int depth, set& beenthere); - DNSName getBestNSNamesFromCache(const DNSName &qname, const QType &qtype, map >& nsset, bool* flawedNSSet, int depth, set&beenthere); + DNSName getBestNSNamesFromCache(const DNSName &qname, const QType &qtype, NsSet& nsset, bool* flawedNSSet, int depth, set&beenthere); - inline vector shuffleInSpeedOrder(map > &nameservers, const string &prefix); + inline vector shuffleInSpeedOrder(NsSet &nameservers, const string &prefix); bool moreSpecificThan(const DNSName& a, const DNSName &b); vector getAddrs(const DNSName &qname, int depth, set& beenthere); private: -- 2.49.0