Browse Source

[1176] Implement the DNSSEC support in query

Only positive matches for now.

This looks like someone already prepared for it, it only needs to
passing the intent to few functions and everything handles itself well.

Also updated tests, since the utility function gets confused by the
RRSIGs. Not fixing the function for now, it looks complicated, because
the problem is in it's design - if it happens to be just this one case,
it will be easier.
Michal 'vorner' Vaner 13 years ago
parent
commit
ddf9da5175
3 changed files with 57 additions and 34 deletions
  1. 25 15
      src/bin/auth/query.cc
  2. 4 1
      src/bin/auth/query.h
  3. 28 18
      src/bin/auth/tests/query_unittest.cc

+ 25 - 15
src/bin/auth/query.cc

@@ -67,20 +67,23 @@ Query::findAddrs(ZoneFinder& zone, const Name& qname,
     // Find A rrset
     // Find A rrset
     if (qname_ != qname || qtype_ != RRType::A()) {
     if (qname_ != qname || qtype_ != RRType::A()) {
         ZoneFinder::FindResult a_result = zone.find(qname, RRType::A(), NULL,
         ZoneFinder::FindResult a_result = zone.find(qname, RRType::A(), NULL,
-                                              options);
+            static_cast<ZoneFinder::FindOptions>(options | dnssec_opt_));
         if (a_result.code == ZoneFinder::SUCCESS) {
         if (a_result.code == ZoneFinder::SUCCESS) {
             response_.addRRset(Message::SECTION_ADDITIONAL,
             response_.addRRset(Message::SECTION_ADDITIONAL,
-                    boost::const_pointer_cast<RRset>(a_result.rrset));
+                    boost::const_pointer_cast<RRset>(a_result.rrset), dnssec_);
         }
         }
     }
     }
 
 
     // Find AAAA rrset
     // Find AAAA rrset
     if (qname_ != qname || qtype_ != RRType::AAAA()) {
     if (qname_ != qname || qtype_ != RRType::AAAA()) {
         ZoneFinder::FindResult aaaa_result =
         ZoneFinder::FindResult aaaa_result =
-            zone.find(qname, RRType::AAAA(), NULL, options);
+            zone.find(qname, RRType::AAAA(), NULL,
+                      static_cast<ZoneFinder::FindOptions>(options |
+                                                           dnssec_opt_));
         if (aaaa_result.code == ZoneFinder::SUCCESS) {
         if (aaaa_result.code == ZoneFinder::SUCCESS) {
             response_.addRRset(Message::SECTION_ADDITIONAL,
             response_.addRRset(Message::SECTION_ADDITIONAL,
-                    boost::const_pointer_cast<RRset>(aaaa_result.rrset));
+                    boost::const_pointer_cast<RRset>(aaaa_result.rrset),
+                    dnssec_);
         }
         }
     }
     }
 }
 }
@@ -88,7 +91,7 @@ Query::findAddrs(ZoneFinder& zone, const Name& qname,
 void
 void
 Query::putSOA(ZoneFinder& zone) const {
 Query::putSOA(ZoneFinder& zone) const {
     ZoneFinder::FindResult soa_result(zone.find(zone.getOrigin(),
     ZoneFinder::FindResult soa_result(zone.find(zone.getOrigin(),
-        RRType::SOA()));
+        RRType::SOA(), NULL, dnssec_opt_));
     if (soa_result.code != ZoneFinder::SUCCESS) {
     if (soa_result.code != ZoneFinder::SUCCESS) {
         isc_throw(NoSOA, "There's no SOA record in zone " <<
         isc_throw(NoSOA, "There's no SOA record in zone " <<
             zone.getOrigin().toText());
             zone.getOrigin().toText());
@@ -99,7 +102,7 @@ Query::putSOA(ZoneFinder& zone) const {
          * to insist.
          * to insist.
          */
          */
         response_.addRRset(Message::SECTION_AUTHORITY,
         response_.addRRset(Message::SECTION_AUTHORITY,
-            boost::const_pointer_cast<RRset>(soa_result.rrset));
+            boost::const_pointer_cast<RRset>(soa_result.rrset), dnssec_);
     }
     }
 }
 }
 
 
@@ -107,14 +110,15 @@ void
 Query::getAuthAdditional(ZoneFinder& zone) const {
 Query::getAuthAdditional(ZoneFinder& zone) const {
     // Fill in authority and addtional sections.
     // Fill in authority and addtional sections.
     ZoneFinder::FindResult ns_result = zone.find(zone.getOrigin(),
     ZoneFinder::FindResult ns_result = zone.find(zone.getOrigin(),
-                                                 RRType::NS());
+                                                 RRType::NS(), NULL,
+                                                 dnssec_opt_);
     // zone origin name should have NS records
     // zone origin name should have NS records
     if (ns_result.code != ZoneFinder::SUCCESS) {
     if (ns_result.code != ZoneFinder::SUCCESS) {
         isc_throw(NoApexNS, "There's no apex NS records in zone " <<
         isc_throw(NoApexNS, "There's no apex NS records in zone " <<
                 zone.getOrigin().toText());
                 zone.getOrigin().toText());
     } else {
     } else {
         response_.addRRset(Message::SECTION_AUTHORITY,
         response_.addRRset(Message::SECTION_AUTHORITY,
-            boost::const_pointer_cast<RRset>(ns_result.rrset));
+            boost::const_pointer_cast<RRset>(ns_result.rrset), dnssec_);
         // Handle additional for authority section
         // Handle additional for authority section
         getAdditional(zone, *ns_result.rrset);
         getAdditional(zone, *ns_result.rrset);
     }
     }
@@ -147,12 +151,14 @@ Query::process() const {
         keep_doing = false;
         keep_doing = false;
         std::auto_ptr<RRsetList> target(qtype_is_any ? new RRsetList : NULL);
         std::auto_ptr<RRsetList> target(qtype_is_any ? new RRsetList : NULL);
         const ZoneFinder::FindResult db_result(
         const ZoneFinder::FindResult db_result(
-            result.zone_finder->find(qname_, qtype_, target.get()));
+            result.zone_finder->find(qname_, qtype_, target.get(),
+                                     dnssec_opt_));
         switch (db_result.code) {
         switch (db_result.code) {
             case ZoneFinder::DNAME: {
             case ZoneFinder::DNAME: {
                 // First, put the dname into the answer
                 // First, put the dname into the answer
                 response_.addRRset(Message::SECTION_ANSWER,
                 response_.addRRset(Message::SECTION_ANSWER,
-                    boost::const_pointer_cast<RRset>(db_result.rrset));
+                    boost::const_pointer_cast<RRset>(db_result.rrset),
+                    dnssec_);
                 /*
                 /*
                  * Empty DNAME should never get in, as it is impossible to
                  * Empty DNAME should never get in, as it is impossible to
                  * create one in master file.
                  * create one in master file.
@@ -188,7 +194,7 @@ Query::process() const {
                     qname_.getLabelCount() -
                     qname_.getLabelCount() -
                     db_result.rrset->getName().getLabelCount()).
                     db_result.rrset->getName().getLabelCount()).
                     concatenate(dname.getDname())));
                     concatenate(dname.getDname())));
-                response_.addRRset(Message::SECTION_ANSWER, cname);
+                response_.addRRset(Message::SECTION_ANSWER, cname, dnssec_);
                 break;
                 break;
             }
             }
             case ZoneFinder::CNAME:
             case ZoneFinder::CNAME:
@@ -202,20 +208,23 @@ Query::process() const {
                  * So, just put it there.
                  * So, just put it there.
                  */
                  */
                 response_.addRRset(Message::SECTION_ANSWER,
                 response_.addRRset(Message::SECTION_ANSWER,
-                    boost::const_pointer_cast<RRset>(db_result.rrset));
+                    boost::const_pointer_cast<RRset>(db_result.rrset),
+                    dnssec_);
                 break;
                 break;
             case ZoneFinder::SUCCESS:
             case ZoneFinder::SUCCESS:
                 if (qtype_is_any) {
                 if (qtype_is_any) {
                     // If quety type is ANY, insert all RRs under the domain
                     // If quety type is ANY, insert all RRs under the domain
                     // into answer section.
                     // into answer section.
                     BOOST_FOREACH(RRsetPtr rrset, *target) {
                     BOOST_FOREACH(RRsetPtr rrset, *target) {
-                        response_.addRRset(Message::SECTION_ANSWER, rrset);
+                        response_.addRRset(Message::SECTION_ANSWER, rrset,
+                                           dnssec_);
                         // Handle additional for answer section
                         // Handle additional for answer section
                         getAdditional(*result.zone_finder, *rrset.get());
                         getAdditional(*result.zone_finder, *rrset.get());
                     }
                     }
                 } else {
                 } else {
                     response_.addRRset(Message::SECTION_ANSWER,
                     response_.addRRset(Message::SECTION_ANSWER,
-                        boost::const_pointer_cast<RRset>(db_result.rrset));
+                        boost::const_pointer_cast<RRset>(db_result.rrset),
+                        dnssec_);
                     // Handle additional for answer section
                     // Handle additional for answer section
                     getAdditional(*result.zone_finder, *db_result.rrset);
                     getAdditional(*result.zone_finder, *db_result.rrset);
                 }
                 }
@@ -233,7 +242,8 @@ Query::process() const {
             case ZoneFinder::DELEGATION:
             case ZoneFinder::DELEGATION:
                 response_.setHeaderFlag(Message::HEADERFLAG_AA, false);
                 response_.setHeaderFlag(Message::HEADERFLAG_AA, false);
                 response_.addRRset(Message::SECTION_AUTHORITY,
                 response_.addRRset(Message::SECTION_AUTHORITY,
-                    boost::const_pointer_cast<RRset>(db_result.rrset));
+                    boost::const_pointer_cast<RRset>(db_result.rrset),
+                    dnssec_);
                 getAdditional(*result.zone_finder, *db_result.rrset);
                 getAdditional(*result.zone_finder, *db_result.rrset);
                 break;
                 break;
             case ZoneFinder::NXDOMAIN:
             case ZoneFinder::NXDOMAIN:

+ 4 - 1
src/bin/auth/query.h

@@ -145,7 +145,9 @@ public:
           const isc::dns::Name& qname, const isc::dns::RRType& qtype,
           const isc::dns::Name& qname, const isc::dns::RRType& qtype,
           isc::dns::Message& response, bool dnssec = false) :
           isc::dns::Message& response, bool dnssec = false) :
         datasrc_client_(datasrc_client), qname_(qname), qtype_(qtype),
         datasrc_client_(datasrc_client), qname_(qname), qtype_(qtype),
-        response_(response), dnssec_(dnssec)
+        response_(response), dnssec_(dnssec),
+        dnssec_opt_(dnssec ?  isc::datasrc::ZoneFinder::FIND_DNSSEC :
+                    isc::datasrc::ZoneFinder::FIND_DEFAULT)
     {}
     {}
 
 
     /// Process the query.
     /// Process the query.
@@ -214,6 +216,7 @@ private:
     const isc::dns::RRType& qtype_;
     const isc::dns::RRType& qtype_;
     isc::dns::Message& response_;
     isc::dns::Message& response_;
     const bool dnssec_;
     const bool dnssec_;
+    const isc::datasrc::ZoneFinder::FindOptions dnssec_opt_;
 };
 };
 
 
 }
 }

+ 28 - 18
src/bin/auth/tests/query_unittest.cc

@@ -215,6 +215,7 @@ MockZoneFinder::find(const Name& name, const RRType& type,
         RRsetStore::const_iterator found_rrset =
         RRsetStore::const_iterator found_rrset =
             found_domain->second.find(type);
             found_domain->second.find(type);
         if (found_rrset != found_domain->second.end()) {
         if (found_rrset != found_domain->second.end()) {
+            // TODO: Drop whatever rrsig is there if options doesn't have the dnssec
             return (FindResult(SUCCESS, found_rrset->second));
             return (FindResult(SUCCESS, found_rrset->second));
         }
         }
 
 
@@ -329,31 +330,40 @@ TEST_F(QueryTest, dnssecPositive) {
     Query query(memory_client, qname, qtype, response, true);
     Query query(memory_client, qname, qtype, response, true);
     EXPECT_NO_THROW(query.process());
     EXPECT_NO_THROW(query.process());
     // find match rrset
     // find match rrset
-    responseCheck(response, Rcode::NOERROR(), AA_FLAG, 1, 3, 3,
+    // 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 "
                   (www_a_txt + std::string("www.example.com. 3600 IN RRSIG "
-                                           "AAAA 5 3 3600 20000101000000 "
+                                           "A 5 3 3600 20000101000000 "
                                            "20000201000000 12345 example.com. "
                                            "20000201000000 12345 example.com. "
                                            "FAKEFAKEFAKE\n")).c_str(),
                                            "FAKEFAKEFAKE\n")).c_str(),
                   (zone_ns_txt + std::string("example.com. 3600 IN RRSIG NS 5 "
                   (zone_ns_txt + std::string("example.com. 3600 IN RRSIG NS 5 "
                                              "3 3600 20000101000000 "
                                              "3 3600 20000101000000 "
                                              "20000201000000 12345 "
                                              "20000201000000 12345 "
                                              "example.com. FAKEFAKEFAKE\n")).
                                              "example.com. FAKEFAKEFAKE\n")).
-                  c_str(),
-                  (ns_addrs_txt + std::string("glue.delegation.example.com. "
-                                              "3600 IN RRSIG A 5 3 3600 "
-                                              "20000101000000 20000201000000 "
-                                              "12345 example.com. "
-                                              "FAKEFAKEFAKE\n"
-                                              "glue.delegation.example.com. "
-                                              "3600 IN RRSIG AAAA 5 3 3600 "
-                                              "20000101000000 20000201000000 "
-                                              "12345 example.com. "
-                                              "FAKEFAKEFAKE\n"
-                                              "noglue.example.com. 3600 IN "
-                                              "RRSIG A 5 3 3600 "
-                                              "20000101000000 20000201000000 "
-                                              "12345 example.com. "
-                                              "FAKEFAKEFAKE\n")).c_str());
+                  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));
 }
 }
 
 
 TEST_F(QueryTest, exactAddrMatch) {
 TEST_F(QueryTest, exactAddrMatch) {