rec: Be more strict when validating DS wrt parent/child NSEC(3)s

The existing code would not have accepted that proof anyway since
the NSEC(3) are signed by a DNSKEY that would require fetching the
corresponding DS, which causes a loop that would be detected, but
it is cleaner to actually check this.

diff --git a/pdns/recursordist/test-syncres_cc6.cc b/pdns/recursordist/test-syncres_cc6.cc
index 9810e0161f83bfe3d5a096bbda200c666d198387..9c5458c8ecf4089df5c2c7af21b4dae723e62e5d 100644 (file)
--- a/pdns/recursordist/test-syncres_cc6.cc
+++ b/pdns/recursordist/test-syncres_cc6.cc
@@ -1036,7 +1036,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_secure_to_insecure_optout)
         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, DNSName("com."), 300);
         /* closest encloser */
-        addNSEC3UnhashedRecordToLW(DNSName("com."), DNSName("com."), DNSName("a.com.").toStringNoDot(), {QType::NS}, 600, res->d_records, 10, true);
+        addNSEC3UnhashedRecordToLW(DNSName("com."), DNSName("com."), DNSName("a.com.").toStringNoDot(), {QType::NS,QType::SOA}, 600, res->d_records, 10, true);
         addRRSIG(keys, res->d_records, DNSName("com."), 300);
         /* next closer */
         addNSEC3UnhashedRecordToLW(DNSName("oowerdns.com."), DNSName("com."), DNSName("qowerdns.com.").toStringNoDot(), {QType::NS}, 600, res->d_records, 10, true);
@@ -1081,7 +1081,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_secure_to_insecure_optout)
           addRecordToLW(res, domain, QType::NS, "ns1.powerdns.com.", DNSResourceRecord::AUTHORITY, 3600);
           /* no DS */
           /* closest encloser */
-          addNSEC3UnhashedRecordToLW(DNSName("com."), DNSName("com."), DNSName("a.com.").toStringNoDot(), {QType::NS}, 600, res->d_records, 10, true);
+          addNSEC3UnhashedRecordToLW(DNSName("com."), DNSName("com."), DNSName("a.com.").toStringNoDot(), {QType::NS,QType::SOA}, 600, res->d_records, 10, true);
           addRRSIG(keys, res->d_records, DNSName("com."), 300);
           /* next closer */
           addNSEC3UnhashedRecordToLW(DNSName("oowerdns.com."), DNSName("com."), DNSName("qowerdns.com.").toStringNoDot(), {QType::NS}, 600, res->d_records, 10, true);

diff --git a/pdns/recursordist/test-syncres_cc8.cc b/pdns/recursordist/test-syncres_cc8.cc
index 747f787f19139d1048c926690deebe1394650ac7..9cfee574e5bc3a3c0064d523d3c9a5851e0dc658 100644 (file)
--- a/pdns/recursordist/test-syncres_cc8.cc
+++ b/pdns/recursordist/test-syncres_cc8.cc
@@ -223,7 +223,8 @@ BOOST_AUTO_TEST_CASE(test_nsec_ancestor_nxqtype_denial)
     The RRSIG from "." denies the existence of any type except NS at a.
     However since it's an ancestor delegation NSEC (NS bit set, SOA bit clear,
     signer field that is shorter than the owner name of the NSEC RR) it can't
-    be used to deny anything except the whole name or a DS.
+    be used to deny anything except the whole name (which does not make sense here)
+    or a DS.
   */
   addNSECRecordToLW(DNSName("a."), DNSName("b."), {QType::NS}, 600, records);
   recordContents.insert(records.at(0).d_content);
@@ -244,7 +245,7 @@ BOOST_AUTO_TEST_CASE(test_nsec_ancestor_nxqtype_denial)
      owner name regardless of type.
   */
 
-  dState denialState = getDenial(denialMap, DNSName("a."), QType::A, false, false);
+  dState denialState = getDenial(denialMap, DNSName("a."), QType::A, false, true);
   /* no data means the qname/qtype is not denied, because an ancestor
      delegation NSEC can only deny the DS */
   BOOST_CHECK_EQUAL(denialState, dState::NODENIAL);
@@ -257,6 +258,38 @@ BOOST_AUTO_TEST_CASE(test_nsec_ancestor_nxqtype_denial)
   BOOST_CHECK_EQUAL(denialState, dState::NXQTYPE);
 }
 
