From: Matthijs Mekking <matthijs@isc.org>
Date: Tue, 14 Jan 2025 14:18:40 +0000 (+0100)
Subject: dnssec-signzone retain signature if key is offline
X-Git-Tag: v9.21.5~35^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5e3aef364fc3c6382a5761eb339ca780df0829ef;p=thirdparty%2Fbind9.git

dnssec-signzone retain signature if key is offline

Track inside the dns_dnsseckey structure whether we have seen the
private key, or if this key only has a public key file.

If the key only has a public key file, or a DNSKEY reference in the
zone, mark the key 'pubkey'. In dnssec-signzone, if the key only
has a public key available, consider the key to be offline. Any
signatures that should be refreshed for which the key is not available,
retain the signature.

So in the code, 'expired' becomes 'refresh', and the new 'expired'
is only used to determine whether we need to keep the signature if
the corresponding key is not available (retaining the signature if
it is not expired).

In the 'keysthatsigned' function, we can remove:
  -	key->force_publish = false;
  -	key->force_sign = false;

because they are redundant ('dns_dnsseckey_create' already sets these
values to false).
---

diff --git a/bin/dnssec/dnssec-signzone.c b/bin/dnssec/dnssec-signzone.c
index fcdfe15e046..748f28f3bcd 100644
--- a/bin/dnssec/dnssec-signzone.c
+++ b/bin/dnssec/dnssec-signzone.c
@@ -419,10 +419,9 @@ keythatsigned(dns_rdata_rrsig_t *rrsig) {
 		dns_dnsseckey_create(mctx, &privkey, &key);
 	} else {
 		dns_dnsseckey_create(mctx, &pubkey, &key);
+		key->pubkey = true;
 	}
 
-	key->force_publish = false;
-	key->force_sign = false;
 	key->index = keycount++;
 	ISC_LIST_APPEND(keylist, key, link);
 
