rec: do not attempt to refresh NS records in refresh-almost-expired.

If the auth responds *very* slowly and the records expire in between,
the capping of TTLs is not enforced for lack of data.

This does not happen on regular resolve as then then the child
records are used immediately if not expired and thus valid, or the
records *are* expired, and in that case not used.  So this case can
only happen if almost expired records are used to refresh the
authoritative NS records.

Signed-off-by: Otto Moerbeek <otto.moerbeek@open-xchange.com>

diff --git a/pdns/recursordist/recpacketcache.cc b/pdns/recursordist/recpacketcache.cc
index 35ac400702e275c242b04eb4ea4845c370bf9d11..1935681ccc59d5661823ab01738285d39ecfd0a2 100644 (file)
--- a/pdns/recursordist/recpacketcache.cc
+++ b/pdns/recursordist/recpacketcache.cc
@@ -146,7 +146,9 @@ bool RecursorPacketCache::checkResponseMatches(MapCombo::LockedContent& shard, s
       *age = static_cast<uint32_t>(now - iter->d_creation);
       // we know ttl is > 0
       auto ttl = static_cast<uint32_t>(iter->d_ttd - now);
-      if (s_refresh_ttlperc > 0 && !iter->d_submitted && taskQTypeIsSupported(qtype)) {
+      // Be wary of refreshing NS records, it could lead to ghosts if the record cache entry expires between
+      // sending out the request and the reply coming back in, as then the TTL capping does not work.
+      if (s_refresh_ttlperc > 0 && !iter->d_submitted && taskQTypeIsSupported(qtype) && qtype != QType::NS) {
         const dnsheader_aligned header(iter->d_packet.data());
         const auto* headerPtr = header.get();
         if (headerPtr->rcode == RCode::NoError) {

diff --git a/pdns/recursordist/recursor_cache.cc b/pdns/recursordist/recursor_cache.cc
index b521b26c4e3b31f987c457e463f2caa01c94a614..1b0f0c9b66e7abba6afe438d62e36ebe17cb0d13 100644 (file)
--- a/pdns/recursordist/recursor_cache.cc
+++ b/pdns/recursordist/recursor_cache.cc
@@ -425,7 +425,10 @@ time_t MemRecursorCache::fakeTTD(MemRecursorCache::OrderedTagIterator_t& entry,
       if (refresh) {
         return -1;
       }
-      if (!entry->d_submitted) {
+      // We do not want to refresh auth NS entries, as it could lead to ghost domains if an entry
+      // expires between submit and response coming in, as the TTL capping then does not work for
+      // lack of current TTL info.
+      if (!entry->d_submitted && (qtype != QType::NS || !entry->d_auth)) {
         pushRefreshTask(qname, qtype, entry->d_ttd, entry->d_netmask);
         entry->d_submitted = true;
       }