From: Otto Moerbeek Date: Wed, 31 Aug 2022 08:34:18 +0000 (+0200) Subject: Failure to retrieve DNSKEYs of an Insecure zone should not be fatal. X-Git-Tag: rec-4.6.4~2^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=refs%2Fpull%2F11941%2Fhead;p=thirdparty%2Fpdns.git Failure to retrieve DNSKEYs of an Insecure zone should not be fatal. This issue happens if a record set is signed even though the zone itself is Insecure. Syncres then tries to retrieve DNSKEYs and a timeout on that would lead to an ImmediateServFailException. Only throw exception later in validateRecordsWithSigs, after checking zone cuts, when we are sure the zone is Secure. (cherry picked from commit 6dc8b0b2c6fb2e628356f8dc5c5de4dfd919ec5d) --- diff --git a/pdns/recursordist/test-syncres_cc5.cc b/pdns/recursordist/test-syncres_cc5.cc index b6230b98e2..1af1d87196 100644 --- a/pdns/recursordist/test-syncres_cc5.cc +++ b/pdns/recursordist/test-syncres_cc5.cc @@ -2179,12 +2179,12 @@ BOOST_AUTO_TEST_CASE(test_dnssec_secure_servfail_ds) } } -BOOST_AUTO_TEST_CASE(test_dnssec_secure_servfail_dnskey) +static void dnssec_secure_servfail_dnskey(DNSSECMode mode, vState expectedValidationResult) { std::unique_ptr sr; initSR(sr, true); - setDNSSECValidation(sr, DNSSECMode::ValidateAll); + setDNSSECValidation(sr, mode); primeHints(); const DNSName target("powerdns.com."); @@ -2289,4 +2289,121 @@ BOOST_AUTO_TEST_CASE(test_dnssec_secure_servfail_dnskey) } } +BOOST_AUTO_TEST_CASE(test_dnssec_secure_servfail_dnskey) +{ + dnssec_secure_servfail_dnskey(DNSSECMode::ValidateAll, vState::Indeterminate); + dnssec_secure_servfail_dnskey(DNSSECMode::Off, vState::Indeterminate); +} + +// Same test as above but powerdns.com is now Insecure according to parent, so failure to retrieve DNSSKEYs +// should be mostly harmless. +static void dnssec_secure_servfail_dnskey_insecure(DNSSECMode mode, vState expectedValidationResult) +{ + std::unique_ptr sr; + initSR(sr, true); + + setDNSSECValidation(sr, mode); + + primeHints(); + const DNSName target("powerdns.com."); + const ComboAddress targetAddr("192.0.2.42"); + + // We use two sets of keys, as powerdns.com is Insecure according to parent but returns signed results, + // triggering a (failing) DNSKEY retrieval. + testkeysset_t keys; + testkeysset_t pdnskeys; + + auto luaconfsCopy = g_luaconfs.getCopy(); + luaconfsCopy.dsAnchors.clear(); + generateKeyMaterial(g_rootdnsname, DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys, luaconfsCopy.dsAnchors); + generateKeyMaterial(DNSName("com."), DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys); + generateKeyMaterial(DNSName("powerdns.com."), DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, pdnskeys); + + g_luaconfs.setState(luaconfsCopy); + + size_t queriesCount = 0; + + /* make sure that the signature inception and validity times are computed + based on the SyncRes time, not the current one, in case the function + takes too long. */ + + const time_t fixedNow = sr->getNow().tv_sec; + + sr->setAsyncCallback([target, targetAddr, &queriesCount, keys, pdnskeys, fixedNow](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++; + + DNSName auth = domain; + if (domain == target) { + auth = DNSName("powerdns.com."); + } + + if (type == QType::DNSKEY && domain == DNSName("powerdns.com.")) { + /* time out */ + return LWResult::Result::Timeout; + } + + if (type == QType::DS || type == QType::DNSKEY) { + // This one does not know about pdnskeys, so it will declare powerdns.com as Insecure + return genericDSAndDNSKEYHandler(res, domain, auth, type, keys, true, fixedNow); + } + + if (isRootServer(ip)) { + setLWResult(res, 0, false, false, true); + addRecordToLW(res, "com.", QType::NS, "a.gtld-servers.com.", DNSResourceRecord::AUTHORITY, 3600); + addDS(DNSName("com."), 300, res->d_records, keys); + addRRSIG(keys, res->d_records, DNSName("."), 300, false, boost::none, boost::none, fixedNow); + addRecordToLW(res, "a.gtld-servers.com.", QType::A, "192.0.2.1", DNSResourceRecord::ADDITIONAL, 3600); + return LWResult::Result::Success; + } + + if (ip == ComboAddress("192.0.2.1:53")) { + if (domain == DNSName("com.")) { + setLWResult(res, 0, true, false, true); + addRecordToLW(res, domain, QType::NS, "a.gtld-servers.com."); + addRRSIG(keys, res->d_records, domain, 300, false, boost::none, boost::none, fixedNow); + addRecordToLW(res, "a.gtld-servers.com.", QType::A, "192.0.2.1", DNSResourceRecord::ADDITIONAL, 3600); + addRRSIG(keys, res->d_records, domain, 300); + } + else { + setLWResult(res, 0, false, false, true); + addRecordToLW(res, auth, QType::NS, "ns1.powerdns.com.", DNSResourceRecord::AUTHORITY, 3600); + addDS(auth, 300, res->d_records, keys); + addRRSIG(keys, res->d_records, DNSName("com."), 300, false, boost::none, boost::none, fixedNow); + addRecordToLW(res, "ns1.powerdns.com.", QType::A, "192.0.2.2", DNSResourceRecord::ADDITIONAL, 3600); + } + return LWResult::Result::Success; + } + + if (ip == ComboAddress("192.0.2.2:53")) { + if (type == QType::NS) { + setLWResult(res, 0, true, false, true); + addRecordToLW(res, domain, QType::NS, "ns1.powerdns.com."); + addRRSIG(pdnskeys, res->d_records, auth, 300, false, boost::none, boost::none, fixedNow); + addRecordToLW(res, "ns1.powerdns.com.", QType::A, "192.0.2.2", DNSResourceRecord::ADDITIONAL, 3600); + addRRSIG(pdnskeys, res->d_records, auth, 300); + } + else { + setLWResult(res, RCode::NoError, true, false, true); + addRecordToLW(res, domain, QType::A, targetAddr.toString(), DNSResourceRecord::ANSWER, 3600); + addRRSIG(pdnskeys, res->d_records, auth, 300, false, boost::none, boost::none, fixedNow); + } + return LWResult::Result::Success; + } + + return LWResult::Result::Timeout; + }); + + vector ret; + auto res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, 0); + BOOST_CHECK_EQUAL(sr->getValidationState(), expectedValidationResult); + BOOST_REQUIRE_EQUAL(ret.size(), 2U); +} + +BOOST_AUTO_TEST_CASE(test_dnssec_secure_servfail_dnskey_insecure) +{ + dnssec_secure_servfail_dnskey_insecure(DNSSECMode::ValidateAll, vState::Insecure); + dnssec_secure_servfail_dnskey_insecure(DNSSECMode::Off, vState::Insecure); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/pdns/syncres.cc b/pdns/syncres.cc index 372b6999b7..389eda7b98 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -2806,7 +2806,7 @@ vState SyncRes::validateDNSKeys(const DNSName& zone, const std::vector records; std::set beenthere; @@ -2818,7 +2818,8 @@ vState SyncRes::getDNSKeys(const DNSName& signer, skeyset_t& keys, unsigned int setCacheOnly(oldCacheOnly); if (rcode == RCode::ServFail) { - throw ImmediateServFailException("Server Failure while retrieving DNSKEY records for " + signer.toLogString()); + servFailOccurred = true; + return vState::BogusUnableToGetDNSKEYs; } if (rcode == RCode::NoError) { @@ -2912,9 +2913,10 @@ vState SyncRes::validateRecordsWithSigs(unsigned int depth, const DNSName& qname } } } + bool servFailOccurred = false; if (state == vState::Secure) { LOG(d_prefix<<"retrieving the DNSKEYs for "<& records, const std::vector >& signatures); vState validateDNSKeys(const DNSName& zone, const std::vector& dnskeys, const std::vector >& signatures, unsigned int depth); - vState getDNSKeys(const DNSName& signer, skeyset_t& keys, unsigned int depth); + vState getDNSKeys(const DNSName& signer, skeyset_t& keys, bool& servFailOccurred, unsigned int depth); dState getDenialValidationState(const NegCache::NegCacheEntry& ne, const dState expectedState, bool referralToUnsigned); void updateDenialValidationState(vState& neValidationState, const DNSName& neName, vState& state, const dState denialState, const dState expectedState, bool isDS, unsigned int depth); void computeNegCacheValidationStatus(const NegCache::NegCacheEntry& ne, const DNSName& qname, QType qtype, const int res, vState& state, unsigned int depth);