From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Wed, 11 Oct 2017 09:25:04 +0000 (+0200)
Subject: rec: Check that the NSEC covers an ENT when looking for NODATA
X-Git-Tag: rec-4.1.0-rc2~40^2~1
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=00e3fef4defb7ca4090992e9cedc69d2ea8ca3b9;p=pdns

rec: Check that the NSEC covers an ENT when looking for NODATA

Otherwise we might consider that a NSEC record covers a name when it
does not.
---

diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc
index 5c82bb41d..3ce10cf89 100644
--- a/pdns/recursordist/test-syncres_cc.cc
+++ b/pdns/recursordist/test-syncres_cc.cc
@@ -6987,10 +6987,10 @@ BOOST_AUTO_TEST_CASE(test_nsec_denial_nowrap) {
   cspmap_t denialMap;
   denialMap[std::make_pair(DNSName("a.example.org."), QType::NSEC)] = pair;
 
-  dState denialState = getDenial(denialMap, DNSName("b.example.org."), QType::A, false);
+  dState denialState = getDenial(denialMap, DNSName("b.example.org."), QType::A, false, false);
   BOOST_CHECK_EQUAL(denialState, NXDOMAIN);
 
-  denialState = getDenial(denialMap, DNSName("d.example.org."), QType::A, false);
+  denialState = getDenial(denialMap, DNSName("d.example.org."), QType::A, false, false);
   /* let's check that d.example.org. is not denied by this proof */
   BOOST_CHECK_EQUAL(denialState, NODATA);
 }
@@ -7022,10 +7022,10 @@ BOOST_AUTO_TEST_CASE(test_nsec_denial_wrap_case_1) {
   cspmap_t denialMap;
   denialMap[std::make_pair(DNSName("z.example.org."), QType::NSEC)] = pair;
 
-  dState denialState = getDenial(denialMap, DNSName("a.example.org."), QType::A, false);
+  dState denialState = getDenial(denialMap, DNSName("a.example.org."), QType::A, false, false);
   BOOST_CHECK_EQUAL(denialState, NXDOMAIN);
 
-  denialState = getDenial(denialMap, DNSName("d.example.org."), QType::A, false);
+  denialState = getDenial(denialMap, DNSName("d.example.org."), QType::A, false, false);
   /* let's check that d.example.org. is not denied by this proof */
   BOOST_CHECK_EQUAL(denialState, NODATA);
 }
@@ -7057,10 +7057,10 @@ BOOST_AUTO_TEST_CASE(test_nsec_denial_wrap_case_2) {
   cspmap_t denialMap;
   denialMap[std::make_pair(DNSName("y.example.org."), QType::NSEC)] = pair;
 
-  dState denialState = getDenial(denialMap, DNSName("z.example.org."), QType::A, false);
+  dState denialState = getDenial(denialMap, DNSName("z.example.org."), QType::A, false, false);
   BOOST_CHECK_EQUAL(denialState, NXDOMAIN);
 
-  denialState = getDenial(denialMap, DNSName("d.example.org."), QType::A, false);
+  denialState = getDenial(denialMap, DNSName("d.example.org."), QType::A, false, false);
   /* let's check that d.example.org. is not denied by this proof */
   BOOST_CHECK_EQUAL(denialState, NODATA);
 }
@@ -7092,10 +7092,10 @@ BOOST_AUTO_TEST_CASE(test_nsec_denial_only_one_nsec) {
   cspmap_t denialMap;
   denialMap[std::make_pair(DNSName("a.example.org."), QType::NSEC)] = pair;
 
-  dState denialState = getDenial(denialMap, DNSName("b.example.org."), QType::A, false);
+  dState denialState = getDenial(denialMap, DNSName("b.example.org."), QType::A, false, false);
   BOOST_CHECK_EQUAL(denialState, NXDOMAIN);
 
-  denialState = getDenial(denialMap, DNSName("a.example.org."), QType::A, false);
+  denialState = getDenial(denialMap, DNSName("a.example.org."), QType::A, false, false);
   /* let's check that d.example.org. is not denied by this proof */
   BOOST_CHECK_EQUAL(denialState, NODATA);
 }
@@ -7127,7 +7127,7 @@ BOOST_AUTO_TEST_CASE(test_nsec_root_nxd_denial) {
   cspmap_t denialMap;
   denialMap[std::make_pair(DNSName("a."), QType::NSEC)] = pair;
 
-  dState denialState = getDenial(denialMap, DNSName("b."), QType::A, false);
+  dState denialState = getDenial(denialMap, DNSName("b."), QType::A, false, false);
   BOOST_CHECK_EQUAL(denialState, NXDOMAIN);
 }
 
