Merge branch '1593-dnssec-policy-new-key-on-restart-bug' into 'master'

Fix kasp bug new KSK on restart [#1593]

Closes #1593

See merge request isc-projects/bind9!3007

CHANGES

@@ -1,3 +1,9 @@
+5354.	[bug]		dnssec-policy created new KSK keys when zone is in
+			initial stage of signing (the DS is not yet in
+			rumoured or omnipresent state).  Fix by checking
+			key goals rather than active state when determining
+			new keys are needed. [GL #1593]
+
 5353.	[doc]		Document port and dscp parameters in forwarders
 			configuration option. [GL !914]
 

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

@@ -107,6 +107,16 @@ zone "pregenerated.kasp" {
 	dnssec-policy "rsasha1";
 };
 
+/*
+ * A configured dnssec-policy with one rumoured key.
+ * Bugfix case for GL #1593.
+ */
+zone "rumoured.kasp" {
+	type master;
+	file "rumoured.kasp.db";
+	dnssec-policy "rsasha1";
+};
+
 /*
  * Different algorithms.
  */

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

@@ -43,7 +43,7 @@ U="UNRETENTIVE"
 # Set up zones that will be initially signed.
 #
 for zn in default rsasha1 dnssec-keygen some-keys legacy-keys pregenerated \
-	  rsasha1-nsec3 rsasha256 rsasha512 ecdsa256 ecdsa384 inherit
+	  rumoured rsasha1-nsec3 rsasha256 rsasha512 ecdsa256 ecdsa384 inherit
 do
 	setup "${zn}.kasp"
 	cp template.db.in "$zonefile"
@@ -72,6 +72,16 @@ zone="pregenerated.kasp"
 $KEYGEN -k rsasha1 -l policies/kasp.conf $zone > keygen.out.$zone.1 2>&1
 $KEYGEN -k rsasha1 -l policies/kasp.conf $zone > keygen.out.$zone.2 2>&1
 
+zone="rumoured.kasp"
+Tpub="now"
+Tact="now+1d"
+KSK=$($KEYGEN  -a RSASHA1 -f KSK  -L 1234 $zone 2> keygen.out.$zone.1)
+ZSK1=$($KEYGEN -a RSASHA1 -b 2000 -L 1234 $zone 2> keygen.out.$zone.2)
+ZSK2=$($KEYGEN -a RSASHA1         -L 1234 $zone 2> keygen.out.$zone.3)
+$SETTIME -s -P $Tpub -A $Tact -g $O -k $R $Tpub -r $R $Tpub -d $H $Tpub  "$KSK"  > settime.out.$zone.1 2>&1
+$SETTIME -s -P $Tpub -A $Tact -g $O -k $R $Tpub -z $R $Tpub              "$ZSK1" > settime.out.$zone.2 2>&1
+$SETTIME -s -P $Tpub -A $Tact -g $O -k $R $Tpub -z $R $Tpub              "$ZSK2" > settime.out.$zone.2 2>&1
+
 #
 # Set up zones that are already signed.
 #

bin/tests/system/kasp/tests.sh

@@ -1056,6 +1056,17 @@ check_apex
 check_subdomain
 dnssec_verify
 
+#
+# Zone: rumoured.kasp.
+#
+# There are three keys in rumoured state.
+zone_properties "ns3" "rumoured.kasp" "rsasha1" "1234" "3" "10.53.0.3"
+# key_properties, key_timings and key_states same as above.
+check_keys
+check_apex
+check_subdomain
+dnssec_verify
+
 #
 # Zone: secondary.kasp.
 #

lib/dns/dst_api.c

@@ -2336,7 +2336,7 @@ dst_key_is_unused(dst_key_t* key)
 		 * This key is used.
 		 */
 		if (!state_type_set) {
-			return false;
+			return (false);
 		}
 		/*
 		 * If the state is not HIDDEN, the key is in use.
@@ -2346,11 +2346,11 @@ dst_key_is_unused(dst_key_t* key)
 			st = DST_KEY_STATE_NA;
 		}
 		if (st != DST_KEY_STATE_HIDDEN) {
-			return false;
+			return (false);
 		}
 	}
 	/* This key is unused. */
-	return true;
+	return (true);
 }
 
 
@@ -2405,7 +2405,7 @@ dst_key_is_published(dst_key_t *key, isc_stdtime_t now,
 		time_ok = true;
 	}
 
-	return state_ok && time_ok;
+	return (state_ok && time_ok);
 }
 
 bool
