From: Matthijs Mekking <matthijs@isc.org>
Date: Mon, 12 Apr 2021 14:22:00 +0000 (+0200)
Subject: Check for keyid conflicts between new keys
X-Git-Tag: v9.16.16~26^2~1
X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=f82d4f0474eaead7b5212f8b6abc73edb71a1854;p=thirdparty%2Fbind9.git

Check for keyid conflicts between new keys

When the keymgr needs to create new keys, it is possible it needs to
create multiple keys. The keymgr checks for keyid conflicts with
already existing keys, but it should also check against that it just
created.

(cherry picked from commit 668301f138ab40cef93a70ee177a97ca85eefef7)
---

diff --git a/lib/dns/keymgr.c b/lib/dns/keymgr.c
index 05ee2e74e49..b4efb3aeb26 100644
--- a/lib/dns/keymgr.c
+++ b/lib/dns/keymgr.c
@@ -392,6 +392,29 @@ keymgr_dnsseckey_kaspkey_match(dns_dnsseckey_t *dkey, dns_kasp_key_t *kkey) {
 	return (true);
 }
 
+static bool
+keymgr_keyid_conflict(dst_key_t *newkey, dns_dnsseckeylist_t *keys) {
+	uint16_t id = dst_key_id(newkey);
+	uint32_t rid = dst_key_rid(newkey);
+	uint32_t alg = dst_key_alg(newkey);
+
+	for (dns_dnsseckey_t *dkey = ISC_LIST_HEAD(*keys); dkey != NULL;
+	     dkey = ISC_LIST_NEXT(dkey, link))
+	{
+		if (dst_key_alg(dkey->key) != alg) {
+			continue;
+		}
+		if (dst_key_id(dkey->key) == id ||
+		    dst_key_rid(dkey->key) == id ||
+		    dst_key_id(dkey->key) == rid ||
+		    dst_key_rid(dkey->key) == rid)
+		{
+			return (true);
+		}
+	}
+	return (false);
+}
+
 /*
  * Create a new key for 'origin' given the kasp key configuration 'kkey'.
  * This will check for key id collisions with keys in 'keylist'.
@@ -401,20 +424,17 @@ keymgr_dnsseckey_kaspkey_match(dns_dnsseckey_t *dkey, dns_kasp_key_t *kkey) {
 static isc_result_t
 keymgr_createkey(dns_kasp_key_t *kkey, const dns_name_t *origin,
 		 dns_rdataclass_t rdclass, isc_mem_t *mctx,
-		 dns_dnsseckeylist_t *keylist, dst_key_t **dst_key) {
-	bool conflict;
+		 dns_dnsseckeylist_t *keylist, dns_dnsseckeylist_t *newkeys,
+		 dst_key_t **dst_key) {
+	bool conflict = false;
 	int keyflags = DNS_KEYOWNER_ZONE;
 	isc_result_t result = ISC_R_SUCCESS;
 	dst_key_t *newkey = NULL;
 
 	do {
-		uint16_t id;
-		uint32_t rid;
 		uint32_t algo = dns_kasp_key_algorithm(kkey);
 		int size = dns_kasp_key_size(kkey);
 
-		conflict = false;
-
 		if (dns_kasp_key_ksk(kkey)) {
 			keyflags |= DNS_KEYFLAG_KSK;
 		}
@@ -423,28 +443,17 @@ keymgr_createkey(dns_kasp_key_t *kkey, const dns_name_t *origin,
 					&newkey, NULL));
 
 		/* Key collision? */
-		id = dst_key_id(newkey);
-		rid = dst_key_rid(newkey);
-		for (dns_dnsseckey_t *dkey = ISC_LIST_HEAD(*keylist);
-		     dkey != NULL; dkey = ISC_LIST_NEXT(dkey, link))
-		{
-			if (dst_key_alg(dkey->key) != algo) {
-				continue;
-			}
-			if (dst_key_id(dkey->key) == id ||
-			    dst_key_rid(dkey->key) == id ||
-			    dst_key_id(dkey->key) == rid ||
-			    dst_key_rid(dkey->key) == rid)
-			{
-				/* Try again. */
-				conflict = true;
-				isc_log_write(dns_lctx, DNS_LOGCATEGORY_DNSSEC,
-					      DNS_LOGMODULE_DNSSEC,
-					      ISC_LOG_WARNING,
-					      "keymgr: key collision id %d",
-					      dst_key_id(newkey));
-				dst_key_free(&newkey);
-			}
+		conflict = keymgr_keyid_conflict(newkey, keylist);
+		if (!conflict) {
+			conflict = keymgr_keyid_conflict(newkey, newkeys);
+		}
+		if (conflict) {
+			/* Try again. */
+			isc_log_write(dns_lctx, DNS_LOGCATEGORY_DNSSEC,
+				      DNS_LOGMODULE_DNSSEC, ISC_LOG_WARNING,
+				      "keymgr: key collision id %d",
+				      dst_key_id(newkey));
+			dst_key_free(&newkey);
 		}
 	} while (conflict);
 
@@ -1732,7 +1741,8 @@ keymgr_key_rollover(dns_kasp_key_t *kaspkey, dns_dnsseckey_t *active_key,
 	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);
+						       mctx, keyring, newkeys,
+						       &dst_key);
 		if (result != ISC_R_SUCCESS) {
 			return (result);
 		}