dnsdist: Add a per-frontend metric for non-compliant queries

diff --git a/pdns/dnsdist-tcp.cc b/pdns/dnsdist-tcp.cc
index e4a872aa8cd4d0d07a39f1e2d685b3a91c63b8e7..cc55ee43646ec52f501967b0baea315a7c0e7fb6 100644 (file)
--- a/pdns/dnsdist-tcp.cc
+++ b/pdns/dnsdist-tcp.cc
@@ -643,6 +643,7 @@ static void handleQuery(std::shared_ptr<IncomingTCPConnectionState>& 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<IncomingTCPConnectionState>& state, cons
   {
     /* this pointer will be invalidated the second the buffer is resized, don't hold onto it! */
     auto* dh = reinterpret_cast<dnsheader*>(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 b3546806bc8868a449438cf55e466dcedebd8cc3..0af3822a37e6d64e7fe610387af25fa9d6d5d375 100644 (file)
--- 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 971f9c1b4146cadb46215627f29513d60e6a171d..ca7d471c2df249ee4aa263da29a70388dfe25305 100644 (file)
--- 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<struct dnsheader*>(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<size_t>(got) < sizeof(struct dnsheader)) {
         ++g_stats.nonCompliantQueries;
+        ++cs->nonCompliantQueries;
         continue;
       }
 
@@ -1739,6 +1742,7 @@ static void udpClientThread(ClientState* cs)
 
         if (got < 0 || static_cast<size_t>(got) < sizeof(struct dnsheader)) {
           ++g_stats.nonCompliantQueries;
+          ++cs->nonCompliantQueries;
           continue;
         }
 

diff --git a/pdns/dnsdist.hh b/pdns/dnsdist.hh
index db8454853a1642e85b4c2045ae66dbaa1ae1f05f..f0b178e551550d8b710adc22534955a794f31533 100644 (file)
--- 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<vector<DNSDistResponseRuleAction> >& 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<std::shared_ptr<DNSCryptContext>> 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 1ec3a2c7e9f043fa8ecdf0b3d762750f9ca3c9ad..daa8efaa6b3f6442f4599db26e12e114fdba7809 100644 (file)
--- 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<struct dnsheader*>(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 9457b62e36d06ba9efbd68a6c43652303b36d6ba..6b73ef9fb16394b4a36fc1118d61a2394b4150c3 100644 (file)
--- 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 7e7d9151d9fd2ecf61353d994c1b451fc7a989ef..7aa0699362a5cdd3d9e693d745ab3f58ffd97a3c 100644 (file)
--- 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']: