From: Otto Moerbeek Date: Mon, 28 Jul 2025 12:41:02 +0000 (+0200) Subject: Don't cache non-auth rrsets if a Bogus rrset was found in the answer X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=fb9e1df0b4bd41f198a2ae522ca96f94d2ff53cf;p=thirdparty%2Fpdns.git Don't cache non-auth rrsets if a Bogus rrset was found in the answer Signed-off-by: Otto Moerbeek --- diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc index 2ccc4607ee..30f0b31d07 100644 --- a/pdns/recursordist/syncres.cc +++ b/pdns/recursordist/syncres.cc @@ -2226,7 +2226,7 @@ bool SyncRes::canUseRecords(const std::string& prefix, const DNSName& qname, con LOG(prefix << qname << ": Cannot use " << name << '/' << qtype << " records from cache: Bogus" << endl); return false; } - // We could validate Indeterminete authoritative records here. + // We could validate Indeterminate authoritative records here. return true; } @@ -2249,8 +2249,7 @@ void SyncRes::getBestNSFromCache(const DNSName& qname, const QType qtype, vector *flawedNSSet = false; vState state{vState::Indeterminate}; - if (bool isAuth = false; g_recCache->get(d_now.tv_sec, subdomain, QType::NS, flags, &nsVector, d_cacheRemote, d_routingTag, nullptr, nullptr, nullptr, nullptr, &isAuth) > 0 && - canUseRecords(prefix, qname, subdomain, QType::NS, state)) { + if (bool isAuth = false; g_recCache->get(d_now.tv_sec, subdomain, QType::NS, flags, &nsVector, d_cacheRemote, d_routingTag, nullptr, nullptr, nullptr, &state, &isAuth) > 0 && canUseRecords(prefix, qname, subdomain, QType::NS, state)) { if (s_maxnsperresolve > 0 && nsVector.size() > s_maxnsperresolve) { vector selected; selected.reserve(s_maxnsperresolve); @@ -4325,9 +4324,6 @@ void SyncRes::sanitizeRecords(const std::string& prefix, LWResult& lwr, const DN } soaInAuth = true; } - if (!haveAnswers && lwr.d_rcode == RCode::NoError) { - acceptDelegation = true; - } } /* dealing with records in additional */ else if (rec->d_place == DNSResourceRecord::ADDITIONAL) { @@ -4340,6 +4336,10 @@ void SyncRes::sanitizeRecords(const std::string& prefix, LWResult& lwr, const DN } } // end of first loop, handled answer and most of authority section + if (!haveAnswers && lwr.d_rcode == RCode::NoError) { + acceptDelegation = true; + } + sanitizeRecordsPass2(prefix, lwr, qname, qtype, auth, allowedAnswerNames, allowedAdditionals, cnameSeen, acceptDelegation && !soaInAuth, skipvec, skipCount); } @@ -4653,6 +4653,7 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, const string& } } + bool seenBogusRRSet = false; for (auto tCacheEntry = tcache.begin(); tCacheEntry != tcache.end(); ++tCacheEntry) { if (tCacheEntry->second.records.empty()) { // this happens when we did store signatures, but passed on the records themselves @@ -4757,6 +4758,7 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, const string& } if (vStateIsBogus(recordState)) { + seenBogusRRSet = true; /* this is a TTD by now, be careful */ for (auto& record : tCacheEntry->second.records) { auto newval = std::min(record.d_ttl, static_cast(s_maxbogusttl + d_now.tv_sec)); @@ -4777,7 +4779,11 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, const string& if (tCacheEntry->first.type != QType::NSEC3 && (tCacheEntry->first.type == QType::DS || tCacheEntry->first.type == QType::NS || tCacheEntry->first.type == QType::A || tCacheEntry->first.type == QType::AAAA || isAA || wasForwardRecurse)) { bool doCache = true; - if (tCacheEntry->first.place == DNSResourceRecord::ANSWER && ednsmask) { + if (!isAA && seenBogusRRSet) { + LOG(prefix << qname << ": Not caching non-authoritative rrsets received with Bogus answer" << endl); + doCache = false; + } + if (doCache && tCacheEntry->first.place == DNSResourceRecord::ANSWER && ednsmask) { const bool isv4 = ednsmask->isIPv4(); if ((isv4 && s_ecsipv4nevercache) || (!isv4 && s_ecsipv6nevercache)) { doCache = false; diff --git a/pdns/recursordist/test-syncres_cc4.cc b/pdns/recursordist/test-syncres_cc4.cc index 5ab63d1067..daf52d2c4c 100644 --- a/pdns/recursordist/test-syncres_cc4.cc +++ b/pdns/recursordist/test-syncres_cc4.cc @@ -1992,7 +1992,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_bogus_bad_algo) BOOST_AUTO_TEST_CASE(test_dnssec_bogus_unsigned_ds) { std::unique_ptr sr; - initSR(sr, true); + initSR(sr, true, false); setDNSSECValidation(sr, DNSSECMode::ValidateAll); @@ -2052,7 +2052,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_bogus_unsigned_ds) BOOST_CHECK_EQUAL(res, RCode::NoError); BOOST_CHECK_EQUAL(sr->getValidationState(), vState::BogusNoRRSIG); BOOST_REQUIRE_EQUAL(ret.size(), 2U); - BOOST_CHECK_EQUAL(queriesCount, 3U); + BOOST_CHECK_EQUAL(queriesCount, 4U); /* again, to test the cache */ ret.clear(); @@ -2060,7 +2060,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_bogus_unsigned_ds) BOOST_CHECK_EQUAL(res, RCode::NoError); BOOST_CHECK_EQUAL(sr->getValidationState(), vState::BogusNoRRSIG); BOOST_REQUIRE_EQUAL(ret.size(), 2U); - BOOST_CHECK_EQUAL(queriesCount, 3U); + BOOST_CHECK_EQUAL(queriesCount, 4U); /* now we ask directly for the DS */ ret.clear(); @@ -2068,7 +2068,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_bogus_unsigned_ds) BOOST_CHECK_EQUAL(res, RCode::NoError); BOOST_CHECK_EQUAL(sr->getValidationState(), vState::BogusNoRRSIG); BOOST_REQUIRE_EQUAL(ret.size(), 1U); - BOOST_CHECK_EQUAL(queriesCount, 3U); + BOOST_CHECK_EQUAL(queriesCount, 4U); } BOOST_AUTO_TEST_CASE(test_dnssec_bogus_unsigned_ds_direct) diff --git a/regression-tests.recursor/ghost-1/expected_result b/regression-tests.recursor/ghost-1/expected_result index bc000e8412..cde4542021 100644 --- a/regression-tests.recursor/ghost-1/expected_result +++ b/regression-tests.recursor/ghost-1/expected_result @@ -4,8 +4,8 @@ Reply to question for qname='a.www.1.ghost.example.net.', qtype=A 0 b.www.1.ghost.example.net. 3600 IN A 192.0.2.7 Rcode: 0 (No Error), RD: 1, QR: 1, TC: 0, AA: 0, opcode: 0 Reply to question for qname='b.www.1.ghost.example.net.', qtype=A -0 c.www.1.ghost.example.net. 3600 IN A 192.0.2.7 -Rcode: 0 (No Error), RD: 1, QR: 1, TC: 0, AA: 0, opcode: 0 +1 ghost.example.net. 3600 IN SOA ns.example.net. hostmaster.example.net. 1 3600 1800 1209600 300 +Rcode: 3 (Non-Existent domain), RD: 1, QR: 1, TC: 0, AA: 0, opcode: 0 Reply to question for qname='c.www.1.ghost.example.net.', qtype=A 1 ghost.example.net. 3600 IN SOA ns.example.net. hostmaster.example.net. 1 3600 1800 1209600 300 Rcode: 3 (Non-Existent domain), RD: 1, QR: 1, TC: 0, AA: 0, opcode: 0 diff --git a/regression-tests.recursor/ghost-2/expected_result b/regression-tests.recursor/ghost-2/expected_result index 7a2e7325d9..e17693e073 100644 --- a/regression-tests.recursor/ghost-2/expected_result +++ b/regression-tests.recursor/ghost-2/expected_result @@ -4,8 +4,8 @@ Reply to question for qname='a.www.2.ghost.example.net.', qtype=A 0 b.www.2.ghost.example.net. 3600 IN A 192.0.2.8 Rcode: 0 (No Error), RD: 1, QR: 1, TC: 0, AA: 0, opcode: 0 Reply to question for qname='b.www.2.ghost.example.net.', qtype=A -0 c.www.2.ghost.example.net. 3600 IN A 192.0.2.8 -Rcode: 0 (No Error), RD: 1, QR: 1, TC: 0, AA: 0, opcode: 0 +1 ghost.example.net. 3600 IN SOA ns.example.net. hostmaster.example.net. 1 3600 1800 1209600 300 +Rcode: 3 (Non-Existent domain), RD: 1, QR: 1, TC: 0, AA: 0, opcode: 0 Reply to question for qname='c.www.2.ghost.example.net.', qtype=A 1 ghost.example.net. 3600 IN SOA ns.example.net. hostmaster.example.net. 1 3600 1800 1209600 300 Rcode: 3 (Non-Existent domain), RD: 1, QR: 1, TC: 0, AA: 0, opcode: 0