From 0cbcfeda96273dd8464325b2709e989be0d99535 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Wed, 30 Dec 2020 18:27:17 +0100 Subject: [PATCH] rec: Better aggressive NSEC/NSEC3. Needs tests, refactoring, perhaps wildcard synth --- pdns/dnsrecords.hh | 4 + pdns/recursordist/aggressive_nsec.cc | 66 +++++++++++- pdns/recursordist/test-syncres_cc8.cc | 148 +++++++++++++++++++++++++- pdns/syncres.cc | 3 - pdns/validate.cc | 18 ++-- pdns/validate.hh | 2 + 6 files changed, 223 insertions(+), 18 deletions(-) diff --git a/pdns/dnsrecords.hh b/pdns/dnsrecords.hh index 551d421b30..588a96670a 100644 --- a/pdns/dnsrecords.hh +++ b/pdns/dnsrecords.hh @@ -726,6 +726,10 @@ public: { return d_bitmap.count(); } + bool isOptOut() const + { + return d_flags & 1; + } private: NSECBitmap d_bitmap; diff --git a/pdns/recursordist/aggressive_nsec.cc b/pdns/recursordist/aggressive_nsec.cc index 068ad27cb5..984b627e8b 100644 --- a/pdns/recursordist/aggressive_nsec.cc +++ b/pdns/recursordist/aggressive_nsec.cc @@ -71,6 +71,10 @@ std::shared_ptr AggressiveNSECCache::getZone(con void AggressiveNSECCache::insertNSEC(const DNSName& zone, const DNSName& owner, const DNSRecord& record, const std::vector>& signatures, bool nsec3) { + if (signatures.empty()) { + return; + } + std::shared_ptr entry = getZone(zone); { std::lock_guard lock(entry->d_lock); @@ -86,23 +90,41 @@ void AggressiveNSECCache::insertNSEC(const DNSName& zone, const DNSName& owner, throw std::runtime_error("Error getting the content from a NSEC record"); } next = content->d_next; + if (next.canonCompare(owner) && next != zone) { + /* not accepting a NSEC whose next domain name is before the owner + unless the next domain name is the apex, sorry */ + return; + } } else { auto content = getRR(record); if (!content) { throw std::runtime_error("Error getting the content from a NSEC3 record"); } - next = DNSName(toBase32Hex(content->d_nexthash)) + zone; + + if (content->isOptOut()) { + /* doesn't prove anything, sorry */ + return; + } + if (g_maxNSEC3Iterations && content->d_iterations > g_maxNSEC3Iterations) { + /* can't use that */ return; } + next = DNSName(toBase32Hex(content->d_nexthash)) + zone; entry->d_iterations = content->d_iterations; entry->d_salt = content->d_salt; } /* the TTL is already a TTD by now */ - entry->d_entries.insert({record.d_content, signatures, owner, std::move(next), record.d_ttl}); + if (!nsec3 && isWildcardExpanded(owner.countLabels(), signatures.at(0))) { + DNSName realOwner = getNSECOwnerName(owner, signatures); + entry->d_entries.insert({record.d_content, signatures, std::move(realOwner), std::move(next), record.d_ttl}); + } + else { + entry->d_entries.insert({record.d_content, signatures, owner, std::move(next), record.d_ttl}); + } } } @@ -274,12 +296,24 @@ bool AggressiveNSECCache::getNSEC3Denial(time_t now, std::shared_ptrd_zone, doDNSSEC, ret); - addRecordToRRSet(now, exactNSEC3.d_owner, QType::NSEC, exactNSEC3.d_ttd - now, exactNSEC3.d_record, exactNSEC3.d_signatures, doDNSSEC, ret); + addRecordToRRSet(now, exactNSEC3.d_owner, QType::NSEC3, exactNSEC3.d_ttd - now, exactNSEC3.d_record, exactNSEC3.d_signatures, doDNSSEC, ret); return true; } -#warning FIXME opt-out / ENT + #warning FIXME ancestor delegation cerr<<"no direct match, looking for closest encloser"<isSet(QType::CNAME)) { return false; } + + res = RCode::NoError; } else { if (!isCoveredByNSEC3Hash(DNSName(wcHash) + zone, wcEntry.d_owner, wcEntry.d_next)) { cerr<<"no covering record found for the wildcard in aggressive cache"<d_zone, doDNSSEC, ret); @@ -436,8 +473,26 @@ bool AggressiveNSECCache::getDenial(time_t now, const DNSName& name, const QType res = RCode::NXDomain; } else { + auto nsecContent = std::dynamic_pointer_cast(wcEntry.d_record); + denial = matchesNSEC(wc, type.getCode(), wcEntry.d_owner, nsecContent, wcEntry.d_signatures); + if (denial == dState::NODENIAL) { + /* too complicated for now */ + return false; + } + else if (denial == dState::NXQTYPE) { + covered = true; + res = RCode::NoError; + } + else if (denial == dState::NXDOMAIN) { + covered = true; + res = RCode::NXDomain; + } + + if (wcEntry.d_owner != wc) { + needWildcard = true; + } +#if 0 if (wcEntry.d_owner == wc) { - auto nsecContent = std::dynamic_pointer_cast(wcEntry.d_record); if (!nsecContent) { return false; } @@ -459,6 +514,7 @@ bool AggressiveNSECCache::getDenial(time_t now, const DNSName& name, const QType res = RCode::NXDomain; needWildcard = true; } +#endif } } diff --git a/pdns/recursordist/test-syncres_cc8.cc b/pdns/recursordist/test-syncres_cc8.cc index 1dd464c5e7..efc33e9a28 100644 --- a/pdns/recursordist/test-syncres_cc8.cc +++ b/pdns/recursordist/test-syncres_cc8.cc @@ -421,7 +421,151 @@ BOOST_AUTO_TEST_CASE(test_nsec3_nxdomain_denial_missing_wildcard) pair.signatures = signatureContents; denialMap[std::make_pair(records.at(0).d_name, records.at(0).d_type)] = pair; - dState denialState = getDenial(denialMap, DNSName("b.powerdns.com."), QType::A, false, false); + dState denialState = getDenial(denialMap, DNSName("a.powerdns.com."), QType::A, false, false); + BOOST_CHECK_EQUAL(denialState, dState::NODENIAL); +} + +BOOST_AUTO_TEST_CASE(test_nsec_wildcard_with_cname) +{ + initSR(); + + testkeysset_t keys; + generateKeyMaterial(DNSName("example.org."), DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys); + + vector records; + + sortedRecords_t recordContents; + vector> signatureContents; + + /* proves that b.example.com does not exist */ + addNSECRecordToLW(DNSName("a.example.org."), DNSName("d.example.org"), {QType::A, QType::TXT, QType::RRSIG, QType::NSEC}, 600, records); + recordContents.insert(records.at(0).d_content); + addRRSIG(keys, records, DNSName("example.org."), 300); + signatureContents.push_back(getRR(records.at(1))); + records.clear(); + + ContentSigPair pair; + pair.records = recordContents; + pair.signatures = signatureContents; + cspmap_t denialMap; + denialMap[std::make_pair(DNSName("a.example.org."), QType::NSEC)] = pair; + + /* add a NSEC proving that a wildcard exists, without a CNAME type */ + recordContents.clear(); + signatureContents.clear(); + addNSECRecordToLW(DNSName("*.example.org."), DNSName("+.example.org"), {QType::A, QType::TXT, QType::RRSIG, QType::NSEC }, 600, records); + recordContents.insert(records.at(0).d_content); + addRRSIG(keys, records, DNSName("example.org."), 300); + signatureContents.push_back(getRR(records.at(1))); + records.clear(); + + pair.records = recordContents; + pair.signatures = signatureContents; + denialMap[std::make_pair(DNSName("*.example.org."), QType::NSEC)] = pair; + + /* A does exist at the wildcard, AAAA does not */ + dState denialState = getDenial(denialMap, DNSName("b.example.org."), QType::A, false, true); + BOOST_CHECK_EQUAL(denialState, dState::NODENIAL); + + denialState = getDenial(denialMap, DNSName("b.example.org."), QType::AAAA, false, true); + BOOST_CHECK_EQUAL(denialState, dState::NXQTYPE); + + /* now we replace the wildcard by one with a CNAME */ + recordContents.clear(); + signatureContents.clear(); + addNSECRecordToLW(DNSName("*.example.org."), DNSName("+.example.org"), {QType::CNAME, QType::RRSIG, QType::NSEC }, 600, records); + recordContents.insert(records.at(0).d_content); + addRRSIG(keys, records, DNSName("example.org."), 300); + signatureContents.push_back(getRR(records.at(1))); + records.clear(); + + pair.records = recordContents; + pair.signatures = signatureContents; + denialMap[std::make_pair(DNSName("*.example.org."), QType::NSEC)] = pair; + + /* A and AAAA do not exist but we have a CNAME so at the wildcard */ + denialState = getDenial(denialMap, DNSName("b.example.org."), QType::A, false, true); + BOOST_CHECK_EQUAL(denialState, dState::NODENIAL); + + denialState = getDenial(denialMap, DNSName("b.example.org."), QType::AAAA, false, true); + BOOST_CHECK_EQUAL(denialState, dState::NODENIAL); +} + +BOOST_AUTO_TEST_CASE(test_nsec3_wildcard_with_cname) +{ + initSR(); + + testkeysset_t keys; + generateKeyMaterial(DNSName("example.org."), DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys); + + vector records; + + sortedRecords_t recordContents; + vector> signatureContents; + + /* proves that b.example.com does not exist */ + addNSEC3NarrowRecordToLW(DNSName("b.example.org"), DNSName("example.org."), {QType::A, QType::TXT, QType::RRSIG, QType::NSEC3}, 600, records); + recordContents.insert(records.at(0).d_content); + addRRSIG(keys, records, DNSName("example.org."), 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; + + /* Add NSEC3 for the closest encloser */ + recordContents.clear(); + signatureContents.clear(); + records.clear(); + addNSEC3UnhashedRecordToLW(DNSName("example.org."), DNSName("example.org."), "whatever", {QType::A, QType::TXT, QType::RRSIG, QType::NSEC}, 600, records); + recordContents.insert(records.at(0).d_content); + addRRSIG(keys, records, DNSName("example.org."), 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; + + /* add wildcard, without a CNAME type */ + recordContents.clear(); + signatureContents.clear(); + records.clear(); + addNSEC3UnhashedRecordToLW(DNSName("*.example.org."), DNSName("example.org"), "whatever", {QType::A, QType::TXT, QType::RRSIG, QType::NSEC3}, 600, records); + recordContents.insert(records.at(0).d_content); + addRRSIG(keys, records, DNSName("example.org."), 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; + + /* A does exist at the wildcard, AAAA does not */ + dState denialState = getDenial(denialMap, DNSName("b.example.org."), QType::A, false, true); + BOOST_CHECK_EQUAL(denialState, dState::NODENIAL); + + denialState = getDenial(denialMap, DNSName("b.example.org."), QType::AAAA, false, true); + BOOST_CHECK_EQUAL(denialState, dState::NXQTYPE); + + /* now we replace the wildcard by one with a CNAME */ + recordContents.clear(); + signatureContents.clear(); + records.clear(); + addNSEC3UnhashedRecordToLW(DNSName("*.example.org."), DNSName("example.org"), "whatever", {QType::CNAME, QType::RRSIG, QType::NSEC3}, 600, records); + recordContents.insert(records.at(0).d_content); + addRRSIG(keys, records, DNSName("example.org."), 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; + + /* A and AAAA do not exist but we have a CNAME so at the wildcard */ + denialState = getDenial(denialMap, DNSName("b.example.org."), QType::A, false, true); + BOOST_CHECK_EQUAL(denialState, dState::NODENIAL); + + denialState = getDenial(denialMap, DNSName("b.example.org."), QType::AAAA, false, true); BOOST_CHECK_EQUAL(denialState, dState::NODENIAL); } @@ -522,7 +666,7 @@ BOOST_AUTO_TEST_CASE(test_nsec3_ancestor_nxqtype_denial) */ dState denialState = getDenial(denialMap, DNSName("a."), QType::A, false, true); - /* no data means the qname/qtype is not denied, because an ancestor + /* no denial means the qname/qtype is not denied, because an ancestor delegation NSEC3 can only deny the DS */ BOOST_CHECK_EQUAL(denialState, dState::NODENIAL); diff --git a/pdns/syncres.cc b/pdns/syncres.cc index 18a8e81bb3..4052e1f5a3 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -1951,9 +1951,6 @@ bool SyncRes::doCacheCheck(const DNSName &qname, const DNSName& authname, bool w return true; } } - else { - cerr<<"no cache"<& } if (isCoveredByNSEC3Hash(h, beginHash, nsec3->d_nexthash)) { - return !(nsec3->d_flags & 1); + return !(nsec3->isOptOut()); } } } @@ -207,7 +207,8 @@ static bool isWildcardExpandedOntoItself(const DNSName& owner, const std::vector /* if this is a wildcard NSEC, the owner name has been modified to match the name. Make sure we use the original '*' form. */ -static DNSName getNSECOwnerName(const DNSName& initialOwner, const std::vector >& signatures) +#warning we should not need to export this +DNSName getNSECOwnerName(const DNSName& initialOwner, const std::vector >& signatures) { DNSName result = initialOwner; @@ -237,7 +238,8 @@ static bool isNSECAncestorDelegation(const DNSName& signer, const DNSName& owner signer.countLabels() < owner.countLabels(); } -static bool isNSEC3AncestorDelegation(const DNSName& signer, const DNSName& owner, const std::shared_ptr& nsec3) +#warning FIXME: should not be exported +bool isNSEC3AncestorDelegation(const DNSName& signer, const DNSName& owner, const std::shared_ptr& nsec3) { return nsec3->isSet(QType::NS) && !nsec3->isSet(QType::SOA) && @@ -269,7 +271,7 @@ static bool provesNoDataWildCard(const DNSName& qname, const uint16_t qtype, con if (qname.isPartOf(wildcard)) { LOG("\tWildcard matches"); - if (qtype == 0 || !nsec->isSet(qtype)) { + if (qtype == 0 || (!nsec->isSet(qtype) && !nsec->isSet(QType::CNAME))) { LOG(" and proves that the type did not exist"<d_next); - unsigned int commonLabelsCount = commonLabels.countLabels(); + const unsigned int commonLabelsCount = commonLabels.countLabels(); DNSName wildcard(qname); unsigned int wildcardLabelsCount = wildcard.countLabels(); while (wildcard.chopOff() && wildcardLabelsCount >= commonLabelsCount) { DNSName target = g_wildcarddnsname + wildcard; - #warning BUG?? should we decerement wildcardLabelsCount?? + --wildcardLabelsCount; LOG("Comparing owner: "<isSet(qtype)) { + if (qtype == 0 || (!nsec3->isSet(qtype) && !nsec3->isSet(QType::CNAME))) { LOG(" and proves that the type did not exist"<d_flags & 1) { + if ((qtype == QType::DS || qtype == 0) && nsec3->isOptOut()) { LOG(" but is opt-out!"); isOptOut = true; } diff --git a/pdns/validate.hh b/pdns/validate.hh index 7f7df41c6a..434e7ab358 100644 --- a/pdns/validate.hh +++ b/pdns/validate.hh @@ -95,3 +95,5 @@ void updateDNSSECValidationState(vState& state, const vState stateUpdate); dState matchesNSEC(const DNSName& name, uint16_t qtype, const DNSName& nsecOwner, const std::shared_ptr& nsecRecord, const std::vector>& signatures); +bool isNSEC3AncestorDelegation(const DNSName& signer, const DNSName& owner, const std::shared_ptr& nsec3); +DNSName getNSECOwnerName(const DNSName& initialOwner, const std::vector >& signatures); -- 2.47.2