[master] Merge branch 'trac1587'

src/bin/auth/query.cc

@@ -167,38 +167,65 @@ Query::addNXDOMAINProofByNSEC(ZoneFinder& finder, ConstRRsetPtr nsec) {
     }
 }
 
+uint8_t
+Query::addClosestEncloserProof(ZoneFinder& finder, const Name& name,
+                               bool exact_ok, bool add_closest)
+{
+    const ZoneFinder::FindNSEC3Result result = finder.findNSEC3(name, true);
+
+    // Validity check (see the method description).  Note that a completely
+    // broken findNSEC3 implementation could even return NULL RRset in
+    // closest_proof.  We don't explicitly check such case; addRRset() will
+    // throw an exception, and it will be converted to SERVFAIL at the caller.
+    if (!exact_ok && !result.next_proof) {
+        isc_throw(BadNSEC3, "Matching NSEC3 found for a non existent name: "
+                  << qname_);
+    }
+
+    if (add_closest) {
+        response_.addRRset(Message::SECTION_AUTHORITY,
+                           boost::const_pointer_cast<AbstractRRset>(
+                               result.closest_proof),
+                           dnssec_);
+    }
+    if (result.next_proof) {
+        response_.addRRset(Message::SECTION_AUTHORITY,
+                           boost::const_pointer_cast<AbstractRRset>(
+                               result.next_proof),
+                           dnssec_);
+    }
+    return (result.closest_labels);
+}
+
 void
-Query::addNXDOMAINProofByNSEC3(ZoneFinder& finder) {
-    // Firstly get the NSEC3 proves for Closest Encloser Proof
-    // See section 7.2.1 of RFC 5155.
-    // Since this is a Name Error case both closest and next proofs should
-    // be available (see addNXRRsetProof).
-    const ZoneFinder::FindNSEC3Result fresult1 = finder.findNSEC3(qname_,
-                                                                  true);
-    response_.addRRset(Message::SECTION_AUTHORITY,
-                       boost::const_pointer_cast<AbstractRRset>(
-                           fresult1.closest_proof),
-                       dnssec_);
+Query::addNSEC3ForName(ZoneFinder& finder, const Name& name, bool match) {
+    const ZoneFinder::FindNSEC3Result result = finder.findNSEC3(name, false);
+
+    // See the comment for addClosestEncloserProof().  We don't check a
+    // totally bogus case where closest_proof is NULL here.
+    if (match != result.matched) {
+        isc_throw(BadNSEC3, "Unexpected "
+                  << (result.matched ? "matching" : "covering")
+                  << " NSEC3 found for " << name);
+    }
     response_.addRRset(Message::SECTION_AUTHORITY,
                        boost::const_pointer_cast<AbstractRRset>(
-                       fresult1.next_proof),
+                           result.closest_proof),
                        dnssec_);
+}
+
+void
+Query::addNXDOMAINProofByNSEC3(ZoneFinder& finder) {
+    // Firstly get the NSEC3 proves for Closest Encloser Proof
+    // See Section 7.2.1 of RFC 5155.
+    const uint8_t closest_labels =
+        addClosestEncloserProof(finder, qname_, false);
 
     // Next, construct the wildcard name at the closest encloser, i.e.,
     // '*' followed by the closest encloser, and add NSEC3 for it.
     const Name wildname(Name("*").concatenate(
-               qname_.split(qname_.getLabelCount() -
-                            fresult1.closest_labels)));
-    const ZoneFinder::FindNSEC3Result fresult2 =
-        finder.findNSEC3(wildname, false);
-    if (fresult2.matched) {
-        isc_throw(BadNSEC3, "Matching NSEC3 found for nonexistent domain "
-                  << wildname);
-    }
-    response_.addRRset(Message::SECTION_AUTHORITY,
-                       boost::const_pointer_cast<AbstractRRset>(
-                           fresult2.closest_proof),
-                       dnssec_);
+               qname_.split(qname_.getLabelCount() - closest_labels)));
+    addNSEC3ForName(finder, wildname, false);
 }
 
 void
