[master] Merge branch 'trac1585'

src/bin/auth/query.cc

@@ -214,10 +214,31 @@ Query::addDS(ZoneFinder& finder, const Name& dname) {
         finder.find(dname, RRType::DS(), dnssec_opt_);
     if (ds_result.code == ZoneFinder::SUCCESS) {
         response_.addRRset(Message::SECTION_AUTHORITY,
-                           boost::const_pointer_cast<AbstractRRset>(ds_result.rrset),
+                           boost::const_pointer_cast<AbstractRRset>(
+                               ds_result.rrset),
                            dnssec_);
-    } else if (ds_result.code == ZoneFinder::NXRRSET) {
+    } else if (ds_result.code == ZoneFinder::NXRRSET &&
+               ds_result.isNSECSigned()) {
         addNXRRsetProof(finder, ds_result);
+    } else if (ds_result.code == ZoneFinder::NXRRSET &&
+               ds_result.isNSEC3Signed()) {
+        // Add no DS proof with NSEC3 as specified in RFC5155 Section 7.2.7.
+        // Depending on whether the zone is optout or not, findNSEC3() may
+        // return non-NULL or NULL next_proof (respectively).  The Opt-Out flag
+        // must be set or cleared accordingly, but we don't check that
+        // in this level (as long as the zone signed validly and findNSEC3()
+        // is valid, the condition should be met; otherwise we'd let the
+        // validator detect the error).
+        const ZoneFinder::FindNSEC3Result nsec3_result =
+            finder.findNSEC3(dname, true);
+        response_.addRRset(Message::SECTION_AUTHORITY,
+                           boost::const_pointer_cast<AbstractRRset>(
+                               nsec3_result.closest_proof), dnssec_);
+        if (nsec3_result.next_proof) {
+            response_.addRRset(Message::SECTION_AUTHORITY,
+                               boost::const_pointer_cast<AbstractRRset>(
+                                   nsec3_result.next_proof), dnssec_);
+        }
     } else {
         // Any other case should be an error
         isc_throw(BadDS, "Unexpected result for DS lookup for delegation");
@@ -236,7 +257,7 @@ Query::addNXRRsetProof(ZoneFinder& finder,
         if (db_result.isWildcard()) {
             addWildcardNXRRSETProof(finder, db_result.rrset);
         }
-    } else if (db_result.isNSEC3Signed()) {
+    } else if (db_result.isNSEC3Signed() && !db_result.isWildcard()) {
         // Handling depends on whether query type is DS or not
         // (see RFC5155, 7.2.3 and 7.2.4):  If qtype == DS, do
         // recursive search (and add next_proof, if necessary),
@@ -256,8 +277,37 @@ Query::addNXRRsetProof(ZoneFinder& finder,
                                        result.next_proof), dnssec_);
             }
         } else {
-            isc_throw(BadNSEC3, "No NSEC3 found for existing domain " <<
-                      qname_.toText());
+            isc_throw(BadNSEC3, "No matching NSEC3 found for existing domain "
+                      << qname_);
+        }
+    } else if (db_result.isNSEC3Signed() && db_result.isWildcard()) {
+        // Case for RFC5155 Section 7.2.5
+        const ZoneFinder::FindNSEC3Result result(finder.findNSEC3(qname_,
+                                                                  true));
+        // We know there's no exact match for the qname, so findNSEC3() should
+        // return both closest and next proofs.  If the latter is NULL, it
+        // means a run time collision (or the zone is broken in other way).
+        // In that case addRRset() will throw, and it will be converted to
+        // SERVFAIL.
+        response_.addRRset(Message::SECTION_AUTHORITY,
+                           boost::const_pointer_cast<AbstractRRset>(
+                               result.closest_proof), dnssec_);
+        response_.addRRset(Message::SECTION_AUTHORITY,
+                           boost::const_pointer_cast<AbstractRRset>(
+                               result.next_proof), dnssec_);
+
+        // Construct the matched wildcard name and add NSEC3 for it.
+        const Name wname = Name("*").concatenate(
+            qname_.split(qname_.getLabelCount() - result.closest_labels));
+        const ZoneFinder::FindNSEC3Result wresult(finder.findNSEC3(wname,
+                                                                   false));
+        if (wresult.matched) {
+            response_.addRRset(Message::SECTION_AUTHORITY,
+                               boost::const_pointer_cast<AbstractRRset>(
+                                   wresult.closest_proof), dnssec_);
+        } else {
+            isc_throw(BadNSEC3, "No matching NSEC3 found for existing domain "
+                      << wname);
         }
     }
 }

