rec: Update the validation of previously inserted records when encountering a DNSSEC...

Signed-off-by: Remi Gacogne <remi.gacogne@powerdns.com>

diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc
index e94c843147bd9f2b9ae24e3a5a8d2d24b37ef1dd..ddaf728a607b754682b90c70cafedff6dc6e54b1 100644 (file)
--- a/pdns/recursordist/syncres.cc
+++ b/pdns/recursordist/syncres.cc
@@ -2706,8 +2706,13 @@ struct CacheEntry
 {
   vector<DNSRecord> records;
   MemRecursorCache::SigRecsVec signatures;
+  // for wildcard records, we need the real owner name for the aggressive cache
+  DNSName realOwner;
   time_t d_ttl_time{0};
   uint32_t signaturesTTL{std::numeric_limits<uint32_t>::max()};
+  vState validationState{vState::Indeterminate};
+  bool inserted{false};
+  bool isAuth{false};
 };
 struct CacheKey
 {
@@ -4710,7 +4715,7 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, const string&
     }
   }
 
-  bool seenBogusRRSet = false;
+  std::optional<vState> seenBogusRRSet{std::nullopt};
   std::vector<tcache_t::value_type> aggrCacheRecords;
   bool insertIntoAggressiveCache = false;
   for (auto tCacheEntry = tcache.begin(); tCacheEntry != tcache.end(); ++tCacheEntry) {
@@ -4813,13 +4818,14 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, const string&
       }
 
       LOG(prefix << qname << ": Validation result is " << recordState << ", current state is " << state << endl);
+      tCacheEntry->second.validationState = recordState;
       if (state != recordState) {
         updateValidationState(qname, state, recordState, prefix);
       }
     }
 
     if (vStateIsBogus(recordState)) {
-      seenBogusRRSet = true;
+      seenBogusRRSet = recordState;
       /* this is a TTD by now, be careful */
       for (auto& record : tCacheEntry->second.records) {
         auto newval = std::min(record.d_ttl, static_cast<uint32_t>(s_maxbogusttl + d_now.tv_sec));
@@ -4882,7 +4888,10 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, const string&
             thisRRNeedsWildcardProof = true;
           }
         }
-        g_recCache->replace(d_now.tv_sec, tCacheEntry->first.name, tCacheEntry->first.type, tCacheEntry->second.records, tCacheEntry->second.signatures, thisRRNeedsWildcardProof ? authorityRecs : *MemRecursorCache::s_emptyAuthRecs, tCacheEntry->first.type == QType::DS ? true : isAA, auth, tCacheEntry->first.place == DNSResourceRecord::ANSWER ? ednsmask : std::nullopt, d_routingTag, recordState, MemRecursorCache::Extra{remoteIP, overTCP}, d_refresh, tCacheEntry->second.d_ttl_time);
+
+        tCacheEntry->second.isAuth = tCacheEntry->first.type == QType::DS ? true : isAA;
+        g_recCache->replace(d_now.tv_sec, tCacheEntry->first.name, tCacheEntry->first.type, tCacheEntry->second.records, tCacheEntry->second.signatures, thisRRNeedsWildcardProof ? authorityRecs : *MemRecursorCache::s_emptyAuthRecs, tCacheEntry->second.isAuth, auth, tCacheEntry->first.place == DNSResourceRecord::ANSWER ? ednsmask : std::nullopt, d_routingTag, recordState, MemRecursorCache::Extra{remoteIP, overTCP}, d_refresh, tCacheEntry->second.d_ttl_time);
+        tCacheEntry->second.inserted = true;
 
         // Delete potential negcache entry. When a record recovers with serve-stale the negcache entry can cause the wrong entry to
         // be served, as negcache entries are checked before record cache entries
@@ -4897,17 +4906,17 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, const string&
           const auto labelCount = tCacheEntry->first.name.countLabels();
 
           if (isWildcardExpanded(labelCount, *rrsig) && !isWildcardExpandedOntoItself(tCacheEntry->first.name, labelCount, *rrsig)) {
-            DNSName realOwner = getNSECOwnerName(tCacheEntry->first.name, tCacheEntry->second.signatures);
+            tCacheEntry->second.realOwner = getNSECOwnerName(tCacheEntry->first.name, tCacheEntry->second.signatures);
 
             std::vector<DNSRecord> content;
             content.reserve(tCacheEntry->second.records.size());
             for (const auto& record : tCacheEntry->second.records) {
               DNSRecord nonExpandedRecord(record);
-              nonExpandedRecord.d_name = realOwner;
+              nonExpandedRecord.d_name = tCacheEntry->second.realOwner;
               content.push_back(std::move(nonExpandedRecord));
             }
 
-            g_recCache->replace(d_now.tv_sec, realOwner, QType(tCacheEntry->first.type), content, tCacheEntry->second.signatures, /* no additional records in that case */ {}, tCacheEntry->first.type == QType::DS ? true : isAA, auth, std::nullopt, MemRecursorCache::NOTAG, recordState, MemRecursorCache::Extra{remoteIP, overTCP}, d_refresh, tCacheEntry->second.d_ttl_time);
+            g_recCache->replace(d_now.tv_sec, tCacheEntry->second.realOwner, QType(tCacheEntry->first.type), content, tCacheEntry->second.signatures, /* no additional records in that case */ {}, tCacheEntry->second.isAuth, auth, std::nullopt, MemRecursorCache::NOTAG, recordState, MemRecursorCache::Extra{remoteIP, overTCP}, d_refresh, tCacheEntry->second.d_ttl_time);
           }
         }
       }
@@ -4941,6 +4950,25 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, const string&
     }
   }
 
+  if (seenBogusRRSet) {
+    /* We might have inserted RRSets with a Secure validation status then
+       later encountered a validation error, so let's go back and update
+       the status and TTD of previous records.
+       This is not ideal but the correct solution would be to move the validation
+       (including denial of existence) BEFORE dealing with the cache.
+    */
+    for (const auto& entry : tcache) {
+      if (!entry.second.inserted || vStateIsBogus(entry.second.validationState)) {
+        continue;
+      }
+
+      g_recCache->updateValidationStatus(d_now.tv_sec, entry.first.name, entry.first.type, d_cacheRemote, d_routingTag, entry.second.isAuth, *seenBogusRRSet, s_maxbogusttl + d_now.tv_sec);
+      if (!entry.second.realOwner.empty()) {
+        g_recCache->updateValidationStatus(d_now.tv_sec, entry.second.realOwner, entry.first.type, d_cacheRemote, MemRecursorCache::NOTAG, entry.second.isAuth, *seenBogusRRSet, s_maxbogusttl + d_now.tv_sec);
+      }
+    }
+  }
+
   // The primary loop determined if we want to take the NSEC(3) records
   if (insertIntoAggressiveCache) {
     for (const auto& entry : aggrCacheRecords) {