Commit b3e55318e6 for bind
commit b3e55318e6f5af3ca86ced35febf52727999ae55
Author: Martin Basti <mbasti@isc.org>
Date: Mon Aug 31 17:51:45 2026 +0000
Use dst_algorithm_format for dst_key_alg() values
dst_key_alg() returns a 16-bit dst_algorithm_t, but several status
and log messages still passed it to dns_secalg_format, truncating
private algorithms to 8 bits: "rndc dnssec -status", the "zone_rekey
done" debug log, remove_key(), and two key-file error paths displayed
the wrong algorithm for PRIVATEOID based keys.
Also add DST_ALG_FORMATSIZE as the buffer size for
dst_algorithm_format(). Its output is always a mnemonic from the DST
algorithm table or a decimal number, never the embedded name or OID,
so 20 bytes covers every dst_algorithm_t value.
Assisted-by: Claude:claude-fable-5
diff --git a/lib/dns/dnssec.c b/lib/dns/dnssec.c
index ea3cdd78b5..9fa095eab7 100644
--- a/lib/dns/dnssec.c
+++ b/lib/dns/dnssec.c
@@ -1677,8 +1677,7 @@ dns_dnssec_keylistfromrdataset(const dns_name_t *origin, dns_kasp_t *kasp,
}
if (result != ISC_R_SUCCESS) {
- char filename[DNS_NAME_FORMATSIZE +
- DNS_SECALG_FORMATSIZE +
+ char filename[DNS_NAME_FORMATSIZE + DST_ALG_FORMATSIZE +
sizeof("key file for //65535")];
isc_result_t result2;
isc_buffer_t buf;
@@ -1692,12 +1691,12 @@ dns_dnssec_keylistfromrdataset(const dns_name_t *origin, dns_kasp_t *kasp,
NULL, mctx, &buf);
if (result2 != ISC_R_SUCCESS) {
char namebuf[DNS_NAME_FORMATSIZE];
- char algbuf[DNS_SECALG_FORMATSIZE];
+ char algbuf[DST_ALG_FORMATSIZE];
dns_name_format(dst_key_name(dnskey), namebuf,
sizeof(namebuf));
- dns_secalg_format(dst_key_alg(dnskey), algbuf,
- sizeof(algbuf));
+ dst_algorithm_format(dst_key_alg(dnskey),
+ algbuf, sizeof(algbuf));
snprintf(filename, sizeof(filename) - 1,
"key file for %s/%s/%d", namebuf,
algbuf, dst_key_id(dnskey));
@@ -1840,10 +1839,10 @@ remove_key(dns_diff_t *diff, dns_dnsseckey_t *key, const dns_name_t *origin,
isc_result_t result = ISC_R_SUCCESS;
unsigned char buf[DNS_RDATA_MAXLENGTH];
dns_rdata_t dnskey = DNS_RDATA_INIT;
- char alg[80];
+ char alg[DST_ALG_FORMATSIZE];
char namebuf[DNS_NAME_FORMATSIZE];
- dns_secalg_format(dst_key_alg(key->key), alg, sizeof(alg));
+ dst_algorithm_format(dst_key_alg(key->key), alg, sizeof(alg));
dns_name_format(dst_key_name(key->key), namebuf, sizeof(namebuf));
report("Removing %s key %s/%d/%s from DNSKEY RRset.", reason, namebuf,
dst_key_id(key->key), alg);
diff --git a/lib/dns/include/dst/dst.h b/lib/dns/include/dst/dst.h
index 6c5de0e429..3692ae26b2 100644
--- a/lib/dns/include/dst/dst.h
+++ b/lib/dns/include/dst/dst.h
@@ -995,6 +995,13 @@ dst_key_setprivateformat(dst_key_t *key, int major, int minor);
* "key" is a valid key.
*/
+/*%
+ * Buffer size for dst_algorithm_format(): the longest mnemonic in the
+ * DST algorithm table is 15 characters and unassigned values are
+ * printed as up to five decimal digits.
+ */
+#define DST_ALG_FORMATSIZE 20
+
#define DST_KEY_FORMATSIZE (DNS_NAME_FORMATSIZE + DNS_SECALG_FORMATSIZE + 7)
void
diff --git a/lib/dns/keymgr.c b/lib/dns/keymgr.c
index bcaec81882..59fd99a755 100644
--- a/lib/dns/keymgr.c
+++ b/lib/dns/keymgr.c
@@ -2873,7 +2873,7 @@ dns_keymgr_status(dns_kasp_t *kasp, dns_dnsseckeylist_t *keyring,
REQUIRE(buf != NULL);
ISC_LIST_FOREACH(*keyring, dkey, link) {
- char algstr[DNS_NAME_FORMATSIZE];
+ char algstr[DST_ALG_FORMATSIZE];
if (dst_key_is_unused(dkey->key)) {
continue;
@@ -2884,8 +2884,8 @@ dns_keymgr_status(dns_kasp_t *kasp, dns_dnsseckeylist_t *keyring,
}
// key data
- dns_secalg_format((dns_secalg_t)dst_key_alg(dkey->key), algstr,
- sizeof(algstr));
+ dst_algorithm_format(dst_key_alg(dkey->key), algstr,
+ sizeof(algstr));
CHECK(isc_buffer_printf(buf, "\n%s %d (%s):\n",
keymgr_keyrole(dkey->key),
dst_key_id(dkey->key), algstr));
diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index d1930727f8..ba3dd34ff3 100644
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -5118,8 +5118,7 @@ findzonekeys(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver,
}
if (result != ISC_R_SUCCESS) {
- char filename[DNS_NAME_FORMATSIZE +
- DNS_SECALG_FORMATSIZE +
+ char filename[DNS_NAME_FORMATSIZE + DST_ALG_FORMATSIZE +
sizeof("key file for //65535")];
isc_result_t result2;
isc_buffer_t buf;
@@ -5133,12 +5132,12 @@ findzonekeys(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver,
NULL, mctx, &buf);
if (result2 != ISC_R_SUCCESS) {
char namebuf[DNS_NAME_FORMATSIZE];
- char algbuf[DNS_SECALG_FORMATSIZE];
+ char algbuf[DST_ALG_FORMATSIZE];
dns_name_format(dst_key_name(pubkey), namebuf,
sizeof(namebuf));
- dns_secalg_format(dst_key_alg(pubkey), algbuf,
- sizeof(algbuf));
+ dst_algorithm_format(dst_key_alg(pubkey),
+ algbuf, sizeof(algbuf));
snprintf(filename, sizeof(filename) - 1,
"key file for %s/%s/%d", namebuf,
algbuf, dst_key_id(pubkey));
@@ -19294,9 +19293,9 @@ zone_rekey(dns_zone_t *zone) {
ISC_LIST_FOREACH(dnskeys, key, link) {
if (isc_log_wouldlog(ISC_LOG_DEBUG(3))) {
/* This debug log is used in the kasp system test */
- char algbuf[DNS_SECALG_FORMATSIZE];
- dns_secalg_format(dst_key_alg(key->key), algbuf,
- sizeof(algbuf));
+ char algbuf[DST_ALG_FORMATSIZE];
+ dst_algorithm_format(dst_key_alg(key->key), algbuf,
+ sizeof(algbuf));
dnssec_log(zone, ISC_LOG_DEBUG(3),
"zone_rekey done: key %d/%s",
dst_key_id(key->key), algbuf);
diff --git a/tests/dns/dnssec_test.c b/tests/dns/dnssec_test.c
new file mode 100644
index 0000000000..94fbe4dee5
--- /dev/null
+++ b/tests/dns/dnssec_test.c
@@ -0,0 +1,128 @@
+/*
+ * Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+ *
+ * SPDX-License-Identifier: MPL-2.0
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, you can obtain one at https://mozilla.org/MPL/2.0/.
+ *
+ * See the COPYRIGHT file distributed with this work for additional
+ * information regarding copyright ownership.
+ */
+
+#include <inttypes.h>
+#include <sched.h> /* IWYU pragma: keep */
+#include <setjmp.h>
+#include <stdarg.h>
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#define UNIT_TESTING
+#include <cmocka.h>
+
+#include <isc/buffer.h>
+#include <isc/formatcheck.h>
+#include <isc/lib.h>
+#include <isc/string.h>
+#include <isc/util.h>
+
+#include <dns/diff.h>
+#include <dns/dnssec.h>
+#include <dns/keyvalues.h>
+#include <dns/lib.h>
+
+#include <dst/dst.h>
+
+#include <tests/dns.h>
+
+static char report_output[BUFSIZ];
+
+static void
+ISC_FORMAT_PRINTF(1, 2) report(const char *fmt, ...) {
+ va_list ap;
+
+ va_start(ap, fmt);
+ vsnprintf(report_output, sizeof(report_output), fmt, ap);
+ va_end(ap);
+}
+
+/*
+ * remove_key() must report the full 16-bit DST algorithm; private
+ * algorithms used to be truncated to their 8-bit DNSSEC algorithm octet.
+ */
+ISC_RUN_TEST_IMPL(updatekeys_remove_privateoid) {
+ isc_result_t result;
+ dst_key_t *key = NULL;
+ dst_key_t *pubkey = NULL;
+ dns_dnsseckey_t *zonekey = NULL;
+ dns_dnsseckey_t *newkey = NULL;
+ dns_dnsseckeylist_t keys;
+ dns_dnsseckeylist_t newkeys;
+ dns_diff_t diff;
+ dns_fixedname_t fname;
+ dns_name_t *name;
+ isc_buffer_t buf;
+ unsigned char keydata[1024];
+
+ UNUSED(state);
+
+ if (!dst_algorithm_supported(DST_ALG_RSASHA256PRIVATEOID)) {
+ skip();
+ return;
+ }
+
+ dns_test_namefromstring("example.", &fname);
+ name = dns_fixedname_name(&fname);
+
+ result = dst_key_generate(name, DST_ALG_RSASHA256PRIVATEOID, 2048, 0,
+ DNS_KEYOWNER_ZONE, DNS_KEYPROTO_DNSSEC,
+ dns_rdataclass_in, NULL, isc_g_mctx, &key,
+ NULL);
+ assert_int_equal(result, ISC_R_SUCCESS);
+
+ /* Public copy of the same key, as if found in the zone. */
+ isc_buffer_init(&buf, keydata, sizeof(keydata));
+ result = dst_key_todns(key, &buf);
+ assert_int_equal(result, ISC_R_SUCCESS);
+ result = dst_key_fromdns(name, dns_rdataclass_in, &buf, isc_g_mctx,
+ &pubkey);
+ assert_int_equal(result, ISC_R_SUCCESS);
+ assert_int_equal(dst_key_alg(pubkey), DST_ALG_RSASHA256PRIVATEOID);
+
+ ISC_LIST_INIT(keys);
+ dns_dnsseckey_create(isc_g_mctx, &pubkey, &zonekey);
+ ISC_LIST_APPEND(keys, zonekey, link);
+
+ /* The key metadata says the key must be removed. */
+ ISC_LIST_INIT(newkeys);
+ dns_dnsseckey_create(isc_g_mctx, &key, &newkey);
+ newkey->hint_remove = true;
+ ISC_LIST_APPEND(newkeys, newkey, link);
+
+ dns_diff_init(isc_g_mctx, &diff);
+ report_output[0] = '\0';
+ result = dns_dnssec_updatekeys(&keys, &newkeys, NULL, name, 3600, &diff,
+ isc_g_mctx, report);
+ assert_int_equal(result, ISC_R_SUCCESS);
+
+ assert_non_null(strstr(report_output, "Removing expired key"));
+ assert_non_null(strstr(report_output, "/RSASHA256OID"));
+ assert_true(ISC_LIST_EMPTY(keys));
+
+ dns_diff_clear(&diff);
+ while (!ISC_LIST_EMPTY(newkeys)) {
+ dns_dnsseckey_t *dk = ISC_LIST_HEAD(newkeys);
+ ISC_LIST_UNLINK(newkeys, dk, link);
+ dns_dnsseckey_destroy(isc_g_mctx, &dk);
+ }
+}
+
+ISC_TEST_LIST_START
+ISC_TEST_ENTRY(updatekeys_remove_privateoid)
+ISC_TEST_LIST_END
+
+ISC_TEST_MAIN
diff --git a/tests/dns/dst_test.c b/tests/dns/dst_test.c
index 73871677e7..6c4aafaa3d 100644
--- a/tests/dns/dst_test.c
+++ b/tests/dns/dst_test.c
@@ -100,6 +100,28 @@ ISC_RUN_TEST_IMPL(algorithm_fromdata) {
}
}
+ISC_RUN_TEST_IMPL(algorithm_format) {
+ struct {
+ dst_algorithm_t alg;
+ const char *text;
+ } totext[] = {
+ { DST_ALG_RSASHA256, "RSASHA256" },
+ { DST_ALG_RSASHA512, "RSASHA512" },
+ /* Private algorithms are 16-bit values with own mnemonics. */
+ { DST_ALG_RSASHA256PRIVATEOID, "RSASHA256OID" },
+ { DST_ALG_RSASHA512PRIVATEOID, "RSASHA512OID" },
+ /* Unknown algorithms fall back to the numeric form. */
+ { 511, "511" },
+ };
+
+ for (size_t i = 0; i < ARRAY_SIZE(totext); i++) {
+ char algstr[DST_ALG_FORMATSIZE];
+
+ dst_algorithm_format(totext[i].alg, algstr, sizeof(algstr));
+ assert_string_equal(algstr, totext[i].text);
+ }
+}
+
/* Read sig in file at path to buf. Check signature ineffability */
static isc_result_t
sig_fromfile(const char *path, isc_buffer_t *buf) {
@@ -541,6 +563,7 @@ ISC_RUN_TEST_IMPL(ecdsa_determinism_test) {
ISC_TEST_LIST_START
ISC_TEST_ENTRY(algorithm_fromdata)
+ISC_TEST_ENTRY(algorithm_format)
ISC_TEST_ENTRY(sig_test)
ISC_TEST_ENTRY(cmp_test)
ISC_TEST_ENTRY(ecdsa_determinism_test)
diff --git a/tests/dns/keymgr_test.c b/tests/dns/keymgr_test.c
new file mode 100644
index 0000000000..4fd9129209
--- /dev/null
+++ b/tests/dns/keymgr_test.c
@@ -0,0 +1,104 @@
+/*
+ * Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+ *
+ * SPDX-License-Identifier: MPL-2.0
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, you can obtain one at https://mozilla.org/MPL/2.0/.
+ *
+ * See the COPYRIGHT file distributed with this work for additional
+ * information regarding copyright ownership.
+ */
+
+#include <inttypes.h>
+#include <sched.h> /* IWYU pragma: keep */
+#include <setjmp.h>
+#include <stdarg.h>
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#define UNIT_TESTING
+#include <cmocka.h>
+
+#include <isc/buffer.h>
+#include <isc/lib.h>
+#include <isc/string.h>
+#include <isc/util.h>
+
+#include <dns/dnssec.h>
+#include <dns/kasp.h>
+#include <dns/keymgr.h>
+#include <dns/keyvalues.h>
+#include <dns/lib.h>
+
+#include <dst/dst.h>
+
+#include <tests/dns.h>
+
+/*
+ * dns_keymgr_status() must report the full 16-bit DST algorithm; private
+ * algorithms used to be truncated to their 8-bit DNSSEC algorithm octet.
+ */
+ISC_RUN_TEST_IMPL(keymgr_status_privateoid) {
+ isc_result_t result;
+ dns_kasp_t *kasp = NULL;
+ dns_dnsseckeylist_t keyring;
+ dns_dnsseckey_t *dkey = NULL;
+ dst_key_t *key = NULL;
+ dns_fixedname_t fname;
+ dns_name_t *name;
+ isc_buffer_t buf;
+ char output[BUFSIZ] = { 0 };
+ isc_stdtime_t now = isc_stdtime_now();
+
+ UNUSED(state);
+
+ if (!dst_algorithm_supported(DST_ALG_RSASHA256PRIVATEOID)) {
+ skip();
+ return;
+ }
+
+ dns_test_namefromstring("example.", &fname);
+ name = dns_fixedname_name(&fname);
+
+ result = dst_key_generate(name, DST_ALG_RSASHA256PRIVATEOID, 2048, 0,
+ DNS_KEYOWNER_ZONE, DNS_KEYPROTO_DNSSEC,
+ dns_rdataclass_in, NULL, isc_g_mctx, &key,
+ NULL);
+ assert_int_equal(result, ISC_R_SUCCESS);
+
+ /* Make the key an in-use ZSK so its status is reported. */
+ dst_key_setbool(key, DST_BOOL_KSK, false);
+ dst_key_setbool(key, DST_BOOL_ZSK, true);
+ dst_key_settime(key, DST_TIME_PUBLISH, now);
+ dst_key_settime(key, DST_TIME_ACTIVATE, now);
+ dst_key_setstate(key, DST_KEY_GOAL, DST_KEY_STATE_OMNIPRESENT);
+ dst_key_setstate(key, DST_KEY_DNSKEY, DST_KEY_STATE_RUMOURED);
+ dst_key_setstate(key, DST_KEY_ZRRSIG, DST_KEY_STATE_RUMOURED);
+
+ ISC_LIST_INIT(keyring);
+ dns_dnsseckey_create(isc_g_mctx, &key, &dkey);
+ ISC_LIST_APPEND(keyring, dkey, link);
+
+ dns_kasp_create(isc_g_mctx, "test", &kasp);
+ dns_kasp_freeze(kasp);
+
+ isc_buffer_init(&buf, output, sizeof(output) - 1);
+ result = dns_keymgr_status(kasp, &keyring, &buf, now, false, false);
+ assert_int_equal(result, ISC_R_SUCCESS);
+
+ assert_non_null(strstr(output, "(RSASHA256OID)"));
+
+ dns_kasp_detach(&kasp);
+ ISC_LIST_UNLINK(keyring, dkey, link);
+ dns_dnsseckey_destroy(isc_g_mctx, &dkey);
+}
+
+ISC_TEST_LIST_START
+ISC_TEST_ENTRY(keymgr_status_privateoid)
+ISC_TEST_LIST_END
+
+ISC_TEST_MAIN
diff --git a/tests/dns/meson.build b/tests/dns/meson.build
index bb173447a8..1474fc52d0 100644
--- a/tests/dns/meson.build
+++ b/tests/dns/meson.build
@@ -22,9 +22,11 @@ dns_tests = [
'diff',
'dispatch',
'dns64',
+ 'dnssec',
'dnssecsignstats',
'dst',
'ede',
+ 'keymgr',
'keytable',
'master',
'message',