rec: Skip NS for the exact zone in CNAME answers

These NS can't be authoritative since we have a CNAME answer for
which only the record describing that alias is necessarily
authoritative.
But if we allow the current auth, which might be serving the child
zone, to raise the TTL of non-authoritative NS in the cache, they
might be able to keep a "ghost" zone alive forever, even after the
delegation is gone from the parent.
So let's just do nothing with them, we can fetch them directly if
we need them.

diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc
index 505310c987899cff94e0f9acdd964d3b20dfa3a5..8b8bae6174c0abf7b67129f873320ad587e5a5f0 100644 (file)
--- a/pdns/recursordist/test-syncres_cc.cc
+++ b/pdns/recursordist/test-syncres_cc.cc
@@ -10119,6 +10119,77 @@ BOOST_AUTO_TEST_CASE(test_getDSRecords_multialgo_two_highest) {
   }
 }
 
+BOOST_AUTO_TEST_CASE(test_cname_plus_authority_ns_ttl) {
+  std::unique_ptr<SyncRes> sr;
+  initSR(sr);
+
+  primeHints();
+
+  const DNSName target("cname.powerdns.com.");
+  const DNSName cnameTarget("cname-target.powerdns.com");
+  size_t queriesCount = 0;
+
+  sr->setAsyncCallback([target, cnameTarget, &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, LWResult* res, bool* chained) {
+
+       queriesCount++;
+
+       if (isRootServer(ip)) {
+        setLWResult(res, 0, false, false, true);
+        addRecordToLW(res, DNSName("powerdns.com"), QType::NS, "a.gtld-servers.net.", DNSResourceRecord::AUTHORITY, 42);
+        addRecordToLW(res, "a.gtld-servers.net.", QType::A, "192.0.2.1", DNSResourceRecord::ADDITIONAL, 3600);
+        return 1;
+      } else if (ip == ComboAddress("192.0.2.1:53")) {
+         if (domain == target) {
+          setLWResult(res, 0, true, false, false);
+          addRecordToLW(res, domain, QType::CNAME, cnameTarget.toString());
+          addRecordToLW(res, cnameTarget, QType::A, "192.0.2.2");
+          addRecordToLW(res, DNSName("powerdns.com."), QType::NS, "a.gtld-servers.net.", DNSResourceRecord::AUTHORITY, 172800);
+          addRecordToLW(res, DNSName("add.powerdns.com."), QType::A, "192.0.2.3", DNSResourceRecord::ADDITIONAL, 42);
+          return 1;
+        }
+        else if (domain == cnameTarget) {
+          setLWResult(res, 0, true, false, false);
+          addRecordToLW(res, domain, QType::A, "192.0.2.2");
+        }
+
+         return 1;
+      }
+
+      return 0;
+    });
+
+  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(), 2);
+  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);
+
+  /* check that the NS in authority has not replaced the one in the cache
+     with auth=0 (or at least has not raised the TTL since it could otherwise
+     be used to create a never-ending ghost zone even after the NS have been
+     changed in the parent.
+     Also check that the the part in additional is still not auth
+  */
+  const ComboAddress who;
+  vector<DNSRecord> cached;
+  bool wasAuth = false;
+
+  auto ttl = t_RC->get(now, DNSName("powerdns.com."), QType(QType::NS), false, &cached, who, nullptr, nullptr, nullptr, nullptr, &wasAuth);
+  BOOST_REQUIRE_GE(ttl, 1);
+  BOOST_REQUIRE_LE(ttl, 42);
+  BOOST_CHECK_EQUAL(cached.size(), 1);
+  BOOST_CHECK_EQUAL(wasAuth, false);
+
+  cached.clear();
+  BOOST_REQUIRE_GE(t_RC->get(now, DNSName("add.powerdns.com."), QType(QType::A), false, &cached, who, nullptr, nullptr, nullptr, nullptr, &wasAuth), 1);
+  BOOST_CHECK_EQUAL(cached.size(), 1);
+  BOOST_CHECK_EQUAL(wasAuth, false);
+}
+
 /*
 // cerr<<"asyncresolve called to ask "<<ip.toStringWithPort()<<" about "<<domain.toString()<<" / "<<QType(type).getName()<<" over "<<(doTCP ? "TCP" : "UDP")<<" (rd: "<<sendRDQuery<<", EDNS0 level: "<<EDNS0Level<<")"<<endl;
 

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 0b892d2a96a3d31c77839e7d16d6720eb2c82154..75d495aa1c8d3a0dc92bbf87db29043eda97f1d2 100644 (file)
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -2137,6 +2137,18 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr
       isAA = false;
     }
 
+    if (isCNAMEAnswer && i->first.place == DNSResourceRecord::AUTHORITY && i->first.type == QType::NS && auth == i->first.name) {
+      /* These NS can't be authoritative since we have a CNAME answer for which (see above) only the
+         record describing that alias is necessarily authoritative.
+         But if we allow the current auth, which might be serving the child zone, to raise the TTL
+         of non-authoritative NS in the cache, they might be able to keep a "ghost" zone alive forever,
+         even after the delegation is gone from the parent.
+         So let's just do nothing with them, we can fetch them directly if we need them.
+      */
+      LOG(d_prefix<<": skipping authority NS from '"<<auth<<"' nameservers in CNAME answer "<<i->first.name<<"|"<<DNSRecordContent::NumberToType(i->first.type)<<endl);
+      continue;
+    }
+
     vState recordState = getValidationStatus(i->first.name, false);
     LOG(d_prefix<<": got initial zone status "<<vStates[recordState]<<" for record "<<i->first.name<<"|"<<DNSRecordContent::NumberToType(i->first.type)<<endl);