Corrected NSEC3 logic. When returning NXDOMAIN for a node that isn't

directly under the zone apex (i.e., a.b.c.d.e.foo.com), we need to return
the NSEC3 covering the closest enclosing name, not for the zone name
itself.  We also need to avoid sending multiple copies of the same NSEC3.


git-svn-id: svn://bind10.isc.org/svn/bind10/trunk@1184 e5f2f494-b856-4b98-b285-d166d9295462

src/lib/auth/TODO

@@ -1,6 +1,2 @@
 - change filenames so we don't have everything starting with "data_source_"?
 - store rdata in the database as binary blobs instead of text
-- correct NSEC3 logic:
-  - closest encloser proof is incorrect; need to send covering NSEC3
-    for the "next closest" name, not necessarily for the name itself
-  - need to check for duplication in the resulting NSEC3's

src/lib/auth/data_source.cc

@@ -293,18 +293,18 @@ addNSEC(Query& q, const QueryTaskPtr task, const Name& name,
 }
 
 static inline DataSrc::Result
-addNSEC3(const string& hash, Query& q, const DataSrc* ds, const Name& zonename)
+getNsec3(Query& q, const DataSrc* ds, const Name& zonename, string& hash, 
+         RRsetPtr target)
 {
-    RRsetList nsec3;
-    Message& m = q.message();
     DataSrc::Result result;
+    RRsetList rl;
 
-    result = ds->findCoveringNSEC3(q, hash, zonename, nsec3);
+    result = ds->findCoveringNSEC3(q, zonename, hash, rl);
     if (result != DataSrc::SUCCESS) {
-        return (DataSrc::ERROR);
+        return (result);
     }
 
-    m.addRRset(Section::AUTHORITY(), nsec3[RRType::NSEC3()], true);
+    target = rl[RRType::NSEC3()];
     return (DataSrc::SUCCESS);
 }
 
