Add a new 'max-ds-per-zone' setting and immediately return BogusNoValidDNSKEY when...

diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc
index 93f39d9f1c7b76c5564a419c9bb2baa8bfc5afdd..57c5995545faea03e9f4a468204ffe3dfc06f0bf 100644 (file)
--- a/pdns/recursordist/rec-main.cc
+++ b/pdns/recursordist/rec-main.cc
@@ -1495,6 +1495,7 @@ static int initDNSSEC(Logr::log_t log)
   g_maxRRSIGsPerRecordToConsider = ::arg().asNum("max-rrsigs-per-record");
   g_maxNSEC3sPerRecordToConsider = ::arg().asNum("max-nsec3s-per-record");
   g_maxDNSKEYsToConsider = ::arg().asNum("max-dnskeys");
+  g_maxDSsToConsider = ::arg().asNum("max-ds-per-zone");
 
   vector<string> nums;
   bool automatic = true;
@@ -3008,6 +3009,7 @@ static void initArgs()
   ::arg().set("max-nsec3-hash-computations-per-query", "Maximum number of NSEC3 hashes that we are willing to compute during DNSSEC validation, per incoming query") = "600";
   ::arg().set("aggressive-cache-max-nsec3-hash-cost", "Maximum estimated NSEC3 cost for a given query to consider aggressive use of the NSEC3 cache") = "150";
   ::arg().set("max-dnskeys", "Maximum number of DNSKEYs with the same algorithm and tag to consider when validating a given record") = "2";
+  ::arg().set("max-ds-per-zone", "Maximum number of DS records to consider per zone") = "8";
 
   ::arg().set("cpu-map", "Thread to CPU mapping, space separated thread-id=cpu1,cpu2..cpuN pairs") = "";
 

diff --git a/pdns/recursordist/test-syncres_cc4.cc b/pdns/recursordist/test-syncres_cc4.cc
index e6545b73aac709c12d11caffe97d189ad5f5b4ff..05cfe35a1cff60e897ea023bd1cd0b93da740f4d 100644 (file)
--- a/pdns/recursordist/test-syncres_cc4.cc
+++ b/pdns/recursordist/test-syncres_cc4.cc
@@ -439,7 +439,6 @@ BOOST_AUTO_TEST_CASE(test_dnssec_rrsig)
 
   auto dcke = DNSCryptoKeyEngine::make(DNSSECKeeper::ECDSA256);
   dcke->create(dcke->getBits());
-  // cerr<<dcke->convertToISC()<<endl;
   DNSSECPrivateKey dpk;
   dpk.setKey(std::move(dcke), 256);
 
@@ -955,6 +954,82 @@ BOOST_AUTO_TEST_CASE(test_dnssec_bogus_dnskey_doesnt_match_ds)
   BOOST_CHECK_EQUAL(queriesCount, 4U);
 }
 
