rec: make max CNAME chain length handled settable, previously fixed at 10

diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc
index 2b2d1705b3ae3b8023ee1af112aa21ce92bba9bb..4dcabb35ef2d1d937b62c8460548d0ee93b37302 100644 (file)
--- a/pdns/recursordist/rec-main.cc
+++ b/pdns/recursordist/rec-main.cc
@@ -1750,6 +1750,7 @@ static int initSyncRes(Logr::log_t log)
   SyncRes::s_event_trace_enabled = ::arg().asNum("event-trace-enabled");
   SyncRes::s_save_parent_ns_set = ::arg().mustDo("save-parent-ns-set");
   SyncRes::s_max_busy_dot_probes = ::arg().asNum("max-busy-dot-probes");
+  SyncRes::s_max_CNAMES_followed = ::arg().asNum("max-cnames-followed");
   {
     uint64_t sse = ::arg().asNum("serve-stale-extensions");
     if (sse > std::numeric_limits<uint16_t>::max()) {

diff --git a/pdns/recursordist/settings/table.py b/pdns/recursordist/settings/table.py
index e16200c7d11e708318c2c1826179766cadfa7704..5a072bc6d1ebdf4a7b14710f0186f51bfdef4c6b 100644 (file)
--- a/pdns/recursordist/settings/table.py
+++ b/pdns/recursordist/settings/table.py
@@ -1525,6 +1525,18 @@ This is used to avoid cycles resolving names.
  ''',
         'versionchanged': ('5.1.0', 'The default used to be 60, with an extra allowance if qname minimization was enabled. Having better algorithms allows for a lower default limit.'),
     },
+    {
+        'name' : 'max_cnames_followed',
+        'section' : 'recursor',
+        'type' : LType.Uint64,
+        'default' : '10',
+        'help' : 'Maximum number CNAME records followed',
+        'doc' : '''
+Maximum length of a CNAME chain. If a CNAME chain exceeds this length, a ``ServFail`` answer will be returned.
+Previously, this limit was fixed at 10.
+ ''',
+    'versionadded': '5.1.0'
+    },    
     {
         'name' : 'max_ns_address_qperq',
         'section' : 'outgoing',

diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc
index e54c88ed5162a8dbcf35090a66ebfc14fba461f9..3a19b0bfcd4dd01f713bd40c7a852a604cdbd5c5 100644 (file)
--- a/pdns/recursordist/syncres.cc
+++ b/pdns/recursordist/syncres.cc
@@ -474,7 +474,7 @@ bool SyncRes::s_dot_to_port_853;
 int SyncRes::s_event_trace_enabled;
 bool SyncRes::s_save_parent_ns_set;
 unsigned int SyncRes::s_max_busy_dot_probes;
-unsigned int SyncRes::s_max_CNAMES_followed = 10;
+unsigned int SyncRes::s_max_CNAMES_followed;
 bool SyncRes::s_addExtendedResolutionDNSErrors;
 
 // NOLINTNEXTLINE(cppcoreguidelines-macro-usage)

diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc
index d0b5892e16704629b1e397c756e527a9f8c9e5cc..7f253647d1183ca301ed23a7c26b77dbbc4fd9b5 100644 (file)
--- a/pdns/recursordist/test-syncres_cc.cc
+++ b/pdns/recursordist/test-syncres_cc.cc
@@ -186,6 +186,7 @@ void initSR(bool debug)
   SyncRes::s_locked_ttlperc = 0;
   SyncRes::s_minimize_one_label = 4;
   SyncRes::s_max_minimize_count = 10;
+  SyncRes::s_max_CNAMES_followed = 10;
 
   SyncRes::clearNSSpeeds();
   BOOST_CHECK_EQUAL(SyncRes::getNSSpeedsSize(), 0U);

diff --git a/pdns/recursordist/test-syncres_cc1.cc b/pdns/recursordist/test-syncres_cc1.cc
index a844564523ffa2f717b5d32385b10a6bd11448e4..86278fbe642e5153b6a61ceaeb6fe0a328fea6d5 100644 (file)
--- a/pdns/recursordist/test-syncres_cc1.cc
+++ b/pdns/recursordist/test-syncres_cc1.cc
@@ -1771,6 +1771,18 @@ BOOST_AUTO_TEST_CASE(test_cname_length)
   BOOST_CHECK_EQUAL(res, RCode::ServFail);
   BOOST_CHECK_EQUAL(ret.size(), length);
   BOOST_CHECK_EQUAL(length, SyncRes::s_max_CNAMES_followed + 1);
+
+  // Currently a CNAME bounds check originating from the record cache causes an ImmediateServFail
+  // exception. This is different from the non-cached case, tested above. There a ServFail is
+  // returned with a partial CNAME chain. This should be fixed one way or another. For details, see
+  // how the result of syncres.cc:scanForCNAMELoop() is handled in the two cases.
+  ret.clear();
+  length = 0;
+  BOOST_CHECK_EXCEPTION(sr->beginResolve(target, QType(QType::A), QClass::IN, ret),
+                        ImmediateServFailException,
+                        [&](const ImmediateServFailException& isfe) {
+                          return isfe.reason == "max number of CNAMEs exceeded";
+                        });
 }
 
 BOOST_AUTO_TEST_CASE(test_cname_target_servfail)