Do cache negcache results, even when wasVariable() is true

See https://datatracker.ietf.org/doc/html/rfc7871#section-7.4
Fixes #10994

(cherry picked from commit 2bcec14adbffbf4b28d698cb607877fb96054e87)

diff --git a/pdns/recursordist/test-syncres_cc2.cc b/pdns/recursordist/test-syncres_cc2.cc
index a4984df78cf2bf68a706c42d0308122d7d5bcf09..b0ccc42439bea78b4771276b6cb15d7f833db7dd 100644 (file)
--- a/pdns/recursordist/test-syncres_cc2.cc
+++ b/pdns/recursordist/test-syncres_cc2.cc
@@ -1107,7 +1107,7 @@ BOOST_AUTO_TEST_CASE(test_rfc8020_nodata_bis)
   BOOST_CHECK_EQUAL(g_negCache->size(), 2U);
 }
 
-BOOST_AUTO_TEST_CASE(test_skip_negcache_for_variable_response)
+BOOST_AUTO_TEST_CASE(test_dont_skip_negcache_for_variable_response)
 {
   std::unique_ptr<SyncRes> sr;
   initSR(sr);
@@ -1160,8 +1160,7 @@ BOOST_AUTO_TEST_CASE(test_skip_negcache_for_variable_response)
   int res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret);
   BOOST_CHECK_EQUAL(res, RCode::NXDomain);
   BOOST_CHECK_EQUAL(ret.size(), 2U);
-  /* no negative cache entry because the response was variable */
-  BOOST_CHECK_EQUAL(g_negCache->size(), 0U);
+  BOOST_CHECK_EQUAL(g_negCache->size(), 1U);
 }
 
 BOOST_AUTO_TEST_CASE(test_ecs_cache_limit_allowed)

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 78ca2da00c988f5b03c33a518e80d64c41064f5e..27ae2645f069b9546ed820b4dbe1dba9585aea6c 100644 (file)
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -3634,7 +3634,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() && putInNegCache) {
+      if (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();
@@ -3808,9 +3808,7 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co
             }
             LOG(prefix<<qname<<": got negative indication of DS record for '"<<newauth<<"'"<<endl);
 
-            if (!wasVariable()) {
-              g_negCache->add(ne);
-            }
+            g_negCache->add(ne);
 
             /* Careful! If the client is asking for a DS that does not exist, we need to provide the SOA along with the NSEC(3) proof
                and we might not have it if we picked up the proof from a delegation, in which case we need to keep on to do the actual DS
@@ -3862,10 +3860,8 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co
         }
         ne.d_ttd = d_now.tv_sec + lowestTTL;
 
-        if (!wasVariable()) {
-          if (qtype.getCode()) {  // prevents us from blacking out a whole domain
-            g_negCache->add(ne);
-          }
+        if (qtype.getCode()) {  // prevents us from NXDOMAIN'ing a whole domain
+          g_negCache->add(ne);
         }
 
         ret.push_back(rec);