From 24bb9b589086f7454267c78e8a607c911dd850ee Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Fri, 14 Apr 2017 16:41:04 +0200 Subject: [PATCH] rec: Only use non-AA data to get NS / DS / glues --- pdns/recursor_cache.cc | 8 +++-- pdns/recursor_cache.hh | 2 +- pdns/recursordist/test-recursorcache_cc.cc | 36 +++++++++++----------- pdns/recursordist/test-syncres_cc.cc | 4 +-- pdns/syncres.cc | 14 ++++++--- pdns/syncres.hh | 1 + 6 files changed, 37 insertions(+), 28 deletions(-) diff --git a/pdns/recursor_cache.cc b/pdns/recursor_cache.cc index 6d0eb5eb6..dd1016ffa 100644 --- a/pdns/recursor_cache.cc +++ b/pdns/recursor_cache.cc @@ -34,7 +34,7 @@ unsigned int MemRecursorCache::bytes() } // returns -1 for no hits -int32_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt, vector* res, const ComboAddress& who, vector>* signatures, bool* variable) +int32_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt, bool requireAuth, vector* res, const ComboAddress& who, vector>* signatures, bool* variable) { time_t ttd=0; // cerr<<"looking up "<< qname<<"|"+qt.getName()<<"\n"; @@ -51,7 +51,10 @@ int32_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt, res->clear(); if(d_cachecache.first!=d_cachecache.second) { - for(cache_t::const_iterator i=d_cachecache.first; i != d_cachecache.second; ++i) + for(cache_t::const_iterator i=d_cachecache.first; i != d_cachecache.second; ++i) { + if (requireAuth && !i->d_auth) + continue; + if(i->d_ttd > now && ((i->d_qtype == qt.getCode() || qt.getCode()==QType::ANY || (qt.getCode()==QType::ADDR && (i->d_qtype == QType::A || i->d_qtype == QType::AAAA) )) && (i->d_netmask.empty() || i->d_netmask.match(who))) @@ -85,6 +88,7 @@ int32_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt, if(qt.getCode()!=QType::ANY && qt.getCode()!=QType::ADDR) // normally if we have a hit, we are done break; } + } // cerr<<"time left : "<size() : 0) <<"\n"; return static_cast(ttd-now); diff --git a/pdns/recursor_cache.hh b/pdns/recursor_cache.hh index 0641ff531..709461424 100644 --- a/pdns/recursor_cache.hh +++ b/pdns/recursor_cache.hh @@ -53,7 +53,7 @@ public: } unsigned int size(); unsigned int bytes(); - int32_t get(time_t, const DNSName &qname, const QType& qt, vector* res, const ComboAddress& who, vector>* signatures=0, bool* variable=0); + int32_t get(time_t, const DNSName &qname, const QType& qt, bool requireAuth, vector* res, const ComboAddress& who, vector>* signatures=0, bool* variable=0); void replace(time_t, const DNSName &qname, const QType& qt, const vector& content, const vector>& signatures, bool auth, boost::optional ednsmask=boost::none); void doPrune(void); diff --git a/pdns/recursordist/test-recursorcache_cc.cc b/pdns/recursordist/test-recursorcache_cc.cc index 1974bbea8..a0539d69b 100644 --- a/pdns/recursordist/test-recursorcache_cc.cc +++ b/pdns/recursordist/test-recursorcache_cc.cc @@ -55,7 +55,7 @@ BOOST_AUTO_TEST_CASE(test_RecursorCacheSimple) { int64_t expected = counter-delcounter; for(; delcounter < counter; ++delcounter) { - if(MRC.get(now, DNSName("hello ")+DNSName(std::to_string(delcounter)), QType(QType::A), &retrieved, who, nullptr)) { + if(MRC.get(now, DNSName("hello ")+DNSName(std::to_string(delcounter)), QType(QType::A), false, &retrieved, who, nullptr)) { matches++; } } @@ -93,13 +93,13 @@ BOOST_AUTO_TEST_CASE(test_RecursorCacheSimple) { retrieved.clear(); // subnet specific should be returned for a matching subnet - BOOST_CHECK_EQUAL(MRC.get(now, power, QType(QType::AAAA), &retrieved, ComboAddress("192.0.2.2"), nullptr), (ttd-now)); + BOOST_CHECK_EQUAL(MRC.get(now, power, QType(QType::AAAA), false, &retrieved, ComboAddress("192.0.2.2"), nullptr), (ttd-now)); BOOST_REQUIRE_EQUAL(retrieved.size(), 1); BOOST_CHECK_EQUAL(getRR(retrieved.at(0))->getCA().toString(), dr1Content.toString()); retrieved.clear(); // subnet specific should not be returned for a different subnet - BOOST_CHECK_LT(MRC.get(now, power, QType(QType::AAAA), &retrieved, ComboAddress("127.0.0.1"), nullptr), 0); + BOOST_CHECK_LT(MRC.get(now, power, QType(QType::AAAA), false, &retrieved, ComboAddress("127.0.0.1"), nullptr), 0); BOOST_CHECK_EQUAL(retrieved.size(), 0); // remove everything @@ -113,7 +113,7 @@ BOOST_AUTO_TEST_CASE(test_RecursorCacheSimple) { BOOST_CHECK_EQUAL(MRC.size(), 1); // NON-subnet specific should always be returned - BOOST_CHECK_EQUAL(MRC.get(now, power, QType(QType::A), &retrieved, ComboAddress("127.0.0.1"), nullptr), (ttd-now)); + BOOST_CHECK_EQUAL(MRC.get(now, power, QType(QType::A), false, &retrieved, ComboAddress("127.0.0.1"), nullptr), (ttd-now)); BOOST_REQUIRE_EQUAL(retrieved.size(), 1); BOOST_CHECK_EQUAL(getRR(retrieved.at(0))->getCA().toString(), dr2Content.toString()); retrieved.clear(); @@ -133,24 +133,24 @@ BOOST_AUTO_TEST_CASE(test_RecursorCacheSimple) { BOOST_CHECK_EQUAL(MRC.size(), 3); // we should still get the NON-subnet specific entry - BOOST_CHECK_EQUAL(MRC.get(now, power, QType(QType::A), &retrieved, ComboAddress("127.0.0.1"), nullptr), (ttd-now)); + BOOST_CHECK_EQUAL(MRC.get(now, power, QType(QType::A), false, &retrieved, ComboAddress("127.0.0.1"), nullptr), (ttd-now)); BOOST_REQUIRE_EQUAL(retrieved.size(), 1); BOOST_CHECK_EQUAL(getRR(retrieved.at(0))->getCA().toString(), dr2Content.toString()); retrieved.clear(); // we should get the subnet specific entry if we are from the right subnet - BOOST_CHECK_EQUAL(MRC.get(now, power, QType(QType::AAAA), &retrieved, ComboAddress("192.0.2.3"), nullptr), (ttd-now)); + BOOST_CHECK_EQUAL(MRC.get(now, power, QType(QType::AAAA), false, &retrieved, ComboAddress("192.0.2.3"), nullptr), (ttd-now)); BOOST_REQUIRE_EQUAL(retrieved.size(), 1); BOOST_CHECK_EQUAL(getRR(retrieved.at(0))->getCA().toString(), dr1Content.toString()); retrieved.clear(); // but nothing from a different subnet - BOOST_CHECK_LT(MRC.get(now, power, QType(QType::AAAA), &retrieved, ComboAddress("127.0.0.1"), nullptr), 0); + BOOST_CHECK_LT(MRC.get(now, power, QType(QType::AAAA), false, &retrieved, ComboAddress("127.0.0.1"), nullptr), 0); BOOST_CHECK_EQUAL(retrieved.size(), 0); retrieved.clear(); // QType::ANY should return any qtype, so from the right subnet we should get all of them - BOOST_CHECK_EQUAL(MRC.get(now, power, QType(QType::ANY), &retrieved, ComboAddress("192.0.2.3"), nullptr), (ttd-now)); + BOOST_CHECK_EQUAL(MRC.get(now, power, QType(QType::ANY), false, &retrieved, ComboAddress("192.0.2.3"), nullptr), (ttd-now)); BOOST_CHECK_EQUAL(retrieved.size(), 3); for (const auto& rec : retrieved) { BOOST_CHECK(rec.d_type == QType::A || rec.d_type == QType::AAAA || rec.d_type == QType::TXT); @@ -162,7 +162,7 @@ BOOST_AUTO_TEST_CASE(test_RecursorCacheSimple) { retrieved.clear(); // but only the non-subnet specific from the another subnet - BOOST_CHECK_EQUAL(MRC.get(now, power, QType(QType::ANY), &retrieved, ComboAddress("127.0.0.1"), nullptr), (ttd-now)); + BOOST_CHECK_EQUAL(MRC.get(now, power, QType(QType::ANY), false, &retrieved, ComboAddress("127.0.0.1"), nullptr), (ttd-now)); BOOST_CHECK_EQUAL(retrieved.size(), 2); for (const auto& rec : retrieved) { BOOST_CHECK(rec.d_type == QType::A || rec.d_type == QType::TXT); @@ -170,7 +170,7 @@ BOOST_AUTO_TEST_CASE(test_RecursorCacheSimple) { retrieved.clear(); // QType::ADDR should return both A and AAAA but no TXT, so two entries from the right subnet - BOOST_CHECK_EQUAL(MRC.get(now, power, QType(QType::ADDR), &retrieved, ComboAddress("192.0.2.3"), nullptr), (ttd-now)); + BOOST_CHECK_EQUAL(MRC.get(now, power, QType(QType::ADDR), false, &retrieved, ComboAddress("192.0.2.3"), nullptr), (ttd-now)); BOOST_CHECK_EQUAL(retrieved.size(), 2); for (const auto& rec : retrieved) { BOOST_CHECK(rec.d_type == QType::A || rec.d_type == QType::AAAA); @@ -178,13 +178,13 @@ BOOST_AUTO_TEST_CASE(test_RecursorCacheSimple) { retrieved.clear(); // but only the non-subnet specific one from the another subnet - BOOST_CHECK_EQUAL(MRC.get(now, power, QType(QType::ADDR), &retrieved, ComboAddress("127.0.0.1"), nullptr), (ttd-now)); + BOOST_CHECK_EQUAL(MRC.get(now, power, QType(QType::ADDR), false, &retrieved, ComboAddress("127.0.0.1"), nullptr), (ttd-now)); BOOST_REQUIRE_EQUAL(retrieved.size(), 1); BOOST_CHECK(retrieved.at(0).d_type == QType::A); retrieved.clear(); // entries are only valid until ttd, we should not get anything after that because they are expired - BOOST_CHECK_LT(MRC.get(ttd + 5, power, QType(QType::ADDR), &retrieved, ComboAddress("127.0.0.1"), nullptr), 0); + BOOST_CHECK_LT(MRC.get(ttd + 5, power, QType(QType::ADDR), false, &retrieved, ComboAddress("127.0.0.1"), nullptr), 0); BOOST_CHECK_EQUAL(retrieved.size(), 0); retrieved.clear(); @@ -193,7 +193,7 @@ BOOST_AUTO_TEST_CASE(test_RecursorCacheSimple) { BOOST_CHECK_EQUAL(MRC.doAgeCache(now, power, QType::TXT, newTTL), true); // we should still be able to retrieve it - BOOST_CHECK_EQUAL(MRC.get(now, power, QType(QType::TXT), &retrieved, ComboAddress("127.0.0.1"), nullptr), newTTL); + BOOST_CHECK_EQUAL(MRC.get(now, power, QType(QType::TXT), false, &retrieved, ComboAddress("127.0.0.1"), nullptr), newTTL); BOOST_CHECK_EQUAL(retrieved.size(), 1); BOOST_CHECK(retrieved.at(0).d_type == QType::TXT); // please note that this is still a TTD at this point @@ -201,7 +201,7 @@ BOOST_AUTO_TEST_CASE(test_RecursorCacheSimple) { retrieved.clear(); // but 10s later it should be gone - BOOST_CHECK_LT(MRC.get(now + 10, power, QType(QType::TXT), &retrieved, ComboAddress("127.0.0.1"), nullptr), 0); + BOOST_CHECK_LT(MRC.get(now + 10, power, QType(QType::TXT), false, &retrieved, ComboAddress("127.0.0.1"), nullptr), 0); BOOST_CHECK_EQUAL(retrieved.size(), 0); retrieved.clear(); @@ -214,7 +214,7 @@ BOOST_AUTO_TEST_CASE(test_RecursorCacheSimple) { records.push_back(dr2); MRC.replace(now, power, QType(QType::A), records, signatures, true, boost::none); BOOST_CHECK_EQUAL(MRC.size(), 1); - BOOST_CHECK_EQUAL(MRC.get(now, power, QType(QType::A), &retrieved, ComboAddress("127.0.0.1"), nullptr), (ttd-now)); + BOOST_CHECK_EQUAL(MRC.get(now, power, QType(QType::A), false, &retrieved, ComboAddress("127.0.0.1"), nullptr), (ttd-now)); BOOST_CHECK_EQUAL(retrieved.size(), 1); DNSRecord dr3; @@ -236,14 +236,14 @@ BOOST_AUTO_TEST_CASE(test_RecursorCacheSimple) { // non-auth should not replace valid auth MRC.replace(now, power, QType(QType::A), records, signatures, false, boost::none); BOOST_CHECK_EQUAL(MRC.size(), 1); - BOOST_CHECK_EQUAL(MRC.get(now, power, QType(QType::A), &retrieved, ComboAddress("127.0.0.1"), nullptr), (ttd-now)); + BOOST_CHECK_EQUAL(MRC.get(now, power, QType(QType::A), false, &retrieved, ComboAddress("127.0.0.1"), nullptr), (ttd-now)); BOOST_REQUIRE_EQUAL(retrieved.size(), 1); BOOST_CHECK_EQUAL(getRR(retrieved.at(0))->getCA().toString(), dr2Content.toString()); // but non-auth _should_ replace expired auth MRC.replace(ttd + 1, power, QType(QType::A), records, signatures, false, boost::none); BOOST_CHECK_EQUAL(MRC.size(), 1); - BOOST_CHECK_EQUAL(MRC.get(ttd + 1, power, QType(QType::A), &retrieved, ComboAddress("127.0.0.1"), nullptr), (dr3.d_ttl - (ttd + 1))); + BOOST_CHECK_EQUAL(MRC.get(ttd + 1, power, QType(QType::A), false, &retrieved, ComboAddress("127.0.0.1"), nullptr), (dr3.d_ttl - (ttd + 1))); BOOST_REQUIRE_EQUAL(retrieved.size(), 1); BOOST_CHECK_EQUAL(getRR(retrieved.at(0))->getCA().toString(), dr3Content.toString()); @@ -252,7 +252,7 @@ BOOST_AUTO_TEST_CASE(test_RecursorCacheSimple) { records.push_back(dr2); MRC.replace(now, power, QType(QType::A), records, signatures, false, boost::none); BOOST_CHECK_EQUAL(MRC.size(), 1); - BOOST_CHECK_EQUAL(MRC.get(now, power, QType(QType::A), &retrieved, ComboAddress("127.0.0.1"), nullptr), (ttd-now)); + BOOST_CHECK_EQUAL(MRC.get(now, power, QType(QType::A), false, &retrieved, ComboAddress("127.0.0.1"), nullptr), (ttd-now)); BOOST_REQUIRE_EQUAL(retrieved.size(), 1); BOOST_CHECK_EQUAL(getRR(retrieved.at(0))->getCA().toString(), dr2Content.toString()); diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc index 1f1bab402..fbe0c86e8 100644 --- a/pdns/recursordist/test-syncres_cc.cc +++ b/pdns/recursordist/test-syncres_cc.cc @@ -1726,13 +1726,13 @@ BOOST_AUTO_TEST_CASE(test_cache_min_max_ttl) { const ComboAddress who; vector cached; const time_t now = time(nullptr); - BOOST_REQUIRE_GT(t_RC->get(now, target, QType(QType::A), &cached, who), 0); + BOOST_REQUIRE_GT(t_RC->get(now, target, QType(QType::A), true, &cached, who), 0); BOOST_REQUIRE_EQUAL(cached.size(), 1); BOOST_REQUIRE_GT(cached[0].d_ttl, now); BOOST_CHECK_EQUAL((cached[0].d_ttl - now), SyncRes::s_minimumTTL); cached.clear(); - BOOST_REQUIRE_GT(t_RC->get(now, target, QType(QType::NS), &cached, who), 0); + BOOST_REQUIRE_GT(t_RC->get(now, target, QType(QType::NS), false, &cached, who), 0); BOOST_REQUIRE_EQUAL(cached.size(), 1); BOOST_REQUIRE_GT(cached[0].d_ttl, now); BOOST_CHECK_LE((cached[0].d_ttl - now), SyncRes::s_maxcachettl); diff --git a/pdns/syncres.cc b/pdns/syncres.cc index 5e8d10fa8..c1e966cbb 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -588,6 +588,8 @@ vector SyncRes::getAddrs(const DNSName &qname, unsigned int depth, ret_t ret; QType type; + bool oldRequireAuthData = d_requireAuthData; + d_requireAuthData = false; for(int j=1; j<2+s_doIPv6; j++) { @@ -618,7 +620,7 @@ vector SyncRes::getAddrs(const DNSName &qname, unsigned int depth, if(done) { if(j==1 && s_doIPv6) { // we got an A record, see if we have some AAAA lying around vector cset; - if(t_RC->get(d_now.tv_sec, qname, QType(QType::AAAA), &cset, d_requestor) > 0) { + if(t_RC->get(d_now.tv_sec, qname, QType(QType::AAAA), false, &cset, d_requestor) > 0) { for(auto k=cset.cbegin();k!=cset.cend();++k) { if(k->d_ttl > (unsigned int)d_now.tv_sec ) { if (auto drc = std::dynamic_pointer_cast(k->d_content)) { @@ -633,6 +635,8 @@ vector SyncRes::getAddrs(const DNSName &qname, unsigned int depth, } } + d_requireAuthData = oldRequireAuthData; + if(ret.size() > 1) { random_shuffle(ret.begin(), ret.end(), dns_random); @@ -669,7 +673,7 @@ void SyncRes::getBestNSFromCache(const DNSName &qname, const QType& qtype, vecto LOG(prefix< ns; *flawedNSSet = false; - if(t_RC->get(d_now.tv_sec, subdomain, QType(QType::NS), &ns, d_requestor) > 0) { + if(t_RC->get(d_now.tv_sec, subdomain, QType(QType::NS), false, &ns, d_requestor, nullptr) > 0) { for(auto k=ns.cbegin();k!=ns.cend(); ++k) { if(k->d_ttl > (unsigned int)d_now.tv_sec ) { vector aset; @@ -677,7 +681,7 @@ void SyncRes::getBestNSFromCache(const DNSName &qname, const QType& qtype, vecto const DNSRecord& dr=*k; auto nrr = getRR(dr); if(nrr && (!nrr->getNS().isPartOf(subdomain) || t_RC->get(d_now.tv_sec, nrr->getNS(), s_doIPv6 ? QType(QType::ADDR) : QType(QType::A), - doLog() ? &aset : 0, d_requestor) > 5)) { + false, doLog() ? &aset : nullptr, d_requestor) > 5)) { bestns.push_back(dr); LOG(prefix< '"<getNS()<<"'"<getNS().isPartOf(subdomain)); @@ -794,7 +798,7 @@ bool SyncRes::doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector LOG(prefix< cset; vector> signatures; - if(t_RC->get(d_now.tv_sec, qname,QType(QType::CNAME), &cset, d_requestor, &signatures, &d_wasVariable) > 0) { + if(t_RC->get(d_now.tv_sec, qname, QType(QType::CNAME), d_requireAuthData, &cset, d_requestor, &signatures, &d_wasVariable) > 0) { for(auto j=cset.cbegin() ; j != cset.cend() ; ++j) { if(j->d_ttl>(unsigned int) d_now.tv_sec) { @@ -913,7 +917,7 @@ bool SyncRes::doCacheCheck(const DNSName &qname, const QType &qtype, vector> signatures; uint32_t ttl=0; - if(t_RC->get(d_now.tv_sec, sqname, sqt, &cset, d_requestor, d_doDNSSEC ? &signatures : 0, &d_wasVariable) > 0) { + if(t_RC->get(d_now.tv_sec, sqname, sqt, d_requireAuthData, &cset, d_requestor, d_doDNSSEC ? &signatures : nullptr, &d_wasVariable) > 0) { LOG(prefix<d_content->getZoneRepresentation()); diff --git a/pdns/syncres.hh b/pdns/syncres.hh index f644fa4c0..30dc6ecac 100644 --- a/pdns/syncres.hh +++ b/pdns/syncres.hh @@ -749,6 +749,7 @@ private: bool d_doDNSSEC; bool d_doEDNS0{true}; bool d_incomingECSFound{false}; + bool d_requireAuthData{true}; bool d_skipCNAMECheck{false}; bool d_updatingRootNS{false}; bool d_wantsRPZ{true}; -- 2.40.0