[1307] suppressed duplicate NSEC

src/bin/auth/query.cc

@@ -12,6 +12,7 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <algorithm>            // for std::max
 #include <vector>
 #include <boost/foreach.hpp>
 
@@ -105,6 +106,41 @@ Query::putSOA(ZoneFinder& zone) const {
 }
 
 void
+Query::addNXDOMAINProof(ZoneFinder& finder, ConstRRsetPtr nsec) const {
+    // TODO: Handle unexpected (buggy case): rrset is not NSEC
+
+    response_.addRRset( Message::SECTION_AUTHORITY,
+                        boost::const_pointer_cast<RRset>(nsec),
+                        dnssec_);
+    const int qlabels = qname_.getLabelCount();
+    const int olabels = qname_.compare(nsec->getName()).getCommonLabels();
+    // Extract NSEC's next domain
+    RdataIteratorPtr it = nsec->getRdataIterator();
+    const int nlabels = qname_.compare(
+        dynamic_cast<const generic::NSEC&>(it->getCurrent()).
+        getNextName()).getCommonLabels();
+    const int common_labels = std::max(olabels, nlabels);
+    const Name wildname(Name("*").concatenate(qname_.split(qlabels -
+                                                           common_labels)));
+    const ZoneFinder::FindResult fresult = finder.find(wildname,
+                                                       RRType::NSEC(), NULL,
+                                                       dnssec_opt_);
+
+    // TODO: check fresult: should be NXDOMAIN, and rrset is NSEC.
+
+    // Add the (no-) wildcard proof only when it's different from the NSEC
+    // that proves NXDOMAIN; sometimes they can be the same.
+    // Note: name comparison is relatively expensive.  When we are at the
+    // stage of performance optimization, we should consider optimizing this
+    // for some optimized data source implementations.
+    if (nsec->getName() != fresult.rrset->getName()) {
+        response_.addRRset(Message::SECTION_AUTHORITY,
+                           boost::const_pointer_cast<RRset>(fresult.rrset),
+                           dnssec_);
+    }
+}
+
+void
 Query::getAuthAdditional(ZoneFinder& zone) const {
     // Fill in authority and addtional sections.
     ZoneFinder::FindResult ns_result = zone.find(zone.getOrigin(),
@@ -250,33 +286,7 @@ Query::process() const {
 
                 // If DNSSEC proof is requested and we've got it, add it.
                 if (dnssec_ && db_result.rrset) {
-                    // TODO: Handle unexpected (buggy case): rrset is not NSEC
-
-                    response_.addRRset(
-                        Message::SECTION_AUTHORITY,
-                        boost::const_pointer_cast<RRset>(db_result.rrset),
-                        dnssec_);
-                    const int qlabels = qname_.getLabelCount();
-                    const int olabels = qname_.compare(
-                        db_result.rrset->getName()).getCommonLabels();
-                    // Extract NSEC's next domain
-                    RdataIteratorPtr it = db_result.rrset->getRdataIterator();
-                    const int nlabels = qname_.compare(
-                        dynamic_cast<const generic::NSEC&>(it->getCurrent()).
-                        getNextName()).getCommonLabels();
-                    const int common_labels = std::max(olabels, nlabels);
-                    const Name wildname(Name("*").concatenate(
-                                            qname_.split(qlabels -
-                                                         common_labels)));
-                    // TODO: check if we need NO_WILDCARD here. (we should do)
-                    const ZoneFinder::FindResult fresult =
-                        zfinder.find(wildname, RRType::NSEC(), NULL,
-                                     dnssec_opt_);
-                    // TODO: check fresult: should be NXDOMAIN, and rrset is NSEC.
-                    response_.addRRset(
-                        Message::SECTION_AUTHORITY,
-                        boost::const_pointer_cast<RRset>(fresult.rrset),
-                        dnssec_);
+                    addNXDOMAINProof(zfinder, db_result.rrset);
                 }
                 break;
             case ZoneFinder::NXRRSET:

src/bin/auth/query.h

@@ -71,6 +71,10 @@ private:
     ///
     void putSOA(isc::datasrc::ZoneFinder& zone) const;
 
+    /// TBD
+    void addNXDOMAINProof(isc::datasrc::ZoneFinder& finder,
+                          isc::dns::ConstRRsetPtr nsec) const;
+
     /// \brief Look up additional data (i.e., address records for the names
     /// included in NS or MX records).
     ///

src/bin/auth/tests/query_unittest.cc

@@ -106,7 +106,19 @@ const char* const nsec_apex_txt =
 const char* const nsec_mx_txt =
     "mx.example.com. 3600 IN NSEC ).no.example.com. MX NSEC RRSIG\n";
 const char* const nsec_no_txt =
-    ").no.example.com. 3600 IN NSEC noglue.example.com. AAAA NSEC RRSIG\n";
+    ").no.example.com. 3600 IN NSEC nz.no.example.com. AAAA NSEC RRSIG\n";
+
+// We'll also test the case where a single NSEC proves both NXDOMAIN and the
+// non existence of wildcard.  The following records will be used for that
+// test.
+// ).no.example.com. (exist, whose NSEC proves everything)
+// *.no.example.com. (best possible wildcard, not exist)
+// nx.no.example.com. (NXDOMAIN)
+// nz.no.example.com. (exist)
+const char* const nz_txt =
+    "nz.no.example.com. 3600 IN AAAA 2001:db8::5300\n";
+const char* const nsec_nz_txt =
+    "nz.no.example.com. 3600 IN NSEC noglue.example.com. AAAA NSEC RRSIG\n";
 const char* const nsec_nxdomain_txt =
     "noglue.example.com. 3600 IN NSEC www.example.com. A\n";
 
@@ -147,8 +159,9 @@ public:
         zone_stream << soa_txt << zone_ns_txt << ns_addrs_txt <<
             delegation_txt << mx_txt << www_a_txt << cname_txt <<
             cname_nxdom_txt << cname_out_txt << dname_txt << dname_a_txt <<
-            other_zone_rrs << no_txt <<
-            nsec_apex_txt << nsec_mx_txt << nsec_no_txt << nsec_nxdomain_txt;
+            other_zone_rrs << no_txt << nz_txt <<
+            nsec_apex_txt << nsec_mx_txt << nsec_no_txt << nsec_nz_txt <<
+            nsec_nxdomain_txt;
 
         masterLoad(zone_stream, origin_, rrclass_,
                    boost::bind(&MockZoneFinder::loadRRset, this, _1));
@@ -569,7 +582,6 @@ TEST_F(QueryTest, nxdomainWithNSEC2) {
     // the first NSEC.
     Query(memory_client, Name("(.no.example.com"), qtype,
           response, true).process();
-    cout << response.toText() << endl;
     responseCheck(response, Rcode::NXDOMAIN(), AA_FLAG, 0, 6, 0,
                   NULL, (string(soa_txt) +
                          string("example.com. 3600 IN RRSIG ") +
@@ -583,6 +595,21 @@ TEST_F(QueryTest, nxdomainWithNSEC2) {
                   NULL, mock_finder->getOrigin());
 }
 
+TEST_F(QueryTest, nxdomainWithNSECDuplicate) {
+    // See comments about nz_txt.  In this case we only need one NSEC,
+    // which proves both NXDOMAIN and the non existence of wildcard.
+    Query(memory_client, Name("nx.no.example.com"), qtype,
+          response, true).process();
+    responseCheck(response, Rcode::NXDOMAIN(), AA_FLAG, 0, 4, 0,
+                  NULL, (string(soa_txt) +
+                         string("example.com. 3600 IN RRSIG ") +
+                         getCommonRRSIGText("SOA") + "\n" +
+                         string(nsec_no_txt) + "\n" +
+                         string(").no.example.com. 3600 IN RRSIG ") +
+                         getCommonRRSIGText("NSEC")).c_str(),
+                  NULL, mock_finder->getOrigin());
+}
+
 TEST_F(QueryTest, nxrrset) {
     EXPECT_NO_THROW(Query(memory_client, Name("www.example.com"),
                           RRType::TXT(), response).process());