From 129e658fb50511961a7bee8d95c07318dfaf20e2 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Mon, 4 Dec 2017 10:38:43 +0100 Subject: [PATCH] rec: Don't cached merged answers from different sections in a single packet We incorrectly merged answers for the same qname and qtype coming from a single packet but from different sections when storing them in the cache. It resulted in duplicates for the IP addresses of some NS, for example. --- pdns/recursordist/test-syncres_cc.cc | 40 +++++++++++++++++++++++++++- pdns/syncres.cc | 2 +- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc index b6cdc579f..0266a5c52 100644 --- a/pdns/recursordist/test-syncres_cc.cc +++ b/pdns/recursordist/test-syncres_cc.cc @@ -2102,7 +2102,8 @@ BOOST_AUTO_TEST_CASE(test_cache_expired_ttl) { }); /* we populate the cache with entries that expired 60s ago*/ - time_t now = sr->getNow().tv_sec; + const time_t now = sr->getNow().tv_sec; + std::vector records; std::vector > sigs; addRecordToList(records, target, QType::A, "192.0.2.42", DNSResourceRecord::ANSWER, now - 60); @@ -2117,6 +2118,43 @@ BOOST_AUTO_TEST_CASE(test_cache_expired_ttl) { BOOST_CHECK_EQUAL(getRR(ret[0])->getCA().toStringWithPort(), ComboAddress("192.0.2.2").toStringWithPort()); } +BOOST_AUTO_TEST_CASE(test_cache_auth) { + std::unique_ptr sr; + initSR(sr); + + primeHints(); + + /* the auth server is sending the same answer in answer and additional, + check that we only return one result, and we only cache one too. */ + const DNSName target("cache-auth.powerdns.com."); + + sr->setAsyncCallback([target](const ComboAddress& ip, const DNSName& domain, int type, bool doTCP, bool sendRDQuery, int EDNS0Level, struct timeval* now, boost::optional& srcmask, boost::optional context, std::shared_ptr outgoingLogger, LWResult* res) { + + setLWResult(res, 0, true, false, true); + addRecordToLW(res, domain, QType::A, "192.0.2.2", DNSResourceRecord::ANSWER, 10); + addRecordToLW(res, domain, QType::A, "192.0.2.2", DNSResourceRecord::ADDITIONAL, 10); + + return 1; + }); + + const time_t now = sr->getNow().tv_sec; + + vector ret; + int res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_REQUIRE_EQUAL(ret.size(), 1); + BOOST_REQUIRE_EQUAL(QType(ret.at(0).d_type).getName(), QType(QType::A).getName()); + BOOST_CHECK_EQUAL(getRR(ret.at(0))->getCA().toString(), ComboAddress("192.0.2.2").toString()); + + /* check that we correctly cached only the answer entry, not the additional one */ + const ComboAddress who; + vector cached; + BOOST_REQUIRE_GT(t_RC->get(now, target, QType(QType::A), true, &cached, who), 0); + BOOST_REQUIRE_EQUAL(cached.size(), 1); + BOOST_REQUIRE_EQUAL(QType(cached.at(0).d_type).getName(), QType(QType::A).getName()); + BOOST_CHECK_EQUAL(getRR(cached.at(0))->getCA().toString(), ComboAddress("192.0.2.2").toString()); +} + BOOST_AUTO_TEST_CASE(test_delegation_only) { std::unique_ptr sr; initSR(sr); diff --git a/pdns/syncres.cc b/pdns/syncres.cc index 812030209..8029c18fd 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -983,7 +983,7 @@ struct CacheKey uint16_t type; DNSResourceRecord::Place place; bool operator<(const CacheKey& rhs) const { - return tie(name, type) < tie(rhs.name, rhs.type); + return tie(name, type, place) < tie(rhs.name, rhs.type, rhs.place); } }; typedef map tcache_t; -- 2.40.0