]> git.ipfire.org Git - thirdparty/pdns.git/commitdiff
remove the signing code from dnspacket, where it was cute but wrong.
authorBert Hubert <bert.hubert@netherlabs.nl>
Tue, 18 Jan 2011 08:43:56 +0000 (08:43 +0000)
committerBert Hubert <bert.hubert@netherlabs.nl>
Tue, 18 Jan 2011 08:43:56 +0000 (08:43 +0000)
git-svn-id: svn://svn.powerdns.com/pdns/trunk/pdns@1892 d19b8d6e-7fed-0310-83ef-9ca221ded41b

pdns/dnspacket.cc
pdns/dnspacket.hh
pdns/dnssecinfra.hh
pdns/dnssecsigner.cc
pdns/packethandler.cc
pdns/tcpreceiver.cc

index 34128b8c2f31f2b724673974ced84171fa2fc0df..f3e78d392b2acb1980718ddfacb6683e293e8607 100644 (file)
@@ -58,10 +58,10 @@ string DNSPacket::getString()
   return stringbuffer;
 }
 
-const char *DNSPacket::getData(DNSSECKeeper* dk)
+const char *DNSPacket::getData()
 {
   if(!d_wrapped)
-    wrapup(dk);
+    wrapup();
 
   return stringbuffer.data();
 }
@@ -170,7 +170,7 @@ void DNSPacket::addRecord(const DNSResourceRecord &rr)
 
 static int rrcomp(const DNSResourceRecord &A, const DNSResourceRecord &B)
 {
-  if(A.d_place<B.d_place)
+  if(A.d_place < B.d_place)
     return 1;
 
   return 0;
@@ -223,10 +223,11 @@ bool DNSPacket::couldBeCached()
   return d_ednsping.empty() && !d_wantsnsid && qclass==QClass::IN;
 }
 
+
 /** Must be called before attempting to access getData(). This function stuffs all resource
  *  records found in rrs into the data buffer. It also frees resource records queued for us.
  */
