From: Matthijs Mekking <matthijs@isc.org>
Date: Thu, 14 May 2020 13:39:57 +0000 (+0200)
Subject: Retire predecessor when creating successor
X-Git-Tag: v9.17.2~38^2
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=e71d60299f5672657209b982ba7760493e364b78;p=thirdparty%2Fbind9.git

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 a98a447cf03..e978a409684 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 c371e2e5cde..aa4bd024575 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 e5f1860c25e..6ede7f975bf 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. */