From: Pieter Lexis <pieter.lexis@powerdns.com>
Date: Wed, 31 Aug 2016 12:11:27 +0000 (+0200)
Subject: DNSSEC: Implement keysearch based on zone-cuts
X-Git-Tag: dnsdist-1.1.0-beta2~81^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7226d53b1fd71e016954e505f2c00ae21fea4d88;p=thirdparty%2Fpdns.git

DNSSEC: Implement keysearch based on zone-cuts

This prevents us sending out useless queries for DS records and doesn't
confuse us anymore when the delegation jumps over several labels.
---

diff --git a/pdns/validate.cc b/pdns/validate.cc
index 534844c048..3bda3ca5aa 100644
--- a/pdns/validate.cc
+++ b/pdns/validate.cc
@@ -237,20 +237,22 @@ vState getKeysFor(DNSRecordOracle& dro, const DNSName& zone, keyset_t &keyset)
     }
   }
 
-  vector<string> labels = zone.getRawLabels();
-
-  dsmap_t dsmap;
   keyset_t validkeys;
+  dsmap_t dsmap;
 
-  DNSName qname = lowestTA;
-  vState state = Secure; // the lowest Trust Anchor is secure
+  dsmap_t* tmp = (dsmap_t*) rplookup(luaLocal->dsAnchors, lowestTA);
+  if (tmp)
+    dsmap = *tmp;
 
