rec: Fix DNSSEC validation of wildcards expanded onto themselves

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 1b71d3c190b1ae2b27b3b308415aa292421e0dfe..72b67e56d20cd58428ca699e2f6e31e9025b0b82 100644 (file)
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -2162,7 +2162,7 @@ void SyncRes::sanitizeRecords(const std::string& prefix, LWResult& lwr, const DN
   }
 }
 
-RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr, const DNSName& qname, const QType& qtype, const DNSName& auth, bool wasForwarded, const boost::optional<Netmask> ednsmask, vState& state, bool& needWildcardProof, unsigned int& wildcardLabelsCount, bool rdQuery)
+RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr, const DNSName& qname, const QType& qtype, const DNSName& auth, bool wasForwarded, const boost::optional<Netmask> ednsmask, vState& state, bool& needWildcardProof, bool& gatherWildcardProof, unsigned int& wildcardLabelsCount, bool rdQuery)
 {
   bool wasForwardRecurse = wasForwarded && rdQuery;
   tcache_t tcache;
@@ -2190,7 +2190,7 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr
     /* if we have a positive answer synthetized from a wildcard,
        we need to store the corresponding NSEC/NSEC3 records proving
        that the exact name did not exist in the negative cache */
-    if(needWildcardProof) {
+    if(gatherWildcardProof) {
       if (nsecTypes.count(rec.d_type)) {
         authorityRecs.push_back(std::make_shared<DNSRecord>(rec));
       }
@@ -2208,9 +2208,20 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr
            count can be lower than the name's label count if it was
            synthetized from the wildcard. Note that the difference might
            be > 1. */
-        if (rec.d_name == qname && rrsig->d_labels < labelCount) {
-          LOG(prefix<<qname<<": RRSIG indicates the name was synthetized from a wildcard, we need a wildcard proof"<<endl);
-          needWildcardProof = true;
+        if (rec.d_name == qname && isWildcardExpanded(labelCount, rrsig)) {
+          gatherWildcardProof = true;
+          if (!isWildcardExpandedOntoItself(rec.d_name, labelCount, rrsig)) {
+            /* if we have a wildcard expanded onto itself, we don't need to prove
+               that the exact name doesn't exist because it actually does.
+               We still want to gather the corresponding NSEC/NSEC3 records
+               to pass them to our client in case it wants to validate by itself.
+            */
+            LOG(prefix<<qname<<": RRSIG indicates the name was synthetized from a wildcard, we need a wildcard proof"<<endl);
+            needWildcardProof = true;
+          }
+          else {
+            LOG(prefix<<qname<<": RRSIG indicates the name was synthetized from a wildcard expanded onto itself, we need to gather wildcard proof"<<endl);
+          }
           wildcardLabelsCount = rrsig->d_labels;
         }
 
@@ -2483,7 +2494,7 @@ dState SyncRes::getDenialValidationState(const NegCache::NegCacheEntry& ne, cons
   return getDenial(csp, ne.d_name, ne.d_qtype.getCode(), referralToUnsigned, expectedState == NXQTYPE);
 }
 
-bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, const QType& qtype, const DNSName& auth, LWResult& lwr, const bool sendRDQuery, vector<DNSRecord>& ret, set<DNSName>& nsset, DNSName& newtarget, DNSName& newauth, bool& realreferral, bool& negindic, vState& state, const bool needWildcardProof, const unsigned int wildcardLabelsCount)
+bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, const QType& qtype, const DNSName& auth, LWResult& lwr, const bool sendRDQuery, vector<DNSRecord>& ret, set<DNSName>& nsset, DNSName& newtarget, DNSName& newauth, bool& realreferral, bool& negindic, vState& state, const bool needWildcardProof, const bool gatherWildcardProof, const unsigned int wildcardLabelsCount)
 {
   bool done = false;
 
@@ -2554,7 +2565,7 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co
     /* if we have a positive answer synthetized from a wildcard, we need to
        return the corresponding NSEC/NSEC3 records from the AUTHORITY section
        proving that the exact name did not exist */
-    else if(needWildcardProof && (rec.d_type==QType::RRSIG || rec.d_type==QType::NSEC || rec.d_type==QType::NSEC3) && rec.d_place==DNSResourceRecord::AUTHORITY) {
+    else if(gatherWildcardProof && (rec.d_type==QType::RRSIG || rec.d_type==QType::NSEC || rec.d_type==QType::NSEC3) && rec.d_place==DNSResourceRecord::AUTHORITY) {
       ret.push_back(rec); // enjoy your DNSSEC
     }
     // for ANY answers we *must* have an authoritative answer, unless we are forwarding recursively
@@ -2857,8 +2868,9 @@ bool SyncRes::processAnswer(unsigned int depth, LWResult& lwr, const DNSName& qn
   }
 
   bool needWildcardProof = false;
+  bool gatherWildcardProof = false;
   unsigned int wildcardLabelsCount;
-  *rcode = updateCacheFromRecords(depth, lwr, qname, qtype, auth, wasForwarded, ednsmask, state, needWildcardProof, wildcardLabelsCount, sendRDQuery);
+  *rcode = updateCacheFromRecords(depth, lwr, qname, qtype, auth, wasForwarded, ednsmask, state, needWildcardProof, gatherWildcardProof, wildcardLabelsCount, sendRDQuery);
   if (*rcode != RCode::NoError) {
     return true;
   }
@@ -2870,7 +2882,7 @@ bool SyncRes::processAnswer(unsigned int depth, LWResult& lwr, const DNSName& qn
   DNSName newauth;
   DNSName newtarget;
 
-  bool done = processRecords(prefix, qname, qtype, auth, lwr, sendRDQuery, ret, nsset, newtarget, newauth, realreferral, negindic, state, needWildcardProof, wildcardLabelsCount);
+  bool done = processRecords(prefix, qname, qtype, auth, lwr, sendRDQuery, ret, nsset, newtarget, newauth, realreferral, negindic, state, needWildcardProof, gatherWildcardProof, wildcardLabelsCount);
 
   if(done){
     LOG(prefix<<qname<<": status=got results, this level of recursion done"<<endl);

diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index a28ca22c57a34c0a35d5cc362cc07c11ed217551..9295811d041ee7740c8c40fdfd3e01b85fc91362 100644 (file)
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -789,8 +789,8 @@ private:
   vector<ComboAddress> retrieveAddressesForNS(const std::string& prefix, const DNSName& qname, vector<DNSName >::const_iterator& tns, const unsigned int depth, set<GetBestNSAnswer>& beenthere, const vector<DNSName >& rnameservers, NsSet& nameservers, bool& sendRDQuery, bool& pierceDontQuery, bool& flawedNSSet, bool cacheOnly);
 
   void sanitizeRecords(const std::string& prefix, LWResult& lwr, const DNSName& qname, const QType& qtype, const DNSName& auth, bool wasForwarded, bool rdQuery);
-  RCode::rcodes_ updateCacheFromRecords(unsigned int depth, LWResult& lwr, const DNSName& qname, const QType& qtype, const DNSName& auth, bool wasForwarded, const boost::optional<Netmask>, vState& state, bool& needWildcardProof, unsigned int& wildcardLabelsCount, bool sendRDQuery);
-  bool processRecords(const std::string& prefix, const DNSName& qname, const QType& qtype, const DNSName& auth, LWResult& lwr, const bool sendRDQuery, vector<DNSRecord>& ret, set<DNSName>& nsset, DNSName& newtarget, DNSName& newauth, bool& realreferral, bool& negindic, vState& state, const bool needWildcardProof, const unsigned int wildcardLabelsCount);
+  RCode::rcodes_ updateCacheFromRecords(unsigned int depth, LWResult& lwr, const DNSName& qname, const QType& qtype, const DNSName& auth, bool wasForwarded, const boost::optional<Netmask>, vState& state, bool& needWildcardProof, bool& gatherWildcardProof, unsigned int& wildcardLabelsCount, bool sendRDQuery);
+  bool processRecords(const std::string& prefix, const DNSName& qname, const QType& qtype, const DNSName& auth, LWResult& lwr, const bool sendRDQuery, vector<DNSRecord>& ret, set<DNSName>& nsset, DNSName& newtarget, DNSName& newauth, bool& realreferral, bool& negindic, vState& state, const bool needWildcardProof, const bool gatherwildcardProof, const unsigned int wildcardLabelsCount);
 
   bool doSpecialNamesResolve(const DNSName &qname, const QType &qtype, const uint16_t qclass, vector<DNSRecord> &ret);
 

diff --git a/pdns/validate.cc b/pdns/validate.cc
index c9d17c86e204b4880e54c90700c8689a960a587d..ab7fa3d9e25d76286fd0f373d6400ef63ba13e43 100644 (file)
--- a/pdns/validate.cc
+++ b/pdns/validate.cc
@@ -112,6 +112,15 @@ bool denialProvesNoDelegation(const DNSName& zone, const std::vector<DNSRecord>&
    Labels field of the covering RRSIG RR, then the RRset and its
    covering RRSIG RR were created as a result of wildcard expansion."
 */
+bool isWildcardExpanded(unsigned int labelCount, const std::shared_ptr<RRSIGRecordContent>& sign)
+{
+  if (sign && sign->d_labels < labelCount) {
+    return true;
+  }
+
+  return false;
+}
+
 static bool isWildcardExpanded(const DNSName& owner, const std::vector<std::shared_ptr<RRSIGRecordContent> >& signatures)
 {
   if (signatures.empty()) {
@@ -120,13 +129,29 @@ static bool isWildcardExpanded(const DNSName& owner, const std::vector<std::shar
 
   const auto& sign = signatures.at(0);
   unsigned int labelsCount = owner.countLabels();
-  if (sign && sign->d_labels < labelsCount) {
+  return isWildcardExpanded(labelsCount, sign);
+}
+
+bool isWildcardExpandedOntoItself(const DNSName& owner, unsigned int labelCount, const std::shared_ptr<RRSIGRecordContent>& sign)
+{
+  if (owner.isWildcard() && (labelCount - 1) == sign->d_labels) {
+    /* this is a wildcard alright, but it has not been expanded */
     return true;
   }
-
   return false;
 }
 
+static bool isWildcardExpandedOntoItself(const DNSName& owner, const std::vector<std::shared_ptr<RRSIGRecordContent> >& signatures)
+{
+  if (signatures.empty()) {
+    return false;
+  }
+
+  const auto& sign = signatures.at(0);
+  unsigned int labelsCount = owner.countLabels();
+  return isWildcardExpandedOntoItself(owner, labelsCount, sign);
+}
+
 /* if this is a wildcard NSEC, the owner name has been modified
    to match the name. Make sure we use the original '*' form. */
 static DNSName getNSECOwnerName(const DNSName& initialOwner, const std::vector<std::shared_ptr<RRSIGRecordContent> >& signatures)
@@ -381,7 +406,7 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
           /* we know that the name exists (but this qtype doesn't) so except
              if the answer was generated by a wildcard expansion, no wildcard
              could have matched (rfc4035 section 5.4 bullet 1) */
-          if (!isWildcardExpanded(owner, v.second.signatures)) {
+          if (!isWildcardExpanded(owner, v.second.signatures) || isWildcardExpandedOntoItself(owner, v.second.signatures)) {
             needWildcardProof = false;
           }
 
@@ -395,6 +420,7 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
 
         /* check if the whole NAME is denied existing */
         if(isCoveredByNSEC(qname, owner, nsec->d_next)) {
+          LOG(qname<<" is covered ");
           /* if the name is an ENT and we received a NODATA answer,
              we are fine with a NSEC proving that the name does not exist. */
           if (wantsNoDataProof && nsecProvesENT(qname, owner, nsec->d_next)) {
@@ -403,15 +429,19 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
           }
 
           if (!needWildcardProof) {
+            LOG("and we did not need a wildcard proof"<<endl);
             return NXDOMAIN;
           }
 
+          LOG("but we do need a wildcard proof so ");
           if (wantsNoDataProof) {
+            LOG("looking for NODATA proof"<<endl);
             if (provesNoDataWildCard(qname, qtype, validrrsets)) {
               return NXQTYPE;
             }
           }
           else {
+            LOG("looking for NO wildcard proof"<<endl);
             if (provesNoWildCard(qname, qtype, validrrsets)) {
               return NXDOMAIN;
             }

diff --git a/pdns/validate.hh b/pdns/validate.hh
index d58b49206307c4cbac36eb9e2dec721fdc6ddfe8..b30fca1c6e79533b316c744f34ee116a418afcec 100644 (file)
--- a/pdns/validate.hh
+++ b/pdns/validate.hh
@@ -77,3 +77,5 @@ 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);
 bool isRRSIGNotExpired(const time_t now, const shared_ptr<RRSIGRecordContent> sig);
+bool isWildcardExpanded(unsigned int labelCount, const std::shared_ptr<RRSIGRecordContent>& sign);
+bool isWildcardExpandedOntoItself(const DNSName& owner, unsigned int labelCount, const std::shared_ptr<RRSIGRecordContent>& sign);