rec: Fix returning more than one cached records

The existing implementation did not properly update the DNSSEC
and authoritative status, and did not include all the needed
RRSIG and additional records.

diff --git a/pdns/recursor_cache.cc b/pdns/recursor_cache.cc
index 4f209d3147c098f40a2e9039ea36d1073954e83b..2169670d696b39d69cc65df18ec8ec707969b5c1 100644 (file)
--- a/pdns/recursor_cache.cc
+++ b/pdns/recursor_cache.cc
@@ -104,20 +104,20 @@ int32_t MemRecursorCache::handleHit(MapCombo& map, MemRecursorCache::OrderedTagI
     }
   }
 
-  if(signatures) { // if you do an ANY lookup you are hosed XXXX
-    *signatures = entry->d_signatures;
+  if (signatures) {
+    signatures->insert(signatures->end(), entry->d_signatures.begin(), entry->d_signatures.end());
   }
 
-  if(authorityRecs) {
-    *authorityRecs = entry->d_authorityRecs;
+  if (authorityRecs) {
+    authorityRecs->insert(authorityRecs->end(), entry->d_authorityRecs.begin(), entry->d_authorityRecs.end());
   }
 
   if (state) {
-    *state = entry->d_state;
+    updateDNSSECValidationState(*state, entry->d_state);
   }
 
   if (wasAuth) {
-    *wasAuth = entry->d_auth;
+    *wasAuth = *wasAuth && entry->d_auth;
   }
 
   moveCacheItemToBack<SequencedTag>(map.d_map, entry);
@@ -224,6 +224,14 @@ int32_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt,
     res->clear();
   }
   const uint16_t qtype = qt.getCode();
+  if (wasAuth) {
+    // we might retrieve more than one entry, we need to set that to true
+    // so it will be set to false if at least one entry is not auth
+    *wasAuth = true;
+  }
+  if (state) {
+    *state = vState::Indeterminate;
+  }
 
   auto& map = getMap(qname);
   const lock l(map);
@@ -494,12 +502,19 @@ bool MemRecursorCache::doAgeCache(time_t now, const DNSName& name, uint16_t qtyp
 
 bool MemRecursorCache::updateValidationStatus(time_t now, const DNSName &qname, const QType& qt, const ComboAddress& who, const OptTag& routingTag, bool requireAuth, vState newState, boost::optional<time_t> capTTD)
 {
+  uint16_t qtype = qt.getCode();
+  if (qtype == QType::ANY) {
+    throw std::runtime_error("Trying to update the DNSSEC validation status of all (via ANY) records for " + qname.toLogString());
+  }
+  if (qtype == QType::ADDR) {
+    throw std::runtime_error("Trying to update the DNSSEC validation status of several (via ADDR) records for " + qname.toLogString());
+  }
+
   auto& map = getMap(qname);
   const lock l(map);
 
   bool updated = false;
-  uint16_t qtype = qt.getCode();
-  if (qtype != QType::ANY && qtype != QType::ADDR && !map.d_ecsIndex.empty() && !routingTag) {
+  if (!map.d_ecsIndex.empty() && !routingTag) {
     auto entry = getEntryUsingECSIndex(map, now, qname, qtype, requireAuth, who);
     if (entry == map.d_map.end()) {
       return false;
@@ -527,8 +542,7 @@ bool MemRecursorCache::updateValidationStatus(time_t now, const DNSName &qname,
     }
     updated = true;
 
-    if(qtype != QType::ANY && qtype != QType::ADDR) // normally if we have a hit, we are done
-      break;
+    break;
   }
 
   return updated;

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 89df89bd86fa38ea0ddd5e2acd56b2737f1048f3..b1049fc0d24c84100da08ef853a9155ba8b2b817 100644 (file)
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -2246,24 +2246,7 @@ uint32_t SyncRes::computeLowestTTD(const std::vector<DNSRecord>& records, const
 void SyncRes::updateValidationState(vState& state, const vState stateUpdate)
 {
   LOG(d_prefix<<"validation state was "<<state<<", state update is "<<stateUpdate);
-
-  if (stateUpdate == vState::TA) {
-    state = vState::Secure;
-  }
-  else if (stateUpdate == vState::NTA) {
-    state = vState::Insecure;
-  }
-  else if (stateUpdate == vState::Bogus) {
-    state = vState::Bogus;
-  }
-  else if (state == vState::Indeterminate) {
-    state = stateUpdate;
-  }
-  else if (stateUpdate == vState::Insecure) {
-    if (state != vState::Bogus) {
-      state = vState::Insecure;
-    }
-  }
+  updateDNSSECValidationState(state, stateUpdate);
   LOG(", validation state is now "<<state<<endl);
 }
 

diff --git a/pdns/validate.cc b/pdns/validate.cc
index 88a120826e6f1d0c15738e161206b05a533b4491..56bdb98842f11bf62c38974164e09e71e656cf70 100644 (file)
--- a/pdns/validate.cc
+++ b/pdns/validate.cc
@@ -1152,3 +1152,24 @@ std::ostream& operator<<(std::ostream &os, const dState d)
   os<<dStates.at(static_cast<size_t>(d));
   return os;
 }
+
+void updateDNSSECValidationState(vState& state, const vState stateUpdate)
+{
+  if (stateUpdate == vState::TA) {
+    state = vState::Secure;
+  }
+  else if (stateUpdate == vState::NTA) {
+    state = vState::Insecure;
+  }
+  else if (stateUpdate == vState::Bogus) {
+    state = vState::Bogus;
+  }
+  else if (state == vState::Indeterminate) {
+    state = stateUpdate;
+  }
+  else if (stateUpdate == vState::Insecure) {
+    if (state != vState::Bogus) {
+      state = vState::Insecure;
+    }
+  }
+}

diff --git a/pdns/validate.hh b/pdns/validate.hh
index 60054aa1303e88b70558185c7d5fb63a1bfa7d4a..6627bee1f1b4cb71dd759a108ccb80ea096eb413 100644 (file)
--- a/pdns/validate.hh
+++ b/pdns/validate.hh
@@ -82,3 +82,4 @@ bool denialProvesNoDelegation(const DNSName& zone, const std::vector<DNSRecord>&
 bool isRRSIGNotExpired(const time_t now, const shared_ptr<RRSIGRecordContent> sig);
 bool isWildcardExpanded(unsigned int labelCount, const std::shared_ptr<RRSIGRecordContent>& sign);
 bool isWildcardExpandedOntoItself(const DNSName& owner, unsigned int labelCount, const std::shared_ptr<RRSIGRecordContent>& sign);
+void updateDNSSECValidationState(vState& state, const vState stateUpdate);