@@ -2465,10 +2465,9 @@ dst_key_is_active(dst_key_t *key, isc_stdtime_t now)
 			inactive = false;
 		}
 	}
-	return ds_ok && zrrsig_ok && time_ok && !inactive;
+	return (ds_ok && zrrsig_ok && time_ok && !inactive);
 }
 
-
 bool
 dst_key_is_signing(dst_key_t *key, int role, isc_stdtime_t now, isc_stdtime_t *active)
 {
@@ -2522,7 +2521,7 @@ dst_key_is_signing(dst_key_t *key, int role, isc_stdtime_t now, isc_stdtime_t *a
 			inactive = false;
 		}
 	}
-	return krrsig_ok && zrrsig_ok && time_ok && !inactive;
+	return (krrsig_ok && zrrsig_ok && time_ok && !inactive);
 }
 
 bool
@@ -2540,7 +2539,7 @@ dst_key_is_revoked(dst_key_t *key, isc_stdtime_t now, isc_stdtime_t *revoke)
 		time_ok = (when <= now);
 	}
 
-	return time_ok;
+	return (time_ok);
 }
 
 bool
@@ -2555,7 +2554,7 @@ dst_key_is_removed(dst_key_t *key, isc_stdtime_t now, isc_stdtime_t *remove)
 
 	if (dst_key_is_unused(key)) {
 		/* This key was never used. */
-		return false;
+		return (false);
 	}
 
 	result = dst_key_gettime(key, DST_TIME_DELETE, &when);
@@ -2579,7 +2578,20 @@ dst_key_is_removed(dst_key_t *key, isc_stdtime_t now, isc_stdtime_t *remove)
 		time_ok = true;
 	}
 
-	return state_ok && time_ok;
+	return (state_ok && time_ok);
+}
+
+dst_key_state_t
+dst_key_goal(dst_key_t *key)
+{
+	dst_key_state_t state;
+	isc_result_t result;
+
+	result = dst_key_getstate(key, DST_KEY_GOAL, &state);
+	if (result == ISC_R_SUCCESS) {
+		return (state);
+	}
+	return (DST_KEY_STATE_HIDDEN);
 }
 
 void

lib/dns/include/dst/dst.h

@@ -1166,6 +1166,18 @@ dst_key_is_removed(dst_key_t *key, isc_stdtime_t now, isc_stdtime_t *remove);
  *	'key' to be valid.
  */
 
+dst_key_state_t
+dst_key_goal(dst_key_t *key);
+/*%<
+ * Get the key goal. Should be OMNIPRESENT or HIDDEN.
+ * This can be used to determine if the key is being introduced or
+ * is on its way out.
+ *
+ * Requires:
+ *	'key' to be valid.
+ */
+
+
 void
 dst_key_copy_metadata(dst_key_t *to, dst_key_t *from);
 /*%<

lib/dns/keymgr.c

@@ -1358,7 +1358,7 @@ dns_keymgr_run(const dns_name_t *origin, dns_rdataclass_t rdclass,
 						       lifetime);
 				}
 
-				if (dst_key_is_active(dkey->key, now)) {
+				if (dst_key_goal(dkey->key) == OMNIPRESENT) {
 					if (active_key != NULL) {
 						/*
 						 * Multiple signing keys match

lib/dns/win32/libdns.def.in

@@ -1432,6 +1432,7 @@ dst_key_getprivateformat
 dst_key_getstate
 dst_key_gettime
 dst_key_getttl
+dst_key_goal
 dst_key_id
 dst_key_is_active
 dst_key_is_published