rec: Make sure that we pass the SOA along the NSEC(3) proof for DS queries

If the client is requesting a DS that does not exist, we need to
provide the SOA (+RRSIGs) along with the NSEC(3) proof (+RRSIGs)
and we might not have it if we picked up the proof from a delegation,
in which case we need to keep on to do the actual DS query.
It used to work before 4.5.0 because the zone cuts determination
code was requesting the DS records before doing any resolution, so we
would get the denial and the SOA at the very beginning and not replace
it on a delegation because we knew the zone was Insecure at that point.
Note that we still want to use the "no SOA denial" for internal zone
cuts computation since we don't care about the SOA at that point,
and that saves quite some outgoing queries.

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index aabf85c2e90968d79745e645f2492ee6e609b2b1..07af5138914bf4cd9265b6374a76bd36b8738a0c 100644 (file)
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -143,6 +143,13 @@ int SyncRes::beginResolve(const DNSName &qname, const QType qtype, QClass qclass
   else if(qclass!=QClass::IN)
     return -1;
 
+  if (qtype == QType::DS) {
+    d_externalDSQuery = qname;
+  }
+  else {
+    d_externalDSQuery.clear();
+  }
+
   set<GetBestNSAnswer> beenthere;
   int res=doResolve(qname, qtype, ret, depth, beenthere, state);
   d_queryValidationState = state;
@@ -1633,6 +1640,11 @@ static void reapRecordsFromNegCacheEntryForValidation(tcache_t& tcache, const ve
   }
 }
 
+static bool negativeCacheEntryHasSOA(const NegCache::NegCacheEntry& ne)
+{
+  return !ne.authoritySOA.records.empty();
+}
+
 static void reapRecordsForValidation(std::map<QType, CacheEntry>& entries, const vector<DNSRecord>& records)
 {
   for (const auto& rec : records) {
@@ -1743,15 +1755,23 @@ bool SyncRes::doCacheCheck(const DNSName &qname, const DNSName& authname, bool w
     if (qtype != QType::DS || ne.d_qtype.getCode() || ne.d_auth != qname ||
         g_negCache->get(qname, qtype, d_now, ne, true))
     {
-      res = RCode::NXDomain;
-      sttl = ne.d_ttd - d_now.tv_sec;
-      giveNegative = true;
-      cachedState = ne.d_validationState;
-      if (ne.d_qtype.getCode()) {
-        LOG(prefix<<qname<<": "<<qtype.toString()<<" is negatively cached via '"<<ne.d_auth<<"' for another "<<sttl<<" seconds"<<endl);
-        res = RCode::NoError;
-      } else {
-        LOG(prefix<<qname<<": Entire name '"<<qname<<"' is negatively cached via '"<<ne.d_auth<<"' for another "<<sttl<<" seconds"<<endl);
+      /* Careful! If the client is asking for a DS that does not exist, we need to provide the SOA along with the NSEC(3) proof
+         and we might not have it if we picked up the proof from a delegation, in which case we need to keep on to do the actual DS
+         query. */
+      if (qtype == QType::DS && ne.d_qtype.getCode() && !d_externalDSQuery.empty() && qname == d_externalDSQuery && !negativeCacheEntryHasSOA(ne)) {
+        giveNegative = false;
+      }
+      else {
+        res = RCode::NXDomain;
+        sttl = ne.d_ttd - d_now.tv_sec;
+        giveNegative = true;
+        cachedState = ne.d_validationState;
+        if (ne.d_qtype.getCode()) {
+          LOG(prefix<<qname<<": "<<qtype.toString()<<" is negatively cached via '"<<ne.d_auth<<"' for another "<<sttl<<" seconds"<<endl);
+          res = RCode::NoError;
+        } else {
+          LOG(prefix<<qname<<": Entire name '"<<qname<<"' is negatively cached via '"<<ne.d_auth<<"' for another "<<sttl<<" seconds"<<endl);
+        }
       }
     }
   } else if (s_hardenNXD != HardenNXD::No && !qname.isRoot() && !wasForwardedOrAuthZone) {
@@ -3643,7 +3663,10 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co
               g_negCache->add(ne);
             }
 
-            if (qtype == QType::DS && qname == newauth) {
+            /* Careful! If the client is asking for a DS that does not exist, we need to provide the SOA along with the NSEC(3) proof
+               and we might not have it if we picked up the proof from a delegation, in which case we need to keep on to do the actual DS
+               query. */
+            if (qtype == QType::DS && qname == newauth && (d_externalDSQuery.empty() || qname != d_externalDSQuery)) {
               /* we are actually done! */
               negindic = true;
               negIndicHasSignatures = !ne.authoritySOA.signatures.empty() || !ne.DNSSECRecords.signatures.empty();

diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index 3a5e71a76d439e459fac3578e0fc7476bbdb2286..6285f6c87b5efc86b766d2f7596692a1fa693503 100644 (file)
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -896,6 +896,9 @@ private:
   boost::optional<const boost::uuids::uuid&> d_initialRequestId;
   asyncresolve_t d_asyncResolve{nullptr};
   struct timeval d_now;
+  /* if the client is asking for a DS that does not exist, we need to provide the SOA along with the NSEC(3) proof
+     and we might not have it if we picked up the proof from a delegation */
+  DNSName d_externalDSQuery;
   string d_prefix;
   vState d_queryValidationState{vState::Indeterminate};