]> git.ipfire.org Git - thirdparty/pdns.git/commitdiff
rec: Only add the NSEC and RRSIG records once in wildcard NODATA answers 10473/head
authorRemi Gacogne <remi.gacogne@powerdns.com>
Mon, 3 May 2021 13:00:04 +0000 (15:00 +0200)
committerOtto <otto.moerbeek@open-xchange.com>
Mon, 7 Jun 2021 07:09:03 +0000 (09:09 +0200)
For wildcard-expanded answers we need to collect the proof that the
exact name does not exist and add them to the response. We also
collect that proof for negative answers.
When the answer is a wildcard-expanded NODATA, we only need to collect
them once, not twice.

(cherry picked from commit d89f023d1bd6ae7d0eb6d72e7b2771363f5e4f79)

pdns/recursordist/test-syncres_cc5.cc
pdns/syncres.cc

index 7f83dccdfddaa497c2736705da02b114eee3c4c4..b6230b98e2175bcee51394e777335f3a940bd403 100644 (file)
@@ -1825,6 +1825,60 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_wildcard_expanded_onto_itself)
   BOOST_REQUIRE_EQUAL(ret.size(), 4U);
 }
 
+BOOST_AUTO_TEST_CASE(test_dnssec_validation_wildcard_expanded_onto_itself_nodata)
+{
+  std::unique_ptr<SyncRes> sr;
+  initSR(sr, true);
+
+  setDNSSECValidation(sr, DNSSECMode::ValidateAll);
+
+  primeHints();
+  const DNSName target("*.powerdns.com.");
+  testkeysset_t keys;
+
+  auto luaconfsCopy = g_luaconfs.getCopy();
+  luaconfsCopy.dsAnchors.clear();
+  generateKeyMaterial(g_rootdnsname, DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys, luaconfsCopy.dsAnchors);
+  generateKeyMaterial(DNSName("com."), DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys, luaconfsCopy.dsAnchors);
+  generateKeyMaterial(DNSName("powerdns.com."), DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys, luaconfsCopy.dsAnchors);
+
+  g_luaconfs.setState(luaconfsCopy);
+
+  sr->setAsyncCallback([target, keys](const ComboAddress& ip, 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) {
+    if (type == QType::DS || type == QType::DNSKEY) {
+      if (domain == target) {
+        const auto auth = DNSName("powerdns.com.");
+        /* we don't want a cut there */
+        setLWResult(res, 0, true, false, true);
+        addRecordToLW(res, auth, QType::SOA, "foo. bar. 2017032800 1800 900 604800 86400", DNSResourceRecord::AUTHORITY, 86400);
+        addRRSIG(keys, res->d_records, auth, 300);
+        /* add a NSEC denying the DS */
+        std::set<uint16_t> types = {QType::NSEC};
+        addNSECRecordToLW(domain, DNSName("z") + domain, types, 600, res->d_records);
+        addRRSIG(keys, res->d_records, auth, 300);
+        return LWResult::Result::Success;
+      }
+      return genericDSAndDNSKEYHandler(res, domain, domain, type, keys);
+    }
+    else {
+      setLWResult(res, 0, true, false, true);
+      addRecordToLW(res, domain, QType::SOA, "powerdns.com. bar. 2017032800 1800 900 604800 86400", DNSResourceRecord::AUTHORITY, 86400);
+      addRRSIG(keys, res->d_records, DNSName("powerdns.com."), 300);
+      /* add the proof that the exact name does exist but that this type does not */
+      addNSECRecordToLW(DNSName("*.powerdns.com."), DNSName("\\000.*.powerdns.com."), {QType::AAAA, QType::NSEC, QType::RRSIG}, 600, res->d_records);
+      addRRSIG(keys, res->d_records, DNSName("powerdns.com"), 300, false, boost::none, DNSName("*.powerdns.com"));
+      return LWResult::Result::Success;
+    }
+  });
+
+  vector<DNSRecord> ret;
+  int res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::NoError);
+  BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Secure);
+  /* SOA + RRSIG, NSEC + RRSIG */
+  BOOST_REQUIRE_EQUAL(ret.size(), 4U);
+}
+
 BOOST_AUTO_TEST_CASE(test_dnssec_validation_wildcard_like_expanded_from_wildcard)
 {
   std::unique_ptr<SyncRes> sr;
index 8cfed6f1faaff1b367a89bdfc0fd5f8b303e8173..8046091df28509c052afb9a9cb4dfd389e172b21 100644 (file)
@@ -3520,8 +3520,9 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co
     }
     /* if we have a positive answer synthesized from a wildcard, we need to
        return the corresponding NSEC/NSEC3 records from the AUTHORITY section
-       proving that the exact name did not exist */
-    else if (gatherWildcardProof && (rec.d_type == QType::RRSIG || rec.d_type == QType::NSEC || rec.d_type == QType::NSEC3) && rec.d_place == DNSResourceRecord::AUTHORITY) {
+       proving that the exact name did not exist.
+       Except if this is a NODATA answer because then we will gather the NXNSEC records later */
+    else if (gatherWildcardProof && !negindic && (rec.d_type == QType::RRSIG || rec.d_type == QType::NSEC || rec.d_type == QType::NSEC3) && rec.d_place == DNSResourceRecord::AUTHORITY) {
       ret.push_back(rec); // enjoy your DNSSEC
     }
     // for ANY answers we *must* have an authoritative answer, unless we are forwarding recursively
@@ -3586,7 +3587,7 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co
       } else if (rec.d_type == QType::RRSIG && qname.isPartOf(rec.d_name)) {
         auto rrsig = getRR<RRSIGRecordContent>(rec);
         if (rrsig != nullptr && rrsig->d_type == QType::DNAME) {
-           ret.push_back(rec);
+          ret.push_back(rec);
         }
       }
     }