If we iterator the loop multiple times for ANY requests, authRecords might not be...

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

diff --git a/pdns/recursordist/recursor_cache.cc b/pdns/recursordist/recursor_cache.cc
index d534b60401fe1edc0112e682b029bc7d9572c2dc..88464ae4e8a0da31558a9003723b5d280d38c0d1 100644 (file)
--- a/pdns/recursordist/recursor_cache.cc
+++ b/pdns/recursordist/recursor_cache.cc
@@ -207,6 +207,8 @@ static void ptrAssign(T* ptr, const T& value)
   }
 }
 
+// If the authorityRecs is non-null, it should refer to a un-set shared_ptr, or a shared_ptr pointing to an empty vector
+// See the assert in the processing of authorityRecs.
 time_t MemRecursorCache::handleHit(time_t now, MapCombo::LockedContent& content, OrderedTagIterator_t& entry, const DNSName& qname, uint32_t& origTTL, vector<DNSRecord>* res, SigRecs* signatures, AuthRecs* authorityRecs, bool* variable, std::optional<vState>& state, bool* wasAuth, DNSName* fromAuthZone, Extra* extra)
 {
   // MUTEX SHOULD BE ACQUIRED (as indicated by the reference to the content which is protected by a lock)
@@ -553,6 +555,10 @@ time_t MemRecursorCache::get(time_t now, const DNSName& qname, const QType qtype
 
       handleServeStaleBookkeeping(now, serveStale, firstIndexIterator);
 
+      if (authorityRecs != nullptr) {
+        // For the case the loop iterates multiple times
+        *authorityRecs = s_emptyAuthRecs;
+      }
       ttd = handleHit(now, *lockedShard, firstIndexIterator, qname, origTTL, res, signatures, authorityRecs, variable, cachedState, wasAuth, fromAuthZone, extra);
 
       if (qtype == QType::ADDR && found == 2) {

diff --git a/pdns/recursordist/test-recursorcache_cc.cc b/pdns/recursordist/test-recursorcache_cc.cc
index 1a04d22a3ed0564cb8d3e68455d298d61f35fc75..a5993a95a81ded302ae03c7a7b6c2cd31e547f8c 100644 (file)
--- a/pdns/recursordist/test-recursorcache_cc.cc
+++ b/pdns/recursordist/test-recursorcache_cc.cc
@@ -1386,6 +1386,60 @@ BOOST_AUTO_TEST_CASE(test_RecursorCacheDumpAndRestore)
   }
 }
 
+BOOST_AUTO_TEST_CASE(test_RecursorAuthRecords)
+{
+  MemRecursorCache::resetStaticsForTests();
+  MemRecursorCache MRC;
+
+  const DNSName authZone(".");
+  MemRecursorCache::AuthRecsVec authRecords;
+  DNSRecord dr;
+  dr.d_place = DNSResourceRecord::ANSWER;
+  dr.d_name = DNSName("hi");
+  dr.d_type = QType::AAAA;
+  dr.d_ttl = 3600;
+  dr.setContent(std::make_shared<ARecordContent>(ComboAddress("1::2:3:4")));
+  authRecords.emplace_back(dr);
+
+  std::vector<std::shared_ptr<const RRSIGRecordContent>> signatures;
+  signatures.emplace_back(std::dynamic_pointer_cast<RRSIGRecordContent>(RRSIGRecordContent::make("DNSKEY 8 0 172800 20241111000000 20241021000000 20326 . alCFgDZS+0l5zcpQ/7R+5OFeCrk9KGkNP2F9ynXIXG6QigPj/9qjm0xx ItRJUUim+SrJywAmLKe+48oTUeSRyDKVVg3LGDekLKcIVz0EBqTL2y44 usDlUlxqx5O0LQVHy4h/hm9+dCXFiSBWoV0LcAplV9OYWhxi+CxmxZU5 8vK6eVAde8E2JHdeDuy23WF5lxYEg1q7ehEt5EdRvZ7hZzfawEFR3Qv3 WMootO2eBAAneIe94daJP/i1iwQJ4p+bGVCZ4sJk+Pk9J7lwEQq6Ghkd SpLsRxArUhvoVgtnh0LkAV7TsajYk8K2JRt7wHNDbBV6+Vdq2bh7ZPGv LiGkIQ==")));
+
+  time_t now = time(nullptr);
+  time_t ttd = now + 30;
+
+  DNSName power("powerdns.com.");
+  DNSRecord dr0;
+  ComboAddress dr0Content("192.0.2.40");
+  dr0.d_name = power;
+  dr0.d_type = QType::A;
+  dr0.d_class = QClass::IN;
+  dr0.setContent(std::make_shared<ARecordContent>(dr0Content));
+  dr0.d_ttl = static_cast<uint32_t>(ttd);
+  dr0.d_place = DNSResourceRecord::ANSWER;
+  std::vector<DNSRecord> rset0;
+  rset0.push_back(dr0);
+
+  const ComboAddress nobody;
+  const ComboAddress somebody("::1");
+  const MemRecursorCache::Extra authAddress{ComboAddress{"::2"}, true};
+  const time_t ttl_time = 90;
+
+  DNSName aname = DNSName("hello ");
+  MRC.replace(now, aname, QType(QType::A), rset0, signatures, authRecords, true, authZone, std::nullopt, std::nullopt, vState::Insecure, authAddress, false, ttl_time);
+  MRC.replace(now, aname, QType(QType::AAAA), rset0, signatures, authRecords, true, authZone, std::nullopt, std::nullopt, vState::Insecure, authAddress, false, ttl_time);
+
+  std::vector<DNSRecord> retrieved;
+  MemRecursorCache::AuthRecs authRecs;
+  MemRecursorCache::SigRecs sigs;
+  bool variable = false;
+  vState state = vState::Indeterminate;
+  bool wasAuth = false;
+  DNSName fromZone;
+  MemRecursorCache::Extra extra;
+  if (MRC.get(now, aname, QType(QType::ANY), MemRecursorCache::None, &retrieved, somebody, std::nullopt, &sigs, &authRecs, &variable, &state, &wasAuth, &fromZone, &extra) > 0) {
+  }
+}
+
 #if 0
 volatile bool g_ret; // make sure the optimizer does not get too smart
 uint64_t g_totalRuns;