From 5e7d145081eee9cd0ee952dde445215853babe10 Mon Sep 17 00:00:00 2001
From: Peter van Dijk <peter.van.dijk@powerdns.com>
Date: Tue, 10 Nov 2020 11:56:52 +0100
Subject: [PATCH] rec: do not send overly long NOD lookups

---
 pdns/pdns_recursor.cc              |  9 ++++++++-
 pdns/rec-snmp.cc                   |  2 ++
 pdns/rec_channel_rec.cc            |  2 ++
 pdns/recursordist/RECURSOR-MIB.txt | 11 ++++++++++-
 pdns/recursordist/rec_metrics.hh   |  3 +++
 pdns/syncres.hh                    |  1 +
 6 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc
index f552d569ad..17315b6047 100644
--- a/pdns/pdns_recursor.cc
+++ b/pdns/pdns_recursor.cc
@@ -1218,7 +1218,14 @@ static void sendNODLookup(const DNSName& dname)
     // Send a DNS A query to <domain>.g_nodLookupDomain
     static const QType qt(QType::A);
     static const uint16_t qc(QClass::IN);
-    DNSName qname = dname + g_nodLookupDomain;
+    DNSName qname;
+    try {
+      qname = dname + g_nodLookupDomain;
+    }
+    catch(const std::range_error &e) {
+      ++g_stats.nodLookupsDroppedOversize;
+      return;
+    }
     vector<DNSRecord> dummy;
     directResolve(qname, qt, qc, dummy);
   }
diff --git a/pdns/rec-snmp.cc b/pdns/rec-snmp.cc
index f7a4138a23..1065be340f 100644
--- a/pdns/rec-snmp.cc
+++ b/pdns/rec-snmp.cc
@@ -121,6 +121,7 @@ static const oid qnameMinFallbackSuccessOID[] = { RECURSOR_STATS_OID, 100 };
 static const oid proxyProtocolInvalidOID[] = { RECURSOR_STATS_OID, 101 };
 static const oid recordCacheContendedOID[] = { RECURSOR_STATS_OID, 102 };
 static const oid recordCacheAcquiredOID[] = { RECURSOR_STATS_OID, 103 };
+static const oid nodLookupsDroppedOversizeOID[] = { RECURSOR_STATS_OID, 104 };
 
 static std::unordered_map<oid, std::string> s_statsMap;
 
@@ -333,5 +334,6 @@ RecursorSNMPAgent::RecursorSNMPAgent(const std::string& name, const std::string&
   registerCounter64Stat("proxy-protocol-invalid", proxyProtocolInvalidOID, OID_LENGTH(proxyProtocolInvalidOID));
   registerCounter64Stat("record-cache-contended", recordCacheContendedOID, OID_LENGTH(recordCacheContendedOID));
   registerCounter64Stat("record-cache-acquired", recordCacheAcquiredOID, OID_LENGTH(recordCacheAcquiredOID));
+  registerCounter64Stat("nod-lookups-dropped-oversize", nodLookupsDroppedOversizeOID, OID_LENGTH(nodLookupsDroppedOversizeOID));
 #endif /* HAVE_NET_SNMP */
 }
diff --git a/pdns/rec_channel_rec.cc b/pdns/rec_channel_rec.cc
index 150ec8014e..af2914b03f 100644
--- a/pdns/rec_channel_rec.cc
+++ b/pdns/rec_channel_rec.cc
@@ -1220,6 +1220,8 @@ void registerAllStats()
 
   addGetStat("proxy-protocol-invalid", &g_stats.proxyProtocolInvalidCount);
 
+  addGetStat("nod-lookups-dropped-oversize", &g_stats.nodLookupsDroppedOversize);
+
   /* make sure that the ECS stats are properly initialized */
   SyncRes::clearECSStats();
   for (size_t idx = 0; idx < SyncRes::s_ecsResponsesBySubnetSize4.size(); idx++) {
diff --git a/pdns/recursordist/RECURSOR-MIB.txt b/pdns/recursordist/RECURSOR-MIB.txt
index 9d735211fb..99903c184a 100644
--- a/pdns/recursordist/RECURSOR-MIB.txt
+++ b/pdns/recursordist/RECURSOR-MIB.txt
@@ -863,6 +863,14 @@ recordCacheAcquired OBJECT-TYPE
         "Number of record cache lock acquisitions"
     ::= { stats 103 }
 
+nodLookupsDroppedOversize OBJECT-TYPE
+    SYNTAX Counter64
+    MAX-ACCESS read-only
+    STATUS current
+    DESCRIPTION
+        "Number of NOD lookups dropped because they would exceed the maximum name length"
+    ::= { stats 104 }
+
 ---
 --- Traps / Notifications
 ---
@@ -1009,7 +1017,8 @@ recGroup OBJECT-GROUP
         qnameMinFallbackSuccess,
         proxyProtocolInvalid,
         recordCacheContended,
-        recordCacheAcquired
+        recordCacheAcquired,
+        nodLookupsDroppedOversize
     }
     STATUS current
     DESCRIPTION "Objects conformance group for PowerDNS Recursor"
diff --git a/pdns/recursordist/rec_metrics.hh b/pdns/recursordist/rec_metrics.hh
index 4a0be9e394..fc363e876c 100644
--- a/pdns/recursordist/rec_metrics.hh
+++ b/pdns/recursordist/rec_metrics.hh
@@ -251,6 +251,9 @@ private:
     {"no-packet-error",
       MetricDefinition(PrometheusMetricType::counter,
         "Number of erroneous received packets")},
+    {"nod-lookups-dropped-oversize",
+      MetricDefinition(PrometheusMetricType::counter,
+        "Number of NOD lookups dropped because they would exceed the maximum name length")},
     {"noedns-outqueries",
       MetricDefinition(PrometheusMetricType::counter,
         "Number of queries sent out without EDNS")},
diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index b38203bada..dab8f86526 100644
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -1012,6 +1012,7 @@ struct RecursorStats
   std::map<DNSFilterEngine::PolicyKind, std::atomic<uint64_t> > policyResults;
   std::atomic<uint64_t> rebalancedQueries{0};
   std::atomic<uint64_t> proxyProtocolInvalidCount{0};
+  std::atomic<uint64_t> nodLookupsDroppedOversize{0};
 };
 
 //! represents a running TCP/IP client session
-- 
2.47.2