From aa5940281af3719fa1e87fee07b3d7b8194bf9ea Mon Sep 17 00:00:00 2001 From: Andreas Gustafsson Date: Wed, 19 Jul 2000 23:19:05 +0000 Subject: [PATCH] When handling the response to an ANY query in a secure zone, deal with the multiple answer RRsets by validating each one separately. Also, eliminated the "done" variable in answer_response() because in the rare situations where it got set to ISC_TRUE, it caused the function to return prematurely by exiting a loop with a result of ISC_R_SUCCESS and hitting a "if (result != ISC_R_NOMORE) return (result);" test immediately following following the loop. This should fix [RT #109], "ANY query in secure zone crashes server". --- lib/dns/resolver.c | 111 ++++++++++++++++++++++++++++++++------------- 1 file changed, 79 insertions(+), 32 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 46faf809df..9bb3380b9a 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -15,7 +15,7 @@ * SOFTWARE. */ -/* $Id: resolver.c,v 1.149 2000/07/16 18:26:18 gson Exp $ */ +/* $Id: resolver.c,v 1.150 2000/07/19 23:19:05 gson Exp $ */ #include @@ -170,7 +170,6 @@ struct fetchctx { dns_adbaddrinfolist_t forwaddrs; isc_sockaddrlist_t forwarders; isc_sockaddrlist_t bad; - dns_validator_t * validator; /* * # of events we're waiting for. */ @@ -678,7 +677,6 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, if (result != ISC_R_SUCCESS) return (result); - INSIST(fctx->validator == NULL); /* Validator needs rmessage. */ dns_message_reset(fctx->rmessage, DNS_MESSAGE_INTENTPARSE); query = isc_mem_get(res->mctx, sizeof *query); @@ -1693,7 +1691,6 @@ fctx_destroy(fetchctx_t *fctx) { REQUIRE(fctx->pending == 0); REQUIRE(fctx->validating == 0); REQUIRE(fctx->references == 0); - REQUIRE(fctx->validator == NULL); FCTXTRACE("destroy"); @@ -2049,7 +2046,6 @@ fctx_create(dns_resolver_t *res, dns_name_t *name, dns_rdatatype_t type, ISC_LIST_INIT(fctx->forwaddrs); ISC_LIST_INIT(fctx->forwarders); ISC_LIST_INIT(fctx->bad); - fctx->validator = NULL; fctx->find = NULL; fctx->pending = 0; fctx->validating = 0; @@ -2270,6 +2266,7 @@ validated(isc_task_t *task, isc_event_t *event) { dns_rdataset_t *ardataset = NULL; dns_rdataset_t *asigrdataset = NULL; dns_dbnode_t *node = NULL; + isc_boolean_t negative; UNUSED(task); /* for now */ @@ -2279,17 +2276,18 @@ validated(isc_task_t *task, isc_event_t *event) { REQUIRE(fctx->validating > 0); vevent = (dns_validatorevent_t *)event; - INSIST(vevent->validator == fctx->validator); - fctx->validating--; + FCTXTRACE("received validation completion event"); - INSIST(fctx->validating == 0); - /* * Destroy the validator early so that we can * destroy the fctx if necessary. */ - dns_validator_destroy(&fctx->validator); + dns_validator_destroy(&vevent->validator); + + fctx->validating--; + + negative = ISC_TF(vevent->rdataset == NULL); /* * If shutting down, ignore the results. Check to see if we're @@ -2304,12 +2302,20 @@ validated(isc_task_t *task, isc_event_t *event) { /* * We're not shutting down. */ - FCTXTRACE("received validation completion event"); hevent = ISC_LIST_HEAD(fctx->events); if (hevent != NULL) { - ardataset = hevent->rdataset; - asigrdataset = hevent->sigrdataset; + if (!negative && + (fctx->type == dns_rdatatype_any || + fctx->type == dns_rdatatype_sig)) { + /* + * Don't bind rdatasets; the caller + * will iterate the node. + */ + } else { + ardataset = hevent->rdataset; + asigrdataset = hevent->sigrdataset; + } } if (vevent->result != ISC_R_SUCCESS) { @@ -2320,7 +2326,7 @@ validated(isc_task_t *task, isc_event_t *event) { isc_stdtime_get(&now); - if (vevent->rdataset == NULL) { + if (negative) { dns_rdatatype_t covers; FCTXTRACE("nonexistence validation OK"); @@ -2374,7 +2380,19 @@ validated(isc_task_t *task, isc_event_t *event) { result != DNS_R_UNCHANGED) goto noanswer_response; } - + + if (fctx->validating > 0) { + INSIST(!negative); + INSIST(fctx->type == dns_rdatatype_any || + fctx->type == dns_rdatatype_sig); + /* + * Don't send a response yet - we have + * more rdatasets that still need to + * be validated. + */ + goto cleanup_event; + } + result = ISC_R_SUCCESS; answer_response: @@ -2419,6 +2437,7 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, isc_stdtime_t now) { dns_fetchevent_t *event; unsigned int options; isc_task_t *task; + dns_validator_t *validator; /* * The appropriate bucket lock must be held. @@ -2428,6 +2447,7 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, isc_stdtime_t now) { need_validation = ISC_FALSE; have_answer = ISC_FALSE; eresult = ISC_R_SUCCESS; + task = res->buckets[fctx->bucketnum].task; /* * Is DNSSEC validation required for this name? @@ -2550,16 +2570,45 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, isc_stdtime_t now) { break; } if (ANSWER(rdataset)) { - /* - * This is the answer. We will - * validate it, but first we cache - * the rest of the response - it may - * contain useful keys. - */ - INSIST(valrdataset == NULL && - valsigrdataset == NULL); - valrdataset = rdataset; - valsigrdataset = sigrdataset; + if (fctx->type != dns_rdatatype_any && + fctx->type != dns_rdatatype_sig) { + /* + * This is The Answer. We will + * validate it, but first we cache + * the rest of the response - it may + * contain useful keys. + */ + INSIST(valrdataset == NULL && + valsigrdataset == NULL); + valrdataset = rdataset; + valsigrdataset = sigrdataset; + } else { + /* + * This is one of (potentially) + * multiple answers to an ANY + * or SIG query. To keep things + * simple, we just start the + * validator right away rather + * than caching first and + * having to remember which + * rdatasets needed validation. + */ + validator = NULL; + result = dns_validator_create( + res->view, + name, + rdataset->type, + rdataset, + sigrdataset, + fctx->rmessage, + 0, + task, + validated, + fctx, + &validator); + if (result == ISC_R_SUCCESS) + fctx->validating++; + } } } else if (!EXTERNAL(rdataset)) { /* @@ -2624,7 +2673,7 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, isc_stdtime_t now) { } if (valrdataset != NULL) { - task = res->buckets[fctx->bucketnum].task; + validator = NULL; result = dns_validator_create(res->view, name, fctx->type, @@ -2635,11 +2684,10 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, isc_stdtime_t now) { task, validated, fctx, - &fctx->validator); + &validator); if (result == ISC_R_SUCCESS) fctx->validating++; } - if (result == ISC_R_SUCCESS && have_answer) { fctx->attributes |= FCTX_ATTR_HAVEANSWER; @@ -2778,12 +2826,13 @@ ncache_message(fetchctx_t *fctx, dns_rdatatype_t covers, isc_stdtime_t now) { /* * Do negative response validation. */ + dns_validator_t *validator = NULL; isc_task_t *task = res->buckets[fctx->bucketnum].task; result = dns_validator_create(res->view, name, fctx->type, NULL, NULL, fctx->rmessage, 0, task, validated, fctx, - &fctx->validator); + &validator); if (result != ISC_R_SUCCESS) return (result); fctx->validating++; @@ -3227,7 +3276,6 @@ answer_response(fetchctx_t *fctx) { * part of the answer and should be cached. */ - done = ISC_FALSE; chaining = ISC_FALSE; have_answer = ISC_FALSE; want_chaining = ISC_FALSE; @@ -3238,7 +3286,7 @@ answer_response(fetchctx_t *fctx) { qname = &fctx->name; type = fctx->type; result = dns_message_firstname(message, DNS_SECTION_ANSWER); - while (!done && result == ISC_R_SUCCESS) { + while (result == ISC_R_SUCCESS) { name = NULL; dns_message_currentname(message, DNS_SECTION_ANSWER, &name); external = ISC_TF(!dns_name_issubdomain(name, &fctx->domain)); @@ -3255,7 +3303,6 @@ answer_response(fetchctx_t *fctx) { * We've found an ordinary answer. */ found = ISC_TRUE; - done = ISC_TRUE; aflag = DNS_RDATASETATTR_ANSWER; } else if (rdataset->type == dns_rdatatype_sig && rdataset->covers == type) {