rec: Fix validation when more than one cached record is returned

We need to validate them RRSet by RRSet.

diff --git a/pdns/recursor_cache.cc b/pdns/recursor_cache.cc
index 2169670d696b39d69cc65df18ec8ec707969b5c1..d819dac29b80adf01693b29e2d43d2dfd03405e6 100644 (file)
--- a/pdns/recursor_cache.cc
+++ b/pdns/recursor_cache.cc
@@ -79,7 +79,39 @@ size_t MemRecursorCache::bytes()
   return ret;
 }
 
-int32_t MemRecursorCache::handleHit(MapCombo& map, MemRecursorCache::OrderedTagIterator_t& entry, const DNSName& qname, vector<DNSRecord>* res, vector<std::shared_ptr<RRSIGRecordContent>>* signatures, std::vector<std::shared_ptr<DNSRecord>>* authorityRecs, bool* variable, vState* state, bool* wasAuth)
+static void updateDNSSECValidationStateFromCache(boost::optional<vState>& state, const vState stateUpdate)
+{
+  // if there was no state it's easy */
+  if (state == boost::none) {
+    state = stateUpdate;
+    return;
+  }
+
+  if (stateUpdate == vState::TA) {
+    state = vState::Secure;
+  }
+  else if (stateUpdate == vState::NTA) {
+    state = vState::Insecure;
+  }
+  else if (stateUpdate == vState::Bogus) {
+    state = stateUpdate;
+  }
+  else if (stateUpdate == vState::Indeterminate) {
+    state = stateUpdate;
+  }
+  else if (stateUpdate == vState::Insecure) {
+    if (*state != vState::Bogus && *state != vState::Indeterminate) {
+      state = stateUpdate;
+    }
+  }
+  else if (stateUpdate == vState::Secure) {
+    if (*state != vState::Bogus && *state != vState::Indeterminate) {
+      state = stateUpdate;
+    }
+  }
+}
+
+int32_t MemRecursorCache::handleHit(MapCombo& map, MemRecursorCache::OrderedTagIterator_t& entry, const DNSName& qname, vector<DNSRecord>* res, vector<std::shared_ptr<RRSIGRecordContent>>* signatures, std::vector<std::shared_ptr<DNSRecord>>* authorityRecs, bool* variable, boost::optional<vState>& state, bool* wasAuth)
 {
   // MUTEX SHOULD BE ACQUIRED
   int32_t ttd = entry->d_ttd;
@@ -112,9 +144,7 @@ int32_t MemRecursorCache::handleHit(MapCombo& map, MemRecursorCache::OrderedTagI
     authorityRecs->insert(authorityRecs->end(), entry->d_authorityRecs.begin(), entry->d_authorityRecs.end());
   }
 
-  if (state) {
-    updateDNSSECValidationState(*state, entry->d_state);
-  }
+  updateDNSSECValidationStateFromCache(state, entry->d_state);
 
   if (wasAuth) {
     *wasAuth = *wasAuth && entry->d_auth;
@@ -218,6 +248,7 @@ bool MemRecursorCache::entryMatches(MemRecursorCache::OrderedTagIterator_t& entr
 // 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, const OptTag& routingTag, vector<std::shared_ptr<RRSIGRecordContent>>* signatures, std::vector<std::shared_ptr<DNSRecord>>* authorityRecs, bool* variable, vState* state, bool* wasAuth)
 {
+  boost::optional<vState> cachedState{boost::none};
   time_t ttd=0;
   //  cerr<<"looking up "<< qname<<"|"+qt.getName()<<"\n";
   if(res) {
@@ -229,9 +260,6 @@ int32_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt,
     // so it will be set to false if at least one entry is not auth
     *wasAuth = true;
   }
-  if (state) {
-    *state = vState::Indeterminate;
-  }
 
   auto& map = getMap(qname);
   const lock l(map);
@@ -244,23 +272,32 @@ int32_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt,
 
       auto entryA = getEntryUsingECSIndex(map, now, qname, QType::A, requireAuth, who);
       if (entryA != map.d_map.end()) {
-        ret = handleHit(map, entryA, qname, res, signatures, authorityRecs, variable, state, wasAuth);
+        ret = handleHit(map, entryA, qname, res, signatures, authorityRecs, variable, cachedState, wasAuth);
       }
       auto entryAAAA = getEntryUsingECSIndex(map, now, qname, QType::AAAA, requireAuth, who);
       if (entryAAAA != map.d_map.end()) {
-        int32_t ttdAAAA = handleHit(map, entryAAAA, qname, res, signatures, authorityRecs, variable, state, wasAuth);
+        int32_t ttdAAAA = handleHit(map, entryAAAA, qname, res, signatures, authorityRecs, variable, cachedState, wasAuth);
         if (ret > 0) {
           ret = std::min(ret, ttdAAAA);
         } else {
           ret = ttdAAAA;
         }
       }
+
+      if (state && cachedState) {
+        *state = *cachedState;
+      }
+
       return ret > 0 ? static_cast<int32_t>(ret-now) : ret;
     }
     else {
       auto entry = getEntryUsingECSIndex(map, now, qname, qtype, requireAuth, who);
       if (entry != map.d_map.end()) {
-        return static_cast<int32_t>(handleHit(map, entry, qname, res, signatures, authorityRecs, variable, state, wasAuth) - now);
+        int32_t ret = handleHit(map, entry, qname, res, signatures, authorityRecs, variable, cachedState, wasAuth);
+        if (state && cachedState) {
+          *state = *cachedState;
+        }
+        return static_cast<int32_t>(ret-now);
       }
       return -1;
     }
@@ -282,12 +319,15 @@ int32_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt,
           continue;
         }
 
-        ttd = handleHit(map, firstIndexIterator, qname, res, signatures, authorityRecs, variable, state, wasAuth);
+        ttd = handleHit(map, firstIndexIterator, qname, res, signatures, authorityRecs, variable, cachedState, wasAuth);
 
         if (qt.getCode() != QType::ANY && qt.getCode() != QType::ADDR) { // normally if we have a hit, we are done
           break;
         }
       }
+      if (state && cachedState) {
+        *state = *cachedState;
+      }
       return static_cast<int32_t>(ttd-now);
     }
   }
