chg: usr: Harden key management when key files have become unavailabe

Prior to doing key management, BIND 9 will check if the key files on disk match the expected keys. If key files for previously observed keys have become unavailable, this will prevent the internal key manager from running.

Merge branch '4763-do-not-roll-if-key-files-are-missing' into 'main'

See merge request isc-projects/bind9!9337

bin/tests/system/kasp/ns3/named-fips.conf.in

@@ -314,6 +314,15 @@ zone "unfresh-sigs.autosign" {
 	dnssec-policy "autosign";
 };
 
+/*
+ * Zone that has missing key files.
+ */
+zone "keyfiles-missing.autosign" {
+	type primary;
+	file "keyfiles-missing.autosign.db";
+	dnssec-policy "autosign";
+};
+
 /*
  * Zone that has missing private KSK.
  */

bin/tests/system/kasp/ns3/setup.sh

@@ -129,15 +129,19 @@ $KEYGEN -G -k rsasha256 -l policies/kasp.conf $zone >keygen.out.$zone.2 2>&1
 
 zone="multisigner-model2.kasp"
 echo_i "setting up zone: $zone"
+KSK=$($KEYGEN -a $DEFAULT_ALGORITHM -f KSK -L 3600 -M 32768:65535 $zone 2>keygen.out.$zone.1)
+ZSK=$($KEYGEN -a $DEFAULT_ALGORITHM -L 3600 $zone -M 32768:65535 2>keygen.out.$zone.2)
+cat "${KSK}.key" | grep -v ";.*" >>"${zone}.db"
+cat "${ZSK}.key" | grep -v ";.*" >>"${zone}.db"
 # Import the ZSK sets of the other providers into their DNSKEY RRset.
-ZSK1=$($KEYGEN -K ../ -a $DEFAULT_ALGORITHM -L 3600 -M 0:32767 $zone 2>keygen.out.$zone.1)
+# ZSK1 is from a different provider and is added to the unsigned zonefile.
-ZSK2=$($KEYGEN -K ../ -a $DEFAULT_ALGORITHM -L 3600 -M 0:32767 $zone 2>keygen.out.$zone.2)
+# ZSK2 is also from a different provider and is added with a Dynamic Update.
-# ZSK1 will be added to the unsigned zonefile.
+ZSK1=$($KEYGEN -K ../ -a $DEFAULT_ALGORITHM -L 3600 -M 0:32767 $zone 2>keygen.out.$zone.3)
+ZSK2=$($KEYGEN -K ../ -a $DEFAULT_ALGORITHM -L 3600 -M 0:32767 $zone 2>keygen.out.$zone.4)
 cat "../${ZSK1}.key" | grep -v ";.*" >>"${zone}.db"
 cat "../${ZSK1}.key" | grep -v ";.*" >"${zone}.zsk1"
-rm -f "../${ZSK1}.*"
-# ZSK2 will be used with a Dynamic Update.
 cat "../${ZSK2}.key" | grep -v ";.*" >"${zone}.zsk2"
+rm -f "../${ZSK1}.*"
 rm -f "../${ZSK2}.*"
 
 zone="rumoured.kasp"
@@ -178,11 +182,12 @@ $SIGNER -PS -x -o $zone -O raw -f "${zonefile}.signed" $infile >signer.out.$zone
 setup dynamic-signed-inline-signing.kasp
 T="now-1d"
 csktimes="-P $T -A $T -P sync $T"
-CSK=$($KEYGEN -a $DEFAULT_ALGORITHM -L 3600 -f KSK $csktimes $zone 2>keygen.out.$zone.1)
+CSK=$($KEYGEN -K keys -a $DEFAULT_ALGORITHM -L 3600 -f KSK $csktimes $zone 2>keygen.out.$zone.1)
-$SETTIME -s -g $O -d $O $T -k $O $T -z $O $T -r $O $T "$CSK" >settime.out.$zone.1 2>&1
+$SETTIME -s -g $O -d $O $T -k $O $T -z $O $T -r $O $T "keys/$CSK" >settime.out.$zone.1 2>&1
-cat template.db.in "${CSK}.key" >"$infile"
+cat template.db.in "keys/${CSK}.key" >"$infile"
+private_type_record $zone $DEFAULT_ALGORITHM_NUMBER "keys/$CSK" >>"$infile"
 cp $infile $zonefile
