From: Otto Moerbeek Date: Mon, 17 Mar 2025 13:19:50 +0000 (+0100) Subject: rec: rework of #14822: fix a difference between record-cache hit and miss in some... X-Git-Tag: dnsdist-2.0.0-alpha2~95^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F15396%2Fhead;p=thirdparty%2Fpdns.git rec: rework of #14822: fix a difference between record-cache hit and miss in some ServFail results Fixes: CNAME with target non-existent record in auth zone causes segfault --- diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc index d5d9f7c275..6d29075199 100644 --- a/pdns/recursordist/syncres.cc +++ b/pdns/recursordist/syncres.cc @@ -1718,22 +1718,34 @@ int SyncRes::doResolve(const DNSName& qname, const QType qtype, vector retq; bool old = setCacheOnly(true); bool fromCache = false; // For cache peeking, we tell doResolveNoQNameMinimization not to consider the (non-recursive) forward case. // Otherwise all queries in a forward domain will be forwarded, while we want to consult the cache. - const auto retSizeBeforeCall = ret.size(); - int res = doResolveNoQNameMinimization(qname, qtype, ret, depth, beenthere, context, &fromCache, nullptr); + int res{}; + try { + // The cache lookup below can have three types of result: + // Case 1: successful. In that case the records will be added to the end result below and we're done. + // Case 2: unsuccessful. In that case the records in retq wil be discarded. E.g. there + // might be records as the lookup found a CNAME chain, but the target is missing from the cache. + // Case 3: an exception is thrown, in that case we're still interested in the (partial) results in retq. + // This can e.g. happen on a too-long CNAME chain. + res = doResolveNoQNameMinimization(qname, qtype, retq, depth, beenthere, context, &fromCache, nullptr); + } + catch (...) { + ret.insert(ret.end(), std::make_move_iterator(retq.begin()), std::make_move_iterator(retq.end())); + throw; + } setCacheOnly(old); if (fromCache) { LOG(prefix << qname << ": Step0 Found in cache" << endl); if (d_appliedPolicy.d_type != DNSFilterEngine::PolicyType::None && (d_appliedPolicy.d_kind == DNSFilterEngine::PolicyKind::NXDOMAIN || d_appliedPolicy.d_kind == DNSFilterEngine::PolicyKind::NODATA)) { ret.clear(); } + ret.insert(ret.end(), std::make_move_iterator(retq.begin()), std::make_move_iterator(retq.end())); return res; } - // Erase unwanted cache lookup preliminary results in the returned records - ret.resize(retSizeBeforeCall); LOG(prefix << qname << ": Step0 Not cached" << endl); const unsigned int qnamelen = qname.countLabels(); @@ -1830,7 +1842,7 @@ int SyncRes::doResolve(const DNSName& qname, const QType qtype, vector retq; + retq.clear(); StopAtDelegation stopAtDelegation = Stop; res = doResolveNoQNameMinimization(child, QType::A, retq, depth, beenthere, context, nullptr, &stopAtDelegation); d_followCNAME = oldFollowCNAME; diff --git a/pdns/recursordist/test-syncres_cc3.cc b/pdns/recursordist/test-syncres_cc3.cc index 2599b0fd25..621802900b 100644 --- a/pdns/recursordist/test-syncres_cc3.cc +++ b/pdns/recursordist/test-syncres_cc3.cc @@ -1478,6 +1478,139 @@ BOOST_AUTO_TEST_CASE(test_auth_zone_oob_cname) BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Indeterminate); } +BOOST_AUTO_TEST_CASE(test_auth_cname_to_oob_target) +{ + std::unique_ptr resolver; + initSR(resolver, false, false); + + resolver->setQNameMinimization(); + primeHints(); + + size_t queriesCount = 0; + const DNSName target("cname.example.com."); + const DNSName existingname("existing.test.xx."); + const DNSName target1Cname("cname1.example.com."); + const DNSName authZone("test.xx"); + + SyncRes::AuthDomain authDomain; + + DNSRecord record; + record.d_place = DNSResourceRecord::ANSWER; + record.d_name = existingname; + record.d_type = QType::A; + record.d_ttl = 1800; + record.setContent(std::make_shared("127.0.0.1")); + authDomain.d_records.insert(record); + + (*SyncRes::t_sstorage.domainmap)[authZone] = authDomain; + + resolver->setAsyncCallback([&](const ComboAddress& address, const DNSName& domain, int /* type */, bool /* doTCP */, bool /* sendRDQuery */, int /* EDNS0Level */, struct timeval* /* now */, boost::optional& /* srcmask */, const ResolveContext& /* context */, LWResult* res, bool* /* chained */) { + queriesCount++; + if (isRootServer(address) || domain == DNSName("com") || domain == DNSName("example.com")) { + setLWResult(res, 0, false, false, true); + addRecordToLW(res, domain, QType::NS, "a.gtld-servers.net.", DNSResourceRecord::AUTHORITY, 172800); + addRecordToLW(res, "a.gtld-servers.net.", QType::A, "192.0.2.1", DNSResourceRecord::ADDITIONAL, 3600); + return LWResult::Result::Success; + } + if (domain == target) { + setLWResult(res, 0, true, false, false); + addRecordToLW(res, domain, QType::CNAME, target1Cname.toString()); + return LWResult::Result::Success; + } + if (domain == target1Cname) { + setLWResult(res, 0, true, false, false); + addRecordToLW(res, domain, QType::CNAME, existingname.toString()); + return LWResult::Result::Success; + } + + return LWResult::Result::Timeout; + }); + + vector ret; + int res = resolver->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, 0); + BOOST_REQUIRE_EQUAL(ret.size(), 3U); + BOOST_CHECK(ret[0].d_type == QType::CNAME); + BOOST_CHECK(ret[1].d_type == QType::CNAME); + BOOST_CHECK(ret[2].d_type == QType::A); + BOOST_CHECK_EQUAL(resolver->getValidationState(), vState::Indeterminate); + + /* a second time, from the cache */ + ret.clear(); + res = resolver->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, 0); + BOOST_REQUIRE_EQUAL(ret.size(), 3U); + BOOST_CHECK(ret[0].d_type == QType::CNAME); + BOOST_CHECK(ret[1].d_type == QType::CNAME); + BOOST_CHECK(ret[2].d_type == QType::A); + BOOST_CHECK_EQUAL(resolver->getValidationState(), vState::Indeterminate); +} + +BOOST_AUTO_TEST_CASE(test_auth_cname_to_non_existent_oob_target) +{ + std::unique_ptr resolver; + initSR(resolver, false, false); + + resolver->setQNameMinimization(); + primeHints(); + + const DNSName target("cname.example.com."); + const DNSName existingname("existing.test.xx."); + const DNSName target1Cname("cname1.example.com."); + const DNSName target2Cname("cname-target.test.xx."); + const DNSName authZone("test.xx"); + + SyncRes::AuthDomain authDomain; + + DNSRecord record; + record.d_place = DNSResourceRecord::ANSWER; + record.d_name = existingname; + record.d_type = QType::A; + record.d_ttl = 1800; + record.setContent(std::make_shared("127.0.0.1")); + authDomain.d_records.insert(record); + + (*SyncRes::t_sstorage.domainmap)[authZone] = authDomain; + + resolver->setAsyncCallback([&](const ComboAddress& address, const DNSName& domain, int /* type */, bool /* doTCP */, bool /* sendRDQuery */, int /* EDNS0Level */, struct timeval* /* now */, boost::optional& /* srcmask */, const ResolveContext& /* context */, LWResult* res, bool* /* chained */) { + if (isRootServer(address) || domain == DNSName("com") || domain == DNSName("example.com")) { + setLWResult(res, 0, false, false, true); + addRecordToLW(res, domain, QType::NS, "a.gtld-servers.net.", DNSResourceRecord::AUTHORITY, 172800); + addRecordToLW(res, "a.gtld-servers.net.", QType::A, "192.0.2.1", DNSResourceRecord::ADDITIONAL, 3600); + return LWResult::Result::Success; + } + if (domain == target) { + setLWResult(res, 0, true, false, false); + addRecordToLW(res, domain, QType::CNAME, target1Cname.toString()); + return LWResult::Result::Success; + } + if (domain == target1Cname) { + setLWResult(res, 0, true, false, false); + addRecordToLW(res, domain, QType::CNAME, target2Cname.toString()); + return LWResult::Result::Success; + } + + return LWResult::Result::Timeout; + }); + + vector ret; + int res = resolver->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, 3); + BOOST_REQUIRE_EQUAL(ret.size(), 2U); + BOOST_CHECK(ret[0].d_type == QType::CNAME); + BOOST_CHECK(ret[1].d_type == QType::CNAME); + BOOST_CHECK_EQUAL(resolver->getValidationState(), vState::Indeterminate); + + /* a second time, from the cache */ + ret.clear(); + res = resolver->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, 3); + BOOST_REQUIRE_EQUAL(ret.size(), 2U); + BOOST_CHECK(ret[0].d_type == QType::CNAME); + BOOST_CHECK(ret[1].d_type == QType::CNAME); + BOOST_CHECK_EQUAL(resolver->getValidationState(), vState::Indeterminate); +} + BOOST_AUTO_TEST_CASE(test_auth_zone) { std::unique_ptr sr;