From: JINMEI Tatuya Date: Fri, 27 Jan 2012 23:56:19 +0000 (-0800) Subject: [1574] revised internal representation of NSEC3 storage from set to map. X-Git-Tag: trac2351_base~268^2~5^2~16^2~6 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=4164bd33ee8f465820a6d22aad0d2a63db7765b7;p=thirdparty%2Fkea.git [1574] revised internal representation of NSEC3 storage from set to map. added some more test cases. --- diff --git a/src/lib/datasrc/memory_datasrc.cc b/src/lib/datasrc/memory_datasrc.cc index 94a21a10b5..373d3c920b 100644 --- a/src/lib/datasrc/memory_datasrc.cc +++ b/src/lib/datasrc/memory_datasrc.cc @@ -12,9 +12,10 @@ // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR // PERFORMANCE OF THIS SOFTWARE. -#include +#include #include -#include +#include +#include #include #include #include @@ -67,14 +68,13 @@ typedef boost::shared_ptr DomainPtr; typedef RBTree DomainTree; typedef RBNode DomainNode; -// Separate storage for NSEC3 RRs (and their RRSIGs) -struct NameCompare : public binary_function -{ - bool operator()(const ConstRRsetPtr& n1, const ConstRRsetPtr& n2) const { - return (n1->getName().compare(n2->getName()).getOrder() < 0); - } -}; -typedef set NSEC3Set; +// Separate storage for NSEC3 RRs (and their RRSIGs). It's an STL map +// from string to the NSEC3 RRset. The map key is the first label +// (upper cased) of the owner name of the corresponding NSEC3 (i.e., map +// value). We can use the standard string comparison (if the comparison +// target is also upper cased) due to the nature of NSEC3 owner names. +typedef map NSEC3Map; +typedef NSEC3Map::value_type NSEC3Pair; // Actual zone data: Essentially a set of zone's RRs. This is defined as // a separate structure so that it'll be replaceable on reload. @@ -85,7 +85,7 @@ struct ZoneData { DomainTree domains_; // The optional NSEC3 storage (TBD: should allocate it on demand) - NSEC3Set nsec3_set_; + NSEC3Map nsec3_map_; }; } @@ -111,7 +111,6 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl { // The actual zone data scoped_ptr zone_data_; - //DomainTree domains_; // Add the necessary magic for any wildcard contained in 'name' // (including itself) to be found in the zone. @@ -316,6 +315,15 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl { return (result::SUCCESS); } + // A helper functor to convert the 1st NSEC3 label to all upper-cased + // characters. Note: technically there's a subtle issue when char + // is signed, but in practice the label should consist of all positive + // character values for a valid NSEC3 hash name (if it's invalid the + // resulting zone doesn't work correctly anyway). + struct ToUpper { + char operator()(char ch) { return (toupper(ch)); } + }; + /* * Implementation of longer methods. We put them here, because the * access is without the impl_-> and it will get inlined anyway. @@ -331,8 +339,10 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl { arg(rrset->getName()).arg(rrset->getType()).arg(origin_); if (rrset->getType() == RRType::NSEC3()) { - // TBD: Duplicate check - zone_data.nsec3_set_.insert(rrset); + string fst_label = rrset->getName().split(0, 1).toText(true); + transform(fst_label.begin(), fst_label.end(), fst_label.begin(), + ToUpper()); + zone_data.nsec3_map_.insert(NSEC3Pair(fst_label, rrset)); return (result::SUCCESS); } @@ -751,6 +761,30 @@ InMemoryZoneFinder::findNSEC3(const Name&, bool) { "data source"); } +ZoneFinder::FindNSEC3Result +InMemoryZoneFinder::findNSEC3Tmp(const Name& name, bool recursive) { + if (recursive) { + isc_throw(Unexpected, "recursive mode isn't expected in tests"); + } + + // A temporary workaround for testing: convert the original name to + // NSEC3-hashed name using hardcoded mapping. + string hname_text; + if (name == Name("example.org")) { + hname_text = "0P9MHAVEQVM6T7VBL5LOP2U3T2RP3TOM"; + } else { + isc_throw(Unexpected, "unexpected name for NSEC3 test: " << name); + } + + NSEC3Map::const_iterator found = + impl_->zone_data_->nsec3_map_.find(hname_text); + if (found != impl_->zone_data_->nsec3_map_.end()) { + return (FindNSEC3Result(true, 2, found->second, ConstRRsetPtr())); + } + + isc_throw(Unexpected, "unexpected NSEC3 search result for " << name); +} + result::Result InMemoryZoneFinder::add(const ConstRRsetPtr& rrset) { return (impl_->add(rrset, *impl_->zone_data_)); diff --git a/src/lib/datasrc/memory_datasrc.h b/src/lib/datasrc/memory_datasrc.h index b960ab9096..10c6b59118 100644 --- a/src/lib/datasrc/memory_datasrc.h +++ b/src/lib/datasrc/memory_datasrc.h @@ -89,6 +89,10 @@ public: virtual FindNSEC3Result findNSEC3(const isc::dns::Name& name, bool recursive); + /// TBD + FindNSEC3Result + findNSEC3Tmp(const isc::dns::Name& name, bool recursive); + /// \brief Imelementation of the ZoneFinder::findPreviousName method /// /// This one throws NotImplemented exception, as InMemory doesn't diff --git a/src/lib/datasrc/tests/memory_datasrc_unittest.cc b/src/lib/datasrc/tests/memory_datasrc_unittest.cc index 453d7a40cc..67bc309f27 100644 --- a/src/lib/datasrc/tests/memory_datasrc_unittest.cc +++ b/src/lib/datasrc/tests/memory_datasrc_unittest.cc @@ -1308,14 +1308,53 @@ TEST_F(InMemoryZoneFinderTest, addbadRRsig) { InMemoryZoneFinder::AddError); } +// +// (Faked) NSEC3 hash data. Arbitrarily borrowed from RFC515 examples. +// +// Commonly used NSEC3 suffix. It's incorrect to use it for all NSEC3s, but +// doesn't matter for the purpose of our tests. +const char* const nsec3_common = " 300 IN NSEC3 1 1 12 aabbccdd " + "2T7B4G4VSA5SMI47K61MV5BV1A22BOJR A RRSIG"; +// For apex (example.org) +const char* const apex_hash = "0P9MHAVEQVM6T7VBL5LOP2U3T2RP3TOM"; +const char* const apex_hash_down = "0p9mhaveqvm6t7vbl5lop2u3t2rp3tom"; + TEST_F(InMemoryZoneFinderTest, addNSEC3) { - zone_finder_.add(textToRRset( - "0P9MHAVEQVM6T7VBL5LOP2U3T2RP3TOM.example.org. " - "300 IN NSEC3 1 1 12 aabbccdd " - "2T7B4G4VSA5SMI47K61MV5BV1A22BOJR A RRSIG")); + const string nsec3_text = string(apex_hash) + ".example.org." + + string(nsec3_common); + // This name shouldn't be found in the normal domain tree. + EXPECT_EQ(result::SUCCESS, zone_finder_.add(textToRRset(nsec3_text))); EXPECT_EQ(ZoneFinder::NXDOMAIN, - zone_finder_.find( - Name("0P9MHAVEQVM6T7VBL5LOP2U3T2RP3TOM.example.org"), - RRType::NSEC3()).code); + zone_finder_.find(Name(string(apex_hash) + ".example.org"), + RRType::NSEC3()).code); + // Dedicated NSEC3 find should be able to find it. + ZoneFinder::FindNSEC3Result result = + zone_finder_.findNSEC3Tmp(Name("example.org"), false); + EXPECT_TRUE(result.matched); + ASSERT_TRUE(result.closest_proof); + actual_rrsets_.push_back(result.closest_proof); + rrsetsCheck(nsec3_text, actual_rrsets_.begin(), actual_rrsets_.end()); +} + +TEST_F(InMemoryZoneFinderTest, addNSEC3Lower) { + // Similar to the previous case, but NSEC3 owner name is lower-cased. + const string nsec3_text = string(apex_hash_down) + ".example.org." + + string(nsec3_common); + EXPECT_EQ(result::SUCCESS, zone_finder_.add(textToRRset(nsec3_text))); + ZoneFinder::FindNSEC3Result result = + zone_finder_.findNSEC3Tmp(Name("example.org"), false); + EXPECT_TRUE(result.matched); + ASSERT_TRUE(result.closest_proof); + actual_rrsets_.push_back(result.closest_proof); + rrsetsCheck(nsec3_text, actual_rrsets_.begin(), actual_rrsets_.end()); + + // - duplicate + // - adding RRSIG for NSEC3 + // - bogus NSEC3 owner name: redundant labels, wildcard + // - case where the main tree has NSEC3 name + // - parameter consistency + // - existence of NSEC3PARAM + // - parameter consistency with NSEC3PARAM + // - add NSEC3PARAM first/second } }