From 0a9dcd1667b302a78b40b96dad33f13683c3d215 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Tue, 20 Jul 2021 11:40:41 +0200 Subject: [PATCH] rec: Be more strict when validating DS wrt parent/child NSEC(3)s The existing code would not have accepted that proof anyway since the NSEC(3) are signed by a DNSKEY that would require fetching the corresponding DS, which causes a loop that would be detected, but it is cleaner to actually check this. --- pdns/recursordist/test-syncres_cc6.cc | 4 +- pdns/recursordist/test-syncres_cc8.cc | 71 ++++++++++++++++++++++++++- pdns/validate.cc | 13 ++++- 3 files changed, 83 insertions(+), 5 deletions(-) diff --git a/pdns/recursordist/test-syncres_cc6.cc b/pdns/recursordist/test-syncres_cc6.cc index 9810e0161f83..9c5458c8ecf4 100644 --- a/pdns/recursordist/test-syncres_cc6.cc +++ b/pdns/recursordist/test-syncres_cc6.cc @@ -1036,7 +1036,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_secure_to_insecure_optout) 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("com."), 300); /* closest encloser */ - addNSEC3UnhashedRecordToLW(DNSName("com."), DNSName("com."), DNSName("a.com.").toStringNoDot(), {QType::NS}, 600, res->d_records, 10, true); + addNSEC3UnhashedRecordToLW(DNSName("com."), DNSName("com."), DNSName("a.com.").toStringNoDot(), {QType::NS,QType::SOA}, 600, res->d_records, 10, true); addRRSIG(keys, res->d_records, DNSName("com."), 300); /* next closer */ addNSEC3UnhashedRecordToLW(DNSName("oowerdns.com."), DNSName("com."), DNSName("qowerdns.com.").toStringNoDot(), {QType::NS}, 600, res->d_records, 10, true); @@ -1081,7 +1081,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_secure_to_insecure_optout) addRecordToLW(res, domain, QType::NS, "ns1.powerdns.com.", DNSResourceRecord::AUTHORITY, 3600); /* no DS */ /* closest encloser */ - addNSEC3UnhashedRecordToLW(DNSName("com."), DNSName("com."), DNSName("a.com.").toStringNoDot(), {QType::NS}, 600, res->d_records, 10, true); + addNSEC3UnhashedRecordToLW(DNSName("com."), DNSName("com."), DNSName("a.com.").toStringNoDot(), {QType::NS,QType::SOA}, 600, res->d_records, 10, true); addRRSIG(keys, res->d_records, DNSName("com."), 300); /* next closer */ addNSEC3UnhashedRecordToLW(DNSName("oowerdns.com."), DNSName("com."), DNSName("qowerdns.com.").toStringNoDot(), {QType::NS}, 600, res->d_records, 10, true); diff --git a/pdns/recursordist/test-syncres_cc8.cc b/pdns/recursordist/test-syncres_cc8.cc index 747f787f1913..9cfee574e5bc 100644 --- a/pdns/recursordist/test-syncres_cc8.cc +++ b/pdns/recursordist/test-syncres_cc8.cc @@ -223,7 +223,8 @@ BOOST_AUTO_TEST_CASE(test_nsec_ancestor_nxqtype_denial) The RRSIG from "." denies the existence of any type except NS at a. However since it's an ancestor delegation NSEC (NS bit set, SOA bit clear, signer field that is shorter than the owner name of the NSEC RR) it can't - be used to deny anything except the whole name or a DS. + be used to deny anything except the whole name (which does not make sense here) + or a DS. */ addNSECRecordToLW(DNSName("a."), DNSName("b."), {QType::NS}, 600, records); recordContents.insert(records.at(0).d_content); @@ -244,7 +245,7 @@ BOOST_AUTO_TEST_CASE(test_nsec_ancestor_nxqtype_denial) owner name regardless of type. */ - dState denialState = getDenial(denialMap, DNSName("a."), QType::A, false, false); + dState denialState = getDenial(denialMap, DNSName("a."), QType::A, false, true); /* no data means the qname/qtype is not denied, because an ancestor delegation NSEC can only deny the DS */ BOOST_CHECK_EQUAL(denialState, dState::NODENIAL); @@ -257,6 +258,38 @@ BOOST_AUTO_TEST_CASE(test_nsec_ancestor_nxqtype_denial) BOOST_CHECK_EQUAL(denialState, dState::NXQTYPE); } +BOOST_AUTO_TEST_CASE(test_nsec_ds_denial_from_child) +{ + initSR(); + + testkeysset_t keys; + generateKeyMaterial(DNSName("org."), DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys); + generateKeyMaterial(DNSName("example.org."), DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys); + + vector records; + + sortedRecords_t recordContents; + vector> signatureContents; + + addNSECRecordToLW(DNSName("example.org."), DNSName("a.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("example.org."), QType::NSEC)] = pair; + + /* check that this NSEC from the child zone can deny a AAAA at the apex */ + BOOST_CHECK_EQUAL(getDenial(denialMap, DNSName("example.org."), QType::AAAA, false, true, true), dState::NXQTYPE); + + /* but not that the DS does not exist, since we need the parent for that */ + BOOST_CHECK_EQUAL(getDenial(denialMap, DNSName("example.org."), QType::DS, false, true, true), dState::NODENIAL); +} + BOOST_AUTO_TEST_CASE(test_nsec_insecure_delegation_denial) { initSR(); @@ -328,6 +361,36 @@ BOOST_AUTO_TEST_CASE(test_nsec_nxqtype_cname) BOOST_CHECK_EQUAL(denialState, dState::NODENIAL); } +BOOST_AUTO_TEST_CASE(test_nsec3_nxqtype_ds) +{ + initSR(); + + testkeysset_t keys; + generateKeyMaterial(DNSName("powerdns.com."), DNSSECKeeper::ECDSA256, DNSSECKeeper::DIGEST_SHA256, keys); + + vector records; + + sortedRecords_t recordContents; + vector> signatureContents; + + addNSEC3UnhashedRecordToLW(DNSName("powerdns.com."), DNSName("powerdns.com."), "whatever", {QType::A}, 600, records); + recordContents.insert(records.at(0).d_content); + addRRSIG(keys, records, DNSName("powerdns.com."), 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(); + + /* this NSEC3 is not valid to deny the DS since it is from the child zone */ + BOOST_CHECK_EQUAL(getDenial(denialMap, DNSName("powerdns.com."), QType::DS, false, true), dState::NODENIAL); + /* AAAA should be fine, though */ + BOOST_CHECK_EQUAL(getDenial(denialMap, DNSName("powerdns.com."), QType::AAAA, false, true), dState::NXQTYPE); +} + BOOST_AUTO_TEST_CASE(test_nsec3_nxqtype_cname) { initSR(); @@ -733,6 +796,10 @@ BOOST_AUTO_TEST_CASE(test_nsec3_ancestor_nxqtype_denial) denialState = getDenial(denialMap, DNSName("sub.a."), QType::A, false, true); BOOST_CHECK_EQUAL(denialState, dState::NODENIAL); + + /* not even the DS! */ + denialState = getDenial(denialMap, DNSName("sub.a."), QType::DS, false, true); + BOOST_CHECK_EQUAL(denialState, dState::NODENIAL); } BOOST_AUTO_TEST_CASE(test_nsec3_denial_too_many_iterations) diff --git a/pdns/validate.cc b/pdns/validate.cc index 6f04090291fc..67631d2204ef 100644 --- a/pdns/validate.cc +++ b/pdns/validate.cc @@ -521,6 +521,11 @@ dState getDenial(const cspmap_t &validrrsets, const DNSName& qname, const uint16 } } + if (qtype == QType::DS && !qname.isRoot() && signer == qname) { + LOG("A NSEC RR from the child zone cannot deny the existence of a DS"<