rec: Only use non-AA data to get NS / DS / glues

diff --git a/pdns/recursor_cache.cc b/pdns/recursor_cache.cc
index 6d0eb5eb618ee71714dd04f203b084efd0e361e1..dd1016ffa14a07c5aeea613cd908a3e1cc4d60c8 100644 (file)
--- 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<DNSRecord>* res, const ComboAddress& who, vector<std::shared_ptr<RRSIGRecordContent>>* signatures, bool* variable)
+int32_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt, bool requireAuth, vector<DNSRecord>* res, const ComboAddress& who, vector<std::shared_ptr<RRSIGRecordContent>>* 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 : "<<ttd - now<<", "<< (res ? res->size() : 0) <<"\n";
     return static_cast<int32_t>(ttd-now);

diff --git a/pdns/recursor_cache.hh b/pdns/recursor_cache.hh
index 0641ff53144a3cce4f2aa4bde757e5c5f7d44e89..709461424cb837b4332456f40afce3405dc38b38 100644 (file)
--- 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<DNSRecord>* res, const ComboAddress& who, vector<std::shared_ptr<RRSIGRecordContent>>* signatures=0, bool* variable=0);
+  int32_t get(time_t, const DNSName &qname, const QType& qt, bool requireAuth, vector<DNSRecord>* res, const ComboAddress& who, vector<std::shared_ptr<RRSIGRecordContent>>* signatures=0, bool* variable=0);
 
   void replace(time_t, const DNSName &qname, const QType& qt,  const vector<DNSRecord>& content, const vector<shared_ptr<RRSIGRecordContent>>& signatures, bool auth, boost::optional<Netmask> ednsmask=boost::none);
   void doPrune(void);

diff --git a/pdns/recursordist/test-recursorcache_cc.cc b/pdns/recursordist/test-recursorcache_cc.cc
index 1974bbea8ac317444d95d033e03d86012f068727..a0539d69b293a35c526fc8c5af68258e0b3ec915 100644 (file)
--- 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<AAAARecordContent>(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<ARecordContent>(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<ARecordContent>(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<AAAARecordContent>(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<ARecordContent>(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<ARecordContent>(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<ARecordContent>(retrieved.at(0))->getCA().toString(), dr2Content.toString());
 

diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc
index 1f1bab402681f2d89505ab4a9737fa6d46b6bfd2..fbe0c86e8b8e1897fed3ee86b022a8ddd248cc62 100644 (file)
--- 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<DNSRecord> 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 5e8d10fa81708bebd4a4927120615b7c7c374e3c..c1e966cbb09cb752e922d9caba913d992bd3dc27 100644 (file)
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -588,6 +588,8 @@ vector<ComboAddress> 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<ComboAddress> 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<DNSRecord> 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<AAAARecordContent>(k->d_content)) {
@@ -633,6 +635,8 @@ vector<ComboAddress> 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<<qname<<": Checking if we have NS in cache for '"<<subdomain<<"'"<<endl);
     vector<DNSRecord> 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<DNSRecord> aset;
@@ -677,7 +681,7 @@ void SyncRes::getBestNSFromCache(const DNSName &qname, const QType& qtype, vecto
           const DNSRecord& dr=*k;
          auto nrr = getRR<NSRecordContent>(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<<qname<<": NS (with ip, or non-glue) in cache for '"<<subdomain<<"' -> '"<<nrr->getNS()<<"'"<<endl);
             LOG(prefix<<qname<<": within bailiwick: "<< nrr->getNS().isPartOf(subdomain));
@@ -794,7 +798,7 @@ bool SyncRes::doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector
   LOG(prefix<<qname<<": Looking for CNAME cache hit of '"<<qname<<"|CNAME"<<"'"<<endl);
   vector<DNSRecord> cset;
   vector<std::shared_ptr<RRSIGRecordContent>> 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<DNSR
   bool found=false, expired=false;
   vector<std::shared_ptr<RRSIGRecordContent>> 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<<sqname<<": Found cache hit for "<<sqt.getName()<<": ");
     for(auto j=cset.cbegin() ; j != cset.cend() ; ++j) {
       LOG(j->d_content->getZoneRepresentation());

diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index f644fa4c04a8a2eea152b1a7702b66a221262b1a..30dc6ecac0d3c61b76c1d94b80761b19c869ce2c 100644 (file)
--- 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};