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] 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; +}; 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` 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);