From 76cf72e65a7ff23d3ad10dc917d58d7520e1d44e Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Tue, 26 Jan 2021 11:32:24 +0100 Subject: [PATCH] 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. --- CHANGES | 4 ++++ doc/notes/notes-current.rst | 4 ++++ lib/dns/keymgr.c | 37 ++++++++++++++++++++++++++++++++++++- 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index 344afbca5f..4372665812 100644 --- 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 82677a00ee..844902dcf4 100644 --- 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 bfd8009c3a..170eca37f4 100644 --- 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. */