Skip to content

Stop resolving invalid names in resume_dslookup()

Michał Kępień requested to merge 3439-stop-resolving-invalid-names-in-resume_dslookup into main

Commit 7b2ea97e 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.

Moved from isc-private/bind9!411

Closes #3439 (closed)

Merge request reports