Merge pull request #6218 from aerique/bugfix/rec-ancestor-delegation-below

rec: Correctly handle ancestor delegation NSEC{,3} for children

diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc
index cd51914836a9458a5e9b15fa977c606fdaf10649..08272004ed3b8982c29273d3d6eb6a396bd64a7f 100644 (file)
--- a/pdns/recursordist/test-syncres_cc.cc
+++ b/pdns/recursordist/test-syncres_cc.cc
@@ -8587,6 +8587,10 @@ BOOST_AUTO_TEST_CASE(test_nsec_ancestor_nxqtype_denial) {
      delegation NSEC can only deny the DS */
   BOOST_CHECK_EQUAL(denialState, NODATA);
 
+  /* it can not be used to deny any RRs below that owner name either */
+  denialState = getDenial(denialMap, DNSName("sub.a."), QType::A, false, false);
+  BOOST_CHECK_EQUAL(denialState, NODATA);
+
   denialState = getDenial(denialMap, DNSName("a."), QType::DS, true, true);
   BOOST_CHECK_EQUAL(denialState, NXQTYPE);
 }
@@ -8850,6 +8854,36 @@ BOOST_AUTO_TEST_CASE(test_nsec3_ancestor_nxqtype_denial) {
 
   denialState = getDenial(denialMap, DNSName("a."), QType::DS, true, true);
   BOOST_CHECK_EQUAL(denialState, NXQTYPE);
+
+  /* it can not be used to deny any RRs below that owner name either */
+  /* Add NSEC3 for the next closer */
+  recordContents.clear();
+  signatureContents.clear();
+  records.clear();
+  addNSEC3NarrowRecordToLW(DNSName("sub.a."), DNSName("."), { QType::A, QType::TXT, QType::RRSIG, QType::NSEC3 }, 600, records);
+  recordContents.push_back(records.at(0).d_content);
+  addRRSIG(keys, records, DNSName("."), 300);
+  signatureContents.push_back(getRR<RRSIGRecordContent>(records.at(1)));
+
+  pair.records = recordContents;
+  pair.signatures = signatureContents;
+  denialMap[std::make_pair(records.at(0).d_name, records.at(0).d_type)] = pair;
+
+  /* add wildcard denial */
+  recordContents.clear();
+  signatureContents.clear();
+  records.clear();
+  addNSEC3NarrowRecordToLW(DNSName("*.a."), DNSName("."), { QType::A, QType::TXT, QType::RRSIG, QType::NSEC3 }, 600, records);
+  recordContents.push_back(records.at(0).d_content);
+  addRRSIG(keys, records, DNSName("."), 300);
+  signatureContents.push_back(getRR<RRSIGRecordContent>(records.at(1)));
+
+  pair.records = recordContents;
+  pair.signatures = signatureContents;
+  denialMap[std::make_pair(records.at(0).d_name, records.at(0).d_type)] = pair;
+
+  denialState = getDenial(denialMap, DNSName("sub.a."), QType::A, false, true);
+  BOOST_CHECK_EQUAL(denialState, NODATA);
 }
 
 BOOST_AUTO_TEST_CASE(test_nsec3_denial_too_many_iterations) {

diff --git a/pdns/validate.cc b/pdns/validate.cc
index 99258de24720a23a40687fa3880415d65ba6d7a1..2aa29748d5212006d969dccf9615d93713206364 100644 (file)
--- a/pdns/validate.cc
+++ b/pdns/validate.cc
@@ -155,6 +155,20 @@ static DNSName getNSECOwnerName(const DNSName& initialOwner, const std::vector<s
   return result;
 }
 
+static bool isNSECAncestorDelegation(const DNSName& signer, const DNSName& owner, const std::shared_ptr<NSECRecordContent> nsec)
+{
+  return nsec->d_set.count(QType::NS) &&
+    !nsec->d_set.count(QType::SOA) &&
+    signer.countLabels() < owner.countLabels();
+}
+
+static bool isNSEC3AncestorDelegation(const DNSName& signer, const DNSName& owner, const std::shared_ptr<NSEC3RecordContent> nsec3)
+{
+  return nsec3->d_set.count(QType::NS) &&
+    !nsec3->d_set.count(QType::SOA) &&
+    signer.countLabels() < owner.countLabels();
+}
+
 static bool provesNoDataWildCard(const DNSName& qname, const uint16_t qtype, const cspmap_t& validrrsets)
 {
   LOG("Trying to prove that there is no data in wildcard for "<<qname<<"/"<<QType(qtype).getName()<<endl);
@@ -334,21 +348,18 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
            that (original) owner name other than DS RRs, and all RRs below that
            owner name regardless of type.
         */
-        if (nsec->d_set.count(QType::NS) && !nsec->d_set.count(QType::SOA) &&
-            signer.countLabels() < owner.countLabels()) {
+        if (qtype != QType::DS && (qname == owner || qname.isPartOf(owner)) && isNSECAncestorDelegation(signer, owner, nsec)) {
           LOG("type is "<<QType(qtype).getName()<<", NS is "<<std::to_string(nsec->d_set.count(QType::NS))<<", SOA is "<<std::to_string(nsec->d_set.count(QType::SOA))<<", signer is "<<signer<<", owner name is "<<owner<<endl);
           /* this is an "ancestor delegation" NSEC RR */
-          if (qname == owner && qtype != QType::DS) {
-            LOG("An ancestor delegation NSEC RR can only deny the existence of a DS"<<endl);
-            continue;
-          }
+          LOG("An ancestor delegation NSEC RR can only deny the existence of a DS"<<endl);
+          return NODATA;
         }
 
         /* check if the type is denied */
         if(qname == owner) {
           if (nsec->d_set.count(qtype)) {
             LOG("Does _not_ deny existence of type "<<QType(qtype).getName()<<endl);
-            continue;
+            return NODATA;
           }
 
           LOG("Denies existence of type "<<QType(qtype).getName()<<endl);
@@ -446,21 +457,18 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
            that (original) owner name other than DS RRs, and all RRs below that
            owner name regardless of type.
         */
-        if (nsec3->d_set.count(QType::NS) && !nsec3->d_set.count(QType::SOA) &&
-            signer.countLabels() < v.first.first.countLabels()) {
+        if (qtype != QType::DS && beginHash == h && isNSEC3AncestorDelegation(signer, v.first.first, nsec3)) {
           LOG("type is "<<QType(qtype).getName()<<", NS is "<<std::to_string(nsec3->d_set.count(QType::NS))<<", SOA is "<<std::to_string(nsec3->d_set.count(QType::SOA))<<", signer is "<<signer<<", owner name is "<<v.first.first<<endl);
           /* this is an "ancestor delegation" NSEC3 RR */
-          if (beginHash == h && qtype != QType::DS) {
-            LOG("An ancestor delegation NSEC3 RR can only deny the existence of a DS"<<endl);
-            continue;
-          }
+          LOG("An ancestor delegation NSEC3 RR can only deny the existence of a DS"<<endl);
+          return NODATA;
         }
 
         // If the name exists, check if the qtype is denied
         if(beginHash == h) {
           if (nsec3->d_set.count(qtype)) {
             LOG("Does _not_ deny existence of type "<<QType(qtype).getName()<<" for name "<<qname<<" (not opt-out)."<<endl);
-            continue;
+            return NODATA;
           }
 
           LOG("Denies existence of type "<<QType(qtype).getName()<<" for name "<<qname<<" (not opt-out)."<<endl);
@@ -513,6 +521,12 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
             if(!nsec3)
               continue;
 
+            const DNSName signer = getSigner(v.second.signatures);
+            if (!v.first.first.isPartOf(signer)) {
+              LOG("Owner "<<v.first.first<<" is not part of the signer "<<signer<<", ignoring"<<endl);
+              continue;
+            }
+
             string h = getHashFromNSEC3(closestEncloser, nsec3);
             if (h.empty()) {
               return INSECURE;
@@ -522,6 +536,11 @@ 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)) {
+                LOG("An ancestor delegation NSEC3 RR can only deny the existence of a DS"<<endl);
+                continue;
+              }
+
               LOG("Closest encloser for "<<qname<<" is "<<closestEncloser<<endl);
               found = true;
               break;