@@ -344,31 +344,60 @@ getNsec3Param(Query& q, const DataSrc* ds, const Name& zonename) {
 static inline DataSrc::Result
 proveNX(Query& q, QueryTaskPtr task, const DataSrc* ds, const Name& zonename)
 {
+    Message& m = q.message();
     DataSrc::Result result;
+
     ConstNsec3ParamPtr nsec3 = getNsec3Param(q, ds, zonename);
     if (nsec3 != NULL) {
-        string node(nsec3->getHash(task->qname));
-        result = addNSEC3(node, q, ds, zonename);
+        // Attach the NSEC3 record covering the QNAME
+        RRsetPtr rrset;
+        string hash1(nsec3->getHash(task->qname));
+        result = getNsec3(q, ds, zonename, hash1, rrset);
         if (result != DataSrc::SUCCESS) {
             return (result);
         }
+        m.addRRset(Section::AUTHORITY(), rrset, true);
+
+        // If this is an NXRRSET or NOERROR/NODATA, we're done
+        if ((task->flags & DataSrc::TYPE_NOT_FOUND) != 0) {
+            return (DataSrc::SUCCESS);
+        }
+
+        // Find the closest provable enclosing name for QNAME
+        Name enclosure(zonename);
+        int nlen = task->qname.getLabelCount();
+        int diff = nlen - enclosure.getLabelCount();
+        for (int i = 1; i <= diff; ++i) {
+            enclosure = task->qname.split(i, nlen - i);
+            string nodehash(nsec3->getHash(enclosure));
+            if (nodehash == hash1) {
+                break;
+            }
+            hash2 = nodehash;
+            RRsetList rl;
 
-        string apex(nsec3->getHash(zonename));
-        if (node != apex) {
-            result = addNSEC3(apex, q, ds, zonename);
+            // hash2 will be overwritten with the actual hash found;
+            // we don't want to use one until we find an exact match
+            result = getNsec3(q, ds, zonename, hash2, rrset);
             if (result != DataSrc::SUCCESS) {
-                return (result);
+                return (DataSrc::ERROR);
+            }
+
+            if (hash2 == nodehash) {
+                m.addRRset(Section::AUTHORITY(), rrset, true);
+                break;
             }
         }
 
-        if ((task->flags & DataSrc::NAME_NOT_FOUND) != 0) {
-            string wild(nsec3->getHash(Name("*").concatenate(zonename)));
-            if (node != wild) {
-                result = addNSEC3(wild, q, ds, zonename);
-                if (result != DataSrc::SUCCESS) {
-                    return (result);
-                }
+        // Now add a covering NSEC3 for a wildcard under the
+        // closest provable enclosing name
+        string hash3(nsec3->getHash(Name("*").concatenate(enclosure)));
+        if (wild != hash1 && wild != hash2) {
+            result = getNsec3(q, ds, zonename, wild, rrset);
+            if (result != DataSrc::SUCCESS) {
+                return (result);
             }
+            m.addRRset(Section::AUTHORITY(), rrset, true);
         }
     } else {
         Name nsecname(task->qname);

src/lib/auth/data_source.h

@@ -146,8 +146,8 @@ public:
    // This MUST be implemented by concrete data sources which support
    // NSEC3, but is optional for others
    virtual Result findCoveringNSEC3(const Query& q,
-                                    const std::string& hash,
                                     const isc::dns::Name& zonename,
+                                    std::string& hash,
                                     isc::dns::RRsetList& target) const = 0;
 };
 
@@ -215,8 +215,8 @@ public:
                                     const isc::dns::Name* zonename) const = 0;
 
    virtual Result findCoveringNSEC3(const Query& q,
-                                    const std::string& hash,
                                     const isc::dns::Name& zonename,
+                                    std::string& hash,
                                     isc::dns::RRsetList& target) const = 0;
 
 private:
@@ -293,8 +293,8 @@ public:
     }
 
    virtual Result findCoveringNSEC3(const Query& q,
-                                    const std::string& qname,
                                     const isc::dns::Name& zonename,
+                                    std::string& hash,
                                     isc::dns::RRsetList& target) const
    {
        return (NOT_IMPLEMENTED);

src/lib/auth/data_source_sqlite3.cc

@@ -549,8 +549,8 @@ Sqlite3DataSrc::findPreviousName(const Query& q,
 
 DataSrc::Result
 Sqlite3DataSrc::findCoveringNSEC3(const Query& q,
-                                  const string& hashstr,
                                   const Name& zonename,
+                                  string& hashstr,
                                   RRsetList& target) const
 {
     int zone_id = findClosest(zonename.toText().c_str(), NULL);
@@ -614,7 +614,7 @@ Sqlite3DataSrc::findCoveringNSEC3(const Query& q,
                           flags) == 0 || flags != 0) {
         result = ERROR;
     }
-
+    hashstr = string(hash);
     sqlite3_reset(q_nsec3);
     return (result);
 }

src/lib/auth/data_source_sqlite3.h

@@ -89,8 +89,8 @@ public:
                                      const isc::dns::Name* zonename) const;
 
     Result findCoveringNSEC3(const Query& q,
-                             const std::string& hash,
                              const isc::dns::Name& zonename,
+                             std::string& hash,
                              isc::dns::RRsetList& target) const;
 
     Result init();

src/lib/auth/data_source_sqlite3_unittest.cc

@@ -677,8 +677,8 @@ TEST_F(Sqlite3DataSourceTest, findRRsetNSEC3) {
 
     const Name nsec3_zonename("sql2.example.com");
     EXPECT_EQ(DataSrc::SUCCESS,
-              data_source.findCoveringNSEC3(*query, hashstr, nsec3_zonename,
-                                            result_sets));
+              data_source.findCoveringNSEC3(*query, nsec3_zonename,
+                                            hashstr, result_sets));
     RRsetList::iterator it = result_sets.begin();
     checkRRset(*it, Name(hashstr).concatenate(nsec3_zonename), RRClass::IN(),
                RRType::NSEC3(), RRTTL(7200), nsec3_data, &nsec3_sig_data);

src/lib/auth/data_source_static.cc

@@ -175,8 +175,8 @@ StaticDataSrc::findPreviousName(const Query& q, const Name& qname,
 }
 
 DataSrc::Result
-StaticDataSrc::findCoveringNSEC3(const Query& q, const string& hash,
-                                 const Name& zonename, RRsetList& target) const
+StaticDataSrc::findCoveringNSEC3(const Query& q, const Name& zonename,
+                                 string& hash, RRsetList& target) const
 {
    return (NOT_IMPLEMENTED);
 }

src/lib/auth/data_source_static.h

@@ -82,8 +82,8 @@ public:
                             const isc::dns::Name* zonename) const;
 
    Result findCoveringNSEC3(const Query& q,
-                            const std::string& hash,
                             const isc::dns::Name& zonename,
+                            std::string& hash,
                             isc::dns::RRsetList& target) const;
 
     Result init();

src/lib/auth/unittest_ds.cc

@@ -775,8 +775,8 @@ TestDataSrc::findPreviousName(const Query& q,
 
 DataSrc::Result
 TestDataSrc::findCoveringNSEC3(const Query& q,
-                               const string& hash,
                                const Name& zonename,
+                               string& hash,
                                RRsetList& target) const
 {
     return (NOT_IMPLEMENTED);

src/lib/auth/unittest_ds.h

@@ -86,8 +86,8 @@ public:
                             const isc::dns::Name* zonename) const;
 
     Result findCoveringNSEC3(const Query& q,
-                             const std::string& hash,
                              const isc::dns::Name& zonename,
+                             std::string& hash,
                              isc::dns::RRsetList& target) const;
 
     Result init();