Correctly initialize old key with state file

The 'key_init()' function is used to initialize a state file for keys
that don't have one yet. This can happen if you are migrating from a
'auto-dnssec' or 'inline-signing' to a 'dnssec-policy' configuration.

It did not look at the "Inactive" and "Delete" timing metadata and so
old keys left behind in the key directory would also be considered as
a possible active key. This commit fixes this and now explicitly sets
the key goal to OMNIPRESENT for keys that have their "Active/Publish"
timing metadata in the past, but their "Inactive/Delete" timing
metadata in the future. If the "Inactive/Delete" timing metadata is
also in the past, the key goal is set to HIDDEN.

If the "Inactive/Delete" timing metadata is in the past, also the
key states are adjusted to either UNRETENTIVE or HIDDEN, depending on
how far in the past the metadata is set.

diff --git a/CHANGES b/CHANGES
index 344afbca5fa80f3e31e8d5a23ef6d40ba5f2cbea..437266581202b9e9bb5f12fc74cd7aee7fa4349b 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,7 @@
+5575.  [bug]           When migrating to dnssec-policy, BIND considered keys
+                       with the "Inactive" and/or "Delete" timing metadata as
+                       possible active keys. This has been fixed. [GL #2406]
+
 5574.  [func]          Incoming zone transfers can now use TLS.
                        Addresses in a "primaries" list take an optional
                        "tls" argument, specifying either a previously

diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst
index 82677a00ee28369b492a5684c58033a1350341d4..844902dcf4ffa6660f5768c946be2ceadb195cc0 100644 (file)
--- a/doc/notes/notes-current.rst
+++ b/doc/notes/notes-current.rst
@@ -92,3 +92,7 @@ Bug Fixes
 
 - Named ``allow-update`` acls where broken in BIND 9.17.9 and BIND 9.16.11
   preventing ``named`` starting. [GL #2413]
+
+- When migrating to ``dnssec-policy``, BIND considered keys with the "Inactive"
+  and/or "Delete" timing metadata as possible active keys. This has been fixed.
+  [GL #2406]

diff --git a/lib/dns/keymgr.c b/lib/dns/keymgr.c
index bfd8009c3a5a92760aa4211fdb781ba50b40877e..170eca37f4ba23a7bd78564996682397e8394c5f 100644 (file)
--- a/lib/dns/keymgr.c
+++ b/lib/dns/keymgr.c
@@ -1407,10 +1407,11 @@ static void
 keymgr_key_init(dns_dnsseckey_t *key, dns_kasp_t *kasp, isc_stdtime_t now) {
        bool ksk, zsk;
        isc_result_t ret;
-       isc_stdtime_t active = 0, pub = 0, syncpub = 0;
+       isc_stdtime_t active = 0, pub = 0, syncpub = 0, retire = 0, remove = 0;
        dst_key_state_t dnskey_state = HIDDEN;
        dst_key_state_t ds_state = HIDDEN;
        dst_key_state_t zrrsig_state = HIDDEN;
+       dst_key_state_t goal_state = HIDDEN;
 
        REQUIRE(key != NULL);
        REQUIRE(key->key != NULL);
@@ -1437,6 +1438,7 @@ keymgr_key_init(dns_dnsseckey_t *key, dns_kasp_t *kasp, isc_stdtime_t now) {
                } else {
                        dnskey_state = RUMOURED;
                }
+               goal_state = OMNIPRESENT;
        }
        ret = dst_key_gettime(key->key, DST_TIME_PUBLISH, &pub);
        if (pub <= now && ret == ISC_R_SUCCESS) {
@@ -1447,6 +1449,7 @@ keymgr_key_init(dns_dnsseckey_t *key, dns_kasp_t *kasp, isc_stdtime_t now) {
                } else {
                        zrrsig_state = RUMOURED;
                }
+               goal_state = OMNIPRESENT;
        }
        ret = dst_key_gettime(key->key, DST_TIME_SYNCPUBLISH, &syncpub);
        if (syncpub <= now && ret == ISC_R_SUCCESS) {
@@ -1457,6 +1460,38 @@ keymgr_key_init(dns_dnsseckey_t *key, dns_kasp_t *kasp, isc_stdtime_t now) {
                } else {
                        ds_state = RUMOURED;
                }
+               goal_state = OMNIPRESENT;
+       }
+       ret = dst_key_gettime(key->key, DST_TIME_INACTIVE, &retire);
+       if (retire <= now && ret == ISC_R_SUCCESS) {
+               dns_ttl_t zone_ttl = dns_kasp_zonemaxttl(kasp);
+               zone_ttl += dns_kasp_zonepropagationdelay(kasp);
+               if ((retire + zone_ttl) <= now) {
+                       zrrsig_state = HIDDEN;
+               } else {
+                       zrrsig_state = UNRETENTIVE;
+               }
+               ds_state = UNRETENTIVE;
+               goal_state = HIDDEN;
+       }
+       ret = dst_key_gettime(key->key, DST_TIME_DELETE, &remove);
+       if (remove <= now && ret == ISC_R_SUCCESS) {
+               dns_ttl_t key_ttl = dst_key_getttl(key->key);
+               key_ttl += dns_kasp_zonepropagationdelay(kasp);
+               if ((remove + key_ttl) <= now) {
+                       dnskey_state = HIDDEN;
+               } else {
+                       dnskey_state = UNRETENTIVE;
+               }
+               zrrsig_state = HIDDEN;
+               ds_state = HIDDEN;
+               goal_state = HIDDEN;
+       }
+
+       /* Set goal if not already set. */
+       if (dst_key_getstate(key->key, DST_KEY_GOAL, &goal_state) !=
+           ISC_R_SUCCESS) {
+               dst_key_setstate(key->key, DST_KEY_GOAL, goal_state);
        }
 
        /* Set key states for all keys that do not have them. */