rec: estimate size and refuse to cache big negcache entries

Signed-off-by: Otto Moerbeek <otto.moerbeek@open-xchange.com>

diff --git a/pdns/recursordist/aggressive_nsec.cc b/pdns/recursordist/aggressive_nsec.cc
index 6a55ca257edf6f48eb0ed9be995d3a13a61573a5..bf6247e875b044e374c60944b0742cba969a99dc 100644 (file)
--- a/pdns/recursordist/aggressive_nsec.cc
+++ b/pdns/recursordist/aggressive_nsec.cc
@@ -32,6 +32,7 @@
 std::unique_ptr<AggressiveNSECCache> 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<MemRecursorCache> 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 12a27e0f7acf0daac3f37ad34ae7c3a49ffd4fe0..842015d0cff770b3f343ba3a520d86df360283ab 100644 (file)
--- 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 c26a82db265e79c501eb66f430c587028095f543..be2791d6ade02efaa43e37f0160df435b3046c2a 100644 (file)
--- 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 a281e8984081d3a433d78426d0061bb8883a8094..6efd1fe14db1845ff5be7009508415dc6a560026 100644 (file)
--- a/pdns/recursordist/negcache.hh
+++ b/pdns/recursordist/negcache.hh
@@ -47,6 +47,18 @@ struct recordsAndSignatures
 {
   vector<DNSRecord> records;
   vector<DNSRecord> 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 72a70c1e4eed4d355f296a903cd57678c9bcfbc1..acee26693227c2cb6f06b2d55e76997466269473 100644 (file)
--- 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<MemRecursorCache>(::arg().asNum("record-cache-shards"));
     g_negCache = std::make_unique<NegCache>(::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 63a44201e9b1cd2aa5cbdcee2ab0c919b08f8a9a..fee6d16f08c739bcc7a5be59770caf9dbe30397f 100644 (file)
--- 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<AggressiveNSECCache>(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<std::shared_ptr<const RRSIGRecordContent>> sigs;
+  for (auto i = 0; i < 100; i++) {
+    auto rrsig = std::make_shared<RRSIGRecordContent>("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<AggressiveNSECCache>(10000);

diff --git a/pdns/recursordist/test-negcache_cc.cc b/pdns/recursordist/test-negcache_cc.cc
index 79e7c46bd13f4b03b5868ed42a59eb0173a75fa6..a39b21201c55df93409fc343f2ae82f3e9fb324a 100644 (file)
--- 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<RRSIGRecordContent>(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