+BOOST_AUTO_TEST_CASE(test_nsec_ds_denial_from_child)
+{
+  initSR();
+
+  testkeysset_t keys;
+  generateKeyMaterial(DNSName("org."), DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys);
+  generateKeyMaterial(DNSName("example.org."), DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys);
+
+  vector<DNSRecord> records;
+
+  sortedRecords_t recordContents;
+  vector<shared_ptr<RRSIGRecordContent>> signatureContents;
+
+  addNSECRecordToLW(DNSName("example.org."), DNSName("a.example.org"), {QType::A, QType::TXT, QType::RRSIG, QType::NSEC}, 600, records);
+  recordContents.insert(records.at(0).d_content);
+  addRRSIG(keys, records, DNSName("example.org."), 300);
+  signatureContents.push_back(getRR<RRSIGRecordContent>(records.at(1)));
+  records.clear();
+
+  ContentSigPair pair;
+  pair.records = recordContents;
+  pair.signatures = signatureContents;
+  cspmap_t denialMap;
+  denialMap[std::make_pair(DNSName("example.org."), QType::NSEC)] = pair;
+
+  /* check that this NSEC from the child zone can deny a AAAA at the apex */
+  BOOST_CHECK_EQUAL(getDenial(denialMap, DNSName("example.org."), QType::AAAA, false, true, true), dState::NXQTYPE);
+
+  /* but not that the DS does not exist, since we need the parent for that */
+  BOOST_CHECK_EQUAL(getDenial(denialMap, DNSName("example.org."), QType::DS, false, true, true), dState::NODENIAL);
+}
+
 BOOST_AUTO_TEST_CASE(test_nsec_insecure_delegation_denial)
 {
   initSR();
@@ -328,6 +361,36 @@ BOOST_AUTO_TEST_CASE(test_nsec_nxqtype_cname)
   BOOST_CHECK_EQUAL(denialState, dState::NODENIAL);
 }
 
+BOOST_AUTO_TEST_CASE(test_nsec3_nxqtype_ds)
+{
+  initSR();
+
+  testkeysset_t keys;
+  generateKeyMaterial(DNSName("powerdns.com."), DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys);
+
+  vector<DNSRecord> records;
+
+  sortedRecords_t recordContents;
+  vector<shared_ptr<RRSIGRecordContent>> signatureContents;
+
+  addNSEC3UnhashedRecordToLW(DNSName("powerdns.com."), DNSName("powerdns.com."), "whatever", {QType::A}, 600, records);
+  recordContents.insert(records.at(0).d_content);
+  addRRSIG(keys, records, DNSName("powerdns.com."), 300);
+  signatureContents.push_back(getRR<RRSIGRecordContent>(records.at(1)));
+
+  ContentSigPair pair;
+  pair.records = recordContents;
+  pair.signatures = signatureContents;
+  cspmap_t denialMap;
+  denialMap[std::make_pair(records.at(0).d_name, records.at(0).d_type)] = pair;
+  records.clear();
+
+  /* this NSEC3 is not valid to deny the DS since it is from the child zone */
+  BOOST_CHECK_EQUAL(getDenial(denialMap, DNSName("powerdns.com."), QType::DS, false, true), dState::NODENIAL);
+  /* AAAA should be fine, though */
+  BOOST_CHECK_EQUAL(getDenial(denialMap, DNSName("powerdns.com."), QType::AAAA, false, true), dState::NXQTYPE);
+}
+
 BOOST_AUTO_TEST_CASE(test_nsec3_nxqtype_cname)
 {
   initSR();
@@ -733,6 +796,10 @@ BOOST_AUTO_TEST_CASE(test_nsec3_ancestor_nxqtype_denial)
 
   denialState = getDenial(denialMap, DNSName("sub.a."), QType::A, false, true);
   BOOST_CHECK_EQUAL(denialState, dState::NODENIAL);
+
+  /* not even the DS! */
+  denialState = getDenial(denialMap, DNSName("sub.a."), QType::DS, false, true);
+  BOOST_CHECK_EQUAL(denialState, dState::NODENIAL);
 }
 
 BOOST_AUTO_TEST_CASE(test_nsec3_denial_too_many_iterations)

diff --git a/pdns/validate.cc b/pdns/validate.cc
index 6f04090291fceccea6d00473fad636cce1403780..67631d2204ef848096b0048b20b971942dc2b86f 100644 (file)
--- a/pdns/validate.cc
+++ b/pdns/validate.cc
@@ -521,6 +521,11 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
           }
         }
 
+        if (qtype == QType::DS && !qname.isRoot() && signer == qname) {
+          LOG("A NSEC RR from the child zone cannot deny the existence of a DS"<<endl);
+          continue;
+        }
+
         /* check if the type is denied */
         if (qname == owner) {
           if (!isTypeDenied(nsec, QType(qtype))) {
@@ -637,6 +642,11 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
           continue;
         }
 
+        if (qtype == QType::DS && !qname.isRoot() && signer == qname) {
+          LOG("A NSEC3 RR from the child zone cannot deny the existence of a DS"<<endl);
+          continue;
+        }
+
         string h = getHashFromNSEC3(qname, nsec3, cache);
         if (h.empty()) {
           LOG("Unsupported hash, ignoring"<<endl);
@@ -729,7 +739,8 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
 
             LOG("Comparing "<<toBase32Hex(h)<<" ("<<closestEncloser<<") against "<<toBase32Hex(beginHash)<<endl);
             if (beginHash == h) {
-              if (qtype != QType::DS && isNSEC3AncestorDelegation(signer, v.first.first, nsec3)) {
+              /* If the closest encloser is a delegation NS we know nothing about the names in the child zone. */
+              if (isNSEC3AncestorDelegation(signer, v.first.first, nsec3)) {
                 LOG("An ancestor delegation NSEC3 RR can only deny the existence of a DS"<<endl);
                 continue;
               }