mir/bind
2
0
Fork 0
mirror of https://gitlab.isc.org/isc-projects/bind9 synced 2025-08-30 05:57:52 +00:00
Code Issues Releases Wiki Activity

Stop resolving invalid names in resume_dslookup()

Browse Source 
Commit 7b2ea97e46034ec3db4c950100708297798826af introduced a logic bug
in resume_dslookup(): that function now only conditionally checks
whether DS chasing can still make progress.  Specifically, that check is
only performed when the previous resume_dslookup() call invokes
dns_resolver_createfetch() with the 'nameservers' argument set to
something else than NULL, which may not always be the case.  Failing to
perform that check may trigger assertion failures as a result of
dns_resolver_createfetch() attempting to resolve an invalid name.

Example scenario that leads to such outcome:

 1. A validating resolver is configured to forward all queries to
    another resolver.  The latter returns broken DS responses that
    trigger DS chasing.

 2. rctx_chaseds() calls dns_resolver_createfetch() with the
    'nameservers' argument set to NULL.

 3. The fetch fails, so resume_dslookup() is called.  Due to
    fevent->result being set to e.g. DNS_R_SERVFAIL, the default branch
    is taken in the switch statement.

 4. Since 'nameservers' was set to NULL for the fetch which caused the
    resume_dslookup() callback to be invoked
    (fctx->nsfetch->private->nameservers), resume_dslookup() chops off
    one label off fctx->nsname and calls dns_resolver_createfetch()
    again, for a name containing one label less than before.

 5. Steps 3-4 are repeated (i.e. all attempts to find the name servers
    authoritative for the DS RRset being chased fail) until fctx->nsname
    becomes stripped down the the root name.

 6. Since resume_dslookup() does not check whether DS chasing can still
    make progress, it strips off a label off the root name and continues
    its attempts at finding the name servers authoritative for the DS
    RRset being chased, passing an invalid name to
    dns_resolver_createfetch().

Fix by ensuring resume_dslookup() always checks whether DS chasing can
still make progress when a name server fetch fails.  Update code
comments to ensure the purpose of the relevant dns_name_equal() check is
clear.
This commit is contained in:
Michał Kępień 2022-07-13 10:31:16 +02:00
parent 75027bc6ce
commit 1a79aeab44
1 changed files with 21 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
lib/dns/resolver.c
View File

@ -7343,22 +7343,34 @@ resume_dslookup(isc_task_t *task, isc_event_t *event) {
}
/*
* Get domain and nameservers from fctx->nsfetch
* before we destroy it.
* Get domain from fctx->nsfetch before we destroy it.
*/
domain = dns_fixedname_initname(&fixed);
dns_name_copy(fctx->nsfetch->private->domain, domain);
/*
* If the chain of resume_dslookup() invocations managed to
* chop off enough labels from the original DS owner name to
* reach the top of the namespace, no further progress can be
* made. Interrupt the DS chasing process, returning SERVFAIL.
*/
if (dns_name_equal(fctx->nsname, domain)) {
dns_resolver_destroyfetch(&fctx->nsfetch);
fctx_done_detach(&fctx, DNS_R_SERVFAIL);
return;
}
/*
* Get nameservers from fctx->nsfetch before we destroy it.
*/
dns_rdataset_init(&nameservers);
if (dns_rdataset_isassociated(
&fctx->nsfetch->private->nameservers)) {
domain = dns_fixedname_initname(&fixed);
dns_name_copy(fctx->nsfetch->private->domain, domain);
if (dns_name_equal(fctx->nsname, domain)) {
dns_resolver_destroyfetch(&fctx->nsfetch);
fctx_done_detach(&fctx, DNS_R_SERVFAIL);
return;
}
dns_rdataset_clone(&fctx->nsfetch->private->nameservers,
&nameservers);
nsrdataset = &nameservers;
} else {
domain = NULL;
}
dns_resolver_destroyfetch(&fctx->nsfetch);