From: Remi Gacogne Date: Mon, 6 Nov 2017 11:27:04 +0000 (+0100) Subject: rec: Fix going Insecure on NSEC3 hashes with too many iterations X-Git-Tag: auth-4.1.0-rc3~26^2 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b7c4061341fb5a7350e442a4253c05c5239eb574;p=pdns rec: Fix going Insecure on NSEC3 hashes with too many iterations --- diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc index c29cff5fd..4d60abd63 100644 --- a/pdns/recursordist/test-syncres_cc.cc +++ b/pdns/recursordist/test-syncres_cc.cc @@ -154,6 +154,7 @@ static void init(bool debug=false) g_dnssecmode = DNSSECMode::Off; g_dnssecLOG = debug; + g_maxNSEC3Iterations = 2500; ::arg().set("version-string", "string reported on version.pdns or version.bind")="PowerDNS Unit Tests"; } @@ -361,19 +362,17 @@ static void addNSEC3RecordToLW(const DNSName& hashedName, const std::string& has records.push_back(rec); } -static void addNSEC3UnhashedRecordToLW(const DNSName& domain, const DNSName& zone, const std::string& next, const std::set& types, uint32_t ttl, std::vector& records) +static void addNSEC3UnhashedRecordToLW(const DNSName& domain, const DNSName& zone, const std::string& next, const std::set& types, uint32_t ttl, std::vector& records, unsigned int iterations=10) { static const std::string salt = "deadbeef"; - static const unsigned int iterations = 10; std::string hashed = hashQNameWithSalt(salt, iterations, domain); addNSEC3RecordToLW(DNSName(toBase32Hex(hashed)) + zone, next, salt, iterations, types, ttl, records); } -static void addNSEC3NarrowRecordToLW(const DNSName& domain, const DNSName& zone, const std::set& types, uint32_t ttl, std::vector& records) +static void addNSEC3NarrowRecordToLW(const DNSName& domain, const DNSName& zone, const std::set& types, uint32_t ttl, std::vector& records, unsigned int iterations=10) { static const std::string salt = "deadbeef"; - static const unsigned int iterations = 10; std::string hashed = hashQNameWithSalt(salt, iterations, domain); std::string hashedNext(hashed); incrementHash(hashedNext); @@ -5060,6 +5059,97 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec3_nodata_nowildcard) { BOOST_CHECK_EQUAL(queriesCount, 6); } +BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec3_nodata_nowildcard_too_many_iterations) { + std::unique_ptr sr; + initSR(sr, true); + + setDNSSECValidation(sr, DNSSECMode::ValidateAll); + + primeHints(); + const DNSName target("www.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) { + if (type == QType::DS && domain == target) { + DNSName auth("com."); + setLWResult(res, 0, true, false, true); + + addRecordToLW(res, auth, QType::SOA, "foo. bar. 2017032800 1800 900 604800 86400", DNSResourceRecord::AUTHORITY, 86400); + addRRSIG(keys, res->d_records, auth, 300); + /* add a NSEC3 denying the DS AND the existence of a cut (no NS) */ + /* first the closest encloser */ + addNSEC3UnhashedRecordToLW(DNSName("com."), auth, "whatever", { QType::A, QType::TXT, QType::RRSIG, QType::NSEC }, 600, res->d_records, g_maxNSEC3Iterations + 100); + addRRSIG(keys, res->d_records, auth, 300); + /* then the next closer */ + addNSEC3NarrowRecordToLW(domain, DNSName("com."), { QType::RRSIG, QType::NSEC }, 600, res->d_records, g_maxNSEC3Iterations + 100); + addRRSIG(keys, res->d_records, auth, 300); + /* a wildcard matches but has no record for this type */ + addNSEC3UnhashedRecordToLW(DNSName("*.com."), DNSName("com."), "whatever", { QType::AAAA, QType::NSEC, QType::RRSIG }, 600, res->d_records, g_maxNSEC3Iterations + 100); + addRRSIG(keys, res->d_records, DNSName("com"), 300, false, boost::none, DNSName("*.com")); + return 1; + } + 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; + } + else if (ip == ComboAddress("192.0.2.1:53")) { + setLWResult(res, 0, true, false, true); + /* no data */ + addRecordToLW(res, DNSName("com."), QType::SOA, "com. com. 2017032301 10800 3600 604800 3600", DNSResourceRecord::AUTHORITY, 3600); + addRRSIG(keys, res->d_records, DNSName("com."), 300); + /* no record for this name */ + /* first the closest encloser */ + addNSEC3UnhashedRecordToLW(DNSName("com."), DNSName("com."), "whatever", { QType::A, QType::TXT, QType::RRSIG, QType::NSEC }, 600, res->d_records, g_maxNSEC3Iterations + 100); + addRRSIG(keys, res->d_records, DNSName("com."), 300); + /* then the next closer */ + addNSEC3NarrowRecordToLW(domain, DNSName("com."), { QType::RRSIG, QType::NSEC }, 600, res->d_records, g_maxNSEC3Iterations + 100); + addRRSIG(keys, res->d_records, DNSName("com."), 300); + /* a wildcard matches but has no record for this type */ + addNSEC3UnhashedRecordToLW(DNSName("*.com."), DNSName("com."), "whatever", { QType::AAAA, QType::NSEC, QType::RRSIG }, 600, res->d_records, g_maxNSEC3Iterations + 100); + addRRSIG(keys, res->d_records, DNSName("com"), 300, false, boost::none, DNSName("*.com")); + return 1; + } + } + + return 0; + }); + + /* we are generating NSEC3 with more iterations than we allow, so we should go Insecure */ + vector ret; + int res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), Insecure); + BOOST_REQUIRE_EQUAL(ret.size(), 8); + BOOST_CHECK_EQUAL(queriesCount, 6); + + /* again, to test the cache */ + ret.clear(); + res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), Insecure); + BOOST_REQUIRE_EQUAL(ret.size(), 8); + BOOST_CHECK_EQUAL(queriesCount, 6); +} + BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec3_wildcard) { std::unique_ptr sr; initSR(sr, true); @@ -5172,6 +5262,120 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec3_wildcard) { BOOST_CHECK_EQUAL(queriesCount, 9); } +BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec3_wildcard_too_many_iterations) { + std::unique_ptr sr; + initSR(sr, true); + + setDNSSECValidation(sr, DNSSECMode::ValidateAll); + + primeHints(); + const DNSName target("www.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) { + if (type == QType::DS && domain == target) { + setLWResult(res, RCode::NoError, true, false, true); + addRecordToLW(res, DNSName("powerdns.com."), QType::SOA, "pdns-public-ns1.powerdns.com. pieter\\.lexis.powerdns.com. 2017032301 10800 3600 604800 3600", DNSResourceRecord::AUTHORITY, 3600); + addRRSIG(keys, res->d_records, DNSName("powerdns.com."), 300); + addNSECRecordToLW(DNSName("www.powerdns.com."), DNSName("wwz.powerdns.com."), { QType::A, QType::NSEC, QType::RRSIG }, 600, res->d_records); + addRRSIG(keys, res->d_records, DNSName("powerdns.com"), 300); + return 1; + } + else { + 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; + } + else 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); + 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, "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("com."), 300); + addRecordToLW(res, "ns1.powerdns.com.", QType::A, "192.0.2.2", DNSResourceRecord::ADDITIONAL, 3600); + } + return 1; + } + else if (ip == ComboAddress("192.0.2.2:53")) { + setLWResult(res, 0, true, false, true); + if (type == QType::NS) { + if (domain == DNSName("powerdns.com.")) { + addRecordToLW(res, domain, QType::NS, "ns1.powerdns.com."); + addRRSIG(keys, res->d_records, DNSName("powerdns.com"), 300); + addRecordToLW(res, "ns1.powerdns.com.", QType::A, "192.0.2.2", DNSResourceRecord::ADDITIONAL, 3600); + addRRSIG(keys, res->d_records, DNSName("powerdns.com"), 300); + } + else { + addRecordToLW(res, domain, QType::SOA, "pdns-public-ns1.powerdns.com. pieter\\.lexis.powerdns.com. 2017032301 10800 3600 604800 3600", DNSResourceRecord::AUTHORITY, 3600); + addRRSIG(keys, res->d_records, DNSName("powerdns.com"), 300); + addNSECRecordToLW(DNSName("www.powerdns.com."), DNSName("wwz.powerdns.com."), { QType::A, QType::NSEC, QType::RRSIG }, 600, res->d_records); + addRRSIG(keys, res->d_records, DNSName("powerdns.com"), 300); + } + } + else { + addRecordToLW(res, domain, QType::A, "192.0.2.42"); + addRRSIG(keys, res->d_records, DNSName("powerdns.com"), 300, false, boost::none, DNSName("*.powerdns.com")); + /* we need to add the proof that this name does not exist, so the wildcard may apply */ + /* first the closest encloser */ + addNSEC3UnhashedRecordToLW(DNSName("powerdns.com."), DNSName("powerdns.com."), "whatever", { QType::A, QType::TXT, QType::RRSIG, QType::NSEC }, 600, res->d_records); + addRRSIG(keys, res->d_records, DNSName("powerdns.com."), 300); + /* then the next closer */ + addNSEC3NarrowRecordToLW(DNSName("www.powerdns.com."), DNSName("powerdns.com."), { QType::A, QType::TXT, QType::RRSIG, QType::NSEC }, 600, res->d_records, g_maxNSEC3Iterations + 100); + addRRSIG(keys, res->d_records, DNSName("powerdns.com."), 300); + } + return 1; + } + } + + return 0; + }); + + /* the NSEC3 providing the denial of existence proof for the next closer has too many iterations, + we should end up Insecure */ + vector ret; + int res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), Insecure); + BOOST_REQUIRE_EQUAL(ret.size(), 6); + BOOST_CHECK_EQUAL(queriesCount, 9); + + /* again, to test the cache */ + ret.clear(); + res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), Insecure); + BOOST_REQUIRE_EQUAL(ret.size(), 6); + BOOST_CHECK_EQUAL(queriesCount, 9); +} + BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec_wildcard_missing) { std::unique_ptr sr; initSR(sr, true); @@ -7682,7 +7886,7 @@ BOOST_AUTO_TEST_CASE(test_nsec_insecure_delegation_denial) { /* Insecure because the NS is not set, so while it does denies the DS, it can't prove an insecure delegation */ dState denialState = getDenial(denialMap, DNSName("a."), QType::DS, true, true); - BOOST_CHECK_EQUAL(denialState, INSECURE); + BOOST_CHECK_EQUAL(denialState, NODATA); } BOOST_AUTO_TEST_CASE(test_nsec_nxqtype_cname) { @@ -7905,6 +8109,35 @@ BOOST_AUTO_TEST_CASE(test_nsec3_ancestor_nxqtype_denial) { BOOST_CHECK_EQUAL(denialState, NXQTYPE); } +BOOST_AUTO_TEST_CASE(test_nsec3_denial_too_many_iterations) { + init(); + + testkeysset_t keys; + generateKeyMaterial(DNSName("."), DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys); + + vector records; + + vector> recordContents; + vector> signatureContents; + + /* adding a NSEC3 with more iterations that we support */ + addNSEC3UnhashedRecordToLW(DNSName("a."), DNSName("."), "whatever", { QType::AAAA }, 600, records, g_maxNSEC3Iterations + 100); + recordContents.push_back(records.at(0).d_content); + addRRSIG(keys, records, DNSName("."), 300); + signatureContents.push_back(getRR(records.at(1))); + + ContentSigPair pair; + pair.records = recordContents; + pair.signatures = signatureContents; + cspmap_t denialMap; + denialMap[std::make_pair(records.at(0).d_name, records.at(0).d_type)] = pair; + records.clear(); + + dState denialState = getDenial(denialMap, DNSName("a."), QType::A, false, true); + /* since we refuse to compute more than g_maxNSEC3Iterations iterations, it should be Insecure */ + BOOST_CHECK_EQUAL(denialState, INSECURE); +} + BOOST_AUTO_TEST_CASE(test_nsec3_insecure_delegation_denial) { init(); @@ -7943,7 +8176,7 @@ BOOST_AUTO_TEST_CASE(test_nsec3_insecure_delegation_denial) { /* Insecure because the NS is not set, so while it does denies the DS, it can't prove an insecure delegation */ dState denialState = getDenial(denialMap, DNSName("a."), QType::DS, true, true); - BOOST_CHECK_EQUAL(denialState, INSECURE); + BOOST_CHECK_EQUAL(denialState, NODATA); } BOOST_AUTO_TEST_CASE(test_dnssec_rrsig_negcache_validity) { diff --git a/pdns/syncres.cc b/pdns/syncres.cc index d79636a5b..a8b27ae06 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -2153,10 +2153,20 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co cspmap_t csp = harvestCSPFromNE(ne); dState res = getDenial(csp, qname, ne.d_qtype.getCode(), false, false, false); if (res != NXDOMAIN) { - LOG(d_prefix<<"Invalid denial in wildcard expanded positive response found for "<d_set.count(QType::NS)) { LOG("However, no NS record exists at this level!"<d_iterations > g_maxNSEC3Iterations) { + string h = getHashFromNSEC3(closestEncloser, nsec3); + if (h.empty()) { return INSECURE; } - string h = hashQNameWithSalt(nsec3->d_salt, nsec3->d_iterations, closestEncloser); string beginHash=fromBase32Hex(v.first.first.getRawLabels()[0]); LOG("Comparing "<(r); if(!nsec3) continue; - if (g_maxNSEC3Iterations && nsec3->d_iterations > g_maxNSEC3Iterations) { + + string h = getHashFromNSEC3(nextCloser, nsec3); + if (h.empty()) { return INSECURE; } - string h = hashQNameWithSalt(nsec3->d_salt, nsec3->d_iterations, nextCloser); string beginHash=fromBase32Hex(v.first.first.getRawLabels()[0]); LOG("Comparing "< "<d_nexthash)<