Process review comments: check pointer conversions, unify record processing plus...

diff --git a/pdns/rec-lua-conf.cc b/pdns/rec-lua-conf.cc
index 823f9ba94e6a24b53d00047537a837f725217613..92840a5d580f48ae1868cc7f843bab62d6af4589 100644 (file)
--- a/pdns/rec-lua-conf.cc
+++ b/pdns/rec-lua-conf.cc
@@ -446,7 +446,7 @@ void loadRecursorLuaConfig(const std::string& fname, luaConfigDelayedThreads& de
         }
         const map<string, pdns::ZoneMD::Config> nameToVal = {
           {"ignore", pdns::ZoneMD::Config::Ignore},
-          {"valdiate", pdns::ZoneMD::Config::Validate},
+          {"validate", pdns::ZoneMD::Config::Validate},
           {"require", pdns::ZoneMD::Config::Require},
         };
         if (have.count("zonemd")) {

diff --git a/pdns/recursordist/docs/lua-config/ztc.rst b/pdns/recursordist/docs/lua-config/ztc.rst
index d546313353699465690aed8c779301347fc9550f..6cc6275cfc00eb1172cc76409520e3fd1ac50f2e 100644 (file)
--- a/pdns/recursordist/docs/lua-config/ztc.rst
+++ b/pdns/recursordist/docs/lua-config/ztc.rst
@@ -31,7 +31,7 @@ For the ``ZONEMD`` part, if the zone has a ``ZONEMD`` record, the digest of the
 
 For both parts failure of validation will prevent the downloaded zone contents to be inserted into the cache.
 Absence of ``DNSSEC`` records is not considered a failure if the parent zone or negative trust anchor indicate the zone is ``Insecure``.
-Absence ``ZONEMD`` records is not considered a failure.
+Absence of ``ZONEMD`` records is not considered a failure.
 This behaviour can be tuned with the ``zoneToCache`` specific `zonemd`_ and `dnssec`_ settings described below.
 
 
@@ -98,7 +98,7 @@ zonemd
 
 .. versionadded:: 4.7.0
 
-A string. possible values: ``ignore``: ignore ZONEMD records, ``validate``: validate ``ZONEMD`` if present, ``require``: require valid ``ZONEMD`` record to be present.
+A string, possible values: ``ignore``: ignore ZONEMD records, ``validate``: validate ``ZONEMD`` if present, ``require``: require valid ``ZONEMD`` record to be present.
 Default ``validate``.
 
 

diff --git a/pdns/recursordist/rec-zonetocache.cc b/pdns/recursordist/rec-zonetocache.cc
index 2aee2c31913e2cb54bad4253744add94c6a0c2c0..4ee5c6ec265668b5ab57e7c972f5d7a81a9c1bcb 100644 (file)
--- a/pdns/recursordist/rec-zonetocache.cc
+++ b/pdns/recursordist/rec-zonetocache.cc
@@ -325,6 +325,10 @@ void ZoneData::ZoneToCache(const RecZoneToCache::Config& config)
     d_log->info("Multiple sources not yet supported, using first");
   }
 
+  if (config.d_dnssec == pdns::ZoneMD::Config::Require && (g_dnssecmode == DNSSECMode::Off || g_dnssecmode == DNSSECMode::ProcessNoValidate)) {
+    throw PDNSException("ZONEMD DNSSEC validation failure: DNSSEC validation is switched off but required by ZoneToCache");
+  }
+
   // First scan all records collecting info about delegations ans sigs
   // A this moment, we ignore NSEC and NSEC3 records. It is not clear to me yet under which conditions
   // they could be entered in into the (neg)cache.
@@ -348,10 +352,6 @@ void ZoneData::ZoneToCache(const RecZoneToCache::Config& config)
     result = processLines(lines, config, zonemd);
   }
 
