From: Peter van Dijk Date: Tue, 23 Jun 2015 15:07:04 +0000 (+0200) Subject: fix compression with DNSName. leaves two broken tests in gsqlite3-dnssec X-Git-Tag: dnsdist-1.0.0-alpha1~248^2~58^2~21^2~5^2~22 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=950cfe0fcfa98ed16699266ce026d0b51059684f;p=pdns fix compression with DNSName. leaves two broken tests in gsqlite3-dnssec --- diff --git a/pdns/dnsname.cc b/pdns/dnsname.cc index 3b4f13038..aeabf9f8e 100644 --- a/pdns/dnsname.cc +++ b/pdns/dnsname.cc @@ -11,11 +11,13 @@ /* 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+="\\\\"; diff --git a/pdns/dnswriter.cc b/pdns/dnswriter.cc index c9816db4e..575f92339 100644 --- a/pdns/dnswriter.cc +++ b/pdns/dnswriter.cc @@ -12,28 +12,28 @@ DNSPacketWriter::DNSPacketWriter(vector& 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& 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 >& options) @@ -98,13 +98,13 @@ void DNSPacketWriter::addOpt(int udpsize, int extRCode, int Z, const vectorfirst); 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=["<first, labellen - i->first -1); - // else - // chopped.assign(label.c_str() + i->first); + cerr<<"xfrName labelpart ["<first<<"]"<second< 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="<first.toString()<<"]"<second< 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 '"< 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 ["< 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 = "< 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 } - - - - - - diff --git a/pdns/dnswriter.hh b/pdns/dnswriter.hh index e7dbc9a1b..6f20ad5c9 100644 --- a/pdns/dnswriter.hh +++ b/pdns/dnswriter.hh @@ -8,7 +8,7 @@ #include "dnsname.hh" #include "namespaces.hh" #include -/** 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 > lmap_t; - enum Place {ANSWER=1, AUTHORITY=2, ADDITIONAL=3}; + typedef vector > 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& 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 > 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& 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