merge #464(Additional processing for authoritative NS), which is a sub-task of authoritative query logic, skip changelog.

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

src/bin/auth/query.cc

@@ -28,9 +28,7 @@ namespace isc {
 namespace auth {
 
 void
-Query::getAdditional(const isc::datasrc::Zone& zone,
-                     const isc::dns::RRset& rrset) const
-{
+Query::getAdditional(const Zone& zone, const RRset& rrset) const {
     if (rrset.getType() == RRType::NS()) {
         // Need to perform the search in the "GLUE OK" mode.
         RdataIteratorPtr rdata_iterator = rrset.getRdataIterator();
@@ -43,9 +41,8 @@ Query::getAdditional(const isc::datasrc::Zone& zone,
 }
 
 void
-Query::findAddrs(const isc::datasrc::Zone& zone,
-                 const isc::dns::Name& qname,
-                 const isc::datasrc::Zone::FindOptions options) const
+Query::findAddrs(const Zone& zone, const Name& qname,
+                 const Zone::FindOptions options) const
 {
     // Out of zone name
     NameComparisonResult result = zone.getOrigin().compare(qname);
@@ -53,17 +50,31 @@ Query::findAddrs(const isc::datasrc::Zone& zone,
         (result.getRelation() != NameComparisonResult::EQUAL))
         return;
 
+    // Omit additional data which has already been provided in the answer
+    // section from the additional.
+    //
+    // All the address rrset with the owner name of qname have been inserted
+    // into ANSWER section.
+    if (qname_ == qname && qtype_ == RRType::ANY())
+        return;
+
     // Find A rrset
-    Zone::FindResult a_result = zone.find(qname, RRType::A(), options);
-    if (a_result.code == Zone::SUCCESS) {
-        response_.addRRset(Message::SECTION_ADDITIONAL,
-                     boost::const_pointer_cast<RRset>(a_result.rrset));
+    if (qname_ != qname || qtype_ != RRType::A()) {
+        Zone::FindResult a_result = zone.find(qname, RRType::A(), options);
+        if (a_result.code == Zone::SUCCESS) {
+            response_.addRRset(Message::SECTION_ADDITIONAL,
+                    boost::const_pointer_cast<RRset>(a_result.rrset));
+        }
     }
+
     // Find AAAA rrset
-    Zone::FindResult aaaa_result = zone.find(qname, RRType::AAAA(), options);
-    if (aaaa_result.code == Zone::SUCCESS) {
-        response_.addRRset(Message::SECTION_ADDITIONAL,
-                     boost::const_pointer_cast<RRset>(aaaa_result.rrset));
+    if (qname_ != qname || qtype_ != RRType::AAAA()) {
+        Zone::FindResult aaaa_result =
+            zone.find(qname, RRType::AAAA(), options);
+        if (aaaa_result.code == Zone::SUCCESS) {
+            response_.addRRset(Message::SECTION_ADDITIONAL,
+                    boost::const_pointer_cast<RRset>(aaaa_result.rrset));
+        }
     }
 }
 
@@ -86,6 +97,21 @@ Query::putSOA(const Zone& zone) const {
 }
 
 void
+Query::getAuthAdditional(const Zone& zone) const {
+    // Fill in authority and addtional sections.
+    Zone::FindResult ns_result = zone.find(zone.getOrigin(), RRType::NS());
+    // zone origin name should have NS records
+    if (ns_result.code != Zone::SUCCESS) {
+        isc_throw(NoApexNS, "There's no apex NS records in zone " <<
+                zone.getOrigin().toText());
+    } else {
+        response_.addRRset(Message::SECTION_AUTHORITY,
+                boost::const_pointer_cast<RRset>(ns_result.rrset));
+        getAdditional(zone, *ns_result.rrset);
+    }
+}
+
+void
 Query::process() const {
     bool keep_doing = true;
     response_.setHeaderFlag(Message::HEADERFLAG_AA, false);
@@ -112,8 +138,8 @@ Query::process() const {
             case Zone::SUCCESS:
                 response_.setRcode(Rcode::NOERROR());
                 response_.addRRset(Message::SECTION_ANSWER,
-                            boost::const_pointer_cast<RRset>(db_result.rrset));
-                // TODO : fill in authority and addtional sections.
+                    boost::const_pointer_cast<RRset>(db_result.rrset));
+                getAuthAdditional(*result.zone);
                 break;
             case Zone::DELEGATION:
                 response_.setHeaderFlag(Message::HEADERFLAG_AA, false);

src/bin/auth/query.h

@@ -66,15 +66,19 @@ namespace auth {
 class Query {
 private:
 
-    /// \short Adds a SOA.
+    /// \brief Adds a SOA.
     ///
     /// Adds a SOA of the zone into the authority zone of response_.
     /// Can throw NoSOA.
     ///
     void putSOA(const isc::datasrc::Zone& zone) const;
 
-    /// Look up additional data (i.e., address records for the names included
-    /// in NS or MX records).
+    /// \brief Look up additional data (i.e., address records for the names
+    /// included in NS or MX records).
+    ///
+    /// Note: Any additional data which has already been provided in the
+    /// answer section (i.e., if the original query happend to be for the
+    /// address of the DNS server), it should be omitted from the additional.
     ///
     /// This method may throw a exception because its underlying methods may
     /// throw exceptions.
@@ -86,7 +90,7 @@ private:
     void getAdditional(const isc::datasrc::Zone& zone,
                        const isc::dns::RRset& rrset) const;
 
-    /// Find address records for a specified name.
+    /// \brief Find address records for a specified name.
     ///
     /// Search the specified zone for AAAA/A RRs of each of the NS/MX RDATA
     /// (domain name), and insert the found ones into the additional section
@@ -98,7 +102,7 @@ private:
     /// The glue records must exactly match the name in the NS RDATA, without
     /// CNAME or wildcard processing.
     ///
-    /// \param zone The Zone wherein the address records is to be found.
+    /// \param zone The \c Zone wherein the address records is to be found.
     /// \param qname The name in rrset RDATA.
     /// \param options The search options.
     void findAddrs(const isc::datasrc::Zone& zone,
@@ -106,6 +110,26 @@ private:
                    const isc::datasrc::Zone::FindOptions options
                    = isc::datasrc::Zone::FIND_DEFAULT) const;
 
+    /// \brief Look up \c Zone's NS and address records for the NS RDATA
+    /// (domain name) for authoritative answer.
+    ///
+    /// On returning an authoritative answer, insert the \c Zone's NS into the
+    /// authority section and AAAA/A RRs of each of the NS RDATA into the
+    /// additional section.
+    ///
+    /// <b>Notes to developer:</b>
+    ///
+    /// We should omit address records which has already been provided in the
+    /// answer section from the additional.
+    ///
+    /// For now, in order to optimize the additional section processing, we
+    /// include AAAA/A RRs under a zone cut in additional section. (BIND 9
+    /// excludes under-cut RRs; NSD include them.)
+    ///
+    /// \param zone The \c Zone wherein the additional data to the query is to
+    /// be found.
+    void getAuthAdditional(const isc::datasrc::Zone& zone) const;
+
 public:
     /// Constructor from query parameters.
     ///
@@ -173,6 +197,16 @@ public:
         {}
     };
 
+    /// \short Zone is missing its apex NS records.
+    ///
+    /// We tried to add apex NS records into the authority section, but the
+    /// zone does not contain any.
+    struct NoApexNS: public BadZone {
+        NoApexNS(const char* file, size_t line, const char* what) :
+            BadZone(file, line, what)
+        {}
+    };
+
 private:
     const isc::datasrc::MemoryDataSrc& memory_datasrc_;
     const isc::dns::Name& qname_;

src/bin/auth/tests/query_unittest.cc

@@ -47,8 +47,8 @@ RRsetPtr glue_aaaa_rrset(RRsetPtr(new RRset(Name("glue.ns.example.com"),
                                             RRClass::IN(), RRType::AAAA(),
                                             RRTTL(3600))));
 RRsetPtr noglue_a_rrset(RRsetPtr(new RRset(Name("noglue.example.com"),
-                                         RRClass::IN(), RRType::A(),
-                                         RRTTL(3600))));
+                                           RRClass::IN(), RRType::A(),
+                                           RRTTL(3600))));
 // This is a mock Zone class for testing.
 // It is a derived class of Zone, and simply hardcode the results of find()
 // return SUCCESS for "www.example.com",
@@ -58,15 +58,19 @@ RRsetPtr noglue_a_rrset(RRsetPtr(new RRset(Name("noglue.example.com"),
 // otherwise return DNAME
 class MockZone : public Zone {
 public:
-    MockZone(bool has_SOA = true) :
+    MockZone(bool has_SOA = true, bool has_apex_NS = true) :
         origin_(Name("example.com")),
         has_SOA_(has_SOA),
+        has_apex_NS_(has_apex_NS),
         delegation_rrset(RRsetPtr(new RRset(Name("delegation.example.com"),
                                             RRClass::IN(), RRType::NS(),
                                             RRTTL(3600)))),
         cname_rrset(RRsetPtr(new RRset(Name("cname.example.com"),
                                        RRClass::IN(), RRType::CNAME(),
-                                       RRTTL(3600))))
+                                       RRTTL(3600)))),
+        auth_ns_rrset(RRsetPtr(new RRset(Name("example.com"),
+                                         RRClass::IN(), RRType::NS(),
+                                         RRTTL(3600))))
     {
         delegation_rrset->addRdata(rdata::generic::NS(
                           Name("glue.ns.example.com")));
@@ -78,6 +82,12 @@ public:
                           Name("example.org")));
         cname_rrset->addRdata(rdata::generic::CNAME(
                           Name("www.example.com")));
+        auth_ns_rrset->addRdata(rdata::generic::NS(
+                          Name("glue.ns.example.com")));
+        auth_ns_rrset->addRdata(rdata::generic::NS(
+                          Name("noglue.example.com")));
+        auth_ns_rrset->addRdata(rdata::generic::NS(
+                          Name("example.net")));
     }
     virtual const isc::dns::Name& getOrigin() const;
     virtual const isc::dns::RRClass& getClass() const;
@@ -89,8 +99,10 @@ public:
 private:
     Name origin_;
     bool has_SOA_;
+    bool has_apex_NS_;
     RRsetPtr delegation_rrset;
     RRsetPtr cname_rrset;
+    RRsetPtr auth_ns_rrset;
 };
 
 const Name&
@@ -113,7 +125,8 @@ MockZone::find(const Name& name, const RRType& type,
     } else if (name == Name("glue.ns.example.com") && type == RRType::A() &&
         options == FIND_GLUE_OK) {
         return (FindResult(SUCCESS, glue_a_rrset));
-    } else if (name == Name("noglue.example.com") && type == RRType::A()) {
+    } else if (name == Name("noglue.example.com") && (type == RRType::A() ||
+        type == RRType::ANY())) {
         return (FindResult(SUCCESS, noglue_a_rrset));
     } else if (name == Name("glue.ns.example.com") && type == RRType::AAAA() &&
         options == FIND_GLUE_OK) {
@@ -122,6 +135,10 @@ MockZone::find(const Name& name, const RRType& type,
         has_SOA_)
     {
         return (FindResult(SUCCESS, soa_rrset));
+    } else if (name == Name("example.com") && type == RRType::NS() &&
+        has_apex_NS_)
+    {
+        return (FindResult(SUCCESS, auth_ns_rrset));
     } else if (name == Name("delegation.example.com")) {
         return (FindResult(DELEGATION, delegation_rrset));
     } else if (name == Name("ns.example.com")) {
@@ -157,24 +174,104 @@ protected:
 TEST_F(QueryTest, noZone) {
     // There's no zone in the memory datasource.  So the response should have
     // REFUSED.
-    query.process();
+    EXPECT_NO_THROW(query.process());
     EXPECT_EQ(Rcode::REFUSED(), response.getRcode());
 }
 
-TEST_F(QueryTest, matchZone) {
+TEST_F(QueryTest, exactMatch) {
     // add a matching zone.
     memory_datasrc.addZone(ZonePtr(new MockZone()));
-    query.process();
+    EXPECT_NO_THROW(query.process());
+    // find match rrset
     EXPECT_TRUE(response.getHeaderFlag(Message::HEADERFLAG_AA));
     EXPECT_EQ(Rcode::NOERROR(), response.getRcode());
     EXPECT_TRUE(response.hasRRset(Message::SECTION_ANSWER,
                                   Name("www.example.com"), RRClass::IN(),
                                   RRType::A()));
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_AUTHORITY,
+                                  Name("example.com"), RRClass::IN(),
+                                  RRType::NS()));
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_ADDITIONAL,
+                                  Name("glue.ns.example.com"),
+                                  RRClass::IN(), RRType::A()));
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_ADDITIONAL,
+                                  Name("glue.ns.example.com"),
+                                  RRClass::IN(), RRType::AAAA()));
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_ADDITIONAL,
+                                  Name("noglue.example.com"),
+                                  RRClass::IN(), RRType::A()));
+}
 
