diff --git a/tdns/README.md b/tdns/README.md index ad19500..6411ac0 100644 --- a/tdns/README.md +++ b/tdns/README.md @@ -57,10 +57,10 @@ Known broken: * ~~RCode after one CNAME chase~~ * ~~On output (to screen) we do not escape DNS names correctly~~ * TCP/IP does not follow recommended timeouts - * EDNS is a bit clunky and should move into DNSMessageWriter + * ~~EDNS is a bit clunky and should move into DNSMessageWriter~~ -The code is not quite in a teachable state and still contains ugly bits. But -well worth [a +The code is not quite in a teachable state yet and still contains ugly bits. +But well worth [a read](https://github.com/ahupowerdns/hello-dns/tree/master/tdns). # Layout @@ -694,9 +694,8 @@ such a larger buffer size, a packet may exceed the available space. In that case, the standard tells us to truncate the packet, and then still put an EDNS record in the response. -This is why the code is currently littered with 'if(haveEDNS)' in a number -of places. This will be moved into `DNSMessageWriter` soon. - +The DNSMessageWriter, in somewhat of a layering violation, takes care of +this in `serialize()`. # Internals `tdns` uses several small pieces of code not core to dns: diff --git a/tdns/dns.hh b/tdns/dns.hh index d346dc5..167362a 100644 --- a/tdns/dns.hh +++ b/tdns/dns.hh @@ -1,7 +1,7 @@ #pragma once struct dnsheader { unsigned id :16; /* query identification number */ -#if BYTE_ORDER == BIG_ENDIAN +#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ /* fields in third byte */ unsigned qr: 1; /* response flag */ unsigned opcode: 4; /* purpose of message */ @@ -14,7 +14,7 @@ struct dnsheader { unsigned ad: 1; /* authentic data from named */ unsigned cd: 1; /* checking disabled by resolver */ unsigned rcode :4; /* response code */ -#elif BYTE_ORDER == LITTLE_ENDIAN || BYTE_ORDER == PDP_ENDIAN +#elif __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ /* fields in third byte */ unsigned rd :1; /* recursion desired */ unsigned tc :1; /* truncated message */ diff --git a/tdns/dnsmessages.cc b/tdns/dnsmessages.cc index 3382b6e..dd92ccb 100644 --- a/tdns/dnsmessages.cc +++ b/tdns/dnsmessages.cc @@ -21,6 +21,7 @@ DNSMessageReader::DNSMessageReader(const char* in, uint16_t size) d_doBit = flags & 0x80; payload.getUInt8(); payload.getUInt16(); // ignore rest cout<<" There was an EDNS section, size supported: "<< d_bufsize< sizeof(dnsheader)) + payload.resize(newsize - sizeof(dnsheader)); + d_doBit = doBit; + haveEDNS=true; +} diff --git a/tdns/dnsmessages.hh b/tdns/dnsmessages.hh index 9d80df0..c15fa16 100644 --- a/tdns/dnsmessages.hh +++ b/tdns/dnsmessages.hh @@ -14,7 +14,6 @@ public: void getQuestion(DNSName& name, DNSType& type) const; bool getEDNS(uint16_t* newsize, bool* doBit) const; - std::string serialize() const; private: DNSName getName(); @@ -26,20 +25,23 @@ private: bool d_haveEDNS; }; -struct DNSMessageWriter +class DNSMessageWriter { +public: struct dnsheader dh=dnsheader{}; std::vector payload; uint16_t payloadpos=0; + DNSName d_qname; + DNSType d_qtype; + DNSClass d_qclass; + bool haveEDNS{false}; + bool d_doBit; - explicit DNSMessageWriter(int maxsize=500) - { - payload.resize(maxsize); - } - - void setQuestion(const DNSName& name, DNSType type); + DNSMessageWriter(const DNSName& name, DNSType type, int maxsize=500); + + void clearRRs(); void putRR(DNSSection section, const DNSName& name, DNSType type, uint32_t ttl, const std::unique_ptr& rr); - void putEDNS(uint16_t bufsize, bool doBit); + void setEDNS(uint16_t bufsize, bool doBit); std::string serialize() const; void putUInt8(uint8_t val) @@ -87,5 +89,8 @@ struct DNSMessageWriter } putUInt8(0); } + +private: + void putEDNS(uint16_t bufsize, bool doBit); }; diff --git a/tdns/tdns.cc b/tdns/tdns.cc index 261c02b..523b747 100644 --- a/tdns/tdns.cc +++ b/tdns/tdns.cc @@ -47,39 +47,25 @@ bool processQuestion(const DNSNode& zones, DNSMessageReader& dm, const ComboAddr DNSName qname; DNSType qtype; dm.getQuestion(qname, qtype); + DNSName origname=qname; // we need this for error reporting, we munch the original name - bool haveEDNS=false; cout<<"Received a query from "< sizeof(dnsheader)) - response.payload.resize(newsize - sizeof(dnsheader)); - try { - response.dh = dm.dh; + response.dh.id = dm.dh.id; response.dh.ad = response.dh.ra = response.dh.aa = 0; response.dh.qr = 1; - response.setQuestion(qname, qtype); if(qtype == DNSType::AXFR || qtype == DNSType::IXFR) { cout<<"Query was for AXFR or IXFR over UDP, can't do that"<zone) { cout<<"No zone matched"<::max()-sizeof(dnsheader)); + DNSMessageWriter response(name, type, std::numeric_limits::max()); if(type == DNSType::AXFR) { cout<<"Should do AXFR for "<find(name, zone); @@ -287,7 +270,7 @@ void tcpClientThread(ComboAddress local, ComboAddress remote, int s, const DNSNo response.putRR(DNSSection::Answer, zone, DNSType::SOA, node->rrsets[DNSType::SOA].ttl, node->rrsets[DNSType::SOA].contents[0]); writeTCPResponse(sock, response); - response.setQuestion(zone, type); + response.clearRRs(); // send all other records node->visit([&response,&sock,&name,&type,&zone](const DNSName& nname, const DNSNode* n) { @@ -301,7 +284,7 @@ void tcpClientThread(ComboAddress local, ComboAddress remote, int s, const DNSNo } catch(std::out_of_range& e) { // exceeded packet size writeTCPResponse(sock, response); - response.setQuestion(zone, type); + response.clearRRs(); goto retry; } } @@ -309,7 +292,7 @@ void tcpClientThread(ComboAddress local, ComboAddress remote, int s, const DNSNo }, zone); writeTCPResponse(sock, response); - response.setQuestion(zone, type); + response.clearRRs(); // send SOA again response.putRR(DNSSection::Answer, zone, DNSType::SOA, node->rrsets[DNSType::SOA].ttl, node->rrsets[DNSType::SOA].contents[0]); @@ -318,8 +301,6 @@ void tcpClientThread(ComboAddress local, ComboAddress remote, int s, const DNSNo return; } else { - dm.payload.rewind(); - if(processQuestion(*zones, dm, local, remote, response)) { writeTCPResponse(sock, response); }