Merge branch 'master' of ssh://bind10.isc.org/var/bind10/git/bind10

ChangeLog

@@ -1,12 +1,19 @@
+  204.	[bug]		jinmei
+	b10-auth, src/lib/datasrc: class ANY queries were not handled
+	correctly in the generic data source (mainly for sqlite3).  It
+	could crash b10-auth in the worst case, and could result in
+	incorrect responses in some other cases.
+	(Trac #80, git c65637dd41c8d94399bd3e3cee965b694b633339)
+
   203.  [bug]		zhang likun
 	Fix resolver cache memory leak.
 	(Trac643, git aba4c4067da0dc63c97c6356dc3137651755ffce)
 
   202.  [func]    vorner
 	It is possible to specify a different directory where we look for
-	configuration files (by -p) and different configuration file to use (-c).
-	Also, it is possible to specify the port on which cmdctl should listen
-	(--cmdctl-port).
+	configuration files (by -p) and different configuration file to
+	use (-c).  Also, it is possible to specify the port on which
+	cmdctl should listen (--cmdctl-port).
 	(Trac #615, git 5514dd78f2d61a222f3069fc94723ca33fb3200b)
 
   201.  [bug]           jerry

src/lib/datasrc/data_source.cc

@@ -48,6 +48,28 @@ using namespace std;
 using namespace isc::dns;
 using namespace isc::dns::rdata;
 
+namespace {
+
+struct MatchRRsetForType {
+    MatchRRsetForType(const RRType rrtype) : rrtype_(rrtype) {}
+    bool operator()(RRsetPtr rrset) {
+        return (rrset->getType() == rrtype_);
+    }
+    const RRType rrtype_;
+};
+
+// This is a helper to retrieve a specified RR type of RRset from RRsetList.
+// In our case the data source search logic should ensure that the class is
+// valid.  We use this find logic of our own so that we can support both
+// specific RR class queries (normal case) and class ANY queries.
+RRsetPtr
+findRRsetFromList(RRsetList& list, const RRType rrtype) {
+    RRsetList::iterator it(find_if(list.begin(), list.end(),
+                                   MatchRRsetForType(rrtype)));
+    return (it != list.end() ? *it : RRsetPtr());
+}
+}
+
 namespace isc {
 namespace datasrc {
 
@@ -129,7 +151,7 @@ synthesizeCname(QueryTaskPtr task, RRsetPtr rrset, RRsetList& target) {
     const generic::DNAME& dname = dynamic_cast<const generic::DNAME&>(rd);
     const Name& dname_target(dname.getDname());
 
-    RRsetPtr cname(new RRset(task->qname, task->qclass, RRType::CNAME(),
+    RRsetPtr cname(new RRset(task->qname, rrset->getClass(), RRType::CNAME(),
                              rrset->getTTL()));
 
     const int qnlen = task->qname.getLabelCount();
@@ -569,17 +591,17 @@ hasDelegation(Query& q, QueryTaskPtr task, ZoneInfo& zoneinfo) {
         // Found a referral while getting answer data;
         // send a delegation.
         if (found) {
-            RRsetPtr r = ref.findRRset(RRType::DNAME(), q.qclass());
+            RRsetPtr r = findRRsetFromList(ref, RRType::DNAME());
             if (r != NULL) {
                 RRsetList syn;
                 addToMessage(q, Message::SECTION_ANSWER, r);
                 q.message().setHeaderFlag(Message::HEADERFLAG_AA);
                 synthesizeCname(task, r, syn);
                 if (syn.size() == 1) {
-                    addToMessage(q, Message::SECTION_ANSWER,
-                                 syn.findRRset(RRType::CNAME(), q.qclass()));
-                    chaseCname(q, task, syn.findRRset(RRType::CNAME(),
-                                                      q.qclass()));
+                    RRsetPtr cname_rrset = findRRsetFromList(syn,
+                                                             RRType::CNAME());
+                    addToMessage(q, Message::SECTION_ANSWER, cname_rrset);
+                    chaseCname(q, task, cname_rrset);
                     return (true);
                 }
             }
@@ -612,7 +634,7 @@ addSOA(Query& q, ZoneInfo& zoneinfo) {
     }
 
     addToMessage(q, Message::SECTION_AUTHORITY,
-                 soa.findRRset(RRType::SOA(), q.qclass()));
+                 findRRsetFromList(soa, RRType::SOA()));
     return (DataSrc::SUCCESS);
 }
 
@@ -624,7 +646,7 @@ addNSEC(Query& q, const Name& name, ZoneInfo& zoneinfo) {
     RETERR(doQueryTask(newtask, zoneinfo, nsec));
     if (newtask.flags == 0) {
         addToMessage(q, Message::SECTION_AUTHORITY,
-                     nsec.findRRset(RRType::NSEC(), q.qclass()));
+                     findRRsetFromList(nsec, RRType::NSEC()));
     }
 
     return (DataSrc::SUCCESS);
@@ -828,7 +850,7 @@ tryWildcard(Query& q, QueryTaskPtr task, ZoneInfo& zoneinfo, bool& found) {
         // match the qname), and then continue as if this were a normal
         // answer: if a CNAME, chase the target, otherwise add authority.
         if (cname) {
-            RRsetPtr rrset = wild.findRRset(RRType::CNAME(), q.qclass());
+            RRsetPtr rrset = findRRsetFromList(wild, RRType::CNAME());
             if (rrset != NULL) {
                 rrset->setName(task->qname);
                 addToMessage(q, Message::SECTION_ANSWER, rrset);
@@ -923,7 +945,7 @@ DataSrc::doQuery(Query& q) {
              ((task->qtype == RRType::NSEC() ||
                task->qtype == RRType::DS() ||
                task->qtype == RRType::DNAME()) &&
-              data.findRRset(task->qtype, task->qclass)))) {
+              findRRsetFromList(data, task->qtype)))) {
             task->flags &= ~REFERRAL;
         }
 
@@ -948,9 +970,8 @@ DataSrc::doQuery(Query& q) {
                     // Add the NS records for the enclosing zone to
                     // the authority section.
                     RRsetList auth;
-                    const DataSrc* ds = zoneinfo.getDataSource();
-                    if (!refQuery(q, Name(*zonename), zoneinfo, auth)  ||
-                        !auth.findRRset(RRType::NS(), ds->getClass())) {
+                    if (!refQuery(q, Name(*zonename), zoneinfo, auth) ||
+                        !findRRsetFromList(auth, RRType::NS())) {
                         isc_throw(DataSourceError,
                                   "NS RR not found in " << *zonename << "/" <<
                                   q.qclass());
@@ -983,7 +1004,7 @@ DataSrc::doQuery(Query& q) {
         } else if ((task->flags & CNAME_FOUND) != 0) {
             // The qname node contains a CNAME.  Add a new task to the
             // queue to look up its target.
-            RRsetPtr rrset = data.findRRset(RRType::CNAME(), q.qclass());
+            RRsetPtr rrset = findRRsetFromList(data, RRType::CNAME());
             if (rrset != NULL) {
                 addToMessage(q, task->section, rrset);
                 chaseCname(q, task, rrset);

src/lib/datasrc/tests/datasrc_unittest.cc

@@ -70,7 +70,7 @@ protected:
     }
     void QueryCommon(const RRClass& qclass);
     void createAndProcessQuery(const Name& qname, const RRClass& qclass,
-                               const RRType& qtype);
+                               const RRType& qtype, bool need_dnssec);
 
     HotCache cache;
     MetaDataSrc meta_source;
@@ -82,23 +82,26 @@ protected:
 };
 
 void
-performQuery(DataSrc& data_source, HotCache& cache, Message& message) {
+performQuery(DataSrc& data_source, HotCache& cache, Message& message,
+             bool need_dnssec = true)
+{
     message.setHeaderFlag(Message::HEADERFLAG_AA);
     message.setRcode(Rcode::NOERROR());
-    Query q(message, cache, true);
+    Query q(message, cache, need_dnssec);
     data_source.doQuery(q);
 }
 
 void
 DataSrcTest::createAndProcessQuery(const Name& qname, const RRClass& qclass,
-                                   const RRType& qtype)
+                                   const RRType& qtype,
+                                   bool need_dnssec = true)
 {
     msg.makeResponse();
     msg.setOpcode(Opcode::QUERY());
     msg.addQuestion(Question(qname, qclass, qtype));
     msg.setHeaderFlag(Message::HEADERFLAG_RD);
     qid = msg.getQid();
-    performQuery(meta_source, cache, msg);
+    performQuery(meta_source, cache, msg, need_dnssec);
 }
 
 void
@@ -165,6 +168,59 @@ TEST_F(DataSrcTest, QueryClassAny) {
     QueryCommon(RRClass::ANY());
 }
 
+TEST_F(DataSrcTest, queryClassAnyNegative) {
+    // There was a bug where Class ANY query triggered a crash due to NULL
+    // pointer dereference.  This test checks that condition.
+
+    // NXDOMAIN case
+    createAndProcessQuery(Name("notexistent.example.com"), RRClass::ANY(),
+                          RRType::A());
+    headerCheck(msg, qid, Rcode::NXDOMAIN(), opcodeval,
+                QR_FLAG | AA_FLAG | RD_FLAG, 1, 0, 6, 0);
+
+    // NXRRSET case
+    msg.clear(Message::PARSE);
+    createAndProcessQuery(Name("www.example.com"), RRClass::ANY(),
+                          RRType::TXT());
+    headerCheck(msg, qid, Rcode::NOERROR(), opcodeval,
+                QR_FLAG | AA_FLAG | RD_FLAG, 1, 0, 4, 0);
+}
+
+TEST_F(DataSrcTest, queryClassAnyDNAME) {
+    // Class ANY query that would match a DNAME.  Everything including the
+    // synthesized CNAME should be the same as the response to class IN query.
+    createAndProcessQuery(Name("www.dname.example.com"), RRClass::ANY(),
+                          RRType::A(), false);
+    headerCheck(msg, qid, Rcode::NOERROR(), opcodeval,
+                QR_FLAG | AA_FLAG | RD_FLAG, 1, 3, 3, 3);
+    rrsetsCheck("dname.example.com. 3600 IN DNAME sql1.example.com.\n"
+                "www.dname.example.com. 3600 IN CNAME www.sql1.example.com.\n"
+                "www.sql1.example.com. 3600 IN A 192.0.2.2\n",
+                msg.beginSection(Message::SECTION_ANSWER),
+                msg.endSection(Message::SECTION_ANSWER));
+
+    // Also check the case of explicit DNAME query.
+    msg.clear(Message::PARSE);
+    createAndProcessQuery(Name("dname.example.com"), RRClass::ANY(),
+                          RRType::DNAME(), false);
+    headerCheck(msg, qid, Rcode::NOERROR(), opcodeval,
+                QR_FLAG | AA_FLAG | RD_FLAG, 1, 1, 3, 3);
+    rrsetsCheck("dname.example.com. 3600 IN DNAME sql1.example.com.\n",
+                msg.beginSection(Message::SECTION_ANSWER),
+                msg.endSection(Message::SECTION_ANSWER));
+}
+
+TEST_F(DataSrcTest, queryClassAnyCNAME) {
+    // Similar test for CNAME
+    createAndProcessQuery(Name("foo.example.com"), RRClass::ANY(),
+                          RRType::A(), false);
+    headerCheck(msg, qid, Rcode::NOERROR(), opcodeval,
+                QR_FLAG | AA_FLAG | RD_FLAG, 1, 1, 0, 0);
+    rrsetsCheck("foo.example.com. 3600 IN CNAME cnametest.example.net.\n",
+                msg.beginSection(Message::SECTION_ANSWER),
+                msg.endSection(Message::SECTION_ANSWER));
+}
+
 TEST_F(DataSrcTest, NSQuery) {
     createAndProcessQuery(Name("example.com"), RRClass::IN(),
                           RRType::NS());
@@ -416,68 +472,36 @@ TEST_F(DataSrcTest, DISABLED_WildcardAgainstMultiLabel) {
 
 TEST_F(DataSrcTest, WildcardCname) {
     // Check that wildcard answers containing CNAMES are followed
-    // correctly
-    createAndProcessQuery(Name("www.wild2.example.com"), RRClass::IN(),
-                          RRType::A());
-
-    headerCheck(msg, qid, Rcode::NOERROR(), opcodeval,
-                QR_FLAG | AA_FLAG | RD_FLAG, 1, 4, 6, 6);
-
-    RRsetIterator rit = msg.beginSection(Message::SECTION_ANSWER);
-    RRsetPtr rrset = *rit;
-    EXPECT_EQ(Name("www.wild2.example.com"), rrset->getName());
-    EXPECT_EQ(RRType::CNAME(), rrset->getType());
-    EXPECT_EQ(RRClass::IN(), rrset->getClass());
-
-    RdataIteratorPtr it = rrset->getRdataIterator();
-    EXPECT_EQ("www.example.com.", it->getCurrent().toText());
-    it->next();
-    EXPECT_TRUE(it->isLast());
-
-    ++rit;
-    ++rit;
-    rrset = *rit;
-    EXPECT_EQ(Name("www.example.com"), rrset->getName());
-    EXPECT_EQ(RRType::A(), rrset->getType());
-    EXPECT_EQ(RRClass::IN(), rrset->getClass());
-
-    it = rrset->getRdataIterator();
-    EXPECT_EQ("192.0.2.1", it->getCurrent().toText());
-    it->next();
-    EXPECT_TRUE(it->isLast());
-
-    rit = msg.beginSection(Message::SECTION_AUTHORITY);
-    rrset = *rit;
-    EXPECT_EQ(Name("*.wild2.example.com"), rrset->getName());
-    EXPECT_EQ(RRType::NSEC(), rrset->getType());
-    EXPECT_EQ(RRClass::IN(), rrset->getClass());
-    ++rit;
-    ++rit;
-
-    rrset = *rit;
-    EXPECT_EQ(Name("example.com"), rrset->getName());
-    EXPECT_EQ(RRType::NS(), rrset->getType());
-    EXPECT_EQ(RRClass::IN(), rrset->getClass());
-
-    it = rrset->getRdataIterator();
-    EXPECT_EQ("dns01.example.com.", it->getCurrent().toText());
-    it->next();
-    EXPECT_EQ("dns02.example.com.", it->getCurrent().toText());
-    it->next();
-    EXPECT_EQ("dns03.example.com.", it->getCurrent().toText());
-    it->next();
-    EXPECT_TRUE(it->isLast());
-
-    rit = msg.beginSection(Message::SECTION_ADDITIONAL);
-    rrset = *rit;
-    EXPECT_EQ(Name("dns01.example.com"), rrset->getName());
-    EXPECT_EQ(RRType::A(), rrset->getType());
-    EXPECT_EQ(RRClass::IN(), rrset->getClass());
-
-    it = rrset->getRdataIterator();
-    EXPECT_EQ("192.0.2.1", it->getCurrent().toText());
-    it->next();
-    EXPECT_TRUE(it->isLast());
+    // correctly.  It should result in the same response for both
+    // class IN and ANY queries.
+    const RRClass classes[2] = { RRClass::IN(), RRClass::ANY() };
+
+    for (int i = 0; i < sizeof(classes) / sizeof(classes[0]); ++i) {
+        SCOPED_TRACE("Wildcard + CNAME test for class " + classes[i].toText());
+
+        msg.clear(Message::PARSE);
+
+        createAndProcessQuery(Name("www.wild2.example.com"), classes[i],
+                              RRType::A(), false);
+
+        headerCheck(msg, qid, Rcode::NOERROR(), opcodeval,
+                    QR_FLAG | AA_FLAG | RD_FLAG, 1, 2, 3, 3);
+
+        rrsetsCheck("www.wild2.example.com. 3600 IN CNAME www.example.com\n"
+                    "www.example.com. 3600 IN A 192.0.2.1\n",
+                    msg.beginSection(Message::SECTION_ANSWER),
+                    msg.endSection(Message::SECTION_ANSWER));
+        rrsetsCheck("example.com. 3600 IN NS dns01.example.com.\n"
+                    "example.com. 3600 IN NS dns02.example.com.\n"
+                    "example.com. 3600 IN NS dns03.example.com.",
+                    msg.beginSection(Message::SECTION_AUTHORITY),
+                    msg.endSection(Message::SECTION_AUTHORITY));
+        rrsetsCheck("dns01.example.com. 3600 IN A 192.0.2.1\n"
+                    "dns02.example.com. 3600 IN A 192.0.2.2\n"
+                    "dns03.example.com. 3600 IN A 192.0.2.3",
+                    msg.beginSection(Message::SECTION_ADDITIONAL),
+                    msg.endSection(Message::SECTION_ADDITIONAL));
+    }
 }
 
 TEST_F(DataSrcTest, WildcardCnameNodata) {
@@ -667,7 +691,7 @@ TEST_F(DataSrcTest, Cname) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     RdataIteratorPtr it = rrset->getRdataIterator();
-    EXPECT_EQ("cnametest.flame.org.", it->getCurrent().toText());
+    EXPECT_EQ("cnametest.example.net.", it->getCurrent().toText());
     it->next();
     EXPECT_TRUE(it->isLast());
 }

src/lib/datasrc/tests/test_datasrc.cc

@@ -154,7 +154,7 @@ const struct RRData example_com_records[] = {
     {"*.wild3.example.com", "RRSIG", "NSEC 5 3 7200 20100410212307 20100311212307 33495 example.com. EuSzh6or8mbvwru2H7fyYeMpW6J8YZ528rabU38V/lMN0TdamghIuCneAvSNaZgwk2MSN1bWpZqB2kAipaM/ZI9/piLlTvVjjOQ8pjk0auwCEqT7Z7Qng3E92O9yVzO+WHT9QZn/fR6t60392In4IvcBGjZyjzQk8njIwbui xGA="},
 
     // foo.example.com
-    {"foo.example.com", "CNAME", "cnametest.flame.org"},
+    {"foo.example.com", "CNAME", "cnametest.example.net"},
     {"foo.example.com", "RRSIG", "CNAME 5 3 3600 20100322084538 20100220084538 33495 example.com. DSqkLnsh0gCeCPVW/Q8viy9GNP+KHmFGfWqyVG1S6koBtGN/VQQ16M4PHZ9Zssmf/JcDVJNIhAChHPE2WJiaPCNGTprsaUshf1Q2vMPVnkrJKgDY8SVRYMptmT8eaT0gGri4KhqRoFpMT5OYfesybwDgfhFSQQAh6ps3bIUsy4o="},
     {"foo.example.com", "NSEC", "mail.example.com. CNAME RRSIG NSEC"},
     {"foo.example.com", "RRSIG", "NSEC 5 3 7200 20100322084538 20100220084538 33495 example.com. RTQwlSqui6StUYye1KCSOEr1d3irndWFqHBpwP7g7n+w8EDXJ8I7lYgwzHvlQt6BLAxe5fUDi7ct8M5hXvsm7FoWPZ5wXH+2/eJUCYxIw4vezKMkMwBP6M/YkJ2CMqY8DppYf60QaLDONQAr7AcK/naSyioeI5h6eaoVitUDMso="},