src/bin/auth/tests/query_unittest.cc

@@ -185,6 +185,16 @@ const char* const nsec3_www_txt =
     "q04jkcevqvmu85r014c7dkba38o0ji5r.example.com. 3600 IN NSEC3 1 1 12 "
     "aabbccdd r53bq7cc2uvmubfu5ocmm6pers9tk9en A RRSIG\n";
 
+// NSEC3 for *.uwild.example.com (will be added on demand not to confuse
+// other tests)
+const char* const nsec3_wild_txt =
+    "b4um86eghhds6nea196smvmlo4ors995.example.com. 3600 IN NSEC3 1 1 12 "
+    "aabbccdd r53bq7cc2uvmubfu5ocmm6pers9tk9en A RRSIG\n";
+// NSEC3 for uwild.example.com. (will be added on demand)
+const char* const nsec3_uwild_txt =
+    "t644ebqk9bibcna874givr6joj62mlhv.example.com. 3600 IN NSEC3 1 1 12 "
+    "aabbccdd r53bq7cc2uvmubfu5ocmm6pers9tk9en A RRSIG\n";
+
 // (Secure) delegation data; Delegation with DS record
 const char* const signed_delegation_txt =
     "signed-delegation.example.com. 3600 IN NS ns.example.net.\n";
@@ -199,6 +209,7 @@ const char* const unsigned_delegation_txt =
 const char* const unsigned_delegation_nsec_txt =
     "unsigned-delegation.example.com. 3600 IN NSEC "
     "unsigned-delegation-optout.example.com. NS RRSIG NSEC\n";
+// This one will be added on demand
 const char* const unsigned_delegation_nsec3_txt =
     "q81r598950igr1eqvc60aedlq66425b5.example.com. 3600 IN NSEC3 1 1 12 "
     "aabbccdd 0p9mhaveqvm6t7vbl5lop2u3t2rp3tom NS RRSIG\n";
@@ -271,7 +282,8 @@ public:
         include_rrsig_anyway_(false),
         use_nsec3_(false),
         nsec_name_(origin_),
-        nsec3_fake_(NULL)
+        nsec3_fake_(NULL),
+        nsec3_name_(NULL)
     {
         stringstream zone_stream;
         zone_stream << soa_txt << zone_ns_txt << ns_addrs_txt <<
@@ -287,7 +299,7 @@ public:
             nsec3_apex_txt << nsec3_www_txt <<
             signed_delegation_txt << signed_delegation_ds_txt <<
             unsigned_delegation_txt << unsigned_delegation_nsec_txt <<
-            unsigned_delegation_nsec3_txt << unsigned_delegation_optout_txt <<
+            unsigned_delegation_optout_txt <<
             unsigned_delegation_optout_nsec_txt <<
             bad_delegation_txt;
 
@@ -318,9 +330,17 @@ public:
         hash_map_[Name("nxdomain3.example.com")] =
             "009mhaveqvm6t7vbl5lop2u3t2rp3tom";
         hash_map_[Name("unsigned-delegation.example.com")] =
-            "q81r598950igr1eqvc60aedlq66425b5";
+            "q81r598950igr1eqvc60aedlq66425b5"; // a bit larger than H(www)
+        hash_map_[Name("*.uwild.example.com")] =
+            "b4um86eghhds6nea196smvmlo4ors995";
         hash_map_[Name("unsigned-delegation-optout.example.com")] =
             "vld46lphhasfapj8og1pglgiasa5o5gt";
+
+        // For closest encloser proof for www1.uwild.example.com:
+        hash_map_[Name("uwild.example.com")] =
+            "t644ebqk9bibcna874givr6joj62mlhv";
+        hash_map_[Name("www1.uwild.example.com")] =
+            "q04jkcevqvmu85r014c7dkba38o0ji6r"; // a bit larger than H(www)
     }
     virtual isc::dns::Name getOrigin() const { return (origin_); }
     virtual isc::dns::RRClass getClass() const { return (rrclass_); }
