From: Ondřej Surý Date: Wed, 29 Apr 2026 15:02:11 +0000 (+0200) Subject: Reject unsafe key names in rndc-confgen, tsig-keygen, ddns-confgen X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d5ba6e1c26f02b634c8749450756efb7d24a4d8a;p=thirdparty%2Fbind9.git 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 6bdf9634de0..3192953a02f 100644 --- a/bin/confgen/keygen.c +++ b/bin/confgen/keygen.c @@ -14,6 +14,7 @@ /*! \file */ #include "keygen.h" +#include #include #include @@ -87,6 +88,26 @@ alg_bits(dns_secalg_t alg) { } } +/*% + * Reject key names that would not embed safely into a named.conf + * 'key "" { ... };' 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 7a8ca3a4b09..711ec195e0e 100644 --- a/bin/confgen/keygen.h +++ b/bin/confgen/keygen.h @@ -20,6 +20,9 @@ #include +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 89ff68f398c..470b7fe8082 100644 --- 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 553a45eaf2d..9e11b22eb8c 100644 --- 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 index 00000000000..2074a064945 --- /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 index 00000000000..23184c28c20 --- /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])