From: Bert Hubert <bert.hubert@netherlabs.nl>
Date: Fri, 7 Jan 2011 22:29:36 +0000 (+0000)
Subject: make sure we don't send back an oversized packet after adding signatures
X-Git-Tag: auth-3.0~415
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=cb6cc5db741534dc21c35d425256351e643f55f3;p=pdns

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 fcf21ce49..8361eda9a 100644
--- 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 ff2f37dba..ae937be79 100644
--- 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 2f0eab297..ca29b4b6a 100644
--- 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);