-    // Delegation
+TEST_F(QueryTest, exactAddrMatch) {
+    // find match rrset, omit additional data which has already been provided
+    // in the answer section from the additional.
+    memory_datasrc.addZone(ZonePtr(new MockZone()));
+    const Name noglue_name(Name("noglue.example.com"));
+    Query noglue_query(memory_datasrc, noglue_name, qtype, response);
+    EXPECT_NO_THROW(noglue_query.process());
+    EXPECT_TRUE(response.getHeaderFlag(Message::HEADERFLAG_AA));
+    EXPECT_EQ(Rcode::NOERROR(), response.getRcode());
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_ANSWER,
+                                  Name("noglue.example.com"), RRClass::IN(),
+                                  RRType::A()));
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_AUTHORITY,
+                                  Name("example.com"), RRClass::IN(),
+                                  RRType::NS()));
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_ADDITIONAL,
+                                  Name("glue.ns.example.com"),
+                                  RRClass::IN(), RRType::A()));
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_ADDITIONAL,
+                                  Name("glue.ns.example.com"),
+                                  RRClass::IN(), RRType::AAAA()));
+    EXPECT_FALSE(response.hasRRset(Message::SECTION_ADDITIONAL,
+                                  Name("noglue.example.com"),
+                                  RRClass::IN(), RRType::A()));
+}
+
+TEST_F(QueryTest, exactAnyMatch) {
+    // find match rrset, omit additional data which has already been provided
+    // in the answer section from the additional.
+    memory_datasrc.addZone(ZonePtr(new MockZone()));
+    const Name noglue_name(Name("noglue.example.com"));
+    Query noglue_query(memory_datasrc, noglue_name, RRType::ANY(), response);
+    EXPECT_NO_THROW(noglue_query.process());
+    EXPECT_TRUE(response.getHeaderFlag(Message::HEADERFLAG_AA));
+    EXPECT_EQ(Rcode::NOERROR(), response.getRcode());
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_ANSWER,
+                                  Name("noglue.example.com"), RRClass::IN(),
+                                  RRType::A()));
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_AUTHORITY,
+                                  Name("example.com"), RRClass::IN(),
+                                  RRType::NS()));
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_ADDITIONAL,
+                                  Name("glue.ns.example.com"),
+                                  RRClass::IN(), RRType::A()));
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_ADDITIONAL,
+                                  Name("glue.ns.example.com"),
+                                  RRClass::IN(), RRType::AAAA()));
+    EXPECT_FALSE(response.hasRRset(Message::SECTION_ADDITIONAL,
+                                  Name("noglue.example.com"),
+                                  RRClass::IN(), RRType::A()));
+}
+
+// This tests that when we need to look up Zone's apex NS records for 
+// authoritative answer, and there is no apex NS records. It should
+// throw in that case.
+TEST_F(QueryTest, noApexNS) {
+    // Add a zone without apex NS records 
+    memory_datasrc.addZone(ZonePtr(new MockZone(true, false)));
+    const Name noglue_name(Name("noglue.example.com"));
+    Query noglue_query(memory_datasrc, noglue_name, qtype, response);
+    EXPECT_THROW(noglue_query.process(), Query::NoApexNS);
+    // We don't look into the response, as it throwed
+}
+
+TEST_F(QueryTest, delegation) {
+    // add a matching zone.
+    memory_datasrc.addZone(ZonePtr(new MockZone()));
     const Name delegation_name(Name("delegation.example.com"));
     Query delegation_query(memory_datasrc, delegation_name, qtype, response);
-    delegation_query.process();
+    EXPECT_NO_THROW(delegation_query.process());
     EXPECT_FALSE(response.getHeaderFlag(Message::HEADERFLAG_AA));
     EXPECT_EQ(Rcode::NOERROR(), response.getRcode());
     EXPECT_TRUE(response.hasRRset(Message::SECTION_AUTHORITY,
@@ -199,21 +296,27 @@ TEST_F(QueryTest, matchZone) {
     EXPECT_FALSE(response.hasRRset(Message::SECTION_ADDITIONAL,
                                   Name("example.org"),
                                   RRClass::IN(), RRType::A()));
+}
 
-    // NXDOMAIN
+TEST_F(QueryTest, nxdomain) {
+    // add a matching zone.
+    memory_datasrc.addZone(ZonePtr(new MockZone()));
     const Name nxdomain_name(Name("nxdomain.example.com"));
     Query nxdomain_query(memory_datasrc, nxdomain_name, qtype, response);
-    nxdomain_query.process();
+    EXPECT_NO_THROW(nxdomain_query.process());
     EXPECT_EQ(Rcode::NXDOMAIN(), response.getRcode());
     EXPECT_EQ(0, response.getRRCount(Message::SECTION_ANSWER));
     EXPECT_EQ(0, response.getRRCount(Message::SECTION_ADDITIONAL));
     EXPECT_TRUE(response.hasRRset(Message::SECTION_AUTHORITY,
         Name("example.com"), RRClass::IN(), RRType::SOA()));
+}
 
-    // NXRRSET
+TEST_F(QueryTest, nxrrset) {
+    // add a matching zone.
+    memory_datasrc.addZone(ZonePtr(new MockZone()));
     const Name nxrrset_name(Name("nxrrset.example.com"));
     Query nxrrset_query(memory_datasrc, nxrrset_name, qtype, response);
-    nxrrset_query.process();
+    EXPECT_NO_THROW(nxrrset_query.process());
     EXPECT_EQ(Rcode::NOERROR(), response.getRcode());
     EXPECT_EQ(0, response.getRRCount(Message::SECTION_ANSWER));
     EXPECT_EQ(0, response.getRRCount(Message::SECTION_ADDITIONAL));