From: Otto Moerbeek Date: Mon, 5 Feb 2024 11:43:15 +0000 (+0100) Subject: Add a new 'max-ds-per-zone' setting and immediately return BogusNoValidDNSKEY when... X-Git-Tag: rec-4.9.3~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=97b2828abb9275bfc1b1d4588ba43ddcccf4d30e;p=thirdparty%2Fpdns.git Add a new 'max-ds-per-zone' setting and immediately return BogusNoValidDNSKEY when we hit a limit in validateDNSKeysAgainstDS() --- diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc index 93f39d9f1c..57c5995545 100644 --- a/pdns/recursordist/rec-main.cc +++ b/pdns/recursordist/rec-main.cc @@ -1495,6 +1495,7 @@ static int initDNSSEC(Logr::log_t log) g_maxRRSIGsPerRecordToConsider = ::arg().asNum("max-rrsigs-per-record"); g_maxNSEC3sPerRecordToConsider = ::arg().asNum("max-nsec3s-per-record"); g_maxDNSKEYsToConsider = ::arg().asNum("max-dnskeys"); + g_maxDSsToConsider = ::arg().asNum("max-ds-per-zone"); vector nums; bool automatic = true; @@ -3008,6 +3009,7 @@ static void initArgs() ::arg().set("max-nsec3-hash-computations-per-query", "Maximum number of NSEC3 hashes that we are willing to compute during DNSSEC validation, per incoming query") = "600"; ::arg().set("aggressive-cache-max-nsec3-hash-cost", "Maximum estimated NSEC3 cost for a given query to consider aggressive use of the NSEC3 cache") = "150"; ::arg().set("max-dnskeys", "Maximum number of DNSKEYs with the same algorithm and tag to consider when validating a given record") = "2"; + ::arg().set("max-ds-per-zone", "Maximum number of DS records to consider per zone") = "8"; ::arg().set("cpu-map", "Thread to CPU mapping, space separated thread-id=cpu1,cpu2..cpuN pairs") = ""; diff --git a/pdns/recursordist/test-syncres_cc4.cc b/pdns/recursordist/test-syncres_cc4.cc index e6545b73aa..05cfe35a1c 100644 --- a/pdns/recursordist/test-syncres_cc4.cc +++ b/pdns/recursordist/test-syncres_cc4.cc @@ -439,7 +439,6 @@ BOOST_AUTO_TEST_CASE(test_dnssec_rrsig) auto dcke = DNSCryptoKeyEngine::make(DNSSECKeeper::ECDSA256); dcke->create(dcke->getBits()); - // cerr<convertToISC()< sr; + initSR(sr, true); + + setDNSSECValidation(sr, DNSSECMode::Process); + + primeHints(); + const DNSName target("."); + testkeysset_t keys; + + g_maxDSsToConsider = 1; + + auto luaconfsCopy = g_luaconfs.getCopy(); + luaconfsCopy.dsAnchors.clear(); + /* generate more DSs for the zone than we are willing to consider: only the last one will be used to generate DNSKEY records */ + for (size_t idx = 0; idx < (g_maxDSsToConsider + 10U); idx++) { + generateKeyMaterial(g_rootdnsname, DNSSECKeeper::RSASHA512, DNSSECKeeper::DIGEST_SHA384, keys, luaconfsCopy.dsAnchors); + } + g_luaconfs.setState(luaconfsCopy); + + size_t queriesCount = 0; + + sr->setAsyncCallback([target, &queriesCount, keys](const ComboAddress& /* ip */, const DNSName& domain, int type, bool /* doTCP */, bool /* sendRDQuery */, int /* EDNS0Level */, struct timeval* /* now */, boost::optional& /* srcmask */, boost::optional /* context */, LWResult* res, bool* /* chained */) { + queriesCount++; + + if (domain == target && type == QType::NS) { + + setLWResult(res, 0, true, false, true); + char addr[] = "a.root-servers.net."; + for (char idx = 'a'; idx <= 'm'; idx++) { + addr[0] = idx; + addRecordToLW(res, domain, QType::NS, std::string(addr), DNSResourceRecord::ANSWER, 3600); + } + + addRRSIG(keys, res->d_records, domain, 300); + + addRecordToLW(res, "a.root-servers.net.", QType::A, "198.41.0.4", DNSResourceRecord::ADDITIONAL, 3600); + addRecordToLW(res, "a.root-servers.net.", QType::AAAA, "2001:503:ba3e::2:30", DNSResourceRecord::ADDITIONAL, 3600); + + return LWResult::Result::Success; + } + else if (domain == target && type == QType::DNSKEY) { + + setLWResult(res, 0, true, false, true); + + addDNSKEY(keys, domain, 300, res->d_records); + addRRSIG(keys, res->d_records, domain, 300); + + return LWResult::Result::Success; + } + + return LWResult::Result::Timeout; + }); + + /* === with validation enabled === */ + sr->setDNSSECValidationRequested(true); + vector ret; + int res = sr->beginResolve(target, QType(QType::NS), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), vState::BogusNoValidDNSKEY); + /* 13 NS + 1 RRSIG */ + BOOST_REQUIRE_EQUAL(ret.size(), 14U); + BOOST_CHECK_EQUAL(queriesCount, 2U); + + /* again, to test the cache */ + ret.clear(); + res = sr->beginResolve(target, QType(QType::NS), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), vState::BogusNoValidDNSKEY); + BOOST_REQUIRE_EQUAL(ret.size(), 14U); + BOOST_CHECK_EQUAL(queriesCount, 2U); + + g_maxDNSKEYsToConsider = 0; +} + BOOST_AUTO_TEST_CASE(test_dnssec_bogus_too_many_dnskeys) { std::unique_ptr sr; @@ -1039,7 +1114,7 @@ PrivateKey: n7SRA4n6NejhZBWQOhjTaICYSpkTl6plJn1ATFG23FI=)PKEY"); vector ret; int res = sr->beginResolve(target, QType(QType::NS), QClass::IN, ret); BOOST_CHECK_EQUAL(res, RCode::NoError); - BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Insecure); + BOOST_CHECK_EQUAL(sr->getValidationState(), vState::BogusNoValidDNSKEY); /* 13 NS + 1 RRSIG */ BOOST_REQUIRE_EQUAL(ret.size(), 14U); BOOST_CHECK_EQUAL(queriesCount, 2U); @@ -1048,7 +1123,7 @@ PrivateKey: n7SRA4n6NejhZBWQOhjTaICYSpkTl6plJn1ATFG23FI=)PKEY"); ret.clear(); res = sr->beginResolve(target, QType(QType::NS), QClass::IN, ret); BOOST_CHECK_EQUAL(res, RCode::NoError); - BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Insecure); + BOOST_CHECK_EQUAL(sr->getValidationState(), vState::BogusNoValidDNSKEY); BOOST_REQUIRE_EQUAL(ret.size(), 14U); BOOST_CHECK_EQUAL(queriesCount, 2U); diff --git a/pdns/validate.cc b/pdns/validate.cc index 7d4c8e2a6c..52cc4b7ac1 100644 --- a/pdns/validate.cc +++ b/pdns/validate.cc @@ -33,6 +33,7 @@ uint16_t g_maxNSEC3Iterations{0}; uint16_t g_maxRRSIGsPerRecordToConsider{0}; uint16_t g_maxNSEC3sPerRecordToConsider{0}; uint16_t g_maxDNSKEYsToConsider{0}; +uint16_t g_maxDSsToConsider{0}; static bool isAZoneKey(const DNSKEYRecordContent& key) { @@ -1154,7 +1155,15 @@ vState validateDNSKeysAgainstDS(time_t now, const DNSName& zone, const dsmap_t& * Check all DNSKEY records against all DS records and place all DNSKEY records * that have DS records (that we support the algo for) in the tentative key storage */ + uint16_t dssConsidered = 0; for (const auto& dsrc : dsmap) { + if (g_maxDSsToConsider > 0 && dssConsidered > g_maxDSsToConsider) { + VLOG(log, zone << ": We have already considered "< 0 && dnskeysConsidered >= g_maxDNSKEYsToConsider) { + VLOG(log, zone << ": We have already considered "< 0 && signaturesConsidered >= g_maxRRSIGsPerRecordToConsider) { VLOG(log, zone << ": We have already considered "< 0 && dnskeysConsidered >= g_maxDNSKEYsToConsider) { VLOG(log, zone << ": We have already considered "<d_tag)<<" and algorithm "<d_algorithm)<<", not considering the remaining ones for this signature"< 0 && signaturesConsidered >= g_maxRRSIGsPerRecordToConsider) { VLOG(log, zone << ": We have already considered "<