From 2bcec14adbffbf4b28d698cb607877fb96054e87 Mon Sep 17 00:00:00 2001 From: Otto 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 --- 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 03d52c1071..8e46deb05f 100644 --- 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 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 2740d452f4..92344dd931 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -3637,7 +3637,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(); @@ -3811,9 +3811,7 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co } LOG(prefix<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 @@ -3865,10 +3863,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 NXDOMAIN'ing 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); -- 2.47.2