rec: Add a new 'max-cache-bogus-ttl' option

diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc
index f4842e9e7c8d02d104872d64e5f2aecb73e18565..a95787e6fef502140ecbe611c13e64e5d7658e1e 100644 (file)
--- a/pdns/pdns_recursor.cc
+++ b/pdns/pdns_recursor.cc
@@ -3634,6 +3634,7 @@ static int serviceMain(int argc, char*argv[])
   SyncRes::s_nopacketcache = ::arg().mustDo("disable-packetcache");
 
   SyncRes::s_maxnegttl=::arg().asNum("max-negative-ttl");
+  SyncRes::s_maxbogusttl=::arg().asNum("max-cache-bogus-ttl");
   SyncRes::s_maxcachettl=max(::arg().asNum("max-cache-ttl"), 15);
   SyncRes::s_packetcachettl=::arg().asNum("packetcache-ttl");
   // Cap the packetcache-servfail-ttl to the packetcache-ttl
@@ -4222,6 +4223,7 @@ int main(int argc, char **argv)
     ::arg().set("hint-file", "If set, load root hints from this file")="";
     ::arg().set("max-cache-entries", "If set, maximum number of entries in the main cache")="1000000";
     ::arg().set("max-negative-ttl", "maximum number of seconds to keep a negative cached entry in memory")="3600";
+    ::arg().set("max-cache-bogus-ttl", "maximum number of seconds to keep a Bogus (positive or negative) cached entry in memory")="3600";
     ::arg().set("max-cache-ttl", "maximum number of seconds to keep a cached entry in memory")="86400";
     ::arg().set("packetcache-ttl", "maximum number of seconds to keep a cached entry in packetcache")="3600";
     ::arg().set("max-packetcache-entries", "maximum number of entries to keep in the packetcache")="500000";

diff --git a/pdns/recursor_cache.cc b/pdns/recursor_cache.cc
index 7dea10c3921cfeba81d8518629f6fd9a7227bd97..ee963cf70c1e26787b2abc14224d05a2d2121425 100644 (file)
--- a/pdns/recursor_cache.cc
+++ b/pdns/recursor_cache.cc
@@ -404,7 +404,7 @@ bool MemRecursorCache::doAgeCache(time_t now, const DNSName& name, uint16_t qtyp
   return false;
 }
 
-bool MemRecursorCache::updateValidationStatus(time_t now, const DNSName &qname, const QType& qt, const ComboAddress& who, bool requireAuth, vState newState)
+bool MemRecursorCache::updateValidationStatus(time_t now, const DNSName &qname, const QType& qt, const ComboAddress& who, bool requireAuth, vState newState, boost::optional<time_t> capTTD)
 {
   bool updated = false;
   uint16_t qtype = qt.getCode();
@@ -415,6 +415,9 @@ bool MemRecursorCache::updateValidationStatus(time_t now, const DNSName &qname,
     }
 
     entry->d_state = newState;
+    if (capTTD) {
+      entry->d_ttd = std::min(entry->d_ttd, *capTTD);
+    }
     return true;
   }
 
@@ -427,6 +430,9 @@ bool MemRecursorCache::updateValidationStatus(time_t now, const DNSName &qname,
       continue;
 
     i->d_state = newState;
+    if (capTTD) {
+      i->d_ttd = std::min(i->d_ttd, *capTTD);
+    }
     updated = true;
 
     if(qtype != QType::ANY && qtype != QType::ADDR) // normally if we have a hit, we are done

diff --git a/pdns/recursor_cache.hh b/pdns/recursor_cache.hh
index 9b4f3f6e077ef0e3a7bf0a081685f8f681ff1565..e9a44c86c747c4edd173fa3d9402b4e47e343a91 100644 (file)
--- a/pdns/recursor_cache.hh
+++ b/pdns/recursor_cache.hh
@@ -64,7 +64,7 @@ public:
 
   int doWipeCache(const DNSName& name, bool sub, uint16_t qtype=0xffff);
   bool doAgeCache(time_t now, const DNSName& name, uint16_t qtype, uint32_t newTTL);
-  bool updateValidationStatus(time_t now, const DNSName &qname, const QType& qt, const ComboAddress& who, bool requireAuth, vState newState);
+  bool updateValidationStatus(time_t now, const DNSName &qname, const QType& qt, const ComboAddress& who, bool requireAuth, vState newState, boost::optional<time_t> capTTD);
 
   uint64_t cacheHits, cacheMisses;
 
@@ -89,7 +89,7 @@ private:
     DNSName d_qname;
     Netmask d_netmask;
     mutable vState d_state;
-    time_t d_ttd;
+    mutable time_t d_ttd;
     uint16_t d_qtype;
     bool d_auth;
   };

diff --git a/pdns/recursordist/docs/settings.rst b/pdns/recursordist/docs/settings.rst
index 5d44334fdbc4aabcc267eb68570542a470ce9047..2e6200539e7abdd4a5f7eb0ec9b14a575cc15a66 100644 (file)
--- a/pdns/recursordist/docs/settings.rst
+++ b/pdns/recursordist/docs/settings.rst
@@ -712,6 +712,17 @@ Path to a lua file to manipulate the Recursor's answers. See :doc:`lua-scripting
 The interval between calls to the Lua user defined `maintenance()` function in seconds.
 See :ref:`hooks-maintenance-callback`
 
