From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Mon, 25 May 2020 14:02:21 +0000 (+0200)
Subject: rec: Fix the handling of DS queries for the root
X-Git-Tag: dnsdist-1.5.0-rc3~29^2~2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=16dde22c73c195d5e5a4a8bc4483627e90a7643b;p=thirdparty%2Fpdns.git

rec: Fix the handling of DS queries for the root
---

diff --git a/pdns/recursordist/test-syncres_cc6.cc b/pdns/recursordist/test-syncres_cc6.cc
index ca0852f9c8..e5b1219492 100644
--- a/pdns/recursordist/test-syncres_cc6.cc
+++ b/pdns/recursordist/test-syncres_cc6.cc
@@ -238,6 +238,56 @@ BOOST_AUTO_TEST_CASE(test_dnssec_ds_sign_loop)
   BOOST_CHECK_EQUAL(queriesCount, 8U);
 }
 
+BOOST_AUTO_TEST_CASE(test_dnssec_ds_root)
+{
+  std::unique_ptr<SyncRes> sr;
+  initSR(sr, true);
+
+  setDNSSECValidation(sr, DNSSECMode::ValidateAll);
+
+  primeHints();
+  const DNSName target(".");
+  testkeysset_t keys;
+
+  auto luaconfsCopy = g_luaconfs.getCopy();
+  luaconfsCopy.dsAnchors.clear();
+  generateKeyMaterial(g_rootdnsname, DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, 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 (type == QType::DS) {
+      setLWResult(res, 0, true, false, true);
+      addRecordToLW(res, ".", QType::SOA, "a.root-servers.net. nstld.verisign-grs.com. 2017032301 10800 3600 604800 3600", DNSResourceRecord::AUTHORITY, 3600);
+      addRRSIG(keys, res->d_records, DNSName("."), 300);
+      addNSECRecordToLW(domain, DNSName("aaa."), { QType::DNSKEY, QType::SOA, QType::NS, QType::NSEC, QType::RRSIG }, 600, res->d_records);
+      addRRSIG(keys, res->d_records, DNSName("."), 300);
+      return 1;
+    }
+
+    return 0;
+  });
+
+  vector<DNSRecord> ret;
+  int res = sr->beginResolve(target, QType(QType::DS), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::NoError);
+  BOOST_CHECK_EQUAL(sr->getValidationState(), Secure);
+  BOOST_REQUIRE_EQUAL(ret.size(), 4U);
+  BOOST_CHECK_EQUAL(queriesCount, 1U);
+
+  /* again, to test the cache */
+  ret.clear();
+  res = sr->beginResolve(target, QType(QType::DS), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::NoError);
+  BOOST_CHECK_EQUAL(sr->getValidationState(), Secure);
+  BOOST_REQUIRE_EQUAL(ret.size(), 4U);
+  BOOST_CHECK_EQUAL(queriesCount, 1U);
+}
+
 BOOST_AUTO_TEST_CASE(test_dnssec_dnskey_signed_child)
 {
   /* check that we don't accept a signer below us */
diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 3fde0ef257..944222f779 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -2341,8 +2341,10 @@ vState SyncRes::validateRecordsWithSigs(unsigned int depth, const DNSName& qname
     if (!signer.empty() && name.isPartOf(signer)) {
       if ((qtype == QType::DNSKEY || qtype == QType::DS) && signer == qname) {
         /* we are already retrieving those keys, sorry */
-        if (qtype == QType::DS) {
-          /* something is very wrong */
+        if (qtype == QType::DS && !signer.isRoot()) {
+          /* Unless we are getting the DS of the root zone, we should never see a 
+             DS (or a denial of a DS) signed by the DS itself, since we should be
+             requesting it from the parent zone. Something is very wrong */
           LOG(d_prefix<<"The DS for "<<qname<<" is signed by itself, going Bogus"<<endl);
           return Bogus;
         }