Merge branch '1551-dnssec-signzone-prepublish-zsk-support' into 'main'

dnssec-signzone ZSK smooth rollover

Closes #1551

See merge request isc-projects/bind9!5285

CHANGES

@@ -1,3 +1,10 @@
+5690.	[func]		Change "dnssec-signzone" to honor the Predecessor and
+			Successor metadata values, and allow for gradual
+			replacement of RRSIGs. In other words, don't sign
+			with the successor key if there is an RRSIG from the
+			predecessor key that does not need to be refreshed.
+			[GL #1551]
+
 5689.	[placeholder]
 
 5688.	[bug]		Inline and dnssec-policy zones could fail to apply

bin/dnssec/dnssec-signzone.c

@@ -555,11 +555,7 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name,
 
 		key = keythatsigned(&rrsig);
 		sig_format(&rrsig, sigstr, sizeof(sigstr));
-		if (key != NULL && issigningkey(key)) {
-			expired = isc_serial_gt(now + cycle, rrsig.timeexpire);
-		} else {
-			expired = isc_serial_gt(now, rrsig.timeexpire);
-		}
+		expired = isc_serial_gt(now + cycle, rrsig.timeexpire);
 
 		if (isc_serial_gt(rrsig.timesigned, rrsig.timeexpire)) {
 			/* rrsig is dropped and not replaced */
@@ -649,7 +645,7 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name,
 			}
 		} else {
 			tuple = NULL;
-			vbprintf(2, "removing signature by %s\n", sigstr);
+			vbprintf(2, "\tremoving signature by %s\n", sigstr);
 			result = dns_difftuple_create(
 				mctx, DNS_DIFFOP_DELRESIGN, name, sigset.ttl,
 				&sigrdata, &tuple);
@@ -695,20 +691,20 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name,
 		    dns_name_equal(name, gorigin))
 		{
 			bool have_ksk;
-			dns_dnsseckey_t *tmpkey;
+			dns_dnsseckey_t *curr;
 
 			have_ksk = isksk(key);
-			for (tmpkey = ISC_LIST_HEAD(keylist); tmpkey != NULL;
-			     tmpkey = ISC_LIST_NEXT(tmpkey, link))
+			for (curr = ISC_LIST_HEAD(keylist); curr != NULL;
+			     curr = ISC_LIST_NEXT(curr, link))
 			{
 				if (dst_key_alg(key->key) !=
-				    dst_key_alg(tmpkey->key)) {
+				    dst_key_alg(curr->key)) {
 					continue;
 				}
-				if (REVOKE(tmpkey->key)) {
+				if (REVOKE(curr->key)) {
 					continue;
 				}
-				if (isksk(tmpkey)) {
+				if (isksk(curr)) {
 					have_ksk = true;
 				}
 			}
@@ -718,8 +714,65 @@ signset(dns_diff_t *del, dns_diff_t *add, dns_dbnode_t *node, dns_name_t *name,
 					    "signing with dnskey");
 			}
 		} else if (iszsk(key)) {
-			signwithkey(name, set, key->key, ttl, add,
-				    "signing with dnskey");
+			/*
+			 * Sign with the ZSK unless there is a predecessor
+			 * key that already signs this RRset.
+			 */
+			bool have_pre_sig = false;
+			dns_dnsseckey_t *curr;
+			uint32_t pre;
+			isc_result_t ret = dst_key_getnum(
+				key->key, DST_NUM_PREDECESSOR, &pre);
+			if (ret == ISC_R_SUCCESS) {
+				/*
+				 * This key has a predecessor, look for the
+				 * corresponding key in the keylist. The
+				 * key we are looking for must be:
+				 * - From the same cryptographic algorithm.
+				 * - Have the ZSK type (iszsk).
+				 * - Have key ID equal to the predecessor id.
+				 * - Have a successor that matches 'key' id.
+				 */
+				for (curr = ISC_LIST_HEAD(keylist);
+				     curr != NULL;
+				     curr = ISC_LIST_NEXT(curr, link))
+				{
+					uint32_t suc;
+
+					if (dst_key_alg(key->key) !=
+						    dst_key_alg(curr->key) ||
+					    !iszsk(curr) ||
+					    dst_key_id(curr->key) != pre)
+					{
+						continue;
+					}
+					ret = dst_key_getnum(curr->key,
+							     DST_NUM_SUCCESSOR,
+							     &suc);
+					if (ret != ISC_R_SUCCESS ||
+					    dst_key_id(key->key) != suc) {
+						continue;
+					}
+
+					/*
+					 * curr is the predecessor we were
+					 * looking for. Check if this key
+					 * signs this RRset.
+					 */
+					if (nowsignedby[curr->index]) {
+						have_pre_sig = true;
+					}
+				}
+			}
+
+			/*
+			 * If we have a signature of a predecessor key,
+			 * skip signing with this key.
+			 */
+			if (!have_pre_sig) {
+				signwithkey(name, set, key->key, ttl, add,
+					    "signing with dnskey");
+			}
 		}
 	}
 
