From 32d1cc1562f51dae332877672869bfa7b91b7700 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 6 Aug 2019 13:44:30 -0700 Subject: [PATCH 01/15] style, braces, whitespace --- lib/dns/validator.c | 884 +++++++++++++++++++++++++++----------------- 1 file changed, 551 insertions(+), 333 deletions(-) diff --git a/lib/dns/validator.c b/lib/dns/validator.c index ceb45260ed..8e996429b2 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -42,68 +42,63 @@ /*! \file * \brief - * Basic processing sequences. + * Basic processing sequences: * * \li When called with rdataset and sigrdataset: - * validator_start -> validate -> proveunsecure - * - * validator_start -> validate -> nsecvalidate (secure wildcard answer) + * validator_start -> validate -> proveunsecure + * validator_start -> validate -> nsecvalidate (secure wildcard answer) * * \li When called with rdataset: - * validator_start -> proveunsecure + * validator_start -> proveunsecure * * \li When called without a rdataset: - * validator_start -> nsecvalidate -> proveunsecure + * 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. + * 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. */ -#define VALIDATOR_MAGIC ISC_MAGIC('V', 'a', 'l', '?') -#define VALID_VALIDATOR(v) ISC_MAGIC_VALID(v, VALIDATOR_MAGIC) +#define VALIDATOR_MAGIC ISC_MAGIC('V', 'a', 'l', '?') +#define VALID_VALIDATOR(v) ISC_MAGIC_VALID(v, VALIDATOR_MAGIC) -#define VALATTR_SHUTDOWN 0x0001 /*%< Shutting down. */ -#define VALATTR_CANCELED 0x0002 /*%< Canceled. */ -#define VALATTR_TRIEDVERIFY 0x0004 /*%< We have found a key and +#define VALATTR_SHUTDOWN 0x0001 /*%< Shutting down. */ +#define VALATTR_CANCELED 0x0002 /*%< Canceled. */ +#define VALATTR_TRIEDVERIFY 0x0004 /*%< We have found a key and * have attempted a verify. */ -#define VALATTR_INSECURITY 0x0010 /*%< Attempting proveunsecure. */ +#define VALATTR_INSECURITY 0x0010 /*%< Attempting proveunsecure. */ /*! * NSEC proofs to be looked for. */ -#define VALATTR_NEEDNOQNAME 0x00000100 -#define VALATTR_NEEDNOWILDCARD 0x00000200 -#define VALATTR_NEEDNODATA 0x00000400 +#define VALATTR_NEEDNOQNAME 0x00000100 +#define VALATTR_NEEDNOWILDCARD 0x00000200 +#define VALATTR_NEEDNODATA 0x00000400 /*! * NSEC proofs that have been found. */ -#define VALATTR_FOUNDNOQNAME 0x00001000 -#define VALATTR_FOUNDNOWILDCARD 0x00002000 -#define VALATTR_FOUNDNODATA 0x00004000 -#define VALATTR_FOUNDCLOSEST 0x00008000 +#define VALATTR_FOUNDNOQNAME 0x00001000 +#define VALATTR_FOUNDNOWILDCARD 0x00002000 +#define VALATTR_FOUNDNODATA 0x00004000 +#define VALATTR_FOUNDCLOSEST 0x00008000 +#define VALATTR_FOUNDOPTOUT 0x00010000 +#define VALATTR_FOUNDUNKNOWN 0x00020000 -/* - * - */ -#define VALATTR_FOUNDOPTOUT 0x00010000 -#define VALATTR_FOUNDUNKNOWN 0x00020000 - -#define NEEDNODATA(val) ((val->attributes & VALATTR_NEEDNODATA) != 0) -#define NEEDNOQNAME(val) ((val->attributes & VALATTR_NEEDNOQNAME) != 0) -#define NEEDNOWILDCARD(val) ((val->attributes & VALATTR_NEEDNOWILDCARD) != 0) -#define FOUNDNODATA(val) ((val->attributes & VALATTR_FOUNDNODATA) != 0) -#define FOUNDNOQNAME(val) ((val->attributes & VALATTR_FOUNDNOQNAME) != 0) +#define NEEDNODATA(val) ((val->attributes & VALATTR_NEEDNODATA) != 0) +#define NEEDNOQNAME(val) ((val->attributes & VALATTR_NEEDNOQNAME) != 0) +#define NEEDNOWILDCARD(val) ((val->attributes & VALATTR_NEEDNOWILDCARD) != 0) +#define FOUNDNODATA(val) ((val->attributes & VALATTR_FOUNDNODATA) != 0) +#define FOUNDNOQNAME(val) ((val->attributes & VALATTR_FOUNDNOQNAME) != 0) #define FOUNDNOWILDCARD(val) ((val->attributes & VALATTR_FOUNDNOWILDCARD) != 0) -#define FOUNDCLOSEST(val) ((val->attributes & VALATTR_FOUNDCLOSEST) != 0) -#define FOUNDOPTOUT(val) ((val->attributes & VALATTR_FOUNDOPTOUT) != 0) +#define FOUNDCLOSEST(val) ((val->attributes & VALATTR_FOUNDCLOSEST) != 0) +#define FOUNDOPTOUT(val) ((val->attributes & VALATTR_FOUNDOPTOUT) != 0) -#define SHUTDOWN(v) (((v)->attributes & VALATTR_SHUTDOWN) != 0) -#define CANCELED(v) (((v)->attributes & VALATTR_CANCELED) != 0) +#define SHUTDOWN(v) (((v)->attributes & VALATTR_SHUTDOWN) != 0) +#define CANCELED(v) (((v)->attributes & VALATTR_CANCELED) != 0) -#define NEGATIVE(r) (((r)->attributes & DNS_RDATASETATTR_NEGATIVE) != 0) +#define NEGATIVE(r) (((r)->attributes & DNS_RDATASETATTR_NEGATIVE) != 0) static void destroy(dns_validator_t *val); @@ -122,17 +117,16 @@ static isc_result_t nsecvalidate(dns_validator_t *val, bool resume); static isc_result_t -proveunsecure(dns_validator_t *val, bool have_ds, - bool resume); +proveunsecure(dns_validator_t *val, bool have_ds, bool resume); static void validator_logv(dns_validator_t *val, isc_logcategory_t *category, isc_logmodule_t *module, int level, const char *fmt, va_list ap) - ISC_FORMAT_PRINTF(5, 0); + ISC_FORMAT_PRINTF(5, 0); static void validator_log(void *val, int level, const char *fmt, ...) - ISC_FORMAT_PRINTF(3, 4); + ISC_FORMAT_PRINTF(3, 4); static void validator_logcreate(dns_validator_t *val, @@ -145,18 +139,21 @@ validator_logcreate(dns_validator_t *val, static inline void markanswer(dns_validator_t *val, const char *where) { validator_log(val, ISC_LOG_DEBUG(3), "marking as answer (%s)", where); - if (val->event->rdataset != NULL) + if (val->event->rdataset != NULL) { dns_rdataset_settrust(val->event->rdataset, dns_trust_answer); - if (val->event->sigrdataset != NULL) + } + if (val->event->sigrdataset != NULL) { dns_rdataset_settrust(val->event->sigrdataset, dns_trust_answer); + } } static inline void marksecure(dns_validatorevent_t *event) { dns_rdataset_settrust(event->rdataset, dns_trust_secure); - if (event->sigrdataset != NULL) + if (event->sigrdataset != NULL) { dns_rdataset_settrust(event->sigrdataset, dns_trust_secure); + } event->secure = true; } @@ -164,8 +161,9 @@ static void validator_done(dns_validator_t *val, isc_result_t result) { isc_task_t *task; - if (val->event == NULL) + if (val->event == NULL) { return; + } /* * Caller must be holding the lock. @@ -185,13 +183,15 @@ exit_check(dns_validator_t *val) { /* * Caller must be holding the lock. */ - if (!SHUTDOWN(val)) + if (!SHUTDOWN(val)) { return (false); + } INSIST(val->event == NULL); - if (val->fetch != NULL || val->subvalidator != NULL) + if (val->fetch != NULL || val->subvalidator != NULL) { return (false); + } return (true); } @@ -222,15 +222,17 @@ isdelegation(dns_name_t *name, dns_rdataset_t *rdataset, REQUIRE(dbresult == DNS_R_NXRRSET || dbresult == DNS_R_NCACHENXRRSET); dns_rdataset_init(&set); - if (dbresult == DNS_R_NXRRSET) + if (dbresult == DNS_R_NXRRSET) { dns_rdataset_clone(rdataset, &set); - else { + } else { result = dns_ncache_getrdataset(rdataset, name, dns_rdatatype_nsec, &set); - if (result == ISC_R_NOTFOUND) + if (result == ISC_R_NOTFOUND) { goto trynsec3; - if (result != ISC_R_SUCCESS) + } + if (result != ISC_R_SUCCESS) { return (false); + } } INSIST(set.type == dns_rdatatype_nsec); @@ -278,14 +280,16 @@ isdelegation(dns_name_t *name, dns_rdataset_t *rdataset, dns_rdata_reset(&rdata); dns_rdataset_current(&set, &rdata); (void)dns_rdata_tostruct(&rdata, &nsec3, NULL); - if (nsec3.hash != 1) + if (nsec3.hash != 1) { continue; + } length = isc_iterated_hash(hash, nsec3.hash, nsec3.iterations, nsec3.salt, nsec3.salt_length, name->ndata, name->length); - if (length != isc_buffer_usedlength(&buffer)) + if (length != isc_buffer_usedlength(&buffer)) { continue; + } order = memcmp(hash, owner, length); if (order == 0) { found = dns_nsec3_typepresent(&rdata, @@ -293,8 +297,9 @@ isdelegation(dns_name_t *name, dns_rdataset_t *rdataset, dns_rdataset_disassociate(&set); return (found); } - if ((nsec3.flags & DNS_NSEC3FLAG_OPTOUT) == 0) + if ((nsec3.flags & DNS_NSEC3FLAG_OPTOUT) == 0) { continue; + } /* * Does this optout span cover the name? */ @@ -302,7 +307,8 @@ isdelegation(dns_name_t *name, dns_rdataset_t *rdataset, if ((scope < 0 && order > 0 && memcmp(hash, nsec3.next, length) < 0) || (scope >= 0 && (order > 0 || - memcmp(hash, nsec3.next, length) < 0))) + memcmp(hash, nsec3.next, + length) < 0))) { dns_rdataset_disassociate(&set); return (true); @@ -337,12 +343,15 @@ fetch_callback_validator(isc_task_t *task, isc_event_t *event) { eresult = devent->result; /* Free resources which are not of interest. */ - if (devent->node != NULL) + if (devent->node != NULL) { dns_db_detachnode(devent->db, &devent->node); - if (devent->db != NULL) + } + if (devent->db != NULL) { dns_db_detach(&devent->db); - if (dns_rdataset_isassociated(&val->fsigrdataset)) + } + if (dns_rdataset_isassociated(&val->fsigrdataset)) { dns_rdataset_disassociate(&val->fsigrdataset); + } isc_event_free(&event); INSIST(val->event != NULL); @@ -362,8 +371,9 @@ fetch_callback_validator(isc_task_t *task, isc_event_t *event) { */ if (rdataset->trust >= dns_trust_secure) { result = get_dst_key(val, val->siginfo, rdataset); - if (result == ISC_R_SUCCESS) + if (result == ISC_R_SUCCESS) { val->keyset = &val->frdataset; + } } result = validate(val, true); if (result == DNS_R_NOVALIDSIG && @@ -374,26 +384,31 @@ fetch_callback_validator(isc_task_t *task, isc_event_t *event) { "falling back to insecurity proof"); val->attributes |= VALATTR_INSECURITY; result = proveunsecure(val, false, false); - if (result == DNS_R_NOTINSECURE) + if (result == DNS_R_NOTINSECURE) { result = saved_result; + } } - if (result != DNS_R_WAIT) + if (result != DNS_R_WAIT) { validator_done(val, result); + } } else { validator_log(val, ISC_LOG_DEBUG(3), "fetch_callback_validator: got %s", isc_result_totext(eresult)); - if (eresult == ISC_R_CANCELED) + if (eresult == ISC_R_CANCELED) { validator_done(val, eresult); - else + } else { validator_done(val, DNS_R_BROKENCHAIN); + } } want_destroy = exit_check(val); UNLOCK(&val->lock); - if (fetch != NULL) + if (fetch != NULL) { dns_resolver_destroyfetch(&fetch); - if (want_destroy) + } + if (want_destroy) { destroy(val); + } } /*% @@ -419,12 +434,15 @@ dsfetched(isc_task_t *task, isc_event_t *event) { eresult = devent->result; /* Free resources which are not of interest. */ - if (devent->node != NULL) + if (devent->node != NULL) { dns_db_detachnode(devent->db, &devent->node); - if (devent->db != NULL) + } + if (devent->db != NULL) { dns_db_detach(&devent->db); - if (dns_rdataset_isassociated(&val->fsigrdataset)) + } + if (dns_rdataset_isassociated(&val->fsigrdataset)) { dns_rdataset_disassociate(&val->fsigrdataset); + } isc_event_free(&event); INSIST(val->event != NULL); @@ -438,38 +456,43 @@ dsfetched(isc_task_t *task, isc_event_t *event) { } else if (eresult == ISC_R_SUCCESS) { validator_log(val, ISC_LOG_DEBUG(3), "dsset with trust %s", - dns_trust_totext(rdataset->trust)); + dns_trust_totext(rdataset->trust)); val->dsset = &val->frdataset; result = validatezonekey(val); - if (result != DNS_R_WAIT) + if (result != DNS_R_WAIT) { validator_done(val, result); + } } else if (eresult == DNS_R_CNAME || eresult == DNS_R_NXRRSET || eresult == DNS_R_NCACHENXRRSET || - eresult == DNS_R_SERVFAIL) /* RFC 1034 parent? */ + eresult == DNS_R_SERVFAIL) /* RFC 1034 parent? */ { validator_log(val, ISC_LOG_DEBUG(3), "falling back to insecurity proof (%s)", dns_result_totext(eresult)); val->attributes |= VALATTR_INSECURITY; result = proveunsecure(val, false, false); - if (result != DNS_R_WAIT) + if (result != DNS_R_WAIT) { validator_done(val, result); + } } else { validator_log(val, ISC_LOG_DEBUG(3), "dsfetched: got %s", isc_result_totext(eresult)); - if (eresult == ISC_R_CANCELED) + if (eresult == ISC_R_CANCELED) { validator_done(val, eresult); - else + } else { validator_done(val, DNS_R_BROKENCHAIN); + } } want_destroy = exit_check(val); UNLOCK(&val->lock); - if (fetch != NULL) + if (fetch != NULL) { dns_resolver_destroyfetch(&fetch); - if (want_destroy) + } + if (want_destroy) { destroy(val); + } } /*% @@ -500,12 +523,15 @@ dsfetched2(isc_task_t *task, isc_event_t *event) { eresult = devent->result; /* Free resources which are not of interest. */ - if (devent->node != NULL) + if (devent->node != NULL) { dns_db_detachnode(devent->db, &devent->node); - if (devent->db != NULL) + } + if (devent->db != NULL) { dns_db_detach(&devent->db); - if (dns_rdataset_isassociated(&val->fsigrdataset)) + } + if (dns_rdataset_isassociated(&val->fsigrdataset)) { dns_rdataset_disassociate(&val->fsigrdataset); + } INSIST(val->event != NULL); @@ -537,8 +563,9 @@ dsfetched2(isc_task_t *task, isc_event_t *event) { } } else { result = proveunsecure(val, false, true); - if (result != DNS_R_WAIT) + if (result != DNS_R_WAIT) { validator_done(val, result); + } } } else if (eresult == ISC_R_SUCCESS || eresult == DNS_R_NXDOMAIN || @@ -551,21 +578,25 @@ dsfetched2(isc_task_t *task, isc_event_t *event) { */ result = proveunsecure(val, (eresult == ISC_R_SUCCESS), true); - if (result != DNS_R_WAIT) + if (result != DNS_R_WAIT) { validator_done(val, result); + } } else { - if (eresult == ISC_R_CANCELED) + if (eresult == ISC_R_CANCELED) { validator_done(val, eresult); - else + } else { validator_done(val, DNS_R_NOVALIDDS); + } } isc_event_free(&event); want_destroy = exit_check(val); UNLOCK(&val->lock); - if (fetch != NULL) + if (fetch != NULL) { dns_resolver_destroyfetch(&fetch); - if (want_destroy) + } + if (want_destroy) { destroy(val); + } } /*% @@ -605,8 +636,9 @@ keyvalidated(isc_task_t *task, isc_event_t *event) { /* * Only extract the dst key if the keyset is secure. */ - if (val->frdataset.trust >= dns_trust_secure) + if (val->frdataset.trust >= dns_trust_secure) { (void) get_dst_key(val, val->siginfo, &val->frdataset); + } result = validate(val, true); if (result == DNS_R_NOVALIDSIG && (val->attributes & VALATTR_TRIEDVERIFY) == 0) @@ -616,17 +648,21 @@ keyvalidated(isc_task_t *task, isc_event_t *event) { "falling back to insecurity proof"); val->attributes |= VALATTR_INSECURITY; result = proveunsecure(val, false, false); - if (result == DNS_R_NOTINSECURE) + if (result == DNS_R_NOTINSECURE) { result = saved_result; + } } - if (result != DNS_R_WAIT) + if (result != DNS_R_WAIT) { validator_done(val, result); + } } else { if (eresult != DNS_R_BROKENCHAIN) { - if (dns_rdataset_isassociated(&val->frdataset)) + if (dns_rdataset_isassociated(&val->frdataset)) { dns_rdataset_expire(&val->frdataset); - if (dns_rdataset_isassociated(&val->fsigrdataset)) + } + if (dns_rdataset_isassociated(&val->fsigrdataset)) { dns_rdataset_expire(&val->fsigrdataset); + } } validator_log(val, ISC_LOG_DEBUG(3), "keyvalidated: got %s", @@ -635,8 +671,9 @@ keyvalidated(isc_task_t *task, isc_event_t *event) { } want_destroy = exit_check(val); UNLOCK(&val->lock); - if (want_destroy) + if (want_destroy) { destroy(val); + } } /*% @@ -681,7 +718,8 @@ dsvalidated(isc_task_t *task, isc_event_t *event) { if ((val->attributes & VALATTR_INSECURITY) != 0 && val->frdataset.covers == dns_rdatatype_ds && NEGATIVE(&val->frdataset) && - isdelegation(name, &val->frdataset, DNS_R_NCACHENXRRSET)) { + isdelegation(name, &val->frdataset, + DNS_R_NCACHENXRRSET)) { if (val->mustbesecure) { validator_log(val, ISC_LOG_WARNING, "must be secure failure, no DS " @@ -689,20 +727,24 @@ dsvalidated(isc_task_t *task, isc_event_t *event) { result = DNS_R_MUSTBESECURE; } else { markanswer(val, "dsvalidated"); - result = ISC_R_SUCCESS;; + result = ISC_R_SUCCESS; } } else if ((val->attributes & VALATTR_INSECURITY) != 0) { result = proveunsecure(val, have_dsset, true); - } else + } else { result = validatezonekey(val); - if (result != DNS_R_WAIT) + } + if (result != DNS_R_WAIT) { validator_done(val, result); + } } else { if (eresult != DNS_R_BROKENCHAIN) { - if (dns_rdataset_isassociated(&val->frdataset)) + if (dns_rdataset_isassociated(&val->frdataset)) { dns_rdataset_expire(&val->frdataset); - if (dns_rdataset_isassociated(&val->fsigrdataset)) + } + if (dns_rdataset_isassociated(&val->fsigrdataset)) { dns_rdataset_expire(&val->fsigrdataset); + } } validator_log(val, ISC_LOG_DEBUG(3), "dsvalidated: got %s", @@ -711,8 +753,9 @@ dsvalidated(isc_task_t *task, isc_event_t *event) { } want_destroy = exit_check(val); UNLOCK(&val->lock); - if (want_destroy) + if (want_destroy) { destroy(val); + } } /*% @@ -749,14 +792,17 @@ cnamevalidated(isc_task_t *task, isc_event_t *event) { validator_log(val, ISC_LOG_DEBUG(3), "cname with trust %s", dns_trust_totext(val->frdataset.trust)); result = proveunsecure(val, false, true); - if (result != DNS_R_WAIT) + if (result != DNS_R_WAIT) { validator_done(val, result); + } } else { if (eresult != DNS_R_BROKENCHAIN) { - if (dns_rdataset_isassociated(&val->frdataset)) + if (dns_rdataset_isassociated(&val->frdataset)) { dns_rdataset_expire(&val->frdataset); - if (dns_rdataset_isassociated(&val->fsigrdataset)) + } + if (dns_rdataset_isassociated(&val->fsigrdataset)) { dns_rdataset_expire(&val->fsigrdataset); + } } validator_log(val, ISC_LOG_DEBUG(3), "cnamevalidated: got %s", @@ -765,8 +811,9 @@ cnamevalidated(isc_task_t *task, isc_event_t *event) { } want_destroy = exit_check(val); UNLOCK(&val->lock); - if (want_destroy) + if (want_destroy) { destroy(val); + } } /*% @@ -804,21 +851,24 @@ authvalidated(isc_task_t *task, isc_event_t *event) { validator_log(val, ISC_LOG_DEBUG(3), "authvalidated: got %s", isc_result_totext(result)); - if (result == DNS_R_BROKENCHAIN) + if (result == DNS_R_BROKENCHAIN) { val->authfail++; - if (result == ISC_R_CANCELED) + } + if (result == ISC_R_CANCELED) { validator_done(val, result); - else { + } else { result = nsecvalidate(val, true); - if (result != DNS_R_WAIT) + if (result != DNS_R_WAIT) { validator_done(val, result); + } } } else { dns_name_t **proofs = val->event->proofs; dns_name_t *wild = dns_fixedname_name(&val->wild); - if (rdataset->trust == dns_trust_secure) + if (rdataset->trust == dns_trust_secure) { val->seensig = true; + } if (rdataset->type == dns_rdatatype_nsec && rdataset->trust == dns_trust_secure && @@ -827,13 +877,14 @@ authvalidated(isc_task_t *task, isc_event_t *event) { dns_nsec_noexistnodata(val->event->type, val->event->name, devent->name, rdataset, &exists, &data, wild, validator_log, val) - == ISC_R_SUCCESS) + == ISC_R_SUCCESS) { if (exists && !data) { val->attributes |= VALATTR_FOUNDNODATA; - if (NEEDNODATA(val)) + if (NEEDNODATA(val)) { proofs[DNS_VALIDATOR_NODATAPROOF] = devent->name; + } } if (!exists) { dns_name_t *closest; @@ -852,25 +903,30 @@ authvalidated(isc_task_t *task, isc_event_t *event) { */ if (clabels == 0 || dns_name_countlabels(wild) == clabels + 1) + { val->attributes |= VALATTR_FOUNDCLOSEST; + } /* * The NSEC noqname proof also contains * the closest encloser. */ - if (NEEDNOQNAME(val)) + if (NEEDNOQNAME(val)) { proofs[DNS_VALIDATOR_NOQNAMEPROOF] = devent->name; + } } } result = nsecvalidate(val, true); - if (result != DNS_R_WAIT) + if (result != DNS_R_WAIT) { validator_done(val, result); + } } want_destroy = exit_check(val); UNLOCK(&val->lock); - if (want_destroy) + if (want_destroy) { destroy(val); + } /* * Free stuff from the event. @@ -900,10 +956,12 @@ view_find(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type) { char namebuf[DNS_NAME_FORMATSIZE]; char typebuf[DNS_RDATATYPE_FORMATSIZE]; - if (dns_rdataset_isassociated(&val->frdataset)) + if (dns_rdataset_isassociated(&val->frdataset)) { dns_rdataset_disassociate(&val->frdataset); - if (dns_rdataset_isassociated(&val->fsigrdataset)) + } + if (dns_rdataset_isassociated(&val->fsigrdataset)) { dns_rdataset_disassociate(&val->fsigrdataset); + } if (isc_time_now(&now) == ISC_R_SUCCESS && dns_resolver_getbadcache(val->view->resolver, name, type, &now)) { @@ -922,25 +980,30 @@ view_find(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type) { &val->frdataset, &val->fsigrdataset); if (result == DNS_R_NXDOMAIN) { - if (dns_rdataset_isassociated(&val->frdataset)) + if (dns_rdataset_isassociated(&val->frdataset)) { dns_rdataset_disassociate(&val->frdataset); - if (dns_rdataset_isassociated(&val->fsigrdataset)) + } + if (dns_rdataset_isassociated(&val->fsigrdataset)) { dns_rdataset_disassociate(&val->fsigrdataset); + } } else if (result != ISC_R_SUCCESS && result != DNS_R_NCACHENXDOMAIN && result != DNS_R_NCACHENXRRSET && result != DNS_R_EMPTYNAME && result != DNS_R_NXRRSET && - result != ISC_R_NOTFOUND) { + result != ISC_R_NOTFOUND) + { goto notfound; } return (result); notfound: - if (dns_rdataset_isassociated(&val->frdataset)) + if (dns_rdataset_isassociated(&val->frdataset)) { dns_rdataset_disassociate(&val->frdataset); - if (dns_rdataset_isassociated(&val->fsigrdataset)) + } + if (dns_rdataset_isassociated(&val->fsigrdataset)) { dns_rdataset_disassociate(&val->fsigrdataset); + } return (ISC_R_NOTFOUND); } @@ -987,10 +1050,12 @@ create_fetch(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type, { unsigned int fopts = 0; - if (dns_rdataset_isassociated(&val->frdataset)) + if (dns_rdataset_isassociated(&val->frdataset)) { dns_rdataset_disassociate(&val->frdataset); - if (dns_rdataset_isassociated(&val->fsigrdataset)) + } + if (dns_rdataset_isassociated(&val->fsigrdataset)) { dns_rdataset_disassociate(&val->fsigrdataset); + } if (check_deadlock(val, name, type, NULL, NULL)) { validator_log(val, ISC_LOG_DEBUG(3), @@ -998,11 +1063,13 @@ create_fetch(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type, return (DNS_R_NOVALIDSIG); } - if ((val->options & DNS_VALIDATOR_NOCDFLAG) != 0) + if ((val->options & DNS_VALIDATOR_NOCDFLAG) != 0) { fopts |= DNS_FETCHOPT_NOCDFLAG; + } - if ((val->options & DNS_VALIDATOR_NONTA) != 0) + if ((val->options & DNS_VALIDATOR_NONTA) != 0) { fopts |= DNS_FETCHOPT_NONTA; + } validator_logcreate(val, name, type, caller, "fetch"); return (dns_resolver_createfetch(val->view->resolver, name, type, @@ -1032,7 +1099,8 @@ create_validator(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type, } /* OK to clear other options, but preserve NOCDFLAG and NONTA. */ - vopts |= (val->options & (DNS_VALIDATOR_NOCDFLAG|DNS_VALIDATOR_NONTA)); + vopts |= (val->options & (DNS_VALIDATOR_NOCDFLAG | + DNS_VALIDATOR_NONTA)); validator_logcreate(val, name, type, caller, "validator"); result = dns_validator_create(val->view, name, type, @@ -1063,16 +1131,17 @@ get_dst_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo, dst_key_t *oldkey = val->key; bool foundold; - if (oldkey == NULL) + if (oldkey == NULL) { foundold = true; - else { + } else { foundold = false; val->key = NULL; } result = dns_rdataset_first(rdataset); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { goto failure; + } do { dns_rdataset_current(rdataset, &rdata); @@ -1081,21 +1150,22 @@ get_dst_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo, INSIST(val->key == NULL); result = dst_key_fromdns(&siginfo->signer, rdata.rdclass, &b, val->view->mctx, &val->key); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { goto failure; + } if (siginfo->algorithm == (dns_secalg_t)dst_key_alg(val->key) && siginfo->keyid == (dns_keytag_t)dst_key_id(val->key) && dst_key_iszonekey(val->key)) { - if (foundold) + if (foundold) { /* * This is the key we're looking for. */ return (ISC_R_SUCCESS); - else if (dst_key_compare(oldkey, val->key) == true) - { + } else if (dst_key_compare(oldkey, + val->key) == true) { foundold = true; dst_key_free(&oldkey); } @@ -1104,12 +1174,14 @@ get_dst_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo, dns_rdata_reset(&rdata); result = dns_rdataset_next(rdataset); } while (result == ISC_R_SUCCESS); - if (result == ISC_R_NOMORE) + if (result == ISC_R_NOMORE) { result = ISC_R_NOTFOUND; + } failure: - if (oldkey != NULL) + if (oldkey != NULL) { dst_key_free(&oldkey); + } return (result); } @@ -1133,23 +1205,26 @@ get_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo) { namereln = dns_name_fullcompare(val->event->name, &siginfo->signer, &order, &nlabels); if (namereln != dns_namereln_subdomain && - namereln != dns_namereln_equal) + namereln != dns_namereln_equal) { return (DNS_R_CONTINUE); + } if (namereln == dns_namereln_equal) { /* * If this is a self-signed keyset, it must not be a zone key * (since get_key is not called from validatezonekey). */ - if (val->event->rdataset->type == dns_rdatatype_dnskey) + if (val->event->rdataset->type == dns_rdatatype_dnskey) { return (DNS_R_CONTINUE); + } /* * Records appearing in the parent zone at delegation * points cannot be self-signed. */ - if (dns_rdatatype_atparent(val->event->rdataset->type)) + if (dns_rdatatype_atparent(val->event->rdataset->type)) { return (DNS_R_CONTINUE); + } } else { /* * SOA and NS RRsets can only be signed by a key with @@ -1160,10 +1235,11 @@ get_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo) { { const char *type; - if (val->event->rdataset->type == dns_rdatatype_soa) + if (val->event->rdataset->type == dns_rdatatype_soa) { type = "SOA"; - else + } else { type = "NS"; + } validator_log(val, ISC_LOG_DEBUG(3), "%s signer mismatch", type); return (DNS_R_CONTINUE); @@ -1194,8 +1270,9 @@ get_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo) { &val->fsigrdataset, keyvalidated, "get_key"); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { return (result); + } return (DNS_R_WAIT); } else if (DNS_TRUST_PENDING(val->frdataset.trust)) { /* @@ -1234,8 +1311,9 @@ get_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo) { result = create_fetch(val, &siginfo->signer, dns_rdatatype_dnskey, fetch_callback_validator, "get_key"); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { return (result); + } return (DNS_R_WAIT); } else if (result == DNS_R_NCACHENXDOMAIN || result == DNS_R_NCACHENXRRSET || @@ -1247,14 +1325,17 @@ get_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo) { * This key doesn't exist. */ result = DNS_R_CONTINUE; - } else if (result == DNS_R_BROKENCHAIN) + } else if (result == DNS_R_BROKENCHAIN) { return (result); + } if (dns_rdataset_isassociated(&val->frdataset) && - val->keyset != &val->frdataset) + val->keyset != &val->frdataset) { dns_rdataset_disassociate(&val->frdataset); - if (dns_rdataset_isassociated(&val->fsigrdataset)) + } + if (dns_rdataset_isassociated(&val->fsigrdataset)) { dns_rdataset_disassociate(&val->fsigrdataset); + } return (result); } @@ -1291,7 +1372,9 @@ isselfsigned(dns_validator_t *val) { if (rdataset->type == dns_rdatatype_cname || rdataset->type == dns_rdatatype_dname) + { return (answer); + } INSIST(rdataset->type == dns_rdatatype_dnskey); @@ -1316,21 +1399,25 @@ isselfsigned(dns_validator_t *val) { if (sig.algorithm != key.algorithm || sig.keyid != keytag || !dns_name_equal(name, &sig.signer)) + { continue; + } dstkey = NULL; result = dns_dnssec_keyfromrdata(name, &rdata, mctx, &dstkey); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { continue; + } result = dns_dnssec_verify(name, rdataset, dstkey, true, val->view->maxbits, mctx, &sigrdata, NULL); dst_key_free(&dstkey); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { continue; + } if ((key.flags & DNS_KEYFLAG_REVOKE) == 0) { answer = true; continue; @@ -1371,19 +1458,23 @@ verify(dns_validator_t *val, dst_key_t *key, dns_rdata_t *rdata, ignore = true; goto again; } - if (ignore && (result == ISC_R_SUCCESS || result == DNS_R_FROMWILDCARD)) + + if (ignore && + (result == ISC_R_SUCCESS || result == DNS_R_FROMWILDCARD)) + { validator_log(val, ISC_LOG_INFO, "accepted expired %sRRSIG (keyid=%u)", (result == DNS_R_FROMWILDCARD) ? "wildcard " : "", keyid); - else if (result == DNS_R_SIGEXPIRED || result == DNS_R_SIGFUTURE) + } else if (result == DNS_R_SIGEXPIRED || result == DNS_R_SIGFUTURE) { validator_log(val, ISC_LOG_INFO, "verify failed due to bad signature (keyid=%u): " "%s", keyid, isc_result_totext(result)); - else + } else { validator_log(val, ISC_LOG_DEBUG(3), "verify rdataset (keyid=%u): %s", keyid, isc_result_totext(result)); + } if (result == DNS_R_FROMWILDCARD) { if (!dns_name_equal(val->event->name, wild)) { dns_name_t *closest; @@ -1446,26 +1537,30 @@ validate(dns_validator_t *val, bool resume) { sizeof(*val->siginfo)); } result = dns_rdata_tostruct(&rdata, val->siginfo, NULL); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { return (result); + } /* * At this point we could check that the signature algorithm * was known and "sufficiently good". */ if (!dns_resolver_algorithm_supported(val->view->resolver, - event->name, - val->siginfo->algorithm)) { + event->name, + val->siginfo->algorithm)) + { resume = false; continue; } if (!resume) { result = get_key(val, val->siginfo); - if (result == DNS_R_CONTINUE) + if (result == DNS_R_CONTINUE) { continue; /* Try the next SIG RR. */ - if (result != ISC_R_SUCCESS) + } + if (result != ISC_R_SUCCESS) { return (result); + } } /* @@ -1479,15 +1574,16 @@ validate(dns_validator_t *val, bool resume) { do { vresult = verify(val, val->key, &rdata, - val->siginfo->keyid); - if (vresult == ISC_R_SUCCESS) + val->siginfo->keyid); + if (vresult == ISC_R_SUCCESS) { break; + } if (val->keynode != NULL) { dns_keynode_t *nextnode = NULL; result = dns_keytable_findnextkeynode( - val->keytable, - val->keynode, - &nextnode); + val->keytable, + val->keynode, + &nextnode); dns_keytable_detachkeynode(val->keytable, &val->keynode); val->keynode = nextnode; @@ -1496,30 +1592,33 @@ validate(dns_validator_t *val, bool resume) { break; } val->key = dns_keynode_key(val->keynode); - if (val->key == NULL) + if (val->key == NULL) { break; + } } else { if (get_dst_key(val, val->siginfo, val->keyset) - != ISC_R_SUCCESS) + != ISC_R_SUCCESS) { break; + } } } while (1); - if (vresult != ISC_R_SUCCESS) + if (vresult != ISC_R_SUCCESS) { validator_log(val, ISC_LOG_DEBUG(3), "failed to verify rdataset"); - else { + } else { dns_rdataset_trimttl(event->rdataset, event->sigrdataset, val->siginfo, val->start, val->view->acceptexpired); } - if (val->keynode != NULL) + if (val->keynode != NULL) { dns_keytable_detachkeynode(val->keytable, &val->keynode); - else { - if (val->key != NULL) + } else { + if (val->key != NULL) { dst_key_free(&val->key); + } if (val->keyset != NULL) { dns_rdataset_disassociate(val->keyset); val->keyset = NULL; @@ -1529,7 +1628,8 @@ validate(dns_validator_t *val, bool resume) { if (NEEDNOQNAME(val)) { if (val->event->message == NULL) { validator_log(val, ISC_LOG_DEBUG(3), - "no message available for noqname proof"); + "no message available " + "for noqname proof"); return (DNS_R_NOVALIDSIG); } validator_log(val, ISC_LOG_DEBUG(3), @@ -1580,25 +1680,29 @@ checkkey(dns_validator_t *val, dns_rdata_t *keyrdata, uint16_t keyid, dns_rdataset_current(val->event->sigrdataset, &rdata); result = dns_rdata_tostruct(&rdata, &sig, NULL); RUNTIME_CHECK(result == ISC_R_SUCCESS); - if (keyid != sig.keyid || algorithm != sig.algorithm) + if (keyid != sig.keyid || algorithm != sig.algorithm) { continue; + } if (dstkey == NULL) { result = dns_dnssec_keyfromrdata(val->event->name, keyrdata, val->view->mctx, &dstkey); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { /* * This really shouldn't happen, but... */ continue; + } } result = verify(val, dstkey, &rdata, sig.keyid); - if (result == ISC_R_SUCCESS) + if (result == ISC_R_SUCCESS) { break; + } } - if (dstkey != NULL) + if (dstkey != NULL) { dst_key_free(&dstkey); + } return (result); } @@ -1626,8 +1730,9 @@ keyfromds(dns_validator_t *val, dns_rdataset_t *rdataset, dns_rdata_t *dsrdata, result = dns_rdata_tostruct(keyrdata, &key, NULL); RUNTIME_CHECK(result == ISC_R_SUCCESS); keytag = compute_keytag(keyrdata); - if (keyid != keytag || algorithm != key.algorithm) + if (keyid != keytag || algorithm != key.algorithm) { continue; + } dns_rdata_reset(&newdsrdata); result = dns_ds_buildrdata(val->event->name, keyrdata, digest, dsbuf, &newdsrdata); @@ -1637,8 +1742,9 @@ keyfromds(dns_validator_t *val, dns_rdataset_t *rdataset, dns_rdata_t *dsrdata, dns_result_totext(result)); continue; } - if (dns_rdata_compare(dsrdata, &newdsrdata) == 0) + if (dns_rdata_compare(dsrdata, &newdsrdata) == 0) { break; + } } return (result); } @@ -1694,8 +1800,9 @@ validatezonekey(dns_validator_t *val) { result = dns_rdata_tostruct(&sigrdata, &sig, NULL); RUNTIME_CHECK(result == ISC_R_SUCCESS); - if (!dns_name_equal(val->event->name, &sig.signer)) + if (!dns_name_equal(val->event->name, &sig.signer)) { continue; + } result = dns_keytable_findkeynode(val->keytable, val->event->name, @@ -1703,11 +1810,15 @@ validatezonekey(dns_validator_t *val) { sig.keyid, &keynode); if (result == ISC_R_NOTFOUND && dns_keytable_finddeepestmatch(val->keytable, - val->event->name, found) != ISC_R_SUCCESS) { + val->event->name, + found) + != ISC_R_SUCCESS) + { if (val->mustbesecure) { validator_log(val, ISC_LOG_WARNING, - "must be secure failure, " - "not beneath secure root"); + "must be secure " + "failure, not beneath " + "secure root"); return (DNS_R_MUSTBESECURE); } validator_log(val, ISC_LOG_DEBUG(3), @@ -1717,14 +1828,16 @@ validatezonekey(dns_validator_t *val) { } if (result == DNS_R_PARTIALMATCH || result == ISC_R_SUCCESS) + { atsep = true; + } while (result == ISC_R_SUCCESS) { dns_keynode_t *nextnode = NULL; dstkey = dns_keynode_key(keynode); if (dstkey == NULL) { dns_keytable_detachkeynode( - val->keytable, - &keynode); + val->keytable, + &keynode); break; } result = verify(val, dstkey, &sigrdata, @@ -1763,8 +1876,7 @@ validatezonekey(dns_validator_t *val) { validator_log(val, ISC_LOG_NOTICE, "unable to find a DNSKEY which verifies " "the DNSKEY RRset and also matches a " - "trusted key for '%s'", - namebuf); + "trusted key for '%s'", namebuf); return (DNS_R_NOVALIDKEY); } @@ -1804,8 +1916,9 @@ validatezonekey(dns_validator_t *val) { &val->fsigrdataset, dsvalidated, "validatezonekey"); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { return (result); + } return (DNS_R_WAIT); } else if (DNS_TRUST_PENDING(val->frdataset.trust)) { /* @@ -1826,8 +1939,9 @@ validatezonekey(dns_validator_t *val) { result = create_fetch(val, val->event->name, dns_rdatatype_ds, dsfetched, "validatezonekey"); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { return (result); + } return (DNS_R_WAIT); } else if (result == DNS_R_NCACHENXDOMAIN || result == DNS_R_NCACHENXRRSET || @@ -1839,14 +1953,17 @@ validatezonekey(dns_validator_t *val) { /* * The DS does not exist. */ - if (dns_rdataset_isassociated(&val->frdataset)) + if (dns_rdataset_isassociated(&val->frdataset)) { dns_rdataset_disassociate(&val->frdataset); - if (dns_rdataset_isassociated(&val->fsigrdataset)) + } + if (dns_rdataset_isassociated(&val->fsigrdataset)) { dns_rdataset_disassociate(&val->fsigrdataset); + } validator_log(val, ISC_LOG_DEBUG(2), "no DS record"); return (DNS_R_NOVALIDSIG); - } else if (result == DNS_R_BROKENCHAIN) + } else if (result == DNS_R_BROKENCHAIN) { return (result); + } } /* @@ -1882,7 +1999,8 @@ validatezonekey(dns_validator_t *val) { memset(digest_types, 1, sizeof(digest_types)); for (result = dns_rdataset_first(val->dsset); result == ISC_R_SUCCESS; - result = dns_rdataset_next(val->dsset)) { + result = dns_rdataset_next(val->dsset)) + { dns_rdata_reset(&dsrdata); dns_rdataset_current(val->dsset, &dsrdata); result = dns_rdata_tostruct(&dsrdata, &ds, NULL); @@ -1891,12 +2009,16 @@ validatezonekey(dns_validator_t *val) { if (!dns_resolver_ds_digest_supported(val->view->resolver, val->event->name, ds.digest_type)) + { continue; + } if (!dns_resolver_algorithm_supported(val->view->resolver, val->event->name, ds.algorithm)) + { continue; + } if ((ds.digest_type == DNS_DSDIGEST_SHA256 && ds.length == ISC_SHA256_DIGESTLENGTH) || @@ -1917,18 +2039,23 @@ validatezonekey(dns_validator_t *val) { result = dns_rdata_tostruct(&dsrdata, &ds, NULL); RUNTIME_CHECK(result == ISC_R_SUCCESS); - if (digest_types[ds.digest_type] == 0) + if (digest_types[ds.digest_type] == 0) { continue; + } if (!dns_resolver_ds_digest_supported(val->view->resolver, val->event->name, ds.digest_type)) + { continue; + } if (!dns_resolver_algorithm_supported(val->view->resolver, val->event->name, ds.algorithm)) + { continue; + } supported_algorithm = true; @@ -1953,8 +2080,9 @@ validatezonekey(dns_validator_t *val) { result = checkkey(val, &keyrdata, ds.key_tag, ds.algorithm); dns_rdataset_disassociate(&trdataset); - if (result == ISC_R_SUCCESS) + if (result == ISC_R_SUCCESS) { break; + } validator_log(val, ISC_LOG_DEBUG(3), "no RRSIG matching DS key"); } @@ -1994,8 +2122,9 @@ start_positive_validation(dns_validator_t *val) { /* * If this is not a key, go straight into validate(). */ - if (val->event->type != dns_rdatatype_dnskey || !isselfsigned(val)) + if (val->event->type != dns_rdatatype_dnskey || !isselfsigned(val)) { return (validate(val, false)); + } return (validatezonekey(val)); } @@ -2024,16 +2153,18 @@ val_rdataset_first(dns_validator_t *val, dns_name_t **namep, if (message != NULL) { result = dns_message_firstname(message, DNS_SECTION_AUTHORITY); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { return (result); + } dns_message_currentname(message, DNS_SECTION_AUTHORITY, namep); *rdatasetp = ISC_LIST_HEAD((*namep)->list); INSIST(*rdatasetp != NULL); } else { result = dns_rdataset_first(val->event->rdataset); - if (result == ISC_R_SUCCESS) + if (result == ISC_R_SUCCESS) { dns_ncache_current(val->event->rdataset, *namep, *rdatasetp); + } } return (result); } @@ -2067,9 +2198,10 @@ val_rdataset_next(dns_validator_t *val, dns_name_t **namep, } else { dns_rdataset_disassociate(*rdatasetp); result = dns_rdataset_next(val->event->rdataset); - if (result == ISC_R_SUCCESS) + if (result == ISC_R_SUCCESS) { dns_ncache_current(val->event->rdataset, *namep, *rdatasetp); + } } return (result); } @@ -2083,7 +2215,8 @@ val_rdataset_next(dns_validator_t *val, dns_name_t **namep, * \li ISC_R_SUCCESS */ static isc_result_t -checkwildcard(dns_validator_t *val, dns_rdatatype_t type, dns_name_t *zonename) +checkwildcard(dns_validator_t *val, dns_rdatatype_t type, + dns_name_t *zonename) { dns_name_t *name, *wild, tname; isc_result_t result; @@ -2118,7 +2251,9 @@ checkwildcard(dns_validator_t *val, dns_rdatatype_t type, dns_name_t *zonename) { if (rdataset->type != type || rdataset->trust != dns_trust_secure) + { continue; + } if (rdataset->type == dns_rdatatype_nsec && (NEEDNODATA(val) || NEEDNOWILDCARD(val)) && @@ -2126,22 +2261,26 @@ checkwildcard(dns_validator_t *val, dns_rdatatype_t type, dns_name_t *zonename) dns_nsec_noexistnodata(val->event->type, wild, name, rdataset, &exists, &data, NULL, validator_log, val) - == ISC_R_SUCCESS) + == ISC_R_SUCCESS) { dns_name_t **proofs = val->event->proofs; - if (exists && !data) + if (exists && !data) { val->attributes |= VALATTR_FOUNDNODATA; - if (exists && !data && NEEDNODATA(val)) + } + if (exists && !data && NEEDNODATA(val)) { proofs[DNS_VALIDATOR_NODATAPROOF] = - name; - if (!exists) + name; + } + if (!exists) { val->attributes |= - VALATTR_FOUNDNOWILDCARD; - if (!exists && NEEDNOQNAME(val)) - proofs[DNS_VALIDATOR_NOWILDCARDPROOF] = - name; - if (dns_rdataset_isassociated(&trdataset)) + VALATTR_FOUNDNOWILDCARD; + } + if (!exists && NEEDNOQNAME(val)) { + proofs[DNS_VALIDATOR_NOWILDCARDPROOF] = name; + } + if (dns_rdataset_isassociated(&trdataset)) { dns_rdataset_disassociate(&trdataset); + } return (ISC_R_SUCCESS); } @@ -2152,29 +2291,33 @@ checkwildcard(dns_validator_t *val, dns_rdatatype_t type, dns_name_t *zonename) rdataset, zonename, &exists, &data, NULL, NULL, NULL, NULL, NULL, NULL, validator_log, val) - == ISC_R_SUCCESS) + == ISC_R_SUCCESS) { dns_name_t **proofs = val->event->proofs; - if (exists && !data) + if (exists && !data) { val->attributes |= VALATTR_FOUNDNODATA; - if (exists && !data && NEEDNODATA(val)) - proofs[DNS_VALIDATOR_NODATAPROOF] = - name; - if (!exists) - val->attributes |= - VALATTR_FOUNDNOWILDCARD; - if (!exists && NEEDNOQNAME(val)) - proofs[DNS_VALIDATOR_NOWILDCARDPROOF] = - name; - if (dns_rdataset_isassociated(&trdataset)) + } + if (exists && !data && NEEDNODATA(val)) { + proofs[DNS_VALIDATOR_NODATAPROOF] = name; + } + if (!exists) { + val->attributes |= VALATTR_FOUNDNOWILDCARD; + } + if (!exists && NEEDNOQNAME(val)) { + proofs[DNS_VALIDATOR_NOWILDCARDPROOF] = name; + } + if (dns_rdataset_isassociated(&trdataset)) { dns_rdataset_disassociate(&trdataset); + } return (ISC_R_SUCCESS); } } - if (result == ISC_R_NOMORE) + if (result == ISC_R_NOMORE) { result = ISC_R_SUCCESS; - if (dns_rdataset_isassociated(&trdataset)) + } + if (dns_rdataset_isassociated(&trdataset)) { dns_rdataset_disassociate(&trdataset); + } return (result); } @@ -2209,7 +2352,9 @@ findnsec3proofs(dns_validator_t *val) { { if (rdataset->type != dns_rdatatype_nsec3 || rdataset->trust != dns_trust_secure) + { continue; + } result = dns_nsec3_noexistnodata(val->event->type, val->event->name, name, @@ -2218,17 +2363,20 @@ findnsec3proofs(dns_validator_t *val) { NULL, NULL, validator_log, val); if (result != ISC_R_IGNORE && result != ISC_R_SUCCESS) { - if (dns_rdataset_isassociated(&trdataset)) + if (dns_rdataset_isassociated(&trdataset)) { dns_rdataset_disassociate(&trdataset); + } return (result); } } - if (result != ISC_R_NOMORE) + if (result != ISC_R_NOMORE) { result = ISC_R_SUCCESS; + } POST(result); - if (dns_name_countlabels(zonename) == 0) + if (dns_name_countlabels(zonename) == 0) { return (ISC_R_SUCCESS); + } /* * If the val->closest is set then we want to use it otherwise @@ -2238,9 +2386,10 @@ findnsec3proofs(dns_validator_t *val) { char namebuf[DNS_NAME_FORMATSIZE]; dns_name_format(dns_fixedname_name(&val->closest), - namebuf, sizeof(namebuf)); - validator_log(val, ISC_LOG_DEBUG(3), "closest encloser from " - "wildcard signature '%s'", namebuf); + namebuf, sizeof(namebuf)); + validator_log(val, ISC_LOG_DEBUG(3), + "closest encloser from wildcard signature '%s'", + namebuf); dns_name_copynf(dns_fixedname_name(&val->closest), closest); closestp = NULL; setclosestp = NULL; @@ -2255,7 +2404,9 @@ findnsec3proofs(dns_validator_t *val) { { if (rdataset->type != dns_rdatatype_nsec3 || rdataset->trust != dns_trust_secure) + { continue; + } /* * We process all NSEC3 records to find the closest @@ -2271,12 +2422,15 @@ findnsec3proofs(dns_validator_t *val) { &unknown, setclosestp, &setnearest, closestp, nearest, validator_log, val); - if (unknown) + if (unknown) { val->attributes |= VALATTR_FOUNDUNKNOWN; - if (result != ISC_R_SUCCESS) + } + if (result != ISC_R_SUCCESS) { continue; - if (setclosest) + } + if (setclosest) { proofs[DNS_VALIDATOR_CLOSESTENCLOSER] = name; + } if (exists && !data && NEEDNODATA(val)) { val->attributes |= VALATTR_FOUNDNODATA; proofs[DNS_VALIDATOR_NODATAPROOF] = name; @@ -2284,12 +2438,14 @@ findnsec3proofs(dns_validator_t *val) { if (!exists && setnearest) { val->attributes |= VALATTR_FOUNDNOQNAME; proofs[DNS_VALIDATOR_NOQNAMEPROOF] = name; - if (optout) + if (optout) { val->attributes |= VALATTR_FOUNDOPTOUT; + } } } - if (result == ISC_R_NOMORE) + if (result == ISC_R_NOMORE) { result = ISC_R_SUCCESS; + } /* * To know we have a valid noqname and optout proofs we need to also @@ -2316,10 +2472,12 @@ findnsec3proofs(dns_validator_t *val) { * Do we need to check for the wildcard? */ if (FOUNDNOQNAME(val) && FOUNDCLOSEST(val) && - ((NEEDNODATA(val) && !FOUNDNODATA(val)) || NEEDNOWILDCARD(val))) { + ((NEEDNODATA(val) && !FOUNDNODATA(val)) || NEEDNOWILDCARD(val))) + { result = checkwildcard(val, dns_rdatatype_nsec3, zonename); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { return (result); + } } return (result); } @@ -2333,10 +2491,11 @@ validate_authority(dns_validator_t *val, bool resume) { dns_message_t *message = val->event->message; isc_result_t result; - if (!resume) + if (!resume) { result = dns_message_firstname(message, DNS_SECTION_AUTHORITY); - else + } else { result = ISC_R_SUCCESS; + } for (; result == ISC_R_SUCCESS; @@ -2350,24 +2509,27 @@ validate_authority(dns_validator_t *val, bool resume) { rdataset = ISC_LIST_NEXT(val->currentset, link); val->currentset = NULL; resume = false; - } else + } else { rdataset = ISC_LIST_HEAD(name->list); + } for (; rdataset != NULL; rdataset = ISC_LIST_NEXT(rdataset, link)) { - if (rdataset->type == dns_rdatatype_rrsig) + if (rdataset->type == dns_rdatatype_rrsig) { continue; + } for (sigrdataset = ISC_LIST_HEAD(name->list); sigrdataset != NULL; - sigrdataset = ISC_LIST_NEXT(sigrdataset, - link)) + sigrdataset = ISC_LIST_NEXT(sigrdataset, link)) { if (sigrdataset->type == dns_rdatatype_rrsig && sigrdataset->covers == rdataset->type) + { break; + } } /* * If a signed zone is missing the zone key, bad @@ -2386,26 +2548,31 @@ validate_authority(dns_validator_t *val, bool resume) { dns_rdata_t nsec = DNS_RDATA_INIT; result = dns_rdataset_first(rdataset); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { return (result); + } dns_rdataset_current(rdataset, &nsec); if (dns_nsec_typepresent(&nsec, - dns_rdatatype_soa)) + dns_rdatatype_soa)) + { continue; + } } val->currentset = rdataset; result = create_validator(val, name, rdataset->type, rdataset, sigrdataset, authvalidated, "validate_authority"); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { return (result); + } val->authcount++; return (DNS_R_WAIT); } } - if (result == ISC_R_NOMORE) + if (result == ISC_R_NOMORE) { result = ISC_R_SUCCESS; + } return (result); } @@ -2417,10 +2584,11 @@ validate_ncache(dns_validator_t *val, bool resume) { dns_name_t *name; isc_result_t result; - if (!resume) + if (!resume) { result = dns_rdataset_first(val->event->rdataset); - else + } else { result = dns_rdataset_next(val->event->rdataset); + } for (; result == ISC_R_SUCCESS; @@ -2428,23 +2596,27 @@ validate_ncache(dns_validator_t *val, bool resume) { { dns_rdataset_t *rdataset, *sigrdataset = NULL; - if (dns_rdataset_isassociated(&val->frdataset)) + if (dns_rdataset_isassociated(&val->frdataset)) { dns_rdataset_disassociate(&val->frdataset); - if (dns_rdataset_isassociated(&val->fsigrdataset)) + } + if (dns_rdataset_isassociated(&val->fsigrdataset)) { dns_rdataset_disassociate(&val->fsigrdataset); + } name = dns_fixedname_initname(&val->fname); rdataset = &val->frdataset; dns_ncache_current(val->event->rdataset, name, rdataset); - if (val->frdataset.type == dns_rdatatype_rrsig) + if (val->frdataset.type == dns_rdatatype_rrsig) { continue; + } result = dns_ncache_getsigrdataset(val->event->rdataset, name, rdataset->type, &val->fsigrdataset); - if (result == ISC_R_SUCCESS) + if (result == ISC_R_SUCCESS) { sigrdataset = &val->fsigrdataset; + } /* * If a signed zone is missing the zone key, bad @@ -2463,25 +2635,27 @@ validate_ncache(dns_validator_t *val, bool resume) { dns_rdata_t nsec = DNS_RDATA_INIT; result = dns_rdataset_first(rdataset); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { return (result); + } dns_rdataset_current(rdataset, &nsec); - if (dns_nsec_typepresent(&nsec, - dns_rdatatype_soa)) + if (dns_nsec_typepresent(&nsec, dns_rdatatype_soa)) { continue; + } } val->currentset = rdataset; result = create_validator(val, name, rdataset->type, rdataset, sigrdataset, - authvalidated, - "validate_ncache"); - if (result != ISC_R_SUCCESS) + authvalidated, "validate_ncache"); + if (result != ISC_R_SUCCESS) { return (result); + } val->authcount++; return (DNS_R_WAIT); } - if (result == ISC_R_NOMORE) + if (result == ISC_R_NOMORE) { result = ISC_R_SUCCESS; + } return (result); } @@ -2501,24 +2675,28 @@ static isc_result_t nsecvalidate(dns_validator_t *val, bool resume) { isc_result_t result; - if (resume) + if (resume) { validator_log(val, ISC_LOG_DEBUG(3), "resuming nsecvalidate"); + } - if (val->event->message == NULL) + if (val->event->message == NULL) { result = validate_ncache(val, resume); - else + } else { result = validate_authority(val, resume); + } - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { return (result); + } /* * Do we only need to check for NOQNAME? To get here we must have * had a secure wildcard answer. */ if (!NEEDNODATA(val) && !NEEDNOWILDCARD(val) && NEEDNOQNAME(val)) { - if (!FOUNDNOQNAME(val)) + if (!FOUNDNOQNAME(val)) { findnsec3proofs(val); + } if (FOUNDNOQNAME(val) && FOUNDCLOSEST(val) && !FOUNDOPTOUT(val)) { validator_log(val, ISC_LOG_DEBUG(3), @@ -2527,7 +2705,8 @@ nsecvalidate(dns_validator_t *val, bool resume) { return (ISC_R_SUCCESS); } else if (FOUNDOPTOUT(val) && dns_name_countlabels(dns_fixedname_name(&val->wild)) - != 0) { + != 0) + { validator_log(val, ISC_LOG_DEBUG(3), "optout proof found"); val->event->optout = true; @@ -2544,36 +2723,43 @@ nsecvalidate(dns_validator_t *val, bool resume) { return (DNS_R_NOVALIDNSEC); } - if (!FOUNDNOQNAME(val) && !FOUNDNODATA(val)) + if (!FOUNDNOQNAME(val) && !FOUNDNODATA(val)) { findnsec3proofs(val); + } /* * Do we need to check for the wildcard? */ if (FOUNDNOQNAME(val) && FOUNDCLOSEST(val) && - ((NEEDNODATA(val) && !FOUNDNODATA(val)) || NEEDNOWILDCARD(val))) { + ((NEEDNODATA(val) && !FOUNDNODATA(val)) || NEEDNOWILDCARD(val))) + { result = checkwildcard(val, dns_rdatatype_nsec, NULL); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { return (result); + } } if ((NEEDNODATA(val) && (FOUNDNODATA(val) || FOUNDOPTOUT(val))) || (NEEDNOQNAME(val) && FOUNDNOQNAME(val) && NEEDNOWILDCARD(val) && FOUNDNOWILDCARD(val) && - FOUNDCLOSEST(val))) { - if ((val->attributes & VALATTR_FOUNDOPTOUT) != 0) + FOUNDCLOSEST(val))) + { + if ((val->attributes & VALATTR_FOUNDOPTOUT) != 0) { val->event->optout = true; + } validator_log(val, ISC_LOG_DEBUG(3), "nonexistence proof(s) found"); - if (val->event->message == NULL) + if (val->event->message == NULL) { marksecure(val->event); - else + } else { val->event->secure = true; + } return (ISC_R_SUCCESS); } - if (val->authfail != 0 && val->authcount == val->authfail) + if (val->authfail != 0 && val->authcount == val->authfail) { return (DNS_R_BROKENCHAIN); + } validator_log(val, ISC_LOG_DEBUG(3), "nonexistence proof(s) not found"); val->attributes |= VALATTR_INSECURITY; @@ -2588,7 +2774,8 @@ check_ds(dns_validator_t *val, dns_name_t *name, dns_rdataset_t *rdataset) { for (result = dns_rdataset_first(rdataset); result == ISC_R_SUCCESS; - result = dns_rdataset_next(rdataset)) { + result = dns_rdataset_next(rdataset)) + { dns_rdataset_current(rdataset, &dsrdata); result = dns_rdata_tostruct(&dsrdata, &ds, NULL); RUNTIME_CHECK(result == ISC_R_SUCCESS); @@ -2596,7 +2783,8 @@ check_ds(dns_validator_t *val, dns_name_t *name, dns_rdataset_t *rdataset) { if (dns_resolver_ds_digest_supported(val->view->resolver, name, ds.digest_type) && dns_resolver_algorithm_supported(val->view->resolver, - name, ds.algorithm)) { + name, ds.algorithm)) + { dns_rdata_reset(&dsrdata); return (true); } @@ -2624,28 +2812,26 @@ check_ds(dns_validator_t *val, dns_name_t *name, dns_rdataset_t *rdataset) { static isc_result_t proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { isc_result_t result; - dns_fixedname_t fixedsecroot; - dns_name_t *secroot; - dns_name_t *tname; char namebuf[DNS_NAME_FORMATSIZE]; - dns_name_t *found; + dns_fixedname_t fixedsecroot; + dns_name_t *secroot = dns_fixedname_initname(&fixedsecroot); dns_fixedname_t fixedfound; + dns_name_t *found = dns_fixedname_initname(&fixedfound); + dns_name_t *tname = NULL; unsigned int labels; - secroot = dns_fixedname_initname(&fixedsecroot); - found = dns_fixedname_initname(&fixedfound); dns_name_copynf(val->event->name, secroot); + /* * If this is a response to a DS query, we need to look in * the parent zone for the trust anchor. */ - labels = dns_name_countlabels(secroot); - if (val->event->type == dns_rdatatype_ds && labels > 1U) - dns_name_getlabelsequence(secroot, 1, labels - 1, - secroot); - result = dns_keytable_finddeepestmatch(val->keytable, - secroot, secroot); + if (val->event->type == dns_rdatatype_ds && labels > 1U) { + dns_name_getlabelsequence(secroot, 1, labels - 1, secroot); + } + + result = dns_keytable_finddeepestmatch(val->keytable, secroot, secroot); if (result == ISC_R_NOTFOUND) { if (val->mustbesecure) { validator_log(val, ISC_LOG_WARNING, @@ -2702,13 +2888,13 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { val->labels <= dns_name_countlabels(val->event->name); val->labels++) { - tname = dns_fixedname_initname(&val->fname); - if (val->labels == dns_name_countlabels(val->event->name)) + if (val->labels == dns_name_countlabels(val->event->name)) { dns_name_copynf(val->event->name, tname); - else + } else { dns_name_split(val->event->name, val->labels, NULL, tname); + } dns_name_format(tname, namebuf, sizeof(namebuf)); validator_log(val, ISC_LOG_DEBUG(3), @@ -2720,20 +2906,21 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { /* * There is no DS. If this is a delegation, * we may be done. - */ - /* + * * If we have "trust == answer" then this namespace * has switched from insecure to should be secure. */ if (DNS_TRUST_PENDING(val->frdataset.trust) || - DNS_TRUST_ANSWER(val->frdataset.trust)) { + DNS_TRUST_ANSWER(val->frdataset.trust)) + { result = create_validator(val, tname, dns_rdatatype_ds, &val->frdataset, NULL, dsvalidated, "proveunsecure"); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { goto out; + } return (DNS_R_WAIT); } /* @@ -2743,10 +2930,11 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { */ if (result == DNS_R_NXRRSET && !dns_rdataset_isassociated(&val->frdataset) && - dns_view_findzonecut(val->view, tname, found, NULL, - 0, 0, false, false, - NULL, NULL) == ISC_R_SUCCESS && - dns_name_equal(tname, found)) { + dns_view_findzonecut(val->view, tname, found, NULL, + 0, 0, false, false, + NULL, NULL) == ISC_R_SUCCESS && + dns_name_equal(tname, found)) + { if (val->mustbesecure) { validator_log(val, ISC_LOG_WARNING, "must be secure failure, " @@ -2783,15 +2971,17 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { continue; } else if (result == DNS_R_CNAME) { if (DNS_TRUST_PENDING(val->frdataset.trust) || - DNS_TRUST_ANSWER(val->frdataset.trust)) { + DNS_TRUST_ANSWER(val->frdataset.trust)) + { result = create_validator(val, tname, dns_rdatatype_cname, &val->frdataset, NULL, cnamevalidated, "proveunsecure " "(cname)"); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { goto out; + } return (DNS_R_WAIT); } continue; @@ -2803,11 +2993,12 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { if (val->frdataset.trust >= dns_trust_secure) { if (!check_ds(val, tname, &val->frdataset)) { validator_log(val, ISC_LOG_DEBUG(3), - "no supported algorithm/" - "digest (%s/DS)", namebuf); + "no supported algorithm/" + "digest (%s/DS)", + namebuf); if (val->mustbesecure) { validator_log(val, - ISC_LOG_WARNING, + ISC_LOG_WARNING, "must be secure failure, " "no supported algorithm/" "digest (%s/DS)", @@ -2820,8 +3011,8 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { goto out; } continue; - } - else if (!dns_rdataset_isassociated(&val->fsigrdataset)) + } else if (!dns_rdataset_isassociated(&val-> + fsigrdataset)) { validator_log(val, ISC_LOG_DEBUG(3), "DS is unsigned"); @@ -2836,13 +3027,15 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { &val->fsigrdataset, dsvalidated, "proveunsecure"); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { 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 + * This is not a zone cut. Assuming things are * as expected, continue. */ if (!dns_rdataset_isassociated(&val->frdataset)) { @@ -2853,7 +3046,8 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { result = DNS_R_NOVALIDNSEC; goto out; } else if (DNS_TRUST_PENDING(val->frdataset.trust) || - DNS_TRUST_ANSWER(val->frdataset.trust)) { + DNS_TRUST_ANSWER(val->frdataset.trust)) + { /* * If we have "trust == answer" then this * namespace has switched from insecure to @@ -2864,8 +3058,9 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { &val->frdataset, NULL, dsvalidated, "proveunsecure"); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { goto out; + } return (DNS_R_WAIT); } else if (val->frdataset.trust < dns_trust_secure) { /* @@ -2888,11 +3083,13 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { */ result = create_fetch(val, tname, dns_rdatatype_ds, dsfetched2, "proveunsecure"); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { goto out; + } return (DNS_R_WAIT); - } else if (result == DNS_R_BROKENCHAIN) + } else if (result == DNS_R_BROKENCHAIN) { return (result); + } } /* Couldn't complete insecurity proof */ @@ -2900,10 +3097,12 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { return (DNS_R_NOTINSECURE); out: - if (dns_rdataset_isassociated(&val->frdataset)) + if (dns_rdataset_isassociated(&val->frdataset)) { dns_rdataset_disassociate(&val->frdataset); - if (dns_rdataset_isassociated(&val->fsigrdataset)) + } + if (dns_rdataset_isassociated(&val->fsigrdataset)) { dns_rdataset_disassociate(&val->fsigrdataset); + } return (result); } @@ -2932,8 +3131,9 @@ validator_start(isc_task_t *task, isc_event_t *event) { val = vevent->validator; /* If the validator has been canceled, val->event == NULL */ - if (val->event == NULL) + if (val->event == NULL) { return; + } validator_log(val, ISC_LOG_DEBUG(3), "starting"); @@ -2960,11 +3160,13 @@ validator_start(isc_task_t *task, isc_event_t *event) { "falling back to insecurity proof"); val->attributes |= VALATTR_INSECURITY; result = proveunsecure(val, false, false); - if (result == DNS_R_NOTINSECURE) + if (result == DNS_R_NOTINSECURE) { result = saved_result; + } } } else if (val->event->rdataset != NULL && - val->event->rdataset->type != 0) { + val->event->rdataset->type != 0) + { /* * This is either an unsecure subdomain or a response from * a broken server. @@ -2975,10 +3177,11 @@ validator_start(isc_task_t *task, isc_event_t *event) { val->attributes |= VALATTR_INSECURITY; result = proveunsecure(val, false, false); - if (result == DNS_R_NOTINSECURE) + if (result == DNS_R_NOTINSECURE) { validator_log(val, ISC_LOG_INFO, "got insecure response; " "parent indicates it should be secure"); + } } else if (val->event->rdataset == NULL && val->event->sigrdataset == NULL) { @@ -2991,11 +3194,12 @@ validator_start(isc_task_t *task, isc_event_t *event) { if (val->event->message->rcode == dns_rcode_nxdomain) { val->attributes |= VALATTR_NEEDNOQNAME; val->attributes |= VALATTR_NEEDNOWILDCARD; - } else + } else { val->attributes |= VALATTR_NEEDNODATA; + } result = nsecvalidate(val, false); } else if (val->event->rdataset != NULL && - NEGATIVE(val->event->rdataset)) + NEGATIVE(val->event->rdataset)) { /* * This is a nonexistence validation. @@ -3006,8 +3210,9 @@ validator_start(isc_task_t *task, isc_event_t *event) { if (val->event->rdataset->covers == dns_rdatatype_any) { val->attributes |= VALATTR_NEEDNOQNAME; val->attributes |= VALATTR_NEEDNOWILDCARD; - } else + } else { val->attributes |= VALATTR_NEEDNODATA; + } result = nsecvalidate(val, false); } else { INSIST(0); @@ -3020,8 +3225,9 @@ validator_start(isc_task_t *task, isc_event_t *event) { } UNLOCK(&val->lock); - if (want_destroy) + if (want_destroy) { destroy(val); + } } isc_result_t @@ -3073,8 +3279,9 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type, val->keytable = NULL; result = dns_view_getsecroots(val->view, &val->keytable); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { goto cleanup_mutex; + } val->keynode = NULL; val->key = NULL; val->siginfo = NULL; @@ -3099,8 +3306,9 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type, ISC_LINK_INIT(val, link); val->magic = VALIDATOR_MAGIC; - if ((options & DNS_VALIDATOR_DEFER) == 0) + if ((options & DNS_VALIDATOR_DEFER) == 0) { isc_task_send(task, ISC_EVENT_PTR(&event)); + } *validatorp = val; @@ -3149,8 +3357,9 @@ dns_validator_cancel(dns_validator_t *validator) { fetch = validator->fetch; validator->fetch = NULL; - if (validator->subvalidator != NULL) + if (validator->subvalidator != NULL) { dns_validator_cancel(validator->subvalidator); + } if ((validator->options & DNS_VALIDATOR_DEFER) != 0) { validator->options &= ~DNS_VALIDATOR_DEFER; validator_done(validator, ISC_R_CANCELED); @@ -3174,21 +3383,27 @@ destroy(dns_validator_t *val) { REQUIRE(val->event == NULL); REQUIRE(val->fetch == NULL); - if (val->keynode != NULL) + if (val->keynode != NULL) { dns_keytable_detachkeynode(val->keytable, &val->keynode); - else if (val->key != NULL) + } else if (val->key != NULL) { dst_key_free(&val->key); - if (val->keytable != NULL) + } + if (val->keytable != NULL) { dns_keytable_detach(&val->keytable); - if (val->subvalidator != NULL) + } + if (val->subvalidator != NULL) { dns_validator_destroy(&val->subvalidator); - if (dns_rdataset_isassociated(&val->frdataset)) + } + if (dns_rdataset_isassociated(&val->frdataset)) { dns_rdataset_disassociate(&val->frdataset); - if (dns_rdataset_isassociated(&val->fsigrdataset)) + } + if (dns_rdataset_isassociated(&val->fsigrdataset)) { dns_rdataset_disassociate(&val->fsigrdataset); + } mctx = val->view->mctx; - if (val->siginfo != NULL) + if (val->siginfo != NULL) { isc_mem_put(mctx, val->siginfo, sizeof(*val->siginfo)); + } isc_mutex_destroy(&val->lock); dns_view_weakdetach(&val->view); val->magic = 0; @@ -3213,8 +3428,9 @@ dns_validator_destroy(dns_validator_t **validatorp) { UNLOCK(&val->lock); - if (want_destroy) + if (want_destroy) { destroy(val); + } *validatorp = NULL; } @@ -3230,8 +3446,9 @@ validator_logv(dns_validator_t *val, isc_logcategory_t *category, vsnprintf(msgbuf, sizeof(msgbuf), fmt, ap); - if ((unsigned int) depth >= sizeof spaces) + if ((unsigned int) depth >= sizeof spaces) { depth = sizeof spaces - 1; + } /* * Log the view name unless it's: @@ -3274,8 +3491,9 @@ static void validator_log(void *val, int level, const char *fmt, ...) { va_list ap; - if (! isc_log_wouldlog(dns_lctx, level)) + if (!isc_log_wouldlog(dns_lctx, level)) { return; + } va_start(ap, fmt); @@ -3285,9 +3503,9 @@ validator_log(void *val, int level, const char *fmt, ...) { } static void -validator_logcreate(dns_validator_t *val, - dns_name_t *name, dns_rdatatype_t type, - const char *caller, const char *operation) +validator_logcreate(dns_validator_t *val, dns_name_t *name, + dns_rdatatype_t type, const char *caller, + const char *operation) { char namestr[DNS_NAME_FORMATSIZE]; char typestr[DNS_RDATATYPE_FORMATSIZE]; From 34d7776f14dc1bb7f201fe01e8c9df5a973518fc Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 6 Aug 2019 13:13:38 -0700 Subject: [PATCH 02/15] reduce redundant code --- lib/dns/validator.c | 175 ++++++++++++++++++-------------------------- 1 file changed, 71 insertions(+), 104 deletions(-) diff --git a/lib/dns/validator.c b/lib/dns/validator.c index 8e996429b2..10518caab3 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -133,6 +133,32 @@ validator_logcreate(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type, const char *caller, const char *operation); +/*% + * Ensure the validator's rdatasets are marked as expired. + */ +static void +expire_rdatasets(dns_validator_t *val) { + if (dns_rdataset_isassociated(&val->frdataset)) { + dns_rdataset_expire(&val->frdataset); + } + if (dns_rdataset_isassociated(&val->fsigrdataset)) { + dns_rdataset_expire(&val->fsigrdataset); + } +} + +/*% + * Ensure the validator's rdatasets are disassociated. + */ +static void +disassociate_rdatasets(dns_validator_t *val) { + if (dns_rdataset_isassociated(&val->frdataset)) { + dns_rdataset_disassociate(&val->frdataset); + } + if (dns_rdataset_isassociated(&val->fsigrdataset)) { + dns_rdataset_disassociate(&val->fsigrdataset); + } +} + /*% * Mark the RRsets as a answer. */ @@ -657,12 +683,7 @@ keyvalidated(isc_task_t *task, isc_event_t *event) { } } else { if (eresult != DNS_R_BROKENCHAIN) { - if (dns_rdataset_isassociated(&val->frdataset)) { - dns_rdataset_expire(&val->frdataset); - } - if (dns_rdataset_isassociated(&val->fsigrdataset)) { - dns_rdataset_expire(&val->fsigrdataset); - } + expire_rdatasets(val); } validator_log(val, ISC_LOG_DEBUG(3), "keyvalidated: got %s", @@ -718,8 +739,8 @@ dsvalidated(isc_task_t *task, isc_event_t *event) { if ((val->attributes & VALATTR_INSECURITY) != 0 && val->frdataset.covers == dns_rdatatype_ds && NEGATIVE(&val->frdataset) && - isdelegation(name, &val->frdataset, - DNS_R_NCACHENXRRSET)) { + isdelegation(name, &val->frdataset, DNS_R_NCACHENXRRSET)) + { if (val->mustbesecure) { validator_log(val, ISC_LOG_WARNING, "must be secure failure, no DS " @@ -739,12 +760,7 @@ dsvalidated(isc_task_t *task, isc_event_t *event) { } } else { if (eresult != DNS_R_BROKENCHAIN) { - if (dns_rdataset_isassociated(&val->frdataset)) { - dns_rdataset_expire(&val->frdataset); - } - if (dns_rdataset_isassociated(&val->fsigrdataset)) { - dns_rdataset_expire(&val->fsigrdataset); - } + expire_rdatasets(val); } validator_log(val, ISC_LOG_DEBUG(3), "dsvalidated: got %s", @@ -797,12 +813,7 @@ cnamevalidated(isc_task_t *task, isc_event_t *event) { } } else { if (eresult != DNS_R_BROKENCHAIN) { - if (dns_rdataset_isassociated(&val->frdataset)) { - dns_rdataset_expire(&val->frdataset); - } - if (dns_rdataset_isassociated(&val->fsigrdataset)) { - dns_rdataset_expire(&val->fsigrdataset); - } + expire_rdatasets(val); } validator_log(val, ISC_LOG_DEBUG(3), "cnamevalidated: got %s", @@ -956,16 +967,11 @@ view_find(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type) { char namebuf[DNS_NAME_FORMATSIZE]; char typebuf[DNS_RDATATYPE_FORMATSIZE]; - if (dns_rdataset_isassociated(&val->frdataset)) { - dns_rdataset_disassociate(&val->frdataset); - } - if (dns_rdataset_isassociated(&val->fsigrdataset)) { - dns_rdataset_disassociate(&val->fsigrdataset); - } + disassociate_rdatasets(val); if (isc_time_now(&now) == ISC_R_SUCCESS && - dns_resolver_getbadcache(val->view->resolver, name, type, &now)) { - + dns_resolver_getbadcache(val->view->resolver, name, type, &now)) + { dns_name_format(name, namebuf, sizeof(namebuf)); dns_rdatatype_format(type, typebuf, sizeof(typebuf)); validator_log(val, ISC_LOG_INFO, "bad cache hit (%s/%s)", @@ -980,12 +986,7 @@ view_find(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type) { &val->frdataset, &val->fsigrdataset); if (result == DNS_R_NXDOMAIN) { - if (dns_rdataset_isassociated(&val->frdataset)) { - dns_rdataset_disassociate(&val->frdataset); - } - if (dns_rdataset_isassociated(&val->fsigrdataset)) { - dns_rdataset_disassociate(&val->fsigrdataset); - } + goto notfound; } else if (result != ISC_R_SUCCESS && result != DNS_R_NCACHENXDOMAIN && result != DNS_R_NCACHENXRRSET && @@ -993,27 +994,25 @@ view_find(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type) { result != DNS_R_NXRRSET && result != ISC_R_NOTFOUND) { - goto notfound; + result = ISC_R_NOTFOUND; + goto notfound; } + return (result); notfound: - if (dns_rdataset_isassociated(&val->frdataset)) { - dns_rdataset_disassociate(&val->frdataset); - } - if (dns_rdataset_isassociated(&val->fsigrdataset)) { - dns_rdataset_disassociate(&val->fsigrdataset); - } - return (ISC_R_NOTFOUND); + disassociate_rdatasets(val); + + return (result); } /*% - * 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. - */ +* 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 bool check_deadlock(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type, - dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset) + dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset) { dns_validator_t *parent; @@ -1050,12 +1049,7 @@ create_fetch(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type, { unsigned int fopts = 0; - if (dns_rdataset_isassociated(&val->frdataset)) { - dns_rdataset_disassociate(&val->frdataset); - } - if (dns_rdataset_isassociated(&val->fsigrdataset)) { - dns_rdataset_disassociate(&val->fsigrdataset); - } + disassociate_rdatasets(val); if (check_deadlock(val, name, type, NULL, NULL)) { validator_log(val, ISC_LOG_DEBUG(3), @@ -1174,6 +1168,7 @@ get_dst_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo, dns_rdata_reset(&rdata); result = dns_rdataset_next(rdataset); } while (result == ISC_R_SUCCESS); + if (result == ISC_R_NOMORE) { result = ISC_R_NOTFOUND; } @@ -1330,7 +1325,8 @@ get_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo) { } if (dns_rdataset_isassociated(&val->frdataset) && - val->keyset != &val->frdataset) { + val->keyset != &val->frdataset) + { dns_rdataset_disassociate(&val->frdataset); } if (dns_rdataset_isassociated(&val->fsigrdataset)) { @@ -1597,7 +1593,8 @@ validate(dns_validator_t *val, bool resume) { } } else { if (get_dst_key(val, val->siginfo, val->keyset) - != ISC_R_SUCCESS) { + != ISC_R_SUCCESS) + { break; } } @@ -1953,12 +1950,7 @@ validatezonekey(dns_validator_t *val) { /* * The DS does not exist. */ - if (dns_rdataset_isassociated(&val->frdataset)) { - dns_rdataset_disassociate(&val->frdataset); - } - if (dns_rdataset_isassociated(&val->fsigrdataset)) { - dns_rdataset_disassociate(&val->fsigrdataset); - } + disassociate_rdatasets(val); validator_log(val, ISC_LOG_DEBUG(2), "no DS record"); return (DNS_R_NOVALIDSIG); } else if (result == DNS_R_BROKENCHAIN) { @@ -2596,12 +2588,7 @@ validate_ncache(dns_validator_t *val, bool resume) { { dns_rdataset_t *rdataset, *sigrdataset = NULL; - if (dns_rdataset_isassociated(&val->frdataset)) { - dns_rdataset_disassociate(&val->frdataset); - } - if (dns_rdataset_isassociated(&val->fsigrdataset)) { - dns_rdataset_disassociate(&val->fsigrdataset); - } + disassociate_rdatasets(val); name = dns_fixedname_initname(&val->fname); rdataset = &val->frdataset; @@ -2902,7 +2889,9 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { namebuf); result = view_find(val, tname, dns_rdatatype_ds); - if (result == DNS_R_NXRRSET || result == DNS_R_NCACHENXRRSET) { + switch (result) { + case DNS_R_NXRRSET: + case DNS_R_NCACHENXRRSET: /* * There is no DS. If this is a delegation, * we may be done. @@ -2969,7 +2958,7 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { return (ISC_R_SUCCESS); } continue; - } else if (result == DNS_R_CNAME) { + case DNS_R_CNAME: if (DNS_TRUST_PENDING(val->frdataset.trust) || DNS_TRUST_ANSWER(val->frdataset.trust)) { @@ -2985,7 +2974,7 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { return (DNS_R_WAIT); } continue; - } else if (result == ISC_R_SUCCESS) { + case ISC_R_SUCCESS: /* * There is a DS here. Verify that it's secure and * continue. @@ -3031,9 +3020,8 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { goto out; } return (DNS_R_WAIT); - } else if (result == DNS_R_NXDOMAIN || - result == DNS_R_NCACHENXDOMAIN) - { + case DNS_R_NXDOMAIN: + case DNS_R_NCACHENXDOMAIN: /* * This is not a zone cut. Assuming things are * as expected, continue. @@ -3077,7 +3065,7 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { goto out; } continue; - } else if (result == ISC_R_NOTFOUND) { + case ISC_R_NOTFOUND: /* * We don't know anything about the DS. Find it. */ @@ -3087,8 +3075,10 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { goto out; } return (DNS_R_WAIT); - } else if (result == DNS_R_BROKENCHAIN) { + case DNS_R_BROKENCHAIN: return (result); + default: + break; } } @@ -3097,12 +3087,7 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { return (DNS_R_NOTINSECURE); out: - if (dns_rdataset_isassociated(&val->frdataset)) { - dns_rdataset_disassociate(&val->frdataset); - } - if (dns_rdataset_isassociated(&val->fsigrdataset)) { - dns_rdataset_disassociate(&val->fsigrdataset); - } + disassociate_rdatasets(val); return (result); } @@ -3182,8 +3167,11 @@ validator_start(isc_task_t *task, isc_event_t *event) { "got insecure response; " "parent indicates it should be secure"); } - } else if (val->event->rdataset == NULL && - val->event->sigrdataset == NULL) + } else if ((val->event->rdataset == NULL && + val->event->sigrdataset == NULL) || + (val->event->rdataset != NULL && + NEGATIVE(val->event->rdataset))) + { /* * This is a nonexistence validation. @@ -3198,22 +3186,6 @@ validator_start(isc_task_t *task, isc_event_t *event) { val->attributes |= VALATTR_NEEDNODATA; } result = nsecvalidate(val, false); - } else if (val->event->rdataset != NULL && - NEGATIVE(val->event->rdataset)) - { - /* - * This is a nonexistence validation. - */ - validator_log(val, ISC_LOG_DEBUG(3), - "attempting negative response validation"); - - if (val->event->rdataset->covers == dns_rdatatype_any) { - val->attributes |= VALATTR_NEEDNOQNAME; - val->attributes |= VALATTR_NEEDNOWILDCARD; - } else { - val->attributes |= VALATTR_NEEDNODATA; - } - result = nsecvalidate(val, false); } else { INSIST(0); ISC_UNREACHABLE(); @@ -3394,12 +3366,7 @@ destroy(dns_validator_t *val) { if (val->subvalidator != NULL) { dns_validator_destroy(&val->subvalidator); } - if (dns_rdataset_isassociated(&val->frdataset)) { - dns_rdataset_disassociate(&val->frdataset); - } - if (dns_rdataset_isassociated(&val->fsigrdataset)) { - dns_rdataset_disassociate(&val->fsigrdataset); - } + disassociate_rdatasets(val); mctx = val->view->mctx; if (val->siginfo != NULL) { isc_mem_put(mctx, val->siginfo, sizeof(*val->siginfo)); From 61456d886e6de464d62e65327a69742a31676673 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 6 Aug 2019 16:11:22 -0700 Subject: [PATCH 03/15] split proveunsecure() --- lib/dns/validator.c | 510 +++++++++++++++++++++++++------------------- 1 file changed, 286 insertions(+), 224 deletions(-) diff --git a/lib/dns/validator.c b/lib/dns/validator.c index 10518caab3..5168ade44d 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -2781,10 +2781,262 @@ check_ds(dns_validator_t *val, dns_name_t *name, dns_rdataset_t *rdataset) { } /*% - * 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 - * not exist at a delegation point or the DS exists at a delegation - * but we don't support the algorithm/digest. + * seek_ds looks for DS rrsets at the label indicated by val->labels, + * for an insecurity proof. + * + * Returns: + * \li ISC_R_COMPLETE a result has been determined and copied + * into `*resp`; ISC_R_SUCCESS indicates that + * the name has been proven insecure and any + * other result indicates failure. + * \li DNS_R_CONTINUE result is indeterminate; caller should + * continue walking down labels. + */ +static isc_result_t +seek_ds(dns_validator_t *val, isc_result_t *resp) { + isc_result_t result; + char namebuf[DNS_NAME_FORMATSIZE]; + dns_fixedname_t fixedfound; + dns_name_t *found = dns_fixedname_initname(&fixedfound); + dns_name_t *tname = dns_fixedname_initname(&val->fname); + + if (val->labels == dns_name_countlabels(val->event->name)) { + dns_name_copynf(val->event->name, tname); + } else { + dns_name_split(val->event->name, val->labels, NULL, tname); + } + + dns_name_format(tname, namebuf, sizeof(namebuf)); + validator_log(val, ISC_LOG_DEBUG(3), + "checking existence of DS at '%s'", namebuf); + + result = view_find(val, tname, dns_rdatatype_ds); + switch (result) { + case DNS_R_NXRRSET: + case DNS_R_NCACHENXRRSET: + /* + * There is no DS. If this is a delegation, + * we may be done. + * + * If we have "trust == answer" then this namespace + * has switched from insecure to should be secure. + */ + if (DNS_TRUST_PENDING(val->frdataset.trust) || + DNS_TRUST_ANSWER(val->frdataset.trust)) + { + result = create_validator(val, tname, + dns_rdatatype_ds, + &val->frdataset, + NULL, dsvalidated, + "proveunsecure"); + *resp = DNS_R_WAIT; + if (result != ISC_R_SUCCESS) { + *resp = result; + } + return (ISC_R_COMPLETE); + } + + /* + * Zones using NSEC3 don't return a NSEC RRset so + * we need to use dns_view_findzonecut2 to find + * the zone cut. + */ + if (result == DNS_R_NXRRSET && + !dns_rdataset_isassociated(&val->frdataset) && + dns_view_findzonecut(val->view, tname, found, NULL, + 0, 0, false, false, + NULL, NULL) == ISC_R_SUCCESS && + dns_name_equal(tname, found)) + { + if (val->mustbesecure) { + validator_log(val, ISC_LOG_WARNING, + "must be secure failure, " + "no DS at zone cut"); + *resp = DNS_R_MUSTBESECURE; + } else { + markanswer(val, "proveunsecure (3)"); + *resp = ISC_R_SUCCESS; + } + return (ISC_R_COMPLETE); + } + + if (val->frdataset.trust < dns_trust_secure) { + /* + * This shouldn't happen, since the negative + * response should have been validated. Since + * there's no way of validating existing + * negative response blobs, give up. + */ + validator_log(val, ISC_LOG_WARNING, + "can't validate existing " + "negative responses (no DS)"); + *resp = DNS_R_MUSTBESECURE; + return (ISC_R_COMPLETE); + } + + if (isdelegation(tname, &val->frdataset, result)) { + if (val->mustbesecure) { + validator_log(val, ISC_LOG_WARNING, + "must be secure failure, " + "%s is a delegation", + namebuf); + *resp = DNS_R_MUSTBESECURE; + } else { + markanswer(val, "proveunsecure (4)"); + *resp = ISC_R_SUCCESS; + } + return (ISC_R_COMPLETE); + } + + break; + + case DNS_R_CNAME: + if (DNS_TRUST_PENDING(val->frdataset.trust) || + DNS_TRUST_ANSWER(val->frdataset.trust)) + { + result = create_validator(val, tname, + dns_rdatatype_cname, + &val->frdataset, + NULL, cnamevalidated, + "proveunsecure " + "(cname)"); + *resp = DNS_R_WAIT; + if (result != ISC_R_SUCCESS) { + *resp = result; + } + return (ISC_R_COMPLETE); + } + + break; + + case ISC_R_SUCCESS: + /* + * There is a DS here. Verify that it's secure and + * continue walking down labels. + */ + if (val->frdataset.trust >= dns_trust_secure) { + if (!check_ds(val, tname, &val->frdataset)) { + validator_log(val, ISC_LOG_DEBUG(3), + "no supported algorithm/" + "digest (%s/DS)", + namebuf); + if (val->mustbesecure) { + validator_log(val, + ISC_LOG_WARNING, + "must be secure failure, " + "no supported algorithm/" + "digest (%s/DS)", + namebuf); + *resp = DNS_R_MUSTBESECURE; + } else { + markanswer(val, "proveunsecure (5)"); + *resp = ISC_R_SUCCESS; + } + return (ISC_R_COMPLETE); + } + + break; + } + + if (!dns_rdataset_isassociated(&val->fsigrdataset)) { + validator_log(val, ISC_LOG_DEBUG(3), "DS is unsigned"); + *resp = DNS_R_NOVALIDSIG; + } else { + /* + * Validate / re-validate answer. + */ + result = create_validator(val, tname, + dns_rdatatype_ds, + &val->frdataset, + &val->fsigrdataset, + dsvalidated, + "proveunsecure"); + *resp = DNS_R_WAIT; + if (result != ISC_R_SUCCESS) { + *resp = result; + } + } + + return (ISC_R_COMPLETE); + + case DNS_R_NXDOMAIN: + case DNS_R_NCACHENXDOMAIN: + /* + * This is not a zone cut. Assuming things are + * as expected, continue. + */ + if (!dns_rdataset_isassociated(&val->frdataset)) { + /* + * There should be an NSEC here, since we + * are still in a secure zone. + */ + *resp = DNS_R_NOVALIDNSEC; + return (ISC_R_COMPLETE); + } else if (DNS_TRUST_PENDING(val->frdataset.trust) || + DNS_TRUST_ANSWER(val->frdataset.trust)) + { + /* + * If we have "trust == answer" then this + * namespace has switched from insecure to + * should be secure. + */ + *resp = DNS_R_WAIT; + result = create_validator(val, tname, + dns_rdatatype_ds, + &val->frdataset, + NULL, dsvalidated, + "proveunsecure"); + if (result != ISC_R_SUCCESS) { + *resp = result; + } + return (ISC_R_COMPLETE); + } else if (val->frdataset.trust < dns_trust_secure) { + /* + * This shouldn't happen, since the negative + * response should have been validated. Since + * there's no way of validating existing + * negative response blobs, give up. + */ + validator_log(val, ISC_LOG_WARNING, + "can't validate existing " + "negative responses " + "(not a zone cut)"); + *resp = DNS_R_NOVALIDSIG; + return (ISC_R_COMPLETE); + } + + break; + + case ISC_R_NOTFOUND: + /* + * We don't know anything about the DS. Find it. + */ + *resp = DNS_R_WAIT; + result = create_fetch(val, tname, dns_rdatatype_ds, + dsfetched2, "proveunsecure"); + if (result != ISC_R_SUCCESS) { + *resp = result; + } + return (ISC_R_COMPLETE); + + default: + *resp = result; + return (ISC_R_COMPLETE); + } + + /* + * No definite answer yet; continue walking down labels. + */ + return (DNS_R_CONTINUE); +} + +/*% + * proveunsecure walks down, label by label, from the closest enclosing + * trust anchor to the name that is being validated, looking for an + * endpoint in the chain of trust. That occurs when we can prove that + * a DS record does not exist at a delegation point, or that a DS exists + * at a delegation point but we don't support its algorithm/digest. If + * no such endpoint is found, then the response should have been secure. * * Returns: * \li ISC_R_SUCCESS val->event->name is in a unsecure zone @@ -2802,9 +3054,6 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { char namebuf[DNS_NAME_FORMATSIZE]; dns_fixedname_t fixedsecroot; dns_name_t *secroot = dns_fixedname_initname(&fixedsecroot); - dns_fixedname_t fixedfound; - dns_name_t *found = dns_fixedname_initname(&fixedfound); - dns_name_t *tname = NULL; unsigned int labels; dns_name_copynf(val->event->name, secroot); @@ -2837,16 +3086,18 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { if (!resume) { /* - * We are looking for breaks below the SEP so add a label. + * We are looking for interruptions in the chain of trust. + * That can only happen *below* the trust anchor, so we + * start looking at the next label down. */ val->labels = dns_name_countlabels(secroot) + 1; } else { validator_log(val, ISC_LOG_DEBUG(3), "resuming proveunsecure"); + /* - * If we have a DS rdataset and it is secure then check if - * the DS rdataset has a supported algorithm combination. - * If not this is an insecure delegation as far as this - * resolver is concerned. + * If we have a DS rdataset and it is secure, check whether + * it has a supported algorithm combination. If not, this is + * an insecure delegation as far as this resolver is concerned. */ if (have_ds && val->frdataset.trust >= dns_trust_secure && !check_ds(val, dns_fixedname_name(&val->fname), @@ -2871,223 +3122,33 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { val->labels++; } - for (; - val->labels <= dns_name_countlabels(val->event->name); - val->labels++) - { - tname = dns_fixedname_initname(&val->fname); - if (val->labels == dns_name_countlabels(val->event->name)) { - dns_name_copynf(val->event->name, tname); - } else { - dns_name_split(val->event->name, val->labels, - NULL, tname); + /* + * Walk down through each of the remaining labels in the name, + * looking for DS records. + */ + while (val->labels <= dns_name_countlabels(val->event->name)) { + isc_result_t tresult; + + result = seek_ds(val, &tresult); + if (result == ISC_R_COMPLETE) { + result = tresult; + goto out; } - dns_name_format(tname, namebuf, sizeof(namebuf)); - validator_log(val, ISC_LOG_DEBUG(3), - "checking existence of DS at '%s'", - namebuf); - - result = view_find(val, tname, dns_rdatatype_ds); - switch (result) { - case DNS_R_NXRRSET: - case DNS_R_NCACHENXRRSET: - /* - * There is no DS. If this is a delegation, - * we may be done. - * - * If we have "trust == answer" then this namespace - * has switched from insecure to should be secure. - */ - if (DNS_TRUST_PENDING(val->frdataset.trust) || - DNS_TRUST_ANSWER(val->frdataset.trust)) - { - result = create_validator(val, tname, - dns_rdatatype_ds, - &val->frdataset, - NULL, dsvalidated, - "proveunsecure"); - if (result != ISC_R_SUCCESS) { - goto out; - } - return (DNS_R_WAIT); - } - /* - * Zones using NSEC3 don't return a NSEC RRset so - * we need to use dns_view_findzonecut2 to find - * the zone cut. - */ - if (result == DNS_R_NXRRSET && - !dns_rdataset_isassociated(&val->frdataset) && - dns_view_findzonecut(val->view, tname, found, NULL, - 0, 0, false, false, - NULL, NULL) == ISC_R_SUCCESS && - dns_name_equal(tname, found)) - { - if (val->mustbesecure) { - validator_log(val, ISC_LOG_WARNING, - "must be secure failure, " - "no DS at zone cut"); - return (DNS_R_MUSTBESECURE); - } - markanswer(val, "proveunsecure (3)"); - return (ISC_R_SUCCESS); - } - if (val->frdataset.trust < dns_trust_secure) { - /* - * This shouldn't happen, since the negative - * response should have been validated. Since - * there's no way of validating existing - * negative response blobs, give up. - */ - validator_log(val, ISC_LOG_WARNING, - "can't validate existing " - "negative responses (no DS)"); - result = DNS_R_NOVALIDSIG; - goto out; - } - if (isdelegation(tname, &val->frdataset, result)) { - if (val->mustbesecure) { - validator_log(val, ISC_LOG_WARNING, - "must be secure failure, " - "%s is a delegation", - namebuf); - return (DNS_R_MUSTBESECURE); - } - markanswer(val, "proveunsecure (4)"); - return (ISC_R_SUCCESS); - } - continue; - case DNS_R_CNAME: - if (DNS_TRUST_PENDING(val->frdataset.trust) || - DNS_TRUST_ANSWER(val->frdataset.trust)) - { - result = create_validator(val, tname, - dns_rdatatype_cname, - &val->frdataset, - NULL, cnamevalidated, - "proveunsecure " - "(cname)"); - if (result != ISC_R_SUCCESS) { - goto out; - } - return (DNS_R_WAIT); - } - continue; - case ISC_R_SUCCESS: - /* - * There is a DS here. Verify that it's secure and - * continue. - */ - if (val->frdataset.trust >= dns_trust_secure) { - if (!check_ds(val, tname, &val->frdataset)) { - validator_log(val, ISC_LOG_DEBUG(3), - "no supported algorithm/" - "digest (%s/DS)", - namebuf); - if (val->mustbesecure) { - validator_log(val, - ISC_LOG_WARNING, - "must be secure failure, " - "no supported algorithm/" - "digest (%s/DS)", - namebuf); - result = DNS_R_MUSTBESECURE; - goto out; - } - markanswer(val, "proveunsecure (5)"); - result = ISC_R_SUCCESS; - goto out; - } - continue; - } else if (!dns_rdataset_isassociated(&val-> - fsigrdataset)) - { - validator_log(val, ISC_LOG_DEBUG(3), - "DS is unsigned"); - result = DNS_R_NOVALIDSIG; - goto out; - } - /* - * Validate / re-validate answer. - */ - result = create_validator(val, tname, dns_rdatatype_ds, - &val->frdataset, - &val->fsigrdataset, - dsvalidated, - "proveunsecure"); - if (result != ISC_R_SUCCESS) { - goto out; - } - return (DNS_R_WAIT); - case DNS_R_NXDOMAIN: - case DNS_R_NCACHENXDOMAIN: - /* - * This is not a zone cut. Assuming things are - * as expected, continue. - */ - if (!dns_rdataset_isassociated(&val->frdataset)) { - /* - * There should be an NSEC here, since we - * are still in a secure zone. - */ - result = DNS_R_NOVALIDNSEC; - goto out; - } else if (DNS_TRUST_PENDING(val->frdataset.trust) || - DNS_TRUST_ANSWER(val->frdataset.trust)) - { - /* - * If we have "trust == answer" then this - * namespace has switched from insecure to - * should be secure. - */ - result = create_validator(val, tname, - dns_rdatatype_ds, - &val->frdataset, - NULL, dsvalidated, - "proveunsecure"); - if (result != ISC_R_SUCCESS) { - goto out; - } - return (DNS_R_WAIT); - } else if (val->frdataset.trust < dns_trust_secure) { - /* - * This shouldn't happen, since the negative - * response should have been validated. Since - * there's no way of validating existing - * negative response blobs, give up. - */ - validator_log(val, ISC_LOG_WARNING, - "can't validate existing " - "negative responses " - "(not a zone cut)"); - result = DNS_R_NOVALIDSIG; - goto out; - } - continue; - case ISC_R_NOTFOUND: - /* - * We don't know anything about the DS. Find it. - */ - result = create_fetch(val, tname, dns_rdatatype_ds, - dsfetched2, "proveunsecure"); - if (result != ISC_R_SUCCESS) { - goto out; - } - return (DNS_R_WAIT); - case DNS_R_BROKENCHAIN: - return (result); - default: - break; - } + INSIST(result == DNS_R_CONTINUE); + val->labels++; } - /* Couldn't complete insecurity proof */ - validator_log(val, ISC_LOG_DEBUG(3), "insecurity proof failed"); + /* Couldn't complete insecurity proof. */ + validator_log(val, ISC_LOG_DEBUG(3), + "insecurity proof failed: %s", + isc_result_totext(result)); return (DNS_R_NOTINSECURE); out: - disassociate_rdatasets(val); + if (result != DNS_R_WAIT) { + disassociate_rdatasets(val); + } return (result); } @@ -3228,6 +3289,7 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type, DNS_EVENT_VALIDATORSTART, validator_start, NULL, sizeof(dns_validatorevent_t)); + isc_task_attach(task, &tclone); event->validator = val; event->result = ISC_R_FAILURE; @@ -3252,7 +3314,7 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type, val->keytable = NULL; result = dns_view_getsecroots(val->view, &val->keytable); if (result != ISC_R_SUCCESS) { - goto cleanup_mutex; + goto cleanup; } val->keynode = NULL; val->key = NULL; @@ -3286,7 +3348,7 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type, return (ISC_R_SUCCESS); - cleanup_mutex: + cleanup: isc_mutex_destroy(&val->lock); isc_task_detach(&tclone); From 6dc5343d6da96d6fea5930ce21f59eb55cce687c Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 6 Aug 2019 16:36:28 -0700 Subject: [PATCH 04/15] move the 'mustbesecure' checks into markanswer() --- lib/dns/validator.c | 160 ++++++++++++++++---------------------------- 1 file changed, 57 insertions(+), 103 deletions(-) diff --git a/lib/dns/validator.c b/lib/dns/validator.c index 5168ade44d..fa42f691b5 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -160,10 +160,20 @@ disassociate_rdatasets(dns_validator_t *val) { } /*% - * Mark the RRsets as a answer. + * Mark the rdatasets in val->event with trust level "answer", + * indicating that they did not validate, but could be cached as insecure. + * + * If we are validating a name that is marked as "must be secure", log a + * warning and return DNS_R_MUSTBESECURE instead. */ -static inline void -markanswer(dns_validator_t *val, const char *where) { +static inline isc_result_t +markanswer(dns_validator_t *val, const char *where, const char *mbstext) { + if (val->mustbesecure && mbstext != NULL) { + validator_log(val, ISC_LOG_WARNING, + "must be secure failure, %s", mbstext); + return (DNS_R_MUSTBESECURE); + } + validator_log(val, ISC_LOG_DEBUG(3), "marking as answer (%s)", where); if (val->event->rdataset != NULL) { dns_rdataset_settrust(val->event->rdataset, dns_trust_answer); @@ -172,8 +182,13 @@ markanswer(dns_validator_t *val, const char *where) { dns_rdataset_settrust(val->event->sigrdataset, dns_trust_answer); } + + return (ISC_R_SUCCESS); } +/*% + * Mark the RRsets with trust level secure. + */ static inline void marksecure(dns_validatorevent_t *event) { dns_rdataset_settrust(event->rdataset, dns_trust_secure); @@ -427,11 +442,14 @@ fetch_callback_validator(isc_task_t *task, isc_event_t *event) { validator_done(val, DNS_R_BROKENCHAIN); } } + want_destroy = exit_check(val); UNLOCK(&val->lock); + if (fetch != NULL) { dns_resolver_destroyfetch(&fetch); } + if (want_destroy) { destroy(val); } @@ -511,11 +529,14 @@ dsfetched(isc_task_t *task, isc_event_t *event) { validator_done(val, DNS_R_BROKENCHAIN); } } + want_destroy = exit_check(val); UNLOCK(&val->lock); + if (fetch != NULL) { dns_resolver_destroyfetch(&fetch); } + if (want_destroy) { destroy(val); } @@ -577,16 +598,11 @@ dsfetched2(isc_task_t *task, isc_event_t *event) { */ tname = dns_fixedname_name(&devent->foundname); if (eresult != DNS_R_CNAME && - isdelegation(tname, &val->frdataset, eresult)) { - if (val->mustbesecure) { - validator_log(val, ISC_LOG_WARNING, - "must be secure failure, no DS" - " and this is a delegation"); - validator_done(val, DNS_R_MUSTBESECURE); - } else { - markanswer(val, "dsfetched2"); - validator_done(val, ISC_R_SUCCESS); - } + isdelegation(tname, &val->frdataset, eresult)) + { + result = markanswer(val, "dsfetched2", + "no DS and this is a delegation"); + validator_done(val, result); } else { result = proveunsecure(val, false, true); if (result != DNS_R_WAIT) { @@ -614,12 +630,15 @@ dsfetched2(isc_task_t *task, isc_event_t *event) { validator_done(val, DNS_R_NOVALIDDS); } } + isc_event_free(&event); want_destroy = exit_check(val); UNLOCK(&val->lock); + if (fetch != NULL) { dns_resolver_destroyfetch(&fetch); } + if (want_destroy) { destroy(val); } @@ -690,6 +709,7 @@ keyvalidated(isc_task_t *task, isc_event_t *event) { isc_result_totext(eresult)); validator_done(val, DNS_R_BROKENCHAIN); } + want_destroy = exit_check(val); UNLOCK(&val->lock); if (want_destroy) { @@ -741,15 +761,8 @@ dsvalidated(isc_task_t *task, isc_event_t *event) { NEGATIVE(&val->frdataset) && isdelegation(name, &val->frdataset, DNS_R_NCACHENXRRSET)) { - if (val->mustbesecure) { - validator_log(val, ISC_LOG_WARNING, - "must be secure failure, no DS " - "and this is a delegation"); - result = DNS_R_MUSTBESECURE; - } else { - markanswer(val, "dsvalidated"); - result = ISC_R_SUCCESS; - } + result = markanswer(val, "dsvalidated", + "no DS and this is a delegation"); } else if ((val->attributes & VALATTR_INSECURITY) != 0) { result = proveunsecure(val, have_dsset, true); } else { @@ -767,6 +780,7 @@ dsvalidated(isc_task_t *task, isc_event_t *event) { isc_result_totext(eresult)); validator_done(val, DNS_R_BROKENCHAIN); } + want_destroy = exit_check(val); UNLOCK(&val->lock); if (want_destroy) { @@ -820,6 +834,7 @@ cnamevalidated(isc_task_t *task, isc_event_t *event) { isc_result_totext(eresult)); validator_done(val, DNS_R_BROKENCHAIN); } + want_destroy = exit_check(val); UNLOCK(&val->lock); if (want_destroy) { @@ -933,6 +948,7 @@ authvalidated(isc_task_t *task, isc_event_t *event) { validator_done(val, result); } } + want_destroy = exit_check(val); UNLOCK(&val->lock); if (want_destroy) { @@ -1811,17 +1827,10 @@ validatezonekey(dns_validator_t *val) { found) != ISC_R_SUCCESS) { - if (val->mustbesecure) { - validator_log(val, ISC_LOG_WARNING, - "must be secure " - "failure, not beneath " - "secure root"); - return (DNS_R_MUSTBESECURE); - } validator_log(val, ISC_LOG_DEBUG(3), - "not beneath secure root"); - markanswer(val, "validatezonekey (1)"); - return (ISC_R_SUCCESS); + "not beneath secure root"); + return (markanswer(val, "validatezonekey (1)", + "not beneath secure root")); } if (result == DNS_R_PARTIALMATCH || result == ISC_R_SUCCESS) @@ -1964,14 +1973,7 @@ validatezonekey(dns_validator_t *val) { INSIST(val->dsset != NULL); if (val->dsset->trust < dns_trust_secure) { - if (val->mustbesecure) { - validator_log(val, ISC_LOG_WARNING, - "must be secure failure," - " insecure DS"); - return (DNS_R_MUSTBESECURE); - } - markanswer(val, "validatezonekey (2)"); - return (ISC_R_SUCCESS); + return (markanswer(val, "validatezonekey (2)", "insecure DS")); } /* @@ -2083,16 +2085,10 @@ validatezonekey(dns_validator_t *val) { validator_log(val, ISC_LOG_DEBUG(3), "marking as secure (DS)"); return (result); } else if (result == ISC_R_NOMORE && !supported_algorithm) { - if (val->mustbesecure) { - validator_log(val, ISC_LOG_WARNING, - "must be secure failure, " - "no supported algorithm/digest (DS)"); - return (DNS_R_MUSTBESECURE); - } validator_log(val, ISC_LOG_DEBUG(3), "no supported algorithm/digest (DS)"); - markanswer(val, "validatezonekey (3)"); - return (ISC_R_SUCCESS); + return (markanswer(val, "validatezonekey (3)", + "no supported algorithm/digest (DS)")); } else { validator_log(val, ISC_LOG_INFO, "no valid signature found (DS)"); @@ -2691,18 +2687,17 @@ nsecvalidate(dns_validator_t *val, bool resume) { marksecure(val->event); return (ISC_R_SUCCESS); } else if (FOUNDOPTOUT(val) && - dns_name_countlabels(dns_fixedname_name(&val->wild)) - != 0) + dns_name_countlabels(dns_fixedname_name(&val->wild)) != 0) { validator_log(val, ISC_LOG_DEBUG(3), "optout proof found"); val->event->optout = true; - markanswer(val, "nsecvalidate (1)"); + markanswer(val, "nsecvalidate (1)", NULL); return (ISC_R_SUCCESS); } else if ((val->attributes & VALATTR_FOUNDUNKNOWN) != 0) { validator_log(val, ISC_LOG_DEBUG(3), "unknown NSEC3 hash algorithm found"); - markanswer(val, "nsecvalidate (2)"); + markanswer(val, "nsecvalidate (2)", NULL); return (ISC_R_SUCCESS); } validator_log(val, ISC_LOG_DEBUG(3), @@ -2848,15 +2843,8 @@ seek_ds(dns_validator_t *val, isc_result_t *resp) { NULL, NULL) == ISC_R_SUCCESS && dns_name_equal(tname, found)) { - if (val->mustbesecure) { - validator_log(val, ISC_LOG_WARNING, - "must be secure failure, " - "no DS at zone cut"); - *resp = DNS_R_MUSTBESECURE; - } else { - markanswer(val, "proveunsecure (3)"); - *resp = ISC_R_SUCCESS; - } + *resp = markanswer(val, "proveunsecure (3)", + "no DS at zone cut"); return (ISC_R_COMPLETE); } @@ -2875,16 +2863,8 @@ seek_ds(dns_validator_t *val, isc_result_t *resp) { } if (isdelegation(tname, &val->frdataset, result)) { - if (val->mustbesecure) { - validator_log(val, ISC_LOG_WARNING, - "must be secure failure, " - "%s is a delegation", - namebuf); - *resp = DNS_R_MUSTBESECURE; - } else { - markanswer(val, "proveunsecure (4)"); - *resp = ISC_R_SUCCESS; - } + *resp = markanswer(val, "proveunsecure (4)", + "this is a delegation"); return (ISC_R_COMPLETE); } @@ -2920,18 +2900,9 @@ seek_ds(dns_validator_t *val, isc_result_t *resp) { "no supported algorithm/" "digest (%s/DS)", namebuf); - if (val->mustbesecure) { - validator_log(val, - ISC_LOG_WARNING, - "must be secure failure, " - "no supported algorithm/" - "digest (%s/DS)", - namebuf); - *resp = DNS_R_MUSTBESECURE; - } else { - markanswer(val, "proveunsecure (5)"); - *resp = ISC_R_SUCCESS; - } + *resp = markanswer(val, "proveunsecure (5)", + "no supported " + "algorithm/digest (DS)"); return (ISC_R_COMPLETE); } @@ -3069,17 +3040,10 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { result = dns_keytable_finddeepestmatch(val->keytable, secroot, secroot); if (result == ISC_R_NOTFOUND) { - if (val->mustbesecure) { - validator_log(val, ISC_LOG_WARNING, - "must be secure failure, " - "not beneath secure root"); - result = DNS_R_MUSTBESECURE; - goto out; - } validator_log(val, ISC_LOG_DEBUG(3), "not beneath secure root"); - markanswer(val, "proveunsecure (1)"); - return (ISC_R_SUCCESS); + return (markanswer(val, "proveunsecure (1)", + "not beneath secure root")); } else if (result != ISC_R_SUCCESS) { return (result); } @@ -3105,18 +3069,10 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { { dns_name_format(dns_fixedname_name(&val->fname), namebuf, sizeof(namebuf)); - if (val->mustbesecure) { - validator_log(val, ISC_LOG_WARNING, - "must be secure failure at '%s'", - namebuf); - result = DNS_R_MUSTBESECURE; - goto out; - } validator_log(val, ISC_LOG_DEBUG(3), "no supported algorithm/digest (%s/DS)", namebuf); - markanswer(val, "proveunsecure (2)"); - result = ISC_R_SUCCESS; + result = markanswer(val, "proveunsecure (2)", namebuf); goto out; } val->labels++; @@ -3454,9 +3410,7 @@ dns_validator_destroy(dns_validator_t **validatorp) { validator_log(val, ISC_LOG_DEBUG(4), "dns_validator_destroy"); want_destroy = exit_check(val); - UNLOCK(&val->lock); - if (want_destroy) { destroy(val); } From 54710873a7a5800e2ca9d53dfc154c69bad1a656 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 6 Aug 2019 23:08:36 -0700 Subject: [PATCH 05/15] move some duplicate code into validate_neg_rrset() --- lib/dns/validator.c | 139 ++++++++++++++++++++++---------------------- 1 file changed, 69 insertions(+), 70 deletions(-) diff --git a/lib/dns/validator.c b/lib/dns/validator.c index fa42f691b5..439930c95b 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -1216,7 +1216,8 @@ get_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo) { namereln = dns_name_fullcompare(val->event->name, &siginfo->signer, &order, &nlabels); if (namereln != dns_namereln_subdomain && - namereln != dns_namereln_equal) { + namereln != dns_namereln_equal) + { return (DNS_R_CONTINUE); } @@ -2119,7 +2120,7 @@ start_positive_validation(dns_validator_t *val) { /*% * val_rdataset_first and val_rdataset_next provide iteration methods - * that hide whether we are iterating across a message or a negative + * that hide whether we are iterating across a message or a negative * cache rdataset. */ static isc_result_t @@ -2470,6 +2471,59 @@ findnsec3proofs(dns_validator_t *val) { return (result); } +/* + * Start a validator for negative response data. + * + * Returns: + * \li DNS_R_CONTINUE Validation skipped, continue + * \li DNS_R_WAIT Validation is in progress + * + * \li Other return codes indicate failure. + */ +static isc_result_t +validate_neg_rrset(dns_validator_t *val, dns_name_t *name, + dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset) +{ + isc_result_t result; + + /* + * If a signed zone is missing the zone key, bad + * things could happen. A query for data in the zone + * would lead to a query for the zone key, which + * would return a negative answer, which would contain + * an SOA and an NSEC signed by the missing key, which + * would trigger another query for the DNSKEY (since + * the first one is still in progress), and go into an + * infinite loop. Avoid that. + */ + if (val->event->type == dns_rdatatype_dnskey && + rdataset->type == dns_rdatatype_nsec && + dns_name_equal(name, val->event->name)) + { + dns_rdata_t nsec = DNS_RDATA_INIT; + + result = dns_rdataset_first(rdataset); + if (result != ISC_R_SUCCESS) { + return (result); + } + dns_rdataset_current(rdataset, &nsec); + if (dns_nsec_typepresent(&nsec, dns_rdatatype_soa)) { + return (DNS_R_CONTINUE); + } + } + + val->currentset = rdataset; + result = create_validator(val, name, rdataset->type, + rdataset, sigrdataset, + authvalidated, "validate_neg_rrset"); + if (result != ISC_R_SUCCESS) { + return (result); + } + + val->authcount++; + return (DNS_R_WAIT); +} + /*% * Validate the authority section records. */ @@ -2519,43 +2573,12 @@ validate_authority(dns_validator_t *val, bool resume) { break; } } - /* - * If a signed zone is missing the zone key, bad - * things could happen. A query for data in the zone - * would lead to a query for the zone key, which - * would return a negative answer, which would contain - * an SOA and an NSEC signed by the missing key, which - * would trigger another query for the DNSKEY (since - * the first one is still in progress), and go into an - * infinite loop. Avoid that. - */ - if (val->event->type == dns_rdatatype_dnskey && - rdataset->type == dns_rdatatype_nsec && - dns_name_equal(name, val->event->name)) - { - dns_rdata_t nsec = DNS_RDATA_INIT; - result = dns_rdataset_first(rdataset); - if (result != ISC_R_SUCCESS) { - return (result); - } - dns_rdataset_current(rdataset, &nsec); - if (dns_nsec_typepresent(&nsec, - dns_rdatatype_soa)) - { - continue; - } - } - val->currentset = rdataset; - result = create_validator(val, name, rdataset->type, - rdataset, sigrdataset, - authvalidated, - "validate_authority"); - if (result != ISC_R_SUCCESS) { + result = validate_neg_rrset(val, name, rdataset, + sigrdataset); + if (result != DNS_R_CONTINUE) { return (result); } - val->authcount++; - return (DNS_R_WAIT); } } if (result == ISC_R_NOMORE) { @@ -2601,40 +2624,12 @@ validate_ncache(dns_validator_t *val, bool resume) { sigrdataset = &val->fsigrdataset; } - /* - * If a signed zone is missing the zone key, bad - * things could happen. A query for data in the zone - * would lead to a query for the zone key, which - * would return a negative answer, which would contain - * an SOA and an NSEC signed by the missing key, which - * would trigger another query for the DNSKEY (since - * the first one is still in progress), and go into an - * infinite loop. Avoid that. - */ - if (val->event->type == dns_rdatatype_dnskey && - rdataset->type == dns_rdatatype_nsec && - dns_name_equal(name, val->event->name)) - { - dns_rdata_t nsec = DNS_RDATA_INIT; + result = validate_neg_rrset(val, name, rdataset, sigrdataset); + if (result == DNS_R_CONTINUE) { + continue; + } - result = dns_rdataset_first(rdataset); - if (result != ISC_R_SUCCESS) { - return (result); - } - dns_rdataset_current(rdataset, &nsec); - if (dns_nsec_typepresent(&nsec, dns_rdatatype_soa)) { - continue; - } - } - val->currentset = rdataset; - result = create_validator(val, name, rdataset->type, - rdataset, sigrdataset, - authvalidated, "validate_ncache"); - if (result != ISC_R_SUCCESS) { - return (result); - } - val->authcount++; - return (DNS_R_WAIT); + return (result); } if (result == ISC_R_NOMORE) { result = ISC_R_SUCCESS; @@ -2680,8 +2675,10 @@ nsecvalidate(dns_validator_t *val, bool resume) { if (!FOUNDNOQNAME(val)) { findnsec3proofs(val); } + if (FOUNDNOQNAME(val) && FOUNDCLOSEST(val) && - !FOUNDOPTOUT(val)) { + !FOUNDOPTOUT(val)) + { validator_log(val, ISC_LOG_DEBUG(3), "marking as secure, noqname proof found"); marksecure(val->event); @@ -2700,6 +2697,7 @@ nsecvalidate(dns_validator_t *val, bool resume) { markanswer(val, "nsecvalidate (2)", NULL); return (ISC_R_SUCCESS); } + validator_log(val, ISC_LOG_DEBUG(3), "noqname proof not found"); return (DNS_R_NOVALIDNSEC); @@ -2742,6 +2740,7 @@ nsecvalidate(dns_validator_t *val, bool resume) { if (val->authfail != 0 && val->authcount == val->authfail) { return (DNS_R_BROKENCHAIN); } + validator_log(val, ISC_LOG_DEBUG(3), "nonexistence proof(s) not found"); val->attributes |= VALATTR_INSECURITY; From 3659cca6245e610c10bc8e7b1b92a65c458e7157 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 7 Aug 2019 16:54:05 -0700 Subject: [PATCH 06/15] rename some functions for better clarity --- lib/dns/validator.c | 56 ++++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/lib/dns/validator.c b/lib/dns/validator.c index 439930c95b..9b0bcfa1b8 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -45,19 +45,19 @@ * Basic processing sequences: * * \li When called with rdataset and sigrdataset: - * validator_start -> validate -> proveunsecure - * validator_start -> validate -> nsecvalidate (secure wildcard answer) + * validator_start -> validate_answer -> proveunsecure + * validator_start -> validate_answer -> validate_nx (if secure wildcard) * - * \li When called with rdataset: + * \li When called with rdataset but no sigrdataset: * validator_start -> proveunsecure * - * \li When called without a rdataset: - * validator_start -> nsecvalidate -> proveunsecure + * \li When called with no rdataset or sigrdataset: + * validator_start -> validate_nx-> 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. + * validator_start: determine what type of validation to do. + * validate_answer: attempt to perform a positive validation. + * proveunsecure: attempt to prove the answer comes from an unsecure zone. + * validate_nx: attempt to prove a negative response. */ #define VALIDATOR_MAGIC ISC_MAGIC('V', 'a', 'l', '?') @@ -108,13 +108,13 @@ get_dst_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo, dns_rdataset_t *rdataset); static isc_result_t -validate(dns_validator_t *val, bool resume); +validate_answer(dns_validator_t *val, bool resume); static isc_result_t validatezonekey(dns_validator_t *val); static isc_result_t -nsecvalidate(dns_validator_t *val, bool resume); +validate_nx(dns_validator_t *val, bool resume); static isc_result_t proveunsecure(dns_validator_t *val, bool have_ds, bool resume); @@ -416,7 +416,7 @@ fetch_callback_validator(isc_task_t *task, isc_event_t *event) { val->keyset = &val->frdataset; } } - result = validate(val, true); + result = validate_answer(val, true); if (result == DNS_R_NOVALIDSIG && (val->attributes & VALATTR_TRIEDVERIFY) == 0) { @@ -684,7 +684,7 @@ keyvalidated(isc_task_t *task, isc_event_t *event) { if (val->frdataset.trust >= dns_trust_secure) { (void) get_dst_key(val, val->siginfo, &val->frdataset); } - result = validate(val, true); + result = validate_answer(val, true); if (result == DNS_R_NOVALIDSIG && (val->attributes & VALATTR_TRIEDVERIFY) == 0) { @@ -847,7 +847,7 @@ cnamevalidated(isc_task_t *task, isc_event_t *event) { * * Looks for NOQNAME, NODATA and OPTOUT proofs. * - * Resumes nsecvalidate. + * Resumes validate_nx. */ static void authvalidated(isc_task_t *task, isc_event_t *event) { @@ -883,7 +883,7 @@ authvalidated(isc_task_t *task, isc_event_t *event) { if (result == ISC_R_CANCELED) { validator_done(val, result); } else { - result = nsecvalidate(val, true); + result = validate_nx(val, true); if (result != DNS_R_WAIT) { validator_done(val, result); } @@ -943,7 +943,7 @@ authvalidated(isc_task_t *task, isc_event_t *event) { } } - result = nsecvalidate(val, true); + result = validate_nx(val, true); if (result != DNS_R_WAIT) { validator_done(val, result); } @@ -1518,7 +1518,7 @@ verify(dns_validator_t *val, dst_key_t *key, dns_rdata_t *rdata, * \li Other return codes are possible and all indicate failure. */ static isc_result_t -validate(dns_validator_t *val, bool resume) { +validate_answer(dns_validator_t *val, bool resume) { isc_result_t result, vresult = DNS_R_NOVALIDSIG; dns_validatorevent_t *event; dns_rdata_t rdata = DNS_RDATA_INIT; @@ -1648,7 +1648,7 @@ validate(dns_validator_t *val, bool resume) { } validator_log(val, ISC_LOG_DEBUG(3), "looking for noqname proof"); - return (nsecvalidate(val, false)); + return (validate_nx(val, false)); } else if (vresult == ISC_R_SUCCESS) { marksecure(event); validator_log(val, ISC_LOG_DEBUG(3), @@ -2109,10 +2109,10 @@ validatezonekey(dns_validator_t *val) { static isc_result_t start_positive_validation(dns_validator_t *val) { /* - * If this is not a key, go straight into validate(). + * If this is not a key, go straight into validate_answer(). */ if (val->event->type != dns_rdatatype_dnskey || !isselfsigned(val)) { - return (validate(val, false)); + return (validate_answer(val, false)); } return (validatezonekey(val)); @@ -2646,15 +2646,15 @@ validate_ncache(dns_validator_t *val, bool resume) { * * If the required proofs are found we are done. * - * If the proofs are not found attempt to prove this is a unsecure + * If the proofs are not found attempt to prove this is an unsecure * response. */ static isc_result_t -nsecvalidate(dns_validator_t *val, bool resume) { +validate_nx(dns_validator_t *val, bool resume) { isc_result_t result; if (resume) { - validator_log(val, ISC_LOG_DEBUG(3), "resuming nsecvalidate"); + validator_log(val, ISC_LOG_DEBUG(3), "resuming validate_nx"); } if (val->event->message == NULL) { @@ -2689,12 +2689,12 @@ nsecvalidate(dns_validator_t *val, bool resume) { validator_log(val, ISC_LOG_DEBUG(3), "optout proof found"); val->event->optout = true; - markanswer(val, "nsecvalidate (1)", NULL); + markanswer(val, "validate_nx (1)", NULL); return (ISC_R_SUCCESS); } else if ((val->attributes & VALATTR_FOUNDUNKNOWN) != 0) { validator_log(val, ISC_LOG_DEBUG(3), "unknown NSEC3 hash algorithm found"); - markanswer(val, "nsecvalidate (2)", NULL); + markanswer(val, "validate_nx (2)", NULL); return (ISC_R_SUCCESS); } @@ -3009,7 +3009,7 @@ seek_ds(dns_validator_t *val, isc_result_t *resp) { * no such endpoint is found, then the response should have been secure. * * Returns: - * \li ISC_R_SUCCESS val->event->name is in a unsecure zone + * \li ISC_R_SUCCESS val->event->name is in an 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. @@ -3116,7 +3116,7 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { * \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 + * Note an answer that appears to be a secure positive answer may actually * be an unsecure positive answer. */ static void @@ -3201,7 +3201,7 @@ validator_start(isc_task_t *task, isc_event_t *event) { } else { val->attributes |= VALATTR_NEEDNODATA; } - result = nsecvalidate(val, false); + result = validate_nx(val, false); } else { INSIST(0); ISC_UNREACHABLE(); From ea1d4d11fc30751dcbcb1b176cc3ec5effe751f9 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 7 Aug 2019 17:26:35 -0700 Subject: [PATCH 07/15] refactor dsfetched/dsfetched2 into a common function --- lib/dns/validator.c | 239 +++++++++++++++++++------------------------- 1 file changed, 102 insertions(+), 137 deletions(-) diff --git a/lib/dns/validator.c b/lib/dns/validator.c index 9b0bcfa1b8..48f5db30fc 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -423,7 +423,6 @@ fetch_callback_validator(isc_task_t *task, isc_event_t *event) { saved_result = result; validator_log(val, ISC_LOG_DEBUG(3), "falling back to insecurity proof"); - val->attributes |= VALATTR_INSECURITY; result = proveunsecure(val, false, false); if (result == DNS_R_NOTINSECURE) { result = saved_result; @@ -455,17 +454,12 @@ fetch_callback_validator(isc_task_t *task, isc_event_t *event) { } } -/*% - * 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; dns_validator_t *val; dns_rdataset_t *rdataset; - bool want_destroy; + bool want_destroy, trustchain; isc_result_t result; isc_result_t eresult; dns_fetch_t *fetch; @@ -477,6 +471,12 @@ dsfetched(isc_task_t *task, isc_event_t *event) { rdataset = &val->frdataset; eresult = devent->result; + /* + * Set to true if we're walking a chain of trust; false if + * we're attempting to prove insecurity. + */ + trustchain = ((val->attributes & VALATTR_INSECURITY) == 0); + /* Free resources which are not of interest. */ if (devent->node != NULL) { dns_db_detachnode(devent->db, &devent->node); @@ -487,7 +487,6 @@ dsfetched(isc_task_t *task, isc_event_t *event) { if (dns_rdataset_isassociated(&val->fsigrdataset)) { dns_rdataset_disassociate(&val->fsigrdataset); } - isc_event_free(&event); INSIST(val->event != NULL); @@ -495,33 +494,98 @@ dsfetched(isc_task_t *task, isc_event_t *event) { LOCK(&val->lock); fetch = val->fetch; val->fetch = NULL; + if (CANCELED(val)) { validator_done(val, ISC_R_CANCELED); - } else if (eresult == ISC_R_SUCCESS) { - validator_log(val, ISC_LOG_DEBUG(3), - "dsset with trust %s", - dns_trust_totext(rdataset->trust)); - val->dsset = &val->frdataset; - result = validatezonekey(val); - if (result != DNS_R_WAIT) { - validator_done(val, result); + goto done; + } + + switch (eresult) { + case DNS_R_NXDOMAIN: + case DNS_R_NCACHENXDOMAIN: + /* + * These results only make sense if we're attempting + * an insecurity proof, not when walking a chain of trust. + */ + if (trustchain) { + goto unexpected; } - } else if (eresult == DNS_R_CNAME || - eresult == DNS_R_NXRRSET || - eresult == DNS_R_NCACHENXRRSET || - eresult == DNS_R_SERVFAIL) /* RFC 1034 parent? */ - { - validator_log(val, ISC_LOG_DEBUG(3), - "falling back to insecurity proof (%s)", - dns_result_totext(eresult)); - val->attributes |= VALATTR_INSECURITY; - result = proveunsecure(val, false, false); - if (result != DNS_R_WAIT) { - validator_done(val, result); + + /* FALLTHROUGH */ + case ISC_R_SUCCESS: + if (trustchain) { + /* + * We looked for a DS record as part of + * following a key chain upwards; resume following + * the chain. + */ + validator_log(val, ISC_LOG_DEBUG(3), + "dsset with trust %s", + dns_trust_totext(rdataset->trust)); + val->dsset = &val->frdataset; + result = validatezonekey(val); + if (result != DNS_R_WAIT) { + validator_done(val, result); + } + } else { + /* + * There is a DS which may or may not be a zone cut. + * In either case we are still in a secure zone, + * so keep looking for the break in the chain + * of trust. + */ + result = proveunsecure(val, (eresult == ISC_R_SUCCESS), + true); + if (result != DNS_R_WAIT) { + validator_done(val, result); + } } - } else { - validator_log(val, ISC_LOG_DEBUG(3), - "dsfetched: got %s", + break; + case DNS_R_CNAME: + case DNS_R_NXRRSET: + case DNS_R_NCACHENXRRSET: + case DNS_R_SERVFAIL: /* RFC 1034 parent? */ + if (trustchain) { + /* + * Failed to find a DS while following the + * chain of trust; now we need to prove insecurity. + */ + validator_log(val, ISC_LOG_DEBUG(3), + "falling back to insecurity proof (%s)", + dns_result_totext(eresult)); + result = proveunsecure(val, false, false); + if (result != DNS_R_WAIT) { + validator_done(val, result); + } + } else if (eresult == DNS_R_SERVFAIL) { + goto unexpected; + } else if (eresult != DNS_R_CNAME && + isdelegation(dns_fixedname_name(&devent->foundname), + &val->frdataset, eresult)) + { + /* + * Failed to find a DS while trying to prove + * insecurity. If this is a zone cut, that + * means we're insecure. + */ + result = markanswer(val, "dsfetched", + "no DS and this is a delegation"); + validator_done(val, result); + } else { + /* + * Not a zone cut, so we have to keep looking for + * the break point in the chain of trust. + */ + result = proveunsecure(val, false, true); + if (result != DNS_R_WAIT) { + validator_done(val, result); + } + } + break; + + default: + unexpected: + validator_log(val, ISC_LOG_DEBUG(3), "dsfetched: got %s", isc_result_totext(eresult)); if (eresult == ISC_R_CANCELED) { validator_done(val, eresult); @@ -529,107 +593,7 @@ dsfetched(isc_task_t *task, isc_event_t *event) { validator_done(val, DNS_R_BROKENCHAIN); } } - - want_destroy = exit_check(val); - UNLOCK(&val->lock); - - if (fetch != NULL) { - dns_resolver_destroyfetch(&fetch); - } - - if (want_destroy) { - destroy(val); - } -} - -/*% - * 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) { - dns_fetchevent_t *devent; - dns_validator_t *val; - dns_name_t *tname; - bool want_destroy; - isc_result_t result; - isc_result_t eresult; - dns_fetch_t *fetch; - - UNUSED(task); - INSIST(event->ev_type == DNS_EVENT_FETCHDONE); - devent = (dns_fetchevent_t *)event; - val = devent->ev_arg; - eresult = devent->result; - - /* Free resources which are not of interest. */ - if (devent->node != NULL) { - dns_db_detachnode(devent->db, &devent->node); - } - if (devent->db != NULL) { - dns_db_detach(&devent->db); - } - if (dns_rdataset_isassociated(&val->fsigrdataset)) { - dns_rdataset_disassociate(&val->fsigrdataset); - } - - INSIST(val->event != NULL); - - validator_log(val, ISC_LOG_DEBUG(3), "in dsfetched2: %s", - dns_result_totext(eresult)); - LOCK(&val->lock); - fetch = val->fetch; - val->fetch = NULL; - if (CANCELED(val)) { - validator_done(val, ISC_R_CANCELED); - } else if (eresult == DNS_R_CNAME || - eresult == DNS_R_NXRRSET || - eresult == DNS_R_NCACHENXRRSET) - { - /* - * There is no DS. If this is a delegation, we're done. - */ - tname = dns_fixedname_name(&devent->foundname); - if (eresult != DNS_R_CNAME && - isdelegation(tname, &val->frdataset, eresult)) - { - result = markanswer(val, "dsfetched2", - "no DS and this is a delegation"); - validator_done(val, result); - } else { - result = proveunsecure(val, false, true); - if (result != DNS_R_WAIT) { - validator_done(val, result); - } - } - } else if (eresult == ISC_R_SUCCESS || - eresult == DNS_R_NXDOMAIN || - eresult == DNS_R_NCACHENXDOMAIN) - { - /* - * 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, (eresult == ISC_R_SUCCESS), - true); - if (result != DNS_R_WAIT) { - validator_done(val, result); - } - } else { - if (eresult == ISC_R_CANCELED) { - validator_done(val, eresult); - } else { - validator_done(val, DNS_R_NOVALIDDS); - } - } + done: isc_event_free(&event); want_destroy = exit_check(val); @@ -691,7 +655,6 @@ keyvalidated(isc_task_t *task, isc_event_t *event) { saved_result = result; validator_log(val, ISC_LOG_DEBUG(3), "falling back to insecurity proof"); - val->attributes |= VALATTR_INSECURITY; result = proveunsecure(val, false, false); if (result == DNS_R_NOTINSECURE) { result = saved_result; @@ -2743,7 +2706,6 @@ validate_nx(dns_validator_t *val, bool resume) { validator_log(val, ISC_LOG_DEBUG(3), "nonexistence proof(s) not found"); - val->attributes |= VALATTR_INSECURITY; return (proveunsecure(val, false, false)); } @@ -2983,7 +2945,7 @@ seek_ds(dns_validator_t *val, isc_result_t *resp) { */ *resp = DNS_R_WAIT; result = create_fetch(val, tname, dns_rdatatype_ds, - dsfetched2, "proveunsecure"); + dsfetched, "proveunsecure"); if (result != ISC_R_SUCCESS) { *resp = result; } @@ -3026,6 +2988,11 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { dns_name_t *secroot = dns_fixedname_initname(&fixedsecroot); unsigned int labels; + /* + * We're attempting to prove insecurity. + */ + val->attributes |= VALATTR_INSECURITY; + dns_name_copynf(val->event->name, secroot); /* @@ -3159,7 +3126,6 @@ validator_start(isc_task_t *task, isc_event_t *event) { saved_result = result; validator_log(val, ISC_LOG_DEBUG(3), "falling back to insecurity proof"); - val->attributes |= VALATTR_INSECURITY; result = proveunsecure(val, false, false); if (result == DNS_R_NOTINSECURE) { result = saved_result; @@ -3176,7 +3142,6 @@ validator_start(isc_task_t *task, isc_event_t *event) { validator_log(val, ISC_LOG_DEBUG(3), "attempting insecurity proof"); - val->attributes |= VALATTR_INSECURITY; result = proveunsecure(val, false, false); if (result == DNS_R_NOTINSECURE) { validator_log(val, ISC_LOG_INFO, From d0f8c5061812dd3a7e67dc3da92561f24a638086 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 7 Aug 2019 17:49:59 -0700 Subject: [PATCH 08/15] convert if statement to switch --- lib/dns/validator.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/lib/dns/validator.c b/lib/dns/validator.c index 48f5db30fc..c5cf919433 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -1870,7 +1870,8 @@ validatezonekey(dns_validator_t *val) { * Otherwise, try to find the DS record. */ result = view_find(val, val->event->name, dns_rdatatype_ds); - if (result == ISC_R_SUCCESS) { + switch (result) { + case ISC_R_SUCCESS: /* * We have DS records. */ @@ -1902,7 +1903,9 @@ validatezonekey(dns_validator_t *val) { result = ISC_R_SUCCESS; POST(result); } - } else if (result == ISC_R_NOTFOUND) { + break; + + case ISC_R_NOTFOUND: /* * We don't have the DS. Find it. */ @@ -1913,21 +1916,25 @@ validatezonekey(dns_validator_t *val) { return (result); } return (DNS_R_WAIT); - } else if (result == DNS_R_NCACHENXDOMAIN || - result == DNS_R_NCACHENXRRSET || - result == DNS_R_EMPTYNAME || - result == DNS_R_NXDOMAIN || - result == DNS_R_NXRRSET || - result == DNS_R_CNAME) - { + + case DNS_R_NCACHENXDOMAIN: + case DNS_R_NCACHENXRRSET: + case DNS_R_EMPTYNAME: + case DNS_R_NXDOMAIN: + case DNS_R_NXRRSET: + case DNS_R_CNAME: /* * The DS does not exist. */ disassociate_rdatasets(val); validator_log(val, ISC_LOG_DEBUG(2), "no DS record"); return (DNS_R_NOVALIDSIG); - } else if (result == DNS_R_BROKENCHAIN) { + + case DNS_R_BROKENCHAIN: return (result); + + default: + break; } } From edc9c79c9cb5ba0f42650a39af03f4aec4fb0ef9 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 8 Aug 2019 08:35:21 -0700 Subject: [PATCH 09/15] reorder switch in validatezonekey to similar order as seek_ds --- lib/dns/validator.c | 144 +++++++++++++++++++++++--------------------- 1 file changed, 74 insertions(+), 70 deletions(-) diff --git a/lib/dns/validator.c b/lib/dns/validator.c index c5cf919433..c6b64f566a 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -2716,6 +2716,10 @@ validate_nx(dns_validator_t *val, bool resume) { return (proveunsecure(val, false, false)); } +/*% + * Check that DS rdataset has at least one record with + * a supported algorithm and digest. + */ static bool check_ds(dns_validator_t *val, dns_name_t *name, dns_rdataset_t *rdataset) { dns_rdata_t dsrdata = DNS_RDATA_INIT; @@ -2775,6 +2779,59 @@ seek_ds(dns_validator_t *val, isc_result_t *resp) { result = view_find(val, tname, dns_rdatatype_ds); switch (result) { + case ISC_R_SUCCESS: + /* + * There is a DS here. Verify that it's secure and + * continue walking down labels. + */ + if (val->frdataset.trust >= dns_trust_secure) { + if (!check_ds(val, tname, &val->frdataset)) { + validator_log(val, ISC_LOG_DEBUG(3), + "no supported algorithm/" + "digest (%s/DS)", + namebuf); + *resp = markanswer(val, "proveunsecure (5)", + "no supported " + "algorithm/digest (DS)"); + return (ISC_R_COMPLETE); + } + + break; + } + + if (!dns_rdataset_isassociated(&val->fsigrdataset)) { + validator_log(val, ISC_LOG_DEBUG(3), "DS is unsigned"); + *resp = DNS_R_NOVALIDSIG; + } else { + /* + * Validate / re-validate answer. + */ + result = create_validator(val, tname, + dns_rdatatype_ds, + &val->frdataset, + &val->fsigrdataset, + dsvalidated, + "proveunsecure"); + *resp = DNS_R_WAIT; + if (result != ISC_R_SUCCESS) { + *resp = result; + } + } + + return (ISC_R_COMPLETE); + + case ISC_R_NOTFOUND: + /* + * We don't know anything about the DS. Find it. + */ + *resp = DNS_R_WAIT; + result = create_fetch(val, tname, dns_rdatatype_ds, + dsfetched, "proveunsecure"); + if (result != ISC_R_SUCCESS) { + *resp = result; + } + return (ISC_R_COMPLETE); + case DNS_R_NXRRSET: case DNS_R_NCACHENXRRSET: /* @@ -2838,66 +2895,6 @@ seek_ds(dns_validator_t *val, isc_result_t *resp) { break; - case DNS_R_CNAME: - if (DNS_TRUST_PENDING(val->frdataset.trust) || - DNS_TRUST_ANSWER(val->frdataset.trust)) - { - result = create_validator(val, tname, - dns_rdatatype_cname, - &val->frdataset, - NULL, cnamevalidated, - "proveunsecure " - "(cname)"); - *resp = DNS_R_WAIT; - if (result != ISC_R_SUCCESS) { - *resp = result; - } - return (ISC_R_COMPLETE); - } - - break; - - case ISC_R_SUCCESS: - /* - * There is a DS here. Verify that it's secure and - * continue walking down labels. - */ - if (val->frdataset.trust >= dns_trust_secure) { - if (!check_ds(val, tname, &val->frdataset)) { - validator_log(val, ISC_LOG_DEBUG(3), - "no supported algorithm/" - "digest (%s/DS)", - namebuf); - *resp = markanswer(val, "proveunsecure (5)", - "no supported " - "algorithm/digest (DS)"); - return (ISC_R_COMPLETE); - } - - break; - } - - if (!dns_rdataset_isassociated(&val->fsigrdataset)) { - validator_log(val, ISC_LOG_DEBUG(3), "DS is unsigned"); - *resp = DNS_R_NOVALIDSIG; - } else { - /* - * Validate / re-validate answer. - */ - result = create_validator(val, tname, - dns_rdatatype_ds, - &val->frdataset, - &val->fsigrdataset, - dsvalidated, - "proveunsecure"); - *resp = DNS_R_WAIT; - if (result != ISC_R_SUCCESS) { - *resp = result; - } - } - - return (ISC_R_COMPLETE); - case DNS_R_NXDOMAIN: case DNS_R_NCACHENXDOMAIN: /* @@ -2946,17 +2943,24 @@ seek_ds(dns_validator_t *val, isc_result_t *resp) { break; - case ISC_R_NOTFOUND: - /* - * We don't know anything about the DS. Find it. - */ - *resp = DNS_R_WAIT; - result = create_fetch(val, tname, dns_rdatatype_ds, - dsfetched, "proveunsecure"); - if (result != ISC_R_SUCCESS) { - *resp = result; + case DNS_R_CNAME: + if (DNS_TRUST_PENDING(val->frdataset.trust) || + DNS_TRUST_ANSWER(val->frdataset.trust)) + { + result = create_validator(val, tname, + dns_rdatatype_cname, + &val->frdataset, + NULL, cnamevalidated, + "proveunsecure " + "(cname)"); + *resp = DNS_R_WAIT; + if (result != ISC_R_SUCCESS) { + *resp = result; + } + return (ISC_R_COMPLETE); } - return (ISC_R_COMPLETE); + + break; default: *resp = result; From 9150688efd1620d915b500cd4dc8833cf446698a Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Fri, 16 Aug 2019 12:28:22 -0700 Subject: [PATCH 10/15] rename fetch/validator callback functions for consistency and clarity --- lib/dns/validator.c | 95 +++++++++++++++++++++++++-------------------- 1 file changed, 52 insertions(+), 43 deletions(-) diff --git a/lib/dns/validator.c b/lib/dns/validator.c index c6b64f566a..fafc1fbdad 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -362,11 +362,11 @@ isdelegation(dns_name_t *name, dns_rdataset_t *rdataset, /*% * We have been asked to look for a key. - * If found resume the validation process. - * If not found fail the validation process. + * 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) { +fetch_callback_dnskey(isc_task_t *task, isc_event_t *event) { dns_fetchevent_t *devent; dns_validator_t *val; dns_rdataset_t *rdataset; @@ -397,7 +397,7 @@ fetch_callback_validator(isc_task_t *task, isc_event_t *event) { INSIST(val->event != NULL); - validator_log(val, ISC_LOG_DEBUG(3), "in fetch_callback_validator"); + validator_log(val, ISC_LOG_DEBUG(3), "in fetch_callback_dnskey"); LOCK(&val->lock); fetch = val->fetch; val->fetch = NULL; @@ -433,7 +433,7 @@ fetch_callback_validator(isc_task_t *task, isc_event_t *event) { } } else { validator_log(val, ISC_LOG_DEBUG(3), - "fetch_callback_validator: got %s", + "fetch_callback_dnskey: got %s", isc_result_totext(eresult)); if (eresult == ISC_R_CANCELED) { validator_done(val, eresult); @@ -454,8 +454,12 @@ fetch_callback_validator(isc_task_t *task, isc_event_t *event) { } } +/*% + * We have been asked to look for a DS. This may be part of + * walking a trust chain, or an insecurity proof. + */ static void -dsfetched(isc_task_t *task, isc_event_t *event) { +fetch_callback_ds(isc_task_t *task, isc_event_t *event) { dns_fetchevent_t *devent; dns_validator_t *val; dns_rdataset_t *rdataset; @@ -472,8 +476,8 @@ dsfetched(isc_task_t *task, isc_event_t *event) { eresult = devent->result; /* - * Set to true if we're walking a chain of trust; false if - * we're attempting to prove insecurity. + * Set 'trustchain' to true if we're walking a chain of + * trust; false if we're attempting to prove insecurity. */ trustchain = ((val->attributes & VALATTR_INSECURITY) == 0); @@ -490,7 +494,7 @@ dsfetched(isc_task_t *task, isc_event_t *event) { INSIST(val->event != NULL); - validator_log(val, ISC_LOG_DEBUG(3), "in dsfetched"); + validator_log(val, ISC_LOG_DEBUG(3), "in fetch_callback_ds"); LOCK(&val->lock); fetch = val->fetch; val->fetch = NULL; @@ -568,7 +572,7 @@ dsfetched(isc_task_t *task, isc_event_t *event) { * insecurity. If this is a zone cut, that * means we're insecure. */ - result = markanswer(val, "dsfetched", + result = markanswer(val, "fetch_callback_ds", "no DS and this is a delegation"); validator_done(val, result); } else { @@ -585,7 +589,8 @@ dsfetched(isc_task_t *task, isc_event_t *event) { default: unexpected: - validator_log(val, ISC_LOG_DEBUG(3), "dsfetched: got %s", + validator_log(val, ISC_LOG_DEBUG(3), + "fetch_callback_ds: got %s", isc_result_totext(eresult)); if (eresult == ISC_R_CANCELED) { validator_done(val, eresult); @@ -614,7 +619,7 @@ dsfetched(isc_task_t *task, isc_event_t *event) { * Resumes the stalled validation process. */ static void -keyvalidated(isc_task_t *task, isc_event_t *event) { +validator_callback_dnskey(isc_task_t *task, isc_event_t *event) { dns_validatorevent_t *devent; dns_validator_t *val; bool want_destroy; @@ -634,7 +639,7 @@ keyvalidated(isc_task_t *task, isc_event_t *event) { INSIST(val->event != NULL); - validator_log(val, ISC_LOG_DEBUG(3), "in keyvalidated"); + validator_log(val, ISC_LOG_DEBUG(3), "in validator_callback_dnskey"); LOCK(&val->lock); if (CANCELED(val)) { validator_done(val, ISC_R_CANCELED); @@ -668,7 +673,7 @@ keyvalidated(isc_task_t *task, isc_event_t *event) { expire_rdatasets(val); } validator_log(val, ISC_LOG_DEBUG(3), - "keyvalidated: got %s", + "validator_callback_dnskey: got %s", isc_result_totext(eresult)); validator_done(val, DNS_R_BROKENCHAIN); } @@ -686,7 +691,7 @@ keyvalidated(isc_task_t *task, isc_event_t *event) { * Resumes validation of the zone key or the unsecure zone proof. */ static void -dsvalidated(isc_task_t *task, isc_event_t *event) { +validator_callback_ds(isc_task_t *task, isc_event_t *event) { dns_validatorevent_t *devent; dns_validator_t *val; bool want_destroy; @@ -705,7 +710,7 @@ dsvalidated(isc_task_t *task, isc_event_t *event) { INSIST(val->event != NULL); - validator_log(val, ISC_LOG_DEBUG(3), "in dsvalidated"); + validator_log(val, ISC_LOG_DEBUG(3), "in validator_callback_ds"); LOCK(&val->lock); if (CANCELED(val)) { validator_done(val, ISC_R_CANCELED); @@ -724,7 +729,7 @@ dsvalidated(isc_task_t *task, isc_event_t *event) { NEGATIVE(&val->frdataset) && isdelegation(name, &val->frdataset, DNS_R_NCACHENXRRSET)) { - result = markanswer(val, "dsvalidated", + result = markanswer(val, "validator_callback_ds", "no DS and this is a delegation"); } else if ((val->attributes & VALATTR_INSECURITY) != 0) { result = proveunsecure(val, have_dsset, true); @@ -739,7 +744,7 @@ dsvalidated(isc_task_t *task, isc_event_t *event) { expire_rdatasets(val); } validator_log(val, ISC_LOG_DEBUG(3), - "dsvalidated: got %s", + "validator_callback_ds: got %s", isc_result_totext(eresult)); validator_done(val, DNS_R_BROKENCHAIN); } @@ -757,7 +762,7 @@ dsvalidated(isc_task_t *task, isc_event_t *event) { * Resumes validation of the unsecure zone proof. */ static void -cnamevalidated(isc_task_t *task, isc_event_t *event) { +validator_callback_cname(isc_task_t *task, isc_event_t *event) { dns_validatorevent_t *devent; dns_validator_t *val; bool want_destroy; @@ -777,7 +782,7 @@ cnamevalidated(isc_task_t *task, isc_event_t *event) { INSIST(val->event != NULL); INSIST((val->attributes & VALATTR_INSECURITY) != 0); - validator_log(val, ISC_LOG_DEBUG(3), "in cnamevalidated"); + validator_log(val, ISC_LOG_DEBUG(3), "in validator_callback_cname"); LOCK(&val->lock); if (CANCELED(val)) { validator_done(val, ISC_R_CANCELED); @@ -793,7 +798,7 @@ cnamevalidated(isc_task_t *task, isc_event_t *event) { expire_rdatasets(val); } validator_log(val, ISC_LOG_DEBUG(3), - "cnamevalidated: got %s", + "validator_callback_cname: got %s", isc_result_totext(eresult)); validator_done(val, DNS_R_BROKENCHAIN); } @@ -813,7 +818,7 @@ cnamevalidated(isc_task_t *task, isc_event_t *event) { * Resumes validate_nx. */ static void -authvalidated(isc_task_t *task, isc_event_t *event) { +validator_callback_nsec(isc_task_t *task, isc_event_t *event) { dns_validatorevent_t *devent; dns_validator_t *val; dns_rdataset_t *rdataset; @@ -832,13 +837,13 @@ authvalidated(isc_task_t *task, isc_event_t *event) { INSIST(val->event != NULL); - validator_log(val, ISC_LOG_DEBUG(3), "in authvalidated"); + validator_log(val, ISC_LOG_DEBUG(3), "in validator_callback_nsec"); LOCK(&val->lock); if (CANCELED(val)) { validator_done(val, ISC_R_CANCELED); } else if (result != ISC_R_SUCCESS) { validator_log(val, ISC_LOG_DEBUG(3), - "authvalidated: got %s", + "validator_callback_nsec: got %s", isc_result_totext(result)); if (result == DNS_R_BROKENCHAIN) { val->authfail++; @@ -1137,8 +1142,7 @@ get_dst_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo, * This is the key we're looking for. */ return (ISC_R_SUCCESS); - } else if (dst_key_compare(oldkey, - val->key) == true) { + } else if (dst_key_compare(oldkey, val->key)) { foundold = true; dst_key_free(&oldkey); } @@ -1243,7 +1247,7 @@ get_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo) { dns_rdatatype_dnskey, &val->frdataset, &val->fsigrdataset, - keyvalidated, + validator_callback_dnskey, "get_key"); if (result != ISC_R_SUCCESS) { return (result); @@ -1285,7 +1289,7 @@ get_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo) { */ result = create_fetch(val, &siginfo->signer, dns_rdatatype_dnskey, - fetch_callback_validator, "get_key"); + fetch_callback_dnskey, "get_key"); if (result != ISC_R_SUCCESS) { return (result); } @@ -1572,9 +1576,11 @@ validate_answer(dns_validator_t *val, bool resume) { break; } } else { - if (get_dst_key(val, val->siginfo, val->keyset) - != ISC_R_SUCCESS) - { + isc_result_t tresult; + + tresult = get_dst_key(val, val->siginfo, + val->keyset); + if (tresult != ISC_R_SUCCESS) { break; } } @@ -1885,7 +1891,7 @@ validatezonekey(dns_validator_t *val) { dns_rdatatype_ds, &val->frdataset, &val->fsigrdataset, - dsvalidated, + validator_callback_ds, "validatezonekey"); if (result != ISC_R_SUCCESS) { return (result); @@ -1910,7 +1916,8 @@ validatezonekey(dns_validator_t *val) { * We don't have the DS. Find it. */ result = create_fetch(val, val->event->name, - dns_rdatatype_ds, dsfetched, + dns_rdatatype_ds, + fetch_callback_ds, "validatezonekey"); if (result != ISC_R_SUCCESS) { return (result); @@ -2485,7 +2492,8 @@ validate_neg_rrset(dns_validator_t *val, dns_name_t *name, val->currentset = rdataset; result = create_validator(val, name, rdataset->type, rdataset, sigrdataset, - authvalidated, "validate_neg_rrset"); + validator_callback_nsec, + "validate_neg_rrset"); if (result != ISC_R_SUCCESS) { return (result); } @@ -2612,7 +2620,8 @@ validate_ncache(dns_validator_t *val, bool resume) { * answer is from a wildcard. * * Loop through the authority section looking for NODATA, NOWILDCARD - * and NOQNAME proofs in the NSEC records by calling authvalidated(). + * and NOQNAME proofs in the NSEC records by calling + * validator_callback_nsec(). * * If the required proofs are found we are done. * @@ -2810,7 +2819,7 @@ seek_ds(dns_validator_t *val, isc_result_t *resp) { dns_rdatatype_ds, &val->frdataset, &val->fsigrdataset, - dsvalidated, + validator_callback_ds, "proveunsecure"); *resp = DNS_R_WAIT; if (result != ISC_R_SUCCESS) { @@ -2826,7 +2835,7 @@ seek_ds(dns_validator_t *val, isc_result_t *resp) { */ *resp = DNS_R_WAIT; result = create_fetch(val, tname, dns_rdatatype_ds, - dsfetched, "proveunsecure"); + fetch_callback_ds, "proveunsecure"); if (result != ISC_R_SUCCESS) { *resp = result; } @@ -2846,8 +2855,8 @@ seek_ds(dns_validator_t *val, isc_result_t *resp) { { result = create_validator(val, tname, dns_rdatatype_ds, - &val->frdataset, - NULL, dsvalidated, + &val->frdataset, NULL, + validator_callback_ds, "proveunsecure"); *resp = DNS_R_WAIT; if (result != ISC_R_SUCCESS) { @@ -2919,8 +2928,8 @@ seek_ds(dns_validator_t *val, isc_result_t *resp) { *resp = DNS_R_WAIT; result = create_validator(val, tname, dns_rdatatype_ds, - &val->frdataset, - NULL, dsvalidated, + &val->frdataset, NULL, + validator_callback_ds, "proveunsecure"); if (result != ISC_R_SUCCESS) { *resp = result; @@ -2949,8 +2958,8 @@ seek_ds(dns_validator_t *val, isc_result_t *resp) { { result = create_validator(val, tname, dns_rdatatype_cname, - &val->frdataset, - NULL, cnamevalidated, + &val->frdataset, NULL, + validator_callback_cname, "proveunsecure " "(cname)"); *resp = DNS_R_WAIT; From 22aa668b7d09f3edba0bc59c5adfee5aa456e024 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Fri, 16 Aug 2019 16:27:50 -0700 Subject: [PATCH 11/15] convert if to switch in get_key --- lib/dns/validator.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/lib/dns/validator.c b/lib/dns/validator.c index fafc1fbdad..33ca5865ff 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -1229,7 +1229,8 @@ get_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo) { * Do we know about this key? */ result = view_find(val, &siginfo->signer, dns_rdatatype_dnskey); - if (result == ISC_R_SUCCESS) { + switch (result) { + case ISC_R_SUCCESS: /* * We have an rrset for the given keyname. */ @@ -1283,7 +1284,9 @@ get_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo) { result = DNS_R_CONTINUE; } } - } else if (result == ISC_R_NOTFOUND) { + break; + + case ISC_R_NOTFOUND: /* * We don't know anything about this key. */ @@ -1294,18 +1297,23 @@ get_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo) { return (result); } return (DNS_R_WAIT); - } else if (result == DNS_R_NCACHENXDOMAIN || - result == DNS_R_NCACHENXRRSET || - result == DNS_R_EMPTYNAME || - result == DNS_R_NXDOMAIN || - result == DNS_R_NXRRSET) - { + + case DNS_R_NCACHENXDOMAIN: + case DNS_R_NCACHENXRRSET: + case DNS_R_EMPTYNAME: + case DNS_R_NXDOMAIN: + case DNS_R_NXRRSET: /* * This key doesn't exist. */ result = DNS_R_CONTINUE; - } else if (result == DNS_R_BROKENCHAIN) { + break; + + case DNS_R_BROKENCHAIN: return (result); + + default: + break; } if (dns_rdataset_isassociated(&val->frdataset) && From 9119dc25fe01d9e873e0a9c6691d7d0c0ac334c9 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Fri, 16 Aug 2019 16:28:18 -0700 Subject: [PATCH 12/15] continue renaming functions for clarity - also simplified some calls: don't pass siginfo where val->siginfo is sufficient, don't INSIST where returning false is sufficient. - also added header comments to several local functions. --- lib/dns/validator.c | 163 ++++++++++++++++++++++---------------------- 1 file changed, 81 insertions(+), 82 deletions(-) diff --git a/lib/dns/validator.c b/lib/dns/validator.c index 33ca5865ff..710dcb234d 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -104,14 +104,13 @@ static void destroy(dns_validator_t *val); static isc_result_t -get_dst_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo, - dns_rdataset_t *rdataset); +select_signing_key(dns_validator_t *val, dns_rdataset_t *rdataset); static isc_result_t validate_answer(dns_validator_t *val, bool resume); static isc_result_t -validatezonekey(dns_validator_t *val); +validate_dnskey(dns_validator_t *val); static isc_result_t validate_nx(dns_validator_t *val, bool resume); @@ -187,7 +186,7 @@ markanswer(dns_validator_t *val, const char *where, const char *mbstext) { } /*% - * Mark the RRsets with trust level secure. + * Mark the RRsets in val->event with trust level secure. */ static inline void marksecure(dns_validatorevent_t *event) { @@ -198,6 +197,10 @@ marksecure(dns_validatorevent_t *event) { event->secure = true; } +/* + * Validator 'val' is finished; send the completion event to the task + * that called dns_validator_create(), with result `result`. + */ static void validator_done(dns_validator_t *val, isc_result_t result) { isc_task_t *task; @@ -219,6 +222,9 @@ validator_done(dns_validator_t *val, isc_result_t result) { isc_task_sendanddetach(&task, (isc_event_t **)&val->event); } +/* + * Called when deciding whether to destroy validator 'val'. + */ static inline bool exit_check(dns_validator_t *val) { /* @@ -411,7 +417,7 @@ fetch_callback_dnskey(isc_task_t *task, isc_event_t *event) { * Only extract the dst key if the keyset is secure. */ if (rdataset->trust >= dns_trust_secure) { - result = get_dst_key(val, val->siginfo, rdataset); + result = select_signing_key(val, rdataset); if (result == ISC_R_SUCCESS) { val->keyset = &val->frdataset; } @@ -527,7 +533,7 @@ fetch_callback_ds(isc_task_t *task, isc_event_t *event) { "dsset with trust %s", dns_trust_totext(rdataset->trust)); val->dsset = &val->frdataset; - result = validatezonekey(val); + result = validate_dnskey(val); if (result != DNS_R_WAIT) { validator_done(val, result); } @@ -651,7 +657,7 @@ validator_callback_dnskey(isc_task_t *task, isc_event_t *event) { * Only extract the dst key if the keyset is secure. */ if (val->frdataset.trust >= dns_trust_secure) { - (void) get_dst_key(val, val->siginfo, &val->frdataset); + (void) select_signing_key(val, &val->frdataset); } result = validate_answer(val, true); if (result == DNS_R_NOVALIDSIG && @@ -720,7 +726,7 @@ validator_callback_ds(isc_task_t *task, isc_event_t *event) { validator_log(val, ISC_LOG_DEBUG(3), "%s with trust %s", val->frdataset.type == dns_rdatatype_ds ? - "dsset" : "ds non-existance", + "dsset" : "ds non-existence", dns_trust_totext(val->frdataset.trust)); have_dsset = (val->frdataset.type == dns_rdatatype_ds); name = dns_fixedname_name(&val->fname); @@ -734,7 +740,7 @@ validator_callback_ds(isc_task_t *task, isc_event_t *event) { } else if ((val->attributes & VALATTR_INSECURITY) != 0) { result = proveunsecure(val, have_dsset, true); } else { - result = validatezonekey(val); + result = validate_dnskey(val); } if (result != DNS_R_WAIT) { validator_done(val, result); @@ -815,7 +821,7 @@ validator_callback_cname(isc_task_t *task, isc_event_t *event) { * * Looks for NOQNAME, NODATA and OPTOUT proofs. * - * Resumes validate_nx. + * Resumes the negative response validation by calling validate_nx(). */ static void validator_callback_nsec(isc_task_t *task, isc_event_t *event) { @@ -1097,13 +1103,17 @@ create_validator(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type, * in 'rdataset'. If found, build a dst_key_t for it and point * val->key at it. * - * If val->key is non-NULL, this returns the next matching key. + * If val->key is already non-NULL, locate it in the rdataset and + * then search past it for the *next* key that could have signed + * 'siginfo', then set val->key to that. + * + * Returns ISC_R_SUCCESS if a possible matching key has been + * found, ISC_R_NOTFOUND if not. Any other value indicates error. */ static isc_result_t -get_dst_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo, - dns_rdataset_t *rdataset) -{ +select_signing_key(dns_validator_t *val, dns_rdataset_t *rdataset) { isc_result_t result; + dns_rdata_rrsig_t *siginfo = val->siginfo; isc_buffer_t b; dns_rdata_t rdata = DNS_RDATA_INIT; dst_key_t *oldkey = val->key; @@ -1165,11 +1175,12 @@ get_dst_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo, } /*% - * Get the key that generated this signature. + * Get the key that generated the signature in val->siginfo. */ static isc_result_t -get_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo) { +seek_dnskey(dns_validator_t *val) { isc_result_t result; + dns_rdata_rrsig_t *siginfo = val->siginfo; unsigned int nlabels; int order; dns_namereln_t namereln; @@ -1191,7 +1202,7 @@ get_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo) { if (namereln == dns_namereln_equal) { /* * If this is a self-signed keyset, it must not be a zone key - * (since get_key is not called from validatezonekey). + * (since seek_dnskey is not called from validate_dnskey). */ if (val->event->rdataset->type == dns_rdatatype_dnskey) { return (DNS_R_CONTINUE); @@ -1249,7 +1260,7 @@ get_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo) { &val->frdataset, &val->fsigrdataset, validator_callback_dnskey, - "get_key"); + "seek_dnskey"); if (result != ISC_R_SUCCESS) { return (result); } @@ -1274,7 +1285,7 @@ get_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo) { validator_log(val, ISC_LOG_DEBUG(3), "keyset with trust %s", dns_trust_totext(val->frdataset.trust)); - result = get_dst_key(val, siginfo, val->keyset); + result = select_signing_key(val, val->keyset); if (result != ISC_R_SUCCESS) { /* * Either the key we're looking for is not @@ -1292,7 +1303,7 @@ get_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo) { */ result = create_fetch(val, &siginfo->signer, dns_rdatatype_dnskey, - fetch_callback_dnskey, "get_key"); + fetch_callback_dnskey, "seek_dnskey"); if (result != ISC_R_SUCCESS) { return (result); } @@ -1328,6 +1339,9 @@ get_key(dns_validator_t *val, dns_rdata_rrsig_t *siginfo) { return (result); } +/* + * Compute the tag for a key represented in a DNSKEY rdata. + */ static dns_keytag_t compute_keytag(dns_rdata_t *rdata) { isc_region_t r; @@ -1337,10 +1351,10 @@ compute_keytag(dns_rdata_t *rdata) { } /*% - * Is this keyset self-signed? + * Is the DNSKEY rrset in val->event->rdataset self-signed? */ static bool -isselfsigned(dns_validator_t *val) { +selfsigned_dnskey(dns_validator_t *val) { dns_rdataset_t *rdataset, *sigrdataset; dns_rdata_t rdata = DNS_RDATA_INIT; dns_rdata_t sigrdata = DNS_RDATA_INIT; @@ -1358,14 +1372,10 @@ isselfsigned(dns_validator_t *val) { name = val->event->name; mctx = val->view->mctx; - if (rdataset->type == dns_rdatatype_cname || - rdataset->type == dns_rdatatype_dname) - { - return (answer); + if (rdataset->type != dns_rdatatype_dnskey) { + return (false); } - INSIST(rdataset->type == dns_rdatatype_dnskey); - for (result = dns_rdataset_first(rdataset); result == ISC_R_SUCCESS; result = dns_rdataset_next(rdataset)) @@ -1413,6 +1423,7 @@ isselfsigned(dns_validator_t *val) { dns_view_untrust(val->view, name, &key, mctx); } } + return (answer); } @@ -1542,7 +1553,7 @@ validate_answer(dns_validator_t *val, bool resume) { } if (!resume) { - result = get_key(val, val->siginfo); + result = seek_dnskey(val); if (result == DNS_R_CONTINUE) { continue; /* Try the next SIG RR. */ } @@ -1586,8 +1597,7 @@ validate_answer(dns_validator_t *val, bool resume) { } else { isc_result_t tresult; - tresult = get_dst_key(val, val->siginfo, - val->keyset); + tresult = select_signing_key(val, val->keyset); if (tresult != ISC_R_SUCCESS) { break; } @@ -1655,8 +1665,8 @@ validate_answer(dns_validator_t *val, bool resume) { * (val->event->rdataset). */ static isc_result_t -checkkey(dns_validator_t *val, dns_rdata_t *keyrdata, uint16_t keyid, - dns_secalg_t algorithm) +check_signer(dns_validator_t *val, dns_rdata_t *keyrdata, + uint16_t keyid, dns_secalg_t algorithm) { dns_rdata_rrsig_t sig; dst_key_t *dstkey = NULL; @@ -1701,9 +1711,8 @@ checkkey(dns_validator_t *val, dns_rdata_t *keyrdata, uint16_t keyid, * Find the DNSKEY that corresponds to the DS. */ static isc_result_t -keyfromds(dns_validator_t *val, dns_rdataset_t *rdataset, dns_rdata_t *dsrdata, - dns_dsdigest_t digest, uint16_t keyid, dns_secalg_t algorithm, - dns_rdata_t *keyrdata) +dnskey_for_ds(dns_validator_t *val, dns_rdataset_t *rdataset, + dns_rdata_t *dsrdata, dns_rdata_ds_t *ds, dns_rdata_t *keyrdata) { dns_keytag_t keytag; dns_rdata_dnskey_t key; @@ -1721,11 +1730,12 @@ keyfromds(dns_validator_t *val, dns_rdataset_t *rdataset, dns_rdata_t *dsrdata, result = dns_rdata_tostruct(keyrdata, &key, NULL); RUNTIME_CHECK(result == ISC_R_SUCCESS); keytag = compute_keytag(keyrdata); - if (keyid != keytag || algorithm != key.algorithm) { + if (ds->key_tag != keytag || ds->algorithm != key.algorithm) { continue; } dns_rdata_reset(&newdsrdata); - result = dns_ds_buildrdata(val->event->name, keyrdata, digest, + result = dns_ds_buildrdata(val->event->name, keyrdata, + ds->digest_type, dsbuf, &newdsrdata); if (result != ISC_R_SUCCESS) { validator_log(val, ISC_LOG_DEBUG(3), @@ -1751,7 +1761,7 @@ keyfromds(dns_validator_t *val, dns_rdataset_t *rdataset, dns_rdata_t *dsrdata, * \li Other return codes are possible and all indicate failure. */ static isc_result_t -validatezonekey(dns_validator_t *val) { +validate_dnskey(dns_validator_t *val) { isc_result_t result; dns_validatorevent_t *event; dns_rdataset_t trdataset; @@ -1807,7 +1817,7 @@ validatezonekey(dns_validator_t *val) { { validator_log(val, ISC_LOG_DEBUG(3), "not beneath secure root"); - return (markanswer(val, "validatezonekey (1)", + return (markanswer(val, "validate_dnskey (1)", "not beneath secure root")); } if (result == DNS_R_PARTIALMATCH || @@ -1900,7 +1910,7 @@ validatezonekey(dns_validator_t *val) { &val->frdataset, &val->fsigrdataset, validator_callback_ds, - "validatezonekey"); + "validate_dnskey"); if (result != ISC_R_SUCCESS) { return (result); } @@ -1909,7 +1919,7 @@ validatezonekey(dns_validator_t *val) { /* * There should never be an unsigned DS. */ - dns_rdataset_disassociate(&val->frdataset); + clear_rdatasets(val); validator_log(val, ISC_LOG_DEBUG(2), "unsigned DS record"); return (DNS_R_NOVALIDSIG); @@ -1926,7 +1936,7 @@ validatezonekey(dns_validator_t *val) { result = create_fetch(val, val->event->name, dns_rdatatype_ds, fetch_callback_ds, - "validatezonekey"); + "validate_dnskey"); if (result != ISC_R_SUCCESS) { return (result); } @@ -1959,7 +1969,7 @@ validatezonekey(dns_validator_t *val) { INSIST(val->dsset != NULL); if (val->dsset->trust < dns_trust_secure) { - return (markanswer(val, "validatezonekey (2)", "insecure DS")); + return (markanswer(val, "validate_dnskey (2)", "insecure DS")); } /* @@ -2043,10 +2053,10 @@ validatezonekey(dns_validator_t *val) { dns_rdataset_clone(val->event->rdataset, &trdataset); /* - * Find matching DNSKEY from DS. + * Find the DNSKEY matching the DS... */ - result = keyfromds(val, &trdataset, &dsrdata, ds.digest_type, - ds.key_tag, ds.algorithm, &keyrdata); + result = dnskey_for_ds(val, &trdataset, &dsrdata, + &ds, &keyrdata); if (result != ISC_R_SUCCESS) { dns_rdataset_disassociate(&trdataset); validator_log(val, ISC_LOG_DEBUG(3), @@ -2055,10 +2065,9 @@ validatezonekey(dns_validator_t *val) { } /* - * Check that this DNSKEY signed the DNSKEY rrset. + * ... and check that it signed the DNSKEY RRset. */ - result = checkkey(val, &keyrdata, ds.key_tag, ds.algorithm); - + result = check_signer(val, &keyrdata, ds.key_tag, ds.algorithm); dns_rdataset_disassociate(&trdataset); if (result == ISC_R_SUCCESS) { break; @@ -2073,7 +2082,7 @@ validatezonekey(dns_validator_t *val) { } else if (result == ISC_R_NOMORE && !supported_algorithm) { validator_log(val, ISC_LOG_DEBUG(3), "no supported algorithm/digest (DS)"); - return (markanswer(val, "validatezonekey (3)", + return (markanswer(val, "validate_dnskey (3)", "no supported algorithm/digest (DS)")); } else { validator_log(val, ISC_LOG_INFO, @@ -2082,31 +2091,10 @@ validatezonekey(dns_validator_t *val) { } } -/*% - * Starts a positive response validation. - * - * Returns: - * \li ISC_R_SUCCESS Validation completed successfully - * \li DNS_R_WAIT Validation has started but is waiting - * for an event. - * \li Other return codes are possible and all indicate failure. - */ -static isc_result_t -start_positive_validation(dns_validator_t *val) { - /* - * If this is not a key, go straight into validate_answer(). - */ - if (val->event->type != dns_rdatatype_dnskey || !isselfsigned(val)) { - return (validate_answer(val, false)); - } - - return (validatezonekey(val)); -} - /*% * val_rdataset_first and val_rdataset_next provide iteration methods - * that hide whether we are iterating across a message or a negative - * cache rdataset. + * that hide whether we are iterating across the AUTHORITY section of + * a message, or a negative cache rdataset. */ static isc_result_t val_rdataset_first(dns_validator_t *val, dns_name_t **namep, @@ -2295,6 +2283,11 @@ checkwildcard(dns_validator_t *val, dns_rdatatype_t type, return (result); } +/* + * Look for the needed proofs for a negative or wildcard response + * from a zone using NSEC3, and set flags in the validator as they + * are found. + */ static isc_result_t findnsec3proofs(dns_validator_t *val) { dns_name_t *name, tname; @@ -2574,7 +2567,7 @@ validate_authority(dns_validator_t *val, bool resume) { } /*% - * Validate the ncache elements. + * Validate negative cache elements. */ static isc_result_t validate_ncache(dns_validator_t *val, bool resume) { @@ -2620,6 +2613,7 @@ validate_ncache(dns_validator_t *val, bool resume) { if (result == ISC_R_NOMORE) { result = ISC_R_SUCCESS; } + return (result); } @@ -2738,7 +2732,8 @@ validate_nx(dns_validator_t *val, bool resume) { * a supported algorithm and digest. */ static bool -check_ds(dns_validator_t *val, dns_name_t *name, dns_rdataset_t *rdataset) { +check_ds_algs(dns_validator_t *val, dns_name_t *name, dns_rdataset_t *rdataset) +{ dns_rdata_t dsrdata = DNS_RDATA_INIT; dns_rdata_ds_t ds; isc_result_t result; @@ -2802,7 +2797,7 @@ seek_ds(dns_validator_t *val, isc_result_t *resp) { * continue walking down labels. */ if (val->frdataset.trust >= dns_trust_secure) { - if (!check_ds(val, tname, &val->frdataset)) { + if (!check_ds_algs(val, tname, &val->frdataset)) { validator_log(val, ISC_LOG_DEBUG(3), "no supported algorithm/" "digest (%s/DS)", @@ -3058,8 +3053,8 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { * an insecure delegation as far as this resolver is concerned. */ if (have_ds && val->frdataset.trust >= dns_trust_secure && - !check_ds(val, dns_fixedname_name(&val->fname), - &val->frdataset)) + !check_ds_algs(val, dns_fixedname_name(&val->fname), + &val->frdataset)) { dns_name_format(dns_fixedname_name(&val->fname), namebuf, sizeof(namebuf)); @@ -3147,7 +3142,11 @@ validator_start(isc_task_t *task, isc_event_t *event) { INSIST(dns_rdataset_isassociated(val->event->rdataset)); INSIST(dns_rdataset_isassociated(val->event->sigrdataset)); - result = start_positive_validation(val); + if (selfsigned_dnskey(val)) { + result = validate_dnskey(val); + } else { + result = validate_answer(val, false); + } if (result == DNS_R_NOVALIDSIG && (val->attributes & VALATTR_TRIEDVERIFY) == 0) { @@ -3163,8 +3162,8 @@ validator_start(isc_task_t *task, isc_event_t *event) { val->event->rdataset->type != 0) { /* - * This is either an unsecure subdomain or a response from - * a broken server. + * This is either an unsecure subdomain or a response + * from a broken server. */ INSIST(dns_rdataset_isassociated(val->event->rdataset)); validator_log(val, ISC_LOG_DEBUG(3), From 3a4334636b5d734037ae5a930d736aef7a502538 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 10 Sep 2019 16:53:28 -0700 Subject: [PATCH 13/15] simplify validate_dnskey and seek_ds - pull out the code that checks whether a key was signed by a trust anchor into a separate function, anchor_signed(). - pull out the code that looks up a DS while validating a zone key into a separate function, get_dsset(). - check in create_validator() whether the sigrdataset is bound, so that we can always pass in &val->fsigrdataset during an insecurity proof; this will allow a reduction of code duplication. --- lib/dns/validator.c | 426 +++++++++++++++++++++++++------------------- 1 file changed, 242 insertions(+), 184 deletions(-) diff --git a/lib/dns/validator.c b/lib/dns/validator.c index 710dcb234d..4aae7bc753 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -1075,8 +1075,13 @@ create_validator(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type, { isc_result_t result; unsigned int vopts = 0; + dns_rdataset_t *sig = NULL; - if (check_deadlock(val, name, type, rdataset, sigrdataset)) { + if (sigrdataset != NULL && dns_rdataset_isassociated(sigrdataset)) { + sig = sigrdataset; + } + + if (check_deadlock(val, name, type, rdataset, sig)) { validator_log(val, ISC_LOG_DEBUG(3), "deadlock found (create_validator)"); return (DNS_R_NOVALIDSIG); @@ -1088,8 +1093,8 @@ create_validator(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type, validator_logcreate(val, name, type, caller, "validator"); result = dns_validator_create(val->view, name, type, - rdataset, sigrdataset, NULL, vopts, - val->task, action, val, + rdataset, sig, NULL, + vopts, val->task, action, val, &val->subvalidator); if (result == ISC_R_SUCCESS) { val->subvalidator->parent = val; @@ -1750,6 +1755,201 @@ dnskey_for_ds(dns_validator_t *val, dns_rdataset_t *rdataset, return (result); } +static isc_result_t +anchor_signed(dns_validator_t *val, isc_result_t *resp) { + isc_result_t result; + bool atsep = false; + + /* + * First, see if this key was signed by a trust anchor. + */ + for (result = dns_rdataset_first(val->event->sigrdataset); + result == ISC_R_SUCCESS; + result = dns_rdataset_next(val->event->sigrdataset)) + { + dns_keynode_t *keynode = NULL; + dns_rdata_t sigrdata = DNS_RDATA_INIT; + dns_fixedname_t fixed; + dns_name_t *found; + dst_key_t *dstkey; + dns_rdata_rrsig_t sig; + + found = dns_fixedname_initname(&fixed); + dns_rdata_reset(&sigrdata); + dns_rdataset_current(val->event->sigrdataset, &sigrdata); + result = dns_rdata_tostruct(&sigrdata, &sig, NULL); + RUNTIME_CHECK(result == ISC_R_SUCCESS); + + if (!dns_name_equal(val->event->name, &sig.signer)) { + continue; + } + + result = dns_keytable_findkeynode(val->keytable, + val->event->name, + sig.algorithm, sig.keyid, + &keynode); + if (result == ISC_R_NOTFOUND) { + result = dns_keytable_finddeepestmatch(val->keytable, + val->event->name, + found); + if (result != ISC_R_SUCCESS) { + validator_log(val, ISC_LOG_DEBUG(3), + "not beneath secure root"); + *resp = markanswer(val, "validate_dnskey (1)", + "not beneath secure root"); + return (ISC_R_COMPLETE); + } + continue; + } + + if (result == DNS_R_PARTIALMATCH || result == ISC_R_SUCCESS) { + atsep = true; + } + + while (result == ISC_R_SUCCESS) { + dns_keynode_t *nextnode = NULL; + dstkey = dns_keynode_key(keynode); + if (dstkey == NULL) { + dns_keytable_detachkeynode(val->keytable, + &keynode); + break; + } + + result = verify(val, dstkey, &sigrdata, sig.keyid); + if (result == ISC_R_SUCCESS) { + dns_keytable_detachkeynode(val->keytable, + &keynode); + break; + } + + result = dns_keytable_findnextkeynode(val->keytable, + keynode, + &nextnode); + dns_keytable_detachkeynode(val->keytable, &keynode); + keynode = nextnode; + } + + if (result == ISC_R_SUCCESS) { + marksecure(val->event); + validator_log(val, ISC_LOG_DEBUG(3), + "signed by trusted key; " + "marking as secure"); + *resp = result; + return (ISC_R_COMPLETE); + } + } + + if (atsep) { + /* + * We have not found a key to verify this DNSKEY + * RRset, but there is a trust anchor defined for this + * name, so we have to assume that the RRset is invalid. + */ + char namebuf[DNS_NAME_FORMATSIZE]; + + dns_name_format(val->event->name, namebuf, sizeof(namebuf)); + validator_log(val, ISC_LOG_NOTICE, + "unable to find a DNSKEY which verifies " + "the DNSKEY RRset and also matches a " + "trusted key for '%s'", namebuf); + *resp = DNS_R_NOVALIDKEY; + return (ISC_R_COMPLETE); + } + + return (result); +} + +/* + * get_dsset is called to look up a DS RRset corresponding to the name + * of a DNSKEY record, either in the cache or, if necessary, by starting a + * fetch. This is done in the context of validating a zone key to build a + * trust chain. + * + * Returns: + * \li ISC_R_COMPLETE a DS has not been found; the caller should + * stop trying to validate the zone key and + * return the result code in '*resp'. + * \li DNS_R_CONTINUE a DS has been found and the caller may + * continue the zone key validation. + */ +static isc_result_t +get_dsset(dns_validator_t *val, dns_name_t *tname, isc_result_t *resp) { + isc_result_t result; + + result = view_find(val, tname, dns_rdatatype_ds); + switch (result) { + case ISC_R_SUCCESS: + /* + * We have a DS RRset. + */ + val->dsset = &val->frdataset; + if ((DNS_TRUST_PENDING(val->frdataset.trust) || + DNS_TRUST_ANSWER(val->frdataset.trust)) && + dns_rdataset_isassociated(&val->fsigrdataset)) + { + /* + * ... which is signed but not yet validated. + */ + result = create_validator(val, tname, + dns_rdatatype_ds, + &val->frdataset, + &val->fsigrdataset, + validator_callback_ds, + "validate_dnskey"); + *resp = DNS_R_WAIT; + if (result != ISC_R_SUCCESS) { + *resp = result; + } + return (ISC_R_COMPLETE); + } else if (DNS_TRUST_PENDING(val->frdataset.trust)) { + /* + * There should never be an unsigned DS. + */ + disassociate_rdatasets(val); + validator_log(val, ISC_LOG_DEBUG(2), + "unsigned DS record"); + *resp = DNS_R_NOVALIDSIG; + return (ISC_R_COMPLETE); + } + break; + + case ISC_R_NOTFOUND: + /* + * We don't have the DS. Find it. + */ + result = create_fetch(val, tname, dns_rdatatype_ds, + fetch_callback_ds, "validate_dnskey"); + *resp = DNS_R_WAIT; + if (result != ISC_R_SUCCESS) { + *resp = result; + } + return (ISC_R_COMPLETE); + + case DNS_R_NCACHENXDOMAIN: + case DNS_R_NCACHENXRRSET: + case DNS_R_EMPTYNAME: + case DNS_R_NXDOMAIN: + case DNS_R_NXRRSET: + case DNS_R_CNAME: + /* + * The DS does not exist. + */ + disassociate_rdatasets(val); + validator_log(val, ISC_LOG_DEBUG(2), "no DS record"); + *resp = DNS_R_NOVALIDSIG; + return (ISC_R_COMPLETE); + + case DNS_R_BROKENCHAIN: + *resp = result; + return (ISC_R_COMPLETE); + + default: + break; + } + + return (DNS_R_CONTINUE); +} + /*% * Attempts positive response validation of an RRset containing zone keys * (i.e. a DNSKEY rrset). @@ -1763,122 +1963,34 @@ dnskey_for_ds(dns_validator_t *val, dns_rdataset_t *rdataset, static isc_result_t validate_dnskey(dns_validator_t *val) { isc_result_t result; - dns_validatorevent_t *event; dns_rdataset_t trdataset; dns_rdata_t dsrdata = DNS_RDATA_INIT; dns_rdata_t keyrdata = DNS_RDATA_INIT; - dns_rdata_t sigrdata = DNS_RDATA_INIT; - char namebuf[DNS_NAME_FORMATSIZE]; dns_rdata_ds_t ds; - dns_rdata_rrsig_t sig; - dst_key_t *dstkey; bool supported_algorithm; - bool atsep = false; char digest_types[256]; /* * Caller must be holding the validator lock. */ - event = val->event; - if (val->dsset == NULL) { + isc_result_t tresult = ISC_R_SUCCESS; + /* - * First, see if this key was signed by a trusted key. + * First, check whether the key to be validated was + * signed by a trust anchor. */ - for (result = dns_rdataset_first(val->event->sigrdataset); - result == ISC_R_SUCCESS; - result = dns_rdataset_next(val->event->sigrdataset)) - { - dns_keynode_t *keynode = NULL; - dns_fixedname_t fixed; - dns_name_t *found; - - found = dns_fixedname_initname(&fixed); - dns_rdata_reset(&sigrdata); - dns_rdataset_current(val->event->sigrdataset, - &sigrdata); - result = dns_rdata_tostruct(&sigrdata, &sig, NULL); - RUNTIME_CHECK(result == ISC_R_SUCCESS); - - if (!dns_name_equal(val->event->name, &sig.signer)) { - continue; - } - - result = dns_keytable_findkeynode(val->keytable, - val->event->name, - sig.algorithm, - sig.keyid, &keynode); - if (result == ISC_R_NOTFOUND && - dns_keytable_finddeepestmatch(val->keytable, - val->event->name, - found) - != ISC_R_SUCCESS) - { - validator_log(val, ISC_LOG_DEBUG(3), - "not beneath secure root"); - return (markanswer(val, "validate_dnskey (1)", - "not beneath secure root")); - } - if (result == DNS_R_PARTIALMATCH || - result == ISC_R_SUCCESS) - { - atsep = true; - } - while (result == ISC_R_SUCCESS) { - dns_keynode_t *nextnode = NULL; - dstkey = dns_keynode_key(keynode); - if (dstkey == NULL) { - dns_keytable_detachkeynode( - val->keytable, - &keynode); - break; - } - result = verify(val, dstkey, &sigrdata, - sig.keyid); - if (result == ISC_R_SUCCESS) { - dns_keytable_detachkeynode( - val->keytable, - &keynode); - break; - } - result = dns_keytable_findnextkeynode( - val->keytable, - keynode, - &nextnode); - dns_keytable_detachkeynode(val->keytable, - &keynode); - keynode = nextnode; - } - if (result == ISC_R_SUCCESS) { - marksecure(event); - validator_log(val, ISC_LOG_DEBUG(3), - "signed by trusted key; " - "marking as secure"); - return (result); - } - } - - if (atsep) { - /* - * We have not found a key to verify this DNSKEY - * RRset. As this is a SEP we have to assume that - * the RRset is invalid. - */ - dns_name_format(val->event->name, namebuf, - sizeof(namebuf)); - validator_log(val, ISC_LOG_NOTICE, - "unable to find a DNSKEY which verifies " - "the DNSKEY RRset and also matches a " - "trusted key for '%s'", namebuf); - return (DNS_R_NOVALIDKEY); + result = anchor_signed(val, &tresult); + if (result == ISC_R_COMPLETE) { + return (tresult); } /* * If this is the root name and there was no trusted key, - * give up, since there's no DS at the root. + * we can give up now, since there's no DS at the root. */ - if (dns_name_equal(event->name, dns_rootname)) { + if (dns_name_equal(val->event->name, dns_rootname)) { if ((val->attributes & VALATTR_TRIEDVERIFY) != 0) { validator_log(val, ISC_LOG_DEBUG(3), "root key failed to validate"); @@ -1891,75 +2003,11 @@ validate_dnskey(dns_validator_t *val) { } /* - * Otherwise, try to find the DS record. + * Otherwise, look up the DS RRset for this name. */ - result = view_find(val, val->event->name, dns_rdatatype_ds); - switch (result) { - case ISC_R_SUCCESS: - /* - * We have DS records. - */ - val->dsset = &val->frdataset; - if ((DNS_TRUST_PENDING(val->frdataset.trust) || - DNS_TRUST_ANSWER(val->frdataset.trust)) && - dns_rdataset_isassociated(&val->fsigrdataset)) - { - result = create_validator(val, - val->event->name, - dns_rdatatype_ds, - &val->frdataset, - &val->fsigrdataset, - validator_callback_ds, - "validate_dnskey"); - if (result != ISC_R_SUCCESS) { - return (result); - } - return (DNS_R_WAIT); - } else if (DNS_TRUST_PENDING(val->frdataset.trust)) { - /* - * There should never be an unsigned DS. - */ - clear_rdatasets(val); - validator_log(val, ISC_LOG_DEBUG(2), - "unsigned DS record"); - return (DNS_R_NOVALIDSIG); - } else { - result = ISC_R_SUCCESS; - POST(result); - } - break; - - case ISC_R_NOTFOUND: - /* - * We don't have the DS. Find it. - */ - result = create_fetch(val, val->event->name, - dns_rdatatype_ds, - fetch_callback_ds, - "validate_dnskey"); - if (result != ISC_R_SUCCESS) { - return (result); - } - return (DNS_R_WAIT); - - case DNS_R_NCACHENXDOMAIN: - case DNS_R_NCACHENXRRSET: - case DNS_R_EMPTYNAME: - case DNS_R_NXDOMAIN: - case DNS_R_NXRRSET: - case DNS_R_CNAME: - /* - * The DS does not exist. - */ - disassociate_rdatasets(val); - validator_log(val, ISC_LOG_DEBUG(2), "no DS record"); - return (DNS_R_NOVALIDSIG); - - case DNS_R_BROKENCHAIN: - return (result); - - default: - break; + result = get_dsset(val, val->event->name, &tresult); + if (result == ISC_R_COMPLETE) { + return (tresult); } } @@ -2076,7 +2124,7 @@ validate_dnskey(dns_validator_t *val) { "no RRSIG matching DS key"); } if (result == ISC_R_SUCCESS) { - marksecure(event); + marksecure(val->event); validator_log(val, ISC_LOG_DEBUG(3), "marking as secure (DS)"); return (result); } else if (result == ISC_R_NOMORE && !supported_algorithm) { @@ -2760,8 +2808,10 @@ check_ds_algs(dns_validator_t *val, dns_name_t *name, dns_rdataset_t *rdataset) } /*% - * seek_ds looks for DS rrsets at the label indicated by val->labels, - * for an insecurity proof. + * seek_ds is called to look up DS rrsets at the label of val->event->name + * indicated by val->labels. This is done while building an insecurity + * proof, and so it will attempt validation of NXDOMAIN, NXRRSET or CNAME + * responses. * * Returns: * \li ISC_R_COMPLETE a result has been determined and copied @@ -2793,8 +2843,8 @@ seek_ds(dns_validator_t *val, isc_result_t *resp) { switch (result) { case ISC_R_SUCCESS: /* - * There is a DS here. Verify that it's secure and - * continue walking down labels. + * There is a DS here. If it's already been + * validated, continue walking down labels. */ if (val->frdataset.trust >= dns_trust_secure) { if (!check_ds_algs(val, tname, &val->frdataset)) { @@ -2802,7 +2852,8 @@ seek_ds(dns_validator_t *val, isc_result_t *resp) { "no supported algorithm/" "digest (%s/DS)", namebuf); - *resp = markanswer(val, "proveunsecure (5)", + *resp = markanswer(val, + "proveunsecure (5)", "no supported " "algorithm/digest (DS)"); return (ISC_R_COMPLETE); @@ -2811,13 +2862,10 @@ seek_ds(dns_validator_t *val, isc_result_t *resp) { break; } - if (!dns_rdataset_isassociated(&val->fsigrdataset)) { - validator_log(val, ISC_LOG_DEBUG(3), "DS is unsigned"); - *resp = DNS_R_NOVALIDSIG; - } else { - /* - * Validate / re-validate answer. - */ + /* + * Otherwise, try to validate it now. + */ + if (dns_rdataset_isassociated(&val->fsigrdataset)) { result = create_validator(val, tname, dns_rdatatype_ds, &val->frdataset, @@ -2828,6 +2876,13 @@ seek_ds(dns_validator_t *val, isc_result_t *resp) { if (result != ISC_R_SUCCESS) { *resp = result; } + } else { + /* + * There should never be an unsigned DS. + */ + validator_log(val, ISC_LOG_DEBUG(3), + "unsigned DS record"); + *resp = DNS_R_NOVALIDSIG; } return (ISC_R_COMPLETE); @@ -2858,7 +2913,8 @@ seek_ds(dns_validator_t *val, isc_result_t *resp) { { result = create_validator(val, tname, dns_rdatatype_ds, - &val->frdataset, NULL, + &val->frdataset, + &val->fsigrdataset, validator_callback_ds, "proveunsecure"); *resp = DNS_R_WAIT; @@ -2931,7 +2987,8 @@ seek_ds(dns_validator_t *val, isc_result_t *resp) { *resp = DNS_R_WAIT; result = create_validator(val, tname, dns_rdatatype_ds, - &val->frdataset, NULL, + &val->frdataset, + &val->fsigrdataset, validator_callback_ds, "proveunsecure"); if (result != ISC_R_SUCCESS) { @@ -2961,7 +3018,8 @@ seek_ds(dns_validator_t *val, isc_result_t *resp) { { result = create_validator(val, tname, dns_rdatatype_cname, - &val->frdataset, NULL, + &val->frdataset, + &val->fsigrdataset, validator_callback_cname, "proveunsecure " "(cname)"); From 692c879e3cc06294fbc51997e669eebf38d40486 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Sun, 15 Sep 2019 16:36:16 -0700 Subject: [PATCH 14/15] remove unneeded members from dns_validator - val->keynode and val->seensig were set but never used. - val->nearest, val->soaset, val->soaname, val->nsecset and val->nsec3set were never used at all. --- lib/dns/include/dns/validator.h | 7 ---- lib/dns/keytable.c | 4 +- lib/dns/nta.c | 3 +- lib/dns/validator.c | 71 +++++++++------------------------ 4 files changed, 22 insertions(+), 63 deletions(-) diff --git a/lib/dns/include/dns/validator.h b/lib/dns/include/dns/validator.h index 51c62239d1..0057e1d5b5 100644 --- a/lib/dns/include/dns/validator.h +++ b/lib/dns/include/dns/validator.h @@ -128,7 +128,6 @@ struct dns_validator { dns_validator_t * subvalidator; dns_validator_t * parent; dns_keytable_t * keytable; - dns_keynode_t * keynode; dst_key_t * key; dns_rdata_rrsig_t * siginfo; isc_task_t * task; @@ -136,18 +135,12 @@ struct dns_validator { void * arg; unsigned int labels; dns_rdataset_t * currentset; - bool seensig; dns_rdataset_t * keyset; dns_rdataset_t * dsset; - dns_rdataset_t * soaset; - dns_rdataset_t * nsecset; - dns_rdataset_t * nsec3set; - dns_name_t * soaname; dns_rdataset_t frdataset; dns_rdataset_t fsigrdataset; dns_fixedname_t fname; dns_fixedname_t wild; - dns_fixedname_t nearest; dns_fixedname_t closest; ISC_LINK(dns_validator_t) link; bool mustbesecure; diff --git a/lib/dns/keytable.c b/lib/dns/keytable.c index 73e6cec89b..a154699d45 100644 --- a/lib/dns/keytable.c +++ b/lib/dns/keytable.c @@ -47,8 +47,8 @@ struct dns_keynode { unsigned int magic; isc_refcount_t refcount; dst_key_t * key; - bool managed; - bool initial; + bool managed; + bool initial; struct dns_keynode * next; }; diff --git a/lib/dns/nta.c b/lib/dns/nta.c index 8ea495cde5..3058ffcaa8 100644 --- a/lib/dns/nta.c +++ b/lib/dns/nta.c @@ -320,8 +320,7 @@ nta_create(dns_ntatable_t *ntatable, const dns_name_t *name, isc_result_t dns_ntatable_add(dns_ntatable_t *ntatable, const dns_name_t *name, - bool force, isc_stdtime_t now, - uint32_t lifetime) + bool force, isc_stdtime_t now, uint32_t lifetime) { isc_result_t result; dns_nta_t *nta = NULL; diff --git a/lib/dns/validator.c b/lib/dns/validator.c index 4aae7bc753..11bafb567e 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -866,10 +866,6 @@ validator_callback_nsec(isc_task_t *task, isc_event_t *event) { dns_name_t **proofs = val->event->proofs; dns_name_t *wild = dns_fixedname_name(&val->wild); - if (rdataset->trust == dns_trust_secure) { - val->seensig = true; - } - if (rdataset->type == dns_rdatatype_nsec && rdataset->trust == dns_trust_secure && (NEEDNODATA(val) || NEEDNOQNAME(val)) && @@ -1104,16 +1100,16 @@ create_validator(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type, } /*% - * 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. + * Try to find a key that could have signed val->siginfo among those in + * 'rdataset'. If found, build a dst_key_t for it and point val->key at + * it. * - * If val->key is already non-NULL, locate it in the rdataset and - * then search past it for the *next* key that could have signed - * 'siginfo', then set val->key to that. + * If val->key is already non-NULL, locate it in the rdataset and then + * search past it for the *next* key that could have signed 'siginfo', then + * set val->key to that. * - * Returns ISC_R_SUCCESS if a possible matching key has been - * found, ISC_R_NOTFOUND if not. Any other value indicates error. + * Returns ISC_R_SUCCESS if a possible matching key has been found, + * ISC_R_NOTFOUND if not. Any other value indicates error. */ static isc_result_t select_signing_key(dns_validator_t *val, dns_rdataset_t *rdataset) { @@ -1577,35 +1573,16 @@ validate_answer(dns_validator_t *val, bool resume) { } do { + isc_result_t tresult; vresult = verify(val, val->key, &rdata, val->siginfo->keyid); if (vresult == ISC_R_SUCCESS) { break; } - if (val->keynode != NULL) { - dns_keynode_t *nextnode = NULL; - result = dns_keytable_findnextkeynode( - val->keytable, - val->keynode, - &nextnode); - dns_keytable_detachkeynode(val->keytable, - &val->keynode); - val->keynode = nextnode; - if (result != ISC_R_SUCCESS) { - val->key = NULL; - break; - } - val->key = dns_keynode_key(val->keynode); - if (val->key == NULL) { - break; - } - } else { - isc_result_t tresult; - tresult = select_signing_key(val, val->keyset); - if (tresult != ISC_R_SUCCESS) { - break; - } + tresult = select_signing_key(val, val->keyset); + if (tresult != ISC_R_SUCCESS) { + break; } } while (1); if (vresult != ISC_R_SUCCESS) { @@ -1618,17 +1595,12 @@ validate_answer(dns_validator_t *val, bool resume) { val->view->acceptexpired); } - if (val->keynode != NULL) { - dns_keytable_detachkeynode(val->keytable, - &val->keynode); - } else { - if (val->key != NULL) { - dst_key_free(&val->key); - } - if (val->keyset != NULL) { - dns_rdataset_disassociate(val->keyset); - val->keyset = NULL; - } + if (val->key != NULL) { + dst_key_free(&val->key); + } + if (val->keyset != NULL) { + dns_rdataset_disassociate(val->keyset); + val->keyset = NULL; } val->key = NULL; if (NEEDNOQNAME(val)) { @@ -3321,7 +3293,6 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type, if (result != ISC_R_SUCCESS) { goto cleanup; } - val->keynode = NULL; val->key = NULL; val->siginfo = NULL; val->task = task; @@ -3331,7 +3302,6 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type, val->currentset = NULL; val->keyset = NULL; val->dsset = NULL; - val->seensig = false; val->depth = 0; val->authcount = 0; val->authfail = 0; @@ -3339,7 +3309,6 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type, dns_rdataset_init(&val->frdataset); dns_rdataset_init(&val->fsigrdataset); dns_fixedname_init(&val->wild); - dns_fixedname_init(&val->nearest); dns_fixedname_init(&val->closest); isc_stdtime_get(&val->start); ISC_LINK_INIT(val, link); @@ -3422,9 +3391,7 @@ destroy(dns_validator_t *val) { REQUIRE(val->event == NULL); REQUIRE(val->fetch == NULL); - if (val->keynode != NULL) { - dns_keytable_detachkeynode(val->keytable, &val->keynode); - } else if (val->key != NULL) { + if (val->key != NULL) { dst_key_free(&val->key); } if (val->keytable != NULL) { From 11cd9d86e4fcf5d4ed65472b322e4bf45d48b11b Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 6 Nov 2019 21:23:07 -0800 Subject: [PATCH 15/15] CHANGES, release note --- CHANGES | 4 ++++ doc/arm/notes-9.15.6.xml | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/CHANGES b/CHANGES index 49802fcf80..61cdf9490c 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5318. [cleanup] The DNSSEC validation code has been refactored + for clarity and to reduce code duplication. + [GL #622] + 5317. [func] A new asynchronous network communications system based on libuv is now used for listening for incoming requests and responding to them. (The diff --git a/doc/arm/notes-9.15.6.xml b/doc/arm/notes-9.15.6.xml index c368fdae41..fca186f62a 100644 --- a/doc/arm/notes-9.15.6.xml +++ b/doc/arm/notes-9.15.6.xml @@ -46,6 +46,12 @@ in the future releases. [GL #1265] + + + The DNSSEC validation code has been refactored for clarity and to + reduce code duplication. [GL #622] + +