Merge branch 'master' into trac1593

ChangeLog

@@ -1,3 +1,45 @@
+382.	[func]	jelte
+	b10-auth now also experimentally supports statistics counters of
+	the rcode reponses it sends. The counters can be shown as
+	rcode.<code name>, where code name is the lowercase textual
+	representation of the rcode (e.g. "noerror", "formerr", etc.).
+	Same note applies as for opcodes, see changelog entry 364.
+	(Trac #1613, git e98da500d7b02e11347431a74f2efce5a7d622aa)
+
+381.	[bug]		jinmei
+	b10-auth: honor the DNSSEC DO bit in the new query handler.
+	(Trac #1695, git 61f4da5053c6a79fbc162fb16f195cdf8f94df64)
+
+380.	[bug]		jinmei
+	libdns++: miscellaneous bug fixes for the NSECPARAM RDATA
+	implementation, including incorrect handling for empty salt and
+	incorrect comparison logic.
+	(Trac #1638, git 966c129cc3c538841421f1e554167d33ef9bdf25)
+
+379.	[bug]		jelte
+	Configuration commands in bindctl now check for list indices if
+	the 'identifier' argument points to a child element of a list
+	item. Previously, it was possible to 'get' non-existent values
+	by leaving out the index, e.g. "config show Auth/listen_on/port,
+	which should be config show Auth/listen_on[<index>]/port, since
+	Auth/listen_on is a list. The command without an index will now
+	show an error. It is still possible to show/set the entire list
+	("config show Auth/listen_on").
+	(Trac #1649, git 003ca8597c8d0eb558b1819dbee203fda346ba77)
+
+378.	[func]		vorner
+	It is possible to start authoritative server or resolver in multiple
+	instances, to use more than one core. Configuration is described in
+	the guide.
+	(Trac #1596, git 17f7af0d8a42a0a67a2aade5bc269533efeb840a)
+
+377.	[bug]		jinmei
+	libdns++: miscellaneous bug fixes for the NSEC and NSEC3 RDATA
+	implementation, including a crash in NSEC3::toText() for some RR
+	types, incorrect handling of empty NSEC3 salt, and incorrect
+	comparison logic in NSEC3::compare().
+	(Trac #1641, git 28ba8bd71ae4d100cb250fd8d99d80a17a6323a2)
+
 376.	[bug]		jinmei, vorner
 	The new query handling module of b10-auth did not handle type DS
 	query correctly: It didn't look for it in the parent zone, and

configure.ac

@@ -907,7 +907,7 @@ AC_PATH_PROGS(AWK, gawk awk)
 AC_SUBST(AWK)
 
 AC_ARG_ENABLE(man, [AC_HELP_STRING([--enable-man],
-  [regenerate man pages [default=no]])], enable_man=yes, enable_man=no)
+  [regenerate man pages [default=no]])], enable_man=$enableval, enable_man=no)
 
 AM_CONDITIONAL(ENABLE_MAN, test x$enable_man != xno)
 

doc/guide/bind10-guide.xml

@@ -913,7 +913,24 @@ address, but the usual ones don't." mean? -->
           In short, you should think twice before disabling something here.
         </para>
       </note>
-
+      <para>
+        It is possible to start some components multiple times (currently
+        <command>b10-auth</command> and <command>b10-resolzer</command>).
+        You might want to do that to gain more performance (each one uses only
+        single core). Just put multiple entries under different names, like
+        this, with the same config:
+        <screen>&gt; <userinput>config add Boss/components b10-resolver-2</userinput>
+&gt; <userinput>config set Boss/components/b10-resolver-2/special resolver</userinput>
+&gt; <userinput>config set Boss/components/b10-resolver-2/kind needed</userinput>
+&gt; <userinput>config commit</userinput></screen>
+      </para>
+      <para>
+        However, this is work in progress and the support is not yet complete.
+        For example, each resolver will have its own cache, each authoritative
+        server will keep its own copy of in-memory data and there could be
+        problems with locking the sqlite database, if used. The configuration
+        might be changed to something more convenient in future.
+      </para>
     </section>
 
   </chapter>

src/bin/auth/auth.spec.pre.in

@@ -97,7 +97,13 @@
       {
         "command_name": "shutdown",
         "command_description": "Shut down authoritative DNS server",
-        "command_args": []
+        "command_args": [
+          {
+            "item_name": "pid",
+            "item_type": "integer",
+            "item_optional": true
+          }
+        ]
       },
       {
         "command_name": "sendstats",
@@ -210,7 +216,7 @@
         "item_optional": true,
         "item_default": 0,
         "item_title": "Received requests opcode 8",
-        "item_description": "The number of total request counts whose opcode is8 (reserved)"
+        "item_description": "The number of total request counts whose opcode is 8 (reserved)"
       },
       {
         "item_name": "opcode.reserved9",
@@ -218,7 +224,7 @@
         "item_optional": true,
         "item_default": 0,
         "item_title": "Received requests opcode 9",
-        "item_description": "The number of total request counts whose opcode is9 (reserved)"
+        "item_description": "The number of total request counts whose opcode is 9 (reserved)"
       },
       {
         "item_name": "opcode.reserved10",
@@ -226,7 +232,7 @@
         "item_optional": true,
         "item_default": 0,
         "item_title": "Received requests opcode 10",
-        "item_description": "The number of total request counts whose opcode is10 (reserved)"
+        "item_description": "The number of total request counts whose opcode is 10 (reserved)"
       },
       {
         "item_name": "opcode.reserved11",
@@ -234,7 +240,7 @@
         "item_optional": true,
         "item_default": 0,
         "item_title": "Received requests opcode 11",
-        "item_description": "The number of total request counts whose opcode is11 (reserved)"
+        "item_description": "The number of total request counts whose opcode is 11 (reserved)"
       },
       {
         "item_name": "opcode.reserved12",
@@ -242,7 +248,7 @@
         "item_optional": true,
         "item_default": 0,
         "item_title": "Received requests opcode 12",
-        "item_description": "The number of total request counts whose opcode is12 (reserved)"
+        "item_description": "The number of total request counts whose opcode is 12 (reserved)"
       },
       {
         "item_name": "opcode.reserved13",
@@ -250,7 +256,7 @@
         "item_optional": true,
         "item_default": 0,
         "item_title": "Received requests opcode 13",
-        "item_description": "The number of total request counts whose opcode is13 (reserved)"
+        "item_description": "The number of total request counts whose opcode is 13 (reserved)"
       },
       {
         "item_name": "opcode.reserved14",
@@ -258,7 +264,7 @@
         "item_optional": true,
         "item_default": 0,
         "item_title": "Received requests opcode 14",
-        "item_description": "The number of total request counts whose opcode is14 (reserved)"
+        "item_description": "The number of total request counts whose opcode is 14 (reserved)"
       },
       {
         "item_name": "opcode.reserved15",
@@ -266,7 +272,143 @@
         "item_optional": true,
         "item_default": 0,
         "item_title": "Received requests opcode 15",
-        "item_description": "The number of total request counts whose opcode is15 (reserved)"
+        "item_description": "The number of total request counts whose opcode is 15 (reserved)"
+      },
+      {
+        "item_name": "rcode.noerror",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Sent success response",
+        "item_description": "The number of total responses with rcode 0 (NOERROR)"
+      },
+      {
+        "item_name": "rcode.formerr",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Sent 'format error' response",
+        "item_description": "The number of total responses with rcode 1 (FORMERR)"
+      },
+      {
+        "item_name": "rcode.servfail",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Sent 'server failure' response",
+        "item_description": "The number of total responses with rcode 2 (SERVFAIL)"
+      },
+      {
+        "item_name": "rcode.nxdomain",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Sent 'name error' response",
+        "item_description": "The number of total responses with rcode 3 (NXDOMAIN)"
+      },
+      {
+        "item_name": "rcode.notimp",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Sent 'not implemented' response",
+        "item_description": "The number of total responses with rcode 4 (NOTIMP)"
+      },
+      {
+        "item_name": "rcode.refused",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Sent 'refused' response",
+        "item_description": "The number of total responses with rcode 5 (REFUSED)"
+      },
+      {
+        "item_name": "rcode.yxdomain",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Sent 'name unexpectedly exists' response",
+        "item_description": "The number of total responses with rcode 6 (YXDOMAIN)"
+      },
+      {
+        "item_name": "rcode.yxrrset",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Sent 'rrset unexpectedly exists' response",
+        "item_description": "The number of total responses with rcode 7 (YXRRSET)"
+      },
+      {
+        "item_name": "rcode.nxrrset",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Sent 'no such rrset' response",
+        "item_description": "The number of total responses with rcode 8 (NXRRSET)"
+      },
+      {
+        "item_name": "rcode.notauth",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Sent 'not authoritative' response",
+        "item_description": "The number of total responses with rcode 9 (NOTAUTH)"
+      },
+      {
+        "item_name": "rcode.notzone",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Sent 'name not in zone' response",
+        "item_description": "The number of total responses with rcode 10 (NOTZONE)"
+      },
+      {
+        "item_name": "rcode.reserved11",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Sent response with rcode 11",
+        "item_description": "The number of total responses with rcode 11 (reserved)"
+      },
+      {
+        "item_name": "rcode.reserved12",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Sent response with rcode 12",
+        "item_description": "The number of total responses with rcode 12 (reserved)"
+      },
+      {
+        "item_name": "rcode.reserved13",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Sent response with rcode 13",
+        "item_description": "The number of total responses with rcode 13 (reserved)"
+      },
+      {
+        "item_name": "rcode.reserved14",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Sent response with rcode 14",
+        "item_description": "The number of total responses with rcode 14 (reserved)"
+      },
+      {
+        "item_name": "rcode.reserved15",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Sent response with rcode 15",
+        "item_description": "The number of total responses with rcode 15 (reserved)"
+      },
+      {
+        "item_name": "rcode.badvers",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Sent 'EDNS version not implemented' response",
+        "item_description": "The number of total responses with rcode 16 (BADVERS)"
       }
     ]
   }

src/bin/auth/auth_log.h

@@ -29,6 +29,8 @@ namespace auth {
 
 // Debug messages indicating normal startup are logged at this debug level.
 const int DBG_AUTH_START = DBGLVL_START_SHUT;
+// Debug messages upon shutdown
+const int DBG_AUTH_SHUT = DBGLVL_START_SHUT;
 
 // Debug level used to log setting information (such as configuration changes).
 const int DBG_AUTH_OPS = DBGLVL_COMMAND;

src/bin/auth/auth_messages.mes

@@ -192,6 +192,10 @@ reason for the failure is included in the message.
 Initialization of the authoritative server has completed successfully
 and it is entering the main loop, waiting for queries to arrive.
 
+% AUTH_SHUTDOWN asked to stop, doing so
+This is a debug message indicating the server was asked to shut down and it is
+complying to the request.
+
 % AUTH_SQLITE3 nothing to do for loading sqlite3
 This is a debug message indicating that the authoritative server has
 found that the data source it is loading is an SQLite3 data source,

src/bin/auth/auth_srv.cc

@@ -128,6 +128,22 @@ public:
     /// Bind the ModuleSpec object in config_session_ with
     /// isc:config::ModuleSpec::validateStatistics.
     void registerStatisticsValidator();
+
+    /// \brief Resume the server
+    ///
+    /// This is a wrapper call for DNSServer::resume(done), if 'done' is true,
+    /// the Rcode set in the given Message is counted in the statistics
+    /// counter.
+    ///
+    /// This method is expected to be called by processMessage()
+    ///
+    /// \param server The DNSServer as passed to processMessage()
+    /// \param message The response as constructed by processMessage()
+    /// \param done If true, the Rcode from the given message is counted,
+    ///             this value is then passed to server->resume(bool)
+    void resumeServer(isc::asiodns::DNSServer* server,
+                      isc::dns::MessagePtr message,
+                      bool done);
 private:
     std::string db_file_;
 
@@ -409,13 +425,13 @@ AuthSrv::processMessage(const IOMessage& io_message, MessagePtr message,
         // Ignore all responses.
         if (message->getHeaderFlag(Message::HEADERFLAG_QR)) {
             LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_RESPONSE_RECEIVED);
-            server->resume(false);
+            impl_->resumeServer(server, message, false);
             return;
         }
     } catch (const Exception& ex) {
         LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_HEADER_PARSE_FAIL)
                   .arg(ex.what());
-        server->resume(false);
+        impl_->resumeServer(server, message, false);
         return;
     }
 
@@ -426,13 +442,13 @@ AuthSrv::processMessage(const IOMessage& io_message, MessagePtr message,
         LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_PACKET_PROTOCOL_ERROR)
                   .arg(error.getRcode().toText()).arg(error.what());
         makeErrorMessage(message, buffer, error.getRcode());
-        server->resume(true);
+        impl_->resumeServer(server, message, true);
         return;
     } catch (const Exception& ex) {
         LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_PACKET_PARSE_ERROR)
                   .arg(ex.what());
         makeErrorMessage(message, buffer, Rcode::SERVFAIL());
-        server->resume(true);
+        impl_->resumeServer(server, message, true);
         return;
     } // other exceptions will be handled at a higher layer.
 
@@ -459,7 +475,7 @@ AuthSrv::processMessage(const IOMessage& io_message, MessagePtr message,
 
     if (tsig_error != TSIGError::NOERROR()) {
         makeErrorMessage(message, buffer, tsig_error.toRcode(), tsig_context);
-        server->resume(true);
+        impl_->resumeServer(server, message, true);
         return;
     }
 
@@ -492,7 +508,7 @@ AuthSrv::processMessage(const IOMessage& io_message, MessagePtr message,
         }
     }
 
-    server->resume(send_answer);
+    impl_->resumeServer(server, message, send_answer);
 }
 
 bool
