[nsec_merge] Merge branch 'trac1808-2' into nsec_merge

Conflicts:
	src/lib/datasrc/tests/memory_datasrc_unittest.cc

src/lib/datasrc/memory_datasrc.cc

@@ -442,7 +442,8 @@ ZoneData::findNode(const Name& name, RBTreeNodeChain<Domain>& node_path,
             return (ResultType(ZoneFinder::NXRRSET, node,
                                getClosestNSEC(node_path, options)));
         }
-        if (node->getFlag(domain_flag::WILD)) { // maybe a wildcard
+        if (node->getFlag(domain_flag::WILD) && // maybe a wildcard, check only
+            (options & ZoneFinder::NO_WILDCARD) == 0) { // if not disabled.
             if (node_path.getLastComparisonResult().getRelation() ==
                 NameComparisonResult::COMMONANCESTOR &&
                 node_path.getLastComparisonResult().getCommonLabels() > 1) {
@@ -456,7 +457,7 @@ ZoneData::findNode(const Name& name, RBTreeNodeChain<Domain>& node_path,
                 LOG_DEBUG(logger, DBG_TRACE_DATA,
                           DATASRC_MEM_WILDCARD_CANCEL).arg(name);
                 return (ResultType(ZoneFinder::NXDOMAIN, NULL,
-                                   ConstRBNodeRRsetPtr()));
+                                   getClosestNSEC(node_path, options)));
             }
             // Now the wildcard should be the best match.
             const Name wildcard(Name("*").concatenate(
@@ -1258,6 +1259,24 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
         }
     }
 
+    // A helper function for the NXRRSET case in find().  If the zone is
+    // NSEC-signed and DNSSEC records are requested, try to find NSEC
+    // on the given node, and return it if found; return NULL for all other
+    // cases.
+    ConstRBNodeRRsetPtr getNSECForNXRRSET(FindOptions options,
+                                          const DomainNode& node) const
+    {
+        if (zone_data_->nsec_signed_ &&
+            (options & ZoneFinder::FIND_DNSSEC) != 0) {
+            const Domain::const_iterator found =
+                node.getData()->find(RRType::NSEC());
+            if (found != node.getData()->end()) {
+                return (found->second);
+            }
+        }
+        return (ConstRBNodeRRsetPtr());
+    }
+
     // Set up FindContext object as a return value of find(), taking into
     // account wildcard matches and DNSSEC information.  We set the NSEC/NSEC3
     // flag when applicable regardless of the find option; the caller would
@@ -1373,10 +1392,9 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
                                           rename));
             }
         }
-        // No exact match or CNAME.  Return NXRRSET.
-        LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_NXRRSET).arg(type).
-            arg(name);
-        return (createFindResult(NXRRSET, ConstRBNodeRRsetPtr(), rename));
+        // No exact match or CNAME.  Get NSEC if necessary and return NXRRSET.
+        return (createFindResult(NXRRSET, getNSECForNXRRSET(options, *node),
+                                 rename));
     }
 };
 

src/lib/datasrc/memory_datasrc.h

@@ -65,11 +65,7 @@ public:
     /// \brief Returns the class of the zone.
     virtual isc::dns::RRClass getClass() const;
 