@@ -224,20 +251,13 @@ Query::addWildcardProof(ZoneFinder& finder,
                                fresult.rrset),
                            dnssec_);
     } else if (db_result.isNSEC3Signed()) {
-        // Case for RFC5155 Section 7.2.6.
+        // Case for RFC 5155 Section 7.2.6.
         //
         // Note that the closest encloser must be the immediate ancestor
-        // of the matching wildcard, so NSEC3 for its next closer is what
-        // we are expected to provided per the RFC (if this assumption isn't
-        // met the zone is broken anyway).
-        const ZoneFinder::FindNSEC3Result NSEC3Result(
-            finder.findNSEC3(qname_, true));
-        // Note that at this point next_proof must not be NULL unless it's
-        // a run time collision (or zone/findNSEC3() is broken).  The
-        // unexpected case will be caught in addRRset() and result in SERVFAIL.
-        response_.addRRset(Message::SECTION_AUTHORITY,
-                           boost::const_pointer_cast<AbstractRRset>(
-                               NSEC3Result.next_proof), dnssec_);
+        // of the matching wildcard, so NSEC3 for its next closer (and only
+        // that NSEC3) is what we are expected to provided per the RFC (if
+        // this assumption isn't met the zone is broken anyway).
+        addClosestEncloserProof(finder, qname_, false, false);
     }
 }
 
