From: Otto <otto.moerbeek@open-xchange.com>
Date: Tue, 6 Apr 2021 11:20:36 +0000 (+0200)
Subject: Do not put results of DS query for auth or forward domains in negcache.
X-Git-Tag: dnsdist-1.6.0-rc2~16^2~2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6fa3df17b1fd369020601e804ee98e2e2c2882f0;p=thirdparty%2Fpdns.git

Do not put results of DS query for auth or forward domains in negcache.

Should fix #10189.
---

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 50586d8603..41cda1d9dc 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -406,6 +406,12 @@ bool SyncRes::isRecursiveForwardOrAuth(const DNSName &qname) const {
   return iter != t_sstorage.domainmap->end() && (iter->second.isAuth() || iter->second.shouldRecurse());
 }
 
+bool SyncRes::isForwardOrAuth(const DNSName &qname) const {
+  DNSName authname(qname);
+  domainmap_t::const_iterator iter = getBestAuthZone(&authname);
+  return iter != t_sstorage.domainmap->end() && (iter->second.isAuth() || !iter->second.shouldRecurse());
+}
+
 uint64_t SyncRes::doEDNSDump(int fd)
 {
   int newfd = dup(fd);
@@ -3418,9 +3424,16 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co
         continue;
       }
     }
+    const bool negCacheIndiction = rec.d_place == DNSResourceRecord::AUTHORITY && rec.d_type == QType::SOA &&
+      lwr.d_rcode == RCode::NXDomain && qname.isPartOf(rec.d_name) && rec.d_name.isPartOf(auth);
+
+    bool putInNegCache = true;
+    if (negCacheIndiction && isForwardOrAuth(qname)) {
+      // #10189, a NXDOMAIN to a DS query for a forwarded or auth domain should not NXDOMAIN the whole domain
+      putInNegCache = false;
+    }
 
-    if (rec.d_place == DNSResourceRecord::AUTHORITY && rec.d_type == QType::SOA &&
-        lwr.d_rcode == RCode::NXDomain && qname.isPartOf(rec.d_name) && rec.d_name.isPartOf(auth)) {
+    if (negCacheIndiction) {
       LOG(prefix<<qname<<": got negative caching indication for name '"<<qname<<"' (accept="<<rec.d_name.isPartOf(auth)<<"), newtarget='"<<newtarget<<"'"<<endl);
 
       rec.d_ttl = min(rec.d_ttl, s_maxnegttl);
@@ -3466,7 +3479,7 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co
          and do an additional query for the CNAME target.
          We have a regression test making sure we do exactly that.
       */
-      if (!wasVariable() && newtarget.empty()) {
+      if (!wasVariable() && newtarget.empty() && putInNegCache) {
         g_negCache->add(ne);
         if (s_rootNXTrust && ne.d_auth.isRoot() && auth.isRoot() && lwr.d_aabit) {
           ne.d_name = ne.d_name.getLastLabel();
diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index 98f8fe2d8d..bc09d5dcfc 100644
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -835,6 +835,7 @@ private:
   bool doOOBResolve(const AuthDomain& domain, const DNSName &qname, QType qtype, vector<DNSRecord>&ret, int& res);
   bool doOOBResolve(const DNSName &qname, QType qtype, vector<DNSRecord>&ret, unsigned int depth, int &res);
   bool isRecursiveForwardOrAuth(const DNSName &qname) const;
+  bool isForwardOrAuth(const DNSName &qname) const;
   domainmap_t::const_iterator getBestAuthZone(DNSName* qname) const;
   bool doCNAMECacheCheck(const DNSName &qname, QType qtype, vector<DNSRecord>&ret, unsigned int depth, int &res, vState& state, bool wasAuthZone, bool wasForwardRecurse);
   bool doCacheCheck(const DNSName &qname, const DNSName& authname, bool wasForwardedOrAuthZone, bool wasAuthZone, bool wasForwardRecurse, QType qtype, vector<DNSRecord>&ret, unsigned int depth, int &res, vState& state);