+.. _setting-max-cache-bogus-ttl:
+
+``max-cache-bogus-ttl``
+-----------------------
+.. versionadded:: 4.2.0
+
+-  Integer
+-  Default: 3600
+
+Maximum number of seconds to cache an item in the DNS cache (negative or positive) if its DNSSEC validation failed, no matter what the original TTL specified, to reduce the impact of a broken domain.
+
 .. _setting-max-cache-entries:
 
 ``max-cache-entries``

diff --git a/pdns/recursordist/negcache.cc b/pdns/recursordist/negcache.cc
index 6552486b7f11a073885f13f65e335e0d78e2c58d..0f5325c33a4f0fe51f51226267b7d299562313ca 100644 (file)
--- a/pdns/recursordist/negcache.cc
+++ b/pdns/recursordist/negcache.cc
@@ -111,11 +111,14 @@ void NegCache::add(const NegCacheEntry& ne) {
  * \param qtype The type of the entry to replace
  * \param newState The new validation state
  */
-void NegCache::updateValidationStatus(const DNSName& qname, const QType& qtype, const vState newState) {
+void NegCache::updateValidationStatus(const DNSName& qname, const QType& qtype, const vState newState, boost::optional<uint32_t> capTTD) {
   auto range = d_negcache.equal_range(tie(qname, qtype));
 
   if (range.first != range.second) {
     range.first->d_validationState = newState;
+    if (capTTD) {
+      range.first->d_ttd = std::min(range.first->d_ttd, *capTTD);
+    }
   }
 }
 

diff --git a/pdns/recursordist/negcache.hh b/pdns/recursordist/negcache.hh
index b78951e21d68fd2af5b3748eaaacbcc477442905..63eae63054efbe4facfe44366877ab1a5cbe9766 100644 (file)
--- a/pdns/recursordist/negcache.hh
+++ b/pdns/recursordist/negcache.hh
@@ -49,7 +49,7 @@ class NegCache : public boost::noncopyable {
       DNSName d_name;                     // The denied name
       QType d_qtype;                      // The denied type
       DNSName d_auth;                     // The denying name (aka auth)
-      uint32_t d_ttd;                     // Timestamp when this entry should die
+      mutable uint32_t d_ttd;             // Timestamp when this entry should die
       recordsAndSignatures authoritySOA;  // The upstream SOA record and RRSIGs
       recordsAndSignatures DNSSECRecords; // The upstream NSEC(3) and RRSIGs
       mutable vState d_validationState{Indeterminate};
@@ -59,7 +59,7 @@ class NegCache : public boost::noncopyable {
     };
 
     void add(const NegCacheEntry& ne);
-    void updateValidationStatus(const DNSName& qname, const QType& qtype, const vState newState);
+    void updateValidationStatus(const DNSName& qname, const QType& qtype, const vState newState, boost::optional<uint32_t> capTTD);
     bool get(const DNSName& qname, const QType& qtype, const struct timeval& now, const NegCacheEntry** ne, bool typeMustMatch=false);
     bool getRootNXTrust(const DNSName& qname, const struct timeval& now, const NegCacheEntry** ne);
     uint64_t count(const DNSName& qname) const;

diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc
index eb38f362c57a98b98a239291952b195e2664727a..d65d623b2fa719c9aab4bcb4413cdd133053a1fa 100644 (file)
--- a/pdns/recursordist/test-syncres_cc.cc
+++ b/pdns/recursordist/test-syncres_cc.cc
@@ -121,6 +121,7 @@ static void init(bool debug=false)
   SyncRes::s_maxtotusec = 1000*7000;
   SyncRes::s_maxdepth = 40;
   SyncRes::s_maxnegttl = 3600;
+  SyncRes::s_maxbogusttl = 3600;
   SyncRes::s_maxcachettl = 86400;
   SyncRes::s_packetcachettl = 3600;
   SyncRes::s_packetcacheservfailttl = 60;
@@ -1852,6 +1853,8 @@ BOOST_AUTO_TEST_CASE(test_root_nx_trust) {
       return 0;
     });
 
+  SyncRes::s_maxnegttl = 3600;
+
   vector<DNSRecord> ret;
   int res = sr->beginResolve(target1, QType(QType::A), QClass::IN, ret);
   BOOST_CHECK_EQUAL(res, RCode::NXDomain);
@@ -1862,7 +1865,8 @@ BOOST_AUTO_TEST_CASE(test_root_nx_trust) {
   ret.clear();
   res = sr->beginResolve(target2, QType(QType::A), QClass::IN, ret);
   BOOST_CHECK_EQUAL(res, RCode::NXDomain);
-  BOOST_CHECK_EQUAL(ret.size(), 1);
+  BOOST_REQUIRE_EQUAL(ret.size(), 1);
+  BOOST_CHECK_LE(ret[0].d_ttl, SyncRes::s_maxnegttl);
   /* one for target1 and one for the entire TLD */
   BOOST_CHECK_EQUAL(SyncRes::getNegCacheSize(), 2);
 
@@ -1926,8 +1930,8 @@ BOOST_AUTO_TEST_CASE(test_root_nx_trust_specific) {
   res = sr->beginResolve(target2, QType(QType::A), QClass::IN, ret);
   BOOST_CHECK_EQUAL(res, RCode::NoError);
   BOOST_REQUIRE_EQUAL(ret.size(), 1);
-  BOOST_REQUIRE(ret[0].d_type == QType::A);
   BOOST_CHECK_EQUAL(ret[0].d_name, target2);
+  BOOST_REQUIRE(ret[0].d_type == QType::A);
   BOOST_CHECK(getRR<ARecordContent>(ret[0])->getCA() == ComboAddress("192.0.2.2"));
 
   BOOST_CHECK_EQUAL(SyncRes::t_sstorage.negcache.size(), 1);
@@ -4414,7 +4418,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_bogus_no_rrsig) {
         char addr[] = "a.root-servers.net.";
         for (char idx = 'a'; idx <= 'm'; idx++) {
           addr[0] = idx;
-          addRecordToLW(res, domain, QType::NS, std::string(addr), DNSResourceRecord::ANSWER, 3600);
+          addRecordToLW(res, domain, QType::NS, std::string(addr), DNSResourceRecord::ANSWER, 86400);
         }
 
         /* No RRSIG */
@@ -4436,6 +4440,9 @@ BOOST_AUTO_TEST_CASE(test_dnssec_bogus_no_rrsig) {
       return 0;
     });
 
+  SyncRes::s_maxcachettl = 86400;
+  SyncRes::s_maxbogusttl = 3600;
+
   vector<DNSRecord> ret;
   int res = sr->beginResolve(target, QType(QType::NS), QClass::IN, ret);
   BOOST_CHECK_EQUAL(res, RCode::NoError);
@@ -4451,6 +4458,10 @@ BOOST_AUTO_TEST_CASE(test_dnssec_bogus_no_rrsig) {
   BOOST_CHECK_EQUAL(res, RCode::NoError);
   BOOST_CHECK_EQUAL(sr->getValidationState(), Bogus);
   BOOST_REQUIRE_EQUAL(ret.size(), 13);
+  /* check that we capped the TTL to max-cache-bogus-ttl */
+  for (const auto& record : ret) {
+    BOOST_CHECK_LE(record.d_ttl, SyncRes::s_maxbogusttl);
+  }
   BOOST_CHECK_EQUAL(queriesCount, 1);
 }
 
@@ -9321,6 +9332,76 @@ BOOST_AUTO_TEST_CASE(test_dnssec_rrsig_negcache_validity) {
   BOOST_CHECK_EQUAL(queriesCount, 4);
 }
 
+BOOST_AUTO_TEST_CASE(test_dnssec_rrsig_negcache_bogus_validity) {
+  std::unique_ptr<SyncRes> sr;
+  initSR(sr, true);
+
+  setDNSSECValidation(sr, DNSSECMode::ValidateAll);
+
+  primeHints();
+  const DNSName target("com.");
+  testkeysset_t keys;
+
+  auto luaconfsCopy = g_luaconfs.getCopy();
+  luaconfsCopy.dsAnchors.clear();
+  generateKeyMaterial(g_rootdnsname, DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys, luaconfsCopy.dsAnchors);
+  generateKeyMaterial(DNSName("com."), DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys);
+  g_luaconfs.setState(luaconfsCopy);
+
+  size_t queriesCount = 0;
+  const time_t fixedNow = sr->getNow().tv_sec;
+
+  sr->setAsyncCallback([target,&queriesCount,keys,fixedNow](const ComboAddress& ip, const DNSName& domain, int type, bool doTCP, bool sendRDQuery, int EDNS0Level, struct timeval* now, boost::optional<Netmask>& srcmask, boost::optional<const ResolveContext&> context, LWResult* res, bool* chained) {
+      queriesCount++;
+
+      DNSName auth = domain;
+      auth.chopOff();
+
+      if (type == QType::DS || type == QType::DNSKEY) {
+        return genericDSAndDNSKEYHandler(res, domain, auth, type, keys);
+      }
+      else {
+        setLWResult(res, RCode::NoError, true, false, true);
+        addRecordToLW(res, domain, QType::SOA, "pdns-public-ns1.powerdns.com. pieter\\.lexis.powerdns.com. 2017032301 10800 3600 604800 3600", DNSResourceRecord::AUTHORITY, 86400);
+        addRRSIG(keys, res->d_records, domain, 86400);
+        addNSECRecordToLW(domain, DNSName("z."), { QType::NSEC, QType::RRSIG }, 86400, res->d_records);
+        /* no RRSIG */
+        return 1;
+      }
+
+      return 0;
+    });
+
+  SyncRes::s_maxnegttl = 3600;
+  SyncRes::s_maxbogusttl = 360;
+
+  vector<DNSRecord> ret;
+  int res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::NoError);
+  BOOST_CHECK_EQUAL(sr->getValidationState(), Bogus);
+  BOOST_REQUIRE_EQUAL(ret.size(), 3);
+  BOOST_CHECK_EQUAL(queriesCount, 4);
+
+  /* check that the entry has not been negatively cached but not longer than s_maxbogusttl */
+  const NegCache::NegCacheEntry* ne = nullptr;
+  BOOST_CHECK_EQUAL(SyncRes::t_sstorage.negcache.size(), 1);
+  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), &ne), true);
+  BOOST_CHECK_EQUAL(ne->d_ttd, fixedNow + SyncRes::s_maxbogusttl);
+  BOOST_CHECK_EQUAL(ne->d_validationState, Bogus);
+  BOOST_CHECK_EQUAL(ne->authoritySOA.records.size(), 1);
+  BOOST_CHECK_EQUAL(ne->authoritySOA.signatures.size(), 1);
+  BOOST_CHECK_EQUAL(ne->DNSSECRecords.records.size(), 1);
+  BOOST_CHECK_EQUAL(ne->DNSSECRecords.signatures.size(), 0);
+
+  /* again, to test the cache */
+  ret.clear();
+  res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::NoError);
+  BOOST_CHECK_EQUAL(sr->getValidationState(), Bogus);
+  BOOST_REQUIRE_EQUAL(ret.size(), 3);
+  BOOST_CHECK_EQUAL(queriesCount, 4);
+}
+
 BOOST_AUTO_TEST_CASE(test_dnssec_rrsig_cache_validity) {
   std::unique_ptr<SyncRes> sr;
   initSR(sr, true);
@@ -9390,7 +9471,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_cache_secure) {
     Validation is optional, and the first query does not ask for it,
     so the answer is cached as Indeterminate.
     The second query asks for validation, answer should be marked as
-    Secure.
+    Secure, after just-in-time validation.
   */
   std::unique_ptr<SyncRes> sr;
   initSR(sr, true);
@@ -9549,7 +9630,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_cache_bogus) {
       else {
         if (domain == target && type == QType::A) {
           setLWResult(res, 0, true, false, true);
-          addRecordToLW(res, target, QType::A, "192.0.2.1");
+          addRecordToLW(res, target, QType::A, "192.0.2.1", DNSResourceRecord::ANSWER, 86400);
           /* no RRSIG */
           return 1;
         }
@@ -9558,6 +9639,8 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_cache_bogus) {
       return 0;
     });
 
