From: Remi Gacogne Date: Tue, 17 Feb 2026 09:03:46 +0000 (+0100) Subject: rec: Better handling of RFC5155 transitions in the aggressive NSEC cache X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f3bdfd86e74f68342fdae811b0dbb59e3ff220c8;p=thirdparty%2Fpdns.git rec: Better handling of RFC5155 transitions in the aggressive NSEC cache This commit ensures that we do not cache NSEC and NSEC3 records for the same zone at the same time, which could lead to surprises during NSEC -> NSEC3 or NSEC3 -> NSEC transitions as described in RFC5155 sections 10.4 and 10.5. The existing code was correctly handling the NSEC -> NSEC3 transition by clearing any existing NSEC records when a NSEC3 record was received for a zone, but this behaviour could have been problematic for NSEC3 to NSEC transitions. The new behaviour is to refuse to insert records during the transition, keeping the existing entries until they expire. This was reported by: - Qifan Zhang (Palo Alto Networks) qzhang@paloaltonetworks.com - Zilin Shen (Purdue University) shen624@purdue.edu - Imtiaz Karim (The University of Texas at Dallas) imtiaz.karim@utdallas.edu - Elisa Bertino (Purdue University) bertino@purdue.edu - Daiping Liu (Palo Alto Networks) dpliu@paloaltonetworks.com - Zhou Li (University of California, Irvine) zhou.li@uci.edu Signed-off-by: Remi Gacogne --- diff --git a/pdns/recursordist/aggressive_nsec.cc b/pdns/recursordist/aggressive_nsec.cc index 6a55ca257e..484061223b 100644 --- a/pdns/recursordist/aggressive_nsec.cc +++ b/pdns/recursordist/aggressive_nsec.cc @@ -285,10 +285,14 @@ void AggressiveNSECCache::insertNSEC(const DNSName& zone, const DNSName& owner, std::shared_ptr> entry = getZone(zone); { auto zoneEntry = entry->lock(); - if (nsec3 && !zoneEntry->d_nsec3) { - d_entriesCount -= zoneEntry->d_entries.size(); - zoneEntry->d_entries.clear(); - zoneEntry->d_nsec3 = true; + if (zoneEntry->d_denialType == ZoneEntry::ZoneDenialType::Unknown) { + zoneEntry->d_denialType = nsec3 ? ZoneEntry::ZoneDenialType::NSEC3 : ZoneEntry::ZoneDenialType::NSEC; + } + else if (nsec3 && zoneEntry->d_denialType != ZoneEntry::ZoneDenialType::NSEC3) { + return; + } + else if (!nsec3 && zoneEntry->d_denialType != ZoneEntry::ZoneDenialType::NSEC) { + return; } DNSName next; @@ -806,7 +810,7 @@ bool AggressiveNSECCache::getDenial(time_t now, const DNSName& name, const QType return false; } zone = entry->d_zone; - nsec3 = entry->d_nsec3; + nsec3 = entry->d_denialType == ZoneEntry::ZoneDenialType::NSEC3; } vState cachedState; @@ -924,12 +928,15 @@ size_t AggressiveNSECCache::dumpToFile(pdns::UniqueFilePtr& filePtr, const struc } auto zone = node.d_value->lock(); + if (zone->d_denialType == ZoneEntry::ZoneDenialType::Unknown || zone->d_entries.empty()) { + return; + } fprintf(filePtr.get(), "; Zone %s\n", zone->d_zone.toString().c_str()); for (const auto& entry : zone->d_entries) { int64_t ttl = entry.d_ttd - now.tv_sec; try { - fprintf(filePtr.get(), "%s %" PRId64 " IN %s %s by %s/%s\n", entry.d_owner.toString().c_str(), ttl, zone->d_nsec3 ? "NSEC3" : "NSEC", entry.d_record->getZoneRepresentation().c_str(), entry.d_qname.toString().c_str(), entry.d_qtype.toString().c_str()); + fprintf(filePtr.get(), "%s %" PRId64 " IN %s %s by %s/%s\n", entry.d_owner.toString().c_str(), ttl, zone->d_denialType == ZoneEntry::ZoneDenialType::NSEC3 ? "NSEC3" : "NSEC", entry.d_record->getZoneRepresentation().c_str(), entry.d_qname.toString().c_str(), entry.d_qtype.toString().c_str()); for (const auto& signature : entry.d_signatures) { fprintf(filePtr.get(), "- RRSIG %s\n", signature->getZoneRepresentation().c_str()); } diff --git a/pdns/recursordist/aggressive_nsec.hh b/pdns/recursordist/aggressive_nsec.hh index 12a27e0f7a..d68bb8fc96 100644 --- a/pdns/recursordist/aggressive_nsec.hh +++ b/pdns/recursordist/aggressive_nsec.hh @@ -105,11 +105,6 @@ private: { } - ZoneEntry(const DNSName& zone, const std::string& salt, uint16_t iterations, bool nsec3) : - d_zone(zone), d_salt(salt), d_iterations(iterations), d_nsec3(nsec3) - { - } - struct HashedTag { }; @@ -132,7 +127,14 @@ private: QType d_qtype; // of the query data that lead to this entry being created/updated }; - typedef multi_index_container< + enum class ZoneDenialType : uint8_t + { + Unknown = 0, + NSEC = 1, + NSEC3 = 2 + }; + + using cache_t = multi_index_container< CacheEntry, indexed_by< ordered_unique, @@ -140,14 +142,13 @@ private: CanonDNSNameCompare>, sequenced>, hashed_non_unique, - member>>> - cache_t; + member>>>; cache_t d_entries; const DNSName d_zone; std::string d_salt; uint16_t d_iterations{0}; - bool d_nsec3{false}; + ZoneDenialType d_denialType{ZoneDenialType::Unknown}; }; std::shared_ptr> getZone(const DNSName& zone); diff --git a/pdns/recursordist/test-aggressive_nsec_cc.cc b/pdns/recursordist/test-aggressive_nsec_cc.cc index 63a44201e9..e7bea9a77b 100644 --- a/pdns/recursordist/test-aggressive_nsec_cc.cc +++ b/pdns/recursordist/test-aggressive_nsec_cc.cc @@ -1454,6 +1454,132 @@ BOOST_AUTO_TEST_CASE(test_aggressive_nsec3_rollover) BOOST_CHECK_EQUAL(getDenialWrapper(cache, now, other, QType::AAAA), false); } +BOOST_AUTO_TEST_CASE(test_aggressive_nsec_to_nsec3_transition) +{ + AggressiveNSECCache::s_maxNSEC3CommonPrefix = 159; + auto cache = make_unique(10000); + g_recCache = std::make_unique(); + + const DNSName zone("powerdns.com"); + time_t now = time(nullptr); + + /* first we need a SOA */ + std::vector records; + time_t ttd = now + 30; + DNSRecord drSOA; + drSOA.d_name = zone; + drSOA.d_type = QType::SOA; + drSOA.d_class = QClass::IN; + drSOA.setContent(std::make_shared("pdns-public-ns1.powerdns.com. pieter\\.lexis.powerdns.com. 2017032301 10800 3600 604800 3600")); + drSOA.d_ttl = static_cast(ttd); // XXX truncation + drSOA.d_place = DNSResourceRecord::ANSWER; + records.push_back(drSOA); + + g_recCache->replace(now, zone, QType(QType::SOA), records, {}, {}, true, zone, std::nullopt, MemRecursorCache::NOTAG, vState::Secure); + BOOST_CHECK_EQUAL(g_recCache->size(), 1U); + + /* insert NSEC */ + DNSRecord rec; + rec.d_name = DNSName("www.powerdns.com"); + rec.d_type = QType::NSEC; + rec.d_ttl = now + 10; + rec.setContent(getRecordContent(QType::NSEC, "z.powerdns.com. A RRSIG NSEC")); + auto rrsig = std::make_shared("NSEC 5 3 10 20370101000000 20370101000000 24567 dummy. data"); + cache->insertNSEC(DNSName("powerdns.com"), rec.d_name, rec, {rrsig}, false); + BOOST_CHECK_EQUAL(cache->getEntriesCount(), 1U); + + const std::string salt = "ab"; + const unsigned int iterationsCount = 1; + const DNSName name("www.powerdns.com"); + std::string hashed = hashQNameWithSalt(salt, iterationsCount, name); + + rec.d_name = DNSName(toBase32Hex(hashed)) + zone; + rec.d_type = QType::NSEC3; + rec.d_ttl = now + 10; + + NSEC3RecordContent nrc; + nrc.d_algorithm = 1; + nrc.d_flags = 0; + nrc.d_iterations = iterationsCount; + nrc.d_salt = salt; + nrc.d_nexthash = hashed; + incrementHash(nrc.d_nexthash); + for (const auto& type : {QType::A}) { + nrc.set(type); + } + + rec.setContent(std::make_shared(nrc)); + rrsig = std::make_shared("NSEC3 5 3 10 20370101000000 20370101000000 24567 dummy. data"); + cache->insertNSEC(zone, rec.d_name, rec, {rrsig}, true); + + /* the entry should NOT be inserted because the zone was in NSEC mode */ + BOOST_CHECK_EQUAL(cache->getEntriesCount(), 1U); +} + +BOOST_AUTO_TEST_CASE(test_aggressive_nsec3_to_nsec_transition) +{ + AggressiveNSECCache::s_maxNSEC3CommonPrefix = 159; + auto cache = make_unique(10000); + g_recCache = std::make_unique(); + + const DNSName zone("powerdns.com"); + time_t now = time(nullptr); + + /* first we need a SOA */ + std::vector records; + time_t ttd = now + 30; + DNSRecord drSOA; + drSOA.d_name = zone; + drSOA.d_type = QType::SOA; + drSOA.d_class = QClass::IN; + drSOA.setContent(std::make_shared("pdns-public-ns1.powerdns.com. pieter\\.lexis.powerdns.com. 2017032301 10800 3600 604800 3600")); + drSOA.d_ttl = static_cast(ttd); // XXX truncation + drSOA.d_place = DNSResourceRecord::ANSWER; + records.push_back(drSOA); + + g_recCache->replace(now, zone, QType(QType::SOA), records, {}, {}, true, zone, std::nullopt, MemRecursorCache::NOTAG, vState::Secure); + BOOST_CHECK_EQUAL(g_recCache->size(), 1U); + + /* insert NSEC3 */ + const std::string salt = "ab"; + const unsigned int iterationsCount = 1; + const DNSName name("www.powerdns.com"); + std::string hashed = hashQNameWithSalt(salt, iterationsCount, name); + + DNSRecord rec; + rec.d_name = DNSName(toBase32Hex(hashed)) + zone; + rec.d_type = QType::NSEC3; + rec.d_ttl = now + 10; + + NSEC3RecordContent nrc; + nrc.d_algorithm = 1; + nrc.d_flags = 0; + nrc.d_iterations = iterationsCount; + nrc.d_salt = salt; + nrc.d_nexthash = hashed; + incrementHash(nrc.d_nexthash); + for (const auto& type : {QType::A}) { + nrc.set(type); + } + + rec.setContent(std::make_shared(nrc)); + auto rrsig = std::make_shared("NSEC3 5 3 10 20370101000000 20370101000000 24567 dummy. data"); + cache->insertNSEC(zone, rec.d_name, rec, {rrsig}, true); + + BOOST_CHECK_EQUAL(cache->getEntriesCount(), 1U); + + /* now try to insert NSEC */ + rec.d_name = DNSName("www.powerdns.com"); + rec.d_type = QType::NSEC; + rec.d_ttl = now + 10; + rec.setContent(getRecordContent(QType::NSEC, "z.powerdns.com. A RRSIG NSEC")); + rrsig = std::make_shared("NSEC 5 3 10 20370101000000 20370101000000 24567 dummy. data"); + cache->insertNSEC(DNSName("powerdns.com"), rec.d_name, rec, {rrsig}, false); + + /* the entry should NOT be inserted because the zone was in NSEC3 mode */ + BOOST_CHECK_EQUAL(cache->getEntriesCount(), 1U); +} + BOOST_AUTO_TEST_CASE(test_aggressive_nsec_ancestor_cases) { auto cache = make_unique(10000);