From: Remi Gacogne Date: Wed, 22 Jun 2022 13:24:21 +0000 (+0200) Subject: dnsdist: Add a per-frontend metric for non-compliant queries X-Git-Tag: auth-4.8.0-alpha0~40^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fd23f5defc0d98701adc47fde1e5a9e316c5a85d;p=thirdparty%2Fpdns.git dnsdist: Add a per-frontend metric for non-compliant queries --- diff --git a/pdns/dnsdist-tcp.cc b/pdns/dnsdist-tcp.cc index e4a872aa8c..cc55ee4364 100644 --- a/pdns/dnsdist-tcp.cc +++ b/pdns/dnsdist-tcp.cc @@ -643,6 +643,7 @@ static void handleQuery(std::shared_ptr& state, cons { if (state->d_querySize < sizeof(dnsheader)) { ++g_stats.nonCompliantQueries; + ++state->d_ci.cs->nonCompliantQueries; state->terminateClientConnection(); return; } @@ -690,7 +691,7 @@ static void handleQuery(std::shared_ptr& state, cons { /* this pointer will be invalidated the second the buffer is resized, don't hold onto it! */ auto* dh = reinterpret_cast(state->d_buffer.data()); - if (!checkQueryHeaders(dh)) { + if (!checkQueryHeaders(dh, *state->d_ci.cs)) { state->terminateClientConnection(); return; } diff --git a/pdns/dnsdist-web.cc b/pdns/dnsdist-web.cc index b3546806bc..0af3822a37 100644 --- a/pdns/dnsdist-web.cc +++ b/pdns/dnsdist-web.cc @@ -596,6 +596,8 @@ static void handlePrometheus(const YaHTTP::Request& req, YaHTTP::Response& resp) const string frontsbase = "dnsdist_frontend_"; output << "# HELP " << frontsbase << "queries " << "Amount of queries received by this frontend" << "\n"; output << "# TYPE " << frontsbase << "queries " << "counter" << "\n"; + output << "# HELP " << frontsbase << "noncompliantqueries " << "Amount of non-compliant queries received by this frontend" << "\n"; + output << "# TYPE " << frontsbase << "noncompliantqueries " << "counter" << "\n"; output << "# HELP " << frontsbase << "responses " << "Amount of responses sent by this frontend" << "\n"; output << "# TYPE " << frontsbase << "responses " << "counter" << "\n"; output << "# HELP " << frontsbase << "tcpdiedreadingquery " << "Amount of TCP connections terminated while reading the query from the client" << "\n"; @@ -648,6 +650,7 @@ static void handlePrometheus(const YaHTTP::Request& req, YaHTTP::Response& resp) % frontName % proto % threadNumber); output << frontsbase << "queries" << label << front->queries.load() << "\n"; + output << frontsbase << "noncompliantqueries" << label << front->nonCompliantQueries.load() << "\n"; output << frontsbase << "responses" << label << front->responses.load() << "\n"; if (front->isTCP()) { output << frontsbase << "tcpdiedreadingquery" << label << front->tcpDiedReadingQuery.load() << "\n"; @@ -1037,6 +1040,7 @@ static void handleStats(const YaHTTP::Request& req, YaHTTP::Response& resp) { "tcp", front->tcpFD >= 0 }, { "type", front->getType() }, { "queries", (double) front->queries.load() }, + { "nonCompliantQueries", (double) front->nonCompliantQueries.load() }, { "responses", (double) front->responses.load() }, { "tcpDiedReadingQuery", (double) front->tcpDiedReadingQuery.load() }, { "tcpDiedSendingResponse", (double) front->tcpDiedSendingResponse.load() }, diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc index 971f9c1b41..ca7d471c2d 100644 --- a/pdns/dnsdist.cc +++ b/pdns/dnsdist.cc @@ -1069,6 +1069,7 @@ static bool isUDPQueryAcceptable(ClientState& cs, LocalHolders& holders, const s if (msgh->msg_flags & MSG_TRUNC) { /* message was too large for our buffer */ vinfolog("Dropping message too large for our buffer"); + ++cs.nonCompliantQueries; ++g_stats.nonCompliantQueries; return false; } @@ -1132,10 +1133,11 @@ bool checkDNSCryptQuery(const ClientState& cs, PacketBuffer& query, std::unique_ return false; } -bool checkQueryHeaders(const struct dnsheader* dh) +bool checkQueryHeaders(const struct dnsheader* dh, ClientState& cs) { if (dh->qr) { // don't respond to responses ++g_stats.nonCompliantQueries; + ++cs.nonCompliantQueries; return false; } @@ -1492,7 +1494,7 @@ static void processUDPQuery(ClientState& cs, LocalHolders& holders, const struct struct dnsheader* dh = reinterpret_cast(query.data()); queryId = ntohs(dh->id); - if (!checkQueryHeaders(dh)) { + if (!checkQueryHeaders(dh, cs)) { return; } @@ -1673,6 +1675,7 @@ static void MultipleMessagesUDPClientThread(ClientState* cs, LocalHolders& holde if (static_cast(got) < sizeof(struct dnsheader)) { ++g_stats.nonCompliantQueries; + ++cs->nonCompliantQueries; continue; } @@ -1739,6 +1742,7 @@ static void udpClientThread(ClientState* cs) if (got < 0 || static_cast(got) < sizeof(struct dnsheader)) { ++g_stats.nonCompliantQueries; + ++cs->nonCompliantQueries; continue; } diff --git a/pdns/dnsdist.hh b/pdns/dnsdist.hh index db8454853a..f0b178e551 100644 --- a/pdns/dnsdist.hh +++ b/pdns/dnsdist.hh @@ -577,6 +577,7 @@ struct ClientState } stat_t queries{0}; + stat_t nonCompliantQueries{0}; mutable stat_t responses{0}; mutable stat_t tcpDiedReadingQuery{0}; mutable stat_t tcpDiedSendingResponse{0}; @@ -1104,7 +1105,7 @@ bool responseContentMatches(const PacketBuffer& response, const DNSName& qname, bool processResponse(PacketBuffer& response, LocalStateHolder >& localRespRuleActions, DNSResponse& dr, bool muted, bool receivedOverUDP); bool processRulesResult(const DNSAction::Action& action, DNSQuestion& dq, std::string& ruleresult, bool& drop); -bool checkQueryHeaders(const struct dnsheader* dh); +bool checkQueryHeaders(const struct dnsheader* dh, ClientState& cs); extern std::vector> g_dnsCryptLocals; int handleDNSCryptQuery(PacketBuffer& packet, DNSCryptQuery& query, bool tcp, time_t now, PacketBuffer& response); diff --git a/pdns/dnsdistdist/doh.cc b/pdns/dnsdistdist/doh.cc index 1ec3a2c7e9..daa8efaa6b 100644 --- a/pdns/dnsdistdist/doh.cc +++ b/pdns/dnsdistdist/doh.cc @@ -563,6 +563,7 @@ static void processDOHQuery(DOHUnitUniquePtr&& du) if (du->query.size() < sizeof(dnsheader)) { ++g_stats.nonCompliantQueries; + ++cs.nonCompliantQueries; du->status_code = 400; sendDoHUnitToTheMainThread(std::move(du), "DoH non-compliant query"); return; @@ -581,7 +582,7 @@ static void processDOHQuery(DOHUnitUniquePtr&& du) /* don't keep that pointer around, it will be invalidated if the buffer is ever resized */ struct dnsheader* dh = reinterpret_cast(du->query.data()); - if (!checkQueryHeaders(dh)) { + if (!checkQueryHeaders(dh, cs)) { du->status_code = 400; sendDoHUnitToTheMainThread(std::move(du), "DoH invalid headers"); return; diff --git a/pdns/dnsdistdist/test-dnsdisttcp_cc.cc b/pdns/dnsdistdist/test-dnsdisttcp_cc.cc index 9457b62e36..6b73ef9fb1 100644 --- a/pdns/dnsdistdist/test-dnsdisttcp_cc.cc +++ b/pdns/dnsdistdist/test-dnsdisttcp_cc.cc @@ -48,7 +48,7 @@ bool checkDNSCryptQuery(const ClientState& cs, PacketBuffer& query, std::unique_ return false; } -bool checkQueryHeaders(const struct dnsheader* dh) +bool checkQueryHeaders(const struct dnsheader* dh, ClientState&) { return true; } diff --git a/regression-tests.dnsdist/test_API.py b/regression-tests.dnsdist/test_API.py index 7e7d9151d9..7aa0699362 100644 --- a/regression-tests.dnsdist/test_API.py +++ b/regression-tests.dnsdist/test_API.py @@ -142,10 +142,10 @@ class TestAPIBasics(APITestsBase): self.assertTrue(server['state'] in ['up', 'down', 'UP', 'DOWN']) for frontend in content['frontends']: - for key in ['id', 'address', 'udp', 'tcp', 'type', 'queries']: + for key in ['id', 'address', 'udp', 'tcp', 'type', 'queries', 'nonCompliantQueries']: self.assertIn(key, frontend) - for key in ['id', 'queries']: + for key in ['id', 'queries', 'nonCompliantQueries']: self.assertTrue(frontend[key] >= 0) for pool in content['pools']: