BIND 10 trac1570, updated. 53cde03994472c09094721f94307f46599c0d1d5 [1570] a supplemental check to the in-memory data source about DS lookup: confirm DS query at a grandparent results in normal delegation. (the code was already correct, just checking it explicitly)

BIND 10 source code commits bind10-changes at lists.isc.org
Fri Feb 3 00:12:34 UTC 2012


The branch, trac1570 has been updated
       via  53cde03994472c09094721f94307f46599c0d1d5 (commit)
       via  a092343db9338c7e6acfb238d8c432291ad31e58 (commit)
       via  f364574080cd8f9d1a2acb27ddb3318157393ebc (commit)
       via  efd87c8cddb380d78a0cef4ee7fcc91559f7352b (commit)
       via  3007ba9ef2cffdf4903b43a14ccf52a0c1d3ec65 (commit)
       via  afc721fe5f2964c9d03bdf0738852556c69ad41b (commit)
       via  0d1406f71c9cd9439017b56957fa9d37ba7729cb (commit)
       via  32d9c527a98f1e417d21799d982361892ca87c6c (commit)
       via  47bca52baed667a6788450e1a0912c7a06363c8c (commit)
       via  00de2fb5e636d527a7d1a7bf97a6e4552d84b1f0 (commit)
       via  5fbff1bd2b79b417955e53dba3e3788ff32ebd6a (commit)
       via  e16deb4665c50c5232cec96ae2bd69348610c926 (commit)
       via  ddbf6fe0f244d0c808408e94b502bc462c5d2f41 (commit)
      from  848e9d9759c84f822a6d866477016b5a95415dfc (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 53cde03994472c09094721f94307f46599c0d1d5
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Thu Feb 2 16:10:51 2012 -0800

    [1570] a supplemental check to the in-memory data source about DS lookup:
    confirm DS query at a grandparent results in normal delegation.
    (the code was already correct, just checking it explicitly)

commit a092343db9338c7e6acfb238d8c432291ad31e58
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Thu Feb 2 15:55:57 2012 -0800

    [1570] added doc for processDSAtChild()

commit f364574080cd8f9d1a2acb27ddb3318157393ebc
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Thu Feb 2 15:49:34 2012 -0800

    [1570] tested the (very add) case for ./DS and DS exists in the root zone.
    added some more comments.

commit efd87c8cddb380d78a0cef4ee7fcc91559f7352b
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Thu Feb 2 15:08:29 2012 -0800

    [1570] added comments about the previous case

commit 3007ba9ef2cffdf4903b43a14ccf52a0c1d3ec65
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Thu Feb 2 15:06:45 2012 -0800

    [1570] added another grandparent case: the server is auth of the child, too.

commit afc721fe5f2964c9d03bdf0738852556c69ad41b
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Thu Feb 2 14:18:16 2012 -0800

    [1570] addd a test case for a DS query at a grandparent zone.  no need for
    updating the implementation.

commit 0d1406f71c9cd9439017b56957fa9d37ba7729cb
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Wed Feb 1 23:40:47 2012 -0800

    [1570] supported the case of DS query at child. dsBelowDelegation now passed.
    added a couple of more related tests.

commit 32d9c527a98f1e417d21799d982361892ca87c6c
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Wed Feb 1 17:21:01 2012 -0800

    [1570] added a real test for "above delegation" that fails and fixed it
    in the implementation.

commit 47bca52baed667a6788450e1a0912c7a06363c8c
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Wed Feb 1 15:55:35 2012 -0800

    [1570] corrected test parameters for dsAboveDelegation so it passes.
    dsBelowDelegation is disabled for now.

commit 00de2fb5e636d527a7d1a7bf97a6e4552d84b1f0
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Wed Feb 1 15:37:32 2012 -0800

    [1570] a bit more refactoring: DS case could have been handled as normal cases.

commit 5fbff1bd2b79b417955e53dba3e3788ff32ebd6a
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Wed Feb 1 15:23:36 2012 -0800

    [1570] intermediate (unrelated) refactoring on test: unify delegation cases.
    now that we have multiple test delegations, it would make sense to introduce
    some generalization.

commit e16deb4665c50c5232cec96ae2bd69348610c926
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Wed Feb 1 13:30:51 2012 -0800

    [1570] cleanup: removed redudancy in test zone setup

commit ddbf6fe0f244d0c808408e94b502bc462c5d2f41
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Wed Feb 1 13:28:32 2012 -0800

    [1570] remove DS from the apex of the test zone to avoid regression
    (A DS normally doesn't exist there.)

-----------------------------------------------------------------------

Summary of changes:
 src/bin/auth/query.cc                            |   76 ++++-
 src/bin/auth/query.h                             |   17 +
 src/bin/auth/tests/query_unittest.cc             |  406 +++++++++++++++-------
 src/lib/datasrc/tests/memory_datasrc_unittest.cc |    4 +-
 4 files changed, 369 insertions(+), 134 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/auth/query.cc b/src/bin/auth/query.cc
index 88624b8..62e4b82 100644
--- a/src/bin/auth/query.cc
+++ b/src/bin/auth/query.cc
@@ -257,13 +257,27 @@ Query::addAuthAdditional(ZoneFinder& finder) {
     }
 }
 
+namespace {
+// A simple wrapper for DataSourceClient::findZone().  Normally we can simply
+// check the closest zone to the qname, but for type DS query we need to
+// look into the parent zone.  Nevertheless, if there is no "parent" (i.e.,
+// the qname consists of a single label, which also means it's the root name),
+// we should search the deepest zone we have (which should be the root zone;
+// otherwise it's a query error).
+DataSourceClient::FindResult
+findZone(const DataSourceClient& client, const Name& qname, RRType qtype) {
+    if (qtype != RRType::DS() || qname.getLabelCount() == 1) {
+        return (client.findZone(qname));
+    }
+    return (client.findZone(qname.split(1)));
+}
+}
+
 void
 Query::process() {
-    const bool qtype_is_any = (qtype_ == RRType::ANY());
-
-    response_.setHeaderFlag(Message::HEADERFLAG_AA, false);
-    const DataSourceClient::FindResult result =
-        datasrc_client_.findZone(qname_);
+    // Found a zone which is the nearest ancestor to QNAME
+    const DataSourceClient::FindResult result = findZone(datasrc_client_,
+                                                         qname_, qtype_);
 
     // If we have no matching authoritative zone for the query name, return
     // REFUSED.  In short, this is to be compatible with BIND 9, but the
@@ -272,16 +286,26 @@ Query::process() {
     // https://lists.isc.org/mailman/htdig/bind10-dev/2010-December/001633.html
     if (result.code != result::SUCCESS &&
         result.code != result::PARTIALMATCH) {
+        // If we tried to find a "parent zone" for a DS query and failed,
+        // we may still have authority at the child side.  If we do, the query
+        // has to be handled there.
+        if (qtype_ == RRType::DS() && qname_.getLabelCount() > 1 &&
+            processDSAtChild()) {
+            return;
+        }
+        response_.setHeaderFlag(Message::HEADERFLAG_AA, false);
         response_.setRcode(Rcode::REFUSED());
         return;
     }
     ZoneFinder& zfinder = *result.zone_finder;
 
-    // Found a zone which is the nearest ancestor to QNAME, set the AA bit
+    // We have authority for a zone that contain the query name (possibly
+    // indirectly via delegation).  Look into the zone.
     response_.setHeaderFlag(Message::HEADERFLAG_AA);
     response_.setRcode(Rcode::NOERROR());
     std::vector<ConstRRsetPtr> target;
     boost::function0<ZoneFinder::FindResult> find;
+    const bool qtype_is_any = (qtype_ == RRType::ANY());
     if (qtype_is_any) {
         find = boost::bind(&ZoneFinder::findAll, &zfinder, qname_,
                            boost::ref(target), dnssec_opt_);
@@ -389,6 +413,14 @@ Query::process() {
             }
             break;
         case ZoneFinder::DELEGATION:
+            // If a DS query resulted in delegation, we also need to check
+            // if we are an authority of the child, too.  If so, we need to
+            // complete the process in the child as specified in Section
+            // 2.2.1.2. of RFC3658.
+            if (qtype_ == RRType::DS() && processDSAtChild()) {
+                return;
+            }
+
             response_.setHeaderFlag(Message::HEADERFLAG_AA, false);
             response_.addRRset(Message::SECTION_AUTHORITY,
                 boost::const_pointer_cast<RRset>(db_result.rrset),
@@ -422,5 +454,37 @@ Query::process() {
     }
 }
 
+bool
+Query::processDSAtChild() {
+    const DataSourceClient::FindResult zresult =
+        datasrc_client_.findZone(qname_);
+
+    if (zresult.code != result::SUCCESS) {
+        return (false);
+    }
+
+    // We are receiving a DS query at the child side of the owner name,
+    // where the DS isn't supposed to belong.  We should return a "no data"
+    // response as described in Section 3.1.4.1 of RFC4035 and Section
+    // 2.2.1.1 of RFC 3658.  find(DS) should result in NXRRSET, in which
+    // case (and if DNSSEC is required) we also add the proof for that,
+    // but even if find() returns an unexpected result, we don't bother.
+    // The important point in this case is to return SOA so that the resolver
+    // that happens to contact us can hunt for the appropriate parent zone
+    // by seeing the SOA.
+    response_.setHeaderFlag(Message::HEADERFLAG_AA);
+    response_.setRcode(Rcode::NOERROR());
+    addSOA(*zresult.zone_finder);
+    const ZoneFinder::FindResult ds_result =
+        zresult.zone_finder->find(qname_, RRType::DS(), dnssec_opt_);
+    if (ds_result.code == ZoneFinder::NXRRSET) {
+        if (dnssec_) {
+            addNXRRsetProof(*zresult.zone_finder, ds_result);
+        }
+    }
+
+    return (true);
+}
+
 }
 }
diff --git a/src/bin/auth/query.h b/src/bin/auth/query.h
index 886ff93..a2dd21e 100644
--- a/src/bin/auth/query.h
+++ b/src/bin/auth/query.h
@@ -178,6 +178,23 @@ private:
     /// data for the query are to be found.
     void addAuthAdditional(isc::datasrc::ZoneFinder& finder);
 
+    /// \brief Process a DS query possible at the child side of zone cut.
+    ///
+    /// This private method is a subroutine of process(), and is called
+    /// there's a possibility that this server has authority for the child
+    /// side of the DS's owner name (and it's detected that the server at
+    /// least doesn't have authority at the parent side).  This method
+    /// first checks if it has authority for the child, and if does,
+    /// just build a "no data" response with SOA for the zone origin
+    /// (possibly with a proof for the no data) as specified in Section
+    /// 2.2.1.1 of RFC3658.
+    ///
+    /// It returns true if this server has authority of the child; otherwise
+    /// it returns false.  In the former case, the caller is expected to
+    /// terminate the query processing, because it should have been completed
+    /// within this method.
+    bool processDSAtChild();
+
 public:
     /// Constructor from query parameters.
     ///
diff --git a/src/bin/auth/tests/query_unittest.cc b/src/bin/auth/tests/query_unittest.cc
index a70dc9e..4832ccd 100644
--- a/src/bin/auth/tests/query_unittest.cc
+++ b/src/bin/auth/tests/query_unittest.cc
@@ -202,13 +202,34 @@ getCommonRRSIGText(const string& type) {
                    "example.com. FAKEFAKEFAKE"));
 }
 
