From b4715a34a07fa9db476a58c5530eeae6e0f3df56 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 15 Feb 2023 11:32:00 -0800 Subject: [PATCH 1/4] additional refactoring of dns_validator refactor validator so that the validation status object (previously called dns_valstatus_t, which was derived from dns_validatorevent_t), is now part of the dns_validator object. when calling validator callbacks, the validator itself is now sent as the argument. (note: this necessitates caution in the callback functions that are internal to validator.c validators spawn other validators, and it can be confusing at times whether we need to be looking at val, val->subvalidator, or val->parent.) --- lib/dns/include/dns/validator.h | 117 +++++------ lib/dns/resolver.c | 110 +++++----- lib/dns/validator.c | 355 ++++++++++++++------------------ 3 files changed, 257 insertions(+), 325 deletions(-) diff --git a/lib/dns/include/dns/validator.h b/lib/dns/include/dns/validator.h index c00e60bf78..9400c216d6 100644 --- a/lib/dns/include/dns/validator.h +++ b/lib/dns/include/dns/validator.h @@ -58,31 +58,62 @@ #include +#define DNS_VALIDATOR_NOQNAMEPROOF 0 +#define DNS_VALIDATOR_NODATAPROOF 1 +#define DNS_VALIDATOR_NOWILDCARDPROOF 2 +#define DNS_VALIDATOR_CLOSESTENCLOSER 3 + /*% - * A dns_valstatus_t is sent when a 'validation' completes. + * A validator object represents a validation in progress. * \brief - * 'name', 'rdataset', 'sigrdataset', and 'message' are the values that were - * supplied when dns_validator_create() was called. They are returned to the - * caller so that they may be freed. - * - * If the RESULT is ISC_R_SUCCESS and the answer is secure then - * proofs[] will contain the names of the NSEC records that hold the - * various proofs. Note the same name may appear multiple times. - * - * The structure is freed by dns_validator_destroy(). + * Clients are strongly discouraged from using this type directly, with + * the exception of the 'link' field, which may be used directly for + * whatever purpose the client desires. */ -typedef struct dns_valstatus { - dns_validator_t *validator; - isc_result_t result; +struct dns_validator { + /* Unlocked. */ + unsigned int magic; + isc_mutex_t lock; + dns_view_t *view; - isc_mem_t *mctx; - - /* - * Name and type of the response to be validated. - */ - dns_fixedname_t fname; + /* Name and type of the response to be validated. */ dns_name_t *name; dns_rdatatype_t type; + + /* Locked by lock. */ + unsigned int options; + unsigned int attributes; + dns_fetch_t *fetch; + dns_validator_t *subvalidator; + dns_validator_t *parent; + dns_keytable_t *keytable; + dst_key_t *key; + dns_rdata_rrsig_t *siginfo; + isc_loop_t *loop; + isc_job_cb cb; + void *arg; + unsigned int labels; + dns_rdataset_t *nxset; + dns_rdataset_t *keyset; + dns_rdataset_t *dsset; + dns_rdataset_t fdsset; + dns_rdataset_t frdataset; + dns_rdataset_t fsigrdataset; + dns_fixedname_t fname; + dns_fixedname_t wild; + dns_fixedname_t closest; + ISC_LINK(dns_validator_t) link; + bool mustbesecure; + unsigned int depth; + unsigned int authcount; + unsigned int authfail; + isc_stdtime_t start; + + /* + * Results of a completed validation. + */ + isc_result_t result; + /* * Rdata and RRSIG (if any) for positive responses. */ @@ -105,54 +136,6 @@ typedef struct dns_valstatus { * Answer is secure. */ bool secure; -} dns_valstatus_t; - -#define DNS_VALIDATOR_NOQNAMEPROOF 0 -#define DNS_VALIDATOR_NODATAPROOF 1 -#define DNS_VALIDATOR_NOWILDCARDPROOF 2 -#define DNS_VALIDATOR_CLOSESTENCLOSER 3 - -/*% - * A validator object represents a validation in progress. - * \brief - * Clients are strongly discouraged from using this type directly, with - * the exception of the 'link' field, which may be used directly for - * whatever purpose the client desires. - */ -struct dns_validator { - /* Unlocked. */ - unsigned int magic; - isc_mutex_t lock; - dns_view_t *view; - /* Locked by lock. */ - unsigned int options; - unsigned int attributes; - dns_valstatus_t *vstat; - dns_fetch_t *fetch; - dns_validator_t *subvalidator; - dns_validator_t *parent; - dns_keytable_t *keytable; - dst_key_t *key; - dns_rdata_rrsig_t *siginfo; - isc_loop_t *loop; - isc_job_cb cb; - void *arg; - unsigned int labels; - dns_rdataset_t *currentset; - dns_rdataset_t *keyset; - dns_rdataset_t *dsset; - dns_rdataset_t fdsset; - dns_rdataset_t frdataset; - dns_rdataset_t fsigrdataset; - dns_fixedname_t fname; - dns_fixedname_t wild; - dns_fixedname_t closest; - ISC_LINK(dns_validator_t) link; - bool mustbesecure; - unsigned int depth; - unsigned int authcount; - unsigned int authfail; - isc_stdtime_t start; }; /*% diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index a87d3d0341..ed31dd9f53 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -5312,8 +5312,7 @@ has_000_label(dns_rdataset_t *nsecset) { */ static void validated(void *arg) { - dns_valstatus_t *vstat = (dns_valstatus_t *)arg; - dns_validator_t *val = vstat->validator; + dns_validator_t *val = (dns_validator_t *)arg; dns_adbaddrinfo_t *addrinfo = NULL; dns_dbnode_t *node = NULL; dns_dbnode_t *nsnode = NULL; @@ -5354,8 +5353,8 @@ validated(void *arg) { res = fctx->res; addrinfo = valarg->addrinfo; - message = vstat->message; - fctx->vresult = vstat->result; + message = val->message; + fctx->vresult = val->result; LOCK(&fctx->lock); ISC_LIST_UNLINK(fctx->validators, val, link); @@ -5366,14 +5365,14 @@ validated(void *arg) { * Destroy the validator early so that we can * destroy the fctx if necessary. Save the wildcard name. */ - if (vstat->proofs[DNS_VALIDATOR_NOQNAMEPROOF] != NULL) { + if (val->proofs[DNS_VALIDATOR_NOQNAMEPROOF] != NULL) { wild = dns_fixedname_initname(&fwild); dns_name_copy(dns_fixedname_name(&val->wild), wild); } isc_mem_put(fctx->mctx, valarg, sizeof(*valarg)); - negative = (vstat->rdataset == NULL); + negative = (val->rdataset == NULL); LOCK(&fctx->lock); sentresponse = ((fctx->options & DNS_FETCHOPT_NOVALIDATE) != 0); @@ -5394,13 +5393,13 @@ validated(void *arg) { * If chaining, we need to make sure that the right result code * is returned, and that the rdatasets are bound. */ - if (vstat->result == ISC_R_SUCCESS && !negative && - vstat->rdataset != NULL && CHAINING(vstat->rdataset)) + if (val->result == ISC_R_SUCCESS && !negative && + val->rdataset != NULL && CHAINING(val->rdataset)) { - if (vstat->rdataset->type == dns_rdatatype_cname) { + if (val->rdataset->type == dns_rdatatype_cname) { eresult = DNS_R_CNAME; } else { - INSIST(vstat->rdataset->type == dns_rdatatype_dname); + INSIST(val->rdataset->type == dns_rdatatype_dname); eresult = DNS_R_DNAME; } chaining = true; @@ -5431,28 +5430,26 @@ validated(void *arg) { } } - if (vstat->result != ISC_R_SUCCESS) { + if (val->result != ISC_R_SUCCESS) { FCTXTRACE("validation failed"); inc_stats(res, dns_resstatscounter_valfail); fctx->valfail++; - fctx->vresult = vstat->result; + fctx->vresult = val->result; if (fctx->vresult != DNS_R_BROKENCHAIN) { result = ISC_R_NOTFOUND; - if (vstat->rdataset != NULL) { - result = dns_db_findnode( - fctx->cache, vstat->name, true, &node); + if (val->rdataset != NULL) { + result = dns_db_findnode(fctx->cache, val->name, + true, &node); } if (result == ISC_R_SUCCESS) { (void)dns_db_deleterdataset(fctx->cache, node, - NULL, vstat->type, - 0); + NULL, val->type, 0); } - if (result == ISC_R_SUCCESS && - vstat->sigrdataset != NULL) + if (result == ISC_R_SUCCESS && val->sigrdataset != NULL) { (void)dns_db_deleterdataset( fctx->cache, node, NULL, - dns_rdatatype_rrsig, vstat->type); + dns_rdatatype_rrsig, val->type); } if (result == ISC_R_SUCCESS) { dns_db_detachnode(fctx->cache, &node); @@ -5464,21 +5461,20 @@ validated(void *arg) { * validation. */ result = ISC_R_NOTFOUND; - if (vstat->rdataset != NULL) { - result = dns_db_findnode( - fctx->cache, vstat->name, true, &node); + if (val->rdataset != NULL) { + result = dns_db_findnode(fctx->cache, val->name, + true, &node); } if (result == ISC_R_SUCCESS) { (void)dns_db_addrdataset( fctx->cache, node, NULL, now, - vstat->rdataset, 0, NULL); + val->rdataset, 0, NULL); } - if (result == ISC_R_SUCCESS && - vstat->sigrdataset != NULL) + if (result == ISC_R_SUCCESS && val->sigrdataset != NULL) { (void)dns_db_addrdataset( fctx->cache, node, NULL, now, - vstat->sigrdataset, 0, NULL); + val->sigrdataset, 0, NULL); } if (result == ISC_R_SUCCESS) { dns_db_detachnode(fctx->cache, &node); @@ -5539,7 +5535,7 @@ validated(void *arg) { covers = fctx->type; } - result = dns_db_findnode(fctx->cache, vstat->name, true, &node); + result = dns_db_findnode(fctx->cache, val->name, true, &node); if (result != ISC_R_SUCCESS) { /* fctx->lock unlocked in noanswer_response */ goto noanswer_response; @@ -5559,7 +5555,7 @@ validated(void *arg) { result = ncache_adderesult(message, fctx->cache, node, covers, now, fctx->res->view->minncachettl, - ttl, vstat->optout, vstat->secure, + ttl, val->optout, val->secure, ardataset, &eresult); if (result != ISC_R_SUCCESS) { goto noanswer_response; @@ -5571,28 +5567,27 @@ validated(void *arg) { FCTXTRACE("validation OK"); - if (vstat->proofs[DNS_VALIDATOR_NOQNAMEPROOF] != NULL) { + if (val->proofs[DNS_VALIDATOR_NOQNAMEPROOF] != NULL) { result = dns_rdataset_addnoqname( - vstat->rdataset, - vstat->proofs[DNS_VALIDATOR_NOQNAMEPROOF]); + val->rdataset, val->proofs[DNS_VALIDATOR_NOQNAMEPROOF]); RUNTIME_CHECK(result == ISC_R_SUCCESS); - INSIST(vstat->sigrdataset != NULL); - vstat->sigrdataset->ttl = vstat->rdataset->ttl; - if (vstat->proofs[DNS_VALIDATOR_CLOSESTENCLOSER] != NULL) { + INSIST(val->sigrdataset != NULL); + val->sigrdataset->ttl = val->rdataset->ttl; + if (val->proofs[DNS_VALIDATOR_CLOSESTENCLOSER] != NULL) { result = dns_rdataset_addclosest( - vstat->rdataset, - vstat->proofs[DNS_VALIDATOR_CLOSESTENCLOSER]); + val->rdataset, + val->proofs[DNS_VALIDATOR_CLOSESTENCLOSER]); RUNTIME_CHECK(result == ISC_R_SUCCESS); } - } else if (vstat->rdataset->trust == dns_trust_answer && - vstat->rdataset->type != dns_rdatatype_rrsig) + } else if (val->rdataset->trust == dns_trust_answer && + val->rdataset->type != dns_rdatatype_rrsig) { isc_result_t tresult; dns_name_t *noqname = NULL; - tresult = findnoqname(fctx, message, vstat->name, - vstat->rdataset->type, &noqname); + tresult = findnoqname(fctx, message, val->name, + val->rdataset->type, &noqname); if (tresult == ISC_R_SUCCESS && noqname != NULL) { - tresult = dns_rdataset_addnoqname(vstat->rdataset, + tresult = dns_rdataset_addnoqname(val->rdataset, noqname); RUNTIME_CHECK(tresult == ISC_R_SUCCESS); } @@ -5604,7 +5599,7 @@ validated(void *arg) { * rdatasets to the first event on the fetch * event list. */ - result = dns_db_findnode(fctx->cache, vstat->name, true, &node); + result = dns_db_findnode(fctx->cache, val->name, true, &node); if (result != ISC_R_SUCCESS) { goto noanswer_response; } @@ -5613,8 +5608,8 @@ validated(void *arg) { if ((fctx->options & DNS_FETCHOPT_PREFETCH) != 0) { options = DNS_DBADD_PREFETCH; } - result = dns_db_addrdataset(fctx->cache, node, NULL, now, - vstat->rdataset, options, ardataset); + result = dns_db_addrdataset(fctx->cache, node, NULL, now, val->rdataset, + options, ardataset); if (result != ISC_R_SUCCESS && result != DNS_R_UNCHANGED) { goto noanswer_response; } @@ -5624,9 +5619,9 @@ validated(void *arg) { } else { eresult = DNS_R_NCACHENXRRSET; } - } else if (vstat->sigrdataset != NULL) { + } else if (val->sigrdataset != NULL) { result = dns_db_addrdataset(fctx->cache, node, NULL, now, - vstat->sigrdataset, options, + val->sigrdataset, options, asigrdataset); if (result != ISC_R_SUCCESS && result != DNS_R_UNCHANGED) { goto noanswer_response; @@ -5760,25 +5755,24 @@ answer_response: /* * Add the wild card entry. */ - if (vstat->proofs[DNS_VALIDATOR_NOQNAMEPROOF] != NULL && - vstat->rdataset != NULL && - dns_rdataset_isassociated(vstat->rdataset) && - vstat->rdataset->trust == dns_trust_secure && - vstat->sigrdataset != NULL && - dns_rdataset_isassociated(vstat->sigrdataset) && - vstat->sigrdataset->trust == dns_trust_secure && wild != NULL) + if (val->proofs[DNS_VALIDATOR_NOQNAMEPROOF] != NULL && + val->rdataset != NULL && dns_rdataset_isassociated(val->rdataset) && + val->rdataset->trust == dns_trust_secure && + val->sigrdataset != NULL && + dns_rdataset_isassociated(val->sigrdataset) && + val->sigrdataset->trust == dns_trust_secure && wild != NULL) { dns_dbnode_t *wnode = NULL; result = dns_db_findnode(fctx->cache, wild, true, &wnode); if (result == ISC_R_SUCCESS) { result = dns_db_addrdataset(fctx->cache, wnode, NULL, - now, vstat->rdataset, 0, + now, val->rdataset, 0, NULL); } if (result == ISC_R_SUCCESS) { (void)dns_db_addrdataset(fctx->cache, wnode, NULL, now, - vstat->sigrdataset, 0, NULL); + val->sigrdataset, 0, NULL); } if (wnode != NULL) { dns_db_detachnode(fctx->cache, &wnode); @@ -5796,7 +5790,7 @@ answer_response: if (hresp != NULL) { /* - * Negative results must be indicated in vstat->result. + * Negative results must be indicated in val->result. */ INSIST(hresp->rdataset != NULL); if (dns_rdataset_isassociated(hresp->rdataset) && @@ -5807,7 +5801,7 @@ answer_response: } hresp->result = eresult; - dns_name_copy(vstat->name, hresp->foundname); + dns_name_copy(val->name, hresp->foundname); dns_db_attach(fctx->cache, &hresp->db); dns_db_transfernode(fctx->cache, &node, &hresp->node); clone_results(fctx); diff --git a/lib/dns/validator.c b/lib/dns/validator.c index baffeb3b9c..3694e44632 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -181,12 +181,11 @@ markanswer(dns_validator_t *val, const char *where, const char *mbstext) { } validator_log(val, ISC_LOG_DEBUG(3), "marking as answer (%s)", where); - if (val->vstat->rdataset != NULL) { - dns_rdataset_settrust(val->vstat->rdataset, dns_trust_answer); + if (val->rdataset != NULL) { + dns_rdataset_settrust(val->rdataset, dns_trust_answer); } - if (val->vstat->sigrdataset != NULL) { - dns_rdataset_settrust(val->vstat->sigrdataset, - dns_trust_answer); + if (val->sigrdataset != NULL) { + dns_rdataset_settrust(val->sigrdataset, dns_trust_answer); } return (ISC_R_SUCCESS); @@ -196,12 +195,12 @@ markanswer(dns_validator_t *val, const char *where, const char *mbstext) { * Mark the RRsets in val->vstat with trust level secure. */ static void -marksecure(dns_valstatus_t *vstat) { - dns_rdataset_settrust(vstat->rdataset, dns_trust_secure); - if (vstat->sigrdataset != NULL) { - dns_rdataset_settrust(vstat->sigrdataset, dns_trust_secure); +marksecure(dns_validator_t *val) { + dns_rdataset_settrust(val->rdataset, dns_trust_secure); + if (val->sigrdataset != NULL) { + dns_rdataset_settrust(val->sigrdataset, dns_trust_secure); } - vstat->secure = true; + val->secure = true; } /* @@ -217,8 +216,8 @@ validator_done(dns_validator_t *val, isc_result_t result) { } val->attributes |= VALATTR_COMPLETE; - val->vstat->result = result; - isc_async_run(val->loop, val->cb, val->vstat); + val->result = result; + isc_async_run(val->loop, val->cb, val); } /* @@ -389,8 +388,6 @@ fetch_callback_dnskey(void *arg) { } isc_mem_putanddetach(&resp->mctx, resp, sizeof(*resp)); - INSIST(val->vstat != NULL); - validator_log(val, ISC_LOG_DEBUG(3), "in fetch_callback_dnskey"); LOCK(&val->lock); fetch = val->fetch; @@ -488,8 +485,6 @@ fetch_callback_ds(void *arg) { dns_rdataset_disassociate(&val->fsigrdataset); } - INSIST(val->vstat != NULL); - validator_log(val, ISC_LOG_DEBUG(3), "in fetch_callback_ds"); LOCK(&val->lock); fetch = val->fetch; @@ -616,11 +611,10 @@ done: */ static void validator_callback_dnskey(void *arg) { - dns_valstatus_t *vstat = (dns_valstatus_t *)arg; - dns_validator_t *val = vstat->validator->arg; - dns_validator_t *subvalidator = val->subvalidator; + dns_validator_t *subvalidator = (dns_validator_t *)arg; + dns_validator_t *val = subvalidator->parent; isc_result_t result; - isc_result_t eresult = vstat->result; + isc_result_t eresult = subvalidator->result; isc_result_t saved_result; bool want_destroy; @@ -680,11 +674,10 @@ validator_callback_dnskey(void *arg) { */ static void validator_callback_ds(void *arg) { - dns_valstatus_t *vstat = (dns_valstatus_t *)arg; - dns_validator_t *val = vstat->validator->arg; - dns_validator_t *subvalidator = val->subvalidator; + dns_validator_t *subvalidator = (dns_validator_t *)arg; + dns_validator_t *val = subvalidator->parent; isc_result_t result; - isc_result_t eresult = vstat->result; + isc_result_t eresult = subvalidator->result; bool want_destroy; val->subvalidator = NULL; @@ -744,11 +737,10 @@ validator_callback_ds(void *arg) { */ static void validator_callback_cname(void *arg) { - dns_valstatus_t *vstat = (dns_valstatus_t *)arg; - dns_validator_t *val = vstat->validator->arg; - dns_validator_t *subvalidator = val->subvalidator; + dns_validator_t *subvalidator = (dns_validator_t *)arg; + dns_validator_t *val = subvalidator->parent; isc_result_t result; - isc_result_t eresult = vstat->result; + isc_result_t eresult = subvalidator->result; bool want_destroy; INSIST((val->attributes & VALATTR_INSECURITY) != 0); @@ -794,11 +786,10 @@ validator_callback_cname(void *arg) { */ static void validator_callback_nsec(void *arg) { - dns_valstatus_t *vstat = (dns_valstatus_t *)arg; - dns_validator_t *val = vstat->validator->arg; - dns_validator_t *subvalidator = val->subvalidator; - dns_rdataset_t *rdataset = vstat->rdataset; - isc_result_t result = vstat->result; + dns_validator_t *subvalidator = (dns_validator_t *)arg; + dns_validator_t *val = subvalidator->parent; + dns_rdataset_t *rdataset = subvalidator->rdataset; + isc_result_t result = subvalidator->result; bool exists, data; bool want_destroy; @@ -824,23 +815,23 @@ validator_callback_nsec(void *arg) { } } } else { - dns_name_t **proofs = val->vstat->proofs; + dns_name_t **proofs = val->proofs; dns_name_t *wild = dns_fixedname_name(&val->wild); if (rdataset->type == dns_rdatatype_nsec && rdataset->trust == dns_trust_secure && (NEEDNODATA(val) || NEEDNOQNAME(val)) && !FOUNDNODATA(val) && !FOUNDNOQNAME(val) && - dns_nsec_noexistnodata(val->vstat->type, val->vstat->name, - vstat->name, rdataset, &exists, - &data, wild, validator_log, + dns_nsec_noexistnodata(val->type, val->name, + subvalidator->name, rdataset, + &exists, &data, wild, validator_log, val) == ISC_R_SUCCESS) { if (exists && !data) { val->attributes |= VALATTR_FOUNDNODATA; if (NEEDNODATA(val)) { proofs[DNS_VALIDATOR_NODATAPROOF] = - vstat->name; + subvalidator->name; } } if (!exists) { @@ -869,7 +860,7 @@ validator_callback_nsec(void *arg) { */ if (NEEDNOQNAME(val)) { proofs[DNS_VALIDATOR_NOQNAMEPROOF] = - vstat->name; + subvalidator->name; } } } @@ -957,18 +948,16 @@ check_deadlock(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type, dns_validator_t *parent; for (parent = val; parent != NULL; parent = parent->parent) { - if (parent->vstat != NULL && parent->vstat->type == type && - dns_name_equal(parent->vstat->name, name) && + if (parent->type == type && + dns_name_equal(parent->name, name) && /* * As NSEC3 records are meta data you sometimes * need to prove a NSEC3 record which says that * itself doesn't exist. */ - (parent->vstat->type != dns_rdatatype_nsec3 || - rdataset == NULL || sigrdataset == NULL || - parent->vstat->message == NULL || - parent->vstat->rdataset != NULL || - parent->vstat->sigrdataset != NULL)) + (parent->type != dns_rdatatype_nsec3 || rdataset == NULL || + sigrdataset == NULL || parent->message == NULL || + parent->rdataset != NULL || parent->sigrdataset != NULL)) { validator_log(val, ISC_LOG_DEBUG(3), "continuing validation would lead to " @@ -1138,8 +1127,8 @@ seek_dnskey(dns_validator_t *val) { * The signer name must be at the same level as the owner name * or closer to the DNS root. */ - namereln = dns_name_fullcompare(val->vstat->name, &siginfo->signer, - &order, &nlabels); + namereln = dns_name_fullcompare(val->name, &siginfo->signer, &order, + &nlabels); if (namereln != dns_namereln_subdomain && namereln != dns_namereln_equal) { @@ -1151,7 +1140,7 @@ seek_dnskey(dns_validator_t *val) { * If this is a self-signed keyset, it must not be a zone key * (since seek_dnskey is not called from validate_dnskey). */ - if (val->vstat->rdataset->type == dns_rdatatype_dnskey) { + if (val->rdataset->type == dns_rdatatype_dnskey) { return (DNS_R_CONTINUE); } @@ -1159,7 +1148,7 @@ seek_dnskey(dns_validator_t *val) { * Records appearing in the parent zone at delegation * points cannot be self-signed. */ - if (dns_rdatatype_atparent(val->vstat->rdataset->type)) { + if (dns_rdatatype_atparent(val->rdataset->type)) { return (DNS_R_CONTINUE); } } else { @@ -1167,12 +1156,12 @@ seek_dnskey(dns_validator_t *val) { * SOA and NS RRsets can only be signed by a key with * the same name. */ - if (val->vstat->rdataset->type == dns_rdatatype_soa || - val->vstat->rdataset->type == dns_rdatatype_ns) + if (val->rdataset->type == dns_rdatatype_soa || + val->rdataset->type == dns_rdatatype_ns) { const char *type; - if (val->vstat->rdataset->type == dns_rdatatype_soa) { + if (val->rdataset->type == dns_rdatatype_soa) { type = "SOA"; } else { type = "NS"; @@ -1296,13 +1285,13 @@ compute_keytag(dns_rdata_t *rdata) { } /*% - * Is the DNSKEY rrset in val->vstat->rdataset self-signed? + * Is the DNSKEY rrset in val->rdataset self-signed? */ static bool selfsigned_dnskey(dns_validator_t *val) { - dns_rdataset_t *rdataset = val->vstat->rdataset; - dns_rdataset_t *sigrdataset = val->vstat->sigrdataset; - dns_name_t *name = val->vstat->name; + dns_rdataset_t *rdataset = val->rdataset; + dns_rdataset_t *sigrdataset = val->sigrdataset; + dns_name_t *name = val->name; isc_result_t result; isc_mem_t *mctx = val->view->mctx; bool answer = false; @@ -1390,9 +1379,9 @@ verify(dns_validator_t *val, dst_key_t *key, dns_rdata_t *rdata, val->attributes |= VALATTR_TRIEDVERIFY; wild = dns_fixedname_initname(&fixed); again: - result = dns_dnssec_verify(val->vstat->name, val->vstat->rdataset, key, - ignore, val->view->maxbits, val->view->mctx, - rdata, wild); + result = dns_dnssec_verify(val->name, val->rdataset, key, ignore, + val->view->maxbits, val->view->mctx, rdata, + wild); if ((result == DNS_R_SIGEXPIRED || result == DNS_R_SIGFUTURE) && val->view->acceptexpired) { @@ -1417,7 +1406,7 @@ again: isc_result_totext(result)); } if (result == DNS_R_FROMWILDCARD) { - if (!dns_name_equal(val->vstat->name, wild)) { + if (!dns_name_equal(val->name, wild)) { dns_name_t *closest; unsigned int labels; @@ -1448,15 +1437,12 @@ again: static isc_result_t validate_answer(dns_validator_t *val, bool resume) { isc_result_t result, vresult = DNS_R_NOVALIDSIG; - dns_valstatus_t *vstat = NULL; dns_rdata_t rdata = DNS_RDATA_INIT; /* * Caller must be holding the validator lock. */ - vstat = val->vstat; - if (resume) { /* * We already have a sigrdataset. @@ -1464,14 +1450,14 @@ validate_answer(dns_validator_t *val, bool resume) { result = ISC_R_SUCCESS; validator_log(val, ISC_LOG_DEBUG(3), "resuming validate"); } else { - result = dns_rdataset_first(vstat->sigrdataset); + result = dns_rdataset_first(val->sigrdataset); } for (; result == ISC_R_SUCCESS; - result = dns_rdataset_next(vstat->sigrdataset)) + result = dns_rdataset_next(val->sigrdataset)) { dns_rdata_reset(&rdata); - dns_rdataset_current(vstat->sigrdataset, &rdata); + dns_rdataset_current(val->sigrdataset, &rdata); if (val->siginfo == NULL) { val->siginfo = isc_mem_get(val->view->mctx, sizeof(*val->siginfo)); @@ -1486,7 +1472,7 @@ validate_answer(dns_validator_t *val, bool resume) { * was known and "sufficiently good". */ if (!dns_resolver_algorithm_supported(val->view->resolver, - vstat->name, + val->name, val->siginfo->algorithm)) { resume = false; @@ -1529,9 +1515,8 @@ validate_answer(dns_validator_t *val, bool resume) { validator_log(val, ISC_LOG_DEBUG(3), "failed to verify rdataset"); } else { - dns_rdataset_trimttl(vstat->rdataset, - vstat->sigrdataset, val->siginfo, - val->start, + dns_rdataset_trimttl(val->rdataset, val->sigrdataset, + val->siginfo, val->start, val->view->acceptexpired); } @@ -1544,7 +1529,7 @@ validate_answer(dns_validator_t *val, bool resume) { } val->key = NULL; if (NEEDNOQNAME(val)) { - if (val->vstat->message == NULL) { + if (val->message == NULL) { validator_log(val, ISC_LOG_DEBUG(3), "no message available " "for noqname proof"); @@ -1554,7 +1539,7 @@ validate_answer(dns_validator_t *val, bool resume) { "looking for noqname proof"); return (validate_nx(val, false)); } else if (vresult == ISC_R_SUCCESS) { - marksecure(vstat); + marksecure(val); validator_log(val, ISC_LOG_DEBUG(3), "marking as secure, " "noqname proof not needed"); @@ -1579,7 +1564,7 @@ validate_answer(dns_validator_t *val, bool resume) { /*% * Check whether this DNSKEY (keyrdata) signed the DNSKEY RRset - * (val->vstat->rdataset). + * (val->rdataset). */ static isc_result_t check_signer(dns_validator_t *val, dns_rdata_t *keyrdata, uint16_t keyid, @@ -1588,13 +1573,13 @@ check_signer(dns_validator_t *val, dns_rdata_t *keyrdata, uint16_t keyid, dst_key_t *dstkey = NULL; isc_result_t result; - for (result = dns_rdataset_first(val->vstat->sigrdataset); + for (result = dns_rdataset_first(val->sigrdataset); result == ISC_R_SUCCESS; - result = dns_rdataset_next(val->vstat->sigrdataset)) + result = dns_rdataset_next(val->sigrdataset)) { dns_rdata_t rdata = DNS_RDATA_INIT; - dns_rdataset_current(val->vstat->sigrdataset, &rdata); + dns_rdataset_current(val->sigrdataset, &rdata); result = dns_rdata_tostruct(&rdata, &sig, NULL); RUNTIME_CHECK(result == ISC_R_SUCCESS); if (keyid != sig.keyid || algorithm != sig.algorithm) { @@ -1602,8 +1587,7 @@ check_signer(dns_validator_t *val, dns_rdata_t *keyrdata, uint16_t keyid, } if (dstkey == NULL) { result = dns_dnssec_keyfromrdata( - val->vstat->name, keyrdata, val->view->mctx, - &dstkey); + val->name, keyrdata, val->view->mctx, &dstkey); if (result != ISC_R_SUCCESS) { /* * This really shouldn't happen, but... @@ -1740,8 +1724,7 @@ validate_dnskey(dns_validator_t *val) { * a DS style trust anchor configured for this key. */ if (val->dsset == NULL) { - result = dns_keytable_find(val->keytable, val->vstat->name, - &keynode); + result = dns_keytable_find(val->keytable, val->name, &keynode); if (result == ISC_R_SUCCESS) { if (dns_keynode_dsset(keynode, &val->fdsset)) { val->dsset = &val->fdsset; @@ -1760,7 +1743,7 @@ validate_dnskey(dns_validator_t *val) { * If this is the root name and there was no trust anchor, * we can give up now, since there's no DS at the root. */ - if (dns_name_equal(val->vstat->name, dns_rootname)) { + if (dns_name_equal(val->name, dns_rootname)) { if ((val->attributes & VALATTR_TRIEDVERIFY) != 0) { validator_log(val, ISC_LOG_DEBUG(3), "root key failed to validate"); @@ -1775,7 +1758,7 @@ validate_dnskey(dns_validator_t *val) { /* * Look up the DS RRset for this name. */ - result = get_dsset(val, val->vstat->name, &tresult); + result = get_dsset(val, val->name, &tresult); if (result == ISC_R_COMPLETE) { result = tresult; goto cleanup; @@ -1815,16 +1798,14 @@ validate_dnskey(dns_validator_t *val) { result = dns_rdata_tostruct(&dsrdata, &ds, NULL); RUNTIME_CHECK(result == ISC_R_SUCCESS); - if (!dns_resolver_ds_digest_supported(val->view->resolver, - val->vstat->name, - ds.digest_type)) + if (!dns_resolver_ds_digest_supported( + val->view->resolver, val->name, ds.digest_type)) { continue; } if (!dns_resolver_algorithm_supported(val->view->resolver, - val->vstat->name, - ds.algorithm)) + val->name, ds.algorithm)) { continue; } @@ -1851,16 +1832,14 @@ validate_dnskey(dns_validator_t *val) { continue; } - if (!dns_resolver_ds_digest_supported(val->view->resolver, - val->vstat->name, - ds.digest_type)) + if (!dns_resolver_ds_digest_supported( + val->view->resolver, val->name, ds.digest_type)) { continue; } if (!dns_resolver_algorithm_supported(val->view->resolver, - val->vstat->name, - ds.algorithm)) + val->name, ds.algorithm)) { continue; } @@ -1870,8 +1849,8 @@ validate_dnskey(dns_validator_t *val) { /* * Find the DNSKEY matching the DS... */ - result = dns_dnssec_matchdskey(val->vstat->name, &dsrdata, - val->vstat->rdataset, &keyrdata); + result = dns_dnssec_matchdskey(val->name, &dsrdata, + val->rdataset, &keyrdata); if (result != ISC_R_SUCCESS) { validator_log(val, ISC_LOG_DEBUG(3), "no DNSKEY matching DS"); @@ -1890,7 +1869,7 @@ validate_dnskey(dns_validator_t *val) { } if (result == ISC_R_SUCCESS) { - marksecure(val->vstat); + marksecure(val); validator_log(val, ISC_LOG_DEBUG(3), "marking as secure (DS)"); } else if (result == ISC_R_NOMORE && !supported_algorithm) { validator_log(val, ISC_LOG_DEBUG(3), @@ -1920,7 +1899,7 @@ cleanup: static isc_result_t val_rdataset_first(dns_validator_t *val, dns_name_t **namep, dns_rdataset_t **rdatasetp) { - dns_message_t *message = val->vstat->message; + dns_message_t *message = val->message; isc_result_t result; REQUIRE(rdatasetp != NULL); @@ -1942,10 +1921,9 @@ val_rdataset_first(dns_validator_t *val, dns_name_t **namep, *rdatasetp = ISC_LIST_HEAD((*namep)->list); INSIST(*rdatasetp != NULL); } else { - result = dns_rdataset_first(val->vstat->rdataset); + result = dns_rdataset_first(val->rdataset); if (result == ISC_R_SUCCESS) { - dns_ncache_current(val->vstat->rdataset, *namep, - *rdatasetp); + dns_ncache_current(val->rdataset, *namep, *rdatasetp); } } return (result); @@ -1954,7 +1932,7 @@ val_rdataset_first(dns_validator_t *val, dns_name_t **namep, static isc_result_t val_rdataset_next(dns_validator_t *val, dns_name_t **namep, dns_rdataset_t **rdatasetp) { - dns_message_t *message = val->vstat->message; + dns_message_t *message = val->message; isc_result_t result = ISC_R_SUCCESS; REQUIRE(rdatasetp != NULL && *rdatasetp != NULL); @@ -1977,10 +1955,9 @@ val_rdataset_next(dns_validator_t *val, dns_name_t **namep, *rdatasetp = rdataset; } else { dns_rdataset_disassociate(*rdatasetp); - result = dns_rdataset_next(val->vstat->rdataset); + result = dns_rdataset_next(val->rdataset); if (result == ISC_R_SUCCESS) { - dns_ncache_current(val->vstat->rdataset, *namep, - *rdatasetp); + dns_ncache_current(val->rdataset, *namep, *rdatasetp); } } return (result); @@ -2016,7 +1993,7 @@ checkwildcard(dns_validator_t *val, dns_rdatatype_t type, dns_name_format(wild, namebuf, sizeof(namebuf)); validator_log(val, ISC_LOG_DEBUG(3), "in checkwildcard: %s", namebuf); - if (val->vstat->message == NULL) { + if (val->message == NULL) { name = &tname; rdataset = &trdataset; } else { @@ -2037,11 +2014,11 @@ checkwildcard(dns_validator_t *val, dns_rdatatype_t type, if (rdataset->type == dns_rdatatype_nsec && (NEEDNODATA(val) || NEEDNOWILDCARD(val)) && !FOUNDNODATA(val) && !FOUNDNOWILDCARD(val) && - dns_nsec_noexistnodata(val->vstat->type, wild, name, - rdataset, &exists, &data, NULL, - validator_log, val) == ISC_R_SUCCESS) + dns_nsec_noexistnodata(val->type, wild, name, rdataset, + &exists, &data, NULL, validator_log, + val) == ISC_R_SUCCESS) { - dns_name_t **proofs = val->vstat->proofs; + dns_name_t **proofs = val->proofs; if (exists && !data) { val->attributes |= VALATTR_FOUNDNODATA; } @@ -2064,11 +2041,11 @@ checkwildcard(dns_validator_t *val, dns_rdatatype_t type, (NEEDNODATA(val) || NEEDNOWILDCARD(val)) && !FOUNDNODATA(val) && !FOUNDNOWILDCARD(val) && dns_nsec3_noexistnodata( - val->vstat->type, wild, name, rdataset, zonename, - &exists, &data, NULL, NULL, NULL, NULL, NULL, NULL, + val->type, wild, name, rdataset, zonename, &exists, + &data, NULL, NULL, NULL, NULL, NULL, NULL, validator_log, val) == ISC_R_SUCCESS) { - dns_name_t **proofs = val->vstat->proofs; + dns_name_t **proofs = val->proofs; if (exists && !data) { val->attributes |= VALATTR_FOUNDNODATA; } @@ -2109,7 +2086,7 @@ findnsec3proofs(dns_validator_t *val) { bool setclosest, setnearest, *setclosestp; dns_fixedname_t fclosest, fnearest, fzonename; dns_name_t *closest, *nearest, *zonename, *closestp; - dns_name_t **proofs = val->vstat->proofs; + dns_name_t **proofs = val->proofs; dns_rdataset_t *rdataset, trdataset; dns_name_init(&tname, NULL); @@ -2118,7 +2095,7 @@ findnsec3proofs(dns_validator_t *val) { nearest = dns_fixedname_initname(&fnearest); zonename = dns_fixedname_initname(&fzonename); - if (val->vstat->message == NULL) { + if (val->message == NULL) { name = &tname; rdataset = &trdataset; } else { @@ -2136,10 +2113,10 @@ findnsec3proofs(dns_validator_t *val) { continue; } - result = dns_nsec3_noexistnodata( - val->vstat->type, val->vstat->name, name, rdataset, - zonename, NULL, NULL, NULL, NULL, NULL, NULL, NULL, - NULL, validator_log, val); + result = dns_nsec3_noexistnodata(val->type, val->name, name, + rdataset, zonename, NULL, NULL, + NULL, NULL, NULL, NULL, NULL, + NULL, validator_log, val); if (result != ISC_R_IGNORE && result != ISC_R_SUCCESS) { if (dns_rdataset_isassociated(&trdataset)) { dns_rdataset_disassociate(&trdataset); @@ -2194,10 +2171,9 @@ findnsec3proofs(dns_validator_t *val) { optout = false; unknown = false; result = dns_nsec3_noexistnodata( - val->vstat->type, val->vstat->name, name, rdataset, - zonename, &exists, &data, &optout, &unknown, - setclosestp, &setnearest, closestp, nearest, - validator_log, val); + val->type, val->name, name, rdataset, zonename, &exists, + &data, &optout, &unknown, setclosestp, &setnearest, + closestp, nearest, validator_log, val); if (unknown) { val->attributes |= VALATTR_FOUNDUNKNOWN; } @@ -2305,9 +2281,9 @@ validate_neg_rrset(dns_validator_t *val, dns_name_t *name, * the first one is still in progress), and go into an * infinite loop. Avoid that. */ - if (val->vstat->type == dns_rdatatype_dnskey && + if (val->type == dns_rdatatype_dnskey && rdataset->type == dns_rdatatype_nsec && - dns_name_equal(name, val->vstat->name)) + dns_name_equal(name, val->name)) { dns_rdata_t nsec = DNS_RDATA_INIT; @@ -2321,7 +2297,7 @@ validate_neg_rrset(dns_validator_t *val, dns_name_t *name, } } - val->currentset = rdataset; + val->nxset = rdataset; result = create_validator(val, name, rdataset->type, rdataset, sigrdataset, validator_callback_nsec, "validate_neg_rrset"); @@ -2339,7 +2315,7 @@ validate_neg_rrset(dns_validator_t *val, dns_name_t *name, static isc_result_t validate_authority(dns_validator_t *val, bool resume) { dns_name_t *name; - dns_message_t *message = val->vstat->message; + dns_message_t *message = val->message; isc_result_t result; if (!resume) { @@ -2356,8 +2332,8 @@ validate_authority(dns_validator_t *val, bool resume) { name = NULL; dns_message_currentname(message, DNS_SECTION_AUTHORITY, &name); if (resume) { - rdataset = ISC_LIST_NEXT(val->currentset, link); - val->currentset = NULL; + rdataset = ISC_LIST_NEXT(val->nxset, link); + val->nxset = NULL; resume = false; } else { rdataset = ISC_LIST_HEAD(name->list); @@ -2403,13 +2379,13 @@ validate_ncache(dns_validator_t *val, bool resume) { isc_result_t result; if (!resume) { - result = dns_rdataset_first(val->vstat->rdataset); + result = dns_rdataset_first(val->rdataset); } else { - result = dns_rdataset_next(val->vstat->rdataset); + result = dns_rdataset_next(val->rdataset); } for (; result == ISC_R_SUCCESS; - result = dns_rdataset_next(val->vstat->rdataset)) + result = dns_rdataset_next(val->rdataset)) { dns_rdataset_t *rdataset, *sigrdataset = NULL; @@ -2417,13 +2393,13 @@ validate_ncache(dns_validator_t *val, bool resume) { name = dns_fixedname_initname(&val->fname); rdataset = &val->frdataset; - dns_ncache_current(val->vstat->rdataset, name, rdataset); + dns_ncache_current(val->rdataset, name, rdataset); if (val->frdataset.type == dns_rdatatype_rrsig) { continue; } - result = dns_ncache_getsigrdataset(val->vstat->rdataset, name, + result = dns_ncache_getsigrdataset(val->rdataset, name, rdataset->type, &val->fsigrdataset); if (result == ISC_R_SUCCESS) { @@ -2465,7 +2441,7 @@ validate_nx(dns_validator_t *val, bool resume) { validator_log(val, ISC_LOG_DEBUG(3), "resuming validate_nx"); } - if (val->vstat->message == NULL) { + if (val->message == NULL) { result = validate_ncache(val, resume); } else { result = validate_authority(val, resume); @@ -2494,7 +2470,7 @@ validate_nx(dns_validator_t *val, bool resume) { { validator_log(val, ISC_LOG_DEBUG(3), "marking as secure, noqname proof found"); - marksecure(val->vstat); + marksecure(val); return (ISC_R_SUCCESS); } else if (FOUNDOPTOUT(val) && dns_name_countlabels( @@ -2502,7 +2478,7 @@ validate_nx(dns_validator_t *val, bool resume) { { validator_log(val, ISC_LOG_DEBUG(3), "optout proof found"); - val->vstat->optout = true; + val->optout = true; markanswer(val, "validate_nx (1)", NULL); return (ISC_R_SUCCESS); } else if ((val->attributes & VALATTR_FOUNDUNKNOWN) != 0) { @@ -2543,14 +2519,14 @@ validate_nx(dns_validator_t *val, bool resume) { FOUNDNOWILDCARD(val) && FOUNDCLOSEST(val))) { if ((val->attributes & VALATTR_FOUNDOPTOUT) != 0) { - val->vstat->optout = true; + val->optout = true; } validator_log(val, ISC_LOG_DEBUG(3), "nonexistence proof(s) found"); - if (val->vstat->message == NULL) { - marksecure(val->vstat); + if (val->message == NULL) { + marksecure(val); } else { - val->vstat->secure = true; + val->secure = true; } return (ISC_R_SUCCESS); } @@ -2595,7 +2571,7 @@ check_ds_algs(dns_validator_t *val, dns_name_t *name, } /*% - * seek_ds is called to look up DS rrsets at the label of val->vstat->name + * seek_ds is called to look up DS rrsets at the label of val->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. @@ -2616,10 +2592,10 @@ seek_ds(dns_validator_t *val, isc_result_t *resp) { dns_name_t *found = dns_fixedname_initname(&fixedfound); dns_name_t *tname = dns_fixedname_initname(&val->fname); - if (val->labels == dns_name_countlabels(val->vstat->name)) { - dns_name_copy(val->vstat->name, tname); + if (val->labels == dns_name_countlabels(val->name)) { + dns_name_copy(val->name, tname); } else { - dns_name_split(val->vstat->name, val->labels, NULL, tname); + dns_name_split(val->name, val->labels, NULL, tname); } dns_name_format(tname, namebuf, sizeof(namebuf)); @@ -2831,9 +2807,9 @@ 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->vstat->name is in an unsecure zone + * \li ISC_R_SUCCESS val->name is in an unsecure zone * \li DNS_R_WAIT validation is in progress. - * \li DNS_R_MUSTBESECURE val->vstat->name is supposed to be secure + * \li DNS_R_MUSTBESECURE val->name is supposed to be secure * (policy) but we proved that it is unsecure. * \li DNS_R_NOVALIDSIG * \li DNS_R_NOVALIDNSEC @@ -2853,14 +2829,14 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { */ val->attributes |= VALATTR_INSECURITY; - dns_name_copy(val->vstat->name, secroot); + dns_name_copy(val->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->vstat->type == dns_rdatatype_ds && labels > 1U) { + if (val->type == dns_rdatatype_ds && labels > 1U) { dns_name_getlabelsequence(secroot, 1, labels - 1, secroot); } @@ -2907,7 +2883,7 @@ proveunsecure(dns_validator_t *val, bool have_ds, bool resume) { * Walk down through each of the remaining labels in the name, * looking for DS records. */ - while (val->labels <= dns_name_countlabels(val->vstat->name)) { + while (val->labels <= dns_name_countlabels(val->name)) { isc_result_t tresult; result = seek_ds(val, &tresult); @@ -2946,15 +2922,11 @@ out: */ static void validator_start(void *arg) { - dns_valstatus_t *vstat = (dns_valstatus_t *)arg; - dns_validator_t *val = NULL; + dns_validator_t *val = (dns_validator_t *)arg; bool want_destroy = false; isc_result_t result = ISC_R_FAILURE; - val = vstat->validator; - - /* If the validator has been canceled, val->vstat == NULL */ - if (val->vstat == NULL) { + if (CANCELED(val)) { return; } @@ -2962,7 +2934,7 @@ validator_start(void *arg) { LOCK(&val->lock); - if (val->vstat->rdataset != NULL && val->vstat->sigrdataset != NULL) { + if (val->rdataset != NULL && val->sigrdataset != NULL) { isc_result_t saved_result; /* @@ -2972,8 +2944,8 @@ validator_start(void *arg) { validator_log(val, ISC_LOG_DEBUG(3), "attempting positive response validation"); - INSIST(dns_rdataset_isassociated(val->vstat->rdataset)); - INSIST(dns_rdataset_isassociated(val->vstat->sigrdataset)); + INSIST(dns_rdataset_isassociated(val->rdataset)); + INSIST(dns_rdataset_isassociated(val->sigrdataset)); if (selfsigned_dnskey(val)) { result = validate_dnskey(val); } else { @@ -2990,14 +2962,12 @@ validator_start(void *arg) { result = saved_result; } } - } else if (val->vstat->rdataset != NULL && - val->vstat->rdataset->type != 0) - { + } else if (val->rdataset != NULL && val->rdataset->type != 0) { /* * This is either an unsecure subdomain or a response * from a broken server. */ - INSIST(dns_rdataset_isassociated(val->vstat->rdataset)); + INSIST(dns_rdataset_isassociated(val->rdataset)); validator_log(val, ISC_LOG_DEBUG(3), "attempting insecurity proof"); @@ -3007,9 +2977,7 @@ validator_start(void *arg) { "got insecure response; " "parent indicates it should be secure"); } - } else if ((val->vstat->rdataset == NULL && - val->vstat->sigrdataset == NULL)) - { + } else if ((val->rdataset == NULL && val->sigrdataset == NULL)) { /* * This is a validation of a negative response. */ @@ -3017,7 +2985,7 @@ validator_start(void *arg) { "attempting negative response validation " "from message"); - if (val->vstat->message->rcode == dns_rcode_nxdomain) { + if (val->message->rcode == dns_rcode_nxdomain) { val->attributes |= VALATTR_NEEDNOQNAME; val->attributes |= VALATTR_NEEDNOWILDCARD; } else { @@ -3025,9 +2993,7 @@ validator_start(void *arg) { } result = validate_nx(val, false); - } else if ((val->vstat->rdataset != NULL && - NEGATIVE(val->vstat->rdataset))) - { + } else if ((val->rdataset != NULL && NEGATIVE(val->rdataset))) { /* * This is a delayed validation of a negative cache entry. */ @@ -3035,7 +3001,7 @@ validator_start(void *arg) { "attempting negative response validation " "from cache"); - if (NXDOMAIN(val->vstat->rdataset)) { + if (NXDOMAIN(val->rdataset)) { val->attributes |= VALATTR_NEEDNOQNAME; val->attributes |= VALATTR_NEEDNOWILDCARD; } else { @@ -3066,33 +3032,26 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type, dns_validator_t **validatorp) { isc_result_t result = ISC_R_FAILURE; dns_validator_t *val = NULL; - dns_valstatus_t *vstat = NULL; REQUIRE(name != NULL); REQUIRE(rdataset != NULL || (rdataset == NULL && sigrdataset == NULL && message != NULL)); REQUIRE(validatorp != NULL && *validatorp == NULL); - vstat = isc_mem_get(view->mctx, sizeof(*vstat)); - *vstat = (dns_valstatus_t){ - .result = ISC_R_FAILURE, - .name = name, - .type = type, - .rdataset = rdataset, - .sigrdataset = sigrdataset, - }; - isc_mem_attach(view->mctx, &vstat->mctx); - if (message != NULL) { - dns_message_attach(message, &vstat->message); - } - val = isc_mem_get(view->mctx, sizeof(*val)); - *val = (dns_validator_t){ .vstat = vstat, + *val = (dns_validator_t){ .result = ISC_R_FAILURE, + .rdataset = rdataset, + .sigrdataset = sigrdataset, + .name = name, + .type = type, .options = options, .link = ISC_LINK_INITIALIZER, .loop = loop, .cb = cb, .arg = arg }; + if (message != NULL) { + dns_message_attach(message, &val->message); + } dns_view_attach(view, &val->view); isc_mutex_init(&val->lock); @@ -3110,10 +3069,8 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type, isc_stdtime_get(&val->start); val->magic = VALIDATOR_MAGIC; - vstat->validator = val; - if ((options & DNS_VALIDATOR_DEFER) == 0) { - isc_async_run(loop, validator_start, vstat); + isc_async_run(loop, validator_start, val); } *validatorp = val; @@ -3122,10 +3079,11 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type, cleanup: isc_mutex_destroy(&val->lock); - isc_mem_putanddetach(&vstat->mctx, vstat, sizeof(*vstat)); - - dns_view_detach(&val->view); + if (val->message != NULL) { + dns_message_detach(&val->message); + } isc_mem_put(view->mctx, val, sizeof(*val)); + dns_view_detach(&view); return (result); } @@ -3140,7 +3098,7 @@ dns_validator_send(dns_validator_t *validator) { validator->options &= ~DNS_VALIDATOR_DEFER; UNLOCK(&validator->lock); - isc_async_run(validator->loop, validator_start, validator->vstat); + isc_async_run(validator->loop, validator_start, validator); } void @@ -3192,11 +3150,9 @@ destroy(dns_validator_t *val) { } isc_mutex_destroy(&val->lock); dns_view_detach(&val->view); - if (val->vstat->message != NULL) { - dns_message_detach(&val->vstat->message); + if (val->message != NULL) { + dns_message_detach(&val->message); } - isc_mem_putanddetach(&val->vstat->mctx, val->vstat, - sizeof(*val->vstat)); isc_mem_put(mctx, val, sizeof(*val)); } @@ -3257,13 +3213,12 @@ validator_logv(dns_validator_t *val, isc_logcategory_t *category, sep2 = ": "; } - if (val->vstat != NULL && val->vstat->name != NULL) { + if (val->name != NULL) { char namebuf[DNS_NAME_FORMATSIZE]; char typebuf[DNS_RDATATYPE_FORMATSIZE]; - dns_name_format(val->vstat->name, namebuf, sizeof(namebuf)); - dns_rdatatype_format(val->vstat->type, typebuf, - sizeof(typebuf)); + dns_name_format(val->name, namebuf, sizeof(namebuf)); + dns_rdatatype_format(val->type, typebuf, sizeof(typebuf)); isc_log_write(dns_lctx, category, module, level, "%s%s%s%.*svalidating %s/%s: %s", sep1, viewname, sep2, depth, spaces, namebuf, typebuf, msgbuf); From 7da99414c080f8dca8d60c957f4fe1893f8bbdbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 16 Feb 2023 14:37:55 +0100 Subject: [PATCH 2/4] Implement proper reference counting in dns_validator use reference counting in dns_validator to prevent use after free. --- lib/dns/include/dns/validator.h | 94 +++++++++++++-------- lib/dns/resolver.c | 17 ++-- lib/dns/validator.c | 141 +++++++++++++------------------- 3 files changed, 124 insertions(+), 128 deletions(-) diff --git a/lib/dns/include/dns/validator.h b/lib/dns/include/dns/validator.h index 9400c216d6..ccd4d9405e 100644 --- a/lib/dns/include/dns/validator.h +++ b/lib/dns/include/dns/validator.h @@ -48,8 +48,10 @@ #include +#include #include #include +#include #include #include @@ -71,43 +73,28 @@ * whatever purpose the client desires. */ struct dns_validator { - /* Unlocked. */ - unsigned int magic; - isc_mutex_t lock; - dns_view_t *view; + /* Unlocked: */ + unsigned int magic; + isc_mutex_t lock; + dns_view_t *view; + isc_loopmgr_t *loopmgr; + + isc_refcount_t references; /* Name and type of the response to be validated. */ dns_name_t *name; dns_rdatatype_t type; - /* Locked by lock. */ - unsigned int options; - unsigned int attributes; - dns_fetch_t *fetch; - dns_validator_t *subvalidator; - dns_validator_t *parent; - dns_keytable_t *keytable; - dst_key_t *key; - dns_rdata_rrsig_t *siginfo; - isc_loop_t *loop; - isc_job_cb cb; - void *arg; - unsigned int labels; - dns_rdataset_t *nxset; - dns_rdataset_t *keyset; - dns_rdataset_t *dsset; - dns_rdataset_t fdsset; - dns_rdataset_t frdataset; - dns_rdataset_t fsigrdataset; - dns_fixedname_t fname; - dns_fixedname_t wild; - dns_fixedname_t closest; - ISC_LINK(dns_validator_t) link; - bool mustbesecure; - unsigned int depth; - unsigned int authcount; - unsigned int authfail; - isc_stdtime_t start; + /* + * Callback and argument to use to inform the caller + * that validation is complete. + */ + isc_job_cb cb; + void *arg; + + /* Locked by 'lock': */ + /* Validation options (_DEFER, _NONTA, etc). */ + unsigned int options; /* * Results of a completed validation. @@ -136,6 +123,31 @@ struct dns_validator { * Answer is secure. */ bool secure; + + /* Internal validator state */ + unsigned int attributes; + dns_fetch_t *fetch; + dns_validator_t *subvalidator; + dns_validator_t *parent; + dns_keytable_t *keytable; + dst_key_t *key; + dns_rdata_rrsig_t *siginfo; + unsigned int labels; + dns_rdataset_t *nxset; + dns_rdataset_t *keyset; + dns_rdataset_t *dsset; + dns_rdataset_t fdsset; + dns_rdataset_t frdataset; + dns_rdataset_t fsigrdataset; + dns_fixedname_t fname; + dns_fixedname_t wild; + dns_fixedname_t closest; + ISC_LINK(dns_validator_t) link; + bool mustbesecure; + unsigned int depth; + unsigned int authcount; + unsigned int authfail; + isc_stdtime_t start; }; /*% @@ -152,7 +164,7 @@ isc_result_t dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type, dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset, dns_message_t *message, unsigned int options, - isc_loop_t *loop, isc_job_cb cb, void *arg, + isc_loopmgr_t *loop, isc_job_cb cb, void *arg, dns_validator_t **validatorp); /*%< * Start a DNSSEC validation. @@ -221,10 +233,24 @@ dns_validator_destroy(dns_validator_t **validatorp); * Requires: *\li '*validatorp' points to a valid DNSSEC validator. * \li The validator must have completed and sent its completion - * event. + * event. * * Ensures: *\li All resources used by the validator are freed. */ +#if DNS_VALIDATOR_TRACE +#define dns_validator_ref(ptr) \ + dns_validator__ref(ptr, __func__, __FILE__, __LINE__) +#define dns_validator_unref(ptr) \ + dns_validator__unref(ptr, __func__, __FILE__, __LINE__) +#define dns_validator_attach(ptr, ptrp) \ + dns_validator__attach(ptr, ptrp, __func__, __FILE__, __LINE__) +#define dns_validator_detach(ptrp) \ + dns_validator__detach(ptrp, __func__, __FILE__, __LINE__) +ISC_REFCOUNT_TRACE_DECL(dns_validator); +#else +ISC_REFCOUNT_DECL(dns_validator); +#endif + ISC_LANG_ENDDECLS diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index ed31dd9f53..f6b6f64d10 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -961,8 +961,7 @@ set_stats(dns_resolver_t *res, isc_statscounter_t counter, uint64_t val) { static isc_result_t valcreate(fetchctx_t *fctx, dns_message_t *message, dns_adbaddrinfo_t *addrinfo, dns_name_t *name, dns_rdatatype_t type, dns_rdataset_t *rdataset, - dns_rdataset_t *sigrdataset, unsigned int valoptions, - isc_loop_t *loop) { + dns_rdataset_t *sigrdataset, unsigned int valoptions) { dns_validator_t *validator = NULL; dns_valarg_t *valarg = NULL; isc_result_t result; @@ -980,9 +979,9 @@ valcreate(fetchctx_t *fctx, dns_message_t *message, dns_adbaddrinfo_t *addrinfo, valoptions &= ~DNS_VALIDATOR_DEFER; } - result = dns_validator_create(fctx->res->view, name, type, rdataset, - sigrdataset, message, valoptions, loop, - validated, valarg, &validator); + result = dns_validator_create( + fctx->res->view, name, type, rdataset, sigrdataset, message, + valoptions, fctx->res->loopmgr, validated, valarg, &validator); RUNTIME_CHECK(result == ISC_R_SUCCESS); inc_stats(fctx->res, dns_resstatscounter_val); if ((valoptions & DNS_VALIDATOR_DEFER) == 0) { @@ -6314,8 +6313,7 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, result = valcreate( fctx, message, addrinfo, name, rdataset->type, rdataset, - sigrdataset, valoptions, - fctx->loop); + sigrdataset, valoptions); } } else if (CHAINING(rdataset)) { if (rdataset->type == dns_rdatatype_cname) { @@ -6423,8 +6421,7 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, } result = valcreate(fctx, message, addrinfo, name, vtype, - valrdataset, valsigrdataset, valoptions, - fctx->loop); + valrdataset, valsigrdataset, valoptions); } if (result == ISC_R_SUCCESS && have_answer) { @@ -6646,7 +6643,7 @@ ncache_message(fetchctx_t *fctx, dns_message_t *message, * Do negative response validation. */ result = valcreate(fctx, message, addrinfo, name, fctx->type, - NULL, NULL, valoptions, fctx->loop); + NULL, NULL, valoptions); /* * If validation is necessary, return now. Otherwise * continue to process the message, letting the diff --git a/lib/dns/validator.c b/lib/dns/validator.c index 3694e44632..075d28ad72 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -16,8 +16,10 @@ #include #include +#include #include #include +#include #include #include #include @@ -63,7 +65,6 @@ #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 \ @@ -97,7 +98,6 @@ #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 COMPLETE(v) (((v)->attributes & VALATTR_COMPLETE) != 0) @@ -105,7 +105,7 @@ #define NXDOMAIN(r) (((r)->attributes & DNS_RDATASETATTR_NXDOMAIN) != 0) static void -destroy(dns_validator_t *val); +destroy_validator(dns_validator_t *val); static isc_result_t select_signing_key(dns_validator_t *val, dns_rdataset_t *rdataset); @@ -203,6 +203,13 @@ marksecure(dns_validator_t *val) { val->secure = true; } +static void +validator_done_cb(void *arg) { + dns_validator_t *val = arg; + val->cb(val); + dns_validator_detach(&val); +} + /* * Validator 'val' is finished; send the completion event to the loop * that called dns_validator_create(), with result `result`. @@ -217,26 +224,9 @@ validator_done(dns_validator_t *val, isc_result_t result) { val->attributes |= VALATTR_COMPLETE; val->result = result; - isc_async_run(val->loop, val->cb, val); -} -/* - * Called when deciding whether to destroy validator 'val'. - */ -static bool -exit_check(dns_validator_t *val) { - /* - * Caller must be holding the lock. - */ - if (!SHUTDOWN(val)) { - return (false); - } - - if (val->fetch != NULL || val->subvalidator != NULL) { - return (false); - } - - return (true); + dns_validator_ref(val); + isc_job_run(val->loopmgr, validator_done_cb, val); } /*% @@ -372,7 +362,6 @@ fetch_callback_dnskey(void *arg) { isc_result_t result; isc_result_t saved_result; dns_fetch_t *fetch = NULL; - bool want_destroy; INSIST(resp->type == FETCHDONE); @@ -439,17 +428,13 @@ fetch_callback_dnskey(void *arg) { 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); - } + dns_validator_detach(&val); } /*% @@ -464,7 +449,7 @@ fetch_callback_ds(void *arg) { isc_result_t eresult = resp->result; isc_result_t result; dns_fetch_t *fetch = NULL; - bool want_destroy, trustchain; + bool trustchain; INSIST(resp->type == FETCHDONE); @@ -592,16 +577,13 @@ fetch_callback_ds(void *arg) { done: isc_mem_putanddetach(&resp->mctx, resp, sizeof(*resp)); - want_destroy = exit_check(val); UNLOCK(&val->lock); if (fetch != NULL) { dns_resolver_destroyfetch(&fetch); } - if (want_destroy) { - destroy(val); - } + dns_validator_detach(&val); } /*% @@ -616,9 +598,9 @@ validator_callback_dnskey(void *arg) { isc_result_t result; isc_result_t eresult = subvalidator->result; isc_result_t saved_result; - bool want_destroy; val->subvalidator = NULL; + subvalidator->parent = NULL; validator_log(val, ISC_LOG_DEBUG(3), "in validator_callback_dnskey"); LOCK(&val->lock); @@ -658,13 +640,10 @@ validator_callback_dnskey(void *arg) { validator_done(val, DNS_R_BROKENCHAIN); } - dns_validator_destroy(&subvalidator); - - want_destroy = exit_check(val); UNLOCK(&val->lock); - if (want_destroy) { - destroy(val); - } + + dns_validator_destroy(&subvalidator); + dns_validator_detach(&val); } /*% @@ -678,9 +657,9 @@ validator_callback_ds(void *arg) { dns_validator_t *val = subvalidator->parent; isc_result_t result; isc_result_t eresult = subvalidator->result; - bool want_destroy; val->subvalidator = NULL; + subvalidator->parent = NULL; validator_log(val, ISC_LOG_DEBUG(3), "in validator_callback_ds"); LOCK(&val->lock); @@ -720,14 +699,10 @@ validator_callback_ds(void *arg) { isc_result_totext(eresult)); validator_done(val, DNS_R_BROKENCHAIN); } + UNLOCK(&val->lock); dns_validator_destroy(&subvalidator); - - want_destroy = exit_check(val); - UNLOCK(&val->lock); - if (want_destroy) { - destroy(val); - } + dns_validator_detach(&val); } /*% @@ -741,7 +716,6 @@ validator_callback_cname(void *arg) { dns_validator_t *val = subvalidator->parent; isc_result_t result; isc_result_t eresult = subvalidator->result; - bool want_destroy; INSIST((val->attributes & VALATTR_INSECURITY) != 0); @@ -768,13 +742,10 @@ validator_callback_cname(void *arg) { validator_done(val, DNS_R_BROKENCHAIN); } - dns_validator_destroy(&subvalidator); - - want_destroy = exit_check(val); UNLOCK(&val->lock); - if (want_destroy) { - destroy(val); - } + + dns_validator_destroy(&subvalidator); + dns_validator_detach(&val); } /*% @@ -791,7 +762,6 @@ validator_callback_nsec(void *arg) { dns_rdataset_t *rdataset = subvalidator->rdataset; isc_result_t result = subvalidator->result; bool exists, data; - bool want_destroy; val->subvalidator = NULL; @@ -871,13 +841,10 @@ validator_callback_nsec(void *arg) { } } - dns_validator_destroy(&subvalidator); - - want_destroy = exit_check(val); UNLOCK(&val->lock); - if (want_destroy) { - destroy(val); - } + + dns_validator_destroy(&subvalidator); + dns_validator_detach(&val); } /*% @@ -993,10 +960,12 @@ create_fetch(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type, } validator_logcreate(val, name, type, caller, "fetch"); + + dns_validator_ref(val); return (dns_resolver_createfetch( val->view->resolver, name, type, NULL, NULL, NULL, NULL, 0, - fopts, 0, NULL, val->loop, callback, val, &val->frdataset, - &val->fsigrdataset, &val->fetch)); + fopts, 0, NULL, isc_loop_current(val->loopmgr), callback, val, + &val->frdataset, &val->fsigrdataset, &val->fetch)); } /*% @@ -1026,10 +995,10 @@ 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, sig, - NULL, vopts, val->loop, cb, val, + NULL, vopts, val->loopmgr, cb, val, &val->subvalidator); if (result == ISC_R_SUCCESS) { - val->subvalidator->parent = val; + dns_validator_attach(val, &val->subvalidator->parent); val->subvalidator->depth = val->depth + 1; } return (result); @@ -1239,6 +1208,7 @@ seek_dnskey(dns_validator_t *val) { dns_rdatatype_dnskey, fetch_callback_dnskey, "seek_dnskey"); if (result != ISC_R_SUCCESS) { + dns_validator_detach(&val); return (result); } return (DNS_R_WAIT); @@ -1668,6 +1638,7 @@ get_dsset(dns_validator_t *val, dns_name_t *tname, isc_result_t *resp) { fetch_callback_ds, "validate_dnskey"); *resp = DNS_R_WAIT; if (result != ISC_R_SUCCESS) { + dns_validator_detach(&val); *resp = result; } return (ISC_R_COMPLETE); @@ -2923,7 +2894,6 @@ out: static void validator_start(void *arg) { dns_validator_t *val = (dns_validator_t *)arg; - bool want_destroy = false; isc_result_t result = ISC_R_FAILURE; if (CANCELED(val)) { @@ -3014,21 +2984,19 @@ validator_start(void *arg) { } if (result != DNS_R_WAIT) { - want_destroy = exit_check(val); validator_done(val, result); } UNLOCK(&val->lock); - if (want_destroy) { - destroy(val); - } + + dns_validator_detach(&val); } isc_result_t dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type, dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset, dns_message_t *message, unsigned int options, - isc_loop_t *loop, isc_job_cb cb, void *arg, + isc_loopmgr_t *loopmgr, isc_job_cb cb, void *arg, dns_validator_t **validatorp) { isc_result_t result = ISC_R_FAILURE; dns_validator_t *val = NULL; @@ -3046,9 +3014,12 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type, .type = type, .options = options, .link = ISC_LINK_INITIALIZER, - .loop = loop, + .loopmgr = loopmgr, .cb = cb, .arg = arg }; + + isc_refcount_init(&val->references, 1); + if (message != NULL) { dns_message_attach(message, &val->message); } @@ -3070,7 +3041,8 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type, val->magic = VALIDATOR_MAGIC; if ((options & DNS_VALIDATOR_DEFER) == 0) { - isc_async_run(loop, validator_start, val); + dns_validator_ref(val); + isc_job_run(val->loopmgr, validator_start, val); } *validatorp = val; @@ -3098,7 +3070,8 @@ dns_validator_send(dns_validator_t *validator) { validator->options &= ~DNS_VALIDATOR_DEFER; UNLOCK(&validator->lock); - isc_async_run(validator->loop, validator_start, validator); + dns_validator_ref(validator); + isc_job_run(validator->loopmgr, validator_start, validator); } void @@ -3127,11 +3100,11 @@ dns_validator_cancel(dns_validator_t *validator) { } static void -destroy(dns_validator_t *val) { +destroy_validator(dns_validator_t *val) { isc_mem_t *mctx = NULL; - REQUIRE(SHUTDOWN(val)); REQUIRE(val->fetch == NULL); + REQUIRE(val->subvalidator == NULL); val->magic = 0; if (val->key != NULL) { @@ -3159,7 +3132,6 @@ destroy(dns_validator_t *val) { void dns_validator_destroy(dns_validator_t **validatorp) { dns_validator_t *val = NULL; - bool want_destroy = false; REQUIRE(validatorp != NULL); @@ -3169,15 +3141,10 @@ dns_validator_destroy(dns_validator_t **validatorp) { REQUIRE(VALID_VALIDATOR(val)); LOCK(&val->lock); - - val->attributes |= VALATTR_SHUTDOWN; validator_log(val, ISC_LOG_DEBUG(4), "dns_validator_destroy"); - - want_destroy = exit_check(val); UNLOCK(&val->lock); - if (want_destroy) { - destroy(val); - } + + dns_validator_detach(&val); } static void @@ -3256,3 +3223,9 @@ validator_logcreate(dns_validator_t *val, dns_name_t *name, validator_log(val, ISC_LOG_DEBUG(9), "%s: creating %s for %s %s", caller, operation, namestr, typestr); } + +#if DNS_VALIDATOR_TRACE +ISC_REFCOUNT_TRACE_IMPL(dns_validator, destroy_validator); +#else +ISC_REFCOUNT_IMPL(dns_validator, destroy_validator); +#endif From 1ee30be7ce07a23c7325fb448e628eac1265c905 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 16 Feb 2023 11:05:39 -0800 Subject: [PATCH 3/4] remove validator lock as every validator function is loop-synchronized, it should no longer be necessary to use a validator lock. calling dns_validator_send(), dns_validator_cancel() or dns_validator_destroy() from a thread other than the one on which the validator is running will now cause an assertion failure; this should be fine since the validator and resolver are tightly coupled, and the fetch contexts and validators run in the same loops. --- lib/dns/include/dns/validator.h | 6 +----- lib/dns/validator.c | 37 ++++++--------------------------- 2 files changed, 7 insertions(+), 36 deletions(-) diff --git a/lib/dns/include/dns/validator.h b/lib/dns/include/dns/validator.h index ccd4d9405e..b1c743e6f5 100644 --- a/lib/dns/include/dns/validator.h +++ b/lib/dns/include/dns/validator.h @@ -50,7 +50,6 @@ #include #include -#include #include #include @@ -73,12 +72,10 @@ * whatever purpose the client desires. */ struct dns_validator { - /* Unlocked: */ unsigned int magic; - isc_mutex_t lock; dns_view_t *view; isc_loopmgr_t *loopmgr; - + uint32_t tid; isc_refcount_t references; /* Name and type of the response to be validated. */ @@ -92,7 +89,6 @@ struct dns_validator { isc_job_cb cb; void *arg; - /* Locked by 'lock': */ /* Validation options (_DEFER, _NONTA, etc). */ unsigned int options; diff --git a/lib/dns/validator.c b/lib/dns/validator.c index 075d28ad72..6e2efe1474 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -378,7 +379,6 @@ fetch_callback_dnskey(void *arg) { isc_mem_putanddetach(&resp->mctx, resp, sizeof(*resp)); validator_log(val, ISC_LOG_DEBUG(3), "in fetch_callback_dnskey"); - LOCK(&val->lock); fetch = val->fetch; val->fetch = NULL; if (CANCELED(val)) { @@ -428,7 +428,6 @@ fetch_callback_dnskey(void *arg) { validator_done(val, DNS_R_BROKENCHAIN); } } - UNLOCK(&val->lock); if (fetch != NULL) { dns_resolver_destroyfetch(&fetch); @@ -471,7 +470,6 @@ fetch_callback_ds(void *arg) { } validator_log(val, ISC_LOG_DEBUG(3), "in fetch_callback_ds"); - LOCK(&val->lock); fetch = val->fetch; val->fetch = NULL; @@ -577,7 +575,6 @@ fetch_callback_ds(void *arg) { done: isc_mem_putanddetach(&resp->mctx, resp, sizeof(*resp)); - UNLOCK(&val->lock); if (fetch != NULL) { dns_resolver_destroyfetch(&fetch); @@ -603,7 +600,6 @@ validator_callback_dnskey(void *arg) { subvalidator->parent = NULL; validator_log(val, ISC_LOG_DEBUG(3), "in validator_callback_dnskey"); - LOCK(&val->lock); if (CANCELED(val)) { validator_done(val, ISC_R_CANCELED); } else if (eresult == ISC_R_SUCCESS) { @@ -640,8 +636,6 @@ validator_callback_dnskey(void *arg) { validator_done(val, DNS_R_BROKENCHAIN); } - UNLOCK(&val->lock); - dns_validator_destroy(&subvalidator); dns_validator_detach(&val); } @@ -662,7 +656,6 @@ validator_callback_ds(void *arg) { subvalidator->parent = NULL; validator_log(val, ISC_LOG_DEBUG(3), "in validator_callback_ds"); - LOCK(&val->lock); if (CANCELED(val)) { validator_done(val, ISC_R_CANCELED); } else if (eresult == ISC_R_SUCCESS) { @@ -699,7 +692,6 @@ validator_callback_ds(void *arg) { isc_result_totext(eresult)); validator_done(val, DNS_R_BROKENCHAIN); } - UNLOCK(&val->lock); dns_validator_destroy(&subvalidator); dns_validator_detach(&val); @@ -722,7 +714,6 @@ validator_callback_cname(void *arg) { val->subvalidator = NULL; validator_log(val, ISC_LOG_DEBUG(3), "in validator_callback_cname"); - LOCK(&val->lock); if (CANCELED(val)) { validator_done(val, ISC_R_CANCELED); } else if (eresult == ISC_R_SUCCESS) { @@ -742,8 +733,6 @@ validator_callback_cname(void *arg) { validator_done(val, DNS_R_BROKENCHAIN); } - UNLOCK(&val->lock); - dns_validator_destroy(&subvalidator); dns_validator_detach(&val); } @@ -766,7 +755,6 @@ validator_callback_nsec(void *arg) { val->subvalidator = NULL; 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) { @@ -841,8 +829,6 @@ validator_callback_nsec(void *arg) { } } - UNLOCK(&val->lock); - dns_validator_destroy(&subvalidator); dns_validator_detach(&val); } @@ -2902,8 +2888,6 @@ validator_start(void *arg) { validator_log(val, ISC_LOG_DEBUG(3), "starting"); - LOCK(&val->lock); - if (val->rdataset != NULL && val->sigrdataset != NULL) { isc_result_t saved_result; @@ -2987,8 +2971,6 @@ validator_start(void *arg) { validator_done(val, result); } - UNLOCK(&val->lock); - dns_validator_detach(&val); } @@ -3007,7 +2989,8 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type, REQUIRE(validatorp != NULL && *validatorp == NULL); val = isc_mem_get(view->mctx, sizeof(*val)); - *val = (dns_validator_t){ .result = ISC_R_FAILURE, + *val = (dns_validator_t){ .tid = isc_tid(), + .result = ISC_R_FAILURE, .rdataset = rdataset, .sigrdataset = sigrdataset, .name = name, @@ -3024,7 +3007,6 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type, dns_message_attach(message, &val->message); } dns_view_attach(view, &val->view); - isc_mutex_init(&val->lock); result = dns_view_getsecroots(val->view, &val->keytable); if (result != ISC_R_SUCCESS) { @@ -3050,7 +3032,6 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type, return (ISC_R_SUCCESS); cleanup: - isc_mutex_destroy(&val->lock); if (val->message != NULL) { dns_message_detach(&val->message); } @@ -3063,12 +3044,10 @@ cleanup: void dns_validator_send(dns_validator_t *validator) { REQUIRE(VALID_VALIDATOR(validator)); - - LOCK(&validator->lock); + REQUIRE(validator->tid == isc_tid()); INSIST((validator->options & DNS_VALIDATOR_DEFER) != 0); validator->options &= ~DNS_VALIDATOR_DEFER; - UNLOCK(&validator->lock); dns_validator_ref(validator); isc_job_run(validator->loopmgr, validator_start, validator); @@ -3077,8 +3056,7 @@ dns_validator_send(dns_validator_t *validator) { void dns_validator_cancel(dns_validator_t *validator) { REQUIRE(VALID_VALIDATOR(validator)); - - LOCK(&validator->lock); + REQUIRE(validator->tid == isc_tid()); validator_log(validator, ISC_LOG_DEBUG(3), "dns_validator_cancel"); @@ -3096,7 +3074,6 @@ dns_validator_cancel(dns_validator_t *validator) { validator->attributes |= VALATTR_CANCELED; } - UNLOCK(&validator->lock); } static void @@ -3121,7 +3098,6 @@ destroy_validator(dns_validator_t *val) { if (val->siginfo != NULL) { isc_mem_put(mctx, val->siginfo, sizeof(*val->siginfo)); } - isc_mutex_destroy(&val->lock); dns_view_detach(&val->view); if (val->message != NULL) { dns_message_detach(&val->message); @@ -3139,10 +3115,9 @@ dns_validator_destroy(dns_validator_t **validatorp) { *validatorp = NULL; REQUIRE(VALID_VALIDATOR(val)); + REQUIRE(val->tid == isc_tid()); - LOCK(&val->lock); validator_log(val, ISC_LOG_DEBUG(4), "dns_validator_destroy"); - UNLOCK(&val->lock); dns_validator_detach(&val); } From e49350721fa6bc5f8835d4466bda77e08ca5f9d6 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 16 Feb 2023 11:32:20 -0800 Subject: [PATCH 4/4] CHANGES for [GL #3797] --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index 043d04185d..121b111af2 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +6103. [func] All uses of the isc_task and isc_event APIs have + been refactored to use isc_loop instead, and the + original APIs have been removed. [GL #3797] + 6102. [cleanup] Several nugatory headers have been removed from libisc. [GL !7464]