-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Proper EDNS handling and cleaner NOERROR response in DNSSERVER #11411
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 1 commit
730b9ce
5323ee2
cfa5ac5
557d9c7
c400f34
e64e96a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -15,7 +15,7 @@ | |||||||||
DNSServer::DNSServer() : _port(DNS_DEFAULT_PORT), _ttl(htonl(DNS_DEFAULT_TTL)), _errorReplyCode(DNSReplyCode::NonExistentDomain) {} | ||||||||||
|
||||||||||
DNSServer::DNSServer(const String &domainName) | ||||||||||
: _port(DNS_DEFAULT_PORT), _ttl(htonl(DNS_DEFAULT_TTL)), _errorReplyCode(DNSReplyCode::NonExistentDomain), _domainName(domainName){}; | ||||||||||
: _port(DNS_DEFAULT_PORT), _ttl(htonl(DNS_DEFAULT_TTL)), _errorReplyCode(DNSReplyCode::NonExistentDomain), _domainName(domainName) {}; | ||||||||||
|
||||||||||
bool DNSServer::start() { | ||||||||||
if (_resolvedIP.operator uint32_t() == 0) { // no address is set, try to obtain AP interface's IP | ||||||||||
|
@@ -111,16 +111,22 @@ void DNSServer::_handleUDP(AsyncUDPPacket &pkt) { | |||||||||
// will reply with IP only to "*" or if domain matches without www. subdomain | ||||||||||
if (dnsHeader.OPCode == DNS_OPCODE_QUERY && requestIncludesOnlyOneQuestion(dnsHeader) | ||||||||||
&& (_domainName.isEmpty() || getDomainNameWithoutWwwPrefix(static_cast<const unsigned char *>(dnsQuestion.QName), dnsQuestion.QNameLength) == _domainName)) { | ||||||||||
replyWithIP(pkt, dnsHeader, dnsQuestion); | ||||||||||
|
||||||||||
// Qtype = A (1) or ANY (255): send an A record otherwise an empty response | ||||||||||
if (ntohs(dnsQuestion.QType) == 1 || ntohs(dnsQuestion.QType) == 255) { | ||||||||||
replyWithIP(pkt, dnsHeader, dnsQuestion); | ||||||||||
} else { | ||||||||||
replyWithNoAnsw(pkt, dnsHeader, dnsQuestion); | ||||||||||
} | ||||||||||
return; | ||||||||||
} | ||||||||||
|
||||||||||
// otherwise reply with custom code | ||||||||||
replyWithCustomCode(pkt, dnsHeader); | ||||||||||
} | ||||||||||
|
||||||||||
bool DNSServer::requestIncludesOnlyOneQuestion(DNSHeader &dnsHeader) { | ||||||||||
return ntohs(dnsHeader.QDCount) == 1 && dnsHeader.ANCount == 0 && dnsHeader.NSCount == 0 && dnsHeader.ARCount == 0; | ||||||||||
dnsHeader.ARCount = 0; // We assume that if ARCount !=0 there is a EDNS OPT packet, just ignore | ||||||||||
return ntohs(dnsHeader.QDCount) == 1 && dnsHeader.ANCount == 0 && dnsHeader.NSCount == 0; // && dnsHeader.ARCount == 0; | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mutating
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there was a bit of dead code here (line 129), I removed it. |
||||||||||
} | ||||||||||
|
||||||||||
String DNSServer::getDomainNameWithoutWwwPrefix(const unsigned char *start, size_t len) { | ||||||||||
|
@@ -139,7 +145,6 @@ String DNSServer::getDomainNameWithoutWwwPrefix(const unsigned char *start, size | |||||||||
|
||||||||||
void DNSServer::replyWithIP(AsyncUDPPacket &req, DNSHeader &dnsHeader, DNSQuestion &dnsQuestion) { | ||||||||||
AsyncUDPMessage rpl; | ||||||||||
|
||||||||||
// Change the type of message to a response and set the number of answers equal to | ||||||||||
// the number of questions in the header | ||||||||||
dnsHeader.QR = DNS_QR_RESPONSE; | ||||||||||
|
@@ -187,3 +192,77 @@ void DNSServer::replyWithCustomCode(AsyncUDPPacket &req, DNSHeader &dnsHeader) { | |||||||||
rpl.write(reinterpret_cast<const uint8_t *>(&dnsHeader), sizeof(DNSHeader)); | ||||||||||
_udp.sendTo(rpl, req.remoteIP(), req.remotePort()); | ||||||||||
} | ||||||||||
|
||||||||||
void DNSServer::replyWithNoAnsw(AsyncUDPPacket &req, DNSHeader &dnsHeader, DNSQuestion &dnsQuestion) { | ||||||||||
|
||||||||||
dnsHeader.QR = DNS_QR_RESPONSE; | ||||||||||
// dnsHeader.QDCount = 1; | ||||||||||
Kolkman marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
dnsHeader.ANCount = 0; | ||||||||||
dnsHeader.NSCount = htons(1); | ||||||||||
|
||||||||||
AsyncUDPMessage rpl; | ||||||||||
rpl.write(reinterpret_cast<const uint8_t *>(&dnsHeader), sizeof(DNSHeader)); | ||||||||||
|
||||||||||
// Write the question | ||||||||||
rpl.write(dnsQuestion.QName, dnsQuestion.QNameLength); | ||||||||||
rpl.write((uint8_t *)&dnsQuestion.QType, 2); | ||||||||||
rpl.write((uint8_t *)&dnsQuestion.QClass, 2); | ||||||||||
|
||||||||||
// An empty answer contains an authority section with a SOA, | ||||||||||
// We take the name of the query as the root of the zone for which the SOA is generated | ||||||||||
// and use a value of DNS_MINIMAL_TTL seconds in order to minimize negative caching | ||||||||||
// Write the authority section: | ||||||||||
// The SOA RR's ownername is set equal to the query name, and we use made up names for | ||||||||||
// the MNAME and RNAME - it doesn't really matter from a protocol perspective - as for | ||||||||||
// a no such QTYPE answer only the timing fields are used. | ||||||||||
// a protocol perspective - it | ||||||||||
// Use DNS name compression : instead of repeating the name in this RNAME occurrence, | ||||||||||
// set the two MSB of the byte corresponding normally to the length to 1. The following | ||||||||||
// 14 bits must be used to specify the offset of the domain name in the message | ||||||||||
// (<255 here so the first byte has the 6 LSB at 0) | ||||||||||
rpl.write((uint8_t)0xC0); | ||||||||||
rpl.write((uint8_t)DNS_OFFSET_DOMAIN_NAME); | ||||||||||
|
||||||||||
// DNS type A : host address, DNS class IN for INternet, returning an IPv4 address | ||||||||||
uint16_t answerType = htons(DNS_TYPE_SOA), answerClass = htons(DNS_CLASS_IN); | ||||||||||
uint32_t Serial = htonl(DNS_SOA_SERIAL); // Date type serial based on the date this piece of code was written | ||||||||||
uint32_t Refresh = htonl(DNS_SOA_REFRESH); // These timers don't matter, we don't serve zone transfers | ||||||||||
uint32_t Retry = htonl(DNS_SOA_RETRY); | ||||||||||
uint32_t Expire = htonl(DNS_SOA_EXPIRE); | ||||||||||
uint32_t MinTTL = htonl(DNS_MINIMAL_TTL); // See RFC2308 section 5 | ||||||||||
char MLabel[] = DNS_SOA_MNAME_LABEL; | ||||||||||
char RLabel[] = DNS_SOA_RNAME_LABEL; | ||||||||||
char PostFixLabel[] = DNS_SOA_POSTFIX_LABEL; | ||||||||||
|
||||||||||
// 4 accounts for len fields and for both rname | ||||||||||
// and lname and their postfix labels and there are 5 32 bit fields | ||||||||||
|
||||||||||
uint16_t RdataLength = htons((uint16_t)(strlen(MLabel) + strlen(RLabel) + 2 * strlen(PostFixLabel) + 4 + 5 * sizeof(Serial))); | ||||||||||
|
||||||||||
rpl.write((unsigned char *)&answerType, 2); | ||||||||||
rpl.write((unsigned char *)&answerClass, 2); | ||||||||||
rpl.write((unsigned char *)&MinTTL, 4); // DNS Time To Live | ||||||||||
|
||||||||||
rpl.write((unsigned char *)&RdataLength, 2); | ||||||||||
|
||||||||||
rpl.write((uint8_t)strlen(MLabel)); | ||||||||||
rpl.write((unsigned char *)&MLabel, strlen(MLabel)); | ||||||||||
|
||||||||||
rpl.write((unsigned char *)&PostFixLabel, strlen(PostFixLabel)); | ||||||||||
rpl.write((uint8_t)0); | ||||||||||
// rpl.write((uint8_t)0xC0); | ||||||||||
// rpl.write((uint8_t)DNS_OFFSET_DOMAIN_NAME); | ||||||||||
|
||||||||||
rpl.write((uint8_t)strlen(RLabel)); | ||||||||||
rpl.write((unsigned char *)&RLabel, strlen(RLabel)); | ||||||||||
rpl.write((unsigned char *)&PostFixLabel, strlen(PostFixLabel)); | ||||||||||
rpl.write((uint8_t)0); | ||||||||||
|
||||||||||
rpl.write((unsigned char *)&Serial, 4); | ||||||||||
rpl.write((unsigned char *)&Refresh, 4); | ||||||||||
rpl.write((unsigned char *)&Retry, 4); | ||||||||||
rpl.write((unsigned char *)&Expire, 4); | ||||||||||
rpl.write((unsigned char *)&MinTTL, 4); | ||||||||||
|
||||||||||
_udp.sendTo(rpl, req.remoteIP(), req.remotePort()); | ||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,23 @@ | |
#define DNS_OFFSET_DOMAIN_NAME DNS_HEADER_SIZE // Offset in bytes to reach the domain name labels in the DNS message | ||
#define DNS_DEFAULT_PORT 53 | ||
|
||
#define DNS_SOA_MNAME_LABEL "ns" | ||
#define DNS_SOA_RNAME_LABEL "esp32" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what are MNAME and RNAME? How are their values chosen? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. MNAME and RNAME are values defined in RFC1035 section 3.1.13. Tne MNAME is the name of the primary nameserver the request came from - that value is only used in the context of dynamic DNS updates RFC 2136. In this contexts it is purely informational. I chose the 'ns' label as that is sort of customary for nameservers. The RNAME is used to construct the email address of the administrator for the zone. The way things are coded now that is esp32@local. There nothing in the DNS protocol that uses that value, it is only for network administrator information siganling purpose. |
||
// The POSTFIX_LABEL will be concatinated to the RName and MName Label label | ||
Kolkman marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// do not use a multilabel name here. "local" is a good choice as it is reserved for | ||
// local use by IANA | ||
// The postfix label is defined as an array of characters that follows the | ||
// definition of RFC1035 3.1 | ||
// for instance, a postfix of example.com would be defined as: | ||
// #define DNS_SOA_POSTFIX_LABEL {'\7', 'e', 'x', 'a', 'm', 'p', 'l', 'e', '\3', 'c', 'o', 'm', '\0'} | ||
#define DNS_SOA_POSTFIX_LABEL {'\5', 'l', 'o', 'c', 'a', 'l', '\0'} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Excellent question, because it surfaces a dilemma that I had. Here are the considerations:
As said, there is nothing in the DNS (except for dynamic updates) that will use the M- and RNAME values, hence no interference with MDNS - the values are purely used in the context of local troubleshooting. |
||
// From the following values only the MINIMAL_TTL has relevance | ||
// in the context of client-server protocol interactions. | ||
#define DNS_SOA_SERIAL 2025052900 // Arbitrary | ||
#define DNS_SOA_REFRESH 100000 // Arbitrary | ||
#define DNS_SOA_RETRY 10000 // Arbitrary | ||
#define DNS_SOA_EXPIRE 1000000 // Arbitrary | ||
Kolkman marked this conversation as resolved.
Show resolved
Hide resolved
|
||
#define DNS_MINIMAL_TTL 5 // Time to live for negative answers RFC2308 | ||
enum class DNSReplyCode : uint16_t { | ||
NoError = 0, | ||
FormError = 1, | ||
|
@@ -82,7 +99,7 @@ class DNSServer { | |
* @param domainName - domain name to serve | ||
*/ | ||
DNSServer(const String &domainName); | ||
~DNSServer(){}; // default d-tor | ||
~DNSServer() {}; // default d-tor | ||
|
||
// Copy semantics not implemented (won't run on same UDP port anyway) | ||
DNSServer(const DNSServer &) = delete; | ||
|
@@ -179,5 +196,7 @@ class DNSServer { | |
inline bool requestIncludesOnlyOneQuestion(DNSHeader &dnsHeader); | ||
void replyWithIP(AsyncUDPPacket &req, DNSHeader &dnsHeader, DNSQuestion &dnsQuestion); | ||
inline void replyWithCustomCode(AsyncUDPPacket &req, DNSHeader &dnsHeader); | ||
inline void replyWithNoAnsw(AsyncUDPPacket &req, DNSHeader &dnsHeader, DNSQuestion &dnsQuestion); | ||
|
||
void _handleUDP(AsyncUDPPacket &pkt); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this version is updated with every new release. Please revert
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted the change with new commit