From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Thu, 6 Sep 2018 17:09:16 +0000 (+0200)
Subject: rec: Drop incoming queries with a qdcount of 0
X-Git-Tag: dnsdist-1.3.3~99^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c0f9be19e4138f638f9e52588ff5681fa16358a3;p=thirdparty%2Fpdns.git

rec: Drop incoming queries with a qdcount of 0
---

diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc
index 90cac60612..12a38591db 100644
--- a/pdns/pdns_recursor.cc
+++ b/pdns/pdns_recursor.cc
@@ -1733,7 +1733,9 @@ static void handleRunningTCPQuestion(int fd, FDMultiplexer::funcparam_t& var)
   else if(conn->state==TCPConnection::GETQUESTION) {
     ssize_t bytes=recv(conn->getFD(), &conn->data[conn->bytesread], conn->qlen - conn->bytesread, 0);
     if(!bytes || bytes < 0 || bytes > std::numeric_limits<std::uint16_t>::max()) {
-      g_log<<Logger::Error<<"TCP client "<< conn->d_remote.toStringWithPort() <<" disconnected while reading question body"<<endl;
+      if(g_logCommonErrors) {
+        g_log<<Logger::Error<<"TCP client "<< conn->d_remote.toStringWithPort() <<" disconnected while reading question body"<<endl;
+      }
       t_fdm->removeReadFD(fd);
       return;
     }
@@ -1818,9 +1820,9 @@ static void handleRunningTCPQuestion(int fd, FDMultiplexer::funcparam_t& var)
         dc->d_uuid = (*t_uuidGenerator)();
       }
 
