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.

diff --git a/CHANGES b/CHANGES
index a98a447cf03bd6cc39cb6a6656c5ddc5140b36e9..e978a4096840356e790c47b5117df6cce919315b 100644 (file)
--- 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 c371e2e5cde02a24afd4a1a413d33915eaae37a8..aa4bd024575d8da81e6ba2bd9ee654b1c6abd4b4 100644 (file)
--- 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 e5f1860c25e33ea7a0c44c9cc7c51181c29b53a0..6ede7f975bfdf5c581c06e24ebd4556b52349825 100644 (file)
--- 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. */