rec: Cache Secure validation state when inserting negcache entries

Fix a bug that prevented Secure negative cache entries to be marked
as such when they were first inserted, marking them as Indeterminate
instead. This would require us to validate them a second time for no
valid reason.

diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc
index 465bc364c3cd206b50d0e6e91bae3536bef983b6..c5ed98d7f558b039785934d11a8ff261ddc9c1c0 100644 (file)
--- a/pdns/recursordist/test-syncres_cc.cc
+++ b/pdns/recursordist/test-syncres_cc.cc
@@ -8295,6 +8295,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_rrsig_negcache_validity) {
   BOOST_CHECK_EQUAL(SyncRes::t_sstorage.negcache.size(), 1);
   BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), ne), true);
   BOOST_CHECK_EQUAL(ne.d_ttd, now + 1);
+  BOOST_CHECK_EQUAL(ne.d_validationState, Secure);
   BOOST_CHECK_EQUAL(ne.authoritySOA.records.size(), 1);
   BOOST_CHECK_EQUAL(ne.authoritySOA.signatures.size(), 1);
   BOOST_CHECK_EQUAL(ne.DNSSECRecords.records.size(), 1);
@@ -8845,6 +8846,15 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_secure) {
   BOOST_CHECK_EQUAL(sr->getValidationState(), Indeterminate);
   BOOST_REQUIRE_EQUAL(ret.size(), 4);
   BOOST_CHECK_EQUAL(queriesCount, 1);
+  /* check that the entry has not been negatively cached */
+  NegCache::NegCacheEntry ne;
+  BOOST_CHECK_EQUAL(SyncRes::t_sstorage.negcache.size(), 1);
+  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), ne), true);
+  BOOST_CHECK_EQUAL(ne.d_validationState, Indeterminate);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.records.size(), 1);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.signatures.size(), 1);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.records.size(), 1);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.signatures.size(), 1);
 
   ret.clear();
   /* second one _does_ require validation */
@@ -8854,6 +8864,13 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_secure) {
   BOOST_CHECK_EQUAL(sr->getValidationState(), Secure);
   BOOST_REQUIRE_EQUAL(ret.size(), 4);
   BOOST_CHECK_EQUAL(queriesCount, 4);
+  BOOST_CHECK_EQUAL(SyncRes::t_sstorage.negcache.size(), 1);
+  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), ne), true);
+  BOOST_CHECK_EQUAL(ne.d_validationState, Secure);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.records.size(), 1);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.signatures.size(), 1);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.records.size(), 1);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.signatures.size(), 1);
 }
 
 BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_insecure) {
@@ -8904,6 +8921,15 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_insecure) {
   BOOST_CHECK_EQUAL(sr->getValidationState(), Indeterminate);
   BOOST_REQUIRE_EQUAL(ret.size(), 1);
   BOOST_CHECK_EQUAL(queriesCount, 1);
+  /* check that the entry has not been negatively cached */
+  NegCache::NegCacheEntry ne;
+  BOOST_CHECK_EQUAL(SyncRes::t_sstorage.negcache.size(), 1);
+  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), ne), true);
+  BOOST_CHECK_EQUAL(ne.d_validationState, Indeterminate);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.records.size(), 1);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.signatures.size(), 0);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.records.size(), 0);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.signatures.size(), 0);
 
   ret.clear();
   /* second one _does_ require validation */
@@ -8913,6 +8939,12 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_insecure) {
   BOOST_CHECK_EQUAL(sr->getValidationState(), Insecure);
   BOOST_REQUIRE_EQUAL(ret.size(), 1);
   BOOST_CHECK_EQUAL(queriesCount, 1);
+  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), ne), true);
+  BOOST_CHECK_EQUAL(ne.d_validationState, Insecure);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.records.size(), 1);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.signatures.size(), 0);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.records.size(), 0);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.signatures.size(), 0);
 }
 
 BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_bogus) {
@@ -8967,6 +8999,14 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_bogus) {
   BOOST_CHECK_EQUAL(sr->getValidationState(), Indeterminate);
   BOOST_REQUIRE_EQUAL(ret.size(), 2);
   BOOST_CHECK_EQUAL(queriesCount, 1);
+  NegCache::NegCacheEntry ne;
+  BOOST_CHECK_EQUAL(SyncRes::t_sstorage.negcache.size(), 1);
+  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), ne), true);
+  BOOST_CHECK_EQUAL(ne.d_validationState, Indeterminate);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.records.size(), 1);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.signatures.size(), 1);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.records.size(), 0);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.signatures.size(), 0);
 
   ret.clear();
   /* second one _does_ require validation */
@@ -8976,6 +9016,12 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_bogus) {
   BOOST_CHECK_EQUAL(sr->getValidationState(), Bogus);
   BOOST_REQUIRE_EQUAL(ret.size(), 2);
   BOOST_CHECK_EQUAL(queriesCount, 4);
+  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), ne), true);
+  BOOST_CHECK_EQUAL(ne.d_validationState, Bogus);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.records.size(), 1);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.signatures.size(), 1);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.records.size(), 0);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.signatures.size(), 0);
 }
 
 BOOST_AUTO_TEST_CASE(test_lowercase_outgoing) {

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 5c026511af11c4ccd4803631f0e3a3c2bcac4c12..570198165b037bf4ce99e24e4d74ada06265107e 100644 (file)
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -1747,7 +1747,7 @@ void SyncRes::computeZoneCuts(const DNSName& begin, const DNSName& end, unsigned
     }
     else {
       /* remove the temporary cut */
-      LOG(d_prefix<<qname<<": removing cut state for "<<qname<<", was "<<vStates[d_cutStates[qname]]<<endl);
+      LOG(d_prefix<<qname<<": removing cut state for "<<qname<<endl);
       d_cutStates.erase(qname);
     }
   }
@@ -2081,7 +2081,10 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr
 
 void SyncRes::updateDenialValidationState(NegCache::NegCacheEntry& ne, vState& state, const dState denialState, const dState expectedState, bool allowOptOut)
 {
-  if (denialState != expectedState) {
+  if (denialState == expectedState) {
+    ne.d_validationState = Secure;
+  }
+  else {
     if (denialState == OPTOUT && allowOptOut) {
       LOG(d_prefix<<"OPT-out denial found for "<<ne.d_name<<endl);
       ne.d_validationState = Secure;