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 <remi.gacogne@powerdns.com>

diff --git a/pdns/recursordist/aggressive_nsec.cc b/pdns/recursordist/aggressive_nsec.cc
index 6a55ca257edf6f48eb0ed9be995d3a13a61573a5..484061223b0c1dcd92dc8264e936335dca96b080 100644 (file)
--- 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<LockGuarded<AggressiveNSECCache::ZoneEntry>> 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 12a27e0f7acf0daac3f37ad34ae7c3a49ffd4fe0..d68bb8fc96a5382a93d4abb946efb100ac4c18a2 100644 (file)
--- 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<tag<OrderedTag>,
@@ -140,14 +142,13 @@ private:
                        CanonDNSNameCompare>,
         sequenced<tag<SequencedTag>>,
         hashed_non_unique<tag<HashedTag>,
-                          member<CacheEntry, const DNSName, &CacheEntry::d_owner>>>>
-      cache_t;
+                          member<CacheEntry, const DNSName, &CacheEntry::d_owner>>>>;
 
     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<LockGuarded<ZoneEntry>> getZone(const DNSName& zone);

diff --git a/pdns/recursordist/test-aggressive_nsec_cc.cc b/pdns/recursordist/test-aggressive_nsec_cc.cc
index 63a44201e9b1cd2aa5cbdcee2ab0c919b08f8a9a..e7bea9a77bb59e5154cc503e68020b1b9cae3538 100644 (file)
--- 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<AggressiveNSECCache>(10000);
+  g_recCache = std::make_unique<MemRecursorCache>();
+
+  const DNSName zone("powerdns.com");
+  time_t now = time(nullptr);
+
+  /* first we need a SOA */
+  std::vector<DNSRecord> 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<SOARecordContent>("pdns-public-ns1.powerdns.com. pieter\\.lexis.powerdns.com. 2017032301 10800 3600 604800 3600"));
+  drSOA.d_ttl = static_cast<uint32_t>(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<RRSIGRecordContent>("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<NSEC3RecordContent>(nrc));
+  rrsig = std::make_shared<RRSIGRecordContent>("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<AggressiveNSECCache>(10000);
+  g_recCache = std::make_unique<MemRecursorCache>();
+
+  const DNSName zone("powerdns.com");
+  time_t now = time(nullptr);
+
+  /* first we need a SOA */
+  std::vector<DNSRecord> 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<SOARecordContent>("pdns-public-ns1.powerdns.com. pieter\\.lexis.powerdns.com. 2017032301 10800 3600 604800 3600"));
+  drSOA.d_ttl = static_cast<uint32_t>(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<NSEC3RecordContent>(nrc));
+  auto rrsig = std::make_shared<RRSIGRecordContent>("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<RRSIGRecordContent>("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<AggressiveNSECCache>(10000);