rec: Skip the current zone when looking for a cut after an invalid DS denial proof

diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc
index e6b542537f3de48358ac21f089ec64b0a358e8be..9a318dfc46ba8a2a2a3276e2796f1ec39137f429 100644 (file)
--- a/pdns/recursordist/syncres.cc
+++ b/pdns/recursordist/syncres.cc
@@ -4116,8 +4116,20 @@ vState SyncRes::validateRecordsWithSigs(unsigned int depth, const string& prefix
   }
 
   LOG(prefix << vStateToString(state) << "!" << endl);
+
+  bool skipThisLevelWhenLookingForMissedCuts = false;
+  if (name == qname && qtype == QType::DS && (type == QType::NSEC || type == QType::NSEC3)) {
+    /* so we have a NSEC(3) record likely proving that the DS we were looking for does not exist,
+       but we cannot validate it:
+       - if there actually is a cut at this level, we will not be able to validate it anyway
+       - if there is no cut at this level, the only thing that can save us is a cut above
+    */
+    LOG(prefix << name << ": We are trying to validate a " << type << " record for " << name << " likely proving that the DS we were initially looking for (" << qname << ") does not exist, no need to check a zone cut at this exact level" << endl);
+    skipThisLevelWhenLookingForMissedCuts = true;
+  }
+
   /* try again to get the missed cuts, harder this time */