@@ -3012,7 +3065,7 @@ writeset(const char *prefix, dns_rdatatype_t type) {
 	isc_buffer_t namebuf;
 	isc_region_t r;
 	isc_result_t result;
-	dns_dnsseckey_t *key, *tmpkey;
+	dns_dnsseckey_t *key, *curr;
 	unsigned char dsbuf[DNS_DS_BUFFERSIZE];
 	unsigned char keybuf[DST_KEY_MAXSIZE];
 	unsigned int filenamelen;
@@ -3053,16 +3106,16 @@ writeset(const char *prefix, dns_rdatatype_t type) {
 			have_ksk = false;
 			have_non_ksk = true;
 		}
-		for (tmpkey = ISC_LIST_HEAD(keylist); tmpkey != NULL;
-		     tmpkey = ISC_LIST_NEXT(tmpkey, link))
+		for (curr = ISC_LIST_HEAD(keylist); curr != NULL;
+		     curr = ISC_LIST_NEXT(curr, link))
 		{
-			if (dst_key_alg(key->key) != dst_key_alg(tmpkey->key)) {
+			if (dst_key_alg(key->key) != dst_key_alg(curr->key)) {
 				continue;
 			}
-			if (REVOKE(tmpkey->key)) {
+			if (REVOKE(curr->key)) {
 				continue;
 			}
-			if (isksk(tmpkey)) {
+			if (isksk(curr)) {
 				have_ksk = true;
 			} else {
 				have_non_ksk = true;

bin/tests/system/dnssec/signer/prepub.db.in

@@ -0,0 +1,15 @@
+; Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+;
+; This Source Code Form is subject to the terms of the Mozilla Public
+; License, v. 2.0. If a copy of the MPL was not distributed with this
+; file, You can obtain one at http://mozilla.org/MPL/2.0/.
+;
+; See the COPYRIGHT file distributed with this work for additional
+; information regarding copyright ownership.
+
+$TTL 60
+prepub. 60 IN SOA prepub. . 0 0 0 0 0
+prepub. 60 IN NS prepub.
+prepub. 60 IN A 1.2.3.4
+; out of zone record
+out-of-zone. 60 IN A 1.2.3.4

bin/tests/system/dnssec/tests.sh

@@ -1417,6 +1417,93 @@ n=$((n+1))
 test "$ret" -eq 0 || echo_i "failed"
 status=$((status+ret))
 
+get_rsasha1_key_ids_from_sigs() {
+	zone=$1
+
+	tr -d '\r' < signer/$zone.db.signed | \
+	awk '
+		NF < 8 { next }
+		$(NF-5) != "RRSIG" { next }
+		$(NF-3) != "5" { next }
+		$NF != "(" { next }
+		{
+			getline;
+			print $3;
+		}
+	' | \
+	sort -u
+}
+
+# Test dnssec-signzone ZSK prepublish smooth rollover.
+echo_i "check dnssec-signzone doesn't sign with prepublished zsk ($n)"
+ret=0
+zone=prepub
+# Generate keys.
+ksk=$("$KEYGEN" -K signer -f KSK -q -a RSASHA1 -b 1024 -n zone "$zone")
+zsk1=$("$KEYGEN" -K signer -q -a RSASHA1 -b 1024 -n zone "$zone")
+zsk2=$("$KEYGEN" -K signer -q -a RSASHA1 -b 1024 -n zone "$zone")
+zskid1=$(keyfile_to_key_id "$zsk1")
+zskid2=$(keyfile_to_key_id "$zsk2")
+(
+cd signer || exit 1
+# Set times such that the current set of keys are introduced 60 days ago and
+# start signing now. The successor key is prepublished now and will be active
+# next day.
+$SETTIME -P now-60d -A now $ksk > /dev/null
+$SETTIME -P now-60d -A now -I now+1d -D now+60d $zsk1 > /dev/null
+$SETTIME -S $zsk1 -i 1h $zsk2.key > /dev/null
+$SETTIME -P now -A now+1d $zsk2.key > /dev/null
+# Sign the zone with initial keys and prepublish successor. The zone signatures
+# are valid for 30 days and the DNSKEY signature is valid for 60 days.
+cp -f $zone.db.in $zone.db
+$SIGNER -SDx -e +2592000 -X +5184000 -o $zone $zone.db > /dev/null
+echo "\$INCLUDE \"$zone.db.signed\"" >> $zone.db
+)
+get_rsasha1_key_ids_from_sigs $zone | grep "^$zskid1$" > /dev/null || ret=1
+get_rsasha1_key_ids_from_sigs $zone | grep "^$zskid2$" > /dev/null && ret=1
+n=$((n+1))
+test "$ret" -eq 0 || echo_i "failed: missing signatures from key $zskid1"
+status=$((status+ret))
+
+echo_i "check dnssec-signzone retains signatures of predecessor zsk ($n)"
+ret=0
+zone=prepub
+(
+cd signer || exit 1
+# Roll the ZSK. The predecessor is inactive from now on and the successor is
+# activated. The zone signatures are valid for 30 days and the DNSKEY
+# signature is valid for 60 days. Because of the predecessor/successor
+# relationship, the signatures of the predecessor are retained and no new
+# signatures with the successor should be generated.
+$SETTIME -A now-30d -I now -D now+30d $zsk1 > /dev/null
+$SETTIME -A now $zsk2 > /dev/null
+$SIGNER -SDx -e +2592000 -X +5184000 -o $zone $zone.db > /dev/null
+)
+get_rsasha1_key_ids_from_sigs $zone | grep "^$zskid1$" > /dev/null || ret=1
+get_rsasha1_key_ids_from_sigs $zone | grep "^$zskid2$" > /dev/null && ret=1
+n=$((n+1))
+test "$ret" -eq 0 || echo_i "failed"
+status=$((status+ret))
+
+echo_i "check dnssec-signzone swaps zone signatures after interval ($n)"
+ret=0
+zone=prepub
+(
+cd signer || exit 1
+# After some time the signatures should be replaced. When signing, set the
+# interval to 30 days plus one second, meaning all predecessor signatures
+# are within the refresh interval and should be replaced with successor
+# signatures.
+$SETTIME -A now-50d -I now-20d -D now+10d $zsk1 > /dev/null
+$SETTIME -A now-20d $zsk2 > /dev/null
+$SIGNER -SDx -e +2592000 -X +5184000 -i 2592001 -o $zone $zone.db > /dev/null
+)
+get_rsasha1_key_ids_from_sigs $zone | grep "^$zskid1$" > /dev/null && ret=1
+get_rsasha1_key_ids_from_sigs $zone | grep "^$zskid2$" > /dev/null || ret=1
+n=$((n+1))
+test "$ret" -eq 0 || echo_i "failed"
+status=$((status+ret))
+
 echo_i "checking that a key using an unsupported algorithm cannot be generated ($n)"
 ret=0
 zone=example
@@ -1458,21 +1545,6 @@ n=$((n+1))
 test "$ret" -eq 0 || echo_i "failed"
 status=$((status+ret))
 
-get_rsasha1_key_ids_from_sigs() {
-	tr -d '\r' < signer/example.db.signed | \
-	awk '
-		NF < 8 { next }
-		$(NF-5) != "RRSIG" { next }
-		$(NF-3) != "5" { next }
-		$NF != "(" { next }
-		{
-			getline;
-			print $3;
-		}
-	' | \
-	sort -u
-}
-
 echo_i "checking that we can sign a zone with out-of-zone records ($n)"
 ret=0
 zone=example
@@ -1573,8 +1645,8 @@ cat example.db.in "$key1.key" "$key3.key" > example.db
 echo "\$INCLUDE \"example.db.signed\"" >> example.db
 $SIGNER -D -o example example.db > /dev/null
 ) || ret=1
-get_rsasha1_key_ids_from_sigs | grep "^$keyid2$" > /dev/null || ret=1
-get_rsasha1_key_ids_from_sigs | grep "^$keyid3$" > /dev/null || ret=1
+get_rsasha1_key_ids_from_sigs $zone | grep "^$keyid2$" > /dev/null || ret=1
+get_rsasha1_key_ids_from_sigs $zone | grep "^$keyid3$" > /dev/null || ret=1
 n=$((n+1))
 test "$ret" -eq 0 || echo_i "failed"
 status=$((status+ret))
@@ -1585,8 +1657,8 @@ ret=0
 cd signer || exit 1
 $SIGNER -RD -o example example.db > /dev/null
 ) || ret=1
-get_rsasha1_key_ids_from_sigs | grep "^$keyid2$" > /dev/null && ret=1
-get_rsasha1_key_ids_from_sigs | grep "^$keyid3$" > /dev/null || ret=1
+get_rsasha1_key_ids_from_sigs $zone | grep "^$keyid2$" > /dev/null && ret=1
+get_rsasha1_key_ids_from_sigs $zone | grep "^$keyid3$" > /dev/null || ret=1
 n=$((n+1))
 test "$ret" -eq 0 || echo_i "failed"
 status=$((status+ret))
@@ -1603,8 +1675,8 @@ echo "\$INCLUDE \"example.db.signed\"" >> example.db
 $SETTIME -I now "$key2" > /dev/null 2>&1
 $SIGNER -SD -o example example.db > /dev/null
 ) || ret=1
-get_rsasha1_key_ids_from_sigs | grep "^$keyid2$" > /dev/null || ret=1
-get_rsasha1_key_ids_from_sigs | grep "^$keyid3$" > /dev/null || ret=1
+get_rsasha1_key_ids_from_sigs $zone | grep "^$keyid2$" > /dev/null || ret=1
+get_rsasha1_key_ids_from_sigs $zone | grep "^$keyid3$" > /dev/null || ret=1
 n=$((n+1))
 test "$ret" -eq 0 || echo_i "failed"
 status=$((status+ret))
@@ -1615,8 +1687,8 @@ ret=0
 cd signer || exit 1
 $SIGNER -SDQ -o example example.db > /dev/null
 ) || ret=1
-get_rsasha1_key_ids_from_sigs | grep "^$keyid2$" > /dev/null && ret=1
-get_rsasha1_key_ids_from_sigs | grep "^$keyid3$" > /dev/null || ret=1
+get_rsasha1_key_ids_from_sigs $zone | grep "^$keyid2$" > /dev/null && ret=1
+get_rsasha1_key_ids_from_sigs $zone | grep "^$keyid3$" > /dev/null || ret=1
 n=$((n+1))
 test "$ret" -eq 0 || echo_i "failed"
 status=$((status+ret))

doc/notes/notes-current.rst

@@ -66,6 +66,11 @@ Feature Changes
   record.  This allows a clean rollover from one DNS provider to another
   when using a multiple-signer DNSSEC configuration. :gl:`#2710`
 
+- ``dnssec-signzone`` is now able to retain signatures from inactive
+  predecessor keys without introducing additional signatures from the successor
+  key. This allows for a gradual replacement of RRSIGs as they reach expiry.
+  :gl:`#1551`
+
 Bug Fixes
 ~~~~~~~~~
 

lib/dns/dst_api.c

@@ -2708,7 +2708,7 @@ dst_key_copy_metadata(dst_key_t *to, dst_key_t *from) {
 		if (result == ISC_R_SUCCESS) {
 			dst_key_setbool(to, i, yesno);
 		} else {
-			dst_key_unsetnum(to, i);
+			dst_key_unsetbool(to, i);
 		}
 	}