-$SIGNER -PS -z -x -s now-2w -e now-1mi -o $zone -f "${zonefile}.signed" $infile >signer.out.$zone.1 2>&1
+$SIGNER -PS -K keys -z -x -s now-2w -e now-1mi -o $zone -f "${zonefile}.signed" $infile >signer.out.$zone.1 2>&1
 
 # We are changing an existing single-signed zone to multi-signed
 # zone where the key tags do not match the dnssec-policy key tag range
@@ -286,6 +291,22 @@ echo "ZSK: yes" >>"${ZSK}".state
 echo "Lifetime: 31536000" >>"${ZSK}".state # PT1Y
 rm -f "${ZSK}".private
 
+# These signatures are still good, but the key files will be removed
+# before a second run of reconfiguring keys.
+setup keyfiles-missing.autosign
+T="now-6mo"
+ksktimes="-P $T -A $T -P sync $T"
+zsktimes="-P $T -A $T"
+KSK=$($KEYGEN -a $DEFAULT_ALGORITHM -L 300 -f KSK $ksktimes $zone 2>keygen.out.$zone.1)
+ZSK=$($KEYGEN -a $DEFAULT_ALGORITHM -L 300 $zsktimes $zone 2>keygen.out.$zone.2)
+$SETTIME -s -g $O -d $O $T -k $O $T -r $O $T "$KSK" >settime.out.$zone.1 2>&1
+$SETTIME -s -g $O -k $O $T -z $O $T "$ZSK" >settime.out.$zone.2 2>&1
+cat template.db.in "${KSK}.key" "${ZSK}.key" >"$infile"
+private_type_record $zone $DEFAULT_ALGORITHM_NUMBER "$KSK" >>"$infile"
+private_type_record $zone $DEFAULT_ALGORITHM_NUMBER "$ZSK" >>"$infile"
+cp $infile $zonefile
+$SIGNER -S -x -s now-1w -e now+1w -o $zone -O raw -f "${zonefile}.signed" $infile >signer.out.$zone.1 2>&1
+
 # These signatures are already expired, and the private ZSK is retired.
 setup zsk-retired.autosign
 T="now-6mo"
@@ -301,6 +322,12 @@ private_type_record $zone $DEFAULT_ALGORITHM_NUMBER "$ZSK" >>"$infile"
 cp $infile $zonefile
 $SIGNER -PS -x -s now-2w -e now-1mi -o $zone -O raw -f "${zonefile}.signed" $infile >signer.out.$zone.1 2>&1
 $SETTIME -s -g HIDDEN "$ZSK" >settime.out.$zone.3 2>&1
+# An old key that is being purged should not prevent keymgr to be run.
+T1="now-1y"
+T2="now-2y"
+oldtimes="-P $T2 -A $T2 -I $T1 -D $T1"
+OLD=$($KEYGEN -a $DEFAULT_ALGORITHM -L 300 $oldtimes $zone 2>keygen.out.$zone.3)
+$SETTIME -s -g $H -k $H $T1 -z $H $T1 "$OLD" >settime.out.$zone.3 2>&1
 
 #
 # The zones at enable-dnssec.autosign represent the various steps of the

bin/tests/system/kasp/tests.sh

@@ -318,7 +318,7 @@ state_stat=$(key_get KEY1 STATE_STAT)
 
 nextpart $DIR/named.run >/dev/null
 rndccmd 10.53.0.3 loadkeys "$ZONE" >/dev/null || log_error "rndc loadkeys zone ${ZONE} failed"
-wait_for_log 3 "keymgr: $ZONE done" $DIR/named.run
+wait_for_log 3 "keymgr: $ZONE done" $DIR/named.run || ret=1
 privkey_stat2=$(key_stat "${basefile}.private")
 pubkey_stat2=$(key_stat "${basefile}.key")
 state_stat2=$(key_stat "${basefile}.state")
