From 644ed7afac2c3851f2d2bec386908b0f99088ce4 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sun, 30 Sep 2018 13:44:40 -0700 Subject: [PATCH 1/3] Cleanup Removed member variables that are not required outside a member call lifetime --- libraries/DNSServer/keywords.txt | 1 + libraries/DNSServer/library.properties | 2 +- libraries/DNSServer/src/DNSServer.cpp | 139 ++++++++++++++----------- libraries/DNSServer/src/DNSServer.h | 13 ++- 4 files changed, 88 insertions(+), 67 deletions(-) diff --git a/libraries/DNSServer/keywords.txt b/libraries/DNSServer/keywords.txt index 86c5799c2b..80f03834c9 100644 --- a/libraries/DNSServer/keywords.txt +++ b/libraries/DNSServer/keywords.txt @@ -33,6 +33,7 @@ stop KEYWORD2 DNS_QR_QUERY LITERAL1 RESERVED_WORD_2 DNS_QR_RESPONSE LITERAL1 RESERVED_WORD_2 DNS_OPCODE_QUERY LITERAL1 RESERVED_WORD_2 +MAX_DNSNAME_LENGTH LITERAL1 RESERVED_WORD_2 NoError LITERAL1 RESERVED_WORD_2 FormError LITERAL1 RESERVED_WORD_2 ServerFailure LITERAL1 RESERVED_WORD_2 diff --git a/libraries/DNSServer/library.properties b/libraries/DNSServer/library.properties index fc9f0f7fa1..e3c8446e88 100644 --- a/libraries/DNSServer/library.properties +++ b/libraries/DNSServer/library.properties @@ -1,5 +1,5 @@ name=DNSServer -version=1.1.0 +version=1.1.1 author=Kristijan Novoselić maintainer=Kristijan Novoselić, sentence=A simple DNS server for ESP8266. diff --git a/libraries/DNSServer/src/DNSServer.cpp b/libraries/DNSServer/src/DNSServer.cpp index e50fa4f26c..67a235f42c 100644 --- a/libraries/DNSServer/src/DNSServer.cpp +++ b/libraries/DNSServer/src/DNSServer.cpp @@ -13,7 +13,7 @@ bool DNSServer::start(const uint16_t &port, const String &domainName, const IPAddress &resolvedIP) { _port = port; - _buffer = NULL; + _domainName = domainName; _resolvedIP[0] = resolvedIP[0]; _resolvedIP[1] = resolvedIP[1]; @@ -36,8 +36,6 @@ void DNSServer::setTTL(const uint32_t &ttl) void DNSServer::stop() { _udp.stop(); - free(_buffer); - _buffer = NULL; } void DNSServer::downcaseAndRemoveWwwPrefix(String &domainName) @@ -48,82 +46,106 @@ void DNSServer::downcaseAndRemoveWwwPrefix(String &domainName) void DNSServer::processNextRequest() { - _currentPacketSize = _udp.parsePacket(); - if (_currentPacketSize) + int packetSize = _udp.parsePacket(); + + if (packetSize) { - if (_buffer != NULL) free(_buffer); - _buffer = (unsigned char*)malloc(_currentPacketSize * sizeof(char)); - if (_buffer == NULL) return; - _udp.read(_buffer, _currentPacketSize); - _dnsHeader = (DNSHeader*) _buffer; - - if (_dnsHeader->QR == DNS_QR_QUERY && - _dnsHeader->OPCode == DNS_OPCODE_QUERY && - requestIncludesOnlyOneQuestion() && - (_domainName == "*" || getDomainNameWithoutWwwPrefix() == _domainName) + uint8_t* buffer = reinterpret_cast(malloc(packetSize)); + if (buffer == NULL) return; + + _udp.read(buffer, packetSize); + + DNSHeader* dnsHeader = reinterpret_cast(buffer); + + if (dnsHeader->QR == DNS_QR_QUERY && + dnsHeader->OPCode == DNS_OPCODE_QUERY && + requestIncludesOnlyOneQuestion(dnsHeader) && + (_domainName == "*" || getDomainNameWithoutWwwPrefix(buffer, packetSize) == _domainName) ) { - replyWithIP(); + replyWithIP(buffer, packetSize); } - else if (_dnsHeader->QR == DNS_QR_QUERY) + else if (dnsHeader->QR == DNS_QR_QUERY) { - replyWithCustomCode(); + replyWithCustomCode(buffer, packetSize); } - free(_buffer); - _buffer = NULL; + free(buffer); } } -bool DNSServer::requestIncludesOnlyOneQuestion() +bool DNSServer::requestIncludesOnlyOneQuestion(const DNSHeader* dnsHeader) { - return ntohs(_dnsHeader->QDCount) == 1 && - _dnsHeader->ANCount == 0 && - _dnsHeader->NSCount == 0 && - _dnsHeader->ARCount == 0; + return ntohs(dnsHeader->QDCount) == 1 && + dnsHeader->ANCount == 0 && + dnsHeader->NSCount == 0 && + dnsHeader->ARCount == 0; } -String DNSServer::getDomainNameWithoutWwwPrefix() +String DNSServer::getDomainNameWithoutWwwPrefix(const uint8_t* buffer, int packetSize) { - String parsedDomainName = ""; - if (_buffer == NULL) return parsedDomainName; - unsigned char *start = _buffer + 12; - if (*start == 0) - { - return parsedDomainName; - } - int pos = 0; - while(true) + String parsedDomainName; + + const uint8_t* pos = buffer + sizeof(DNSHeader); + const uint8_t* end = buffer + packetSize; + + // to minimize reallocations due to concats below + // we reserve enough space that a median or average domain + // name size cold be easily contained without a reallocation + // - max size would be 253, in 2013, average is 11 and max was 42 + // + parsedDomainName.reserve(32); + + uint8_t labelLength = *pos; + + while (true) { - unsigned char labelLength = *(start + pos); - for(int i = 0; i < labelLength; i++) + if (labelLength == 0) + { + // no more labels + downcaseAndRemoveWwwPrefix(parsedDomainName); + return parsedDomainName; + } + + // append next label + for (int i = 0; i < labelLength && pos < end; i++) { pos++; - parsedDomainName += (char)*(start + pos); + parsedDomainName += static_cast(*pos); } - pos++; - if (*(start + pos) == 0) + + if (pos >= end) { - downcaseAndRemoveWwwPrefix(parsedDomainName); - return parsedDomainName; + // malformed packet, return an empty domain name + parsedDomainName = ""; + return parsedDomainName; } else { - parsedDomainName += "."; + // next label + pos++; + labelLength = *pos; + + // if there is another label, add delimiter + if (labelLength != 0) + { + parsedDomainName += "."; + } } } } -void DNSServer::replyWithIP() +void DNSServer::replyWithIP(uint8_t* buffer, int packetSize) { - if (_buffer == NULL) return; - _dnsHeader->QR = DNS_QR_RESPONSE; - _dnsHeader->ANCount = _dnsHeader->QDCount; - _dnsHeader->QDCount = _dnsHeader->QDCount; - //_dnsHeader->RA = 1; + DNSHeader* dnsHeader = reinterpret_cast(buffer); + + dnsHeader->QR = DNS_QR_RESPONSE; + dnsHeader->ANCount = dnsHeader->QDCount; + dnsHeader->QDCount = dnsHeader->QDCount; + //dnsHeader->RA = 1; _udp.beginPacket(_udp.remoteIP(), _udp.remotePort()); - _udp.write(_buffer, _currentPacketSize); + _udp.write(buffer, packetSize); _udp.write((uint8_t)192); // answer name is a pointer _udp.write((uint8_t)12); // pointer to offset at 0x00c @@ -142,22 +164,21 @@ void DNSServer::replyWithIP() _udp.write(_resolvedIP, sizeof(_resolvedIP)); _udp.endPacket(); - - #ifdef DEBUG_ESP_DNS DEBUG_ESP_PORT.printf("DNS responds: %s for %s\n", - IPAddress(_resolvedIP).toString().c_str(), getDomainNameWithoutWwwPrefix().c_str() ); + IPAddress(_resolvedIP).toString().c_str(), getDomainNameWithoutWwwPrefix(buffer, packetSize).c_str() ); #endif } -void DNSServer::replyWithCustomCode() +void DNSServer::replyWithCustomCode(uint8_t* buffer, int packetSize) { - if (_buffer == NULL) return; - _dnsHeader->QR = DNS_QR_RESPONSE; - _dnsHeader->RCode = (unsigned char)_errorReplyCode; - _dnsHeader->QDCount = 0; + DNSHeader* dnsHeader = reinterpret_cast(buffer); + + dnsHeader->QR = DNS_QR_RESPONSE; + dnsHeader->RCode = (unsigned char)_errorReplyCode; + dnsHeader->QDCount = 0; _udp.beginPacket(_udp.remoteIP(), _udp.remotePort()); - _udp.write(_buffer, sizeof(DNSHeader)); + _udp.write(buffer, sizeof(DNSHeader)); _udp.endPacket(); } diff --git a/libraries/DNSServer/src/DNSServer.h b/libraries/DNSServer/src/DNSServer.h index ca96afea3a..129b1184d2 100644 --- a/libraries/DNSServer/src/DNSServer.h +++ b/libraries/DNSServer/src/DNSServer.h @@ -6,6 +6,8 @@ #define DNS_QR_RESPONSE 1 #define DNS_OPCODE_QUERY 0 +#define MAX_DNSNAME_LENGTH 253 + enum class DNSReplyCode { NoError = 0, @@ -56,16 +58,13 @@ class DNSServer uint16_t _port; String _domainName; unsigned char _resolvedIP[4]; - int _currentPacketSize; - unsigned char* _buffer; - DNSHeader* _dnsHeader; uint32_t _ttl; DNSReplyCode _errorReplyCode; void downcaseAndRemoveWwwPrefix(String &domainName); - String getDomainNameWithoutWwwPrefix(); - bool requestIncludesOnlyOneQuestion(); - void replyWithIP(); - void replyWithCustomCode(); + String getDomainNameWithoutWwwPrefix(const uint8_t* buffer, int packetSize); + bool requestIncludesOnlyOneQuestion(const DNSHeader* dnsHeader); + void replyWithIP(uint8_t* buffer, int packetSize); + void replyWithCustomCode(uint8_t* buffer, int packetSize); }; #endif \ No newline at end of file From 1887ec64ada84f4484aeea09b3fb2c7e62102747 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sun, 30 Sep 2018 13:58:24 -0700 Subject: [PATCH 2/3] add destructor --- libraries/DNSServer/src/DNSServer.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libraries/DNSServer/src/DNSServer.h b/libraries/DNSServer/src/DNSServer.h index 129b1184d2..1480c95601 100644 --- a/libraries/DNSServer/src/DNSServer.h +++ b/libraries/DNSServer/src/DNSServer.h @@ -42,6 +42,9 @@ class DNSServer { public: DNSServer(); + ~DNSServer() { + stop(); + }; void processNextRequest(); void setErrorReplyCode(const DNSReplyCode &replyCode); void setTTL(const uint32_t &ttl); From 4ee3fd6df7f589153c644042cfd1be6b9b26fe80 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sun, 30 Sep 2018 15:25:53 -0700 Subject: [PATCH 3/3] Use size_t and test it --- libraries/DNSServer/src/DNSServer.cpp | 15 ++++++++++----- libraries/DNSServer/src/DNSServer.h | 6 +++--- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/libraries/DNSServer/src/DNSServer.cpp b/libraries/DNSServer/src/DNSServer.cpp index 67a235f42c..351d05e334 100644 --- a/libraries/DNSServer/src/DNSServer.cpp +++ b/libraries/DNSServer/src/DNSServer.cpp @@ -46,9 +46,9 @@ void DNSServer::downcaseAndRemoveWwwPrefix(String &domainName) void DNSServer::processNextRequest() { - int packetSize = _udp.parsePacket(); + size_t packetSize = _udp.parsePacket(); - if (packetSize) + if (packetSize >= sizeof(DNSHeader)) { uint8_t* buffer = reinterpret_cast(malloc(packetSize)); if (buffer == NULL) return; @@ -82,7 +82,7 @@ bool DNSServer::requestIncludesOnlyOneQuestion(const DNSHeader* dnsHeader) dnsHeader->ARCount == 0; } -String DNSServer::getDomainNameWithoutWwwPrefix(const uint8_t* buffer, int packetSize) +String DNSServer::getDomainNameWithoutWwwPrefix(const uint8_t* buffer, size_t packetSize) { String parsedDomainName; @@ -135,7 +135,7 @@ String DNSServer::getDomainNameWithoutWwwPrefix(const uint8_t* buffer, int packe } } -void DNSServer::replyWithIP(uint8_t* buffer, int packetSize) +void DNSServer::replyWithIP(uint8_t* buffer, size_t packetSize) { DNSHeader* dnsHeader = reinterpret_cast(buffer); @@ -170,8 +170,13 @@ void DNSServer::replyWithIP(uint8_t* buffer, int packetSize) #endif } -void DNSServer::replyWithCustomCode(uint8_t* buffer, int packetSize) +void DNSServer::replyWithCustomCode(uint8_t* buffer, size_t packetSize) { + if (packetSize < sizeof(DNSHeader)) + { + return; + } + DNSHeader* dnsHeader = reinterpret_cast(buffer); dnsHeader->QR = DNS_QR_RESPONSE; diff --git a/libraries/DNSServer/src/DNSServer.h b/libraries/DNSServer/src/DNSServer.h index 1480c95601..d6e7de444d 100644 --- a/libraries/DNSServer/src/DNSServer.h +++ b/libraries/DNSServer/src/DNSServer.h @@ -65,9 +65,9 @@ class DNSServer DNSReplyCode _errorReplyCode; void downcaseAndRemoveWwwPrefix(String &domainName); - String getDomainNameWithoutWwwPrefix(const uint8_t* buffer, int packetSize); + String getDomainNameWithoutWwwPrefix(const uint8_t* buffer, size_t packetSize); bool requestIncludesOnlyOneQuestion(const DNSHeader* dnsHeader); - void replyWithIP(uint8_t* buffer, int packetSize); - void replyWithCustomCode(uint8_t* buffer, int packetSize); + void replyWithIP(uint8_t* buffer, size_t packetSize); + void replyWithCustomCode(uint8_t* buffer, size_t packetSize); }; #endif \ No newline at end of file