Skip to content

Commit eaac1e8

Browse files
SimonWilkinsondevyte
SimonWilkinson
authored and
devyte
committed
Rework DNSServer to be more robust (#5573)
* DNSServer: Handle examplewww.com correctly Just replacing 'www.' with the empty string when we assign the domainname will remove all occurrences of 'www.', not just those at the start of the string. Change this to a startsWith check so that only "www." at the beginning of the string is removed. * DNSServer: Rework request handling Rewrite the request handling in the DNSServer code to address the following issues: Compatibility with EDNS #1: RFC6891 says that "Responders that choose not to implement the protocol extensions defined in this document MUST respond with a return code (RCODE) of FORMERR to messages containing an OPT record in the additional section and MUST NOT include an OPT record in the response" If we have any additional records in the request, then we need to return a FORMERR, and not whatever custom error code the user may have set. Compatibility with EDNS #2: If we're returning an error, we need to explicitly zero all of the record counters. In the existing code, if there is an additional record present in the request, we return an ARCOUNT of 1 in the response, despite including no additional records in the payload. Don't answer non-A requests If we receive an AAAA request (or any other non-A record) requests, we shouldn't respond to it with an A record. Don't answer non-IN requests If we receive a request for a non-IN type, don't answer it (it's unlikely that we'd see this in the real world) Don't read off the end of malformed packets If a packet claims to have a query, but then doesn't include one, or includes a query with malformed labels, don't read off the end of the allocated data structure. * DNSServer: Clarify and tidy writing the answer record Modify the code used to write the answer record back to the server so that it is clearer that we are writing network byte order 16-bit quantities, and to clarify what's happening with the pointer used at the start of the answer.
1 parent 2f0f49d commit eaac1e8

File tree

2 files changed

+188
-112
lines changed

2 files changed

+188
-112
lines changed

Diff for: ‎libraries/DNSServer/src/DNSServer.cpp

+169-107
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
11
#include "DNSServer.h"
22
#include <lwip/def.h>
33
#include <Arduino.h>
4+
#include <memory>
45

56
#ifdef DEBUG_ESP_PORT
67
#define DEBUG_OUTPUT DEBUG_ESP_PORT
78
#else
89
#define DEBUG_OUTPUT Serial
910
#endif
1011

12+
#define DNS_HEADER_SIZE sizeof(DNSHeader)
13+
1114
DNSServer::DNSServer()
1215
{
1316
_ttl = lwip_htonl(60);
@@ -46,149 +49,208 @@ void DNSServer::stop()
4649
void DNSServer::downcaseAndRemoveWwwPrefix(String &domainName)
4750
{
4851
domainName.toLowerCase();
49-
domainName.replace("www.", "");
52+
if (domainName.startsWith("www."))
53+
domainName.remove(0, 4);
5054
}
5155

52-
void DNSServer::processNextRequest()
56+
void DNSServer::respondToRequest(uint8_t *buffer, size_t length)
5357
{
54-
size_t packetSize = _udp.parsePacket();
58+
DNSHeader *dnsHeader;
59+
uint8_t *query, *start;
60+
const char *matchString;
61+
size_t remaining, labelLength, queryLength;
62+
uint16_t qtype, qclass;
63+
64+
dnsHeader = (DNSHeader *)buffer;
5565

56-
if (packetSize >= sizeof(DNSHeader))
57-
{
58-
uint8_t* buffer = reinterpret_cast<uint8_t*>(malloc(packetSize));
59-
if (buffer == NULL) return;
66+
// Must be a query for us to do anything with it
67+
if (dnsHeader->QR != DNS_QR_QUERY)
68+
return;
6069

61-
_udp.read(buffer, packetSize);
70+
// If operation is anything other than query, we don't do it
71+
if (dnsHeader->OPCode != DNS_OPCODE_QUERY)
72+
return replyWithError(dnsHeader, DNSReplyCode::NotImplemented);
73+
74+
// Only support requests containing single queries - everything else
75+
// is badly defined
76+
if (dnsHeader->QDCount != lwip_htons(1))
77+
return replyWithError(dnsHeader, DNSReplyCode::FormError);
78+
79+
// We must return a FormError in the case of a non-zero ARCount to
80+
// be minimally compatible with EDNS resolvers
81+
if (dnsHeader->ANCount != 0 || dnsHeader->NSCount != 0
82+
|| dnsHeader->ARCount != 0)
83+
return replyWithError(dnsHeader, DNSReplyCode::FormError);
84+
85+
// Even if we're not going to use the query, we need to parse it
86+
// so we can check the address type that's being queried
87+
88+
query = start = buffer + DNS_HEADER_SIZE;
89+
remaining = length - DNS_HEADER_SIZE;
90+
while (remaining != 0 && *start != 0) {
91+
labelLength = *start;
92+
if (labelLength + 1 > remaining)
93+
return replyWithError(dnsHeader, DNSReplyCode::FormError);
94+
remaining -= (labelLength + 1);
95+
start += (labelLength + 1);
96+
}
6297

63-
DNSHeader* dnsHeader = reinterpret_cast<DNSHeader*>(buffer);
98+
// 1 octet labelLength, 2 octet qtype, 2 octet qclass
99+
if (remaining < 5)
100+
return replyWithError(dnsHeader, DNSReplyCode::FormError);
64101

65-
if (dnsHeader->QR == DNS_QR_QUERY &&
66-
dnsHeader->OPCode == DNS_OPCODE_QUERY &&
67-
requestIncludesOnlyOneQuestion(dnsHeader) &&
68-
(_domainName == "*" || getDomainNameWithoutWwwPrefix(buffer, packetSize) == _domainName)
69-
)
70-
{
71-
replyWithIP(buffer, packetSize);
72-
}
73-
else if (dnsHeader->QR == DNS_QR_QUERY)
74-
{
75-
replyWithCustomCode(buffer, packetSize);
102+
start += 1; // Skip the 0 length label that we found above
103+
104+
memcpy(&qtype, start, sizeof(qtype));
105+
start += 2;
106+
memcpy(&qclass, start, sizeof(qclass));
107+
start += 2;
108+
109+
queryLength = start - query;
110+
111+
if (qclass != lwip_htons(DNS_QCLASS_ANY)
112+
&& qclass != lwip_htons(DNS_QCLASS_IN))
113+
return replyWithError(dnsHeader, DNSReplyCode::NonExistentDomain,
114+
query, queryLength);
115+
116+
if (qtype != lwip_htons(DNS_QTYPE_A)
117+
&& qtype != lwip_htons(DNS_QTYPE_ANY))
118+
return replyWithError(dnsHeader, DNSReplyCode::NonExistentDomain,
119+
query, queryLength);
120+
121+
// If we have no domain name configured, just return an error
122+
if (_domainName == "")
123+
return replyWithError(dnsHeader, _errorReplyCode,
124+
query, queryLength);
125+
126+
// If we're running with a wildcard we can just return a result now
127+
if (_domainName == "*")
128+
return replyWithIP(dnsHeader, query, queryLength);
129+
130+
matchString = _domainName.c_str();
131+
132+
start = query;
133+
134+
// If there's a leading 'www', skip it
135+
if (*start == 3 && strncasecmp("www", (char *) start + 1, 3) == 0)
136+
start += 4;
137+
138+
while (*start != 0) {
139+
labelLength = *start;
140+
start += 1;
141+
while (labelLength > 0) {
142+
if (tolower(*start) != *matchString)
143+
return replyWithError(dnsHeader, _errorReplyCode,
144+
query, queryLength);
145+
++start;
146+
++matchString;
147+
--labelLength;
76148
}
149+
if (*start == 0 && *matchString == '\0')
150+
return replyWithIP(dnsHeader, query, queryLength);
77151

78-
free(buffer);
152+
if (*matchString != '.')
153+
return replyWithError(dnsHeader, _errorReplyCode,
154+
query, queryLength);
155+
++matchString;
79156
}
80-
}
81157

82-
bool DNSServer::requestIncludesOnlyOneQuestion(const DNSHeader* dnsHeader)
83-
{
84-
return lwip_ntohs(dnsHeader->QDCount) == 1 &&
85-
dnsHeader->ANCount == 0 &&
86-
dnsHeader->NSCount == 0 &&
87-
dnsHeader->ARCount == 0;
158+
return replyWithError(dnsHeader, _errorReplyCode,
159+
query, queryLength);
88160
}
89161

90-
String DNSServer::getDomainNameWithoutWwwPrefix(const uint8_t* buffer, size_t packetSize)
162+
void DNSServer::processNextRequest()
91163
{
92-
String parsedDomainName;
93-
94-
const uint8_t* pos = buffer + sizeof(DNSHeader);
95-
const uint8_t* end = buffer + packetSize;
96-
97-
// to minimize reallocations due to concats below
98-
// we reserve enough space that a median or average domain
99-
// name size cold be easily contained without a reallocation
100-
// - max size would be 253, in 2013, average is 11 and max was 42
101-
//
102-
parsedDomainName.reserve(32);
103-
104-
uint8_t labelLength = *pos;
105-
106-
while (true)
107-
{
108-
if (labelLength == 0)
109-
{
110-
// no more labels
111-
downcaseAndRemoveWwwPrefix(parsedDomainName);
112-
return parsedDomainName;
113-
}
164+
size_t currentPacketSize;
114165

115-
// append next label
116-
for (int i = 0; i < labelLength && pos < end; i++)
117-
{
118-
pos++;
119-
parsedDomainName += static_cast<char>(*pos);
120-
}
166+
currentPacketSize = _udp.parsePacket();
167+
if (currentPacketSize == 0)
168+
return;
121169

122-
if (pos >= end)
123-
{
124-
// malformed packet, return an empty domain name
125-
parsedDomainName = "";
126-
return parsedDomainName;
127-
}
128-
else
129-
{
130-
// next label
131-
pos++;
132-
labelLength = *pos;
133-
134-
// if there is another label, add delimiter
135-
if (labelLength != 0)
136-
{
137-
parsedDomainName += ".";
138-
}
139-
}
140-
}
170+
// The DNS RFC requires that DNS packets be less than 512 bytes in size,
171+
// so just discard them if they are larger
172+
if (currentPacketSize > MAX_DNS_PACKETSIZE)
173+
return;
174+
175+
// If the packet size is smaller than the DNS header, then someone is
176+
// messing with us
177+
if (currentPacketSize < DNS_HEADER_SIZE)
178+
return;
179+
180+
std::unique_ptr<uint8_t[]> buffer(new (std::nothrow) uint8_t[currentPacketSize]);
181+
182+
if (buffer == NULL)
183+
return;
184+
185+
_udp.read(buffer.get(), currentPacketSize);
186+
respondToRequest(buffer.get(), currentPacketSize);
187+
}
188+
189+
void DNSServer::writeNBOShort(uint16_t value)
190+
{
191+
_udp.write((unsigned char *)&value, 2);
141192
}
142193

143-
void DNSServer::replyWithIP(uint8_t* buffer, size_t packetSize)
194+
void DNSServer::replyWithIP(DNSHeader *dnsHeader,
195+
unsigned char * query,
196+
size_t queryLength)
144197
{
145-
DNSHeader* dnsHeader = reinterpret_cast<DNSHeader*>(buffer);
198+
uint16_t value;
146199

147200
dnsHeader->QR = DNS_QR_RESPONSE;
148-
dnsHeader->ANCount = dnsHeader->QDCount;
149-
dnsHeader->QDCount = dnsHeader->QDCount;
150-
//dnsHeader->RA = 1;
201+
dnsHeader->QDCount = lwip_htons(1);
202+
dnsHeader->ANCount = lwip_htons(1);
203+
dnsHeader->NSCount = 0;
204+
dnsHeader->ARCount = 0;
151205

152206
_udp.beginPacket(_udp.remoteIP(), _udp.remotePort());
153-
_udp.write(buffer, packetSize);
207+
_udp.write((unsigned char *) dnsHeader, sizeof(DNSHeader));
208+
_udp.write(query, queryLength);
209+
210+
// Rather than restate the name here, we use a pointer to the name contained
211+
// in the query section. Pointers have the top two bits set.
212+
value = 0xC000 | DNS_HEADER_SIZE;
213+
writeNBOShort(lwip_htons(value));
154214

155-
_udp.write((uint8_t)192); // answer name is a pointer
156-
_udp.write((uint8_t)12); // pointer to offset at 0x00c
215+
// Answer is type A (an IPv4 address)
216+
writeNBOShort(lwip_htons(DNS_QTYPE_A));
157217

158-
_udp.write((uint8_t)0); // 0x0001 answer is type A query (host address)
159-
_udp.write((uint8_t)1);
218+
// Answer is in the Internet Class
219+
writeNBOShort(lwip_htons(DNS_QCLASS_IN));
160220

161-
_udp.write((uint8_t)0); //0x0001 answer is class IN (internet address)
162-
_udp.write((uint8_t)1);
163-
221+
// Output TTL (already NBO)
164222
_udp.write((unsigned char*)&_ttl, 4);
165223

166224
// Length of RData is 4 bytes (because, in this case, RData is IPv4)
167-
_udp.write((uint8_t)0);
168-
_udp.write((uint8_t)4);
225+
writeNBOShort(lwip_htons(sizeof(_resolvedIP)));
169226
_udp.write(_resolvedIP, sizeof(_resolvedIP));
170227
_udp.endPacket();
171-
172-
#ifdef DEBUG_ESP_DNS
173-
DEBUG_OUTPUT.printf("DNS responds: %s for %s\n",
174-
IPAddress(_resolvedIP).toString().c_str(), getDomainNameWithoutWwwPrefix(buffer, packetSize).c_str() );
175-
#endif
176228
}
177229

178-
void DNSServer::replyWithCustomCode(uint8_t* buffer, size_t packetSize)
230+
void DNSServer::replyWithError(DNSHeader *dnsHeader,
231+
DNSReplyCode rcode,
232+
unsigned char *query,
233+
size_t queryLength)
179234
{
180-
if (packetSize < sizeof(DNSHeader))
181-
{
182-
return;
183-
}
184-
185-
DNSHeader* dnsHeader = reinterpret_cast<DNSHeader*>(buffer);
186-
187235
dnsHeader->QR = DNS_QR_RESPONSE;
188-
dnsHeader->RCode = (unsigned char)_errorReplyCode;
189-
dnsHeader->QDCount = 0;
236+
dnsHeader->RCode = (unsigned char) rcode;
237+
if (query)
238+
dnsHeader->QDCount = lwip_htons(1);
239+
else
240+
dnsHeader->QDCount = 0;
241+
dnsHeader->ANCount = 0;
242+
dnsHeader->NSCount = 0;
243+
dnsHeader->ARCount = 0;
190244

191245
_udp.beginPacket(_udp.remoteIP(), _udp.remotePort());
192-
_udp.write(buffer, sizeof(DNSHeader));
246+
_udp.write((unsigned char *)dnsHeader, sizeof(DNSHeader));
247+
if (query != NULL)
248+
_udp.write(query, queryLength);
193249
_udp.endPacket();
194250
}
251+
252+
void DNSServer::replyWithError(DNSHeader *dnsHeader,
253+
DNSReplyCode rcode)
254+
{
255+
replyWithError(dnsHeader, rcode, NULL, 0);
256+
}

Diff for: ‎libraries/DNSServer/src/DNSServer.h

+19-5
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,14 @@
66
#define DNS_QR_RESPONSE 1
77
#define DNS_OPCODE_QUERY 0
88

9+
#define DNS_QCLASS_IN 1
10+
#define DNS_QCLASS_ANY 255
11+
12+
#define DNS_QTYPE_A 1
13+
#define DNS_QTYPE_ANY 255
14+
915
#define MAX_DNSNAME_LENGTH 253
16+
#define MAX_DNS_PACKETSIZE 512
1017

1118
enum class DNSReplyCode
1219
{
@@ -65,9 +72,16 @@ class DNSServer
6572
DNSReplyCode _errorReplyCode;
6673

6774
void downcaseAndRemoveWwwPrefix(String &domainName);
68-
String getDomainNameWithoutWwwPrefix(const uint8_t* buffer, size_t packetSize);
69-
bool requestIncludesOnlyOneQuestion(const DNSHeader* dnsHeader);
70-
void replyWithIP(uint8_t* buffer, size_t packetSize);
71-
void replyWithCustomCode(uint8_t* buffer, size_t packetSize);
75+
void replyWithIP(DNSHeader *dnsHeader,
76+
unsigned char * query,
77+
size_t queryLength);
78+
void replyWithError(DNSHeader *dnsHeader,
79+
DNSReplyCode rcode,
80+
unsigned char *query,
81+
size_t queryLength);
82+
void replyWithError(DNSHeader *dnsHeader,
83+
DNSReplyCode rcode);
84+
void respondToRequest(uint8_t *buffer, size_t length);
85+
void writeNBOShort(uint16_t value);
7286
};
73-
#endif
87+
#endif

0 commit comments

Comments
 (0)