From 4e58282058fe9d57f0ef1d557e88ffdbdbf01166 Mon Sep 17 00:00:00 2001 From: Attila Molnar Date: Tue, 21 Apr 2015 15:08:10 +0200 Subject: core_dns Add typedef for request id, change it to uint16_t --- include/modules/dns.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'include/modules') diff --git a/include/modules/dns.h b/include/modules/dns.h index 400d2085d..04e3df16c 100644 --- a/include/modules/dns.h +++ b/include/modules/dns.h @@ -68,6 +68,8 @@ namespace DNS ERROR_INVALIDTYPE }; + typedef uint16_t RequestId; + const int PORT = 53; /** @@ -147,7 +149,7 @@ namespace DNS /* Use result cache if available */ bool use_cache; /* Request id */ - unsigned short id; + RequestId id; /* Creator of this request */ Module* const creator; -- cgit v1.2.3 From a28f095db832e44aad66bfd73bfd8176a97c6de2 Mon Sep 17 00:00:00 2001 From: Attila Molnar Date: Tue, 21 Apr 2015 15:14:29 +0200 Subject: core_dns Don't store query class code in Question --- include/modules/dns.h | 9 ++++----- src/coremods/core_dns.cpp | 8 ++++---- 2 files changed, 8 insertions(+), 9 deletions(-) (limited to 'include/modules') diff --git a/include/modules/dns.h b/include/modules/dns.h index 04e3df16c..f4071e399 100644 --- a/include/modules/dns.h +++ b/include/modules/dns.h @@ -88,11 +88,10 @@ namespace DNS { std::string name; QueryType type; - unsigned short qclass; - Question() : type(QUERY_NONE), qclass(0) { } - Question(const std::string& n, QueryType t, unsigned short c = 1) : name(n), type(t), qclass(c) { } - inline bool operator==(const Question& other) const { return name == other.name && type == other.type && qclass == other.qclass; } + Question() : type(QUERY_NONE) { } + Question(const std::string& n, QueryType t) : name(n), type(t) { } + bool operator==(const Question& other) const { return ((name == other.name) && (type == other.type)); } struct hash { @@ -109,7 +108,7 @@ namespace DNS std::string rdata; time_t created; - ResourceRecord(const std::string& n, QueryType t, unsigned short c = 1) : Question(n, t, c), ttl(0), created(ServerInstance->Time()) { } + ResourceRecord(const std::string& n, QueryType t) : Question(n, t), ttl(0), created(ServerInstance->Time()) { } ResourceRecord(const Question& question) : Question(question), ttl(0), created(ServerInstance->Time()) { } }; diff --git a/src/coremods/core_dns.cpp b/src/coremods/core_dns.cpp index 891c6705a..999da8356 100644 --- a/src/coremods/core_dns.cpp +++ b/src/coremods/core_dns.cpp @@ -126,7 +126,7 @@ class Packet : public Query question.type = static_cast(input[pos] << 8 | input[pos + 1]); pos += 2; - question.qclass = input[pos] << 8 | input[pos + 1]; + // Skip over query class code pos += 2; return question; @@ -307,9 +307,9 @@ class Packet : public Query memcpy(&output[pos], &s, 2); pos += 2; - s = htons(q.qclass); - memcpy(&output[pos], &s, 2); - pos += 2; + // Query class, always IN + output[pos++] = 0; + output[pos++] = 1; } return pos; -- cgit v1.2.3 From edfe7f035ea83731b9316d40d64402411a531426 Mon Sep 17 00:00:00 2001 From: Attila Molnar Date: Tue, 21 Apr 2015 15:24:10 +0200 Subject: core_dns Remove incomplete support for multiple questions per query --- include/modules/dns.h | 4 ++-- src/coremods/core_dns.cpp | 28 +++++++++++++--------------- 2 files changed, 15 insertions(+), 17 deletions(-) (limited to 'include/modules') diff --git a/include/modules/dns.h b/include/modules/dns.h index f4071e399..4da31b805 100644 --- a/include/modules/dns.h +++ b/include/modules/dns.h @@ -114,13 +114,13 @@ namespace DNS struct Query { - std::vector questions; + Question question; std::vector answers; Error error; bool cached; Query() : error(ERROR_NONE), cached(false) { } - Query(const Question& question) : error(ERROR_NONE), cached(false) { questions.push_back(question); } + Query(const Question& q) : question(q), error(ERROR_NONE), cached(false) { } }; class ReplySocket; diff --git a/src/coremods/core_dns.cpp b/src/coremods/core_dns.cpp index 8911f5b55..a7ed8d3ed 100644 --- a/src/coremods/core_dns.cpp +++ b/src/coremods/core_dns.cpp @@ -116,20 +116,20 @@ class Packet : public Query Question UnpackQuestion(const unsigned char* input, unsigned short input_size, unsigned short& pos) { - Question question; + Question q; - question.name = this->UnpackName(input, input_size, pos); + q.name = this->UnpackName(input, input_size, pos); if (pos + 4 > input_size) throw Exception("Unable to unpack question"); - question.type = static_cast(input[pos] << 8 | input[pos + 1]); + q.type = static_cast(input[pos] << 8 | input[pos + 1]); pos += 2; // Skip over query class code pos += 2; - return question; + return q; } ResourceRecord UnpackResourceRecord(const unsigned char* input, unsigned short input_size, unsigned short& pos) @@ -239,8 +239,7 @@ class Packet : public Query if (qdcount != 1) throw Exception("Question count != 1 in incoming packet"); - for (unsigned i = 0; i < qdcount; ++i) - this->questions.push_back(this->UnpackQuestion(input, len, packet_pos)); + this->question = this->UnpackQuestion(input, len, packet_pos); for (unsigned i = 0; i < ancount; ++i) this->answers.push_back(this->UnpackResourceRecord(input, len, packet_pos)); @@ -257,8 +256,8 @@ class Packet : public Query output[pos++] = this->id & 0xFF; output[pos++] = this->flags >> 8; output[pos++] = this->flags & 0xFF; - output[pos++] = this->questions.size() >> 8; - output[pos++] = this->questions.size() & 0xFF; + output[pos++] = 0; // Question count, high byte + output[pos++] = 1; // Question count, low byte output[pos++] = 0; // Answer count, high byte output[pos++] = 0; // Answer count, low byte output[pos++] = 0; @@ -266,9 +265,8 @@ class Packet : public Query output[pos++] = 0; output[pos++] = 0; - for (unsigned i = 0; i < this->questions.size(); ++i) { - Question& q = this->questions[i]; + Question& q = this->question; if (q.type == QUERY_PTR) { @@ -363,7 +361,7 @@ class MyManager : public Manager, public Timer, public EventHandler { const ResourceRecord& rr = r.answers[0]; ServerInstance->Logs->Log(MODNAME, LOG_DEBUG, "cache: added cache for " + rr.name + " -> " + rr.rdata + " ttl: " + ConvToStr(rr.ttl)); - this->cache[r.questions[0]] = r; + this->cache[r.question] = r; } public: @@ -431,15 +429,15 @@ class MyManager : public Manager, public Timer, public EventHandler Packet p; p.flags = QUERYFLAGS_RD; p.id = req->id; - p.questions.push_back(*req); + p.question = *req; unsigned char buffer[524]; unsigned short len = p.Pack(buffer, sizeof(buffer)); - /* Note that calling Pack() above can actually change the contents of p.questions[0].name, if the query is a PTR, + /* Note that calling Pack() above can actually change the contents of p.question.name, if the query is a PTR, * to contain the value that would be in the DNS cache, which is why this is here. */ - if (req->use_cache && this->CheckCache(req, p.questions[0])) + if (req->use_cache && this->CheckCache(req, p.question)) { ServerInstance->Logs->Log(MODNAME, LOG_DEBUG, "Using cached result"); delete req; @@ -567,7 +565,7 @@ class MyManager : public Manager, public Timer, public EventHandler recv_packet.error = error; request->OnError(&recv_packet); } - else if (recv_packet.questions.empty() || recv_packet.answers.empty()) + else if (recv_packet.answers.empty()) { ServerInstance->Logs->Log(MODNAME, LOG_DEBUG, "No resource records returned"); ServerInstance->stats.DnsBad++; -- cgit v1.2.3 From 3d32852e7e459d403179e8f79354e2e52bbc4a52 Mon Sep 17 00:00:00 2001 From: Attila Molnar Date: Mon, 27 Apr 2015 19:28:00 +0200 Subject: core_dns Drop incoming packets containing a different question from what we asked --- include/modules/dns.h | 1 + src/coremods/core_dns.cpp | 7 +++++++ 2 files changed, 8 insertions(+) (limited to 'include/modules') diff --git a/include/modules/dns.h b/include/modules/dns.h index 4da31b805..cfa048184 100644 --- a/include/modules/dns.h +++ b/include/modules/dns.h @@ -92,6 +92,7 @@ namespace DNS Question() : type(QUERY_NONE) { } Question(const std::string& n, QueryType t) : name(n), type(t) { } bool operator==(const Question& other) const { return ((name == other.name) && (type == other.type)); } + bool operator!=(const Question& other) const { return (!(*this == other)); } struct hash { diff --git a/src/coremods/core_dns.cpp b/src/coremods/core_dns.cpp index f4f85e253..d4214b9a5 100644 --- a/src/coremods/core_dns.cpp +++ b/src/coremods/core_dns.cpp @@ -527,6 +527,13 @@ class MyManager : public Manager, public Timer, public EventHandler return; } + if (static_cast(*request) != recv_packet.question) + { + // This can happen under high latency, drop it silently, do not fail the request + ServerInstance->Logs->Log(MODNAME, LOG_DEBUG, "Received an answer that isn't for a question we asked"); + return; + } + if (recv_packet.flags & QUERYFLAGS_OPCODE) { ServerInstance->Logs->Log(MODNAME, LOG_DEBUG, "Received a nonstandard query"); -- cgit v1.2.3