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.8.0-alpha1~23^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6dc8b0b2c6fb2e628356f8dc5c5de4dfd919ec5d;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. --- 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 2e37749d3d..17721259ee 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -3774,7 +3774,7 @@ vState SyncRes::validateDNSKeys(const DNSName& zone, const std::vector records; std::set beenthere; @@ -3786,7 +3786,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) { @@ -3880,9 +3881,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);