@@ -334,7 +334,7 @@ ret=0
 
 nextpart $DIR/named.run >/dev/null
 rndccmd 10.53.0.3 loadkeys "$ZONE" >/dev/null || log_error "rndc loadkeys zone ${ZONE} failed"
-wait_for_log 3 "keymgr: $ZONE done" $DIR/named.run
+wait_for_log 3 "keymgr: $ZONE done" $DIR/named.run || ret=1
 privkey_stat2=$(key_stat "${basefile}.private")
 pubkey_stat2=$(key_stat "${basefile}.key")
 state_stat2=$(key_stat "${basefile}.state")
@@ -385,7 +385,7 @@ echo_i "test that if private key files are inaccessible this doesn't trigger a r
 basefile=$(key_get KEY1 BASEFILE)
 mv "${basefile}.private" "${basefile}.offline"
 rndccmd 10.53.0.3 loadkeys "$ZONE" >/dev/null || log_error "rndc loadkeys zone ${ZONE} failed"
-wait_for_log 3 "offline, policy default" $DIR/named.run || ret=1
+wait_for_log 3 "zone $ZONE/IN (signed): zone_rekey:zone_verifykeys failed: some key files are missing" $DIR/named.run || ret=1
 mv "${basefile}.offline" "${basefile}.private"
 test "$ret" -eq 0 || echo_i "failed"
 status=$((status + ret))
@@ -1647,6 +1647,15 @@ check_subdomain
 dnssec_verify
 check_rrsig_refresh
 
+# Load again, make sure the purged key is not an issue when verifying keys.
+echo_i "load keys for $ZONE, making sure a recently purged key is not an issue when verifying keys ($n)"
+ret=0
+rndccmd 10.53.0.3 loadkeys "$ZONE" >/dev/null || log_error "rndc loadkeys zone ${ZONE} failed"
+wait_for_log 3 "keymgr: $ZONE done" $DIR/named.run || ret=1
+grep "zone $ZONE/IN (signed): zone_rekey:zone_verifykeys failed: some key files are missing" $DIR/named.run && ret=1
+test "$ret" -eq 0 || echo_i "failed"
+status=$((status + ret))
+
 #
 # Zone: legacy-keys.kasp.
 #
@@ -1743,6 +1752,68 @@ check_apex
 check_subdomain
 dnssec_verify
 
+#
+# Zone: keyfiles-missing.autosign.
+#
+set_zone "keyfiles-missing.autosign"
+set_policy "autosign" "2" "300"
+set_server "ns3" "10.53.0.3"
+# Key properties.
+key_clear "KEY1"
+set_keyrole "KEY1" "ksk"
+set_keylifetime "KEY1" "63072000"
+set_keyalgorithm "KEY1" "$DEFAULT_ALGORITHM_NUMBER" "$DEFAULT_ALGORITHM" "$DEFAULT_BITS"
+set_keysigning "KEY1" "yes"
+set_zonesigning "KEY1" "no"
+
+key_clear "KEY2"
+set_keyrole "KEY2" "zsk"
+set_keylifetime "KEY2" "31536000"
+set_keyalgorithm "KEY2" "$DEFAULT_ALGORITHM_NUMBER" "$DEFAULT_ALGORITHM" "$DEFAULT_BITS"
+set_keysigning "KEY2" "no"
+set_zonesigning "KEY2" "yes"
+
+# Both KSK and ZSK stay OMNIPRESENT.
+set_keystate "KEY1" "GOAL" "omnipresent"
+set_keystate "KEY1" "STATE_DNSKEY" "omnipresent"
+set_keystate "KEY1" "STATE_KRRSIG" "omnipresent"
+set_keystate "KEY1" "STATE_DS" "omnipresent"
+
+set_keystate "KEY2" "GOAL" "omnipresent"
+set_keystate "KEY2" "STATE_DNSKEY" "omnipresent"
+set_keystate "KEY2" "STATE_ZRRSIG" "omnipresent"
+
+check_keys
+check_dnssecstatus "$SERVER" "$POLICY" "$ZONE"
+set_keytimes_autosign_policy
+check_keytimes
+check_apex
+check_subdomain
+dnssec_verify
+# All good, now remove key files and reload keys.
+rm_keyfiles() {
+  _basefile=$(key_get "$1" BASEFILE)
+  echo_i "remove key files $_basefile"
+  _keyfile="${_basefile}.key"
+  _privatefile="${_basefile}.private"
+  _statefile="${_basefile}.state"
+  rm -f $_keyfile
+  rm -f $_privatefile
+  rm -f $_statefile
+}
+rm_keyfiles "KEY1"
+rm_keyfiles "KEY2"
+
+rndccmd 10.53.0.3 loadkeys "$ZONE" >/dev/null || log_error "rndc loadkeys zone ${ZONE} failed"
+wait_for_log 3 "zone $ZONE/IN (signed): zone_rekey:zone_verifykeys failed: some key files are missing" $DIR/named.run || ret=1
+# Check keys again, make sure no new keys are created.
+set_policy "autosign" "0" "300"
+key_clear "KEY1"
+key_clear "KEY2"
+check_keys
+# Zone is still signed correctly.
+dnssec_verify
+
 #
 # Test dnssec-policy inheritance.
 #
