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

diff --git a/bin/confgen/keygen.c b/bin/confgen/keygen.c
index 6bdf9634de01e031ca4b041a3d9f07c52147b524..3192953a02f4297f95170d2a8552539644cd52e8 100644 (file)
--- a/bin/confgen/keygen.c
+++ b/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'
  */

diff --git a/bin/confgen/keygen.h b/bin/confgen/keygen.h
index 7a8ca3a4b09354ed0eb5da9d7631a6e27c6166f6..711ec195e0e51ed3bbf5048107251625c8801e70 100644 (file)
--- a/bin/confgen/keygen.h
+++ b/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);

diff --git a/bin/confgen/rndc-confgen.c b/bin/confgen/rndc-confgen.c
index 89ff68f398c845da9df7cdc6180459a1f793b616..470b7fe8082ec01812aca9f008b1cdeabb197bcd 100644 (file)
--- a/bin/confgen/rndc-confgen.c
+++ b/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 "

diff --git a/bin/confgen/tsig-keygen.c b/bin/confgen/tsig-keygen.c
index 553a45eaf2d4b91dd7b2de1e7e536247f20f2ba4..9e11b22eb8c2f8ebcd2786750f0c134198448fb5 100644 (file)
--- a/bin/confgen/tsig-keygen.c
+++ b/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);

diff --git a/bin/tests/system/rndc_confgen/setup.sh b/bin/tests/system/rndc_confgen/setup.sh
new file mode 100644 (file)
index 0000000..2074a06
--- /dev/null
+++ b/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

diff --git a/bin/tests/system/rndc_confgen/tests_rndc_confgen.py b/bin/tests/system/rndc_confgen/tests_rndc_confgen.py
new file mode 100644 (file)
index 0000000..23184c2
--- /dev/null
+++ b/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])