From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Mon, 8 Jan 2024 14:04:13 +0000 (+0100)
Subject: rec: Fix validation accounting in validateDNSKeysAgainstDS()
X-Git-Tag: rec-4.9.3~3
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5831e0dfa97d16cfcc65d6bd3c7ad79659a13afa;p=thirdparty%2Fpdns.git

rec: Fix validation accounting in validateDNSKeysAgainstDS()

The counter was sometimes increased even though no actual validation
was performed, because the corresponding DNSKEY was not (yet) trusted.
---

diff --git a/pdns/validate.cc b/pdns/validate.cc
index 4bfb9f1cc0..7d4c8e2a6c 100644
--- a/pdns/validate.cc
+++ b/pdns/validate.cc
@@ -1200,14 +1200,6 @@ vState validateDNSKeysAgainstDS(time_t now, const DNSName& zone, const dsmap_t&
         continue;
       }
 
-      if (g_maxRRSIGsPerRecordToConsider > 0 && signaturesConsidered >= g_maxRRSIGsPerRecordToConsider) {
-        VLOG(log, zone << ": We have already considered "<<std::to_string(signaturesConsidered)<<" RRSIG"<<addS(signaturesConsidered)<<" for this record, stopping now"<<endl;);
-        // possibly going Bogus, the RRSIGs have not been validated so Insecure would be wrong
-        break;
-      }
-      signaturesConsidered++;
-      context.d_validationsCounter++;
-
       //        cerr<<"got sig for keytag "<<i->d_tag<<" matching "<<getByTag(tkeys, i->d_tag).size()<<" keys of which "<<getByTag(validkeys, i->d_tag).size()<<" valid"<<endl;
       auto bytag = getByTag(validkeys, sig->d_tag, sig->d_algorithm, log);
 
@@ -1215,6 +1207,12 @@ vState validateDNSKeysAgainstDS(time_t now, const DNSName& zone, const dsmap_t&
         continue;
       }
 
+      if (g_maxRRSIGsPerRecordToConsider > 0 && signaturesConsidered >= g_maxRRSIGsPerRecordToConsider) {
+        VLOG(log, zone << ": We have already considered "<<std::to_string(signaturesConsidered)<<" RRSIG"<<addS(signaturesConsidered)<<" for this record, stopping now"<<endl;);
+        // possibly going Bogus, the RRSIGs have not been validated so Insecure would be wrong
+        break;
+      }
+
       string msg = getMessageForRRSET(zone, *sig, toSign);
       uint16_t dnskeysConsidered = 0;
       for (const auto& key : bytag) {
@@ -1224,8 +1222,15 @@ vState validateDNSKeysAgainstDS(time_t now, const DNSName& zone, const dsmap_t&
         }
         dnskeysConsidered++;
 
+        if (g_maxRRSIGsPerRecordToConsider > 0 && signaturesConsidered >= g_maxRRSIGsPerRecordToConsider) {
+          VLOG(log, zone << ": We have already considered "<<std::to_string(signaturesConsidered)<<" RRSIG"<<addS(signaturesConsidered)<<" for this record, stopping now"<<endl;);
+          // possibly going Bogus, the RRSIGs have not been validated so Insecure would be wrong
+          break;
+        }
         //          cerr<<"validating : ";
         bool signIsValid = checkSignatureWithKey(zone, *sig, *key, msg, ede, log);
+        signaturesConsidered++;
+        context.d_validationsCounter++;
 
         if (signIsValid) {
           VLOG(log, zone << ": Validation succeeded - whole DNSKEY set is valid"<<endl);