From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Tue, 9 May 2017 14:19:09 +0000 (+0200)
Subject: rec: Check NSEC3 closest encloser
X-Git-Tag: rec-4.1.0-alpha1~50^2~25
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=812c3a693593a96e6108da97c04cafb1245f7b65;p=pdns

rec: Check NSEC3 closest encloser
---

diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc
index cbb1687b1..fc2e4fbec 100644
--- a/pdns/recursordist/test-syncres_cc.cc
+++ b/pdns/recursordist/test-syncres_cc.cc
@@ -268,7 +268,7 @@ static void computeRRSIG(const DNSSECPrivateKey& dpk, const DNSName& signer, con
   rrc.d_type = signQType;
   rrc.d_labels = signQName.countLabels() - signQName.isWildcard();
   rrc.d_originalttl = signTTL;
-  rrc.d_siginception = now - 1;
+  rrc.d_siginception = now - 10;
   rrc.d_sigexpire = now + sigValidity;
   rrc.d_signer = signer;
   rrc.d_tag = 0;
diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 72e67709b..60b4f4998 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -1393,7 +1393,7 @@ vState SyncRes::getValidationStatus(const DNSName& subdomain, unsigned int depth
   return Secure;
 }
 
-void SyncRes::updateValidationStatusAfterReferral(const DNSName& newauth, vState& state, unsigned int depth)
+void SyncRes::updateValidationStatusAfterReferral(const DNSName& oldauth, const DNSName& newauth, vState& state, unsigned int depth)
 {
   if (!validationEnabled()) {
     return;
@@ -1790,7 +1790,8 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co
         if (denialState == NXQTYPE || denialState == OPTOUT) {
           ne.d_validationState = Secure;
           rec.d_ttl = min(s_maxnegttl, rec.d_ttl);
-          LOG(prefix<<qname<<": got negative indication of DS record for '"<<newauth<<endl);
+          LOG(prefix<<qname<<": got negative indication of DS record for '"<<newauth<<"'"<<endl);
+          updateValidationState(state, Insecure);
           if(!wasVariable()) {
             t_sstorage.negcache.add(ne);
           }
@@ -2026,7 +2027,6 @@ bool SyncRes::processAnswer(unsigned int depth, LWResult& lwr, const DNSName& qn
 
   if(realreferral) {
     LOG(prefix<<qname<<": status=did not resolve, got "<<(unsigned int)nsset.size()<<" NS, ");
-    auth=newauth;
 
     nameservers.clear();
     for (auto const &nameserver : nsset) {
@@ -2042,7 +2042,8 @@ bool SyncRes::processAnswer(unsigned int depth, LWResult& lwr, const DNSName& qn
     }
     LOG("looping to them"<<endl);
     *gotNewServers = true;
-    updateValidationStatusAfterReferral(newauth, state, depth);
+    updateValidationStatusAfterReferral(auth, newauth, state, depth);
+    auth=newauth;
 
     return false;
   }
diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index b1b3c0c05..f4c11c791 100644
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -734,7 +734,7 @@ private:
   bool validationEnabled() const;
   uint32_t computeLowestTTD(const std::vector<DNSRecord>& records, const std::vector<std::shared_ptr<RRSIGRecordContent> >& signatures, uint32_t signaturesTTL) const;
   void updateValidationState(vState& state, const vState stateUpdate);
-  void updateValidationStatusAfterReferral(const DNSName& newauth, vState& state, unsigned int depth);
+  void updateValidationStatusAfterReferral(const DNSName& oldauth, const DNSName& newauth, vState& state, unsigned int depth);
   vState validateRecordsWithSigs(unsigned int depth, const DNSName& name, const std::vector<DNSRecord>& records, const std::vector<std::shared_ptr<RRSIGRecordContent> >& signatures);
   vState validateDNSKeys(const DNSName& zone, const std::vector<DNSRecord>& dnskeys, const std::vector<std::shared_ptr<RRSIGRecordContent> >& signatures, unsigned int depth);
   vState getDSRecords(const DNSName& zone, dsmap_t& ds, bool onlyTA, unsigned int depth);
diff --git a/pdns/validate.cc b/pdns/validate.cc
index f5b6de0e6..fe6661ee0 100644
--- a/pdns/validate.cc
+++ b/pdns/validate.cc
@@ -24,6 +24,14 @@ static vector<shared_ptr<DNSKEYRecordContent > > getByTag(const skeyset_t& keys,
   return ret;
 }
 
+static bool isCoveredByNSEC3Hash(const std::string& h, const std::string& beginHash, const std::string& nextHash)
+{
+  return ((beginHash < h && h < nextHash) ||          // no wrap          BEGINNING --- HASH -- END
+          (nextHash > h  && beginHash > nextHash) ||  // wrap             HASH --- END --- BEGINNING
+          (nextHash < beginHash  && beginHash < h) || // wrap other case  END --- BEGINNING --- HASH
+          beginHash == nextHash);                     // "we have only 1 NSEC3 record, LOL!"
+}
+
 // FIXME: needs a zone argument, to avoid things like 6840 4.1
 // FIXME: Add ENT support
 // FIXME: Make usable for non-DS records and hook up to validateRecords (or another place)
@@ -75,7 +83,7 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
           continue;
 
         string h = hashQNameWithSalt(nsec3->d_salt, nsec3->d_iterations, qname);
-        //              cerr<<"Salt length: "<<nsec3->d_salt.length()<<", iterations: "<<nsec3->d_iterations<<", hashed: "<<*(zoneCutIter+1)<<endl;
+        //              cerr<<"Salt length: "<<nsec3->d_salt.length()<<", iterations: "<<nsec3->d_iterations<<", hashed: "<<qname<<endl;
         LOG("\tquery hash: "<<toBase32Hex(h)<<endl);
         string beginHash=fromBase32Hex(v.first.first.getRawLabels()[0]);
 
@@ -98,11 +106,7 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
         }
 
         /* check if the whole NAME does not exist */
-        if( ((beginHash < h && h < nsec3->d_nexthash) ||                   // no wrap          BEGINNING --- HASH -- END
-              (nsec3->d_nexthash > h  && beginHash > nsec3->d_nexthash) || // wrap             HASH --- END --- BEGINNING
-              (nsec3->d_nexthash < beginHash  && beginHash < h) ||         // wrap other case  END --- BEGINNING --- HASH
-              beginHash == nsec3->d_nexthash))                             // "we have only 1 NSEC3 record, LOL!"
-        {
+        if(isCoveredByNSEC3Hash(h, beginHash, nsec3->d_nexthash)) {
           LOG("Denies existence of name "<<qname<<"/"<<QType(qtype).getName());
           if (qtype == QType::DS && nsec3->d_flags & 1) {
             LOG(" but is opt-out!"<<endl);
@@ -116,6 +120,73 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
       }
     }
   }
+
+  /* check closest encloser */
+  LOG("Now looking for the closest encloser for "<<qname<<endl);
+  DNSName sname(qname);
+  bool found = false;
+
+  while (found == false && sname.chopOff()) {
+    for(const auto& v : validrrsets) {
+      if(v.first.second==QType::NSEC3) {
+        for(const auto& r : v.second.records) {
+          LOG("\t"<<r->getZoneRepresentation()<<endl);
+          auto nsec3 = std::dynamic_pointer_cast<NSEC3RecordContent>(r);
+          if(!nsec3)
+            continue;
+
+          string h = hashQNameWithSalt(nsec3->d_salt, nsec3->d_iterations, sname);
+          string beginHash=fromBase32Hex(v.first.first.getRawLabels()[0]);
+
+          LOG("Comparing "<<toBase32Hex(h)<<" against "<<toBase32Hex(beginHash)<<endl);
+          if(beginHash == h) {
+            LOG("Closest encloser for "<<qname<<" is "<<sname<<endl);
+            found = true;
+            break;
+          }
+        }
+      }
+      if (found == true) {
+        break;
+      }
+    }
+  }
+
+  if (found == true) {
+    /* we now need a NSEC3 RR covering the next closer name */
+    unsigned int labelIdx = qname.countLabels() - sname.countLabels();
+    if (labelIdx >= 1) {
+      DNSName nextCloser(sname);
+      nextCloser.prependRawLabel(qname.getRawLabel(labelIdx - 1));
+      LOG("Looking for a NSEC3 covering the next closer name "<<nextCloser<<endl);
+
+      for(const auto& v : validrrsets) {
+        if(v.first.second==QType::NSEC3) {
+          for(const auto& r : v.second.records) {
+            LOG("\t"<<r->getZoneRepresentation()<<endl);
+            auto nsec3 = std::dynamic_pointer_cast<NSEC3RecordContent>(r);
+            if(!nsec3)
+              continue;
+
+            string h = hashQNameWithSalt(nsec3->d_salt, nsec3->d_iterations, nextCloser);
+            string beginHash=fromBase32Hex(v.first.first.getRawLabels()[0]);
+
+            LOG("Comparing "<<toBase32Hex(h)<<" against "<<toBase32Hex(beginHash)<<endl);
+            if(isCoveredByNSEC3Hash(h, beginHash, nsec3->d_nexthash)) {
+              LOG("Denies existence of name "<<qname<<"/"<<QType(qtype).getName());
+              if (qtype == QType::DS && nsec3->d_flags & 1) {
+                LOG(" but is opt-out!"<<endl);
+                return OPTOUT;
+              }
+              LOG(endl);
+              return NXDOMAIN;
+            }
+          }
+        }
+      }
+    }
+  }
+
   // There were no valid NSEC(3) records
   // XXX maybe this should be INSECURE... it depends on the semantics of this function
   return NODATA;
@@ -166,7 +237,7 @@ static bool checkSignatureWithKey(time_t now, const shared_ptr<RRSIGRecordConten
       LOG("signature by key with tag "<<sig->d_tag<<" was " << (result ? "" : "NOT ")<<"valid"<<endl);
     }
     else {
-      LOG("Signature is "<<((sig->d_siginception >= now) ? "not yet valid" : "expired")<<endl);
+      LOG("Signature is "<<((sig->d_siginception >= now) ? "not yet valid" : "expired")<<" (inception: "<<sig->d_siginception<<", expiration: "<<sig->d_sigexpire<<", now: "<<now<<")"<<endl);
     }
   }
   catch(std::exception& e) {