From: Otto Moerbeek Date: Tue, 4 Mar 2025 08:52:15 +0000 (+0100) Subject: If we see both a CNAME and answer records, follow CNAME and discard the answer records X-Git-Tag: rec-5.1.4~1^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=aaf67219111017f1031013f7ff6b983bcc281047;p=thirdparty%2Fpdns.git If we see both a CNAME and answer records, follow CNAME and discard the answer records --- diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc index af1c9ec69a..f9da0515ce 100644 --- a/pdns/recursordist/syncres.cc +++ b/pdns/recursordist/syncres.cc @@ -4194,6 +4194,7 @@ void SyncRes::sanitizeRecords(const std::string& prefix, LWResult& lwr, const DN /* list of names for which we will allow A and AAAA records in the additional section to remain */ std::unordered_set allowedAdditionals = {qname}; + bool cnameSeen = false; bool haveAnswers = false; bool isNXDomain = false; bool isNXQType = false; @@ -4251,6 +4252,9 @@ void SyncRes::sanitizeRecords(const std::string& prefix, LWResult& lwr, const DN } if (rec->d_place == DNSResourceRecord::ANSWER) { + if (rec->d_type == QType::CNAME) { + cnameSeen = cnameSeen || qname == rec->d_name; + } allowAdditionalEntry(allowedAdditionals, *rec); } @@ -4324,6 +4328,18 @@ void SyncRes::sanitizeRecords(const std::string& prefix, LWResult& lwr, const DN ++rec; } + + if (cnameSeen) { + for (auto rec = lwr.d_records.begin(); rec != lwr.d_records.end();) { + // If we have a CNAME, skip answer records for the requested type + if (rec->d_place == DNSResourceRecord::ANSWER && rec->d_type == qtype && rec->d_name == qname && qtype != QType::CNAME) { + LOG(prefix << qname << ": Removing answer record in presence of CNAME record '" << rec->d_name << "|" << DNSRecordContent::NumberToType(rec->d_type) << "|" << rec->getContent()->getZoneRepresentation() << "' in the ANSWER section received from " << auth << endl); + rec = lwr.d_records.erase(rec); + continue; + } + ++rec; + } + } } void SyncRes::rememberParentSetIfNeeded(const DNSName& domain, const vector& newRecords, unsigned int depth, const string& prefix) @@ -5605,6 +5621,11 @@ bool SyncRes::processAnswer(unsigned int depth, const string& prefix, LWResult& bool done = processRecords(prefix, qname, qtype, auth, lwr, sendRDQuery, ret, nsset, newtarget, newauth, realreferral, negindic, state, needWildcardProof, gatherWildcardProof, wildcardLabelsCount, *rcode, negIndicHasSignatures, depth); + // If we both have a CNAME and an answer, let the CNAME take precedence. This *should* not happen + // (because CNAMEs cannot co-exist with other records), but reality says otherwise. Other + // resolvers choose to follow the CNAME in this case as well. We removed the answer record from + // the records received from the auth when sanitizing, so `done' should not be set when a CNAME is + // present. if (done) { LOG(prefix << qname << ": Status=got results, this level of recursion done" << endl); LOG(prefix << qname << ": Validation status is " << state << endl); diff --git a/pdns/recursordist/test-syncres_cc1.cc b/pdns/recursordist/test-syncres_cc1.cc index 86278fbe64..6d86357ca4 100644 --- a/pdns/recursordist/test-syncres_cc1.cc +++ b/pdns/recursordist/test-syncres_cc1.cc @@ -1462,6 +1462,112 @@ BOOST_AUTO_TEST_CASE(test_following_cname) BOOST_CHECK_EQUAL(ret[1].d_name, cnameTarget); } +BOOST_AUTO_TEST_CASE(test_following_cname_with_a) +{ + std::unique_ptr resolver; + initSR(resolver); + + primeHints(); + + const DNSName target("cname.powerdns.com."); + const DNSName cnameTarget("cname-target.powerdns.com"); + + 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)) { + 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 (address == ComboAddress("192.0.2.1:53")) { + + if (domain == target) { + setLWResult(res, 0, true, false, false); + addRecordToLW(res, domain, QType::A, "192.1.1.1"); + addRecordToLW(res, domain, QType::CNAME, cnameTarget.toString()); + return LWResult::Result::Success; + } + if (domain == cnameTarget) { + setLWResult(res, 0, true, false, false); + addRecordToLW(res, domain, QType::A, "192.0.2.2"); + } + + 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, RCode::NoError); + BOOST_REQUIRE_EQUAL(ret.size(), 2U); + BOOST_CHECK(ret[0].d_type == QType::CNAME); + BOOST_CHECK_EQUAL(ret[0].d_name, target); + BOOST_CHECK(ret[1].d_type == QType::A); + BOOST_CHECK_EQUAL(ret[1].d_name, cnameTarget); + BOOST_CHECK_EQUAL(ret[1].getContent()->getZoneRepresentation(), "192.0.2.2"); +} + +BOOST_AUTO_TEST_CASE(test_following_cname_chain_with_a) +{ + std::unique_ptr resolver; + initSR(resolver); + + primeHints(); + + const DNSName target("cname.powerdns.com."); + const DNSName cnameTarget1("cname-target1.powerdns.com"); + const DNSName cnameTarget("cname-target.powerdns.com"); + + 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)) { + 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 (address == ComboAddress("192.0.2.1:53")) { + + if (domain == target) { + setLWResult(res, 0, true, false, false); + addRecordToLW(res, domain, QType::A, "192.1.1.1"); + addRecordToLW(res, domain, QType::CNAME, cnameTarget1.toString()); + addRecordToLW(res, cnameTarget1, QType::A, "192.1.1.2"); + addRecordToLW(res, cnameTarget1, QType::CNAME, cnameTarget.toString()); + } + if (domain == cnameTarget1) { + setLWResult(res, 0, true, false, false); + addRecordToLW(res, cnameTarget1, QType::A, "192.1.1.2"); + addRecordToLW(res, cnameTarget1, QType::CNAME, cnameTarget.toString()); + return LWResult::Result::Success; + } + if (domain == cnameTarget) { + setLWResult(res, 0, true, false, false); + addRecordToLW(res, domain, QType::A, "192.0.2.2"); + } + + 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, RCode::NoError); + BOOST_REQUIRE_EQUAL(ret.size(), 3U); + BOOST_CHECK(ret[0].d_type == QType::CNAME); + BOOST_CHECK_EQUAL(ret[0].d_name, target); + BOOST_CHECK_EQUAL(ret[0].getContent()->getZoneRepresentation(), cnameTarget1.toString()); + BOOST_CHECK(ret[1].d_type == QType::CNAME); + BOOST_CHECK_EQUAL(ret[1].d_name, cnameTarget1); + BOOST_CHECK_EQUAL(ret[1].getContent()->getZoneRepresentation(), cnameTarget.toString()); + BOOST_CHECK(ret[2].d_type == QType::A); + BOOST_CHECK_EQUAL(ret[2].d_name, cnameTarget); + BOOST_CHECK_EQUAL(ret[2].getContent()->getZoneRepresentation(), "192.0.2.2"); +} + BOOST_AUTO_TEST_CASE(test_cname_nxdomain) { std::unique_ptr sr;