]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Error if key lifetime is too short
authorMatthijs Mekking <matthijs@isc.org>
Mon, 9 May 2022 11:56:45 +0000 (13:56 +0200)
committerMatthijs Mekking <matthijs@isc.org>
Tue, 31 May 2022 15:16:53 +0000 (17:16 +0200)
The key lifetime should not be shorter than the time it costs to
introduce the successor key, otherwise keys will be created faster than
they are removed, resulting in a large key set.

The time it takes to replace a key is determined by the publication
interval (Ipub) of the successor key and the retire interval of the
predecessor key (Iret).

For the ZSK, Ipub is the sum of the DNSKEY TTL and zone propagation
delay (and publish safety). Iret is the sum of Dsgn, the maximum zone
TTL and zone propagation delay (and retire safety). The sign delay is
the signature validity period minus the refresh interval: The time to
ensure that all existing RRsets have been re-signed with the new key.
The ZSK lifetime should be larger than both values.

For the KSK, Ipub is the sum of the DNSKEY TTL and zone propagation
delay (and publish safety). Iret is the sum of the DS TTL and parent
zone propagation delay (and retire safety). The KSK lifetime should be
larger than both values.

(cherry picked from commit 8134d46cdb2ed592c4c4a3b21f7419621c25527f)

bin/tests/system/checkconf/kasp-bad-lifetime.conf [new file with mode: 0644]
bin/tests/system/checkconf/tests.sh
lib/isccfg/kaspconf.c

diff --git a/bin/tests/system/checkconf/kasp-bad-lifetime.conf b/bin/tests/system/checkconf/kasp-bad-lifetime.conf
new file mode 100644 (file)
index 0000000..225b386
--- /dev/null
@@ -0,0 +1,91 @@
+/*
+ * 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 "bad-lifetime-ksk" {
+       /*
+        * The KSK lifetime is too short.
+        * The ZSK lifetime is good enough but should trigger a warning.
+        */
+       keys {
+               ksk lifetime PT3H algorithm 13;
+               zsk lifetime P8DT2H1S algorithm 13;
+       };
+
+       dnskey-ttl PT1H;
+       publish-safety PT1H;
+       retire-safety PT1H;
+       zone-propagation-delay PT1H;
+       max-zone-ttl P1D;
+       signatures-validity P10D;
+       signatures-refresh P3D;
+       parent-ds-ttl PT1H;
+       parent-propagation-delay PT5M;
+};
+
+dnssec-policy "bad-lifetime-zsk" {
+       /*
+        * The ZSK lifetime is too short.
+        * The KSK lifetime is good enough but should trigger a warning.
+        */
+       keys {
+               ksk lifetime PT3H1S algorithm 13;
+               zsk lifetime P8DT2H algorithm 13;
+       };
+
+       dnskey-ttl PT1H;
+       publish-safety PT1H;
+       retire-safety PT1H;
+       zone-propagation-delay PT1H;
+       max-zone-ttl P1D;
+       signatures-validity P10D;
+       signatures-refresh P3D;
+       parent-ds-ttl PT1H;
+       parent-propagation-delay PT5M;
+};
+
+dnssec-policy "bad-lifetime-csk" {
+       /*
+        * The CSK lifetime is too short.
+        */
+       keys {
+               csk lifetime PT3H algorithm 13;
+       };
+
+       dnskey-ttl PT1H;
+       publish-safety PT1H;
+       retire-safety PT1H;
+       zone-propagation-delay PT1H;
+       max-zone-ttl P1D;
+       signatures-validity P10D;
+       signatures-refresh P3D;
+       parent-ds-ttl PT1H;
+       parent-propagation-delay PT5M;
+};
+
+zone "bad-lifetime-ksk.example.net" {
+       type primary;
+       file "bad-lifetime-ksk.example.db";
+       dnssec-policy "bad-lifetime-ksk";
+};
+
+zone "bad-lifetime-zsk.example.net" {
+       type primary;
+       file "bad-lifetime-zsk.example.db";
+       dnssec-policy "bad-lifetime-zsk";
+};
+
+zone "bad-lifetime-csk.example.net" {
+       type primary;
+       file "bad-lifetime-csk.example.db";
+       dnssec-policy "bad-lifetime-csk";
+};
index 21b41f09e5941216f24b1426eb115a98358747fc..e1131dab514e7c71351ff137a154d9dcb9da9b20 100644 (file)
@@ -539,6 +539,15 @@ if [ $lines != 2 ]; then ret=1; fi
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
+n=`expr $n + 1`
+echo_i "checking named-checkconf kasp key lifetime errors ($n)"
+ret=0
+$CHECKCONF kasp-bad-lifetime.conf > checkconf.out$n 2>&1 && ret=1
+lines=$(grep "dnssec-policy: key lifetime is shorter than the time it takes to do a rollover" < checkconf.out$n | wc -l) || ret=1
+if [ $lines != 3 ]; then ret=1; fi
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=`expr $status + $ret`
+
 n=`expr $n + 1`
 echo_i "checking named-checkconf kasp predefined key length ($n)"
 ret=0