@@ -2130,16 +2201,23 @@ check_apex
 check_subdomain
 dnssec_verify
 
-# Check that the ZSKs from the other provider are published.
+# Check that the ZSKs from the other providers are published.
 zsks_are_published() {
+  num=$1
   dig_with_opts +short "$ZONE" "@${SERVER}" DNSKEY >"dig.out.$DIR.test$n" || return 1
   # We should have three ZSKs.
   lines=$(grep "256 3 13" dig.out.$DIR.test$n | wc -l)
-  test "$lines" -eq 3 || return 1
+  test "$lines" -eq $num || return 1
   # And one KSK.
   lines=$(grep "257 3 13" dig.out.$DIR.test$n | wc -l)
   test "$lines" -eq 1 || return 1
 }
+n=$((n + 1))
+echo_i "check initial number of ZSKs (one from us and one from another provider) for zone ${ZONE} ($n)"
+ret=0
+retry_quiet 10 zsks_are_published 2 || ret=1
+test "$ret" -eq 0 || echo_i "failed"
+status=$((status + ret))
 
 n=$((n + 1))
 echo_i "update zone with ZSK from another provider for zone ${ZONE} ($n)"
@@ -2150,7 +2228,21 @@ ret=0
   echo update add $(cat "${DIR}/${ZONE}.zsk2")
   echo send
 ) | $NSUPDATE
-retry_quiet 10 zsks_are_published || ret=1
+retry_quiet 10 zsks_are_published 3 || ret=1
+test "$ret" -eq 0 || echo_i "failed"
+status=$((status + ret))
+
+n=$((n + 1))
+echo_i "remove ZSKs from the other providers for zone ${ZONE} ($n)"
+ret=0
+(
+  echo zone ${ZONE}
+  echo server 10.53.0.3 "$PORT"
+  echo update del $(cat "${DIR}/${ZONE}.zsk1")
+  echo update del $(cat "${DIR}/${ZONE}.zsk2")
+  echo send
+) | $NSUPDATE
+retry_quiet 10 zsks_are_published 1 || ret=1
 test "$ret" -eq 0 || echo_i "failed"
 status=$((status + ret))
 
@@ -5199,7 +5291,7 @@ dig_with_opts @10.53.0.6 example SOA >dig.out.ns6.test$n.soa1 || ret=1
 cp ns6/example2.db.in ns6/example.db || ret=1
 nextpart ns6/named.run >/dev/null
 rndccmd 10.53.0.6 reload || ret=1
-wait_for_log 3 "all zones loaded" ns6/named.run
+wait_for_log 3 "all zones loaded" ns6/named.run || ret=1
 # Check that the SOA SERIAL increases and check the TTLs (should be 300 as
 # defined in ns6/example2.db.in).
 retry_quiet 10 _check_soa_ttl 300 300 || ret=1