@@ -526,7 +542,8 @@ AuthSrvImpl::processNormalQuery(const IOMessage& io_message, MessagePtr message,
         if (memory_client_ && memory_client_class_ == question->getClass()) {
             const RRType& qtype = question->getType();
             const Name& qname = question->getName();
-            auth::Query(*memory_client_, qname, qtype, *message).process();
+            auth::Query(*memory_client_, qname, qtype, *message,
+                        dnssec_ok).process();
         } else {
             datasrc::Query query(*message, cache_, dnssec_ok);
             data_sources_.doQuery(query);
@@ -754,6 +771,14 @@ AuthSrvImpl::setDbFile(ConstElementPtr config) {
     return (answer);
 }
 
+void
+AuthSrvImpl::resumeServer(DNSServer* server, MessagePtr message, bool done) {
+    if (done) {
+        counters_.inc(message->getRcode());
+    }
+    server->resume(done);
+}
+
 ConstElementPtr
 AuthSrv::updateConfig(ConstElementPtr new_config) {
     try {
@@ -783,6 +808,11 @@ AuthSrv::getCounter(const Opcode opcode) const {
     return (impl_->counters_.getCounter(opcode));
 }
 
+uint64_t
+AuthSrv::getCounter(const Rcode rcode) const {
+    return (impl_->counters_.getCounter(rcode));
+}
+
 const AddressList&
 AuthSrv::getListenAddresses() const {
     return (impl_->listen_addresses_);

src/bin/auth/auth_srv.h

@@ -344,6 +344,7 @@ public:
     /// \param type Type of a counter to get the value of
     ///
     /// \return the value of the counter.
+
     uint64_t getCounter(const AuthCounters::ServerCounterType type) const;
 
     /// \brief Get the value of per Opcode counter in the Auth Counters.
@@ -360,6 +361,20 @@ public:
     /// \return the value of the counter.
     uint64_t getCounter(const isc::dns::Opcode opcode) const;
 
+    /// \brief Get the value of per Rcode counter in the Auth Counters.
+    ///
+    /// This function calls AuthCounters::getCounter(isc::dns::Rcode) and
+    /// returns its return value.
+    ///
+    /// \note This is a tentative interface as an attempt of experimentally
+    /// supporting more statistics counters.  This should eventually be more
+    /// generalized.  In any case, this method is mainly for testing.
+    ///
+    /// \throw None
+    /// \param rcode The rcode of the counter to get the value of
+    /// \return the value of the counter.
+    uint64_t getCounter(const isc::dns::Rcode rcode) const;
+
     /**
      * \brief Set and get the addresses we listen on.
      */

src/bin/auth/command.cc

@@ -12,24 +12,23 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <string>
-
-#include <boost/scoped_ptr.hpp>
-#include <boost/shared_ptr.hpp>
+#include <auth/command.h>
+#include <auth/auth_log.h>
+#include <auth/auth_srv.h>
 
+#include <cc/data.h>
+#include <datasrc/memory_datasrc.h>
+#include <config/ccsession.h>
 #include <exceptions/exceptions.h>
-
 #include <dns/rrclass.h>
 
-#include <cc/data.h>
-
-#include <datasrc/memory_datasrc.h>
+#include <string>
 
-#include <config/ccsession.h>
+#include <boost/scoped_ptr.hpp>
+#include <boost/shared_ptr.hpp>
 
-#include <auth/auth_log.h>
-#include <auth/auth_srv.h>
-#include <auth/command.h>
+#include <sys/types.h>
+#include <unistd.h>
 
 using boost::scoped_ptr;
 using namespace isc::auth;
@@ -104,10 +103,30 @@ public:
     virtual void exec(AuthSrv& server, isc::data::ConstElementPtr args) = 0;
 };
 
-// Handle the "shutdown" command.  No argument is assumed.
+// Handle the "shutdown" command. An optional parameter "pid" is used to
+// see if it is really for our instance.
 class ShutdownCommand : public AuthCommand {
 public:
-    virtual void exec(AuthSrv& server, isc::data::ConstElementPtr) {
+    virtual void exec(AuthSrv& server, isc::data::ConstElementPtr args) {
+        // Is the pid argument provided?
+        if (args && args->contains("pid")) {
+            // If it is, we check it is the same as our PID
+
+            // This might throw in case the type is not an int, but that's
+            // OK, as it'll get converted to an error on higher level.
+            const int pid(args->get("pid")->intValue());
+            const pid_t my_pid(getpid());
+            if (my_pid != pid) {
+                // It is not for us
+                //
+                // Note that this is completely expected situation, if
+                // there are multiple instances of the server running and
+                // another instance is being shut down, we get the message
+                // too, due to the multicast nature of our message bus.
+                return;
+            }
+        }
+        LOG_DEBUG(auth_logger, DBG_AUTH_SHUT, AUTH_SHUTDOWN);
         server.stop();
     }
 };

src/bin/auth/query.cc

@@ -117,7 +117,7 @@ Query::addSOA(ZoneFinder& finder) {
 // either an SERVFAIL response or just ignoring the query.  We at least prevent
 // a complete crash due to such broken behavior.
 void
-Query::addNXDOMAINProof(ZoneFinder& finder, ConstRRsetPtr nsec) {
+Query::addNXDOMAINProofByNSEC(ZoneFinder& finder, ConstRRsetPtr nsec) {
     if (nsec->getRdataCount() == 0) {
         isc_throw(BadNSEC, "NSEC for NXDOMAIN is empty");
     }
@@ -168,23 +168,80 @@ Query::addNXDOMAINProof(ZoneFinder& finder, ConstRRsetPtr nsec) {
 }
 
 void
-Query::addWildcardProof(ZoneFinder& finder) {
-    // The query name shouldn't exist in the zone if there were no wildcard
-    // substitution.  Confirm that by specifying NO_WILDCARD.  It should result
-    // in NXDOMAIN and an NSEC RR that proves it should be returned.
-    const ZoneFinder::FindResult fresult =
-        finder.find(qname_, RRType::NSEC(),
-                    dnssec_opt_ | ZoneFinder::NO_WILDCARD);
-    if (fresult.code != ZoneFinder::NXDOMAIN || !fresult.rrset ||
-        fresult.rrset->getRdataCount() == 0) {
-        isc_throw(BadNSEC, "Unexpected result for wildcard proof");
+Query::addNXDOMAINProofByNSEC3(ZoneFinder& finder) {
+    // Firstly get the NSEC3 proves for Closest Encloser Proof
+    // See section 7.2.1 of RFC 5155.
+    // Since this is a Name Error case both closest and next proofs should
+    // be available (see addNXRRsetProof).
+    const ZoneFinder::FindNSEC3Result fresult1 = finder.findNSEC3(qname_,
+                                                                  true);
+    response_.addRRset(Message::SECTION_AUTHORITY,
+                       boost::const_pointer_cast<AbstractRRset>(
+                           fresult1.closest_proof),
+                       dnssec_);
+    response_.addRRset(Message::SECTION_AUTHORITY,
+                       boost::const_pointer_cast<AbstractRRset>(
+                       fresult1.next_proof),
+                       dnssec_);
+
+    // Next, construct the wildcard name at the closest encloser, i.e.,
+    // '*' followed by the closest encloser, and add NSEC3 for it.
+    const Name wildname(Name("*").concatenate(
+               qname_.split(qname_.getLabelCount() -
+                            fresult1.closest_labels)));
+    const ZoneFinder::FindNSEC3Result fresult2 =
+        finder.findNSEC3(wildname, false);
+    if (fresult2.matched) {
+        isc_throw(BadNSEC3, "Matching NSEC3 found for nonexistent domain "
+                  << wildname);
     }
     response_.addRRset(Message::SECTION_AUTHORITY,
-                       boost::const_pointer_cast<AbstractRRset>(fresult.rrset),
+                       boost::const_pointer_cast<AbstractRRset>(
+                           fresult2.closest_proof),
                        dnssec_);
 }
 
 void
+Query::addWildcardProof(ZoneFinder& finder,
+                        const ZoneFinder::FindResult& db_result)
+{
+    if (db_result.isNSECSigned()) {
+        // Case for RFC4035 Section 3.1.3.3.
+        //
+        // The query name shouldn't exist in the zone if there were no wildcard
+        // substitution.  Confirm that by specifying NO_WILDCARD.  It should
+        // result in NXDOMAIN and an NSEC RR that proves it should be returned.
+        const ZoneFinder::FindResult fresult =
+            finder.find(qname_, RRType::NSEC(),
+                        dnssec_opt_ | ZoneFinder::NO_WILDCARD);
+        if (fresult.code != ZoneFinder::NXDOMAIN || !fresult.rrset ||
+            fresult.rrset->getRdataCount() == 0) {
+            isc_throw(BadNSEC,
+                      "Unexpected NSEC result for wildcard proof");
+        }
+        response_.addRRset(Message::SECTION_AUTHORITY,
+                           boost::const_pointer_cast<AbstractRRset>(
+                               fresult.rrset),
+                           dnssec_);
+    } else if (db_result.isNSEC3Signed()) {
+        // Case for RFC5155 Section 7.2.6.
+        //
+        // Note that the closest encloser must be the immediate ancestor
+        // of the matching wildcard, so NSEC3 for its next closer is what
+        // we are expected to provided per the RFC (if this assumption isn't
+        // met the zone is broken anyway).
+        const ZoneFinder::FindNSEC3Result NSEC3Result(
+            finder.findNSEC3(qname_, true));
+        // Note that at this point next_proof must not be NULL unless it's
+        // a run time collision (or zone/findNSEC3() is broken).  The
+        // unexpected case will be caught in addRRset() and result in SERVFAIL.
+        response_.addRRset(Message::SECTION_AUTHORITY,
+                           boost::const_pointer_cast<AbstractRRset>(
+                               NSEC3Result.next_proof), dnssec_);
+    }
+}
+
+void
 Query::addWildcardNXRRSETProof(ZoneFinder& finder, ConstRRsetPtr nsec) {
     // There should be one NSEC RR which was found in the zone to prove
     // that there is not matched <QNAME,QTYPE> via wildcard expansion.
@@ -214,10 +271,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,6 +314,58 @@ Query::addNXRRsetProof(ZoneFinder& finder,
         if (db_result.isWildcard()) {
             addWildcardNXRRSETProof(finder, db_result.rrset);
         }
+    } 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),
+        // otherwise, do non-recursive search
+        const bool qtype_ds = (qtype_ == RRType::DS());
+        ZoneFinder::FindNSEC3Result result(finder.findNSEC3(qname_, qtype_ds));
+        if (result.matched) {
+            response_.addRRset(Message::SECTION_AUTHORITY,
+                               boost::const_pointer_cast<AbstractRRset>(
+                                   result.closest_proof), dnssec_);
+            // For qtype == DS, next_proof could be set
+            // (We could check for opt-out here, but that's really the
+            // responsibility of the datasource)
+            if (qtype_ds && result.next_proof != ConstRRsetPtr()) {
+                response_.addRRset(Message::SECTION_AUTHORITY,
+                                   boost::const_pointer_cast<AbstractRRset>(
+                                       result.next_proof), dnssec_);
+            }
+        } else {
+            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);
+        }
     }
 }
 
@@ -375,7 +505,7 @@ Query::process() {
             // If the answer is a result of wildcard substitution,
             // add a proof that there's no closer name.
             if (dnssec_ && db_result.isWildcard()) {
-                addWildcardProof(*result.zone_finder);
+                addWildcardProof(*result.zone_finder,db_result);
             }
             break;
         case ZoneFinder::SUCCESS:
@@ -409,7 +539,7 @@ Query::process() {
             // If the answer is a result of wildcard substitution,
             // add a proof that there's no closer name.
             if (dnssec_ && db_result.isWildcard()) {
-                addWildcardProof(*result.zone_finder);
+                addWildcardProof(*result.zone_finder,db_result);
             }
             break;
         case ZoneFinder::DELEGATION:
@@ -435,8 +565,12 @@ Query::process() {
         case ZoneFinder::NXDOMAIN:
             response_.setRcode(Rcode::NXDOMAIN());
             addSOA(*result.zone_finder);
-            if (dnssec_ && db_result.rrset) {
-                addNXDOMAINProof(zfinder, db_result.rrset);
+            if (dnssec_) {
+                if (db_result.isNSECSigned() && db_result.rrset) {
+                    addNXDOMAINProofByNSEC(zfinder, db_result.rrset);
+                } else if (db_result.isNSEC3Signed()) {
+                    addNXDOMAINProofByNSEC3(zfinder);
+                }
             }
             break;
         case ZoneFinder::NXRRSET:

src/bin/auth/query.h

@@ -86,10 +86,11 @@ private:
     void addDS(isc::datasrc::ZoneFinder& finder,
                const isc::dns::Name& ds_name);
 
-    /// \brief Adds NSEC denial proof for the given NXRRset result
+    /// \brief Adds NSEC(3) denial proof for the given NXRRset result
     ///
-    /// NSEC records, if available (signaled by isNSECSigned(), are added
-    /// to the authority section.
+    /// If available, NSEC or NSEC3 records are added to the authority
+    /// section (depending on whether isNSECSigned() or isNSEC3Signed()
+    /// returns true).
     ///
     /// \param finder The ZoneFinder that was used to search for the missing
     ///               data
@@ -100,13 +101,21 @@ private:
     /// Add NSEC RRs that prove an NXDOMAIN result.
     ///
     /// This corresponds to Section 3.1.3.2 of RFC 4035.
-    void addNXDOMAINProof(isc::datasrc::ZoneFinder& finder,
-                          isc::dns::ConstRRsetPtr nsec);
+    void addNXDOMAINProofByNSEC(isc::datasrc::ZoneFinder& finder,
+                                isc::dns::ConstRRsetPtr nsec);
 
-    /// Add NSEC RRs that prove a wildcard answer is the best one.
+    /// Add NSEC3 RRs that prove an NXDOMAIN result.
     ///
-    /// This corresponds to Section 3.1.3.3 of RFC 4035.
-    void addWildcardProof(isc::datasrc::ZoneFinder& finder);
+    /// This corresponds to Section 7.2.2 of RFC 5155.
+    void addNXDOMAINProofByNSEC3(isc::datasrc::ZoneFinder& finder);
+
+    /// Add NSEC or NSEC3 RRs that prove a wildcard answer is the best one.
+    ///
+    /// This corresponds to Section 3.1.3.3 of RFC 4035 and Section 7.2.6
+    /// of RFC5155.
+    void addWildcardProof(
+        isc::datasrc::ZoneFinder& finder,
+        const isc::datasrc::ZoneFinder::FindResult& dbResult);
 
     /// \brief Adds one NSEC RR proved no matched QNAME,one NSEC RR proved no
     /// matched <QNAME,QTYPE> through wildcard extension.
@@ -119,7 +128,7 @@ private:
     /// <QNAME,QTTYPE>.
     void addWildcardNXRRSETProof(isc::datasrc::ZoneFinder& finder,
                                  isc::dns::ConstRRsetPtr nsec);
-    
+
     /// \brief Look up additional data (i.e., address records for the names
     /// included in NS or MX records) and add them to the additional section.
     ///
@@ -287,6 +296,17 @@ public:
         {}
     };
 
+    /// An invalid result is given when a valid NSEC3 is expected
+    ///
+    /// This can only happen when the underlying data source implementation or
+    /// the zone is broken.  By throwing an exception we treat such cases
+    /// as SERVFAIL.
+    struct BadNSEC3 : public BadZone {
+        BadNSEC3(const char* file, size_t line, const char* what) :
+            BadZone(file, line, what)
+        {}
+    };
+
     /// An invalid result is given when a valid DS records (or NXRRSET) is
     /// expected
     ///

src/bin/auth/statistics.cc

@@ -50,6 +50,9 @@ public:
     void inc(const Opcode opcode) {
         opcode_counter_.inc(opcode.getCode());
     }
+    void inc(const Rcode rcode) {
+        rcode_counter_.inc(rcode.getCode());
+    }
     void inc(const std::string& zone,
              const AuthCounters::PerZoneCounterType type);
     bool submitStatistics() const;
@@ -61,10 +64,15 @@ public:
     uint64_t getCounter(const Opcode opcode) const {
         return (opcode_counter_.get(opcode.getCode()));
     }
+    uint64_t getCounter(const Rcode rcode) const {
+        return (rcode_counter_.get(rcode.getCode()));
+    }
 private:
     Counter server_counter_;
     Counter opcode_counter_;
     static const size_t NUM_OPCODES = 16;
+    Counter rcode_counter_;
+    static const size_t NUM_RCODES = 17;
     CounterDictionary per_zone_counter_;
     isc::cc::AbstractSession* statistics_session_;
     AuthCounters::validator_type validator_;
@@ -75,7 +83,7 @@ AuthCountersImpl::AuthCountersImpl() :
     // size of server_counter_: AuthCounters::SERVER_COUNTER_TYPES
     // size of per_zone_counter_: AuthCounters::PER_ZONE_COUNTER_TYPES
     server_counter_(AuthCounters::SERVER_COUNTER_TYPES),
-    opcode_counter_(NUM_OPCODES),
+    opcode_counter_(NUM_OPCODES), rcode_counter_(NUM_RCODES),
     per_zone_counter_(AuthCounters::PER_ZONE_COUNTER_TYPES),
     statistics_session_(NULL)
 {
@@ -125,6 +133,19 @@ AuthCountersImpl::submitStatistics() const {
                               << counter;
         }
     }
+    // Insert non 0 Rcode counters.
+    for (int i = 0; i < NUM_RCODES; ++i) {
+        const Counter::Type counter = rcode_counter_.get(i);
+        if (counter != 0) {
+            // The counter item name should be derived lower-cased textual
+            // representation of the code.
+            std::string rcode_txt = Rcode(i).toText();
+            std::transform(rcode_txt.begin(), rcode_txt.end(),
+                           rcode_txt.begin(), ::tolower);
+            statistics_string << ", \"rcode." << rcode_txt << "\": "
+                              << counter;
+        }
+    }
     statistics_string <<   " }"
                       <<   "}"
                       << "]}";
@@ -194,6 +215,11 @@ AuthCounters::inc(const Opcode opcode) {
     impl_->inc(opcode);
 }
 
+void
+AuthCounters::inc(const Rcode rcode) {
+    impl_->inc(rcode);
+}
+
 bool
 AuthCounters::submitStatistics() const {
     return (impl_->submitStatistics());
@@ -216,6 +242,11 @@ AuthCounters::getCounter(const Opcode opcode) const {
     return (impl_->getCounter(opcode));
 }
 
+uint64_t
+AuthCounters::getCounter(const Rcode rcode) const {
+    return (impl_->getCounter(rcode));
+}
+
 void
 AuthCounters::registerStatisticsValidator
     (AuthCounters::validator_type validator) const

src/bin/auth/statistics.h

@@ -16,6 +16,7 @@
 #define __STATISTICS_H 1
 
 #include <dns/opcode.h>
+#include <dns/rcode.h>
 
 #include <cc/session.h>
 #include <stdint.h>
@@ -98,6 +99,15 @@ public:
     /// \throw None
     void inc(const isc::dns::Opcode opcode);
 
+    /// \brief Increment the counter of a per rcode counter.
+    ///
+    /// \note This is a tentative interface.  See \c getCounter().
+    ///
+    /// \param rcode The rcode of the counter to increment.
+    ///
+    /// \throw None
+    void inc(const isc::dns::Rcode rcode);
+
     /// \brief Submit statistics counters to statistics module.
     ///
     /// This method is desinged to be called periodically
@@ -162,6 +172,21 @@ public:
     /// \return the value of the counter.
     uint64_t getCounter(const isc::dns::Opcode opcode) const;
 
+    /// \brief Get the value of a per rcode counter.
+    ///
+    /// This method returns the value of the per rcode counter for the
+    /// specified \c rcode.
+    ///
+    /// \note As mentioned in getCounter(const isc::dns::Opcode opcode),
+    /// This is a tentative interface as an attempt of experimentally
+    /// supporting more statistics counters.  This should eventually be more
+    /// generalized.  In any case, this method is mainly for testing.
+    ///
+    /// \throw None
+    /// \param rcode The rcode of the counter to get the value of
+    /// \return the value of the counter.
+    uint64_t getCounter(const isc::dns::Rcode rcode) const;
+
     /// \brief A type of validation function for the specification in
     /// isc::config::ModuleSpec.
     ///

src/bin/auth/tests/auth_srv_unittest.cc

@@ -65,6 +65,13 @@ const char* const CONFIG_TESTDB =
 const char* const BADCONFIG_TESTDB =
     "{ \"database_file\": \"" TEST_DATA_DIR "/nodir/notexist\"}";
 
+// This is a configuration that uses the in-memory data source containing
+// a signed example zone.
+const char* const CONFIG_INMEMORY_EXAMPLE =
+    "{\"datasources\": [{\"type\": \"memory\","
+    "\"zones\": [{\"origin\": \"example\","
+    "\"file\": \"" TEST_DATA_DIR "/rfc5155-example.zone.signed\"}]}]}";
+
 class AuthSrvTest : public SrvTestBase {
 protected:
     AuthSrvTest() :
@@ -84,6 +91,35 @@ protected:
         server.processMessage(*io_message, parse_message, response_obuffer,
                               &dnsserv);
     }
+
+    // Helper for checking Rcode statistic counters;
+    // Checks for one specific Rcode statistics counter value
+    void checkRcodeCounter(const Rcode& rcode, int expected_value) const {
+        EXPECT_EQ(expected_value, server.getCounter(rcode)) <<
+                  "Expected Rcode count for " << rcode.toText() <<
+                  " " << expected_value << ", was: " <<
+                  server.getCounter(rcode);
+    }
+
+    // Checks whether all Rcode counters are set to zero
+    void checkAllRcodeCountersZero() const {
+        for (int i = 0; i < 17; i++) {
+            checkRcodeCounter(Rcode(i), 0);
+        }
+    }
+
+    // Checks whether all Rcode counters are set to zero except the given
+    // rcode (it is checked to be set to 'value')
+    void checkAllRcodeCountersZeroExcept(const Rcode& rcode, int value) const {
+        for (int i = 0; i < 17; i++) {
+            const Rcode rc(i);
+            if (rc == rcode) {
+                checkRcodeCounter(Rcode(i), value);
+            } else {
+                checkRcodeCounter(Rcode(i), 0);
+            }
+        }
+    }
     IOService ios_;
     DNSService dnss_;
     MockSession statistics_session;
@@ -145,6 +181,7 @@ TEST_F(AuthSrvTest, builtInQuery) {
                         response_obuffer->getData(),
                         response_obuffer->getLength(),
                         &response_data[0], response_data.size());
+    checkAllRcodeCountersZeroExcept(Rcode::NOERROR(), 1);
 }
 
 // Same test emulating the UDPServer class behavior (defined in libasiolink).
@@ -195,38 +232,46 @@ TEST_F(AuthSrvTest, iqueryViaDNSServer) {
 // Unsupported requests.  Should result in NOTIMP.
 TEST_F(AuthSrvTest, unsupportedRequest) {
     unsupportedRequest();
+    // unsupportedRequest tries 14 different opcodes
+    checkAllRcodeCountersZeroExcept(Rcode::NOTIMP(), 14);
 }
 
 // Multiple questions.  Should result in FORMERR.
 TEST_F(AuthSrvTest, multiQuestion) {
     multiQuestion();
+    checkAllRcodeCountersZeroExcept(Rcode::FORMERR(), 1);
 }
 
 // Incoming data doesn't even contain the complete header.  Must be silently
 // dropped.
 TEST_F(AuthSrvTest, shortMessage) {
     shortMessage();
+    checkAllRcodeCountersZero();
 }
 
 // Response messages.  Must be silently dropped, whether it's a valid response
 // or malformed or could otherwise cause a protocol error.
 TEST_F(AuthSrvTest, response) {
     response();
+    checkAllRcodeCountersZero();
 }
 
 // Query with a broken question
 TEST_F(AuthSrvTest, shortQuestion) {
     shortQuestion();
+    checkAllRcodeCountersZeroExcept(Rcode::FORMERR(), 1);
 }
 
 // Query with a broken answer section
 TEST_F(AuthSrvTest, shortAnswer) {
     shortAnswer();
+    checkAllRcodeCountersZeroExcept(Rcode::FORMERR(), 1);
 }
 
 // Query with unsupported version of EDNS.
 TEST_F(AuthSrvTest, ednsBadVers) {
     ednsBadVers();
+    checkAllRcodeCountersZeroExcept(Rcode::BADVERS(), 1);
 }
 
 TEST_F(AuthSrvTest, AXFROverUDP) {
@@ -244,6 +289,7 @@ TEST_F(AuthSrvTest, AXFRSuccess) {
     server.processMessage(*io_message, parse_message, response_obuffer, &dnsserv);
     EXPECT_FALSE(dnsserv.hasAnswer());
     EXPECT_TRUE(xfrout.isConnected());
+    checkAllRcodeCountersZero();
 }
 
 // Try giving the server a TSIG signed request and see it can anwer signed as
@@ -279,6 +325,8 @@ TEST_F(AuthSrvTest, TSIGSigned) {
                                    response_obuffer->getLength()));
     EXPECT_EQ(TSIGError::NOERROR(), error) <<
         "The server signed the response, but it doesn't seem to be valid";
+
+    checkAllRcodeCountersZeroExcept(Rcode::NOERROR(), 1);
 }
 
 // Give the server a signed request, but don't give it the key. It will
@@ -311,6 +359,8 @@ TEST_F(AuthSrvTest, TSIGSignedBadKey) {
     EXPECT_EQ(TSIGError::BAD_KEY_CODE, tsig->getRdata().getError());
     EXPECT_EQ(0, tsig->getRdata().getMACSize()) <<
         "It should be unsigned with this error";
+
+    checkAllRcodeCountersZeroExcept(Rcode::NOTAUTH(), 1);
 }
 
 // Give the server a signed request, but signed by a different key
@@ -344,6 +394,8 @@ TEST_F(AuthSrvTest, TSIGBadSig) {
     EXPECT_EQ(TSIGError::BAD_SIG_CODE, tsig->getRdata().getError());
     EXPECT_EQ(0, tsig->getRdata().getMACSize()) <<
         "It should be unsigned with this error";
+
+    checkAllRcodeCountersZeroExcept(Rcode::NOTAUTH(), 1);
 }
 
 // Give the server a signed unsupported request with a bad signature.
@@ -383,6 +435,8 @@ TEST_F(AuthSrvTest, TSIGCheckFirst) {
     // TSIG should have failed, and so the per opcode counter shouldn't be
     // incremented.
     EXPECT_EQ(0, server.getCounter(Opcode::RESERVED14()));
+
+    checkAllRcodeCountersZeroExcept(Rcode::NOTAUTH(), 1);
 }
 
 TEST_F(AuthSrvTest, AXFRConnectFail) {
@@ -531,6 +585,8 @@ TEST_F(AuthSrvTest, notify) {
     EXPECT_EQ(Name("example.com"), question->getName());
     EXPECT_EQ(RRClass::IN(), question->getClass());
     EXPECT_EQ(RRType::SOA(), question->getType());
+
+    checkAllRcodeCountersZeroExcept(Rcode::NOERROR(), 1);
 }
 
 TEST_F(AuthSrvTest, notifyForCHClass) {
@@ -759,6 +815,41 @@ TEST_F(AuthSrvTest, updateWithInMemoryClient) {
                 opcode.getCode(), QR_FLAG, 1, 0, 0, 0);
 }
 
+TEST_F(AuthSrvTest, queryWithInMemoryClientNoDNSSEC) {
+    // In this example, we do simple check that query is handled from the
+    // query handler class, and confirm it returns no error and a non empty
+    // answer section.  Detailed examination on the response content
+    // for various types of queries are tested in the query tests.
+    updateConfig(&server, CONFIG_INMEMORY_EXAMPLE, true);
+    ASSERT_NE(AuthSrv::InMemoryClientPtr(), server.getInMemoryClient(rrclass));
+    EXPECT_EQ(1, server.getInMemoryClient(rrclass)->getZoneCount());
+
+    createDataFromFile("nsec3query_nodnssec_fromWire.wire");
+    server.processMessage(*io_message, parse_message, response_obuffer,
+                          &dnsserv);
+
+    EXPECT_TRUE(dnsserv.hasAnswer());
+    headerCheck(*parse_message, default_qid, Rcode::NOERROR(),
+                opcode.getCode(), QR_FLAG | AA_FLAG, 1, 1, 2, 1);
+}
+
+TEST_F(AuthSrvTest, queryWithInMemoryClientDNSSEC) {
+    // Similar to the previous test, but the query has the DO bit on.
+    // The response should contain RRSIGs, and should have more RRs than
+    // the previous case.
+    updateConfig(&server, CONFIG_INMEMORY_EXAMPLE, true);
+    ASSERT_NE(AuthSrv::InMemoryClientPtr(), server.getInMemoryClient(rrclass));
+    EXPECT_EQ(1, server.getInMemoryClient(rrclass)->getZoneCount());
+
+    createDataFromFile("nsec3query_fromWire.wire");
+    server.processMessage(*io_message, parse_message, response_obuffer,
+                          &dnsserv);
+
+    EXPECT_TRUE(dnsserv.hasAnswer());
+    headerCheck(*parse_message, default_qid, Rcode::NOERROR(),
+                opcode.getCode(), QR_FLAG | AA_FLAG, 1, 2, 3, 3);
+}
+
 TEST_F(AuthSrvTest, chQueryWithInMemoryClient) {
     // Configure memory data source for class IN
     updateConfig(&server, "{\"datasources\": "
@@ -799,6 +890,10 @@ TEST_F(AuthSrvTest, queryCounterUDPNormal) {
                           &dnsserv);
     // After processing UDP query, the counter should be 1.
     EXPECT_EQ(1, server.getCounter(AuthCounters::SERVER_UDP_QUERY));
+    // The counter for opcode Query should also be one
+    EXPECT_EQ(1, server.getCounter(Opcode::QUERY()));
+    // The counter for REFUSED responses should also be one, the rest zero
+    checkAllRcodeCountersZeroExcept(Rcode::REFUSED(), 1);
 }
 
 // Submit TCP normal query and check query counter
@@ -814,6 +909,10 @@ TEST_F(AuthSrvTest, queryCounterTCPNormal) {
                           &dnsserv);
     // After processing TCP query, the counter should be 1.
     EXPECT_EQ(1, server.getCounter(AuthCounters::SERVER_TCP_QUERY));
+    // The counter for SUCCESS responses should also be one
+    EXPECT_EQ(1, server.getCounter(Opcode::QUERY()));
+    // The counter for REFUSED responses should also be one, the rest zero
+    checkAllRcodeCountersZeroExcept(Rcode::REFUSED(), 1);
 }
 
 // Submit TCP AXFR query and check query counter
@@ -829,6 +928,8 @@ TEST_F(AuthSrvTest, queryCounterTCPAXFR) {
     EXPECT_FALSE(dnsserv.hasAnswer());
     // After processing TCP AXFR query, the counter should be 1.
     EXPECT_EQ(1, server.getCounter(AuthCounters::SERVER_TCP_QUERY));
+    // No rcodes should be incremented
+    checkAllRcodeCountersZero();
 }
 
 // Submit TCP IXFR query and check query counter

src/bin/auth/tests/command_unittest.cc

@@ -14,14 +14,9 @@
 
 #include <config.h>
 
-#include <cassert>
-#include <cstdlib>
-#include <string>
-#include <stdexcept>
-
-#include <boost/bind.hpp>
-
-#include <gtest/gtest.h>
+#include <auth/auth_srv.h>
+#include <auth/auth_config.h>
+#include <auth/command.h>
 
 #include <dns/name.h>
 #include <dns/rrclass.h>
@@ -33,14 +28,22 @@
 
 #include <datasrc/memory_datasrc.h>
 
-#include <auth/auth_srv.h>
-#include <auth/auth_config.h>
-#include <auth/command.h>
-
 #include <asiolink/asiolink.h>
 
 #include <testutils/mockups.h>
 
+#include <cassert>
+#include <cstdlib>
+#include <string>
+#include <stdexcept>
+
+#include <boost/bind.hpp>
+
+#include <gtest/gtest.h>
+
+#include <sys/types.h>
+#include <unistd.h>
+
 using namespace std;
 using namespace isc::dns;
 using namespace isc::data;
@@ -50,58 +53,119 @@ using namespace isc::config;
 namespace {
 class AuthCommandTest : public ::testing::Test {
 protected:
-    AuthCommandTest() : server(false, xfrout), rcode(-1) {
-        server.setStatisticsSession(&statistics_session);
+    AuthCommandTest() :
+        server_(false, xfrout_),
+        rcode_(-1),
+        expect_rcode_(0),
+        itimer_(server_.getIOService())
+    {
+        server_.setStatisticsSession(&statistics_session_);
     }
     void checkAnswer(const int expected_code) {
-        parseAnswer(rcode, result);
-        EXPECT_EQ(expected_code, rcode);
+        parseAnswer(rcode_, result_);
+        EXPECT_EQ(expected_code, rcode_);
     }
-    MockSession statistics_session;
-    MockXfroutClient xfrout;
-    AuthSrv server;
-    ConstElementPtr result;
-    int rcode;
+    MockSession statistics_session_;
+    MockXfroutClient xfrout_;
+    AuthSrv server_;
+    ConstElementPtr result_;
+    // The shutdown command parameter
+    ConstElementPtr param_;
+    int rcode_, expect_rcode_;
+    isc::asiolink::IntervalTimer itimer_;
 public:
     void stopServer();          // need to be public for boost::bind
+    void dontStopServer();          // need to be public for boost::bind
 };
 
 TEST_F(AuthCommandTest, unknownCommand) {
-    result = execAuthServerCommand(server, "no_such_command",
-                                   ConstElementPtr());
-    parseAnswer(rcode, result);
-    EXPECT_EQ(1, rcode);
+    result_ = execAuthServerCommand(server_, "no_such_command",
+                                    ConstElementPtr());
+    parseAnswer(rcode_, result_);
+    EXPECT_EQ(1, rcode_);
 }
 
 TEST_F(AuthCommandTest, DISABLED_unexpectedException) {
     // execAuthServerCommand() won't catch standard exceptions.
     // Skip this test for now: ModuleCCSession doesn't seem to validate
     // commands.
-    EXPECT_THROW(execAuthServerCommand(server, "_throw_exception",
+    EXPECT_THROW(execAuthServerCommand(server_, "_throw_exception",
                                        ConstElementPtr()),
                  runtime_error);
 }
 
 TEST_F(AuthCommandTest, sendStatistics) {
-    result = execAuthServerCommand(server, "sendstats", ConstElementPtr());
+    result_ = execAuthServerCommand(server_, "sendstats", ConstElementPtr());
     // Just check some message has been sent.  Detailed tests specific to
     // statistics are done in its own tests.
-    EXPECT_EQ("Stats", statistics_session.getMessageDest());
+    EXPECT_EQ("Stats", statistics_session_.getMessageDest());
     checkAnswer(0);
 }
 
 void
 AuthCommandTest::stopServer() {
-    result = execAuthServerCommand(server, "shutdown", ConstElementPtr());
-    parseAnswer(rcode, result);
-    assert(rcode == 0); // make sure the test stops when something is wrong
+    result_ = execAuthServerCommand(server_, "shutdown", param_);
+    parseAnswer(rcode_, result_);
+    assert(rcode_ == 0); // make sure the test stops when something is wrong
 }
 
 TEST_F(AuthCommandTest, shutdown) {
-    isc::asiolink::IntervalTimer itimer(server.getIOService());
-    itimer.setup(boost::bind(&AuthCommandTest::stopServer, this), 1);
-    server.getIOService().run();
-    EXPECT_EQ(0, rcode);
+    // Param defaults to empty/null pointer on creation
+    itimer_.setup(boost::bind(&AuthCommandTest::stopServer, this), 1);
+    server_.getIOService().run();
+    EXPECT_EQ(0, rcode_);
+}
+
+TEST_F(AuthCommandTest, shutdownCorrectPID) {
+    // Put the pid parameter there
+    const pid_t pid(getpid());
+    ElementPtr param(new isc::data::MapElement());
+    param->set("pid", ConstElementPtr(new isc::data::IntElement(pid)));
+    param_ = param;
+    // With the correct PID, it should act exactly the same as in case
+    // of no parameter
+    itimer_.setup(boost::bind(&AuthCommandTest::stopServer, this), 1);
+    server_.getIOService().run();
+    EXPECT_EQ(0, rcode_);
+}
+
+// This is like stopServer, but the server should not stop after the
+// command, it should be running
+void
+AuthCommandTest::dontStopServer() {
+    result_ = execAuthServerCommand(server_, "shutdown", param_);
+    parseAnswer(rcode_, result_);
+    EXPECT_EQ(expect_rcode_, rcode_);
+    rcode_ = -1;
+    // We run the stopServer now, to really stop the server.
+    // If it had stopped already, it won't be run and the rcode -1 will
+    // be left here.
+    param_ = ConstElementPtr();
+    itimer_.cancel();
+    itimer_.setup(boost::bind(&AuthCommandTest::stopServer, this), 1);
+}
+
+// If we provide something not an int, the PID is not really specified, so
+// act as if nothing came.
+TEST_F(AuthCommandTest, shutdownNotInt) {
+    // Put the pid parameter there
+    ElementPtr param(new isc::data::MapElement());
+    param->set("pid", ConstElementPtr(new isc::data::StringElement("pid")));
+    param_ = param;
+    expect_rcode_ = 1;
+    // It should reject to stop if the PID is not an int.
+    itimer_.setup(boost::bind(&AuthCommandTest::dontStopServer, this), 1);
+    server_.getIOService().run();
+    EXPECT_EQ(0, rcode_);
+}
+
+TEST_F(AuthCommandTest, shutdownIncorrectPID) {
+    // The PID = 0 should be taken by init, so we are not init and the
+    // PID should be different
+    param_ = Element::fromJSON("{\"pid\": 0}");
+    itimer_.setup(boost::bind(&AuthCommandTest::dontStopServer, this), 1);
+    server_.getIOService().run();
+    EXPECT_EQ(0, rcode_);
 }
 
 // A helper function commonly used for the "loadzone" command tests.
@@ -165,7 +229,7 @@ newZoneChecks(AuthSrv& server) {
 }
 
 TEST_F(AuthCommandTest, loadZone) {
-    configureZones(server);
+    configureZones(server_);
 
     ASSERT_EQ(0, system(INSTALL_PROG " " TEST_DATA_DIR
                         "/test1-new.zone.in "
@@ -174,118 +238,118 @@ TEST_F(AuthCommandTest, loadZone) {
                         "/test2-new.zone.in "
                         TEST_DATA_BUILDDIR "/test2.zone.copied"));
 
-    result = execAuthServerCommand(server, "loadzone",
-                                   Element::fromJSON(
-                                       "{\"origin\": \"test1.example\"}"));
+    result_ = execAuthServerCommand(server_, "loadzone",
+                                    Element::fromJSON(
+                                        "{\"origin\": \"test1.example\"}"));
     checkAnswer(0);
-    newZoneChecks(server);
+    newZoneChecks(server_);
 }
 
 TEST_F(AuthCommandTest, loadBrokenZone) {
-    configureZones(server);
+    configureZones(server_);
 
     ASSERT_EQ(0, system(INSTALL_PROG " " TEST_DATA_DIR
                         "/test1-broken.zone.in "
                         TEST_DATA_BUILDDIR "/test1.zone.copied"));
-    result = execAuthServerCommand(server, "loadzone",
-                                   Element::fromJSON(
-                                       "{\"origin\": \"test1.example\"}"));
+    result_ = execAuthServerCommand(server_, "loadzone",
+                                    Element::fromJSON(
+                                        "{\"origin\": \"test1.example\"}"));
     checkAnswer(1);
-    zoneChecks(server);     // zone shouldn't be replaced
+    zoneChecks(server_);     // zone shouldn't be replaced
 }
 
 TEST_F(AuthCommandTest, loadUnreadableZone) {
-    configureZones(server);
+    configureZones(server_);
 
     // install the zone file as unreadable
     ASSERT_EQ(0, system(INSTALL_PROG " -m 000 " TEST_DATA_DIR
                         "/test1.zone.in "
                         TEST_DATA_BUILDDIR "/test1.zone.copied"));
-    result = execAuthServerCommand(server, "loadzone",
-                                   Element::fromJSON(
-                                       "{\"origin\": \"test1.example\"}"));
+    result_ = execAuthServerCommand(server_, "loadzone",
+                                    Element::fromJSON(
+                                        "{\"origin\": \"test1.example\"}"));
     checkAnswer(1);
-    zoneChecks(server);     // zone shouldn't be replaced
+    zoneChecks(server_);     // zone shouldn't be replaced
 }
 
 TEST_F(AuthCommandTest, loadZoneWithoutDataSrc) {
     // try to execute load command without configuring the zone beforehand.
     // it should fail.
-    result = execAuthServerCommand(server, "loadzone",
-                                   Element::fromJSON(
-                                       "{\"origin\": \"test1.example\"}"));
+    result_ = execAuthServerCommand(server_, "loadzone",
+                                    Element::fromJSON(
+                                        "{\"origin\": \"test1.example\"}"));
     checkAnswer(1);
 }
 
 TEST_F(AuthCommandTest, loadSqlite3DataSrc) {
     // For sqlite3 data source we don't have to do anything (the data source
     // (re)loads itself automatically)
-    result = execAuthServerCommand(server, "loadzone",
-                                   Element::fromJSON(
-                                       "{\"origin\": \"test1.example\","
-                                       " \"datasrc\": \"sqlite3\"}"));
+    result_ = execAuthServerCommand(server_, "loadzone",
+                                    Element::fromJSON(
+                                        "{\"origin\": \"test1.example\","
+                                        " \"datasrc\": \"sqlite3\"}"));
     checkAnswer(0);
 }
 
 TEST_F(AuthCommandTest, loadZoneInvalidParams) {
-    configureZones(server);
+    configureZones(server_);
 
     // null arg
-    result = execAuthServerCommand(server, "loadzone", ElementPtr());
+    result_ = execAuthServerCommand(server_, "loadzone", ElementPtr());
     checkAnswer(1);
 
     // zone class is bogus
-    result = execAuthServerCommand(server, "loadzone",
-                                   Element::fromJSON(
-                                       "{\"origin\": \"test1.example\","
-                                       " \"class\": \"no_such_class\"}"));
+    result_ = execAuthServerCommand(server_, "loadzone",
+                                    Element::fromJSON(
+                                        "{\"origin\": \"test1.example\","
+                                        " \"class\": \"no_such_class\"}"));
     checkAnswer(1);
 
-    result = execAuthServerCommand(server, "loadzone",
-                                   Element::fromJSON(
-                                       "{\"origin\": \"test1.example\","
-                                       " \"class\": 1}"));
+    result_ = execAuthServerCommand(server_, "loadzone",
+                                    Element::fromJSON(
+                                        "{\"origin\": \"test1.example\","
+                                        " \"class\": 1}"));
     checkAnswer(1);
 
     // unsupported zone class
-    result = execAuthServerCommand(server, "loadzone",
-                                   Element::fromJSON(
-                                       "{\"origin\": \"test1.example\","
-                                       " \"class\": \"CH\"}"));
+    result_ = execAuthServerCommand(server_, "loadzone",
+                                    Element::fromJSON(
+                                        "{\"origin\": \"test1.example\","
+                                        " \"class\": \"CH\"}"));
     checkAnswer(1);
 
     // unsupported data source class
-    result = execAuthServerCommand(server, "loadzone",
-                                   Element::fromJSON(
-                                       "{\"origin\": \"test1.example\","
-                                       " \"datasrc\": \"not supported\"}"));
+    result_ = execAuthServerCommand(server_, "loadzone",
+                                    Element::fromJSON(
+                                        "{\"origin\": \"test1.example\","
+                                        " \"datasrc\": \"not supported\"}"));
     checkAnswer(1);
 
     // data source is bogus
-    result = execAuthServerCommand(server, "loadzone",
-                                   Element::fromJSON(
-                                       "{\"origin\": \"test1.example\","
-                                       " \"datasrc\": 0}"));
+    result_ = execAuthServerCommand(server_, "loadzone",
+                                    Element::fromJSON(
+                                        "{\"origin\": \"test1.example\","
+                                        " \"datasrc\": 0}"));
     checkAnswer(1);
 
     // origin is missing
-    result = execAuthServerCommand(server, "loadzone",
-                                   Element::fromJSON("{}"));
+    result_ = execAuthServerCommand(server_, "loadzone",
+                                    Element::fromJSON("{}"));
     checkAnswer(1);
 
     // zone doesn't exist in the data source
-    result = execAuthServerCommand(server, "loadzone",
-                                   Element::fromJSON("{\"origin\": \"xx\"}"));
+    result_ = execAuthServerCommand(server_, "loadzone",
+                                    Element::fromJSON("{\"origin\": \"xx\"}"));
     checkAnswer(1);
 
     // origin is bogus
-    result = execAuthServerCommand(server, "loadzone",
-                                   Element::fromJSON(
-                                       "{\"origin\": \"...\"}"));
+    result_ = execAuthServerCommand(server_, "loadzone",
+                                    Element::fromJSON(
+                                        "{\"origin\": \"...\"}"));
     checkAnswer(1);
 
-    result = execAuthServerCommand(server, "loadzone",
-                                   Element::fromJSON("{\"origin\": 10}"));
+    result_ = execAuthServerCommand(server_, "loadzone",
+                                    Element::fromJSON("{\"origin\": 10}"));
     checkAnswer(1);
 }
 }

src/bin/auth/tests/query_unittest.cc

@@ -185,6 +185,28 @@ const char* const nsec3_www_txt =
     "q04jkcevqvmu85r014c7dkba38o0ji5r.example.com. 3600 IN NSEC3 1 1 12 "
     "aabbccdd r53bq7cc2uvmubfu5ocmm6pers9tk9en A RRSIG\n";
 
+// NSEC3 for wild.example.com (used in wildcard tests, will be added on
+// demand not to confuse other tests)
+const char* const nsec3_atwild_txt =
+    "ji6neoaepv8b5o6k4ev33abha8ht9fgc.example.com. 3600 IN NSEC3 1 1 12 "
+    "aabbccdd r53bq7cc2uvmubfu5ocmm6pers9tk9en\n";
+
+// NSEC3 for cnamewild.example.com (used in wildcard tests, will be added on
+// demand not to confuse other tests)
+const char* const nsec3_atcnamewild_txt =
+    "k8udemvp1j2f7eg6jebps17vp3n8i58h.example.com. 3600 IN NSEC3 1 1 12 "
+    "aabbccdd r53bq7cc2uvmubfu5ocmm6pers9tk9en\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";
@@ -192,12 +214,23 @@ const char* const signed_delegation_ds_txt =
     "signed-delegation.example.com. 3600 IN DS 12345 8 2 "
     "764501411DE58E8618945054A3F620B36202E115D015A7773F4B78E0F952CECA\n";
 
-// (Secure) delegation data; Delegation without DS record (and NSEC denying
-// its existence.
+// (Secure) delegation data; Delegation without DS record (and both NSEC
+// and NSEC3 denying its existence)
 const char* const unsigned_delegation_txt =
     "unsigned-delegation.example.com. 3600 IN NS ns.example.net.\n";
 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";
+
+// Delegation without DS record, and no direct matching NSEC3 record
+const char* const unsigned_delegation_optout_txt =
+    "unsigned-delegation-optout.example.com. 3600 IN NS ns.example.net.\n";
+const char* const unsigned_delegation_optout_nsec_txt =
+    "unsigned-delegation-optout.example.com. 3600 IN NSEC "
     "*.uwild.example.com. NS RRSIG NSEC\n";
 
 // (Secure) delegation data; Delegation where the DS lookup will raise an
@@ -260,7 +293,9 @@ public:
         rrclass_(RRClass::IN()),
         include_rrsig_anyway_(false),
         use_nsec3_(false),
-        nsec_name_(origin_)
+        nsec_name_(origin_),
+        nsec3_fake_(NULL),
+        nsec3_name_(NULL)
     {
         stringstream zone_stream;
         zone_stream << soa_txt << zone_ns_txt << ns_addrs_txt <<
@@ -276,6 +311,8 @@ public:
             nsec3_apex_txt << nsec3_www_txt <<
             signed_delegation_txt << signed_delegation_ds_txt <<
             unsigned_delegation_txt << unsigned_delegation_nsec_txt <<
+            unsigned_delegation_optout_txt <<
+            unsigned_delegation_optout_nsec_txt <<
             bad_delegation_txt;
 
         masterLoad(zone_stream, origin_, rrclass_,
@@ -288,8 +325,12 @@ public:
 
         // (Faked) NSEC3 hash map.  For convenience we use hardcoded built-in
         // map instead of calculating and using actual hash.
-        // The used hash values are borrowed from RFC5155 examples.
+        // The used hash values are borrowed from RFC5155 examples (they are
+        // based on the query name, not that they would correspond directly
+        // to the name).
         hash_map_[Name("example.com")] = "0p9mhaveqvm6t7vbl5lop2u3t2rp3tom";
+        hash_map_[Name("www.example.com")] =
+            "q04jkcevqvmu85r014c7dkba38o0ji5r";
         hash_map_[Name("nxdomain.example.com")] =
             "v644ebqk9bibcna874givr6joj62mlhv";
         hash_map_[Name("nx.domain.example.com")] =
@@ -300,6 +341,32 @@ public:
             "q00jkcevqvmu85r014c7dkba38o0ji5r";
         hash_map_[Name("nxdomain3.example.com")] =
             "009mhaveqvm6t7vbl5lop2u3t2rp3tom";
+        hash_map_[Name("*.example.com")] =
+            "r53bq7cc2uvmubfu5ocmm6pers9tk9en";
+        hash_map_[Name("unsigned-delegation.example.com")] =
+            "q81r598950igr1eqvc60aedlq66425b5"; // a bit larger than H(www)
+        hash_map_[Name("*.uwild.example.com")] =
+            "b4um86eghhds6nea196smvmlo4ors995";
+        hash_map_[Name("unsigned-delegation-optout.example.com")] =
+            "vld46lphhasfapj8og1pglgiasa5o5gt";
+
+        // For wildcard proofs
+        hash_map_[Name("wild.example.com")] =
+            "ji6neoaepv8b5o6k4ev33abha8ht9fgc";
+        hash_map_[Name("y.wild.example.com")] =
+            "0p9mhaveqvm6t7vbl5lop2u3t2rp3ton"; // a bit larger than H(<apex>)
+        hash_map_[Name("x.y.wild.example.com")] =
+            "q04jkcevqvmu85r014c7dkba38o0ji6r"; // a bit larger than H(www)
+        hash_map_[Name("cnamewild.example.com")] =
+            "k8udemvp1j2f7eg6jebps17vp3n8i58h";
+        hash_map_[Name("www.cnamewild.example.com")] =
+            "q04jkcevqvmu85r014c7dkba38o0ji6r"; // a bit larger than H(www)
+
+        // 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_); }
@@ -330,7 +397,19 @@ public:
                        ConstRRsetPtr rrset)
     {
         nsec_name_ = nsec_name;
-        nsec_result_.reset(new ZoneFinder::FindResult(code, rrset));
+        nsec_result_.reset(new ZoneFinder::FindResult(code, rrset,
+                                                      RESULT_NSEC_SIGNED));
+    }
+
+    // Once called, the findNSEC3 will return the provided result for the next
+    // 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,
+                        const Name* name = NULL)
+    {
+        nsec3_fake_ = result;
+        nsec3_name_ = name;
     }
 
     // If true is passed return an empty NSEC3 RRset for some negative
@@ -427,6 +506,12 @@ private:
     // The following two will be used for faked NSEC cases
     Name nsec_name_;
     boost::scoped_ptr<ZoneFinder::FindResult> nsec_result_;
+    // 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_;
 };
 
@@ -471,6 +556,13 @@ 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 &&
+        (nsec3_name_ == NULL || *nsec3_name_ == name)) {
+        const FindNSEC3Result* result(nsec3_fake_);
+        return (*result);
+    }
+
     ConstRRsetPtr covering_proof;
     const int labels = name.getLabelCount();
 
@@ -478,6 +570,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);
@@ -634,11 +730,13 @@ MockZoneFinder::find(const Name& name, const RRType& type,
     // hardcoded specific cases, ignoring other details such as canceling
     // due to the existence of closer name.
     if ((options & NO_WILDCARD) == 0) {
-        const Name wild_suffix(name.split(1));
+        const Name wild_suffix(name == Name("x.y.wild.example.com") ?
+                               Name("wild.example.com") : name.split(1));
         // Unit Tests use those domains for Wildcard test.
-        if (name.equals(Name("www.wild.example.com"))||
-           name.equals(Name("www1.uwild.example.com"))||
-           name.equals(Name("a.t.example.com"))) {
+        if (name.equals(Name("www.wild.example.com")) ||
+            name.equals(Name("x.y.wild.example.com")) ||
+            name.equals(Name("www1.uwild.example.com")) ||
+            name.equals(Name("a.t.example.com"))) {
             if (name.compare(wild_suffix).getRelation() ==
                 NameComparisonResult::SUBDOMAIN) {
                 domain = domains_.find(Name("*").concatenate(wild_suffix));
@@ -656,7 +754,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 |
@@ -986,6 +1085,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
@@ -1223,6 +1369,69 @@ TEST_F(QueryTest, CNAMEwildNSEC) {
                   mock_finder->getOrigin());
 }
 
+TEST_F(QueryTest, wildcardNSEC3) {
+    // Similar to wildcardNSEC, but the zone is signed with NSEC3.
+    // The next closer is y.wild.example.com, the covering NSEC3 for it
+    // is (in our setup) the NSEC3 for the apex.
+    mock_finder->setNSEC3Flag(true);
+
+    // This is NSEC3 for wild.example.com, which will be used in the middle
+    // of identifying the next closer name.
+    mock_finder->addRecord(nsec3_atwild_txt);
+
+    Query(memory_client, Name("x.y.wild.example.com"), RRType::A(), response,
+          true).process();
+    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 ") +
+                   getCommonRRSIGText("A") + "\n").c_str(),
+                  // 3 NSes and their RRSIG
+                  (zone_ns_txt + string("example.com. 3600 IN RRSIG ") +
+                   getCommonRRSIGText("NS") + "\n" +
+                   // NSEC3 for the wildcard proof and its RRSIG
+                   string(nsec3_apex_txt) +
+                   mock_finder->hash_map_[Name("example.com.")] +
+                   string(".example.com. 3600 IN RRSIG ") +
+                   getCommonRRSIGText("NSEC3")).c_str(),
+                  NULL, // we are not interested in additionals in this test
+                  mock_finder->getOrigin());
+}
+
+TEST_F(QueryTest, CNAMEwildNSEC3) {
+    // Similar to CNAMEwildNSEC, but with NSEC3.
+    // The next closer is qname itself, the covering NSEC3 for it
+    // is (in our setup) the NSEC3 for the www.example.com.
+    mock_finder->setNSEC3Flag(true);
+    mock_finder->addRecord(nsec3_atcnamewild_txt);
+
+    Query(memory_client, Name("www.cnamewild.example.com"), RRType::A(),
+          response, true).process();
+    responseCheck(response, Rcode::NOERROR(), AA_FLAG, 2, 2, 0,
+                  (string(cnamewild_txt).replace(0, 1, "www") +
+                   string("www.cnamewild.example.com. 3600 IN RRSIG ") +
+                   getCommonRRSIGText("CNAME") + "\n").c_str(),
+                  (string(nsec3_www_txt) +
+                   mock_finder->hash_map_[Name("www.example.com.")] +
+                   string(".example.com. 3600 IN RRSIG ") +
+                   getCommonRRSIGText("NSEC3")).c_str(),
+                  NULL, // we are not interested in additionals in this test
+                  mock_finder->getOrigin());
+}
+
+TEST_F(QueryTest, badWildcardNSEC3) {
+    // Similar to wildcardNSEC3, 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);
+
+    EXPECT_THROW(Query(memory_client, Name("www.wild.example.com"),
+                       RRType::A(), response, true).process(),
+                 isc::InvalidParameter);
+}
+
 TEST_F(QueryTest, badWildcardProof1) {
     // Unexpected case in wildcard proof: ZoneFinder::find() returns SUCCESS
     // when NXDOMAIN is expected.
@@ -1254,8 +1463,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();
@@ -1270,11 +1479,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 ") +
@@ -1288,9 +1498,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();
@@ -1614,35 +1889,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, nsec3_www_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(nsec3_www_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(nsec3_www_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, nsec3_www_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
@@ -1860,56 +2160,164 @@ TEST_F(QueryTest, dsAtRootWithDS) {
                   NULL);
 }
 
-// The following are tentative tests until we really add tests for the
-// query logic for these cases.  At that point it's probably better to
-// clean them up.
-TEST_F(QueryTest, nxdomainWithNSEC3) {
+// Check the signature is present when an NXRRSET is returned
+TEST_F(QueryTest, nxrrsetWithNSEC3) {
     mock_finder->setNSEC3Flag(true);
-    ZoneFinder::FindResult result = mock_finder->find(
-        Name("nxdomain.example.com"), RRType::A(), ZoneFinder::FIND_DNSSEC);
-    EXPECT_EQ(ZoneFinder::NXDOMAIN, result.code);
-    EXPECT_FALSE(result.rrset);
-    EXPECT_TRUE(result.isNSEC3Signed());
-    EXPECT_FALSE(result.isWildcard());
+
+    // NXRRSET with DNSSEC proof.  We should have SOA, NSEC3 that proves the
+    // NXRRSET and their RRSIGs.
+    Query(memory_client, Name("www.example.com"), RRType::TXT(), response,
+          true).process();
+
+    responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 4, 0, NULL,
+                  (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
+                   getCommonRRSIGText("SOA") + "\n" +
+                   string(nsec3_www_txt) + "\n" +
+                   mock_finder->hash_map_[Name("www.example.com.")] +
+                   ".example.com. 3600 IN RRSIG " +
+                   getCommonRRSIGText("NSEC3") + "\n").c_str(),
+                  NULL, mock_finder->getOrigin());
 }
 
-TEST_F(QueryTest, nxrrsetWithNSEC3) {
+// Check the exception is correctly raised when the NSEC3 thing isn't in the
+// zone
+TEST_F(QueryTest, nxrrsetMissingNSEC3) {
     mock_finder->setNSEC3Flag(true);
-    ZoneFinder::FindResult result = mock_finder->find(
-        Name("www.example.com"), RRType::TXT(), ZoneFinder::FIND_DNSSEC);
-    EXPECT_EQ(ZoneFinder::NXRRSET, result.code);
-    EXPECT_FALSE(result.rrset);
-    EXPECT_TRUE(result.isNSEC3Signed());
-    EXPECT_FALSE(result.isWildcard());
+    // We just need it to return false for "matched". This indicates
+    // there's no exact match for NSEC3 on www.example.com.
+    ZoneFinder::FindNSEC3Result nsec3(false, 0, ConstRRsetPtr(),
+                                      ConstRRsetPtr());
+    mock_finder->setNSEC3Result(&nsec3);
+
+    EXPECT_THROW(Query(memory_client, Name("www.example.com"), RRType::TXT(),
+                       response, true).process(), Query::BadNSEC3);
 }
 
-TEST_F(QueryTest, emptyNameWithNSEC3) {
+TEST_F(QueryTest, nxrrsetWithNSEC3_ds_exact) {
+    mock_finder->addRecord(unsigned_delegation_nsec3_txt);
     mock_finder->setNSEC3Flag(true);
-    ZoneFinder::FindResult result = mock_finder->find(
-        Name("no.example.com"), RRType::A(), ZoneFinder::FIND_DNSSEC);
-    EXPECT_EQ(ZoneFinder::NXRRSET, result.code);
-    EXPECT_FALSE(result.rrset);
-    EXPECT_TRUE(result.isNSEC3Signed());
-    EXPECT_FALSE(result.isWildcard());
+
+    // This delegation has no DS, but does have a matching NSEC3 record
+    // (See RFC5155 section 7.2.4)
+    Query(memory_client, Name("unsigned-delegation.example.com."),
+          RRType::DS(), response, true).process();
+    responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 4, 0, NULL,
+                  (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
+                   getCommonRRSIGText("SOA") + "\n" +
+                   string(unsigned_delegation_nsec3_txt) + "\n" +
+                   mock_finder->
+                        hash_map_[Name("unsigned-delegation.example.com.")] +
+                   ".example.com. 3600 IN RRSIG " +
+                   getCommonRRSIGText("NSEC3") + "\n").c_str(),
+                  NULL, mock_finder->getOrigin());
 }
 
-TEST_F(QueryTest, wildcardNxrrsetWithNSEC3) {
+TEST_F(QueryTest, nxrrsetWithNSEC3_ds_no_exact) {
+    mock_finder->addRecord(unsigned_delegation_nsec3_txt);
     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());
+
+    // This delegation has no DS, and no directly matching NSEC3 record
+    // So the response should contain closest encloser proof (and the
+    // 'next closer' should have opt-out set, though that is not
+    // actually checked)
+    // (See RFC5155 section 7.2.4)
+    Query(memory_client, Name("unsigned-delegation-optout.example.com."),
+          RRType::DS(), response, true).process();
+    responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 6, 0, NULL,
+                  (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
+                   getCommonRRSIGText("SOA") + "\n" +
+                   string(nsec3_apex_txt) + "\n" +
+                   mock_finder->hash_map_[Name("example.com.")] +
+                   ".example.com. 3600 IN RRSIG " +
+                   getCommonRRSIGText("NSEC3") + "\n" +
+                   string(unsigned_delegation_nsec3_txt) + "\n" +
+                   mock_finder->
+                        hash_map_[Name("unsigned-delegation.example.com.")] +
+                   ".example.com. 3600 IN RRSIG " +
+                   getCommonRRSIGText("NSEC3") + "\n").c_str(),
+                  NULL, mock_finder->getOrigin());
+}
+
+TEST_F(QueryTest, nxdomainWithNSEC3Proof) {
+    // Name Error (NXDOMAIN) case with NSEC3 proof per RFC5155 Section 7.2.2.
+
+    // Enable NSEC3
+    mock_finder->setNSEC3Flag(true);
+    // This will be the covering NSEC3 for the next closer
+    mock_finder->addRecord(nsec3_uwild_txt);
+    // This will be the covering NSEC3 for the possible wildcard
+    mock_finder->addRecord(unsigned_delegation_nsec3_txt);
+
+    Query(memory_client, Name("nxdomain.example.com"), qtype,
+          response, true).process();
+    responseCheck(response, Rcode::NXDOMAIN(), 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_apex_txt) + "\n" +
+                   mock_finder->hash_map_[mock_finder->getOrigin()] +
+                   string(".example.com. 3600 IN RRSIG ") +
+                   getCommonRRSIGText("NSEC3") + "\n" +
+                   // NSEC3 for the next closer + its RRSIG
+                   string(nsec3_uwild_txt) + "\n" +
+                   mock_finder->hash_map_[Name("uwild.example.com")] +
+                   ".example.com. 3600 IN RRSIG " +
+                   getCommonRRSIGText("NSEC3") + "\n" +
+                   // NSEC3 for the wildcard + its RRSIG
+                   string(unsigned_delegation_nsec3_txt) +
+                   mock_finder->hash_map_[
+                       Name("unsigned-delegation.example.com")] +
+                   ".example.com. 3600 IN RRSIG " +
+                   getCommonRRSIGText("NSEC3")).c_str(),
+                  NULL, mock_finder->getOrigin());
 }
 
-TEST_F(QueryTest, wildcardEmptyWithNSEC3) {
+TEST_F(QueryTest, nxdomainWithBadNextNSEC3Proof) {
+    // 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("nxdomain.example.com"),
+                       RRType::TXT(), response, true).process(),
+                 isc::InvalidParameter);
+}
+
+TEST_F(QueryTest, nxdomainWithBadWildcardNSEC3Proof) {
+    // Similar to nxdomainWithNSEC3Proof, but let findNSEC3() return a matching
+    // NSEC3 for the possible wildcard name, emulating run-time collision.
+    // This should result in BadNSEC3 exception.
+
+    mock_finder->setNSEC3Flag(true);
+    mock_finder->addRecord(nsec3_uwild_txt);
+    mock_finder->addRecord(unsigned_delegation_nsec3_txt);
+
+    const Name wname("*.example.com");
+    ZoneFinder::FindNSEC3Result nsec3(true, 0, textToRRset(nsec3_apex_txt),
+                                      ConstRRsetPtr());
+    mock_finder->setNSEC3Result(&nsec3, &wname);
+
+    EXPECT_THROW(Query(memory_client, Name("nxdomain.example.com"), qtype,
+                       response, true).process(),
+                 Query::BadNSEC3);
+}
+
+// The following are tentative tests until we really add tests for the
+// query logic for these cases.  At that point it's probably better to
+// clean them up.
+TEST_F(QueryTest, emptyNameWithNSEC3) {
     mock_finder->setNSEC3Flag(true);
     ZoneFinder::FindResult result = mock_finder->find(
-        Name("a.t.example.com"), RRType::A(), ZoneFinder::FIND_DNSSEC);
+        Name("no.example.com"), RRType::A(), ZoneFinder::FIND_DNSSEC);
     EXPECT_EQ(ZoneFinder::NXRRSET, result.code);
+    EXPECT_FALSE(result.rrset);
     EXPECT_TRUE(result.isNSEC3Signed());
-    EXPECT_TRUE(result.isWildcard());
+    EXPECT_FALSE(result.isWildcard());
 }
 }

src/bin/auth/tests/statistics_unittest.cc

@@ -21,6 +21,7 @@
 #include <boost/bind.hpp>
 
 #include <dns/opcode.h>
+#include <dns/rcode.h>
 
 #include <cc/data.h>
 #include <cc/session.h>
@@ -184,6 +185,16 @@ TEST_F(AuthCountersTest, incrementOpcodeCounter) {
     }
 }
 
+TEST_F(AuthCountersTest, incrementRcodeCounter) {
+    // The counter should be initialized to 0.  If we increment it by 1
+    // the counter should be 1.
+    for (int i = 0; i < 17; ++i) {
+        EXPECT_EQ(0, counters.getCounter(Rcode(i)));
+        counters.inc(Rcode(i));
+        EXPECT_EQ(1, counters.getCounter(Rcode(i)));
+    }
+}
+
 TEST_F(AuthCountersTest, submitStatisticsWithoutSession) {
     // Set statistics_session to NULL and call submitStatistics().
     // Expect to return false.
@@ -225,6 +236,29 @@ opcodeDataCheck(ConstElementPtr data, const int expected[16]) {
     ASSERT_EQ(static_cast<const char*>(NULL), item_names[i]);
 }
 
+void
+rcodeDataCheck(ConstElementPtr data, const int expected[17]) {
+    const char* item_names[] = {
+        "noerror", "formerr", "servfail", "nxdomain", "notimp", "refused",
+        "yxdomain", "yxrrset", "nxrrset", "notauth", "notzone", "reserved11",
+        "reserved12", "reserved13", "reserved14", "reserved15", "badvers",
+        NULL
+    };
+    int i;
+    for (i = 0; i < 17; ++i) {
+        ASSERT_NE(static_cast<const char*>(NULL), item_names[i]);
+        const string item_name = "rcode." + string(item_names[i]);
+        if (expected[i] == 0) {
+            EXPECT_FALSE(data->get(item_name));
+        } else {
+            EXPECT_EQ(expected[i], data->get(item_name)->intValue());
+        }
+    }
+    // We should have examined all names
+    ASSERT_EQ(static_cast<const char*>(NULL), item_names[i]);
+}
+
+
 TEST_F(AuthCountersTest, submitStatisticsWithoutValidator) {
     // Submit statistics data.
     // Validate if it submits correct data.
@@ -258,6 +292,10 @@ TEST_F(AuthCountersTest, submitStatisticsWithoutValidator) {
     const int opcode_results[16] = { 0, 0, 0, 0, 0, 0, 0, 0,
                                      0, 0, 0, 0, 0, 0, 0, 0 };
     opcodeDataCheck(statistics_data, opcode_results);
+    // By default rcode counters are all 0 and omitted
+    const int rcode_results[17] = { 0, 0, 0, 0, 0, 0, 0, 0, 0,
+                                    0, 0, 0, 0, 0, 0, 0, 0 };
+    rcodeDataCheck(statistics_data, rcode_results);
 }
 
 void
@@ -269,6 +307,15 @@ updateOpcodeCounters(AuthCounters &counters, const int expected[16]) {
     }
 }
 
+void
+updateRcodeCounters(AuthCounters &counters, const int expected[17]) {
+    for (int i = 0; i < 17; ++i) {
+        for (int j = 0; j < expected[i]; ++j) {
+            counters.inc(Rcode(i));
+        }
+    }
+}
+
 TEST_F(AuthCountersTest, submitStatisticsWithOpcodeCounters) {
     // Increment some of the opcode counters.  Then they should appear in the
     // submitted data; others shouldn't
@@ -293,6 +340,30 @@ TEST_F(AuthCountersTest, submitStatisticsWithAllOpcodeCounters) {
     opcodeDataCheck(statistics_data, opcode_results);
 }
 
+TEST_F(AuthCountersTest, submitStatisticsWithRcodeCounters) {
+    // Increment some of the rcode counters.  Then they should appear in the
+    // submitted data; others shouldn't
+    const int rcode_results[17] = { 1, 2, 3, 4, 5, 6, 7, 8, 9,
+                                    10, 0, 0, 0, 0, 0, 0, 11 };
+    updateRcodeCounters(counters, rcode_results);
+    counters.submitStatistics();
+    ConstElementPtr statistics_data = statistics_session_.sent_msg
+        ->get("command")->get(1)->get("data");
+    rcodeDataCheck(statistics_data, rcode_results);
+}
+
+TEST_F(AuthCountersTest, submitStatisticsWithAllRcodeCounters) {
+    // Increment all rcode counters.  Then they should appear in the
+    // submitted data.
+    const int rcode_results[17] = { 1, 1, 1, 1, 1, 1, 1, 1, 1,
+                                     1, 1, 1, 1, 1, 1, 1, 1 };
+    updateOpcodeCounters(counters, rcode_results);
+    counters.submitStatistics();
+    ConstElementPtr statistics_data = statistics_session_.sent_msg
+        ->get("command")->get(1)->get("data");
+    opcodeDataCheck(statistics_data, rcode_results);
+}
+
 TEST_F(AuthCountersTest, submitStatisticsWithValidator) {
 
     //a validator for the unittest

src/bin/bind10/bind10_src.py.in

@@ -193,9 +193,13 @@ class BoB:
         self.nocache = nocache
         self.component_config = {}
         # Some time in future, it may happen that a single component has
-        # multple processes. If so happens, name "components" may be
-        # inapropriate. But as the code isn't probably completely ready
-        # for it, we leave it at components for now.
+        # multple processes (like a pipeline-like component). If so happens,
+        # name "components" may be inapropriate. But as the code isn't probably
+        # completely ready for it, we leave it at components for now. We also
+        # want to support multiple instances of a single component. If it turns
+        # out that we'll have a single component with multiple same processes
+        # or if we start multiple components with the same configuration (we do
+        # this now, but it might change) is an open question.
         self.components = {}
         # Simply list of components that died and need to wait for a
         # restart. Components manage their own restart schedule now
@@ -649,14 +653,17 @@ class BoB:
         self.__started = True
         return None
 
-    def stop_process(self, process, recipient):
+    def stop_process(self, process, recipient, pid):
         """
         Stop the given process, friendly-like. The process is the name it has
-        (in logs, etc), the recipient is the address on msgq.
+        (in logs, etc), the recipient is the address on msgq. The pid is the
+        pid of the process (if we have multiple processes of the same name,
+        it might want to choose if it is for this one).
         """
         logger.info(BIND10_STOP_PROCESS, process)
-        self.cc_session.group_sendmsg({'command': ['shutdown']}, recipient,
-            recipient)
+        self.cc_session.group_sendmsg(isc.config.ccsession.
+                                      create_command('shutdown', {'pid': pid}),
+                                      recipient, recipient)
 
     def component_shutdown(self, exitcode=0):
         """

src/bin/bind10/tests/bind10_test.py.in

@@ -460,6 +460,22 @@ class TestBoB(unittest.TestCase):
         # The drop_socket is not tested here, but in TestCacheCommands.
         # It needs the cache mocks to be in place and they are there.
 
+    def test_stop_process(self):
+        """
+        Test checking the stop_process method sends the right message over
+        the message bus.
+        """
+        class DummySession():
+            def group_sendmsg(self, msg, group, instance="*"):
+                (self.msg, self.group, self.instance) = (msg, group, instance)
+        bob = BoB()
+        bob.cc_session = DummySession()
+        bob.stop_process('process', 'address', 42)
+        self.assertEqual('address', bob.cc_session.group)
+        self.assertEqual('address', bob.cc_session.instance)
+        self.assertEqual({'command': ['shutdown', {'pid': 42}]},
+                         bob.cc_session.msg)
+
 # Class for testing the BoB without actually starting processes.
 # This is used for testing the start/stop components routines and
 # the BoB commands.
@@ -598,7 +614,7 @@ class MockBob(BoB):
         procinfo.pid = 14
         return procinfo
 
-    def stop_process(self, process, recipient):
+    def stop_process(self, process, recipient, pid):
         procmap = { 'b10-auth': self.stop_auth,
                     'b10-resolver': self.stop_resolver,
                     'b10-xfrout': self.stop_xfrout,

src/bin/cmdctl/cmdctl.spec.pre.in

@@ -31,7 +31,13 @@
       {
         "command_name": "shutdown",
         "command_description": "shutdown cmdctl",
-        "command_args": []
+        "command_args": [
+          {
+            "item_name": "pid",
+            "item_type": "integer",
+            "item_optional": true
+          }
+        ]
       }
     ]
   }

src/bin/ddns/ddns.spec

@@ -34,7 +34,13 @@
       {
         "command_name": "shutdown",
         "command_description": "Shut down DDNS",
-        "command_args": []
+        "command_args": [
+          {
+            "item_name": "pid",
+            "item_type": "integer",
+            "item_optional": true
+          }
+        ]
       }
     ]
   }

src/bin/resolver/main.cc

@@ -14,18 +14,10 @@
 
 #include <config.h>
 
-#include <sys/types.h>
-#include <sys/socket.h>
-#include <sys/select.h>
-#include <netdb.h>
-#include <netinet/in.h>
-#include <stdlib.h>
-#include <errno.h>
-
-#include <string>
-#include <iostream>
-
-#include <boost/foreach.hpp>
+#include <resolver/spec_config.h>
+#include <resolver/resolver.h>
+#include "resolver_log.h"
+#include "common.h"
 
 #include <asiodns/asiodns.h>
 #include <asiolink/asiolink.h>
@@ -47,16 +39,26 @@
 
 #include <auth/common.h>
 
-#include <resolver/spec_config.h>
-#include <resolver/resolver.h>
-
 #include <cache/resolver_cache.h>
 #include <nsas/nameserver_address_store.h>
 
 #include <log/logger_support.h>
 #include <log/logger_level.h>
 #include "resolver_log.h"
-#include "common.h"
+
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <sys/select.h>
+#include <netdb.h>
+#include <netinet/in.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <unistd.h>
+
+#include <string>
+#include <iostream>
+
+#include <boost/foreach.hpp>
 
 using namespace std;
 using namespace isc::cc;
@@ -81,15 +83,31 @@ ConstElementPtr
 my_command_handler(const string& command, ConstElementPtr args) {
     ConstElementPtr answer = createAnswer();
 
-    if (command == "print_message") {
-        LOG_INFO(resolver_logger, RESOLVER_PRINT_COMMAND).arg(args);
-        /* let's add that message to our answer as well */
-        answer = createAnswer(0, args);
-    } else if (command == "shutdown") {
-        io_service.stop();
-    }
+    try {
+        if (command == "print_message") {
+            LOG_INFO(resolver_logger, RESOLVER_PRINT_COMMAND).arg(args);
+            /* let's add that message to our answer as well */
+            answer = createAnswer(0, args);
+        } else if (command == "shutdown") {
+            // Is the pid argument provided?
+            if (args && args->contains("pid")) {
+                // If it is, we check it is the same as our PID
+                const int pid(args->get("pid")->intValue());
+                const pid_t my_pid(getpid());
+                if (my_pid != pid) {
+                    // It is not for us (this is expected, see auth/command.cc
+                    // and the ShutdownCommand there).
+                    return (answer);
+                }
+            }
+            LOG_DEBUG(resolver_logger, RESOLVER_DBG_INIT, RESOLVER_SHUTDOWN);
+            io_service.stop();
+        }
 
-    return (answer);
+        return (answer);
+    } catch (const std::exception& e) {
+        return (createAnswer(1, e.what()));
+    }
 }
 
 void

src/bin/resolver/resolver.spec.pre.in

@@ -154,7 +154,13 @@
       {
         "command_name": "shutdown",
         "command_description": "Shut down recursive DNS server",
-        "command_args": []
+        "command_args": [
+          {
+            "item_name": "pid",
+            "item_type": "integer",
+            "item_optional": true
+          }
+        ]
       }
     ]
   }

src/bin/resolver/resolver_messages.mes

@@ -246,3 +246,7 @@ RESOLVER_QUERY_REJECTED case, the server does not return any response.
 The log message shows the query in the form of <query name>/<query
 type>/<query class>, and the client that sends the query in the form of
 <Source IP address>#<source port>.
+
+% RESOLVER_SHUTDOWN asked to shut down, doing so
+A debug message noting that the server was asked to terminate and is
+complying to the request.

src/bin/stats/stats-httpd.spec

@@ -47,7 +47,13 @@
       {
         "command_name": "shutdown",
         "command_description": "Shut down the stats httpd",
-        "command_args": []
+        "command_args": [
+          {
+            "item_name": "pid",
+            "item_type": "integer",
+            "item_optional": true
+          }
+        ]
       }
     ]
   }

src/bin/stats/stats.py.in

@@ -304,9 +304,11 @@ class Stats:
         return isc.config.create_answer(
             0, "Stats is up. (PID " + str(os.getpid()) + ")")
 
-    def command_shutdown(self):
+    def command_shutdown(self, pid=None):
         """
         handle shutdown command
+
+        The pid argument is ignored, it is here to match the signature.
         """
         logger.info(STATS_RECEIVED_SHUTDOWN_COMMAND)
         self.running = False

src/bin/stats/stats.spec

@@ -12,7 +12,13 @@
       {
         "command_name": "shutdown",
         "command_description": "Shut down the stats module",
-        "command_args": []
+        "command_args": [
+          {
+            "item_name": "pid",
+            "item_type": "integer",
+            "item_optional": true
+          }
+        ]
       },
       {
         "command_name": "show",

src/bin/stats/tests/b10-stats_test.py

@@ -202,15 +202,20 @@ class TestStats(unittest.TestCase):
         self.assertEqual(send_command("status", "Stats"),
                 (0, "Stats is up. (PID " + str(os.getpid()) + ")"))
         self.assertTrue(self.stats.running)
+        # Due to a race-condition related to the threads used in these
+        # tests, use of the mock session and the stopped check (see
+        # below), are temporarily disabled
+        # See ticket #1668
         # Override moduleCCSession so we can check if send_stopping is called
-        self.stats.mccs = MockModuleCCSession()
+        #self.stats.mccs = MockModuleCCSession()
         self.assertEqual(send_shutdown("Stats"), (0, None))
         self.assertFalse(self.stats.running)
         # Call server.shutdown with argument True so the thread.join() call
         # blocks and we are sure the main loop has finished (and set
         # mccs.stopped)
         self.stats_server.shutdown(True)
-        self.assertTrue(self.stats.mccs.stopped)
+        # Also temporarily disabled for #1668, see above
+        #self.assertTrue(self.stats.mccs.stopped)
 
         # start with err
         self.stats = stats.Stats()

src/bin/xfrin/xfrin.spec

@@ -86,7 +86,13 @@
       {
         "command_name": "shutdown",
         "command_description": "Shut down xfrin module",
-        "command_args": []
+        "command_args": [
+          {
+            "item_name": "pid",
+            "item_type": "integer",
+            "item_optional": true
+          }
+        ]
       }
     ]
   }

src/bin/xfrout/xfrout.spec.pre.in

@@ -106,7 +106,13 @@
         {
           "command_name": "shutdown",
           "command_description": "Shut down Xfrout",
-          "command_args": []
+          "command_args": [
+          {
+            "item_name": "pid",
+            "item_type": "integer",
+            "item_optional": true
+          }
+        ]
         }
       ]
   }

src/bin/zonemgr/zonemgr.spec.pre.in

@@ -63,7 +63,13 @@
         {
           "command_name": "shutdown",
           "command_description": "Shut down Zonemgr",
-          "command_args": []
+          "command_args": [
+          {
+            "item_name": "pid",
+            "item_type": "integer",
+            "item_optional": true
+          }
+        ]
         }
       ]
   }

src/lib/datasrc/datasrc_messages.mes

@@ -332,6 +332,30 @@ should be followed. The requested domain is an apex of some zone.
 % DATASRC_MEM_FIND find '%1/%2'
 Debug information. A search for the requested RRset is being started.
 
+% DATASRC_MEM_FINDNSEC3 finding NSEC3 for %1, mode %2
+Debug information. A search in an in-memory data source for NSEC3 that
+matches or covers the given name is being started.
+
+% DATASRC_MEM_FINDNSEC3_COVER found a covering NSEC3 for %1: %2
+Debug information. An NSEC3 that covers the given name is found and
+being returned.  The found NSEC3 RRset is also displayed.
+
+% DATASRC_MEM_FINDNSEC3_MATCH found a matching NSEC3 for %1 at label count %2: %3
+Debug information. An NSEC3 that matches (a possibly superdomain of)
+the given name is found and being returned.  When the shown label
+count is smaller than that of the given name, the matching NSEC3 is
+for a superdomain of the given name (see DATASRC_MEM_FINDNSEC3_TRYHASH).
+The found NSEC3 RRset is also displayed.
+
+% DATASRC_MEM_FINDNSEC3_TRYHASH looking for NSEC3 for %1 at label count %2 (hash %3)
+Debug information. In an attempt of finding an NSEC3 for the give name,
+(a possibly superdomain of) the name is hashed and searched for in the
+NSEC3 name space.  When the shown label count is smaller than that of the
+shown name, the search tries the superdomain name that share the shown
+(higher) label count of the shown name (e.g., for
+www.example.com. with shown label count of 3, example.com. is being
+tried).
+
 % DATASRC_MEM_FIND_ZONE looking for zone '%1'
 Debug information. A zone object for this zone is being searched for in the
 in-memory data source.

src/lib/datasrc/memory_datasrc.cc

@@ -881,57 +881,80 @@ InMemoryZoneFinder::findAll(const Name& name,
 }
 
 ZoneFinder::FindNSEC3Result
-InMemoryZoneFinder::findNSEC3(const Name&, bool) {
-    isc_throw(NotImplemented, "findNSEC3 is not yet implemented for in memory "
-              "data source");
-}
+InMemoryZoneFinder::findNSEC3(const Name& name, bool recursive) {
+    LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEM_FINDNSEC3).arg(name).
+        arg(recursive ? "recursive" : "non-recursive");
 
-ZoneFinder::FindNSEC3Result
-InMemoryZoneFinder::findNSEC3Tmp(const Name& name, bool recursive) {
     if (!impl_->zone_data_->nsec3_data_) {
-        isc_throw(Unexpected, "findNSEC3 is called for non NSEC3 zone");
+        isc_throw(DataSourceError,
+                  "findNSEC3 attempt for non NSEC3 signed zone: " <<
+                  impl_->origin_ << "/" << impl_->zone_class_);
     }
-    if (recursive) {
-        isc_throw(Unexpected, "recursive mode isn't expected in tests");
+    const NSEC3Map& map = impl_->zone_data_->nsec3_data_->map_;
+    if (map.empty()) {
+        isc_throw(DataSourceError,
+                  "findNSEC3 attempt but zone has no NSEC3 RR: " <<
+                  impl_->origin_ << "/" << impl_->zone_class_);
     }
-
-    // A temporary workaround for testing: convert the original name to
-    // NSEC3-hashed name using hardcoded mapping.
-    string hname_text;
-    if (name == Name("example.org")) {
-        hname_text = "0P9MHAVEQVM6T7VBL5LOP2U3T2RP3TOM";
-    } else if (name == Name("www.example.org")) {
-        hname_text = "2S9MHAVEQVM6T7VBL5LOP2U3T2RP3TOM";
-    } else if (name == Name("xxx.example.org")) {
-        hname_text = "Q09MHAVEQVM6T7VBL5LOP2U3T2RP3TOM";
-    } else if (name == Name("yyy.example.org")) {
-        hname_text = "0A9MHAVEQVM6T7VBL5LOP2U3T2RP3TOM";
-    } else {
-        isc_throw(Unexpected, "unexpected name for NSEC3 test: " << name);
+    const NameComparisonResult cmp_result = name.compare(impl_->origin_);
+    if (cmp_result.getRelation() != NameComparisonResult::EQUAL &&
+        cmp_result.getRelation() != NameComparisonResult::SUBDOMAIN) {
+        isc_throw(InvalidParameter, "findNSEC3 attempt for out-of-zone name: "
+                  << name << ", zone: " << impl_->origin_ << "/"
+                  << impl_->zone_class_);
     }
 
-    // Below we assume the map is not empty for simplicity.
-    NSEC3Map::const_iterator found =
-        impl_->zone_data_->nsec3_data_->map_.lower_bound(hname_text);
-    if (found != impl_->zone_data_->nsec3_data_->map_.end() &&
-        found->first == hname_text) {
-        // exact match
-        return (FindNSEC3Result(true, 2, found->second, ConstRRsetPtr()));
-    } else if (found == impl_->zone_data_->nsec3_data_->map_.end() ||
-               found == impl_->zone_data_->nsec3_data_->map_.begin()) {
-        // the search key is "smaller" than the smallest or "larger" than
-        // largest.  In either case "previous" is the largest one.
-        return (FindNSEC3Result(false, 2,
-                                impl_->zone_data_->nsec3_data_->map_.
-                                rbegin()->second, ConstRRsetPtr()));
-    } else {
-        // Otherwise, H(found_domain-1) < given_hash < H(found_domain)
-        // The covering proof is the first one.
-        return (FindNSEC3Result(false, 2, (--found)->second, ConstRRsetPtr()));
+    // Convenient shortcuts
+    const NSEC3Hash& nsec3hash = *impl_->zone_data_->nsec3_data_->hash_;
+    const unsigned int olabels = impl_->origin_.getLabelCount();
+    const unsigned int qlabels = name.getLabelCount();
+
+    ConstRRsetPtr covering_proof; // placeholder of the next closer proof
+    // Examine all names from the query name to the origin name, stripping
+    // the deepest label one by one, until we find a name that has a matching
+    // NSEC3 hash.
+    for (unsigned int labels = qlabels; labels >= olabels; --labels) {
+        const string hlabel = nsec3hash.calculate(
+            labels == qlabels ? name : name.split(qlabels - labels, labels));
+        NSEC3Map::const_iterator found = map.lower_bound(hlabel);
+        LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEM_FINDNSEC3_TRYHASH).
+            arg(name).arg(labels).arg(hlabel);
+
+        // If the given hash is larger than the largest stored hash or
+        // the first label doesn't match the target, identify the "previous"
+        // hash value and remember it as the candidate next closer proof.
+        if (found == map.end() || found->first != hlabel) {
+            // If the given hash is larger or smaller than everything,
+            // the covering proof is the NSEC3 that has the largest hash.
+            // Note that we know the map isn't empty, so rbegin() is
+            // safe.
+            if (found == map.end() || found == map.begin()) {
+                covering_proof = map.rbegin()->second;
+            } else {
+                // Otherwise, H(found_entry-1) < given_hash < H(found_entry).
+                // The covering proof is the first one (and it's valid
+                // because found is neither begin nor end)
+                covering_proof = (--found)->second;
+            }
+            if (!recursive) {   // in non recursive mode, we are done.
+                LOG_DEBUG(logger, DBG_TRACE_BASIC,
+                          DATASRC_MEM_FINDNSEC3_COVER).
+                    arg(name).arg(*covering_proof);
+                return (FindNSEC3Result(false, labels, covering_proof,
+                                        ConstRRsetPtr()));
+            }
+        } else {                // found an exact match.
+                LOG_DEBUG(logger, DBG_TRACE_BASIC,
+                          DATASRC_MEM_FINDNSEC3_MATCH).arg(name).arg(labels).
+                    arg(*found->second);
+            return (FindNSEC3Result(true, labels, found->second,
+                                    covering_proof));
+        }
     }
 
-    // We should have covered all cases.
-    isc_throw(Unexpected, "Impossible NSEC3 search result for " << name);
+    isc_throw(DataSourceError, "recursive findNSEC3 mode didn't stop, likely "
+              "a broken NSEC3 zone: " << impl_->origin_ << "/"
+              << impl_->zone_class_);
 }
 
 result::Result

src/lib/datasrc/memory_datasrc.h

@@ -89,16 +89,6 @@ public:
     virtual FindNSEC3Result
     findNSEC3(const isc::dns::Name& name, bool recursive);
 
-    // A temporary fake version of findNSEC3 for tests
-    //
-    // This method intentionally has the same interface as findNSEC3 but
-    // uses internally hardcoded hash values and offers a limited set
-    // of functionality for the convenience of tests.  This is a temporary
-    // workaround until #1577 is completed.  At that point this method
-    // should be removed.
-    FindNSEC3Result
-    findNSEC3Tmp(const isc::dns::Name& name, bool recursive);
-
     /// \brief Imelementation of the ZoneFinder::findPreviousName method
     ///
     /// This one throws NotImplemented exception, as InMemory doesn't

src/lib/datasrc/tests/Makefile.am

@@ -5,6 +5,7 @@ AM_CPPFLAGS += -I$(top_builddir)/src/lib/dns -I$(top_srcdir)/src/lib/dns
 AM_CPPFLAGS += $(BOOST_INCLUDES)
 AM_CPPFLAGS += $(SQLITE_CFLAGS)
 AM_CPPFLAGS += -DTEST_DATA_DIR=\"$(abs_srcdir)/testdata\"
+AM_CPPFLAGS += -DTEST_DATA_COMMONDIR=\"$(abs_top_srcdir)/src/lib/testutils/testdata\"
 AM_CPPFLAGS += -DTEST_DATA_BUILDDIR=\"$(abs_builddir)/testdata\"
 AM_CPPFLAGS += -DINSTALL_PROG=\"$(abs_top_srcdir)/install-sh\"
 

src/lib/datasrc/tests/memory_datasrc_unittest.cc

@@ -22,6 +22,7 @@
 
 #include <dns/masterload.h>
 #include <dns/name.h>
+#include <dns/nsec3hash.h>
 #include <dns/rdata.h>
 #include <dns/rdataclass.h>
 #include <dns/rrclass.h>
@@ -275,6 +276,83 @@ setRRset(RRsetPtr rrset, vector<RRsetPtr*>::iterator& it) {
     ++it;
 }
 
+ConstRRsetPtr
+textToRRset(const string& text_rrset, const RRClass& rrclass = RRClass::IN()) {
+    stringstream ss(text_rrset);
+    RRsetPtr rrset;
+    vector<RRsetPtr*> rrsets;
+    rrsets.push_back(&rrset);
+    masterLoad(ss, Name::ROOT_NAME(), rrclass,
+               boost::bind(setRRset, _1, rrsets.begin()));
+    return (rrset);
+}
+
+// Some faked NSEC3 hash values commonly used in tests and the faked NSEC3Hash
+// object.
+//
+// For apex (example.org)
+const char* const apex_hash = "0P9MHAVEQVM6T7VBL5LOP2U3T2RP3TOM";
+const char* const apex_hash_lower = "0p9mhaveqvm6t7vbl5lop2u3t2rp3tom";
+// For ns1.example.org
+const char* const ns1_hash = "2T7B4G4VSA5SMI47K61MV5BV1A22BOJR";
+// For w.example.org
+const char* const w_hash = "01UDEMVP1J2F7EG6JEBPS17VP3N8I58H";
+// For x.y.w.example.org (lower-cased)
+const char* const xyw_hash = "2vptu5timamqttgl4luu9kg21e0aor3s";
+// For zzz.example.org.
+const char* const zzz_hash = "R53BQ7CC2UVMUBFU5OCMM6PERS9TK9EN";
+
+// A simple faked NSEC3 hash calculator with a dedicated creator for it.
+//
+// This is used in some NSEC3-related tests below.
+class TestNSEC3HashCreator : public NSEC3HashCreator {
+    class TestNSEC3Hash : public NSEC3Hash {
+    private:
+        typedef map<Name, string> NSEC3HashMap;
+        typedef NSEC3HashMap::value_type NSEC3HashPair;
+        NSEC3HashMap map_;
+    public:
+        TestNSEC3Hash() {
+            // Build pre-defined hash
+            map_[Name("example.org")] = apex_hash;
+            map_[Name("www.example.org")] = "2S9MHAVEQVM6T7VBL5LOP2U3T2RP3TOM";
+            map_[Name("xxx.example.org")] = "Q09MHAVEQVM6T7VBL5LOP2U3T2RP3TOM";
+            map_[Name("yyy.example.org")] = "0A9MHAVEQVM6T7VBL5LOP2U3T2RP3TOM";
+            map_[Name("x.y.w.example.org")] =
+                "2VPTU5TIMAMQTTGL4LUU9KG21E0AOR3S";
+            map_[Name("y.w.example.org")] = "K8UDEMVP1J2F7EG6JEBPS17VP3N8I58H";
+            map_[Name("w.example.org")] = w_hash;
+            map_[Name("zzz.example.org")] = zzz_hash;
+            map_[Name("smallest.example.org")] =
+                "00000000000000000000000000000000";
+            map_[Name("largest.example.org")] =
+                "UUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUU";
+        }
+        virtual string calculate(const Name& name) const {
+            const NSEC3HashMap::const_iterator found = map_.find(name);
+            if (found != map_.end()) {
+                return (found->second);
+            }
+            isc_throw(isc::Unexpected, "unexpected name for NSEC3 test: "
+                      << name);
+        }
+        virtual bool match(const generic::NSEC3PARAM&) const {
+            return (true);
+        }
+        virtual bool match(const generic::NSEC3&) const {
+            return (true);
+        }
+    };
+
+public:
+    virtual NSEC3Hash* create(const generic::NSEC3PARAM&) const {
+        return (new TestNSEC3Hash);
+    }
+    virtual NSEC3Hash* create(const generic::NSEC3&) const {
+        return (new TestNSEC3Hash);
+    }
+};
+
 /// \brief Test fixture for the InMemoryZoneFinder class
 class InMemoryZoneFinderTest : public ::testing::Test {
     // A straightforward pair of textual RR(set) and a RRsetPtr variable
@@ -368,6 +446,12 @@ public:
         masterLoad(zone_data_stream, Name::ROOT_NAME(), class_,
                    boost::bind(setRRset, _1, rrsets.begin()));
     }
+
+    ~InMemoryZoneFinderTest() {
+        // Make sure we reset the hash creator to the default
+        setNSEC3HashCreator(NULL);
+    }
+
     // Some data to test with
     const RRClass class_;
     const Name origin_;
@@ -414,6 +498,12 @@ public:
     RRsetPtr rr_not_wild_another_;
     RRsetPtr rr_nsec3_;
 
+    // A faked NSEC3 hash calculator for convenience.
+    // Tests that need to use the faked hashed values should call
+    // setNSEC3HashCreator() with a pointer to this variable at the beginning
+    // of the test (at least before adding any NSEC3/NSEC3PARAM RR).
+    TestNSEC3HashCreator nsec3_hash_creator_;
+
     /**
      * \brief Test one find query to the zone finder.
      *
@@ -522,18 +612,6 @@ public:
         rrsetsCheck(expected_rrsets.begin(), expected_rrsets.end(),
                     target.begin(), target.end());
     }
-
-    ConstRRsetPtr textToRRset(const string& text_rrset,
-                              const RRClass& rrclass = RRClass::IN()) const
-    {
-        stringstream ss(text_rrset);
-        RRsetPtr rrset;
-        vector<RRsetPtr*> rrsets;
-        rrsets.push_back(&rrset);
-        masterLoad(ss, Name::ROOT_NAME(), rrclass,
-                   boost::bind(setRRset, _1, rrsets.begin()));
-        return (rrset);
-    }
 };
 
 /**
@@ -895,7 +973,7 @@ TEST_F(InMemoryZoneFinderTest, find) {
     findCheck();
 }
 
-TEST_F(InMemoryZoneFinderTest, findNSEC3) {
+TEST_F(InMemoryZoneFinderTest, findNSEC3Signed) {
     findCheck(ZoneFinder::RESULT_NSEC3_SIGNED);
 }
 
@@ -1518,40 +1596,45 @@ const char* const nsec3_common = " 300 IN NSEC3 1 1 12 aabbccdd "
 const char* const nsec3_rrsig_common = " 300 IN RRSIG NSEC3 5 3 3600 "
     "20000101000000 20000201000000 12345 example.org. FAKEFAKEFAKE";
 
-// For apex (example.org)
-const char* const apex_hash = "0P9MHAVEQVM6T7VBL5LOP2U3T2RP3TOM";
-const char* const apex_hash_lower = "0p9mhaveqvm6t7vbl5lop2u3t2rp3tom";
-// For ns1.example.org
-const char* const ns1_hash = "2T7B4G4VSA5SMI47K61MV5BV1A22BOJR";
-// For x.y.w.example.org (lower-cased)
-const char* const xrw_hash = "2vptu5timamqttgl4luu9kg21e0aor3s";
-
 void
-nsec3Check(bool expected_matched, const string& expected_rrsets_txt,
-           const ZoneFinder::FindNSEC3Result& result,
-           bool expected_sig = false)
+findNSEC3Check(bool expected_matched, uint8_t expected_labels,
+               const string& expected_closest,
+               const string& expected_next,
+               const ZoneFinder::FindNSEC3Result& result,
+               bool expected_sig = false)
 {
-    vector<ConstRRsetPtr> actual_rrsets;
     EXPECT_EQ(expected_matched, result.matched);
+    // Convert to int so the error messages would be more readable:
+    EXPECT_EQ(static_cast<int>(expected_labels),
+              static_cast<int>(result.closest_labels));
+
+    vector<ConstRRsetPtr> actual_rrsets;
     ASSERT_TRUE(result.closest_proof);
-    if (expected_sig) {
-        ASSERT_TRUE(result.closest_proof->getRRsig());
-    }
     actual_rrsets.push_back(result.closest_proof);
     if (expected_sig) {
         actual_rrsets.push_back(result.closest_proof->getRRsig());
     }
-    rrsetsCheck(expected_rrsets_txt, actual_rrsets.begin(),
+    rrsetsCheck(expected_closest, actual_rrsets.begin(),
                 actual_rrsets.end());
-}
 
-// In the following tests we use a temporary faked version of findNSEC3
-// as the real version isn't implemented yet (it's a task for #1577).
-// When #1577 is done the tests should be updated using the real version.
-// If we can use fake hash calculator (see #1575), we should be able to
-// just replace findNSEC3Tmp with findNSEC3.
+    actual_rrsets.clear();
+    if (expected_next.empty()) {
+        EXPECT_FALSE(result.next_proof);
+    } else {
+        ASSERT_TRUE(result.next_proof);
+        actual_rrsets.push_back(result.next_proof);
+        if (expected_sig) {
+            actual_rrsets.push_back(result.next_proof->getRRsig());
+        }
+        rrsetsCheck(expected_next, actual_rrsets.begin(),
+                    actual_rrsets.end());
+    }
+}
 
 TEST_F(InMemoryZoneFinderTest, addNSEC3) {
+    // Set up the faked hash calculator.
+    setNSEC3HashCreator(&nsec3_hash_creator_);
+
     const string nsec3_text = string(apex_hash) + ".example.org." +
         string(nsec3_common);
     // This name shouldn't be found in the normal domain tree.
@@ -1560,8 +1643,8 @@ TEST_F(InMemoryZoneFinderTest, addNSEC3) {
               zone_finder_.find(Name(string(apex_hash) + ".example.org"),
                                 RRType::NSEC3()).code);
     // Dedicated NSEC3 find should be able to find it.
-    nsec3Check(true, nsec3_text,
-               zone_finder_.findNSEC3Tmp(Name("example.org"), false));
+    findNSEC3Check(true, origin_.getLabelCount(), nsec3_text, "",
+                   zone_finder_.findNSEC3(Name("example.org"), false));
 
     // This implementation rejects duplicate/update add of the same hash name
     EXPECT_EQ(result::EXIST,
@@ -1569,8 +1652,8 @@ TEST_F(InMemoryZoneFinderTest, addNSEC3) {
                                    string(apex_hash) + ".example.org." +
                                    string(nsec3_common) + " AAAA")));
     // The original NSEC3 should be intact
-    nsec3Check(true, nsec3_text,
-               zone_finder_.findNSEC3Tmp(Name("example.org"), false));
+    findNSEC3Check(true, origin_.getLabelCount(), nsec3_text, "",
+                   zone_finder_.findNSEC3(Name("example.org"), false));
 
     // NSEC3-like name but of ordinary RR type should go to normal tree.
     const string nonsec3_text = string(apex_hash) + ".example.org. " +
@@ -1582,15 +1665,21 @@ TEST_F(InMemoryZoneFinderTest, addNSEC3) {
 }
 
 TEST_F(InMemoryZoneFinderTest, addNSEC3Lower) {
+    // Set up the faked hash calculator.
+    setNSEC3HashCreator(&nsec3_hash_creator_);
+
     // Similar to the previous case, but NSEC3 owner name is lower-cased.
     const string nsec3_text = string(apex_hash_lower) + ".example.org." +
         string(nsec3_common);
     EXPECT_EQ(result::SUCCESS, zone_finder_.add(textToRRset(nsec3_text)));
-    nsec3Check(true, nsec3_text,
-               zone_finder_.findNSEC3Tmp(Name("example.org"), false));
+    findNSEC3Check(true, origin_.getLabelCount(), nsec3_text, "",
+                   zone_finder_.findNSEC3(Name("example.org"), false));
 }
 
 TEST_F(InMemoryZoneFinderTest, addNSEC3Ordering) {
+    // Set up the faked hash calculator.
+    setNSEC3HashCreator(&nsec3_hash_creator_);
+
     // Check that the internal storage ensures comparison based on the NSEC3
     // semantics, regardless of the add order or the letter-case of hash.
 
@@ -1599,7 +1688,7 @@ TEST_F(InMemoryZoneFinderTest, addNSEC3Ordering) {
         string(nsec3_common);
     const string middle = string(ns1_hash) + ".example.org." +
         string(nsec3_common);
-    const string largest = string(xrw_hash) + ".example.org." +
+    const string largest = string(xyw_hash) + ".example.org." +
         string(nsec3_common);
     zone_finder_.add(textToRRset(smallest));
     zone_finder_.add(textToRRset(largest));
@@ -1607,15 +1696,15 @@ TEST_F(InMemoryZoneFinderTest, addNSEC3Ordering) {
 
     // Then look for NSEC3 that covers a name whose hash is "2S.."
     // The covering NSEC3 should be "0P.."
-    nsec3Check(false, smallest,
-               zone_finder_.findNSEC3Tmp(Name("www.example.org"), false));
+    findNSEC3Check(false, 4, smallest, "",
+                   zone_finder_.findNSEC3(Name("www.example.org"), false));
 
     // Look for NSEC3 that covers names whose hash are "Q0.." and "0A.."
     // The covering NSEC3 should be "2v.." in both cases
-    nsec3Check(false, largest,
-               zone_finder_.findNSEC3Tmp(Name("xxx.example.org"), false));
-    nsec3Check(false, largest,
-               zone_finder_.findNSEC3Tmp(Name("yyy.example.org"), false));
+    findNSEC3Check(false, 4, largest, "",
+                   zone_finder_.findNSEC3(Name("xxx.example.org"), false));
+    findNSEC3Check(false, 4, largest, "",
+                   zone_finder_.findNSEC3(Name("yyy.example.org"), false));
 }
 
 TEST_F(InMemoryZoneFinderTest, badNSEC3Name) {
@@ -1643,6 +1732,9 @@ TEST_F(InMemoryZoneFinderTest, addMultiNSEC3) {
 }
 
 TEST_F(InMemoryZoneFinderTest, addNSEC3WithRRSIG) {
+    // Set up the faked hash calculator.
+    setNSEC3HashCreator(&nsec3_hash_creator_);
+
     // Adding NSEC3 and its RRSIG
     const string nsec3_text = string(apex_hash) + ".example.org." +
         string(nsec3_common);
@@ -1652,8 +1744,9 @@ TEST_F(InMemoryZoneFinderTest, addNSEC3WithRRSIG) {
     EXPECT_EQ(result::SUCCESS, zone_finder_.add(textToRRset(nsec3_rrsig_text)));
 
     // Then look for it.  The NSEC3 should have the RRSIG that was just added.
-    nsec3Check(true, nsec3_text + "\n" + nsec3_rrsig_text,
-               zone_finder_.findNSEC3Tmp(Name("example.org"), false), true);
+    findNSEC3Check(true, origin_.getLabelCount(),
+                   nsec3_text + "\n" + nsec3_rrsig_text, "",
+                   zone_finder_.findNSEC3(Name("example.org"), false), true);
 
     // Duplicate add of RRSIG for the same NSEC3 is prohibited.
     EXPECT_THROW(zone_finder_.add(textToRRset(nsec3_rrsig_text)),
@@ -1804,4 +1897,162 @@ TEST_F(InMemoryZoneFinderTest, queryToNSEC3NameNonterminal) {
              ZoneFinder::FIND_DNSSEC);
 }
 
+TEST_F(InMemoryZoneFinderTest, findNSEC3) {
+    // Set up the faked hash calculator.
+    setNSEC3HashCreator(&nsec3_hash_creator_);
+
+    // Add a few NSEC3 records:
+    // apex (example.org.): hash=0P..
+    // ns1.example.org:     hash=2T..
+    // w.example.org:       hash=01..
+    // zzz.example.org:     hash=R5..
+    const string apex_nsec3_text = string(apex_hash) + ".example.org." +
+        string(nsec3_common);
+    EXPECT_EQ(result::SUCCESS, zone_finder_.add(textToRRset(apex_nsec3_text)));
+    const string ns1_nsec3_text = string(ns1_hash) + ".example.org." +
+        string(nsec3_common);
+    EXPECT_EQ(result::SUCCESS, zone_finder_.add(textToRRset(ns1_nsec3_text)));
+    const string w_nsec3_text = string(w_hash) + ".example.org." +
+        string(nsec3_common);
+    EXPECT_EQ(result::SUCCESS, zone_finder_.add(textToRRset(w_nsec3_text)));
+    const string zzz_nsec3_text = string(zzz_hash) + ".example.org." +
+        string(nsec3_common);
+    EXPECT_EQ(result::SUCCESS, zone_finder_.add(textToRRset(zzz_nsec3_text)));
+
+    // Parameter validation: the query name must be in or below the zone
+    EXPECT_THROW(zone_finder_.findNSEC3(Name("example.com"), false),
+                 isc::InvalidParameter);
+    EXPECT_THROW(zone_finder_.findNSEC3(Name("org"), true),
+                 isc::InvalidParameter);
+
+    // Apex name.  It should have a matching NSEC3.
+    {
+        SCOPED_TRACE("apex, non recursive mode");
+        findNSEC3Check(true, origin_.getLabelCount(), apex_nsec3_text, "",
+                       zone_finder_.findNSEC3(origin_, false));
+    }
+
+    // Recursive mode doesn't change the result in this case.
+    {
+        SCOPED_TRACE("apex, recursive mode");
+        findNSEC3Check(true, origin_.getLabelCount(), apex_nsec3_text, "",
+                       zone_finder_.findNSEC3(origin_, true));
+    }
+
+    // Non existent name.  Disabling recursion, a covering NSEC3 should be
+    // returned.
+    const Name www_name("www.example.org");
+    {
+        SCOPED_TRACE("non existent name, non recursive mode");
+        findNSEC3Check(false, www_name.getLabelCount(), apex_nsec3_text, "",
+                       zone_finder_.findNSEC3(www_name, false));
+    }
+
+    // Non existent name.  The closest provable encloser is the apex,
+    // and next closer is the query name itself (which NSEC3 for ns1
+    // covers)
+    // H(ns1) = 2T... < H(xxx) = Q0... < H(zzz) = R5...
+    {
+        SCOPED_TRACE("non existent name, recursive mode");
+        findNSEC3Check(true, origin_.getLabelCount(), apex_nsec3_text,
+                       ns1_nsec3_text,
+                       zone_finder_.findNSEC3(Name("xxx.example.org"), true));
+    }
+
+    // Similar to the previous case, but next closer name is different
+    // from the query name.  The closet encloser is w.example.org, and
+    // next closer is y.w.example.org.
+    // H(ns1) = 2T.. < H(y.w) = K8.. < H(zzz) = R5
+    {
+        SCOPED_TRACE("non existent name, non qname next closer");
+        findNSEC3Check(true, Name("w.example.org").getLabelCount(),
+                       w_nsec3_text, ns1_nsec3_text,
+                       zone_finder_.findNSEC3(Name("x.y.w.example.org"),
+                                              true));
+    }
+
+    // In the rest of test we check hash comparison for wrap around cases.
+    {
+        SCOPED_TRACE("very small hash");
+        const Name smallest_name("smallest.example.org");
+        findNSEC3Check(false, smallest_name.getLabelCount(),
+                       zzz_nsec3_text, "",
+                       zone_finder_.findNSEC3(smallest_name, false));
+    }
+    {
+        SCOPED_TRACE("very large hash");
+        const Name largest_name("largest.example.org");
+        findNSEC3Check(false, largest_name.getLabelCount(),
+                       zzz_nsec3_text, "",
+                       zone_finder_.findNSEC3(largest_name, false));
+    }
+}
+
+TEST_F(InMemoryZoneFinderTest, findNSEC3ForBadZone) {
+    // Set up the faked hash calculator.
+    setNSEC3HashCreator(&nsec3_hash_creator_);
+
+    // If the zone has nothing about NSEC3 (neither NSEC3 or NSEC3PARAM),
+    // findNSEC3() should be rejected.
+    EXPECT_THROW(zone_finder_.findNSEC3(Name("www.example.org"), true),
+                 DataSourceError);
+
+    // Only having NSEC3PARAM isn't enough
+    EXPECT_EQ(result::SUCCESS,
+              zone_finder_.add(textToRRset("example.org. 300 IN NSEC3PARAM "
+                                           "1 0 12 aabbccdd")));
+    EXPECT_THROW(zone_finder_.findNSEC3(Name("www.example.org"), true),
+                 DataSourceError);
+
+    // Unless NSEC3 for apex is added the result in the recursive mode
+    // is guaranteed.
+    const string ns1_nsec3_text = string(ns1_hash) + ".example.org." +
+        string(nsec3_common);
+    EXPECT_EQ(result::SUCCESS, zone_finder_.add(textToRRset(ns1_nsec3_text)));
+    EXPECT_THROW(zone_finder_.findNSEC3(Name("www.example.org"), true),
+                 DataSourceError);
+}
+
+TEST_F(InMemoryZoneFinderTest, loadAndFindNSEC3) {
+    // Using more realistic example, borrowed from RFC5155, with compliant
+    // hash calculator.  We only confirm the data source can load it
+    // successfully and find correct NSEC3 RRs for some selected cases
+    // (detailed tests have been done above).
+
+    InMemoryZoneFinder finder(class_, Name("example"));
+    finder.load(TEST_DATA_COMMONDIR "/rfc5155-example.zone.signed");
+
+    // See RFC5155 B.1
+    ZoneFinder::FindNSEC3Result result1 =
+        finder.findNSEC3(Name("c.x.w.example"), true);
+    ASSERT_TRUE(result1.closest_proof);
+    // We compare closest_labels as int so the error report will be more
+    // readable in case it fails.
+    EXPECT_EQ(4, static_cast<int>(result1.closest_labels));
+    EXPECT_EQ(Name("b4um86eghhds6nea196smvmlo4ors995.example"),
+              result1.closest_proof->getName());
+    ASSERT_TRUE(result1.next_proof);
+    EXPECT_EQ(Name("0p9mhaveqvm6t7vbl5lop2u3t2rp3tom.example"),
+              result1.next_proof->getName());
+
+    // See RFC5155 B.2.
+    ZoneFinder::FindNSEC3Result result2 =
+        finder.findNSEC3(Name("ns1.example"), true);
+    ASSERT_TRUE(result2.closest_proof);
+    EXPECT_EQ(3, static_cast<int>(result2.closest_labels));
+    EXPECT_EQ(Name("2t7b4g4vsa5smi47k61mv5bv1a22bojr.example"),
+              result2.closest_proof->getName());
+    ASSERT_FALSE(result2.next_proof);
+
+    // See RFC5155 B.5.
+    ZoneFinder::FindNSEC3Result result3 =
+        finder.findNSEC3(Name("a.z.w.example"), true);
+    ASSERT_TRUE(result3.closest_proof);
+    EXPECT_EQ(3, static_cast<int>(result3.closest_labels));
+    EXPECT_EQ(Name("k8udemvp1j2f7eg6jebps17vp3n8i58h.example"),
+              result3.closest_proof->getName());
+    ASSERT_TRUE(result3.next_proof);
+    EXPECT_EQ(Name("q04jkcevqvmu85r014c7dkba38o0ji5r.example"),
+              result3.next_proof->getName());
+}
 }

src/lib/dns/Makefile.am

@@ -23,6 +23,8 @@ EXTRA_DIST += rdata/generic/cname_5.cc
 EXTRA_DIST += rdata/generic/cname_5.h
 EXTRA_DIST += rdata/generic/detail/nsec_bitmap.cc
 EXTRA_DIST += rdata/generic/detail/nsec_bitmap.h
+EXTRA_DIST += rdata/generic/detail/nsec3param_common.cc
+EXTRA_DIST += rdata/generic/detail/nsec3param_common.h
 EXTRA_DIST += rdata/generic/detail/txt_like.h
 EXTRA_DIST += rdata/generic/detail/ds_like.h
 EXTRA_DIST += rdata/generic/dlv_32769.cc
@@ -113,6 +115,8 @@ libdns___la_SOURCES += tsigrecord.h tsigrecord.cc
 libdns___la_SOURCES += character_string.h character_string.cc
 libdns___la_SOURCES += rdata/generic/detail/nsec_bitmap.h
 libdns___la_SOURCES += rdata/generic/detail/nsec_bitmap.cc
+libdns___la_SOURCES += rdata/generic/detail/nsec3param_common.cc
+libdns___la_SOURCES += rdata/generic/detail/nsec3param_common.h
 libdns___la_SOURCES += rdata/generic/detail/txt_like.h
 libdns___la_SOURCES += rdata/generic/detail/ds_like.h
 

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/nsec3hash.cc

@@ -142,6 +142,23 @@ NSEC3HashRFC5155::match(const generic::NSEC3PARAM& nsec3param) const {
     return (match(nsec3param.getHashalg(), nsec3param.getIterations(),
                   nsec3param.getSalt()));
 }
+
+// A static pointer that refers to the currently usable creator.
+// Only get/setNSEC3HashCreator are expected to get access to this variable
+// directly.
+const NSEC3HashCreator* creator;
+
+// The accessor to the current creator.  If it's not explicitly set or has
+// been reset from a customized one, the default creator will be used.
+const NSEC3HashCreator*
+getNSEC3HashCreator() {
+    static DefaultNSEC3HashCreator default_creator;
+    if (creator == NULL) {
+        creator = &default_creator;
+    }
+    return (creator);
+}
+
 } // end of unnamed namespace
 
 namespace isc {
@@ -149,15 +166,30 @@ namespace dns {
 
 NSEC3Hash*
 NSEC3Hash::create(const generic::NSEC3PARAM& param) {
+    return (getNSEC3HashCreator()->create(param));
+}
+
+NSEC3Hash*
+NSEC3Hash::create(const generic::NSEC3& nsec3) {
+    return (getNSEC3HashCreator()->create(nsec3));
+}
+
+NSEC3Hash*
+DefaultNSEC3HashCreator::create(const generic::NSEC3PARAM& param) const {
     return (new NSEC3HashRFC5155(param.getHashalg(), param.getIterations(),
                                  param.getSalt()));
 }
 
 NSEC3Hash*
-NSEC3Hash::create(const generic::NSEC3& nsec3) {
+DefaultNSEC3HashCreator::create(const generic::NSEC3& nsec3) const {
     return (new NSEC3HashRFC5155(nsec3.getHashalg(), nsec3.getIterations(),
                                  nsec3.getSalt()));
 }
 
+void
+setNSEC3HashCreator(const NSEC3HashCreator* new_creator) {
+    creator = new_creator;
+}
+
 } // namespace dns
 } // namespace isc

src/lib/dns/nsec3hash.h

@@ -154,6 +154,96 @@ public:
     virtual bool match(const rdata::generic::NSEC3PARAM& nsec3param) const = 0;
 };
 
+/// \brief Factory class of NSEC3Hash.
+///
+/// This class is an abstract base class that provides the creation interfaces
+/// of \c NSEC3Hash objects.  By defining a specific derived class of the
+/// creator, normally with a different specific class of \c NSEC3Hash,
+/// the application can use a customized implementation of \c NSEC3Hash
+/// without changing the library itself.  The intended primary application of
+/// such customization is tests (it would be convenient for a test to produce
+/// a faked hash value regardless of the input so it doesn't have to identify
+/// a specific input value to produce a particular hash).  Another possibility
+/// would be an experimental extension for a newer hash algorithm or
+/// implementation.
+///
+/// The two main methods named \c create() correspond to the static factory
+/// methods of \c NSEC3Hash of the same name.
+///
+/// By default, the library uses the \c DefaultNSEC3HashCreator creator.
+/// The \c setNSEC3HashCreator() function can be used to replace it with a
+/// user defined version.  For such customization purposes as implementing
+/// experimental new hash algorithms, the application may internally want to
+/// use the \c DefaultNSEC3HashCreator in general cases while creating a
+/// customized type of \c NSEC3Hash object for that particular hash algorithm.
+///
+/// The creator objects are generally expected to be stateless; they will
+/// only encapsulate the factory logic.  The \c create() methods are declared
+/// as const member functions for this reason.  But if we see the need for
+/// having a customized creator that benefits from its own state in future,
+/// this condition can be loosened.
+class NSEC3HashCreator {
+protected:
+    /// \brief The default constructor.
+    ///
+    /// Make very sure this isn't directly instantiated by making it protected
+    /// even if this class is modified to lose all pure virtual methods.
+    NSEC3HashCreator() {}
+
+public:
+    /// \brief The destructor.
+    ///
+    /// This does nothing; defined only for allowing derived classes to
+    /// specialize its behavior.
+    virtual ~NSEC3HashCreator() {}
+
+    /// \brief Factory method of NSECHash from NSEC3PARAM RDATA.
+    ///
+    /// See
+    /// <code>NSEC3Hash::create(const rdata::generic::NSEC3PARAM& param)</code>
+    virtual NSEC3Hash* create(const rdata::generic::NSEC3PARAM& nsec3param)
+        const = 0;
+
+    /// \brief Factory method of NSECHash from NSEC3 RDATA.
+    ///
+    /// See
+    /// <code>NSEC3Hash::create(const rdata::generic::NSEC3& param)</code>
+    virtual NSEC3Hash* create(const rdata::generic::NSEC3& nsec3)
+        const = 0;
+};
+
+/// \brief The default NSEC3Hash creator.
+///
+/// This derived class implements the \c NSEC3HashCreator interfaces for
+/// the standard NSEC3 hash calculator as defined in RFC5155.  The library
+/// will use this creator by default, so normal applications don't have to
+/// be aware of this class at all.  This class is publicly visible for the
+/// convenience of special applications that want to customize the creator
+/// behavior for a particular type of parameters while preserving the default
+/// behavior for others.
+class DefaultNSEC3HashCreator : public NSEC3HashCreator {
+public:
+    virtual NSEC3Hash* create(const rdata::generic::NSEC3PARAM& param) const;
+    virtual NSEC3Hash* create(const rdata::generic::NSEC3& nsec3) const;
+};
+
+/// \brief The registrar of \c NSEC3HashCreator.
+///
+/// This function sets or resets the system-wide \c NSEC3HashCreator that
+/// is used by \c NSEC3Hash::create().
+///
+/// If \c new_creator is non NULL, the given creator object will replace
+/// any existing creator.  If it's NULL, the default builtin creator will be
+/// used again from that point.
+///
+/// When \c new_creator is non NULL, the caller is responsible for keeping
+/// the referenced object valid as long as it can be used via
+/// \c NSEC3Hash::create().
+///
+/// \exception None
+/// \param new_creator A pointer to the new creator object or NULL.
+void setNSEC3HashCreator(const NSEC3HashCreator* new_creator);
+
 }
 }
 #endif  // __NSEC3HASH_H

src/lib/dns/rdata/generic/detail/nsec3param_common.cc

@@ -0,0 +1,130 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <exceptions/exceptions.h>
+
+#include <util/encode/hex.h>
+#include <util/buffer.h>
+
+#include <dns/exceptions.h>
+#include <dns/rdata.h>
+#include <dns/rdata/generic/detail/nsec3param_common.h>
+
+#include <boost/lexical_cast.hpp>
+
+#include <sstream>
+#include <vector>
+#include <stdint.h>
+
+using namespace std;
+using namespace isc::util;
+using namespace isc::util::encode;
+
+namespace isc {
+namespace dns {
+namespace rdata {
+namespace generic {
+namespace detail {
+namespace nsec3 {
+
+ParseNSEC3ParamResult
+parseNSEC3ParamText(const char* const rrtype_name,
+                    const string& rdata_str, istringstream& iss,
+                    vector<uint8_t>& salt)
+{
+    unsigned int hashalg, flags, iterations;
+    string iterations_str, salthex;
+
+    iss >> hashalg >> flags >> iterations_str >> salthex;
+    if (iss.bad() || iss.fail()) {
+        isc_throw(InvalidRdataText, "Invalid " << rrtype_name <<
+                  " text: " << rdata_str);
+    }
+    if (hashalg > 0xff) {
+        isc_throw(InvalidRdataText, rrtype_name <<
+                  " hash algorithm out of range: " << hashalg);
+    }
+    if (flags > 0xff) {
+        isc_throw(InvalidRdataText, rrtype_name << " flags out of range: " <<
+                  flags);
+    }
+    // Convert iteration.  To reject an invalid case where there's no space
+    // between iteration and salt, we extract this field as string and convert
+    // to integer.
+    try {
+        iterations = boost::lexical_cast<unsigned int>(iterations_str);
+    } catch (const boost::bad_lexical_cast&) {
+        isc_throw(InvalidRdataText, "Bad " << rrtype_name <<
+                  " iteration: " << iterations_str);
+    }
+    if (iterations > 0xffff) {
+        isc_throw(InvalidRdataText, rrtype_name <<
+                  " iterations out of range: " <<
+            iterations);
+    }
+
+    // Salt is up to 255 bytes, and space is not allowed in the HEX encoding,
+    // so the encoded string cannot be longer than the double of max length
+    // of the actual salt.
+    if (salthex.size() > 255 * 2) {
+        isc_throw(InvalidRdataText, rrtype_name << " salt is too long: "
+                  << salthex.size() << " (encoded) bytes");
+    }
+    if (salthex != "-") {       // "-" means a 0-length salt
+        decodeHex(salthex, salt);
+    }
+
+    return (ParseNSEC3ParamResult(hashalg, flags, iterations));
+}
+
+ParseNSEC3ParamResult
+parseNSEC3ParamWire(const char* const rrtype_name,
+                    InputBuffer& buffer,
+                    size_t& rdata_len, std::vector<uint8_t>& salt)
+{
+    // NSEC3/NSEC3PARAM RR must have at least 5 octets:
+    // hash algorithm(1), flags(1), iteration(2), saltlen(1)
+    if (rdata_len < 5) {
+        isc_throw(DNSMessageFORMERR, rrtype_name << " too short, length: "
+                  << rdata_len);
+    }
+
+    const uint8_t hashalg = buffer.readUint8();
+    const uint8_t flags = buffer.readUint8();
+    const uint16_t iterations = buffer.readUint16();
+
+    const uint8_t saltlen = buffer.readUint8();
+    rdata_len -= 5;
+    if (rdata_len < saltlen) {
+        isc_throw(DNSMessageFORMERR, rrtype_name <<
+                  " salt length is too large: " <<
+                  static_cast<unsigned int>(saltlen));
+    }
+
+    salt.resize(saltlen);
+    if (saltlen > 0) {
+        buffer.readData(&salt[0], saltlen);
+        rdata_len -= saltlen;
+    }
+
+    return (ParseNSEC3ParamResult(hashalg, flags, iterations));
+}
+
+} // end of nsec3
+} // end of detail
+} // end of generic
+} // end of rdata
+} // end of dns
+} // end of isc
+

src/lib/dns/rdata/generic/detail/nsec3param_common.h

@@ -0,0 +1,134 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef __NSEC3PARAM_COMMON_H
+#define __NSEC3PARAM_COMMON_H 1
+
+#include <util/buffer.h>
+
+#include <stdint.h>
+
+#include <sstream>
+#include <string>
+#include <vector>
+
+namespace isc {
+namespace dns {
+namespace rdata {
+namespace generic {
+namespace detail {
+namespace nsec3 {
+
+/// \file
+///
+/// This helper module provides some utilities that handle NSEC3 and
+/// NSEC3PARAM RDATA.  They share the first few fields, and some operations
+/// on these fields are sufficiently complicated, so it would make sense to
+/// consolidate the processing logic into a single implementation module.
+///
+/// The functions defined here are essentially private and are only expected
+/// to be called from the \c NSEC3 and \c NSEC3PARAM class implementations.
+
+/// \brief Result values of the utilities.
+///
+/// This structure encapsulates a tuple of NSEC3/NSEC3PARAM algorithm,
+/// flags and iterations field values.  This is used as the return value
+/// of the utility functions defined in this module so the caller can
+/// use it for constructing the corresponding RDATA.
+struct ParseNSEC3ParamResult {
+    ParseNSEC3ParamResult(uint8_t param_algorithm, uint8_t param_flags,
+                          uint16_t param_iterations) :
+        algorithm(param_algorithm), flags(param_flags),
+        iterations(param_iterations)
+    {}
+    const uint8_t algorithm;
+    const uint8_t flags;
+    const uint16_t iterations;
+};
+
+/// \brief Convert textual representation of NSEC3 parameters.
+///
+/// This function takes an input string stream that consists of a complete
+/// textual representation of an NSEC3 or NSEC3PARAM RDATA and parses it
+/// extracting the hash algorithm, flags, iterations, and salt fields.
+///
+/// The first three fields are returned as the return value of this function.
+/// The salt will be stored in the given vector.  The vector is expected
+/// to be empty, but if not, the existing content will be overridden.
+///
+/// On successful return the given input stream will reach the end of the
+/// salt field.
+///
+/// \exception isc::BadValue The salt is not a valid hex string.
+/// \exception InvalidRdataText The given string is otherwise invalid for
+/// NSEC3 or NSEC3PARAM fields.
+///
+/// \param rrtype_name Either "NSEC3" or "NSEC3PARAM"; used as part of
+/// exception messages.
+/// \param rdata_str A complete textual string of the RDATA; used as part of
+/// exception messages.
+/// \param iss Input stream that consists of a complete textual string of
+/// the RDATA.
+/// \param salt A placeholder for the salt field value of the RDATA.
+/// Expected to be empty, but it's not checked (and will be overridden).
+///
+/// \return The hash algorithm, flags, iterations in the form of
+/// ParseNSEC3ParamResult.
+ParseNSEC3ParamResult parseNSEC3ParamText(const char* const rrtype_name,
+                                          const std::string& rdata_str,
+                                          std::istringstream& iss,
+                                          std::vector<uint8_t>& salt);
+
+/// \brief Extract NSEC3 parameters from wire-format data.
+///
+/// This function takes an input buffer that stores wire-format NSEC3 or
+/// NSEC3PARAM RDATA and parses it extracting the hash algorithm, flags,
+/// iterations, and salt fields.
+///
+/// The first three fields are returned as the return value of this function.
+/// The salt will be stored in the given vector.  The vector is expected
+/// to be empty, but if not, the existing content will be overridden.
+///
+/// On successful return the input buffer will point to the end of the
+/// salt field; rdata_len will be the length of the rest of RDATA
+/// (in the case of a valid NSEC3PARAM, it should be 0).
+///
+/// \exception DNSMessageFORMERR The wire data is invalid.
+///
+/// \param rrtype_name Either "NSEC3" or "NSEC3PARAM"; used as part of
+/// exception messages.
+/// \param buffer An input buffer that stores wire-format RDATA.  It must
+/// point to the beginning of the data.
+/// \param rdata_len The total length of the RDATA.
+/// \param salt A placeholder for the salt field value of the RDATA.
+/// Expected to be empty, but it's not checked (and will be overridden).
+///
+/// \return The hash algorithm, flags, iterations in the form of
+/// ParseNSEC3ParamResult.
+ParseNSEC3ParamResult parseNSEC3ParamWire(const char* const rrtype_name,
+                                          isc::util::InputBuffer& buffer,
+                                          size_t& rdata_len,
+                                          std::vector<uint8_t>& salt);
+}
+}
+}
+}
+}
+}
+
+#endif  // __NSEC3PARAM_COMMON_H
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/dns/rdata/generic/detail/nsec_bitmap.cc

@@ -12,11 +12,17 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <stdint.h>
-
-#include <vector>
+#include <exceptions/exceptions.h>
 
 #include <dns/exceptions.h>
+#include <dns/rdata.h>
+#include <dns/rrtype.h>
+
+#include <cassert>
+#include <sstream>
+#include <vector>
+#include <cstring>
+#include <stdint.h>
 
 using namespace std;
 
@@ -70,6 +76,78 @@ checkRRTypeBitmaps(const char* const rrtype_name,
         first = false;
     }
 }
+
+void
+buildBitmapsFromText(const char* const rrtype_name,
+                     istringstream& iss, vector<uint8_t>& typebits)
+{
+    uint8_t bitmap[8 * 1024];       // 64k bits
+    memset(bitmap, 0, sizeof(bitmap));
+
+    do {
+        string type;
+        iss >> type;
+        if (iss.bad() || iss.fail()) {
+            isc_throw(InvalidRdataText, "Unexpected input for "
+                      << rrtype_name << " bitmap");
+        }
+        try {
+            const int code = RRType(type).getCode();
+            bitmap[code / 8] |= (0x80 >> (code % 8));
+        } catch (const InvalidRRType&) {
+            isc_throw(InvalidRdataText, "Invalid RRtype in "
+                      << rrtype_name << " bitmap: " << type);
+        }
+    } while (!iss.eof());
+
+    for (int window = 0; window < 256; ++window) {
+        int octet;
+        for (octet = 31; octet >= 0; octet--) {
+            if (bitmap[window * 32 + octet] != 0) {
+                break;
+            }
+        }
+        if (octet < 0) {
+            continue;
+        }
+        typebits.push_back(window);
+        typebits.push_back(octet + 1);
+        for (int i = 0; i <= octet; ++i) {
+            typebits.push_back(bitmap[window * 32 + i]);
+        }
+    }
+}
+
+void
+bitmapsToText(const vector<uint8_t>& typebits, ostringstream& oss) {
+    // In the following loop we use string::at() rather than operator[].
+    // Since the index calculation is a bit complicated, it will be safer
+    // and easier to find a bug (if any).  Note that this conversion method
+    // is generally not expected to be very efficient, so the slight overhead
+    // of at() should be acceptable.
+    const size_t typebits_len = typebits.size();
+    size_t len = 0;
+    for (size_t i = 0; i < typebits_len; i += len) {
+        assert(i + 2 <= typebits.size());
+        const unsigned int block = typebits.at(i);
+        len = typebits.at(i + 1);
+        assert(len > 0 && len <= 32);
+        i += 2;
+        for (size_t j = 0; j < len; ++j) {
+            if (typebits.at(i + j) == 0) {
+                continue;
+            }
+            for (size_t k = 0; k < 8; ++k) {
+                if ((typebits.at(i + j) & (0x80 >> k)) == 0) {
+                    continue;
+                }
+                const unsigned int t = block * 256 + j * 8 + k;
+                assert(t < 65536);
+                oss << " " << RRType(t);
+            }
+        }
+    }
+}
 }
 }
 }

src/lib/dns/rdata/generic/detail/nsec_bitmap.h

@@ -12,8 +12,12 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#ifndef __NSECBITMAP_H
+#define __NSECBITMAP_H 1
+
 #include <stdint.h>
 
+#include <sstream>
 #include <vector>
 
 namespace isc {
@@ -22,14 +26,22 @@ namespace rdata {
 namespace generic {
 namespace detail {
 namespace nsec {
-/// Check if a given "type bitmap" for NSEC/NSEC3 is valid.
+
+/// \file
+///
+/// This helper module provides some utilities that handle NSEC and NSEC3
+/// type bitmaps.  The format and processing of the type bitmaps are generally
+/// the same for these two RRs, so it would make sense to consolidate
+/// the processing logic into a single implementation module.
+///
+/// The functions defined here are essentially private and are only expected
+/// to be called from the \c NSEC and \c NSEC3 class implementations.
+
+/// \brief Check if a given "type bitmap" for NSEC/NSEC3 is valid.
 ///
-/// This helper function checks given wire format data (stored in a
+/// This function checks given wire format data (stored in a
 /// \c std::vector) is a valid type bitmaps used for the NSEC and NSEC3 RRs
-/// according to RFC4034 and RFC5155.  The validation logic is the same
-/// for these two RRs, so a unified check function is provided.
-/// This function is essentially private and is only expected to be called
-/// from the \c NSEC and \c NSEC3 class implementations.
+/// according to RFC4034 and RFC5155.
 ///
 /// \exception DNSMessageFORMERR The bitmap is not valid.
 ///
@@ -39,6 +51,48 @@ namespace nsec {
 /// is the total length of the bitmaps.
 void checkRRTypeBitmaps(const char* const rrtype_name,
                         const std::vector<uint8_t>& typebits);
+
+/// \brief Convert textual sequence of RR types into type bitmaps.
+///
+/// This function extracts a sequence of strings, converts each sequence
+/// into an RR type, and builds NSEC/NSEC3 type bitmaps with the corresponding
+/// bits for the extracted types being on.  The resulting bitmaps (which are
+/// in the wire format according to RFC4034 and RFC5155) are stored in the
+/// given vector.  This function expects the given string stream ends with
+/// the sequence.
+///
+/// \exception InvalidRdataText The given input stream does not meet the
+/// assumption (e.g. including invalid form of RR type, not ending with
+/// an RR type string).
+///
+/// \param rrtype_name Either "NSEC" or "NSEC3"; used as part of exception
+/// messages.
+/// \param iss Input stream that consists of a complete sequence of textual
+/// RR types for which the corresponding bits are set.
+/// \param typebits A placeholder for the resulting bitmaps.  Expected to be
+/// empty, but it's not checked.
+void buildBitmapsFromText(const char* const rrtype_name,
+                          std::istringstream& iss,
+                          std::vector<uint8_t>& typebits);
+
+/// \brief Convert type bitmaps to textual sequence of RR types.
+///
+/// This function converts wire-format data of type bitmaps for NSEC/NSEC3
+/// into a sequence of corresponding RR type strings, and inserts them
+/// into the given output stream with separating them by a single space
+/// character.
+///
+/// This function assumes the given bitmaps are valid in terms of RFC4034
+/// and RFC5155 (in practice, it assumes it's from a validly constructed
+/// NSEC or NSEC3 object); if it detects a format error, it aborts the
+/// program with assert().
+///
+/// \param typebits The type bitmaps in wire format.  The size of vector
+/// is the total length of the bitmaps.
+/// \param oss The output stream to which the converted RR type sequence
+/// are to be inserted.
+void bitmapsToText(const std::vector<uint8_t>& typebits,
+                   std::ostringstream& oss);
 }
 }
 }
@@ -46,6 +100,8 @@ void checkRRTypeBitmaps(const char* const rrtype_name,
 }
 }
 
+#endif  // __NSECBITMAP_H
+
 // Local Variables:
 // mode: c++
 // End:

src/lib/dns/rdata/generic/nsec3_50.cc

@@ -17,6 +17,7 @@
 #include <string>
 #include <sstream>
 #include <vector>
+#include <cassert>
 
 #include <boost/lexical_cast.hpp>
 
@@ -32,12 +33,14 @@
 #include <dns/rdata.h>
 #include <dns/rdataclass.h>
 #include <dns/rdata/generic/detail/nsec_bitmap.h>
+#include <dns/rdata/generic/detail/nsec3param_common.h>
 
 #include <stdio.h>
 #include <time.h>
 
 using namespace std;
 using namespace isc::dns::rdata::generic::detail::nsec;
+using namespace isc::dns::rdata::generic::detail::nsec3;
 using namespace isc::util::encode;
 using namespace isc::util;
 
@@ -53,54 +56,32 @@ struct NSEC3Impl {
         salt_(salt), next_(next), typebits_(typebits)
     {}
 
-    uint8_t hashalg_;
-    uint8_t flags_;
-    uint16_t iterations_;
-    vector<uint8_t> salt_;
-    vector<uint8_t> next_;
-    vector<uint8_t> typebits_;
+    const uint8_t hashalg_;
+    const uint8_t flags_;
+    const uint16_t iterations_;
+    const vector<uint8_t> salt_;
+    const vector<uint8_t> next_;
+    const vector<uint8_t> typebits_;
 };
 
 NSEC3::NSEC3(const string& nsec3_str) :
     impl_(NULL)
 {
     istringstream iss(nsec3_str);
-    unsigned int hashalg, flags, iterations;
-    string iterations_str, salthex, nexthash;
+    vector<uint8_t> salt;
+    const ParseNSEC3ParamResult params =
+        parseNSEC3ParamText("NSEC3", nsec3_str, iss, salt);
 
-    iss >> hashalg >> flags >> iterations_str >> salthex >> nexthash;
+    // Extract Next hash.  It must be an unpadded base32hex string.
+    string nexthash;
+    iss >> nexthash;
     if (iss.bad() || iss.fail()) {
         isc_throw(InvalidRdataText, "Invalid NSEC3 text: " << nsec3_str);
     }
-    if (hashalg > 0xff) {
-        isc_throw(InvalidRdataText,
-                  "NSEC3 hash algorithm out of range: " << hashalg);
-    }
-    if (flags > 0xff) {
-        isc_throw(InvalidRdataText, "NSEC3 flags out of range: " << flags);
-    }
-    // Convert iteration.  To reject an invalid case where there's no space
-    // between iteration and salt, we extract this field as string and convert
-    // to integer.
-    try {
-        iterations = boost::lexical_cast<unsigned int>(iterations_str);
-    } catch (const boost::bad_lexical_cast&) {
-        isc_throw(InvalidRdataText, "Bad NSEC3 iteration: " << iterations_str);
-    }
-    if (iterations > 0xffff) {
-        isc_throw(InvalidRdataText, "NSEC3 iterations out of range: " <<
-            iterations);
-    }
-
-    vector<uint8_t> salt;
-    if (salthex != "-") {       // "-" means a 0-length salt
-        decodeHex(salthex, salt);
-    }
-    if (salt.size() > 255) {
-        isc_throw(InvalidRdataText, "NSEC3 salt is too long: "
-                  << salt.size() << " bytes");
+    assert(!nexthash.empty());
+    if (*nexthash.rbegin() == '=') {
+        isc_throw(InvalidRdataText, "NSEC3 hash has padding: " << nsec3_str);
     }
-
     vector<uint8_t> next;
     decodeBase32Hex(nexthash, next);
     if (next.size() > 255) {
@@ -110,70 +91,28 @@ NSEC3::NSEC3(const string& nsec3_str) :
 
     // For NSEC3 empty bitmap is possible and allowed.
     if (iss.eof()) {
-        impl_ = new NSEC3Impl(hashalg, flags, iterations, salt, next,
+        impl_ = new NSEC3Impl(params.algorithm, params.flags,
+                              params.iterations, salt, next,
                               vector<uint8_t>());
         return;
     }
 
     vector<uint8_t> typebits;
-    uint8_t bitmap[8 * 1024];       // 64k bits
-    memset(bitmap, 0, sizeof(bitmap));
-    do { 
-        string type;
-        iss >> type;
-        if (type.length() != 0) {
-            try {
-                const int code = RRType(type).getCode();
-                bitmap[code / 8] |= (0x80 >> (code % 8));
-            } catch (...) {
-                isc_throw(InvalidRdataText, "Invalid RRtype in NSEC3");
-            }
-        }
-    } while (!iss.eof());
-
-    for (int window = 0; window < 256; window++) {
-        int octet;
-        for (octet = 31; octet >= 0; octet--) {
-            if (bitmap[window * 32 + octet] != 0) {
-                break;
-            }
-        }
-        if (octet < 0)
-            continue;
-        typebits.push_back(window);
-        typebits.push_back(octet + 1);
-        for (int i = 0; i <= octet; i++) {
-            typebits.push_back(bitmap[window * 32 + i]);
-        }
-    }
+    buildBitmapsFromText("NSEC3", iss, typebits);
 
-    impl_ = new NSEC3Impl(hashalg, flags, iterations, salt, next, typebits);
+    impl_ = new NSEC3Impl(params.algorithm, params.flags, params.iterations,
+                          salt, next, typebits);
 }
 
 NSEC3::NSEC3(InputBuffer& buffer, size_t rdata_len) {
-    // NSEC3 RR must have at least 5 octets:
-    // hash algorithm(1), flags(1), iteration(2), saltlen(1)
-    if (rdata_len < 5) {
-        isc_throw(DNSMessageFORMERR, "NSEC3 too short, length: " << rdata_len);
-    }
-
-    const uint8_t hashalg = buffer.readUint8();
-    const uint8_t flags = buffer.readUint8();
-    const uint16_t iterations = buffer.readUint16();
-
-    const uint8_t saltlen = buffer.readUint8();
-    rdata_len -= 5;
-    if (rdata_len < saltlen) {
-        isc_throw(DNSMessageFORMERR, "NSEC3 salt length is too large: " <<
-                  static_cast<unsigned int>(saltlen));
-    }
+    vector<uint8_t> salt;
+    const ParseNSEC3ParamResult params =
+        parseNSEC3ParamWire("NSEC3", buffer, rdata_len, salt);
 
-    vector<uint8_t> salt(saltlen);
-    if (saltlen > 0) {
-        buffer.readData(&salt[0], saltlen);
-        rdata_len -= saltlen;
+    if (rdata_len < 1) {
+        isc_throw(DNSMessageFORMERR, "NSEC3 too short to contain hash length, "
+                  "length: " << rdata_len + salt.size() + 5);
     }
-
     const uint8_t nextlen = buffer.readUint8();
     --rdata_len;
     if (nextlen == 0 || rdata_len < nextlen) {
@@ -193,7 +132,8 @@ NSEC3::NSEC3(InputBuffer& buffer, size_t rdata_len) {
         checkRRTypeBitmaps("NSEC3", typebits);
     }
 
-    impl_ = new NSEC3Impl(hashalg, flags, iterations, salt, next, typebits);
+    impl_ = new NSEC3Impl(params.algorithm, params.flags, params.iterations,
+                          salt, next, typebits);
 }
 
 NSEC3::NSEC3(const NSEC3& source) :
@@ -220,57 +160,78 @@ NSEC3::~NSEC3() {
 string
 NSEC3::toText() const {
     ostringstream s;
-    int len = 0;
-    for (size_t i = 0; i < impl_->typebits_.size(); i += len) {
-        assert(i + 2 <= impl_->typebits_.size());
-        int window = impl_->typebits_[i];
-        len = impl_->typebits_[i + 1];
-        assert(len >= 0 && len < 32);
-        i += 2;
-        for (int j = 0; j < len; j++) {
-            if (impl_->typebits_[i + j] == 0) {
-                continue;
-            }
-            for (int k = 0; k < 8; k++) {
-                if ((impl_->typebits_[i + j] & (0x80 >> k)) == 0) {
-                    continue;
-                }
-                int t = window * 256 + j * 8 + k;
-                s << " " << RRType(t).toText();
-            }
-        }
-    }
+    bitmapsToText(impl_->typebits_, s);
 
     using namespace boost;
     return (lexical_cast<string>(static_cast<int>(impl_->hashalg_)) +
-        " " + lexical_cast<string>(static_cast<int>(impl_->flags_)) +
-        " " + lexical_cast<string>(static_cast<int>(impl_->iterations_)) +
-        " " + encodeHex(impl_->salt_) +
-        " " + encodeBase32Hex(impl_->next_) + s.str());
+            " " + lexical_cast<string>(static_cast<int>(impl_->flags_)) +
+            " " + lexical_cast<string>(static_cast<int>(impl_->iterations_)) +
+            " " + (impl_->salt_.empty() ? "-" : encodeHex(impl_->salt_)) +
+            " " + encodeBase32Hex(impl_->next_) + s.str());
+}
+
+template <typename OUTPUT_TYPE>
+void
+toWireHelper(const NSEC3Impl& impl, OUTPUT_TYPE& output) {
+    output.writeUint8(impl.hashalg_);
+    output.writeUint8(impl.flags_);
+    output.writeUint16(impl.iterations_);
+    output.writeUint8(impl.salt_.size());
+    if (!impl.salt_.empty()) {
+        output.writeData(&impl.salt_[0], impl.salt_.size());
+    }
+    assert(!impl.next_.empty());
+    output.writeUint8(impl.next_.size());
+    output.writeData(&impl.next_[0], impl.next_.size());
+    if (!impl.typebits_.empty()) {
+        output.writeData(&impl.typebits_[0], impl.typebits_.size());
+    }
 }
 
 void
 NSEC3::toWire(OutputBuffer& buffer) const {
-    buffer.writeUint8(impl_->hashalg_);
-    buffer.writeUint8(impl_->flags_);
-    buffer.writeUint16(impl_->iterations_);
-    buffer.writeUint8(impl_->salt_.size());
-    buffer.writeData(&impl_->salt_[0], impl_->salt_.size());
-    buffer.writeUint8(impl_->next_.size());
-    buffer.writeData(&impl_->next_[0], impl_->next_.size());
-    buffer.writeData(&impl_->typebits_[0], impl_->typebits_.size());
+    toWireHelper(*impl_, buffer);
 }
 
 void
 NSEC3::toWire(AbstractMessageRenderer& renderer) const {
-    renderer.writeUint8(impl_->hashalg_);
-    renderer.writeUint8(impl_->flags_);
-    renderer.writeUint16(impl_->iterations_);
-    renderer.writeUint8(impl_->salt_.size());
-    renderer.writeData(&impl_->salt_[0], impl_->salt_.size());
-    renderer.writeUint8(impl_->next_.size());
-    renderer.writeData(&impl_->next_[0], impl_->next_.size());
-    renderer.writeData(&impl_->typebits_[0], impl_->typebits_.size());
+    toWireHelper(*impl_, renderer);
+}
+
+namespace {
+// This is a helper subroutine for compare().  It compares two binary
+// data stored in vector<uint8_t> objects based on the "Canonical RR Ordering"
+// as defined in Section 6.3 of RFC4034, that is, the data are treated
+// "as a left-justified unsigned octet sequence in which the absence of an
+// octet sorts before a zero octet."
+//
+// If check_length_first is true, it treats the compared data as if they
+// began with a single-octet "length" field whose value is the size of the
+// corresponding vector.  In this case, if the sizes of the two vectors are
+// different the shorter one is always considered the "smaller"; the contents
+// of the vector don't matter.
+//
+// This function returns:
+// -1 if v1 is considered smaller than v2
+// 1 if v1 is considered larger than v2
+// 0 otherwise
+int
+compareVectors(const vector<uint8_t>& v1, const vector<uint8_t>& v2,
+               bool check_length_first = true)
+{
+    const size_t len1 = v1.size();
+    const size_t len2 = v2.size();
+    if (check_length_first && len1 != len2) {
+        return (len1 - len2);
+    }
+    const size_t cmplen = min(len1, len2);
+    const int cmp = cmplen == 0 ? 0 : memcmp(&v1.at(0), &v2.at(0), cmplen);
+    if (cmp != 0) {
+        return (cmp);
+    } else {
+        return (len1 - len2);
+    }
+}
 }
 
 int
@@ -287,44 +248,18 @@ NSEC3::compare(const Rdata& other) const {
         return (impl_->iterations_ < other_nsec3.impl_->iterations_ ? -1 : 1);
     }
 
-    size_t this_len = impl_->salt_.size();
-    size_t other_len = other_nsec3.impl_->salt_.size();
-    size_t cmplen = min(this_len, other_len);
-    int cmp = memcmp(&impl_->salt_[0], &other_nsec3.impl_->salt_[0], cmplen);
-    if (cmp != 0) {
-        return (cmp);
-    } else if (this_len < other_len) {
-        return (-1);
-    } else if (this_len > other_len) {
-        return (1);
-    }
-
-    this_len = impl_->salt_.size();
-    other_len = other_nsec3.impl_->salt_.size();
-    cmplen = min(this_len, other_len);
-    cmp = memcmp(&impl_->next_[0], &other_nsec3.impl_->next_[0], cmplen);
+    int cmp = compareVectors(impl_->salt_, other_nsec3.impl_->salt_);
     if (cmp != 0) {
         return (cmp);
-    } else if (this_len < other_len) {
-        return (-1);
-    } else if (this_len > other_len) {
-        return (1);
     }
-
-    this_len = impl_->typebits_.size();
-    other_len = other_nsec3.impl_->typebits_.size();
-    cmplen = min(this_len, other_len);
-    cmp = memcmp(&impl_->typebits_[0], &other_nsec3.impl_->typebits_[0],
-                 cmplen);
+    cmp = compareVectors(impl_->next_, other_nsec3.impl_->next_);
     if (cmp != 0) {
         return (cmp);
-    } else if (this_len < other_len) {
-        return (-1);
-    } else if (this_len > other_len) {
-        return (1);
-    } else {
-        return (0);
     }
+    // Note that bitmap doesn't have a dedicated length field, so we shouldn't
+    // terminate the comparison just because the lengths are different.
+    return (compareVectors(impl_->typebits_, other_nsec3.impl_->typebits_,
+                           false));
 }
 
 uint8_t

src/lib/dns/rdata/generic/nsec3param_51.cc

@@ -12,22 +12,19 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <iostream>
-#include <string>
-#include <sstream>
-#include <vector>
-
-#include <boost/lexical_cast.hpp>
-
 #include <util/buffer.h>
 #include <util/encode/hex.h>
+
 #include <dns/messagerenderer.h>
-#include <dns/name.h>
 #include <dns/rdata.h>
 #include <dns/rdataclass.h>
+#include <dns/rdata/generic/detail/nsec3param_common.h>
+
+#include <boost/lexical_cast.hpp>
 
-#include <stdio.h>
-#include <time.h>
+#include <string>
+#include <sstream>
+#include <vector>
 
 using namespace std;
 using namespace isc::util;
@@ -43,9 +40,9 @@ struct NSEC3PARAMImpl {
         hashalg_(hashalg), flags_(flags), iterations_(iterations), salt_(salt)
     {}
 
-    uint8_t hashalg_;
-    uint8_t flags_;
-    uint16_t iterations_;
+    const uint8_t hashalg_;
+    const uint8_t flags_;
+    const uint16_t iterations_;
     const vector<uint8_t> salt_;
 };
 
@@ -53,50 +50,26 @@ NSEC3PARAM::NSEC3PARAM(const string& nsec3param_str) :
     impl_(NULL)
 {
     istringstream iss(nsec3param_str);
-    uint16_t hashalg, flags, iterations;
-    stringbuf saltbuf;
-
-    iss >> hashalg >> flags >> iterations >> &saltbuf;
-    if (iss.bad() || iss.fail()) {
-        isc_throw(InvalidRdataText, "Invalid NSEC3PARAM text");
-    }
-    if (hashalg > 0xf) {
-        isc_throw(InvalidRdataText, "NSEC3PARAM hash algorithm out of range");
-    }
-    if (flags > 0xff) {
-        isc_throw(InvalidRdataText, "NSEC3PARAM flags out of range");
-    }
-
-    const string salt_str = saltbuf.str();
     vector<uint8_t> salt;
-    if (salt_str != "-") { // "-" means an empty salt, no need to touch vector
-        decodeHex(saltbuf.str(), salt);
+    const ParseNSEC3ParamResult params =
+        parseNSEC3ParamText("NSEC3PARAM", nsec3param_str, iss, salt);
+
+    if (!iss.eof()) {
+        isc_throw(InvalidRdataText, "Invalid NSEC3PARAM (redundant text): "
+                  << nsec3param_str);
     }
 
-    impl_ = new NSEC3PARAMImpl(hashalg, flags, iterations, salt);
+    impl_ = new NSEC3PARAMImpl(params.algorithm, params.flags,
+                               params.iterations, salt);
 }
 
 NSEC3PARAM::NSEC3PARAM(InputBuffer& buffer, size_t rdata_len) {
-    if (rdata_len < 4) {
-        isc_throw(InvalidRdataLength, "NSEC3PARAM too short");
-    }
-
-    uint8_t hashalg = buffer.readUint8();
-    uint8_t flags = buffer.readUint8();
-    uint16_t iterations = buffer.readUint16();
-    rdata_len -= 4;
-
-    uint8_t saltlen = buffer.readUint8();
-    --rdata_len;
-
-    if (rdata_len < saltlen) {
-        isc_throw(InvalidRdataLength, "NSEC3PARAM salt too short");
-    }
-
-    vector<uint8_t> salt(saltlen);
-    buffer.readData(&salt[0], saltlen);
+    vector<uint8_t> salt;
+    const ParseNSEC3ParamResult params =
+        parseNSEC3ParamWire("NSEC3PARAM", buffer, rdata_len, salt);
 
-    impl_ = new NSEC3PARAMImpl(hashalg, flags, iterations, salt);
+    impl_ = new NSEC3PARAMImpl(params.algorithm, params.flags,
+                               params.iterations, salt);
 }
 
 NSEC3PARAM::NSEC3PARAM(const NSEC3PARAM& source) :
@@ -124,27 +97,31 @@ string
 NSEC3PARAM::toText() const {
     using namespace boost;
     return (lexical_cast<string>(static_cast<int>(impl_->hashalg_)) +
-        " " + lexical_cast<string>(static_cast<int>(impl_->flags_)) +
-        " " + lexical_cast<string>(static_cast<int>(impl_->iterations_)) +
-        " " + encodeHex(impl_->salt_));
+            " " + lexical_cast<string>(static_cast<int>(impl_->flags_)) +
+            " " + lexical_cast<string>(static_cast<int>(impl_->iterations_)) +
+            " " + (impl_->salt_.empty() ? "-" : encodeHex(impl_->salt_)));
+}
+
+template <typename OUTPUT_TYPE>
+void
+toWireHelper(const NSEC3PARAMImpl& impl, OUTPUT_TYPE& output) {
+    output.writeUint8(impl.hashalg_);
+    output.writeUint8(impl.flags_);
+    output.writeUint16(impl.iterations_);
+    output.writeUint8(impl.salt_.size());
+    if (!impl.salt_.empty()) {
+        output.writeData(&impl.salt_[0], impl.salt_.size());
+    }
 }
 
 void
 NSEC3PARAM::toWire(OutputBuffer& buffer) const {
-    buffer.writeUint8(impl_->hashalg_);
-    buffer.writeUint8(impl_->flags_);
-    buffer.writeUint16(impl_->iterations_);
-    buffer.writeUint8(impl_->salt_.size());
-    buffer.writeData(&impl_->salt_[0], impl_->salt_.size());
+    toWireHelper(*impl_, buffer);
 }
 
 void
 NSEC3PARAM::toWire(AbstractMessageRenderer& renderer) const {
-    renderer.writeUint8(impl_->hashalg_);
-    renderer.writeUint8(impl_->flags_);
-    renderer.writeUint16(impl_->iterations_);
-    renderer.writeUint8(impl_->salt_.size());
-    renderer.writeData(&impl_->salt_[0], impl_->salt_.size());
+    toWireHelper(*impl_, renderer);
 }
 
 int
@@ -161,15 +138,18 @@ NSEC3PARAM::compare(const Rdata& other) const {
         return (impl_->iterations_ < other_param.impl_->iterations_ ? -1 : 1);
     }
 
-    size_t this_len = impl_->salt_.size();
-    size_t other_len = other_param.impl_->salt_.size();
-    size_t cmplen = min(this_len, other_len);
-    int cmp = memcmp(&impl_->salt_[0], &other_param.impl_->salt_[0],
-                     cmplen);
+    const size_t this_len = impl_->salt_.size();
+    const size_t other_len = other_param.impl_->salt_.size();
+    if (this_len != other_len) {
+        return (this_len - other_len);
+    }
+    const size_t cmplen = min(this_len, other_len);
+    const int cmp = (cmplen == 0) ? 0 :
+        memcmp(&impl_->salt_.at(0), &other_param.impl_->salt_.at(0), cmplen);
     if (cmp != 0) {
         return (cmp);
     } else {
-        return ((this_len == other_len) ? 0 : (this_len < other_len) ? -1 : 1);
+        return (this_len - other_len);
     }
 }
 
@@ -193,6 +173,5 @@ NSEC3PARAM::getSalt() const {
     return (impl_->salt_);
 }
 
-
 // END_RDATA_NAMESPACE
 // END_ISC_NAMESPACE

src/lib/dns/rdata/generic/nsec_47.cc

@@ -54,42 +54,18 @@ NSEC::NSEC(const string& nsec_str) :
 {
     istringstream iss(nsec_str);
     string nextname;
-    uint8_t bitmap[8 * 1024];       // 64k bits
-    vector<uint8_t> typebits;
 
     iss >> nextname;
     if (iss.bad() || iss.fail()) {
         isc_throw(InvalidRdataText, "Invalid NSEC name");
     }
-
-    memset(bitmap, 0, sizeof(bitmap));
-    do { 
-        string type;
-        iss >> type;
-        try {
-            const int code = RRType(type).getCode();
-            bitmap[code / 8] |= (0x80 >> (code % 8));
-        } catch (...) {
-            isc_throw(InvalidRdataText, "Invalid RRtype in NSEC");
-        }
-    } while (!iss.eof());
-
-    for (int window = 0; window < 256; window++) {
-        int octet;
-        for (octet = 31; octet >= 0; octet--) {
-            if (bitmap[window * 32 + octet] != 0) {
-                break;
-            }
-        }
-        if (octet < 0)
-            continue;
-        typebits.push_back(window);
-        typebits.push_back(octet + 1);
-        for (int i = 0; i <= octet; i++) {
-            typebits.push_back(bitmap[window * 32 + i]);
-        }
+    if (iss.eof()) {
+        isc_throw(InvalidRdataText, "NSEC bitmap is missing");
     }
 
+    vector<uint8_t> typebits;
+    buildBitmapsFromText("NSEC", iss, typebits);
+
     impl_ = new NSECImpl(Name(nextname), typebits);
 }
 
@@ -135,34 +111,8 @@ NSEC::~NSEC() {
 string
 NSEC::toText() const {
     ostringstream s;
-    int len = 0;
     s << impl_->nextname_;
-
-    // In the following loop we use string::at() rather than operator[].
-    // Since the index calculation is a bit complicated, it will be safer
-    // and easier to find a bug (if any).  Note that this conversion method
-    // is generally not expected to be very efficient, so the slight overhead
-    // of at() should be acceptable.
-    for (size_t i = 0; i < impl_->typebits_.size(); i += len) {
-        assert(i + 2 <= impl_->typebits_.size());
-        const int block = impl_->typebits_.at(i);
-        len = impl_->typebits_.at(i + 1);
-        assert(len > 0 && len <= 32);
-        i += 2;
-        for (int j = 0; j < len; j++) {
-            if (impl_->typebits_.at(i + j) == 0) {
-                continue;
-            }
-            for (int k = 0; k < 8; k++) {
-                if ((impl_->typebits_.at(i + j) & (0x80 >> k)) == 0) {
-                    continue;
-                }
-                const int t = block * 256 + j * 8 + k;
-                s << " " << RRType(t);
-            }
-        }
-    }
-
+    bitmapsToText(impl_->typebits_, s);
     return (s.str());
 }
 

src/lib/dns/tests/Makefile.am

@@ -43,6 +43,7 @@ run_unittests_SOURCES += rdata_nsec_unittest.cc
 run_unittests_SOURCES += rdata_nsec3_unittest.cc
 run_unittests_SOURCES += rdata_nsecbitmap_unittest.cc
 run_unittests_SOURCES += rdata_nsec3param_unittest.cc
+run_unittests_SOURCES += rdata_nsec3param_like_unittest.cc
 run_unittests_SOURCES += rdata_rrsig_unittest.cc
 run_unittests_SOURCES += rdata_rp_unittest.cc
 run_unittests_SOURCES += rdata_srv_unittest.cc

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) {

src/lib/dns/tests/nsec3hash_unittest.cc

@@ -29,7 +29,7 @@ using namespace isc::dns::rdata;
 namespace {
 typedef scoped_ptr<NSEC3Hash> NSEC3HashPtr;
 
-// Commonly used NSEC3 suffix, defined to reduce amount of type
+// Commonly used NSEC3 suffix, defined to reduce the amount of typing
 const char* const nsec3_common = "2T7B4G4VSA5SMI47K61MV5BV1A22BOJR A RRSIG";
 
 class NSEC3HashTest : public ::testing::Test {
@@ -41,6 +41,11 @@ protected:
                                            string(nsec3_common))))
     {}
 
+    ~NSEC3HashTest() {
+        // Make sure we reset the hash creator to the default
+        setNSEC3HashCreator(NULL);
+    }
+
     // An NSEC3Hash object commonly used in tests.  Parameters are borrowed
     // from the RFC5155 example.  Construction of this object implicitly
     // checks a successful case of the creation.
@@ -142,4 +147,76 @@ TEST_F(NSEC3HashTest, matchWithNSEC3PARAM) {
     }
 }
 
+// A simple faked hash calculator and a dedicated creator for it.
+class TestNSEC3Hash : public NSEC3Hash {
+    virtual string calculate(const Name&) const {
+        return ("00000000000000000000000000000000");
+    }
+    virtual bool match(const generic::NSEC3PARAM&) const {
+        return (true);
+    }
+    virtual bool match(const generic::NSEC3&) const {
+        return (true);
+    }
+};
+
+// This faked creator basically creates the faked calculator regardless of
+// the passed NSEC3PARAM or NSEC3.  But if the most significant bit of flags
+// is set, it will behave like the default creator.
+class TestNSEC3HashCreator : public NSEC3HashCreator {
+public:
+    virtual NSEC3Hash* create(const generic::NSEC3PARAM& param) const {
+        if ((param.getFlags() & 0x80) != 0) {
+            return (default_creator_.create(param));
+        }
+        return (new TestNSEC3Hash);
+    }
+    virtual NSEC3Hash* create(const generic::NSEC3& nsec3) const {
+        if ((nsec3.getFlags() & 0x80) != 0) {
+            return (default_creator_.create(nsec3));
+        }
+        return (new TestNSEC3Hash);
+    }
+private:
+    DefaultNSEC3HashCreator default_creator_;
+};
+
+TEST_F(NSEC3HashTest, setCreator) {
+    // Re-check an existing case using the default creator/hash implementation
+    EXPECT_EQ("0P9MHAVEQVM6T7VBL5LOP2U3T2RP3TOM",
+              test_hash->calculate(Name("example")));
+
+    // Replace the creator, and confirm the hash values are faked
+    TestNSEC3HashCreator test_creator;
+    setNSEC3HashCreator(&test_creator);
+    // Re-create the hash object with the new creator
+    test_hash.reset(NSEC3Hash::create(generic::NSEC3PARAM("1 0 12 aabbccdd")));
+    EXPECT_EQ("00000000000000000000000000000000",
+              test_hash->calculate(Name("example")));
+    // Same for hash from NSEC3 RDATA
+    test_hash.reset(NSEC3Hash::create(generic::NSEC3
+                                      ("1 0 12 aabbccdd " +
+                                       string(nsec3_common))));
+    EXPECT_EQ("00000000000000000000000000000000",
+              test_hash->calculate(Name("example")));
+
+    // If we set a special flag big (0x80) on creation, it will act like the
+    // default creator.
+    test_hash.reset(NSEC3Hash::create(generic::NSEC3PARAM(
+                                          "1 128 12 aabbccdd")));
+    EXPECT_EQ("0P9MHAVEQVM6T7VBL5LOP2U3T2RP3TOM",
+              test_hash->calculate(Name("example")));
+    test_hash.reset(NSEC3Hash::create(generic::NSEC3
+                                      ("1 128 12 aabbccdd " +
+                                       string(nsec3_common))));
+    EXPECT_EQ("0P9MHAVEQVM6T7VBL5LOP2U3T2RP3TOM",
+              test_hash->calculate(Name("example")));
+
+    // Reset the creator to default, and confirm that
+    setNSEC3HashCreator(NULL);
+    test_hash.reset(NSEC3Hash::create(generic::NSEC3PARAM("1 0 12 aabbccdd")));
+    EXPECT_EQ("0P9MHAVEQVM6T7VBL5LOP2U3T2RP3TOM",
+              test_hash->calculate(Name("example")));
+}
+
 } // end namespace

src/lib/dns/tests/rdata_nsec3_unittest.cc

@@ -39,13 +39,19 @@ using namespace isc::util::encode;
 using namespace isc::dns::rdata;
 
 namespace {
+
+// Note: some tests can be shared with NSEC3PARAM.  They are unified as
+// typed tests defined in nsec3param_like_unittest.
 class Rdata_NSEC3_Test : public RdataTest {
     // there's nothing to specialize
 public:
     Rdata_NSEC3_Test() :
         nsec3_txt("1 1 1 D399EAAB H9RSFB7FPF2L8HG35CMPC765TDK23RP6 "
-                  "NS SOA RRSIG DNSKEY NSEC3PARAM") {}
-    string nsec3_txt;
+                  "NS SOA RRSIG DNSKEY NSEC3PARAM"),
+        nsec3_nosalt_txt("1 1 1 - H9RSFB7FPF2L8HG35CMPC765TDK23RP6 A" )
+    {}
+    const string nsec3_txt;
+    const string nsec3_nosalt_txt;
 };
 
 TEST_F(Rdata_NSEC3_Test, fromText) {
@@ -53,21 +59,6 @@ TEST_F(Rdata_NSEC3_Test, fromText) {
     // text and construct nsec3_txt.  It will be tested against the wire format
     // representation in the createFromWire test.
 
-    // Numeric parameters have possible maximum values.  Unusual, but must
-    // be accepted.
-    EXPECT_NO_THROW(generic::NSEC3("255 255 65535 D399EAAB "
-                                   "H9RSFB7FPF2L8HG35CMPC765TDK23RP6 "
-                                   "NS SOA RRSIG DNSKEY NSEC3PARAM"));
-
-    // 0-length salt
-    EXPECT_EQ(0, generic::NSEC3("1 1 1 - H9RSFB7FPF2L8HG35CMPC765TDK23RP6 "
-                                "A").getSalt().size());
-
-    // salt that has the possible max length
-    EXPECT_EQ(255, generic::NSEC3("1 1 1 " + string(255 * 2, '0') +
-                                  " H9RSFB7FPF2L8HG35CMPC765TDK23RP6 "
-                                  "NS").getSalt().size());
-
     // hash that has the possible max length (see badText about the magic
     // numbers)
     EXPECT_EQ(255, generic::NSEC3("1 1 1 D399EAAB " +
@@ -79,43 +70,20 @@ TEST_F(Rdata_NSEC3_Test, fromText) {
                         "1 1 1 D399EAAB H9RSFB7FPF2L8HG35CMPC765TDK23RP6"));
 }
 
-TEST_F(Rdata_NSEC3_Test, toText) {
-    const generic::NSEC3 rdata_nsec3(nsec3_txt);
-    EXPECT_EQ(nsec3_txt, rdata_nsec3.toText());
-}
-
 TEST_F(Rdata_NSEC3_Test, badText) {
     EXPECT_THROW(generic::NSEC3("1 1 1 ADDAFEEE "
                                 "0123456789ABCDEFGHIJKLMNOPQRSTUV "
                                 "BIFF POW SPOON"),
                  InvalidRdataText);
-    EXPECT_THROW(generic::NSEC3("1 1 1 ADDAFEE "
-                                "WXYZWXYZWXYZ=WXYZWXYZ==WXYZWXYZW A NS SOA"),
-                 BadValue);     // bad hex
-    EXPECT_THROW(generic::NSEC3("1 1 1 -- H9RSFB7FPF2L8HG35CMPC765TDK23RP6 "
-                                "A"),
-                 BadValue); // this shouldn't be confused a valid empty salt
     EXPECT_THROW(generic::NSEC3("1 1 1 ADDAFEEE "
                                 "WXYZWXYZWXYZ=WXYZWXYZ==WXYZWXYZW A NS SOA"),
                  BadValue);     // bad base32hex
-    EXPECT_THROW(generic::NSEC3("1000000 1 1 ADDAFEEE "
-                                "0123456789ABCDEFGHIJKLMNOPQRSTUV A NS SOA"),
-                 InvalidRdataText);
-    EXPECT_THROW(generic::NSEC3("1 1000000 1 ADDAFEEE "
-                                "0123456789ABCDEFGHIJKLMNOPQRSTUV A NS SOA"),
-                 InvalidRdataText);
     EXPECT_THROW(generic::NSEC3("1 1 1000000 ADDAFEEE "
                                 "0123456789ABCDEFGHIJKLMNOPQRSTUV A NS SOA"),
                  InvalidRdataText);
 
-    // There should be a space between "1" and "D399EAAB" (salt)
-    EXPECT_THROW(generic::NSEC3(
-                     "1 1 1D399EAAB H9RSFB7FPF2L8HG35CMPC765TDK23RP6 "
-                     "NS SOA RRSIG DNSKEY NSEC3PARAM"), InvalidRdataText);
-
-    // Salt is too long (possible max + 1 bytes)
-    EXPECT_THROW(generic::NSEC3("1 1 1 " + string(256 * 2, '0') +
-                                " H9RSFB7FPF2L8HG35CMPC765TDK23RP6 NS"),
+    // Next hash shouldn't be padded
+    EXPECT_THROW(generic::NSEC3("1 1 1 ADDAFEEE CPNMU=== A NS SOA"),
                  InvalidRdataText);
 
     // Hash is too long.  Max = 255 bytes, base32-hex converts each 5 bytes
@@ -127,34 +95,12 @@ TEST_F(Rdata_NSEC3_Test, badText) {
 }
 
 TEST_F(Rdata_NSEC3_Test, createFromWire) {
-    // Normal case
-    const generic::NSEC3 rdata_nsec3(nsec3_txt);
-    EXPECT_EQ(0, rdata_nsec3.compare(
-                  *rdataFactoryFromFile(RRType::NSEC3(), RRClass::IN(),
-                                        "rdata_nsec3_fromWire1")));
-
     // A valid NSEC3 RR with empty type bitmap.
     EXPECT_NO_THROW(rdataFactoryFromFile(RRType::NSEC3(), RRClass::IN(),
                                          "rdata_nsec3_fromWire15.wire"));
 
-    // Too short RDLENGTH: it doesn't even contain the first 5 octets.
-    EXPECT_THROW(rdataFactoryFromFile(RRType::NSEC3(), RRClass::IN(),
-                                      "rdata_nsec3_fromWire2.wire"),
-                 DNSMessageFORMERR);
-
     // Invalid bitmap cases are tested in Rdata_NSECBITMAP_Test.
 
-    // salt length is too large
-    EXPECT_THROW(rdataFactoryFromFile(RRType::NSEC3(), RRClass::IN(),
-                                      "rdata_nsec3_fromWire11.wire"),
-                 DNSMessageFORMERR);
-
-    // empty salt.  unusual, but valid.
-    ConstRdataPtr rdata =
-        rdataFactoryFromFile(RRType::NSEC3(), RRClass::IN(),
-                             "rdata_nsec3_fromWire13.wire");
-    EXPECT_EQ(0, dynamic_cast<const generic::NSEC3&>(*rdata).getSalt().size());
-
     // hash length is too large
     EXPECT_THROW(rdataFactoryFromFile(RRType::NSEC3(), RRClass::IN(),
                                       "rdata_nsec3_fromWire12.wire"),
@@ -165,7 +111,11 @@ TEST_F(Rdata_NSEC3_Test, createFromWire) {
                                       "rdata_nsec3_fromWire14.wire"),
                  DNSMessageFORMERR);
 
-    //
+    // RDLEN is too short to hold the hash length field
+    EXPECT_THROW(rdataFactoryFromFile(RRType::NSEC3(), RRClass::IN(),
+                                      "rdata_nsec3_fromWire17.wire"),
+                 DNSMessageFORMERR);
+
     // Short buffer cases.  The data is valid NSEC3 RDATA, but the buffer
     // is trimmed at the end.  All cases should result in an exception from
     // the buffer class.
@@ -180,27 +130,35 @@ TEST_F(Rdata_NSEC3_Test, createFromWire) {
     }
 }
 
-TEST_F(Rdata_NSEC3_Test, toWireRenderer) {
-    renderer.skip(2);
-    const generic::NSEC3 rdata_nsec3(nsec3_txt);
-    rdata_nsec3.toWire(renderer);
-
-    vector<unsigned char> data;
-    UnitTestUtil::readWireData("rdata_nsec3_fromWire1", data);
-    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
-                        static_cast<const uint8_t *>(obuffer.getData()) + 2,
-                        obuffer.getLength() - 2, &data[2], data.size() - 2);
-}
-
-TEST_F(Rdata_NSEC3_Test, toWireBuffer) {
-    const generic::NSEC3 rdata_nsec3(nsec3_txt);
-    rdata_nsec3.toWire(obuffer);
-}
-
 TEST_F(Rdata_NSEC3_Test, assign) {
     generic::NSEC3 rdata_nsec3(nsec3_txt);
     generic::NSEC3 other_nsec3 = rdata_nsec3;
     EXPECT_EQ(0, rdata_nsec3.compare(other_nsec3));
 }
 
+TEST_F(Rdata_NSEC3_Test, compare) {
+    // trivial case: self equivalence
+    EXPECT_EQ(0, generic::NSEC3(nsec3_txt).compare(generic::NSEC3(nsec3_txt)));
+
+    // comparison attempt between incompatible RR types should be rejected
+    EXPECT_THROW(generic::NSEC3(nsec3_txt).compare(*rdata_nomatch),
+                 bad_cast);
+
+    // test RDATAs, sorted in the ascendent order.  We only check comparison
+    // on NSEC3-specific fields.  Bitmap comparison is tested in the bitmap
+    // tests.  Common cases for NSEC3 and NSECPARAM3 are in their shared tests.
+    vector<generic::NSEC3> compare_set;
+    compare_set.push_back(generic::NSEC3("1 1 1 FF99EA0000 D1K6GQ38"));
+    compare_set.push_back(generic::NSEC3("1 1 1 FF99EA0000 D1K6GQ0000000000"));
+    compare_set.push_back(generic::NSEC3("1 1 1 FF99EA0000 D1K6GQ00UUUUUUUU"));
+
+    vector<generic::NSEC3>::const_iterator it;
+    const vector<generic::NSEC3>::const_iterator it_end = compare_set.end();
+    for (it = compare_set.begin(); it != it_end - 1; ++it) {
+        SCOPED_TRACE("compare " + it->toText() + " to " + (it + 1)->toText());
+        EXPECT_GT(0, (*it).compare(*(it + 1)));
+        EXPECT_LT(0, (*(it + 1)).compare(*it));
+    }
+}
+
 }

src/lib/dns/tests/rdata_nsec3param_like_unittest.cc

@@ -0,0 +1,260 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <dns/exceptions.h>
+#include <dns/rdata.h>
+#include <dns/rdataclass.h>
+#include <dns/rrclass.h>
+#include <dns/rrtype.h>
+
+#include <gtest/gtest.h>
+
+#include <dns/tests/unittest_util.h>
+#include <dns/tests/rdata_unittest.h>
+
+#include <string>
+#include <vector>
+
+using namespace std;
+using isc::UnitTestUtil;
+using namespace isc::dns;
+using namespace isc::dns::rdata;
+
+namespace {
+
+// Template for shared tests for NSEC3 and NSEC3PARAM
+template <typename RDATA_TYPE>
+class NSEC3PARAMLikeTest : public RdataTest {
+protected:
+    NSEC3PARAMLikeTest() :
+        salt_txt("1 1 1 D399EAAB" + getCommonText()),
+        nosalt_txt("1 1 1 -" + getCommonText()),
+        obuffer(0), renderer(obuffer)
+    {}
+
+    RDATA_TYPE fromText(const string& rdata_text) {
+        return (RDATA_TYPE(rdata_text));
+    }
+
+    void compareCheck() const {
+        typename vector<RDATA_TYPE>::const_iterator it;
+        typename vector<RDATA_TYPE>::const_iterator const it_end =
+            compare_set.end();
+        for (it = compare_set.begin(); it != it_end - 1; ++it) {
+            SCOPED_TRACE("compare " + it->toText() + " to " +
+                         (it + 1)->toText());
+            EXPECT_GT(0, (*it).compare(*(it + 1)));
+            EXPECT_LT(0, (*(it + 1)).compare(*it));
+        }
+    }
+
+    const string salt_txt;      // RDATA text with salt
+    const string nosalt_txt;    // RDATA text without salt
+    OutputBuffer obuffer;       // used in toWire() tests
+    MessageRenderer renderer;   // ditto
+    vector<RDATA_TYPE> compare_set; // used in compare() tests
+
+    // Convert generic Rdata to the corresponding derived Rdata class object.
+    // Defined here because it depends on the template parameter.
+    static const RDATA_TYPE& convert(const Rdata& rdata) {
+        return (dynamic_cast<const RDATA_TYPE&>(rdata));
+    }
+
+    // These depend on the specific RR type.  We use specialized methods
+    // for them.
+    static RRType getType(); // return either RRType::NSEC3() or NSEC3PARAM()
+    static string getWireFilePrefix();
+    static string getCommonText(); // commonly used part of textual form
+};
+
+// Instantiate specific typed tests
+typedef ::testing::Types<generic::NSEC3, generic::NSEC3PARAM> TestRdataTypes;
+TYPED_TEST_CASE(NSEC3PARAMLikeTest, TestRdataTypes);
+
+template <>
+RRType
+NSEC3PARAMLikeTest<generic::NSEC3>::getType() {
+    return (RRType::NSEC3());
+}
+
+template <>
+RRType
+NSEC3PARAMLikeTest<generic::NSEC3PARAM>::getType() {
+    return (RRType::NSEC3PARAM());
+}
+
+template <>
+string
+NSEC3PARAMLikeTest<generic::NSEC3>::getWireFilePrefix() {
+    return ("rdata_nsec3_");
+}
+
+template <>
+string
+NSEC3PARAMLikeTest<generic::NSEC3PARAM>::getWireFilePrefix() {
+    return ("rdata_nsec3param_");
+}
+
+template <>
+string
+NSEC3PARAMLikeTest<generic::NSEC3>::getCommonText() {
+    // next hash + RR type bitmap
+    return (" H9RSFB7FPF2L8HG35CMPC765TDK23RP6 "
+            "NS SOA RRSIG DNSKEY NSEC3PARAM");
+}
+
+template <>
+string
+NSEC3PARAMLikeTest<generic::NSEC3PARAM>::getCommonText() {
+    // there's no more text for NSEC3PARAM
+    return ("");
+}
+
+TYPED_TEST(NSEC3PARAMLikeTest, fromText) {
+    // Numeric parameters have possible maximum values.  Unusual, but must
+    // be accepted.
+    EXPECT_NO_THROW(this->fromText("255 255 65535 D399EAAB" +
+                                   this->getCommonText()));
+
+    // 0-length salt
+    EXPECT_EQ(0, this->fromText(this->nosalt_txt).getSalt().size());
+
+    // salt that has the possible max length
+    EXPECT_EQ(255, this->fromText("1 1 1 " + string(255 * 2, '0') +
+                                  this->getCommonText()).getSalt().size());
+}
+
+TYPED_TEST(NSEC3PARAMLikeTest, badText) {
+    // Bad salt hex
+    EXPECT_THROW(this->fromText("1 1 1 SPORK0" + this->getCommonText()),
+                 isc::BadValue);
+    EXPECT_THROW(this->fromText("1 1 1 ADDAFEE" + this->getCommonText()),
+                 isc::BadValue);
+
+    // Space within salt
+    EXPECT_THROW(this->fromText("1 1 1 ADDAFE ADDAFEEE" +
+                                this->getCommonText()),
+                 InvalidRdataText);
+
+    // Similar to empty salt, but not really.  This shouldn't cause confusion.
+    EXPECT_THROW(this->fromText("1 1 1 --" + this->getCommonText()),
+                 isc::BadValue);
+
+    // Too large algorithm
+    EXPECT_THROW(this->fromText("1000000 1 1 ADDAFEEE" + this->getCommonText()),
+                 InvalidRdataText);
+
+    // Too large flags
+    EXPECT_THROW(this->fromText("1 1000000 1 ADDAFEEE" + this->getCommonText()),
+                 InvalidRdataText);
+
+    // Too large iterations
+    EXPECT_THROW(this->fromText("1 1 65536 ADDAFEEE" + this->getCommonText()),
+                 InvalidRdataText);
+
+    // There should be a space between "1" and "D399EAAB" (salt)
+    EXPECT_THROW(this->fromText("1 1 1D399EAAB" + this->getCommonText()),
+                 InvalidRdataText);
+
+    // Salt is too long (possible max + 1 bytes)
+    EXPECT_THROW(this->fromText("1 1 1 " + string(256 * 2, '0') +
+                                this->getCommonText()),
+                 InvalidRdataText);
+}
+
+TYPED_TEST(NSEC3PARAMLikeTest, toText) {
+    // normal case
+    EXPECT_EQ(this->salt_txt, this->fromText(this->salt_txt).toText());
+
+    // empty salt case
+    EXPECT_EQ(this->nosalt_txt, this->fromText(this->nosalt_txt).toText());
+}
+
+TYPED_TEST(NSEC3PARAMLikeTest, createFromWire) {
+    // Normal case
+    EXPECT_EQ(0, this->fromText(this->salt_txt).compare(
+                  *this->rdataFactoryFromFile(this->getType(), RRClass::IN(),
+                                              (this->getWireFilePrefix() +
+                                               "fromWire1").c_str())));
+
+    // Too short RDLENGTH: it doesn't even contain the first 5 octets.
+    EXPECT_THROW(this->rdataFactoryFromFile(this->getType(), RRClass::IN(),
+                                            (this->getWireFilePrefix() +
+                                             "fromWire2.wire").c_str()),
+                 DNSMessageFORMERR);
+
+    // salt length is too large
+    EXPECT_THROW(this->rdataFactoryFromFile(this->getType(), RRClass::IN(),
+                                            (this->getWireFilePrefix() +
+                                             "fromWire11.wire").c_str()),
+                 DNSMessageFORMERR);
+
+    // empty salt.  not so usual, but valid.
+    ConstRdataPtr rdata =
+        this->rdataFactoryFromFile(this->getType(), RRClass::IN(),
+                                   (this->getWireFilePrefix() +
+                                    "fromWire13.wire").c_str());
+    EXPECT_EQ(0, this->convert(*rdata).getSalt().size());
+}
+
+template <typename OUTPUT_TYPE>
+void
+toWireCheck(RRType rrtype, OUTPUT_TYPE& output, const string& data_file) {
+    vector<uint8_t> data;
+    UnitTestUtil::readWireData(data_file.c_str(), data);
+    InputBuffer buffer(&data[0], data.size());
+    const uint16_t rdlen = buffer.readUint16();
+
+    output.clear();
+    output.writeUint16(rdlen);
+    createRdata(rrtype, RRClass::IN(), buffer, rdlen)->toWire(output);
+    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, output.getData(),
+                        output.getLength(), &data[0], data.size());
+}
+
+TYPED_TEST(NSEC3PARAMLikeTest, toWire) {
+    // normal case
+    toWireCheck(this->getType(), this->renderer,
+                this->getWireFilePrefix() + "fromWire1");
+    toWireCheck(this->getType(), this->obuffer,
+                this->getWireFilePrefix() + "fromWire1");
+
+    // empty salt
+    toWireCheck(this->getType(), this->renderer,
+                this->getWireFilePrefix() + "fromWire13.wire");
+    toWireCheck(this->getType(), this->obuffer,
+                this->getWireFilePrefix() + "fromWire13.wire");
+}
+
+TYPED_TEST(NSEC3PARAMLikeTest, compare) {
+    // test RDATAs, sorted in the ascendent order.
+    this->compare_set.push_back(this->fromText("0 0 0 D399EAAB" +
+                                               this->getCommonText()));
+    this->compare_set.push_back(this->fromText("1 0 0 D399EAAB" +
+                                               this->getCommonText()));
+    this->compare_set.push_back(this->fromText("1 1 0 D399EAAB" +
+                                               this->getCommonText()));
+    this->compare_set.push_back(this->fromText("1 1 1 -" +
+                                               this->getCommonText()));
+    this->compare_set.push_back(this->fromText("1 1 1 D399EAAB" +
+                                               this->getCommonText()));
+    this->compare_set.push_back(this->fromText("1 1 1 FF99EAAB" +
+                                               this->getCommonText()));
+    this->compare_set.push_back(this->fromText("1 1 1 FF99EA0000" +
+                                               this->getCommonText()));
+
+    this->compareCheck();
+}
+
+}

src/lib/dns/tests/rdata_nsec3param_unittest.cc

@@ -41,14 +41,14 @@ using namespace isc::dns::rdata;
 namespace {
 class Rdata_NSEC3PARAM_Test : public RdataTest {
 public:
-    Rdata_NSEC3PARAM_Test() : nsec3param_txt("1 0 1 D399EAAB") {}
+    Rdata_NSEC3PARAM_Test() : nsec3param_txt("1 1 1 D399EAAB") {}
     const string nsec3param_txt;
 };
 
 TEST_F(Rdata_NSEC3PARAM_Test, fromText) {
     // With a salt
     EXPECT_EQ(1, generic::NSEC3PARAM(nsec3param_txt).getHashalg());
-    EXPECT_EQ(0, generic::NSEC3PARAM(nsec3param_txt).getFlags());
+    EXPECT_EQ(1, generic::NSEC3PARAM(nsec3param_txt).getFlags());
     // (salt is checked in the toText test)
 
     // With an empty salt
@@ -61,16 +61,9 @@ TEST_F(Rdata_NSEC3PARAM_Test, toText) {
 }
 
 TEST_F(Rdata_NSEC3PARAM_Test, badText) {
-    EXPECT_THROW(generic::NSEC3PARAM("1 1 1 SPORK"), BadValue); // bad hex
-    EXPECT_THROW(generic::NSEC3PARAM("100000 1 1 ADDAFEE"), InvalidRdataText);
-    EXPECT_THROW(generic::NSEC3PARAM("1 100000 1 ADDAFEE"), InvalidRdataText);
-    EXPECT_THROW(generic::NSEC3PARAM("1 1 100000 ADDAFEE"), InvalidRdataText);
-    EXPECT_THROW(generic::NSEC3PARAM("1"), InvalidRdataText);
-}
-
-TEST_F(Rdata_NSEC3PARAM_Test, DISABLED_badText) {
-    // this currently fails
-    EXPECT_THROW(generic::NSEC3PARAM("1 0 1D399EAAB"), InvalidRdataText);
+    // garbage space at the end
+    EXPECT_THROW(generic::NSEC3PARAM("1 1 1 D399EAAB "),
+                 InvalidRdataText);
 }
 
 TEST_F(Rdata_NSEC3PARAM_Test, createFromWire) {
@@ -78,6 +71,19 @@ TEST_F(Rdata_NSEC3PARAM_Test, createFromWire) {
     EXPECT_EQ(0, rdata_nsec3param.compare(
                   *rdataFactoryFromFile(RRType::NSEC3PARAM(), RRClass::IN(),
                                        "rdata_nsec3param_fromWire1")));
+
+    // Short buffer cases.  The data is valid NSEC3PARAM RDATA, but the buffer
+    // is trimmed at the end.  All cases should result in an exception from
+    // the buffer class.
+    vector<uint8_t> data;
+    UnitTestUtil::readWireData("rdata_nsec3param_fromWire1", data);
+    const uint16_t rdlen = (data.at(0) << 8) + data.at(1);
+    for (int i = 0; i < rdlen; ++i) {
+        // intentionally construct a short buffer
+        InputBuffer b(&data[0] + 2, i);
+        EXPECT_THROW(createRdata(RRType::NSEC3PARAM(), RRClass::IN(), b, 9),
+                     InvalidBufferPosition);
+    }
 }
 
 TEST_F(Rdata_NSEC3PARAM_Test, toWireRenderer) {
@@ -103,4 +109,16 @@ TEST_F(Rdata_NSEC3PARAM_Test, assign) {
     EXPECT_EQ(0, rdata_nsec3param.compare(other_nsec3param));
 }
 
+TEST_F(Rdata_NSEC3PARAM_Test, compare) {
+    // trivial case: self equivalence
+    EXPECT_EQ(0, generic::NSEC3PARAM(nsec3param_txt).
+              compare(generic::NSEC3PARAM(nsec3param_txt)));
+    EXPECT_EQ(0, generic::NSEC3PARAM("1 1 1 -").
+              compare(generic::NSEC3PARAM("1 1 1 -")));
+
+    // comparison attempt between incompatible RR types should be rejected
+    EXPECT_THROW(generic::NSEC3PARAM(nsec3param_txt).compare(*rdata_nomatch),
+                 bad_cast);
+}
+
 }

src/lib/dns/tests/rdata_nsec_unittest.cc

@@ -38,7 +38,7 @@ class Rdata_NSEC_Test : public RdataTest {
     // there's nothing to specialize
 };
 
-string nsec_txt("www2.isc.org. CNAME RRSIG NSEC");
+const char* const nsec_txt = "www2.isc.org. CNAME RRSIG NSEC";
 
 TEST_F(Rdata_NSEC_Test, toText_NSEC) {
     const generic::NSEC rdata_nsec(nsec_txt);
@@ -95,4 +95,31 @@ TEST_F(Rdata_NSEC_Test, getNextName) {
     EXPECT_EQ(Name("www2.isc.org"), generic::NSEC((nsec_txt)).getNextName());
 }
 
+TEST_F(Rdata_NSEC_Test, compare) {
+    // trivial case: self equivalence
+    EXPECT_EQ(0, generic::NSEC("example A").
+              compare(generic::NSEC("example. A")));
+    EXPECT_EQ(0, generic::NSEC("EXAMPLE A"). // should be case insensitive
+              compare(generic::NSEC("example. A")));
+
+    // comparison attempt between incompatible RR types should be rejected
+    EXPECT_THROW(generic::NSEC(nsec_txt).compare(*rdata_nomatch),
+                 bad_cast);
+
+    // test RDATAs, sorted in the ascendent order.  We only compare the
+    // next name here.  Bitmap comparison is tested in the bitmap tests.
+    // Note that names are compared as wire-format data, not based on the
+    // domain name comparison.
+    vector<generic::NSEC> compare_set;
+    compare_set.push_back(generic::NSEC("a.example. A"));
+    compare_set.push_back(generic::NSEC("example. A"));
+    vector<generic::NSEC>::const_iterator it;
+    const vector<generic::NSEC>::const_iterator it_end = compare_set.end();
+    for (it = compare_set.begin(); it != it_end - 1; ++it) {
+        SCOPED_TRACE("compare " + it->toText() + " to " + (it + 1)->toText());
+        EXPECT_GT(0, (*it).compare(*(it + 1)));
+        EXPECT_LT(0, (*(it + 1)).compare(*it));
+    }
+}
+
 }

src/lib/dns/tests/rdata_nsecbitmap_unittest.cc

@@ -12,6 +12,8 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <dns/tests/unittest_util.h>
+
 #include <dns/exceptions.h>
 #include <dns/rdata.h>
 #include <dns/rdataclass.h>
@@ -22,82 +24,251 @@
 
 #include <dns/tests/rdata_unittest.h>
 
+#include <boost/lexical_cast.hpp>
+
+#include <string>
+#include <vector>
+
+using namespace std;
+using boost::lexical_cast;
+using isc::UnitTestUtil;
 using namespace isc::dns;
 using namespace isc::dns::rdata;
 
 namespace {
-class Rdata_NSECBITMAP_Test : public RdataTest {
-    // there's nothing to specialize
+
+// Template for shared tests for NSEC and NSEC3 bitmaps
+template <typename RDATA_TYPE>
+class NSECLikeBitmapTest : public RdataTest {
+protected:
+    RDATA_TYPE fromText(const string& rdata_text) {
+        return (RDATA_TYPE(rdata_text));
+    }
+
+    vector<RDATA_TYPE> compare_set; // used in compare() tests
+
+    void compareCheck() const {
+        typename vector<RDATA_TYPE>::const_iterator it;
+        typename vector<RDATA_TYPE>::const_iterator const it_end =
+            compare_set.end();
+        for (it = compare_set.begin(); it != it_end - 1; ++it) {
+            SCOPED_TRACE("compare " + it->toText() + " to " +
+                         (it + 1)->toText());
+            EXPECT_GT(0, (*it).compare(*(it + 1)));
+            EXPECT_LT(0, (*(it + 1)).compare(*it));
+        }
+    }
+
+    // These depend on the specific RR type.  We use specialized methods
+    // for them.
+    static RRType getType();    // return either RRType::NSEC() or NSEC3()
+    static string getWireFilePrefix();
+    static string getCommonText(); // commonly used part of textual form
 };
 
+// Instantiate specific typed tests
+typedef ::testing::Types<generic::NSEC, generic::NSEC3> TestRdataTypes;
+TYPED_TEST_CASE(NSECLikeBitmapTest, TestRdataTypes);
+
+// NSEC and NSEC3 bitmaps have some subtle differences, in which case we
+// need to test them separately.  Using these typedef type names with TEST_F
+// will do the trick.
+typedef NSECLikeBitmapTest<generic::NSEC3> NSEC3BitmapTest;
+typedef NSECLikeBitmapTest<generic::NSEC> NSECBitmapTest;
+
+template <>
+string
+NSECLikeBitmapTest<generic::NSEC>::getWireFilePrefix() {
+    return ("rdata_nsec_");
+}
+
+template <>
+RRType
+NSECLikeBitmapTest<generic::NSEC>::getType() {
+    return (RRType::NSEC());
+}
+
+template <>
+string
+NSECLikeBitmapTest<generic::NSEC3>::getWireFilePrefix() {
+    return ("rdata_nsec3_");
+}
+
+template <>
+RRType
+NSECLikeBitmapTest<generic::NSEC3>::getType() {
+    return (RRType::NSEC3());
+}
+
+template <>
+string
+NSECLikeBitmapTest<generic::NSEC>::getCommonText() {
+    return ("next. ");
+}
+
+template <>
+string
+NSECLikeBitmapTest<generic::NSEC3>::getCommonText() {
+    return ("1 1 12 AABBCCDD 2T7B4G4VSA5SMI47K61MV5BV1A22BOJR ");
+}
+
 // Tests against various types of bogus NSEC/NSEC3 type bitmaps.
 // The syntax and semantics are common for both RR types, and our
 // implementation of that part is shared, so in theory it should be sufficient
 // to test for only one RR type.  But we check for both just in case.
-TEST_F(Rdata_NSECBITMAP_Test, createFromWire_NSEC) {
+TYPED_TEST(NSECLikeBitmapTest, createFromWire) {
     // A malformed NSEC bitmap length field that could cause overflow.
-    EXPECT_THROW(rdataFactoryFromFile(RRType::NSEC(), RRClass::IN(),
-                                      "rdata_nsec_fromWire4.wire"),
-                 DNSMessageFORMERR);
-    EXPECT_THROW(rdataFactoryFromFile(RRType::NSEC3(), RRClass::IN(),
-                                      "rdata_nsec3_fromWire4.wire"),
+    EXPECT_THROW(this->rdataFactoryFromFile(this->getType(), RRClass::IN(),
+                                            (this->getWireFilePrefix() +
+                                             "fromWire4.wire").c_str()),
                  DNSMessageFORMERR);
 
     // The bitmap field is incomplete (only the first byte is included)
-    EXPECT_THROW(rdataFactoryFromFile(RRType::NSEC(), RRClass::IN(),
-                                      "rdata_nsec_fromWire5.wire"),
-                 DNSMessageFORMERR);
-    EXPECT_THROW(rdataFactoryFromFile(RRType::NSEC3(), RRClass::IN(),
-                                      "rdata_nsec3_fromWire5.wire"),
+    EXPECT_THROW(this->rdataFactoryFromFile(this->getType(), RRClass::IN(),
+                                            (this->getWireFilePrefix() +
+                                             "fromWire5.wire").c_str()),
                  DNSMessageFORMERR);
 
     // Bitmap length is 0, which is invalid.
-    EXPECT_THROW(rdataFactoryFromFile(RRType::NSEC(), RRClass::IN(),
-                                      "rdata_nsec_fromWire6.wire"),
-                 DNSMessageFORMERR);
-    EXPECT_THROW(rdataFactoryFromFile(RRType::NSEC3(), RRClass::IN(),
-                                      "rdata_nsec3_fromWire6.wire"),
+    EXPECT_THROW(this->rdataFactoryFromFile(this->getType(), RRClass::IN(),
+                                            (this->getWireFilePrefix() +
+                                             "fromWire6.wire").c_str()),
                  DNSMessageFORMERR);
 
     // Too large bitmap length with a short buffer.
-    EXPECT_THROW(rdataFactoryFromFile(RRType::NSEC(), RRClass::IN(),
-                                      "rdata_nsec_fromWire3"),
-                 DNSMessageFORMERR);
-    EXPECT_THROW(rdataFactoryFromFile(RRType::NSEC3(), RRClass::IN(),
-                                      "rdata_nsec3_fromWire3"),
+    EXPECT_THROW(this->rdataFactoryFromFile(this->getType(), RRClass::IN(),
+                                            (this->getWireFilePrefix() +
+                                             "fromWire3").c_str()),
                  DNSMessageFORMERR);
 
     // A boundary case: longest possible bitmaps (32 maps).  This should be
     // accepted.
-    EXPECT_NO_THROW(rdataFactoryFromFile(RRType::NSEC(), RRClass::IN(),
-                                         "rdata_nsec_fromWire7.wire"));
-    EXPECT_NO_THROW(rdataFactoryFromFile(RRType::NSEC3(), RRClass::IN(),
-                                         "rdata_nsec3_fromWire7.wire"));
+    EXPECT_NO_THROW(this->rdataFactoryFromFile(this->getType(), RRClass::IN(),
+                                               (this->getWireFilePrefix() +
+                                                "fromWire7.wire").c_str()));
 
     // Another boundary condition: 33 bitmaps, which should be rejected.
-    EXPECT_THROW(rdataFactoryFromFile(RRType::NSEC(), RRClass::IN(),
-                                      "rdata_nsec_fromWire8.wire"),
-                 DNSMessageFORMERR);
-    EXPECT_THROW(rdataFactoryFromFile(RRType::NSEC3(), RRClass::IN(),
-                                      "rdata_nsec3_fromWire8.wire"),
+    EXPECT_THROW(this->rdataFactoryFromFile(this->getType(), RRClass::IN(),
+                                            (this->getWireFilePrefix() +
+                                             "fromWire8.wire").c_str()),
                  DNSMessageFORMERR);
 
     // Disordered bitmap window blocks.
-    EXPECT_THROW(rdataFactoryFromFile(RRType::NSEC(), RRClass::IN(),
-                                      "rdata_nsec_fromWire9.wire"),
-                 DNSMessageFORMERR);
-    EXPECT_THROW(rdataFactoryFromFile(RRType::NSEC3(), RRClass::IN(),
-                                      "rdata_nsec3_fromWire9.wire"),
+    EXPECT_THROW(this->rdataFactoryFromFile(this->getType(), RRClass::IN(),
+                                            (this->getWireFilePrefix() +
+                                             "fromWire9.wire").c_str()),
                  DNSMessageFORMERR);
 
     // Bitmap ending with all-zero bytes.  Not necessarily harmful except
     // the additional overhead of parsing, but invalid according to the
     // spec anyway.
-    EXPECT_THROW(rdataFactoryFromFile(RRType::NSEC(), RRClass::IN(),
-                                      "rdata_nsec_fromWire10.wire"),
+    EXPECT_THROW(this->rdataFactoryFromFile(this->getType(), RRClass::IN(),
+                                            (this->getWireFilePrefix() +
+                                             "fromWire10.wire").c_str()),
                  DNSMessageFORMERR);
-    EXPECT_THROW(rdataFactoryFromFile(RRType::NSEC3(), RRClass::IN(),
-                                      "rdata_nsec3_fromWire10.wire"),
+}
+
+TYPED_TEST(NSECLikeBitmapTest, badText) {
+    // redundant space after the sequence
+    EXPECT_THROW(this->fromText(this->getCommonText() + "A "),
+                 InvalidRdataText);
+}
+
+// This tests the result of toText() with various kinds of NSEC/NSEC3 bitmaps.
+// It also tests the "from text" constructor as a result.
+TYPED_TEST(NSECLikeBitmapTest, toText) {
+    // A simple case (some commonly seen RR types in NSEC(3) bitmaps)
+    string rdata_text = this->getCommonText() + "NS SOA RRSIG DNSKEY";
+    EXPECT_EQ(rdata_text, this->fromText(rdata_text).toText());
+
+    // Similar to above, but involves more than one bitmap window blocks.
+    rdata_text = this->getCommonText() + "NS DLV";
+    EXPECT_EQ(rdata_text, this->fromText(rdata_text).toText());
+
+    // Make sure all possible bits in a one-octet bitmap field are handled
+    // correctly.
+    // We use the range around 1024 (reasonably higher number) so it's
+    // unlikely that they have predefined mnemonic and can be safely converted
+    // to TYPEnnnn by toText().
+    for (unsigned int i = 1024; i < 1032; ++i) {
+        rdata_text = this->getCommonText() + "TYPE" + lexical_cast<string>(i);
+        EXPECT_EQ(rdata_text, this->fromText(rdata_text).toText());
+    }
+
+    // Make sure all possible 32 octets in a longest possible block are
+    // handled correctly.
+    for (unsigned int i = 1024; i < 1024 + 256; i += 8) {
+        rdata_text = this->getCommonText() + "TYPE" + lexical_cast<string>(i);
+        EXPECT_EQ(rdata_text, this->fromText(rdata_text).toText());
+    }
+
+    // Check for the highest window block.
+    rdata_text = this->getCommonText() + "TYPE65535";
+    EXPECT_EQ(rdata_text, this->fromText(rdata_text).toText());
+}
+
+TYPED_TEST(NSECLikeBitmapTest, compare) {
+    // Bit map: [win=0][len=1] 00000010
+    this->compare_set.push_back(this->fromText(this->getCommonText() + "SOA"));
+    // Bit map: [win=0][len=1] 00000010, [win=4][len=1] 10000000
+    this->compare_set.push_back(this->fromText(this->getCommonText() +
+                                               "SOA TYPE1024"));
+    // Bit map: [win=0][len=1] 00100000
+    this->compare_set.push_back(this->fromText(this->getCommonText() + "NS"));
+    // Bit map: [win=0][len=1] 00100010
+    this->compare_set.push_back(this->fromText(this->getCommonText() +
+                                               "NS SOA"));
+    // Bit map: [win=0][len=2] 00100000, 00000001
+    this->compare_set.push_back(this->fromText(this->getCommonText() +
+                                               "NS MX"));
+    // Bit map: [win=4][len=1] 10000000
+    this->compare_set.push_back(this->fromText(this->getCommonText() +
+                                               "TYPE1024"));
+
+    this->compareCheck();
+}
+
+// NSEC bitmaps must not be empty
+TEST_F(NSECBitmapTest, emptyMap) {
+    EXPECT_THROW(this->fromText("next.example").toText(), InvalidRdataText);
+
+    EXPECT_THROW(this->rdataFactoryFromFile(this->getType(), RRClass::IN(),
+                                            (this->getWireFilePrefix() +
+                                             "fromWire16.wire").c_str()),
                  DNSMessageFORMERR);
 }
+
+// NSEC3 bitmaps can be empty
+TEST_F(NSEC3BitmapTest, emptyMap) {
+    // Read wire data wit an empty NSEC3 bitmap.  This should succeed.
+    vector<uint8_t> data;
+    UnitTestUtil::readWireData((this->getWireFilePrefix() +
+                                "fromWire16.wire").c_str(), data);
+    InputBuffer buffer(&data[0], data.size());
+    const uint16_t rdlen = buffer.readUint16();
+    const generic::NSEC3 empty_nsec3 =
+        dynamic_cast<const generic::NSEC3&>(*createRdata(
+                                                RRType::NSEC3(), RRClass::IN(),
+                                                buffer, rdlen));
+
+    // Check the toText() result.
+    EXPECT_EQ("1 0 1 7373737373 D1K6GQ38D1K6GQ38D1K6GQ38D1K6GQ38",
+              empty_nsec3.toText());
+
+    // Check the toWire() result.
+    OutputBuffer obuffer(0);
+    obuffer.writeUint16(rdlen);
+    empty_nsec3.toWire(obuffer);
+    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, obuffer.getData(),
+                        obuffer.getLength(), &data[0], data.size());
+
+    // Same for MessageRenderer.
+    obuffer.clear();
+    MessageRenderer renderer(obuffer);
+    renderer.writeUint16(rdlen);
+    empty_nsec3.toWire(renderer);
+    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, renderer.getData(),
+                        renderer.getLength(), &data[0], data.size());
+}
+
 }

src/lib/dns/tests/testdata/Makefile.am

@@ -20,6 +20,8 @@ BUILT_SOURCES += rdata_nsec_fromWire4.wire rdata_nsec_fromWire5.wire
 BUILT_SOURCES += rdata_nsec_fromWire6.wire rdata_nsec_fromWire7.wire
 BUILT_SOURCES += rdata_nsec_fromWire8.wire rdata_nsec_fromWire9.wire
 BUILT_SOURCES += rdata_nsec_fromWire10.wire
+# 11-15 are skipped to be consistent with NSEC3 test data
+BUILT_SOURCES += rdata_nsec_fromWire16.wire
 BUILT_SOURCES += rdata_nsec3_fromWire2.wire
 BUILT_SOURCES += rdata_nsec3_fromWire4.wire rdata_nsec3_fromWire5.wire
 BUILT_SOURCES += rdata_nsec3_fromWire6.wire rdata_nsec3_fromWire7.wire
@@ -27,6 +29,10 @@ BUILT_SOURCES += rdata_nsec3_fromWire8.wire rdata_nsec3_fromWire9.wire
 BUILT_SOURCES += rdata_nsec3_fromWire10.wire rdata_nsec3_fromWire11.wire
 BUILT_SOURCES += rdata_nsec3_fromWire12.wire rdata_nsec3_fromWire13.wire
 BUILT_SOURCES += rdata_nsec3_fromWire14.wire rdata_nsec3_fromWire15.wire
+BUILT_SOURCES += rdata_nsec3_fromWire16.wire rdata_nsec3_fromWire17.wire
+BUILT_SOURCES += rdata_nsec3param_fromWire2.wire
+BUILT_SOURCES += rdata_nsec3param_fromWire11.wire
+BUILT_SOURCES += rdata_nsec3param_fromWire13.wire
 BUILT_SOURCES += rdata_rrsig_fromWire2.wire
 BUILT_SOURCES += rdata_minfo_fromWire1.wire rdata_minfo_fromWire2.wire
 BUILT_SOURCES += rdata_minfo_fromWire3.wire rdata_minfo_fromWire4.wire
@@ -99,7 +105,11 @@ EXTRA_DIST += rdata_nsec_fromWire4.spec rdata_nsec_fromWire5.spec
 EXTRA_DIST += rdata_nsec_fromWire6.spec rdata_nsec_fromWire7.spec
 EXTRA_DIST += rdata_nsec_fromWire8.spec rdata_nsec_fromWire9.spec
 EXTRA_DIST += rdata_nsec_fromWire10.spec
+EXTRA_DIST += rdata_nsec_fromWire16.spec
 EXTRA_DIST += rdata_nsec3param_fromWire1
+EXTRA_DIST += rdata_nsec3param_fromWire2.spec
+EXTRA_DIST += rdata_nsec3param_fromWire11.spec
+EXTRA_DIST += rdata_nsec3param_fromWire13.spec
 EXTRA_DIST += rdata_nsec3_fromWire1
 EXTRA_DIST += rdata_nsec3_fromWire2.spec rdata_nsec3_fromWire3
 EXTRA_DIST += rdata_nsec3_fromWire4.spec rdata_nsec3_fromWire5.spec
@@ -108,6 +118,7 @@ EXTRA_DIST += rdata_nsec3_fromWire8.spec rdata_nsec3_fromWire9.spec
 EXTRA_DIST += rdata_nsec3_fromWire10.spec rdata_nsec3_fromWire11.spec
 EXTRA_DIST += rdata_nsec3_fromWire12.spec rdata_nsec3_fromWire13.spec
 EXTRA_DIST += rdata_nsec3_fromWire14.spec rdata_nsec3_fromWire15.spec
+EXTRA_DIST += rdata_nsec3_fromWire16.spec rdata_nsec3_fromWire17.spec
 EXTRA_DIST += rdata_opt_fromWire rdata_rrsig_fromWire1
 EXTRA_DIST += rdata_rrsig_fromWire2.spec
 EXTRA_DIST += rdata_rp_fromWire1.spec rdata_rp_fromWire2.spec

src/lib/dns/tests/testdata/rdata_nsec3_fromWire16.spec

@@ -0,0 +1,8 @@
+#
+# NSEC3 RDATA with an empty bitmap
+#
+
+[custom]
+sections: nsec3
+[nsec3]
+nbitmap: 0

src/lib/dns/tests/testdata/rdata_nsec3_fromWire17.spec

@@ -0,0 +1,8 @@
+#
+# An invalid NSEC3 RDATA: RDLEN is too short to include the hash len field.
+#
+
+[custom]
+sections: nsec3
+[nsec3]
+rdlen: 10

src/lib/dns/tests/testdata/rdata_nsec3param_fromWire1

@@ -1,5 +1,5 @@
 # RDLENGTH, 9 bytes
 00 09
 # NSEC3PARAM record
-# 1 0 1 D399EAAB
-01 00 00 01 04 d3 99 ea ab
+# 1 1 1 D399EAAB
+01 01 00 01 04 d3 99 ea ab

src/lib/dns/tests/testdata/rdata_nsec3param_fromWire11.spec

@@ -0,0 +1,8 @@
+#
+# An invalid NSEC3PARAM RDATA: Saltlen is too large
+#
+
+[custom]
+sections: nsec3param
+[nsec3param]
+rdlen: 7

src/lib/dns/tests/testdata/rdata_nsec3param_fromWire13.spec

@@ -0,0 +1,9 @@
+#
+# A valid (but unusual) NSEC3PARAM RDATA: salt is empty.
+#
+
+[custom]
+sections: nsec3param
+[nsec3param]
+saltlen: 0
+salt: ''

src/lib/dns/tests/testdata/rdata_nsec3param_fromWire2.spec

@@ -0,0 +1,9 @@
+#
+# A malformed NSEC3PARAM RDATA: RDLEN indicates it doesn't even contain the
+# fixed 5 octects
+#
+
+[custom]
+sections: nsec3param
+[nsec3param]
+rdlen: 4

src/lib/dns/tests/testdata/rdata_nsec_fromWire16.spec

@@ -0,0 +1,8 @@
+#
+# An invalid NSEC RDATA: with an empty bitmap
+#
+
+[custom]
+sections: nsec
+[nsec]
+nbitmap: 0

src/lib/python/isc/bind10/component.py

@@ -22,9 +22,9 @@ Dependencies between them are not yet handled. It might turn out they are
 needed, in that case they will be added sometime in future.
 
 This framework allows for a single process to be started multiple times (by
-specifying multiple components with the same configuration). However, the rest
-of the system might not handle such situation well, so until it is made so,
-it would be better to start each process at most once.
+specifying multiple components with the same configuration). We might want
+to add a more convenient support (like providing a count argument to the
+configuration). This is yet to be designed.
 """
 
 import isc.log
@@ -408,7 +408,7 @@ class Component(BaseComponent):
         self._boss.register_process(self.pid(), self)
 
     def _stop_internal(self):
-        self._boss.stop_process(self._process, self._address)
+        self._boss.stop_process(self._process, self._address, self.pid())
         # TODO Some way to wait for the process that doesn't want to
         # terminate and kill it would prove nice (or add it to boss somewhere?)
 

src/lib/python/isc/bind10/tests/component_test.py

@@ -553,11 +553,11 @@ class ComponentTests(BossUtils, unittest.TestCase):
         self.assertEqual(42, component.pid())
         self.assertEqual(component, self.__registered_processes.get(42))
 
-    def stop_process(self, process, address):
+    def stop_process(self, process, address, pid):
         """
         Part of pretending to be boss.
         """
-        self.__stop_process_params = (process, address)
+        self.__stop_process_params = (process, address, pid)
 
     def start_simple(self, process):
         """
@@ -573,9 +573,11 @@ class ComponentTests(BossUtils, unittest.TestCase):
         component.start()
         self.assertTrue(component.running())
         self.assertEqual('component', self.__start_simple_params)
+        component.pid = lambda: 42
         component.stop()
         self.assertFalse(component.running())
-        self.assertEqual(('component', 'Address'), self.__stop_process_params)
+        self.assertEqual(('component', 'Address', 42),
+                         self.__stop_process_params)
 
     def test_component_kill(self):
         """

src/lib/python/isc/cc/data.py

@@ -21,6 +21,7 @@
 #
 
 import json
+import re
 
 class DataNotFoundError(Exception):
     """Raised if an identifier does not exist according to a spec file,
@@ -86,6 +87,13 @@ def split_identifier(identifier):
     id_parts[:] = (value for value in id_parts if value != "")
     return id_parts
 
+def identifier_has_list_index(identifier):
+    """Returns True if the given identifier string has at least one
+       list index (with [I], where I is a number"""
+    return (type(identifier) == str and
+            re.search("\[\d+\]", identifier) is not None)
+
+
 def split_identifier_list_indices(identifier):
     """Finds list indexes in the given identifier, which are of the
        format [integer].

src/lib/python/isc/config/config_data.py

@@ -28,6 +28,22 @@ class ConfigDataError(Exception): pass
 
 BIND10_CONFIG_DATA_VERSION = 2
 
+# Helper functions
+def spec_part_is_list(spec_part):
+    """Returns True if the given spec_part is a dict that contains a
+       list specification, and False otherwise."""
+    return (type(spec_part) == dict and 'list_item_spec' in spec_part)
+
+def spec_part_is_map(spec_part):
+    """Returns True if the given spec_part is a dict that contains a
+       map specification, and False otherwise."""
+    return (type(spec_part) == dict and 'map_item_spec' in spec_part)
+
+def spec_part_is_named_set(spec_part):
+    """Returns True if the given spec_part is a dict that contains a
+       named_set specification, and False otherwise."""
+    return (type(spec_part) == dict and 'named_map_item_spec' in spec_part)
+
 def check_type(spec_part, value):
     """Does nothing if the value is of the correct type given the
        specification part relevant for the value. Raises an
@@ -112,9 +128,9 @@ def _get_map_or_list(spec_part):
     """Returns the list or map specification if this is a list or a
        map specification part. If not, returns the given spec_part
        itself"""
-    if "map_item_spec" in spec_part:
+    if spec_part_is_map(spec_part):
         return spec_part["map_item_spec"]
-    elif "list_item_spec" in spec_part:
+    elif spec_part_is_list(spec_part):
         return spec_part["list_item_spec"]
     else:
         return spec_part
@@ -134,13 +150,13 @@ def _find_spec_part_single(cur_spec, id_part):
     # list or a map, which is internally represented by a dict with
     # an element 'map_item_spec', a dict with an element 'list_item_spec',
     # or a list (when it is the 'main' config_data element of a module).
-    if type(cur_spec) == dict and 'map_item_spec' in cur_spec.keys():
+    if spec_part_is_map(cur_spec):
         for cur_spec_item in cur_spec['map_item_spec']:
             if cur_spec_item['item_name'] == id:
                 return cur_spec_item
         # not found
         raise isc.cc.data.DataNotFoundError(id + " not found")
-    elif type(cur_spec) == dict and 'list_item_spec' in cur_spec.keys():
+    elif spec_part_is_list(cur_spec):
         if cur_spec['item_name'] == id:
             return cur_spec['list_item_spec']
         # not found
@@ -156,9 +172,22 @@ def _find_spec_part_single(cur_spec, id_part):
     else:
         raise isc.cc.data.DataNotFoundError("Not a correct config specification")
 
-def find_spec_part(element, identifier):
+def find_spec_part(element, identifier, strict_identifier = True):
     """find the data definition for the given identifier
-       returns either a map with 'item_name' etc, or a list of those"""
+       returns either a map with 'item_name' etc, or a list of those
+       Parameters:
+       element: The specification element to start the search in
+       identifier: The element to find (relative to element above)
+       strict_identifier: If True (the default), additional checking occurs.
+                          Currently the only check is whether a list index is
+                          specified (except for the last part of the
+                          identifier)
+       Raises a DataNotFoundError if the data is not found, or if
+       strict_identifier is True and any non-final identifier parts
+       (i.e. before the last /) identify a list element and do not contain
+       an index.
+       Returns the spec element identified by the given identifier.
+    """
     if identifier == "":
         return element
     id_parts = identifier.split("/")
@@ -171,6 +200,10 @@ def find_spec_part(element, identifier):
     # always want the 'full' spec of the item
     for id_part in id_parts[:-1]:
         cur_el = _find_spec_part_single(cur_el, id_part)
+        if strict_identifier and spec_part_is_list(cur_el) and\
+           not isc.cc.data.identifier_has_list_index(id_part):
+            raise isc.cc.data.DataNotFoundError(id_part +
+                                                " is a list and needs an index")
         cur_el = _get_map_or_list(cur_el)
 
     cur_el = _find_spec_part_single(cur_el, id_parts[-1])
@@ -184,12 +217,12 @@ def spec_name_list(spec, prefix="", recurse=False):
     if prefix != "" and not prefix.endswith("/"):
         prefix += "/"
     if type(spec) == dict:
-        if 'map_item_spec' in spec:
+        if spec_part_is_map(spec):
             for map_el in spec['map_item_spec']:
                 name = map_el['item_name']
                 if map_el['item_type'] == 'map':
                     name += "/"
-                if recurse and 'map_item_spec' in map_el:
+                if recurse and spec_part_is_map(map_el):
                     result.extend(spec_name_list(map_el['map_item_spec'], prefix + map_el['item_name'], recurse))
                 else:
                     result.append(prefix + name)
@@ -244,7 +277,12 @@ class ConfigData:
     def get_default_value(self, identifier):
         """Returns the default from the specification, or None if there
            is no default"""
-        spec = find_spec_part(self.specification.get_config_spec(), identifier)
+        # We are searching for the default value, so we can set
+        # strict_identifier to false (in fact, we need to; we may not know
+        # some list indices, or they may not exist, we are looking for
+        # a default value for a reason here).
+        spec = find_spec_part(self.specification.get_config_spec(),
+                              identifier, False)
         if spec and 'item_default' in spec:
             return spec['item_default']
         else:
@@ -607,7 +645,7 @@ class MultiConfigData:
            Throws DataNotFoundError if the identifier is bad
         """
         result = []
-        if not identifier:
+        if not identifier or identifier == "/":
             # No identifier, so we need the list of current modules
             for module in self._specifications.keys():
                 if all:
@@ -619,8 +657,11 @@ class MultiConfigData:
                     entry = _create_value_map_entry(module, 'module', None)
                     result.append(entry)
         else:
-            if identifier[0] == '/':
+            # Strip off start and end slashes, if they are there
+            if len(identifier) > 0 and identifier[0] == '/':
                 identifier = identifier[1:]
+            if len(identifier) > 0 and identifier[-1] == '/':
+                identifier = identifier[:-1]
             module, sep, id = identifier.partition('/')
             spec = self.get_module_spec(module)
             if spec:

src/lib/python/isc/config/tests/config_data_test.py

@@ -185,6 +185,43 @@ class TestConfigData(unittest.TestCase):
         spec_part = find_spec_part(config_spec, "item6/value1")
         self.assertEqual({'item_name': 'value1', 'item_type': 'string', 'item_optional': True, 'item_default': 'default'}, spec_part)
 
+    def test_find_spec_part_lists(self):
+        # A few specific tests for list data
+        module_spec = isc.config.module_spec_from_file(self.data_path +
+                                                       os.sep +
+                                                       "spec31.spec")
+        config_spec = module_spec.get_config_spec()
+
+        expected_spec_part = {'item_name': 'number',
+                              'item_type': 'integer',
+                              'item_default': 1,
+                              'item_optional': False}
+
+        # First a check for a correct fetch
+        spec_part = find_spec_part(config_spec,
+                                   "/first_list_items[0]/second_list_items[1]/"
+                                   "map_element/list1[1]/list2[1]")
+        self.assertEqual(expected_spec_part, spec_part)
+
+        # Leaving out an index should fail by default
+        self.assertRaises(isc.cc.data.DataNotFoundError,
+                          find_spec_part, config_spec,
+                          "/first_list_items[0]/second_list_items/"
+                          "map_element/list1[1]/list2[1]")
+
+        # But not for the last element
+        spec_part = find_spec_part(config_spec,
+                                   "/first_list_items[0]/second_list_items[1]/"
+                                   "map_element/list1[1]/list2")
+        self.assertEqual(expected_spec_part, spec_part)
+
+        # And also not if strict_identifier is false (third argument)
+        spec_part = find_spec_part(config_spec,
+                                   "/first_list_items[0]/second_list_items/"
+                                   "map_element/list1[1]/list2[1]", False)
+        self.assertEqual(expected_spec_part, spec_part)
+
+
     def test_spec_name_list(self):
         name_list = spec_name_list(self.cd.get_module_spec().get_config_spec())
         self.assertEqual(['item1', 'item2', 'item3', 'item4', 'item5', 'item6'], name_list)
@@ -483,15 +520,25 @@ class TestMultiConfigData(unittest.TestCase):
         self.assertEqual(MultiConfigData.DEFAULT, status)
 
 
-
     def test_get_value_maps(self):
         maps = self.mcd.get_value_maps()
         self.assertEqual([], maps)
         
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec1.spec")
         self.mcd.set_specification(module_spec)
+
+        expected = [{'default': False,
+                     'type': 'module',
+                     'name': 'Spec1',
+                     'value': None,
+                     'modified': False}]
+
         maps = self.mcd.get_value_maps()
-        self.assertEqual([{'default': False, 'type': 'module', 'name': 'Spec1', 'value': None, 'modified': False}], maps)
+        self.assertEqual(expected, maps)
+
+        maps = self.mcd.get_value_maps("/")
+        self.assertEqual(expected, maps)
+
         maps = self.mcd.get_value_maps('Spec2')
         self.assertEqual([], maps)
         maps = self.mcd.get_value_maps('Spec1')
@@ -566,6 +613,10 @@ class TestMultiConfigData(unittest.TestCase):
         maps = self.mcd.get_value_maps("/Spec22/value9")
         self.assertEqual(expected, maps)
 
+        # A slash at the end should not produce different output
+        maps = self.mcd.get_value_maps("/Spec22/value9/")
+        self.assertEqual(expected, maps)
+
     def test_get_value_maps_named_set(self):
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec32.spec")
         self.mcd.set_specification(module_spec)

src/lib/testutils/srv_test.cc

@@ -86,6 +86,7 @@ SrvTestBase::createRequestPacket(Message& message,
                                   IOAddress(DEFAULT_REMOTE_ADDRESS), 53210);
     io_sock = (protocol == IPPROTO_UDP) ? &IOSocket::getDummyUDPSocket() :
         &IOSocket::getDummyTCPSocket();
+
     io_message = new IOMessage(request_renderer.getData(),
                                request_renderer.getLength(),
                                *io_sock, *endpoint);

src/lib/testutils/testdata/Makefile.am

@@ -5,6 +5,7 @@ BUILT_SOURCES += iqueryresponse_fromWire.wire multiquestion_fromWire.wire
 BUILT_SOURCES += queryBadEDNS_fromWire.wire shortanswer_fromWire.wire
 BUILT_SOURCES += simplequery_fromWire.wire simpleresponse_fromWire.wire
 BUILT_SOURCES += iquery_fromWire.wire iquery_response_fromWire.wire
+BUILT_SOURCES += nsec3query_nodnssec_fromWire.wire nsec3query_fromWire.wire
 
 # NOTE: keep this in sync with real file listing
 # so is included in tarball
@@ -19,8 +20,10 @@ EXTRA_DIST += shortquestion_fromWire
 EXTRA_DIST += shortresponse_fromWire
 EXTRA_DIST += simplequery_fromWire.spec
 EXTRA_DIST += simpleresponse_fromWire.spec
+EXTRA_DIST += nsec3query_nodnssec_fromWire.spec nsec3query_fromWire.spec
 EXTRA_DIST += iquery_fromWire.spec iquery_response_fromWire.spec
 EXTRA_DIST += example.com.zone example.net.zone example.org.zone example.zone
+EXTRA_DIST += rfc5155-example.zone.signed
 
 EXTRA_DIST += example.com
 EXTRA_DIST += example.sqlite3

src/lib/testutils/testdata/nsec3query_fromWire.spec

@@ -0,0 +1,11 @@
+#
+# A simple QUERY message (with DO bit on) for "example" zone signed with NSEC3
+#
+
+[header]
+arcount: 1
+[question]
+# use default
+name: ns2.example
+[edns]
+do: 1

src/lib/testutils/testdata/nsec3query_nodnssec_fromWire.spec

@@ -0,0 +1,9 @@
+#
+# A simple QUERY message (without DO bit) for "example" zone signed with NSEC3
+#
+
+[header]
+# use default
+[question]
+# use default
+name: ns2.example

src/lib/testutils/testdata/rfc5155-example.zone.signed

@@ -0,0 +1,72 @@
+;; The example NSEC3-signed zone used in RFC5155.
+
+example.				      3600 IN SOA	ns1.example. bugs.x.w.example. 1 3600 300 3600000 3600
+example.				      3600 IN RRSIG	SOA 7 1 3600 20150420235959 20051021000000 40430 example. Hu25UIyNPmvPIVBrldN+9Mlp9Zql39qaUd8iq4ZLlYWfUUbbAS41pG+6 8z81q1xhkYAcEyHdVI2LmKusbZsT0Q==
+example.				      3600 IN NS	ns1.example.
+example.				      3600 IN NS	ns2.example.
+example.				      3600 IN RRSIG	NS 7 1 3600 20150420235959 20051021000000 40430 example. PVOgtMK1HHeSTau+HwDWC8Ts+6C8qtqd4pQJqOtdEVgg+MA+ai4fWDEh u3qHJyLcQ9tbD2vvCnMXjtz6SyObxA==
+example.				      3600 IN MX	1 xx.example.
+example.				      3600 IN RRSIG	MX 7 1 3600 20150420235959 20051021000000 40430 example. GgQ1A9xs47k42VPvpL/a1BWUz/6XsnHkjotw9So8MQtZtl2wJBsnOQsa oHrRCrRbyriEl/GZn9Mto/Kx+wBo+w==
+example.				      3600 IN DNSKEY	256 3 7 AwEAAaetidLzsKWUt4swWR8yu0wPHPiUi8LUsAD0QPWU+wzt89epO6tH zkMBVDkC7qphQO2hTY4hHn9npWFRw5BYubE=
+example.				      3600 IN DNSKEY	257 3 7 AwEAAcUlFV1vhmqx6NSOUOq2R/dsR7Xm3upJj7IommWSpJABVfW8Q0rO vXdM6kzt+TAu92L9AbsUdblMFin8CVF3n4s=
+example.				      3600 IN RRSIG	DNSKEY 7 1 3600 20150420235959 20051021000000 12708 example. AuU4juU9RaxescSmStrQks3Gh9FblGBlVU31uzMZ/U/FpsUb8aC6QZS+ sTsJXnLnz7flGOsmMGQZf3bH+QsCtg==
+example.				      3600 IN NSEC3PARAM 1 0 12 AABBCCDD
+example.				      3600 IN RRSIG	NSEC3PARAM 7 1 3600 20150420235959 20051021000000 40430 example. C1Gl8tPZNtnjlrYWDeeUV/sGLCyy/IHie2rerN05XSA3Pq0U3+4VvGWY WdUMfflOdxqnXHwJTLQsjlkynhG6Cg==
+2t7b4g4vsa5smi47k61mv5bv1a22bojr.example.     3600 IN A		192.0.2.127
+2t7b4g4vsa5smi47k61mv5bv1a22bojr.example.     3600 IN RRSIG	A 7 2 3600 20150420235959 20051021000000 40430 example. h6c++bzhRuWWt2bykN6mjaTNBcXNq5UuL5EdK+iDP4eY8I0kSiKaCjg3 tC1SQkeloMeub2GWk8p6xHMPZumXlw==
+a.example.				      3600 IN NS	ns1.a.example.
+a.example.				      3600 IN NS	ns2.a.example.
+a.example.				      3600 IN DS	58470 5 1 3079F1593EBAD6DC121E202A8B766A6A4837206C
+a.example.				      3600 IN RRSIG	DS 7 2 3600 20150420235959 20051021000000 40430 example. XacFcQVHLVzdoc45EJhN616zQ4mEXtE8FzUhM2KWjfy1VfRKD9r1MeVG wwoukOKgJxBPFsWoo722vZ4UZ2dIdA==
+ns1.a.example.				      3600 IN A		192.0.2.5
+ns2.a.example.				      3600 IN A		192.0.2.6
+ai.example.				      3600 IN A		192.0.2.9
+ai.example.				      3600 IN RRSIG	A 7 2 3600 20150420235959 20051021000000 40430 example. hVe+wKYMlObTRPhX0NL67GxeZfdxqr/QeR6FtfdAj5+FgYxyzPEjIzvK Wy00hWIl6wD3Vws+rznEn8sQ64UdqA==
+ai.example.				      3600 IN HINFO	"KLH-10" "ITS"
+ai.example.				      3600 IN RRSIG	HINFO 7 2 3600 20150420235959 20051021000000 40430 example. Yi42uOq43eyO6qXHNvwwfFnIustWgV5urFcxenkLvs6pKRh00VBjODmf 3Z4nMO7IOl6nHSQ1v0wLHpEZG7Xj2w==
+ai.example.				      3600 IN AAAA	2001:db8::f00:baa9
+ai.example.				      3600 IN RRSIG	AAAA 7 2 3600 20150420235959 20051021000000 40430 example. LcdxKaCB5bGZwPDg+3JJ4O02zoMBrjxqlf6WuaHQZZfTUpb9Nf2nxFGe 2XRPfR5tpJT6GdRGcHueLuXkMjBArQ==
+c.example.				      3600 IN NS	ns1.c.example.
+c.example.				      3600 IN NS	ns2.c.example.
+ns1.c.example.				      3600 IN A		192.0.2.7
+ns2.c.example.				      3600 IN A		192.0.2.8
+ns1.example.				      3600 IN A		192.0.2.1
+ns1.example.				      3600 IN RRSIG	A 7 2 3600 20150420235959 20051021000000 40430 example. bu6kx73n6XEunoVGuRfAgY7EF/AJqHy7hj0jkiqJjB0dOrx3wuz9SaBe GfqWIdn/uta3SavN4FRvZR9SCFHF5Q==
+ns2.example.				      3600 IN A		192.0.2.2
+ns2.example.				      3600 IN RRSIG	A 7 2 3600 20150420235959 20051021000000 40430 example. ktQ3TqE0CfRfki0Rb/Ip5BM0VnxelbuejCC4zpLbFKA/7eD7UNAwxMgx JPtbdST+syjYSJaj4IHfeX6n8vfoGA==
+*.w.example.				      3600 IN MX	1 ai.example.
+*.w.example.				      3600 IN RRSIG	MX 7 2 3600 20150420235959 20051021000000 40430 example. CikebjQwGQPwijVcxgcZcSJKtfynugtlBiKb9FcBTrmOoyQ4InoWVudh CWsh/URX3lc4WRUMivEBP6+4KS3ldA==
+x.w.example.				      3600 IN MX	1 xx.example.
+x.w.example.				      3600 IN RRSIG	MX 7 3 3600 20150420235959 20051021000000 40430 example. IrK3tq/tHFIBF0scHiE/1IwMAvckS/55hAVvQyxTFbkAdDloP3NbZzu+ yoSsr3b3OX6qbBpY7WCtwwekLKRAwQ==
+x.y.w.example.				      3600 IN MX	1 xx.example.
+x.y.w.example.				      3600 IN RRSIG	MX 7 4 3600 20150420235959 20051021000000 40430 example. MqSt5HqJIN8+SLlzTOImrh5h9Xa6gDvAW/GnnbdPc6Z7nXvCpLPJj/5l Cwx3VuzVOjkbvXze8/8Ccl2Zn2hbug==
+xx.example.				      3600 IN A		192.0.2.10
+xx.example.				      3600 IN RRSIG	A 7 2 3600 20150420235959 20051021000000 40430 example. T35hBWEZ017VC5u2c4OriKyVn/pu+fVK4AlXYOxJ6iQylfV2HQIKjv6b 7DzINB3aF/wjJqgXpQvhq+Ac6+ZiFg==
+xx.example.				      3600 IN HINFO	"KLH-10" "TOPS-20"
+xx.example.				      3600 IN RRSIG	HINFO 7 2 3600 20150420235959 20051021000000 40430 example. KimG+rDd+7VA1zRsu0ITNAQUTRlpnsmqWrihFRnU+bRa93v2e5oFNFYC s3Rqgv62K93N7AhW6Jfqj/8NzWjvKg==
+xx.example.				      3600 IN AAAA	2001:db8::f00:baaa
+xx.example.				      3600 IN RRSIG	AAAA 7 2 3600 20150420235959 20051021000000 40430 example. IXBcXORITNwd8h3gNwyxtYFvAupS/CYWufVeuBUX0O25ivBCULjZjpDx FSxfohb/KA7YRdxENzYfMItpILl/Xw==
+0p9mhaveqvm6t7vbl5lop2u3t2rp3tom.example.     3600 IN NSEC3	1 1 12 AABBCCDD 2T7B4G4VSA5SMI47K61MV5BV1A22BOJR NS SOA MX RRSIG DNSKEY NSEC3PARAM
+0p9mhaveqvm6t7vbl5lop2u3t2rp3tom.example.     3600 IN RRSIG	NSEC3 7 2 3600 20150420235959 20051021000000 40430 example. OSgWSm26B+cS+dDL8b5QrWr/dEWhtCsKlwKLIBHYH6blRxK9rC0bMJPw Q4mLIuw85H2EY762BOCXJZMnpuwhpA==
+2t7b4g4vsa5smi47k61mv5bv1a22bojr.example.     3600 IN NSEC3	1 1 12 AABBCCDD 2VPTU5TIMAMQTTGL4LUU9KG21E0AOR3S A RRSIG
+2t7b4g4vsa5smi47k61mv5bv1a22bojr.example.     3600 IN RRSIG	NSEC3 7 2 3600 20150420235959 20051021000000 40430 example. OmBvJ1Vgg1hCKMXHFiNeIYHK9XVW0iLDLwJN4TFoNxZuP03gAXEI634Y wOc4YBNITrj413iqNI6mRk/r1dOSUw==
+2vptu5timamqttgl4luu9kg21e0aor3s.example.     3600 IN NSEC3	1 1 12 AABBCCDD 35MTHGPGCU1QG68FAB165KLNSNK3DPVL MX RRSIG
+2vptu5timamqttgl4luu9kg21e0aor3s.example.     3600 IN RRSIG	NSEC3 7 2 3600 20150420235959 20051021000000 40430 example. KL1V2oFYghNV0Hm7Tf2vpJjM6l+0g1JCcVYGVfI0lKrhPmTsOA96cLEA Cgo1x8I7kApJX+obTuktZ+sdsZPY1w==
+35mthgpgcu1qg68fab165klnsnk3dpvl.example.     3600 IN NSEC3	1 1 12 AABBCCDD B4UM86EGHHDS6NEA196SMVMLO4ORS995 NS DS RRSIG
+35mthgpgcu1qg68fab165klnsnk3dpvl.example.     3600 IN RRSIG	NSEC3 7 2 3600 20150420235959 20051021000000 40430 example. g6jPUUpduAJKRljUsN8gB4UagAX0NxY9shwQAynzo8EUWH+z6hEIBlUT PGj15eZll6VhQqgZXtAIR3chwgW+SA==
+b4um86eghhds6nea196smvmlo4ors995.example.     3600 IN NSEC3	1 1 12 AABBCCDD GJEQE526PLBF1G8MKLP59ENFD789NJGI MX RRSIG
+b4um86eghhds6nea196smvmlo4ors995.example.     3600 IN RRSIG	NSEC3 7 2 3600 20150420235959 20051021000000 40430 example. ZkPG3M32lmoHM6pa3D6gZFGB/rhL//Bs3Omh5u4m/CUiwtblEVOaAKKZ d7S959OeiX43aLX3pOv0TSTyiTxIZg==
+gjeqe526plbf1g8mklp59enfd789njgi.example.     3600 IN NSEC3	1 1 12 AABBCCDD JI6NEOAEPV8B5O6K4EV33ABHA8HT9FGC A HINFO AAAA RRSIG
+gjeqe526plbf1g8mklp59enfd789njgi.example.     3600 IN RRSIG	NSEC3 7 2 3600 20150420235959 20051021000000 40430 example. IVnezTJ9iqblFF97vPSmfXZ5Zozngx3KX3byLTZC4QBH2dFWhf6scrGF ZB980AfCxoD9qbbKDy+rdGIeRSVNyw==
+ji6neoaepv8b5o6k4ev33abha8ht9fgc.example.     3600 IN NSEC3	1 1 12 AABBCCDD K8UDEMVP1J2F7EG6JEBPS17VP3N8I58H
+ji6neoaepv8b5o6k4ev33abha8ht9fgc.example.     3600 IN RRSIG	NSEC3 7 2 3600 20150420235959 20051021000000 40430 example. gPkFp1s2QDQ6wQzcg1uSebZ61W33rUBDcTj72F3kQ490fEdp7k1BUIfb cZtPbX3YCpE+sIt0MpzVSKfTwx4uYA==
+k8udemvp1j2f7eg6jebps17vp3n8i58h.example.     3600 IN NSEC3	1 1 12 AABBCCDD KOHAR7MBB8DC2CE8A9QVL8HON4K53UHI
+k8udemvp1j2f7eg6jebps17vp3n8i58h.example.     3600 IN RRSIG	NSEC3 7 2 3600 20150420235959 20051021000000 40430 example. FtXGbvF0+wf8iWkyo73enAuVx03klN+pILBKS6qCcftVtfH4yVzsEZqu J27NHR7ruxJWDNMtOtx7w9WfcIg62A==
+kohar7mbb8dc2ce8a9qvl8hon4k53uhi.example.     3600 IN NSEC3	1 1 12 AABBCCDD Q04JKCEVQVMU85R014C7DKBA38O0JI5R A RRSIG
+kohar7mbb8dc2ce8a9qvl8hon4k53uhi.example.     3600 IN RRSIG	NSEC3 7 2 3600 20150420235959 20051021000000 40430 example. VrDXs2uVW21N08SyQIz88zml+y4ZCInTwgDr6zz43yAg+LFERjOrj3Oj ct51ac7Dp4eZbf9FQJazmASFKGxGXg==
+q04jkcevqvmu85r014c7dkba38o0ji5r.example.     3600 IN NSEC3	1 1 12 AABBCCDD R53BQ7CC2UVMUBFU5OCMM6PERS9TK9EN A RRSIG
+q04jkcevqvmu85r014c7dkba38o0ji5r.example.     3600 IN RRSIG	NSEC3 7 2 3600 20150420235959 20051021000000 40430 example. hV5I89b+4FHJDATp09g4bbN0R1F845CaXpL3ZxlMKimoPAyqletMlEWw LfFia7sdpSzn+ZlNNlkxWcLsIlMmUg==
+r53bq7cc2uvmubfu5ocmm6pers9tk9en.example.     3600 IN NSEC3	1 1 12 AABBCCDD T644EBQK9BIBCNA874GIVR6JOJ62MLHV MX RRSIG
+r53bq7cc2uvmubfu5ocmm6pers9tk9en.example.     3600 IN RRSIG	NSEC3 7 2 3600 20150420235959 20051021000000 40430 example. aupviViruXs4bDg9rCbezzBMf9h1ZlDvbW/CZFKulIGXXLj8B/fsDJar XVDA9bnUoRhEbKp+HF1FWKW7RIJdtQ==
+t644ebqk9bibcna874givr6joj62mlhv.example.     3600 IN NSEC3	1 1 12 AABBCCDD 0P9MHAVEQVM6T7VBL5LOP2U3T2RP3TOM A HINFO AAAA RRSIG
+t644ebqk9bibcna874givr6joj62mlhv.example.     3600 IN RRSIG	NSEC3 7 2 3600 20150420235959 20051021000000 40430 example. RAjGECB8P7O+F4Pa4Dx3tC0M+Z3KmlLKImcafb9XWwx+NWUNz7NBEDBQ HivIyKPVDkChcePIX1xPl1ATNa+8Dw==

src/lib/util/python/gen_wiredata.py.in

@@ -949,12 +949,11 @@ class NSEC(NSECBASE):
                                                  int(len(name_wire) / 2)))
         f.write('%s\n' % name_wire)
 
-class NSEC3(NSECBASE):
-    '''Implements rendering NSEC3 RDATA in the test data format.
+class NSEC3PARAM(RR):
+    '''Implements rendering NSEC3PARAM RDATA in the test data format.
 
     Configurable parameters are as follows (see the description of the
     same name of attribute for the default value):
-    - Type bitmap related parameters: see class NSECBASE
     - hashalg (8-bit int): The Hash Algorithm field.  Note that
       currently the only defined algorithm is SHA-1, for which a value
       of 1 will be used, and it's the default.  So this implementation
@@ -967,9 +966,6 @@ class NSEC3(NSECBASE):
     - saltlen (int): The Salt Length field.
     - salt (string): The Salt field.  It is converted to a sequence of
       ascii codes and its hexadecimal representation will be used.
-    - hashlen (int): The Hash Length field.
-    - hash (string): The Next Hashed Owner Name field.  This parameter
-      is interpreted as "salt".
     '''
 
     hashalg = 1                 # SHA-1
@@ -978,15 +974,18 @@ class NSEC3(NSECBASE):
     iterations = 1
     saltlen = 5
     salt = 's' * saltlen
-    hashlen = 20
-    hash = 'h' * hashlen
-    def dump_fixedpart(self, f, bitmap_totallen):
+
+    def dump(self, f):
         if self.rdlen is None:
-            # if rdlen needs to be calculated, it must be based on the bitmap
-            # length, because the configured maplen can be fake.
-            self.rdlen = 4 + 1 + len(self.salt) + 1 + len(self.hash) \
-                + bitmap_totallen
+            self.rdlen = 4 + 1 + len(self.salt)
         self.dump_header(f, self.rdlen)
+        self._dump_params(f)
+
+    def _dump_params(self, f):
+        '''This method is intended to be shared with NSEC3 class.
+
+        '''
+
         optout_val = 1 if self.optout else 0
         f.write('# Hash Alg=%s, Opt-Out=%d, Other Flags=%0x, Iterations=%d\n' %
                 (code_totext(self.hashalg, rdict_nsec3_algorithm),
@@ -997,6 +996,29 @@ class NSEC3(NSECBASE):
         f.write('%02x%s%s\n' % (self.saltlen,
                                 ' ' if len(self.salt) > 0 else '',
                                 encode_string(self.salt)))
+
+class NSEC3(NSECBASE, NSEC3PARAM):
+    '''Implements rendering NSEC3 RDATA in the test data format.
+
+    Configurable parameters are as follows (see the description of the
+    same name of attribute for the default value):
+    - Type bitmap related parameters: see class NSECBASE
+    - Hash parameter related parameters: see class NSEC3PARAM
+    - hashlen (int): The Hash Length field.
+    - hash (string): The Next Hashed Owner Name field.  This parameter
+      is interpreted as "salt".
+    '''
+
+    hashlen = 20
+    hash = 'h' * hashlen
+    def dump_fixedpart(self, f, bitmap_totallen):
+        if self.rdlen is None:
+            # if rdlen needs to be calculated, it must be based on the bitmap
+            # length, because the configured maplen can be fake.
+            self.rdlen = 4 + 1 + len(self.salt) + 1 + len(self.hash) \
+                + bitmap_totallen
+        self.dump_header(f, self.rdlen)
+        self._dump_params(f)
         f.write("# Hash Len=%d, Hash='%s'\n" % (self.hashlen, self.hash))
         f.write('%02x%s%s\n' % (self.hashlen,
                                 ' ' if len(self.hash) > 0 else '',