From: Kees Monshouwer <mind04@monshouwer.org>
Date: Tue, 15 Apr 2014 22:26:48 +0000 (+0200)
Subject: fix axfr in metadata handling and code cleanup
X-Git-Tag: rec-3.6.0-rc1~38^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5356820394b97445d476245fc8808f4462958932;p=thirdparty%2Fpdns.git

fix axfr in metadata handling and code cleanup
---

diff --git a/pdns/slavecommunicator.cc b/pdns/slavecommunicator.cc
index dd36a504fc..38a1fee16e 100644
--- a/pdns/slavecommunicator.cc
+++ b/pdns/slavecommunicator.cc
@@ -65,7 +65,6 @@ void CommunicatorClass::addSuckRequest(const string &domain, const string &maste
 void CommunicatorClass::suck(const string &domain,const string &remote)
 {
   L<<Logger::Error<<"Initiating transfer of '"<<domain<<"' from remote '"<<remote<<"'"<<endl;
-  uint32_t domain_id;
   PacketHandler P; // fresh UeberBackend
 
   DomainInfo di;
@@ -73,50 +72,28 @@ void CommunicatorClass::suck(const string &domain,const string &remote)
   bool transaction=false;
   try {
     UeberBackend *B=dynamic_cast<UeberBackend *>(P.getBackend());  // copy of the same UeberBackend
-    NSEC3PARAMRecordContent ns3pr, hadNs3pr;
-    bool narrow, hadNarrow=false;
-    DNSSECKeeper dk (B); // reuse our backend for DNSSECKeeper
-    bool dnssecZone = false;
-    bool haveNSEC3=false;
-    if(dk.isSecuredZone(domain)) {
-      dnssecZone=true;
-      haveNSEC3=dk.getNSEC3PARAM(domain, &ns3pr, &narrow);
-      if (haveNSEC3) {
-        hadNs3pr = ns3pr;
-        hadNarrow = narrow;
-      }
-    }
+    DNSSECKeeper dk (B); // reuse our UeberBackend copy for DNSSECKeeper
 
-    const bool hadNSEC3 = haveNSEC3;
-    const bool hadPresigned = dk.isPresigned(domain);
-    const bool hadDnssecZone = dnssecZone;
 
     if(!B->getDomainInfo(domain, di) || !di.backend) { // di.backend and B are mostly identical
       L<<Logger::Error<<"Can't determine backend for domain '"<<domain<<"'"<<endl;
       return;
     }
-    domain_id=di.id;
+    uint32_t domain_id=di.id;
+
 
-    Resolver::res_t recs;
-    set<string> nsset, qnames;
-    
-    ComboAddress raddr(remote, 53);
-    
     string tsigkeyname, tsigalgorithm, tsigsecret;
-  
     if(dk.getTSIGForAccess(domain, remote, &tsigkeyname)) {
       string tsigsecret64;
-      if(B->getTSIGKey(tsigkeyname, &tsigalgorithm, &tsigsecret64))
-      {
+      if(B->getTSIGKey(tsigkeyname, &tsigalgorithm, &tsigsecret64)) {
         B64Decode(tsigsecret64, tsigsecret);
-      }
-      else
-      {
-        L<<Logger::Error<<"TSIG key '"<<tsigkeyname<<"' not found, ignoring 'AXFR-MASTER-TSIG' for domain '"<<domain<<"'"<<endl;
-        tsigkeyname="";
+      } else {
+        L<<Logger::Error<<"TSIG key '"<<tsigkeyname<<"' for domain '"<<domain<<"' not found"<<endl;
+        return;
       }
     }
-    
+
+
     scoped_ptr<AuthLua> pdl;
     vector<string> scripts;
     if(B->getDomainMetadata(domain, "LUA-AXFR-SCRIPT", scripts) && !scripts.empty()) {
@@ -129,10 +106,10 @@ void CommunicatorClass::suck(const string &domain,const string &remote)
         return;
       }
     }
-    
+
+
     vector<string> localaddr;
     ComboAddress laddr;
-    
     if(B->getDomainMetadata(domain, "AXFR-SOURCE", localaddr) && !localaddr.empty()) {
       try {
         laddr = ComboAddress(localaddr[0]);
@@ -143,20 +120,41 @@ void CommunicatorClass::suck(const string &domain,const string &remote)
         return;
       }
     } else {
-                  laddr.sin4.sin_family = 0;
+      laddr.sin4.sin_family = 0;
     }
 
-    AXFRRetriever retriever(raddr, domain.c_str(), tsigkeyname, tsigalgorithm, tsigsecret,
-                (laddr.sin4.sin_family == 0) ? NULL : &laddr);
 
-    bool gotPresigned = false;
-    bool gotNSEC3 = false;
-    bool gotOptOutFlag = false;
-    unsigned int soa_serial = 0;
-    vector<DNSResourceRecord> rrs;
-    set<string> secured;
+    bool hadDnssecZone = false;
+    bool hadPresigned = false;
+    bool hadNSEC3 = false;
+    NSEC3PARAMRecordContent ns3pr, hadNs3pr;
+    bool isNarrow, hadNarrow=false;
+
+    if(dk.isSecuredZone(domain)) {
+      hadDnssecZone=true;
+      hadPresigned=dk.isPresigned(domain);
+      if (dk.getNSEC3PARAM(domain, &ns3pr, &isNarrow)) {
+        hadNSEC3 = true;
+        hadNs3pr = ns3pr;
+        hadNarrow = isNarrow;
+      }
+    }
+
+
+    bool isDnssecZone = false;
+    bool isPresigned = false;
+    bool isNSEC3 = false;
+    bool optOutFlag = false;
+
     bool first=true;
     bool firstNSEC3=true;
+    unsigned int soa_serial = 0;
+    set<string> nsset, qnames, secured;
+    vector<DNSResourceRecord> rrs;
+
+    ComboAddress raddr(remote, 53);
+    AXFRRetriever retriever(raddr, domain.c_str(), tsigkeyname, tsigalgorithm, tsigsecret, (laddr.sin4.sin_family == 0) ? NULL : &laddr);
+    Resolver::res_t recs;
     while(retriever.getChunk(recs)) {
       if(first) {
         L<<Logger::Error<<"AXFR started for '"<<domain<<"'"<<endl;
@@ -167,29 +165,29 @@ void CommunicatorClass::suck(const string &domain,const string &remote)
         if(i->qtype.getCode() == QType::OPT || i->qtype.getCode() == QType::TSIG) // ignore EDNS0 & TSIG
           continue;
 
-        if(!endsOn(i->qname, domain)) { 
+        if(!endsOn(i->qname, domain)) {
           L<<Logger::Error<<"Remote "<<remote<<" tried to sneak in out-of-zone data '"<<i->qname<<"'|"<<i->qtype.getName()<<" during AXFR of zone '"<<domain<<"', ignoring"<<endl;
           continue;
         }
 
         if (i->qtype.getCode() == QType::NSEC3PARAM) {
           ns3pr = NSEC3PARAMRecordContent(i->content);
-          narrow = false;
-          dnssecZone = haveNSEC3 = gotPresigned = gotNSEC3 = true;
+          isDnssecZone = isNSEC3 = true;
+          isNarrow = false;
           continue;
         } else if (i->qtype.getCode() == QType::NSEC3) {
           NSEC3RecordContent ns3rc(i->content);
           if (firstNSEC3) {
-            dnssecZone = gotPresigned = true;
+            isDnssecZone = isPresigned = true;
             firstNSEC3 = false;
-          } else if (gotOptOutFlag != (ns3rc.d_flags & 1))
+          } else if (optOutFlag != (ns3rc.d_flags & 1))
             throw PDNSException("Zones with a mixture of Opt-Out NSEC3 RRs and non-Opt-Out NSEC3 RRs are not supported.");
-          gotOptOutFlag = ns3rc.d_flags & 1;
+          optOutFlag = ns3rc.d_flags & 1;
           if (ns3rc.d_set.count(QType::NS) && !pdns_iequals(i->qname, domain))
             secured.insert(toLower(makeRelative(i->qname, domain)));
           continue;
         } else if (i->qtype.getCode() == QType::NSEC) {
-          dnssecZone = gotPresigned = true;
+          isDnssecZone = isPresigned = true;
           continue;
         }
 
@@ -203,11 +201,6 @@ void CommunicatorClass::suck(const string &domain,const string &remote)
 
         i->domain_id=domain_id;
 
-#if 0
-        if(i->qtype.getCode()>=60000)
-          throw DBException("Database can't store unknown record type "+lexical_cast<string>(i->qtype.getCode()-1024));
-#endif
-
         vector<DNSResourceRecord> out;
         if(pdl && pdl->axfrfilter(raddr, domain, *i, out)) {
           BOOST_FOREACH(const DNSResourceRecord& rr, out) {
@@ -218,7 +211,9 @@ void CommunicatorClass::suck(const string &domain,const string &remote)
         }
       }
     }
-
+    if(isNSEC3) {
+      ns3pr.d_flags = optOutFlag ? 1 : 0;
+    }
 
     BOOST_FOREACH(const DNSResourceRecord& rr, rrs) {
       if(rr.qtype.getCode() == QType::NS && !pdns_iequals(rr.qname, domain))
@@ -226,53 +221,66 @@ void CommunicatorClass::suck(const string &domain,const string &remote)
       qnames.insert(rr.qname);
     }
 
-    if(dnssecZone) {
-      if(!haveNSEC3) 
+
+    if(!isPresigned) {
+      DNSSECKeeper::keyset_t keys = dk.getKeys(domain);
+      if(!keys.empty()) {
+        isDnssecZone = true;
+        isNSEC3 = hadNSEC3;
+        ns3pr = hadNs3pr;
+        optOutFlag = (hadNs3pr.d_flags & 1);
+        isNarrow = hadNarrow;
+      }
+    }
+
+
+    if(isDnssecZone) {
+      if(!isNSEC3)
         L<<Logger::Info<<"Adding NSEC ordering information"<<endl;
-      else if(!narrow)
+      else if(!isNarrow)
         L<<Logger::Info<<"Adding NSEC3 hashed ordering information for '"<<domain<<"'"<<endl;
       else
         L<<Logger::Info<<"Erasing NSEC3 ordering since we are narrow, only setting 'auth' fields"<<endl;
     }
 
-    if (hadPresigned && !gotNSEC3) { // not sure why this is here
-      // we only had NSEC3 because we were a presigned zone...
-      haveNSEC3 = false;
-    }
 
-
-    di.backend->startTransaction(domain, domain_id);
-    transaction = true;
+    transaction=di.backend->startTransaction(domain, domain_id);
     L<<Logger::Error<<"Transaction started for '"<<domain<<"'"<<endl;
 
     // update the presigned flag and NSEC3PARAM
-    if (gotPresigned) {
-      if (!hadDnssecZone && !hadPresigned) {
+    if (isDnssecZone) {
+      // update presigned if there was a change
+      if (isPresigned && !hadPresigned) {
         // zone is now presigned
         dk.setPresigned(domain);
+      } else if (hadPresigned && !isPresigned) {
+        // zone is no longer presigned
+        dk.unsetPresigned(domain);
       }
-
-      if (hadPresigned || !hadDnssecZone)
-      {
-        // this is a presigned zone, update NSEC3PARAM
-        if (gotNSEC3) {
-          ns3pr.d_flags = gotOptOutFlag ? 1 : 0;
-         // only update if there was a change
-          if (!hadNSEC3 || (narrow != hadNarrow) ||
-              (ns3pr.d_algorithm != hadNs3pr.d_algorithm) ||
-              (ns3pr.d_flags != hadNs3pr.d_flags) ||
-              (ns3pr.d_iterations != hadNs3pr.d_iterations) ||
-              (ns3pr.d_salt != hadNs3pr.d_salt)) {
-            dk.setNSEC3PARAM(domain, ns3pr, narrow);
-          }
-        } else if (hadNSEC3) {
-          dk.unsetNSEC3PARAM(domain);
+      // update NSEC3PARAM
+      if (isNSEC3) {
+        // zone is NSEC3, only update if there was a change
+        if (!hadNSEC3 || (hadNarrow  != isNarrow) ||
+            (ns3pr.d_algorithm != hadNs3pr.d_algorithm) ||
+            (ns3pr.d_flags != hadNs3pr.d_flags) ||
+            (ns3pr.d_iterations != hadNs3pr.d_iterations) ||
+            (ns3pr.d_salt != hadNs3pr.d_salt)) {
+          dk.setNSEC3PARAM(domain, ns3pr, isNarrow);
         }
+      } else if (hadNSEC3 ) {
+         // zone is no longer NSEC3
+         dk.unsetNSEC3PARAM(domain);
+      }
+    } else if (hadDnssecZone) {
+      // zone is no longer signed
+      if (hadPresigned) {
+        // remove presigned
+        dk.unsetPresigned(domain);
+      }
+      if (hadNSEC3) {
+        // unset NSEC3PARAM
+        dk.unsetNSEC3PARAM(domain);
       }
-    } else if (hadPresigned) {
-      // zone is no longer presigned
-      dk.unsetPresigned(domain);
-      dk.unsetNSEC3PARAM(domain);
     }
 
     bool doent=true;
@@ -281,8 +289,16 @@ void CommunicatorClass::suck(const string &domain,const string &remote)
     set<string> rrterm;
     map<string,bool> nonterm;
 
+
     BOOST_FOREACH(DNSResourceRecord& rr, rrs) {
 
+      if(!isPresigned) {
+        if (rr.qtype.getCode() == QType::RRSIG)
+          continue;
+        if(isDnssecZone && rr.qtype.getCode() == QType::DNSKEY && !::arg().mustDo("direct-dnskey"))
+          continue;
+      }
+
       // Figure out auth and ents
       rr.auth=true;
       shorter=rr.qname;
@@ -304,7 +320,7 @@ void CommunicatorClass::suck(const string &domain,const string &remote)
         bool auth;
         if (!rr.auth && rr.qtype.getCode() == QType::NS) {
           ordername=toBase32Hex(hashQNameWithSalt(ns3pr.d_iterations, ns3pr.d_salt, rr.qname));
-          auth=(!gotOptOutFlag || secured.count(ordername));
+          auth=(!optOutFlag || secured.count(ordername));
         } else
           auth=rr.auth;
 
@@ -327,11 +343,11 @@ void CommunicatorClass::suck(const string &domain,const string &remote)
         rr.auth=true;
 
       // Add ordername and insert record
-      if (dnssecZone && rr.qtype.getCode() != QType::RRSIG) {
-        if (haveNSEC3) {
+      if (isDnssecZone && rr.qtype.getCode() != QType::RRSIG) {
+        if (isNSEC3) {
           // NSEC3
           ordername=toBase32Hex(hashQNameWithSalt(ns3pr.d_iterations, ns3pr.d_salt, rr.qname));
-          if(!narrow && (rr.auth || (rr.qtype.getCode() == QType::NS && (!gotOptOutFlag || secured.count(ordername))))) {
+          if(!isNarrow && (rr.auth || (rr.qtype.getCode() == QType::NS && (!optOutFlag || secured.count(ordername))))) {
             di.backend->feedRecord(rr, &ordername);
           } else
             di.backend->feedRecord(rr);
@@ -349,8 +365,8 @@ void CommunicatorClass::suck(const string &domain,const string &remote)
 
     // Insert empty non-terminals
     if(doent && !nonterm.empty()) {
-      if (haveNSEC3) {
-        di.backend->feedEnts3(domain_id, domain, nonterm, ns3pr.d_iterations, ns3pr.d_salt, narrow);
+      if (isNSEC3) {
+        di.backend->feedEnts3(domain_id, domain, nonterm, ns3pr.d_iterations, ns3pr.d_salt, isNarrow);
       } else
         di.backend->feedEnts(domain_id, nonterm);
     }
@@ -421,33 +437,33 @@ struct DomainNotificationInfo
 struct SlaveSenderReceiver
 {
   typedef pair<string, uint16_t> Identifier;
-  
+
   struct Answer {
     uint32_t theirSerial;
     uint32_t theirInception;
     uint32_t theirExpire;
   };
-  
+
   map<uint32_t, Answer> d_freshness;
-  
+
   SlaveSenderReceiver()
   {
   }
-  
+
   void deliverTimeout(const Identifier& i)
   {
   }
-  
+
   Identifier send(DomainNotificationInfo& dni)
   {
     random_shuffle(dni.di.masters.begin(), dni.di.masters.end());
     try {
       ComboAddress remote(*dni.di.masters.begin());
       if (dni.localaddr.sin4.sin_family == 0) {
-        return make_pair(dni.di.zone, 
-          d_resolver.sendResolve(ComboAddress(*dni.di.masters.begin(), 53), 
-            dni.di.zone.c_str(), 
-            QType::SOA, 
+        return make_pair(dni.di.zone,
+          d_resolver.sendResolve(ComboAddress(*dni.di.masters.begin(), 53),
+            dni.di.zone.c_str(),
+            QType::SOA,
             dni.dnssecOk, dni.tsigkeyname, dni.tsigalgname, dni.tsigsecret)
         );
       } else {