From: Matthijs Mekking Date: Thu, 6 Feb 2020 16:43:54 +0000 (+0100) Subject: Warn if key lengths are out of range/predefined X-Git-Tag: v9.16.0~18^2~4 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=8c0db909ee9428a30cdc14738979bbd21179de80;p=thirdparty%2Fbind9.git Warn if key lengths are out of range/predefined --- diff --git a/bin/dnssec/dnssec-keygen.c b/bin/dnssec/dnssec-keygen.c index 220cbe7a00d..7d12aa75464 100644 --- a/bin/dnssec/dnssec-keygen.c +++ b/bin/dnssec/dnssec-keygen.c @@ -68,6 +68,8 @@ const char *program = "dnssec-keygen"; +isc_log_t *lctx = NULL; + ISC_PLATFORM_NORETURN_PRE static void usage(void) ISC_PLATFORM_NORETURN_POST; @@ -266,7 +268,8 @@ kasp_from_conf(cfg_obj_t* config, isc_mem_t* mctx, const char* name, continue; } - result = cfg_kasp_fromconfig(kconfig, mctx, &kasplist, &kasp); + result = cfg_kasp_fromconfig(kconfig, mctx, lctx, &kasplist, + &kasp); if (result != ISC_R_SUCCESS) { fatal("failed to configure dnssec-policy '%s': %s", cfg_obj_asstring(cfg_tuple_get(kconfig, "name")), @@ -809,7 +812,6 @@ main(int argc, char **argv) { isc_mem_t *mctx = NULL; isc_result_t ret; isc_textregion_t r; - isc_log_t *log = NULL; const char *engine = NULL; unsigned char c; int ch; @@ -1083,7 +1085,7 @@ main(int argc, char **argv) { fatal("could not initialize dst: %s", isc_result_totext(ret)); - setup_logging(mctx, &log); + setup_logging(mctx, &lctx); ctx.rdclass = strtoclass(classname); @@ -1173,7 +1175,7 @@ main(int argc, char **argv) { dns_kasp_t* kasp = NULL; dns_kasp_key_t* kaspkey = NULL; - RUNTIME_CHECK(cfg_parser_create(mctx, log, &parser) + RUNTIME_CHECK(cfg_parser_create(mctx, lctx, &parser) == ISC_R_SUCCESS); if (cfg_parse_file(parser, ctx.configfile, &cfg_type_namedconf, &config) != ISC_R_SUCCESS) @@ -1220,7 +1222,7 @@ main(int argc, char **argv) { keygen(&ctx, mctx, argc, argv); } - cleanup_logging(&log); + cleanup_logging(&lctx); dst_lib_destroy(); if (verbose > 10) isc_mem_stats(mctx, stdout); diff --git a/bin/named/server.c b/bin/named/server.c index 6a818a31493..e01c4e78e45 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -8742,8 +8742,8 @@ load_configuration(const char *filename, named_server_t *server, { cfg_obj_t *kconfig = cfg_listelt_value(element); kasp = NULL; - CHECK(cfg_kasp_fromconfig(kconfig, named_g_mctx, &kasplist, - &kasp)); + CHECK(cfg_kasp_fromconfig(kconfig, named_g_mctx, named_g_lctx, + &kasplist, &kasp)); INSIST(kasp != NULL); dns_kasp_freeze(kasp); dns_kasp_detach(&kasp); @@ -8752,7 +8752,8 @@ load_configuration(const char *filename, named_server_t *server, * Create the default kasp. */ kasp = NULL; - CHECK(cfg_kasp_fromconfig(NULL, named_g_mctx, &kasplist, &kasp)); + CHECK(cfg_kasp_fromconfig(NULL, named_g_mctx, named_g_lctx, &kasplist, + &kasp)); INSIST(kasp != NULL); dns_kasp_freeze(kasp); dns_kasp_detach(&kasp); diff --git a/bin/tests/system/checkconf/good-kasp.conf b/bin/tests/system/checkconf/good-kasp.conf index e6be1484665..6dcc404559d 100644 --- a/bin/tests/system/checkconf/good-kasp.conf +++ b/bin/tests/system/checkconf/good-kasp.conf @@ -17,7 +17,7 @@ dnssec-policy "test" { dnskey-ttl 3600; keys { - ksk key-directory lifetime P1Y algorithm 13 256; + ksk key-directory lifetime P1Y algorithm 13; zsk lifetime P30D algorithm 13; csk key-directory lifetime unlimited algorithm 8 2048; }; diff --git a/bin/tests/system/checkconf/kasp-ignore-keylen.conf b/bin/tests/system/checkconf/kasp-ignore-keylen.conf new file mode 100644 index 00000000000..5874bdb8873 --- /dev/null +++ b/bin/tests/system/checkconf/kasp-ignore-keylen.conf @@ -0,0 +1,26 @@ +/* + * Copyright (C) Internet Systems Consortium, Inc. ("ISC") + * + * 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 http://mozilla.org/MPL/2.0/. + * + * See the COPYRIGHT file distributed with this work for additional + * information regarding copyright ownership. + */ + +dnssec-policy "warn-length" { + keys { + // Algorithm 13 has predefined length, warn about length param. + csk lifetime unlimited algorithm 13 2048; + // Algorithm 5 length out of range, warn about length param. + csk lifetime unlimited algorithm 5 4097; + }; +}; + +zone "example.net" { + type master; + file "example.db"; + dnssec-policy "warn-length"; +}; + diff --git a/bin/tests/system/checkconf/tests.sh b/bin/tests/system/checkconf/tests.sh index 1ae5b93601e..814b3b761b3 100644 --- a/bin/tests/system/checkconf/tests.sh +++ b/bin/tests/system/checkconf/tests.sh @@ -489,6 +489,15 @@ grep "update-check-ksk: cannot be configured if dnssec-policy is also set" < che if [ $ret != 0 ]; then echo_i "failed"; fi status=`expr $status + $ret` +n=`expr $n + 1` +echo_i "checking named-checkconf kasp key warnings ($n)" +ret=0 +$CHECKCONF kasp-ignore-keylen.conf > checkconf.out$n 2>&1 +grep "dnssec-policy: key algorithm 13 has predefined length, ignoring length value 2048" < checkconf.out$n > /dev/null || ret=1 +grep "dnssec-policy: key with algorithm 5 has invalid key length, ignoring length value 4097" < checkconf.out$n > /dev/null || ret=1 +if [ $ret != 0 ]; then echo_i "failed"; fi +status=`expr $status + $ret` + n=`expr $n + 1` echo_i "check that a good 'kasp' configuration is accepted ($n)" ret=0 diff --git a/lib/bind9/check.c b/lib/bind9/check.c index 4cb376f9767..f085dd2a33b 100644 --- a/lib/bind9/check.c +++ b/lib/bind9/check.c @@ -40,6 +40,7 @@ #include #include #include +#include #include #include #include @@ -54,6 +55,7 @@ #include #include #include +#include #include #include @@ -977,21 +979,45 @@ check_options(const cfg_obj_t *options, isc_log_t *logctx, isc_mem_t *mctx, if (optlevel != optlevel_config && !cfg_obj_isstring(obj)) { bad_kasp = true; } else if (optlevel == optlevel_config) { + dns_kasplist_t list; + dns_kasp_t* kasp, *kasp_next; + + ISC_LIST_INIT(list); + if (cfg_obj_islist(obj)) { for (element = cfg_list_first(obj); element != NULL; element = cfg_list_next(element)) { - if (!cfg_obj_istuple( - cfg_listelt_value(element))) + cfg_obj_t *kconfig = + cfg_listelt_value(element); + + if (!cfg_obj_istuple(kconfig)) { bad_kasp = true; + continue; } if (!kasp_name_allowed(element)) { bad_name = true; + continue; + } + kasp = NULL; + (void)cfg_kasp_fromconfig(kconfig, mctx, + logctx, + &list, &kasp); + if (kasp != NULL) { + dns_kasp_detach(&kasp); } } } + + for (kasp = ISC_LIST_HEAD(list); kasp != NULL; + kasp = kasp_next) + { + kasp_next = ISC_LIST_NEXT(kasp, link); + ISC_LIST_UNLINK(list, kasp, link); + dns_kasp_detach(&kasp); + } } if (bad_kasp) { diff --git a/lib/isccfg/include/isccfg/kaspconf.h b/lib/isccfg/include/isccfg/kaspconf.h index 9d18f445dad..febfa680f9f 100644 --- a/lib/isccfg/include/isccfg/kaspconf.h +++ b/lib/isccfg/include/isccfg/kaspconf.h @@ -26,7 +26,7 @@ ISC_LANG_BEGINDECLS isc_result_t -cfg_kasp_fromconfig(const cfg_obj_t *config, isc_mem_t* mctx, +cfg_kasp_fromconfig(const cfg_obj_t *config, isc_mem_t* mctx, isc_log_t *logctx, dns_kasplist_t *kasplist, dns_kasp_t **kaspp); /*%< * Create and configure a KASP. If 'config' is NULL, the default configuration @@ -38,6 +38,8 @@ cfg_kasp_fromconfig(const cfg_obj_t *config, isc_mem_t* mctx, * *\li 'mctx' is a valid memory context. * + *\li 'logctx' is a valid logging context. + * *\li 'name' is a valid C string. * *\li kaspp != NULL && *kaspp == NULL diff --git a/lib/isccfg/kaspconf.c b/lib/isccfg/kaspconf.c index 8001c122e79..9538be6376a 100644 --- a/lib/isccfg/kaspconf.c +++ b/lib/isccfg/kaspconf.c @@ -65,7 +65,8 @@ get_duration(const cfg_obj_t **maps, const char* option, uint32_t dfl) * Create a new kasp key derived from configuration. */ static isc_result_t -cfg_kaspkey_fromconfig(const cfg_obj_t *config, dns_kasp_t* kasp) +cfg_kaspkey_fromconfig(const cfg_obj_t *config, dns_kasp_t* kasp, + isc_log_t *logctx) { isc_result_t result; dns_kasp_key_t *key = NULL; @@ -75,6 +76,7 @@ cfg_kaspkey_fromconfig(const cfg_obj_t *config, dns_kasp_t* kasp) if (result != ISC_R_SUCCESS) { return (result); } + if (config == NULL) { /* We are creating a key reference for the default kasp. */ key->role |= DNS_KASP_KEY_ROLE_KSK | DNS_KASP_KEY_ROLE_ZSK; @@ -82,8 +84,8 @@ cfg_kaspkey_fromconfig(const cfg_obj_t *config, dns_kasp_t* kasp) key->algorithm = DNS_KEYALG_ECDSA256; key->length = -1; } else { - const char* rolestr; - const cfg_obj_t* obj; + const char *rolestr = NULL; + const cfg_obj_t *obj = NULL; rolestr = cfg_obj_asstring(cfg_tuple_get(config, "role")); if (strcmp(rolestr, "ksk") == 0) { @@ -106,15 +108,48 @@ cfg_kaspkey_fromconfig(const cfg_obj_t *config, dns_kasp_t* kasp) obj = cfg_tuple_get(config, "length"); if (cfg_obj_isuint32(obj)) { - key->length = cfg_obj_asuint32(obj); + uint32_t min, size; + size = cfg_obj_asuint32(obj); + + switch (key->algorithm) { + case DNS_KEYALG_RSASHA1: + case DNS_KEYALG_NSEC3RSASHA1: + case DNS_KEYALG_RSASHA256: + case DNS_KEYALG_RSASHA512: + min = DNS_KEYALG_RSASHA512 ? 1024 : 512; + if (size < min || size > 4096) { + cfg_obj_log(obj, logctx, + ISC_LOG_ERROR, + "dnssec-policy: key with " + "algorithm %u has invalid " + "key length", + key->algorithm); + return (ISC_R_RANGE); + } + break; + case DNS_KEYALG_ECDSA256: + case DNS_KEYALG_ECDSA384: + case DNS_KEYALG_ED25519: + case DNS_KEYALG_ED448: + cfg_obj_log(obj, logctx, ISC_LOG_WARNING, + "dnssec-policy: key algorithm %u " + "has predefined length; ignoring " + "length value %u", key->algorithm, + size); + default: + break; + } + + key->length = size; } } + dns_kasp_addkey(kasp, key); return (result); } isc_result_t -cfg_kasp_fromconfig(const cfg_obj_t *config, isc_mem_t* mctx, +cfg_kasp_fromconfig(const cfg_obj_t *config, isc_mem_t *mctx, isc_log_t *logctx, dns_kasplist_t *kasplist, dns_kasp_t **kaspp) { isc_result_t result; @@ -179,7 +214,7 @@ cfg_kasp_fromconfig(const cfg_obj_t *config, isc_mem_t* mctx, (void)confget(maps, "keys", &keys); if (keys == NULL) { - result = cfg_kaspkey_fromconfig(NULL, kasp); + result = cfg_kaspkey_fromconfig(NULL, kasp, logctx); if (result != ISC_R_SUCCESS) { goto cleanup; } @@ -188,7 +223,7 @@ cfg_kasp_fromconfig(const cfg_obj_t *config, isc_mem_t* mctx, element = cfg_list_next(element)) { cfg_obj_t *kobj = cfg_listelt_value(element); - result = cfg_kaspkey_fromconfig(kobj, kasp); + result = cfg_kaspkey_fromconfig(kobj, kasp, logctx); if (result != ISC_R_SUCCESS) { goto cleanup; }