From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Fri, 29 Sep 2017 12:35:16 +0000 (+0200)
Subject: rec: When looking for a DS, skip NXD if the auth matches the qname
X-Git-Tag: rec-4.1.0-rc1~12^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b0c164a2b2ec8c71f67283ab1e72aeaeeeb4f178;p=thirdparty%2Fpdns.git

rec: When looking for a DS, skip NXD if the auth matches the qname
---

diff --git a/pdns/recursordist/negcache.cc b/pdns/recursordist/negcache.cc
index bba304be48..a9f306b7c6 100644
--- a/pdns/recursordist/negcache.cc
+++ b/pdns/recursordist/negcache.cc
@@ -69,13 +69,13 @@ bool NegCache::getRootNXTrust(const DNSName& qname, const struct timeval& now, N
  * \param ne       A NegCacheEntry that is filled when there is a cache entry
  * \return         true if ne was filled out, false otherwise
  */
-bool NegCache::get(const DNSName& qname, const QType& qtype, const struct timeval& now, NegCacheEntry& ne) {
+bool NegCache::get(const DNSName& qname, const QType& qtype, const struct timeval& now, NegCacheEntry& ne, bool typeMustMatch) {
   auto range = d_negcache.equal_range(tie(qname));
   negcache_t::iterator ni = range.first;
 
   while (ni != range.second) {
     // We have an entry
-    if (ni->d_qtype.getCode() == 0 || ni->d_qtype == qtype) {
+    if ((!typeMustMatch && ni->d_qtype.getCode() == 0) || ni->d_qtype == qtype) {
       // We match the QType or the whole name is denied
       if((uint32_t) now.tv_sec < ni->d_ttd) {
         // Not expired
diff --git a/pdns/recursordist/negcache.hh b/pdns/recursordist/negcache.hh
index e25fe5da98..336dc853cc 100644
--- a/pdns/recursordist/negcache.hh
+++ b/pdns/recursordist/negcache.hh
@@ -58,7 +58,7 @@ class NegCache : public boost::noncopyable {
     };
 
     void add(const NegCacheEntry& ne);
-    bool get(const DNSName& qname, const QType& qtype, const struct timeval& now, NegCacheEntry& ne);
+    bool get(const DNSName& qname, const QType& qtype, const struct timeval& now, NegCacheEntry& ne, bool typeMustMatch=false);
     bool getRootNXTrust(const DNSName& qname, const struct timeval& now, NegCacheEntry& ne);
     uint64_t count(const DNSName& qname) const;
     uint64_t count(const DNSName& qname, const QType qtype) const;
diff --git a/pdns/recursordist/test-negcache_cc.cc b/pdns/recursordist/test-negcache_cc.cc
index bc25c3a5cc..da1394750e 100644
--- a/pdns/recursordist/test-negcache_cc.cc
+++ b/pdns/recursordist/test-negcache_cc.cc
@@ -66,6 +66,27 @@ BOOST_AUTO_TEST_CASE(test_get_entry) {
   BOOST_CHECK_EQUAL(ne.d_auth, auth);
 }
 
+BOOST_AUTO_TEST_CASE(test_get_entry_exact_type) {
+  /* Add a full name negative entry to the cache and attempt to get an entry for
+   * the A record, asking only for an exact match.
+   */
+  DNSName qname("www2.powerdns.com");
+  DNSName auth("powerdns.com");
+
+  struct timeval now;
+  Utility::gettimeofday(&now, 0);
+
+  NegCache cache;
+  cache.add(genNegCacheEntry(qname, auth, now));
+
+  BOOST_CHECK_EQUAL(cache.size(), 1);
+
+  NegCache::NegCacheEntry ne;
+  bool ret = cache.get(qname, QType(1), now, ne, true);
+
+  BOOST_CHECK_EQUAL(ret, false);
+}
+
 BOOST_AUTO_TEST_CASE(test_get_NODATA_entry) {
   DNSName qname("www2.powerdns.com");
   DNSName auth("powerdns.com");
diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 9e039da927..c75c21901f 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -948,17 +948,24 @@ bool SyncRes::doCacheCheck(const DNSName &qname, const QType &qtype, vector<DNSR
   }
   else if (t_sstorage.negcache.get(qname, qtype, d_now, ne) &&
            !(wasForwardedOrAuth && ne.d_auth != authname)) { // Only the authname nameserver can neg cache entries
-    res = 0;
-    sttl = ne.d_ttd - d_now.tv_sec;
-    giveNegative = true;
-    cachedState = ne.d_validationState;
-    if(ne.d_qtype.getCode()) {
-      LOG(prefix<<qname<<": "<<qtype.getName()<<" is negatively cached via '"<<ne.d_auth<<"' for another "<<sttl<<" seconds"<<endl);
-      res = RCode::NoError;
-    }
-    else {
-      LOG(prefix<<qname<<": Entire name '"<<qname<<"', is negatively cached via '"<<ne.d_auth<<"' for another "<<sttl<<" seconds"<<endl);
-      res = RCode::NXDomain;
+
+    /* If we are looking for a DS, discard NXD if auth == qname
+       and ask for a specific denial instead */
+    if (qtype != QType::DS || ne.d_qtype.getCode() || ne.d_auth != qname ||
+        t_sstorage.negcache.get(qname, qtype, d_now, ne, true))
+    {
+      res = 0;
+      sttl = ne.d_ttd - d_now.tv_sec;
+      giveNegative = true;
+      cachedState = ne.d_validationState;
+      if(ne.d_qtype.getCode()) {
+        LOG(prefix<<qname<<": "<<qtype.getName()<<" is negatively cached via '"<<ne.d_auth<<"' for another "<<sttl<<" seconds"<<endl);
+        res = RCode::NoError;
+      }
+      else {
+        LOG(prefix<<qname<<": Entire name '"<<qname<<"', is negatively cached via '"<<ne.d_auth<<"' for another "<<sttl<<" seconds"<<endl);
+        res = RCode::NXDomain;
+      }
     }
   }