-  if (config.d_dnssec == pdns::ZoneMD::Config::Require && g_dnssecmode == DNSSECMode::Off) {
-    throw PDNSException("ZONEMD DNSSEC validation failure: dnssec is switched off but required by ZoneToCache");
-  }
-
   // Validate DNSKEYs and ZONEMD, rest of records are validated on-demand by SyncRes
   if (config.d_dnssec == pdns::ZoneMD::Config::Require || (g_dnssecmode != DNSSECMode::Off && g_dnssecmode != DNSSECMode::ProcessNoValidate && config.d_dnssec != pdns::ZoneMD::Config::Ignore)) {
     size_t zonemdCount;

diff --git a/pdns/zonemd.cc b/pdns/zonemd.cc
index 5a604fb1e2128159c9b8ed5052388493a08be38b..843a68cc77454a8746bdd20b9361213bc0877395 100644 (file)
--- a/pdns/zonemd.cc
+++ b/pdns/zonemd.cc
@@ -11,12 +11,6 @@ void pdns::ZoneMD::readRecords(ZoneParserTNG& zpt)
   DNSResourceRecord dnsResourceRecord;
 
   while (zpt.get(dnsResourceRecord)) {
-    if (!dnsResourceRecord.qname.isPartOf(d_zone) && dnsResourceRecord.qname != d_zone) {
-      continue;
-    }
-    if (dnsResourceRecord.qtype == QType::SOA && d_soaRecordContent) {
-      continue;
-    }
     std::shared_ptr<DNSRecordContent> drc;
     try {
       drc = DNSRecordContent::mastermake(dnsResourceRecord.qtype, QClass::IN, dnsResourceRecord.content);
@@ -29,48 +23,14 @@ void pdns::ZoneMD::readRecords(ZoneParserTNG& zpt)
       std::string err = "Bad record content in record for '" + dnsResourceRecord.qname.toStringNoDot() + "|" + dnsResourceRecord.qtype.toString() + "': " + e.what();
       throw PDNSException(err);
     }
-
-    if (dnsResourceRecord.qname == d_zone) {
-      switch (dnsResourceRecord.qtype) {
-      case QType::SOA:
-        d_soaRecordContent = std::dynamic_pointer_cast<SOARecordContent>(drc);
-        break;
-      case QType::DNSKEY:
-        d_dnskeys.emplace(std::dynamic_pointer_cast<DNSKEYRecordContent>(drc));
-        break;
-      case QType::ZONEMD: {
-        auto zonemd = std::dynamic_pointer_cast<ZONEMDRecordContent>(drc);
-        auto inserted = d_zonemdRecords.insert({pair(zonemd->d_scheme, zonemd->d_hashalgo), {zonemd, false}});
-        if (!inserted.second) {
-          // Mark as duplicate
-          inserted.first->second.duplicate = true;
-        }
-        break;
-      }
-      case QType::RRSIG: {
-        auto rrsig = std::dynamic_pointer_cast<RRSIGRecordContent>(drc);
-        if (rrsig->d_type == QType::NSEC) {
-          d_nsecs.signatures.emplace_back(rrsig);
-        }
-        else if (rrsig->d_type == QType::NSEC3) {
-          d_nsecs3.signatures.emplace_back(rrsig);
-        }
-        d_rrsigs.emplace_back(rrsig);
-        break;
-      }
-      case QType::NSEC:
-        d_nsecs.records.emplace(std::dynamic_pointer_cast<NSECRecordContent>(drc));
-        break;
-      case QType::NSEC3:
-        break;
-      }
-    }
-    if (dnsResourceRecord.qtype == QType::NSEC3) {
-        d_nsecs3.records.emplace(std::dynamic_pointer_cast<NSEC3RecordContent>(drc));
-    }
-    RRSetKey_t key = std::pair(dnsResourceRecord.qname, dnsResourceRecord.qtype);
-    d_resourceRecordSets[key].push_back(drc);
-    d_resourceRecordSetTTLs[key] = dnsResourceRecord.ttl;
+    DNSRecord rec;
+    rec.d_name = dnsResourceRecord.qname;
+    rec.d_content = drc;
+    rec.d_type = dnsResourceRecord.qtype;
+    rec.d_class = dnsResourceRecord.qclass;
+    rec.d_ttl = dnsResourceRecord.ttl;
+    rec.d_clen = dnsResourceRecord.content.length(); // XXX is this correct?
+    readRecord(rec);
   }
 }
 
