From: bert hubert Date: Fri, 26 Aug 2016 21:11:48 +0000 (+0200) Subject: remove d_record idea from DNSPacketWriter: write the packet directly now. Solves... X-Git-Tag: dnsdist-1.1.0-beta2~166^2~2 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e636cab291ad76131abf453cf90d410e09d27b1e;p=pdns remove d_record idea from DNSPacketWriter: write the packet directly now. Solves SOA-inter record compression bug. Still left to do: add check for names with more labels than we are prepared to handle. Plus handle that case. --- diff --git a/pdns/dnsparser.hh b/pdns/dnsparser.hh index 651dc6e61..79a29737b 100644 --- a/pdns/dnsparser.hh +++ b/pdns/dnsparser.hh @@ -186,10 +186,9 @@ public: pw.startRecord(qname, this->getType()); this->toPacket(pw); - pw.commit(); string record; - pw.getRecords(record); + pw.getRecordPayload(record); // needs to be called before commit() return record; } diff --git a/pdns/dnswriter.cc b/pdns/dnswriter.cc index 7d818dfd6..fb90cc145 100644 --- a/pdns/dnswriter.cc +++ b/pdns/dnswriter.cc @@ -29,6 +29,15 @@ #include +/* d_content: <---- d_stuff ----> + v d_truncatemarker + dnsheader | qname | qtype | qclass | {recordname| dnsrecordheader | record } + ^ d_rollbackmarker ^ d_sor + + +*/ + + DNSPacketWriter::DNSPacketWriter(vector& content, const DNSName& qname, uint16_t qtype, uint16_t qclass, uint8_t opcode) : d_content(content), d_qname(qname), d_canonic(false), d_lowerCase(false) { @@ -46,37 +55,14 @@ DNSPacketWriter::DNSPacketWriter(vector& content, const DNSName& qname, uint8_t* dptr=(&*d_content.begin()) + len; memcpy(dptr, ptr, sizeof(dnsheader)); - d_stuff=0; d_namepositions.reserve(16); xfrName(qname, false); + xfr16BitInt(qtype); + xfr16BitInt(qclass); - 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(); - d_record.clear(); - - qtype=htons(qtype); - qclass=htons(qclass); - - vector::iterator i=d_content.begin()+len; // this works around a gcc 3.4 bug - memcpy(&*i, &qtype, 2); - i+=2; - memcpy(&*i, &qclass, 2); - - d_stuff=0xffff; d_truncatemarker=d_content.size(); d_sor = 0; d_rollbackmarker = 0; - d_recordttl = 0; - d_recordqtype = 0; - d_recordqclass = QClass::IN; - d_recordplace = DNSResourceRecord::ANSWER; } dnsheader* DNSPacketWriter::getHeader() @@ -84,32 +70,25 @@ dnsheader* DNSPacketWriter::getHeader() return reinterpret_cast(&*d_content.begin()); } + void DNSPacketWriter::startRecord(const DNSName& name, uint16_t qtype, uint32_t ttl, uint16_t qclass, DNSResourceRecord::Place place, bool compress) { - if(!d_record.empty()) - commit(); - - d_recordqname=name; - d_recordqtype=qtype; - d_recordqclass=qclass; - d_recordttl=ttl; - d_recordplace=place; - - d_stuff = 0; + commit(); d_rollbackmarker=d_content.size(); - if(compress && d_recordqname.countLabels() && d_qname==d_recordqname) { // don't do the whole label compression thing if we *know* we can get away with "see question" - except when compressing the root + if(compress && !name.empty() && d_qname==name) { // don't do the whole label compression thing if we *know* we can get away with "see question" - except when compressing the root static unsigned char marker[2]={0xc0, 0x0c}; d_content.insert(d_content.end(), (const char *) &marker[0], (const char *) &marker[2]); } else { - xfrName(d_recordqname, compress); - d_content.insert(d_content.end(), d_record.begin(), d_record.end()); - d_record.clear(); + xfrName(name, compress); } - - 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 + xfr16BitInt(qtype); + xfr16BitInt(qclass); + xfr32BitInt(ttl); + xfr16BitInt(0); // this will be the record size + d_recordplace = place; + d_sor=d_content.size(); // this will remind us where to stuff the record size } void DNSPacketWriter::addOpt(uint16_t udpsize, int extRCode, int Z, const vector >& options) @@ -143,7 +122,7 @@ void DNSPacketWriter::xfr48BitInt(uint64_t val) memcpy(bytes, (void*)&theLeft, sizeof(theLeft)); memcpy(bytes+2, (void*)&theRight, sizeof(theRight)); - d_record.insert(d_record.end(), bytes, bytes + sizeof(bytes)); + d_content.insert(d_content.end(), bytes, bytes + sizeof(bytes)); } @@ -151,19 +130,19 @@ void DNSPacketWriter::xfr32BitInt(uint32_t val) { int rval=htonl(val); uint8_t* ptr=reinterpret_cast(&rval); - d_record.insert(d_record.end(), ptr, ptr+4); + d_content.insert(d_content.end(), ptr, ptr+4); } void DNSPacketWriter::xfr16BitInt(uint16_t val) { uint16_t rval=htons(val); uint8_t* ptr=reinterpret_cast(&rval); - d_record.insert(d_record.end(), ptr, ptr+2); + d_content.insert(d_content.end(), ptr, ptr+2); } void DNSPacketWriter::xfr8BitInt(uint8_t val) { - d_record.push_back(val); + d_content.push_back(val); } @@ -183,26 +162,26 @@ void DNSPacketWriter::xfr8BitInt(uint8_t val) void DNSPacketWriter::xfrText(const string& text, bool, bool lenField) { if(text.empty()) { - d_record.push_back(0); + d_content.push_back(0); return; } vector segments = segmentDNSText(text); for(const string& str : segments) { if(lenField) - d_record.push_back(str.length()); - d_record.insert(d_record.end(), str.c_str(), str.c_str() + str.length()); + d_content.push_back(str.length()); + d_content.insert(d_content.end(), str.c_str(), str.c_str() + str.length()); } } void DNSPacketWriter::xfrUnquotedText(const string& text, bool lenField) { if(text.empty()) { - d_record.push_back(0); + d_content.push_back(0); return; } if(lenField) - d_record.push_back(text.length()); - d_record.insert(d_record.end(), text.c_str(), text.c_str() + text.length()); + d_content.push_back(text.length()); + d_content.insert(d_content.end(), text.c_str(), text.c_str() + text.length()); } @@ -239,28 +218,15 @@ uint16_t DNSPacketWriter::lookupName(const DNSName& name, uint16_t* matchLen) cout<<"Have "<* source=0; - if(p < d_content.size()) - source = &d_content; - else { - source = &d_record; - p-= (d_content.size() + d_stuff); - - } if(l_verbose) { - if(source == &d_content) { - DNSName pname((const char*)&(*source)[0], (*source).size(), p, true); // only for debugging - cout<<"Looking at '"<> 8)); - d_record.push_back((char)(offset & 0xff)); + d_content.push_back((char)(offset >> 8)); + d_content.push_back((char)(offset & 0xff)); } else { - unsigned int pos=d_content.size() + d_record.size() + d_stuff; + unsigned int pos=d_content.size(); if(l_verbose) cout<<"Found nothing, we are at pos "<(blob.c_str()); - d_record.insert(d_record.end(), ptr, ptr+blob.size()); + d_content.insert(d_content.end(), ptr, ptr+blob.size()); } void DNSPacketWriter::xfrBlobNoSpaces(const string& blob, int ) @@ -389,52 +355,37 @@ void DNSPacketWriter::xfrHexBlob(const string& blob, bool keepReading) xfrBlob(blob); } -void DNSPacketWriter::getRecords(string& records) +// call __before commit__ +void DNSPacketWriter::getRecordPayload(string& records) { records.assign(d_content.begin() + d_sor, d_content.end()); } uint32_t DNSPacketWriter::size() { - return d_content.size() + d_stuff + d_record.size(); + return d_content.size(); } void DNSPacketWriter::rollback() { d_content.resize(d_rollbackmarker); - d_record.clear(); - d_stuff=0; } void DNSPacketWriter::truncate() { d_content.resize(d_truncatemarker); - d_record.clear(); - d_stuff=0; dnsheader* dh=reinterpret_cast( &*d_content.begin()); dh->ancount = dh->nscount = dh->arcount = 0; } void DNSPacketWriter::commit() { - if(d_stuff==0xffff && (d_content.size()!=d_sor || !d_record.empty())) - throw MOADNSException("DNSPacketWriter::commit() called without startRecord ever having been called, but a record was added"); - // build dnsrecordheader - struct dnsrecordheader drh; - drh.d_type=htons(d_recordqtype); - 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)); - - d_stuff=0; - - // write out pending d_record - d_content.insert(d_content.end(), d_record.begin(), d_record.end()); - + if(!d_sor) + return; + uint16_t rlen = d_content.size() - d_sor; + d_content[d_sor-2]=rlen >> 8; + d_content[d_sor-1]=rlen & 0xff; + d_sor=0; dnsheader* dh=reinterpret_cast( &*d_content.begin()); switch(d_recordplace) { case DNSResourceRecord::QUESTION: @@ -451,5 +402,4 @@ void DNSPacketWriter::commit() break; } - d_record.clear(); // clear d_record, ready for next record } diff --git a/pdns/dnswriter.hh b/pdns/dnswriter.hh index fd75c27b1..024cfae9e 100644 --- a/pdns/dnswriter.hh +++ b/pdns/dnswriter.hh @@ -115,8 +115,8 @@ public: void xfrHexBlob(const string& blob, bool keepReading=false); dnsheader* getHeader(); - void getRecords(string& records); - const vector& getRecordBeingWritten() { return d_record; } + void getRecordPayload(string& records); // call __before commit__ + const vector getRecordBeingWritten() { return vector(d_content.begin()+d_sor, d_content.end()); } void setCanonic(bool val) { @@ -137,17 +137,11 @@ private: uint16_t lookupName(const DNSName& name, uint16_t* matchlen); vector d_namepositions; // We declare 1 uint_16 in the public section, these 3 align on a 8-byte boundry - uint16_t d_stuff; uint16_t d_sor; uint16_t d_rollbackmarker; // start of last complete packet, for rollback vector & d_content; - vector d_record; DNSName d_qname; - DNSName d_recordqname; - - uint32_t d_recordttl; - uint16_t d_recordqtype, d_recordqclass; uint16_t d_truncatemarker; // end of header, for truncate DNSResourceRecord::Place d_recordplace;