fix: usr: Fix a bug in dnssec-signzone related to keys being offline

In the case when `dnssec-signzone` is called on an already signed zone, and the private key file is unavailable, a signature that needs to be refreshed may be dropped without being able to generate a replacement. This has been fixed.

Closes #5126

Merge branch '5126-dnssec-signzone-retain-rrsig-if-key-is-offline' into 'main'

See merge request isc-projects/bind9!9951

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) {

bin/tests/system/dnssec/tests.sh

@@ -1358,6 +1358,33 @@ n=$((n + 1))
 test "$ret" -eq 0 || echo_i "failed"
 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
+) || ret=1
+n=$((n + 1))
+test "$ret" -eq 0 || echo_i "failed"
+status=$((status + ret))
+
+echo_ic "two DNSKEYs, DNSKEY RRset only by KSK, private key missing ($n)"
+ret=0
+(
+  cd signer/general || exit 1
+  cp signed.zone signed.expect
+  grep "example\.com\..*3600.*IN.*RRSIG.*DNSKEY.*10.*2.*3600.*28633.*example\.com\." signed.expect >dnskey.expect || exit 1
+  mv Kexample.com.+010+28633.private Kexample.com.+010+28633.offline
+  $SIGNER -P -x -f signed.zone -O full -o example.com. signed.zone >signer.out.$n
+  mv Kexample.com.+010+28633.offline Kexample.com.+010+28633.private
+  grep "$(cat dnskey.expect)" signed.zone >/dev/null || exit 1
+) || ret=1
+n=$((n + 1))
+test "$ret" -eq 0 || echo_i "failed"
+status=$((status + ret))
+
 echo_ic "one non-KSK DNSKEY ($n)"
 ret=0
 (

bin/tests/system/dnssec/tests_sh_dnssec.py

@@ -146,7 +146,9 @@ pytestmark = pytest.mark.extra_artifacts(
         "signer/example.db.changed",
         "signer/example2.db",
         "signer/example3.db",
+        "signer/general/dnskey.expect",
         "signer/general/dsset-*",
+        "signer/general/signed.expect",
         "signer/general/signed.zone",
         "signer/general/signer.out.*",
         "signer/nsec3param.out",

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);

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;
 };