+// A helper callback of masterLoad() used in InMemoryZoneFinderTest.
+void
+setRRset(RRsetPtr rrset, vector<RRsetPtr*>::iterator& it) {
+    *(*it) = rrset;
+    ++it;
+}
+
+// A helper function that converts a textual form of a single RR into a
+// RRsetPtr object.  If it's SOA, origin must be set to its owner name;
+// otherwise masterLoad() will reject it.
+RRsetPtr
+textToRRset(const string& text_rrset, const Name& origin = Name::ROOT_NAME()) {
+    stringstream ss(text_rrset);
+    RRsetPtr rrset;
+    vector<RRsetPtr*> rrsets;
+    rrsets.push_back(&rrset);
+    masterLoad(ss, origin, RRClass::IN(),
+               boost::bind(setRRset, _1, rrsets.begin()));
+    return (rrset);
+}
+
 // This is a mock Zone Finder class for testing.
 // It is a derived class of ZoneFinder for the convenient of tests.
 // Its find() method emulates the common behavior of protocol compliant
 // ZoneFinder classes, but simplifies some minor cases and also supports broken
 // behavior.
-// For simplicity, most names are assumed to be "in zone"; there's only
-// one zone cut at the point of name "delegation.example.com".
+// For simplicity, most names are assumed to be "in zone"; delegations
+// to child zones are identified by the existence of non origin NS records.
 // Another special name is "dname.example.com".  Query names under this name
 // will result in DNAME.
 // This mock zone doesn't handle empty non terminal nodes (if we need to test