-  while(zone.isPartOf(qname))
-  {
-    dsmap_t* tmp = (dsmap_t*) rplookup(luaLocal->dsAnchors, qname);
-    if (tmp)
-      dsmap = *tmp;
+  auto zoneCuts = getZoneCuts(zone, lowestTA, dro);
 
+  LOG("Found the following zonecuts:")
+  for(const auto& zonecut : zoneCuts)
+    LOG(" => "<<zonecut);
+  LOG(endl);
+
+  for(auto zoneCutIter = zoneCuts.begin(); zoneCutIter != zoneCuts.end(); ++zoneCutIter)
+  {
     vector<RRSIGRecordContent> sigs;
     vector<shared_ptr<DNSRecordContent> > toSign;
     vector<uint16_t> toSignTags;
@@ -258,23 +260,22 @@ vState getKeysFor(DNSRecordOracle& dro, const DNSName& zone, keyset_t &keyset)
     keyset_t tkeys; // tentative keys
     validkeys.clear();
 
-    // start of this iteration
-    // we can trust that dsmap has valid DS records for qname
-
     //    cerr<<"got DS for ["<<qname<<"], grabbing DNSKEYs"<<endl;
-    auto recs=dro.get(qname, (uint16_t)QType::DNSKEY);
+    auto records=dro.get(*zoneCutIter, (uint16_t)QType::DNSKEY);
     // this should use harvest perhaps
-    for(const auto& rec : recs) {
-      if(rec.d_name != qname)
+    for(const auto& rec : records) {
+      if(rec.d_name != *zoneCutIter)
         continue;
 
       if(rec.d_type == QType::RRSIG)
       {
         auto rrc=getRR<RRSIGRecordContent> (rec);
-        LOG("Got signature: "<<rrc->getZoneRepresentation()<<" with tag "<<rrc->d_tag<<", for type "<<DNSRecordContent::NumberToType(rrc->d_type)<<endl);
-        if(rrc && rrc->d_type != QType::DNSKEY)
-          continue;
-        sigs.push_back(*rrc);
+        if(rrc) {
+          LOG("Got signature: "<<rrc->getZoneRepresentation()<<" with tag "<<rrc->d_tag<<", for type "<<DNSRecordContent::NumberToType(rrc->d_type)<<endl);
+          if(rrc->d_type != QType::DNSKEY)
+            continue;
+          sigs.push_back(*rrc);
+        }
       }
       else if(rec.d_type == QType::DNSKEY)
       {
@@ -282,7 +283,7 @@ vState getKeysFor(DNSRecordOracle& dro, const DNSName& zone, keyset_t &keyset)
         if(drc) {
           tkeys.insert(*drc);
           LOG("Inserting key with tag "<<drc->getTag()<<": "<<drc->getZoneRepresentation()<<endl);
-          //          dotNode("DNSKEY", qname, std::to_string(drc->getTag()), (boost::format("tag=%d, algo=%d") % drc->getTag() % static_cast<int>(drc->d_algorithm)).str());
+          //          dotNode("DNSKEY", *zoneCutIter, std::to_string(drc->getTag()), (boost::format("tag=%d, algo=%d") % drc->getTag() % static_cast<int>(drc->d_algorithm)).str());
 
           toSign.push_back(rec.d_content);
           toSignTags.push_back(drc->getTag());
@@ -291,6 +292,10 @@ vState getKeysFor(DNSRecordOracle& dro, const DNSName& zone, keyset_t &keyset)
     }
     LOG("got "<<tkeys.size()<<" keys and "<<sigs.size()<<" sigs from server"<<endl);
 
+    /*
+     * Check all DNSKEY records against all DS records and place all DNSKEY records
+     * that have DS records (that we support the algo for) in the tentative key storage
+     */
     for(auto const& dsrc : dsmap)
     {
       auto r = getByTag(tkeys, dsrc.d_tag);
@@ -298,28 +303,28 @@ vState getKeysFor(DNSRecordOracle& dro, const DNSName& zone, keyset_t &keyset)
 
       for(const auto& drc : r)
       {
-	bool isValid = false;
-	DSRecordContent dsrc2;
-	try {
-	  dsrc2=makeDSFromDNSKey(qname, drc, dsrc.d_digesttype);
-	  isValid = dsrc == dsrc2;
-	} 
-	catch(std::exception &e) {
-	  LOG("Unable to make DS from DNSKey: "<<e.what()<<endl);
-	}
+        bool isValid = false;
+        DSRecordContent dsrc2;
+        try {
+          dsrc2=makeDSFromDNSKey(*zoneCutIter, drc, dsrc.d_digesttype);
+          isValid = dsrc == dsrc2;
+        }
+        catch(std::exception &e) {
+          LOG("Unable to make DS from DNSKey: "<<e.what()<<endl);
+        }
 
         if(isValid) {
-	  LOG("got valid DNSKEY (it matches the DS) with tag "<<dsrc.d_tag<<" for "<<qname<<endl);
-	  
+          LOG("got valid DNSKEY (it matches the DS) with tag "<<dsrc.d_tag<<" for "<<*zoneCutIter<<endl);
+
           validkeys.insert(drc);
-	  dotNode("DS", qname, "" /*std::to_string(dsrc.d_tag)*/, (boost::format("tag=%d, digest algo=%d, algo=%d") % dsrc.d_tag % static_cast<int>(dsrc.d_digesttype) % static_cast<int>(dsrc.d_algorithm)).str());
+          dotNode("DS", *zoneCutIter, "" /*std::to_string(dsrc.d_tag)*/, (boost::format("tag=%d, digest algo=%d, algo=%d") % dsrc.d_tag % static_cast<int>(dsrc.d_digesttype) % static_cast<int>(dsrc.d_algorithm)).str());
         }
-	else {
-	  LOG("DNSKEY did not match the DS, parent DS: "<<drc.getZoneRepresentation() << " ! = "<<dsrc2.getZoneRepresentation()<<endl);
-	}
-        // cout<<"    subgraph "<<dotEscape("cluster "+qname)<<" { "<<dotEscape("DS "+qname)<<" -> "<<dotEscape("DNSKEY "+qname)<<" [ label = \""<<dsrc.d_tag<<"/"<<static_cast<int>(dsrc.d_digesttype)<<"\" ]; label = \"zone: "<<qname<<"\"; }"<<endl;
-	dotEdge(g_rootdnsname, "DS", qname, "" /*std::to_string(dsrc.d_tag)*/, "DNSKEY", qname, std::to_string(drc.getTag()), isValid ? "green" : "red");
-        // dotNode("DNSKEY", qname, (boost::format("tag=%d, algo=%d") % drc.getTag() % static_cast<int>(drc.d_algorithm)).str());
+        else {
+          LOG("DNSKEY did not match the DS, parent DS: "<<drc.getZoneRepresentation() << " ! = "<<dsrc2.getZoneRepresentation()<<endl);
+        }
+        // cout<<"    subgraph "<<dotEscape("cluster "+*zoneCutIter)<<" { "<<dotEscape("DS "+*zoneCutIter)<<" -> "<<dotEscape("DNSKEY "+*zoneCutIter)<<" [ label = \""<<dsrc.d_tag<<"/"<<static_cast<int>(dsrc.d_digesttype)<<"\" ]; label = \"zone: "<<*zoneCutIter<<"\"; }"<<endl;
+        dotEdge(g_rootdnsname, "DS", *zoneCutIter, "" /*std::to_string(dsrc.d_tag)*/, "DNSKEY", *zoneCutIter, std::to_string(drc.getTag()), isValid ? "green" : "red");
+        // dotNode("DNSKEY", *zoneCutIter, (boost::format("tag=%d, algo=%d") % drc.getTag() % static_cast<int>(drc.d_algorithm)).str());
       }
     }
 
@@ -334,163 +339,152 @@ vState getKeysFor(DNSRecordOracle& dro, const DNSName& zone, keyset_t &keyset)
       // whole set
       for(auto i=sigs.begin(); i!=sigs.end(); i++)
       {
-	//        cerr<<"got sig for keytag "<<i->d_tag<<" matching "<<getByTag(tkeys, i->d_tag).size()<<" keys of which "<<getByTag(validkeys, i->d_tag).size()<<" valid"<<endl;
-        string msg=getMessageForRRSET(qname, *i, toSign);
+        //        cerr<<"got sig for keytag "<<i->d_tag<<" matching "<<getByTag(tkeys, i->d_tag).size()<<" keys of which "<<getByTag(validkeys, i->d_tag).size()<<" valid"<<endl;
+        string msg=getMessageForRRSET(*zoneCutIter, *i, toSign);
         auto bytag = getByTag(validkeys, i->d_tag);
         for(const auto& j : bytag) {
-	  //          cerr<<"validating : ";
+          //          cerr<<"validating : ";
           bool isValid = false;
-	  try {
-	    unsigned int now = time(0);
-	    if(i->d_siginception < now && i->d_sigexpire > now) {
-	      std::shared_ptr<DNSCryptoKeyEngine> dke = shared_ptr<DNSCryptoKeyEngine>(DNSCryptoKeyEngine::makeFromPublicKeyString(j.d_algorithm, j.d_key));
-	      isValid = dke->verify(msg, i->d_signature);
-	    }
+          try {
+            unsigned int now = time(0);
+            if(i->d_siginception < now && i->d_sigexpire > now) {
+              std::shared_ptr<DNSCryptoKeyEngine> dke = shared_ptr<DNSCryptoKeyEngine>(DNSCryptoKeyEngine::makeFromPublicKeyString(j.d_algorithm, j.d_key));
+              isValid = dke->verify(msg, i->d_signature);
+            }
             else {
               LOG("Signature on DNSKEY expired"<<endl);
             }
-	  }
-	  catch(std::exception& e) {
-	    LOG("Could not make a validator for signature: "<<e.what()<<endl);
-	  }
-	  for(uint16_t tag : toSignTags) {
-	    dotEdge(qname,
-		    "DNSKEY", qname, std::to_string(i->d_tag),
-		    "DNSKEY", qname, std::to_string(tag), isValid ? "green" : "red");
-	  }
-	  
+          }
+          catch(std::exception& e) {
+            LOG("Could not make a validator for signature: "<<e.what()<<endl);
+          }
+          for(uint16_t tag : toSignTags) {
+            dotEdge(*zoneCutIter,
+                "DNSKEY", *zoneCutIter, std::to_string(i->d_tag),
+                "DNSKEY", *zoneCutIter, std::to_string(tag), isValid ? "green" : "red");
+          }
+
           if(isValid)
           {
-	    LOG("validation succeeded - whole DNSKEY set is valid"<<endl);
-            // cout<<"    "<<dotEscape("DNSKEY "+stripDot(i->d_signer))<<" -> "<<dotEscape("DNSKEY "+qname)<<";"<<endl;
+            LOG("validation succeeded - whole DNSKEY set is valid"<<endl);
+            // cout<<"    "<<dotEscape("DNSKEY "+stripDot(i->d_signer))<<" -> "<<dotEscape("DNSKEY "+*zoneCutIter)<<";"<<endl;
             validkeys=tkeys;
             break;
           }
-	  else {
-	    LOG("Validation did not succeed!"<<endl);
+          else {
+            LOG("Validation did not succeed!"<<endl);
           }
         }
-	//        if(validkeys.empty()) cerr<<"did not manage to validate DNSKEY set based on DS-validated KSK, only passing KSK on"<<endl;
+        //        if(validkeys.empty()) cerr<<"did not manage to validate DNSKEY set based on DS-validated KSK, only passing KSK on"<<endl;
       }
     }
 
     if(validkeys.empty())
     {
       LOG("ended up with zero valid DNSKEYs, going Bogus"<<endl);
-      state=Bogus;
-      break;
+      return Bogus;
     }
-    LOG("situation: we have one or more valid DNSKEYs for ["<<qname<<"] (want ["<<zone<<"])"<<endl);
-    if(qname == zone) {
+    LOG("situation: we have one or more valid DNSKEYs for ["<<*zoneCutIter<<"] (want ["<<zone<<"])"<<endl);
+
+    if(zoneCutIter == zoneCuts.end()-1) {
       LOG("requested keyset found! returning Secure for the keyset"<<endl);
       keyset.insert(validkeys.begin(), validkeys.end());
       return Secure;
     }
-    //    cerr<<"walking downwards to find DS"<<endl;
-    DNSName keyqname=qname;
-    do {
-      qname=DNSName(labels.back())+qname;
-      labels.pop_back();
-      LOG("next name ["<<qname<<"], trying to get DS"<<endl);
 
-      dsmap_t tdsmap; // tentative DSes
-      dsmap.clear();
-      toSign.clear();
-      toSignTags.clear();
+    // We now have the DNSKEYs, use them to validate the DS records at the next zonecut
+    LOG("next name ["<<*(zoneCutIter+1)<<"], trying to get DS"<<endl);
 
-      auto recs=dro.get(qname, QType::DS);
+    dsmap_t tdsmap; // tentative DSes
+    dsmap.clear();
+    toSign.clear();
+    toSignTags.clear();
 
-      cspmap_t cspmap=harvestCSPFromRecs(recs);
+    auto recs=dro.get(*(zoneCutIter+1), QType::DS);
 
-      cspmap_t validrrsets;
-      validateWithKeySet(cspmap, validrrsets, validkeys);
+    cspmap_t cspmap=harvestCSPFromRecs(recs);
 
-      LOG("got "<<cspmap.count(make_pair(qname,QType::DS))<<" records for DS query of which "<<validrrsets.count(make_pair(qname,QType::DS))<<" valid "<<endl);
+    cspmap_t validrrsets;
+    validateWithKeySet(cspmap, validrrsets, validkeys);
 
-      auto r = validrrsets.equal_range(make_pair(qname, QType::DS));
-      if(r.first == r.second) {
-        LOG("No DS for "<<qname<<", now look for a secure denial"<<endl);
+    LOG("got "<<cspmap.count(make_pair(*(zoneCutIter+1),QType::DS))<<" records for DS query of which "<<validrrsets.count(make_pair(*(zoneCutIter+1),QType::DS))<<" valid "<<endl);
 
-        for(const auto& v : validrrsets) {
-          LOG("Do have: "<<v.first.first<<"/"<<DNSRecordContent::NumberToType(v.first.second)<<endl);
-          if(v.first.second==QType::CNAME) {
-            LOG("Found CNAME for "<< v.first.first << ", ignoring records at this level."<<endl);
-            goto skipLevel;
-          }
-          else if(v.first.second==QType::NSEC) { // check that it covers us!
-            for(const auto& r : v.second.records) {
-              LOG("\t"<<r->getZoneRepresentation()<<endl);
-              auto nsec = std::dynamic_pointer_cast<NSECRecordContent>(r);
-              if(nsec) {
-                if(v.first.first == qname && !nsec->d_set.count(QType::DS)) {
-                  LOG("Denies existence of DS!"<<endl);
-                  return Insecure;
-                }
-                else if(v.first.first.canonCompare(qname) && qname.canonCompare(nsec->d_next) ) {
-                  LOG("Did not find DS for this level, trying one lower"<<endl);
-                  goto skipLevel;
-                }
-                else {
-                  LOG("Did not deny existence of DS, "<<v.first.first<<"?="<<qname<<", "<<nsec->d_set.count(QType::DS)<<", next: "<<nsec->d_next<<endl);
-                }
-              }
-            }
+    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);
 
-          }
-          else 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);
-              string h = hashQNameWithSalt(nsec3->d_salt, nsec3->d_iterations, qname);
-              //              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]);
-              if( (beginHash < h && h < nsec3->d_nexthash) ||
-                  (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!"  
-              {
+      for(const auto& v : validrrsets) {
+        LOG("Do have: "<<v.first.first<<"/"<<DNSRecordContent::NumberToType(v.first.second)<<endl);
+        if(v.first.second==QType::NSEC) { // check that it covers us!
+          for(const auto& r : v.second.records) {
+            LOG("\t"<<r->getZoneRepresentation()<<endl);
+            auto nsec = std::dynamic_pointer_cast<NSECRecordContent>(r);
+            if(nsec) {
+              if(v.first.first == *(zoneCutIter+1) && !nsec->d_set.count(QType::DS)) {
                 LOG("Denies existence of DS!"<<endl);
                 return Insecure;
               }
-              else if(beginHash == h && !nsec3->d_set.count(QType::DS)) {
-                LOG("Denies existence of DS (not opt-out)"<<endl);
-                return Insecure;
-              }
               else {
-                LOG("Did not cover us, start="<<v.first.first<<", us="<<toBase32Hex(h)<<", end="<<toBase32Hex(nsec3->d_nexthash)<<endl);
+                LOG("Did not deny existence of DS, "<<v.first.first<<"?="<<*(zoneCutIter+1)<<", "<<nsec->d_set.count(QType::DS)<<", next: "<<nsec->d_next<<endl);
               }
             }
           }
+
         }
-        return Bogus;
-      }
-      for(auto cspiter =r.first;  cspiter!=r.second; cspiter++) {
-        for(auto j=cspiter->second.records.cbegin(); j!=cspiter->second.records.cend(); j++)
-        {
-          const auto dsrc=std::dynamic_pointer_cast<DSRecordContent>(*j);
-          if(dsrc) {
-            dsmap.insert(*dsrc);
-            // dotEdge(keyqname,
-            //         "DNSKEY", keyqname, ,
-            //         "DS", qname, std::to_string(dsrc.d_tag));
-            // cout<<"    "<<dotEscape("DNSKEY "+keyqname)<<" -> "<<dotEscape("DS "+qname)<<";"<<endl;
+        else 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);
+            string h = hashQNameWithSalt(nsec3->d_salt, nsec3->d_iterations, *(zoneCutIter+1));
+            //              cerr<<"Salt length: "<<nsec3->d_salt.length()<<", iterations: "<<nsec3->d_iterations<<", hashed: "<<*(zoneCutIter+1)<<endl;
+            LOG("\tquery hash: "<<toBase32Hex(h)<<endl);
+            string beginHash=fromBase32Hex(v.first.first.getRawLabels()[0]);
+            if( (beginHash < h && h < nsec3->d_nexthash) ||
+                (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!"  
+            {
+              LOG("Denies existence of DS!"<<endl);
+              return Insecure;
+            }
+            else if(beginHash == h && !nsec3->d_set.count(QType::DS)) {
+              LOG("Denies existence of DS (not opt-out)"<<endl);
+              return Insecure;
+            }
+            else {
+              LOG("Did not cover us, start="<<v.first.first<<", us="<<toBase32Hex(h)<<", end="<<toBase32Hex(nsec3->d_nexthash)<<endl);
+            }
           }
         }
       }
-      if(!dsmap.size()) {
-	//        cerr<<"no DS at this level, checking for denials"<<endl;
-        dState dres = getDenial(validrrsets, qname, QType::DS);
-        if(dres == INSECURE) return Insecure;
-      }
-    skipLevel:;
-    } while(!dsmap.size() && labels.size());
+      return Bogus;
+    }
 
-    // break;
+    /*
+     * Collect all DS records and add them to the dsmap for the next iteration
+     */
+    for(auto cspiter =r.first;  cspiter!=r.second; cspiter++) {
+      for(auto j=cspiter->second.records.cbegin(); j!=cspiter->second.records.cend(); j++)
+      {
+        const auto dsrc=std::dynamic_pointer_cast<DSRecordContent>(*j);
+        if(dsrc) {
+          dsmap.insert(*dsrc);
+          // dotEdge(key*(zoneCutIter+1),
+          //         "DNSKEY", key*(zoneCutIter+1), ,
+          //         "DS", *(zoneCutIter+1), std::to_string(dsrc.d_tag));
+          // cout<<"    "<<dotEscape("DNSKEY "+key*(zoneCutIter+1))<<" -> "<<dotEscape("DS "+*(zoneCutIter+1))<<";"<<endl;
+        }
+      }
+    }
+    if(!dsmap.size()) {
+      //        cerr<<"no DS at this level, checking for denials"<<endl;
+      dState dres = getDenial(validrrsets, *(zoneCutIter+1), QType::DS);
+      if(dres == INSECURE) return Insecure;
+    }
   }
-
-  return state;
+  // There were no zone cuts (aka, we should never get here)
+  return Bogus;
 }
 
 
diff --git a/pdns/validate.hh b/pdns/validate.hh
index 84a2afb866..e3e0202fa7 100644
--- a/pdns/validate.hh
+++ b/pdns/validate.hh
@@ -37,7 +37,6 @@ extern const char *vStates[];
 enum dState { NODATA, NXDOMAIN, ENT, INSECURE };
 extern const char *dStates[];
 
-
 class DNSRecordOracle
 {
 public: