rec: Fix DNSSEC validation of DS denial from the negative cache

There is two reasons you can get a proper DS denial:
* Secure to insecure cut, and if we are getting a referral with a
DS denial, we know that we have to check that the NS bit is set
as described in section 8.9 of rfc5155
* No zone cut inside a secure zone, and then of course the NS is
not set

When we retrieve the DS denial from the negative cache with a
validation status of Indeterminate, most likely because validation
was not enabled during the query that landed it in the cache, we
don't have enough data to know which case we are looking at, so
let's just skip the NS check.

diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc
index 465bc364c3cd206b50d0e6e91bae3536bef983b6..c91f95cae2a122d97fbedbfc945c5ed64f8b688a 100644 (file)
--- a/pdns/recursordist/test-syncres_cc.cc
+++ b/pdns/recursordist/test-syncres_cc.cc
@@ -8856,6 +8856,67 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_secure) {
   BOOST_CHECK_EQUAL(queriesCount, 4);
 }
 
+BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_secure_ds) {
+  /*
+    Validation is optional, and the first query does not ask for it,
+    so the answer is negatively cached as Indeterminate.
+    The second query asks for validation, answer should be marked as
+    Secure.
+    The difference with test_dnssec_validation_from_negcache_secure is
+    that have one more level here, so we are going to look for the proof
+    that the DS does not exist for the last level. Since there is no cut,
+    we should accept the fact that the NSEC denies DS and NS both.
+  */
+  std::unique_ptr<SyncRes> sr;
+  initSR(sr, true);
+
+  setDNSSECValidation(sr, DNSSECMode::Process);
+
+  primeHints();
+  const DNSName target("www.com.");
+  testkeysset_t keys;
+
+  auto luaconfsCopy = g_luaconfs.getCopy();
+  luaconfsCopy.dsAnchors.clear();
+  generateKeyMaterial(g_rootdnsname, DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys, luaconfsCopy.dsAnchors);
+  generateKeyMaterial(DNSName("com."), DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys);
+  g_luaconfs.setState(luaconfsCopy);
+
+  size_t queriesCount = 0;
+
+  sr->setAsyncCallback([target,&queriesCount,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, std::shared_ptr<RemoteLogger> outgoingLogger, LWResult* res) {
+      queriesCount++;
+
+      if (type == QType::DS || type == QType::DNSKEY) {
+        if (domain == target) {
+          /* there is no cut */
+          return genericDSAndDNSKEYHandler(res, domain, domain, type, keys, false);
+        }
+        return genericDSAndDNSKEYHandler(res, domain, domain, type, keys);
+      }
+
+      return 0;
+    });
+
+  vector<DNSRecord> ret;
+  /* first query does not require validation */
+  sr->setDNSSECValidationRequested(false);
+  int res = sr->beginResolve(target, QType(QType::DS), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::NoError);
+  BOOST_CHECK_EQUAL(sr->getValidationState(), Indeterminate);
+  BOOST_REQUIRE_EQUAL(ret.size(), 4);
+  BOOST_CHECK_EQUAL(queriesCount, 1);
+
+  ret.clear();
+  /* second one _does_ require validation */
+  sr->setDNSSECValidationRequested(true);
+  res = sr->beginResolve(target, QType(QType::DS), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::NoError);
+  BOOST_CHECK_EQUAL(sr->getValidationState(), Secure);
+  BOOST_REQUIRE_EQUAL(ret.size(), 4);
+  BOOST_CHECK_EQUAL(queriesCount, 4);
+}
+
 BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_insecure) {
   /*
     Validation is optional, and the first query does not ask for it,

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 5c026511af11c4ccd4803631f0e3a3c2bcac4c12..40c69b777006316dcd17df2d5e510c0996f4e0c1 100644 (file)
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -1030,7 +1030,7 @@ void SyncRes::computeNegCacheValidationStatus(NegCache::NegCacheEntry& ne, const
 
   if (state == Secure) {
     dState expectedState = res == RCode::NXDomain ? NXDOMAIN : NXQTYPE;
-    dState denialState = getDenialValidationState(ne, state, expectedState, qtype == QType::DS);
+    dState denialState = getDenialValidationState(ne, state, expectedState, false);
     updateDenialValidationState(ne, state, denialState, expectedState, qtype == QType::DS);
   }
   if (state != Indeterminate) {

diff --git a/pdns/validate.cc b/pdns/validate.cc
index 66b7d13eafb027e65a817b470d779acaef1646b8..f7f37e6fa6937d80b73dab3dcda39803101cc39a 100644 (file)
--- a/pdns/validate.cc
+++ b/pdns/validate.cc
@@ -298,7 +298,7 @@ static bool provesNSEC3NoWildCard(DNSName wildcard, uint16_t const qtype, const
 /*
   This function checks whether the existence of qname|qtype is denied by the NSEC and NSEC3
   in validrrsets.
-  - If `referralToUnsigned` is true and qtype is QType::DS, this functions returns Insecure
+  - If `referralToUnsigned` is true and qtype is QType::DS, this functions returns NODATA
   if a NSEC or NSEC3 proves that the name exists but no NS type exists, as specified in RFC 5155 section 8.9.
   - If `wantsNoDataProof` is set but a NSEC proves that the whole name does not exist, the function will return
   NXQTYPE is the name is proven to be ENT and NXDOMAIN otherwise.