make sure we don't send back an oversized packet after adding signatures

git-svn-id: svn://svn.powerdns.com/pdns/trunk/pdns@1832 d19b8d6e-7fed-0310-83ef-9ca221ded41b

diff --git a/pdns/dbdnsseckeeper.cc b/pdns/dbdnsseckeeper.cc
index fcf21ce4958a25ab286ebf23f4be299385656c90..8361eda9a370d93d40160151c0052f3e3bc44f05 100644 (file)
--- a/pdns/dbdnsseckeeper.cc
+++ b/pdns/dbdnsseckeeper.cc
@@ -246,7 +246,9 @@ int getRRSIGForRRSET(DNSSECKeeper& dk, const std::string signQName, uint16_t sig
   return 0;
 }
 
-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, DNSPacketWriter& pw)
+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 maxReplyLen, DNSPacketWriter& pw)
 {
   // cerr<<"Asked to sign '"<<signQName<<"'|"<<DNSRecordContent::NumberToType(signQType)<<", "<<toSign.size()<<" records\n";
 
@@ -261,8 +263,13 @@ void addSignature(DNSSECKeeper& dk, const std::string signQName, const std::stri
     }
     
     pw.startRecord(signQName, QType::RRSIG, 3600, 1, 
-                  signQType==QType::DNSKEY ? DNSPacketWriter:: ANSWER : signPlace); 
+    signQType==QType::DNSKEY ? DNSPacketWriter:: ANSWER : signPlace); 
     rrc.toPacket(pw);
+    if(maxReplyLen &&  (pw.size() + 20) > maxReplyLen) {
+      pw.rollback();
+      pw.getHeader()->tc=1;
+      return;
+    }
     
     pw.commit();
     if(signQType != QType::DNSKEY)

diff --git a/pdns/dnspacket.cc b/pdns/dnspacket.cc
index ff2f37dba5b58805d422cd21754b1620adabd48b..ae937be79096746d8d948779e030b819ae194479 100644 (file)
--- a/pdns/dnspacket.cc
+++ b/pdns/dnspacket.cc
@@ -296,7 +296,7 @@ void DNSPacket::wrapup(DNSSECKeeper* dk)
 
        if(d_dnssecOk) {
          if(pos != d_rrs.begin() && (signQType != pos->qtype.getCode()  || signQName != pos->qname)) {
-           addSignature(*dk, signQName, wildcardQName, signQType, signTTL, signPlace, toSign, pw);
+           addSignature(*dk, signQName, wildcardQName, signQType, signTTL, signPlace, toSign, d_tcp ? 0 : getMaxReplyLen(), pw);
          }
          signQName= pos->qname;
          wildcardQName = pos->wildcardname;
@@ -310,7 +310,7 @@ void DNSPacket::wrapup(DNSSECKeeper* dk)
        pw.startRecord(pos->qname, pos->qtype.getCode(), pos->ttl, pos->qclass, (DNSPacketWriter::Place)pos->d_place); 
 
         drc->toPacket(pw);
-       if(!d_tcp && pw.size() + 20 > getMaxReplyLen()) { // XXX FIXME, 20? what does it mean?
+       if(!d_tcp && pw.size() + 20 > getMaxReplyLen()) { // 20 = room for EDNS0
          pw.rollback();
          if(pos->d_place == DNSResourceRecord::ANSWER) {
            pw.getHeader()->tc=1;
@@ -322,14 +322,14 @@ void DNSPacket::wrapup(DNSSECKeeper* dk)
       }
       // 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)) {
-       // cerr<<"Last signature.. "<<d_tcp<<", "<<d_rrs.rbegin()->priority<<", "<<d_rrs.rbegin()->qtype.getCode()<<", "<< d_rrs.size()<<endl;
-       addSignature(*dk, signQName, wildcardQName, signQType, signTTL, signPlace, toSign, pw);
+       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);
 
-      pw.commit();
+      if(!pw.getHeader()->tc) // protect against double commit from addSignature
+       pw.commit();
     noCommit:;
     }
     catch(std::exception& e) {

diff --git a/pdns/dnssecinfra.hh b/pdns/dnssecinfra.hh
index 2f0eab29779f491117c39ad5669a14d0d214f066..ca29b4b6af3ed368d87d23cd4e93abf8009e34d3 100644 (file)
--- a/pdns/dnssecinfra.hh
+++ b/pdns/dnssecinfra.hh
@@ -41,7 +41,8 @@ bool getSignerFor(DNSSECKeeper& dk, const std::string& keyrepodir, const std::st
 DNSKEYRecordContent getDNSKEYFor(DNSSECKeeper& dk, const std::string& keyrepodir, const std::string& qname, bool withKSK, RSAContext* rc);
 void fillOutRRSIG(DNSSECKeeper& dk, const std::string& signQName, RRSIGRecordContent& rrc, const std::string& hash, vector<shared_ptr<DNSRecordContent> >& toSign, bool withKSK=false);
 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, DNSPacketWriter& pw);
+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 getRRSIGForRRSET(DNSSECKeeper& dk, const std::string signQName, uint16_t signQType, uint32_t signTTL, 
                     vector<shared_ptr<DNSRecordContent> >& toSign, RRSIGRecordContent &rrc, bool ksk);