From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Mon, 4 Dec 2017 09:38:43 +0000 (+0100)
Subject: rec: Don't cached merged answers from different sections in a single packet
X-Git-Tag: dnsdist-1.3.0~188^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F6042%2Fhead;p=thirdparty%2Fpdns.git

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.
---

diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc
index b6cdc579f6..0266a5c52a 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<DNSRecord> records;
   std::vector<shared_ptr<RRSIGRecordContent> > 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<ARecordContent>(ret[0])->getCA().toStringWithPort(), ComboAddress("192.0.2.2").toStringWithPort());
 }
 
+BOOST_AUTO_TEST_CASE(test_cache_auth) {
+  std::unique_ptr<SyncRes> 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<Netmask>& srcmask, boost::optional<const ResolveContext&> context, std::shared_ptr<RemoteLogger> 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<DNSRecord> 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<ARecordContent>(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<DNSRecord> 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<ARecordContent>(cached.at(0))->getCA().toString(), ComboAddress("192.0.2.2").toString());
+}
+
 BOOST_AUTO_TEST_CASE(test_delegation_only) {
   std::unique_ptr<SyncRes> sr;
   initSR(sr);
diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 8120302091..8029c18fd5 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<CacheKey, CacheEntry> tcache_t;