Changeset 1892

Timestamp:

01/18/11 09:43:56 (2 years ago)

Author:

ahu

Message:

remove the signing code from dnspacket, where it was cute but wrong.

Location:

trunk/pdns/pdns

Files:

• dnspacket.cc (modified) (7 diffs)
• dnspacket.hh (modified) (2 diffs)
• dnssecinfra.hh (modified) (1 diff)
• dnssecsigner.cc (modified) (6 diffs)
• packethandler.cc (modified) (2 diffs)
• tcpreceiver.cc (modified) (5 diffs)

trunk/pdns/pdns/dnspacket.cc

@@ -59 +59 @@
 }
 
-const char *DNSPacket::getData(DNSSECKeeper* dk)
+const char *DNSPacket::getData()
 {
   if(!d_wrapped)
-    wrapup(dk);
+    wrapup();
 
   return stringbuffer.data();
@@ -171 +171 @@
 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;
 
@@ -224 +224 @@
 }
 
+
 /** 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) {
@@ -237 +238 @@
   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");
@@ -276 +271 @@
   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:
@@ -293 +282 @@
         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
@@ -321 +296 @@
         }
       }
-      // 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)

trunk/pdns/pdns/dnspacket.hh

@@ -119 +119 @@
 
   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'
@@ -149 +149 @@
   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

trunk/pdns/pdns/dnssecinfra.hh

@@ -43 +43 @@
 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

trunk/pdns/pdns/dnssecsigner.cc

@@ -22 +22 @@
 #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)
 {
@@ -45 +32 @@
   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);
@@ -66 +49 @@
   // 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;
@@ -91 +75 @@
 
 // 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";
@@ -101 +85 @@
     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();
@@ -160 +143 @@
   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);
+}

trunk/pdns/pdns/packethandler.cc

@@ -998 +998 @@
     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();
@@ -1347 +1347 @@
     //    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
   }

trunk/pdns/pdns/tcpreceiver.cc

@@ -176 +176 @@
 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);
@@ -297 +296 @@
         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;
@@ -534 +533 @@
     if(!((++count)%chunk)) {
       count=0;
-    
+      addRRSigs(dk, sd.qname, *outpacket);
       sendPacket(outpacket, outsock);
 
@@ -596 +595 @@
   
   if(count) {
+    addRRSigs(dk, sd.qname, *outpacket);
     sendPacket(outpacket, outsock);
   }
@@ -602 +602 @@
   /* 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);