Reject unsafe key names in rndc-confgen, tsig-keygen, ddns-confgen

The three tools interpolated their key-name argument verbatim into the
generated 'key "..." { ... };' clause. A name containing '"', '{', '}',
or ';' could close the clause and append additional named.conf
statements — for example, a second key block with an attacker-chosen
secret. The injected output passes named-checkconf and is loaded by
named as a valid configuration. The risk shows up when an automation
wrapper feeds tenant or zone names from a less-trusted source through
-k / -y / -s / -z (or the tsig-keygen positional argument).

Validate the final key name (after the optional -s / -z suffix is
concatenated in tsig-keygen) against [A-Za-z0-9._-]+ and exit with an
error otherwise. The allowlist covers the documented usage; every
character used in the injection vectors is excluded.

Add a system test that runs the documented PoC payloads through each
tool and asserts a non-zero exit, plus sanity coverage for the default
key names and dotted DNS-style names.

Assisted-by: Claude:claude-opus-4-7

bin/confgen/keygen.c

@@ -14,6 +14,7 @@
 /*! \file */
 
 #include "keygen.h"
+#include <ctype.h>
 #include <stdarg.h>
 #include <stdlib.h>
 
@@ -87,6 +88,26 @@ alg_bits(dns_secalg_t alg) {
 	}
 }
 
+/*%
+ * Reject key names that would not embed safely into a named.conf
+ * 'key "<name>" { ... };' clause. Allowed: alphanumerics, '.', '-', '_'.
+ */
+void
+validate_keyname(const char *keyname) {
+	if (keyname == NULL || keyname[0] == '\0') {
+		fatal("key name must not be empty");
+	}
+	for (const char *p = keyname; *p != '\0'; p++) {
+		unsigned char c = (unsigned char)*p;
+		if (!isalnum(c) && c != '.' && c != '-' && c != '_') {
+			fatal("key name '%s' contains invalid character; "
+			      "only alphanumerics, '.', '-', and '_' are "
+			      "allowed",
+			      keyname);
+		}
+	}
+}
+
 /*%
  * Generate a key of size 'keysize' and place it in 'key_txtbuffer'
  */

bin/confgen/keygen.h

@@ -20,6 +20,9 @@
 
 #include <dns/secalg.h>
 
+void
+validate_keyname(const char *keyname);
+
 void
 generate_key(isc_mem_t *mctx, dns_secalg_t alg, int keysize,
 	     isc_buffer_t *key_txtbuffer);

bin/confgen/rndc-confgen.c

@@ -207,6 +207,8 @@ main(int argc, char **argv) {
 		usage(EXIT_FAILURE);
 	}
 
+	validate_keyname(keyname);
+
 	if (alg == DST_ALG_HMACMD5) {
 		fprintf(stderr, "warning: use of hmac-md5 for RNDC keys "
 				"is deprecated; hmac-sha256 is now "

bin/confgen/tsig-keygen.c

@@ -212,6 +212,8 @@ main(int argc, char **argv) {
 		}
 	}
 
+	validate_keyname(keyname);
+
 	isc_buffer_init(&key_txtbuffer, &key_txtsecret, sizeof(key_txtsecret));
 
 	generate_key(isc_g_mctx, alg, keysize, &key_txtbuffer);

bin/tests/system/rndc_confgen/setup.sh

@@ -0,0 +1,19 @@
+#!/bin/sh
+
+# 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.
+
+set -e
+
+# tsig-keygen and ddns-confgen are the same binary; the install layout
+# provides ddns-confgen as a symlink, but the build tree does not. Create
+# one here so the test can exercise the ddns-confgen mode.
+ln -sf "$TSIGKEYGEN" ddns-confgen

bin/tests/system/rndc_confgen/tests_rndc_confgen.py

@@ -0,0 +1,69 @@
+# 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.
+
+import os
+import subprocess
+
+import pytest
+
+import isctest
+
+INJECTION = (
+    'backdoor" { algorithm hmac-sha256; '
+    'secret "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA="; }; key "rndc-key'
+)
+
+
+def test_rndc_confgen_default():
+    cmd = isctest.run.cmd([os.environ["RNDCCONFGEN"]])
+    assert b'key "rndc-key" {' in cmd.proc.stdout
+
+
+def test_rndc_confgen_keyname_with_dots():
+    cmd = isctest.run.cmd([os.environ["RNDCCONFGEN"], "-k", "key.example.com"])
+    assert b'key "key.example.com" {' in cmd.proc.stdout
+
+
+def test_rndc_confgen_rejects_injection():
+    with pytest.raises(subprocess.CalledProcessError):
+        isctest.run.cmd([os.environ["RNDCCONFGEN"], "-k", INJECTION])
+
+
+def test_tsig_keygen_default():
+    cmd = isctest.run.cmd([os.environ["TSIGKEYGEN"]])
+    assert b'key "tsig-key" {' in cmd.proc.stdout
+
+
+def test_tsig_keygen_rejects_injection_positional():
+    with pytest.raises(subprocess.CalledProcessError):
+        isctest.run.cmd([os.environ["TSIGKEYGEN"], INJECTION])
+
+
+DDNSCONFGEN = "./ddns-confgen"
+
+
+def test_ddns_confgen_default():
+    cmd = isctest.run.cmd([DDNSCONFGEN, "-q"])
+    assert b'key "ddns-key" {' in cmd.proc.stdout
+
+
+@pytest.mark.parametrize(
+    "args",
+    [
+        ["-k", INJECTION],
+        ["-y", INJECTION],
+        ["-z", INJECTION],
+        ["-s", INJECTION],
+    ],
+)
+def test_ddns_confgen_rejects_injection(args):
+    with pytest.raises(subprocess.CalledProcessError):
+        isctest.run.cmd([DDNSCONFGEN, "-q", *args])