From 1b5464253523b615882427ab8392a265cf945b5d Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 15 Feb 2022 16:24:52 +1100 Subject: [PATCH 1/4] Add test configurations with invalid dnssec-policy clauses bad-ksk-without-zsk.conf only has a ksk defined without a matching zsk for the same algorithm. bad-zsk-without-ksk.conf only has a zsk defined without a matching ksk for the same algorithm. bad-unpaired-keys.conf has two keys of different algorithms one ksk only and the other zsk only (cherry picked from commit f23e86b96b77bb9fd485a2c8f6d3cd8a02afd7bd) --- .../system/checkconf/bad-ksk-without-zsk.conf | 24 +++++++++++++++++ .../system/checkconf/bad-unpaired-keys.conf | 27 +++++++++++++++++++ .../system/checkconf/bad-zsk-without-ksk.conf | 24 +++++++++++++++++ 3 files changed, 75 insertions(+) create mode 100644 bin/tests/system/checkconf/bad-ksk-without-zsk.conf create mode 100644 bin/tests/system/checkconf/bad-unpaired-keys.conf create mode 100644 bin/tests/system/checkconf/bad-zsk-without-ksk.conf diff --git a/bin/tests/system/checkconf/bad-ksk-without-zsk.conf b/bin/tests/system/checkconf/bad-ksk-without-zsk.conf new file mode 100644 index 0000000000..66e1b7f0c8 --- /dev/null +++ b/bin/tests/system/checkconf/bad-ksk-without-zsk.conf @@ -0,0 +1,24 @@ +/* + * 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. + */ + +dnssec-policy ksk-without-zsk { + keys { + ksk lifetime 30d algorithm 13; + }; +}; + +zone "example" { + type primary; + file "example.db"; + dnssec-policy ksk-without-zsk; +}; diff --git a/bin/tests/system/checkconf/bad-unpaired-keys.conf b/bin/tests/system/checkconf/bad-unpaired-keys.conf new file mode 100644 index 0000000000..63b6dc2c65 --- /dev/null +++ b/bin/tests/system/checkconf/bad-unpaired-keys.conf @@ -0,0 +1,27 @@ +/* + * 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. + */ + +dnssec-policy unpaired-keys { + keys { + /* zsk without ksk */ + zsk lifetime 30d algorithm 13; + /* ksk without zsk */ + ksk lifetime 30d algorithm 7; + }; +}; + +zone "example" { + type primary; + file "example.db"; + dnssec-policy unpaired-keys; +}; diff --git a/bin/tests/system/checkconf/bad-zsk-without-ksk.conf b/bin/tests/system/checkconf/bad-zsk-without-ksk.conf new file mode 100644 index 0000000000..31b031cdc8 --- /dev/null +++ b/bin/tests/system/checkconf/bad-zsk-without-ksk.conf @@ -0,0 +1,24 @@ +/* + * 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. + */ + +dnssec-policy zsk-without-ksk { + keys { + zsk lifetime 30d algorithm 13; + }; +}; + +zone "example" { + type primary; + file "example.db"; + dnssec-policy zsk-without-ksk; +}; From d752bbfb2207c0cc2640dd1ae41f17260eec7f6a Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 15 Feb 2022 17:12:27 +1100 Subject: [PATCH 2/4] Check dnssec-policy key roles for validity For each algorithm there must be a key performing the KSK and ZSK rolls. After reading the keys from named.conf check that each algorithm present has both rolls. CSK implicitly has both rolls. (cherry picked from commit 9bcf45f4cecdb2fe577c426aae23e5d105531472) --- lib/isccfg/kaspconf.c | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/lib/isccfg/kaspconf.c b/lib/isccfg/kaspconf.c index f2eb77aab2..a8a078f0c7 100644 --- a/lib/isccfg/kaspconf.c +++ b/lib/isccfg/kaspconf.c @@ -262,7 +262,7 @@ cfg_kasp_fromconfig(const cfg_obj_t *config, const char *name, isc_mem_t *mctx, const cfg_listelt_t *element = NULL; const char *kaspname = NULL; dns_kasp_t *kasp = NULL; - int i = 0; + size_t i = 0; REQUIRE(kaspp != NULL && *kaspp == NULL); @@ -323,6 +323,9 @@ cfg_kasp_fromconfig(const cfg_obj_t *config, const char *name, isc_mem_t *mctx, (void)confget(maps, "keys", &keys); if (keys != NULL) { + char role[256] = { 0 }; + dns_kasp_key_t *kkey = NULL; + for (element = cfg_list_first(keys); element != NULL; element = cfg_list_next(element)) { @@ -333,6 +336,36 @@ cfg_kasp_fromconfig(const cfg_obj_t *config, const char *name, isc_mem_t *mctx, } } INSIST(!(dns_kasp_keylist_empty(kasp))); + dns_kasp_freeze(kasp); + for (kkey = ISC_LIST_HEAD(dns_kasp_keys(kasp)); kkey != NULL; + kkey = ISC_LIST_NEXT(kkey, link)) + { + uint32_t keyalg = dns_kasp_key_algorithm(kkey); + INSIST(keyalg < ARRAY_SIZE(role)); + + if (dns_kasp_key_zsk(kkey)) { + role[keyalg] |= DNS_KASP_KEY_ROLE_ZSK; + } + + if (dns_kasp_key_ksk(kkey)) { + role[keyalg] |= DNS_KASP_KEY_ROLE_KSK; + } + } + dns_kasp_thaw(kasp); + for (i = 0; i < ARRAY_SIZE(role); i++) { + if (role[i] != 0 && role[i] != (DNS_KASP_KEY_ROLE_ZSK | + DNS_KASP_KEY_ROLE_KSK)) + { + cfg_obj_log(keys, logctx, ISC_LOG_ERROR, + "dnssec-policy: algorithm %zu " + "requires both KSK and ZSK roles", + i); + result = ISC_R_FAILURE; + } + } + if (result != ISC_R_SUCCESS) { + goto cleanup; + } } else if (strcmp(kaspname, "insecure") == 0) { /* "dnssec-policy insecure": key list must be empty */ INSIST(strcmp(kaspname, "insecure") == 0); From d1766d4515c140d32cff15302c1d9a3afb32b1db Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 15 Feb 2022 17:42:45 +1100 Subject: [PATCH 3/4] Add CHANGES entry for [GL #3142] (cherry picked from commit d4c2395fff9a7bbed403d3e2a8dc931b163cd032) --- CHANGES | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGES b/CHANGES index 47b854433c..5cd237b445 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,9 @@ +5824. [bug] Invalid dnssec-policy definitions were being accepted + where the defined keys did not cover both KSK and ZSK + roles for a given algorithm. This is now checked for + and the dnssec-policy is rejected if both roles are + not present for all algorithms in use. [GL #3142] + 5816. [bug] Make BIND compile with LibreSSL 3.5.0, as it was using not very accurate pre-processor checks for using shims. [GL #3172] From 63f194995a20fa2ce5a9050e44e26272d250be8b Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Fri, 18 Feb 2022 13:36:15 +1100 Subject: [PATCH 4/4] Add release note for [GL #3142] (cherry picked from commit e48af3698100b2e69971e2cc5fc09ee300bbedc7) --- doc/notes/notes-current.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 3cff9db120..28f5cb04fb 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -94,3 +94,9 @@ Bug Fixes miscalculated in certain resolution scenarios, potentially causing the value of the counter to drop below zero. This has been fixed. :gl:`#3147` + +- Invalid dnssec-policy definitions were being accepted where the + defined keys did not cover both KSK and ZSK roles for a given + algorithm. This is now checked for and the dnssec-policy is + rejected if both roles are not present for all algorithms in use. + :gl:`#3142`