From f37a904f4cc53ccb74b5904948b819920401ed24 Mon Sep 17 00:00:00 2001
From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Fri, 21 Jan 2022 14:42:18 +0100
Subject: [PATCH] rec: The NSEC3 ancestor check must be done against the
 original owner name

---
 pdns/validate.cc | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/pdns/validate.cc b/pdns/validate.cc
index a35e1148c2..db6eff87d6 100644
--- a/pdns/validate.cc
+++ b/pdns/validate.cc
@@ -653,10 +653,10 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
           continue;
         }
 
-        const DNSName& owner = v.first.first;
+        const DNSName& hashedOwner = v.first.first;
         const DNSName signer = getSigner(v.second.signatures);
-        if (!owner.isPartOf(signer)) {
-          LOG("Owner "<<owner<<" is not part of the signer "<<signer<<", ignoring"<<endl);
+        if (!hashedOwner.isPartOf(signer)) {
+          LOG("Owner "<<hashedOwner<<" is not part of the signer "<<signer<<", ignoring"<<endl);
           continue;
         }
 
@@ -665,16 +665,6 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
           continue;
         }
 
-        /* The NSEC is either a delegation one, from the parent zone, and
-         * must have the NS bit set but not the SOA one, or a regular NSEC
-         * either at apex (signer == owner) or with the SOA or NS bits clear.
-         */
-        const bool notApex = signer.countLabels() < owner.countLabels();
-        if (notApex && nsec3->isSet(QType::NS) && nsec3->isSet(QType::SOA)) {
-          LOG("However, that NSEC3 is not at the apex and has both the NS and the SOA bits set!"<<endl);
-          continue;
-        }
-
         string h = getHashFromNSEC3(qname, nsec3, cache);
         if (h.empty()) {
           LOG("Unsupported hash, ignoring"<<endl);
@@ -684,18 +674,28 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
         nsec3Seen = true;
 
         LOG("\tquery hash: "<<toBase32Hex(h)<<endl);
-        string beginHash=fromBase32Hex(owner.getRawLabels()[0]);
+        string beginHash = fromBase32Hex(hashedOwner.getRawLabels()[0]);
 
         // If the name exists, check if the qtype is denied
         if (beginHash == h) {
 
+          /* The NSEC is either a delegation one, from the parent zone, and
+           * must have the NS bit set but not the SOA one, or a regular NSEC
+           * either at apex (signer == owner) or with the SOA or NS bits clear.
+           */
+          const bool notApex = signer.countLabels() < qname.countLabels();
+          if (notApex && nsec3->isSet(QType::NS) && nsec3->isSet(QType::SOA)) {
+            LOG("However, that NSEC3 is not at the apex and has both the NS and the SOA bits set!"<<endl);
+            continue;
+          }
+
           /* RFC 6840 section 4.1 "Clarifications on Nonexistence Proofs":
              Ancestor delegation NSEC or NSEC3 RRs MUST NOT be used to assume
              nonexistence of any RRs below that zone cut, which include all RRs at
              that (original) owner name other than DS RRs, and all RRs below that
              owner name regardless of type.
           */
-          if (qtype != QType::DS && isNSEC3AncestorDelegation(signer, owner, nsec3)) {
+          if (qtype != QType::DS && isNSEC3AncestorDelegation(signer, qname, nsec3)) {
             /* this is an "ancestor delegation" NSEC3 RR */
             LOG("An ancestor delegation NSEC3 RR can only deny the existence of a DS"<<endl);
             return dState::NODENIAL;
-- 
2.47.3