+  SyncRes::s_maxbogusttl = 3600;
+
   vector<DNSRecord> ret;
   /* first query does not require validation */
   sr->setDNSSECValidationRequested(false);
@@ -9567,6 +9650,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_cache_bogus) {
   BOOST_REQUIRE_EQUAL(ret.size(), 1);
   for (const auto& record : ret) {
     BOOST_CHECK(record.d_type == QType::A);
+    BOOST_CHECK_EQUAL(record.d_ttl, 86400);
   }
   BOOST_CHECK_EQUAL(queriesCount, 1);
 
@@ -9577,9 +9661,26 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_cache_bogus) {
   res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret);
   BOOST_CHECK_EQUAL(res, RCode::NoError);
   BOOST_CHECK_EQUAL(sr->getValidationState(), Bogus);
+  /* check that we correctly capped the TTD for a Bogus record after
+     just-in-time validation */
+  BOOST_REQUIRE_EQUAL(ret.size(), 1);
+  for (const auto& record : ret) {
+    BOOST_CHECK(record.d_type == QType::A);
+    BOOST_CHECK_EQUAL(record.d_ttl, SyncRes::s_maxbogusttl);
+  }
+  BOOST_CHECK_EQUAL(queriesCount, 3);
+
+  ret.clear();
+  /* third time also _does_ require validation, so we
+     can check that the cache has been updated */
+  sr->setDNSSECValidationRequested(true);
+  res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::NoError);
+  BOOST_CHECK_EQUAL(sr->getValidationState(), Bogus);
   BOOST_REQUIRE_EQUAL(ret.size(), 1);
   for (const auto& record : ret) {
     BOOST_CHECK(record.d_type == QType::A);
+    BOOST_CHECK_EQUAL(record.d_ttl, SyncRes::s_maxbogusttl);
   }
   BOOST_CHECK_EQUAL(queriesCount, 3);
 }
