rec: Update validation status of records cached as Indeterminate

diff --git a/pdns/recursor_cache.cc b/pdns/recursor_cache.cc
index 765216efa91241e82120f99102de78b21f098b59..4105055633c8e6bb8a723a7bf19eec131f30557b 100644 (file)
--- a/pdns/recursor_cache.cc
+++ b/pdns/recursor_cache.cc
@@ -34,9 +34,8 @@ unsigned int MemRecursorCache::bytes()
 }
 
 // returns -1 for no hits
-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, std::vector<std::shared_ptr<DNSRecord>>* authorityRecs, bool* variable, vState* state, bool* wasAuth)
+std::pair<MemRecursorCache::cache_t::const_iterator, MemRecursorCache::cache_t::const_iterator> MemRecursorCache::getEntries(const DNSName &qname, const QType& qt)
 {
-  time_t ttd=0;
   //  cerr<<"looking up "<< qname<<"|"+qt.getName()<<"\n";
 
   if(!d_cachecachevalid || d_cachedqname!= qname) {
@@ -47,58 +46,79 @@ int32_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt,
   }
   //  else cerr<<"had cache cache hit!"<<endl;
 
+  return d_cachecache;
+}
+
+bool MemRecursorCache::entryMatches(cache_t::const_iterator& entry, const QType& qt, bool requireAuth, const ComboAddress& who)
+{
+  if (requireAuth && !entry->d_auth)
+    return false;
+
+  return ((entry->d_qtype == qt.getCode() || qt.getCode()==QType::ANY ||
+           (qt.getCode()==QType::ADDR && (entry->d_qtype == QType::A || entry->d_qtype == QType::AAAA)))
+          && (entry->d_netmask.empty() || entry->d_netmask.match(who)));
+}
+
+// returns -1 for no hits
+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, std::vector<std::shared_ptr<DNSRecord>>* authorityRecs, bool* variable, vState* state, bool* wasAuth)
+{
+  time_t ttd=0;
+  //  cerr<<"looking up "<< qname<<"|"+qt.getName()<<"\n";
+
+  auto entries = getEntries(qname, qt);
+
   if(res)
     res->clear();
 
-  if(d_cachecache.first!=d_cachecache.second) {
-    for(cache_t::const_iterator i=d_cachecache.first; i != d_cachecache.second; ++i) {
-      if (requireAuth && !i->d_auth)
+  if(entries.first!=entries.second) {
+    for(cache_t::const_iterator i=entries.first; i != entries.second; ++i) {
+
+      if (i->d_ttd <= now) {
+        moveCacheItemToFront(d_cache, i);
         continue;
+      }
 
-      //cerr<<"TTD is "<<i->d_ttd<<", now is "<<now<<", type is "<<i->d_qtype<<endl;
-      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)))
-         ) {
-        if(variable && !i->d_netmask.empty()) {
-          *variable=true;
-        }
-       ttd = i->d_ttd; 
-        //        cerr<<"Looking at "<<i->d_records.size()<<" records for this name"<<endl;
-       for(auto k=i->d_records.begin(); k != i->d_records.end(); ++k) {
-         if(res) {
-           DNSRecord dr;
-           dr.d_name = qname;
-           dr.d_type = i->d_qtype;
-           dr.d_class = QClass::IN;
-           dr.d_content = *k; 
-           dr.d_ttl = static_cast<uint32_t>(i->d_ttd);
-           dr.d_place = DNSResourceRecord::ANSWER;
-           res->push_back(dr);
-         }
-       }
-      
-       if(signatures)  // if you do an ANY lookup you are hosed XXXX
-         *signatures=i->d_signatures;
-
-       if(authorityRecs)  // if you do an ANY lookup you are hosed here too XXXX
-         *authorityRecs=i->d_authorityRecs;
+      if (!entryMatches(i, qt, requireAuth, who))
+        continue;
+
+      if(variable && !i->d_netmask.empty()) {
+        *variable=true;
+      }
+
+      ttd = i->d_ttd;
 
+      //        cerr<<"Looking at "<<i->d_records.size()<<" records for this name"<<endl;
+      for(auto k=i->d_records.begin(); k != i->d_records.end(); ++k) {
         if(res) {
-          if(res->empty())
-            moveCacheItemToFront(d_cache, i);
-          else
-            moveCacheItemToBack(d_cache, i);
+          DNSRecord dr;
+          dr.d_name = qname;
+          dr.d_type = i->d_qtype;
+          dr.d_class = QClass::IN;
+          dr.d_content = *k;
+          dr.d_ttl = static_cast<uint32_t>(i->d_ttd);
+          dr.d_place = DNSResourceRecord::ANSWER;
+          res->push_back(dr);
         }
-        if(state) {
-          *state = i->d_state;
-        }
-        if(wasAuth) {
-          *wasAuth = i->d_auth;
-        }
-        if(qt.getCode()!=QType::ANY && qt.getCode()!=QType::ADDR) // normally if we have a hit, we are done
-          break;
       }
+
+      if(signatures)  // if you do an ANY lookup you are hosed XXXX
+        *signatures=i->d_signatures;
+
+      if(authorityRecs)  // if you do an ANY lookup you are hosed here too XXXX
+        *authorityRecs=i->d_authorityRecs;
+
+      moveCacheItemToBack(d_cache, i);
+
+      if(state) {
+        *state = i->d_state;
+      }
+
+      if(wasAuth) {
+        *wasAuth = i->d_auth;
+      }
+
+      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";
@@ -107,8 +127,6 @@ int32_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt,
   return -1;
 }
 
-
-
 bool MemRecursorCache::attemptToRefreshNSTTL(const QType& qt, const vector<DNSRecord>& content, const CacheEntry& stored)
 {
   if(!stored.d_auth) {
@@ -261,6 +279,27 @@ bool MemRecursorCache::doAgeCache(time_t now, const DNSName& name, uint16_t qtyp
   return false;
 }
 
+bool MemRecursorCache::updateValidationStatus(const DNSName &qname, const QType& qt, const ComboAddress& who, bool requireAuth, vState newState)
+{
+  bool updated = false;
+  auto entries = getEntries(qname, qt);
+
+  for(auto i = entries.first; i != entries.second; ++i) {
+
+    if (!entryMatches(i, qt, requireAuth, who))
+      continue;
+
+    i->d_state = newState;
+    updated = true;
+
+    if(qt.getCode()!=QType::ANY && qt.getCode()!=QType::ADDR) // normally if we have a hit, we are done
+      break;
+
+  }
+
+  return updated;
+}
+
 uint64_t MemRecursorCache::doDump(int fd)
 {
   FILE* fp=fdopen(dup(fd), "w");

diff --git a/pdns/recursor_cache.hh b/pdns/recursor_cache.hh
index 1eacf12b34aabcc89f2ec2c86eaf58143a89c9a7..0f3fbc91bb5527d27102660db576ee6bad2517b0 100644 (file)
--- a/pdns/recursor_cache.hh
+++ b/pdns/recursor_cache.hh
@@ -63,6 +63,7 @@ public:
 
   int doWipeCache(const DNSName& name, bool sub, uint16_t qtype=0xffff);
   bool doAgeCache(time_t now, const DNSName& name, uint16_t qtype, uint32_t newTTL);
+  bool updateValidationStatus(const DNSName &qname, const QType& qt, const ComboAddress& who, bool requireAuth, vState newState);
 
   uint64_t cacheHits, cacheMisses;
 
@@ -88,7 +89,7 @@ private:
     records_t d_records;
     std::vector<std::shared_ptr<DNSRecord>> d_authorityRecs;
     Netmask d_netmask;
-    vState d_state;
+    mutable vState d_state;
   };
 
   typedef multi_index_container<
@@ -111,6 +112,10 @@ private:
   pair<cache_t::iterator, cache_t::iterator> d_cachecache;
   DNSName d_cachedqname;
   bool d_cachecachevalid;
+
   bool attemptToRefreshNSTTL(const QType& qt, const vector<DNSRecord>& content, const CacheEntry& stored);
+  bool entryMatches(cache_t::const_iterator& entry, const QType& qt, bool requireAuth, const ComboAddress& who);
+  std::pair<cache_t::const_iterator, cache_t::const_iterator> getEntries(const DNSName &qname, const QType& qt);
+
 };
 #endif

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 07957a3f7889f4b98aed454bd19b26c2b3980472..4ec365d4e153d4fc165fc3541cb185a858960018 100644 (file)
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -823,9 +823,15 @@ bool SyncRes::doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector
         if (validationEnabled() && 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. */
-          LOG(prefix<<qname<<": got Indeterminate state from the CNAME cache, validating.."<<endl);
-          state = SyncRes::validateRecordsWithSigs(depth, qname, QType(QType::CNAME), qname, cset, signatures);
-          LOG(prefix<<qname<<": got Indeterminate state from the CNAME cache, validation result is "<<vStates[state]<<endl);
+          vState recordState = getValidationStatus(qname);
+          if (recordState == Secure) {
+            LOG(prefix<<qname<<": got Indeterminate state from the CNAME cache, validating.."<<endl);
+            state = SyncRes::validateRecordsWithSigs(depth, qname, QType(QType::CNAME), qname, cset, signatures);
+            if (state != Indeterminate) {
+              LOG(prefix<<qname<<": got Indeterminate state from the CNAME cache, new validation result is "<<vStates[state]<<endl);
+              t_RC->updateValidationStatus(qname, QType(QType::CNAME), d_requestor, d_requireAuthData, state);
+            }
+          }
         }
 
         LOG(prefix<<qname<<": Found cache CNAME hit for '"<< qname << "|CNAME" <<"' to '"<<j->d_content->getZoneRepresentation()<<"', validation state is "<<vStates[state]<<endl);
@@ -971,9 +977,16 @@ bool SyncRes::doCacheCheck(const DNSName &qname, const QType &qtype, vector<DNSR
 
       /* 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. */
-      LOG(prefix<<sqname<<": got Indeterminate state from the cache, validating.."<<endl);
-      cachedState = SyncRes::validateRecordsWithSigs(depth, sqname, sqt, sqname, cset, signatures);
-      LOG(prefix<<qname<<": got Indeterminate state from the cache, validation result is "<<vStates[cachedState]<<endl);
+      vState recordState = getValidationStatus(qname);
+      if (recordState == Secure) {
+        LOG(prefix<<sqname<<": got Indeterminate state from the cache, validating.."<<endl);
+        cachedState = SyncRes::validateRecordsWithSigs(depth, sqname, sqt, sqname, cset, signatures);
+
+        if (cachedState != Indeterminate) {
+          LOG(prefix<<qname<<": got Indeterminate state from the cache, validation result is "<<vStates[cachedState]<<endl);
+          t_RC->updateValidationStatus(sqname, sqt, d_requestor, d_requireAuthData, cachedState);
+        }
+      }
     }
 
     for(auto j=cset.cbegin() ; j != cset.cend() ; ++j) {