@@ -5217,7 +5309,7 @@ cp ns6/example3.db.in ns6/example.db || ret=1
 rm ns6/example.db.jnl
 nextpart ns6/named.run >/dev/null
 start_server --noclean --restart --port ${PORT} ns6
-wait_for_log 3 "all zones loaded" ns6/named.run
+wait_for_log 3 "all zones loaded" ns6/named.run || ret=1
 # Check that the SOA SERIAL increases and check the TTLs (should be changed
 # from 300 to 400 as defined in ns6/example3.db.in).
 retry_quiet 10 _check_soa_ttl 300 400 || ret=1

doc/arm/reference.rst

@@ -6213,6 +6213,14 @@ zone is generated even if they have the same policy.  If multiple views
 are configured with different versions of the same zone, each separate
 version uses the same set of signing keys.
 
+If the expected key files that were previously observed have gone missing or
+are inaccessible, key management is halted. This will prevent rollovers
+from being started if there is a temporary file access issue. If his problem
+is permanent it will eventually lead to expired signatures in your zone.
+Note that if the key files are missing or inaccessible during :iscman:`named`
+startup, BIND 9 will try to generate new keys according to the DNSSEC policy,
+because it has no cached information about existing keys yet.
+
 The :any:`dnssec-policy` statement requires dynamic DNS to be set up, or
 :any:`inline-signing` to be enabled (which is the default for DNSSEC zones).
 

lib/dns/keymgr.c

@@ -1465,6 +1465,11 @@ transition:
 		char keystr[DST_KEY_FORMATSIZE];
 		dst_key_format(dkey->key, keystr, sizeof(keystr));
 