@@ -9763,13 +9864,13 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_cname_cache_bogus) {
       else {
         if (domain == target && type == QType::A) {
           setLWResult(res, 0, true, false, true);
-          addRecordToLW(res, target, QType::CNAME, cnameTarget.toString());
-          addRecordToLW(res, cnameTarget, QType::A, "192.0.2.1");
+          addRecordToLW(res, target, QType::CNAME, cnameTarget.toString(), DNSResourceRecord::ANSWER, 86400);
+          addRecordToLW(res, cnameTarget, QType::A, "192.0.2.1", DNSResourceRecord::ANSWER, 86400);
           /* no RRSIG */
           return 1;
         } else if (domain == cnameTarget && type == QType::A) {
           setLWResult(res, 0, true, false, true);
-          addRecordToLW(res, cnameTarget, QType::A, "192.0.2.1");
+          addRecordToLW(res, cnameTarget, QType::A, "192.0.2.1", DNSResourceRecord::ANSWER, 86400);
           /* no RRSIG */
           return 1;
         }
@@ -9778,6 +9879,9 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_cname_cache_bogus) {
       return 0;
     });
 
+  SyncRes::s_maxbogusttl = 60;
+  SyncRes::s_maxnegttl = 3600;
+
   vector<DNSRecord> ret;
   /* first query does not require validation */
   sr->setDNSSECValidationRequested(false);
