From: Otto Moerbeek Date: Mon, 24 Jan 2022 12:44:05 +0000 (+0100) Subject: Process review comments: check pointer conversions, unify record processing plus... X-Git-Tag: auth-4.7.0-alpha1~42^2~7 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=94223b01029ed341a01c9069e7a9df42723ec3bc;p=thirdparty%2Fpdns.git Process review comments: check pointer conversions, unify record processing plus assorted small fixes --- diff --git a/pdns/rec-lua-conf.cc b/pdns/rec-lua-conf.cc index 823f9ba94e..92840a5d58 100644 --- 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 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 d546313353..6cc6275cfc 100644 --- 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 2aee2c3191..4ee5c6ec26 100644 --- 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 5a604fb1e2..843a68cc77 100644 --- 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 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(drc); - break; - case QType::DNSKEY: - d_dnskeys.emplace(std::dynamic_pointer_cast(drc)); - break; - case QType::ZONEMD: { - auto zonemd = std::dynamic_pointer_cast(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(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(drc)); - break; - case QType::NSEC3: - break; - } - } - if (dnsResourceRecord.qtype == QType::NSEC3) { - d_nsecs3.records.emplace(std::dynamic_pointer_cast(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(record.d_content); + if (d_soaRecordContent == nullptr) { + throw PDNSException("Invalid SOA record"); + } break; - case QType::DNSKEY: - d_dnskeys.emplace(std::dynamic_pointer_cast(record.d_content)); + } + case QType::DNSKEY: { + auto dnskey = std::dynamic_pointer_cast(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(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(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(record.d_content)); + case QType::NSEC: { + auto nsec = std::dynamic_pointer_cast(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(record.d_content); + case QType::NSEC3PARAM: { + auto param = std::dynamic_pointer_cast(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(record.d_content)); + case QType::NSEC3: { + auto nsec3 = std::dynamic_pointer_cast(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(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);