-  auto zState = getValidationStatus(name, false, type == QType::DS, depth, prefix);
+  auto zState = getValidationStatus(name, false, type == QType::DS || skipThisLevelWhenLookingForMissedCuts, depth, prefix);
   LOG(prefix << name << ": Checking whether we missed a zone cut before returning a Bogus state" << endl);
   if (zState == vState::Secure) {
     /* too bad */

diff --git a/pdns/recursordist/test-syncres_cc10.cc b/pdns/recursordist/test-syncres_cc10.cc
index fe0738a7392c9260c48a0d4dfa67eb9c383043da..fb1c85cff0c5eb86dfcb88e4d5e43651acea5b70 100644 (file)
--- a/pdns/recursordist/test-syncres_cc10.cc
+++ b/pdns/recursordist/test-syncres_cc10.cc
@@ -142,10 +142,10 @@ BOOST_AUTO_TEST_CASE(test_outgoing_v6_only_no_AAAA_in_delegation)
 
 BOOST_AUTO_TEST_CASE(test_dnssec_secure_to_insecure_skipped_cut_invalid_ds_denial)
 {
-  std::unique_ptr<SyncRes> sr;
-  initSR(sr, true);
+  std::unique_ptr<SyncRes> resolver;
+  initSR(resolver, true);
 
-  setDNSSECValidation(sr, DNSSECMode::ValidateAll);
+  setDNSSECValidation(resolver, DNSSECMode::ValidateAll);
 
   primeHints();
   const DNSName target("www.sub.powerdns.com.");
@@ -163,7 +163,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_secure_to_insecure_skipped_cut_invalid_ds_denia
 
   size_t queriesCount = 0;
 
-  sr->setAsyncCallback([&](const ComboAddress& address, const DNSName& domain, int type, bool /* doTCP */, bool /* sendRDQuery */, int /* EDNS0Level */, struct timeval* /* now */, boost::optional<Netmask>& /* srcmask */, const ResolveContext& /* context */, LWResult* res, bool* /* chained */) {
+  resolver->setAsyncCallback([&](const ComboAddress& address, const DNSName& domain, int type, bool /* doTCP */, bool /* sendRDQuery */, int /* EDNS0Level */, struct timeval* /* now */, boost::optional<Netmask>& /* srcmask */, const ResolveContext& /* context */, LWResult* res, bool* /* chained */) {
     queriesCount++;
 
     if (type == QType::DS) {
@@ -237,9 +237,9 @@ BOOST_AUTO_TEST_CASE(test_dnssec_secure_to_insecure_skipped_cut_invalid_ds_denia
      to get the NS in cache so we don't learn the zone cut before
      validating */
   vector<DNSRecord> ret;
-  int res = sr->beginResolve(DNSName("nx.powerdns.com."), QType(QType::A), QClass::IN, ret);
+  int res = resolver->beginResolve(DNSName("nx.powerdns.com."), QType(QType::A), QClass::IN, ret);
   BOOST_CHECK_EQUAL(res, RCode::NoError);
-  BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Insecure);
+  BOOST_CHECK_EQUAL(resolver->getValidationState(), vState::Insecure);
   BOOST_REQUIRE_EQUAL(ret.size(), 4U);
   BOOST_CHECK(ret.at(0).d_type == QType::SOA);
   BOOST_CHECK(ret.at(1).d_type == QType::RRSIG);
@@ -249,18 +249,18 @@ BOOST_AUTO_TEST_CASE(test_dnssec_secure_to_insecure_skipped_cut_invalid_ds_denia
 
   /* now we query the sub zone */
   ret.clear();
-  res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret);
+  res = resolver->beginResolve(target, QType(QType::A), QClass::IN, ret);
   BOOST_CHECK_EQUAL(res, RCode::NoError);
-  BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Insecure);
+  BOOST_CHECK_EQUAL(resolver->getValidationState(), vState::Insecure);
   BOOST_REQUIRE_EQUAL(ret.size(), 2U);
   BOOST_CHECK(ret[0].d_type == QType::A);
   BOOST_CHECK_EQUAL(queriesCount, 9U);
 
   /* again, to test the cache */
   ret.clear();
-  res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret);
+  res = resolver->beginResolve(target, QType(QType::A), QClass::IN, ret);
   BOOST_CHECK_EQUAL(res, RCode::NoError);
-  BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Insecure);
+  BOOST_CHECK_EQUAL(resolver->getValidationState(), vState::Insecure);
   BOOST_REQUIRE_EQUAL(ret.size(), 2U);
   BOOST_CHECK(ret[0].d_type == QType::A);
   BOOST_CHECK_EQUAL(queriesCount, 9U);
@@ -1002,6 +1002,107 @@ BOOST_AUTO_TEST_CASE(test_dnssec_bogus_dnskey_loop)
   BOOST_CHECK_EQUAL(queriesCount, 5U);
 }
 
+BOOST_AUTO_TEST_CASE(test_dnssec_bogus_ds_loop)
+{
+  // Test the case where the RRSIG on the name *and* the RRSIG of the NSEC denying the DS is broken.
+  // This sends te zone cut code trying extra hard to find a zone cut into an endless recursion.
+  std::unique_ptr<SyncRes> resolver;
+  initSR(resolver, true, false);
+
+  setDNSSECValidation(resolver, DNSSECMode::ValidateAll);
+  resolver->setQNameMinimization();
+
+  primeHints();
+  const ComboAddress targetAddr("192.0.2.42");
+  testkeysset_t keys;
+
+  auto luaconfsCopy = g_luaconfs.getCopy();
+  luaconfsCopy.dsAnchors.clear();
+  generateKeyMaterial(g_rootdnsname, DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys, luaconfsCopy.dsAnchors);
+  generateKeyMaterial(DNSName("com."), DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys);
+
+  /* Generate key material for "powerdns.com." */
+  auto dcke = DNSCryptoKeyEngine::make(DNSSECKeeper::ECDSA256);
+  dcke->create(dcke->getBits());
+  DNSSECPrivateKey key;
+  key.setKey(std::move(dcke), 257);
+  DSRecordContent drc = makeDSFromDNSKey(DNSName("powerdns.com."), key.getDNSKEY(), DNSSECKeeper::DIGEST_SHA256);
+
+  keys[DNSName("powerdns.com.")] = std::pair<DNSSECPrivateKey, DSRecordContent>(key, drc);
+  g_luaconfs.setState(luaconfsCopy);
+
+  size_t queriesCount = 0;
+
+  resolver->setAsyncCallback([&](const ComboAddress& address, const DNSName& domain, int type, bool /* doTCP */, bool /* sendRDQuery */, int /* EDNS0Level */, struct timeval* /* now */, boost::optional<Netmask>& /* srcmask */, const ResolveContext& /* context */, LWResult* res, bool* /* chained */) {
+    queriesCount++;
+
+    if (type == QType::DNSKEY) {
+      return genericDSAndDNSKEYHandler(res, domain, domain, type, keys);
+    }
+    if (type == QType::DS) {
+      if (domain == DNSName("www.powerdns.com.")) {
+        auto ret = genericDSAndDNSKEYHandler(res, domain, domain, type, keys, true, boost::none, false, false);
+        for (auto& rec : res->d_records) {
+          // We know the NSEC RRSIG for the DS is the only one
+          if (rec.d_name == DNSName("www.powerdns.com") && rec.d_type == QType::RRSIG) {
+            auto ptr = getRR<RRSIGRecordContent>(rec);
+            ((char*)(void*)(ptr->d_signature.data()))[0] ^= 0x42; // NOLINT
+          }
+        }
+        return ret;
+      }
+      return genericDSAndDNSKEYHandler(res, domain, domain, type, keys);
+    }
+    {
+      if (isRootServer(address)) {
+        setLWResult(res, 0, false, false, true);
+        addRecordToLW(res, "com.", QType::NS, "a.gtld-servers.com.", DNSResourceRecord::AUTHORITY, 3600);
+        addDS(DNSName("com."), 300, res->d_records, keys);
+        addRRSIG(keys, res->d_records, DNSName("."), 300);
+        addRecordToLW(res, "a.gtld-servers.com.", QType::A, "192.0.2.1", DNSResourceRecord::ADDITIONAL, 3600);
+        return LWResult::Result::Success;
+      }
+      if (address == ComboAddress("192.0.2.1:53")) {
+        if (domain.isPartOf(DNSName("powerdns.com."))) {
+          setLWResult(res, 0, false, false, true);
+          addRecordToLW(res, DNSName("powerdns.com."), QType::NS, "ns1.powerdns.com.", DNSResourceRecord::AUTHORITY, 3600);
+          addDS(DNSName("powerdns.com."), 300, res->d_records, keys);
+          addRRSIG(keys, res->d_records, DNSName("."), 300);
+          addRecordToLW(res, "ns1.powerdns.com.", QType::A, "192.0.2.2", DNSResourceRecord::ADDITIONAL, 3600);
+          return LWResult::Result::Success;
+        }
+      }
+      else if (address == ComboAddress("192.0.2.2:53")) {
+        if (domain == DNSName("www.powerdns.com.")) {
+          setLWResult(res, 0, true, false, true);
+          addRecordToLW(res, domain, QType::A, targetAddr.toString(), DNSResourceRecord::ANSWER, 3600);
+          addRRSIG(keys, res->d_records, DNSName("powerdns.com."), 300, true);
+          return LWResult::Result::Success;
+        }
+      }
+    }
+
+    return LWResult::Result::Timeout;
+  });
+
+  vector<DNSRecord> ret;
+  int res = resolver->beginResolve(DNSName("www.powerdns.com."), QType(QType::A), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::NoError);
+  BOOST_CHECK_EQUAL(resolver->getValidationState(), vState::BogusNoValidRRSIG);
+  BOOST_REQUIRE_EQUAL(ret.size(), 2U);
+  BOOST_CHECK(ret.at(0).d_type == QType::A);
+  BOOST_CHECK_EQUAL(queriesCount, 6U);
+
+  /* again, to test the cache */
+  ret.clear();
+  res = resolver->beginResolve(DNSName("www.powerdns.com."), QType(QType::A), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::NoError);
+  BOOST_CHECK_EQUAL(resolver->getValidationState(), vState::BogusNoValidRRSIG);
+  BOOST_REQUIRE_EQUAL(ret.size(), 2U);
+  BOOST_CHECK(ret[0].d_type == QType::A);
+  BOOST_CHECK_EQUAL(queriesCount, 6U);
+}
+
 static auto createPID(std::string rem, int tcpsock, uint16_t type, std::string domain, int fd, uint16_t id)
 {
   PacketID pid;