From 9a0d2e45023476f937f499a69ccfbcf4a78e0b4b Mon Sep 17 00:00:00 2001
From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Tue, 4 Aug 2020 10:47:25 +0200
Subject: [PATCH] Added depth check for qnameRPZHit() and some comments.

---
 pdns/syncres.cc | 17 ++++++++++++++---
 pdns/syncres.hh |  2 +-
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 5111311455..97ce0595ca 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -638,11 +638,18 @@ int SyncRes::asyncresolveWrapper(const ComboAddress& ip, bool ednsMANDATORY, con
   return ret;
 }
 
-bool SyncRes::qnameRPZHit(const DNSFilterEngine& dfe, DNSName& target, const QType& qtype, vector<DNSRecord> &ret)
+bool SyncRes::qnameRPZHit(const DNSFilterEngine& dfe, DNSName& target, const QType& qtype, vector<DNSRecord> &ret, unsigned int depth)
 {
   if (!d_wantsRPZ) {
     return false;
   }
+  if (s_maxdepth && depth > s_maxdepth) {
+    string prefix = d_prefix;
+    prefix.append(depth, ' ');
+    string msg = "More than " + std::to_string(s_maxdepth) + " (max-recursion-depth) levels of recursion needed while resolving " + target.toLogString();
+    LOG(prefix << target << ": " << msg << endl);
+    throw ImmediateServFailException(msg);
+  }
 
   bool match = dfe.getQueryPolicy(target, d_discardedPolicies, d_appliedPolicy, true);
   if (!match) {
@@ -662,6 +669,8 @@ bool SyncRes::qnameRPZHit(const DNSFilterEngine& dfe, DNSName& target, const QTy
     return true;
   }
   auto spoofed = d_appliedPolicy.getCustomRecords(target, qtype.getCode());
+
+  // Add the record to the result vector being built, chase if we hit a CNAME
   for (const auto& dr : spoofed) {
     if (dr.d_place != DNSResourceRecord::ANSWER) {
       continue;
@@ -670,7 +679,9 @@ bool SyncRes::qnameRPZHit(const DNSFilterEngine& dfe, DNSName& target, const QTy
     auto content = getRR<CNAMERecordContent>(dr);
     if (content) {
       target = content->getTarget();
-      return qnameRPZHit(dfe, target, qtype, ret);
+      // This call wil return true if we hit a policy that needs an throw PolicyHitException
+      // For CNAME chasing, we don't want that since resolving should continue with the new target
+      return qnameRPZHit(dfe, target, qtype, ret, depth + 1);
     }
   }
 
@@ -685,7 +696,7 @@ int SyncRes::doResolve(const DNSName &qnameArg, const QType &qtype, vector<DNSRe
   auto luaconfsLocal = g_luaconfs.getLocal();
 
   // Can change qname
-  bool hit = qnameRPZHit(luaconfsLocal->dfe, qname, qtype, ret);
+  bool hit = qnameRPZHit(luaconfsLocal->dfe, qname, qtype, ret, depth + 1);
   if (hit) {
     throw PolicyHitException();
   }
diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index 426c048075..48d4ea86f6 100644
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -823,7 +823,7 @@ private:
   bool doResolveAtThisIP(const std::string& prefix, const DNSName& qname, const QType& qtype, LWResult& lwr, boost::optional<Netmask>& ednsmask, const DNSName& auth, bool const sendRDQuery, const DNSName& nsName, const ComboAddress& remoteIP, bool doTCP, bool* truncated);
   bool processAnswer(unsigned int depth, LWResult& lwr, const DNSName& qname, const QType& qtype, DNSName& auth, bool wasForwarded, const boost::optional<Netmask> ednsmask, bool sendRDQuery, NsSet &nameservers, std::vector<DNSRecord>& ret, const DNSFilterEngine& dfe, bool* gotNewServers, int* rcode, vState& state);
 
-  bool qnameRPZHit(const DNSFilterEngine& dfe, DNSName& target, const QType& qtype, vector<DNSRecord>& ret);
+  bool qnameRPZHit(const DNSFilterEngine& dfe, DNSName& target, const QType& qtype, vector<DNSRecord>& ret, unsigned int depth);
   int doResolve(const DNSName &qname, const QType &qtype, vector<DNSRecord>&ret, unsigned int depth, set<GetBestNSAnswer>& beenthere, vState& state);
   int doResolveNoQNameMinimization(const DNSName &qname, const QType &qtype, vector<DNSRecord>&ret, unsigned int depth, set<GetBestNSAnswer>& beenthere, vState& state, bool* fromCache = NULL, StopAtDelegation* stopAtDelegation = NULL, bool considerforwards = true);
   bool doOOBResolve(const AuthDomain& domain, const DNSName &qname, const QType &qtype, vector<DNSRecord>&ret, int& res);
-- 
2.47.2