@@ -7167,12 +7167,12 @@ BOOST_AUTO_TEST_CASE(test_nsec_ancestor_nxqtype_denial) {
      owner name regardless of type.
   */
 
-  dState denialState = getDenial(denialMap, DNSName("a."), QType::A, false);
+  dState denialState = getDenial(denialMap, DNSName("a."), QType::A, false, false);
   /* no data means the qname/qtype is not denied, because an ancestor
      delegation NSEC can only deny the DS */
   BOOST_CHECK_EQUAL(denialState, NODATA);
 
-  denialState = getDenial(denialMap, DNSName("a."), QType::DS, true);
+  denialState = getDenial(denialMap, DNSName("a."), QType::DS, true, true);
   BOOST_CHECK_EQUAL(denialState, NXQTYPE);
 }
 
@@ -7213,10 +7213,43 @@ BOOST_AUTO_TEST_CASE(test_nsec_insecure_delegation_denial) {
 
   /* Insecure because the NS is not set, so while it does
      denies the DS, it can't prove an insecure delegation */
-  dState denialState = getDenial(denialMap, DNSName("a."), QType::DS, true);
+  dState denialState = getDenial(denialMap, DNSName("a."), QType::DS, true, true);
   BOOST_CHECK_EQUAL(denialState, INSECURE);
 }
 