-    /// \brief Looks up an RRset in the zone.
-    ///
-    /// See documentation in \c Zone.
-    ///
-    /// It returns NULL pointer in case of NXDOMAIN and NXRRSET.
+    /// \brief Find an RRset in the datasource
     virtual ZoneFinderContextPtr find(const isc::dns::Name& name,
                                       const isc::dns::RRType& type,
                                       const FindOptions options =

src/lib/datasrc/tests/memory_datasrc_unittest.cc

@@ -373,14 +373,22 @@ protected:
     // expected_flags is set to either RESULT_NSEC_SIGNED or
     // RESULT_NSEC3_SIGNED when it's NSEC/NSEC3 signed respectively and
     // find() is expected to set the corresponding flags.
+    // find_options should be set to FIND_DNSSEC for NSEC-signed case when
+    // NSEC is expected to be returned.
     void findCheck(ZoneFinder::FindResultFlags expected_flags =
-                   ZoneFinder::RESULT_DEFAULT);
+                   ZoneFinder::RESULT_DEFAULT,
+                   ZoneFinder::FindOptions find_options =
+                   ZoneFinder::FIND_DEFAULT);
     void emptyNodeCheck(ZoneFinder::FindResultFlags expected_flags =
                         ZoneFinder::RESULT_DEFAULT);
     void wildcardCheck(ZoneFinder::FindResultFlags expected_flags =
-                       ZoneFinder::RESULT_DEFAULT);
+                       ZoneFinder::RESULT_DEFAULT,
+                       ZoneFinder::FindOptions find_options =
+                       ZoneFinder::FIND_DEFAULT);
     void doCancelWildcardCheck(ZoneFinder::FindResultFlags expected_flags =
-                               ZoneFinder::RESULT_DEFAULT);
+                               ZoneFinder::RESULT_DEFAULT,
+                               ZoneFinder::FindOptions find_options =
+                               ZoneFinder::FIND_DEFAULT);
     void anyWildcardCheck(ZoneFinder::FindResultFlags expected_flags =
                           ZoneFinder::RESULT_DEFAULT);
     void emptyWildcardCheck(ZoneFinder::FindResultFlags expected_flags =
@@ -457,6 +465,11 @@ public:
              &rr_ent_nsec3_},
             {"foo.wild.example.org. 3600 IN NSEC example.org. A RRSIG NSEC",
              &rr_ent_nsec4_},
+            // And these are NSECs used in different tests
+            {"ns.example.org. 300 IN NSEC *.nswild.example.org. A AAAA NSEC",
+             &rr_ns_nsec_},
+            {"*.wild.example.org. 300 IN NSEC foo.wild.example.org. A NSEC",
+             &rr_wild_nsec_},
             {NULL, NULL}
         };
 
@@ -525,6 +538,8 @@ public:
     RRsetPtr rr_ent_nsec2_;
     RRsetPtr rr_ent_nsec3_;
     RRsetPtr rr_ent_nsec4_;
+    RRsetPtr rr_ns_nsec_;
+    RRsetPtr rr_wild_nsec_;
 
     // A faked NSEC3 hash calculator for convenience.
     // Tests that need to use the faked hashed values should call
