]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
1939. [bug] The resolver could dereference a null pointer after
authorMark Andrews <marka@isc.org>
Thu, 3 Nov 2005 00:58:00 +0000 (00:58 +0000)
committerMark Andrews <marka@isc.org>
Thu, 3 Nov 2005 00:58:00 +0000 (00:58 +0000)
                        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
doc/arm/Bv9ARM-book.xml
lib/dns/include/dns/validator.h
lib/dns/resolver.c
lib/dns/validator.c

diff --git a/CHANGES b/CHANGES
index 7a15c0801f0e9a6cbd36cba1df493765f6262fbb..70230cacda7e4c2c9a234c914bd742eaf29c0fa3 100644 (file)
--- a/CHANGES
+++ b/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]
index de23706acc75f281716364aacaebe63c0d31a531..03dfbcc76bb074fd96aa2a8e256ad34ce0bd036a 100644 (file)
@@ -18,7 +18,7 @@
  - PERFORMANCE OF THIS SOFTWARE.
 -->
 
-<!-- File: $Id: Bv9ARM-book.xml,v 1.241.18.38 2005/11/02 22:24:51 marka Exp $ -->
+<!-- File: $Id: Bv9ARM-book.xml,v 1.241.18.39 2005/11/03 00:57:58 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
-            DNSSEC
+            The <command>trusted-keys</command> statement defines 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
index a6f55bfa0542506082033190c3a9998923ab1d6d..a202b548a7408346a026dcec96fa386a98ae26ee 100644 (file)
@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: validator.h,v 1.27.18.5 2005/08/25 01:54:01 marka Exp $ */
+/* $Id: validator.h,v 1.27.18.6 2005/11/03 00:58:00 marka Exp $ */
 
 #ifndef DNS_VALIDATOR_H
 #define DNS_VALIDATOR_H 1
  *****/
 
 /*! \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.
+ *
+ * It can also optionally implement ISC's DNSSEC look-aside validation.
  *
- * XXX TBS XXX
+ * 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  Drafts: TBS
+ *\li  RFCs:   1034, 1035, 2181, 4033, 4034, 4035.
  */
 
 #include <isc/lang.h>
  * '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
index 586546ff56a6baed57b80c6287048696d2bfb882..e9581f4ad320a6e05bb42753ef3bd053c69e4811 100644 (file)
@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: resolver.c,v 1.284.18.34 2005/10/14 01:28:27 marka Exp $ */
+/* $Id: resolver.c,v 1.284.18.35 2005/11/03 00:57:59 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
-                * entry we're trying to add.
+                * If the cache now contains a negative entry and we
+                * 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
-                        * entry.
+                        * The cache data is a negative cache 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) {
-               if (NXDOMAIN(ardataset))
-                       *eresultp = DNS_R_NCACHENXDOMAIN;
-               else
-                       *eresultp = DNS_R_NCACHENXRRSET;
+               result = ISC_R_SUCCESS;
        }
 
        return (result);
index 1193c053879db02e88c33d8f65c5346573733e44..c0b9e2c69dbe83abc1a5a2d6466cb36edb1a96cb 100644 (file)
@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: validator.c,v 1.119.18.15 2005/11/02 01:53:25 marka Exp $ */
+/* $Id: validator.c,v 1.119.18.16 2005/11/03 00:57:59 marka Exp $ */
 
 /*! \file */
 
 #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_FOUNDNONEXISTENCE      0x0002
-#define VALATTR_TRIEDVERIFY            0x0004
-#define VALATTR_NEGATIVE               0x0008
-#define VALATTR_INSECURITY             0x0010
-#define VALATTR_DLVTRIED               0x0020
+#define VALATTR_SHUTDOWN               0x0001  /*%< Shutting down. */
+#define VALATTR_TRIEDVERIFY            0x0004  /*%< We have found a key and
+                                                * have attempted a verify. */
+#define VALATTR_INSECURITY             0x0010  /*%< Attempting proveunsecure. */
+#define VALATTR_DLVTRIED               0x0020  /*%< Looked for a DLV record. */
+#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) {
-       dns_validatorevent_t *devent;
-       dns_validator_t *val;
-       isc_boolean_t want_destroy;
-       isc_result_t eresult;
-
-       UNUSED(task);
-       INSIST(event->ev_type == DNS_EVENT_VALIDATORDONE);
-
-       devent = (dns_validatorevent_t *)event;
-       val = devent->ev_arg;
-       eresult = devent->result;
-       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);
-}
-
+/*%
+ * Looks for the requested name and type in the view (zones and cache).
+ *
+ * When looking for a DLV record also checks to make sure the NSEC record
+ * returns covers the query name as part of aggressive negative caching.
+ *
+ * Returns:
+ * \li ISC_R_SUCCESS
+ * \li ISC_R_NOTFOUND
+ * \li DNS_R_NCACHENXDOMAIN
+ * \li DNS_R_NCACHENXRRSET
+ * \li DNS_R_NXRRSET
+ * \li DNS_R_NXDOMAIN
+ */
 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",
-                     isc_result_totext(result));
+                     "verify rdataset (keyid=%u): %s",
+                     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
- *     DNS_R_WAIT      Validation has started but is waiting
+ * \li ISC_R_SUCCESS   Validation completed successfully
+ * \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
- *     DNS_R_WAIT      Validation has started but is waiting
+ * \li ISC_R_SUCCESS   Validation completed successfully
+ * \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);
+                       result = verify(val, dstkey, &sigrdata, sig.keyid);
                        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
- *     DNS_R_WAIT      Validation has started but is waiting
+ * \li ISC_R_SUCCESS   Validation completed successfully
+ * \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_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");
-               return (DNS_R_NOVALIDNSEC);
-       } else {
+            (val->attributes & VALATTR_FOUNDNOWILDCARD) != 0)) {
                validator_log(val, ISC_LOG_DEBUG(3),
-                             "nonexistence proof found");
+                             "nonexistence proof(s) 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);
-               if (result != DNS_R_WAIT)
-                       validator_done(val, result);
+               dlv_validator_start(val);
        } 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);
-                       if (result != DNS_R_WAIT)
-                               validator_done(val, result);
+                       dlv_validator_start(val);
                } 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;