From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Wed, 7 Dec 2022 09:54:49 +0000 (+0100)
Subject: rec: refactor unsuppored qtype code and make sure we ServFail on all unsupported... 
X-Git-Tag: rec-4.8.0^2~2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=90d3db1cfcac4783d8031cc57fcc489d457f6836;p=thirdparty%2Fpdns.git

rec: refactor unsuppored qtype code and make sure we ServFail on all unsupported qtypes

This fixes #12251

Also I'd like to know why we ServFail on NSEC3 but not on NSEC: we should either fix that or add a comment explaining this.

(cherry picked from commit e7cc83fd617c877f272b8344d5fb7252acfdc297)
---

diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc
index 986da266ab..ee826d784d 100644
--- a/pdns/pdns_recursor.cc
+++ b/pdns/pdns_recursor.cc
@@ -2203,16 +2203,6 @@ static string* doProcessUDPQuestion(const std::string& question, const ComboAddr
 
   auto dc = std::make_unique<DNSComboWriter>(question, g_now, std::move(policyTags), t_pdl, std::move(data), std::move(records));
 
-  if (SyncRes::isUnsupported(dc->d_mdp.d_qtype)) {
-    g_stats.ignoredCount++;
-    if (!g_quiet) {
-      SLOG(g_log << Logger::Notice << RecThreadInfo::id() << " Unsupported qtype " << dc->d_mdp.d_qtype << " from " << source.toStringWithPort() << (source != fromaddr ? " (via " + fromaddr.toStringWithPort() + ")" : "") << endl,
-           g_slogudpin->info(Logr::Notice, "Unsupported qtype", "qtype", Logging::Loggable(QType(dc->d_mdp.d_qtype)), "source", Logging::Loggable(source), "remote", Logging::Loggable(fromaddr)));
-    }
-
-    return 0;
-  }
-
   dc->setSocket(fd);
   dc->d_tag = ctag;
   dc->d_qhash = qhash;
diff --git a/pdns/qtype.hh b/pdns/qtype.hh
index d712317129..ab9d713c63 100644
--- a/pdns/qtype.hh
+++ b/pdns/qtype.hh
@@ -133,6 +133,10 @@ public:
 #endif
   };
 
+  const static uint16_t rfc6895MetaLowerBound = 128;
+  const static uint16_t rfc6895MetaUpperBound = 254; // Note 255: ANY is not included
+  const static uint16_t rfc6896Reserved = 65535;
+
   const static map<const string, uint16_t> names;
   const static map<uint16_t, const string> numbers;
 
diff --git a/pdns/recursordist/rec-tcp.cc b/pdns/recursordist/rec-tcp.cc
index b6deec10b5..e1bc143b6e 100644
--- a/pdns/recursordist/rec-tcp.cc
+++ b/pdns/recursordist/rec-tcp.cc
@@ -355,14 +355,6 @@ static void handleRunningTCPQuestion(int fd, FDMultiplexer::funcparam_t& var)
         }
         return;
       }
-      if (SyncRes::isUnsupported(dc->d_mdp.d_qtype)) {
-        g_stats.ignoredCount++;
-        if (g_logCommonErrors) {
-          SLOG(g_log << Logger::Error << "Unsupported qtype " << dc->d_mdp.d_qtype << " from TCP client " << conn->d_remote.toStringWithPort() << endl,
-               g_slogtcpin->info(Logr::Error, "Unsupported qtype from TCP client", "remote", Logging::Loggable(conn->d_remote), "qtype", Logging::Loggable(dc->d_mdp.d_qtype)));
-        }
-        return;
-      }
 
       dc->d_tcpConnection = conn; // carry the torch
       dc->setSocket(conn->getFD()); // this is the only time a copy is made of the actual fd
diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 006095d83d..88a55a3789 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -704,9 +704,7 @@ int SyncRes::beginResolve(const DNSName &qname, const QType qtype, QClass qclass
     return 0;                          // so do check before updating counters (we do now)
   }
 
-  auto qtypeCode = qtype.getCode();
-  /* rfc6895 section 3.1 */
-  if (qtypeCode == 0 || (qtypeCode >= 128 && qtypeCode <= 254) || qtypeCode == QType::RRSIG || qtypeCode == QType::NSEC3 || qtypeCode == QType::OPT || qtypeCode == 65535) {
+  if (isUnsupported(qtype)) {
     return -1;
   }
 
diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index dbd59d6fb3..2caf8d62ab 100644
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -436,10 +436,21 @@ public:
 
   static bool isUnsupported(QType qtype)
   {
-    switch (qtype.getCode()) {
+    auto qcode = qtype.getCode();
+    // rfc6895 section 3.1, note ANY is 255 and falls outside the range
+    if (qcode >= QType::rfc6895MetaLowerBound && qcode <= QType::rfc6895MetaUpperBound) {
+      return true;
+    }
+    switch (qcode) {
       // Internal types
-    case QType::ENT:
+    case QType::ENT: // aka TYPE0
     case QType::ADDR:
+      // RFC
+    case QType::rfc6896Reserved:
+      // Other
+    case QType::RRSIG:
+    case QType::NSEC3: // what about NSEC?
+    case QType::OPT:
       return true;
     }
     return false;