[2165] Make Message::addRRset() to be unaware of signedness

* adjust counts_[section] for any RRSIGs using RRset::getSIGRdataCount()
* remove the 'sign' parameter of Message::addRRset()
* stop extracting the RRSIG
* update code to compile in other places

src/bin/auth/auth_srv.cc

@@ -666,7 +666,7 @@ AuthSrvImpl::processNormalQuery(const IOMessage& io_message, Message& message,
         if (list) {
             const RRType& qtype = question->getType();
             const Name& qname = question->getName();
-            query_.process(*list, qname, qtype, message, dnssec_ok);
+            query_.process(*list, qname, qtype, message);
         } else {
             makeErrorMessage(renderer_, message, buffer, Rcode::REFUSED());
             return (true);

src/bin/auth/query.cc

@@ -57,7 +57,7 @@ namespace auth {
 void
 Query::ResponseCreator::addRRset(isc::dns::Message& message,
                                  const isc::dns::Message::Section section,
-                                 const ConstRRsetPtr& rrset, const bool dnssec)
+                                 const ConstRRsetPtr& rrset)
 {
     /// Is this RRset already in the list of RRsets added to the message?
     const std::vector<const AbstractRRset*>::const_iterator i =
@@ -68,8 +68,7 @@ Query::ResponseCreator::addRRset(isc::dns::Message& message,
         // No - add it to both the message and the list of RRsets processed.
         // The const-cast is wrong, but the message interface seems to insist.
         message.addRRset(section,
-                         boost::const_pointer_cast<AbstractRRset>(rrset),
-                         dnssec);
+                         boost::const_pointer_cast<AbstractRRset>(rrset));
         added_.push_back(rrset.get());
     }
 }
@@ -78,8 +77,7 @@ void
 Query::ResponseCreator::create(Message& response,
                                const vector<ConstRRsetPtr>& answers,
                                const vector<ConstRRsetPtr>& authorities,
-                               const vector<ConstRRsetPtr>& additionals,
-                               const bool dnssec)
+                               const vector<ConstRRsetPtr>& additionals)
 {
     // Inserter should be reset each time the query is reset, so should be
     // empty at this point.
@@ -91,13 +89,13 @@ Query::ResponseCreator::create(Message& response,
     // guarantee that if there are duplicates, the single RRset added will
     // appear in the most important section.
     BOOST_FOREACH(const ConstRRsetPtr& rrset, answers) {
-        addRRset(response, Message::SECTION_ANSWER, rrset, dnssec);
+        addRRset(response, Message::SECTION_ANSWER, rrset);
     }
     BOOST_FOREACH(const ConstRRsetPtr& rrset, authorities) {
-        addRRset(response, Message::SECTION_AUTHORITY, rrset, dnssec);
+        addRRset(response, Message::SECTION_AUTHORITY, rrset);
     }
     BOOST_FOREACH(const ConstRRsetPtr& rrset, additionals) {
-        addRRset(response, Message::SECTION_ADDITIONAL, rrset, dnssec);
+        addRRset(response, Message::SECTION_ADDITIONAL, rrset);
     }
 }
 
@@ -354,14 +352,14 @@ findZone(const ClientList& list, const Name& qname, RRType qtype) {
 void
 Query::process(datasrc::ClientList& client_list,
                const isc::dns::Name& qname, const isc::dns::RRType& qtype,
-               isc::dns::Message& response, bool dnssec)
+               isc::dns::Message& response)
 {
     // Set up the cleaner object so internal pointers and vectors are
     // always reset after scope leaves this method
     QueryCleaner cleaner(*this);
 
     // Set up query parameters for the rest of the (internal) methods
-    initialize(client_list, qname, qtype, response, dnssec);
+    initialize(client_list, qname, qtype, response);
 
     // Found a zone which is the nearest ancestor to QNAME
     const ClientList::FindResult result = findZone(*client_list_, *qname_,
@@ -533,8 +531,7 @@ Query::process(datasrc::ClientList& client_list,
             break;
     }
 
-    response_creator_.create(*response_, answers_, authorities_, additionals_,
-                             dnssec_);
+    response_creator_.create(*response_, answers_, authorities_, additionals_);
 }
 
 void
@@ -592,8 +589,7 @@ Query::processDSAtChild() {
         }
     }
 
-    response_creator_.create(*response_, answers_, authorities_, additionals_,
-                             dnssec_);
+    response_creator_.create(*response_, answers_, authorities_, additionals_);
     return (true);
 }
 

src/bin/auth/query.h

@@ -312,6 +312,8 @@ public:
     /// providing compatible behavior may have its own benefit, so this point
     /// should be revisited later.
     ///
+    /// The answer will include signatures and NSEC/NSEC3 if possible.
+    ///
     /// This might throw BadZone or any of its specific subclasses, but that
     /// shouldn't happen in real-life (as BadZone means wrong data, it should
     /// have been rejected upon loading).
@@ -321,11 +323,9 @@ public:
     /// \param qname The query name
     /// \param qtype The RR type of the query
     /// \param response The response message to store the answer to the query.
-    /// \param dnssec If the answer should include signatures and NSEC/NSEC3 if
-    ///     possible.
     void process(datasrc::ClientList& client_list,
                  const isc::dns::Name& qname, const isc::dns::RRType& qtype,
-                 isc::dns::Message& response, bool dnssec = false);
+                 isc::dns::Message& response);
 
     /// \short Bad zone data encountered.
     ///
@@ -441,16 +441,13 @@ public:
         /// authority, and additional sections, and add them to their
         /// corresponding sections in the given message.  The RRsets are
         /// filtered such that a particular RRset appears only once in the
