]> granicus.if.org Git - pdns/commitdiff
fix compression with DNSName. leaves two broken tests in gsqlite3-dnssec
authorPeter van Dijk <peter.van.dijk@netherlabs.nl>
Tue, 23 Jun 2015 15:07:04 +0000 (17:07 +0200)
committermind04 <mind04@monshouwer.org>
Tue, 30 Jun 2015 06:12:50 +0000 (08:12 +0200)
pdns/dnsname.cc
pdns/dnswriter.cc
pdns/dnswriter.hh

index 3b4f13038b456cd425fc067970916a4f403339ff..aeabf9f8ed6981748fc093ba45a94d7cb9268556 100644 (file)
 /* raw storage
    in DNS label format, without trailing 0. So the root is of length 0.
 
-   www.powerdns.com = 3www8powerdns3com 
-   
+   www.powerdns.com = 3www8powerdns3com
+
    a primitive is nextLabel()
 */
 
+/* FIXME400: @nlyan suggests that we should only have a string constructor, and make sure
+ * char* does not implicitly map to it, to avoid issues with embedded NULLs */
 DNSName::DNSName(const char* p)
 {
   d_empty=false;
@@ -64,11 +66,11 @@ void DNSName::packetParser(const char* pos, int len, int offset, bool uncompress
   }
   if(consumed)
     *consumed = pos - opos - offset;
-  if(qtype && pos + labellen + 2 <= end)  
+  if(qtype && pos + labellen + 2 <= end)
     *qtype=(*(const unsigned char*)pos)*256 + *((const unsigned char*)pos+1);
 
   pos+=2;
-  if(qclass && pos + labellen + 2 <= end)  
+  if(qclass && pos + labellen + 2 <= end)
     *qclass=(*(const unsigned char*)pos)*256 + *((const unsigned char*)pos+1);
 
 }
