From: JINMEI Tatuya Date: Tue, 21 Feb 2012 22:44:49 +0000 (-0800) Subject: [1587] extracted cases with recursive findNSEC3 into a unified helper method, X-Git-Tag: trac2351_base~226^2~116^2~183^2~3 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=d121f511463ae2c469ec5ddeac1eb1ad60ca4cd9;p=thirdparty%2Fkea.git [1587] extracted cases with recursive findNSEC3 into a unified helper method, addClosestEncloserProof(). some of failure exceptions are changed, so adjusted the affected tests, too. --- diff --git a/src/bin/auth/query.cc b/src/bin/auth/query.cc index 73681e4674..fcd9396680 100644 --- a/src/bin/auth/query.cc +++ b/src/bin/auth/query.cc @@ -167,28 +167,41 @@ Query::addNXDOMAINProofByNSEC(ZoneFinder& finder, ConstRRsetPtr nsec) { } } +uint8_t +Query::addClosestEncloserProof(ZoneFinder& finder, const Name& name, + bool exact_ok, bool add_closest) +{ + const ZoneFinder::FindNSEC3Result result = finder.findNSEC3(name, true); + if (!exact_ok && !result.next_proof) { + isc_throw(BadNSEC3, "Matching NSEC3 found for a non existent name: " + << qname_); + } + if (add_closest) { + response_.addRRset(Message::SECTION_AUTHORITY, + boost::const_pointer_cast( + result.closest_proof), + dnssec_); + } + if (result.next_proof) { + response_.addRRset(Message::SECTION_AUTHORITY, + boost::const_pointer_cast( + result.next_proof), + dnssec_); + } + return (result.closest_labels); +} + void Query::addNXDOMAINProofByNSEC3(ZoneFinder& finder) { // Firstly get the NSEC3 proves for Closest Encloser Proof // See section 7.2.1 of RFC 5155. - // Since this is a Name Error case both closest and next proofs should - // be available (see addNXRRsetProof). - const ZoneFinder::FindNSEC3Result fresult1 = finder.findNSEC3(qname_, - true); - response_.addRRset(Message::SECTION_AUTHORITY, - boost::const_pointer_cast( - fresult1.closest_proof), - dnssec_); - response_.addRRset(Message::SECTION_AUTHORITY, - boost::const_pointer_cast( - fresult1.next_proof), - dnssec_); + const uint8_t closest_labels = + addClosestEncloserProof(finder, qname_, false); // Next, construct the wildcard name at the closest encloser, i.e., // '*' followed by the closest encloser, and add NSEC3 for it. const Name wildname(Name("*").concatenate( - qname_.split(qname_.getLabelCount() - - fresult1.closest_labels))); + qname_.split(qname_.getLabelCount() - closest_labels))); const ZoneFinder::FindNSEC3Result fresult2 = finder.findNSEC3(wildname, false); if (fresult2.matched) { @@ -227,17 +240,10 @@ Query::addWildcardProof(ZoneFinder& finder, // Case for RFC5155 Section 7.2.6. // // Note that the closest encloser must be the immediate ancestor - // of the matching wildcard, so NSEC3 for its next closer is what - // we are expected to provided per the RFC (if this assumption isn't - // met the zone is broken anyway). - const ZoneFinder::FindNSEC3Result NSEC3Result( - finder.findNSEC3(qname_, true)); - // Note that at this point next_proof must not be NULL unless it's - // a run time collision (or zone/findNSEC3() is broken). The - // unexpected case will be caught in addRRset() and result in SERVFAIL. - response_.addRRset(Message::SECTION_AUTHORITY, - boost::const_pointer_cast( - NSEC3Result.next_proof), dnssec_); + // of the matching wildcard, so NSEC3 for its next closer (and only + // that NSEC3) is what we are expected to provided per the RFC (if + // this assumption isn't met the zone is broken anyway). + addClosestEncloserProof(finder, qname_, false, false); } } @@ -280,22 +286,7 @@ Query::addDS(ZoneFinder& finder, const Name& dname) { } else if (ds_result.code == ZoneFinder::NXRRSET && ds_result.isNSEC3Signed()) { // Add no DS proof with NSEC3 as specified in RFC5155 Section 7.2.7. - // Depending on whether the zone is optout or not, findNSEC3() may - // return non-NULL or NULL next_proof (respectively). The Opt-Out flag - // must be set or cleared accordingly, but we don't check that - // in this level (as long as the zone signed validly and findNSEC3() - // is valid, the condition should be met; otherwise we'd let the - // validator detect the error). - const ZoneFinder::FindNSEC3Result nsec3_result = - finder.findNSEC3(dname, true); - response_.addRRset(Message::SECTION_AUTHORITY, - boost::const_pointer_cast( - nsec3_result.closest_proof), dnssec_); - if (nsec3_result.next_proof) { - response_.addRRset(Message::SECTION_AUTHORITY, - boost::const_pointer_cast( - nsec3_result.next_proof), dnssec_); - } + addClosestEncloserProof(finder, dname, true); } else { // Any other case should be an error isc_throw(BadDS, "Unexpected result for DS lookup for delegation"); @@ -320,42 +311,29 @@ Query::addNXRRsetProof(ZoneFinder& finder, // recursive search (and add next_proof, if necessary), // otherwise, do non-recursive search const bool qtype_ds = (qtype_ == RRType::DS()); - ZoneFinder::FindNSEC3Result result(finder.findNSEC3(qname_, qtype_ds)); - if (result.matched) { - response_.addRRset(Message::SECTION_AUTHORITY, - boost::const_pointer_cast( - result.closest_proof), dnssec_); - // For qtype == DS, next_proof could be set - // (We could check for opt-out here, but that's really the - // responsibility of the datasource) - if (qtype_ds && result.next_proof != ConstRRsetPtr()) { + if (qtype_ds) { + addClosestEncloserProof(finder, qname_, true); + } else { + ZoneFinder::FindNSEC3Result result(finder.findNSEC3(qname_, + false)); + if (result.matched) { response_.addRRset(Message::SECTION_AUTHORITY, boost::const_pointer_cast( - result.next_proof), dnssec_); + result.closest_proof), dnssec_); + } else { + isc_throw(BadNSEC3, + "No matching NSEC3 found for existing domain " + << qname_); } - } else { - isc_throw(BadNSEC3, "No matching NSEC3 found for existing domain " - << qname_); } } else if (db_result.isNSEC3Signed() && db_result.isWildcard()) { // Case for RFC5155 Section 7.2.5 - const ZoneFinder::FindNSEC3Result result(finder.findNSEC3(qname_, - true)); - // We know there's no exact match for the qname, so findNSEC3() should - // return both closest and next proofs. If the latter is NULL, it - // means a run time collision (or the zone is broken in other way). - // In that case addRRset() will throw, and it will be converted to - // SERVFAIL. - response_.addRRset(Message::SECTION_AUTHORITY, - boost::const_pointer_cast( - result.closest_proof), dnssec_); - response_.addRRset(Message::SECTION_AUTHORITY, - boost::const_pointer_cast( - result.next_proof), dnssec_); + const uint8_t closest_labels = + addClosestEncloserProof(finder, qname_, false); // Construct the matched wildcard name and add NSEC3 for it. const Name wname = Name("*").concatenate( - qname_.split(qname_.getLabelCount() - result.closest_labels)); + qname_.split(qname_.getLabelCount() - closest_labels)); const ZoneFinder::FindNSEC3Result wresult(finder.findNSEC3(wname, false)); if (wresult.matched) { diff --git a/src/bin/auth/query.h b/src/bin/auth/query.h index b6e6f05dce..3fcf2af7c2 100644 --- a/src/bin/auth/query.h +++ b/src/bin/auth/query.h @@ -204,6 +204,10 @@ private: /// within this method. bool processDSAtChild(); + uint8_t addClosestEncloserProof(isc::datasrc::ZoneFinder& finder, + const isc::dns::Name& name, bool exact_ok, + bool add_closet = true); + public: /// Constructor from query parameters. /// diff --git a/src/bin/auth/tests/query_unittest.cc b/src/bin/auth/tests/query_unittest.cc index 624db2f83d..0c413a1fdd 100644 --- a/src/bin/auth/tests/query_unittest.cc +++ b/src/bin/auth/tests/query_unittest.cc @@ -1429,7 +1429,7 @@ TEST_F(QueryTest, badWildcardNSEC3) { EXPECT_THROW(Query(memory_client, Name("www.wild.example.com"), RRType::A(), response, true).process(), - isc::InvalidParameter); + Query::BadNSEC3); } TEST_F(QueryTest, badWildcardProof1) { @@ -1540,10 +1540,9 @@ TEST_F(QueryTest, wildcardNxrrsetWithNSEC3Collision) { ConstRRsetPtr()); mock_finder->setNSEC3Result(&nsec3); - // Message::addRRset() will detect it and throw InvalidParameter. EXPECT_THROW(Query(memory_client, Name("www1.uwild.example.com"), RRType::TXT(), response, true).process(), - isc::InvalidParameter); + Query::BadNSEC3); } TEST_F(QueryTest, wildcardNxrrsetWithNSEC3Broken) { @@ -2283,10 +2282,9 @@ TEST_F(QueryTest, nxdomainWithBadNextNSEC3Proof) { ConstRRsetPtr()); mock_finder->setNSEC3Result(&nsec3); - // Message::addRRset() will detect it and throw InvalidParameter. EXPECT_THROW(Query(memory_client, Name("nxdomain.example.com"), RRType::TXT(), response, true).process(), - isc::InvalidParameter); + Query::BadNSEC3); } TEST_F(QueryTest, nxdomainWithBadWildcardNSEC3Proof) {