From: Ondřej Surý Date: Mon, 29 Jun 2026 06:48:03 +0000 (+0200) Subject: Add a regression test for the find_zone_keys() key leak X-Git-Tag: v9.21.24~37^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3d0c452cadc34b2dc09c4019ced10a35655aaf3b;p=thirdparty%2Fbind9.git Add a regression test for the find_zone_keys() key leak Generate more than DNS_MAXZONEKEYS distinct matching private keys for a zone and call find_zone_keys() through it. The keys past the limit must be released; the default memory context's leak check, armed with isc_mem_checkdestroyed(), fails the test if any are abandoned. Assisted-by: Claude:claude-opus-4-8 --- diff --git a/tests/dns/update_test.c b/tests/dns/update_test.c index a8f65f9f057..14c6f6be36f 100644 --- a/tests/dns/update_test.c +++ b/tests/dns/update_test.c @@ -12,6 +12,7 @@ */ #include +#include #include /* IWYU pragma: keep */ #include #include @@ -24,13 +25,25 @@ #define UNIT_TESTING #include +#include #include +#include +#include #include #include #include +#include +#include +#include #include +#include +#include #include +#include +#include + +#include #define KEEP_BEFORE /* @@ -292,6 +305,128 @@ ISC_RUN_TEST_IMPL(future_to_date) { assert_int_equal(dns_updatemethod_increment, used); } +/* Remove every file in 'keydir' and then the directory itself. */ +static void +cleanup_keydir(const char *keydir) { + isc_dir_t dir; + isc_result_t result; + + isc_dir_init(&dir); + result = isc_dir_open(&dir, keydir); + if (result != ISC_R_SUCCESS) { + return; + } + while (isc_dir_read(&dir) == ISC_R_SUCCESS) { + char path[PATH_MAX]; + + if (strcmp(dir.entry.name, ".") == 0 || + strcmp(dir.entry.name, "..") == 0) + { + continue; + } + snprintf(path, sizeof(path), "%s/%s", keydir, dir.entry.name); + (void)remove(path); + } + isc_dir_close(&dir); + (void)rmdir(keydir); +} + +/* + * Regression test for find_zone_keys() (GL #6051): when more than + * DNS_MAXZONEKEYS matching private keys are present, the keys beyond the + * limit must be released rather than leaked. Before the fix the function + * destroyed only the first over-limit key and then broke out of the loop, + * abandoning every key after it on a local list. + */ +ISC_LOOP_TEST_IMPL(find_zone_keys_overflow) { + isc_result_t result; + dns_zonemgr_t *mgr = NULL; + dns_zone_t *zone = NULL; + dns_fixedname_t fname; + dns_name_t *name = NULL; + dst_key_t *keys[DNS_MAXZONEKEYS] = { NULL }; + unsigned int nkeys = 0; + /* A few keys past the limit, so a tail survives the overflow entry. */ + const unsigned int total = DNS_MAXZONEKEYS + 3; + uint16_t ids[DNS_MAXZONEKEYS + 3]; + unsigned int generated = 0; + char keydir[] = BUILDDIR "/find_zone_keys.XXXXXX"; + + assert_non_null(mkdtemp(keydir)); + + name = dns_fixedname_initname(&fname); + result = dns_name_fromstring(name, "example.", dns_rootname, 0, NULL); + assert_int_equal(result, ISC_R_SUCCESS); + + /* + * Generate 'total' distinct zone keys into the temporary key + * directory. Skip the rare key-tag collision so each key gets its own + * K*.private file instead of overwriting an earlier one. + */ + while (generated < total) { + dst_key_t *key = NULL; + uint16_t id; + bool dup = false; + + result = dst_key_generate( + name, DST_ALG_ECDSA256, 256, 0, DNS_KEYOWNER_ZONE, + DNS_KEYPROTO_DNSSEC, dns_rdataclass_in, NULL, + isc_g_mctx, &key, NULL); + assert_int_equal(result, ISC_R_SUCCESS); + + id = dst_key_id(key); + for (unsigned int i = 0; i < generated; i++) { + if (ids[i] == id) { + dup = true; + break; + } + } + if (dup) { + dst_key_free(&key); + continue; + } + + result = dst_key_tofile(key, DST_TYPE_PUBLIC | DST_TYPE_PRIVATE, + keydir); + assert_int_equal(result, ISC_R_SUCCESS); + ids[generated++] = id; + dst_key_free(&key); + } + + /* find_zone_keys() reads the keystore list off the zone manager. */ + dns_zonemgr_create(isc_g_mctx, &mgr); + result = dns_test_makezone("example", &zone, NULL, false); + assert_int_equal(result, ISC_R_SUCCESS); + result = dns_zonemgr_managezone(mgr, zone); + assert_int_equal(result, ISC_R_SUCCESS); + dns_zone_setkeydirectory(zone, keydir); + + result = find_zone_keys(zone, isc_g_mctx, DNS_MAXZONEKEYS, keys, + &nkeys); + assert_int_equal(result, ISC_R_NOSPACE); + assert_int_equal(nkeys, DNS_MAXZONEKEYS); + + /* + * Free the keys handed back to the caller. Any over-limit key that + * find_zone_keys() failed to drain stays allocated in the default + * memory context and is reported by its end-of-run leak check. + */ + for (unsigned int i = 0; i < nkeys; i++) { + dst_key_free(&keys[i]); + } + + dns_zonemgr_releasezone(mgr, zone); + dns_zone_detach(&zone); + dns_zonemgr_shutdown(mgr); + dns_zonemgr_detach(&mgr); + + cleanup_keydir(keydir); + + isc_mem_checkdestroyed(stderr); + + isc_loopmgr_shutdown(); +} + ISC_TEST_LIST_START ISC_TEST_ENTRY_CUSTOM(increment, setup_test, NULL) ISC_TEST_ENTRY_CUSTOM(increment_past_zero, setup_test, NULL) @@ -305,6 +440,8 @@ ISC_TEST_ENTRY_CUSTOM(unixtime_zero, setup_test, NULL) ISC_TEST_ENTRY_CUSTOM(past_to_date, setup_test, NULL) ISC_TEST_ENTRY_CUSTOM(now_to_date, setup_test, NULL) ISC_TEST_ENTRY_CUSTOM(future_to_date, setup_test, NULL) +ISC_TEST_ENTRY_CUSTOM(find_zone_keys_overflow, setup_managers, + teardown_managers) ISC_TEST_LIST_END ISC_TEST_MAIN