From: Remi Gacogne Date: Fri, 1 Mar 2024 13:07:35 +0000 (+0100) Subject: rec: Fix gathering of denial of existence proof for wildcard-expanded names X-Git-Tag: dnsdist-1.10.0-alpha0~20^2~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2eb9f095fe06f77cd816135c03c7ac558e0f324d;p=thirdparty%2Fpdns.git rec: Fix gathering of denial of existence proof for wildcard-expanded names When the recursor is forwarding to a resolver, we accept the names composing the CNAME chain starting at the queried name. This means we also need to gather the denial of existence proof for CNAMEs that were expanded from a wildcard, otherwise the response sent to the client cannot be DNSSEC-validated. --- diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc index 48c4acc2b3..7044bb0408 100644 --- a/pdns/recursordist/syncres.cc +++ b/pdns/recursordist/syncres.cc @@ -4353,11 +4353,37 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, const string& sanitizeRecords(prefix, lwr, qname, qtype, auth, wasForwarded, rdQuery); std::vector> authorityRecs; - const unsigned int labelCount = qname.countLabels(); bool isCNAMEAnswer = false; bool isDNAMEAnswer = false; DNSName seenAuth; + // names that might be expanded from a wildcard, and thus require denial of existence proof + // this is the queried name and any part of the CNAME chain from the queried name + // the key is the name itself, the value is initially false and is set to true once we have + // confirmed it was actually expanded from a wildcard + std::map wildcardCandidates{{qname, false}}; + + if (rdQuery) { + std::unordered_map cnames; + for (const auto& rec : lwr.d_records) { + if (rec.d_type != QType::CNAME || rec.d_class != QClass::IN) { + continue; + } + if (auto content = getRR(rec)) { + cnames[rec.d_name] = DNSName(content->getTarget()); + } + } + auto initial = qname; + while (true) { + auto it = cnames.find(initial); + if (it == cnames.end()) { + break; + } + initial = it->second; + wildcardCandidates.emplace(initial, false); + } + } + for (auto& rec : lwr.d_records) { if (rec.d_type == QType::OPT || rec.d_class != QClass::IN) { continue; @@ -4377,6 +4403,7 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, const string& seenAuth = rec.d_name; } + const auto labelCount = rec.d_name.countLabels(); if (rec.d_type == QType::RRSIG) { auto rrsig = getRR(rec); if (rrsig) { @@ -4384,7 +4411,8 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, const string& count can be lower than the name's label count if it was synthesized from the wildcard. Note that the difference might be > 1. */ - if (rec.d_name == qname && isWildcardExpanded(labelCount, *rrsig)) { + if (wildcardCandidates.count(rec.d_name) > 0 && isWildcardExpanded(labelCount, *rrsig)) { + wildcardCandidates[rec.d_name] = true; gatherWildcardProof = true; if (!isWildcardExpandedOntoItself(rec.d_name, labelCount, *rrsig)) { /* if we have a wildcard expanded onto itself, we don't need to prove @@ -4661,7 +4689,13 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, const string& if (isAA && tCacheEntry->first.type == QType::NS && s_save_parent_ns_set) { rememberParentSetIfNeeded(tCacheEntry->first.name, tCacheEntry->second.records, depth, prefix); } - g_recCache->replace(d_now.tv_sec, tCacheEntry->first.name, tCacheEntry->first.type, tCacheEntry->second.records, tCacheEntry->second.signatures, authorityRecs, tCacheEntry->first.type == QType::DS ? true : isAA, auth, tCacheEntry->first.place == DNSResourceRecord::ANSWER ? ednsmask : boost::none, d_routingTag, recordState, remoteIP, d_refresh, tCacheEntry->second.d_ttl_time); + bool thisRRNeedsWildcardProof = false; + if (gatherWildcardProof) { + if (auto wcIt = wildcardCandidates.find(tCacheEntry->first.name); wcIt != wildcardCandidates.end() && wcIt->second == true) { + thisRRNeedsWildcardProof = true; + } + } + g_recCache->replace(d_now.tv_sec, tCacheEntry->first.name, tCacheEntry->first.type, tCacheEntry->second.records, tCacheEntry->second.signatures, thisRRNeedsWildcardProof ? authorityRecs : std::vector>(), tCacheEntry->first.type == QType::DS ? true : isAA, auth, tCacheEntry->first.place == DNSResourceRecord::ANSWER ? ednsmask : boost::none, d_routingTag, recordState, remoteIP, d_refresh, tCacheEntry->second.d_ttl_time); // 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 @@ -4669,10 +4703,11 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, const string& g_negCache->wipeTyped(tCacheEntry->first.name, tCacheEntry->first.type); } - if (g_aggressiveNSECCache && needWildcardProof && recordState == vState::Secure && tCacheEntry->first.place == DNSResourceRecord::ANSWER && tCacheEntry->first.name == qname && !tCacheEntry->second.signatures.empty() && !d_routingTag && !ednsmask) { + if (g_aggressiveNSECCache && thisRRNeedsWildcardProof && recordState == vState::Secure && tCacheEntry->first.place == DNSResourceRecord::ANSWER && !tCacheEntry->second.signatures.empty() && !d_routingTag && !ednsmask) { /* we have an answer synthesized from a wildcard and aggressive NSEC is enabled, we need to store the wildcard in its non-expanded form in the cache to be able to synthesize wildcard answers later */ const auto& rrsig = tCacheEntry->second.signatures.at(0); + 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); @@ -4705,6 +4740,13 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, const string& } } + if (gatherWildcardProof) { + if (auto it = wildcardCandidates.find(qname); it != wildcardCandidates.end() && it->second == false) { + // the queried name was not expended from a wildcard, a record in the CNAME chain was, so we don't need to gather wildcard proof now: we will do that when looking up the CNAME chain + gatherWildcardProof = false; + } + } + return RCode::NoError; }