@@ -279,23 +299,8 @@ Query::addDS(ZoneFinder& finder, const Name& dname) {
         addNXRRsetProof(finder, ds_result);
     } else if (ds_result.code == ZoneFinder::NXRRSET &&
                ds_result.isNSEC3Signed()) {
-        // Add no DS proof with NSEC3 as specified in RFC5155 Section 7.2.7.
-        // Depending on whether the zone is optout or not, findNSEC3() may
-        // return non-NULL or NULL next_proof (respectively).  The Opt-Out flag
-        // must be set or cleared accordingly, but we don't check that
-        // in this level (as long as the zone signed validly and findNSEC3()
-        // is valid, the condition should be met; otherwise we'd let the
-        // validator detect the error).
-        const ZoneFinder::FindNSEC3Result nsec3_result =
-            finder.findNSEC3(dname, true);
-        response_.addRRset(Message::SECTION_AUTHORITY,
-                           boost::const_pointer_cast<AbstractRRset>(
-                               nsec3_result.closest_proof), dnssec_);
-        if (nsec3_result.next_proof) {
-            response_.addRRset(Message::SECTION_AUTHORITY,
-                               boost::const_pointer_cast<AbstractRRset>(
-                                   nsec3_result.next_proof), dnssec_);
-        }
+        // Add no DS proof with NSEC3 as specified in RFC 5155 Section 7.2.7.
+        addClosestEncloserProof(finder, dname, true);
     } else {
         // Any other case should be an error
         isc_throw(BadDS, "Unexpected result for DS lookup for delegation");
@@ -315,57 +320,22 @@ Query::addNXRRsetProof(ZoneFinder& finder,
             addWildcardNXRRSETProof(finder, db_result.rrset);
         }
     } else if (db_result.isNSEC3Signed() && !db_result.isWildcard()) {
-        // Handling depends on whether query type is DS or not
-        // (see RFC5155, 7.2.3 and 7.2.4):  If qtype == DS, do
-        // recursive search (and add next_proof, if necessary),
-        // otherwise, do non-recursive search
-        const bool qtype_ds = (qtype_ == RRType::DS());
-        ZoneFinder::FindNSEC3Result result(finder.findNSEC3(qname_, qtype_ds));
-        if (result.matched) {
-            response_.addRRset(Message::SECTION_AUTHORITY,
-                               boost::const_pointer_cast<AbstractRRset>(
-                                   result.closest_proof), dnssec_);
-            // For qtype == DS, next_proof could be set
-            // (We could check for opt-out here, but that's really the
-            // responsibility of the datasource)
-            if (qtype_ds && result.next_proof != ConstRRsetPtr()) {
-                response_.addRRset(Message::SECTION_AUTHORITY,
-                                   boost::const_pointer_cast<AbstractRRset>(
-                                       result.next_proof), dnssec_);
-            }
+        if (qtype_ == RRType::DS()) {
+            // RFC 5155, Section 7.2.4.  Add either NSEC3 for the qname or
+            // closest (provable) encloser proof in case of optout.
+            addClosestEncloserProof(finder, qname_, true);
         } else {
-            isc_throw(BadNSEC3, "No matching NSEC3 found for existing domain "
-                      << qname_);
+            // RFC 5155, Section 7.2.3.  Just add NSEC3 for the qname.
+            addNSEC3ForName(finder, qname_, true);
         }
     } else if (db_result.isNSEC3Signed() && db_result.isWildcard()) {
-        // Case for RFC5155 Section 7.2.5
-        const ZoneFinder::FindNSEC3Result result(finder.findNSEC3(qname_,
-                                                                  true));
-        // We know there's no exact match for the qname, so findNSEC3() should
-        // return both closest and next proofs.  If the latter is NULL, it
-        // means a run time collision (or the zone is broken in other way).
-        // In that case addRRset() will throw, and it will be converted to
-        // SERVFAIL.
-        response_.addRRset(Message::SECTION_AUTHORITY,
-                           boost::const_pointer_cast<AbstractRRset>(
-                               result.closest_proof), dnssec_);
-        response_.addRRset(Message::SECTION_AUTHORITY,
-                           boost::const_pointer_cast<AbstractRRset>(
-                               result.next_proof), dnssec_);
-
-        // Construct the matched wildcard name and add NSEC3 for it.
+        // Case for RFC 5155 Section 7.2.5: add closest encloser proof for the
+        // qname, construct the matched wildcard name and add NSEC3 for it.
+        const uint8_t closest_labels =
+            addClosestEncloserProof(finder, qname_, false);
         const Name wname = Name("*").concatenate(
-            qname_.split(qname_.getLabelCount() - result.closest_labels));
-        const ZoneFinder::FindNSEC3Result wresult(finder.findNSEC3(wname,
-                                                                   false));
-        if (wresult.matched) {
-            response_.addRRset(Message::SECTION_AUTHORITY,
-                               boost::const_pointer_cast<AbstractRRset>(
-                                   wresult.closest_proof), dnssec_);
-        } else {
-            isc_throw(BadNSEC3, "No matching NSEC3 found for existing domain "
-                      << wname);
-        }
+            qname_.split(qname_.getLabelCount() - closest_labels));
+        addNSEC3ForName(finder, wname, true);
     }
 }
 

src/bin/auth/query.h

@@ -204,6 +204,62 @@ private:
     /// within this method.
     bool processDSAtChild();
 