-        /// message.
-        ///
-        /// If \c dnssec is true, it tells the message to include any RRSIGs
-        /// attached to the RRsets.
+        /// message. Any RRSIGs attached to the RRsets will be included
+        /// when they are rendered.
         void create(
             isc::dns::Message& message,
             const std::vector<isc::dns::ConstRRsetPtr>& answers_,
             const std::vector<isc::dns::ConstRRsetPtr>& authorities_,
-            const std::vector<isc::dns::ConstRRsetPtr>& additionals_,
-            const bool dnssec);
+            const std::vector<isc::dns::ConstRRsetPtr>& additionals_);
 
     private:
         // \brief RRset comparison functor.
@@ -469,10 +466,9 @@ public:
         /// \param message Message to which the RRset is to be added
         /// \param section Section of the message in which the RRset is put
         /// \param rrset Pointer to RRset to be added to the message
-        /// \param dnssec Whether RRSIG records should be added as well
         void addRRset(isc::dns::Message& message,
                       const isc::dns::Message::Section section,
-                      const isc::dns::ConstRRsetPtr& rrset, const bool dnssec);
+                      const isc::dns::ConstRRsetPtr& rrset);
 
 
     private:

src/bin/auth/tests/query_unittest.cc

@@ -1026,8 +1026,7 @@ TEST_F(QueryTest, exactMatchIgnoreSIG) {
 
 TEST_F(QueryTest, dnssecPositive) {
     // Just like exactMatch, but the signatures should be included as well
-    EXPECT_NO_THROW(query.process(list, qname, qtype, response,
-                                  true));
+    EXPECT_NO_THROW(query.process(list, qname, qtype, response));
     // find match rrset
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 2, 4, 6,
                   (www_a_txt + std::string("www.example.com. 3600 IN RRSIG "
@@ -1139,7 +1138,7 @@ TEST_F(QueryTest, delegationWithDNSSEC) {
     // In this case the parent zone would behave as unsigned, so the result
     // should be just like non DNSSEC delegation.
     query.process(list, Name("www.nosec-delegation.example.com"),
-                  qtype, response, true);
+                  qtype, response);
 
     responseCheck(response, Rcode::NOERROR(), 0, 0, 1, 0,
                   NULL, nosec_delegation_txt, NULL);
@@ -1148,7 +1147,7 @@ TEST_F(QueryTest, delegationWithDNSSEC) {
 TEST_F(QueryTest, secureDelegation) {
     EXPECT_NO_THROW(query.process(list,
                                   Name("foo.signed-delegation.example.com"),
-                                  qtype, response, true));
+                                  qtype, response));
 
     // Should now contain RRSIG and DS record as well.
     responseCheck(response, Rcode::NOERROR(), 0, 0, 3, 0,
@@ -1163,7 +1162,7 @@ TEST_F(QueryTest, secureDelegation) {
 TEST_F(QueryTest, secureUnsignedDelegation) {
     EXPECT_NO_THROW(query.process(list,
                                   Name("foo.unsigned-delegation.example.com"),
-                                  qtype, response, true));
+                                  qtype, response));
 
     // Should now contain RRSIG and NSEC record as well.
     responseCheck(response, Rcode::NOERROR(), 0, 0, 3, 0,
@@ -1184,7 +1183,7 @@ TEST_F(QueryTest, secureUnsignedDelegationWithNSEC3) {
 
     query.process(list,
                   Name("foo.unsigned-delegation.example.com"),
-                  qtype, response, true);
+                  qtype, response);
 
     // The response should contain the NS and matching NSEC3 with its RRSIG
     responseCheck(response, Rcode::NOERROR(), 0, 0, 3, 0,
@@ -1203,7 +1202,7 @@ TEST_F(QueryTest, secureUnsignedDelegationWithNSEC3OptOut) {
 
     query.process(list,
                   Name("foo.unsigned-delegation.example.com"),
-                  qtype, response, true);
+                  qtype, response);
 
     // The response should contain the NS and the closest provable encloser
     // proof (and their RRSIGs).  The closest encloser is the apex (origin),
@@ -1228,7 +1227,7 @@ TEST_F(QueryTest, badSecureDelegation) {
     // something different than SUCCESS or NXRRSET
     EXPECT_THROW(query.process(list,
                                Name("bad-delegation.example.com"),
-                               qtype, response, true), Query::BadDS);
+                               qtype, response), Query::BadDS);
 
     // But only if DNSSEC is requested (it shouldn't even try to look for
     // the DS otherwise)
@@ -1252,7 +1251,7 @@ TEST_F(QueryTest, nxdomainWithNSEC) {
     // as well as their RRSIGs.
     EXPECT_NO_THROW(query.process(list,
                                   Name("nxdomain.example.com"), qtype,
-                                  response, true));
+                                  response));
     responseCheck(response, Rcode::NXDOMAIN(), AA_FLAG, 0, 6, 0,
                   NULL, (string(soa_txt) +
                          string("example.com. 3600 IN RRSIG ") +
@@ -1271,8 +1270,7 @@ TEST_F(QueryTest, nxdomainWithNSEC2) {
     // is derived from the next domain of the NSEC that proves NXDOMAIN, and
     // the NSEC to provide the non existence of wildcard is different from
     // the first NSEC.
-    query.process(list, Name("(.no.example.com"), qtype, response,
-                  true);
+    query.process(list, Name("(.no.example.com"), qtype, response);
     responseCheck(response, Rcode::NXDOMAIN(), AA_FLAG, 0, 6, 0,
                   NULL, (string(soa_txt) +
                          string("example.com. 3600 IN RRSIG ") +
@@ -1289,8 +1287,7 @@ TEST_F(QueryTest, nxdomainWithNSEC2) {
 TEST_F(QueryTest, nxdomainWithNSECDuplicate) {
     // See comments about nz_txt.  In this case we only need one NSEC,
     // which proves both NXDOMAIN and the non existence of wildcard.
-    query.process(list, Name("nx.no.example.com"), qtype, response,
-                  true);
+    query.process(list, Name("nx.no.example.com"), qtype, response);
     responseCheck(response, Rcode::NXDOMAIN(), AA_FLAG, 0, 4, 0,
                   NULL, (string(soa_txt) +
                          string("example.com. 3600 IN RRSIG ") +
@@ -1307,7 +1304,7 @@ TEST_F(QueryTest, nxdomainBadNSEC1) {
                                ZoneFinder::NXDOMAIN,
                                mock_finder->dname_rrset_);
     EXPECT_THROW(query.process(list, Name("badnsec.example.com"),
-                               qtype, response, true),
+                               qtype, response),
                  std::bad_cast);
 }
 
@@ -1317,7 +1314,7 @@ TEST_F(QueryTest, nxdomainBadNSEC2) {
                                ZoneFinder::NXDOMAIN,
                                mock_finder->empty_nsec_rrset_);
     EXPECT_THROW(query.process(list, Name("emptynsec.example.com"),
-                               qtype, response, true),
+                               qtype, response),
                  Query::BadNSEC);
 }
 
@@ -1327,7 +1324,7 @@ TEST_F(QueryTest, nxdomainBadNSEC3) {
                                ZoneFinder::SUCCESS,
                                mock_finder->dname_rrset_);
     EXPECT_THROW(query.process(list, Name("nxdomain.example.com"),
-                               qtype, response, true),
+                               qtype, response),
                  Query::BadNSEC);
 }
 
@@ -1336,7 +1333,7 @@ TEST_F(QueryTest, nxdomainBadNSEC4) {
     mock_finder->setNSECResult(Name("*.example.com"),
                                ZoneFinder::NXDOMAIN, ConstRRsetPtr());
     EXPECT_THROW(query.process(list, Name("nxdomain.example.com"),
-                               qtype, response, true),
+                               qtype, response),
                  Query::BadNSEC);
 }
 
@@ -1346,8 +1343,7 @@ TEST_F(QueryTest, nxdomainBadNSEC5) {
                                ZoneFinder::NXDOMAIN,
                                mock_finder->dname_rrset_);
     // This is a bit odd, but we'll simply include the returned RRset.
-    query.process(list, Name("nxdomain.example.com"), qtype,
-                  response, true);
+    query.process(list, Name("nxdomain.example.com"), qtype, response);
     responseCheck(response, Rcode::NXDOMAIN(), AA_FLAG, 0, 6, 0,
                   NULL, (string(soa_txt) +
                          string("example.com. 3600 IN RRSIG ") +
@@ -1367,7 +1363,7 @@ TEST_F(QueryTest, nxdomainBadNSEC6) {
                                ZoneFinder::NXDOMAIN,
                                mock_finder->empty_nsec_rrset_);
     EXPECT_THROW(query.process(list, Name("nxdomain.example.com"),
-                               qtype, response, true),
+                               qtype, response),
                  Query::BadNSEC);
 }
 
@@ -1382,8 +1378,7 @@ TEST_F(QueryTest, nxrrset) {
 TEST_F(QueryTest, nxrrsetWithNSEC) {
     // NXRRSET with DNSSEC proof.  We should have SOA, NSEC that proves the
     // NXRRSET and their RRSIGs.
-    query.process(list, Name("www.example.com"), RRType::TXT(),
-                  response, true);
+    query.process(list, Name("www.example.com"), RRType::TXT(), response);
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 4, 0, NULL,
                   (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
@@ -1403,8 +1398,7 @@ TEST_F(QueryTest, emptyNameWithNSEC) {
     // exact match), so we only need one NSEC.
     // From the point of the Query::process(), this is actually no different
     // from the other NXRRSET case, but we check that explicitly just in case.
-    query.process(list, Name("no.example.com"), RRType::A(),
-                  response, true);
+    query.process(list, Name("no.example.com"), RRType::A(), response);
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 4, 0, NULL,
                   (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
@@ -1419,8 +1413,7 @@ TEST_F(QueryTest, nxrrsetWithoutNSEC) {
     // NXRRSET with DNSSEC proof requested, but there's no NSEC at that node.
     // This is an unexpected event (if the zone is supposed to be properly
     // signed with NSECs), but we accept and ignore the oddity.
-    query.process(list, Name("nonsec.example.com"), RRType::TXT(),
-                  response, true);
+    query.process(list, Name("nonsec.example.com"), RRType::TXT(), response);
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 2, 0, NULL,
                   (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
@@ -1431,8 +1424,7 @@ TEST_F(QueryTest, nxrrsetWithoutNSEC) {
 TEST_F(QueryTest, wildcardNSEC) {
     // The qname matches *.wild.example.com.  The response should contain
     // an NSEC that proves the non existence of a closer name.
-    query.process(list, Name("www.wild.example.com"), RRType::A(),
-                  response, true);
+    query.process(list, Name("www.wild.example.com"), RRType::A(), response);
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 2, 6, 6,
                   (string(wild_txt).replace(0, 1, "www") +
                    string("www.wild.example.com. 3600 IN RRSIG ") +
@@ -1452,7 +1444,7 @@ TEST_F(QueryTest, CNAMEwildNSEC) {
     // Similar to the previous case, but the matching wildcard record is
     // CNAME.
     query.process(list, Name("www.cnamewild.example.com"),
-                  RRType::A(), response, true);
+                  RRType::A(), response);
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 2, 2, 0,
                   (string(cnamewild_txt).replace(0, 1, "www") +
                    string("www.cnamewild.example.com. 3600 IN RRSIG ") +
@@ -1474,8 +1466,7 @@ TEST_F(QueryTest, wildcardNSEC3) {
     // of identifying the next closer name.
     mock_finder->addRecord(nsec3_atwild_txt);
 
-    query.process(list, Name("x.y.wild.example.com"), RRType::A(),
-                  response, true);
+    query.process(list, Name("x.y.wild.example.com"), RRType::A(), response);
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 2, 6, 6,
                   (string(wild_txt).replace(0, 1, "x.y") +
                    string("x.y.wild.example.com. 3600 IN RRSIG ") +
@@ -1500,7 +1491,7 @@ TEST_F(QueryTest, CNAMEwildNSEC3) {
     mock_finder->addRecord(nsec3_atcnamewild_txt);
 
     query.process(list, Name("www.cnamewild.example.com"),
-                  RRType::A(), response, true);
+                  RRType::A(), response);
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 2, 2, 0,
                   (string(cnamewild_txt).replace(0, 1, "www") +
                    string("www.cnamewild.example.com. 3600 IN RRSIG ") +
@@ -1523,7 +1514,7 @@ TEST_F(QueryTest, badWildcardNSEC3) {
     mock_finder->setNSEC3Result(&nsec3);
 
     EXPECT_THROW(query.process(list, Name("www.wild.example.com"),
-                               RRType::A(), response, true),
+                               RRType::A(), response),
                  Query::BadNSEC3);
 }
 
@@ -1534,7 +1525,7 @@ TEST_F(QueryTest, badWildcardProof1) {
                                ZoneFinder::SUCCESS,
                                mock_finder->dname_rrset_);
     EXPECT_THROW(query.process(list, Name("www.wild.example.com"),
-                               RRType::A(), response, true),
+                               RRType::A(), response),
                  Query::BadNSEC);
 }
 
@@ -1543,7 +1534,7 @@ TEST_F(QueryTest, badWildcardProof2) {
     mock_finder->setNSECResult(Name("www.wild.example.com"),
                                ZoneFinder::NXDOMAIN, ConstRRsetPtr());
     EXPECT_THROW(query.process(list, Name("www.wild.example.com"),
-                               RRType::A(), response, true),
+                               RRType::A(), response),
                  Query::BadNSEC);
 }
 
@@ -1553,7 +1544,7 @@ TEST_F(QueryTest, badWildcardProof3) {
                                ZoneFinder::NXDOMAIN,
                                mock_finder->empty_nsec_rrset_);
     EXPECT_THROW(query.process(list, Name("www.wild.example.com"),
-                               RRType::A(), response, true),
+                               RRType::A(), response),
                  Query::BadNSEC);
 }
 
@@ -1562,7 +1553,7 @@ TEST_F(QueryTest, wildcardNxrrsetWithDuplicateNSEC) {
     // proves the NXRRSET and their RRSIGs. In this case we only need one NSEC,
     // which proves both NXDOMAIN and the non existence RRSETs of wildcard.
     query.process(list, Name("www.wild.example.com"), RRType::TXT(),
-                  response, true);
+                  response);
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 4, 0, NULL,
                   (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
@@ -1579,7 +1570,7 @@ TEST_F(QueryTest, wildcardNxrrsetWithNSEC) {
     // one proves NXDOMAIN and the other proves non existence RRSETs of
     // wildcard.
     query.process(list, Name("www1.uwild.example.com"),
-                  RRType::TXT(), response, true);
+                  RRType::TXT(), response);
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 6, 0, NULL,
                   (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
@@ -1602,7 +1593,7 @@ TEST_F(QueryTest, wildcardNxrrsetWithNSEC3) {
     mock_finder->setNSEC3Flag(true);
 
     query.process(list, Name("www1.uwild.example.com"),
-                  RRType::TXT(), response, true);
+                  RRType::TXT(), response);
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 8, 0, NULL,
                   // SOA + its RRSIG
@@ -1636,7 +1627,7 @@ TEST_F(QueryTest, wildcardNxrrsetWithNSEC3Collision) {
     mock_finder->setNSEC3Result(&nsec3);
 
     EXPECT_THROW(query.process(list, Name("www1.uwild.example.com"),
-                               RRType::TXT(), response, true),
+                               RRType::TXT(), response),
                  Query::BadNSEC3);
 }
 
@@ -1653,7 +1644,7 @@ TEST_F(QueryTest, wildcardNxrrsetWithNSEC3Broken) {
     mock_finder->addRecord(nsec3_uwild_txt);
 
     EXPECT_THROW(query.process(list, Name("www1.uwild.example.com"),
-                               RRType::TXT(), response, true),
+                               RRType::TXT(), response),
                  Query::BadNSEC3);
 }
 
@@ -1661,8 +1652,7 @@ TEST_F(QueryTest, wildcardEmptyWithNSEC) {
     // Empty WILDCARD with DNSSEC proof.  We should have SOA, NSEC that proves
     // the NXDOMAIN and their RRSIGs. In this case we need two NSEC RRs,
     // one proves NXDOMAIN and the other proves non existence wildcard.
-    query.process(list, Name("a.t.example.com"), RRType::A(),
-                  response, true);
+    query.process(list, Name("a.t.example.com"), RRType::A(), response);
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 6, 0, NULL,
                   (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
@@ -2113,7 +2103,7 @@ TEST_F(QueryTest, dsAboveDelegation) {
     // zone, not the child one we added above.
     EXPECT_NO_THROW(query.process(list,
                                   Name("delegation.example.com"),
-                                  RRType::DS(), response, true));
+                                  RRType::DS(), response));
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 2, 4, 6,
                   (string(delegation_ds_txt) + "\n" +
@@ -2137,7 +2127,7 @@ TEST_F(QueryTest, dsAboveDelegationNoData) {
     // zone, not the child one we added above.
     EXPECT_NO_THROW(query.process(list,
                                   Name("unsigned-delegation.example.com"),
-                                  RRType::DS(), response, true));
+                                  RRType::DS(), response));
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 4, 0, NULL,
                   (string(soa_txt) +
@@ -2154,7 +2144,7 @@ TEST_F(QueryTest, dsAboveDelegationNoData) {
 // section 3.1.4.1. The example is inspired by the B.8. example from the RFC.
 TEST_F(QueryTest, dsBelowDelegation) {
     EXPECT_NO_THROW(query.process(list, Name("example.com"),
-                                  RRType::DS(), response, true));
+                                  RRType::DS(), response));
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 4, 0, NULL,
                   (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
@@ -2171,7 +2161,7 @@ TEST_F(QueryTest, dsBelowDelegation) {
 TEST_F(QueryTest, dsBelowDelegationWithDS) {
     mock_finder->addRecord(zone_ds_txt); // add the DS to the child's apex
     EXPECT_NO_THROW(query.process(list, Name("example.com"),
-                                  RRType::DS(), response, true));
+                                  RRType::DS(), response));
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 2, 0, NULL,
                   (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
@@ -2183,8 +2173,7 @@ TEST_F(QueryTest, dsBelowDelegationWithDS) {
 // 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.process(list, Name("example"), RRType::DS(), response,
-                  true);
+    query.process(list, Name("example"), RRType::DS(), response);
     responseCheck(response, Rcode::REFUSED(), 0, 0, 0, 0, NULL, NULL, NULL);
 }
 
@@ -2192,7 +2181,7 @@ TEST_F(QueryTest, dsNoZone) {
 // delegation (unless this server also has authority of the grandchild zone).
 TEST_F(QueryTest, dsAtGrandParent) {
     query.process(list, Name("grand.delegation.example.com"),
-                  RRType::DS(), response, true);
+                  RRType::DS(), response);
     responseCheck(response, Rcode::NOERROR(), 0, 0, 6, 6, NULL,
                   (string(delegation_txt) + string(delegation_ds_txt) +
                    "delegation.example.com. 3600 IN RRSIG " +
@@ -2210,7 +2199,7 @@ TEST_F(QueryTest, dsAtGrandParentAndChild) {
     const Name childname("grand.delegation.example.com");
     memory_client.addZone(ZoneFinderPtr(
                               new AlternateZoneFinder(childname)));
-    query.process(list, childname, RRType::DS(), response, true);
+    query.process(list, childname, RRType::DS(), response);
     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 " +
@@ -2228,8 +2217,7 @@ TEST_F(QueryTest, dsAtRoot) {
     // Pretend to be a root server.
     memory_client.addZone(ZoneFinderPtr(
                               new AlternateZoneFinder(Name::ROOT_NAME())));
-    query.process(list, Name::ROOT_NAME(), RRType::DS(), response,
-                  true);
+    query.process(list, Name::ROOT_NAME(), RRType::DS(), response);
     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" +
@@ -2245,8 +2233,7 @@ TEST_F(QueryTest, dsAtRootWithDS) {
     memory_client.addZone(ZoneFinderPtr(
                               new AlternateZoneFinder(Name::ROOT_NAME(),
                                                       true)));
-    query.process(list, Name::ROOT_NAME(), RRType::DS(), response,
-                  true);
+    query.process(list, Name::ROOT_NAME(), RRType::DS(), response);
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 2, 2, 0,
                   (string(". 3600 IN DS 57855 5 1 49FD46E6C4B45C55D4AC69CBD"
                           "3CD34AC1AFE51DE\n") +
@@ -2262,8 +2249,7 @@ TEST_F(QueryTest, nxrrsetWithNSEC3) {
 
     // NXRRSET with DNSSEC proof.  We should have SOA, NSEC3 that proves the
     // NXRRSET and their RRSIGs.
-    query.process(list, Name("www.example.com"), RRType::TXT(),
-                  response, true);
+    query.process(list, Name("www.example.com"), RRType::TXT(), response);
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 4, 0, NULL,
                   (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
@@ -2286,7 +2272,7 @@ TEST_F(QueryTest, nxrrsetMissingNSEC3) {
     mock_finder->setNSEC3Result(&nsec3);
 
     EXPECT_THROW(query.process(list, Name("www.example.com"),
-                               RRType::TXT(), response, true),
+                               RRType::TXT(), response),
                  Query::BadNSEC3);
 }
 
@@ -2297,7 +2283,7 @@ TEST_F(QueryTest, nxrrsetWithNSEC3_ds_exact) {
     // This delegation has no DS, but does have a matching NSEC3 record
     // (See RFC5155 section 7.2.4)
     query.process(list, Name("unsigned-delegation.example.com."),
-                  RRType::DS(), response, true);
+                  RRType::DS(), response);
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 4, 0, NULL,
                   (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
                    getCommonRRSIGText("SOA") + "\n" +
@@ -2319,7 +2305,7 @@ TEST_F(QueryTest, nxrrsetWithNSEC3_ds_no_exact) {
     // actually checked)
     // (See RFC5155 section 7.2.4)
     query.process(list, Name("unsigned-delegation-optout.example.com."),
-                  RRType::DS(), response, true);
+                  RRType::DS(), response);
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 6, 0, NULL,
                   (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
                    getCommonRRSIGText("SOA") + "\n" +
@@ -2345,8 +2331,7 @@ TEST_F(QueryTest, nxdomainWithNSEC3Proof) {
     // This will be the covering NSEC3 for the possible wildcard
     mock_finder->addRecord(unsigned_delegation_nsec3_txt);
 
-    query.process(list, Name("nxdomain.example.com"), qtype,
-                  response, true);
+    query.process(list, Name("nxdomain.example.com"), qtype, response);
     responseCheck(response, Rcode::NXDOMAIN(), AA_FLAG, 0, 8, 0, NULL,
                   // SOA + its RRSIG
                   (string(soa_txt) +
@@ -2381,7 +2366,7 @@ TEST_F(QueryTest, nxdomainWithBadNextNSEC3Proof) {
     mock_finder->setNSEC3Result(&nsec3);
 
     EXPECT_THROW(query.process(list, Name("nxdomain.example.com"),
-                               RRType::TXT(), response, true),
+                               RRType::TXT(), response),
                  Query::BadNSEC3);
 }
 
@@ -2400,7 +2385,7 @@ TEST_F(QueryTest, nxdomainWithBadWildcardNSEC3Proof) {
     mock_finder->setNSEC3Result(&nsec3, &wname);
 
     EXPECT_THROW(query.process(list, Name("nxdomain.example.com"), qtype,
-                               response, true),
+                               response),
                  Query::BadNSEC3);
 }
 
@@ -2516,8 +2501,7 @@ TEST_F(QueryTest, DuplicateNameRemoval) {
     EXPECT_EQ(0, message.getRRCount(Message::SECTION_ADDITIONAL));
 
     // ... and fill it.
-    Query::ResponseCreator().create(message, answer, authority, additional,
-                                    false);
+    Query::ResponseCreator().create(message, answer, authority, additional);
 
     // Check counts in each section.  Note that these are RR counts,
     // not RRset counts.

src/lib/cache/message_entry.cc

@@ -110,8 +110,7 @@ MessageEntry::getRRsetEntries(vector<RRsetEntryPtr>& rrset_entry_vec,
 void
 MessageEntry::addRRset(isc::dns::Message& message,
                        const vector<RRsetEntryPtr>& rrset_entry_vec,
-                       const isc::dns::Message::Section& section,
-                       bool dnssec_need)
+                       const isc::dns::Message::Section& section)
 {
     uint16_t start_index = 0;
     uint16_t end_index = answer_count_;
@@ -126,8 +125,7 @@ MessageEntry::addRRset(isc::dns::Message& message,
     }
 
     for (uint16_t index = start_index; index < end_index; ++index) {
-        message.addRRset(section, rrset_entry_vec[index]->getRRset(),
-                         dnssec_need);
+        message.addRRset(section, rrset_entry_vec[index]->getRRset());
     }
 }
 
@@ -156,10 +154,9 @@ MessageEntry::genMessage(const time_t& time_now,
         msg.setHeaderFlag(Message::HEADERFLAG_AA, false);
         msg.setHeaderFlag(Message::HEADERFLAG_TC, headerflag_tc_);
 
-        bool dnssec_need = msg.getEDNS().get();
-        addRRset(msg, rrset_entry_vec, Message::SECTION_ANSWER, dnssec_need);
-        addRRset(msg, rrset_entry_vec, Message::SECTION_AUTHORITY, dnssec_need);
-        addRRset(msg, rrset_entry_vec, Message::SECTION_ADDITIONAL, dnssec_need);
+        addRRset(msg, rrset_entry_vec, Message::SECTION_ANSWER);
+        addRRset(msg, rrset_entry_vec, Message::SECTION_AUTHORITY);
+        addRRset(msg, rrset_entry_vec, Message::SECTION_ADDITIONAL);
 
         return (true);
     }

src/lib/cache/message_entry.h

@@ -154,11 +154,9 @@ protected:
     /// \param rrset_entry_vec vector for rrset entries in
     ///        different sections.
     /// \param section The section to add to
-    /// \param dnssec_need need dnssec records or not.
     void addRRset(isc::dns::Message& message,
                   const std::vector<RRsetEntryPtr>& rrset_entry_vec,
-                  const isc::dns::Message::Section& section,
-                  bool dnssec_need);
+                  const isc::dns::Message::Section& section);
 
     /// \brief Get the all the rrset entries for the message entry.
     ///

src/lib/datasrc/data_source.cc

@@ -564,12 +564,12 @@ addToMessage(Query& q, const Message::Section sect, RRsetPtr rrset,
         if (rrset->getType() == RRType::RRSIG() ||
             !m.hasRRset(sect, rrset->getName(), rrset->getClass(),
                         rrset->getType())) {
-            m.addRRset(sect, rrset, false);
+            m.addRRset(sect, rrset);
         }
     } else {
         if (!m.hasRRset(sect, rrset->getName(), rrset->getClass(),
                         rrset->getType())) {
-            m.addRRset(sect, rrset, q.wantDnssec());
+            m.addRRset(sect, rrset);
         }
     }
 }

src/lib/dns/message.cc

@@ -493,7 +493,7 @@ Message::getRRCount(const Section section) const {
 }
 
 void
-Message::addRRset(const Section section, RRsetPtr rrset, const bool sign) {
+Message::addRRset(const Section section, RRsetPtr rrset) {
     if (!rrset) {
         isc_throw(InvalidParameter,
                   "NULL RRset is given to Message::addRRset");
@@ -508,12 +508,7 @@ Message::addRRset(const Section section, RRsetPtr rrset, const bool sign) {
 
     impl_->rrsets_[section].push_back(rrset);
     impl_->counts_[section] += rrset->getRdataCount();
-
-    RRsetPtr sp = rrset->getRRsig();
-    if (sign && sp != NULL) {
-        impl_->rrsets_[section].push_back(sp);
-        impl_->counts_[section] += sp->getRdataCount();
-    }
+    impl_->counts_[section] += rrset->getRRsigDataCount();
 }
 
 bool
@@ -555,6 +550,7 @@ Message::removeRRset(const Section section, RRsetIterator& iterator) {
 
             // Found the matching RRset so remove it & ignore rest
             impl_->counts_[section] -= (*iterator)->getRdataCount();
+            impl_->counts_[section] -= (*iterator)->getRRsigDataCount();
             impl_->rrsets_[section].erase(i);
             removed = true;
             break;

src/lib/dns/message.h

@@ -459,9 +459,6 @@ public:
     /// \brief Add a (pointer like object of) RRset to the given section
     /// of the message.
     ///
-    /// This interface takes into account the RRSIG possibly attached to
-    /// \c rrset.  This interface design needs to be revisited later.
-    ///
     /// Note that \c addRRset() does not currently check for duplicate
     /// data before inserting RRsets.  The caller is responsible for
     /// checking for these (see \c hasRRset() below).
@@ -473,9 +470,7 @@ public:
     ///
     /// \param section The message section to which the rrset is to be added
     /// \param rrset The rrset to be added.  Must not be NULL.
-    /// \param sign If true, and if \c rrset has associated RRSIGs, the
-    /// RRSIGs will also be added to the same section of the message.
-    void addRRset(const Section section, RRsetPtr rrset, bool sign = false);
+    void addRRset(const Section section, RRsetPtr rrset);
 
     /// \brief Determine whether the given section already has an RRset
     /// matching the given name, RR class and RR type.

src/lib/dns/python/message_python.cc

@@ -143,9 +143,7 @@ PyMethodDef Message_methods[] = {
     { "add_rrset", reinterpret_cast<PyCFunction>(Message_addRRset), METH_VARARGS,
       "Add an RRset to the given section of the message.\n"
       "The first argument is of type Section\n"
-      "The second is of type RRset\n"
-      "The third argument is an optional Boolean specifying whether "
-      "the RRset is signed"},
+      "The second is of type RRset"},
     { "clear", reinterpret_cast<PyCFunction>(Message_clear), METH_VARARGS,
       "Clears the message content (if any) and reinitialize the "
       "message in the given mode\n"
@@ -571,17 +569,15 @@ Message_addQuestion(s_Message* self, PyObject* args) {
 
 PyObject*
 Message_addRRset(s_Message* self, PyObject* args) {
-    PyObject *sign = Py_False;
     int section;
     PyObject* rrset;
-    if (!PyArg_ParseTuple(args, "iO!|O!", &section, &rrset_type, &rrset,
-                          &PyBool_Type, &sign)) {
+    if (!PyArg_ParseTuple(args, "iO!", &section, &rrset_type, &rrset)) {
         return (NULL);
     }
 
     try {
         self->cppobj->addRRset(static_cast<Message::Section>(section),
-                               PyRRset_ToRRsetPtr(rrset), sign == Py_True);
+                               PyRRset_ToRRsetPtr(rrset));
         Py_RETURN_NONE;
     } catch (const InvalidMessageOperation& imo) {
         PyErr_SetString(po_InvalidMessageOperation, imo.what());

src/lib/dns/tests/message_unittest.cc

@@ -289,32 +289,22 @@ TEST_F(MessageTest, getRRCount) {
 }
 
 TEST_F(MessageTest, addRRset) {
-    // default case
+    // initially, we have 0
+    EXPECT_EQ(0, message_render.getRRCount(Message::SECTION_ANSWER));
+
+    // add two A RRs (unsigned)
     message_render.addRRset(Message::SECTION_ANSWER, rrset_a);
     EXPECT_EQ(rrset_a,
               *message_render.beginSection(Message::SECTION_ANSWER));
     EXPECT_EQ(2, message_render.getRRCount(Message::SECTION_ANSWER));
 
-    // signed RRset, default case
     message_render.clear(Message::RENDER);
-    message_render.addRRset(Message::SECTION_ANSWER, rrset_aaaa);
-    EXPECT_EQ(rrset_aaaa,
-              *message_render.beginSection(Message::SECTION_ANSWER));
-    EXPECT_EQ(1, message_render.getRRCount(Message::SECTION_ANSWER));
 
-    // signed RRset, add with the RRSIG.  getRRCount() should return 2
-    message_render.clear(Message::RENDER);
-    message_render.addRRset(Message::SECTION_ANSWER, rrset_aaaa, true);
+    // add one AAAA RR (signed)
+    message_render.addRRset(Message::SECTION_ANSWER, rrset_aaaa);
     EXPECT_EQ(rrset_aaaa,
               *message_render.beginSection(Message::SECTION_ANSWER));
     EXPECT_EQ(2, message_render.getRRCount(Message::SECTION_ANSWER));
-
-    // signed RRset, add explicitly without RRSIG.
-    message_render.clear(Message::RENDER);
-    message_render.addRRset(Message::SECTION_ANSWER, rrset_aaaa, false);
-    EXPECT_EQ(rrset_aaaa,
-              *message_render.beginSection(Message::SECTION_ANSWER));
-    EXPECT_EQ(1, message_render.getRRCount(Message::SECTION_ANSWER));
 }
 
 TEST_F(MessageTest, badAddRRset) {
@@ -381,7 +371,7 @@ TEST_F(MessageTest, removeRRset) {
         RRClass::IN(), RRType::A()));
     EXPECT_TRUE(message_render.hasRRset(Message::SECTION_ANSWER, test_name,
         RRClass::IN(), RRType::AAAA()));
-    EXPECT_EQ(3, message_render.getRRCount(Message::SECTION_ANSWER));
+    EXPECT_EQ(4, message_render.getRRCount(Message::SECTION_ANSWER));
 
     // Locate the AAAA RRset and remove it; this has one RR in it.
     RRsetIterator i = message_render.beginSection(Message::SECTION_ANSWER);
@@ -420,7 +410,7 @@ TEST_F(MessageTest, clearAnswerSection) {
         RRClass::IN(), RRType::A()));
     ASSERT_TRUE(message_render.hasRRset(Message::SECTION_ANSWER, test_name,
         RRClass::IN(), RRType::AAAA()));
-    ASSERT_EQ(3, message_render.getRRCount(Message::SECTION_ANSWER));
+    ASSERT_EQ(4, message_render.getRRCount(Message::SECTION_ANSWER));
 
     message_render.clearSection(Message::SECTION_ANSWER);
     EXPECT_FALSE(message_render.hasRRset(Message::SECTION_ANSWER, test_name,
@@ -439,7 +429,7 @@ TEST_F(MessageTest, clearAuthoritySection) {
         RRClass::IN(), RRType::A()));
     ASSERT_TRUE(message_render.hasRRset(Message::SECTION_AUTHORITY, test_name,
         RRClass::IN(), RRType::AAAA()));
-    ASSERT_EQ(3, message_render.getRRCount(Message::SECTION_AUTHORITY));
+    ASSERT_EQ(4, message_render.getRRCount(Message::SECTION_AUTHORITY));
 
     message_render.clearSection(Message::SECTION_AUTHORITY);
     EXPECT_FALSE(message_render.hasRRset(Message::SECTION_AUTHORITY, test_name,
@@ -458,7 +448,7 @@ TEST_F(MessageTest, clearAdditionalSection) {
         RRClass::IN(), RRType::A()));
     ASSERT_TRUE(message_render.hasRRset(Message::SECTION_ADDITIONAL, test_name,
         RRClass::IN(), RRType::AAAA()));
-    ASSERT_EQ(3, message_render.getRRCount(Message::SECTION_ADDITIONAL));
+    ASSERT_EQ(4, message_render.getRRCount(Message::SECTION_ADDITIONAL));
 
     message_render.clearSection(Message::SECTION_ADDITIONAL);
     EXPECT_FALSE(message_render.hasRRset(Message::SECTION_ADDITIONAL, test_name,
@@ -529,7 +519,7 @@ TEST_F(MessageTest, appendSection) {
         RRClass::IN(), RRType::A()));
 
     target.appendSection(Message::SECTION_ADDITIONAL, message_render);
-    EXPECT_EQ(3, target.getRRCount(Message::SECTION_ADDITIONAL));
+    EXPECT_EQ(4, target.getRRCount(Message::SECTION_ADDITIONAL));
     EXPECT_TRUE(target.hasRRset(Message::SECTION_ADDITIONAL, test_name,
         RRClass::IN(), RRType::A()));
     EXPECT_TRUE(target.hasRRset(Message::SECTION_ADDITIONAL, test_name,
@@ -539,7 +529,7 @@ TEST_F(MessageTest, appendSection) {
     Message source2(Message::RENDER);
     source2.addRRset(Message::SECTION_ANSWER, rrset_aaaa);
     target.appendSection(Message::SECTION_ANSWER, source2);
-    EXPECT_EQ(3, target.getRRCount(Message::SECTION_ANSWER));
+    EXPECT_EQ(4, target.getRRCount(Message::SECTION_ANSWER));
     EXPECT_TRUE(target.hasRRset(Message::SECTION_ANSWER, test_name,
         RRClass::IN(), RRType::A()));
     EXPECT_TRUE(target.hasRRset(Message::SECTION_ANSWER, test_name,
@@ -766,7 +756,7 @@ TEST_F(MessageTest, toWireSigned) {
     rrset_a->addRRsig(rrset_rrsig);
     EXPECT_EQ(2, rrset_a->getRRsigDataCount());
 
-    message_render.addRRset(Message::SECTION_ANSWER, rrset_a, true);
+    message_render.addRRset(Message::SECTION_ANSWER, rrset_a);
 
     EXPECT_EQ(1, message_render.getRRCount(Message::SECTION_QUESTION));
     EXPECT_EQ(4, message_render.getRRCount(Message::SECTION_ANSWER));

src/lib/resolve/resolve.cc

@@ -26,7 +26,7 @@ namespace {
             message_(message), section_(sect)
         {}
         void operator()(const RRsetPtr rrset) {
-            message_->addRRset(section_, rrset, true);
+            message_->addRRset(section_, rrset);
         }
         MessagePtr message_;
         const Message::Section section_;