From: Peter van Dijk <peter.van.dijk@powerdns.com>
Date: Tue, 15 Aug 2023 11:33:08 +0000 (+0200)
Subject: ixfrdist: check opcode of incoming query
X-Git-Tag: rec-5.0.0-beta1~38^2~6
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=5b5465661fae296bd192fa23136fdaf8c0896092;p=thirdparty%2Fpdns.git

ixfrdist: check opcode of incoming query
---

diff --git a/pdns/ixfrdist-stats.cc b/pdns/ixfrdist-stats.cc
index 7d3bdfb0b5..0a545c59c1 100644
--- a/pdns/ixfrdist-stats.cc
+++ b/pdns/ixfrdist-stats.cc
@@ -85,6 +85,15 @@ std::string ixfrdistStats::getStats() {
     stats<<prefix<<"ixfr_failures_total{domain=\""<<d.first<<"\"} "<<d.second.numIXFRFailures<<std::endl;
   }
 
+  if (!notimpStats.empty()) {
+    stats<<"# HELP "<<prefix<<"notimp An unimplemented opcode"<<std::endl;
+    stats<<"# TYPE "<<prefix<<"notimp counter"<<std::endl;
+  }
+
+  for (auto const &d : notimpStats) {
+    stats<<prefix<<"notimp{opcode=\""<<d.first<<"\"} "<<d.second<<std::endl;
+  }
+
   stats<<"# HELP "<<prefix<<"unknown_domain_inqueries_total Number of queries received for domains unknown to us"<<std::endl;
   stats<<"# TYPE "<<prefix<<"unknown_domain_inqueries_total counter"<<std::endl;
   stats<<prefix<<"unknown_domain_inqueries_total "<<progStats.unknownDomainInQueries<<std::endl;
diff --git a/pdns/ixfrdist-stats.hh b/pdns/ixfrdist-stats.hh
index 755e57bfb6..cb1f402d71 100644
--- a/pdns/ixfrdist-stats.hh
+++ b/pdns/ixfrdist-stats.hh
@@ -24,6 +24,7 @@
 #include <map>
 #include <string>
 
+#include "dns.hh"
 #include "dnsname.hh"
 #include "pdnsexception.hh"
 
@@ -70,6 +71,11 @@ class ixfrdistStats {
       progStats.unknownDomainInQueries += 1;
     }
 
