rec: Skip NSEC records signed by a subzone when validating a denial p…

…roof
PowerDNS · May 17, 2024 · 3d975ec · 3d975ec
1 parent c556a37
commit 3d975ec
Show file tree

Hide file tree

Showing 2 changed files with 107 additions and 2 deletions.
diff --git a/pdns/recursordist/test-syncres_cc8.cc b/pdns/recursordist/test-syncres_cc8.cc
@@ -777,6 +777,99 @@ BOOST_AUTO_TEST_CASE(test_nsec_ent_denial)
   BOOST_CHECK_EQUAL(denialState, dState::NODENIAL);
 }
 
+BOOST_AUTO_TEST_CASE(test_nsec_denial_invalid_signer)
+{
+  initSR();
+
+  testkeysset_t keys;
+  generateKeyMaterial(DNSName("powerdns.com."), DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys);
+  generateKeyMaterial(DNSName("sub.powerdns.com."), DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys);
+
+  vector<DNSRecord> records;
+
+  sortedRecords_t recordContents;
+  vector<shared_ptr<const RRSIGRecordContent>> signatureContents;
+
+  addNSECRecordToLW(DNSName("sub.powerdns.com."), DNSName("z.powerdns.com."), {QType::SOA, QType::A}, 600, records);
+  recordContents.insert(records.at(0).getContent());
+  addRRSIG(keys, records, DNSName("sub.powerdns.com."), 300);
+  signatureContents.push_back(getRR<RRSIGRecordContent>(records.at(1)));
+  records.clear();
+  addNSECRecordToLW(DNSName(").powerdns.com."), DNSName("+.powerdns.com."), {}, 600, records);
+  recordContents.insert(records.at(0).getContent());
+  addRRSIG(keys, records, DNSName("powerdns.com."), 300);
+  signatureContents.push_back(getRR<RRSIGRecordContent>(records.at(1)));
+  records.clear();
+
+  ContentSigPair pair;
+  pair.records = recordContents;
+  pair.signatures = signatureContents;
+  cspmap_t denialMap;
+  denialMap[std::pair(DNSName("sub.powerdns.com."), QType::NSEC)] = pair;
+
+  /* this NSEC cannot prove that sub2 does not exist, because it is signed
+     by the child side of sub.powerdns.com but tries to refute the existence
+     of a name in the parent zone */
+  dState denialState = getDenial(denialMap, DNSName("sub2.powerdns.com."), QType::A, false, false, std::nullopt);
+  BOOST_CHECK_EQUAL(denialState, dState::NODENIAL);
+}
+
+BOOST_AUTO_TEST_CASE(test_nsec3_denial_invalid_signer)
+{
+  initSR();
+
+  testkeysset_t keys;
+  generateKeyMaterial(DNSName("example.org."), DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys);
+  generateKeyMaterial(DNSName("sub.example.org."), DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys);
+
+  vector<DNSRecord> records;
+
+  sortedRecords_t recordContents;
+  vector<shared_ptr<const RRSIGRecordContent>> signatureContents;
+
+  /* proves that sub2.example.com does not exist */
+  addNSEC3NarrowRecordToLW(DNSName("sub2.example.org"), DNSName("example.org."), {QType::A, QType::TXT, QType::RRSIG, QType::NSEC3}, 600, records);
+  recordContents.insert(records.at(0).getContent());
+  /* but is signed by a subzone */
+  addRRSIG(keys, records, DNSName("sub.example.org."), 300);
+  signatureContents.push_back(getRR<RRSIGRecordContent>(records.at(1)));
+
+  ContentSigPair pair;
+  pair.records = recordContents;
+  pair.signatures = signatureContents;
+  cspmap_t denialMap;
+  denialMap[std::pair(records.at(0).d_name, records.at(0).d_type)] = pair;
+
+  /* Add NSEC3 for the closest encloser */
+  recordContents.clear();
+  signatureContents.clear();
+  records.clear();
+  addNSEC3UnhashedRecordToLW(DNSName("example.org."), DNSName("example.org."), "whatever", {QType::A, QType::TXT, QType::RRSIG, QType::NSEC}, 600, records);
+  recordContents.insert(records.at(0).getContent());
+  addRRSIG(keys, records, DNSName("example.org."), 300);
+  signatureContents.push_back(getRR<RRSIGRecordContent>(records.at(1)));
+
+  pair.records = recordContents;
+  pair.signatures = signatureContents;
+  denialMap[std::pair(records.at(0).d_name, records.at(0).d_type)] = pair;
+
+  /* add wildcard, without the type */
+  recordContents.clear();
+  signatureContents.clear();
+  records.clear();
+  addNSEC3UnhashedRecordToLW(DNSName("*.example.org."), DNSName("example.org"), "whatever", {QType::AAAA}, 600, records);
+  recordContents.insert(records.at(0).getContent());
+  addRRSIG(keys, records, DNSName("example.org."), 300);
+  signatureContents.push_back(getRR<RRSIGRecordContent>(records.at(1)));
+
+  pair.records = recordContents;
+  pair.signatures = signatureContents;
+  denialMap[std::pair(records.at(0).d_name, records.at(0).d_type)] = pair;
+
+  dState denialState = getDenial(denialMap, DNSName("sub2.example.org."), QType::A, false, false);
+  BOOST_CHECK_EQUAL(denialState, dState::NODENIAL);
+}
+
 BOOST_AUTO_TEST_CASE(test_nsec3_ancestor_nxqtype_denial)
 {
   initSR();

diff --git a/pdns/validate.cc b/pdns/validate.cc
@@ -388,7 +388,7 @@ static bool provesNSEC3NoWildCard(const DNSName& closestEncloser, uint16_t const
         }
 
         const DNSName signer = getSigner(validset.second.signatures);
-        if (!validset.first.first.isPartOf(signer)) {
+        if (!validset.first.first.isPartOf(signer) || !closestEncloser.isPartOf(signer)) {
           continue;
         }
 
@@ -547,7 +547,7 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
 
         const DNSName owner = getNSECOwnerName(validset.first.first, validset.second.signatures);
         const DNSName signer = getSigner(validset.second.signatures);
-        if (!validset.first.first.isPartOf(signer) || !owner.isPartOf(signer) ) {
+        if (!validset.first.first.isPartOf(signer) || !owner.isPartOf(signer) || !qname.isPartOf(signer)) {
            continue;
         }
 
@@ -698,6 +698,10 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
         }
         numberOfLabelsOfParentZone = std::min(numberOfLabelsOfParentZone, static_cast<uint8_t>(signer.countLabels()));
 
+        if (!qname.isPartOf(signer)) {
+          continue;
+        }
+
         if (qtype == QType::DS && !qname.isRoot() && signer == qname) {
           VLOG(log, qname << ": A NSEC3 RR from the child zone cannot deny the existence of a DS"<<endl);
           continue;
@@ -805,6 +809,10 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
               continue;
             }
 
+            if (!closestEncloser.isPartOf(signer)) {
+              continue;
+            }
+
             if (g_maxNSEC3sPerRecordToConsider > 0 && nsec3sConsidered >= g_maxNSEC3sPerRecordToConsider) {
               VLOG(log, qname << ": Too many NSEC3s for this record"<<endl);
               context.d_limitHit = true;
@@ -911,6 +919,10 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
               continue;
             }
 
+            if (!nextCloser.isPartOf(signer)) {
+              continue;
+            }
+
             string beginHash=fromBase32Hex(validset.first.first.getRawLabels()[0]);
 
             VLOG(log, qname << ": Comparing "<<toBase32Hex(hash)<<" against "<<toBase32Hex(beginHash)<<" -> "<<toBase32Hex(nsec3->d_nexthash)<<endl);