From: Remi Gacogne Date: Thu, 4 Feb 2021 15:21:13 +0000 (+0100) Subject: rec: Make sure we don't miss insecure cuts, fix several DNSSEC issues X-Git-Tag: rec-4.5.0-beta1~4^2~11 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ce4546389ecd628380e22c5d8939fb6766a5d555;p=thirdparty%2Fpdns.git rec: Make sure we don't miss insecure cuts, fix several DNSSEC issues --- diff --git a/pdns/recursordist/test-syncres_cc5.cc b/pdns/recursordist/test-syncres_cc5.cc index 042dd6eacb..ed076331c9 100644 --- a/pdns/recursordist/test-syncres_cc5.cc +++ b/pdns/recursordist/test-syncres_cc5.cc @@ -932,7 +932,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec_wildcard) BOOST_CHECK_EQUAL(res, RCode::NoError); BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Secure); BOOST_REQUIRE_EQUAL(ret.size(), 4U); - BOOST_CHECK_EQUAL(queriesCount, 6U); + BOOST_CHECK_EQUAL(queriesCount, 7U); /* again, to test the cache */ ret.clear(); @@ -944,7 +944,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec_wildcard) /* check that we applied the lowest TTL, here this is from the NSEC proving that the exact name did not exist */ BOOST_CHECK_LE(rec.d_ttl, 60U); } - BOOST_CHECK_EQUAL(queriesCount, 6U); + BOOST_CHECK_EQUAL(queriesCount, 7U); } BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec_wildcard_proof_before_rrsig) @@ -1054,7 +1054,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec_wildcard_proof_before_rrsig) BOOST_CHECK_EQUAL(res, RCode::NoError); BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Secure); BOOST_REQUIRE_EQUAL(ret.size(), 4U); - BOOST_CHECK_EQUAL(queriesCount, 6U); + BOOST_CHECK_EQUAL(queriesCount, 7U); /* again, to test the cache */ ret.clear(); @@ -1066,7 +1066,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec_wildcard_proof_before_rrsig) /* check that we applied the lowest TTL, here this is from the NSEC proving that the exact name did not exist */ BOOST_CHECK_LE(rec.d_ttl, 60U); } - BOOST_CHECK_EQUAL(queriesCount, 6U); + BOOST_CHECK_EQUAL(queriesCount, 7U); } BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec_nodata_nowildcard) @@ -1534,7 +1534,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec3_wildcard) BOOST_CHECK_EQUAL(res, RCode::NoError); BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Secure); BOOST_REQUIRE_EQUAL(ret.size(), 6U); - BOOST_CHECK_EQUAL(queriesCount, 6U); + BOOST_CHECK_EQUAL(queriesCount, 8U); /* again, to test the cache */ ret.clear(); @@ -1546,7 +1546,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec3_wildcard) /* check that we applied the lowest TTL, here this is from the NSEC3 proving that the exact name did not exist (next closer) */ BOOST_CHECK_LE(rec.d_ttl, 60U); } - BOOST_CHECK_EQUAL(queriesCount, 6U); + BOOST_CHECK_EQUAL(queriesCount, 8U); } BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec3_wildcard_too_many_iterations) @@ -1653,7 +1653,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec3_wildcard_too_many_iterations) BOOST_CHECK_EQUAL(res, RCode::NoError); BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Insecure); BOOST_REQUIRE_EQUAL(ret.size(), 6U); - BOOST_CHECK_EQUAL(queriesCount, 6U); + BOOST_CHECK_EQUAL(queriesCount, 7U); /* again, to test the cache */ ret.clear(); @@ -1661,7 +1661,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec3_wildcard_too_many_iterations) BOOST_CHECK_EQUAL(res, RCode::NoError); BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Insecure); BOOST_REQUIRE_EQUAL(ret.size(), 6U); - BOOST_CHECK_EQUAL(queriesCount, 6U); + BOOST_CHECK_EQUAL(queriesCount, 7U); } BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec_wildcard_missing) @@ -1759,7 +1759,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec_wildcard_missing) BOOST_CHECK_EQUAL(res, RCode::NoError); BOOST_CHECK_EQUAL(sr->getValidationState(), vState::BogusInvalidDenial); BOOST_REQUIRE_EQUAL(ret.size(), 2U); - BOOST_CHECK_EQUAL(queriesCount, 6U); + BOOST_CHECK_EQUAL(queriesCount, 7U); /* again, to test the cache */ ret.clear(); @@ -1767,7 +1767,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec_wildcard_missing) BOOST_CHECK_EQUAL(res, RCode::NoError); BOOST_CHECK_EQUAL(sr->getValidationState(), vState::BogusInvalidDenial); BOOST_REQUIRE_EQUAL(ret.size(), 2U); - BOOST_CHECK_EQUAL(queriesCount, 6U); + BOOST_CHECK_EQUAL(queriesCount, 7U); } BOOST_AUTO_TEST_CASE(test_dnssec_validation_wildcard_expanded_onto_itself) diff --git a/pdns/recursordist/test-syncres_cc6.cc b/pdns/recursordist/test-syncres_cc6.cc index 5f8fb4ef8b..acfa4c5c50 100644 --- a/pdns/recursordist/test-syncres_cc6.cc +++ b/pdns/recursordist/test-syncres_cc6.cc @@ -1281,4 +1281,202 @@ BOOST_AUTO_TEST_CASE(test_dnssec_secure_to_insecure_skipped_cut) BOOST_CHECK_EQUAL(queriesCount, 7U); } +BOOST_AUTO_TEST_CASE(test_dnssec_secure_to_secure_without_ds) +{ + /* the last zone has signatures but the DS has not been added + to the parent zone yet, so it should be insecure */ + std::unique_ptr sr; + initSR(sr, true); + + setDNSSECValidation(sr, DNSSECMode::ValidateAll); + + primeHints(); + const DNSName target("www.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; + + sr->setAsyncCallback([target, targetAddr, &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, LWResult* res, bool* chained) { + queriesCount++; + + if (type == QType::DS) { + if (domain == DNSName("powerdns.com.")) { + setLWResult(res, 0, true, false, true); + addRecordToLW(res, DNSName("com."), QType::SOA, "foo. bar. 2017032800 1800 900 604800 86400", DNSResourceRecord::AUTHORITY, 86400); + addRRSIG(keys, res->d_records, DNSName("com."), 300); + addNSECRecordToLW(domain, DNSName("z") + domain, {QType::NS, QType::RRSIG}, 600, res->d_records); + addRRSIG(keys, res->d_records, DNSName("com."), 300); + return LWResult::Result::Success; + } + else { + return genericDSAndDNSKEYHandler(res, domain, domain, type, keys); + } + } + else if (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 LWResult::Result::Success; + } + else if (ip == ComboAddress("192.0.2.1:53")) { + if (domain == DNSName("com.")) { + setLWResult(res, 0, true, false, true); + addRecordToLW(res, DNSName("com."), QType::NS, "a.gtld-servers.com."); + addRRSIG(keys, res->d_records, DNSName("com."), 300); + addRecordToLW(res, "a.gtld-servers.com.", QType::A, "192.0.2.1", DNSResourceRecord::ADDITIONAL, 3600); + } + else { + setLWResult(res, 0, false, false, true); + addRecordToLW(res, DNSName("powerdns.com."), QType::NS, "ns1.powerdns.com.", DNSResourceRecord::AUTHORITY, 3600); + /* no DS */ + addNSECRecordToLW(DNSName("powerdns.com."), DNSName("z.powerdns.com."), {QType::NS, QType::RRSIG}, 600, res->d_records); + addRRSIG(keys, res->d_records, DNSName("com."), 300); + addRecordToLW(res, "ns1.powerdns.com.", QType::A, "192.0.2.2", DNSResourceRecord::ADDITIONAL, 3600); + } + return LWResult::Result::Success; + } + else if (ip == ComboAddress("192.0.2.2:53")) { + 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); + + return LWResult::Result::Success; + } + } + + return LWResult::Result::Timeout; + }); + + vector ret; + int res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Insecure); + BOOST_REQUIRE_EQUAL(ret.size(), 2U); + BOOST_CHECK(ret[0].d_type == QType::A); + BOOST_CHECK_EQUAL(queriesCount, 6U); + + /* 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(), vState::Insecure); + BOOST_REQUIRE_EQUAL(ret.size(), 2U); + BOOST_CHECK(ret[0].d_type == QType::A); + BOOST_CHECK_EQUAL(queriesCount, 6U); +} + +BOOST_AUTO_TEST_CASE(test_dnssec_secure_to_broken_without_ds) +{ + /* the last zone has INVALID signatures but the DS has not been added + to the parent zone yet, so it should be insecure */ + std::unique_ptr sr; + initSR(sr, true); + + setDNSSECValidation(sr, DNSSECMode::ValidateAll); + + primeHints(); + const DNSName target("www.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; + + sr->setAsyncCallback([target, targetAddr, &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, LWResult* res, bool* chained) { + queriesCount++; + + if (type == QType::DS) { + if (domain == DNSName("powerdns.com.")) { + setLWResult(res, 0, true, false, true); + addRecordToLW(res, DNSName("com."), QType::SOA, "foo. bar. 2017032800 1800 900 604800 86400", DNSResourceRecord::AUTHORITY, 86400); + addRRSIG(keys, res->d_records, DNSName("com."), 300); + addNSECRecordToLW(domain, DNSName("z") + domain, {QType::NS, QType::RRSIG}, 600, res->d_records); + addRRSIG(keys, res->d_records, DNSName("com."), 300); + return LWResult::Result::Success; + } + else { + return genericDSAndDNSKEYHandler(res, domain, domain, type, keys); + } + } + else if (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 LWResult::Result::Success; + } + else if (ip == ComboAddress("192.0.2.1:53")) { + if (domain == DNSName("com.")) { + setLWResult(res, 0, true, false, true); + addRecordToLW(res, DNSName("com."), QType::NS, "a.gtld-servers.com."); + addRRSIG(keys, res->d_records, DNSName("com."), 300); + addRecordToLW(res, "a.gtld-servers.com.", QType::A, "192.0.2.1", DNSResourceRecord::ADDITIONAL, 3600); + } + else { + setLWResult(res, 0, false, false, true); + addRecordToLW(res, DNSName("powerdns.com."), QType::NS, "ns1.powerdns.com.", DNSResourceRecord::AUTHORITY, 3600); + /* no DS */ + addNSECRecordToLW(DNSName("powerdns.com."), DNSName("z.powerdns.com."), {QType::NS, QType::RRSIG}, 600, res->d_records); + addRRSIG(keys, res->d_records, DNSName("com."), 300); + addRecordToLW(res, "ns1.powerdns.com.", QType::A, "192.0.2.2", DNSResourceRecord::ADDITIONAL, 3600); + } + return LWResult::Result::Success; + } + else if (ip == ComboAddress("192.0.2.2:53")) { + 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, /* broken SIG */ true); + + return LWResult::Result::Success; + } + } + + return LWResult::Result::Timeout; + }); + + vector ret; + int res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Insecure); + BOOST_REQUIRE_EQUAL(ret.size(), 2U); + BOOST_CHECK(ret[0].d_type == QType::A); + BOOST_CHECK_EQUAL(queriesCount, 6U); + + /* 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(), vState::Insecure); + BOOST_REQUIRE_EQUAL(ret.size(), 2U); + BOOST_CHECK(ret[0].d_type == QType::A); + BOOST_CHECK_EQUAL(queriesCount, 6U); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/pdns/recursordist/test-syncres_cc8.cc b/pdns/recursordist/test-syncres_cc8.cc index b85431750d..747f787f19 100644 --- a/pdns/recursordist/test-syncres_cc8.cc +++ b/pdns/recursordist/test-syncres_cc8.cc @@ -807,6 +807,60 @@ BOOST_AUTO_TEST_CASE(test_nsec3_insecure_delegation_denial) BOOST_CHECK_EQUAL(denialState, dState::NODENIAL); } +BOOST_AUTO_TEST_CASE(test_nsec3_ent_opt_out) +{ + initSR(); + + testkeysset_t keys; + generateKeyMaterial(DNSName("."), DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys); + + vector records; + + sortedRecords_t recordContents; + vector> signatureContents; + + /* + * RFC 7129 section 5.1: + * A recently discovered corner case (see RFC Errata ID 3441 [Err3441]) + * shows that not only those delegations remain insecure but also the + * empty non-terminal space that is derived from those delegations. + */ + /* + We have a NSEC3 proving that was.here does exist, and a second + one proving that ent.was.here. does not, + There NSEC3 are opt-out, so the result should be insecure (and we don't need + a wildcard proof). + */ + addNSEC3UnhashedRecordToLW(DNSName("was.here."), DNSName("."), "whatever", {}, 600, records, 10, true /* opt out */); + recordContents.insert(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; + + /* it can not be used to deny any RRs below that owner name either */ + /* Add NSEC3 for the next closer */ + recordContents.clear(); + signatureContents.clear(); + records.clear(); + addNSEC3NarrowRecordToLW(DNSName("ent.was.here."), DNSName("."), {QType::RRSIG, QType::NSEC3}, 600, records, 10, true /* opt-out */); + recordContents.insert(records.at(0).d_content); + addRRSIG(keys, records, DNSName("."), 300); + signatureContents.push_back(getRR(records.at(1))); + + pair.records = recordContents; + pair.signatures = signatureContents; + denialMap[std::make_pair(records.at(0).d_name, records.at(0).d_type)] = pair; + + /* Insecure because the opt-out bit is set */ + dState denialState = getDenial(denialMap, DNSName("ent.was.here."), QType::A, false, true); + BOOST_CHECK_EQUAL(denialState, dState::OPTOUT); +} + BOOST_AUTO_TEST_CASE(test_dnssec_rrsig_negcache_validity) { std::unique_ptr sr; diff --git a/pdns/syncres.cc b/pdns/syncres.cc index 721ade584a..93f3b94c3b 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -1448,7 +1448,7 @@ bool SyncRes::doCNAMECacheCheck(const DNSName &qname, const QType qtype, vector< if (!wasAuthZone && shouldValidate() && (wasAuth || wasForwardRecurse) && state == vState::Indeterminate && d_requireAuthData) { /* This means we couldn't figure out the state when this entry was cached */ - vState recordState = getValidationStatus(foundName, signatures, qtype == QType::DS, depth); + vState recordState = getValidationStatus(foundName, !signatures.empty(), qtype == QType::DS, depth); if (recordState == vState::Secure) { LOG(prefix<>& signatures, bool typeIsDS, unsigned int depth) +vState SyncRes::getValidationStatus(const DNSName& name, bool hasSignatures, bool typeIsDS, unsigned int depth) { vState result = vState::Indeterminate; @@ -2610,9 +2610,9 @@ vState SyncRes::getValidationStatus(const DNSName& name, const std::vector labelsToAdd = subdomain.makeRelative(ds).getRawLabels(); @@ -2629,6 +2629,8 @@ vState SyncRes::getValidationStatus(const DNSName& name, const std::vectord_labels; } - // cerr<<"Got an RRSIG for "<d_type)<<" with name '"<d_type)<<" with name '"<d_type, rec.d_place}].signatures.push_back(rrsig); tcache[{rec.d_name, rrsig->d_type, rec.d_place}].signaturesTTL = std::min(tcache[{rec.d_name, rrsig->d_type, rec.d_place}].signaturesTTL, rec.d_ttl); } @@ -3077,7 +3078,7 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr else { bool haveLogged = false; if (isDNAMEAnswer && rec.d_type == QType::CNAME) { - LOG("NO - we already have a DNAME answer for this domain"); + LOG("NO - we already have a DNAME answer for this domain"<empty()) { @@ -3137,7 +3138,7 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr for(tcache_t::iterator i = tcache.begin(); i != tcache.end(); ++i) { - if(i->second.records.empty()) // this happens when we did store signatures, but passed on the records themselves + if (i->second.records.empty()) // this happens when we did store signatures, but passed on the records themselves continue; /* Even if the AA bit is set, additional data cannot be considered @@ -3189,10 +3190,26 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr continue; } + /* + * RFC 6672 section 5.3.1 + * In any response, a signed DNAME RR indicates a non-terminal + * redirection of the query. There might or might not be a server- + * synthesized CNAME in the answer section; if there is, the CNAME will + * never be signed. For a DNSSEC validator, verification of the DNAME + * RR and then that the CNAME was properly synthesized is sufficient + * proof. + * + * We do the synthesis check in processRecords, here we make sure we + * don't validate the CNAME. + */ + if (isDNAMEAnswer && i->first.type == QType::CNAME) { + expectSignature = false; + } + vState recordState = vState::Indeterminate; if (expectSignature && shouldValidate()) { - vState initialState = getValidationStatus(i->first.name, i->second.signatures, i->first.type == QType::DS, depth); + vState initialState = getValidationStatus(i->first.name, !i->second.signatures.empty(), i->first.type == QType::DS, depth); LOG(d_prefix<<": got initial zone status "<first.name<<"|"<first.type)<first.name, i->second.records, i->second.signatures, depth); } else { - /* - * RFC 6672 section 5.3.1 - * In any response, a signed DNAME RR indicates a non-terminal - * redirection of the query. There might or might not be a server- - * synthesized CNAME in the answer section; if there is, the CNAME will - * never be signed. For a DNSSEC validator, verification of the DNAME - * RR and then that the CNAME was properly synthesized is sufficient - * proof. - * - * We do the synthesis check in processRecords, here we make sure we - * don't validate the CNAME. - */ - if (!(isDNAMEAnswer && i->first.type == QType::CNAME)) { - LOG(d_prefix<<"Validating non-additional "<first.type).getName()<<" record for "<first.name<first.name, QType(i->first.type), i->second.records, i->second.signatures); - /* we might have missed a cut (zone cut within the same auth servers), causing the NS query for an Insecure zone to seem Bogus during zone cut determination */ - if (qtype == QType::NS && i->second.signatures.empty() && vStateIsBogus(recordState) && haveExactValidationStatus(i->first.name) && getValidationStatus(i->first.name, i->second.signatures, i->first.type == QType::DS, depth) == vState::Indeterminate) { - recordState = vState::Indeterminate; - } + LOG(d_prefix<<"Validating non-additional "<first.type).getName()<<" record for "<first.name<first.name, QType(i->first.type), i->second.records, i->second.signatures); + /* we might have missed a cut (zone cut within the same auth servers), causing the NS query for an Insecure zone to seem Bogus during zone cut determination */ + if (qtype == QType::NS && i->second.signatures.empty() && vStateIsBogus(recordState) && haveExactValidationStatus(i->first.name) && initialState == vState::Indeterminate) { + recordState = vState::Indeterminate; } } } @@ -3314,13 +3317,13 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr return RCode::NoError; } -void SyncRes::updateDenialValidationState(vState& neValidationState, const DNSName& neName, vState& state, const dState denialState, const dState expectedState, bool allowOptOut) +void SyncRes::updateDenialValidationState(vState& neValidationState, const DNSName& neName, vState& state, const dState denialState, const dState expectedState) { if (denialState == expectedState) { neValidationState = vState::Secure; } else { - if (denialState == dState::OPTOUT && allowOptOut) { + if (denialState == dState::OPTOUT) { LOG(d_prefix<<"OPT-out denial found for "<add(ne); - } + if (!wasVariable()) { + g_negCache->add(ne); + } - if (qname == newauth && qtype == QType::DS) { - /* we are actually done! */ - negindic=true; - nsset.clear(); + if (qname == newauth && qtype == QType::DS) { + /* we are actually done! */ + negindic=true; + nsset.clear(); + } } } } @@ -3589,12 +3614,19 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co ne.d_qtype = qtype; harvestNXRecords(lwr.d_records, ne, d_now.tv_sec, &lowestTTL); - if (state == vState::Secure) { - dState denialState = getDenialValidationState(ne, state, dState::NXQTYPE, false); - updateDenialValidationState(ne.d_validationState, ne.d_name, state, denialState, dState::NXQTYPE, qtype == QType::DS); - } else { + if (vStateIsBogus(state)) { ne.d_validationState = state; } + else { + auto recordState = getValidationStatus(qname, ne.authoritySOA.signatures.empty(), qtype == QType::DS, depth); + if (recordState == vState::Secure) { + dState denialState = getDenialValidationState(ne, dState::NXQTYPE, false); + updateDenialValidationState(ne.d_validationState, ne.d_name, state, denialState, dState::NXQTYPE); + } else { + ne.d_validationState = recordState; + updateValidationState(state, ne.d_validationState); + } + } if (vStateIsBogus(ne.d_validationState)) { lowestTTL = min(lowestTTL, s_maxbogusttl); @@ -3913,7 +3945,7 @@ bool SyncRes::processAnswer(unsigned int depth, LWResult& lwr, const DNSName& qn if (lwr.d_rcode == RCode::NXDomain) { LOG(prefix<& 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); - dState getDenialValidationState(const NegCache::NegCacheEntry& ne, const vState state, const dState expectedState, bool referralToUnsigned); - void updateDenialValidationState(vState& neValidationState, const DNSName& neName, vState& state, const dState denialState, const dState expectedState, bool allowOptOut); + 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); void computeNegCacheValidationStatus(const NegCache::NegCacheEntry& ne, const DNSName& qname, QType qtype, const int res, vState& state, unsigned int depth); vState getTA(const DNSName& zone, dsmap_t& ds); bool haveExactValidationStatus(const DNSName& domain); - - vState getValidationStatus(const DNSName& subdomain, const std::vector>& signatures, bool typeIsDS, unsigned int depth); + vState getValidationStatus(const DNSName& subdomain, bool hasSignatures, bool typeIsDS, unsigned int depth); void updateValidationStatusInCache(const DNSName &qname, QType qt, bool aa, vState newState) const; void initZoneCutsFromTA(const DNSName& from); diff --git a/pdns/validate.cc b/pdns/validate.cc index 9002c17edb..bf37dbe9fe 100644 --- a/pdns/validate.cc +++ b/pdns/validate.cc @@ -485,13 +485,17 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16 throw PDNSException("Invalid wildcard labels count for the validation of a positive answer synthesized from a wildcard"); } - for(const auto& v : validrrsets) { + for (const auto& v : validrrsets) { LOG("Do have: "<getZoneRepresentation()<(r); if (!nsec) { continue; @@ -501,7 +505,7 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16 const DNSName signer = getSigner(v.second.signatures); if (!v.first.first.isPartOf(signer) || !owner.isPartOf(signer) ) { continue; - } + } /* RFC 6840 section 4.1 "Clarifications on Nonexistence Proofs": Ancestor delegation NSEC or NSEC3 RRs MUST NOT be used to assume @@ -616,11 +620,16 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16 LOG("Did not deny existence of "<isSet(qtype)<<", next: "<d_next<getZoneRepresentation()<(r); - if(!nsec3) + if (!nsec3) { + continue; + } + + if (v.second.signatures.empty()) { continue; + } const DNSName signer = getSigner(v.second.signatures); if (!v.first.first.isPartOf(signer)) { @@ -805,10 +814,11 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16 LOG("Denies existence of name "<isOptOut()) { + if (nsec3->isOptOut()) { LOG(" but is opt-out!"); isOptOut = true; } + LOG(endl); break; }