From: Remi Gacogne Date: Fri, 26 Jun 2026 14:59:36 +0000 (+0200) Subject: rec: Update the validation of previously inserted records when encountering a DNSSEC... X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=89ac4dc8303675def73c3f5a71a659352c8fc36b;p=thirdparty%2Fpdns.git rec: Update the validation of previously inserted records when encountering a DNSSEC validation failure Signed-off-by: Remi Gacogne --- diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc index e94c843147..ddaf728a60 100644 --- a/pdns/recursordist/syncres.cc +++ b/pdns/recursordist/syncres.cc @@ -2706,8 +2706,13 @@ struct CacheEntry { vector 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::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 seenBogusRRSet{std::nullopt}; std::vector 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(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 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) {