+    /// \brief Add NSEC3 to the response for a closest encloser proof for a
+    /// given name.
+    ///
+    /// This method calls \c findNSEC3() of the given zone finder for the
+    /// given name in the recursive mode, and adds the returned NSEC3(s) to
+    /// the authority section of the response message associated with the
+    /// \c Query object.
+    ///
+    /// It returns the number of labels of the closest encloser (returned via
+    /// the \c findNSEC3() call) in case the caller needs to use that value
+    /// for subsequent processing, i.e, constructing the best possible wildcard
+    /// name that (would) match the query name.
+    ///
+    /// Unless \c exact_ok is true, \c name is expected to be non existent,
+    /// in which case findNSEC3() in the recursive mode must return both
+    /// closest and next proofs.  If the latter is NULL, it means a run time
+    /// collision (or the zone is broken in other way), and this method throws
+    /// a BadNSEC3 exception.
+    ///
+    /// If \c exact_ok is true, this method takes into account the case
+    /// where the name exists and may or may not be at a zone cut to an
+    /// optout zone.  In this case, depending on whether the zone is optout
+    /// or not, findNSEC3() may return non-NULL or NULL next_proof
+    /// (respectively).  This method adds the next proof if and only if
+    /// findNSEC3() returns non NULL value for it.  The Opt-Out flag
+    /// must be set or cleared accordingly, but this method doesn't check that
+    /// in this level (as long as the zone is signed validly and findNSEC3()
+    /// for the data source is implemented as documented, the condition
+    /// should be met; otherwise we'd let the validator detect the error).
+    ///
+    /// By default this method always adds the closest proof.
+    /// If \c add_closest is false, it only adds the next proof to the message.
+    /// This correspond to the case of "wildcard answer responses" as described
+    /// in Section 7.2.6 of RFC5155.
+    uint8_t addClosestEncloserProof(isc::datasrc::ZoneFinder& finder,
+                                    const isc::dns::Name& name, bool exact_ok,
+                                    bool add_closest = true);
+
+    /// \brief Add matching or covering NSEC3 to the response for a give name.
+    ///
+    /// This method calls \c findNSEC3() of the given zone finder for the
+    /// given name in the non recursive mode, and adds the returned NSEC3 to
+    /// the authority section of the response message associated with the
+    /// \c Query object.
+    ///
+    /// Depending on the caller's context, the returned NSEC3 is one and
+    /// only one of matching or covering NSEC3.  If \c match is true the
+    /// returned NSEC3 must be a matching one; otherwise it must be a covering
+    /// one.  If this assumption isn't met this method throws a BadNSEC3
+    /// exception (if it must be a matching NSEC3 but is not, it means a broken
+    /// zone, maybe with incorrect optout NSEC3s; if it must be a covering
+    /// NSEC3 but is not, it means a run time collision; or the \c findNSEC3()
+    /// implementation is broken for both cases.)
+    void addNSEC3ForName(isc::datasrc::ZoneFinder& finder,
+                         const isc::dns::Name& name, bool match);
+
 public:
     /// Constructor from query parameters.
     ///

src/bin/auth/tests/query_unittest.cc

@@ -1429,7 +1429,7 @@ TEST_F(QueryTest, badWildcardNSEC3) {
 
     EXPECT_THROW(Query(memory_client, Name("www.wild.example.com"),
                        RRType::A(), response, true).process(),
-                 isc::InvalidParameter);
+                 Query::BadNSEC3);
 }
 
 TEST_F(QueryTest, badWildcardProof1) {
@@ -1540,10 +1540,9 @@ TEST_F(QueryTest, wildcardNxrrsetWithNSEC3Collision) {
                                       ConstRRsetPtr());
     mock_finder->setNSEC3Result(&nsec3);
 
-    // Message::addRRset() will detect it and throw InvalidParameter.
     EXPECT_THROW(Query(memory_client, Name("www1.uwild.example.com"),
                        RRType::TXT(), response, true).process(),
-                 isc::InvalidParameter);
+                 Query::BadNSEC3);
 }
 
 TEST_F(QueryTest, wildcardNxrrsetWithNSEC3Broken) {
@@ -2283,10 +2282,9 @@ TEST_F(QueryTest, nxdomainWithBadNextNSEC3Proof) {
                                       ConstRRsetPtr());
     mock_finder->setNSEC3Result(&nsec3);
 
-    // Message::addRRset() will detect it and throw InvalidParameter.
     EXPECT_THROW(Query(memory_client, Name("nxdomain.example.com"),
                        RRType::TXT(), response, true).process(),
-                 isc::InvalidParameter);
+                 Query::BadNSEC3);
 }
 
 TEST_F(QueryTest, nxdomainWithBadWildcardNSEC3Proof) {