@@ -9787,6 +9891,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_cname_cache_bogus) {
   BOOST_REQUIRE_EQUAL(ret.size(), 2);
   for (const auto& record : ret) {
     BOOST_CHECK(record.d_type == QType::CNAME || record.d_type == QType::A);
+    BOOST_CHECK_EQUAL(record.d_ttl, 86400);
   }
   BOOST_CHECK_EQUAL(queriesCount, 2);
 
@@ -9798,8 +9903,25 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_cname_cache_bogus) {
   BOOST_CHECK_EQUAL(res, RCode::NoError);
   BOOST_CHECK_EQUAL(sr->getValidationState(), Bogus);
   BOOST_REQUIRE_EQUAL(ret.size(), 2);
+  /* check that we correctly capped the TTD for a Bogus record after
+     just-in-time validation */
+  for (const auto& record : ret) {
+    BOOST_CHECK(record.d_type == QType::CNAME || record.d_type == QType::A);
+    BOOST_CHECK_EQUAL(record.d_ttl, SyncRes::s_maxbogusttl);
+  }
+  BOOST_CHECK_EQUAL(queriesCount, 5);
+
+  ret.clear();
+  /* and a third time to make sure that the validation status (and TTL!)
+     was properly updated in the cache */
+  sr->setDNSSECValidationRequested(true);
+  res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::NoError);
+  BOOST_CHECK_EQUAL(sr->getValidationState(), Bogus);
+  BOOST_REQUIRE_EQUAL(ret.size(), 2);
   for (const auto& record : ret) {
     BOOST_CHECK(record.d_type == QType::CNAME || record.d_type == QType::A);
+    BOOST_CHECK_EQUAL(record.d_ttl, SyncRes::s_maxbogusttl);
   }
   BOOST_CHECK_EQUAL(queriesCount, 5);
 }
@@ -10134,8 +10256,8 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_bogus) {
       }
       else {
         setLWResult(res, RCode::NoError, true, false, true);
-        addRecordToLW(res, domain, QType::SOA, "pdns-public-ns1.powerdns.com. pieter\\.lexis.powerdns.com. 2017032301 10800 3600 604800 3600", DNSResourceRecord::AUTHORITY, 3600);
-        addRRSIG(keys, res->d_records, domain, 300);
+        addRecordToLW(res, domain, QType::SOA, "pdns-public-ns1.powerdns.com. pieter\\.lexis.powerdns.com. 2017032301 10800 3600 604800 3600", DNSResourceRecord::AUTHORITY, 86400);
+        addRRSIG(keys, res->d_records, domain, 86400);
         /* no denial */
         return 1;
       }
@@ -10143,6 +10265,10 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_bogus) {
       return 0;
     });
 
+  SyncRes::s_maxbogusttl = 60;
+  SyncRes::s_maxnegttl = 3600;
+  const auto now = sr->getNow().tv_sec;
+
   vector<DNSRecord> ret;
   /* first query does not require validation */
   sr->setDNSSECValidationRequested(false);
