From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Tue, 18 Apr 2017 08:27:27 +0000 (+0200)
Subject: rec: Only apply root-nx-trust if the received SOA is "."
X-Git-Tag: rec-4.1.0-alpha1~133^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=898856caed7cdfc584dfb77fe99ac76b90245227;p=thirdparty%2Fpdns.git

rec: Only apply root-nx-trust if the received SOA is "."

If `root-nx-trust` is enabled and we got a NX answer from the root, check that the received SOA is for the root before negatively caching the entire TLD. This might happen if "." is forwarded, for example.
---

diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc
index 812ccb32f3..be0f17ae1f 100644
--- a/pdns/recursordist/test-syncres_cc.cc
+++ b/pdns/recursordist/test-syncres_cc.cc
@@ -1277,6 +1277,71 @@ BOOST_AUTO_TEST_CASE(test_root_nx_trust) {
   BOOST_CHECK_EQUAL(queriesCount, 1);
 }
 
+BOOST_AUTO_TEST_CASE(test_root_nx_trust_specific) {
+  std::unique_ptr<SyncRes> sr;
+  init();
+  initSR(sr, true, false);
+
+  primeHints();
+
+  const DNSName target1("powerdns.com.");
+  const DNSName target2("notpowerdns.com.");
+  const ComboAddress ns("192.0.2.1:53");
+  size_t queriesCount = 0;
+
+  /* This time the root denies target1 with a "com." SOA instead of a "." one.
+     We should add target1 to the negcache, but not "com.". */
+
+  sr->setAsyncCallback([target1, target2, ns, &queriesCount](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) {
+
+      queriesCount++;
+
+      if (isRootServer(ip)) {
+
+        if (domain == target1) {
+          setLWResult(res, RCode::NXDomain, true, false, true);
+          addRecordToLW(res, "com.", QType::SOA, "a.root-servers.net. nstld.verisign-grs.com. 2017032800 1800 900 604800 86400", DNSResourceRecord::AUTHORITY, 86400);
+        }
+        else {
+          setLWResult(res, 0, true, false, true);
+          addRecordToLW(res, domain, QType::NS, "a.gtld-servers.net.", DNSResourceRecord::AUTHORITY, 172800);
+          addRecordToLW(res, "a.gtld-servers.net.", QType::A, ns.toString(), DNSResourceRecord::ADDITIONAL, 3600);
+        }
+
+        return 1;
+      } else if (ip == ns) {
+
+        setLWResult(res, 0, true, false, false);
+        addRecordToLW(res, domain, QType::A, "192.0.2.2");
+
+        return 1;
+      }
+
+      return 0;
+    });
+
+  vector<DNSRecord> ret;
+  int res = sr->beginResolve(target1, QType(QType::A), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::NXDomain);
+  BOOST_CHECK_EQUAL(ret.size(), 1);
+
+  /* even with root-nx-trust on and a NX answer from the root,
+     we should not have cached the entire TLD this time. */
+  BOOST_CHECK_EQUAL(t_sstorage->negcache.size(), 1);
+
+  ret.clear();
+  res = sr->beginResolve(target2, QType(QType::A), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::NoError);
+  BOOST_REQUIRE_EQUAL(ret.size(), 1);
+  BOOST_REQUIRE(ret[0].d_type == QType::A);
+  BOOST_CHECK_EQUAL(ret[0].d_name, target2);
+  BOOST_CHECK(getRR<ARecordContent>(ret[0])->getCA() == ComboAddress("192.0.2.2"));
+
+  BOOST_CHECK_EQUAL(t_sstorage->negcache.size(), 1);
+
+  BOOST_CHECK_EQUAL(queriesCount, 3);
+}
+
 BOOST_AUTO_TEST_CASE(test_root_nx_dont_trust) {
   std::unique_ptr<SyncRes> sr;
   init();
diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 75a9430fc7..6d49dbf740 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -1229,14 +1229,13 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co
       if(!wasVariable()) {
         NegCache::NegCacheEntry ne;
 
-        ne.d_name = rec.d_name;
         ne.d_ttd = d_now.tv_sec + rec.d_ttl;
         ne.d_name = qname;
         ne.d_qtype = QType(0); // this encodes 'whole record'
-        ne.d_auth = auth;
+        ne.d_auth = rec.d_name;
         harvestNXRecords(lwr.d_records, ne);
         t_sstorage->negcache.add(ne);
-        if(s_rootNXTrust && auth.isRoot()) { // We should check if it was forwarded here, see issue #5107
+        if(s_rootNXTrust && ne.d_auth.isRoot()) { // We should check if it was forwarded here, see issue #5107
           ne.d_name = ne.d_name.getLastLabel();
           t_sstorage->negcache.add(ne);
         }