From: Otto Moerbeek Date: Tue, 3 Dec 2024 10:57:23 +0000 (+0100) Subject: rec: Skip the current zone when looking for a cut after an invalid DS denial proof X-Git-Tag: rec-5.2.0-rc1~7^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F14943%2Fhead;p=thirdparty%2Fpdns.git rec: Skip the current zone when looking for a cut after an invalid DS denial proof --- diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc index e6b542537f..9a318dfc46 100644 --- a/pdns/recursordist/syncres.cc +++ b/pdns/recursordist/syncres.cc @@ -4116,8 +4116,20 @@ vState SyncRes::validateRecordsWithSigs(unsigned int depth, const string& prefix } LOG(prefix << vStateToString(state) << "!" << endl); + + bool skipThisLevelWhenLookingForMissedCuts = false; + if (name == qname && qtype == QType::DS && (type == QType::NSEC || type == QType::NSEC3)) { + /* so we have a NSEC(3) record likely proving that the DS we were looking for does not exist, + but we cannot validate it: + - if there actually is a cut at this level, we will not be able to validate it anyway + - if there is no cut at this level, the only thing that can save us is a cut above + */ + LOG(prefix << name << ": We are trying to validate a " << type << " record for " << name << " likely proving that the DS we were initially looking for (" << qname << ") does not exist, no need to check a zone cut at this exact level" << endl); + skipThisLevelWhenLookingForMissedCuts = true; + } + /* try again to get the missed cuts, harder this time */ - auto zState = getValidationStatus(name, false, type == QType::DS, depth, prefix); + auto zState = getValidationStatus(name, false, type == QType::DS || skipThisLevelWhenLookingForMissedCuts, depth, prefix); LOG(prefix << name << ": Checking whether we missed a zone cut before returning a Bogus state" << endl); if (zState == vState::Secure) { /* too bad */ diff --git a/pdns/recursordist/test-syncres_cc10.cc b/pdns/recursordist/test-syncres_cc10.cc index fe0738a739..fb1c85cff0 100644 --- a/pdns/recursordist/test-syncres_cc10.cc +++ b/pdns/recursordist/test-syncres_cc10.cc @@ -142,10 +142,10 @@ BOOST_AUTO_TEST_CASE(test_outgoing_v6_only_no_AAAA_in_delegation) BOOST_AUTO_TEST_CASE(test_dnssec_secure_to_insecure_skipped_cut_invalid_ds_denial) { - std::unique_ptr sr; - initSR(sr, true); + std::unique_ptr resolver; + initSR(resolver, true); - setDNSSECValidation(sr, DNSSECMode::ValidateAll); + setDNSSECValidation(resolver, DNSSECMode::ValidateAll); primeHints(); const DNSName target("www.sub.powerdns.com."); @@ -163,7 +163,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_secure_to_insecure_skipped_cut_invalid_ds_denia size_t queriesCount = 0; - sr->setAsyncCallback([&](const ComboAddress& address, const DNSName& domain, int type, bool /* doTCP */, bool /* sendRDQuery */, int /* EDNS0Level */, struct timeval* /* now */, boost::optional& /* srcmask */, const ResolveContext& /* context */, LWResult* res, bool* /* chained */) { + resolver->setAsyncCallback([&](const ComboAddress& address, const DNSName& domain, int type, bool /* doTCP */, bool /* sendRDQuery */, int /* EDNS0Level */, struct timeval* /* now */, boost::optional& /* srcmask */, const ResolveContext& /* context */, LWResult* res, bool* /* chained */) { queriesCount++; if (type == QType::DS) { @@ -237,9 +237,9 @@ BOOST_AUTO_TEST_CASE(test_dnssec_secure_to_insecure_skipped_cut_invalid_ds_denia to get the NS in cache so we don't learn the zone cut before validating */ vector ret; - int res = sr->beginResolve(DNSName("nx.powerdns.com."), QType(QType::A), QClass::IN, ret); + int res = resolver->beginResolve(DNSName("nx.powerdns.com."), QType(QType::A), QClass::IN, ret); BOOST_CHECK_EQUAL(res, RCode::NoError); - BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Insecure); + BOOST_CHECK_EQUAL(resolver->getValidationState(), vState::Insecure); BOOST_REQUIRE_EQUAL(ret.size(), 4U); BOOST_CHECK(ret.at(0).d_type == QType::SOA); BOOST_CHECK(ret.at(1).d_type == QType::RRSIG); @@ -249,18 +249,18 @@ BOOST_AUTO_TEST_CASE(test_dnssec_secure_to_insecure_skipped_cut_invalid_ds_denia /* now we query the sub zone */ ret.clear(); - res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + res = resolver->beginResolve(target, QType(QType::A), QClass::IN, ret); BOOST_CHECK_EQUAL(res, RCode::NoError); - BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Insecure); + BOOST_CHECK_EQUAL(resolver->getValidationState(), vState::Insecure); BOOST_REQUIRE_EQUAL(ret.size(), 2U); BOOST_CHECK(ret[0].d_type == QType::A); BOOST_CHECK_EQUAL(queriesCount, 9U); /* again, to test the cache */ ret.clear(); - res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + res = resolver->beginResolve(target, QType(QType::A), QClass::IN, ret); BOOST_CHECK_EQUAL(res, RCode::NoError); - BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Insecure); + BOOST_CHECK_EQUAL(resolver->getValidationState(), vState::Insecure); BOOST_REQUIRE_EQUAL(ret.size(), 2U); BOOST_CHECK(ret[0].d_type == QType::A); BOOST_CHECK_EQUAL(queriesCount, 9U); @@ -1002,6 +1002,107 @@ BOOST_AUTO_TEST_CASE(test_dnssec_bogus_dnskey_loop) BOOST_CHECK_EQUAL(queriesCount, 5U); } +BOOST_AUTO_TEST_CASE(test_dnssec_bogus_ds_loop) +{ + // Test the case where the RRSIG on the name *and* the RRSIG of the NSEC denying the DS is broken. + // This sends te zone cut code trying extra hard to find a zone cut into an endless recursion. + std::unique_ptr resolver; + initSR(resolver, true, false); + + setDNSSECValidation(resolver, DNSSECMode::ValidateAll); + resolver->setQNameMinimization(); + + primeHints(); + 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); + + /* Generate key material for "powerdns.com." */ + auto dcke = DNSCryptoKeyEngine::make(DNSSECKeeper::ECDSA256); + dcke->create(dcke->getBits()); + DNSSECPrivateKey key; + key.setKey(std::move(dcke), 257); + DSRecordContent drc = makeDSFromDNSKey(DNSName("powerdns.com."), key.getDNSKEY(), DNSSECKeeper::DIGEST_SHA256); + + keys[DNSName("powerdns.com.")] = std::pair(key, drc); + g_luaconfs.setState(luaconfsCopy); + + size_t queriesCount = 0; + + resolver->setAsyncCallback([&](const ComboAddress& address, const DNSName& domain, int type, bool /* doTCP */, bool /* sendRDQuery */, int /* EDNS0Level */, struct timeval* /* now */, boost::optional& /* srcmask */, const ResolveContext& /* context */, LWResult* res, bool* /* chained */) { + queriesCount++; + + if (type == QType::DNSKEY) { + return genericDSAndDNSKEYHandler(res, domain, domain, type, keys); + } + if (type == QType::DS) { + if (domain == DNSName("www.powerdns.com.")) { + auto ret = genericDSAndDNSKEYHandler(res, domain, domain, type, keys, true, boost::none, false, false); + for (auto& rec : res->d_records) { + // We know the NSEC RRSIG for the DS is the only one + if (rec.d_name == DNSName("www.powerdns.com") && rec.d_type == QType::RRSIG) { + auto ptr = getRR(rec); + ((char*)(void*)(ptr->d_signature.data()))[0] ^= 0x42; // NOLINT + } + } + return ret; + } + return genericDSAndDNSKEYHandler(res, domain, domain, type, keys); + } + { + if (isRootServer(address)) { + 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); + addRecordToLW(res, "a.gtld-servers.com.", QType::A, "192.0.2.1", DNSResourceRecord::ADDITIONAL, 3600); + return LWResult::Result::Success; + } + if (address == ComboAddress("192.0.2.1:53")) { + if (domain.isPartOf(DNSName("powerdns.com."))) { + setLWResult(res, 0, false, false, true); + addRecordToLW(res, DNSName("powerdns.com."), QType::NS, "ns1.powerdns.com.", DNSResourceRecord::AUTHORITY, 3600); + addDS(DNSName("powerdns.com."), 300, res->d_records, keys); + addRRSIG(keys, res->d_records, DNSName("."), 300); + addRecordToLW(res, "ns1.powerdns.com.", QType::A, "192.0.2.2", DNSResourceRecord::ADDITIONAL, 3600); + return LWResult::Result::Success; + } + } + else if (address == ComboAddress("192.0.2.2:53")) { + if (domain == DNSName("www.powerdns.com.")) { + setLWResult(res, 0, true, false, true); + addRecordToLW(res, domain, QType::A, targetAddr.toString(), DNSResourceRecord::ANSWER, 3600); + addRRSIG(keys, res->d_records, DNSName("powerdns.com."), 300, true); + return LWResult::Result::Success; + } + } + } + + return LWResult::Result::Timeout; + }); + + vector ret; + int res = resolver->beginResolve(DNSName("www.powerdns.com."), QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(resolver->getValidationState(), vState::BogusNoValidRRSIG); + BOOST_REQUIRE_EQUAL(ret.size(), 2U); + BOOST_CHECK(ret.at(0).d_type == QType::A); + BOOST_CHECK_EQUAL(queriesCount, 6U); + + /* again, to test the cache */ + ret.clear(); + res = resolver->beginResolve(DNSName("www.powerdns.com."), QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(resolver->getValidationState(), vState::BogusNoValidRRSIG); + BOOST_REQUIRE_EQUAL(ret.size(), 2U); + BOOST_CHECK(ret[0].d_type == QType::A); + BOOST_CHECK_EQUAL(queriesCount, 6U); +} + static auto createPID(std::string rem, int tcpsock, uint16_t type, std::string domain, int fd, uint16_t id) { PacketID pid;