From: JINMEI Tatuya <jinmei@isc.org>
Date: Thu, 12 Apr 2012 21:25:41 +0000 (-0700)
Subject: [1579] suggested change: made findNoNameResult a bit more DNSSEC-agnostic.
X-Git-Tag: trac2351_base~226^2~116^2~41^2~8
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=07274f662a772c856f0bf80213b246e689582409;p=thirdparty%2Fkea.git

[1579] suggested change: made findNoNameResult a bit more DNSSEC-agnostic.

to make a related test pass, incorporated the NotImplemented exception
handling from findNSECCover to getDNSSECRRset.
note: in this commit I reversed the behavior when we encounter the situation
where NSEC is expected but cannot be found; it now returns a NULL pointer,
although the NSEC_SIGNED flag can be still set.

It's a quite broken case of zone config so it's probably up to the
implementation, and in that sense throwing an exception is not necessary
wrong.  But we may have such a situation in a valid scenario if and when we
support incremental zone signing.  Also, this behavior is consistent with
BIND 9.

The emptyNonterminalNSEC test case was adjusted accordingly.
---

diff --git a/src/lib/datasrc/database.cc b/src/lib/datasrc/database.cc
index dc3da71488..38a917c95f 100644
--- a/src/lib/datasrc/database.cc
+++ b/src/lib/datasrc/database.cc
@@ -780,16 +780,28 @@ DatabaseClient::Finder::FindDNSSECContext::getDNSSECRRset(const Name &name,
         return (ConstRRsetPtr());
     }
 
-    const Name& nsec_name = covering ? finder_.findPreviousName(name) : name;
-    const bool need_nscheck = (nsec_name == finder_.getOrigin());
-    const FoundRRsets found = finder_.getRRsets(nsec_name.toText(),
-                                                NSEC_TYPES(), need_nscheck);
-    const FoundIterator nci = found.second.find(RRType::NSEC());
-    if (nci != found.second.end()) {
-        return (nci->second);
-    } else {
-        return (ConstRRsetPtr());
+    try {
+        const Name& nsec_name =
+            covering ? finder_.findPreviousName(name) : name;
+        const bool need_nscheck = (nsec_name == finder_.getOrigin());
+        const FoundRRsets found = finder_.getRRsets(nsec_name.toText(),
+                                                    NSEC_TYPES(),
+                                                    need_nscheck);
+        const FoundIterator nci = found.second.find(RRType::NSEC());
+        if (nci != found.second.end()) {
+            return (nci->second);
+        }
+    } catch (const isc::NotImplemented&) {
+        // This happens when the underlying database accessor doesn't support
+        // findPreviousName() (it probably doesn't support DNSSEC at all) but
+        // there is somehow an NSEC RR at the zone apex.  We log the fact but
+        // otherwise let the caller decide what to do (so, for example, a
+        // higher level query processing won't completely fail but can return
+        // anything it can get).
+        LOG_INFO(logger, DATASRC_DATABASE_COVER_NSEC_UNSUPPORTED).
+            arg(finder_.accessor_->getDBName()).arg(name);
     }
+    return (ConstRRsetPtr());
 }
 
 ZoneFinder::FindResultFlags
@@ -924,13 +936,8 @@ DatabaseClient::Finder::findNoNameResult(const Name& name, const RRType& type,
         LOG_DEBUG(logger, DBG_TRACE_DETAILED,
                   DATASRC_DATABASE_FOUND_EMPTY_NONTERMINAL).
             arg(accessor_->getDBName()).arg(name);
-        const ConstRRsetPtr nsec = dnssec_ctx.isNSEC() ? findNSECCover(name) :
-            ConstRRsetPtr();
-        if (dnssec_ctx.isNSEC() && !nsec) {
-            isc_throw(DataSourceError,
-                      "no NSEC RR covers in the NSEC signed zone");
-        }
-        return (ResultContext(NXRRSET, nsec, dnssec_ctx.getResultFlags()));
+        return (ResultContext(NXRRSET, dnssec_ctx.getDNSSECRRset(name, true),
+                              dnssec_ctx.getResultFlags()));
     } else if ((options & NO_WILDCARD) == 0) {
         // It's not an empty non-terminal and wildcard matching is not
         // disabled, so check for wildcards. If there is a wildcard match
diff --git a/src/lib/datasrc/tests/database_unittest.cc b/src/lib/datasrc/tests/database_unittest.cc
index 80607b532b..3e0d75ea7b 100644
--- a/src/lib/datasrc/tests/database_unittest.cc
+++ b/src/lib/datasrc/tests/database_unittest.cc
@@ -2471,14 +2471,13 @@ TYPED_TEST(DatabaseClientTest, emptyNonterminalNSEC) {
     if (!this->is_mock_) {
         return; // We don't make the real DB to throw
     }
-    // If the zone is signed with NSEC, find function should throw an error
-    // when no NSEC RRset for NXRRset case
-    EXPECT_THROW(doFindTest(*finder, Name("here.wild.example.org."),
-                            RRType::TXT(), RRType::NSEC(), this->rrttl_,
-                            ZoneFinder::NXRRSET, this->empty_rdatas_,
-                            this->empty_rdatas_, ZoneFinder::RESULT_DEFAULT,
-                            Name::ROOT_NAME(), ZoneFinder::FIND_DNSSEC),
-                 DataSourceError);
+    // In this case the accessor doesn't support findPreviousName(), but the
+    // zone apex has NSEC, and the zone itself is considered NSEC-signed.
+    doFindTest(*finder, Name("here.wild.example.org."),
+               RRType::TXT(), RRType::NSEC(), this->rrttl_,
+               ZoneFinder::NXRRSET, this->empty_rdatas_,
+               this->empty_rdatas_, ZoneFinder::RESULT_NSEC_SIGNED,
+               Name::ROOT_NAME(), ZoneFinder::FIND_DNSSEC);
 }
 
 TYPED_TEST(DatabaseClientTest, anyFromFind) {