+BOOST_AUTO_TEST_CASE(test_dnssec_bogus_too_many_dss)
+{
+  std::unique_ptr<SyncRes> sr;
+  initSR(sr, true);
+
+  setDNSSECValidation(sr, DNSSECMode::Process);
+
+  primeHints();
+  const DNSName target(".");
+  testkeysset_t keys;
+
+  g_maxDSsToConsider = 1;
+
+  auto luaconfsCopy = g_luaconfs.getCopy();
+  luaconfsCopy.dsAnchors.clear();
+  /* generate more DSs for the zone than we are willing to consider: only the last one will be used to generate DNSKEY records */
+  for (size_t idx = 0; idx < (g_maxDSsToConsider + 10U); idx++) {
+    generateKeyMaterial(g_rootdnsname, DNSSECKeeper::RSASHA512, DNSSECKeeper::DIGEST_SHA384, keys, luaconfsCopy.dsAnchors);
+  }
+  g_luaconfs.setState(luaconfsCopy);
+
+  size_t queriesCount = 0;
+
+  sr->setAsyncCallback([target, &queriesCount, keys](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++;
+
+    if (domain == target && type == QType::NS) {
+
+      setLWResult(res, 0, true, false, true);
+      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);
+      }
+
+      addRRSIG(keys, res->d_records, domain, 300);
+
+      addRecordToLW(res, "a.root-servers.net.", QType::A, "198.41.0.4", DNSResourceRecord::ADDITIONAL, 3600);
+      addRecordToLW(res, "a.root-servers.net.", QType::AAAA, "2001:503:ba3e::2:30", DNSResourceRecord::ADDITIONAL, 3600);
+
+      return LWResult::Result::Success;
+    }
+    else if (domain == target && type == QType::DNSKEY) {
+
+      setLWResult(res, 0, true, false, true);
+
+      addDNSKEY(keys, domain, 300, res->d_records);
+      addRRSIG(keys, res->d_records, domain, 300);
+
+      return LWResult::Result::Success;
+    }
+
+    return LWResult::Result::Timeout;
+  });
+
+  /* === with validation enabled === */
+  sr->setDNSSECValidationRequested(true);
+  vector<DNSRecord> ret;
+  int res = sr->beginResolve(target, QType(QType::NS), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::NoError);
+  BOOST_CHECK_EQUAL(sr->getValidationState(), vState::BogusNoValidDNSKEY);
+  /* 13 NS + 1 RRSIG */
+  BOOST_REQUIRE_EQUAL(ret.size(), 14U);
+  BOOST_CHECK_EQUAL(queriesCount, 2U);
+
+  /* again, to test the cache */
+  ret.clear();
+  res = sr->beginResolve(target, QType(QType::NS), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::NoError);
+  BOOST_CHECK_EQUAL(sr->getValidationState(), vState::BogusNoValidDNSKEY);
+  BOOST_REQUIRE_EQUAL(ret.size(), 14U);
+  BOOST_CHECK_EQUAL(queriesCount, 2U);
+
+  g_maxDNSKEYsToConsider = 0;
+}
+
 BOOST_AUTO_TEST_CASE(test_dnssec_bogus_too_many_dnskeys)
 {
   std::unique_ptr<SyncRes> sr;
@@ -1039,7 +1114,7 @@ PrivateKey: n7SRA4n6NejhZBWQOhjTaICYSpkTl6plJn1ATFG23FI=)PKEY");
   vector<DNSRecord> ret;
   int res = sr->beginResolve(target, QType(QType::NS), QClass::IN, ret);
   BOOST_CHECK_EQUAL(res, RCode::NoError);
-  BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Insecure);
+  BOOST_CHECK_EQUAL(sr->getValidationState(), vState::BogusNoValidDNSKEY);
   /* 13 NS + 1 RRSIG */
   BOOST_REQUIRE_EQUAL(ret.size(), 14U);
   BOOST_CHECK_EQUAL(queriesCount, 2U);
@@ -1048,7 +1123,7 @@ PrivateKey: n7SRA4n6NejhZBWQOhjTaICYSpkTl6plJn1ATFG23FI=)PKEY");
   ret.clear();
   res = sr->beginResolve(target, QType(QType::NS), QClass::IN, ret);
   BOOST_CHECK_EQUAL(res, RCode::NoError);
-  BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Insecure);
+  BOOST_CHECK_EQUAL(sr->getValidationState(), vState::BogusNoValidDNSKEY);
   BOOST_REQUIRE_EQUAL(ret.size(), 14U);
   BOOST_CHECK_EQUAL(queriesCount, 2U);
 

diff --git a/pdns/validate.cc b/pdns/validate.cc
index 7d4c8e2a6cea1d940c233fd201dfa69c11f5ef04..52cc4b7ac1fee8c14d975efa9afd06292323c42e 100644 (file)
--- a/pdns/validate.cc
+++ b/pdns/validate.cc
@@ -33,6 +33,7 @@ uint16_t g_maxNSEC3Iterations{0};
 uint16_t g_maxRRSIGsPerRecordToConsider{0};
 uint16_t g_maxNSEC3sPerRecordToConsider{0};
 uint16_t g_maxDNSKEYsToConsider{0};