index de538690b65ecadb1d2ba37534e342b60fab1c82..d7a01ccd04371e881fc894b10d56134de6b30f81 100644 (file)
@@ -72,7 +72,8 @@ get_duration(const cfg_obj_t **maps, const char *option, uint32_t dfl) {
  */
 static isc_result_t
 cfg_kaspkey_fromconfig(const cfg_obj_t *config, dns_kasp_t *kasp,
-                      isc_log_t *logctx) {
+                      isc_log_t *logctx, uint32_t ksk_min_lifetime,
+                      uint32_t zsk_min_lifetime) {
        isc_result_t result;
        dns_kasp_key_t *key = NULL;
 
@@ -92,6 +93,7 @@ cfg_kaspkey_fromconfig(const cfg_obj_t *config, dns_kasp_t *kasp,
                const char *rolestr = NULL;
                const cfg_obj_t *obj = NULL;
                isc_consttextregion_t alg;
+               bool error = false;
 
                rolestr = cfg_obj_asstring(cfg_tuple_get(config, "role"));
                if (strcmp(rolestr, "ksk") == 0) {
@@ -108,10 +110,30 @@ cfg_kaspkey_fromconfig(const cfg_obj_t *config, dns_kasp_t *kasp,
                if (cfg_obj_isduration(obj)) {
                        key->lifetime = cfg_obj_asduration(obj);
                }
-               if (key->lifetime > 0 && key->lifetime < 30 * (24 * 3600)) {
-                       cfg_obj_log(obj, logctx, ISC_LOG_WARNING,
-                                   "dnssec-policy: key lifetime is shorter "
-                                   "than 30 days");
+               if (key->lifetime > 0) {
+                       if (key->lifetime < 30 * (24 * 3600)) {
+                               cfg_obj_log(obj, logctx, ISC_LOG_WARNING,
+                                           "dnssec-policy: key lifetime is "
+                                           "shorter than 30 days");
+                       }
+                       if ((key->role & DNS_KASP_KEY_ROLE_KSK) != 0 &&
+                           key->lifetime <= ksk_min_lifetime)
+                       {
+                               error = true;
+                       }
+                       if ((key->role & DNS_KASP_KEY_ROLE_ZSK) != 0 &&
+                           key->lifetime <= zsk_min_lifetime)
+                       {
+                               error = true;
+                       }
+                       if (error) {
+                               cfg_obj_log(obj, logctx, ISC_LOG_ERROR,
+                                           "dnssec-policy: key lifetime is "
+                                           "shorter than the time it takes to "
+                                           "do a rollover");
+                               result = ISC_R_FAILURE;
+                               goto cleanup;
+                       }
                }
 
                obj = cfg_tuple_get(config, "algorithm");
@@ -269,6 +291,8 @@ cfg_kasp_fromconfig(const cfg_obj_t *config, const char *name, isc_mem_t *mctx,
        dns_kasp_t *kasp = NULL;
        size_t i = 0;
        uint32_t sigrefresh = 0, sigvalidity = 0;
+       uint32_t ipub = 0, iret = 0;
+       uint32_t ksk_min_lifetime = 0, zsk_min_lifetime = 0;
 
        REQUIRE(kaspp != NULL && *kaspp == NULL);
 
@@ -313,17 +337,6 @@ cfg_kasp_fromconfig(const cfg_obj_t *config, const char *name, isc_mem_t *mctx,
                                  DNS_KASP_SIG_REFRESH);
        dns_kasp_setsigrefresh(kasp, sigrefresh);
 
-       sigvalidity = get_duration(maps, "signatures-validity",
-                                  DNS_KASP_SIG_VALIDITY);
-       if (sigrefresh >= (sigvalidity * 0.9)) {
-               cfg_obj_log(config, logctx, ISC_LOG_ERROR,
-                           "dnssec-policy: policy '%s' signatures-refresh "
-                           "must be at most 90%% of the signatures-validity",
-                           kaspname);
-               result = ISC_R_FAILURE;
-       }
-       dns_kasp_setsigvalidity(kasp, sigvalidity);
-
        sigvalidity = get_duration(maps, "signatures-validity-dnskey",
                                   DNS_KASP_SIG_VALIDITY_DNSKEY);
        if (sigrefresh >= (sigvalidity * 0.9)) {
@@ -336,6 +349,17 @@ cfg_kasp_fromconfig(const cfg_obj_t *config, const char *name, isc_mem_t *mctx,
        }
        dns_kasp_setsigvalidity_dnskey(kasp, sigvalidity);
 
+       sigvalidity = get_duration(maps, "signatures-validity",
+                                  DNS_KASP_SIG_VALIDITY);
+       if (sigrefresh >= (sigvalidity * 0.9)) {
+               cfg_obj_log(config, logctx, ISC_LOG_ERROR,
+                           "dnssec-policy: policy '%s' signatures-refresh "
+                           "must be at most 90%% of the signatures-validity",
+                           kaspname);
+               result = ISC_R_FAILURE;
+       }
+       dns_kasp_setsigvalidity(kasp, sigvalidity);
+
        if (result != ISC_R_SUCCESS) {
                goto cleanup;
        }
@@ -350,6 +374,26 @@ cfg_kasp_fromconfig(const cfg_obj_t *config, const char *name, isc_mem_t *mctx,
        dns_kasp_setpurgekeys(
                kasp, get_duration(maps, "purge-keys", DNS_KASP_PURGE_KEYS));
 
+       ipub = get_duration(maps, "dnskey-ttl", DNS_KASP_KEY_TTL) +
+              get_duration(maps, "publish-safety", DNS_KASP_PUBLISH_SAFETY) +
+              get_duration(maps, "zone-propagation-delay",
+                           DNS_KASP_ZONE_PROPDELAY);
+
+       iret = get_duration(maps, "parent-ds-ttl", DNS_KASP_DS_TTL) +
+              get_duration(maps, "retire-safety", DNS_KASP_RETIRE_SAFETY) +
+              get_duration(maps, "parent-propagation-delay",
+                           DNS_KASP_PARENT_PROPDELAY);
+
+       ksk_min_lifetime = ISC_MAX(ipub, iret);
+
+       iret = (sigvalidity - sigrefresh) +
+              get_duration(maps, "max-zone-ttl", DNS_KASP_ZONE_MAXTTL) +
+              get_duration(maps, "retire-safety", DNS_KASP_RETIRE_SAFETY) +
+              get_duration(maps, "zone-propagation-delay",
+                           DNS_KASP_ZONE_PROPDELAY);
+
+       zsk_min_lifetime = ISC_MAX(ipub, iret);
+
        (void)confget(maps, "keys", &keys);
        if (keys != NULL) {
                char role[256] = { 0 };
@@ -360,7 +404,9 @@ cfg_kasp_fromconfig(const cfg_obj_t *config, const char *name, isc_mem_t *mctx,
                     element = cfg_list_next(element))
                {
                        cfg_obj_t *kobj = cfg_listelt_value(element);
-                       result = cfg_kaspkey_fromconfig(kobj, kasp, logctx);
+                       result = cfg_kaspkey_fromconfig(kobj, kasp, logctx,
+                                                       ksk_min_lifetime,
+                                                       zsk_min_lifetime);
                        if (result != ISC_R_SUCCESS) {
                                goto cleanup;
                        }
@@ -424,7 +470,7 @@ cfg_kasp_fromconfig(const cfg_obj_t *config, const char *name, isc_mem_t *mctx,
                INSIST(dns_kasp_keylist_empty(kasp));
        } else {
                /* No keys clause configured, use the "default". */
-               result = cfg_kaspkey_fromconfig(NULL, kasp, logctx);
+               result = cfg_kaspkey_fromconfig(NULL, kasp, logctx, 0, 0);
                if (result != ISC_R_SUCCESS) {
                        goto cleanup;
                }