@@ -10150,6 +10276,11 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_bogus) {
   BOOST_CHECK_EQUAL(res, RCode::NoError);
   BOOST_CHECK_EQUAL(sr->getValidationState(), Indeterminate);
   BOOST_REQUIRE_EQUAL(ret.size(), 2);
+  for (const auto& record : ret) {
+    if (record.d_type == QType::SOA) {
+      BOOST_CHECK_EQUAL(record.d_ttl, SyncRes::s_maxnegttl);
+    }
+  }
   BOOST_CHECK_EQUAL(queriesCount, 1);
   const NegCache::NegCacheEntry* ne = nullptr;
   BOOST_CHECK_EQUAL(SyncRes::t_sstorage.negcache.size(), 1);
@@ -10157,6 +10288,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_bogus) {
   BOOST_CHECK_EQUAL(ne->d_validationState, Indeterminate);
   BOOST_CHECK_EQUAL(ne->authoritySOA.records.size(), 1);
   BOOST_CHECK_EQUAL(ne->authoritySOA.signatures.size(), 1);
+  BOOST_CHECK_EQUAL(ne->d_ttd, now + SyncRes::s_maxnegttl);
   BOOST_CHECK_EQUAL(ne->DNSSECRecords.records.size(), 0);
   BOOST_CHECK_EQUAL(ne->DNSSECRecords.signatures.size(), 0);
 
@@ -10167,11 +10299,35 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_bogus) {
   BOOST_CHECK_EQUAL(res, RCode::NoError);
   BOOST_CHECK_EQUAL(sr->getValidationState(), Bogus);
   BOOST_REQUIRE_EQUAL(ret.size(), 2);
+  for (const auto& record : ret) {
+    BOOST_CHECK_EQUAL(record.d_ttl, SyncRes::s_maxbogusttl);
+  }
+  BOOST_CHECK_EQUAL(queriesCount, 4);
+  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), &ne), true);
+  BOOST_CHECK_EQUAL(ne->d_validationState, Bogus);
+  BOOST_CHECK_EQUAL(ne->authoritySOA.records.size(), 1);
+  BOOST_CHECK_EQUAL(ne->authoritySOA.signatures.size(), 1);
+  BOOST_CHECK_EQUAL(ne->d_ttd, now + SyncRes::s_maxbogusttl);
+  BOOST_CHECK_EQUAL(ne->DNSSECRecords.records.size(), 0);
+  BOOST_CHECK_EQUAL(ne->DNSSECRecords.signatures.size(), 0);
+
+  ret.clear();
+  /* third one _does_ not require validation, we just check that
+     the cache (status and TTL) has been correctly updated */
+  sr->setDNSSECValidationRequested(false);
+  res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::NoError);
+  BOOST_CHECK_EQUAL(sr->getValidationState(), Bogus);
+  BOOST_REQUIRE_EQUAL(ret.size(), 2);
+  for (const auto& record : ret) {
+    BOOST_CHECK_EQUAL(record.d_ttl, SyncRes::s_maxbogusttl);
+  }
   BOOST_CHECK_EQUAL(queriesCount, 4);
   BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), &ne), true);
   BOOST_CHECK_EQUAL(ne->d_validationState, Bogus);
   BOOST_CHECK_EQUAL(ne->authoritySOA.records.size(), 1);
   BOOST_CHECK_EQUAL(ne->authoritySOA.signatures.size(), 1);
+  BOOST_CHECK_EQUAL(ne->d_ttd, now + SyncRes::s_maxbogusttl);
   BOOST_CHECK_EQUAL(ne->DNSSECRecords.records.size(), 0);
   BOOST_CHECK_EQUAL(ne->DNSSECRecords.signatures.size(), 0);
 }

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 52d3e1b659e2eb24b02f732346f2d7de8937e843..a13d720c36714cb0c9c662b7e25959d9e558f651 100644 (file)
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -49,6 +49,7 @@ string SyncRes::s_serverID;
 SyncRes::LogMode SyncRes::s_lm;
 
 unsigned int SyncRes::s_maxnegttl;
+unsigned int SyncRes::s_maxbogusttl;
 unsigned int SyncRes::s_maxcachettl;
 unsigned int SyncRes::s_maxqperq;
 unsigned int SyncRes::s_maxtotusec;
@@ -912,6 +913,16 @@ DNSName SyncRes::getBestNSNamesFromCache(const DNSName &qname, const QType& qtyp
   return subdomain;
 }
 
+void SyncRes::updateValidationStatusInCache(const DNSName &qname, const QType& qt, bool aa, vState newState) const
+{
+  if (newState == Bogus) {
+    t_RC->updateValidationStatus(d_now.tv_sec, qname, qt, d_cacheRemote, aa, newState, s_maxbogusttl + d_now.tv_sec);
+  }
+  else {
+    t_RC->updateValidationStatus(d_now.tv_sec, qname, qt, d_cacheRemote, aa, newState, boost::none);
+  }
+}
+
 bool SyncRes::doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector<DNSRecord>& ret, unsigned int depth, int &res, vState& state, bool wasAuthZone, bool wasForwardRecurse)
 {
   string prefix;
@@ -931,6 +942,7 @@ bool SyncRes::doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector
   vector<std::shared_ptr<RRSIGRecordContent>> signatures;
   vector<std::shared_ptr<DNSRecord>> authorityRecs;
   bool wasAuth;
+  uint32_t capTTL = std::numeric_limits<uint32_t>::max();
   /* we don't require auth data for forward-recurse lookups */
   if(t_RC->get(d_now.tv_sec, qname, QType(QType::CNAME), !wasForwardRecurse && d_requireAuthData, &cset, d_cacheRemote, d_doDNSSEC ? &signatures : nullptr, d_doDNSSEC ? &authorityRecs : nullptr, &d_wasVariable, &state, &wasAuth) > 0) {
 
@@ -958,7 +970,10 @@ bool SyncRes::doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector
             state = SyncRes::validateRecordsWithSigs(depth, qname, QType(QType::CNAME), qname, cset, signatures);
             if (state != Indeterminate) {
               LOG(prefix<<qname<<": got Indeterminate state from the CNAME cache, new validation result is "<<vStates[state]<<endl);
-              t_RC->updateValidationStatus(d_now.tv_sec, qname, QType(QType::CNAME), d_cacheRemote, d_requireAuthData, state);
+              if (state == Bogus) {
+                capTTL = s_maxbogusttl;
+              }
+              updateValidationStatusInCache(qname, QType(QType::CNAME), wasAuth, state);
             }
           }
         }
