From 3cef03e9a49e80e7ebf03ad120a7e69a05c35bb8 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Tue, 3 Oct 2017 22:41:12 +0200 Subject: [PATCH] rec: For DS queries, only the cuts down to the parent matter --- pdns/recursordist/test-syncres_cc.cc | 137 ++++++++++++++++++++++++++- pdns/syncres.cc | 8 +- 2 files changed, 139 insertions(+), 6 deletions(-) diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc index 774c1e80a..9961fb138 100644 --- a/pdns/recursordist/test-syncres_cc.cc +++ b/pdns/recursordist/test-syncres_cc.cc @@ -4959,6 +4959,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_ds_sign_loop) { luaconfsCopy.dsAnchors.clear(); generateKeyMaterial(g_rootdnsname, DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys, luaconfsCopy.dsAnchors); generateKeyMaterial(DNSName("com."), DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys); + generateKeyMaterial(DNSName("powerdns.com."), DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys); generateKeyMaterial(DNSName("www.powerdns.com."), DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys); g_luaconfs.setState(luaconfsCopy); @@ -5047,7 +5048,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_ds_sign_loop) { BOOST_CHECK_EQUAL(res, RCode::NoError); BOOST_CHECK_EQUAL(sr->getValidationState(), Bogus); BOOST_REQUIRE_EQUAL(ret.size(), 2); - BOOST_CHECK_EQUAL(queriesCount, 10); + BOOST_CHECK_EQUAL(queriesCount, 9); /* again, to test the cache */ ret.clear(); @@ -5055,7 +5056,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_ds_sign_loop) { BOOST_CHECK_EQUAL(res, RCode::NoError); BOOST_CHECK_EQUAL(sr->getValidationState(), Bogus); BOOST_REQUIRE_EQUAL(ret.size(), 2); - BOOST_CHECK_EQUAL(queriesCount, 10); + BOOST_CHECK_EQUAL(queriesCount, 9); } BOOST_AUTO_TEST_CASE(test_dnssec_no_ds_on_referral_insecure) { @@ -5450,6 +5451,138 @@ BOOST_AUTO_TEST_CASE(test_dnssec_secure_to_insecure) { BOOST_CHECK_EQUAL(queriesCount, 7); } + +BOOST_AUTO_TEST_CASE(test_dnssec_secure_direct_ds) { + /* + Direct DS query: + - parent is secure, zone is secure: DS should be secure + */ + std::unique_ptr sr; + initSR(sr, true); + + setDNSSECValidation(sr, DNSSECMode::ValidateAll); + + primeHints(); + const DNSName target("powerdns.com."); + testkeysset_t keys; + + auto luaconfsCopy = g_luaconfs.getCopy(); + luaconfsCopy.dsAnchors.clear(); + generateKeyMaterial(g_rootdnsname, DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys, luaconfsCopy.dsAnchors); + generateKeyMaterial(DNSName("com."), DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys); + generateKeyMaterial(DNSName("powerdns.com."), DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys); + + 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, std::shared_ptr outgoingLogger, LWResult* res) { + queriesCount++; + + if (type == QType::DS || type == QType::DNSKEY) { + return genericDSAndDNSKEYHandler(res, domain, domain, type, keys); + } + else { + 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); + addRecordToLW(res, "a.gtld-servers.com.", QType::A, "192.0.2.1", DNSResourceRecord::ADDITIONAL, 3600); + return 1; + } + } + + return 0; + }); + + vector ret; + int res = sr->beginResolve(target, QType(QType::DS), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), Secure); + BOOST_REQUIRE_EQUAL(ret.size(), 2); + for (const auto& record : ret) { + BOOST_CHECK(record.d_type == QType::DS || record.d_type == QType::RRSIG); + } + BOOST_CHECK_EQUAL(queriesCount, 4); + + /* again, to test the cache */ + ret.clear(); + res = sr->beginResolve(target, QType(QType::DS), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), Secure); + BOOST_REQUIRE_EQUAL(ret.size(), 2); + for (const auto& record : ret) { + BOOST_CHECK(record.d_type == QType::DS || record.d_type == QType::RRSIG); + } + BOOST_CHECK_EQUAL(queriesCount, 4); +} + +BOOST_AUTO_TEST_CASE(test_dnssec_insecure_direct_ds) { + /* + Direct DS query: + - parent is secure, zone is insecure: DS denial should be secure + */ + std::unique_ptr sr; + initSR(sr, true); + + setDNSSECValidation(sr, DNSSECMode::ValidateAll); + + primeHints(); + const DNSName target("powerdns.com."); + testkeysset_t keys; + + auto luaconfsCopy = g_luaconfs.getCopy(); + luaconfsCopy.dsAnchors.clear(); + generateKeyMaterial(g_rootdnsname, DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys, luaconfsCopy.dsAnchors); + generateKeyMaterial(DNSName("com."), DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys); + + 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, std::shared_ptr outgoingLogger, LWResult* res) { + queriesCount++; + + if (type == QType::DS || type == QType::DNSKEY) { + return genericDSAndDNSKEYHandler(res, domain, domain, type, keys); + } + else { + 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); + addRecordToLW(res, "a.gtld-servers.com.", QType::A, "192.0.2.1", DNSResourceRecord::ADDITIONAL, 3600); + return 1; + } + } + + return 0; + }); + + vector ret; + int res = sr->beginResolve(target, QType(QType::DS), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), Secure); + BOOST_REQUIRE_EQUAL(ret.size(), 4); + for (const auto& record : ret) { + BOOST_CHECK(record.d_type == QType::SOA || record.d_type == QType::NSEC || record.d_type == QType::RRSIG); + } + BOOST_CHECK_EQUAL(queriesCount, 4); + + /* again, to test the cache */ + ret.clear(); + res = sr->beginResolve(target, QType(QType::DS), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), Secure); + BOOST_REQUIRE_EQUAL(ret.size(), 4); + for (const auto& record : ret) { + BOOST_CHECK(record.d_type == QType::SOA || record.d_type == QType::NSEC || record.d_type == QType::RRSIG); + } + BOOST_CHECK_EQUAL(queriesCount, 4); +} + BOOST_AUTO_TEST_CASE(test_dnssec_secure_to_insecure_skipped_cut) { std::unique_ptr sr; initSR(sr, true); diff --git a/pdns/syncres.cc b/pdns/syncres.cc index 14f3dfc73..79253ecbc 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -569,7 +569,8 @@ int SyncRes::doResolve(const DNSName &qname, const QType &qtype, vector& records, const /* even if it was not requested for that request (Process, and neither AD nor DO set), it might be requested at a later time so we need to be careful with the TTL. */ if (validationEnabled() && !signatures.empty()) { - /* if we are validating, we don't want to cache records after their signatures - expires. */ + /* if we are validating, we don't want to cache records after their signatures expire. */ /* records TTL are now TTD, let's add 'now' to the signatures lowest TTL */ lowestTTD = min(lowestTTD, static_cast(signaturesTTL + d_now.tv_sec)); for(const auto& sig : signatures) { if (sig->d_siginception <= d_now.tv_sec && sig->d_sigexpire > d_now.tv_sec) { - // we don't decerement d_sigexpire by 'now' because we actually want a TTD, not a TTL */ + // we don't decrement d_sigexpire by 'now' because we actually want a TTD, not a TTL */ lowestTTD = min(lowestTTD, static_cast(sig->d_sigexpire)); } } -- 2.40.0