+      const struct dnsheader* dh = reinterpret_cast<const struct dnsheader*>(&conn->data[0]);
       if(t_protobufServer) {
         try {
-          const struct dnsheader* dh = reinterpret_cast<const struct dnsheader*>(&conn->data[0]);
 
           if (logQuery && !(luaconfsLocal->protobufExportConfig.taggedOnly && dc->d_policyTags.empty())) {
             protobufLogQuery(t_protobufServer, luaconfsLocal->protobufMaskV4, luaconfsLocal->protobufMaskV6, dc->d_uuid, dc->d_source, dc->d_destination, dc->d_ednssubnet.source, true, dh->id, conn->qlen, qname, qtype, qclass, dc->d_policyTags, dc->d_requestorId, dc->d_deviceId);
@@ -1834,13 +1836,25 @@ static void handleRunningTCPQuestion(int fd, FDMultiplexer::funcparam_t& var)
 #endif
       if(dc->d_mdp.d_header.qr) {
         g_stats.ignoredCount++;
-        g_log<<Logger::Error<<"Ignoring answer from TCP client "<< dc->getRemote() <<" on server socket!"<<endl;
+        if(g_logCommonErrors) {
+          g_log<<Logger::Error<<"Ignoring answer from TCP client "<< dc->getRemote() <<" on server socket!"<<endl;
+        }
         delete dc;
         return;
       }
       if(dc->d_mdp.d_header.opcode) {
         g_stats.ignoredCount++;
-        g_log<<Logger::Error<<"Ignoring non-query opcode from TCP client "<< dc->getRemote() <<" on server socket!"<<endl;
+        if(g_logCommonErrors) {
+          g_log<<Logger::Error<<"Ignoring non-query opcode from TCP client "<< dc->getRemote() <<" on server socket!"<<endl;
+        }
+        delete dc;
+        return;
+      }
+      else if (dh->qdcount == 0) {
+        g_stats.emptyQueriesCount++;
+        if(g_logCommonErrors) {
+          g_log<<Logger::Error<<"Ignoring empty (qdcount == 0) query from "<< dc->getRemote() <<" on server socket!"<<endl;
+        }
         delete dc;
         return;
       }
@@ -2202,6 +2216,12 @@ static void handleNewUDPQuestion(int fd, FDMultiplexer::funcparam_t& var)
             g_log<<Logger::Error<<"Ignoring non-query opcode "<<dh->opcode<<" from "<<fromaddr.toString()<<" on server socket!"<<endl;
           }
         }
+        else if (dh->qdcount == 0) {
+          g_stats.emptyQueriesCount++;
+          if(g_logCommonErrors) {
+            g_log<<Logger::Error<<"Ignoring empty (qdcount == 0) query from "<<fromaddr.toString()<<" on server socket!"<<endl;
+          }
+        }
         else {
           struct timeval tv={0,0};
           HarvestTimestamp(&msgh, &tv);
diff --git a/pdns/rec-snmp.cc b/pdns/rec-snmp.cc
index b5884a8110..ce7dd34bd0 100644
--- a/pdns/rec-snmp.cc
+++ b/pdns/rec-snmp.cc
@@ -109,6 +109,7 @@ static const oid policyResultTruncateOID[] = { RECURSOR_STATS_OID, 90 };
 static const oid policyResultCustomOID[] = { RECURSOR_STATS_OID, 91 };
 static const oid queryPipeFullDropsOID[] = { RECURSOR_STATS_OID, 92 };
 static const oid truncatedDropsOID[] = { RECURSOR_STATS_OID, 93 };
+static const oid emptyQueriesOID[] = { RECURSOR_STATS_OID, 94 };
 
 static std::unordered_map<oid, std::string> s_statsMap;
 
@@ -222,6 +223,7 @@ RecursorSNMPAgent::RecursorSNMPAgent(const std::string& name, const std::string&
   registerCounter64Stat("too-old-drops", tooOldDropsOID, OID_LENGTH(tooOldDropsOID));
   registerCounter64Stat("query-pipe-full-drops", queryPipeFullDropsOID, OID_LENGTH(queryPipeFullDropsOID));
   registerCounter64Stat("truncated-drops", truncatedDropsOID, OID_LENGTH(truncatedDropsOID));
+  registerCounter64Stat("empty-queries", emptyQueriesOID, OID_LENGTH(emptyQueriesOID));
   registerCounter64Stat("answers0-1", answers01OID, OID_LENGTH(answers01OID));
   registerCounter64Stat("answers1-10", answers110OID, OID_LENGTH(answers110OID));
   registerCounter64Stat("answers10-100", answers10100OID, OID_LENGTH(answers10100OID));
diff --git a/pdns/rec_channel_rec.cc b/pdns/rec_channel_rec.cc
index 6ff2a84e01..d4b44f5b35 100644
--- a/pdns/rec_channel_rec.cc
+++ b/pdns/rec_channel_rec.cc
@@ -941,6 +941,7 @@ void registerAllStats()
   addGetStat("no-packet-error", &g_stats.noPacketError);
   addGetStat("dlg-only-drops", &SyncRes::s_nodelegated);
   addGetStat("ignored-packets", &g_stats.ignoredCount);
+  addGetStat("empty-queries", &g_stats.emptyQueriesCount);
   addGetStat("max-mthread-stack", &g_stats.maxMThreadStackUsage);
   
   addGetStat("negcache-entries", boost::bind(getNegCacheSize));
diff --git a/pdns/recursordist/RECURSOR-MIB.txt b/pdns/recursordist/RECURSOR-MIB.txt
index 35d933ea29..143d10d31e 100644
--- a/pdns/recursordist/RECURSOR-MIB.txt
+++ b/pdns/recursordist/RECURSOR-MIB.txt
@@ -774,6 +774,14 @@ truncatedDrops OBJECT-TYPE
         "Number of queries dropped because they were larger than 512 bytes"
     ::= { stats 93 }
 
+emptyQueries OBJECT-TYPE
+    SYNTAX Counter64
+    MAX-ACCESS read-only
+    STATUS current
+    DESCRIPTION
+        "Number of queries dropped because they had a QD count of 0"
+    ::= { stats 94 }
+
 ---
 --- Traps / Notifications
 ---
diff --git a/pdns/recursordist/docs/metrics.rst b/pdns/recursordist/docs/metrics.rst
index 4ee63b45c7..6bd2e45ab9 100644
--- a/pdns/recursordist/docs/metrics.rst
+++ b/pdns/recursordist/docs/metrics.rst
@@ -432,6 +432,12 @@ truncated-drops
 
 questions dropped because they were larger than 512 bytes
 
+empty-queries
+^^^^^^^^^^^^^
+.. versionadded:: 4.2
+
+questions dropped because they had a QD count of 0
+
 unauthorized-tcp
 ^^^^^^^^^^^^^^^^
 number of TCP questions denied because of   allow-from restrictions
diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index 6310c5bea1..053142f5b1 100644
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -930,6 +930,7 @@ struct RecursorStats
   std::atomic<uint64_t> packetCacheHits;
   std::atomic<uint64_t> noPacketError;
   std::atomic<uint64_t> ignoredCount;
+  std::atomic<uint64_t> emptyQueriesCount;
   time_t startupTime;
   std::atomic<uint64_t> dnssecQueries;
   unsigned int maxMThreadStackUsage;