+		if (dkey->purge) {
+			/* Skip purged keys. */
+			continue;
+		}
+
 		/* For all records related to this key. */
 		for (int i = 0; i < NUM_KEYSTATES; i++) {
 			isc_result_t ret;

lib/dns/zone.c

@@ -371,6 +371,7 @@ struct dns_zone {
 	dns_view_t *prev_view;
 	dns_kasp_t *kasp;
 	dns_kasp_t *defaultkasp;
+	dns_dnsseckeylist_t keyring;
 	dns_checkmxfunc_t checkmx;
 	dns_checksrvfunc_t checksrv;
 	dns_checknsfunc_t checkns;
@@ -1177,6 +1178,7 @@ dns_zone_create(dns_zone_t **zonep, isc_mem_t *mctx, unsigned int tid) {
 	zone->parentals = r;
 	zone->notify = r;
 	zone->defaultkasp = NULL;
+	ISC_LIST_INIT(zone->keyring);
 
 	isc_stats_create(mctx, &zone->gluecachestats,
 			 dns_gluecachestatscounter_max);
@@ -1279,6 +1281,9 @@ zone_free(dns_zone_t *zone) {
 	if (zone->defaultkasp != NULL) {
 		dns_kasp_detach(&zone->defaultkasp);
 	}
+	if (!ISC_LIST_EMPTY(zone->keyring)) {
+		clear_keylist(&zone->keyring, zone->mctx);
+	}
 	if (!ISC_LIST_EMPTY(zone->checkds_ok)) {
 		clear_keylist(&zone->checkds_ok, zone->mctx);
 	}
@@ -21914,6 +21919,47 @@ update_ttl(dns_rdataset_t *rdataset, dns_name_t *name, dns_ttl_t ttl,
 	return (ISC_R_SUCCESS);
 }
 
+static isc_result_t
+zone_verifykeys(dns_zone_t *zone, dns_dnsseckeylist_t *newkeys) {
+	dns_dnsseckey_t *key1, *key2, *next;
+
+	/*
+	 * Make sure that the existing keys are also present in the new keylist.
+	 */
+	for (key1 = ISC_LIST_HEAD(zone->keyring); key1 != NULL; key1 = next) {
+		bool found = false;
+		next = ISC_LIST_NEXT(key1, link);
+
+		if (dst_key_is_unused(key1->key)) {
+			continue;
+		}
+		if (key1->purge) {
+			continue;
+		}
+
+		for (key2 = ISC_LIST_HEAD(*newkeys); key2 != NULL;
+		     key2 = ISC_LIST_NEXT(key2, link))
+		{
+			if (dst_key_compare(key1->key, key2->key)) {
+				found = true;
+				break;
+			}
+		}
+
+		if (!found) {
+			char keystr[DST_KEY_FORMATSIZE];
+			dst_key_format(key1->key, keystr, sizeof(keystr));
+			dnssec_log(zone, ISC_LOG_DEBUG(1),
+				   "verifykeys: key %s - not available",
+				   keystr);
+			return (ISC_R_NOTFOUND);
+		}
+	}
+
+	/* All good. */
+	return (ISC_R_SUCCESS);
+}
+
 static void
 remove_rdataset(dns_zone_t *zone, dns_diff_t *diff, dns_rdataset_t *rdataset) {
 	if (!dns_rdataset_isassociated(rdataset)) {
@@ -22202,6 +22248,16 @@ zone_rekey(dns_zone_t *zone) {
 	}
 
 	if (kasp != NULL && !offlineksk) {
+		/* Verify new keys. */
+		isc_result_t ret = zone_verifykeys(zone, &keys);
+		if (ret != ISC_R_SUCCESS) {
+			dnssec_log(zone, ISC_LOG_ERROR,
+				   "zone_rekey:zone_verifykeys failed: "
+				   "some key files are missing");
+			KASP_UNLOCK(kasp);
+			goto failure;
+		}
+
 		/*
 		 * Check DS at parental agents. Clear ongoing checks.
 		 */
@@ -22211,8 +22267,8 @@ zone_rekey(dns_zone_t *zone) {
 		ISC_LIST_INIT(zone->checkds_ok);
 		UNLOCK_ZONE(zone);
 
-		isc_result_t ret = dns_zone_getdnsseckeys(zone, db, ver, now,
+		ret = dns_zone_getdnsseckeys(zone, db, ver, now,
-							  &zone->checkds_ok);
+					     &zone->checkds_ok);
 		if (ret == ISC_R_SUCCESS) {
 			zone_checkds(zone);
 		} else {
@@ -22224,7 +22280,7 @@ zone_rekey(dns_zone_t *zone) {
 				   isc_result_totext(ret));
 		}
 
-		/* Run keymgr */
+		/* Run keymgr. */
 		if (result == ISC_R_SUCCESS || result == ISC_R_NOTFOUND) {
 			dns_zone_lock_keyfiles(zone);
 			result = dns_keymgr_run(&zone->origin, zone->rdclass,
@@ -22695,10 +22751,14 @@ zone_rekey(dns_zone_t *zone) {
 	}
 	UNLOCK_ZONE(zone);
 
-	if (isc_log_wouldlog(ISC_LOG_DEBUG(3))) {
+	/*
-		for (key = ISC_LIST_HEAD(dnskeys); key != NULL;
+	 * Remember which keys have been used.
-		     key = ISC_LIST_NEXT(key, link))
+	 */
-		{
+	if (!ISC_LIST_EMPTY(zone->keyring)) {
+		clear_keylist(&zone->keyring, zone->mctx);
+	}
+	while ((key = ISC_LIST_HEAD(dnskeys)) != NULL) {
+		if (isc_log_wouldlog(ISC_LOG_DEBUG(3))) {
 			/* This debug log is used in the kasp system test */
 			char algbuf[DNS_SECALG_FORMATSIZE];
 			dns_secalg_format(dst_key_alg(key->key), algbuf,
@@ -22707,6 +22767,8 @@ zone_rekey(dns_zone_t *zone) {
 				   "zone_rekey done: key %d/%s",
 				   dst_key_id(key->key), algbuf);
 		}
+		ISC_LIST_UNLINK(dnskeys, key, link);
+		ISC_LIST_APPEND(zone->keyring, key, link);
 	}
 
 	result = ISC_R_SUCCESS;