From: Otto Moerbeek Date: Mon, 2 Mar 2026 10:29:47 +0000 (+0100) Subject: rec: estimate size and refuse to cache big negcache entries X-Git-Tag: auth-5.1.0-beta1~29^2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=89d18898cf4ee81999d79218fdbbbc0e52ec59df;p=thirdparty%2Fpdns.git rec: estimate size and refuse to cache big negcache entries Signed-off-by: Otto Moerbeek --- diff --git a/pdns/recursordist/aggressive_nsec.cc b/pdns/recursordist/aggressive_nsec.cc index 6a55ca257e..bf6247e875 100644 --- a/pdns/recursordist/aggressive_nsec.cc +++ b/pdns/recursordist/aggressive_nsec.cc @@ -32,6 +32,7 @@ std::unique_ptr g_aggressiveNSECCache{nullptr}; uint64_t AggressiveNSECCache::s_nsec3DenialProofMaxCost{0}; uint8_t AggressiveNSECCache::s_maxNSEC3CommonPrefix = AggressiveNSECCache::s_default_maxNSEC3CommonPrefix; +uint32_t AggressiveNSECCache::s_maxEntrySize{8192}; /* this is defined in syncres.hh and we are not importing that here */ extern std::unique_ptr g_recCache; @@ -261,6 +262,23 @@ static bool commonPrefixIsLong(const string& one, const string& two, size_t boun return length > bound; } +size_t AggressiveNSECCache::ZoneEntry::CacheEntry::sizeEstimate() const +{ + size_t ret = sizeof(AggressiveNSECCache::ZoneEntry::CacheEntry); + if (d_record) { + ret += d_record->sizeEstimate(); + } + for (const auto& entry : d_signatures) { + if (entry) { + ret += entry->sizeEstimate(); + } + } + ret += d_owner.sizeEstimate(); + ret += d_next.sizeEstimate(); + ret += d_qname.sizeEstimate(); + return ret; +} + // If the NSEC3 hashes have a long common prefix, they deny only a small subset of all possible hashes // So don't take the trouble to store those. bool AggressiveNSECCache::isSmallCoveringNSEC3(const DNSName& owner, const std::string& nextHash) @@ -340,26 +358,22 @@ void AggressiveNSECCache::insertNSEC(const DNSName& zone, const DNSName& owner, zoneEntry->d_entries.clear(); } } + DNSName realOwner = owner; + if (!nsec3 && isWildcardExpanded(owner.countLabels(), *signatures.at(0))) { + realOwner = getNSECOwnerName(owner, signatures); + } + ZoneEntry::CacheEntry cacheEntry{record.getContent(), signatures, realOwner, next, qname, record.d_ttl, qtype}; + if (s_maxEntrySize > 0 && cacheEntry.sizeEstimate() > s_maxEntrySize) { + return; + } /* the TTL is already a TTD by now */ - if (!nsec3 && isWildcardExpanded(owner.countLabels(), *signatures.at(0))) { - DNSName realOwner = getNSECOwnerName(owner, signatures); - auto pair = zoneEntry->d_entries.insert({record.getContent(), signatures, realOwner, next, qname, record.d_ttl, qtype}); - if (pair.second) { - ++d_entriesCount; - } - else { - zoneEntry->d_entries.replace(pair.first, {record.getContent(), signatures, std::move(realOwner), std::move(next), qname, record.d_ttl, qtype}); - } + auto pair = zoneEntry->d_entries.emplace(cacheEntry); + if (pair.second) { + ++d_entriesCount; } else { - auto pair = zoneEntry->d_entries.insert({record.getContent(), signatures, owner, next, qname, record.d_ttl, qtype}); - if (pair.second) { - ++d_entriesCount; - } - else { - zoneEntry->d_entries.replace(pair.first, {record.getContent(), signatures, owner, std::move(next), qname, record.d_ttl, qtype}); - } + zoneEntry->d_entries.replace(pair.first, std::move(cacheEntry)); } } } diff --git a/pdns/recursordist/aggressive_nsec.hh b/pdns/recursordist/aggressive_nsec.hh index 12a27e0f7a..842015d0cf 100644 --- a/pdns/recursordist/aggressive_nsec.hh +++ b/pdns/recursordist/aggressive_nsec.hh @@ -46,6 +46,7 @@ public: static constexpr uint8_t s_default_maxNSEC3CommonPrefix = 10; static uint64_t s_nsec3DenialProofMaxCost; static uint8_t s_maxNSEC3CommonPrefix; + static uint32_t s_maxEntrySize; AggressiveNSECCache(uint64_t entries) : d_maxEntries(entries) @@ -130,6 +131,8 @@ private: DNSName d_qname; // of the query data that lead to this entry being created/updated time_t d_ttd; QType d_qtype; // of the query data that lead to this entry being created/updated + + [[nodiscard]] size_t sizeEstimate() const; }; typedef multi_index_container< diff --git a/pdns/recursordist/negcache.cc b/pdns/recursordist/negcache.cc index c26a82db26..be2791d6ad 100644 --- a/pdns/recursordist/negcache.cc +++ b/pdns/recursordist/negcache.cc @@ -28,6 +28,7 @@ // For a description on how ServeStale works, see recursor_cache.cc, the general structure is the same. uint16_t NegCache::s_maxServedStaleExtensions; +uint32_t NegCache::s_maxEntrySize{8192}; // Same default as positive cache NegCache::NegCache(size_t mapsCount) : d_maps(mapsCount == 0 ? 1 : mapsCount) @@ -43,6 +44,17 @@ size_t NegCache::size() const return count; } +size_t NegCache::NegCacheEntry::sizeEstimate() const +{ + auto ret = sizeof(NegCacheEntry); + ret += authoritySOA.sizeEstimate(); + ret += DNSSECRecords.sizeEstimate(); + ret += d_name.sizeEstimate(); + ret += d_auth.sizeEstimate(); + + return ret; +} + /*! * Set ne to the NegCacheEntry for the last label in qname and return true if there * was one. @@ -140,6 +152,9 @@ bool NegCache::get(const DNSName& qname, QType qtype, const struct timeval& now, */ void NegCache::add(const NegCacheEntry& negEntry) { + if (s_maxEntrySize > 0 && negEntry.sizeEstimate() > s_maxEntrySize) { + return; + } bool inserted = false; auto& map = getMap(negEntry.d_name); auto content = map.lock(); diff --git a/pdns/recursordist/negcache.hh b/pdns/recursordist/negcache.hh index a281e89840..6efd1fe14d 100644 --- a/pdns/recursordist/negcache.hh +++ b/pdns/recursordist/negcache.hh @@ -47,6 +47,18 @@ struct recordsAndSignatures { vector records; vector signatures; + + [[nodiscard]] size_t sizeEstimate() const + { + size_t ret = sizeof(recordsAndSignatures); + for (const auto& entry : records) { + ret += entry.sizeEstimate(); + } + for (const auto& entry : signatures) { + ret += entry.sizeEstimate(); + } + return ret; + } }; class NegCache : public boost::noncopyable @@ -59,6 +71,7 @@ public: static uint16_t s_maxServedStaleExtensions; // The time a stale cache entry is extended static constexpr uint32_t s_serveStaleExtensionPeriod = 30; + static uint32_t s_maxEntrySize; struct NegCacheEntry { @@ -86,6 +99,7 @@ public: // When serving stale, we consider expired records return d_ttd > now || serveStale || d_servedStale != 0; } + [[nodiscard]] size_t sizeEstimate() const; }; void add(const NegCacheEntry& negEntry); diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc index 72a70c1e4e..acee266932 100644 --- a/pdns/recursordist/rec-main.cc +++ b/pdns/recursordist/rec-main.cc @@ -3233,6 +3233,8 @@ int main(int argc, char** argv) } MemRecursorCache::s_maxEntrySize = ::arg().asNum("max-recordcache-entry-size"); + NegCache::s_maxEntrySize = MemRecursorCache::s_maxEntrySize; + AggressiveNSECCache::s_maxEntrySize = MemRecursorCache::s_maxEntrySize; RecursorPacketCache::s_maxEntrySize = ::arg().asNum("max-packetcache-entry-size"); g_recCache = std::make_unique(::arg().asNum("record-cache-shards")); g_negCache = std::make_unique(::arg().asNum("record-cache-shards") / 8); diff --git a/pdns/recursordist/test-aggressive_nsec_cc.cc b/pdns/recursordist/test-aggressive_nsec_cc.cc index 63a44201e9..fee6d16f08 100644 --- a/pdns/recursordist/test-aggressive_nsec_cc.cc +++ b/pdns/recursordist/test-aggressive_nsec_cc.cc @@ -1130,6 +1130,28 @@ BOOST_AUTO_TEST_CASE(test_aggressive_nsec_replace) BOOST_CHECK(diff1 < diff2 * 2 && diff2 < diff1 * 2); } +BOOST_AUTO_TEST_CASE(test_aggressive_nsec_very_big_replace) +{ + auto cache = make_unique(1000); + + struct timeval now{}; + Utility::gettimeofday(&now, nullptr); + + DNSRecord rec; + rec.d_name = DNSName("powerdns.com"); + rec.d_type = QType::NSEC3; + rec.d_ttl = now.tv_sec + 10; + rec.setContent(getRecordContent(QType::NSEC3, "1 0 500 ab HASG==== A RRSIG NSEC3")); + std::vector> sigs; + for (auto i = 0; i < 100; i++) { + auto rrsig = std::make_shared("NSEC3 5 3 10 20370101000000 20370101000000 24567 dummy. data"); + sigs.emplace_back(std::move(rrsig)); + } + cache->insertNSEC(DNSName("powerdns.com"), rec.d_name, rec, sigs, true); + + BOOST_CHECK_EQUAL(cache->getEntriesCount(), 0U); +} + BOOST_AUTO_TEST_CASE(test_aggressive_nsec_wiping) { auto cache = make_unique(10000); diff --git a/pdns/recursordist/test-negcache_cc.cc b/pdns/recursordist/test-negcache_cc.cc index 79e7c46bd1..a39b21201c 100644 --- a/pdns/recursordist/test-negcache_cc.cc +++ b/pdns/recursordist/test-negcache_cc.cc @@ -73,6 +73,39 @@ BOOST_AUTO_TEST_CASE(test_get_entry) BOOST_CHECK_EQUAL(ne.d_auth, auth); } +BOOST_AUTO_TEST_CASE(test_get_verybig_entry) +{ + /* Try adddd a full name negative entry to the cache that's to big and attempt to get an entry for + * the A record. Should not yield the full name not exist entry + */ + DNSName qname("www2.powerdns.com"); + DNSName auth("powerdns.com"); + + struct timeval now; + Utility::gettimeofday(&now, 0); + + NegCache cache; + + auto entry = genNegCacheEntry(qname, auth, now); + for (auto i = 0; i < 100; i++) { + DNSRecord rec; + rec.d_name = qname; + rec.d_type = QType::RRSIG; + rec.d_ttl = 600; + rec.d_place = DNSResourceRecord::AUTHORITY; + rec.setContent(std::make_shared(QType(QType::A).toString() + " 5 3 600 2037010100000000 2037010100000000 24567 dummy data")); + entry.DNSSECRecords.signatures.emplace_back(std::move(rec)); + } + cache.add(entry); + + BOOST_CHECK_EQUAL(cache.size(), 0U); + + NegCache::NegCacheEntry ne; + bool ret = cache.get(qname, QType(1), now, ne); + + BOOST_CHECK(!ret); +} + BOOST_AUTO_TEST_CASE(test_get_entry2038) { /* Add a full name negative entry to the cache and attempt to get an entry for