From e1636f8201dbb30b2445a460aab8c2512ede39ec Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Fri, 24 Nov 2017 17:48:19 +0100 Subject: [PATCH] rec: Skip validation (including cached entries) for auth zones --- pdns/recursordist/test-syncres_cc.cc | 120 +++++++++++++++++++++++---- pdns/syncres.cc | 64 +++++++------- pdns/syncres.hh | 9 +- 3 files changed, 145 insertions(+), 48 deletions(-) diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc index bd0cb1a88..d51c43d41 100644 --- a/pdns/recursordist/test-syncres_cc.cc +++ b/pdns/recursordist/test-syncres_cc.cc @@ -2824,39 +2824,30 @@ BOOST_AUTO_TEST_CASE(test_forward_zone_recurse_rd) { BOOST_CHECK_EQUAL(ret.size(), 1); } -BOOST_AUTO_TEST_CASE(test_auth_zone_delegation_oob) { +BOOST_AUTO_TEST_CASE(test_auth_zone_oob) { std::unique_ptr sr; - init(); - initSR(sr, true, false); + initSR(sr, true); primeHints(); size_t queriesCount = 0; const DNSName target("test.xx."); const ComboAddress targetAddr("127.0.0.1"); - const DNSName ns("localhost."); - const ComboAddress nsAddr("127.0.0.1"); const DNSName authZone("test.xx"); SyncRes::AuthDomain ad; DNSRecord dr; - dr.d_place = DNSResourceRecord::ANSWER; - dr.d_name = authZone; - dr.d_type = QType::NS; - dr.d_ttl = 1800; - dr.d_content = std::make_shared("localhost."); - ad.d_records.insert(dr); dr.d_place = DNSResourceRecord::ANSWER; - dr.d_name = authZone; + dr.d_name = target; dr.d_type = QType::A; dr.d_ttl = 1800; - dr.d_content = std::make_shared(nsAddr); + dr.d_content = std::make_shared(targetAddr); ad.d_records.insert(dr); (*SyncRes::t_sstorage.domainmap)[authZone] = ad; - sr->setAsyncCallback([&queriesCount,nsAddr,target,targetAddr](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) { + sr->setAsyncCallback([&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) { queriesCount++; return 0; }); @@ -2868,6 +2859,7 @@ BOOST_AUTO_TEST_CASE(test_auth_zone_delegation_oob) { BOOST_CHECK(ret[0].d_type == QType::A); BOOST_CHECK_EQUAL(queriesCount, 0); BOOST_CHECK(sr->wasOutOfBand()); + BOOST_CHECK_EQUAL(sr->getValidationState(), Indeterminate); /* a second time, to check that the OOB flag is set when the query cache is used */ ret.clear(); @@ -2877,6 +2869,88 @@ BOOST_AUTO_TEST_CASE(test_auth_zone_delegation_oob) { BOOST_CHECK(ret[0].d_type == QType::A); BOOST_CHECK_EQUAL(queriesCount, 0); BOOST_CHECK(sr->wasOutOfBand()); + BOOST_CHECK_EQUAL(sr->getValidationState(), Indeterminate); + + /* a third time, to check that the validation is disabled when the OOB flag is set */ + ret.clear(); + sr->setDNSSECValidationRequested(true); + res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, 0); + BOOST_REQUIRE_EQUAL(ret.size(), 1); + BOOST_CHECK(ret[0].d_type == QType::A); + BOOST_CHECK_EQUAL(queriesCount, 0); + BOOST_CHECK(sr->wasOutOfBand()); + BOOST_CHECK_EQUAL(sr->getValidationState(), Indeterminate); +} + +BOOST_AUTO_TEST_CASE(test_auth_zone_oob_cname) { + std::unique_ptr sr; + initSR(sr, true); + + primeHints(); + + size_t queriesCount = 0; + const DNSName target("cname.test.xx."); + const DNSName targetCname("cname-target.test.xx."); + const ComboAddress targetCnameAddr("127.0.0.1"); + const DNSName authZone("test.xx"); + + SyncRes::AuthDomain ad; + DNSRecord dr; + + dr.d_place = DNSResourceRecord::ANSWER; + dr.d_name = target; + dr.d_type = QType::CNAME; + dr.d_ttl = 1800; + dr.d_content = std::make_shared(targetCname); + ad.d_records.insert(dr); + + dr.d_place = DNSResourceRecord::ANSWER; + dr.d_name = targetCname; + dr.d_type = QType::A; + dr.d_ttl = 1800; + dr.d_content = std::make_shared(targetCnameAddr); + ad.d_records.insert(dr); + + (*SyncRes::t_sstorage.domainmap)[authZone] = ad; + + sr->setAsyncCallback([&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) { + queriesCount++; + return 0; + }); + + vector ret; + int res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, 0); + BOOST_REQUIRE_EQUAL(ret.size(), 2); + BOOST_CHECK(ret[0].d_type == QType::CNAME); + BOOST_CHECK(ret[1].d_type == QType::A); + BOOST_CHECK_EQUAL(queriesCount, 0); + BOOST_CHECK(sr->wasOutOfBand()); + BOOST_CHECK_EQUAL(sr->getValidationState(), Indeterminate); + + /* a second time, to check that the OOB flag is set when the query cache is used */ + ret.clear(); + res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, 0); + BOOST_REQUIRE_EQUAL(ret.size(), 2); + BOOST_CHECK(ret[0].d_type == QType::CNAME); + BOOST_CHECK(ret[1].d_type == QType::A); + BOOST_CHECK_EQUAL(queriesCount, 0); + BOOST_CHECK(sr->wasOutOfBand()); + BOOST_CHECK_EQUAL(sr->getValidationState(), Indeterminate); + + /* a third time, to check that the validation is disabled when the OOB flag is set */ + ret.clear(); + sr->setDNSSECValidationRequested(true); + res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, 0); + BOOST_REQUIRE_EQUAL(ret.size(), 2); + BOOST_CHECK(ret[0].d_type == QType::CNAME); + BOOST_CHECK(ret[1].d_type == QType::A); + BOOST_CHECK_EQUAL(queriesCount, 0); + BOOST_CHECK(sr->wasOutOfBand()); + BOOST_CHECK_EQUAL(sr->getValidationState(), Indeterminate); } BOOST_AUTO_TEST_CASE(test_auth_zone) { @@ -3126,7 +3200,7 @@ BOOST_AUTO_TEST_CASE(test_auth_zone_nx) { BOOST_AUTO_TEST_CASE(test_auth_zone_delegation) { std::unique_ptr sr; - initSR(sr); + initSR(sr, true, false); primeHints(); @@ -3165,9 +3239,19 @@ BOOST_AUTO_TEST_CASE(test_auth_zone_delegation) { (*map)[authZone] = ad; SyncRes::setDomainMap(map); - sr->setAsyncCallback([&queriesCount,target,targetAddr,nsAddr](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) { + testkeysset_t keys; + auto luaconfsCopy = g_luaconfs.getCopy(); + luaconfsCopy.dsAnchors.clear(); + generateKeyMaterial(g_rootdnsname, DNSSECKeeper::RSASHA512, DNSSECKeeper::SHA384, keys, luaconfsCopy.dsAnchors); + g_luaconfs.setState(luaconfsCopy); + + sr->setAsyncCallback([&queriesCount,target,targetAddr,nsAddr,authZone,keys](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) { queriesCount++; + if (type == QType::DS || type == QType::DNSKEY) { + return genericDSAndDNSKEYHandler(res, domain, DNSName("."), type, keys, domain == authZone); + } + if (ip == ComboAddress(nsAddr.toString(), 53) && domain == target) { setLWResult(res, 0, true, false, true); addRecordToLW(res, domain, QType::A, targetAddr.toString(), DNSResourceRecord::ANSWER, 3600); @@ -3177,12 +3261,14 @@ BOOST_AUTO_TEST_CASE(test_auth_zone_delegation) { return 0; }); + sr->setDNSSECValidationRequested(true); vector ret; int res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); BOOST_CHECK_EQUAL(res, RCode::NoError); BOOST_REQUIRE_EQUAL(ret.size(), 1); BOOST_CHECK(ret[0].d_type == QType::A); - BOOST_CHECK_EQUAL(queriesCount, 1); + BOOST_CHECK_EQUAL(queriesCount, 4); + BOOST_CHECK_EQUAL(sr->getValidationState(), Indeterminate); } BOOST_AUTO_TEST_CASE(test_auth_zone_delegation_point) { diff --git a/pdns/syncres.cc b/pdns/syncres.cc index 91211cf0f..111128258 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -141,7 +141,7 @@ int SyncRes::beginResolve(const DNSName &qname, const QType &qtype, uint16_t qcl if (d_queryValidationState != Indeterminate) { g_stats.dnssecValidations++; } - if (d_DNSSECValidationRequested) { + if (shouldValidate()) { increaseDNSSECStateCounter(d_queryValidationState); } @@ -562,11 +562,28 @@ int SyncRes::doResolve(const DNSName &qname, const QType &qtype, vectorend()) { + wasForwardedOrAuthZone = true; + const vector& servers = iter->second.d_servers; + if(servers.empty()) { + wasAuthZone = true; + } + } + + if(!d_skipCNAMECheck && doCNAMECacheCheck(qname, qtype, ret, depth, res, state, wasAuthZone)) { // will reroute us if needed + d_wasOutOfBand = wasAuthZone; return res; + } - if(doCacheCheck(qname,qtype,ret,depth,res,state)) // we done + if(doCacheCheck(qname, authname, wasForwardedOrAuthZone, wasAuthZone, qtype, ret, depth, res, state)) { + // we done + d_wasOutOfBand = wasAuthZone; return res; + } } if(d_cacheonly) @@ -852,7 +869,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 SyncRes::doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector& ret, unsigned int depth, int &res, vState& state, bool wasAuthZone) { string prefix; if(doLog()) { @@ -876,7 +893,7 @@ bool SyncRes::doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector for(auto j=cset.cbegin() ; j != cset.cend() ; ++j) { if(j->d_ttl>(unsigned int) d_now.tv_sec) { - if (d_DNSSECValidationRequested && wasAuth && state == Indeterminate && d_requireAuthData) { + if (!wasAuthZone && shouldValidate() && wasAuth && state == Indeterminate && d_requireAuthData) { /* This means we couldn't figure out the state when this entry was cached, most likely because we hadn't computed the zone cuts yet. */ /* make sure they are computed before validating */ @@ -1039,7 +1056,7 @@ void SyncRes::computeNegCacheValidationStatus(NegCache::NegCacheEntry& ne, const } } -bool SyncRes::doCacheCheck(const DNSName &qname, 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, const QType &qtype, vector&ret, unsigned int depth, int &res, vState& state) { bool giveNegative=false; @@ -1054,25 +1071,13 @@ bool SyncRes::doCacheCheck(const DNSName &qname, const QType &qtype, vector "<end()) { - wasForwardedOrAuth = true; - const vector& servers = iter->second.d_servers; - if(servers.empty()) { - wasAuth = true; - } - } NegCache::NegCacheEntry ne; if(s_rootNXTrust && t_sstorage.negcache.getRootNXTrust(qname, d_now, ne) && ne.d_auth.isRoot() && - !(wasForwardedOrAuth && !authname.isRoot())) { // when forwarding, the root may only neg-cache if it was forwarded to. + !(wasForwardedOrAuthZone && !authname.isRoot())) { // when forwarding, the root may only neg-cache if it was forwarded to. sttl = ne.d_ttd - d_now.tv_sec; LOG(prefix<first.name, false); LOG(d_prefix<<": got initial zone status "<first.name<empty() && !wasForwarded) { LOG(prefix<&ret, int& res) const; 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 doCacheCheck(const DNSName &qname, 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 doCacheCheck(const DNSName &qname, const DNSName& authname, bool wasForwardedOrAuthZone, bool wasAuthZone, 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); -- 2.40.0