@@ -540,7 +539,7 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name,
 	}
 
 	while (result == ISC_R_SUCCESS) {
-		bool expired, future;
+		bool expired, refresh, future, offline;
 		bool keep = false, resign = false;
 
 		dns_rdataset_current(&sigset, &sigrdata);
@@ -551,8 +550,10 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name,
 		future = isc_serial_lt(now, rrsig.timesigned);
 
 		key = keythatsigned(&rrsig);
+		offline = key->pubkey;
 		sig_format(&rrsig, sigstr, sizeof(sigstr));
-		expired = isc_serial_gt(now + cycle, rrsig.timeexpire);
+		expired = isc_serial_gt(now, rrsig.timeexpire);
+		refresh = isc_serial_gt(now + cycle, rrsig.timeexpire);
 
 		if (isc_serial_gt(rrsig.timesigned, rrsig.timeexpire)) {
 			/* rrsig is dropped and not replaced */
@@ -581,15 +582,21 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name,
 		} else if (issigningkey(key)) {
 			wassignedby[key->index] = true;
 
-			if (!expired && rrsig.originalttl == set->ttl &&
+			if (!refresh && rrsig.originalttl == set->ttl &&
 			    setverifies(name, set, key->key, &sigrdata))
 			{
 				vbprintf(2, "\trrsig by %s retained\n", sigstr);
 				keep = true;
+			} else if (offline) {
+				vbprintf(2,
+					 "\trrsig by %s retained - private key "
+					 "missing\n",
+					 sigstr);
+				keep = true;
 			} else {
 				vbprintf(2, "\trrsig by %s dropped - %s\n",
 					 sigstr,
-					 expired ? "expired"
+					 refresh ? "refresh"
 					 : rrsig.originalttl != set->ttl
 						 ? "ttl change"
 						 : "failed to "
@@ -602,25 +609,32 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name,
 		} else if (iszonekey(key)) {
 			wassignedby[key->index] = true;
 
-			if (!expired && rrsig.originalttl == set->ttl &&
+			if (!refresh && rrsig.originalttl == set->ttl &&
 			    setverifies(name, set, key->key, &sigrdata))
 			{
 				vbprintf(2, "\trrsig by %s retained\n", sigstr);
 				keep = true;
+			} else if (offline) {
+				vbprintf(2,
+					 "\trrsig by %s retained - private key "
+					 "missing\n",
+					 sigstr);
+				keep = true;
 			} else {
 				vbprintf(2, "\trrsig by %s dropped - %s\n",
 					 sigstr,
-					 expired ? "expired"
+					 refresh ? "refresh"
 					 : rrsig.originalttl != set->ttl
 						 ? "ttl change"
 						 : "failed to "
 						   "verify");
 			}
-		} else if (!expired) {
+		} else if (!refresh) {
 			vbprintf(2, "\trrsig by %s retained\n", sigstr);
 			keep = true;
 		} else {
-			vbprintf(2, "\trrsig by %s expired\n", sigstr);
+			vbprintf(2, "\trrsig by %s %s\n", sigstr,
+				 expired ? "expired" : "needs refresh");
 		}
 
 		if (keep) {
diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh
index 1c3876e0fba..400d9db8062 100644
--- a/bin/tests/system/dnssec/tests.sh
+++ b/bin/tests/system/dnssec/tests.sh
@@ -1361,14 +1361,14 @@ status=$((status + ret))
 echo_ic "two DNSKEYs, DNSKEY RRset only by KSK ($n)"
 ret=0
 (
-cd signer/general || exit 1
-rm -f signed.zone
-$SIGNER -s now-1mo -e now+2d -P -x -f signed.zone -O full -o example.com. test1.zone >signer.out.$n
-test -f signed.zone
+  cd signer/general || exit 1
+  rm -f signed.zone
+  $SIGNER -s now-1mo -e now+2d -P -x -f signed.zone -O full -o example.com. test1.zone >signer.out.$n
+  test -f signed.zone
 ) || ret=1
-n=$((n+1))
+n=$((n + 1))
 test "$ret" -eq 0 || echo_i "failed"
-status=$((status+ret))
+status=$((status + ret))
 
 echo_ic "two DNSKEYs, DNSKEY RRset only by KSK, private key missing ($n)"
 ret=0
diff --git a/lib/dns/dnssec.c b/lib/dns/dnssec.c
index dcc418f9551..ef3aefd3d74 100644
--- a/lib/dns/dnssec.c
+++ b/lib/dns/dnssec.c
@@ -1121,6 +1121,7 @@ dns_dnsseckey_create(isc_mem_t *mctx, dst_key_t **dstkey,
 	dk->hint_remove = false;
 	dk->first_sign = false;
 	dk->is_active = false;
+	dk->pubkey = false;
 	dk->purge = false;
 	dk->prepublish = 0;
 	dk->source = dns_keysource_unknown;
@@ -1409,7 +1410,7 @@ failure:
  */
 static void
 addkey(dns_dnsseckeylist_t *keylist, dst_key_t **newkey, bool savekeys,
-       isc_mem_t *mctx) {
+       bool pubkey_only, isc_mem_t *mctx) {
 	dns_dnsseckey_t *key = NULL;
 
 	/* Skip duplicates */
@@ -1444,6 +1445,7 @@ addkey(dns_dnsseckeylist_t *keylist, dst_key_t **newkey, bool savekeys,
 	}
 
 	dns_dnsseckey_create(mctx, newkey, &key);
+	key->pubkey = pubkey_only;
 	if (key->legacy || savekeys) {
 		key->force_publish = true;
 		key->force_sign = dst_key_isprivate(key->key);
@@ -1584,7 +1586,7 @@ dns_dnssec_keylistfromrdataset(const dns_name_t *origin, dns_kasp_t *kasp,
 		}
 
 		if (publickey) {
-			addkey(keylist, &dnskey, savekeys, mctx);
+			addkey(keylist, &dnskey, savekeys, true, mctx);
 			goto skip;
 		}
 
@@ -1672,9 +1674,9 @@ dns_dnssec_keylistfromrdataset(const dns_name_t *origin, dns_kasp_t *kasp,
 	addkey:
 		if (result == ISC_R_FILENOTFOUND || result == ISC_R_NOPERM) {
 			if (pubkey != NULL) {
-				addkey(keylist, &pubkey, savekeys, mctx);
+				addkey(keylist, &pubkey, savekeys, true, mctx);
 			} else {
-				addkey(keylist, &dnskey, savekeys, mctx);
+				addkey(keylist, &dnskey, savekeys, false, mctx);
 			}
 			goto skip;
 		}
@@ -1691,7 +1693,7 @@ dns_dnssec_keylistfromrdataset(const dns_name_t *origin, dns_kasp_t *kasp,
 		 */
 		dst_key_setttl(privkey, dst_key_getttl(dnskey));
 
-		addkey(keylist, &privkey, savekeys, mctx);
+		addkey(keylist, &privkey, savekeys, false, mctx);
 	skip:
 		if (dnskey != NULL) {
 			dst_key_free(&dnskey);
diff --git a/lib/dns/include/dns/dnssec.h b/lib/dns/include/dns/dnssec.h
index 09281eb9cc0..daacd69a5c3 100644
--- a/lib/dns/include/dns/dnssec.h
+++ b/lib/dns/include/dns/dnssec.h
@@ -68,6 +68,7 @@ struct dns_dnsseckey {
 				      *  an older version of BIND9) and
 				      *  should be ignored when searching
 				      *  for keys to import into the zone */
+	bool	     pubkey;	     /*% public key only */
 	unsigned int index;	     /*% position in list */
 	ISC_LINK(dns_dnsseckey_t) link;
 };