@@ -994,7 +1009,9 @@ TEST_F(InMemoryZoneFinderTest, glue) {
  *     directly there, it just tells it doesn't exist.
  */
 void
-InMemoryZoneFinderTest::findCheck(ZoneFinder::FindResultFlags expected_flags) {
+InMemoryZoneFinderTest::findCheck(ZoneFinder::FindResultFlags expected_flags,
+                                  ZoneFinder::FindOptions find_options)
+{
     // Fill some data inside
     // Now put all the data we have there. It should throw nothing
     EXPECT_NO_THROW(EXPECT_EQ(SUCCESS, zone_finder_.add(rr_ns_)));
@@ -1013,46 +1030,63 @@ InMemoryZoneFinderTest::findCheck(ZoneFinder::FindResultFlags expected_flags) {
     findTest(rr_ns_a_->getName(), RRType::A(), ZoneFinder::SUCCESS, true,
              rr_ns_a_);
 
-    // These domain exist but don't have the provided RRType
-    findTest(origin_, RRType::AAAA(), ZoneFinder::NXRRSET, true,
-             ConstRRsetPtr(), expected_flags);
-    findTest(rr_ns_a_->getName(), RRType::NS(), ZoneFinder::NXRRSET, true,
-             ConstRRsetPtr(), expected_flags);
-
     // These domains don't exist. (and one is out of the zone).  In an
-    // NSEC-signed zone, it should return the covering NSEC for the query
-    // name (the actual NSEC in the test data may not really "cover" it,
-    // but for the purpose of this test it's okay).
-    if ((expected_flags & ZoneFinder::RESULT_NSEC_SIGNED) != 0) {
-        // There's no other name between this one and the origin, so it
-        // should return the origin NSEC.
-        findTest(Name("nothere.example.org"), RRType::A(),
-                 ZoneFinder::NXDOMAIN, true, rr_nsec_, expected_flags,
-                 NULL, ZoneFinder::FIND_DNSSEC);
-
-        // The previous name in the zone is "ns.example.org", but it doesn't
-        // have an NSEC.  It should be skipped and the origin NSEC will be
-        // returned as the "closest NSEC".
-        findTest(Name("nxdomain.example.org"), RRType::A(),
-                 ZoneFinder::NXDOMAIN, true, rr_nsec_, expected_flags,
-                 NULL, ZoneFinder::FIND_DNSSEC);
-    } else {
-        findTest(Name("nothere.example.org"), RRType::A(),
-                 ZoneFinder::NXDOMAIN, true, ConstRRsetPtr(), expected_flags);
+    // NSEC-signed zone with DNSSEC records requested, it should return the
+    // covering NSEC for the query name (the actual NSEC in the test data may
+    // not really "cover" it, but for the purpose of this test it's okay).
+    ConstRRsetPtr expected_nsec; // by default it's NULL
+    if ((expected_flags & ZoneFinder::RESULT_NSEC_SIGNED) != 0 &&
+        (find_options & ZoneFinder::FIND_DNSSEC) != 0) {
+        expected_nsec = rr_nsec_;
     }
+
+    // There's no other name between this one and the origin, so when NSEC
+    // is to be returned it should be the origin NSEC.
+    findTest(Name("nothere.example.org"), RRType::A(),
+             ZoneFinder::NXDOMAIN, true, expected_nsec, expected_flags,
+             NULL, find_options);
+
+    // The previous name in the zone is "ns.example.org", but it doesn't
+    // have an NSEC.  It should be skipped and the origin NSEC will be
+    // returned as the "closest NSEC".
+    findTest(Name("nxdomain.example.org"), RRType::A(),
+             ZoneFinder::NXDOMAIN, true, expected_nsec, expected_flags,
+             NULL, find_options);
     EXPECT_THROW(zone_finder_.find(Name("example.net"), RRType::A()),
                  OutOfZone);
+
+    // These domain exist but don't have the provided RRType.  For the latter
+    // one we now add its NSEC (which was delayed so that it wouldn't break
+    // other cases above).
+    findTest(origin_, RRType::AAAA(), ZoneFinder::NXRRSET, true,
+             expected_nsec, expected_flags, NULL, find_options);
+    if ((expected_flags & ZoneFinder::RESULT_NSEC_SIGNED) != 0) {
+        EXPECT_EQ(SUCCESS, zone_finder_.add(rr_ns_nsec_));
+        if ((find_options & ZoneFinder::FIND_DNSSEC) != 0) {
+            expected_nsec = rr_ns_nsec_;
+        }
+    }
+    findTest(rr_ns_a_->getName(), RRType::NS(), ZoneFinder::NXRRSET, true,
+             expected_nsec, expected_flags, NULL, find_options);
 }
 
 TEST_F(InMemoryZoneFinderTest, find) {
     findCheck();
 }
 
-TEST_F(InMemoryZoneFinderTest, findNSEC3Signed) {
+TEST_F(InMemoryZoneFinderTest, findNSEC3Signe) {
     findCheck(ZoneFinder::RESULT_NSEC3_SIGNED);
 }
 
+TEST_F(InMemoryZoneFinderTest, findNSEC3SignedWithDNSSEC) {
+    // For NSEC3-signed zones, specifying the DNSSEC option shouldn't change
+    // anything (the NSEC3_SIGNED flag is always set, and no records are
+    // returned for negative cases regardless).
+    findCheck(ZoneFinder::RESULT_NSEC3_SIGNED, ZoneFinder::FIND_DNSSEC);
+}
+
 TEST_F(InMemoryZoneFinderTest, findNSECSigned) {
+    // NSEC-signed zone, without requesting DNSSEC (no NSEC should be provided)
     findCheck(ZoneFinder::RESULT_NSEC_SIGNED);
 }
 
@@ -1107,6 +1141,11 @@ TEST_F(InMemoryZoneFinderTest,findNSECEmptyNonterminalAtWildcard) {
                      ZoneFinder::RESULT_WILDCARD);
 }
 
+TEST_F(InMemoryZoneFinderTest, findNSECSignedWithDNSSEC) {
+    // NSEC-signed zone, requesting DNSSEC (NSEC should be provided)
+    findCheck(ZoneFinder::RESULT_NSEC_SIGNED, ZoneFinder::FIND_DNSSEC);
+}
+
 void
 InMemoryZoneFinderTest::emptyNodeCheck(
     ZoneFinder::FindResultFlags expected_flags)
@@ -1269,7 +1308,8 @@ TEST_F(InMemoryZoneFinderTest, loadFromIterator) {
  */
 void
 InMemoryZoneFinderTest::wildcardCheck(
-    ZoneFinder::FindResultFlags expected_flags)
+    ZoneFinder::FindResultFlags expected_flags,
+    ZoneFinder::FindOptions find_options)
 {
     /*
      *            example.org.
@@ -1281,7 +1321,6 @@ InMemoryZoneFinderTest::wildcardCheck(
 
     // If the zone is "signed" (detecting it by the NSEC/NSEC3 signed flags),
     // add RRSIGs to the records.
-    ZoneFinder::FindOptions find_options = ZoneFinder::FIND_DEFAULT;
     if ((expected_flags & ZoneFinder::RESULT_NSEC_SIGNED) != 0 ||
         (expected_flags & ZoneFinder::RESULT_NSEC3_SIGNED) != 0) {
         // Convenience shortcut.  The RDATA is not really validatable, but
@@ -1322,6 +1361,16 @@ InMemoryZoneFinderTest::wildcardCheck(
         }
     }
 
+    // For the test setup of "NSEC-signed" zone, we might expect it will
+    // be returned with a negative result, either because wildcard match is
+    // disabled by the search option or because wildcard match is canceled
+    // per protocol.
+    ConstRRsetPtr expected_nsec; // by default it's NULL
+    if ((expected_flags & ZoneFinder::RESULT_NSEC_SIGNED) != 0 &&
+        (find_options & ZoneFinder::FIND_DNSSEC) != 0) {
+        expected_nsec = rr_nsec_;
+    }
+
     // Search the original name of wildcard
     {
         SCOPED_TRACE("Search directly at *");
@@ -1329,53 +1378,69 @@ InMemoryZoneFinderTest::wildcardCheck(
                  true, rr_wild_, ZoneFinder::RESULT_DEFAULT, NULL,
                  find_options);
     }
+
+    // Below some of the test cases will normally result in a wildcard match;
+    // if NO_WILDCARD is specified, it should result in NXDOMAIN instead,
+    // and, when available and requested, the covering NSEC will be returned.
+    // The following are shortcut parameters to unify these cases.
+    const bool wild_ok = ((find_options & ZoneFinder::NO_WILDCARD) == 0);
+    const ZoneFinder::FindResultFlags wild_expected_flags =
+        wild_ok ? (ZoneFinder::RESULT_WILDCARD | expected_flags) :
+        expected_flags;
+
     // Search "created" name.
     {
         SCOPED_TRACE("Search at created child");
-        findTest(Name("a.wild.example.org"), RRType::A(), ZoneFinder::SUCCESS,
-                 false, rr_wild_,
-                 ZoneFinder::RESULT_WILDCARD | expected_flags, NULL,
-                 find_options, true);
-        // Wildcard match, but no data
-        findTest(Name("a.wild.example.org"), RRType::AAAA(),
-                 ZoneFinder::NXRRSET, true, ConstRRsetPtr(),
-                 ZoneFinder::RESULT_WILDCARD | expected_flags, NULL,
-                 find_options);
+        findTest(Name("a.wild.example.org"), RRType::A(),
+                 wild_ok ? ZoneFinder::SUCCESS : ZoneFinder::NXDOMAIN, false,
+                 wild_ok ? rr_wild_ : expected_nsec,
+                 wild_expected_flags, NULL, find_options, wild_ok);
     }
 
     // Search name that has CNAME.
     {
         SCOPED_TRACE("Matching CNAME");
         findTest(Name("a.cnamewild.example.org"), RRType::A(),
-                 ZoneFinder::CNAME, false, rr_cnamewild_,
-                 ZoneFinder::RESULT_WILDCARD | expected_flags, NULL,
-                 find_options, true);
+                 wild_ok ? ZoneFinder::CNAME : ZoneFinder::NXDOMAIN, false,
+                 wild_ok ? rr_cnamewild_ : expected_nsec,
+                 wild_expected_flags, NULL, find_options, wild_ok);
     }
 
     // Search another created name, this time little bit lower
     {
         SCOPED_TRACE("Search at created grand-child");
         findTest(Name("a.b.wild.example.org"), RRType::A(),
-                 ZoneFinder::SUCCESS, false, rr_wild_,
-                 ZoneFinder::RESULT_WILDCARD | expected_flags, NULL,
-                 find_options, true);
+                 wild_ok ? ZoneFinder::SUCCESS : ZoneFinder::NXDOMAIN, false,
+                 wild_ok ? rr_wild_ : expected_nsec,
+                 wild_expected_flags, NULL, find_options, wild_ok);
     }
 
     EXPECT_EQ(SUCCESS, zone_finder_.add(rr_under_wild_));
     {
         SCOPED_TRACE("Search under non-wildcard");
-        if ((expected_flags & ZoneFinder::RESULT_NSEC_SIGNED) != 0) {
-            // For the test setup of "nsec-signed" zone, there's actually
-            // only one NSEC; the one at the apex.  So find() will return it
-            // as the covering NSEC.
-            findTest(Name("bar.foo.wild.example.org"), RRType::A(),
-                     ZoneFinder::NXDOMAIN, true, rr_nsec_, expected_flags,
-                     NULL, find_options);
-        } else {
-            findTest(Name("bar.foo.wild.example.org"), RRType::A(),
-                     ZoneFinder::NXDOMAIN, true, ConstRRsetPtr(),
-                     expected_flags, NULL, find_options);
-        }
+        findTest(Name("bar.foo.wild.example.org"), RRType::A(),
+                 ZoneFinder::NXDOMAIN, true, expected_nsec, expected_flags,
+                 NULL, find_options);
+    }
+
+    // Wildcard match, but no data.  We add the additional NSEC at the wildcard
+    // at this point so that it wouldn't break other tests above.  Note also
+    // that in the NO_WILDCARD case the resulting NSEC is the same.  Ideally
+    // we could use a more tricky setup so we can distinguish these cases,
+    // but for this purpose it's not bad; what we'd like to test here is that
+    // wildcard substitution doesn't happen for either case, and the
+    // NO_WILDCARD effect itself can be checked by the result code (NXDOMAIN).
+    ConstRRsetPtr expected_wild_nsec; // by default it's NULL
+    if ((expected_flags & ZoneFinder::RESULT_NSEC_SIGNED) != 0) {
+        EXPECT_EQ(SUCCESS, zone_finder_.add(rr_wild_nsec_));
+        expected_wild_nsec = rr_wild_nsec_;
+    }
+    {
+        SCOPED_TRACE("Search at wildcard, no data");
+        findTest(Name("a.wild.example.org"), RRType::AAAA(),
+                 wild_ok ? ZoneFinder::NXRRSET : ZoneFinder::NXDOMAIN, true,
+                 wild_ok ? expected_wild_nsec : expected_wild_nsec,
+                 wild_expected_flags, NULL, find_options);
     }
 }
 
@@ -1390,10 +1455,22 @@ TEST_F(InMemoryZoneFinderTest, wildcardNSEC3) {
 }
 
 TEST_F(InMemoryZoneFinderTest, wildcardNSEC) {
-    // Similar to the previous one, but the zone signed with NSEC
+    // Similar to the previous one, but the zone is signed with NSEC
     wildcardCheck(ZoneFinder::RESULT_NSEC_SIGNED);
 }
 
+TEST_F(InMemoryZoneFinderTest, wildcardDisabledWithNSEC) {
+    // Wildcard is disabled.  In practice, this is used as part of query
+    // processing for an NSEC-signed zone, so we test that case specifically.
+    wildcardCheck(ZoneFinder::RESULT_NSEC_SIGNED, ZoneFinder::NO_WILDCARD);
+}
+
+TEST_F(InMemoryZoneFinderTest, wildcardDisabledWithoutNSEC) {
+    // Similar to the previous once, but check the behavior for a non signed
+    // zone just in case.
+    wildcardCheck(ZoneFinder::RESULT_DEFAULT, ZoneFinder::NO_WILDCARD);
+}
+
 /*
  * Test that we don't match a wildcard if we get under delegation.
  * By 4.3.3 of RFC1034:
@@ -1597,15 +1674,29 @@ TEST_F(InMemoryZoneFinderTest, nestedEmptyWildcard) {
 // situations
 void
 InMemoryZoneFinderTest::doCancelWildcardCheck(
-    ZoneFinder::FindResultFlags expected_flags)
+    ZoneFinder::FindResultFlags expected_flags,
+    ZoneFinder::FindOptions find_options)
 {
     // These should be canceled
     {
         SCOPED_TRACE("Canceled under foo.wild.example.org");
+
+        // For an NSEC-signed zone with DNSSEC requested, the covering NSEC
+        // should be returned.  The expected NSEC is actually just the only
+        // NSEC in the test data, but in this context it doesn't matter;
+        // it's sufficient just to check any NSEC is returned (or not).
+        ConstRRsetPtr expected_nsec; // by default it's NULL
+        if ((expected_flags & ZoneFinder::RESULT_NSEC_SIGNED) != 0 &&
+            (find_options & ZoneFinder::FIND_DNSSEC)) {
+            expected_nsec = rr_nsec_;
+        }
+
         findTest(Name("aaa.foo.wild.example.org"), RRType::A(),
-                 ZoneFinder::NXDOMAIN, true, ConstRRsetPtr(), expected_flags);
+                 ZoneFinder::NXDOMAIN, true, expected_nsec, expected_flags,
+                 NULL, find_options);
         findTest(Name("zzz.foo.wild.example.org"), RRType::A(),
-                 ZoneFinder::NXDOMAIN, true, ConstRRsetPtr(), expected_flags);
+                 ZoneFinder::NXDOMAIN, true, expected_nsec, expected_flags,
+                 NULL, find_options);
     }
 
     // This is existing, non-wildcard domain, shouldn't wildcard at all
@@ -1673,6 +1764,7 @@ TEST_F(InMemoryZoneFinderTest, cancelWildcard) {
     }
 }
 
+// Same tests as cancelWildcard for NSEC3-signed zone
 TEST_F(InMemoryZoneFinderTest, cancelWildcardNSEC3) {
     EXPECT_EQ(SUCCESS, zone_finder_.add(rr_wild_));
     EXPECT_EQ(SUCCESS, zone_finder_.add(rr_not_wild_));
@@ -1689,6 +1781,29 @@ TEST_F(InMemoryZoneFinderTest, cancelWildcardNSEC3) {
     }
 }
 
+// Same tests as cancelWildcard for NSEC-signed zone.  Check both cases with
+// or without FIND_DNSSEC option.  NSEC should be returned only when the option
+// is given.
+TEST_F(InMemoryZoneFinderTest, cancelWildcardNSEC) {
+    EXPECT_EQ(SUCCESS, zone_finder_.add(rr_wild_));
+    EXPECT_EQ(SUCCESS, zone_finder_.add(rr_not_wild_));
+    EXPECT_EQ(SUCCESS, zone_finder_.add(rr_nsec_));
+
+    {
+        SCOPED_TRACE("Runnig with single entry under foo.wild.example.org");
+        doCancelWildcardCheck(ZoneFinder::RESULT_NSEC_SIGNED,
+                              ZoneFinder::FIND_DNSSEC);
+        doCancelWildcardCheck(ZoneFinder::RESULT_NSEC_SIGNED);
+    }
+    EXPECT_EQ(SUCCESS, zone_finder_.add(rr_not_wild_another_));
+    {
+        SCOPED_TRACE("Runnig with two entries under foo.wild.example.org");
+        doCancelWildcardCheck(ZoneFinder::RESULT_NSEC_SIGNED,
+                              ZoneFinder::FIND_DNSSEC);
+        doCancelWildcardCheck(ZoneFinder::RESULT_NSEC_SIGNED);
+    }
+}
+
 TEST_F(InMemoryZoneFinderTest, loadBadWildcard) {
     // We reject loading the zone if it contains a wildcard name for
     // NS or DNAME.