]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Add a regression test for the find_zone_keys() key leak
authorOndřej Surý <ondrej@isc.org>
Mon, 29 Jun 2026 06:48:03 +0000 (08:48 +0200)
committerMark Andrews <marka@isc.org>
Wed, 1 Jul 2026 06:45:56 +0000 (16:45 +1000)
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
tests/dns/update_test.c

index a8f65f9f057a14904dd52bc6a89b91a33f3427a6..14c6f6be36f62fe916f73b52d7e109cd8b62c3de 100644 (file)
@@ -12,6 +12,7 @@
  */
 
 #include <inttypes.h>
+#include <limits.h>
 #include <sched.h> /* IWYU pragma: keep */
 #include <setjmp.h>
 #include <stdarg.h>
 #define UNIT_TESTING
 #include <cmocka.h>
 
+#include <isc/dir.h>
 #include <isc/lib.h>
+#include <isc/loop.h>
+#include <isc/mem.h>
 #include <isc/serial.h>
 #include <isc/stdtime.h>
 #include <isc/util.h>
 
+#include <dns/dnssec.h>
+#include <dns/fixedname.h>
+#include <dns/keyvalues.h>
 #include <dns/lib.h>
+#include <dns/name.h>
+#include <dns/rdataclass.h>
 #include <dns/update.h>
+#include <dns/zone.h>
+#include <dns/zonemgr.h>
+
+#include <dst/dst.h>
 #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