From 99b5a2c6dff2e686472872a0059761af3f0b8ffb Mon Sep 17 00:00:00 2001
From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Mon, 6 Jul 2020 15:00:44 +0200
Subject: [PATCH] rec: Validate cached DNSKEYs against the DSs, not the RRSIGs
 only

DNSKEYs might be cached in a non-validated state ("Indeterminate")
when the DNSSEC mode is set to "Process" and the initial query did
not ask for validation.
We would then validate the DNSKEY records against the RRSIGs, like
for regular records, but not against the DSs.

(cherry picked from commit 453f37736a4d372e16755a903f5b5d5ac52b0c17)
---
 pdns/recursordist/test-syncres_cc4.cc | 41 +++++++++++++++++++++++++--
 pdns/syncres.cc                       |  7 ++++-
 2 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/pdns/recursordist/test-syncres_cc4.cc b/pdns/recursordist/test-syncres_cc4.cc
index 49f712069b..17fe09009f 100644
--- a/pdns/recursordist/test-syncres_cc4.cc
+++ b/pdns/recursordist/test-syncres_cc4.cc
@@ -623,7 +623,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_bogus_dnskey_doesnt_match_ds) {
   std::unique_ptr<SyncRes> sr;
   initSR(sr, true);
 
-  setDNSSECValidation(sr, DNSSECMode::ValidateAll);
+  setDNSSECValidation(sr, DNSSECMode::Process);
 
   primeHints();
   const DNSName target(".");
@@ -687,6 +687,8 @@ BOOST_AUTO_TEST_CASE(test_dnssec_bogus_dnskey_doesnt_match_ds) {
       return 0;
     });
 
+  /* === with validation enabled === */
+  sr->setDNSSECValidationRequested(true);
   vector<DNSRecord> ret;
   int res = sr->beginResolve(target, QType(QType::NS), QClass::IN, ret);
   BOOST_CHECK_EQUAL(res, RCode::NoError);
@@ -700,8 +702,41 @@ BOOST_AUTO_TEST_CASE(test_dnssec_bogus_dnskey_doesnt_match_ds) {
   res = sr->beginResolve(target, QType(QType::NS), QClass::IN, ret);
   BOOST_CHECK_EQUAL(res, RCode::NoError);
   BOOST_CHECK_EQUAL(sr->getValidationState(), Bogus);
-  BOOST_REQUIRE_EQUAL(ret.size(), 14);
-  BOOST_CHECK_EQUAL(queriesCount, 2);
+  BOOST_REQUIRE_EQUAL(ret.size(), 14U);
+  BOOST_CHECK_EQUAL(queriesCount, 2U);
+
+  /* === first without validation, then with (just-in-time validation) === */
+  /* clear the caches */
+  t_RC = std::unique_ptr<MemRecursorCache>(new MemRecursorCache());
+  SyncRes::clearNegCache();
+  sr->setDNSSECValidationRequested(false);
+  primeHints();
+
+  ret.clear();
+  res = sr->beginResolve(target, QType(QType::NS), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::NoError);
+  BOOST_CHECK_EQUAL(sr->getValidationState(), Indeterminate);
+  /* 13 NS + 1 RRSIG */
+  BOOST_REQUIRE_EQUAL(ret.size(), 14U);
+  BOOST_CHECK_EQUAL(queriesCount, 3U);
+
+  /* now we ask for the DNSKEYs (still without validation) */
+  ret.clear();
+  res = sr->beginResolve(target, QType(QType::DNSKEY), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::NoError);
+  BOOST_CHECK_EQUAL(sr->getValidationState(), Indeterminate);
+  /* 1 SOA + 1 RRSIG */
+  BOOST_REQUIRE_EQUAL(ret.size(), 2U);
+  BOOST_CHECK_EQUAL(queriesCount, 4U);
+
+  /* again, to test the cache WITH validation */
+  sr->setDNSSECValidationRequested(true);
+  ret.clear();
+  res = sr->beginResolve(target, QType(QType::NS), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::NoError);
+  BOOST_CHECK_EQUAL(sr->getValidationState(), Bogus);
+  BOOST_REQUIRE_EQUAL(ret.size(), 14U);
+  BOOST_CHECK_EQUAL(queriesCount, 4U);
 }
 
 BOOST_AUTO_TEST_CASE(test_dnssec_bogus_rrsig_signed_with_unknown_dnskey) {
diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index dbd5ea1c03..c8e41feded 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -1339,7 +1339,12 @@ bool SyncRes::doCacheCheck(const DNSName &qname, const DNSName& authname, bool w
       vState recordState = getValidationStatus(qname, false);
       if (recordState == Secure) {
         LOG(prefix<<sqname<<": got Indeterminate state from the cache, validating.."<<endl);
-        cachedState = SyncRes::validateRecordsWithSigs(depth, sqname, sqt, sqname, cset, signatures);
+        if (sqt == QType::DNSKEY) {
+          cachedState = validateDNSKeys(sqname, cset, signatures, depth);
+        }
+        else {
+          cachedState = SyncRes::validateRecordsWithSigs(depth, sqname, sqt, sqname, cset, signatures);
+        }
       }
       else {
         cachedState = recordState;
-- 
2.47.2