@@ -92,14 +52,26 @@ void pdns::ZoneMD::readRecord(const DNSRecord& record)
 
   if (record.d_name == d_zone) {
     switch (record.d_type) {
-    case QType::SOA:
+    case QType::SOA: {
       d_soaRecordContent = std::dynamic_pointer_cast<SOARecordContent>(record.d_content);
+      if (d_soaRecordContent == nullptr) {
+        throw PDNSException("Invalid SOA record");
+      }
       break;
-    case QType::DNSKEY:
-      d_dnskeys.emplace(std::dynamic_pointer_cast<DNSKEYRecordContent>(record.d_content));
+    }
+    case QType::DNSKEY: {
+      auto dnskey = std::dynamic_pointer_cast<DNSKEYRecordContent>(record.d_content);
+      if (dnskey == nullptr) {
+        throw PDNSException("Invalid DNSKEY record");
+      }
+      d_dnskeys.emplace(dnskey);
       break;
+    }
     case QType::ZONEMD: {
       auto zonemd = std::dynamic_pointer_cast<ZONEMDRecordContent>(record.d_content);
+      if (zonemd == nullptr) {
+        throw PDNSException("Invalid ZONEMD record");
+      }
       auto inserted = d_zonemdRecords.insert({pair(zonemd->d_scheme, zonemd->d_hashalgo), {zonemd, false}});
       if (!inserted.second) {
         // Mark as duplicate
@@ -109,6 +81,9 @@ void pdns::ZoneMD::readRecord(const DNSRecord& record)
     }
     case QType::RRSIG: {
       auto rrsig = std::dynamic_pointer_cast<RRSIGRecordContent>(record.d_content);
+      if (rrsig == nullptr) {
+        throw PDNSException("Invalid RRSIG record");
+      }
       if (rrsig->d_type == QType::NSEC) {
         d_nsecs.signatures.emplace_back(rrsig);
       }
@@ -118,32 +93,53 @@ void pdns::ZoneMD::readRecord(const DNSRecord& record)
       d_rrsigs.emplace_back(rrsig);
       break;
     }
-    case QType::NSEC:
-      d_nsecs.records.emplace(std::dynamic_pointer_cast<NSECRecordContent>(record.d_content));
+    case QType::NSEC: {
+      auto nsec = std::dynamic_pointer_cast<NSECRecordContent>(record.d_content);
+      if (nsec == nullptr) {
+        throw PDNSException("Invalid NSEC record");
+      }
+      d_nsecs.records.emplace(nsec);
       break;
+    }
     case QType::NSEC3:
       // Handled below
       break;
-    case QType::NSEC3PARAM:
-      auto param = *std::dynamic_pointer_cast<NSEC3PARAMRecordContent>(record.d_content);
+    case QType::NSEC3PARAM: {
+      auto param = std::dynamic_pointer_cast<NSEC3PARAMRecordContent>(record.d_content);
+      if (param == nullptr) {
+        throw PDNSException("Invalid NSEC3PARAM record");
+      }
+      if (g_maxNSEC3Iterations && param->d_iterations > g_maxNSEC3Iterations) {
+        return;
+      }
       d_nsec3label = d_zone;
-      d_nsec3label.prependRawLabel(toBase32Hex(hashQNameWithSalt(param, d_zone)));
+      d_nsec3label.prependRawLabel(toBase32Hex(hashQNameWithSalt(param->d_salt, param->d_iterations, d_zone)));
       // XXX We could filter the collected NSEC3 and their RRSIGs in d_nsecs3 at this point as we now know the right label
       break;
     }
+    }
   }
   if (d_nsec3label.empty() || record.d_name == d_nsec3label) {
     switch (record.d_type) {
-    case QType::NSEC3:
-      d_nsecs3.records.emplace(std::dynamic_pointer_cast<NSEC3RecordContent>(record.d_content));
+    case QType::NSEC3: {
+      auto nsec3 = std::dynamic_pointer_cast<NSEC3RecordContent>(record.d_content);
+      if (nsec3 == nullptr) {
+        throw PDNSException("Invalid NSEC3 record");
+      }
+      d_nsecs3.records.emplace(nsec3);
       break;
-    case  QType::RRSIG:
+    }
+    case QType::RRSIG: {
       auto rrsig = std::dynamic_pointer_cast<RRSIGRecordContent>(record.d_content);
+      if (rrsig == nullptr) {
+        throw PDNSException("Invalid RRSIG record");
+      }
       if (rrsig->d_type == QType::NSEC3) {
         d_nsecs3.signatures.emplace_back(rrsig);
       }
       break;
     }
+    }
   }
   RRSetKey_t key = std::pair(record.d_name, record.d_type);
   d_resourceRecordSets[key].push_back(record.d_content);