From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Wed, 16 Dec 2020 14:51:19 +0000 (+0100)
Subject: rec: Lookup DS entries before CNAME entries
X-Git-Tag: rec-4.4.3~4^2~6
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=921eafbcefdfb4728143af7df92f1f71ada7af1d;p=thirdparty%2Fpdns.git

rec: Lookup DS entries before CNAME entries

When we are looking for a DS, we want to do the non-CNAME cache check first
because we can actually have a DS (from the parent zone) AND a CNAME (from
the child zone), and what we really want is the DS.

(cherry picked from commit 38263b88f2b3fa979cb1067ce602f48a1a4678bb)
---

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 8ca0fa71ca..9d19be6a46 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -896,7 +896,10 @@ int SyncRes::doResolveNoQNameMinimization(const DNSName &qname, const QType &qty
       }
     }
 
-    if (doCNAMECacheCheck(qname, qtype, ret, depth, res, state, wasAuthZone, wasForwardRecurse)) { // will reroute us if needed
+    /* When we are looking for a DS, we want to the non-CNAME cache check first
+       because we can actually have a DS (from the parent zone) AND a CNAME (from
+       the child zone), and what we really want is the DS */
+    if (qtype != QType::DS && doCNAMECacheCheck(qname, qtype, ret, depth, res, state, wasAuthZone, wasForwardRecurse)) { // will reroute us if needed
       d_wasOutOfBand = wasAuthZone;
       // Here we have an issue. If we were prevented from going out to the network (cache-only was set, possibly because we
       // are in QM Step0) we might have a CNAME but not the corresponding target.
@@ -944,6 +947,37 @@ int SyncRes::doResolveNoQNameMinimization(const DNSName &qname, const QType &qty
 
       return res;
     }
+
+    /* if we have not found a cached DS (or denial of), now is the time to look for a CNAME */ 
+    if (qtype == QType::DS && doCNAMECacheCheck(qname, qtype, ret, depth, res, state, wasAuthZone, wasForwardRecurse)) { // will reroute us if needed
+      d_wasOutOfBand = wasAuthZone;
+      // Here we have an issue. If we were prevented from going out to the network (cache-only was set, possibly because we
+      // are in QM Step0) we might have a CNAME but not the corresponding target.
+      // It means that we will sometimes go to the next steps when we are in fact done, but that's fine since
+      // we will get the records from the cache, resulting in a small overhead.
+      // This might be a real problem if we had a RPZ hit, though, because we do not want the processing to continue, since
+      // RPZ rules will not be evaluated anymore (we already matched).
+      const bool stoppedByPolicyHit = d_appliedPolicy.wasHit();
+
+      if (fromCache && (!d_cacheonly || stoppedByPolicyHit)) {
+        *fromCache = true;
+      }
+      /* Apply Post filtering policies */
+
+      if (d_wantsRPZ && !stoppedByPolicyHit) {
+        auto luaLocal = g_luaconfs.getLocal();
+        if (luaLocal->dfe.getPostPolicy(ret, d_discardedPolicies, d_appliedPolicy)) {
+          mergePolicyTags(d_policyTags, d_appliedPolicy.getTags());
+          bool done = false;
+          handlePolicyHit(prefix, qname, qtype, ret, done, res, depth);
+          if (done && fromCache) {
+            *fromCache = true;
+          }
+        }
+      }
+
+      return res;
+    }
   }
 
   if (d_cacheonly) {