From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Thu, 23 Mar 2023 10:42:35 +0000 (+0100)
Subject: rec: re-establish "recursion depth is always increasing" invariant
X-Git-Tag: dnsdist-1.8.0~1^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8fb1bf41b3708780cd76cbdf922e1886ef7efd3e;p=thirdparty%2Fpdns.git

rec: re-establish "recursion depth is always increasing" invariant

Now that we have getQMFallbackMode(), we can go back to always increase depth
and never decrease it and adapt the upper bound check if needed.

This should prevent a re-occurence of a bug similar to PowerDNS Security Advisory 2023-01.
---

diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc
index 122ad98aa5..dd42ba82c0 100644
--- a/pdns/recursordist/syncres.cc
+++ b/pdns/recursordist/syncres.cc
@@ -1768,10 +1768,7 @@ int SyncRes::doResolve(const DNSName& qname, const QType qtype, vector<DNSRecord
         setQNameMinimization(false);
         setQMFallbackMode(true);
 
-        // We might have hit a depth level check, but we still want to allow some recursion levels in the fallback
-        // no-qname-minimization case. This has the effect that a qname minimization fallback case might reach 150% of
-        // maxdepth.
-        res = doResolveNoQNameMinimization(qname, qtype, ret, depth / 2, beenthere, context);
+        res = doResolveNoQNameMinimization(qname, qtype, ret, depth + 1, beenthere, context);
 
         if (res == RCode::NoError) {
           t_Counters.at(rec::Counter::qnameminfallbacksuccess)++;
@@ -1805,11 +1802,21 @@ int SyncRes::doResolveNoQNameMinimization(const DNSName& qname, const QType qtyp
 
   LOG(prefix << qname << ": Wants " << (d_doDNSSEC ? "" : "NO ") << "DNSSEC processing, " << (d_requireAuthData ? "" : "NO ") << "auth data required by query for " << qtype << endl);
 
-  if (s_maxdepth > 0 && depth > s_maxdepth) {
-    string msg = "More than " + std::to_string(s_maxdepth) + " (max-recursion-depth) levels of recursion needed while resolving " + qname.toLogString();
-    LOG(prefix << qname << ": " << msg << endl);
-    throw ImmediateServFailException(msg);
+  if (s_maxdepth > 0) {
+    auto bound = s_maxdepth;
+    if (getQMFallbackMode()) {
+      // We might have hit a depth level check, but we still want to allow some recursion levels in the fallback
+      // no-qname-minimization case. This has the effect that a qname minimization fallback case might reach 150% of
+      // maxdepth, taking care to not repeatedly increase the bound.
+      bound += s_maxdepth / 2;
+    }
+    if (depth > bound) {
+      string msg = "More than " + std::to_string(bound) + " (adjusted max-recursion-depth) levels of recursion needed while resolving " + qname.toLogString();
+      LOG(prefix << qname << ": " << msg << endl);
+      throw ImmediateServFailException(msg);
+    }
   }
+
   int res = 0;
 
   const int iterations = !d_refresh && MemRecursorCache::s_maxServedStaleExtensions > 0 ? 2 : 1;