@@ -246,7 +248,7 @@ string DNSName::escapeLabel(const std::string& label)
 {
   string ret;
   for(uint8_t p : label) {
-    if(p=='.') 
+    if(p=='.')
       ret+="\\.";
     else if(p=='\\')
       ret+="\\\\";
index c9816db4e311d12ceb49546af00d587f9d6591d5..575f92339aa0a91cbec57b36f935f8c1e978a36e 100644 (file)
@@ -12,28 +12,28 @@ DNSPacketWriter::DNSPacketWriter(vector<uint8_t>& content, const DNSName& qname,
 {
   d_content.clear();
   dnsheader dnsheader;
-  
+
   memset(&dnsheader, 0, sizeof(dnsheader));
   dnsheader.id=0;
   dnsheader.qdcount=htons(1);
   dnsheader.opcode=opcode;
-  
+
   const uint8_t* ptr=(const uint8_t*)&dnsheader;
   uint32_t len=d_content.size();
   d_content.resize(len + sizeof(dnsheader));
   uint8_t* dptr=(&*d_content.begin()) + len;
-  
+
   memcpy(dptr, ptr, sizeof(dnsheader));
   d_stuff=0;
 
   xfrName(qname, false);
-  
+
   len=d_content.size();
   d_content.resize(len + d_record.size() + 4);
 
   ptr=&*d_record.begin();
   dptr=(&*d_content.begin()) + len;
-  
+
   memcpy(dptr, ptr, d_record.size());
 
   len+=d_record.size();
@@ -48,7 +48,7 @@ DNSPacketWriter::DNSPacketWriter(vector<uint8_t>& content, const DNSName& qname,
   memcpy(&*i, &qclass, 2);
 
   d_stuff=0xffff;
-  d_labelmap.reserve(16);
+  d_namemap.reserve(16);
   d_truncatemarker=d_content.size();
 }
 
@@ -59,7 +59,7 @@ dnsheader* DNSPacketWriter::getHeader()
 
 void DNSPacketWriter::startRecord(const DNSName& name, uint16_t qtype, uint32_t ttl, uint16_t qclass, Place place, bool compress)
 {
-  if(!d_record.empty()) 
+  if(!d_record.empty())
     commit();
 
   d_recordqname=name;
@@ -82,7 +82,7 @@ void DNSPacketWriter::startRecord(const DNSName& name, uint16_t qtype, uint32_t
   }
 
   d_stuff = sizeof(dnsrecordheader); // this is needed to get compressed label offsets right, the dnsrecordheader will be interspersed
-  d_sor=d_content.size() + d_stuff; // start of real record 
+  d_sor=d_content.size() + d_stuff; // start of real record
 }
 
 void DNSPacketWriter::addOpt(int udpsize, int extRCode, int Z, const vector<pair<uint16_t,string> >& options)
@@ -98,13 +98,13 @@ void DNSPacketWriter::addOpt(int udpsize, int extRCode, int Z, const vector<pair
   memcpy(&ttl, &stuff, sizeof(stuff));
 
   ttl=ntohl(ttl); // will be reversed later on
-  
+
   startRecord("", QType::OPT, ttl, udpsize, ADDITIONAL, false);
   for(optvect_t::const_iterator iter = options.begin(); iter != options.end(); ++iter) {
     xfr16BitInt(iter->first);
     xfr16BitInt(iter->second.length());
     xfrBlob(iter->second);
-  } 
+  }
 }
 
 void DNSPacketWriter::xfr48BitInt(uint64_t val)
@@ -139,7 +139,7 @@ void DNSPacketWriter::xfr8BitInt(uint8_t val)
 }
 
 
-/* input: 
+/* input:
   "" -> 0
   "blah" -> 4blah
   "blah" "blah" -> output 4blah4blah
@@ -160,11 +160,12 @@ void DNSPacketWriter::xfrText(const string& text, bool)
   }
 }
 
-DNSPacketWriter::lmap_t::iterator find(DNSPacketWriter::lmap_t& lmap, const string& label)
+/* FIXME400: check that this beats a map */
+DNSPacketWriter::nmap_t::iterator find(DNSPacketWriter::nmap_t& nmap, const DNSName& name)
 {
-  DNSPacketWriter::lmap_t::iterator ret;
-  for(ret=lmap.begin(); ret != lmap.end(); ++ret)
-    if(pdns_iequals(ret->first ,label))
+  DNSPacketWriter::nmap_t::iterator ret;
+  for(ret=nmap.begin(); ret != nmap.end(); ++ret)
+    if(pdns_iequals(ret->first ,name))
       break;
   return ret;
 }
@@ -189,15 +190,16 @@ DNSPacketWriter::lmap_t::iterator find(DNSPacketWriter::lmap_t& lmap, const stri
 //       lpos=pos+1;
 //     }
 //   }
-  
+
 //   if(lpos < pos)
 //     parts.push_back(make_pair(lpos, pos));
 //   return unescapedSomething;
 // }
 
-// this is the absolute hottest function in the pdns recursor 
+// this is the absolute hottest function in the pdns recursor
 void DNSPacketWriter::xfrName(const DNSName& name, bool compress)
 {
+  cerr<<"xfrName: name=["<<name.toString()<<"] compress="<<compress<<endl;
   // string label = d_lowerCase ? toLower(Label) : Label;
   // FIXME: we ignore d_lowerCase for now
   cerr<<"xfrName writing ["<<name.toString()<<"]"<<endl;
@@ -208,84 +210,66 @@ void DNSPacketWriter::xfrName(const DNSName& name, bool compress)
   if(d_canonic)
     compress=false;
 
-  // string::size_type labellen = label.size();
   if(!parts.size()) { // otherwise we encode '..'
     d_record.push_back(0);
     return;
   }
-  // bool unescaped=labeltokUnescape(parts, label); 
-  
+
   // d_stuff is amount of stuff that is yet to be written out - the dnsrecordheader for example
-  unsigned int pos=d_content.size() + d_record.size() + d_stuff; 
+  unsigned int pos=d_content.size() + d_record.size() + d_stuff;
   // bool deDot = labellen && (label[labellen-1]=='.'); // make sure we don't store trailing dots in the labelmap
 
   unsigned int startRecordSize=d_record.size();
   unsigned int startPos;
 
+  DNSName towrite = name;
+  /* FIXME400: if we are not compressing, there is no reason to work per-label */
   for(auto &label: parts) {
-    cerr<<"xfrName labelpart ["<<label<<"]"<<endl;
-    // if(deDot)
-    //   chopped.assign(label.c_str() + i->first, labellen - i->first -1);
-    // else
-    //   chopped.assign(label.c_str() + i->first);
+    cerr<<"xfrName labelpart ["<<label<<"], left to write ["<<towrite.toString()<<"]"<<endl;
 
-    lmap_t::iterator li=d_labelmap.end();
+    auto li=d_namemap.end();
     // see if we've written out this domain before
-    // cerr<<"Searching for compression pointer to '"<<chopped<<"', "<<d_labelmap.size()<<" cmp-records"<<endl;
-    // if(compress && (li=find(d_labelmap, label))!=d_labelmap.end()) {
-    //   cerr<<"doing compression, my label=["<<label<<"] found match ["<<li->first<<"]"<<endl;
-    //   // cerr<<"\tFound a compression pointer to '"<<chopped<<"': "<<li->second<<endl;
-    //   if (d_record.size() - startRecordSize + label.size() > 253) // chopped does not include a length octet for the first label and the root label
-    //     throw MOADNSException("DNSPacketWriter::xfrName() found overly large (compressed) name");
-    //   uint16_t offset=li->second;
-    //   offset|=0xc000;
-    //   d_record.push_back((char)(offset >> 8));
-    //   d_record.push_back((char)(offset & 0xff));
-    //   goto out;                                 // skip trailing 0 in case of compression
-    // }
-
-    if(li==d_labelmap.end() && pos< 16384) {
+    cerr<<"compress="<<compress<<", searching? for compression pointer to '"<<towrite.toString()<<"', "<<d_namemap.size()<<" cmp-records"<<endl;
+    if(compress && (li=find(d_namemap, towrite))!=d_namemap.end()) {
+      cerr<<"doing compression, my label=["<<label<<"] found match ["<<li->first.toString()<<"]"<<endl;
+      cerr<<"\tFound a compression pointer to '"<<towrite.toString()<<"': "<<li->second<<endl;
+      if (d_record.size() - startRecordSize + label.size() > 253) // chopped does not include a length octet for the first label and the root label
+        throw MOADNSException("DNSPacketWriter::xfrName() found overly large (compressed) name");
+      uint16_t offset=li->second;
+      offset|=0xc000;
+      d_record.push_back((char)(offset >> 8));
+      d_record.push_back((char)(offset & 0xff));
+      goto out;                                 // skip trailing 0 in case of compression
+    }
+
+    if(li==d_namemap.end() && pos< 16384) {
       //      cerr<<"\tStoring a compression pointer to '"<<chopped<<"': "<<pos<<endl;
-      d_labelmap.push_back(make_pair(label, pos));                       //  if untrue, we need to count - also, don't store offsets > 16384, won't work
+      d_namemap.push_back(make_pair(towrite, pos));                       //  if untrue, we need to count - also, don't store offsets > 16384, won't work
+      cerr<<"stored ["<<towrite.toString()<<"] at pos "<<pos<<endl;
     }
 
     startPos=pos;
 
-    // if(unescaped) {
-    //   string part(label.c_str() + i -> first, i->second - i->first);
-
-    //   // FIXME: this relies on the semi-canonical escaped output from getName
-    //   boost::replace_all(part, "\\.", ".");
-    //   boost::replace_all(part, "\\032", " ");
-    //   boost::replace_all(part, "\\\\", "\\"); 
-
-    //   d_record.push_back(part.size());
-    //   unsigned int len=d_record.size();
-    //   d_record.resize(len + part.size());
-    //   memcpy(((&*d_record.begin()) + len), part.c_str(), part.size());
-    //   pos+=(part.size())+1;
-    // }
-    // else {
-      char labelsize=label.size();
-      cerr<<"labelsize = "<<int(labelsize)<<" for label ["<<label<<"]"<<endl;
-      d_record.push_back(labelsize);
-      unsigned int len=d_record.size();
-      d_record.resize(len + labelsize);
-      memcpy(((&*d_record.begin()) + len), label.c_str(), labelsize); // FIXME do not want memcpy
-      pos+=labelsize+1;
-    // }
+    char labelsize=label.size();
+    cerr<<"labelsize = "<<int(labelsize)<<" for label ["<<label<<"]"<<endl;
+    d_record.push_back(labelsize);
+    unsigned int len=d_record.size();
+    d_record.resize(len + labelsize);
+    memcpy(((&*d_record.begin()) + len), label.c_str(), labelsize); // FIXME do not want memcpy
+    pos+=labelsize+1;
 
     if(pos - startPos == 1)
       throw MOADNSException("DNSPacketWriter::xfrName() found empty label in the middle of name");
     if(pos - startPos > 64)
       throw MOADNSException("DNSPacketWriter::xfrName() found overly large label in name");
+    towrite.chopOff();   /* FIXME400: iterating the label vector while keeping this chopoff in sync is a hack */
   }
   d_record.push_back(0); // insert root label
 
   if (d_record.size() - startRecordSize > 255)
     throw MOADNSException("DNSPacketWriter::xfrName() found overly large name");
 
// out:;
+ out:;
 }
 
 void DNSPacketWriter::xfrBlob(const string& blob, int  )
@@ -340,7 +324,7 @@ void DNSPacketWriter::commit()
   drh.d_class=htons(d_recordqclass);
   drh.d_ttl=htonl(d_recordttl);
   drh.d_clen=htons(d_record.size());
-  
+
   // and write out the header
   const uint8_t* ptr=(const uint8_t*)&drh;
   d_content.insert(d_content.end(), ptr, ptr+sizeof(drh));
@@ -365,9 +349,3 @@ void DNSPacketWriter::commit()
 
   d_record.clear();   // clear d_record, ready for next record
 }
-
-
-
-
-
-
index e7dbc9a1b054c64093f430d3560e601d8d665f05..6f20ad5c967f9dc123bacb56d8d92fe828728785 100644 (file)
@@ -8,7 +8,7 @@
 #include "dnsname.hh"
 #include "namespaces.hh"
 #include <arpa/inet.h>
-/** this class can be used to write DNS packets. It knows about DNS in the sense that it makes 
+/** this class can be used to write DNS packets. It knows about DNS in the sense that it makes
     the packet header and record headers.
 
     The model is:
@@ -16,7 +16,7 @@
     packetheader (recordheader recordcontent)*
 
     The packetheader needs to be updated with the amount of packets of each kind (answer, auth, additional)
-    
+
     Each recordheader contains the length of a dns record.
 
     Calling convention:
@@ -38,15 +38,15 @@ class DNSPacketWriter : public boost::noncopyable
 {
 
 public:
-  typedef vector<pair<string, uint16_t> > lmap_t;
-  enum Place {ANSWER=1, AUTHORITY=2, ADDITIONAL=3}; 
+  typedef vector<pair<DNSName, uint16_t> > nmap_t;
+  enum Place {ANSWER=1, AUTHORITY=2, ADDITIONAL=3};
 
   //! Start a DNS Packet in the vector passed, with question qname, qtype and qclass
   DNSPacketWriter(vector<uint8_t>& content, const DNSName& qname, uint16_t  qtype, uint16_t qclass=QClass::IN, uint8_t opcode=0);
-  
-  /** Start a new DNS record within this packet for namq, qtype, ttl, class and in the requested place. Note that packets can only be written in natural order - 
+
+  /** Start a new DNS record within this packet for namq, qtype, ttl, class and in the requested place. Note that packets can only be written in natural order -
       ANSWER, AUTHORITY, ADDITIONAL */
-  void startRecord(const DNSName& name, uint16_t qtype, uint32_t ttl=3600, uint16_t qclass=QClass::IN, Place place=ANSWER, bool compress=false);
+  void startRecord(const DNSName& name, uint16_t qtype, uint32_t ttl=3600, uint16_t qclass=QClass::IN, Place place=ANSWER, bool compress=true);
 
   /** Shorthand way to add an Opt-record, for example for EDNS0 purposes */
   typedef vector<pair<uint16_t,std::string> > optvect_t;
@@ -76,7 +76,7 @@ public:
   {
     xfr32BitInt(htonl(val));
   }
-  void xfrIP6(const std::string& val) 
+  void xfrIP6(const std::string& val)
   {
     xfrBlob(val,16);
   }
@@ -94,17 +94,17 @@ public:
   void xfrHexBlob(const string& blob, bool keepReading=false);
 
   uint16_t d_pos;
-  
+
   dnsheader* getHeader();
   void getRecords(string& records);
   const vector<uint8_t>& getRecordBeingWritten() { return d_record; }
 
-  void setCanonic(bool val) 
+  void setCanonic(bool val)
   {
     d_canonic=val;
   }
 
-  void setLowercase(bool val) 
+  void setLowercase(bool val)
   {
     d_lowerCase=val;
   }
@@ -121,7 +121,7 @@ private:
   DNSName d_recordqname;
   uint16_t d_recordqtype, d_recordqclass;
   uint32_t d_recordttl;
-  lmap_t d_labelmap;
+  nmap_t d_namemap;
   uint16_t d_stuff;
   uint16_t d_sor;
   uint16_t d_rollbackmarker; // start of last complete packet, for rollback