rec: backport 13847 to rec-4.8.x

Manual backport as syncres.c's location changed

diff --git a/pdns/recursordist/test-syncres_cc3.cc b/pdns/recursordist/test-syncres_cc3.cc
index a75dcf2267caa18ab3705a99744d5980c8491e79..da92c57e9f3378accc08e11a3a2af50df9e77dde 100644 (file)
--- a/pdns/recursordist/test-syncres_cc3.cc
+++ b/pdns/recursordist/test-syncres_cc3.cc
@@ -1214,6 +1214,79 @@ BOOST_AUTO_TEST_CASE(test_forward_zone_recurse_rd_dnssec_nodata_bogus)
   BOOST_CHECK_EQUAL(queriesCount, 4U);
 }
 
+BOOST_AUTO_TEST_CASE(test_forward_zone_recurse_rd_dnssec_cname_wildcard_expanded)
+{
+  std::unique_ptr<SyncRes> testSR;
+  initSR(testSR, true);
+
+  setDNSSECValidation(testSR, DNSSECMode::ValidateAll);
+
+  primeHints();
+  /* unsigned */
+  const DNSName target("test.");
+  /* signed */
+  const DNSName cnameTarget("cname.");
+  testkeysset_t keys;
+
+  auto luaconfsCopy = g_luaconfs.getCopy();
+  luaconfsCopy.dsAnchors.clear();
+  generateKeyMaterial(g_rootdnsname, DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys, luaconfsCopy.dsAnchors);
+  generateKeyMaterial(cnameTarget, DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys);
+  g_luaconfs.setState(luaconfsCopy);
+
+  const ComboAddress forwardedNS("192.0.2.42:53");
+  size_t queriesCount = 0;
+
+  SyncRes::AuthDomain authDomain;
+  authDomain.d_rdForward = true;
+  authDomain.d_servers.push_back(forwardedNS);
+  (*SyncRes::t_sstorage.domainmap)[g_rootdnsname] = authDomain;
+
+  testSR->setAsyncCallback([&](const ComboAddress& address, 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++;
+
+    BOOST_CHECK_EQUAL(sendRDQuery, true);
+
+    if (address != forwardedNS) {
+      return LWResult::Result::Timeout;
+    }
+
+    if (type == QType::DS || type == QType::DNSKEY) {
+      return genericDSAndDNSKEYHandler(res, domain, DNSName("."), type, keys);
+    }
+
+    if (domain == target && type == QType::A) {
+
+      setLWResult(res, 0, false, false, true);
+      addRecordToLW(res, target, QType::CNAME, cnameTarget.toString());
+      addRecordToLW(res, cnameTarget, QType::A, "192.0.2.1");
+      /* the RRSIG proves that the cnameTarget was expanded from a wildcard */
+      addRRSIG(keys, res->d_records, cnameTarget, 300, false, boost::none, DNSName("*"));
+      /* we need to add the proof that this name does not exist, so the wildcard may apply */
+      addNSECRecordToLW(DNSName("cnamd."), DNSName("cnamf."), {QType::A, QType::NSEC, QType::RRSIG}, 60, res->d_records);
+      addRRSIG(keys, res->d_records, cnameTarget, 300);
+
+      return LWResult::Result::Success;
+    }
+    return LWResult::Result::Timeout;
+  });
+
+  vector<DNSRecord> ret;
+  int res = testSR->beginResolve(target, QType(QType::A), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::NoError);
+  BOOST_CHECK_EQUAL(testSR->getValidationState(), vState::Insecure);
+  BOOST_REQUIRE_EQUAL(ret.size(), 5U);
+  BOOST_CHECK_EQUAL(queriesCount, 5U);
+
+  /* again, to test the cache */
+  ret.clear();
+  res = testSR->beginResolve(target, QType(QType::A), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::NoError);
+  BOOST_CHECK_EQUAL(testSR->getValidationState(), vState::Insecure);
+  BOOST_REQUIRE_EQUAL(ret.size(), 5U);
+  BOOST_CHECK_EQUAL(queriesCount, 5U);
+}
+
 BOOST_AUTO_TEST_CASE(test_auth_zone_oob)
 {
   std::unique_ptr<SyncRes> sr;

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index bbdba357c329cbb63d2b13e72bf77dcda79fffc2..fc3a6a4c449d7d223976a30be43213caf2e306d8 100644 (file)
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -4290,11 +4290,37 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr
   sanitizeRecords(prefix, lwr, qname, qtype, auth, wasForwarded, rdQuery);
 
   std::vector<std::shared_ptr<DNSRecord>> authorityRecs;
-  const unsigned int labelCount = qname.countLabels();
   bool isCNAMEAnswer = false;
   bool isDNAMEAnswer = false;
   DNSName seenAuth;
 
+  // names that might be expanded from a wildcard, and thus require denial of existence proof
+  // this is the queried name and any part of the CNAME chain from the queried name
+  // the key is the name itself, the value is initially false and is set to true once we have
+  // confirmed it was actually expanded from a wildcard
+  std::map<DNSName, bool> wildcardCandidates{{qname, false}};
+
+  if (rdQuery) {
+    std::unordered_map<DNSName, DNSName> cnames;
+    for (const auto& rec : lwr.d_records) {
+      if (rec.d_type != QType::CNAME || rec.d_class != QClass::IN) {
+        continue;
+      }
+      if (auto content = getRR<CNAMERecordContent>(rec)) {
+        cnames[rec.d_name] = DNSName(content->getTarget());
+      }
+    }
+    auto initial = qname;
+    while (true) {
+      auto cnameIt = cnames.find(initial);
+      if (cnameIt == cnames.end()) {
+        break;
+      }
+      initial = cnameIt->second;
+      wildcardCandidates.emplace(initial, false);
+    }
+  }
+
   for (auto& rec : lwr.d_records) {
     if (rec.d_type == QType::OPT || rec.d_class != QClass::IN) {
       continue;
@@ -4314,6 +4340,7 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr
       seenAuth = rec.d_name;
     }
 
+    const auto labelCount = rec.d_name.countLabels();
     if (rec.d_type == QType::RRSIG) {
       auto rrsig = getRR<RRSIGRecordContent>(rec);
       if (rrsig) {
@@ -4321,7 +4348,8 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr
            count can be lower than the name's label count if it was
            synthesized from the wildcard. Note that the difference might
            be > 1. */
-        if (rec.d_name == qname && isWildcardExpanded(labelCount, rrsig)) {
+        if (auto wcIt = wildcardCandidates.find(rec.d_name); wcIt != wildcardCandidates.end() && isWildcardExpanded(labelCount, rrsig)) {
+          wcIt->second = true;
           gatherWildcardProof = true;
           if (!isWildcardExpandedOntoItself(rec.d_name, labelCount, rrsig)) {
             /* if we have a wildcard expanded onto itself, we don't need to prove
@@ -4598,7 +4626,13 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr
         if (isAA && i->first.type == QType::NS && s_save_parent_ns_set) {
           rememberParentSetIfNeeded(i->first.name, i->second.records, depth);
         }
-        g_recCache->replace(d_now.tv_sec, i->first.name, i->first.type, i->second.records, i->second.signatures, authorityRecs, i->first.type == QType::DS ? true : isAA, auth, i->first.place == DNSResourceRecord::ANSWER ? ednsmask : boost::none, d_routingTag, recordState, remoteIP, d_refresh);
+        bool thisRRNeedsWildcardProof = false;
+        if (gatherWildcardProof) {
+          if (auto wcIt = wildcardCandidates.find(i->first.name); wcIt != wildcardCandidates.end() && wcIt->second) {
+            thisRRNeedsWildcardProof = true;
+          }
+        }
+        g_recCache->replace(d_now.tv_sec, i->first.name, i->first.type, i->second.records, i->second.signatures,  thisRRNeedsWildcardProof ? authorityRecs : std::vector<std::shared_ptr<DNSRecord>>(), i->first.type == QType::DS ? true : isAA, auth, i->first.place == DNSResourceRecord::ANSWER ? ednsmask : boost::none, d_routingTag, recordState, remoteIP, d_refresh);
 
         // Delete potential negcache entry. When a record recovers with serve-stale the negcache entry can cause the wrong entry to
         // be served, as negcache entries are checked before record cache entries
@@ -4606,10 +4640,11 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr
           g_negCache->wipeTyped(i->first.name, i->first.type);
         }
 
-        if (g_aggressiveNSECCache && needWildcardProof && recordState == vState::Secure && i->first.place == DNSResourceRecord::ANSWER && i->first.name == qname && !i->second.signatures.empty() && !d_routingTag && !ednsmask) {
+        if (g_aggressiveNSECCache && thisRRNeedsWildcardProof && recordState == vState::Secure && i->first.place == DNSResourceRecord::ANSWER && i->first.name == qname && !i->second.signatures.empty() && !d_routingTag && !ednsmask) {
           /* we have an answer synthesized from a wildcard and aggressive NSEC is enabled, we need to store the
              wildcard in its non-expanded form in the cache to be able to synthesize wildcard answers later */
           const auto& rrsig = i->second.signatures.at(0);
+          const auto labelCount = i->first.name.countLabels();
 
           if (isWildcardExpanded(labelCount, rrsig) && !isWildcardExpandedOntoItself(i->first.name, labelCount, rrsig)) {
             DNSName realOwner = getNSECOwnerName(i->first.name, i->second.signatures);
@@ -4642,6 +4677,13 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr
     }
   }
 
+  if (gatherWildcardProof) {
+    if (auto wcIt = wildcardCandidates.find(qname); wcIt != wildcardCandidates.end() && !wcIt->second) {
+      // the queried name was not expanded from a wildcard, a record in the CNAME chain was, so we don't need to gather wildcard proof now: we will do that when looking up the CNAME chain
+      gatherWildcardProof = false;
+    }
+  }
+
   return RCode::NoError;
 }