From: Remi Gacogne Date: Thu, 2 Jul 2020 08:31:31 +0000 (+0200) Subject: rec: A ServFail while retrieving DS/DNSKEY records is just that X-Git-Tag: rec-4.4.0-alpha2~18^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e122af1cf073cab4bd0b1b346b6e166b49870d70;p=thirdparty%2Fpdns.git rec: A ServFail while retrieving DS/DNSKEY records is just that Before that commit, failing to get the DS or DNSKEY records needed during validation because of a network issue would trigger a Bogus DNSSEC validation result because validation could not be performed, but that should just be a Server Failure instead. This is especially an issue because the Bogus result would get inserted into the cache and could stay there for as long as 'max-cache-bogus-ttl' seconds. --- diff --git a/pdns/recursordist/test-syncres_cc5.cc b/pdns/recursordist/test-syncres_cc5.cc index 48b566d21b..6d1bfc56ad 100644 --- a/pdns/recursordist/test-syncres_cc5.cc +++ b/pdns/recursordist/test-syncres_cc5.cc @@ -1758,4 +1758,225 @@ BOOST_AUTO_TEST_CASE(test_dnssec_incomplete_cache_zonecut_qm) BOOST_CHECK_EQUAL(queriesCount, 16U); } +BOOST_AUTO_TEST_CASE(test_dnssec_secure_servfail_ds) +{ + std::unique_ptr sr; + initSR(sr, true); + + setDNSSECValidation(sr, DNSSECMode::ValidateAll); + + primeHints(); + const DNSName target("powerdns.com."); + const ComboAddress targetAddr("192.0.2.42"); + testkeysset_t keys; + + 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, keys); + + 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, 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::DS && domain == DNSName("powerdns.com.")) { + /* time out */ + return 0; + } + + if (type == QType::DS || type == QType::DNSKEY) { + 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 1; + } + + 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); + /* do NOT include the DS here */ + //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 1; + } + + 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(keys, 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(keys, res->d_records, auth, 300); + } + else { + setLWResult(res, RCode::NoError, true, false, true); + addRecordToLW(res, domain, QType::A, targetAddr.toString(), DNSResourceRecord::ANSWER, 3600); + addRRSIG(keys, res->d_records, auth, 300, false, boost::none, boost::none, fixedNow); + } + return 1; + } + + return 0; + }); + + vector ret; + try { + sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK(false); + } + catch (const ImmediateServFailException& e) { + BOOST_CHECK(e.reason.find("Server Failure while retrieving DS records for powerdns.com") != string::npos); + } + + /* and a second time to check nothing was cached */ + try { + sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK(false); + } + catch (const ImmediateServFailException& e) { + BOOST_CHECK(e.reason.find("Server Failure while retrieving DS records for powerdns.com") != string::npos); + } +} + +BOOST_AUTO_TEST_CASE(test_dnssec_secure_servfail_dnskey) +{ + std::unique_ptr sr; + initSR(sr, true); + + setDNSSECValidation(sr, DNSSECMode::ValidateAll); + + primeHints(); + const DNSName target("powerdns.com."); + const ComboAddress targetAddr("192.0.2.42"); + testkeysset_t keys; + + 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, keys); + + 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, 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 0; + } + + if (type == QType::DS || type == QType::DNSKEY) { + 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 1; + } + + 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 1; + } + + 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(keys, 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(keys, res->d_records, auth, 300); + } + else { + setLWResult(res, RCode::NoError, true, false, true); + addRecordToLW(res, domain, QType::A, targetAddr.toString(), DNSResourceRecord::ANSWER, 3600); + addRRSIG(keys, res->d_records, auth, 300, false, boost::none, boost::none, fixedNow); + } + return 1; + } + + return 0; + }); + + vector ret; + try { + sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK(false); + } + catch (const ImmediateServFailException& e) { + BOOST_CHECK(e.reason.find("Server Failure while retrieving DNSKEY records for powerdns.com") != string::npos); + } + + /* and a second time to check nothing was cached */ + try { + sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK(false); + } + catch (const ImmediateServFailException& e) { + BOOST_CHECK(e.reason.find("Server Failure while retrieving DNSKEY records for powerdns.com") != string::npos); + } +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/pdns/syncres.cc b/pdns/syncres.cc index 19eaa418a4..313a16cf0c 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -2064,6 +2064,10 @@ vState SyncRes::getDSRecords(const DNSName& zone, dsmap_t& ds, bool taOnly, unsi vState state = Indeterminate; int rcode = doResolve(zone, QType(QType::DS), dsrecords, depth + 1, beenthere, state); + if (rcode == RCode::ServFail) { + throw ImmediateServFailException("Server Failure while retrieving DS records for " + zone.toLogString()); + } + if (rcode == RCode::NoError || (rcode == RCode::NXDomain && !bogusOnNXD)) { uint8_t bestDigestType = 0; @@ -2332,6 +2336,10 @@ vState SyncRes::getDNSKeys(const DNSName& signer, skeyset_t& keys, unsigned int vState state = Indeterminate; int rcode = doResolve(signer, QType(QType::DNSKEY), records, depth + 1, beenthere, state); + if (rcode == RCode::ServFail) { + throw ImmediateServFailException("Server Failure while retrieving DNSKEY records for " + signer.toLogString()); + } + if (rcode == RCode::NoError) { if (state == Secure) { for (const auto& key : records) {