From c090cc8b9198a9ee9155486894505a86878e30ee Mon Sep 17 00:00:00 2001 From: Otto Moerbeek Date: Mon, 4 Mar 2024 11:05:23 +0100 Subject: [PATCH] rec: backport 13847 to rec-4.8.x Manual backport as syncres.c's location changed --- pdns/recursordist/test-syncres_cc3.cc | 73 +++++++++++++++++++++++++++ pdns/syncres.cc | 50 ++++++++++++++++-- 2 files changed, 119 insertions(+), 4 deletions(-) diff --git a/pdns/recursordist/test-syncres_cc3.cc b/pdns/recursordist/test-syncres_cc3.cc index a75dcf2267..da92c57e9f 100644 --- a/pdns/recursordist/test-syncres_cc3.cc +++ b/pdns/recursordist/test-syncres_cc3.cc @@ -1214,6 +1214,79 @@ BOOST_AUTO_TEST_CASE(test_forward_zone_recurse_rd_dnssec_nodata_bogus) BOOST_CHECK_EQUAL(queriesCount, 4U); } +BOOST_AUTO_TEST_CASE(test_forward_zone_recurse_rd_dnssec_cname_wildcard_expanded) +{ + std::unique_ptr testSR; + initSR(testSR, true); + + setDNSSECValidation(testSR, DNSSECMode::ValidateAll); + + primeHints(); + /* unsigned */ + const DNSName target("test."); + /* signed */ + const DNSName cnameTarget("cname."); + testkeysset_t keys; + + auto luaconfsCopy = g_luaconfs.getCopy(); + luaconfsCopy.dsAnchors.clear(); + generateKeyMaterial(g_rootdnsname, DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys, luaconfsCopy.dsAnchors); + generateKeyMaterial(cnameTarget, DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys); + g_luaconfs.setState(luaconfsCopy); + + const ComboAddress forwardedNS("192.0.2.42:53"); + size_t queriesCount = 0; + + SyncRes::AuthDomain authDomain; + authDomain.d_rdForward = true; + authDomain.d_servers.push_back(forwardedNS); + (*SyncRes::t_sstorage.domainmap)[g_rootdnsname] = authDomain; + + testSR->setAsyncCallback([&](const ComboAddress& address, const DNSName& domain, int type, bool /* doTCP */, bool sendRDQuery, int /* EDNS0Level */, struct timeval* /* now */, boost::optional& /* srcmask */, boost::optional /* context */, LWResult* res, bool* /* chained */) { + queriesCount++; + + BOOST_CHECK_EQUAL(sendRDQuery, true); + + if (address != forwardedNS) { + return LWResult::Result::Timeout; + } + + if (type == QType::DS || type == QType::DNSKEY) { + return genericDSAndDNSKEYHandler(res, domain, DNSName("."), type, keys); + } + + if (domain == target && type == QType::A) { + + setLWResult(res, 0, false, false, true); + addRecordToLW(res, target, QType::CNAME, cnameTarget.toString()); + addRecordToLW(res, cnameTarget, QType::A, "192.0.2.1"); + /* the RRSIG proves that the cnameTarget was expanded from a wildcard */ + addRRSIG(keys, res->d_records, cnameTarget, 300, false, boost::none, DNSName("*")); + /* we need to add the proof that this name does not exist, so the wildcard may apply */ + addNSECRecordToLW(DNSName("cnamd."), DNSName("cnamf."), {QType::A, QType::NSEC, QType::RRSIG}, 60, res->d_records); + addRRSIG(keys, res->d_records, cnameTarget, 300); + + return LWResult::Result::Success; + } + return LWResult::Result::Timeout; + }); + + vector ret; + int res = testSR->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(testSR->getValidationState(), vState::Insecure); + BOOST_REQUIRE_EQUAL(ret.size(), 5U); + BOOST_CHECK_EQUAL(queriesCount, 5U); + + /* again, to test the cache */ + ret.clear(); + res = testSR->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(testSR->getValidationState(), vState::Insecure); + BOOST_REQUIRE_EQUAL(ret.size(), 5U); + BOOST_CHECK_EQUAL(queriesCount, 5U); +} + BOOST_AUTO_TEST_CASE(test_auth_zone_oob) { std::unique_ptr sr; diff --git a/pdns/syncres.cc b/pdns/syncres.cc index bbdba357c3..fc3a6a4c44 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -4290,11 +4290,37 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr 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 cnameIt = cnames.find(initial); + if (cnameIt == cnames.end()) { + break; + } + initial = cnameIt->second; + wildcardCandidates.emplace(initial, false); + } + } + for (auto& rec : lwr.d_records) { if (rec.d_type == QType::OPT || rec.d_class != QClass::IN) { continue; @@ -4314,6 +4340,7 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr seenAuth = rec.d_name; } + const auto labelCount = rec.d_name.countLabels(); if (rec.d_type == QType::RRSIG) { auto rrsig = getRR(rec); if (rrsig) { @@ -4321,7 +4348,8 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr 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 (auto wcIt = wildcardCandidates.find(rec.d_name); wcIt != wildcardCandidates.end() && isWildcardExpanded(labelCount, rrsig)) { + wcIt->second = true; gatherWildcardProof = true; if (!isWildcardExpandedOntoItself(rec.d_name, labelCount, rrsig)) { /* if we have a wildcard expanded onto itself, we don't need to prove @@ -4598,7 +4626,13 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr if (isAA && i->first.type == QType::NS && s_save_parent_ns_set) { rememberParentSetIfNeeded(i->first.name, i->second.records, depth); } - g_recCache->replace(d_now.tv_sec, i->first.name, i->first.type, i->second.records, i->second.signatures, authorityRecs, i->first.type == QType::DS ? true : isAA, auth, i->first.place == DNSResourceRecord::ANSWER ? ednsmask : boost::none, d_routingTag, recordState, remoteIP, d_refresh); + bool thisRRNeedsWildcardProof = false; + if (gatherWildcardProof) { + if (auto wcIt = wildcardCandidates.find(i->first.name); wcIt != wildcardCandidates.end() && wcIt->second) { + thisRRNeedsWildcardProof = true; + } + } + g_recCache->replace(d_now.tv_sec, i->first.name, i->first.type, i->second.records, i->second.signatures, thisRRNeedsWildcardProof ? authorityRecs : std::vector>(), i->first.type == QType::DS ? true : isAA, auth, i->first.place == DNSResourceRecord::ANSWER ? ednsmask : boost::none, d_routingTag, recordState, remoteIP, d_refresh); // 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 @@ -4606,10 +4640,11 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr g_negCache->wipeTyped(i->first.name, i->first.type); } - if (g_aggressiveNSECCache && needWildcardProof && recordState == vState::Secure && i->first.place == DNSResourceRecord::ANSWER && i->first.name == qname && !i->second.signatures.empty() && !d_routingTag && !ednsmask) { + if (g_aggressiveNSECCache && thisRRNeedsWildcardProof && recordState == vState::Secure && i->first.place == DNSResourceRecord::ANSWER && i->first.name == qname && !i->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 = i->second.signatures.at(0); + const auto labelCount = i->first.name.countLabels(); if (isWildcardExpanded(labelCount, rrsig) && !isWildcardExpandedOntoItself(i->first.name, labelCount, rrsig)) { DNSName realOwner = getNSECOwnerName(i->first.name, i->second.signatures); @@ -4642,6 +4677,13 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr } } + if (gatherWildcardProof) { + if (auto wcIt = wildcardCandidates.find(qname); wcIt != wildcardCandidates.end() && !wcIt->second) { + // the queried name was not expanded 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; } -- 2.47.2