@@ -307,12 +347,15 @@ int32_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt,
         continue;
       }
 
-      ttd = handleHit(map, firstIndexIterator, qname, res, signatures, authorityRecs, variable, state, wasAuth);
+      ttd = handleHit(map, firstIndexIterator, qname, res, signatures, authorityRecs, variable, cachedState, wasAuth);
 
       if (qt.getCode() != QType::ANY && qt.getCode() != QType::ADDR) { // normally if we have a hit, we are done
         break;
       }
     }
+    if (state && cachedState) {
+      *state = *cachedState;
+    }
     return static_cast<int32_t>(ttd-now);
   }
   return -1;

diff --git a/pdns/recursor_cache.hh b/pdns/recursor_cache.hh
index 8166ddab8afc76e94c134e56b850a22040181947..b2cbca1125b044cad6b4a912a4e0f110306495d5 100644 (file)
--- a/pdns/recursor_cache.hh
+++ b/pdns/recursor_cache.hh
@@ -230,7 +230,7 @@ private:
   bool entryMatches(OrderedTagIterator_t& entry, uint16_t qt, bool requireAuth, const ComboAddress& who);
   Entries getEntries(MapCombo& map, const DNSName &qname, const QType& qt, const OptTag& rtag);
   cache_t::const_iterator getEntryUsingECSIndex(MapCombo& map, time_t now, const DNSName &qname, uint16_t qtype, bool requireAuth, const ComboAddress& who);
-  int32_t handleHit(MapCombo& map, OrderedTagIterator_t& entry, const DNSName& qname, vector<DNSRecord>* res, vector<std::shared_ptr<RRSIGRecordContent>>* signatures, std::vector<std::shared_ptr<DNSRecord>>* authorityRecs, bool* variable, vState* state, bool* wasAuth);
+  int32_t handleHit(MapCombo& map, OrderedTagIterator_t& entry, const DNSName& qname, vector<DNSRecord>* res, vector<std::shared_ptr<RRSIGRecordContent>>* signatures, std::vector<std::shared_ptr<DNSRecord>>* authorityRecs, bool* variable, boost::optional<vState>& state, bool* wasAuth);
 
 public:
   struct lock {

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index b1049fc0d24c84100da08ef853a9155ba8b2b817..87db2e7c9a8dfc3037bd17e1d21e23a65e550b31 100644 (file)
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -1551,6 +1551,20 @@ static void reapRecordsFromNegCacheEntryForValidation(tcache_t& tcache, const ve
   }
 }
 
+static void reapRecordsForValidation(std::map<uint16_t, CacheEntry>& entries, const vector<DNSRecord>& records)
+{
+  for (const auto& rec : records) {
+    entries[rec.d_type].records.push_back(rec);
+  }
+}
+
+static void reapSignaturesForValidation(std::map<uint16_t, CacheEntry>& entries, const vector<std::shared_ptr<RRSIGRecordContent>>& signatures)
+{
+  for (const auto& sig : signatures) {
+    entries[sig->d_type].signatures.push_back(sig);
+  }
+}
+
 /*!
  * Convenience function to push the records from records into ret with a new TTL
  *
@@ -1748,7 +1762,25 @@ bool SyncRes::doCacheCheck(const DNSName &qname, const DNSName& authname, bool w
           cachedState = validateDNSKeys(sqname, cset, signatures, depth);
         }
         else {
-          cachedState = SyncRes::validateRecordsWithSigs(depth, sqname, sqt, sqname, cset, signatures);
+          if (sqt == QType::ANY) {
+            std::map<uint16_t, CacheEntry> types;
+            reapRecordsForValidation(types, cset);
+            reapSignaturesForValidation(types, signatures);
+
+            for (const auto& type : types) {
+              vState cachedRecordState;
+              if (type.first == QType::DNSKEY) {
+                cachedRecordState = validateDNSKeys(sqname, type.second.records, type.second.signatures, depth);
+              }
+              else {
+                cachedRecordState = SyncRes::validateRecordsWithSigs(depth, sqname, QType(type.first), sqname, type.second.records, type.second.signatures);
+              }
+              updateDNSSECValidationState(cachedState, cachedRecordState);
+            }
+          }
+          else {
+            cachedState = SyncRes::validateRecordsWithSigs(depth, sqname, sqt, sqname, cset, signatures);
+          }
         }
       }
       else {
@@ -2661,7 +2693,7 @@ vState SyncRes::validateRecordsWithSigs(unsigned int depth, const DNSName& qname
     recordcontents.insert(record.d_content);
   }
 
-  LOG(d_prefix<<"Going to validate "<<recordcontents.size()<< " record contents with "<<signatures.size()<<" sigs and "<<keys.size()<<" keys for "<<name<<endl);
+  LOG(d_prefix<<"Going to validate "<<recordcontents.size()<< " record contents with "<<signatures.size()<<" sigs and "<<keys.size()<<" keys for "<<name<<"|"<<qtype.getName()<<endl);
   if (validateWithKeySet(d_now.tv_sec, name, recordcontents, signatures, keys, false)) {
     LOG(d_prefix<<"Secure!"<<endl);
     return vState::Secure;