From: Ondřej Surý Date: Thu, 9 Apr 2026 10:06:31 +0000 (+0200) Subject: Fix delegation database NOEXACT lookup for top-level names X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9191dc7acbdf4a6a4a6a06cedad2d90baed0fcee;p=thirdparty%2Fbind9.git Fix delegation database NOEXACT lookup for top-level names dns__deleg_lookup() with DNS_DBFIND_NOEXACT is supposed to return the deepest proper ancestor of the lookup name. It called getparentnode() to step up from an exact match, but getparentnode() only iterated while the chain length was >= 2. When the chain contained a single entry (the exact match itself with no ancestor stored in the trie), the loop did not execute and left the caller looking at the exact match. The subsequent isactive() check then returned success and the function reported the exact match as the "deepest ancestor", violating NOEXACT semantics. This was observable as the resolver picking the child-side delegation for an at-parent type (e.g. a DS query for a TLD), then sending the query to the child's own nameservers and recovering via the "chase DS servers" path. Have getparentnode() set '*node' to NULL when it cannot find an active proper ancestor, and make dns__deleg_lookup() NULL-check before returning, matching the canonical NOEXACT implementation in dns_zt_find(). Update the deleg unit test to expect NOTFOUND for the top-level-no-parent case. --- diff --git a/lib/dns/deleg.c b/lib/dns/deleg.c index dc7f5a42dac..28fb777ef88 100644 --- a/lib/dns/deleg.c +++ b/lib/dns/deleg.c @@ -267,14 +267,22 @@ getparentnode(dns_qpchain_t *chain, delegdb_node_t **node, dns_ttl_t now) { size_t len = dns_qpchain_length(chain); while (len >= 2) { - *node = NULL; - dns_qpchain_node(chain, len - 2, (void **)node, NULL); + delegdb_node_t *parent = NULL; + dns_qpchain_node(chain, len - 2, (void **)&parent, NULL); - if (isactive(*node, now)) { - break; + if (isactive(parent, now)) { + *node = parent; + return; } len--; } + + /* + * No active proper ancestor was found in the chain. Signal + * "no parent" so the caller does not mistake the original + * matched node for an ancestor. + */ + *node = NULL; } /* @@ -309,27 +317,36 @@ dns__deleg_lookup(dns_delegdb_t *delegdb, dns_qpread_t *qpr, dns_name_copy(&node->zonecut, deepestzonecut); } - if (result == ISC_R_SUCCESS && (noexact || !isactive(node, now))) { - getparentnode(&chain, &node, now); - } else if (result == DNS_R_PARTIALMATCH && !isactive(node, now)) { + /* + * Walk up the chain when: + * - we have an exact match but the caller asked for NOEXACT + * (i.e. the caller wants the deepest *proper* ancestor), or + * - the matched node is no longer active and we need to fall + * back to the closest still-active ancestor (this applies + * equally to exact and partial matches). + * + * getparentnode() sets 'node' to NULL when no active ancestor + * exists in the chain, so we must NULL-check before dereferencing + * 'node' below. + */ + if ((result == ISC_R_SUCCESS && noexact) || !isactive(node, now)) { getparentnode(&chain, &node, now); } - result = isactive(node, now) ? ISC_R_SUCCESS : ISC_R_NOTFOUND; - if (result == ISC_R_SUCCESS) { + if (node != NULL && isactive(node, now)) { dns_name_copy(&node->zonecut, zonecut); INSIST(node->delegset); dns_delegset_attach(node->delegset, delegsetp); ISC_SIEVE_MARK(node, visited); - } else { - /* - * FIXME: if we lookup something that has expired, we need - * either the "deadnodes" (see qpcache) mechanism here - or call - * something like isc_async_run(delete_me, node). - */ + return ISC_R_SUCCESS; } - return result; + /* + * FIXME: if we lookup something that has expired, we need + * either the "deadnodes" (see qpcache) mechanism here - or call + * something like isc_async_run(delete_me, node). + */ + return ISC_R_NOTFOUND; } isc_result_t diff --git a/tests/dns/deleg_test.c b/tests/dns/deleg_test.c index 904eaeef00c..e27b7b1eb74 100644 --- a/tests/dns/deleg_test.c +++ b/tests/dns/deleg_test.c @@ -124,13 +124,16 @@ lookupdb(dns_delegdb_t *db, const char *namestr, isc_stdtime_t now, *expectedzc = dns_fixedname_initname(&fexpectedzc), *zonecut = dns_fixedname_initname(&fzonecut); - dns_name_fromstring(expectedzc, expectedzcstr, NULL, 0, NULL); + if (expectedzcstr != NULL) { + dns_name_fromstring(expectedzc, expectedzcstr, NULL, 0, NULL); + } dns_name_fromstring(name, namestr, NULL, 0, NULL); result = dns_delegdb_lookup(db, name, now, options, zonecut, NULL, delegsetp); if (result == ISC_R_SUCCESS) { assert_non_null(*delegsetp); + assert_non_null(expectedzcstr); assert_true(dns_name_equal(zonecut, expectedzc)); } else { assert_null(*delegsetp); @@ -411,15 +414,22 @@ noexacttests(ISC_ATTR_UNUSED void *arg) { const char *name; const char *expected; const char *noexactexpected; + isc_result_t noexactresult; dns_ttl_t ttl; } zonecuts[] = { - { "stuff.", "stuff.", "stuff.", 30 }, - { "foo.stuff.", "foo.stuff.", "stuff.", 30 }, - { "expired.foo.stuff.", "foo.stuff.", "foo.stuff.", 1 }, + /* + * "stuff." has no proper ancestor in the trie, so a + * NOEXACT lookup must return NOTFOUND rather than the + * exact match itself. + */ + { "stuff.", "stuff.", NULL, ISC_R_NOTFOUND, 30 }, + { "foo.stuff.", "foo.stuff.", "stuff.", ISC_R_SUCCESS, 30 }, + { "expired.foo.stuff.", "foo.stuff.", "foo.stuff.", + ISC_R_SUCCESS, 1 }, { "bar.expired.foo.stuff.", "bar.expired.foo.stuff.", - "foo.stuff.", 30 }, + "foo.stuff.", ISC_R_SUCCESS, 30 }, { "baz.bar.expired.foo.stuff.", "baz.bar.expired.foo.stuff.", - "bar.expired.foo.stuff.", 30 } + "bar.expired.foo.stuff.", ISC_R_SUCCESS, 30 } }; for (size_t i = 0; i < ARRAY_SIZE(zonecuts); i++) { @@ -440,8 +450,10 @@ noexacttests(ISC_ATTR_UNUSED void *arg) { result = lookupdb(db, zonecuts[i].name, now + 1, DNS_DBFIND_NOEXACT, zonecuts[i].noexactexpected, &delegset); - assert_int_equal(result, ISC_R_SUCCESS); - dns_delegset_detach(&delegset); + assert_int_equal(result, zonecuts[i].noexactresult); + if (result == ISC_R_SUCCESS) { + dns_delegset_detach(&delegset); + } } result = lookupdb(db, "gee.expired.foo.stuff.", now + 1, 0,