+uint16_t g_maxDSsToConsider{0};
 
 static bool isAZoneKey(const DNSKEYRecordContent& key)
 {
@@ -1154,7 +1155,15 @@ vState validateDNSKeysAgainstDS(time_t now, const DNSName& zone, const dsmap_t&
    * Check all DNSKEY records against all DS records and place all DNSKEY records
    * that have DS records (that we support the algo for) in the tentative key storage
    */
+  uint16_t dssConsidered = 0;
   for (const auto& dsrc : dsmap) {
+    if (g_maxDSsToConsider > 0 && dssConsidered > g_maxDSsToConsider) {
+      VLOG(log, zone << ": We have already considered "<<std::to_string(dssConsidered)<<" DS"<<addS(dssConsidered)<<", not considering the remaining ones"<<endl;);
+      return vState::BogusNoValidDNSKEY;
+    }
+    ++dssConsidered;
+
+    uint16_t dnskeysConsidered = 0;
     auto record = getByTag(tkeys, dsrc.d_tag, dsrc.d_algorithm, log);
     // cerr<<"looking at DS with tag "<<dsrc.d_tag<<", algo "<<DNSSECKeeper::algorithm2name(dsrc.d_algorithm)<<", digest "<<std::to_string(dsrc.d_digesttype)<<" for "<<zone<<", got "<<r.size()<<" DNSKEYs for tag"<<endl;
 
@@ -1162,6 +1171,13 @@ vState validateDNSKeysAgainstDS(time_t now, const DNSName& zone, const dsmap_t&
       bool isValid = false;
       bool dsCreated = false;
       DSRecordContent dsrc2;
+
+      if (g_maxDNSKEYsToConsider > 0 && dnskeysConsidered >= g_maxDNSKEYsToConsider) {
+        VLOG(log, zone << ": We have already considered "<<std::to_string(dnskeysConsidered)<<" DNSKEY"<<addS(dnskeysConsidered)<<" for tag "<<std::to_string(dsrc.d_tag)<<" and algorithm "<<std::to_string(dsrc.d_algorithm)<<", not considering the remaining ones for this DS"<<endl;);
+        return vState::BogusNoValidDNSKEY;
+      }
+      dnskeysConsidered++;
+
       try {
         dsrc2 = makeDSFromDNSKey(zone, *drc, dsrc.d_digesttype);
         dsCreated = true;
@@ -1210,7 +1226,7 @@ vState validateDNSKeysAgainstDS(time_t now, const DNSName& zone, const dsmap_t&
       if (g_maxRRSIGsPerRecordToConsider > 0 && signaturesConsidered >= g_maxRRSIGsPerRecordToConsider) {
         VLOG(log, zone << ": We have already considered "<<std::to_string(signaturesConsidered)<<" RRSIG"<<addS(signaturesConsidered)<<" for this record, stopping now"<<endl;);
         // possibly going Bogus, the RRSIGs have not been validated so Insecure would be wrong
-        break;
+        return vState::BogusNoValidDNSKEY;
       }
 
       string msg = getMessageForRRSET(zone, *sig, toSign);
@@ -1218,14 +1234,14 @@ vState validateDNSKeysAgainstDS(time_t now, const DNSName& zone, const dsmap_t&
       for (const auto& key : bytag) {
         if (g_maxDNSKEYsToConsider > 0 && dnskeysConsidered >= g_maxDNSKEYsToConsider) {
           VLOG(log, zone << ": We have already considered "<<std::to_string(dnskeysConsidered)<<" DNSKEY"<<addS(dnskeysConsidered)<<" for tag "<<std::to_string(sig->d_tag)<<" and algorithm "<<std::to_string(sig->d_algorithm)<<", not considering the remaining ones for this signature"<<endl;);
-          return vState::Insecure;
+          return vState::BogusNoValidDNSKEY;
         }
         dnskeysConsidered++;
 
         if (g_maxRRSIGsPerRecordToConsider > 0 && signaturesConsidered >= g_maxRRSIGsPerRecordToConsider) {
           VLOG(log, zone << ": We have already considered "<<std::to_string(signaturesConsidered)<<" RRSIG"<<addS(signaturesConsidered)<<" for this record, stopping now"<<endl;);
           // possibly going Bogus, the RRSIGs have not been validated so Insecure would be wrong
-          break;
+          return vState::BogusNoValidDNSKEY;
         }
         //          cerr<<"validating : ";
         bool signIsValid = checkSignatureWithKey(zone, *sig, *key, msg, ede, log);

diff --git a/pdns/validate.hh b/pdns/validate.hh
index f47153747f8a4939ba7320de3918bbbd0370e837..7d844bf11c6d07ce66fe5137139a85512aa998cb 100644 (file)
--- a/pdns/validate.hh
+++ b/pdns/validate.hh
@@ -34,6 +34,7 @@ extern uint16_t g_maxNSEC3Iterations;
 extern uint16_t g_maxRRSIGsPerRecordToConsider;
 extern uint16_t g_maxNSEC3sPerRecordToConsider;
 extern uint16_t g_maxDNSKEYsToConsider;
+extern uint16_t g_maxDSsToConsider;
 
 // 4033 5
 enum class vState : uint8_t { Indeterminate, Insecure, Secure, NTA, TA, BogusNoValidDNSKEY, BogusInvalidDenial, BogusUnableToGetDSs, BogusUnableToGetDNSKEYs, BogusSelfSignedDS, BogusNoRRSIG, BogusNoValidRRSIG, BogusMissingNegativeIndication, BogusSignatureNotYetValid, BogusSignatureExpired, BogusUnsupportedDNSKEYAlgo, BogusUnsupportedDSDigestType, BogusNoZoneKeyBitSet, BogusRevokedDNSKEY, BogusInvalidDNSKEYProtocol };