From: Remi Gacogne Date: Fri, 15 Jun 2018 15:01:07 +0000 (+0200) Subject: rec: Don't require authoritative answers for forward-recurse zones X-Git-Tag: rec-4.1.5~1^2~6^2 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=f24f20eff602a002cbfcc5346c9228213cace3f1;p=pdns rec: Don't require authoritative answers for forward-recurse zones (cherry picked from commit ad797d945527040202105b6a775ab6df94b103c6) --- diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc index 73206f56b..af3d07606 100644 --- a/pdns/recursordist/test-syncres_cc.cc +++ b/pdns/recursordist/test-syncres_cc.cc @@ -2729,17 +2729,21 @@ BOOST_AUTO_TEST_CASE(test_forward_zone_rd) { const ComboAddress ns("192.0.2.1:53"); const ComboAddress forwardedNS("192.0.2.42:53"); + size_t queriesCount = 0; SyncRes::AuthDomain ad; - ad.d_rdForward = false; + ad.d_rdForward = true; ad.d_servers.push_back(forwardedNS); (*SyncRes::t_sstorage.domainmap)[target] = ad; - sr->setAsyncCallback([forwardedNS](const ComboAddress& ip, const DNSName& domain, int type, bool doTCP, bool sendRDQuery, int EDNS0Level, struct timeval* now, boost::optional& srcmask, boost::optional context, std::shared_ptr outgoingLogger, LWResult* res, bool* chained) { + sr->setAsyncCallback([forwardedNS, &queriesCount](const ComboAddress& ip, const DNSName& domain, int type, bool doTCP, bool sendRDQuery, int EDNS0Level, struct timeval* now, boost::optional& srcmask, boost::optional context, std::shared_ptr outgoingLogger, LWResult* res, bool* chained) { + + queriesCount++; if (ip == forwardedNS) { - BOOST_CHECK_EQUAL(sendRDQuery, false); + BOOST_CHECK_EQUAL(sendRDQuery, true); - setLWResult(res, 0, true, false, true); + /* set AA=0, we are a recursor */ + setLWResult(res, 0, false, false, true); addRecordToLW(res, domain, QType::A, "192.0.2.42"); return 1; } @@ -2751,6 +2755,18 @@ BOOST_AUTO_TEST_CASE(test_forward_zone_rd) { int res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); BOOST_CHECK_EQUAL(res, RCode::NoError); BOOST_CHECK_EQUAL(ret.size(), 1); + BOOST_CHECK_EQUAL(queriesCount, 1); + + /* now make sure we can resolve from the cache (see #6340 + where the entries were added to the cache but not retrieved, + because the recursor doesn't set the AA bit and we require + it. We fixed it by not requiring the AA bit for forward-recurse + answers. */ + ret.clear(); + res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(ret.size(), 1); + BOOST_CHECK_EQUAL(queriesCount, 1); } BOOST_AUTO_TEST_CASE(test_forward_zone_recurse_nord) { diff --git a/pdns/syncres.cc b/pdns/syncres.cc index 3c8bad405..3d9cfa42d 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -570,21 +570,25 @@ int SyncRes::doResolve(const DNSName &qname, const QType &qtype, vectorend()) { + const auto& domain = iter->second; wasForwardedOrAuthZone = true; - const vector& servers = iter->second.d_servers; - if(servers.empty()) { + + if (domain.isAuth()) { wasAuthZone = true; + } else if (domain.shouldRecurse()) { + wasForwardRecurse = true; } } - if(!d_skipCNAMECheck && doCNAMECacheCheck(qname, qtype, ret, depth, res, state, wasAuthZone)) { // will reroute us if needed + if(!d_skipCNAMECheck && doCNAMECacheCheck(qname, qtype, ret, depth, res, state, wasAuthZone, wasForwardRecurse)) { // will reroute us if needed d_wasOutOfBand = wasAuthZone; return res; } - if(doCacheCheck(qname, authname, wasForwardedOrAuthZone, wasAuthZone, qtype, ret, depth, res, state)) { + if(doCacheCheck(qname, authname, wasForwardedOrAuthZone, wasAuthZone, wasForwardRecurse, qtype, ret, depth, res, state)) { // we done d_wasOutOfBand = wasAuthZone; return res; @@ -880,7 +884,7 @@ DNSName SyncRes::getBestNSNamesFromCache(const DNSName &qname, const QType& qtyp return subdomain; } -bool SyncRes::doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector& ret, unsigned int depth, int &res, vState& state, bool wasAuthZone) +bool SyncRes::doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector& ret, unsigned int depth, int &res, vState& state, bool wasAuthZone, bool wasForwardRecurse) { string prefix; if(doLog()) { @@ -899,7 +903,8 @@ bool SyncRes::doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector vector> signatures; vector> authorityRecs; bool wasAuth; - if(t_RC->get(d_now.tv_sec, qname, QType(QType::CNAME), d_requireAuthData, &cset, d_incomingECSFound ? d_incomingECSNetwork : d_requestor, d_doDNSSEC ? &signatures : nullptr, d_doDNSSEC ? &authorityRecs : nullptr, &d_wasVariable, &state, &wasAuth) > 0) { + /* we don't require auth data for forward-recurse lookups */ + if(t_RC->get(d_now.tv_sec, qname, QType(QType::CNAME), !wasForwardRecurse && d_requireAuthData, &cset, d_incomingECSFound ? d_incomingECSNetwork : d_requestor, d_doDNSSEC ? &signatures : nullptr, d_doDNSSEC ? &authorityRecs : nullptr, &d_wasVariable, &state, &wasAuth) > 0) { for(auto j=cset.cbegin() ; j != cset.cend() ; ++j) { if (j->d_class != QClass::IN) { @@ -1074,7 +1079,7 @@ void SyncRes::computeNegCacheValidationStatus(NegCache::NegCacheEntry& ne, const } } -bool SyncRes::doCacheCheck(const DNSName &qname, const DNSName& authname, bool wasForwardedOrAuthZone, bool wasAuthZone, const QType &qtype, vector&ret, unsigned int depth, int &res, vState& state) +bool SyncRes::doCacheCheck(const DNSName &qname, const DNSName& authname, bool wasForwardedOrAuthZone, bool wasAuthZone, bool wasForwardRecurse, const QType &qtype, vector&ret, unsigned int depth, int &res, vState& state) { bool giveNegative=false; @@ -1152,7 +1157,7 @@ bool SyncRes::doCacheCheck(const DNSName &qname, const DNSName& authname, bool w vector> authorityRecs; uint32_t ttl=0; bool wasCachedAuth; - if(t_RC->get(d_now.tv_sec, sqname, sqt, d_requireAuthData, &cset, d_incomingECSFound ? d_incomingECSNetwork : d_requestor, d_doDNSSEC ? &signatures : nullptr, d_doDNSSEC ? &authorityRecs : nullptr, &d_wasVariable, &cachedState, &wasCachedAuth) > 0) { + if(t_RC->get(d_now.tv_sec, sqname, sqt, !wasForwardRecurse && d_requireAuthData, &cset, d_incomingECSFound ? d_incomingECSNetwork : d_requestor, d_doDNSSEC ? &signatures : nullptr, d_doDNSSEC ? &authorityRecs : nullptr, &d_wasVariable, &cachedState, &wasCachedAuth) > 0) { LOG(prefix<&ret, int& res); bool doOOBResolve(const DNSName &qname, const QType &qtype, vector&ret, unsigned int depth, int &res); domainmap_t::const_iterator getBestAuthZone(DNSName* qname) const; - bool doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector&ret, unsigned int depth, int &res, vState& state, bool wasAuthZone); - bool doCacheCheck(const DNSName &qname, const DNSName& authname, bool wasForwardedOrAuthZone, bool wasAuthZone, const QType &qtype, vector&ret, unsigned int depth, int &res, vState& state); + bool doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector&ret, unsigned int depth, int &res, vState& state, bool wasAuthZone, bool wasForwardRecurse); + bool doCacheCheck(const DNSName &qname, const DNSName& authname, bool wasForwardedOrAuthZone, bool wasAuthZone, bool wasForwardRecurse, const QType &qtype, vector&ret, unsigned int depth, int &res, vState& state); void getBestNSFromCache(const DNSName &qname, const QType &qtype, vector&bestns, bool* flawedNSSet, unsigned int depth, set& beenthere); DNSName getBestNSNamesFromCache(const DNSName &qname, const QType &qtype, NsSet& nsset, bool* flawedNSSet, unsigned int depth, set&beenthere);