@@ -217,10 +238,7 @@ class MockZoneFinder : public ZoneFinder {
 public:
     MockZoneFinder() :
         origin_(Name("example.com")),
-        delegation_name_("delegation.example.com"),
-        signed_delegation_name_("signed-delegation.example.com"),
         bad_signed_delegation_name_("bad-delegation.example.com"),
-        unsigned_delegation_name_("unsigned-delegation.example.com"),
         dname_name_("dname.example.com"),
         has_SOA_(true),
         has_apex_NS_(true),
@@ -230,9 +248,7 @@ public:
         nsec_name_(origin_)
     {
         stringstream zone_stream;
-        zone_stream << soa_txt << zone_ns_txt << zone_ds_txt << ns_addrs_txt <<
-            delegation_txt << mx_txt << www_a_txt << cname_txt <<
-            cname_nxdom_txt << cname_out_txt << dname_txt <<
+        zone_stream << soa_txt << zone_ns_txt << ns_addrs_txt <<
             delegation_txt << delegation_ds_txt << mx_txt << www_a_txt <<
             cname_txt << cname_nxdom_txt << cname_out_txt << dname_txt <<
             dname_a_txt << other_zone_rrs << no_txt << nz_txt <<
@@ -306,24 +322,47 @@ public:
     // answers when DNSSEC is required.
     void setNSEC3Flag(bool on) { use_nsec3_ = on; }
 
-    Name findPreviousName(const Name&) const {
+    virtual Name findPreviousName(const Name&) const {
         isc_throw(isc::NotImplemented, "Mock doesn't support previous name");
     }
 
+    // This method allows tests to insert new record in the middle of the test.
+    //
+    // \param record_txt textual RR representation of RR (such as soa_txt, etc)
+    void addRecord(const string& record_txt) {
+        stringstream record_stream;
+        record_stream << record_txt;
+        masterLoad(record_stream, origin_, rrclass_,
+                   boost::bind(&MockZoneFinder::loadRRset, this, _1));
+    }
+
 public:
     // We allow the tests to use these for convenience
-    ConstRRsetPtr delegation_rrset_;
-    ConstRRsetPtr signed_delegation_rrset_;
-    ConstRRsetPtr signed_delegation_ds_rrset_;
-    ConstRRsetPtr bad_signed_delegation_rrset_;
-    ConstRRsetPtr unsigned_delegation_rrset_;
+    ConstRRsetPtr dname_rrset_; // could be used as an arbitrary bogus RRset
     ConstRRsetPtr empty_nsec_rrset_;
 
 private:
     typedef map<RRType, ConstRRsetPtr> RRsetStore;
     typedef map<Name, RRsetStore> Domains;
     Domains domains_;
+    Domains delegations_;
     Domains nsec3_domains_;
+
+    // This is used to identify delegation to a child zone, and used to
+    // find a matching entry in delegations_.  Note that first found entry
+    // is returned, so it's not a longest match.  Test data must be set up
+    // to ensure the first match is always the longest match.
+    struct SubdomainMatch {
+        SubdomainMatch(const Name& name) : name_(name) {}
+        bool operator()(const pair<Name, RRsetStore>& domain_elem) const {
+            return (name_ == domain_elem.first ||
+                    name_.compare(domain_elem.first).getRelation() ==
+                    NameComparisonResult::SUBDOMAIN);
+        }
+    private:
+        const Name& name_;
+    };
+
     void loadRRset(RRsetPtr rrset) {
         if (rrset->getType() == RRType::NSEC3()) {
             // NSEC3 should go to the dedicated table
@@ -337,39 +376,24 @@ private:
             return;
         }
         domains_[rrset->getName()][rrset->getType()] = rrset;
-        if (rrset->getName() == delegation_name_ &&
-            rrset->getType() == RRType::NS()) {
-            delegation_rrset_ = rrset;
-        } else if (rrset->getName() == signed_delegation_name_ &&
-                   rrset->getType() == RRType::NS()) {
-            signed_delegation_rrset_ = rrset;
-        } else if (rrset->getName() == bad_signed_delegation_name_ &&
-                   rrset->getType() == RRType::NS()) {
-            bad_signed_delegation_rrset_ = rrset;
-        } else if (rrset->getName() == unsigned_delegation_name_ &&
-                   rrset->getType() == RRType::NS()) {
-            unsigned_delegation_rrset_ = rrset;
-        } else if (rrset->getName() == signed_delegation_name_ &&
-                   rrset->getType() == RRType::DS()) {
-            signed_delegation_ds_rrset_ = rrset;
-            // Like NSEC(3), by nature it should have an RRSIG.
-            rrset->addRRsig(RdataPtr(new generic::RRSIG(
-                                         getCommonRRSIGText(rrset->getType().
-                                                            toText()))));
+
+        // Remember delegation (NS/DNAME) related RRsets separately.
+        if (rrset->getType() == RRType::NS() && rrset->getName() != origin_) {
+            delegations_[rrset->getName()][rrset->getType()] = rrset;
         } else if (rrset->getName() == dname_name_ &&
-            rrset->getType() == RRType::DNAME()) {
+                   rrset->getType() == RRType::DNAME()) {
             dname_rrset_ = rrset;
-        // Add some signatures
-        } else if (rrset->getName() == Name("example.com.") &&
-                   rrset->getType() == RRType::NS()) {
-            // For NS, we only have RRSIG for the origin name.
-            rrset->addRRsig(RdataPtr(new generic::RRSIG(
-                                         getCommonRRSIGText("NS"))));
-        } else {
-            // For others generate RRSIG unconditionally.  Technically this
-            // is wrong because we shouldn't have it for names under a zone
-            // cut.  But in our tests that doesn't matter, so we add them
-            // just for simplicity.
+        }
+
+        // Add some signatures.  For NS, we only have RRSIG for the origin
+        // name. For others generate RRSIG unconditionally.  Technically this
+        // is wrong because we shouldn't have it for names under a zone
+        // cut.  But in our tests that doesn't matter, so we add them
+        // just for simplicity.
+        // Note that this includes RRSIG for DS with secure delegations.
+        // They should have RRSIGs, so that's actually expected data, not just
+        // for simplicity.
+        if (rrset->getType() != RRType::NS() || rrset->getName() == origin_) {
             rrset->addRRsig(RdataPtr(new generic::RRSIG(
                                          getCommonRRSIGText(rrset->getType().
                                                             toText()))));
@@ -378,14 +402,10 @@ private:
 
     const Name origin_;
     // Names where we delegate somewhere else
-    const Name delegation_name_;
-    const Name signed_delegation_name_;
     const Name bad_signed_delegation_name_;
-    const Name unsigned_delegation_name_;
     const Name dname_name_;
     bool has_SOA_;
     bool has_apex_NS_;
-    ConstRRsetPtr dname_rrset_;
     const RRClass rrclass_;
     bool include_rrsig_anyway_;
     bool use_nsec3_;
@@ -491,40 +511,23 @@ MockZoneFinder::find(const Name& name, const RRType& type,
         return (FindResult(NXDOMAIN, RRsetPtr()));
     }
 
-    // Special case for names on or under a zone cut
+    // Special case for names on or under a zone cut and under DNAME
+    Domains::iterator it;
     if ((options & FIND_GLUE_OK) == 0 &&
-        (name == delegation_name_ ||
-         name.compare(delegation_name_).getRelation() ==
-         NameComparisonResult::SUBDOMAIN)) {
-        return (FindResult(DELEGATION, delegation_rrset_));
-    // And under DNAME
-    } else if (name.compare(dname_name_).getRelation() ==
-        NameComparisonResult::SUBDOMAIN) {
-        if (type != RRType::DS()) {
-            return (FindResult(DNAME, dname_rrset_));
-        }
-    } else if (name == signed_delegation_name_ ||
-               name.compare(signed_delegation_name_).getRelation() ==
-               NameComparisonResult::SUBDOMAIN) {
-        if (type != RRType::DS()) {
-            return (FindResult(DELEGATION, signed_delegation_rrset_));
-        } else {
-            return (FindResult(SUCCESS, signed_delegation_ds_rrset_));
+        (it = find_if(delegations_.begin(), delegations_.end(),
+                      SubdomainMatch(name))) != delegations_.end()) {
+        ConstRRsetPtr delegation_ns = it->second[RRType::NS()];
+        assert(delegation_ns); // should be ensured by how we construct it
+
+        // DS query for the delegated domain (i.e. an exact match) will be
+        // handled just like an in-zone case below.  Others result in
+        // DELEGATION.
+        if (type != RRType::DS() || it->first != name) {
+            return (FindResult(DELEGATION, delegation_ns));
         }
-    } else if (name == unsigned_delegation_name_ ||
-               name.compare(unsigned_delegation_name_).getRelation() ==
-               NameComparisonResult::SUBDOMAIN) {
-        if (type != RRType::DS()) {
-            return (FindResult(DELEGATION, unsigned_delegation_rrset_));
-        }
-    } else if (name == bad_signed_delegation_name_ ||
-               name.compare(bad_signed_delegation_name_).getRelation() ==
+    } else if (name.compare(dname_name_).getRelation() ==
                NameComparisonResult::SUBDOMAIN) {
-        if (type != RRType::DS()) {
-            return (FindResult(DELEGATION, bad_signed_delegation_rrset_));
-        } else {
-            return (FindResult(NXDOMAIN, RRsetPtr()));
-        }
+        return (FindResult(DNAME, dname_rrset_));
     }
 
     // normal cases.  names are searched for only per exact-match basis
@@ -563,7 +566,13 @@ MockZoneFinder::find(const Name& name, const RRType& type,
             return (FindResult(CNAME, found_rrset->second));
         }
 
-        // Otherwise it's NXRRSET case.
+        // Otherwise it's NXRRSET case...
+        // ...but a special pathological case first:
+        if (found_domain->first == bad_signed_delegation_name_ &&
+            type == RRType::DS()) {
+            return (FindResult(NXDOMAIN, RRsetPtr()));
+        }
+        // normal cases follow.
         if ((options & FIND_DNSSEC) != 0) {
             if (use_nsec3_) {
                 return (FindResult(NXRRSET, RRsetPtr(), RESULT_NSEC3_SIGNED));
@@ -729,7 +738,14 @@ protected:
     QueryTest() :
         qname(Name("www.example.com")), qclass(RRClass::IN()),
         qtype(RRType::A()), response(Message::RENDER),
-        qid(response.getQid()), query_code(Opcode::QUERY().getCode())
+        qid(response.getQid()), query_code(Opcode::QUERY().getCode()),
+        ns_addrs_and_sig_txt(string(ns_addrs_txt) +
+                             "glue.delegation.example.com. 3600 IN RRSIG " +
+                             getCommonRRSIGText("A") + "\n" +
+                             "glue.delegation.example.com. 3600 IN RRSIG " +
+                             getCommonRRSIGText("AAAA") + "\n" +
+                             "noglue.example.com. 3600 IN RRSIG " +
+                             getCommonRRSIGText("A"))
     {
         response.setRcode(Rcode::NOERROR());
         response.setOpcode(Opcode::QUERY());
@@ -749,6 +765,7 @@ protected:
     Message response;
     const qid_t qid;
     const uint16_t query_code;
+    const string ns_addrs_and_sig_txt; // convenient shortcut
 };
 
 // A wrapper to check resulting response message commonly used in
@@ -821,10 +838,6 @@ TEST_F(QueryTest, dnssecPositive) {
     Query query(memory_client, qname, qtype, response, true);
     EXPECT_NO_THROW(query.process());
     // find match rrset
-    // We can't let responseCheck to check the additional section as well,
-    // it gets confused by the two RRs for glue.delegation.../RRSIG due
-    // to it's design and fixing it would be hard. Therefore we simply
-    // check manually this one time.
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 2, 4, 6,
                   (www_a_txt + std::string("www.example.com. 3600 IN RRSIG "
                                            "A 5 3 3600 20000101000000 "
@@ -834,27 +847,8 @@ TEST_F(QueryTest, dnssecPositive) {
                                              "3 3600 20000101000000 "
                                              "20000201000000 12345 "
                                              "example.com. FAKEFAKEFAKE\n")).
-                  c_str(), NULL);
-    RRsetIterator iterator(response.beginSection(Message::SECTION_ADDITIONAL));
-    const char* additional[] = {
-        "glue.delegation.example.com. 3600 IN A 192.0.2.153\n",
-        "glue.delegation.example.com. 3600 IN RRSIG A 5 3 3600 20000101000000 "
-            "20000201000000 12345 example.com. FAKEFAKEFAKE\n",
-        "glue.delegation.example.com. 3600 IN AAAA 2001:db8::53\n",
-        "glue.delegation.example.com. 3600 IN RRSIG AAAA 5 3 3600 "
-            "20000101000000 20000201000000 12345 example.com. FAKEFAKEFAKE\n",
-        "noglue.example.com. 3600 IN A 192.0.2.53\n",
-        "noglue.example.com. 3600 IN RRSIG A 5 3 3600 20000101000000 "
-            "20000201000000 12345 example.com. FAKEFAKEFAKE\n",
-        NULL
-    };
-    for (const char** rr(additional); *rr != NULL; ++ rr) {
-        ASSERT_FALSE(iterator ==
-                     response.endSection(Message::SECTION_ADDITIONAL));
-        EXPECT_EQ(*rr, (*iterator)->toText());
-        iterator ++;
-    }
-    EXPECT_TRUE(iterator == response.endSection(Message::SECTION_ADDITIONAL));
+                  c_str(),
+                  ns_addrs_and_sig_txt.c_str());
 }
 
 TEST_F(QueryTest, exactAddrMatch) {
@@ -1055,7 +1049,7 @@ TEST_F(QueryTest, nxdomainBadNSEC1) {
     // ZoneFinder::find() returns NXDOMAIN with non NSEC RR.
     mock_finder->setNSECResult(Name("badnsec.example.com"),
                                ZoneFinder::NXDOMAIN,
-                               mock_finder->delegation_rrset_);
+                               mock_finder->dname_rrset_);
     EXPECT_THROW(Query(memory_client, Name("badnsec.example.com"), qtype,
                        response, true).process(),
                  std::bad_cast);
@@ -1075,7 +1069,7 @@ TEST_F(QueryTest, nxdomainBadNSEC3) {
     // "no-wildcard proof" returns SUCCESS.  it should be NXDOMAIN.
     mock_finder->setNSECResult(Name("*.example.com"),
                                ZoneFinder::SUCCESS,
-                               mock_finder->delegation_rrset_);
+                               mock_finder->dname_rrset_);
     EXPECT_THROW(Query(memory_client, Name("nxdomain.example.com"), qtype,
                        response, true).process(),
                  Query::BadNSEC);
@@ -1094,18 +1088,20 @@ TEST_F(QueryTest, nxdomainBadNSEC5) {
     // "no-wildcard proof" returns non NSEC.
     mock_finder->setNSECResult(Name("*.example.com"),
                                ZoneFinder::NXDOMAIN,
-                               mock_finder->delegation_rrset_);
+                               mock_finder->dname_rrset_);
     // This is a bit odd, but we'll simply include the returned RRset.
     Query(memory_client, Name("nxdomain.example.com"), qtype,
           response, true).process();
-    responseCheck(response, Rcode::NXDOMAIN(), AA_FLAG, 0, 8, 0,
+    responseCheck(response, Rcode::NXDOMAIN(), AA_FLAG, 0, 6, 0,
                   NULL, (string(soa_txt) +
                          string("example.com. 3600 IN RRSIG ") +
                          getCommonRRSIGText("SOA") + "\n" +
                          string(nsec_nxdomain_txt) + "\n" +
                          string("noglue.example.com. 3600 IN RRSIG ") +
                          getCommonRRSIGText("NSEC") + "\n" +
-                         delegation_txt).c_str(),
+                         dname_txt + "\n" +
+                         string("dname.example.com. 3600 IN RRSIG ") +
+                         getCommonRRSIGText("DNAME")).c_str(),
                   NULL, mock_finder->getOrigin());
 }
 
@@ -1217,7 +1213,7 @@ TEST_F(QueryTest, badWildcardProof1) {
     // when NXDOMAIN is expected.
     mock_finder->setNSECResult(Name("www.wild.example.com"),
                                ZoneFinder::SUCCESS,
-                               mock_finder->delegation_rrset_);
+                               mock_finder->dname_rrset_);
     EXPECT_THROW(Query(memory_client, Name("www.wild.example.com"),
                        RRType::A(), response, true).process(),
                  Query::BadNSEC);
@@ -1634,25 +1630,92 @@ TEST_F(QueryTest, findNSEC3) {
                mock_finder->findNSEC3(Name("nxdomain3.example.com"), false));
 }
 
-// TODO: Check the additional/authority sections are correct. The first one
-// probably misses some of the RRSigs anyway, they need to be added.
-
 // This tests that the DS is returned above the delegation point as
 // an authoritative answer, not a delegation. This is as described in
 // RFC 4035, section 3.1.4.1.
+
+// This mock finder is used for some DS-query tests to support the cases
+// where the query is expected to be handled in a different zone than our
+// main test zone, example.com.  Only limited methods are expected to called
+// (and for limited purposes) on this class object in these tests, which
+// are overridden below.
+class AlternateZoneFinder : public MockZoneFinder {
+public:
+    // This zone is expected not to have a DS by default and return NXRRSET
+    // for a DS query.  If have_ds is set to true on construction, it will
+    // return a faked DS answer.
+    AlternateZoneFinder(const Name& origin, bool have_ds = false) :
+        MockZoneFinder(), origin_(origin), have_ds_(have_ds)
+    {}
+    virtual isc::dns::Name getOrigin() const { return (origin_); }
+    virtual FindResult find(const isc::dns::Name&,
+                            const isc::dns::RRType& type,
+                            const FindOptions)
+    {
+        if (type == RRType::SOA()) {
+            RRsetPtr soa = textToRRset(origin_.toText() + " 3600 IN SOA . . "
+                                       "0 0 0 0 0\n", origin_);
+            soa->addRRsig(RdataPtr(new generic::RRSIG(
+                                       getCommonRRSIGText("SOA"))));
+            return (FindResult(SUCCESS, soa));
+        }
+        if (type == RRType::NS()) {
+            RRsetPtr ns = textToRRset(origin_.toText() + " 3600 IN NS " +
+                                      Name("ns").concatenate(origin_).toText());
+            ns->addRRsig(RdataPtr(new generic::RRSIG(
+                                      getCommonRRSIGText("NS"))));
+            return (FindResult(SUCCESS, ns));
+        }
+        if (type == RRType::DS()) {
+            if (have_ds_) {
+                RRsetPtr ds = textToRRset(origin_.toText() +
+                                          " 3600 IN DS 57855 5 1 " +
+                                          "49FD46E6C4B45C55D4AC69CBD"
+                                          "3CD34AC1AFE51DE");
+                ds->addRRsig(RdataPtr(new generic::RRSIG(
+                                          getCommonRRSIGText("DS"))));
+                return (FindResult(SUCCESS, ds));
+            } else {
+                RRsetPtr nsec = textToRRset(origin_.toText() +
+                                            " 3600 IN NSEC " +
+                                            origin_.toText() +
+                                            " SOA NSEC RRSIG");
+                nsec->addRRsig(RdataPtr(new generic::RRSIG(
+                                            getCommonRRSIGText("NSEC"))));
+                return (FindResult(NXRRSET, nsec, RESULT_NSEC_SIGNED));
+            }
+        }
+
+        // Returning NXDOMAIN is not correct, but doesn't matter for our tests.
+        return (FindResult(NXDOMAIN, ConstRRsetPtr()));
+    }
+private:
+    const Name origin_;
+    const bool have_ds_;
+};
+
 TEST_F(QueryTest, dsAboveDelegation) {
+    // Pretending to have authority for the child zone, too.
+    memory_client.addZone(ZoneFinderPtr(new AlternateZoneFinder(
+                                            Name("delegation.example.com"))));
+
+    // The following will succeed only if the search goes to the parent
+    // zone, not the child one we added above.
     EXPECT_NO_THROW(Query(memory_client, Name("delegation.example.com"),
                           RRType::DS(), response, true).process());
 
-    responseCheck(response, Rcode::NOERROR(), AA_FLAG, 1, 3, 2,
-                  delegation_ds_txt,
-                  zone_ns_txt,
-                  "glue.delegation.example.com. 3600 IN A 192.0.2.153\n"
-                  "glue.delegation.example.com. 3600 IN AAAA 2001:db8::53\n");
+    responseCheck(response, Rcode::NOERROR(), AA_FLAG, 2, 4, 6,
+                  (string(delegation_ds_txt) + "\n" +
+                   "delegation.example.com. 3600 IN RRSIG " +
+                   getCommonRRSIGText("DS")).c_str(),
+                  (string(zone_ns_txt) + "\n" +
+                   "example.com. 3600 IN RRSIG " +
+                   getCommonRRSIGText("NS")).c_str(),
+                  ns_addrs_and_sig_txt.c_str());
 }
 
-// This one checks a DS record at the apex is not returned even if it exists,
-// as it is authoritative above the delegation and does not exist below it,
+// This one checks a DS record at the apex is not returned, as it is
+// authoritative above the delegation and does not exist below it,
 // as described in RFC 4035, section 3.1.4.1. The example is inspired by the
 // B.8. example from the RFC.
 TEST_F(QueryTest, dsBelowDelegation) {
@@ -1662,10 +1725,99 @@ TEST_F(QueryTest, dsBelowDelegation) {
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 4, 0, NULL,
                   (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
                    getCommonRRSIGText("SOA") + "\n" +
-                   string(nsec_www_txt) + "\n" +
-                   string("www.example.com. 3600 IN RRSIG ") +
-                   getCommonRRSIGText("NSEC")).c_str(),
-                  NULL, mock_finder->getOrigin());
+                   string(nsec_apex_txt) + "\n" +
+                   string("example.com. 3600 IN RRSIG ") +
+                   getCommonRRSIGText("NSEC")).c_str(), NULL,
+                  mock_finder->getOrigin());
+}
+
+// Similar to the previous case, but even more pathological: the DS somehow
+// exists in the child zone.  The Query module should still return SOA.
+// In our implementation NSEC/NSEC3 isn't attached in this case.
+TEST_F(QueryTest, dsBelowDelegationWithDS) {
+    mock_finder->addRecord(zone_ds_txt); // add the DS to the child's apex
+    EXPECT_NO_THROW(Query(memory_client, Name("example.com"),
+                          RRType::DS(), response, true).process());
+
+    responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 2, 0, NULL,
+                  (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
+                   getCommonRRSIGText("SOA")).c_str(), NULL,
+                  mock_finder->getOrigin());
+}
+
+// DS query received at a completely irrelevant (neither parent nor child)
+// server.  It should just like the "noZone" test case, but DS query involves
+// special processing, so we test it explicitly.
+TEST_F(QueryTest, dsNoZone) {
+    Query(memory_client, Name("example"), RRType::DS(), response,
+          true).process();
+    responseCheck(response, Rcode::REFUSED(), 0, 0, 0, 0, NULL, NULL, NULL);
+}
+
+// DS query for a "grandchild" zone.  This should be result in normal
+// delegation (unless this server also has authority of grandchild zone).
+TEST_F(QueryTest, dsAtGrandParent) {
+    Query(memory_client, Name("grand.delegation.example.com"), RRType::DS(),
+          response, true).process();
+    responseCheck(response, Rcode::NOERROR(), 0, 0, 6, 6, NULL,
+                  (string(delegation_txt) + string(delegation_ds_txt) +
+                   "delegation.example.com. 3600 IN RRSIG " +
+                   getCommonRRSIGText("DS")).c_str(),
+                  ns_addrs_and_sig_txt.c_str());
+}
+
+// DS query for a "grandchild" zone, and the server has authority of the
+// child zone, too.  In this case the query should be handled in the child
+// side and should result in no data with SOA.
+TEST_F(QueryTest, dsAtGrandParentAndChild) {
+    // Pretending to have authority for the grandchild zone, too.
+    const Name childname("grand.delegation.example.com");
+    memory_client.addZone(ZoneFinderPtr(
+                              new AlternateZoneFinder(childname)));
+    Query(memory_client, childname, RRType::DS(), response, true).process();
+    responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 4, 0, NULL,
+                  (childname.toText() + " 3600 IN SOA . . 0 0 0 0 0\n" +
+                   childname.toText() + " 3600 IN RRSIG " +
+                   getCommonRRSIGText("SOA") + "\n" +
+                   childname.toText() + " 3600 IN NSEC " +
+                   childname.toText() + " SOA NSEC RRSIG\n" +
+                   childname.toText() + " 3600 IN RRSIG " +
+                   getCommonRRSIGText("NSEC")).c_str(), NULL, childname);
+}
+
+// DS query for the root name (quite pathological).  Since there's no "parent",
+// the query will be handled in the root zone anyway, and should (normally)
+// result in no data.
+TEST_F(QueryTest, dsAtRoot) {
+    // Pretend to be a root server.
+    memory_client.addZone(ZoneFinderPtr(
+                              new AlternateZoneFinder(Name::ROOT_NAME())));
+    Query(memory_client, Name::ROOT_NAME(), RRType::DS(), response,
+          true).process();
+    responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 4, 0, NULL,
+                  (string(". 3600 IN SOA . . 0 0 0 0 0\n") +
+                   ". 3600 IN RRSIG " + getCommonRRSIGText("SOA") + "\n" +
+                   ". 3600 IN NSEC " + ". SOA NSEC RRSIG\n" +
+                   ". 3600 IN RRSIG " +
+                   getCommonRRSIGText("NSEC")).c_str(), NULL);
+}
+
+// Even more pathological case: A faked root zone actually has its own DS
+// query.  How we respond wouldn't matter much in practice, but check if
+// it behaves as it's intended.  This implementation should return the DS.
+TEST_F(QueryTest, dsAtRootWithDS) {
+    memory_client.addZone(ZoneFinderPtr(
+                              new AlternateZoneFinder(Name::ROOT_NAME(),
+                                                      true)));
+    Query(memory_client, Name::ROOT_NAME(), RRType::DS(), response,
+          true).process();
+    responseCheck(response, Rcode::NOERROR(), AA_FLAG, 2, 2, 0,
+                  (string(". 3600 IN DS 57855 5 1 49FD46E6C4B45C55D4AC69CBD"
+                          "3CD34AC1AFE51DE\n") +
+                   ". 3600 IN RRSIG " + getCommonRRSIGText("DS")).c_str(),
+                  (string(". 3600 IN NS ns.\n") +
+                   ". 3600 IN RRSIG " + getCommonRRSIGText("NS")).c_str(),
+                  NULL);
 }
 
 // The following are tentative tests until we really add tests for the
diff --git a/src/lib/datasrc/tests/memory_datasrc_unittest.cc b/src/lib/datasrc/tests/memory_datasrc_unittest.cc
index c841cb9..e243a7b 100644
--- a/src/lib/datasrc/tests/memory_datasrc_unittest.cc
+++ b/src/lib/datasrc/tests/memory_datasrc_unittest.cc
@@ -697,11 +697,13 @@ TEST_F(InMemoryZoneFinderTest, delegationWithDS) {
     EXPECT_EQ(SUCCESS, zone_finder_.add(rr_child_ds_));
 
     // Normal types of query should result in delegation, but DS query
-    // should be considered in-zone.
+    // should be considered in-zone (but only exactly at the delegation point).
     findTest(Name("child.example.org"), RRType::A(), ZoneFinder::DELEGATION,
              true, rr_child_ns_);
     findTest(Name("child.example.org"), RRType::DS(), ZoneFinder::SUCCESS,
              true, rr_child_ds_);
+    findTest(Name("grand.child.example.org"), RRType::DS(),
+             ZoneFinder::DELEGATION, true, rr_child_ns_);
 
     // There's nothing special for DS query at the zone apex.  It would
     // normally result in NXRRSET.




More information about the bind10-changes mailing list