-void DNSPacket::wrapup(DNSSECKeeper* dk)
+void DNSPacket::wrapup()
 {
   if(d_wrapped) {
     return;
@@ -236,16 +237,10 @@ void DNSPacket::wrapup(DNSSECKeeper* dk)
   DNSResourceRecord rr;
   vector<DNSResourceRecord>::iterator pos;
 
-  vector<DNSResourceRecord> additional;
-
-  int ipos=d_rrs.size();
-  d_rrs.resize(d_rrs.size()+additional.size());
-  copy(additional.begin(), additional.end(), d_rrs.begin()+ipos);
-
   // we now need to order rrs so that the different sections come at the right place
   // we want a stable sort, based on the d_place field
 
-  stable_sort(d_rrs.begin(),d_rrs.end(),rrcomp);
+  stable_sort(d_rrs.begin(),d_rrs.end(), rrcomp);
 
   static bool mustShuffle =::arg().mustDo("no-shuffle");
 
@@ -275,12 +270,6 @@ void DNSPacket::wrapup(DNSSECKeeper* dk)
 
   if(!d_rrs.empty() || !opts.empty()) {
     try {
-      string signQName, wildcardQName;
-      uint16_t signQType=0;
-      uint32_t signTTL=0;
-      DNSPacketWriter::Place signPlace=DNSPacketWriter::ANSWER;
-      vector<shared_ptr<DNSRecordContent> > toSign;
-
       for(pos=d_rrs.begin(); pos < d_rrs.end(); ++pos) {
         // this needs to deal with the 'prio' mismatch:
         if(pos->qtype.getCode()==QType::MX || pos->qtype.getCode() == QType::SRV) {  
@@ -292,23 +281,9 @@ void DNSPacket::wrapup(DNSSECKeeper* dk)
         }
         if(pos->content.empty())  // empty contents confuse the MOADNS setup
           pos->content=".";
-        shared_ptr<DNSRecordContent> drc(DNSRecordContent::mastermake(pos->qtype.getCode(), 1, pos->content)); 
-
-       if(d_dnssecOk) {
-         if(pos != d_rrs.begin() && (signQType != pos->qtype.getCode()  || signQName != pos->qname)) {
-           addSignature(*dk, signQName, wildcardQName, signQType, signTTL, signPlace, toSign, d_tcp ? 0 : getMaxReplyLen(), pw);
-         }
-         signQName= pos->qname;
-         wildcardQName = pos->wildcardname;
-         signQType = pos ->qtype.getCode();
-         signTTL = pos->ttl;
-         signPlace = (DNSPacketWriter::Place) pos->d_place;
-         if(pos->auth || pos->qtype.getCode() == QType::DS)
-           toSign.push_back(drc);
-       }
-       
+        
        pw.startRecord(pos->qname, pos->qtype.getCode(), pos->ttl, pos->qclass, (DNSPacketWriter::Place)pos->d_place); 
-
+       shared_ptr<DNSRecordContent> drc(DNSRecordContent::mastermake(pos->qtype.getCode(), 1, pos->content)); 
         drc->toPacket(pw);
        if(!d_tcp && pw.size() + 20 > getMaxReplyLen()) { // 20 = room for EDNS0
          pw.rollback();
@@ -320,10 +295,7 @@ void DNSPacket::wrapup(DNSSECKeeper* dk)
          break;
        }
       }
-      // I assume this is some dirty hack to prevent us from signing the last SOA record in an AXFR.. XXX FIXME
-      if(d_dnssecOk && !(d_tcp && d_rrs.rbegin()->qtype.getCode() == QType::SOA && d_rrs.rbegin()->priority == 1234)) {
-       addSignature(*dk, signQName, wildcardQName, signQType, signTTL, signPlace, toSign, d_tcp ? 0 : getMaxReplyLen(), pw);
-      }
+      
 
       if(!opts.empty() || d_dnssecOk)
        pw.addOpt(2800, 0, d_dnssecOk ? EDNSOpts::DNSSECOK : 0, opts);
index 0cc1307d58733b6386cf7a2469212792bb643216..a051ea112abee22d65c356af339447b843768bd3 100644 (file)
@@ -118,8 +118,8 @@ public:
   void setQuestion(int op, const string &qdomain, int qtype);  // wipes 'd', sets a random id, creates start of packet (label, type, class etc)
 
   DTime d_dt; //!< the time this packet was created. replyPacket() copies this in for you, so d_dt becomes the time spent processing the question+answer
-  void wrapup(DNSSECKeeper* dk=0);  // writes out queued rrs, and generates the binary packet. also shuffles. also rectifies dnsheader 'd', and copies it to the stringbuffer
-  const char *getData(DNSSECKeeper* dk=0); //!< get binary representation of packet, will call 'wrapup' for you
+  void wrapup();  // writes out queued rrs, and generates the binary packet. also shuffles. also rectifies dnsheader 'd', and copies it to the stringbuffer
+  const char *getData(); //!< get binary representation of packet, will call 'wrapup' for you
 
   const char *getRaw(void); //!< provides access to the raw packet, possibly on a packet that has never been 'wrapped'
   void spoofQuestion(const string &qd); //!< paste in the exact right case of the question. Useful for PacketCache
@@ -148,6 +148,7 @@ public:
   string qdomain;  //!< qname of the question 4 - unsure how this is used
   bool d_tcp;
   bool d_dnssecOk;
+  vector<DNSResourceRecord>& getRRS() { return d_rrs; }
 private:
   void pasteQ(const char *question, int length); //!< set the question of this packet, useful for crafting replies
 
index 49682e2637bead740241e6b374d8c489f3012728..33219424e563921bc9acdc111a13ea7600ff6ed5 100644 (file)
@@ -42,12 +42,13 @@ struct DNSSECPrivateKey;
 bool getSignerApexFor(DNSSECKeeper& dk, const std::string& keyrepodir, const std::string& qname, std::string &signer);
 void fillOutRRSIG(DNSSECPrivateKey& dpk, const std::string& signQName, RRSIGRecordContent& rrc, vector<shared_ptr<DNSRecordContent> >& toSign);
 uint32_t getCurrentInception();
-void addSignature(DNSSECKeeper& dk, const std::string signQName, const std::string& wildcardname, uint16_t signQType, uint32_t signTTL, DNSPacketWriter::Place signPlace, vector<shared_ptr<DNSRecordContent> >& toSign, 
-  uint16_t maxReplyLength, DNSPacketWriter& pw);
-int getRRSIGsForRRSET(DNSSECKeeper& dk, const std::string signQName, uint16_t signQType, uint32_t signTTL, 
+void addSignature(DNSSECKeeper& dk, const std::string signQName, const std::string& wildcardname, uint16_t signQType, uint32_t signTTL, DNSPacketWriter::Place signPlace, 
+  vector<shared_ptr<DNSRecordContent> >& toSign, vector<DNSResourceRecord>& outsigned);
+int getRRSIGsForRRSET(DNSSECKeeper& dk, const std::string& signer, const std::string signQName, uint16_t signQType, uint32_t signTTL, 
                     vector<shared_ptr<DNSRecordContent> >& toSign, vector<RRSIGRecordContent> &rrc, bool ksk);
 
 std::string hashQNameWithSalt(unsigned int times, const std::string& salt, const std::string& qname);
 void decodeDERIntegerSequence(const std::string& input, vector<string>& output);
-
+class DNSPacket;
+void addRRSigs(DNSSECKeeper& dk, const std::string& signer, DNSPacket& p);
 #endif
index ee98cac19abf9870ee6b610ea0d336ee15d1590f..1153479920226bd8a384eed72d0c91408ccea13f 100644 (file)
 #include "dnsseckeeper.hh"
 #include "lock.hh"
 
-// nobody should ever call this function, you know the SOA/auth already!
-bool getSignerApexFor(DNSSECKeeper& dk, const std::string& qname, std::string &signer)
-{
-  // cerr<<"getSignerApexFor: called, and should not be, should go away!"<<endl;
-  signer=qname;
-  do {
-    if(dk.haveActiveKSKFor(signer)) {
-      return true;
-    }
-  } while(chopOff(signer));
-  return false;
-}
-
 /* this is where the RRSIGs begin, key apex *name* gets found, keys are retrieved,
    but the actual signing happens in fillOutRRSIG */
-int getRRSIGsForRRSET(DNSSECKeeper& dk, const std::string signQName, uint16_t signQType, uint32_t signTTL, 
+int getRRSIGsForRRSET(DNSSECKeeper& dk, const std::string& signer, const std::string signQName, uint16_t signQType, uint32_t signTTL, 
                     vector<shared_ptr<DNSRecordContent> >& toSign, vector<RRSIGRecordContent>& rrcs, bool ksk)
 {
   if(toSign.empty())
@@ -44,19 +31,15 @@ int getRRSIGsForRRSET(DNSSECKeeper& dk, const std::string signQName, uint16_t si
   RRSIGRecordContent rrc;
   rrc.d_type=signQType;
 
-  // d_algorithm gets filled out by getSignerAPEX, since only it looks up the key
+  
   rrc.d_labels=countLabels(signQName); 
   rrc.d_originalttl=signTTL; 
   rrc.d_siginception=getCurrentInception();;
   rrc.d_sigexpire = rrc.d_siginception + 14*86400; // XXX should come from zone metadata
+  rrc.d_signer = signer;
   rrc.d_tag = 0;
   
-  // XXX we know the apex already.. is is the SOA name which we determined earlier
-  if(!getSignerApexFor(dk, signQName, rrc.d_signer)) { // this is the cutout for signing non-dnssec enabled zones
-    // cerr<<"No signer known for '"<<signQName<<"'\n";
-    return -1;
-  }
-  // we sign the RRSET in toSign + the rrc w/o key
+  // we sign the RRSET in toSign + the rrc w/o hash
   
   DNSSECKeeper::keyset_t keys = dk.getKeys(rrc.d_signer);
   vector<DNSSECPrivateKey> KSKs, ZSKs;
@@ -65,6 +48,7 @@ int getRRSIGsForRRSET(DNSSECKeeper& dk, const std::string signQName, uint16_t si
   // if ksk==1, only get KSKs
   // if ksk==0, get ZSKs, unless there is no ZSK, then get KSK
   BOOST_FOREACH(DNSSECKeeper::keyset_t::value_type& keymeta, keys) {
+    rrc.d_algorithm = keymeta.first.d_algorithm;
     if(!keymeta.second.active) 
       continue;
       
@@ -90,9 +74,9 @@ int getRRSIGsForRRSET(DNSSECKeeper& dk, const std::string signQName, uint16_t si
 }
 
 // this is the entrypoint from DNSPacket
-void addSignature(DNSSECKeeper& dk, const std::string signQName, const std::string& wildcardname, uint16_t signQType, 
+void addSignature(DNSSECKeeper& dk, const std::string& signer, const std::string signQName, const std::string& wildcardname, uint16_t signQType, 
   uint32_t signTTL, DNSPacketWriter::Place signPlace, 
-  vector<shared_ptr<DNSRecordContent> >& toSign, uint16_t maxReplyLen, DNSPacketWriter& pw)
+  vector<shared_ptr<DNSRecordContent> >& toSign, vector<DNSResourceRecord>& outsigned)
 {
   // cerr<<"Asked to sign '"<<signQName<<"'|"<<DNSRecordContent::NumberToType(signQType)<<", "<<toSign.size()<<" records\n";
 
@@ -100,21 +84,20 @@ void addSignature(DNSSECKeeper& dk, const std::string signQName, const std::stri
   if(toSign.empty())
     return;
 
-  if(getRRSIGsForRRSET(dk, wildcardname.empty() ? signQName : wildcardname, signQType, signTTL, toSign, rrcs, signQType == QType::DNSKEY) < 0) {
+  if(getRRSIGsForRRSET(dk, signer, wildcardname.empty() ? signQName : wildcardname, signQType, signTTL, toSign, rrcs, signQType == QType::DNSKEY) < 0) {
     // cerr<<"Error signing a record!"<<endl;
     return;
   }
+  DNSResourceRecord rr;
+  rr.qname=signQName;
+  rr.qtype=QType::RRSIG;
+  rr.ttl=signTTL;
+  rr.auth=false;
+  
   BOOST_FOREACH(RRSIGRecordContent& rrc, rrcs) {
-    pw.startRecord(signQName, QType::RRSIG, signTTL, 1, 
-      signQType==QType::DNSKEY ? DNSPacketWriter:: ANSWER : signPlace); 
-    rrc.toPacket(pw);
-    if(maxReplyLen &&  (pw.size() + 20) > maxReplyLen) {
-      pw.rollback();
-      pw.getHeader()->tc=1;
-      return;
-    }
+    rr.content = rrc.getZoneRepresentation();
+    outsigned.push_back(rr);
   }
-  pw.commit();
 
   toSign.clear();
 }
@@ -159,3 +142,53 @@ void fillOutRRSIG(DNSSECPrivateKey& dpk, const std::string& signQName, RRSIGReco
   Lock l(&g_signatures_lock);
   g_signatures[lookup] = rrc.d_signature;
 }
+
+static bool rrsigncomp(const DNSResourceRecord& a, const DNSResourceRecord& b)
+{
+  return a.d_place < b.d_place;
+}
+
+void addRRSigs(DNSSECKeeper& dk, const std::string& signer, DNSPacket& p)
+{
+  vector<DNSResourceRecord>& rrs=p.getRRS();
+  
+  stable_sort(rrs.begin(), rrs.end(), rrsigncomp);
+  
+  string signQName, wildcardQName;
+  uint16_t signQType=0;
+  uint32_t signTTL=0;
+  
+  DNSPacketWriter::Place signPlace=DNSPacketWriter::ANSWER;
+  vector<shared_ptr<DNSRecordContent> > toSign;
+
+  vector<DNSResourceRecord> signedRecords;
+
+  for(vector<DNSResourceRecord>::const_iterator pos = rrs.begin(); pos != rrs.end(); ++pos) {
+    signedRecords.push_back(*pos);
+    if(pos != rrs.begin() && (signQType != pos->qtype.getCode()  || signQName != pos->qname)) {
+      addSignature(dk, signer, signQName, wildcardQName, signQType, signTTL, signPlace, toSign, signedRecords);
+    }
+    signQName= pos->qname;
+    wildcardQName = pos->wildcardname;
+    signQType = pos ->qtype.getCode();
+    signTTL = pos->ttl;
+    signPlace = (DNSPacketWriter::Place) pos->d_place;
+    if(pos->auth || pos->qtype.getCode() == QType::DS) {
+      string content = pos ->content;
+      if(pos->qtype.getCode()==QType::MX || pos->qtype.getCode() == QType::SRV) {  
+        content = lexical_cast<string>(pos->priority) + " " + pos->content;
+      }
+      if(!pos->content.empty() && pos->qtype.getCode()==QType::TXT && pos->content[0]!='"') {
+        content="\""+pos->content+"\"";
+      }
+      if(pos->content.empty())  // empty contents confuse the MOADNS setup
+        content=".";
+      
+      shared_ptr<DNSRecordContent> drc(DNSRecordContent::mastermake(pos->qtype.getCode(), 1, content)); 
+      toSign.push_back(drc);
+    }
+  }
+  addSignature(dk, signer, signQName, wildcardQName, signQType, signTTL, signPlace, toSign, signedRecords);
+  
+  rrs.swap(signedRecords);
+}
index 53afc84effd22c36d7c510f5a883d8ab8f415e07..785d52cdbbe84a4b089999e3631ff0dd9c1f0d4a 100644 (file)
@@ -997,7 +997,7 @@ void PacketHandler::synthesiseRRSIGs(DNSPacket* p, DNSPacket* r)
   BOOST_FOREACH(records_t::value_type& iter, records) {
     vector<RRSIGRecordContent> rrcs;
     
-    getRRSIGsForRRSET(d_dk, p->qdomain, iter.first, 3600, iter.second, rrcs, iter.first == QType::DNSKEY);
+    getRRSIGsForRRSET(d_dk, sd.qname, p->qdomain, iter.first, 3600, iter.second, rrcs, iter.first == QType::DNSKEY);
     BOOST_FOREACH(RRSIGRecordContent& rrc, rrcs) {
       rr.content=rrc.getZoneRepresentation();
       r->addRecord(rr);
@@ -1346,7 +1346,9 @@ DNSPacket *PacketHandler::questionOrRecurse(DNSPacket *p, bool *shouldRecurse)
 
     //    doDNSSECProcessing(p, r);
 
-    r->wrapup(&d_dk); // needed for inserting in cache
+    if(p->d_dnssecOk)
+      addRRSigs(d_dk, sd.qname, *r);
+    r->wrapup(); // needed for inserting in cache
     PC.insert(p, r); // in the packet cache
   }
   catch(DBException &e) {
index 804cebf264587cca1affa6dcf34d0d53ff303bd9..724676813b7c7260152d56a7a9a7befb67e38b13 100644 (file)
@@ -175,8 +175,7 @@ void connectWithTimeout(int fd, struct sockaddr* remote, size_t socklen)
 
 void TCPNameserver::sendPacket(shared_ptr<DNSPacket> p, int outsock)
 {
-  DNSSECKeeper dk;
-  const char *buf=p->getData(&dk);
+  const char *buf=p->getData();
   uint16_t len=htons(p->len);
   writenWithTimeout(outsock, &len, 2);
   writenWithTimeout(outsock, buf, p->len);
@@ -296,7 +295,7 @@ void *TCPNameserver::doConnection(void *data)
         cached->d.rd=packet->d.rd; // copy in recursion desired bit 
         cached->commitD(); // commit d to the packet                        inlined
 
-        sendPacket(cached, fd);
+        sendPacket(cached, fd); // presigned, don't do it again
         S.inc("tcp-answers");
         continue;
       }
@@ -533,7 +532,7 @@ int TCPNameserver::doAXFR(const string &target, shared_ptr<DNSPacket> q, int out
 
     if(!((++count)%chunk)) {
       count=0;
-    
+      addRRSigs(dk, sd.qname, *outpacket);
       sendPacket(outpacket, outsock);
 
       outpacket=shared_ptr<DNSPacket>(q->replyPacket());
@@ -595,13 +594,15 @@ int TCPNameserver::doAXFR(const string &target, shared_ptr<DNSPacket> q, int out
   }
   
   if(count) {
+    addRRSigs(dk, sd.qname, *outpacket);
     sendPacket(outpacket, outsock);
   }
 
   DLOG(L<<"Done writing out records"<<endl);
   /* and terminate with yet again the SOA record */
   outpacket=shared_ptr<DNSPacket>(q->replyPacket());
-  soa.priority=1234;
+  
+  addRRSigs(dk, sd.qname, *outpacket); // don't sign the SOA!
   outpacket->addRecord(soa);
   sendPacket(outpacket, outsock);
   DLOG(L<<"last packet - close"<<endl);