@@ -358,8 +378,11 @@ public:
     // query. After that, it'll return to operate normally.
     // NULL disables. Does not take ownership of the pointer (it is generally
     // expected to be a local variable in the test function).
-    void setNSEC3Result(const FindNSEC3Result* result) {
+    void setNSEC3Result(const FindNSEC3Result* result,
+                        const Name* name = NULL)
+    {
         nsec3_fake_ = result;
+        nsec3_name_ = name;
     }
 
     // If true is passed return an empty NSEC3 RRset for some negative
@@ -459,6 +482,7 @@ private:
     // The following two are for faking bad NSEC3 responses
     // Enabled when not NULL
     const FindNSEC3Result* nsec3_fake_;
+    const Name* nsec3_name_;
 public:
     // Public, to allow tests looking up the right names for something
     map<Name, string> hash_map_;
@@ -506,10 +530,9 @@ MockZoneFinder::findAll(const Name& name, std::vector<ConstRRsetPtr>& target,
 ZoneFinder::FindNSEC3Result
 MockZoneFinder::findNSEC3(const Name& name, bool recursive) {
     // Do we have a fake result set? If so, use it.
-    if (nsec3_fake_ != NULL) {
+    if (nsec3_fake_ != NULL &&
+        (nsec3_name_ == NULL || *nsec3_name_ == name)) {
         const FindNSEC3Result* result(nsec3_fake_);
-        // Disable the fake for the next call
-        nsec3_fake_ = NULL;
         return (*result);
     }
 
@@ -520,6 +543,10 @@ MockZoneFinder::findNSEC3(const Name& name, bool recursive) {
     // expected entry when operator[] is used; maps are not empty.
     for (int i = 0; i < labels; ++i) {
         const string hlabel = hash_map_[name.split(i, labels - i)];
+        if (hlabel.empty()) {
+            isc_throw(isc::Unexpected, "findNSEC3() hash failure for " <<
+                      name.split(i, labels - i));
+        }
         const Name hname = Name(hlabel + ".example.com");
         // We don't use const_iterator so that we can use operator[] below
         Domains::iterator found_domain = nsec3_domains_.lower_bound(hname);
@@ -698,7 +725,8 @@ MockZoneFinder::find(const Name& name, const RRType& type,
                                             RESULT_NSEC3_SIGNED :
                                             RESULT_NSEC_SIGNED)));
                     } else {
-                        // No matched QTYPE, this case is for WILDCARD_NXRRSET
+                        // No matched QTYPE, this case is for NXRRSET with
+                        // WILDCARD
                         if (use_nsec3_) {
                             return (FindResult(NXRRSET, RRsetPtr(),
                                                RESULT_WILDCARD |
@@ -1028,6 +1056,53 @@ TEST_F(QueryTest, secureUnsignedDelegation) {
                   NULL);
 }
 
+TEST_F(QueryTest, secureUnsignedDelegationWithNSEC3) {
+    // Similar to the previous case, but the zone is signed with NSEC3,
+    // and this delegation is NOT an optout.
+    const Name insecurechild_name("unsigned-delegation.example.com");
+    mock_finder->setNSEC3Flag(true);
+    mock_finder->addRecord(unsigned_delegation_nsec3_txt);
+
+    Query(memory_client, Name("foo.unsigned-delegation.example.com"),
+          qtype, response, true).process();
+
+    // The response should contain the NS and matching NSEC3 with its RRSIG
+    responseCheck(response, Rcode::NOERROR(), 0, 0, 3, 0,
+                  NULL,
+                  (string(unsigned_delegation_txt) +
+                   string(unsigned_delegation_nsec3_txt) +
+                   mock_finder->hash_map_[insecurechild_name] +
+                   ".example.com. 3600 IN RRSIG " +
+                   getCommonRRSIGText("NSEC3")).c_str(),
+                  NULL);
+}
+
+TEST_F(QueryTest, secureUnsignedDelegationWithNSEC3OptOut) {
+    // Similar to the previous case, but the delegation is an optout.
+    mock_finder->setNSEC3Flag(true);
+
+    Query(memory_client, Name("foo.unsigned-delegation.example.com"),
+          qtype, response, true).process();
+
+    // The response should contain the NS and the closest provable encloser
+    // proof (and their RRSIGs).  The closest encloser is the apex (origin),
+    // and with our faked hash the covering NSEC3 for the next closer
+    // (= child zone name) is that for www.example.com.
+    cout << response << endl;
+    responseCheck(response, Rcode::NOERROR(), 0, 0, 5, 0,
+                  NULL,
+                  (string(unsigned_delegation_txt) +
+                   string(nsec3_apex_txt) +
+                   mock_finder->hash_map_[mock_finder->getOrigin()] +
+                   ".example.com. 3600 IN RRSIG " +
+                   getCommonRRSIGText("NSEC3") + "\n" +
+                   string(nsec3_www_txt) +
+                   mock_finder->hash_map_[Name("www.example.com")] +
+                   ".example.com. 3600 IN RRSIG " +
+                   getCommonRRSIGText("NSEC3")).c_str(),
+                  NULL);
+}
+
 TEST_F(QueryTest, badSecureDelegation) {
     // Test whether exception is raised if DS query at delegation results in
     // something different than SUCCESS or NXRRSET
@@ -1296,8 +1371,8 @@ TEST_F(QueryTest, badWildcardProof3) {
 }
 
 TEST_F(QueryTest, wildcardNxrrsetWithDuplicateNSEC) {
-    // WILDCARD_NXRRSET with DNSSEC proof.  We should have SOA, NSEC that proves the
-    // NXRRSET and their RRSIGs. In this case we only need one NSEC,
+    // NXRRSET on WILDCARD with DNSSEC proof.  We should have SOA, NSEC that
+    // 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(memory_client, Name("www.wild.example.com"), RRType::TXT(), response,
           true).process();
@@ -1312,11 +1387,12 @@ TEST_F(QueryTest, wildcardNxrrsetWithDuplicateNSEC) {
 }
 
 TEST_F(QueryTest, wildcardNxrrsetWithNSEC) {
-    // WILDCARD_NXRRSET with DNSSEC proof.  We should have SOA, NSEC that proves the
-    // NXRRSET and their RRSIGs. In this case we need two NSEC RRs,
-    // one proves NXDOMAIN and the other proves non existence RRSETs of wildcard.
-    Query(memory_client, Name("www1.uwild.example.com"), RRType::TXT(), response,
-          true).process();
+    // WILDCARD + NXRRSET with DNSSEC proof.  We should have SOA, NSEC that
+    // proves the NXRRSET and their RRSIGs. In this case we need two NSEC RRs,
+    // one proves NXDOMAIN and the other proves non existence RRSETs of
+    // wildcard.
+    Query(memory_client, Name("www1.uwild.example.com"), RRType::TXT(),
+          response, true).process();
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 6, 0, NULL,
                   (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
@@ -1330,9 +1406,74 @@ TEST_F(QueryTest, wildcardNxrrsetWithNSEC) {
                   NULL, mock_finder->getOrigin());
 }
 
+TEST_F(QueryTest, wildcardNxrrsetWithNSEC3) {
+    // Similar to the previous case, but providing NSEC3 proofs according to
+    // RFC5155 Section 7.2.5.
+
+    mock_finder->addRecord(nsec3_wild_txt);
+    mock_finder->addRecord(nsec3_uwild_txt);
+    mock_finder->setNSEC3Flag(true);
+
+    Query(memory_client, Name("www1.uwild.example.com"), RRType::TXT(),
+          response, true).process();
+
+    responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 8, 0, NULL,
+                  // SOA + its RRSIG
+                  (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
+                   getCommonRRSIGText("SOA") + "\n" +
+                   // NSEC3 for the closest encloser + its RRSIG
+                   string(nsec3_uwild_txt) +
+                   mock_finder->hash_map_[Name("uwild.example.com.")] +
+                   ".example.com. 3600 IN RRSIG " +
+                   getCommonRRSIGText("NSEC3") + "\n" +
+                   // NSEC3 for the next closer + its RRSIG
+                   string(nsec3_www_txt) +
+                   mock_finder->hash_map_[Name("www.example.com.")] +
+                   ".example.com. 3600 IN RRSIG " +
+                   getCommonRRSIGText("NSEC3") + "\n" +
+                   // NSEC3 for the wildcard + its RRSIG
+                   string(nsec3_wild_txt) +
+                   mock_finder->hash_map_[Name("*.uwild.example.com.")] +
+                   ".example.com. 3600 IN RRSIG " +
+                   getCommonRRSIGText("NSEC3")).c_str(),
+                  NULL, mock_finder->getOrigin());
+}
+
+TEST_F(QueryTest, wildcardNxrrsetWithNSEC3Collision) {
+    // Similar to the previous case, but emulating run time collision by
+    // returning NULL in the next closer proof for the closest encloser
+    // proof.
+    mock_finder->setNSEC3Flag(true);
+    ZoneFinder::FindNSEC3Result nsec3(true, 0, textToRRset(nsec3_apex_txt),
+                                      ConstRRsetPtr());
+    mock_finder->setNSEC3Result(&nsec3);
+
+    // Message::addRRset() will detect it and throw InvalidParameter.
+    EXPECT_THROW(Query(memory_client, Name("www1.uwild.example.com"),
+                       RRType::TXT(), response, true).process(),
+                 isc::InvalidParameter);
+}
+
+TEST_F(QueryTest, wildcardNxrrsetWithNSEC3Broken) {
+    // Similar to wildcardNxrrsetWithNSEC3, but no matching NSEC3 for the
+    // wildcard name will be returned.  This shouldn't happen in a reasonably
+    // NSEC-signed zone, and should result in an exception.
+    mock_finder->setNSEC3Flag(true);
+    const Name wname("*.uwild.example.com.");
+    ZoneFinder::FindNSEC3Result nsec3(false, 0, textToRRset(nsec3_apex_txt),
+                                      ConstRRsetPtr());
+    mock_finder->setNSEC3Result(&nsec3, &wname);
+    mock_finder->addRecord(nsec3_wild_txt);
+    mock_finder->addRecord(nsec3_uwild_txt);
+
+    EXPECT_THROW(Query(memory_client, Name("www1.uwild.example.com"),
+                       RRType::TXT(), response, true).process(),
+                 Query::BadNSEC3);
+}
+
 TEST_F(QueryTest, wildcardEmptyWithNSEC) {
-    // WILDCARD_EMPTY with DNSSEC proof.  We should have SOA, NSEC that proves the
-    // NXDOMAIN and their RRSIGs. In this case we need two NSEC RRs,
+    // 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(memory_client, Name("a.t.example.com"), RRType::A(), response,
           true).process();
@@ -1656,35 +1797,60 @@ TEST_F(QueryTest, findNSEC3) {
         Name("example.com").getLabelCount();
 
     // Apex name.  It should have a matching NSEC3
-    nsec3Check(true, expected_closest_labels, nsec3_apex_txt,
-               mock_finder->findNSEC3(Name("example.com"), false));
+    {
+        SCOPED_TRACE("apex, non recursive");
+        nsec3Check(true, expected_closest_labels, nsec3_apex_txt,
+                   mock_finder->findNSEC3(Name("example.com"), false));
+    }
 
     // Recursive mode doesn't change the result in this case.
-    nsec3Check(true, expected_closest_labels, nsec3_apex_txt,
-               mock_finder->findNSEC3(Name("example.com"), true)); 
+    {
+        SCOPED_TRACE("apex, recursive");
+        nsec3Check(true, expected_closest_labels, nsec3_apex_txt,
+                   mock_finder->findNSEC3(Name("example.com"), true));
+    }
 
     // Non existent name.  Disabling recursion, a covering NSEC3 should be
     // returned.
-    nsec3Check(false, 4, unsigned_delegation_nsec3_txt,
-               mock_finder->findNSEC3(Name("nxdomain.example.com"), false));
+    {
+        SCOPED_TRACE("nxdomain, non recursive");
+        nsec3Check(false, 4, nsec3_www_txt,
+                   mock_finder->findNSEC3(Name("nxdomain.example.com"),
+                                          false));
+    }
 
     // Non existent name.  The closest provable encloser is the apex,
     // and next closer is the query name.
-    nsec3Check(true, expected_closest_labels,
-               string(nsec3_apex_txt) + string(unsigned_delegation_nsec3_txt),
-               mock_finder->findNSEC3(Name("nxdomain.example.com"), true));
+    {
+        SCOPED_TRACE("nxdomain, recursive");
+        nsec3Check(true, expected_closest_labels,
+                   string(nsec3_apex_txt) + string(nsec3_www_txt),
+                   mock_finder->findNSEC3(Name("nxdomain.example.com"), true));
+    }
 
     // Similar to the previous case, but next closer name is different
     // (is the parent) of the non existent name.
-    nsec3Check(true, expected_closest_labels,
-               string(nsec3_apex_txt) + string(unsigned_delegation_nsec3_txt),
-               mock_finder->findNSEC3(Name("nx.domain.example.com"), true));
+    {
+        SCOPED_TRACE("nxdomain, next closer != qname");
+        nsec3Check(true, expected_closest_labels,
+                   string(nsec3_apex_txt) + string(nsec3_www_txt),
+                   mock_finder->findNSEC3(Name("nx.domain.example.com"),
+                                          true));
+    }
 
     // In the rest of test we check hash comparison for wrap around cases.
-    nsec3Check(false, 4, nsec3_apex_txt,
-               mock_finder->findNSEC3(Name("nxdomain2.example.com"), false));
-    nsec3Check(false, 4, unsigned_delegation_nsec3_txt,
-               mock_finder->findNSEC3(Name("nxdomain3.example.com"), false));
+    {
+        SCOPED_TRACE("largest");
+        nsec3Check(false, 4, nsec3_apex_txt,
+                   mock_finder->findNSEC3(Name("nxdomain2.example.com"),
+                                          false));
+    }
+    {
+        SCOPED_TRACE("smallest");
+        nsec3Check(false, 4, nsec3_www_txt,
+                   mock_finder->findNSEC3(Name("nxdomain3.example.com"),
+                                          false));
+    }
 }
 
 // This tests that the DS is returned above the delegation point as
@@ -1936,6 +2102,7 @@ TEST_F(QueryTest, nxrrsetMissingNSEC3) {
 }
 
 TEST_F(QueryTest, nxrrsetWithNSEC3_ds_exact) {
+    mock_finder->addRecord(unsigned_delegation_nsec3_txt);
     mock_finder->setNSEC3Flag(true);
 
     // This delegation has no DS, but does have a matching NSEC3 record
@@ -1954,6 +2121,7 @@ TEST_F(QueryTest, nxrrsetWithNSEC3_ds_exact) {
 }
 
 TEST_F(QueryTest, nxrrsetWithNSEC3_ds_no_exact) {
+    mock_finder->addRecord(unsigned_delegation_nsec3_txt);
     mock_finder->setNSEC3Flag(true);
 
     // This delegation has no DS, and no directly matching NSEC3 record
@@ -2000,24 +2168,4 @@ TEST_F(QueryTest, emptyNameWithNSEC3) {
     EXPECT_TRUE(result.isNSEC3Signed());
     EXPECT_FALSE(result.isWildcard());
 }
-
-TEST_F(QueryTest, wildcardNxrrsetWithNSEC3) {
-    mock_finder->setNSEC3Flag(true);
-    ZoneFinder::FindResult result = mock_finder->find(
-        Name("www1.uwild.example.com"), RRType::TXT(),
-        ZoneFinder::FIND_DNSSEC);
-    EXPECT_EQ(ZoneFinder::NXRRSET, result.code);
-    EXPECT_FALSE(result.rrset);
-    EXPECT_TRUE(result.isNSEC3Signed());
-    EXPECT_TRUE(result.isWildcard());
-}
-
-TEST_F(QueryTest, wildcardEmptyWithNSEC3) {
-    mock_finder->setNSEC3Flag(true);
-    ZoneFinder::FindResult result = mock_finder->find(
-        Name("a.t.example.com"), RRType::A(), ZoneFinder::FIND_DNSSEC);
-    EXPECT_EQ(ZoneFinder::NXRRSET, result.code);
-    EXPECT_TRUE(result.isNSEC3Signed());
-    EXPECT_TRUE(result.isWildcard());
-}
 }

src/lib/dns/message.cc

@@ -489,6 +489,10 @@ Message::getRRCount(const Section section) const {
 
 void
 Message::addRRset(const Section section, RRsetPtr rrset, const bool sign) {
+    if (!rrset) {
+        isc_throw(InvalidParameter,
+                  "NULL RRset is given to Message::addRRset");
+    }
     if (impl_->mode_ != Message::RENDER) {
         isc_throw(InvalidMessageOperation,
                   "addRRset performed in non-render mode");

src/lib/dns/message.h

@@ -462,15 +462,19 @@ public:
     /// This interface takes into account the RRSIG possibly attached to
     /// \c rrset.  This interface design needs to be revisited later.
     ///
-    /// This method is only allowed in the \c RENDER mode;
-    /// if the \c Message is in other mode, an exception of class
-    /// InvalidMessageOperation will be thrown.
-    /// \c section must be a valid constant of the \c Section type;
-    /// otherwise, an exception of class \c OutOfRange will be thrown.
-    ///
     /// 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).
+    ///
+    /// \throw InvalidParameter rrset is NULL
+    /// \throw InvalidMessageOperation The message is not in the \c RENDER
+    /// mode.
+    /// \throw OutOfRange \c section doesn't specify a valid \c Section value.
+    ///
+    /// \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);
 
     /// \brief Determine whether the given section already has an RRset

src/lib/dns/tests/message_unittest.cc

@@ -324,6 +324,10 @@ TEST_F(MessageTest, badAddRRset) {
                                         rrset_a), InvalidMessageOperation);
     // out-of-band section ID
     EXPECT_THROW(message_render.addRRset(bogus_section, rrset_a), OutOfRange);
+
+    // NULL RRset
+    EXPECT_THROW(message_render.addRRset(Message::SECTION_ANSWER, RRsetPtr()),
+                 InvalidParameter);
 }
 
 TEST_F(MessageTest, hasRRset) {