From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Wed, 17 May 2023 10:22:18 +0000 (+0200)
Subject: Delint validate.cc and related files
X-Git-Tag: rec-4.9.0-beta1~5^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F12837%2Fhead;p=thirdparty%2Fpdns.git

Delint validate.cc and related files
---

diff --git a/.clang-tidy.full b/.clang-tidy.full
index bf8c8c8eca..f6ba38ee2e 100644
--- a/.clang-tidy.full
+++ b/.clang-tidy.full
@@ -113,7 +113,7 @@ CheckOptions:
   - key:             readability-function-size.LineThreshold
     value:           '4294967295'
   - key:             bugprone-easily-swappable-parameters.MinimumLength
-    value:           '2'
+    value:           '4'
   - key:             portability-simd-intrinsics.Suggest
     value:           'false'
   - key:             cppcoreguidelines-pro-bounds-constant-array-index.GslHeader
diff --git a/pdns/recursordist/validate-recursor.cc b/pdns/recursordist/validate-recursor.cc
index ba8c2cae21..1a676792a9 100644
--- a/pdns/recursordist/validate-recursor.cc
+++ b/pdns/recursordist/validate-recursor.cc
@@ -46,26 +46,26 @@ bool updateTrustAnchorsFromFile(const std::string& fname, map<DNSName, dsmap_t>&
 {
   map<DNSName, dsmap_t> newDSAnchors;
   try {
-    auto zp = ZoneParserTNG(fname);
-    zp.disableGenerate();
-    DNSResourceRecord rr;
-    DNSRecord dr;
-    while (zp.get(rr)) {
-      dr = DNSRecord(rr);
-      if (rr.qtype == QType::DS) {
-        auto dsr = getRR<DSRecordContent>(dr);
+    auto zoneParser = ZoneParserTNG(fname);
+    zoneParser.disableGenerate();
+    DNSResourceRecord resourceRecord;
+    DNSRecord dnsrecord;
+    while (zoneParser.get(resourceRecord)) {
+      dnsrecord = DNSRecord(resourceRecord);
+      if (resourceRecord.qtype == QType::DS) {
+        auto dsr = getRR<DSRecordContent>(dnsrecord);
         if (dsr == nullptr) {
-          throw PDNSException("Unable to parse DS record '" + rr.qname.toString() + " " + rr.getZoneRepresentation() + "'");
+          throw PDNSException("Unable to parse DS record '" + resourceRecord.qname.toString() + " " + resourceRecord.getZoneRepresentation() + "'");
         }
-        newDSAnchors[rr.qname].insert(*dsr);
+        newDSAnchors[resourceRecord.qname].insert(*dsr);
       }
-      if (rr.qtype == QType::DNSKEY) {
-        auto dnskeyr = getRR<DNSKEYRecordContent>(dr);
+      if (resourceRecord.qtype == QType::DNSKEY) {
+        auto dnskeyr = getRR<DNSKEYRecordContent>(dnsrecord);
         if (dnskeyr == nullptr) {
-          throw PDNSException("Unable to parse DNSKEY record '" + rr.qname.toString() + " " + rr.getZoneRepresentation() + "'");
+          throw PDNSException("Unable to parse DNSKEY record '" + resourceRecord.qname.toString() + " " + resourceRecord.getZoneRepresentation() + "'");
         }
-        auto dsr = makeDSFromDNSKey(rr.qname, *dnskeyr, DNSSECKeeper::DIGEST_SHA256);
-        newDSAnchors[rr.qname].insert(dsr);
+        auto dsr = makeDSFromDNSKey(resourceRecord.qname, *dnskeyr, DNSSECKeeper::DIGEST_SHA256);
+        newDSAnchors[resourceRecord.qname].insert(dsr);
       }
     }
     if (dsAnchors == newDSAnchors) {
diff --git a/pdns/validate.cc b/pdns/validate.cc
index af4f9d69af..8306d9aae5 100644
--- a/pdns/validate.cc
+++ b/pdns/validate.cc
@@ -1,3 +1,25 @@
+/*
+ * This file is part of PowerDNS or dnsdist.
+ * Copyright -- PowerDNS.COM B.V. and its contributors
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * In addition, for the avoidance of any doubt, permission is granted to
+ * link this program with OpenSSL and to (re)distribute the binaries
+ * produced as the result of such linking.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
 #include "validate.hh"
 #include "misc.hh"
 #include "dnssecinfra.hh"
@@ -53,20 +75,20 @@ static vector<shared_ptr<const DNSKEYRecordContent > > getByTag(const skeyset_t&
   return ret;
 }
 
-bool isCoveredByNSEC3Hash(const std::string& h, const std::string& beginHash, const std::string& nextHash)
+bool isCoveredByNSEC3Hash(const std::string& hash, const std::string& beginHash, const std::string& nextHash)
 {
-  return ((beginHash < h && h < nextHash) ||          // no wrap          BEGINNING --- HASH -- END
-          (nextHash > h  && beginHash > nextHash) ||  // wrap             HASH --- END --- BEGINNING
-          (nextHash < beginHash  && beginHash < h) || // wrap other case  END --- BEGINNING --- HASH
-          (beginHash == nextHash && h != beginHash));   // "we have only 1 NSEC3 record, LOL!"
+  return ((beginHash < hash && hash < nextHash) ||          // no wrap          BEGINNING --- HASH -- END
+          (nextHash > hash  && beginHash > nextHash) ||  // wrap             HASH --- END --- BEGINNING
+          (nextHash < beginHash  && beginHash < hash) || // wrap other case  END --- BEGINNING --- HASH
+          (beginHash == nextHash && hash != beginHash));   // "we have only 1 NSEC3 record, LOL!"
 }
 
-bool isCoveredByNSEC3Hash(const DNSName& h, const DNSName& beginHash, const DNSName& nextHash)
+bool isCoveredByNSEC3Hash(const DNSName& name, const DNSName& beginHash, const DNSName& nextHash)
 {
-  return ((beginHash.canonCompare(h) && h.canonCompare(nextHash)) ||          // no wrap          BEGINNING --- HASH -- END
-          (h.canonCompare(nextHash) && nextHash.canonCompare(beginHash)) ||  // wrap             HASH --- END --- BEGINNING
-          (nextHash.canonCompare(beginHash) && beginHash.canonCompare(h)) || // wrap other case  END --- BEGINNING --- HASH
-          (beginHash == nextHash && h != beginHash));   // "we have only 1 NSEC3 record, LOL!"
+  return ((beginHash.canonCompare(name) && name.canonCompare(nextHash)) ||          // no wrap          BEGINNING --- HASH -- END
+          (name.canonCompare(nextHash) && nextHash.canonCompare(beginHash)) ||  // wrap             HASH --- END --- BEGINNING
+          (nextHash.canonCompare(beginHash) && beginHash.canonCompare(name)) || // wrap other case  END --- BEGINNING --- HASH
+          (beginHash == nextHash && name != beginHash));   // "we have only 1 NSEC3 record, LOL!"
 }
 
 bool isCoveredByNSEC(const DNSName& name, const DNSName& begin, const DNSName& next)
@@ -92,15 +114,15 @@ static std::string getHashFromNSEC3(const DNSName& qname, const NSEC3RecordConte
 {
   std::string result;
 
-  if (g_maxNSEC3Iterations && nsec3.d_iterations > g_maxNSEC3Iterations) {
+  if (g_maxNSEC3Iterations != 0 && nsec3.d_iterations > g_maxNSEC3Iterations) {
     return result;
   }
 
   auto key = std::make_tuple(qname, nsec3.d_salt, nsec3.d_iterations);
-  auto it = cache.find(key);
-  if (it != cache.end())
+  auto iter = cache.find(key);
+  if (iter != cache.end())
   {
-    return it->second;
+    return iter->second;
   }
 
   result = hashQNameWithSalt(nsec3.d_salt, nsec3.d_iterations, qname);
@@ -139,17 +161,17 @@ bool denialProvesNoDelegation(const DNSName& zone, const std::vector<DNSRecord>&
         continue;
       }
 
-      const string h = getHashFromNSEC3(zone, *nsec3, cache);
-      if (h.empty()) {
+      const string hash = getHashFromNSEC3(zone, *nsec3, cache);
+      if (hash.empty()) {
         return false;
       }
 
       const string beginHash = fromBase32Hex(record.d_name.getRawLabels()[0]);
-      if (beginHash == h) {
+      if (beginHash == hash) {
         return !nsec3->isSet(QType::NS);
       }
 
-      if (isCoveredByNSEC3Hash(h, beginHash, nsec3->d_nexthash)) {
+      if (isCoveredByNSEC3Hash(hash, beginHash, nsec3->d_nexthash)) {
         return !(nsec3->isOptOut());
       }
     }
@@ -165,11 +187,7 @@ bool denialProvesNoDelegation(const DNSName& zone, const std::vector<DNSRecord>&
 */
 bool isWildcardExpanded(unsigned int labelCount, const RRSIGRecordContent& sign)
 {
-  if (sign.d_labels < labelCount) {
-    return true;
-  }
-
-  return false;
+  return sign.d_labels < labelCount;
 }
 
 static bool isWildcardExpanded(const DNSName& owner, const std::vector<std::shared_ptr<const RRSIGRecordContent> >& signatures)
@@ -185,11 +203,8 @@ static bool isWildcardExpanded(const DNSName& owner, const std::vector<std::shar
 
 bool isWildcardExpandedOntoItself(const DNSName& owner, unsigned int labelCount, const RRSIGRecordContent& sign)
 {
-  if (owner.isWildcard() && (labelCount - 1) == sign.d_labels) {
-    /* this is a wildcard alright, but it has not been expanded */
-    return true;
-  }
-  return false;
+  /* this is a wildcard alright, but it has not been expanded */
+  return owner.isWildcard() && (labelCount - 1) == sign.d_labels;
 }
 
 static bool isWildcardExpandedOntoItself(const DNSName& owner, const std::vector<std::shared_ptr<const RRSIGRecordContent> >& signatures)
@@ -246,17 +261,17 @@ static bool provesNoDataWildCard(const DNSName& qname, const uint16_t qtype, con
 {
   const DNSName wildcard = g_wildcarddnsname + closestEncloser;
   VLOG(log, qname << ": Trying to prove that there is no data in wildcard for "<<qname<<"/"<<QType(qtype)<<endl);
-  for (const auto& v : validrrsets) {
-    VLOG(log, qname << ": Do have: "<<v.first.first<<"/"<<DNSRecordContent::NumberToType(v.first.second)<<endl);
-    if (v.first.second == QType::NSEC) {
-      for (const auto& r : v.second.records) {
-        VLOG(log, ":\t"<<r->getZoneRepresentation()<<endl);
-        auto nsec = std::dynamic_pointer_cast<const NSECRecordContent>(r);
+  for (const auto& validset : validrrsets) {
+    VLOG(log, qname << ": Do have: "<<validset.first.first<<"/"<<DNSRecordContent::NumberToType(validset.first.second)<<endl);
+    if (validset.first.second == QType::NSEC) {
+      for (const auto& record : validset.second.records) {
+        VLOG(log, ":\t"<<record->getZoneRepresentation()<<endl);
+        auto nsec = std::dynamic_pointer_cast<const NSECRecordContent>(record);
         if (!nsec) {
           continue;
         }
 
-        DNSName owner = getNSECOwnerName(v.first.first, v.second.signatures);
+        DNSName owner = getNSECOwnerName(validset.first.first, validset.second.signatures);
         if (owner != wildcard) {
           continue;
         }
@@ -293,17 +308,17 @@ static bool provesNoWildCard(const DNSName& qname, const uint16_t qtype, const D
 {
   VLOG(log, qname << ": Trying to prove that there is no wildcard for "<<qname<<"/"<<QType(qtype)<<endl);
   const DNSName wildcard = g_wildcarddnsname + closestEncloser;
-  for (const auto& v : validrrsets) {
-    VLOG(log, qname << ": Do have: "<<v.first.first<<"/"<<DNSRecordContent::NumberToType(v.first.second)<<endl);
-    if (v.first.second == QType::NSEC) {
-      for (const auto& r : v.second.records) {
-        VLOG(log, qname << ":\t"<<r->getZoneRepresentation()<<endl);
-        auto nsec = std::dynamic_pointer_cast<const NSECRecordContent>(r);
+  for (const auto& validset : validrrsets) {
+    VLOG(log, qname << ": Do have: "<<validset.first.first<<"/"<<DNSRecordContent::NumberToType(validset.first.second)<<endl);
+    if (validset.first.second == QType::NSEC) {
+      for (const auto& records : validset.second.records) {
+        VLOG(log, qname << ":\t"<<records->getZoneRepresentation()<<endl);
+        auto nsec = std::dynamic_pointer_cast<const NSECRecordContent>(records);
         if (!nsec) {
           continue;
         }
 
-        const DNSName owner = getNSECOwnerName(v.first.first, v.second.signatures);
+        const DNSName owner = getNSECOwnerName(validset.first.first, validset.second.signatures);
         VLOG(log, qname << ": Comparing owner: "<<owner<<" with target: "<<wildcard<<endl);
 
         if (qname != owner && qname.isPartOf(owner) && nsec->isSet(QType::DNAME)) {
@@ -342,32 +357,32 @@ static bool provesNSEC3NoWildCard(const DNSName& closestEncloser, uint16_t const
   auto wildcard = g_wildcarddnsname + closestEncloser;
   VLOG(log, closestEncloser << ": Trying to prove that there is no wildcard for "<<wildcard<<"/"<<QType(qtype)<<endl);
 
-  for (const auto& v : validrrsets) {
-    VLOG(log, closestEncloser << ": Do have: "<<v.first.first<<"/"<<DNSRecordContent::NumberToType(v.first.second)<<endl);
-    if (v.first.second == QType::NSEC3) {
-      for (const auto& r : v.second.records) {
-        VLOG(log, closestEncloser << ":\t"<<r->getZoneRepresentation()<<endl);
-        auto nsec3 = std::dynamic_pointer_cast<const NSEC3RecordContent>(r);
+  for (const auto& validset : validrrsets) {
+    VLOG(log, closestEncloser << ": Do have: "<<validset.first.first<<"/"<<DNSRecordContent::NumberToType(validset.first.second)<<endl);
+    if (validset.first.second == QType::NSEC3) {
+      for (const auto& records : validset.second.records) {
+        VLOG(log, closestEncloser << ":\t"<<records->getZoneRepresentation()<<endl);
+        auto nsec3 = std::dynamic_pointer_cast<const NSEC3RecordContent>(records);
         if (!nsec3) {
           continue;
         }
 
-        const DNSName signer = getSigner(v.second.signatures);
-        if (!v.first.first.isPartOf(signer)) {
+        const DNSName signer = getSigner(validset.second.signatures);
+        if (!validset.first.first.isPartOf(signer)) {
           continue;
         }
 
-        string h = getHashFromNSEC3(wildcard, *nsec3, cache);
-        if (h.empty()) {
+        string hash = getHashFromNSEC3(wildcard, *nsec3, cache);
+        if (hash.empty()) {
           return false;
         }
-        VLOG(log, closestEncloser << ":\tWildcard hash: "<<toBase32Hex(h)<<endl);
-        string beginHash=fromBase32Hex(v.first.first.getRawLabels()[0]);
+        VLOG(log, closestEncloser << ":\tWildcard hash: "<<toBase32Hex(hash)<<endl);
+        string beginHash=fromBase32Hex(validset.first.first.getRawLabels()[0]);
         VLOG(log, closestEncloser << ":\tNSEC3 hash: "<<toBase32Hex(beginHash)<<" -> "<<toBase32Hex(nsec3->d_nexthash)<<endl);
 
-        if (beginHash == h) {
+        if (beginHash == hash) {
           VLOG(log, closestEncloser << ":\tWildcard hash matches");
-          if (wildcardExists) {
+          if (wildcardExists != nullptr) {
             *wildcardExists = true;
           }
 
@@ -377,7 +392,7 @@ static bool provesNSEC3NoWildCard(const DNSName& closestEncloser, uint16_t const
              that (original) owner name other than DS RRs, and all RRs below that
              owner name regardless of type.
           */
-          if (qtype != QType::DS && isNSEC3AncestorDelegation(signer, v.first.first, *nsec3)) {
+          if (qtype != QType::DS && isNSEC3AncestorDelegation(signer, validset.first.first, *nsec3)) {
             /* this is an "ancestor delegation" NSEC3 RR */
             VLOG_NO_PREFIX(log, " BUT an ancestor delegation NSEC3 RR can only deny the existence of a DS"<<endl);
             return false;
@@ -391,7 +406,7 @@ static bool provesNSEC3NoWildCard(const DNSName& closestEncloser, uint16_t const
           return false;
         }
 
-        if (isCoveredByNSEC3Hash(h, beginHash, nsec3->d_nexthash)) {
+        if (isCoveredByNSEC3Hash(hash, beginHash, nsec3->d_nexthash)) {
           VLOG(log, closestEncloser << ":\tWildcard hash is covered"<<endl);
           return true;
         }
@@ -418,7 +433,7 @@ dState matchesNSEC(const DNSName& name, uint16_t qtype, const DNSName& nsecOwner
   */
   if (name.isPartOf(owner) && isNSECAncestorDelegation(signer, owner, nsec)) {
     /* this is an "ancestor delegation" NSEC RR */
-    if (!(qtype == QType::DS && name == owner)) {
+    if (qtype != QType::DS || name != owner) {
       VLOG_NO_PREFIX(log, "An ancestor delegation NSEC RR can only deny the existence of a DS"<<endl);
       return dState::NODENIAL;
     }
@@ -480,7 +495,7 @@ dState matchesNSEC(const DNSName& name, uint16_t qtype, const DNSName& nsecOwner
   name does not exist.
 */
 
-dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16_t qtype, bool referralToUnsigned, bool wantsNoDataProof, const OptLog& log, bool needWildcardProof, unsigned int wildcardLabelsCount)
+dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16_t qtype, bool referralToUnsigned, bool wantsNoDataProof, const OptLog& log, bool needWildcardProof, unsigned int wildcardLabelsCount) // NOLINT(readability-function-cognitive-complexity): https://github.com/PowerDNS/pdns/issues/12791
 {
   nsec3HashesCache cache;
   bool nsec3Seen = false;
@@ -488,25 +503,25 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
     throw PDNSException("Invalid wildcard labels count for the validation of a positive answer synthesized from a wildcard");
   }
 
-  for (const auto& v : validrrsets) {
-    VLOG(log, qname << ": Do have: "<<v.first.first<<"/"<<DNSRecordContent::NumberToType(v.first.second)<<endl);
+  for (const auto& validset : validrrsets) {
+    VLOG(log, qname << ": Do have: "<<validset.first.first<<"/"<<DNSRecordContent::NumberToType(validset.first.second)<<endl);
 
-    if (v.first.second==QType::NSEC) {
-      for (const auto& r : v.second.records) {
-        VLOG(log, qname << ":\t"<<r->getZoneRepresentation()<<endl);
+    if (validset.first.second==QType::NSEC) {
+      for (const auto& record : validset.second.records) {
+        VLOG(log, qname << ":\t"<<record->getZoneRepresentation()<<endl);
 
-        if (v.second.signatures.empty()) {
+        if (validset.second.signatures.empty()) {
           continue;
         }
 
-        auto nsec = std::dynamic_pointer_cast<const NSECRecordContent>(r);
+        auto nsec = std::dynamic_pointer_cast<const NSECRecordContent>(record);
         if (!nsec) {
           continue;
         }
 
-        const DNSName owner = getNSECOwnerName(v.first.first, v.second.signatures);
-        const DNSName signer = getSigner(v.second.signatures);
-        if (!v.first.first.isPartOf(signer) || !owner.isPartOf(signer) ) {
+        const DNSName owner = getNSECOwnerName(validset.first.first, validset.second.signatures);
+        const DNSName signer = getSigner(validset.second.signatures);
+        if (!validset.first.first.isPartOf(signer) || !owner.isPartOf(signer) ) {
            continue;
         }
 
@@ -528,7 +543,7 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
         */
         if (qname.isPartOf(owner) && isNSECAncestorDelegation(signer, owner, *nsec)) {
           /* this is an "ancestor delegation" NSEC RR */
-          if (!(qtype == QType::DS && qname == owner)) {
+          if (qtype != QType::DS || qname != owner) {
             VLOG(log, qname << ": An ancestor delegation NSEC RR can only deny the existence of a DS"<<endl);
             return dState::NODENIAL;
           }
@@ -564,7 +579,7 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
           /* we know that the name exists (but this qtype doesn't) so except
              if the answer was generated by a wildcard expansion, no wildcard
              could have matched (rfc4035 section 5.4 bullet 1) */
-          if (needWildcardProof && (!isWildcardExpanded(owner, v.second.signatures) || isWildcardExpandedOntoItself(owner, v.second.signatures))) {
+          if (needWildcardProof && (!isWildcardExpanded(owner, validset.second.signatures) || isWildcardExpandedOntoItself(owner, validset.second.signatures))) {
             needWildcardProof = false;
           }
 
@@ -606,11 +621,9 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
               VLOG_NO_PREFIX(log, "Denies existence of type "<<qname<<"/"<<QType(qtype)<<" by proving that "<<qname<<" is an ENT"<<endl);
               return dState::NXQTYPE;
             }
-            else {
-              /* but for a NXDOMAIN proof, this doesn't make sense! */
-              VLOG_NO_PREFIX(log, "but it tries to deny the existence of "<<qname<<" by proving that "<<qname<<" is an ENT, this does not make sense!"<<endl);
-              return dState::NODENIAL;
-            }
+            /* but for a NXDOMAIN proof, this doesn't make sense! */
+            VLOG_NO_PREFIX(log, "but it tries to deny the existence of "<<qname<<" by proving that "<<qname<<" is an ENT, this does not make sense!"<<endl);
+            return dState::NODENIAL;
           }
 
           if (!needWildcardProof) {
@@ -637,22 +650,22 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
           return dState::NODENIAL;
         }
 
-        VLOG(log, qname << ": Did not deny existence of "<<QType(qtype)<<", "<<v.first.first<<"?="<<qname<<", "<<nsec->isSet(qtype)<<", next: "<<nsec->d_next<<endl);
+        VLOG(log, qname << ": Did not deny existence of "<<QType(qtype)<<", "<<validset.first.first<<"?="<<qname<<", "<<nsec->isSet(qtype)<<", next: "<<nsec->d_next<<endl);
       }
-    } else if(v.first.second==QType::NSEC3) {
-      for (const auto& r : v.second.records) {
-        VLOG(log, qname << ":\t"<<r->getZoneRepresentation()<<endl);
-        auto nsec3 = std::dynamic_pointer_cast<const NSEC3RecordContent>(r);
+    } else if(validset.first.second==QType::NSEC3) {
+      for (const auto& record : validset.second.records) {
+        VLOG(log, qname << ":\t"<<record->getZoneRepresentation()<<endl);
+        auto nsec3 = std::dynamic_pointer_cast<const NSEC3RecordContent>(record);
         if (!nsec3) {
           continue;
         }
 
-        if (v.second.signatures.empty()) {
+        if (validset.second.signatures.empty()) {
           continue;
         }
 
-        const DNSName& hashedOwner = v.first.first;
-        const DNSName signer = getSigner(v.second.signatures);
+        const DNSName& hashedOwner = validset.first.first;
+        const DNSName signer = getSigner(validset.second.signatures);
         if (!hashedOwner.isPartOf(signer)) {
           VLOG(log, qname << ": Owner "<<hashedOwner<<" is not part of the signer "<<signer<<", ignoring"<<endl);
           continue;
@@ -663,19 +676,19 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
           continue;
         }
 
-        string h = getHashFromNSEC3(qname, *nsec3, cache);
-        if (h.empty()) {
+        string hash = getHashFromNSEC3(qname, *nsec3, cache);
+        if (hash.empty()) {
           VLOG(log, qname << ": Unsupported hash, ignoring"<<endl);
           return dState::INSECURE;
         }
 
         nsec3Seen = true;
 
-        VLOG(log, qname << ":\tquery hash: "<<toBase32Hex(h)<<endl);
+        VLOG(log, qname << ":\tquery hash: "<<toBase32Hex(hash)<<endl);
         string beginHash = fromBase32Hex(hashedOwner.getRawLabels()[0]);
 
         // If the name exists, check if the qtype is denied
-        if (beginHash == h) {
+        if (beginHash == hash) {
 
           /* The NSEC3 is either a delegation one, from the parent zone, and
            * must have the NS bit set but not the SOA one, or a regular NSEC3
@@ -741,34 +754,34 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
     */
     VLOG(log, qname << ": Now looking for the closest encloser for "<<qname<<endl);
 
-    while (found == false && closestEncloser.chopOff()) {
+    while (!found && closestEncloser.chopOff()) {
 
-      for(const auto& v : validrrsets) {
-        if(v.first.second==QType::NSEC3) {
-          for(const auto& r : v.second.records) {
-            VLOG(log, qname << ":\t"<<r->getZoneRepresentation()<<endl);
-            auto nsec3 = std::dynamic_pointer_cast<const NSEC3RecordContent>(r);
+      for(const auto& validset : validrrsets) {
+        if(validset.first.second==QType::NSEC3) {
+          for(const auto& record : validset.second.records) {
+            VLOG(log, qname << ":\t"<<record->getZoneRepresentation()<<endl);
+            auto nsec3 = std::dynamic_pointer_cast<const NSEC3RecordContent>(record);
             if (!nsec3) {
               continue;
             }
 
-            const DNSName signer = getSigner(v.second.signatures);
-            if (!v.first.first.isPartOf(signer)) {
-              VLOG(log, qname << ": Owner "<<v.first.first<<" is not part of the signer "<<signer<<", ignoring"<<endl);
+            const DNSName signer = getSigner(validset.second.signatures);
+            if (!validset.first.first.isPartOf(signer)) {
+              VLOG(log, qname << ": Owner "<<validset.first.first<<" is not part of the signer "<<signer<<", ignoring"<<endl);
               continue;
             }
 
-            string h = getHashFromNSEC3(closestEncloser, *nsec3, cache);
-            if (h.empty()) {
+            string hash = getHashFromNSEC3(closestEncloser, *nsec3, cache);
+            if (hash.empty()) {
               return dState::INSECURE;
             }
 
-            string beginHash=fromBase32Hex(v.first.first.getRawLabels()[0]);
+            string beginHash=fromBase32Hex(validset.first.first.getRawLabels()[0]);
 
-            VLOG(log, qname << ": Comparing "<<toBase32Hex(h)<<" ("<<closestEncloser<<") against "<<toBase32Hex(beginHash)<<endl);
-            if (beginHash == h) {
+            VLOG(log, qname << ": Comparing "<<toBase32Hex(hash)<<" ("<<closestEncloser<<") against "<<toBase32Hex(beginHash)<<endl);
+            if (beginHash == hash) {
               /* If the closest encloser is a delegation NS we know nothing about the names in the child zone. */
-              if (isNSEC3AncestorDelegation(signer, v.first.first, *nsec3)) {
+              if (isNSEC3AncestorDelegation(signer, validset.first.first, *nsec3)) {
                 VLOG(log, qname << ": An ancestor delegation NSEC3 RR can only deny the existence of a DS"<<endl);
                 continue;
               }
@@ -794,7 +807,7 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
             }
           }
         }
-        if (found == true) {
+        if (found) {
           break;
         }
       }
@@ -817,7 +830,7 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
   bool nextCloserFound = false;
   bool isOptOut = false;
 
-  if (found == true) {
+  if (found) {
     /* now that we have found the closest (provable) encloser,
        we can construct the next closer (RFC7129 section-5.5) name
        and look for a NSEC3 RR covering it */
@@ -827,29 +840,30 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
       nextCloser.prependRawLabel(qname.getRawLabel(labelIdx - 1));
       VLOG(log, qname << ":Looking for a NSEC3 covering the next closer name "<<nextCloser<<endl);
 
-      for(const auto& v : validrrsets) {
-        if(v.first.second==QType::NSEC3) {
-          for(const auto& r : v.second.records) {
-            VLOG(log, qname << ":\t"<<r->getZoneRepresentation()<<endl);
-            auto nsec3 = std::dynamic_pointer_cast<const NSEC3RecordContent>(r);
-            if(!nsec3)
+      for(const auto& validset : validrrsets) {
+        if(validset.first.second==QType::NSEC3) {
+          for(const auto& record : validset.second.records) {
+            VLOG(log, qname << ":\t"<<record->getZoneRepresentation()<<endl);
+            auto nsec3 = std::dynamic_pointer_cast<const NSEC3RecordContent>(record);
+            if (!nsec3) {
               continue;
+            }
 
-            string h = getHashFromNSEC3(nextCloser, *nsec3, cache);
-            if (h.empty()) {
+            string hash = getHashFromNSEC3(nextCloser, *nsec3, cache);
+            if (hash.empty()) {
               return dState::INSECURE;
             }
 
-            const DNSName signer = getSigner(v.second.signatures);
-            if (!v.first.first.isPartOf(signer)) {
-              VLOG(log, qname << ": Owner "<<v.first.first<<" is not part of the signer "<<signer<<", ignoring"<<endl);
+            const DNSName signer = getSigner(validset.second.signatures);
+            if (!validset.first.first.isPartOf(signer)) {
+              VLOG(log, qname << ": Owner "<<validset.first.first<<" is not part of the signer "<<signer<<", ignoring"<<endl);
               continue;
             }
 
-            string beginHash=fromBase32Hex(v.first.first.getRawLabels()[0]);
+            string beginHash=fromBase32Hex(validset.first.first.getRawLabels()[0]);
 
-            VLOG(log, qname << ": Comparing "<<toBase32Hex(h)<<" against "<<toBase32Hex(beginHash)<<" -> "<<toBase32Hex(nsec3->d_nexthash)<<endl);
-            if (isCoveredByNSEC3Hash(h, beginHash, nsec3->d_nexthash)) {
+            VLOG(log, qname << ": Comparing "<<toBase32Hex(hash)<<" against "<<toBase32Hex(beginHash)<<" -> "<<toBase32Hex(nsec3->d_nexthash)<<endl);
+            if (isCoveredByNSEC3Hash(hash, beginHash, nsec3->d_nexthash)) {
               VLOG(log, qname << ": Denies existence of name "<<qname<<"/"<<QType(qtype));
               nextCloserFound = true;
 
@@ -861,7 +875,7 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
               VLOG_NO_PREFIX(log, endl);
               break;
             }
-            VLOG(log, qname << ": Did not cover us ("<<qname<<"), start="<<v.first.first<<", us="<<toBase32Hex(h)<<", end="<<toBase32Hex(nsec3->d_nexthash)<<endl);
+            VLOG(log, qname << ": Did not cover us ("<<qname<<"), start="<<validset.first.first<<", us="<<toBase32Hex(hash)<<", end="<<toBase32Hex(nsec3->d_nexthash)<<endl);
           }
         }
         if (nextCloserFound) {
@@ -884,12 +898,10 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16
     if (isOptOut) {
       return dState::OPTOUT;
     }
-    else {
-      if (wildcardExists) {
-        return dState::NXQTYPE;
-      }
-      return dState::NXDOMAIN;
+    if (wildcardExists) {
+      return dState::NXQTYPE;
     }
+    return dState::NXDOMAIN;
   }
 
   // There were no valid NSEC(3) records
@@ -959,7 +971,7 @@ vState validateWithKeySet(time_t now, const DNSName& name, const sortedRecords_t
 
     string msg = getMessageForRRSET(name, *signature, toSign, true);
     for (const auto& key : keysMatchingTag) {
-      vState ede;
+      vState ede = vState::Indeterminate;
       bool signIsValid = checkSignatureWithKey(name, now, *signature, *key, msg, ede, log);
       foundKey = true;
 
@@ -1014,7 +1026,9 @@ cspmap_t harvestCSPFromRecs(const vector<DNSRecord>& recs)
   cspmap_t cspmap;
   for(const auto& rec : recs) {
     //        cerr<<"res "<<rec.d_name<<"/"<<rec.d_type<<endl;
-    if(rec.d_type == QType::OPT) continue;
+    if (rec.d_type == QType::OPT) {
+      continue;
+    }
 
     if(rec.d_type == QType::RRSIG) {
       auto rrc = getRR<RRSIGRecordContent>(rec);
@@ -1031,25 +1045,25 @@ cspmap_t harvestCSPFromRecs(const vector<DNSRecord>& recs)
 
 bool getTrustAnchor(const map<DNSName,dsmap_t>& anchors, const DNSName& zone, dsmap_t &res)
 {
-  const auto& it = anchors.find(zone);
+  const auto& iter = anchors.find(zone);
 
-  if (it == anchors.cend()) {
+  if (iter == anchors.cend()) {
     return false;
   }
 
-  res = it->second;
+  res = iter->second;
   return true;
 }
 
 bool haveNegativeTrustAnchor(const map<DNSName,std::string>& negAnchors, const DNSName& zone, std::string& reason)
 {
-  const auto& it = negAnchors.find(zone);
+  const auto& iter = negAnchors.find(zone);
 
-  if (it == negAnchors.cend()) {
+  if (iter == negAnchors.cend()) {
     return false;
   }
 
-  reason = it->second;
+  reason = iter->second;
   return true;
 }
 
@@ -1061,10 +1075,10 @@ vState validateDNSKeysAgainstDS(time_t now, const DNSName& zone, const dsmap_t&
    */
   for (const auto& dsrc : dsmap)
   {
-    auto r = getByTag(tkeys, dsrc.d_tag, dsrc.d_algorithm, log);
+    auto record = getByTag(tkeys, dsrc.d_tag, dsrc.d_algorithm, log);
     // cerr<<"looking at DS with tag "<<dsrc.d_tag<<", algo "<<DNSSECKeeper::algorithm2name(dsrc.d_algorithm)<<", digest "<<std::to_string(dsrc.d_digesttype)<<" for "<<zone<<", got "<<r.size()<<" DNSKEYs for tag"<<endl;
 
-    for (const auto& drc : r)
+    for (const auto& drc : record)
     {
       bool isValid = false;
       bool dsCreated = false;
@@ -1121,9 +1135,7 @@ vState validateDNSKeysAgainstDS(time_t now, const DNSName& zone, const dsmap_t&
           validkeys = tkeys;
           break;
         }
-        else {
-          VLOG(log, zone << ": Validation did not succeed!"<<endl);
-        }
+        VLOG(log, zone << ": Validation did not succeed!"<<endl);
       }
       //        if(validkeys.empty()) cerr<<"did not manage to validate DNSKEY set based on DS-validated KSK, only passing KSK on"<<endl;
     }
@@ -1188,15 +1200,15 @@ vState validateDNSKeysAgainstDS(time_t now, const DNSName& zone, const dsmap_t&
   return vState::Secure;
 }
 
-bool isSupportedDS(const DSRecordContent& ds, const OptLog& log)
+bool isSupportedDS(const DSRecordContent& dsrec, const OptLog& log)
 {
-  if (!DNSCryptoKeyEngine::isAlgorithmSupported(ds.d_algorithm)) {
-    VLOG(log, "Discarding DS "<<ds.d_tag<<" because we don't support algorithm number "<<std::to_string(ds.d_algorithm)<<endl);
+  if (!DNSCryptoKeyEngine::isAlgorithmSupported(dsrec.d_algorithm)) {
+    VLOG(log, "Discarding DS "<<dsrec.d_tag<<" because we don't support algorithm number "<<std::to_string(dsrec.d_algorithm)<<endl);
     return false;
   }
 
-  if (!DNSCryptoKeyEngine::isDigestSupported(ds.d_digesttype)) {
-    VLOG(log, "Discarding DS "<<ds.d_tag<<" because we don't support digest number "<<std::to_string(ds.d_digesttype)<<endl);
+  if (!DNSCryptoKeyEngine::isDigestSupported(dsrec.d_digesttype)) {
+    VLOG(log, "Discarding DS "<<dsrec.d_tag<<" because we don't support digest number "<<std::to_string(dsrec.d_digesttype)<<endl);
     return false;
   }
 
@@ -1211,7 +1223,7 @@ DNSName getSigner(const std::vector<std::shared_ptr<const RRSIGRecordContent> >&
     }
   }
 
-  return DNSName();
+  return {};
 }
 
 const std::string& vStateToString(vState state)
@@ -1220,17 +1232,17 @@ const std::string& vStateToString(vState state)
   return vStates.at(static_cast<size_t>(state));
 }
 
-std::ostream& operator<<(std::ostream &os, const vState d)
+std::ostream& operator<<(std::ostream &ostr, const vState dstate)
 {
-  os<<vStateToString(d);
-  return os;
+  ostr<<vStateToString(dstate);
+  return ostr;
 }
 
-std::ostream& operator<<(std::ostream &os, const dState d)
+std::ostream& operator<<(std::ostream &ostr, const dState dstate)
 {
   static const std::vector<std::string> dStates = {"no denial", "inconclusive", "nxdomain", "nxqtype", "empty non-terminal", "insecure", "opt-out"};
-  os<<dStates.at(static_cast<size_t>(d));
-  return os;
+  ostr<<dStates.at(static_cast<size_t>(dstate));
+  return ostr;
 }
 
 void updateDNSSECValidationState(vState& state, const vState stateUpdate)
@@ -1241,10 +1253,7 @@ void updateDNSSECValidationState(vState& state, const vState stateUpdate)
   else if (stateUpdate == vState::NTA) {
     state = vState::Insecure;
   }
-  else if (vStateIsBogus(stateUpdate)) {
-    state = stateUpdate;
-  }
-  else if (state == vState::Indeterminate) {
+  else if (vStateIsBogus(stateUpdate) || state == vState::Indeterminate) {
     state = stateUpdate;
   }
   else if (stateUpdate == vState::Insecure) {
diff --git a/pdns/validate.hh b/pdns/validate.hh
index 1660d98eac..9ca589572d 100644
--- a/pdns/validate.hh
+++ b/pdns/validate.hh
@@ -43,13 +43,18 @@ inline bool vStateIsBogus(vState state)
 // NSEC(3) results
 enum class dState : uint8_t { NODENIAL, INCONCLUSIVE, NXDOMAIN, NXQTYPE, ENT, INSECURE, OPTOUT};
 
-std::ostream& operator<<(std::ostream &os, const vState d);
-std::ostream& operator<<(std::ostream &os, const dState d);
+std::ostream& operator<<(std::ostream &, vState);
+std::ostream& operator<<(std::ostream &, dState);
 
 class DNSRecordOracle
 {
 public:
-  virtual std::vector<DNSRecord> get(const DNSName& qname, uint16_t qtype)=0;
+  virtual ~DNSRecordOracle() = default;
+  DNSRecordOracle(const DNSRecordOracle&) = default;
+  DNSRecordOracle(DNSRecordOracle&&) = default;
+  DNSRecordOracle& operator=(const DNSRecordOracle&) = default;
+  DNSRecordOracle& operator=(DNSRecordOracle&&) = default;
+  virtual std::vector<DNSRecord> get(const DNSName& qname, uint16_t qtype) = 0;
 };
 
 
@@ -59,37 +64,37 @@ struct ContentSigPair
   vector<shared_ptr<const RRSIGRecordContent>> signatures;
   // ponder adding a validate method that accepts a key
 };
-typedef map<pair<DNSName,uint16_t>, ContentSigPair> cspmap_t;
-typedef std::set<DSRecordContent> dsmap_t;
+using cspmap_t = map<pair<DNSName, uint16_t>, ContentSigPair>;
+using dsmap_t = std::set<DSRecordContent>;
 
 struct sharedDNSKeyRecordContentCompare
 {
-  bool operator() (const shared_ptr<const DNSKEYRecordContent>& a, const shared_ptr<const DNSKEYRecordContent>& b) const
+  bool operator() (const shared_ptr<const DNSKEYRecordContent>& lhs, const shared_ptr<const DNSKEYRecordContent>& rhs) const
   {
-    return *a < *b;
+    return *lhs < *rhs;
   }
 };
 
-typedef set<shared_ptr<const DNSKEYRecordContent>, sharedDNSKeyRecordContentCompare > skeyset_t;
+using skeyset_t = set<shared_ptr<const DNSKEYRecordContent>, sharedDNSKeyRecordContentCompare>;
 
 
-vState validateWithKeySet(time_t now, const DNSName& name, const sortedRecords_t& records, const vector<shared_ptr<const RRSIGRecordContent> >& signatures, const skeyset_t& keys, const OptLog& log, bool validateAllSigs=true);
+vState validateWithKeySet(time_t now, const DNSName& name, const sortedRecords_t& toSign, const vector<shared_ptr<const RRSIGRecordContent> >& signatures, const skeyset_t& keys, const OptLog& log, bool validateAllSigs=true);
 bool isCoveredByNSEC(const DNSName& name, const DNSName& begin, const DNSName& next);
-bool isCoveredByNSEC3Hash(const std::string& h, const std::string& beginHash, const std::string& nextHash);
-bool isCoveredByNSEC3Hash(const DNSName& h, const DNSName& beginHash, const DNSName& nextHash);
+bool isCoveredByNSEC3Hash(const std::string& hash, const std::string& beginHash, const std::string& nextHash);
+bool isCoveredByNSEC3Hash(const DNSName& name, const DNSName& beginHash, const DNSName& nextHash);
 cspmap_t harvestCSPFromRecs(const vector<DNSRecord>& recs);
 bool getTrustAnchor(const map<DNSName,dsmap_t>& anchors, const DNSName& zone, dsmap_t &res);
 bool haveNegativeTrustAnchor(const map<DNSName,std::string>& negAnchors, const DNSName& zone, std::string& reason);
 vState validateDNSKeysAgainstDS(time_t now, const DNSName& zone, const dsmap_t& dsmap, const skeyset_t& tkeys, const sortedRecords_t& toSign, const vector<shared_ptr<const RRSIGRecordContent> >& sigs, skeyset_t& validkeys, const OptLog&);
-dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16_t qtype, bool referralToUnsigned, bool wantsNoDataProof, const OptLog& log = std::nullopt, bool needsWildcardProof=true, unsigned int wildcardLabelsCount=0);
-bool isSupportedDS(const DSRecordContent& ds, const OptLog&);
+dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, uint16_t qtype, bool referralToUnsigned, bool wantsNoDataProof, const OptLog& log = std::nullopt, bool needWildcardProof=true, unsigned int wildcardLabelsCount=0);
+bool isSupportedDS(const DSRecordContent& dsRecordContent, const OptLog&);
 DNSName getSigner(const std::vector<std::shared_ptr<const RRSIGRecordContent> >& signatures);
 bool denialProvesNoDelegation(const DNSName& zone, const std::vector<DNSRecord>& dsrecords);
-bool isRRSIGNotExpired(const time_t now, const RRSIGRecordContent& sig);
-bool isRRSIGIncepted(const time_t now, const RRSIGRecordContent& sig);
+bool isRRSIGNotExpired(time_t now, const RRSIGRecordContent& sig);
+bool isRRSIGIncepted(time_t now, const RRSIGRecordContent& sig);
 bool isWildcardExpanded(unsigned int labelCount, const RRSIGRecordContent& sign);
 bool isWildcardExpandedOntoItself(const DNSName& owner, unsigned int labelCount, const RRSIGRecordContent& sign);
-void updateDNSSECValidationState(vState& state, const vState stateUpdate);
+void updateDNSSECValidationState(vState& state, vState stateUpdate);
 
 dState matchesNSEC(const DNSName& name, uint16_t qtype, const DNSName& nsecOwner, const NSECRecordContent& nsec, const std::vector<std::shared_ptr<const RRSIGRecordContent>>& signatures, const OptLog&);