1939. [bug] The resolver could dereference a null pointer after

validation if all the queries have timed out.
                        [RT #15528]

1938.   [bug]           The validator was not correctly handling unsecure
                        negative responses at or below a SEP. [RT #15528]

CHANGES

@@ -1,3 +1,10 @@
+1939.	[bug]		The resolver could dereference a null pointer after
+			validation if all the queries have timed out.
+			[RT #15528]
+
+1938.	[bug]		The validator was not correctly handling unsecure
+			negative responses at or below a SEP. [RT #15528]
+
 1937.	[bug]		sdlz doesn't handle RRSIG records. [RT #15564]
 
 1936.	[bug]		The validator could leak memory. [RT #15544]

doc/arm/Bv9ARM-book.xml

@@ -18,7 +18,7 @@
  - PERFORMANCE OF THIS SOFTWARE.
 -->
 
-<!-- File: $Id: Bv9ARM-book.xml,v 1.281 2005/11/02 22:26:48 marka Exp $ -->
+<!-- File: $Id: Bv9ARM-book.xml,v 1.282 2005/11/03 00:51:54 marka Exp $ -->
 <book xmlns:xi="http://www.w3.org/2001/XInclude">
   <title>BIND 9 Administrator Reference Manual</title>
 
@@ -7581,19 +7581,22 @@ query-source-v6 address * port *;
           <title><command>trusted-keys</command> Statement Definition
             and Usage</title>
           <para>
-            The <command>trusted-keys</command> statement defines
+            The <command>trusted-keys</command> statement defines DNSSEC
-            DNSSEC
             security roots. DNSSEC is described in <xref linkend="DNSSEC"/>. A
             security root is defined when the public key for a
             non-authoritative
             zone is known, but cannot be securely obtained through DNS, either
-            because it is the  DNS root zone or because its parent zone is
+            because it is the DNS root zone or because its parent zone is
             unsigned.
             Once a key has been configured as a trusted key, it is treated as
             if it had been validated and proven secure. The resolver attempts
             DNSSEC validation on all DNS data in subdomains of a security
             root.
-          </para>
+	  </para>
+	  <para>
+	    All zones listed in <command>trusted-keys</command> are deemed
+	    to exist regardless of what parent zones say.
+	  </para>
           <para>
             The <command>trusted-keys</command> statement can
             contain

lib/dns/include/dns/validator.h

@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: validator.h,v 1.32 2005/08/25 00:56:08 marka Exp $ */
+/* $Id: validator.h,v 1.33 2005/11/03 00:51:55 marka Exp $ */
 
 #ifndef DNS_VALIDATOR_H
 #define DNS_VALIDATOR_H 1
@@ -25,10 +25,18 @@
  *****/
 
 /*! \file
+ *
  * \brief
  * DNS Validator
+ * This is the BIND 9 validator, the module responsible for validating the
+ * rdatasets and negative responses (messages).  It makes use of zones in
+ * the view and may fetch RRset to complete trust chains.  It implements
+ * DNSSEC as specified in RFC 4033, 4034 and 4035.
  *
- * XXX TBS XXX
+ * It can also optionally implement ISC's DNSSEC look-aside validation.
+ *
+ * Correct operation is critical to preventing spoofed answers from secure
+ * zones being accepted.
  *
  * MP:
  *\li	The module ensures appropriate synchronization of data structures it
@@ -44,8 +52,7 @@
  *\li	No anticipated impact.
  *
  * Standards:
- *\li	RFCs:	1034, 1035, 2181, 2535, TBS
+ *\li	RFCs:	1034, 1035, 2181, 4033, 4034, 4035.
- *\li	Drafts:	TBS
  */
 
 #include <isc/lang.h>
@@ -65,6 +72,10 @@
  * 'name', 'rdataset', 'sigrdataset', and 'message' are the values that were
  * supplied when dns_validator_create() was called.  They are returned to the
  * caller so that they may be freed.
+ *
+ * If the RESULT is ISC_R_SUCCESS and the answer is secure then
+ * proofs[] will contain the the names of the NSEC records that hold the
+ * various proofs.  Note the same name may appear multiple times.
  */
 typedef struct dns_validatorevent {
 	ISC_EVENT_COMMON(struct dns_validatorevent);
@@ -129,7 +140,10 @@ struct dns_validator {
 	unsigned int			depth;
 };
 
-#define DNS_VALIDATOR_DLV 1
+/*%
+ * dns_validator_create() options.
+ */
+#define DNS_VALIDATOR_DLV 1U
 
 ISC_LANG_BEGINDECLS
 
@@ -164,13 +178,17 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type,
  * arguments must be provided.
  *
  * The validation is performed in the context of 'view'.
- * 'options' must be zero.
  *
  * When the validation finishes, a dns_validatorevent_t with
  * the given 'action' and 'arg' are sent to 'task'.
  * Its 'result' field will be ISC_R_SUCCESS iff the
  * response was successfully proven to be either secure or
  * part of a known insecure domain.
+ *
+ * options:
+ * If DNS_VALIDATOR_DLV is set the caller knows there is not a
+ * trusted key and the validator should immediately attempt to validate
+ * the answer by looking for a appopriate DLV RRset.
  */
 
 void

lib/dns/resolver.c

@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: resolver.c,v 1.318 2005/10/14 01:14:09 marka Exp $ */
+/* $Id: resolver.c,v 1.319 2005/11/03 00:51:54 marka Exp $ */
 
 /*! \file */
 
@@ -3809,21 +3809,20 @@ ncache_adderesult(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node,
 	isc_result_t result;
 	result = dns_ncache_add(message, cache, node, covers, now,
 				maxttl, ardataset);
-	if (result == DNS_R_UNCHANGED) {
+	if (result == DNS_R_UNCHANGED || result == ISC_R_SUCCESS) {
 		/*
-		 * The data in the cache are better than the negative cache
+		 * If the cache now contains a negative entry and we
-		 * entry we're trying to add.
+		 * care about whether it is DNS_R_NCACHENXDOMAIN or
+		 * DNS_R_NCACHENXRRSET then extract it.
 		 */
 		if (ardataset != NULL && ardataset->type == 0) {
 			/*
-			 * The cache data is also a negative cache
+			 * The cache data is a negative cache entry.
-			 * entry.
 			 */
 			if (NXDOMAIN(ardataset))
 				*eresultp = DNS_R_NCACHENXDOMAIN;
 			else
 				*eresultp = DNS_R_NCACHENXRRSET;
-			result = ISC_R_SUCCESS;
 		} else {
 			/*
 			 * Either we don't care about the nature of the
@@ -3835,13 +3834,8 @@ ncache_adderesult(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node,
 			 * XXXRTH  There's a CNAME/DNAME problem here.
 			 */
 			*eresultp = ISC_R_SUCCESS;
-			result = ISC_R_SUCCESS;
 		}
-	} else if (result == ISC_R_SUCCESS) {
+		result = ISC_R_SUCCESS;
-		if (NXDOMAIN(ardataset))
-			*eresultp = DNS_R_NCACHENXDOMAIN;
-		else
-			*eresultp = DNS_R_NCACHENXRRSET;
 	}
 
 	return (result);

lib/dns/validator.c

@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: validator.c,v 1.134 2005/11/02 01:46:31 marka Exp $ */
+/* $Id: validator.c,v 1.135 2005/11/03 00:51:55 marka Exp $ */
 
 /*! \file */
 
@@ -45,20 +45,65 @@
 #include <dns/validator.h>
 #include <dns/view.h>
 
+/*! \file
+ * \brief
+ * Basic processing sequences.
+ *
+ * \li When called with rdataset and sigrdataset:
+ * validator_start -> validate -> proveunsecure -> startfinddlvsep ->
+ *	dlv_validator_start -> validator_start -> validate -> proveunsecure
+ *
+ * validator_start -> validate -> nsecvalidate	(secure wildcard answer)
+ * 
+ * \li When called with rdataset, sigrdataset and with DNS_VALIDATOR_DLV:
+ * validator_start -> startfinddlvsep -> dlv_validator_start ->
+ *	validator_start -> validate -> proveunsecure
+ *
+ * \li When called with rdataset:
+ * validator_start -> proveunsecure -> startfinddlvsep ->
+ *	dlv_validator_start -> validator_start -> proveunsecure
+ *
+ * \li When called with rdataset and with DNS_VALIDATOR_DLV:
+ * validator_start -> startfinddlvsep -> dlv_validator_start ->
+ *	validator_start -> proveunsecure
+ *
+ * \li When called without a rdataset:
+ * validator_start -> nsecvalidate -> proveunsecure -> startfinddlvsep ->
+ *	dlv_validator_start -> validator_start -> nsecvalidate -> proveunsecure
+ *
+ * \li When called without a rdataset and with DNS_VALIDATOR_DLV:
+ * validator_start -> startfinddlvsep -> dlv_validator_start ->
+ *	validator_start -> nsecvalidate -> proveunsecure
+ *
+ * validator_start: determines what type of validation to do.
+ * validate: attempts to perform a positive validation.
+ * proveunsecure: attempts to prove the answer comes from a unsecure zone.
+ * nsecvalidate: attempts to prove a negative response.
+ * startfinddlvsep: starts the DLV record lookup.
+ * dlv_validator_start: resets state and restarts the lookup using the
+ *	DLV RRset found by startfinddlvsep.
+ */
+
 #define VALIDATOR_MAGIC			ISC_MAGIC('V', 'a', 'l', '?')
 #define VALID_VALIDATOR(v)		ISC_MAGIC_VALID(v, VALIDATOR_MAGIC)
 
-#define VALATTR_SHUTDOWN		0x0001
+#define VALATTR_SHUTDOWN		0x0001	/*%< Shutting down. */
-#define VALATTR_FOUNDNONEXISTENCE	0x0002
+#define VALATTR_TRIEDVERIFY		0x0004  /*%< We have found a key and
-#define VALATTR_TRIEDVERIFY		0x0004
+						 * have attempted a verify. */
-#define VALATTR_NEGATIVE		0x0008
+#define VALATTR_INSECURITY		0x0010	/*%< Attempting proveunsecure. */
-#define VALATTR_INSECURITY		0x0010
+#define VALATTR_DLVTRIED		0x0020	/*%< Looked for a DLV record. */
-#define VALATTR_DLVTRIED		0x0020
+#define VALATTR_AUTHNONPENDING		0x0040	/*%< Tidy up pending auth. */
 
+/*!
+ * NSEC proofs to be looked for.
+ */
 #define VALATTR_NEEDNOQNAME		0x0100
 #define VALATTR_NEEDNOWILDCARD		0x0200
 #define VALATTR_NEEDNODATA		0x0400
 
+/*!
+ * NSEC proofs that have been found.
+ */
 #define VALATTR_FOUNDNOQNAME		0x1000
 #define VALATTR_FOUNDNOWILDCARD		0x2000
 #define VALATTR_FOUNDNODATA		0x4000
@@ -106,19 +151,35 @@ validator_logcreate(dns_validator_t *val,
 static isc_result_t
 dlv_validatezonekey(dns_validator_t *val);
 
-static isc_result_t
+static void
 dlv_validator_start(dns_validator_t *val);
 
 static isc_result_t
 finddlvsep(dns_validator_t *val, isc_boolean_t resume);
 
+static void
+auth_nonpending(dns_message_t *message);
+
+static isc_result_t
+startfinddlvsep(dns_validator_t *val, dns_name_t *unsecure);
+
+/*%
+ * Mark the RRsets as a answer.
+ *
+ * If VALATTR_AUTHNONPENDING is set then this is a negative answer
+ * in a insecure zone.  We need to mark any pending RRsets as
+ * dns_trust_authauthority answers (this is deferred from resolver.c).
+ */
 static inline void
 markanswer(dns_validator_t *val) {
 	validator_log(val, ISC_LOG_DEBUG(3), "marking as answer");
-	if (val->event->rdataset)
+	if (val->event->rdataset != NULL)
 		val->event->rdataset->trust = dns_trust_answer;
-	if (val->event->sigrdataset)
+	if (val->event->sigrdataset != NULL)
 		val->event->sigrdataset->trust = dns_trust_answer;
+	if (val->event->message != NULL &&
+	    (val->attributes & VALATTR_AUTHNONPENDING) != 0)
+		auth_nonpending(val->event->message);
 }
 
 static void
@@ -157,6 +218,9 @@ exit_check(dns_validator_t *val) {
 	return (ISC_TRUE);
 }
 
+/*%
+ * Mark pending answers in the authority section as dns_trust_authauthority.
+ */
 static void
 auth_nonpending(dns_message_t *message) {
 	isc_result_t result;
@@ -179,6 +243,10 @@ auth_nonpending(dns_message_t *message) {
 	}
 }
 
+/*%
+ * Look in the NSEC record returned from a DS query to see if there is
+ * a NS RRset at this name.  If it is found we are at a delegation point.
+ */
 static isc_boolean_t
 isdelegation(dns_name_t *name, dns_rdataset_t *rdataset,
 	     isc_result_t dbresult)
@@ -212,6 +280,11 @@ isdelegation(dns_name_t *name, dns_rdataset_t *rdataset,
 	return (found);
 }
 
+/*%
+ * We have been asked to to look for a key.
+ * If found resume the validation process.
+ * If not found fail the validation process.
+ */
 static void
 fetch_callback_validator(isc_task_t *task, isc_event_t *event) {
 	dns_fetchevent_t *devent;
@@ -271,6 +344,11 @@ fetch_callback_validator(isc_task_t *task, isc_event_t *event) {
 		destroy(val);
 }
 
+/*%
+ * We were asked to look for a DS record as part of following a key chain
+ * upwards.  If found resume the validation process.  If not found fail the
+ * validation process.
+ */
 static void
 dsfetched(isc_task_t *task, isc_event_t *event) {
 	dns_fetchevent_t *devent;
@@ -332,8 +410,16 @@ dsfetched(isc_task_t *task, isc_event_t *event) {
 		destroy(val);
 }
 
-/*
+/*%
- * XXX there's too much duplicated code here.
+ * We were asked to look for the DS record as part of proving that a
+ * name is unsecure.
+ *
+ * If the DS record doesn't exist and the query name corresponds to
+ * a delegation point we are transitioning from a secure zone to a
+ * unsecure zone.
+ *
+ * If the DS record exists it will be secure.  We can continue looking
+ * for the break point in the chain of trust.
  */
 static void
 dsfetched2(isc_task_t *task, isc_event_t *event) {
@@ -361,7 +447,8 @@ dsfetched2(isc_task_t *task, isc_event_t *event) {
 
 	INSIST(val->event != NULL);
 
-	validator_log(val, ISC_LOG_DEBUG(3), "in dsfetched2");
+	validator_log(val, ISC_LOG_DEBUG(3), "in dsfetched2: %s",
+		      dns_result_totext(eresult));
 	LOCK(&val->lock);
 	if (eresult == DNS_R_NXRRSET || eresult == DNS_R_NCACHENXRRSET) {
 		/*
@@ -373,9 +460,13 @@ dsfetched2(isc_task_t *task, isc_event_t *event) {
 				validator_log(val, ISC_LOG_WARNING,
 					      "must be secure failure");
 				validator_done(val, DNS_R_MUSTBESECURE);
-			} else {
+			} else if (val->view->dlv == NULL || DLVTRIED(val)) {
 				markanswer(val);
 				validator_done(val, ISC_R_SUCCESS);
+			} else {
+				result = startfinddlvsep(val, tname);
+				if (result != DNS_R_WAIT)
+					validator_done(val, result);
 			}
 		} else {
 			result = proveunsecure(val, ISC_TRUE);
@@ -387,7 +478,9 @@ dsfetched2(isc_task_t *task, isc_event_t *event) {
 		   eresult == DNS_R_NCACHENXDOMAIN)
 	{
 		/*
-		 * Either there is a DS or this is not a zone cut.  Continue.
+		 * There is a DS which may or may not be a zone cut. 
+		 * In either case we are still in a secure zone resume
+		 * validation.
 		 */
 		result = proveunsecure(val, ISC_TRUE);
 		if (result != DNS_R_WAIT)
@@ -405,6 +498,11 @@ dsfetched2(isc_task_t *task, isc_event_t *event) {
 		destroy(val);
 }
 
+/*%
+ * Callback from when a DNSKEY RRset has been validated.
+ *
+ * Resumes the stalled validation process.
+ */
 static void
 keyvalidated(isc_task_t *task, isc_event_t *event) {
 	dns_validatorevent_t *devent;
@@ -450,6 +548,11 @@ keyvalidated(isc_task_t *task, isc_event_t *event) {
 		destroy(val);
 }
 
+/*%
+ * Callback when the DS record has been validated.
+ *
+ * Resumes validation of the zone key or the unsecure zone proof.
+ */
 static void
 dsvalidated(isc_task_t *task, isc_event_t *event) {
 	dns_validatorevent_t *devent;
@@ -493,10 +596,12 @@ dsvalidated(isc_task_t *task, isc_event_t *event) {
 		destroy(val);
 }
 
-/*
+/*%
  * Return ISC_R_SUCCESS if we can determine that the name doesn't exist
  * or we can determine whether there is data or not at the name.
  * If the name does not exist return the wildcard name.
+ *
+ * Return ISC_R_IGNORE when the NSEC is not the appropriate one.
  */
 static isc_result_t
 nsecnoexistnodata(dns_validator_t *val, dns_name_t* name, dns_name_t *nsecname,
@@ -639,6 +744,13 @@ nsecnoexistnodata(dns_validator_t *val, dns_name_t* name, dns_name_t *nsecname,
 	return (ISC_R_SUCCESS);
 }
 
+/*%
+ * Callback for when NSEC records have been validated.
+ *
+ * Looks for NOQNAME and NODATA proofs.
+ *
+ * Resumes nsecvalidate.
+ */
 static void
 authvalidated(isc_task_t *task, isc_event_t *event) {
 	dns_validatorevent_t *devent;
@@ -717,44 +829,20 @@ authvalidated(isc_task_t *task, isc_event_t *event) {
 	isc_event_free(&event);
 }
 
-static void
+/*%
-negauthvalidated(isc_task_t *task, isc_event_t *event) {
+ * Looks for the requested name and type in the view (zones and cache).
-	dns_validatorevent_t *devent;
+ *
-	dns_validator_t *val;
+ * When looking for a DLV record also checks to make sure the NSEC record
-	isc_boolean_t want_destroy;
+ * returns covers the query name as part of aggressive negative caching.
-	isc_result_t eresult;
+ *
-
+ * Returns:
-	UNUSED(task);
+ * \li	ISC_R_SUCCESS
-	INSIST(event->ev_type == DNS_EVENT_VALIDATORDONE);
+ * \li	ISC_R_NOTFOUND
-
+ * \li	DNS_R_NCACHENXDOMAIN
-	devent = (dns_validatorevent_t *)event;
+ * \li	DNS_R_NCACHENXRRSET
-	val = devent->ev_arg;
+ * \li	DNS_R_NXRRSET
-	eresult = devent->result;
+ * \li	DNS_R_NXDOMAIN
-	isc_event_free(&event);
+ */
-	dns_validator_destroy(&val->subvalidator);
-
-	INSIST(val->event != NULL);
-
-	validator_log(val, ISC_LOG_DEBUG(3), "in negauthvalidated");
-	LOCK(&val->lock);
-	if (eresult == ISC_R_SUCCESS) {
-		val->attributes |= VALATTR_FOUNDNONEXISTENCE;
-		validator_log(val, ISC_LOG_DEBUG(3),
-			      "nonexistence proof found");
-		auth_nonpending(val->event->message);
-		validator_done(val, ISC_R_SUCCESS);
-	} else {
-		validator_log(val, ISC_LOG_DEBUG(3),
-			      "negauthvalidated: got %s",
-			      isc_result_totext(eresult));
-		validator_done(val, eresult);
-	}
-	want_destroy = exit_check(val);
-	UNLOCK(&val->lock);
-	if (want_destroy)
-		destroy(val);
-}
-
 static inline isc_result_t
 view_find(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type) {
 	dns_fixedname_t fixedname;
@@ -857,12 +945,9 @@ view_find(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type) {
 		dns_rdata_freestruct(&nsec);
 		result = DNS_R_NCACHENXDOMAIN;
 	} else if (result != ISC_R_SUCCESS &&
-                   result != DNS_R_GLUE &&
-                   result != DNS_R_HINT &&
                    result != DNS_R_NCACHENXDOMAIN &&
                    result != DNS_R_NCACHENXRRSET &&
                    result != DNS_R_NXRRSET &&
-                   result != DNS_R_HINTNXRRSET &&
                    result != ISC_R_NOTFOUND) {
 		goto  notfound;
 	}
@@ -876,11 +961,15 @@ view_find(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type) {
 	return (ISC_R_NOTFOUND);
 }
 
+/*%
+ * Checks to make sure we are not going to loop.  As we use a SHARED fetch
+ * the validation process will stall if looping was to occur.
+ */
 static inline isc_boolean_t
 check_deadlock(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type) {
 	dns_validator_t *parent;
 
-	for (parent = val->parent; parent != NULL; parent = parent->parent) {
+	for (parent = val; parent != NULL; parent = parent->parent) {
 		if (parent->event != NULL &&
 		    parent->event->type == type &&
 		    dns_name_equal(parent->event->name, name))
@@ -894,6 +983,9 @@ check_deadlock(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type) {
 	return (ISC_FALSE);
 }
 
+/*%
+ * Start a fetch for the requested name and type.
+ */
 static inline isc_result_t
 create_fetch(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type,
 	     isc_taskaction_t callback, const char *caller)
@@ -916,6 +1008,9 @@ create_fetch(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type,
 					 &val->fetch));
 }
 
+/*%
+ * Start a subvalidation process.
+ */
 static inline isc_result_t
 create_validator(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type,
 		 dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset,
@@ -938,7 +1033,7 @@ create_validator(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type,
 	return (result);
 }
 
-/*
+/*%
  * Try to find a key that could have signed 'siginfo' among those
  * in 'rdataset'.  If found, build a dst_key_t for it and point
  * val->key at it.
@@ -1006,6 +1101,9 @@ get_dst_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo,
 	return (result);
 }
 
+/*%
+ * Get the key that genertated this signature.
+ */
 static isc_result_t
 get_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo) {
 	isc_result_t result;
@@ -1132,7 +1230,7 @@ compute_keytag(dns_rdata_t *rdata, dns_rdata_dnskey_t *key) {
 	return (dst_region_computeid(&r, key->algorithm));
 }
 
-/*
+/*%
  * Is this keyset self-signed?
  */
 static isc_boolean_t
@@ -1174,8 +1272,19 @@ isselfsigned(dns_validator_t *val) {
 	return (ISC_FALSE);
 }
 
+/*%
+ * Attempt to verify the rdataset using the given key and rdata (RRSIG).
+ * The signature was good and from a wildcard record and the QNAME does
+ * not match the wildcard we need to look for a NOQNAME proof.
+ *
+ * Returns:
+ * \li	ISC_R_SUCCESS if the verification succeeds.
+ * \li	Others if the verification fails.
+ */
 static isc_result_t
-verify(dns_validator_t *val, dst_key_t *key, dns_rdata_t *rdata) {
+verify(dns_validator_t *val, dst_key_t *key, dns_rdata_t *rdata,
+       isc_uint16_t keyid)
+{
 	isc_result_t result;
 	dns_fixedname_t fixed;
 
@@ -1185,8 +1294,8 @@ verify(dns_validator_t *val, dst_key_t *key, dns_rdata_t *rdata) {
 				    key, ISC_FALSE, val->view->mctx, rdata,
 				    dns_fixedname_name(&fixed));
 	validator_log(val, ISC_LOG_DEBUG(3),
-		      "verify rdataset: %s",
+		      "verify rdataset (keyid=%u): %s",
-		      isc_result_totext(result));
+		      keyid, isc_result_totext(result));
 	if (result == DNS_R_FROMWILDCARD) {
 		if (!dns_name_equal(val->event->name,
 				    dns_fixedname_name(&fixed)))
@@ -1196,14 +1305,14 @@ verify(dns_validator_t *val, dst_key_t *key, dns_rdata_t *rdata) {
 	return (result);
 }
 
-/*
+/*%
  * Attempts positive response validation of a normal RRset.
  *
  * Returns:
- *	ISC_R_SUCCESS	Validation completed successfully
+ * \li	ISC_R_SUCCESS	Validation completed successfully
- *	DNS_R_WAIT	Validation has started but is waiting
+ * \li	DNS_R_WAIT	Validation has started but is waiting
  *			for an event.
- *	Other return codes are possible and all indicate failure.
+ * \li	Other return codes are possible and all indicate failure.
  */
 static isc_result_t
 validate(dns_validator_t *val, isc_boolean_t resume) {
@@ -1274,7 +1383,8 @@ validate(dns_validator_t *val, isc_boolean_t resume) {
 		}
 
 		do {
-			result = verify(val, val->key, &rdata);
+			result = verify(val, val->key, &rdata,
+					val->siginfo->keyid);
 			if (result == ISC_R_SUCCESS)
 				break;
 			if (val->keynode != NULL) {
@@ -1358,6 +1468,10 @@ validate(dns_validator_t *val, isc_boolean_t resume) {
 	return (DNS_R_NOVALIDSIG);
 }
 
+/*%
+ * Validate the DNSKEY RRset by looking for a DNSKEY that matches a
+ * DLV record and that also verifies the DNSKEY RRset.
+ */
 static isc_result_t
 dlv_validatezonekey(dns_validator_t *val) {
 	dns_keytag_t keytag;
@@ -1375,12 +1489,12 @@ dlv_validatezonekey(dns_validator_t *val) {
 	unsigned char dsbuf[DNS_DS_BUFFERSIZE];
 
 	validator_log(val, ISC_LOG_DEBUG(3), "dlv_validatezonekey");
+
 	/*
 	 * Look through the DLV record and find the keys that can sign the
 	 * key set and the matching signature.  For each such key, attempt
 	 * verification.
 	 */
-
 	supported_algorithm = ISC_FALSE;
 
 	for (result = dns_rdataset_first(&val->dlv);
@@ -1460,7 +1574,7 @@ dlv_validatezonekey(dns_validator_t *val) {
 				 */
 				continue;
 
-			result = verify(val, dstkey, &sigrdata);
+			result = verify(val, dstkey, &sigrdata, sig.keyid);
 			dst_key_free(&dstkey);
 			if (result == ISC_R_SUCCESS)
 				break;
@@ -1490,14 +1604,14 @@ dlv_validatezonekey(dns_validator_t *val) {
 		return (DNS_R_NOVALIDSIG);
 }
 
-/*
+/*%
  * Attempts positive response validation of an RRset containing zone keys.
  *
  * Returns:
- *	ISC_R_SUCCESS	Validation completed successfully
+ * \li	ISC_R_SUCCESS	Validation completed successfully
- *	DNS_R_WAIT	Validation has started but is waiting
+ * \li	DNS_R_WAIT	Validation has started but is waiting
  *			for an event.
- *	Other return codes are possible and all indicate failure.
+ * \li	Other return codes are possible and all indicate failure.
  */
 static isc_result_t
 validatezonekey(dns_validator_t *val) {
@@ -1547,7 +1661,8 @@ validatezonekey(dns_validator_t *val) {
 							  &keynode);
 			while (result == ISC_R_SUCCESS) {
 				dstkey = dns_keynode_key(keynode);
-				result = verify(val, dstkey, &sigrdata);
+				result = verify(val, dstkey, &sigrdata,
+						sig.keyid);
 				if (result == ISC_R_SUCCESS) {
 					dns_keytable_detachkeynode(val->keytable,
 								   &keynode);
@@ -1686,6 +1801,9 @@ validatezonekey(dns_validator_t *val) {
 		dns_rdataset_init(&trdataset);
 		dns_rdataset_clone(val->event->rdataset, &trdataset);
 
+		/*
+		 * Look for the KEY that matches the DS record.
+		 */
 		for (result = dns_rdataset_first(&trdataset);
 		     result == ISC_R_SUCCESS;
 		     result = dns_rdataset_next(&trdataset))
@@ -1720,7 +1838,7 @@ validatezonekey(dns_validator_t *val) {
 			dns_rdataset_current(val->event->sigrdataset,
 					     &sigrdata);
 			(void)dns_rdata_tostruct(&sigrdata, &sig, NULL);
-			if (ds.key_tag != sig.keyid &&
+			if (ds.key_tag != sig.keyid ||
 			    ds.algorithm != sig.algorithm)
 				continue;
 
@@ -1734,8 +1852,7 @@ validatezonekey(dns_validator_t *val) {
 				 * This really shouldn't happen, but...
 				 */
 				continue;
-
+			result = verify(val, dstkey, &sigrdata, sig.keyid);
-			result = verify(val, dstkey, &sigrdata);
 			dst_key_free(&dstkey);
 			if (result == ISC_R_SUCCESS)
 				break;
@@ -1765,14 +1882,14 @@ validatezonekey(dns_validator_t *val) {
 		return (DNS_R_NOVALIDSIG);
 }
 
-/*
+/*%
  * Starts a positive response validation.
  *
  * Returns:
- *	ISC_R_SUCCESS	Validation completed successfully
+ * \li	ISC_R_SUCCESS	Validation completed successfully
- *	DNS_R_WAIT	Validation has started but is waiting
+ * \li	DNS_R_WAIT	Validation has started but is waiting
  *			for an event.
- *	Other return codes are possible and all indicate failure.
+ * \li	Other return codes are possible and all indicate failure.
  */
 static isc_result_t
 start_positive_validation(dns_validator_t *val) {
@@ -1785,6 +1902,14 @@ start_positive_validation(dns_validator_t *val) {
 	return (validatezonekey(val));
 }
 
+/*%
+ * Look for NODATA at the wildcard and NOWILDCARD proofs in the
+ * previously validated NSEC records.  As these proofs are mutually
+ * exclusive we stop when one is found.
+ *
+ * Returns
+ * \li	ISC_R_SUCCESS 
+ */
 static isc_result_t
 checkwildcard(dns_validator_t *val) {
 	dns_name_t *name, *wild;
@@ -1857,6 +1982,18 @@ checkwildcard(dns_validator_t *val) {
 	return (result);
 }
 
+/*%
+ * Prove a negative answer is good or that there is a NOQNAME when the
+ * answer is from a wildcard.
+ *
+ * Loop through the authority section looking for NODATA, NOWILDCARD
+ * and NOQNAME proofs in the NSEC records by calling authvalidated().
+ *
+ * If the required proofs are found we are done.
+ *
+ * If the proofs are not found attempt to prove this is a unsecure
+ * response.
+ */
 static isc_result_t
 nsecvalidate(dns_validator_t *val, isc_boolean_t resume) {
 	dns_name_t *name;
@@ -1952,7 +2089,8 @@ nsecvalidate(dns_validator_t *val, isc_boolean_t resume) {
 		return (result);
 
 	/*
-	 * Do we only need to check for NOQNAME?
+	 * Do we only need to check for NOQNAME?  To get here we must have
+	 * had a secure wildcard answer.
 	 */
 	if ((val->attributes & VALATTR_NEEDNODATA) == 0 &&
 	    (val->attributes & VALATTR_NEEDNOWILDCARD) == 0 &&
@@ -1988,28 +2126,17 @@ nsecvalidate(dns_validator_t *val, isc_boolean_t resume) {
 	    ((val->attributes & VALATTR_NEEDNOQNAME) != 0 &&
 	     (val->attributes & VALATTR_FOUNDNOQNAME) != 0 &&
 	     (val->attributes & VALATTR_NEEDNOWILDCARD) != 0 &&
-	     (val->attributes & VALATTR_FOUNDNOWILDCARD) != 0))
+	     (val->attributes & VALATTR_FOUNDNOWILDCARD) != 0)) {
-		val->attributes |= VALATTR_FOUNDNONEXISTENCE;
-
-	if ((val->attributes & VALATTR_FOUNDNONEXISTENCE) == 0) {
-		if (!val->seensig && val->soaset != NULL) {
-			result = create_validator(val, val->soaname,
-						  dns_rdatatype_soa,
-						  val->soaset, NULL,
-						  negauthvalidated,
-						  "nsecvalidate");
-			if (result != ISC_R_SUCCESS)
-				return (result);
-			return (DNS_R_WAIT);
-		}
 		validator_log(val, ISC_LOG_DEBUG(3),
-			      "nonexistence proof not found");
+			      "nonexistence proof(s) found");
-		return (DNS_R_NOVALIDNSEC);
-	} else {
-		validator_log(val, ISC_LOG_DEBUG(3),
-			      "nonexistence proof found");
 		return (ISC_R_SUCCESS);
 	}
+
+	validator_log(val, ISC_LOG_DEBUG(3),
+		      "nonexistence proof(s) not found");
+	val->attributes |= VALATTR_AUTHNONPENDING;
+	val->attributes |= VALATTR_INSECURITY;
+	return (proveunsecure(val, ISC_FALSE));
 }
 
 static isc_boolean_t
@@ -2036,6 +2163,11 @@ check_ds(dns_validator_t *val, dns_name_t *name, dns_rdataset_t *rdataset) {
 	return (ISC_FALSE);
 }
 
+/*%
+ * Callback from fetching a DLV record.
+ * 
+ * Resumes the DLV lookup process.
+ */
 static void
 dlvfetched(isc_task_t *task, isc_event_t *event) {
 	char namebuf[DNS_NAME_FORMATSIZE];
@@ -2072,9 +2204,7 @@ dlvfetched(isc_task_t *task, isc_event_t *event) {
 		dns_rdataset_clone(&val->frdataset, &val->dlv);
 		val->havedlvsep = ISC_TRUE;
 		validator_log(val, ISC_LOG_DEBUG(3), "DLV %s found", namebuf);
-		result = dlv_validator_start(val);
+		dlv_validator_start(val);
-		if (result != DNS_R_WAIT)
-			validator_done(val, result);
 	} else if (eresult == DNS_R_NXRRSET ||
 		   eresult == DNS_R_NXDOMAIN ||
 		   eresult == DNS_R_NCACHENXRRSET ||
@@ -2085,9 +2215,7 @@ dlvfetched(isc_task_t *task, isc_event_t *event) {
 					namebuf, sizeof(namebuf));
 			validator_log(val, ISC_LOG_DEBUG(3), "DLV %s found",
 				      namebuf);
-			result = dlv_validator_start(val);
+			dlv_validator_start(val);
-			if (result != DNS_R_WAIT)
-				validator_done(val, result);
 		} else if (result == ISC_R_NOTFOUND) {
 			validator_log(val, ISC_LOG_DEBUG(3), "DLV not found");
 			markanswer(val);
@@ -2101,6 +2229,7 @@ dlvfetched(isc_task_t *task, isc_event_t *event) {
 	} else {
 		validator_log(val, ISC_LOG_DEBUG(3), "DLV lookup: %s",
 			      dns_result_totext(eresult));
+		validator_done(val, eresult);
 	}
 	want_destroy = exit_check(val);
 	UNLOCK(&val->lock);
@@ -2108,6 +2237,14 @@ dlvfetched(isc_task_t *task, isc_event_t *event) {
 		destroy(val);
 }
 
+/*%
+ * Start the DLV lookup proccess.
+ * 
+ * Returns
+ * \li	ISC_R_SUCCESS
+ * \li	DNS_R_WAIT
+ * \li	Others on validation failures.
+ */
 static isc_result_t
 startfinddlvsep(dns_validator_t *val, dns_name_t *unsecure) {
 	char namebuf[DNS_NAME_FORMATSIZE];
@@ -2142,9 +2279,19 @@ startfinddlvsep(dns_validator_t *val, dns_name_t *unsecure) {
 	dns_name_format(dns_fixedname_name(&val->dlvsep), namebuf,
 			sizeof(namebuf));
 	validator_log(val, ISC_LOG_DEBUG(3), "DLV %s found", namebuf);
-	return (dlv_validator_start(val));
+	dlv_validator_start(val);
+	return (DNS_R_WAIT);
 }
 
+/*%
+ * Continue the DLV lookup process.
+ *
+ * Returns
+ * \li	ISC_R_SUCCESS
+ * \li	ISC_R_NOTFOUND
+ * \li	DNS_R_WAIT
+ * \li	Others on validation failure.
+ */
 static isc_result_t
 finddlvsep(dns_validator_t *val, isc_boolean_t resume) {
 	char namebuf[DNS_NAME_FORMATSIZE];
@@ -2238,11 +2385,24 @@ finddlvsep(dns_validator_t *val, isc_boolean_t resume) {
 	return (ISC_R_NOTFOUND);
 }
 
-/*
+/*%
  * proveunsecure walks down from the SEP looking for a break in the
- * chain of trust.   That occurs when we can prove the DS record does
+ * chain of trust.  That occurs when we can prove the DS record does
  * not exist at a delegation point or the DS exists at a delegation
  * but we don't support the algorithm/digest.
+ *
+ * If DLV is active and we look for a DLV record at or below the
+ * point we go insecure.  If found we restart the validation process.
+ * If not found or DLV isn't active we mark the response as a answer.
+ *
+ * Returns:
+ * \li	ISC_R_SUCCESS		val->event->name is in a unsecure zone
+ * \li	DNS_R_WAIT		validation is in progress.
+ * \li	DNS_R_MUSTBESECURE	val->event->name is supposed to be secure
+ *				(policy) but we proved that it is unsecure.
+ * \li	DNS_R_NOVALIDSIG
+ * \li	DNS_R_NOVALIDNSEC
+ * \li	DNS_R_NOTINSECURE
  */
 static isc_result_t
 proveunsecure(dns_validator_t *val, isc_boolean_t resume) {
@@ -2402,8 +2562,7 @@ proveunsecure(dns_validator_t *val, isc_boolean_t resume) {
 				goto out;
 			return (DNS_R_WAIT);
 		} else if (result == DNS_R_NXDOMAIN ||
-			   result == DNS_R_NCACHENXDOMAIN)
+			   result == DNS_R_NCACHENXDOMAIN) {
-		{
 			/*
 			 * This is not a zone cut.  Assuming things are
 			 * as expected, continue.
@@ -2448,7 +2607,10 @@ proveunsecure(dns_validator_t *val, isc_boolean_t resume) {
 	return (result);
 }
 
-static isc_result_t
+/*%
+ * Reset state and revalidate the answer using DLV.
+ */
+static void
 dlv_validator_start(dns_validator_t *val) {
 	isc_event_t *event;
 
@@ -2462,9 +2624,20 @@ dlv_validator_start(dns_validator_t *val) {
 
 	event = (isc_event_t *)val->event;
 	isc_task_send(val->task, &event);
-	return (DNS_R_WAIT);
 }
 
+/*%
+ * Start the validation process.
+ *
+ * Attempt to valididate the answer based on the category it appears to
+ * fall in.
+ * \li	1. secure positive answer.
+ * \li	2. unsecure positive answer.
+ * \li	3. a negative answer (secure or unsecure).
+ *
+ * Note a answer that appears to be a secure positive answer may actually
+ * be a unsecure positive answer.
+ */
 static void
 validator_start(isc_task_t *task, isc_event_t *event) {
 	dns_validator_t *val;
@@ -2536,7 +2709,6 @@ validator_start(isc_task_t *task, isc_event_t *event) {
 		validator_log(val, ISC_LOG_DEBUG(3),
 			      "attempting negative response validation");
 
-		val->attributes |= VALATTR_NEGATIVE;
 		if (val->event->message->rcode == dns_rcode_nxdomain) {
 			val->attributes |= VALATTR_NEEDNOQNAME;
 			val->attributes |= VALATTR_NEEDNOWILDCARD;