Warn if key lengths are out of range/predefined

diff --git a/bin/dnssec/dnssec-keygen.c b/bin/dnssec/dnssec-keygen.c
index 220cbe7a00dfaf96af04a8559db4232096af1ae4..7d12aa75464902a842edc0b9407b50f1b4874142 100644 (file)
--- 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 6a818a31493d731fd1f89ec9f737fd0af745d541..e01c4e78e45e3e04035697cd7b1cf18b68ce3269 100644 (file)
--- 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 e6be14846658da75cbcdc7ffec4ee0f633e45a49..6dcc404559df83c3e4bafa708cdab41171fd0b8f 100644 (file)
--- 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 (file)
index 0000000..5874bdb
--- /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 1ae5b93601efffb468adea228d576a18df497f4e..814b3b761b3da2112adcad504b99533351781b09 100644 (file)
--- 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 4cb376f9767b32971e56994c407066e6fadb589f..f085dd2a33ba5897ef2f64a5fe48f8d1a43615f8 100644 (file)
--- a/lib/bind9/check.c
+++ b/lib/bind9/check.c
@@ -40,6 +40,7 @@
 #include <dns/acl.h>
 #include <dns/dnstap.h>
 #include <dns/fixedname.h>
+#include <dns/kasp.h>
 #include <dns/keyvalues.h>
 #include <dns/rbt.h>
 #include <dns/rdataclass.h>
@@ -54,6 +55,7 @@
 #include <isccfg/aclconf.h>
 #include <isccfg/cfg.h>
 #include <isccfg/grammar.h>
+#include <isccfg/kaspconf.h>
 #include <isccfg/namedconf.h>
 
 #include <ns/hooks.h>
@@ -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 9d18f445dad1f05a10e3e07a24e94e205a13571b..febfa680f9f209926e5eb4d7e9a5dddfdbb63f0e 100644 (file)
--- 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 8001c122e7958c3b08af59cbb940cd789b679654..9538be6376a2e2f543bfe5979aac8e101eeebd58 100644 (file)
--- 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;
                        }