+    void incrementNotImplemented(const string& opcode)
+    {
+      notimpStats[opcode] ++;
+    }
+
   private:
     class perDomainStat {
       public:
@@ -93,6 +99,7 @@ class ixfrdistStats {
     };
 
     std::map<DNSName, perDomainStat> domainStats;
+    std::map<string, std::atomic<uint64_t>> notimpStats;
     programStats progStats;
 
     std::map<DNSName, perDomainStat>::iterator getRegisteredDomain(const DNSName& d) {
diff --git a/pdns/ixfrdist.cc b/pdns/ixfrdist.cc
index 8fba8199c3..fde9933454 100644
--- a/pdns/ixfrdist.cc
+++ b/pdns/ixfrdist.cc
@@ -19,6 +19,7 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
+#include "dns.hh"
 #ifdef HAVE_CONFIG_H
 #include "config.h"
 #endif
@@ -474,33 +475,52 @@ static void updateThread(const string& workdir, const uint16_t& keep, const uint
   } /* while (true) */
 } /* updateThread */
 
-static bool checkQuery(const MOADNSParser& mdp, const ComboAddress& saddr, const bool udp = true, const string& logPrefix="") {
+enum class ResponseType {
+  // Unknown,
+  ValidQuery,
+  RefusedOpcode,
+  RefusedQuery
+};
+
+static ResponseType checkQuery(const MOADNSParser& mdp, const ComboAddress& saddr, const bool udp = true, const string& logPrefix="") {
   vector<string> info_msg;
 
-  g_log<<Logger::Debug<<logPrefix<<"Had "<<mdp.d_qname<<"|"<<QType(mdp.d_qtype).toString()<<" query from "<<saddr.toStringWithPort()<<endl;
+  auto ret = ResponseType::ValidQuery;
 
-  if (udp && mdp.d_qtype != QType::SOA && mdp.d_qtype != QType::IXFR) {
-    info_msg.push_back("QType is unsupported (" + QType(mdp.d_qtype).toString() + " is not in {SOA,IXFR})");
-  }
+  g_log<<Logger::Debug<<logPrefix<<"Had "<<mdp.d_qname<<"|"<<QType(mdp.d_qtype).toString()<<" query from "<<saddr.toStringWithPort()<<endl;
 
-  if (!udp && mdp.d_qtype != QType::SOA && mdp.d_qtype != QType::IXFR && mdp.d_qtype != QType::AXFR) {
-    info_msg.push_back("QType is unsupported (" + QType(mdp.d_qtype).toString() + " is not in {SOA,IXFR,AXFR})");
+  if (mdp.d_header.opcode != Opcode::Query) {
+    info_msg.push_back("Opcode is unsupported (" + Opcode::to_s(mdp.d_header.opcode) + "), expected QUERY");
+    ret = ResponseType::RefusedOpcode;
   }
+  else {
+    if (udp && mdp.d_qtype != QType::SOA && mdp.d_qtype != QType::IXFR) {
+      info_msg.push_back("QType is unsupported (" + QType(mdp.d_qtype).toString() + " is not in {SOA,IXFR})");
+      ret = ResponseType::RefusedQuery;
+    }
 
-  {
-    if (g_domainConfigs.find(mdp.d_qname) == g_domainConfigs.end()) {
-      info_msg.push_back("Domain name '" + mdp.d_qname.toLogString() + "' is not configured for distribution");
+    if (!udp && mdp.d_qtype != QType::SOA && mdp.d_qtype != QType::IXFR && mdp.d_qtype != QType::AXFR) {
+      info_msg.push_back("QType is unsupported (" + QType(mdp.d_qtype).toString() + " is not in {SOA,IXFR,AXFR})");
+      ret = ResponseType::RefusedQuery;
     }
-    else {
-      const auto zoneInfo = getCurrentZoneInfo(mdp.d_qname);
-      if (zoneInfo == nullptr) {
-        info_msg.push_back("Domain has not been transferred yet");
+
+    {
+      if (g_domainConfigs.find(mdp.d_qname) == g_domainConfigs.end()) {
+        info_msg.push_back("Domain name '" + mdp.d_qname.toLogString() + "' is not configured for distribution");
+        ret = ResponseType::RefusedQuery;
+      }
+      else {
+        const auto zoneInfo = getCurrentZoneInfo(mdp.d_qname);
+        if (zoneInfo == nullptr) {
+          info_msg.push_back("Domain has not been transferred yet");
+          ret = ResponseType::RefusedQuery;
+        }
       }
     }
   }
 
-  if (!info_msg.empty()) {
-    g_log<<Logger::Warning<<logPrefix<<"Refusing "<<mdp.d_qname<<"|"<<QType(mdp.d_qtype).toString()<<" query from "<<saddr.toStringWithPort();
+  if (!info_msg.empty()) {  // which means ret is not SOA
+    g_log<<Logger::Warning<<logPrefix<<"Refusing "<<mdp.d_qname<<"|"<<QType(mdp.d_qtype).toString()<<" "<< Opcode::to_s(mdp.d_header.opcode) <<" from "<<saddr.toStringWithPort();
     g_log<<Logger::Warning<<": ";
     bool first = true;
     for (const auto& s : info_msg) {
@@ -511,10 +531,10 @@ static bool checkQuery(const MOADNSParser& mdp, const ComboAddress& saddr, const
       g_log<<Logger::Warning<<s;
     }
     g_log<<Logger::Warning<<endl;
-    return false;
+    // fall through to return below
   }
 
-  return true;
+  return ret;
 }
 
 /*
@@ -555,6 +575,21 @@ static bool makeRefusedPacket(const MOADNSParser& mdp, vector<uint8_t>& packet)
   return true;
 }
 
+/*
+ * Returns a vector<uint8_t> that represents the full NOTIMP response to a
+ * query. QNAME and type are read from mdp.
+ */
+static bool makeNotimpPacket(const MOADNSParser& mdp, vector<uint8_t>& packet) {
+  DNSPacketWriter pw(packet, mdp.d_qname, mdp.d_qtype);
+  pw.getHeader()->id = mdp.d_header.id;
+  pw.getHeader()->rd = mdp.d_header.rd;
+  pw.getHeader()->qr = 1;
+  pw.getHeader()->rcode = RCode::NotImp;
+  pw.getHeader()->opcode = mdp.d_header.opcode;
+
+  return true;
+}
+
 static vector<uint8_t> getSOAPacket(const MOADNSParser& mdp, const shared_ptr<const SOARecordContent>& soa, uint32_t soaTTL) {
   vector<uint8_t> packet;
   DNSPacketWriter pw(packet, mdp.d_qname, mdp.d_qtype);
@@ -783,7 +818,9 @@ static bool allowedByACL(const ComboAddress& addr) {
   return g_acl.match(addr);
 }
 
-static void handleUDPRequest(int fd, boost::any&) {
+static void handleUDPRequest(int fd, boost::any&)
+try
+{
   // TODO make the buffer-size configurable
   char buf[4096];
   ComboAddress saddr;
@@ -813,7 +850,8 @@ static void handleUDPRequest(int fd, boost::any&) {
 
   MOADNSParser mdp(true, string(buf, res));
   vector<uint8_t> packet;
-  if (checkQuery(mdp, saddr)) {
+  auto respt = checkQuery(mdp, saddr);
+  if (respt == ResponseType::ValidQuery) {
     /* RFC 1995 Section 2
      *    Transport of a query may be by either UDP or TCP.  If an IXFR query
      *    is via UDP, the IXFR server may attempt to reply using UDP if the
@@ -827,9 +865,12 @@ static void handleUDPRequest(int fd, boost::any&) {
      */
     g_stats.incrementSOAinQueries(mdp.d_qname); // FIXME: this also counts IXFR queries (but the response is the same as to a SOA query)
     makeSOAPacket(mdp, packet);
-  } else {
+  } else if (respt == ResponseType::RefusedQuery) {
     g_stats.incrementUnknownDomainInQueries(mdp.d_qname);
     makeRefusedPacket(mdp, packet);
+  } else if (respt == ResponseType::RefusedOpcode) {
+    g_stats.incrementNotImplemented(Opcode::to_s(mdp.d_header.opcode));
+    makeNotimpPacket(mdp, packet);
   }
 
   if(sendto(fd, &packet[0], packet.size(), 0, (struct sockaddr*) &saddr, fromlen) < 0) {
@@ -838,6 +879,10 @@ static void handleUDPRequest(int fd, boost::any&) {
   }
   return;
 }
+catch(std::exception& e) {
+  return;
+}
+
 
 static void handleTCPRequest(int fd, boost::any&) {
   ComboAddress saddr;
@@ -909,7 +954,9 @@ static void tcpWorker(int tid) {
     try {
       MOADNSParser mdp(true, string(buf, res));
 
-      if (!checkQuery(mdp, saddr, false, prefix)) {
+      auto respt = checkQuery(mdp, saddr, false, prefix);
+
+      if (respt != ResponseType::ValidQuery) { // on TCP, we currently do not bother with sending useful errors
         close(cfd);
         continue;
       }