From a57dd67c9f32714859d3c700f3675dd6665a4a00 Mon Sep 17 00:00:00 2001
From: Otto <otto.moerbeek@open-xchange.com>
Date: Fri, 19 Nov 2021 11:57:28 +0100
Subject: [PATCH] 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)
---
 pdns/recursordist/test-syncres_cc2.cc |  5 ++---
 pdns/syncres.cc                       | 12 ++++--------
 2 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/pdns/recursordist/test-syncres_cc2.cc b/pdns/recursordist/test-syncres_cc2.cc
index 2a3f01eae5..f4cefa1834 100644
--- a/pdns/recursordist/test-syncres_cc2.cc
+++ b/pdns/recursordist/test-syncres_cc2.cc
@@ -873,7 +873,7 @@ BOOST_AUTO_TEST_CASE(test_rfc8020_nodata_bis)
   BOOST_CHECK_EQUAL(SyncRes::getNegCacheSize(), 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);
@@ -926,8 +926,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(SyncRes::getNegCacheSize(), 0U);
+  BOOST_CHECK_EQUAL(SyncRes::getNegCacheSize(), 1U);
 }
 
 BOOST_AUTO_TEST_CASE(test_ecs_cache_limit_allowed)
diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 2463b89fed..39facfe30a 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -3443,7 +3443,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 (newtarget.empty()) {
         t_sstorage.negcache.add(ne);
         if(s_rootNXTrust && ne.d_auth.isRoot() && auth.isRoot() && lwr.d_aabit) {
           ne.d_name = ne.d_name.getLastLabel();
@@ -3591,9 +3591,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()) {
-            t_sstorage.negcache.add(ne);
-          }
+          t_sstorage.negcache.add(ne);
 
           if (qname == newauth && qtype == QType::DS) {
             /* we are actually done! */
@@ -3633,10 +3631,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
-            t_sstorage.negcache.add(ne);
-          }
+        if (qtype.getCode()) {  // prevents us from NXDOMAIN'ing a whole domain
+          t_sstorage.negcache.add(ne);
         }
 
         ret.push_back(rec);
-- 
2.47.2