+BOOST_AUTO_TEST_CASE(test_nsec_ent_denial) {
+  init();
+
+  testkeysset_t keys;
+  generateKeyMaterial(DNSName("powerdns.com."), DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys);
+
+  vector<DNSRecord> records;
+
+  vector<shared_ptr<DNSRecordContent>> recordContents;
+  vector<shared_ptr<RRSIGRecordContent>> signatureContents;
+
+  addNSECRecordToLW(DNSName("a.powerdns.com."), DNSName("a.c.powerdns.com."), { QType::A }, 600, records);
+  recordContents.push_back(records.at(0).d_content);
+  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::make_pair(DNSName("a.powerdns.com."), QType::NSEC)] = pair;
+
+  /* this NSEC is not valid to prove a NXQTYPE at b.powerdns.com */
+  dState denialState = getDenial(denialMap, DNSName("b.powerdns.com."), QType::AAAA, true, true);
+  BOOST_CHECK_EQUAL(denialState, NXDOMAIN);
+
+  /* it is valid to prove a NXQTYPE at c.powerdns.com because it proves that
+     it is an ENT */
+  denialState = getDenial(denialMap, DNSName("c.powerdns.com."), QType::AAAA, true, true);
+  BOOST_CHECK_EQUAL(denialState, NXQTYPE);
+}
+
 BOOST_AUTO_TEST_CASE(test_nsec3_ancestor_nxqtype_denial) {
   init();
 
@@ -7253,12 +7286,12 @@ BOOST_AUTO_TEST_CASE(test_nsec3_ancestor_nxqtype_denial) {
      owner name regardless of type.
   */
 
-  dState denialState = getDenial(denialMap, DNSName("a."), QType::A, false);
+  dState denialState = getDenial(denialMap, DNSName("a."), QType::A, false, true);
   /* no data means the qname/qtype is not denied, because an ancestor
      delegation NSEC3 can only deny the DS */
   BOOST_CHECK_EQUAL(denialState, NODATA);
 
-  denialState = getDenial(denialMap, DNSName("a."), QType::DS, true);
+  denialState = getDenial(denialMap, DNSName("a."), QType::DS, true, true);
   BOOST_CHECK_EQUAL(denialState, NXQTYPE);
 }
 
@@ -7299,7 +7332,7 @@ BOOST_AUTO_TEST_CASE(test_nsec3_insecure_delegation_denial) {
 
   /* Insecure because the NS is not set, so while it does
      denies the DS, it can't prove an insecure delegation */
-  dState denialState = getDenial(denialMap, DNSName("a."), QType::DS, true);
+  dState denialState = getDenial(denialMap, DNSName("a."), QType::DS, true, true);
   BOOST_CHECK_EQUAL(denialState, INSECURE);
 }
 
diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index e964ffc83..8caede3ac 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -1929,7 +1929,7 @@ void SyncRes::getDenialValidationState(NegCache::NegCacheEntry& ne, vState& stat
 
   if (state == Secure) {
     cspmap_t csp = harvestCSPFromNE(ne);
-    dState res = getDenial(csp, ne.d_name, ne.d_qtype.getCode(), referralToUnsigned);
+    dState res = getDenial(csp, ne.d_name, ne.d_qtype.getCode(), referralToUnsigned, expectedState == NXQTYPE);
     if (res != expectedState) {
       if (res == OPTOUT && allowOptOut) {
         LOG(d_prefix<<"OPT-out denial found for "<<ne.d_name<<endl);
@@ -1940,10 +1940,6 @@ void SyncRes::getDenialValidationState(NegCache::NegCacheEntry& ne, vState& stat
         LOG(d_prefix<<"Insecure denial found for "<<ne.d_name<<", returning Insecure"<<endl);
         ne.d_validationState = Insecure;
       }
-      if (res == NXDOMAIN && expectedState == NXQTYPE) {
-        /* might happen for empty non-terminal, have fun */
-        return;
-      }
       else {
         LOG(d_prefix<<"Invalid denial found for "<<ne.d_name<<", returning Bogus, res="<<res<<", expectedState="<<expectedState<<endl);
         ne.d_validationState = Bogus;
@@ -2043,7 +2039,7 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co
         harvestNXRecords(lwr.d_records, ne, d_now.tv_sec, &lowestTTL);
 
         cspmap_t csp = harvestCSPFromNE(ne);
-        dState denialState = getDenial(csp, newauth, QType::DS, true);
+        dState denialState = getDenial(csp, newauth, QType::DS, true, true);
         if (denialState == NXQTYPE || denialState == OPTOUT || denialState == INSECURE) {
           ne.d_ttd = lowestTTL + d_now.tv_sec;
           ne.d_validationState = Secure;
diff --git a/pdns/validate.cc b/pdns/validate.cc
index a84600ed3..790031d87 100644
--- a/pdns/validate.cc
+++ b/pdns/validate.cc
@@ -42,6 +42,15 @@ static bool isCoveredByNSEC(const DNSName& name, const DNSName& begin, const DNS
           (begin == next && name != begin));                        // "we have only 1 NSEC record, LOL!"
 }
 
+static bool nsecProvesENT(const DNSName& name, const DNSName& begin, const DNSName& next)
+{
+  /* if name is an ENT:
+     - begin < name
+     - next is a child of name
+  */
+  return begin.canonCompare(name) && next.isPartOf(name);
+}
+
 static std::string getHashFromNSEC3(const DNSName& qname, const std::shared_ptr<NSEC3RecordContent> nsec3)
 {
   std::string result;
@@ -95,10 +104,7 @@ bool denialProvesNoDelegation(const DNSName& zone, const std::vector<DNSRecord>&
   return false;
 }
 
-// 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)
-dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16_t qtype, bool referralToUnsigned)
+dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16_t qtype, bool referralToUnsigned, bool wantsNoDataProof)
 {
   for(const auto& v : validrrsets) {
     LOG("Do have: "<<v.first.first<<"/"<<DNSRecordContent::NumberToType(v.first.second)<<endl);
@@ -155,6 +161,12 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
 
         /* check if the whole NAME is denied existing */
         if(isCoveredByNSEC(qname, v.first.first, nsec->d_next)) {
+
+          if (wantsNoDataProof && nsecProvesENT(qname, v.first.first, nsec->d_next)) {
+            LOG("Denies existence of type "<<qname<<"/"<<QType(qtype).getName()<<" by proving that "<<qname<<" is an ENT"<<endl);
+            return NXQTYPE;
+          }
+
           LOG("Denies existence of name "<<qname<<"/"<<QType(qtype).getName()<<endl);
           return NXDOMAIN;
         }
@@ -712,7 +724,7 @@ vState getKeysFor(DNSRecordOracle& dro, const DNSName& zone, skeyset_t& keyset)
     auto r = validrrsets.equal_range(make_pair(*(zoneCutIter+1), QType::DS));
     if(r.first == r.second) {
       LOG("No DS for "<<*(zoneCutIter+1)<<", now look for a secure denial"<<endl);
-      dState res = getDenial(validrrsets, *(zoneCutIter+1), QType::DS, true);
+      dState res = getDenial(validrrsets, *(zoneCutIter+1), QType::DS, true, true);
       if (res == INSECURE || res == NXDOMAIN)
         return Bogus;
       if (res == NXQTYPE || res == OPTOUT)
diff --git a/pdns/validate.hh b/pdns/validate.hh
index 85399b52b..b9b0f4444 100644
--- a/pdns/validate.hh
+++ b/pdns/validate.hh
@@ -71,7 +71,7 @@ vState getKeysFor(DNSRecordOracle& dro, const DNSName& zone, skeyset_t& keyset);
 bool getTrustAnchor(const map<DNSName,dsmap_t>& anchors, const DNSName& zone, dsmap_t &res);
 bool haveNegativeTrustAnchor(const map<DNSName,std::string>& negAnchors, const DNSName& zone, std::string& reason);
 void validateDNSKeysAgainstDS(time_t now, const DNSName& zone, const dsmap_t& dsmap, const skeyset_t& tkeys, vector<shared_ptr<DNSRecordContent> >& toSign, const vector<shared_ptr<RRSIGRecordContent> >& sigs, skeyset_t& validkeys);
-dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16_t qtype, bool referralToUnsigned);
+dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16_t qtype, bool referralToUnsigned, bool wantsNoDataProof);
 bool isSupportedDS(const DSRecordContent& ds);
 DNSName getSigner(const std::vector<std::shared_ptr<RRSIGRecordContent> >& signatures);
 bool denialProvesNoDelegation(const DNSName& zone, const std::vector<DNSRecord>& dsrecords);