@@ -966,7 +981,9 @@ bool SyncRes::doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector
         LOG(prefix<<qname<<": Found cache CNAME hit for '"<< qname << "|CNAME" <<"' to '"<<j->d_content->getZoneRepresentation()<<"', validation state is "<<vStates[state]<<endl);
 
         DNSRecord dr=*j;
-        dr.d_ttl-=d_now.tv_sec;
+        dr.d_ttl -= d_now.tv_sec;
+        dr.d_ttl = std::min(dr.d_ttl, capTTL);
+        const uint32_t ttl = dr.d_ttl;
         ret.reserve(ret.size() + 1 + signatures.size() + authorityRecs.size());
         ret.push_back(dr);
 
@@ -974,7 +991,7 @@ bool SyncRes::doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector
           DNSRecord sigdr;
           sigdr.d_type=QType::RRSIG;
           sigdr.d_name=qname;
-          sigdr.d_ttl=j->d_ttl - d_now.tv_sec;
+          sigdr.d_ttl=ttl;
           sigdr.d_content=signature;
           sigdr.d_place=DNSResourceRecord::ANSWER;
           sigdr.d_class=QClass::IN;
@@ -983,7 +1000,7 @@ bool SyncRes::doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector
 
         for(const auto& rec : authorityRecs) {
           DNSRecord authDR(*rec);
-          authDR.d_ttl=j->d_ttl - d_now.tv_sec;
+          authDR.d_ttl=ttl;
           ret.push_back(authDR);
         }
 
@@ -1105,7 +1122,11 @@ void SyncRes::computeNegCacheValidationStatus(const NegCache::NegCacheEntry* ne,
   }
   if (state != Indeterminate) {
     /* validation succeeded, let's update the cache entry so we don't have to validate again */
-    t_sstorage.negcache.updateValidationStatus(ne->d_name, ne->d_qtype, state);
+    boost::optional<uint32_t> capTTD = boost::none;
+    if (state == Bogus) {
+      capTTD = d_now.tv_sec + s_maxbogusttl;
+    }
+    t_sstorage.negcache.updateValidationStatus(ne->d_name, ne->d_qtype, state, capTTD);
   }
 }
 
