[1574] handle RRSIG for NSEC3s for properly (set in the NSEC3 namespace)

src/lib/datasrc/memory_datasrc.cc

@@ -267,7 +267,7 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
         // Owner names of NSEC3 have special format as defined in RFC5155,
         // and cannot be a wildcard name or must be one label longer than
         // the zone origin.  While the RFC doesn't prohibit other forms of
-        // names, no sane zone wouldn't have such names for NSEC3.
+        // names, no sane zone would have such names for NSEC3.
         // BIND 9 also refuses NSEC3 at wildcard.
         if (rrset->getType() == RRType::NSEC3() &&
             (rrset->getName().isWildcard() ||
@@ -282,14 +282,6 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
 
     result::Result addRRsig(const ConstRRsetPtr sig_rrset, ZoneData& zone_data)
     {
-        DomainNode* node = NULL;
-        if (zone_data.domains_.find(sig_rrset->getName(), &node) !=
-            DomainTree::EXACTMATCH || node == NULL || !node->getData()) {
-            isc_throw(AddError,
-                      "RRSIG is being added, but no RR to be covered: "
-                      << sig_rrset->getName());
-        }
-
         // Check consistency of the type covered.
         // We know the RRset isn't empty, so the following check is safe.
         RdataIteratorPtr rit = sig_rrset->getRdataIterator();
@@ -305,16 +297,43 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
 
         // Find the RRset to be covered; if not found, treat it as an error
         // for now.
-        const Domain::const_iterator it = node->getData()->find(covered);
-        if (it == node->getData()->end()) {
-            isc_throw(AddError,
-                      "RRSIG is being added, but no RR of covered type found: "
-                      << sig_rrset->toText());
+        ConstRRsetPtr covered_rrset;
+        if (covered != RRType::NSEC3()) {
+            DomainNode* node = NULL;
+            if (zone_data.domains_.find(sig_rrset->getName(), &node) !=
+                DomainTree::EXACTMATCH || node == NULL || !node->getData()) {
+                isc_throw(AddError,
+                          "RRSIG is being added, but no RR to be covered: "
+                          << sig_rrset->getName());
+            }
+            const Domain::const_iterator it = node->getData()->find(covered);
+            if (it != node->getData()->end()) {
+                covered_rrset = it->second;
+            }
+        } else {
+            // In case of NSEC3 if something is found it must be NSEC3 RRset
+            // under the assumption of our current implementation.
+            if (zone_data.nsec3_data_) {
+                string fst_label = sig_rrset->getName().split(0, 1).
+                    toText(true);
+                transform(fst_label.begin(), fst_label.end(),
+                          fst_label.begin(), ToUpper());
+                NSEC3Map::const_iterator found =
+                    zone_data.nsec3_data_->map_.find(fst_label);
+                if (found != zone_data.nsec3_data_->map_.end()) {
+                    covered_rrset = found->second;
+                    assert(covered_rrset->getType() == covered);
+                }
+            }
+        }
+        if (!covered_rrset) {
+            isc_throw(AddError, "RRSIG is being added, but no RR of "
+                      "covered type found: " << sig_rrset->toText());
         }
 
         // The current implementation doesn't allow an existing RRSIG to be
         // overridden (or updated with additional ones).
-        if ((it->second)->getRRsig()) {
+        if (covered_rrset->getRRsig()) {
             isc_throw(AddError,
                       "RRSIG is being added to override an existing one: "
                       << sig_rrset->toText());
@@ -329,7 +348,7 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
         // Note: there's a slight chance of getting an exception.
         // As noted in add(), we give up strong exception guarantee in such
         // cases.
-        boost::const_pointer_cast<RRset>(it->second)->addRRsig(sig_rrset);
+        boost::const_pointer_cast<RRset>(covered_rrset)->addRRsig(sig_rrset);
 
         return (result::SUCCESS);
     }

src/lib/datasrc/tests/memory_datasrc_unittest.cc

@@ -1315,9 +1315,15 @@ TEST_F(InMemoryZoneFinderTest, addbadRRsig) {
 // doesn't matter for the purpose of our tests.
 const char* const nsec3_common = " 300 IN NSEC3 1 1 12 aabbccdd "
     "2T7B4G4VSA5SMI47K61MV5BV1A22BOJR A RRSIG";
+// Likewise, common RRSIG suffix for NSEC3s.
+const char* const nsec3_rrsig_common = " 300 IN RRSIG NSEC3 5 3 3600 "
+    "20000101000000 20000201000000 12345 example.org. FAKEFAKEFAKE";
+
 // For apex (example.org)
 const char* const apex_hash = "0P9MHAVEQVM6T7VBL5LOP2U3T2RP3TOM";
-const char* const apex_hash_down = "0p9mhaveqvm6t7vbl5lop2u3t2rp3tom";
+const char* const apex_hash_lower = "0p9mhaveqvm6t7vbl5lop2u3t2rp3tom";
+// For ns1.example.org
+const char* const ns1_hash = "2t7b4g4vsa5smi47k61mv5bv1a22bojr";
 
 TEST_F(InMemoryZoneFinderTest, addNSEC3) {
     const string nsec3_text = string(apex_hash) + ".example.org." +
@@ -1346,11 +1352,19 @@ TEST_F(InMemoryZoneFinderTest, addNSEC3) {
         zone_finder_.findNSEC3Tmp(Name("example.org"), false);
     actual_rrsets_.push_back(result2.closest_proof);
     rrsetsCheck(nsec3_text, actual_rrsets_.begin(), actual_rrsets_.end());
+
+    // NSEC3-like name but of ordinary RR type should go to normal tree.
+    const string nonsec3_text = string(apex_hash) + ".example.org. " +
+        "300 IN A 192.0.2.1";
+    EXPECT_EQ(result::SUCCESS, zone_finder_.add(textToRRset(nonsec3_text)));
+    EXPECT_EQ(ZoneFinder::SUCCESS,
+              zone_finder_.find(Name(string(apex_hash) + ".example.org"),
+                                RRType::A()).code);
 }
 
 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." +
+    const string nsec3_text = string(apex_hash_lower) + ".example.org." +
         string(nsec3_common);
     EXPECT_EQ(result::SUCCESS, zone_finder_.add(textToRRset(nsec3_text)));
     ZoneFinder::FindNSEC3Result result =
@@ -1372,12 +1386,63 @@ TEST_F(InMemoryZoneFinderTest, badNSEC3Name) {
                                               ".example.org." +
                                               string(nsec3_common))),
                  InMemoryZoneFinder::AddError);
+}
+
+TEST_F(InMemoryZoneFinderTest, addNSEC3WithRRSIG) {
+    // Adding NSEC3 and its RRSIG
+    const string nsec3_text = string(apex_hash) + ".example.org." +
+        string(nsec3_common);
+    EXPECT_EQ(result::SUCCESS, zone_finder_.add(textToRRset(nsec3_text)));
+    const string nsec3_rrsig_text = string(apex_hash) + ".example.org." +
+        string(nsec3_rrsig_common);
+    EXPECT_EQ(result::SUCCESS, zone_finder_.add(textToRRset(nsec3_rrsig_text)));
+
+    // Then look for it.  The NSEC3 should have the RRSIG that was just added.
+    ZoneFinder::FindNSEC3Result result =
+        zone_finder_.findNSEC3Tmp(Name("example.org"), false);
+    EXPECT_TRUE(result.matched);
+    ASSERT_TRUE(result.closest_proof);
+    ASSERT_TRUE(result.closest_proof->getRRsig());
+    actual_rrsets_.push_back(result.closest_proof);
+    actual_rrsets_.push_back(result.closest_proof->getRRsig());
+    rrsetsCheck(nsec3_text + "\n" + nsec3_rrsig_text, actual_rrsets_.begin(),
+                actual_rrsets_.end());
+
+    // Duplicate add of RRSIG for the same NSEC3 is prohibited.
+    EXPECT_THROW(zone_finder_.add(textToRRset(nsec3_rrsig_text)),
+                 InMemoryZoneFinder::AddError);
+
+    // Same check using the lower-cased name.  This also confirms matching
+    // is case-insensitive.
+    EXPECT_THROW(zone_finder_.add(textToRRset(string(apex_hash_lower) +
+                                              ".example.org."
+                                              + string(nsec3_rrsig_common))),
+                 InMemoryZoneFinder::AddError);
+}
+
+TEST_F(InMemoryZoneFinderTest, badRRsigForNSEC3) {
+    // adding RRSIG for NSEC3 even before adding any NSEC3 (internally,
+    // a space for NSEC3 namespace isn't yet allocated)
+    EXPECT_THROW(zone_finder_.add(textToRRset(string(apex_hash) +
+                                              ".example.org." +
+                                              string(nsec3_rrsig_common))),
+                 InMemoryZoneFinder::AddError);
+
+    // Add an NSEC3
+    EXPECT_EQ(result::SUCCESS, zone_finder_.add(
+                  textToRRset(string(apex_hash) + ".example.org." +
+                              string(nsec3_common))));
+
+    // Then add an NSEC3 for a non existent NSEC3.  It should fail in the
+    // current implementation.
+    EXPECT_THROW(zone_finder_.add(textToRRset(string(ns1_hash) +
+                                              ".example.org." +
+                                              string(nsec3_rrsig_common))),
+                 InMemoryZoneFinder::AddError);
+}
 
-    // - adding RRSIG for NSEC3
-    // - case where the main tree has NSEC3 name
     // - parameter consistency
     // - existence of NSEC3PARAM
     // - parameter consistency with NSEC3PARAM
     // - add NSEC3PARAM first/second
 }
-}