rec: Only add the NSEC and RRSIG records once in wildcard NODATA answers

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.

diff --git a/pdns/recursordist/test-syncres_cc5.cc b/pdns/recursordist/test-syncres_cc5.cc
index 7f83dccdfddaa497c2736705da02b114eee3c4c4..b6230b98e2175bcee51394e777335f3a940bd403 100644 (file)
--- a/pdns/recursordist/test-syncres_cc5.cc
+++ b/pdns/recursordist/test-syncres_cc5.cc
@@ -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;

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index d3c57be009ab00303fe866bdd85dfcb7e8359137..103bfa73af7ae05d4b8bd1b2b836942b0a7cdc4d 100644 (file)
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -3530,8 +3530,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
@@ -3596,7 +3597,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);
         }
       }
     }