@@ -1165,6 +1186,10 @@ bool SyncRes::doCacheCheck(const DNSName &qname, const DNSName& authname, bool w
     if (!wasAuthZone && shouldValidate() && state == Indeterminate) {
       LOG(prefix<<qname<<": got Indeterminate state for records retrieved from the negative cache, validating.."<<endl);
       computeNegCacheValidationStatus(ne, qname, qtype, res, state, depth);
+
+      if (state != cachedState && state == Bogus) {
+        sttl = std::min(sttl, s_maxbogusttl);
+      }
     }
 
     // Transplant SOA to the returned packet
@@ -1184,6 +1209,7 @@ bool SyncRes::doCacheCheck(const DNSName &qname, const DNSName& authname, bool w
   vector<std::shared_ptr<RRSIGRecordContent>> signatures;
   vector<std::shared_ptr<DNSRecord>> authorityRecs;
   uint32_t ttl=0;
+  uint32_t capTTL = std::numeric_limits<uint32_t>::max();
   bool wasCachedAuth;
   if(t_RC->get(d_now.tv_sec, sqname, sqt, !wasForwardRecurse && d_requireAuthData, &cset, d_cacheRemote, d_doDNSSEC ? &signatures : nullptr, d_doDNSSEC ? &authorityRecs : nullptr, &d_wasVariable, &cachedState, &wasCachedAuth) > 0) {
 
@@ -1212,7 +1238,10 @@ bool SyncRes::doCacheCheck(const DNSName &qname, const DNSName& authname, bool w
 
       if (cachedState != Indeterminate) {
         LOG(prefix<<qname<<": got Indeterminate state from the cache, validation result is "<<vStates[cachedState]<<endl);
-        t_RC->updateValidationStatus(d_now.tv_sec, sqname, sqt, d_cacheRemote, d_requireAuthData, cachedState);
+        if (cachedState == Bogus) {
+          capTTL = s_maxbogusttl;
+        }
+        updateValidationStatusInCache(sqname, sqt, wasCachedAuth, cachedState);
       }
     }
 
@@ -1226,7 +1255,9 @@ bool SyncRes::doCacheCheck(const DNSName &qname, const DNSName& authname, bool w
 
       if(j->d_ttl>(unsigned int) d_now.tv_sec) {
         DNSRecord dr=*j;
-        ttl = (dr.d_ttl-=d_now.tv_sec);
+        dr.d_ttl -= d_now.tv_sec;
+        dr.d_ttl = std::min(dr.d_ttl, capTTL);
+        ttl = dr.d_ttl;
         ret.push_back(dr);
         LOG("[ttl="<<dr.d_ttl<<"] ");
         found=true;
@@ -2366,6 +2397,13 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr
       }
     }
 
+    if (recordState == Bogus) {
+      /* this is a TTD by now, be careful */
+      for(auto& record : i->second.records) {
+        record.d_ttl = std::min(record.d_ttl, static_cast<uint32_t>(s_maxbogusttl + d_now.tv_sec));
+      }
+    }
+
     /* We don't need to store NSEC3 records in the positive cache because:
        - we don't allow direct NSEC3 queries
        - denial of existence proofs in wildcard expanded positive responses are stored in authorityRecs
@@ -2448,7 +2486,6 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co
       ne.d_qtype = QType(0); // this encodes 'whole record'
       ne.d_auth = rec.d_name;
       harvestNXRecords(lwr.d_records, ne, d_now.tv_sec, &lowestTTL);
-      ne.d_ttd = d_now.tv_sec + lowestTTL;
 
       if (state == Secure) {
         dState denialState = getDenialValidationState(ne, state, NXDOMAIN, false);
@@ -2458,6 +2495,11 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co
         ne.d_validationState = state;
       }
 
+      if (ne.d_validationState == Bogus) {
+        lowestTTL = min(lowestTTL, s_maxbogusttl);
+      }
+
+      ne.d_ttd = d_now.tv_sec + lowestTTL;
       /* if we get an NXDomain answer with a CNAME, let's not cache the
          target, even the server was authoritative for it,
          and do an additional query for the CNAME target.
@@ -2495,7 +2537,6 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co
       LOG(prefix<<qname<<": answer is in: resolved to '"<< rec.d_content->getZoneRepresentation()<<"|"<<DNSRecordContent::NumberToType(rec.d_type)<<"'"<<endl);
 
       done=true;
-      ret.push_back(rec);
 
       if (state == Secure && needWildcardProof) {
         /* We have a positive answer synthetized from a wildcard, we need to check that we have
@@ -2521,13 +2562,15 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co
           }
           else {
             LOG(d_prefix<<"Invalid denial in wildcard expanded positive response found for "<<qname<<", returning Bogus, res="<<res<<endl);
+            rec.d_ttl = std::min(rec.d_ttl, s_maxbogusttl);
           }
 
           updateValidationState(state, st);
           /* we already stored the record with a different validation status, let's fix it */
-          t_RC->updateValidationStatus(d_now.tv_sec, qname, qtype, d_cacheRemote, lwr.d_aabit, st);
+          updateValidationStatusInCache(qname, qtype, lwr.d_aabit, st);
         }
       }
+      ret.push_back(rec);
     }
     else if((rec.d_type==QType::RRSIG || rec.d_type==QType::NSEC || rec.d_type==QType::NSEC3) && rec.d_place==DNSResourceRecord::ANSWER) {
       if(rec.d_type != QType::RRSIG || rec.d_name == qname)
@@ -2588,7 +2631,6 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co
       }
       else {
         rec.d_ttl = min(s_maxnegttl, rec.d_ttl);
-        ret.push_back(rec);
 
         NegCache::NegCacheEntry ne;
         ne.d_auth = rec.d_name;
@@ -2596,7 +2638,6 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co
         ne.d_name = qname;
         ne.d_qtype = qtype;
         harvestNXRecords(lwr.d_records, ne, d_now.tv_sec, &lowestTTL);
-        ne.d_ttd = d_now.tv_sec + lowestTTL;
 
         if (state == Secure) {
           dState denialState = getDenialValidationState(ne, state, NXQTYPE, false);
@@ -2605,11 +2646,19 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co
           ne.d_validationState = state;
         }
 
+        if (ne.d_validationState == Bogus) {
+          lowestTTL = min(lowestTTL, s_maxbogusttl);
+          rec.d_ttl = min(rec.d_ttl, s_maxbogusttl);
+        }
+        ne.d_ttd = d_now.tv_sec + lowestTTL;
+
         if(!wasVariable()) {
           if(qtype.getCode()) {  // prevents us from blacking out a whole domain
             t_sstorage.negcache.add(ne);
           }
         }
+
+        ret.push_back(rec);
         negindic=true;
       }
     }

diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index 53825ac81ed7568872079cccee3f40048610fb3f..a19f21e54e6d9ead23d52b2238e582f1690e5f08 100644 (file)
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -693,6 +693,7 @@ public:
   static unsigned int s_maxtotusec;
   static unsigned int s_maxdepth;
   static unsigned int s_maxnegttl;
+  static unsigned int s_maxbogusttl;
   static unsigned int s_maxcachettl;
   static unsigned int s_packetcachettl;
   static unsigned int s_packetcacheservfailttl;
@@ -789,6 +790,7 @@ private:
   vState getTA(const DNSName& zone, dsmap_t& ds);
   bool haveExactValidationStatus(const DNSName& domain);
   vState getValidationStatus(const DNSName& subdomain, bool allowIndeterminate=true);
+  void updateValidationStatusInCache(const DNSName &qname, const QType& qt, bool aa, vState newState) const;
 
   bool lookForCut(const DNSName& qname, unsigned int depth, const vState existingState, vState& newState);
   void computeZoneCuts(const DNSName& begin, const DNSName& end, unsigned int depth);