From 0d578097ef528b0eb8119c1a19d1fb7dccbb18b1 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Thu, 14 May 2020 15:06:54 +0200 Subject: [PATCH 1/4] Fix bug in keymgr_key_has_successor The logic in `keymgr_key_has_successor(key, keyring)` is flawed, it returns true if there is any key in the keyring that has a successor, while what we really want here is to make sure that the given key has a successor in the given keyring. Rather than relying on `keymgr_key_exists_with_state`, walk the list of keys in the keyring and check if the key is a successor of the given predecessor key. --- CHANGES | 4 ++++ bin/tests/system/kasp/tests.sh | 8 ++++---- doc/notes/notes-current.rst | 4 ++++ lib/dns/keymgr.c | 15 ++++++++++----- 4 files changed, 22 insertions(+), 9 deletions(-) diff --git a/CHANGES b/CHANGES index 9583c7030a..a98a447cf0 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5423. [bug] Fix a bug in keymgr_key_has_successor: it would + return a false positive if any other key in the + keyring has a successor. [GL #1845] + 5422. [bug] When using dnssec-policy, print correct keytiming metadata. [GL #1843] diff --git a/bin/tests/system/kasp/tests.sh b/bin/tests/system/kasp/tests.sh index b0f2d727e2..6547053d05 100644 --- a/bin/tests/system/kasp/tests.sh +++ b/bin/tests/system/kasp/tests.sh @@ -2533,6 +2533,7 @@ set_keyalgorithm "KEY3" "13" "ECDSAP256SHA256" "256" set_keysigning "KEY3" "no" set_zonesigning "KEY3" "no" # Key states. +set_keystate "KEY2" "GOAL" "hidden" set_keystate "KEY3" "GOAL" "omnipresent" set_keystate "KEY3" "STATE_DNSKEY" "rumoured" set_keystate "KEY3" "STATE_ZRRSIG" "hidden" @@ -2570,7 +2571,6 @@ set_server "ns3" "10.53.0.3" # ZSK (KEY2) no longer is actively signing, RRSIG state in UNRETENTIVE. # New ZSK (KEY3) is now actively signing, RRSIG state in RUMOURED. set_zonesigning "KEY2" "no" -set_keystate "KEY2" "GOAL" "hidden" set_keystate "KEY2" "STATE_ZRRSIG" "unretentive" set_zonesigning "KEY3" "yes" set_keystate "KEY3" "STATE_DNSKEY" "omnipresent" @@ -2749,6 +2749,7 @@ set_keyalgorithm "KEY3" "13" "ECDSAP256SHA256" "256" set_keysigning "KEY3" "yes" set_zonesigning "KEY3" "no" # Key states. +set_keystate "KEY1" "GOAL" "hidden" set_keystate "KEY3" "GOAL" "omnipresent" set_keystate "KEY3" "STATE_DNSKEY" "rumoured" set_keystate "KEY3" "STATE_KRRSIG" "rumoured" @@ -2792,7 +2793,6 @@ set_zone "step3.ksk-doubleksk.autosign" set_policy "ksk-doubleksk" "3" "7200" set_server "ns3" "10.53.0.3" # KSK (KEY1) DS will be removed, so it is UNRETENTIVE. -set_keystate "KEY1" "GOAL" "hidden" set_keystate "KEY1" "STATE_DS" "unretentive" # New KSK (KEY3) has its DS submitted. set_keystate "KEY3" "STATE_DNSKEY" "omnipresent" @@ -2978,6 +2978,7 @@ set_keyalgorithm "KEY2" "13" "ECDSAP256SHA256" "256" set_keysigning "KEY2" "yes" set_zonesigning "KEY2" "no" # Key states. +set_keystate "KEY1" "GOAL" "hidden" set_keystate "KEY2" "GOAL" "omnipresent" set_keystate "KEY2" "STATE_DNSKEY" "rumoured" set_keystate "KEY2" "STATE_KRRSIG" "rumoured" @@ -3019,7 +3020,6 @@ set_server "ns3" "10.53.0.3" set_zonesigning "KEY1" "no" set_zonesigning "KEY2" "yes" # CSK (KEY1) DS and ZRRSIG will be removed, so it is UNRETENTIVE. -set_keystate "KEY1" "GOAL" "hidden" set_keystate "KEY1" "STATE_ZRRSIG" "unretentive" set_keystate "KEY1" "STATE_DS" "unretentive" # New CSK (KEY2) has its DS submitted, and is signing, so the DS and ZRRSIG @@ -3277,6 +3277,7 @@ set_keyalgorithm "KEY2" "13" "ECDSAP256SHA256" "256" set_keysigning "KEY2" "yes" set_zonesigning "KEY2" "no" # Key states. +set_keystate "KEY1" "GOAL" "hidden" set_keystate "KEY2" "GOAL" "omnipresent" set_keystate "KEY2" "STATE_DNSKEY" "rumoured" set_keystate "KEY2" "STATE_KRRSIG" "rumoured" @@ -3315,7 +3316,6 @@ set_policy "csk-roll2" "2" "3600" set_server "ns3" "10.53.0.3" # CSK (KEY1) DS and ZRRSIG will be removed, so it is UNRETENTIVE. set_zonesigning "KEY1" "no" -set_keystate "KEY1" "GOAL" "hidden" set_keystate "KEY1" "STATE_ZRRSIG" "unretentive" set_keystate "KEY1" "STATE_DS" "unretentive" # New CSK (KEY2) has its DS submitted, and is signing, so the DS and ZRRSIG diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 4f71ff1e04..c371e2e5cd 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -128,3 +128,7 @@ Bug Fixes - ``named`` could crash with an assertion failure if the name of a database node was looked up while the database was being modified. [GL #1857] + +- Fix a bug in dnssec-policy keymgr where the check if a key has a + successor would return a false positive if any other key in the + keyring has a successor. [GL #1845] diff --git a/lib/dns/keymgr.c b/lib/dns/keymgr.c index 9a5480e24c..7a0831db50 100644 --- a/lib/dns/keymgr.c +++ b/lib/dns/keymgr.c @@ -633,11 +633,16 @@ keymgr_key_exists_with_state(dns_dnsseckeylist_t *keyring, dns_dnsseckey_t *key, * Check if a key has a successor. */ static bool -keymgr_key_has_successor(dns_dnsseckey_t *key, dns_dnsseckeylist_t *keyring) { - /* Don't worry about key states. */ - dst_key_state_t na[4] = { NA, NA, NA, NA }; - return (keymgr_key_exists_with_state(keyring, key, DST_KEY_DNSKEY, NA, - na, na, true, true)); +keymgr_key_has_successor(dns_dnsseckey_t *predecessor, + dns_dnsseckeylist_t *keyring) { + for (dns_dnsseckey_t *successor = ISC_LIST_HEAD(*keyring); + successor != NULL; successor = ISC_LIST_NEXT(successor, link)) + { + if (keymgr_key_is_successor(predecessor->key, successor->key)) { + return (true); + } + } + return (false); } /* From bcf81924381b43c2c14e3e22c371a18b68ffa719 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Thu, 14 May 2020 15:34:13 +0200 Subject: [PATCH 2/4] Put new key rollover logic in separate function The `dns_keymgr_run()` function became quite long, put the logic that looks if a new key needs to be created (start a key rollover) in a separate function. --- lib/dns/keymgr.c | 334 ++++++++++++++++++++++++----------------------- 1 file changed, 173 insertions(+), 161 deletions(-) diff --git a/lib/dns/keymgr.c b/lib/dns/keymgr.c index 7a0831db50..87abec4e70 100644 --- a/lib/dns/keymgr.c +++ b/lib/dns/keymgr.c @@ -1441,6 +1441,174 @@ keymgr_key_init(dns_dnsseckey_t *key, dns_kasp_t *kasp, isc_stdtime_t now) { } } +static isc_result_t +keymgr_key_rollover(dns_kasp_key_t *kaspkey, dns_dnsseckey_t *active_key, + dns_dnsseckeylist_t *keyring, dns_dnsseckeylist_t *newkeys, + const dns_name_t *origin, dns_rdataclass_t rdclass, + dns_kasp_t *kasp, uint32_t lifetime, isc_stdtime_t now, + isc_stdtime_t *nexttime, isc_mem_t *mctx) { + char keystr[DST_KEY_FORMATSIZE]; + isc_stdtime_t retire = 0, active = 0, prepub = 0; + dns_dnsseckey_t *new_key = NULL; + dns_dnsseckey_t *candidate = NULL; + dst_key_t *dst_key = NULL; + + /* Do we need to create a successor for the active key? */ + if (active_key != NULL) { + if (isc_log_wouldlog(dns_lctx, ISC_LOG_DEBUG(1))) { + dst_key_format(active_key->key, keystr, sizeof(keystr)); + isc_log_write( + dns_lctx, DNS_LOGCATEGORY_DNSSEC, + DNS_LOGMODULE_DNSSEC, ISC_LOG_DEBUG(1), + "keymgr: DNSKEY %s (%s) is active in policy %s", + keystr, keymgr_keyrole(active_key->key), + dns_kasp_getname(kasp)); + } + + /* + * Calculate when the successor needs to be published + * in the zone. + */ + prepub = keymgr_prepublication_time(active_key, kasp, lifetime, + now); + if (prepub == 0 || prepub > now) { + /* No need to start rollover now. */ + if (*nexttime == 0 || prepub < *nexttime) { + *nexttime = prepub; + } + return (ISC_R_SUCCESS); + } + + if (keymgr_key_has_successor(active_key, keyring)) { + /* Key already has successor. */ + return (ISC_R_SUCCESS); + } + + if (isc_log_wouldlog(dns_lctx, ISC_LOG_DEBUG(1))) { + dst_key_format(active_key->key, keystr, sizeof(keystr)); + isc_log_write(dns_lctx, DNS_LOGCATEGORY_DNSSEC, + DNS_LOGMODULE_DNSSEC, ISC_LOG_DEBUG(1), + "keymgr: need successor for DNSKEY %s " + "(%s) (policy %s)", + keystr, keymgr_keyrole(active_key->key), + dns_kasp_getname(kasp)); + } + } else if (isc_log_wouldlog(dns_lctx, ISC_LOG_DEBUG(1))) { + char namestr[DNS_NAME_FORMATSIZE]; + dns_name_format(origin, namestr, sizeof(namestr)); + isc_log_write(dns_lctx, DNS_LOGCATEGORY_DNSSEC, + DNS_LOGMODULE_DNSSEC, ISC_LOG_DEBUG(1), + "keymgr: no active key found for %s (policy %s)", + namestr, dns_kasp_getname(kasp)); + } + + /* It is time to do key rollover, we need a new key. */ + + /* + * Check if there is a key available in pool because keys + * may have been pregenerated with dnssec-keygen. + */ + for (candidate = ISC_LIST_HEAD(*keyring); candidate != NULL; + candidate = ISC_LIST_NEXT(candidate, link)) + { + if (keymgr_dnsseckey_kaspkey_match(candidate, kaspkey) && + dst_key_is_unused(candidate->key)) + { + /* Found a candidate in keyring. */ + break; + } + } + + if (candidate == NULL) { + /* No key available in keyring, create a new one. */ + isc_result_t result = keymgr_createkey(kaspkey, origin, rdclass, + mctx, keyring, &dst_key); + if (result != ISC_R_SUCCESS) { + return (result); + } + dst_key_setttl(dst_key, dns_kasp_dnskeyttl(kasp)); + dst_key_settime(dst_key, DST_TIME_CREATED, now); + result = dns_dnsseckey_create(mctx, &dst_key, &new_key); + if (result != ISC_R_SUCCESS) { + return (result); + } + keymgr_key_init(new_key, kasp, now); + } else { + new_key = candidate; + } + dst_key_setnum(new_key->key, DST_NUM_LIFETIME, lifetime); + + /* Got a key. */ + if (active_key == NULL) { + /* + * If there is no active key found yet for this kasp + * key configuration, immediately make this key active. + */ + dst_key_settime(new_key->key, DST_TIME_PUBLISH, now); + dst_key_settime(new_key->key, DST_TIME_ACTIVATE, now); + keymgr_settime_syncpublish(new_key, kasp, true); + active = now; + } else { + /* + * This is a successor. Mark the relationship. + */ + isc_stdtime_t created; + (void)dst_key_gettime(new_key->key, DST_TIME_CREATED, &created); + + dst_key_setnum(new_key->key, DST_NUM_PREDECESSOR, + dst_key_id(active_key->key)); + dst_key_setnum(active_key->key, DST_NUM_SUCCESSOR, + dst_key_id(new_key->key)); + (void)dst_key_gettime(active_key->key, DST_TIME_INACTIVE, + &retire); + active = retire; + + /* + * If prepublication time and/or retire time are + * in the past (before the new key was created), use + * creation time as published and active time, + * effectively immediately making the key active. + */ + if (prepub < created) { + active += (created - prepub); + prepub = created; + } + if (active < created) { + active = created; + } + dst_key_settime(new_key->key, DST_TIME_PUBLISH, prepub); + dst_key_settime(new_key->key, DST_TIME_ACTIVATE, active); + keymgr_settime_syncpublish(new_key, kasp, false); + } + + /* This key wants to be present. */ + dst_key_setstate(new_key->key, DST_KEY_GOAL, OMNIPRESENT); + + /* Do we need to set retire time? */ + if (lifetime > 0) { + dst_key_settime(new_key->key, DST_TIME_INACTIVE, + (active + lifetime)); + keymgr_settime_remove(new_key, kasp); + } + + /* Append dnsseckey to list of new keys. */ + dns_dnssec_get_hints(new_key, now); + new_key->source = dns_keysource_repository; + INSIST(!new_key->legacy); + if (candidate == NULL) { + ISC_LIST_APPEND(*newkeys, new_key, link); + } + + /* Logging. */ + dst_key_format(new_key->key, keystr, sizeof(keystr)); + isc_log_write(dns_lctx, DNS_LOGCATEGORY_DNSSEC, DNS_LOGMODULE_DNSSEC, + ISC_LOG_INFO, "keymgr: DNSKEY %s (%s) %s for policy %s", + keystr, keymgr_keyrole(new_key->key), + (candidate != NULL) ? "selected" : "created", + dns_kasp_getname(kasp)); + return (ISC_R_SUCCESS); +} + /* * Examine 'keys' and match 'kasp' policy. * @@ -1453,9 +1621,7 @@ dns_keymgr_run(const dns_name_t *origin, dns_rdataclass_t rdclass, isc_result_t result = ISC_R_SUCCESS; dns_dnsseckeylist_t newkeys; dns_kasp_key_t *kkey; - dns_dnsseckey_t *candidate = NULL; dns_dnsseckey_t *newkey = NULL; - dst_key_t *dst_key = NULL; isc_dir_t dir; bool dir_open = false; int options = (DST_TYPE_PRIVATE | DST_TYPE_PUBLIC | DST_TYPE_STATE); @@ -1465,6 +1631,7 @@ dns_keymgr_run(const dns_name_t *origin, dns_rdataclass_t rdclass, REQUIRE(keyring != NULL); ISC_LIST_INIT(newkeys); + isc_dir_init(&dir); if (directory == NULL) { directory = "."; @@ -1525,7 +1692,6 @@ dns_keymgr_run(const dns_name_t *origin, dns_rdataclass_t rdclass, for (kkey = ISC_LIST_HEAD(dns_kasp_keys(kasp)); kkey != NULL; kkey = ISC_LIST_NEXT(kkey, link)) { - isc_stdtime_t retire = 0, active = 0, prepub = 0; uint32_t lifetime = dns_kasp_key_lifetime(kkey); dns_dnsseckey_t *active_key = NULL; @@ -1602,163 +1768,10 @@ dns_keymgr_run(const dns_name_t *origin, dns_rdataclass_t rdclass, } } - /* Do we need to create a successor for the active key? */ - if (active_key != NULL) { - if (isc_log_wouldlog(dns_lctx, ISC_LOG_DEBUG(1))) { - dst_key_format(active_key->key, keystr, - sizeof(keystr)); - isc_log_write( - dns_lctx, DNS_LOGCATEGORY_DNSSEC, - DNS_LOGMODULE_DNSSEC, ISC_LOG_DEBUG(1), - "keymgr: DNSKEY %s (%s) is active in " - "policy %s", - keystr, keymgr_keyrole(active_key->key), - dns_kasp_getname(kasp)); - } - - /* - * Calculate when the successor needs to be published - * in the zone. - */ - prepub = keymgr_prepublication_time(active_key, kasp, - lifetime, now); - if (prepub == 0 || prepub > now) { - /* No need to start rollover now. */ - if (*nexttime == 0 || prepub < *nexttime) { - *nexttime = prepub; - } - continue; - } - if (keymgr_key_has_successor(active_key, keyring)) { - /* Key already has successor. */ - continue; - } - - if (isc_log_wouldlog(dns_lctx, ISC_LOG_DEBUG(1))) { - dst_key_format(active_key->key, keystr, - sizeof(keystr)); - isc_log_write( - dns_lctx, DNS_LOGCATEGORY_DNSSEC, - DNS_LOGMODULE_DNSSEC, ISC_LOG_DEBUG(1), - "keymgr: need successor for " - "DNSKEY %s (%s) (policy %s)", - keystr, keymgr_keyrole(active_key->key), - dns_kasp_getname(kasp)); - } - } else if (isc_log_wouldlog(dns_lctx, ISC_LOG_DEBUG(1))) { - char namestr[DNS_NAME_FORMATSIZE]; - dns_name_format(origin, namestr, sizeof(namestr)); - isc_log_write(dns_lctx, DNS_LOGCATEGORY_DNSSEC, - DNS_LOGMODULE_DNSSEC, ISC_LOG_DEBUG(1), - "keymgr: no active key found for %s " - "(policy %s)", - namestr, dns_kasp_getname(kasp)); - } - - /* It is time to do key rollover, we need a new key. */ - - /* - * Check if there is a key available in pool because keys - * may have been pregenerated with dnssec-keygen. - */ - for (candidate = ISC_LIST_HEAD(*keyring); candidate != NULL; - candidate = ISC_LIST_NEXT(candidate, link)) - { - if (keymgr_dnsseckey_kaspkey_match(candidate, kkey) && - dst_key_is_unused(candidate->key)) - { - /* Found a candidate in keyring. */ - break; - } - } - - if (candidate == NULL) { - /* No key available in keyring, create a new one. */ - RETERR(keymgr_createkey(kkey, origin, rdclass, mctx, - keyring, &dst_key)); - dst_key_setttl(dst_key, dns_kasp_dnskeyttl(kasp)); - dst_key_settime(dst_key, DST_TIME_CREATED, now); - RETERR(dns_dnsseckey_create(mctx, &dst_key, &newkey)); - keymgr_key_init(newkey, kasp, now); - } else { - newkey = candidate; - } - dst_key_setnum(newkey->key, DST_NUM_LIFETIME, lifetime); - - /* Got a key. */ - if (active_key == NULL) { - /* - * If there is no active key found yet for this kasp - * key configuration, immediately make this key active. - */ - dst_key_settime(newkey->key, DST_TIME_PUBLISH, now); - dst_key_settime(newkey->key, DST_TIME_ACTIVATE, now); - keymgr_settime_syncpublish(newkey, kasp, true); - active = now; - } else { - /* - * This is a successor. Mark the relationship. - */ - isc_stdtime_t created; - (void)dst_key_gettime(newkey->key, DST_TIME_CREATED, - &created); - - dst_key_setnum(newkey->key, DST_NUM_PREDECESSOR, - dst_key_id(active_key->key)); - dst_key_setnum(active_key->key, DST_NUM_SUCCESSOR, - dst_key_id(newkey->key)); - (void)dst_key_gettime(active_key->key, - DST_TIME_INACTIVE, &retire); - active = retire; - - /* - * If prepublication time and/or retire time are - * in the past (before the new key was created), use - * creation time as published and active time, - * effectively immediately making the key active. - */ - if (prepub < created) { - active += (created - prepub); - prepub = created; - } - if (active < created) { - active = created; - } - dst_key_settime(newkey->key, DST_TIME_PUBLISH, prepub); - dst_key_settime(newkey->key, DST_TIME_ACTIVATE, active); - keymgr_settime_syncpublish(newkey, kasp, false); - } - - /* This key wants to be present. */ - dst_key_setstate(newkey->key, DST_KEY_GOAL, OMNIPRESENT); - - /* Do we need to set retire time? */ - if (lifetime > 0) { - dst_key_settime(newkey->key, DST_TIME_INACTIVE, - (active + lifetime)); - keymgr_settime_remove(newkey, kasp); - } - - /* Append dnsseckey to list of new keys. */ - dns_dnssec_get_hints(newkey, now); - newkey->source = dns_keysource_repository; - INSIST(!newkey->legacy); - if (candidate == NULL) { - ISC_LIST_APPEND(newkeys, newkey, link); - } - - /* Logging. */ - dst_key_format(newkey->key, keystr, sizeof(keystr)); - isc_log_write(dns_lctx, DNS_LOGCATEGORY_DNSSEC, - DNS_LOGMODULE_DNSSEC, ISC_LOG_INFO, - "keymgr: DNSKEY %s (%s) %s for policy %s", keystr, - keymgr_keyrole(newkey->key), - (candidate != NULL) ? "selected" : "created", - dns_kasp_getname(kasp)); - - /* Onwards to next kasp key configuration. */ - candidate = NULL; - newkey = NULL; + /* See if this key requires a rollover. */ + RETERR(keymgr_key_rollover(kkey, active_key, keyring, &newkeys, + origin, rdclass, kasp, lifetime, now, + nexttime, mctx)); } /* Walked all kasp key configurations. Append new keys. */ @@ -1784,7 +1797,6 @@ failure: isc_dir_close(&dir); } - INSIST(newkey == NULL); if (result != ISC_R_SUCCESS) { while ((newkey = ISC_LIST_HEAD(newkeys)) != NULL) { ISC_LIST_UNLINK(newkeys, newkey, link); From c08d0f7dd68811bbcacab93555959e30e3aada38 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Thu, 14 May 2020 15:36:13 +0200 Subject: [PATCH 3/4] If prepub > retire, prepub now Catch a case where if the prepublication time of the successor key is later than the retire time of the predecessor. If that is the case we should prepublish as soon as possible, a.k.a. now. --- lib/dns/keymgr.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/dns/keymgr.c b/lib/dns/keymgr.c index 87abec4e70..e5f1860c25 100644 --- a/lib/dns/keymgr.c +++ b/lib/dns/keymgr.c @@ -287,6 +287,10 @@ keymgr_prepublication_time(dns_dnsseckey_t *key, dns_kasp_t *kasp, /* * Publish successor 'prepub' time before the 'retire' time of 'key'. */ + if (prepub > retire) { + /* We should have already prepublished the new key. */ + return (now); + } return (retire - prepub); } From e71d60299f5672657209b982ba7760493e364b78 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Thu, 14 May 2020 15:39:57 +0200 Subject: [PATCH 4/4] Retire predecessor when creating successor When creating the successor, the current active key (predecessor) should change its goal state to HIDDEN. Also add two useful debug logs in the keymgr_key_rollover function. --- CHANGES | 5 +++++ doc/notes/notes-current.rst | 5 +++++ lib/dns/keymgr.c | 29 +++++++++++++++++++++++++++++ 3 files changed, 39 insertions(+) diff --git a/CHANGES b/CHANGES index a98a447cf0..e978a40968 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,8 @@ +5424. [bug] With kasp, when creating a successor key, the goal + state of the current active key (predecessor) was not + changed and thus was never is removed from the zone. + [GL #1846] + 5423. [bug] Fix a bug in keymgr_key_has_successor: it would return a false positive if any other key in the keyring has a successor. [GL #1845] diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index c371e2e5cd..aa4bd02457 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -132,3 +132,8 @@ Bug Fixes - Fix a bug in dnssec-policy keymgr where the check if a key has a successor would return a false positive if any other key in the keyring has a successor. [GL #1845] + +- With dnssec-policy, when creating a successor key, the goal state of + the current active key (the predecessor) was not changed and thus was + never is removed from the zone. [GL #1846] + diff --git a/lib/dns/keymgr.c b/lib/dns/keymgr.c index e5f1860c25..6ede7f975b 100644 --- a/lib/dns/keymgr.c +++ b/lib/dns/keymgr.c @@ -1476,6 +1476,19 @@ keymgr_key_rollover(dns_kasp_key_t *kaspkey, dns_dnsseckey_t *active_key, prepub = keymgr_prepublication_time(active_key, kasp, lifetime, now); if (prepub == 0 || prepub > now) { + if (isc_log_wouldlog(dns_lctx, ISC_LOG_DEBUG(1))) { + dst_key_format(active_key->key, keystr, + sizeof(keystr)); + isc_log_write( + dns_lctx, DNS_LOGCATEGORY_DNSSEC, + DNS_LOGMODULE_DNSSEC, ISC_LOG_DEBUG(1), + "keymgr: new successor needed for " + "DNSKEY %s (%s) (policy %s) in %u " + "seconds", + keystr, keymgr_keyrole(active_key->key), + dns_kasp_getname(kasp), (prepub - now)); + } + /* No need to start rollover now. */ if (*nexttime == 0 || prepub < *nexttime) { *nexttime = prepub; @@ -1485,6 +1498,17 @@ keymgr_key_rollover(dns_kasp_key_t *kaspkey, dns_dnsseckey_t *active_key, if (keymgr_key_has_successor(active_key, keyring)) { /* Key already has successor. */ + if (isc_log_wouldlog(dns_lctx, ISC_LOG_DEBUG(1))) { + dst_key_format(active_key->key, keystr, + sizeof(keystr)); + isc_log_write( + dns_lctx, DNS_LOGCATEGORY_DNSSEC, + DNS_LOGMODULE_DNSSEC, ISC_LOG_DEBUG(1), + "keymgr: key DNSKEY %s (%s) (policy " + "%s) already has successor", + keystr, keymgr_keyrole(active_key->key), + dns_kasp_getname(kasp)); + } return (ISC_R_SUCCESS); } @@ -1583,6 +1607,11 @@ keymgr_key_rollover(dns_kasp_key_t *kaspkey, dns_dnsseckey_t *active_key, dst_key_settime(new_key->key, DST_TIME_PUBLISH, prepub); dst_key_settime(new_key->key, DST_TIME_ACTIVATE, active); keymgr_settime_syncpublish(new_key, kasp, false); + + /* + * Retire predecessor. + */ + dst_key_setstate(active_key->key, DST_KEY_GOAL, HIDDEN); } /* This key wants to be present. */