From 537a88e40336a5181da24a6f5d5ceedab53a59ef Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Thu, 14 Mar 2019 09:32:20 +0100
Subject: [PATCH 1/6] Add test for ZSK rollover while KSK offline

This commit adds a lengthy test where the ZSK is rolled but the
KSK is offline (except for when the DNSKEY RRset is changed).  The
specific scenario has the `dnskey-kskonly` configuration option set
meaning the DNSKEY RRset should only be signed with the KSK.

A new zone `updatecheck-kskonly.secure` is added to test against,
that can be dynamically updated, and that can be controlled with rndc
to load the DNSSEC keys.

There are some pre-checks for this test to make sure everything is
fine before the ZSK roll, after the new ZSK is published, and after
the old ZSK is deleted.  Note there are actually two ZSK rolls in
quick succession.

When the latest added ZSK becomes active and its predecessor becomes
inactive, the KSK is offline.  However, the DNSKEY RRset did not
change and it has a good signature that is valid for long enough.
The expected behavior is that the DNSKEY RRset stays signed with
the KSK only (signature does not need to change).  However, the
test will fail because after reconfiguring the keys for the zone,
it wants to add re-sign tasks for the new active keys (in sign_apex).
Because the KSK is offline, named determines that the only other
active key, the latest ZSK, will be used to resign the DNSKEY RRset,
in addition to keeping the RRSIG of the KSK.

The question is: Why do we need to resign the DNSKEY RRset
immediately when a new key becomes active?  This is not required,
only once the next resign task is triggered the new active key
should replace signatures that are in need of refreshing.

(cherry-picked from commit c48b85d0a3c34480179d44e736e3e535dbae1001)
---
 bin/tests/system/dnssec/clean.sh              |   5 +-
 bin/tests/system/dnssec/ns2/named.conf.in     |  26 +++
 bin/tests/system/dnssec/ns2/sign.sh           |  17 ++
 .../system/dnssec/ns2/template.secure.db.in   |  12 +
 bin/tests/system/dnssec/tests.sh              | 217 +++++++++++++++++-
 5 files changed, 274 insertions(+), 3 deletions(-)
 create mode 100644 bin/tests/system/dnssec/ns2/template.secure.db.in

diff --git a/bin/tests/system/dnssec/clean.sh b/bin/tests/system/dnssec/clean.sh
index f67c61d52f..9ca3f2c003 100644
--- a/bin/tests/system/dnssec/clean.sh
+++ b/bin/tests/system/dnssec/clean.sh
@@ -15,7 +15,7 @@ rm -f ./*/K* ./*/keyset-* ./*/dsset-* ./*/dlvset-* ./*/signedkey-* ./*/*.signed
 rm -f ./*/example.bk
 rm -f ./*/named.conf
 rm -f ./*/named.memstats
-rm -f ./*/named.run
+rm -f ./*/named.run ./*/named.run.prev
 rm -f ./*/named.secroots
 rm -f ./*/tmp* ./*/*.jnl ./*/*.bk ./*/*.jbk
 rm -f ./*/trusted.conf ./*/managed.conf ./*/revoked.conf
@@ -48,6 +48,9 @@ rm -f ./ns2/in-addr.arpa.db
 rm -f ./ns2/nsec3chain-test.db
 rm -f ./ns2/private.secure.example.db
 rm -f ./ns2/single-nsec3.db
+rm -f ./ns2/updatecheck-kskonly.secure.*
+rm -f ./ns3/secure.example.db ./ns3/*.managed.db ./ns3/*.trusted.db
+rm -f ./ns3/unsupported.managed.db.tmp ./ns3/unsupported.trusted.db.tmp
 rm -f ./ns3/auto-nsec.example.db ./ns3/auto-nsec3.example.db
 rm -f ./ns3/badds.example.db
 rm -f ./ns3/dname-at-apex-nsec3.example.db
diff --git a/bin/tests/system/dnssec/ns2/named.conf.in b/bin/tests/system/dnssec/ns2/named.conf.in
index 67cff87b76..22991b34ee 100644
--- a/bin/tests/system/dnssec/ns2/named.conf.in
+++ b/bin/tests/system/dnssec/ns2/named.conf.in
@@ -26,6 +26,15 @@ options {
 	notify-delay 1;
 };
 
+key rndc_key {
+        secret "1234abcd8765";
+        algorithm hmac-sha256;
+};
+
+controls {
+        inet 10.53.0.2 port @CONTROLPORT@ allow { any; } keys { rndc_key; };
+};
+
 zone "." {
 	type hint;
 	file "../../common/root.hint";
@@ -133,4 +142,21 @@ zone "cdnskey-auto.secure" {
 	allow-update { any; };
 };
 
+zone "updatecheck-kskonly.secure" {
+	type master;
+	auto-dnssec maintain;
+	key-directory ".";
+	dnssec-dnskey-kskonly yes;
+	update-check-ksk yes;
+	sig-validity-interval 10;
+	dnskey-sig-validity 40;
+	file "updatecheck-kskonly.secure.db.signed";
+	allow-update { any; };
+};
+
+zone "corp" {
+	type master;
+	file "corp.db";
+};
+
 include "trusted.conf";
diff --git a/bin/tests/system/dnssec/ns2/sign.sh b/bin/tests/system/dnssec/ns2/sign.sh
index ca186084ca..6ee989b14f 100644
--- a/bin/tests/system/dnssec/ns2/sign.sh
+++ b/bin/tests/system/dnssec/ns2/sign.sh
@@ -239,3 +239,20 @@ key1=`$KEYGEN -q -r $RANDFILE -a RSASHA1 -b 1024 -n zone -fk $zone`
 key2=`$KEYGEN -q -r $RANDFILE -a RSASHA1 -b 1024 -n zone $zone`
 sed 's/DNSKEY/CDNSKEY/' $key1.key > $key1.cds
 cat $infile $key1.cds > $zonefile.signed
+
+zone=updatecheck-kskonly.secure
+infile=template.secure.db.in
+zonefile=${zone}.db
+key1=`$KEYGEN -q -r $RANDFILE -a RSASHA1 -b 1024 -n zone -fk $zone`
+key2=`$KEYGEN -q -r $RANDFILE -a RSASHA1 -b 1024 -n zone $zone`
+# Save key id's for checking active key usage
+echo $key1 | sed -e 's/.*[+]//' -e 's/^0*//' > $zone.ksk.id
+echo $key2 | sed -e 's/.*[+]//' -e 's/^0*//' > $zone.zsk.id
+echo ${key1} > $zone.ksk.key
+echo ${key2} > $zone.zsk.key
+# Add CDS and CDNSKEY records
+sed 's/DNSKEY/CDNSKEY/' $key1.key > $key1.cdnskey
+$DSFROMKEY -C $key1.key > $key1.cds
+cat $infile $key1.key $key2.key $key1.cdnskey $key1.cds > $zonefile
+# Don't sign, let auto-dnssec maintain do it.
+mv $zonefile $zonefile.signed
diff --git a/bin/tests/system/dnssec/ns2/template.secure.db.in b/bin/tests/system/dnssec/ns2/template.secure.db.in
new file mode 100644
index 0000000000..e42cb4a29e
--- /dev/null
+++ b/bin/tests/system/dnssec/ns2/template.secure.db.in
@@ -0,0 +1,12 @@
+; 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 3600
+@	SOA	ns2.example. . 1 3600 1200 86400 1200
+@	NS	ns2.example.
diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh
index 8fedb8a878..54e5219b09 100644
--- a/bin/tests/system/dnssec/tests.sh
+++ b/bin/tests/system/dnssec/tests.sh
@@ -23,6 +23,26 @@ ANSWEROPTS="+noall +answer +dnssec -p ${PORT}"
 DELVOPTS="-a ns1/trusted.conf -p ${PORT}"
 RNDCCMD="$RNDC -c $SYSTEMTESTTOP/common/rndc.conf -p ${CONTROLPORT} -s"
 
+# TODO: Move wait_for_log and loadkeys_on to conf.sh.common
+wait_for_log() {
+        msg=$1
+        file=$2
+        for i in 1 2 3 4 5 6 7 8 9 10; do
+                nextpart "$file" | grep "$msg" > /dev/null && return
+                sleep 1
+        done
+        echo_i "exceeded time limit waiting for '$msg' in $file"
+        ret=1
+}
+
+dnssec_loadkeys_on() {
+	nsidx=$1
+	zone=$2
+	nextpart ns${nsidx}/named.run > /dev/null
+	rndccmd 10.53.0.${nsidx} loadkeys ${zone} | sed "s/^/ns${nsidx} /" | cat_i
+	wait_for_log "next key event" ns${nsidx}/named.run
+}
+
 # convert private-type records to readable form
 showprivate () {
     echo "-- $@ --"
@@ -2586,7 +2606,7 @@ n=`expr $n + 1`
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
-echo_i "checking dnskey query with no data still gets put in cache ($n)"
+echo_i "checking DNSKEY query with no data still gets put in cache ($n)"
 ret=0
 myDIGOPTS="+noadd +nosea +nostat +noquest +nocomm +nocmd -p ${PORT} @10.53.0.4"
 firstVal=`$DIG $myDIGOPTS insecure.example. dnskey| awk '$1 != ";;" { print $2 }'`
@@ -2643,7 +2663,7 @@ do
 	fi
 	echo_i "sleeping ...."
 	sleep 3
-done;
+done
 grep "ANSWER: 3," dig.out.ns2.test$n > /dev/null || ret=1
 if [ $ret != 0 ]; then echo_i "nsec3 chain generation not complete"; fi
 $DIG $DIGOPTS +noauth +nodnssec soa nsec3chain-test @10.53.0.2 > dig.out.ns2.test$n || ret=1
@@ -3573,5 +3593,198 @@ n=`expr $n + 1`
 test "$ret" -eq 0 || echo_i "failed"
 status=`expr $status + $ret`
 
+###
+### Additional checks for when the KSK is offline.
+###
+
+# Save some useful information
+zone="updatecheck-kskonly.secure"
+KSK=`cat ns2/${zone}.ksk.key`
+ZSK=`cat ns2/${zone}.zsk.key`
+KSK_ID=`cat ns2/${zone}.ksk.id`
+ZSK_ID=`cat ns2/${zone}.zsk.id`
+SECTIONS="+answer +noauthority +noadditional"
+echo_i "testing zone $zone KSK=$KSK_ID ZSK=$ZSK_ID"
+
+# Basic checks to make sure everything is fine before the KSK is made offline.
+for qtype in "DNSKEY" "CDNSKEY" "CDS"
+do
+  echo_i "checking $qtype RRset is signed with KSK only (update-check-ksk, dnssec-ksk-only) ($n)"
+  ret=0
+  dig_with_opts $SECTIONS @10.53.0.2 $qtype $zone > dig.out.test$n
+  lines=$(awk -v qt="$qtype" '$4 == "RRSIG" && $5 == qt {print}' dig.out.test$n | wc -l)
+  test "$lines" -eq 1 || ret=1
+  grep $KSK_ID dig.out.test$n > /dev/null || ret=1
+  grep $ZSK_ID dig.out.test$n > /dev/null && ret=1
+  n=$((n+1))
+  test "$ret" -eq 0 || echo_i "failed"
+  status=$((status+ret))
+done
+
+echo_i "checking SOA RRset is signed with ZSK only (update-check-ksk and dnssec-ksk-only) ($n)"
+ret=0
+dig_with_opts $SECTIONS @10.53.0.2 soa $zone > dig.out.test$n
+lines=$(awk '$4 == "RRSIG" && $5 == "SOA" {print}' dig.out.test$n | wc -l)
+grep $KSK_ID dig.out.test$n > /dev/null && ret=1
+grep $ZSK_ID dig.out.test$n > /dev/null || ret=1
+test "$lines" -eq 1 || ret=1
+n=$((n+1))
+test "$ret" -eq 0 || echo_i "failed"
+status=$((status+ret))
+
+# Roll the ZSK.
+sleep 1
+zsk2=$("$KEYGEN" -q -a "$DEFAULT_ALGORITHM" -b "$DEFAULT_BITS" -K ns2 -n zone "$zone")
+echo_i "new ZSK $zsk2 created for zone $zone"
+echo "$zsk2" | sed -e 's/.*[+]//' -e 's/^0*//' > ns2/$zone.zsk.id2
+ZSK_ID2=`cat ns2/$zone.zsk.id2`
+dnssec_loadkeys_on 2 $zone
+
+# Wait until new ZSK becomes active.
+sleep 1
+echo_i "make ZSK $ZSK inactive and make new ZSK $zsk2 active for zone $zone"
+$SETTIME -I now -K ns2 $ZSK > /dev/null
+$SETTIME -A now -K ns2 $zsk2 > /dev/null
+dnssec_loadkeys_on 2 $zone
+
+# Remove the KSK from disk.
+sleep 1
+echo_i "remove the KSK $KSK for zone $zone from disk"
+mv ns2/$KSK.key ns2/$KSK.key.bak
+mv ns2/$KSK.private ns2/$KSK.private.bak
+
+# Update the zone that requires a resign of the SOA RRset.
+sleep 1
+echo_i "update the zone with $zone IN TXT nsupdate added me"
+(
+echo zone $zone
+echo server 10.53.0.2 "$PORT"
+echo update add $zone. 300 in txt "nsupdate added me"
+echo send
+) | $NSUPDATE
+
+# Redo the tests now that the zone is updated and the KSK is offline.
+for qtype in "DNSKEY" "CDNSKEY" "CDS"
+do
+  echo_i "checking $qtype RRset is signed with KSK only, KSK offline (update-check-ksk, dnssec-ksk-only) ($n)"
+  ret=0
+  dig_with_opts $SECTIONS @10.53.0.2 $qtype $zone > dig.out.test$n
+  lines=$(awk -v qt="$qtype" '$4 == "RRSIG" && $5 == qt {print}' dig.out.test$n | wc -l)
+  test "$lines" -eq 1 || ret=1
+  grep $KSK_ID  dig.out.test$n > /dev/null || ret=1
+  grep $ZSK_ID  dig.out.test$n > /dev/null && ret=1
+  grep $ZSK_ID2 dig.out.test$n > /dev/null && ret=1
+  n=$((n+1))
+  test "$ret" -eq 0 || echo_i "failed"
+  status=$((status+ret))
+done
+
+for qtype in "SOA" "TXT"
+do
+  echo_i "checking $qtype RRset is signed with ZSK only, KSK offline (update-check-ksk and dnssec-ksk-only) ($n)"
+  ret=0
+  dig_with_opts $SECTIONS @10.53.0.2 $qtype $zone > dig.out.test$n
+  lines=$(awk -v qt="$qtype" '$4 == "RRSIG" && $5 == qt {print}' dig.out.test$n | wc -l)
+  grep $KSK_ID  dig.out.test$n > /dev/null && ret=1
+  grep $ZSK_ID  dig.out.test$n > /dev/null && ret=1
+  grep $ZSK_ID2 dig.out.test$n > /dev/null || ret=1
+  test "$lines" -eq 1 || ret=1
+  n=$((n+1))
+  test "$ret" -eq 0 || echo_i "failed"
+  status=$((status+ret))
+done
+
+# Put back the KSK.
+sleep 1
+echo_i "put back the KSK $KSK for zone $zone from disk"
+mv ns2/$KSK.key.bak ns2/$KSK.key
+mv ns2/$KSK.private.bak ns2/$KSK.private
+
+# Roll the ZSK again.
+sleep 1
+zsk3=$("$KEYGEN" -q -a "$DEFAULT_ALGORITHM" -b "$DEFAULT_BITS" -K ns2 -n zone "$zone")
+echo_i "new ZSK $zsk3 created for zone $zone"
+echo "$zsk3" | sed -e 's/.*[+]//' -e 's/^0*//' > ns2/$zone.zsk.id3
+ZSK_ID3=`cat ns2/$zone.zsk.id3`
+dnssec_loadkeys_on 2 $zone
+
+# Wait until new ZSK becomes active.
+sleep 1
+echo_i "delete old ZSK $ZSK make ZSK $ZSK2 inactive and make new ZSK $zsk3 active for zone $zone"
+$SETTIME -D now -K ns2 $ZSK > /dev/null
+$SETTIME -I +5 -K ns2 $zsk2 > /dev/null
+$SETTIME -A +5 -K ns2 $zsk3 > /dev/null
+dnssec_loadkeys_on 2 $zone
+
+# Remove the KSK from disk.
+sleep 1
+echo_i "remove the KSK $KSK for zone $zone from disk"
+mv ns2/$KSK.key ns2/$KSK.key.bak
+mv ns2/$KSK.private ns2/$KSK.private.bak
+
+# Update the zone that requires a resign of the SOA RRset.
+sleep 1
+echo_i "update the zone with $zone IN TXT nsupdate added me again"
+(
+echo zone $zone
+echo server 10.53.0.2 "$PORT"
+echo update add $zone. 300 in txt "nsupdate added me again"
+echo send
+) | $NSUPDATE
+
+# Redo the tests now that the ZSK roll has deleted the old key.
+for qtype in "DNSKEY" "CDNSKEY" "CDS"
+do
+  echo_i "checking $qtype RRset is signed with KSK only, old ZSK deleted (update-check-ksk, dnssec-ksk-only) ($n)"
+  ret=0
+  dig_with_opts $SECTIONS @10.53.0.2 $qtype $zone > dig.out.test$n
+  lines=$(awk -v qt="$qtype" '$4 == "RRSIG" && $5 == qt {print}' dig.out.test$n | wc -l)
+  test "$lines" -eq 1 || ret=1
+  grep $KSK_ID  dig.out.test$n > /dev/null || ret=1
+  grep $ZSK_ID  dig.out.test$n > /dev/null && ret=1
+  grep $ZSK_ID2 dig.out.test$n > /dev/null && ret=1
+  grep $ZSK_ID3 dig.out.test$n > /dev/null && ret=1
+  n=$((n+1))
+  test "$ret" -eq 0 || echo_i "failed"
+  status=$((status+ret))
+done
+
+for qtype in "SOA" "TXT"
+do
+  echo_i "checking $qtype RRset is signed with ZSK only, old ZSK deleted (update-check-ksk and dnssec-ksk-only) ($n)"
+  ret=0
+  dig_with_opts $SECTIONS @10.53.0.2 $qtype $zone > dig.out.test$n
+  lines=$(awk -v qt="$qtype" '$4 == "RRSIG" && $5 == qt {print}' dig.out.test$n | wc -l)
+  grep $KSK_ID  dig.out.test$n > /dev/null && ret=1
+  grep $ZSK_ID  dig.out.test$n > /dev/null && ret=1
+  grep $ZSK_ID2 dig.out.test$n > /dev/null || ret=1
+  grep $ZSK_ID3 dig.out.test$n > /dev/null && ret=1
+  test "$lines" -eq 1 || ret=1
+  n=$((n+1))
+  test "$ret" -eq 0 || echo_i "failed"
+  status=$((status+ret))
+done
+
+# Wait for newest ZSK to become active.
+echo_i "sleep 6 to make new ZSK $zsk3 active and ZSK $zsk2 inactive"
+sleep 6
+
+# Redo the tests one more time.
+for qtype in "DNSKEY" "CDNSKEY" "CDS"
+do
+  echo_i "checking $qtype RRset is signed with KSK only, new ZSK active (update-check-ksk, dnssec-ksk-only) ($n)"
+  ret=0
+  dig_with_opts $SECTIONS @10.53.0.2 $qtype $zone > dig.out.test$n
+  lines=$(awk -v qt="$qtype" '$4 == "RRSIG" && $5 == qt {print}' dig.out.test$n | wc -l)
+  test "$lines" -eq 1 || ret=1
+  grep $KSK_ID  dig.out.test$n > /dev/null || ret=1
+  grep $ZSK_ID  dig.out.test$n > /dev/null && ret=1
+  grep $ZSK_ID2 dig.out.test$n > /dev/null && ret=1
+  grep $ZSK_ID3 dig.out.test$n > /dev/null && ret=1
+  n=$((n+1))
+  test "$ret" -eq 0 || echo_i "failed"
+  status=$((status+ret))
+done
+
 echo_i "exit status: $status"
 [ $status -eq 0 ] || exit 1

From 944c2b5a747da6b505d6c563536edec74888a029 Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Thu, 14 Mar 2019 09:43:14 +0100
Subject: [PATCH 2/6] Add detail on echo message in autosign test

(cherry picked from commit d330986374d434e8a5101478cc6b476b42fa588a)
(cherry picked from commit d281d9ae99985772db13fb3dce0c0e7e2fb5f5b8)
---
 bin/tests/system/autosign/tests.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bin/tests/system/autosign/tests.sh b/bin/tests/system/autosign/tests.sh
index f4bf6d9f48..fd237cd4c9 100755
--- a/bin/tests/system/autosign/tests.sh
+++ b/bin/tests/system/autosign/tests.sh
@@ -998,7 +998,7 @@ $RNDCCMD 10.53.0.2 loadkeys bar. 2>&1 | sed 's/^/ns2 /' | cat_i
 echo_i "waiting for changes to take effect"
 sleep 5
 
-echo_i "checking former standby key is now active ($n)"
+echo_i "checking former standby key $newid is now active ($n)"
 ret=0
 $DIG $DIGOPTS dnskey . @10.53.0.1 > dig.out.ns1.test$n || ret=1
 grep 'RRSIG.*'" $newid "'\. ' dig.out.ns1.test$n > /dev/null || ret=1

From 9079ae03c7804fa60edacf92b76a902b5dcf567f Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Thu, 14 Mar 2019 09:44:01 +0100
Subject: [PATCH 3/6] Style: some curly brackets

(cherry picked from commit 2e83e3255a9c0096e1d386839ff2b72ea0185ac5)
(cherry picked from commit 42b0bf4d3bab180876d4803fe2ec1f6e93064b28)
---
 lib/dns/update.c | 33 ++++++++++++++++++---------
 lib/dns/zone.c   | 58 +++++++++++++++++++++++++++++++++---------------
 2 files changed, 63 insertions(+), 28 deletions(-)

diff --git a/lib/dns/update.c b/lib/dns/update.c
index 47cc4cf4ef..7c9afc24e8 100644
--- a/lib/dns/update.c
+++ b/lib/dns/update.c
@@ -1105,10 +1105,13 @@ add_sigs(dns_update_log_t *log, dns_zone_t *zone, dns_db_t *db,
 	for (i = 0; i < nkeys; i++) {
 		bool both = false;
 
-		if (!dst_key_isprivate(keys[i]))
+		/* Don't add signatures for offline or inactive keys */
+		if (!dst_key_isprivate(keys[i])) {
 			continue;
-		if (dst_key_inactive(keys[i]))	/* Should be redundant. */
+		}
+		if (dst_key_inactive(keys[i])) {
 			continue;
+		}
 
 		if (check_ksk && !REVOKE(keys[i])) {
 			bool have_ksk, have_nonksk;
@@ -1120,21 +1123,31 @@ add_sigs(dns_update_log_t *log, dns_zone_t *zone, dns_db_t *db,
 				have_nonksk = true;
 			}
 			for (j = 0; j < nkeys; j++) {
-				if (j == i || ALG(keys[i]) != ALG(keys[j]))
+				if (j == i || ALG(keys[i]) != ALG(keys[j])) {
 					continue;
-				if (!dst_key_isprivate(keys[j]))
+				}
+
+				/* Don't consider inactive keys, however
+				 * the key may be temporary offline, so do
+				 * consider keys which private key files are
+				 * unavailable.
+				 */
+				if (dst_key_inactive(keys[j])) {
 					continue;
-				if (dst_key_inactive(keys[j]))	/* SBR */
+				}
+
+				if (REVOKE(keys[j])) {
 					continue;
-				if (REVOKE(keys[j]))
-					continue;
-				if (KSK(keys[j]))
+				}
+				if (KSK(keys[j])) {
 					have_ksk = true;
-				else
+				} else {
 					have_nonksk = true;
+				}
 				both = have_ksk && have_nonksk;
-				if (both)
+				if (both) {
 					break;
+				}
 			}
 		}
 
diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index 3f1bc448c6..57295fc00f 100644
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -6365,10 +6365,11 @@ del_sigs(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name,
 		 * If there is not a matching DNSKEY then
 		 * delete the RRSIG.
 		 */
-		if (!found)
+		if (!found) {
 			result = update_one_rr(db, ver, zonediff->diff,
 					       DNS_DIFFOP_DELRESIGN, name,
 					       rdataset.ttl, &rdata);
+		}
 		if (result != ISC_R_SUCCESS)
 			break;
 	}
@@ -6433,10 +6434,13 @@ add_sigs(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name,
 	for (i = 0; i < nkeys; i++) {
 		bool both = false;
 
-		if (!dst_key_isprivate(keys[i]))
+		/* Don't add signatures for offline or inactive keys */
+		if (!dst_key_isprivate(keys[i])) {
 			continue;
-		if (dst_key_inactive(keys[i]))	/* Should be redundant. */
+		}
+		if (dst_key_inactive(keys[i])) {
 			continue;
+		}
 
 		if (check_ksk && !REVOKE(keys[i])) {
 			bool have_ksk, have_nonksk;
@@ -6447,24 +6451,36 @@ add_sigs(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name,
 				have_ksk = false;
 				have_nonksk = true;
 			}
+
 			for (j = 0; j < nkeys; j++) {
-				if (j == i || ALG(keys[i]) != ALG(keys[j]))
+				if (j == i || ALG(keys[i]) != ALG(keys[j])) {
 					continue;
-				if (!dst_key_isprivate(keys[j]))
+				}
+
+				/* Don't consider inactive keys, however
+				 * the key may be temporary offline, so do
+				 * consider keys which private key files are
+				 * unavailable.
+				 */
+				if (dst_key_inactive(keys[j])) {
 					continue;
-				if (dst_key_inactive(keys[j]))	/* SBR */
+				}
+
+				if (REVOKE(keys[j])) {
 					continue;
-				if (REVOKE(keys[j]))
-					continue;
-				if (KSK(keys[j]))
+				}
+				if (KSK(keys[j])) {
 					have_ksk = true;
-				else
+				} else {
 					have_nonksk = true;
+				}
 				both = have_ksk && have_nonksk;
-				if (both)
+				if (both) {
 					break;
+				}
 			}
 		}
+
 		if (both) {
 			if (type == dns_rdatatype_dnskey) {
 				if (!KSK(keys[i]) && keyset_kskonly)
@@ -10220,14 +10236,17 @@ zone_maintenance(dns_zone_t *zone) {
 		if (zone->rss_event != NULL)
 			break;
 		if (!isc_time_isepoch(&zone->signingtime) &&
-		    isc_time_compare(&now, &zone->signingtime) >= 0)
+		    isc_time_compare(&now, &zone->signingtime) >= 0) {
 			zone_sign(zone);
+		}
 		else if (!isc_time_isepoch(&zone->resigntime) &&
-		    isc_time_compare(&now, &zone->resigntime) >= 0)
+		    isc_time_compare(&now, &zone->resigntime) >= 0) {
 			zone_resigninc(zone);
+		}
 		else if (!isc_time_isepoch(&zone->nsec3chaintime) &&
-			isc_time_compare(&now, &zone->nsec3chaintime) >= 0)
+			isc_time_compare(&now, &zone->nsec3chaintime) >= 0) {
 			zone_nsec3chain(zone);
+		}
 		/*
 		 * Do we need to issue a key expiry warning?
 		 */
@@ -17770,15 +17789,18 @@ add_signing_records(dns_db_t *db, dns_rdatatype_t privatetype,
 	for (tuple = ISC_LIST_HEAD(diff->tuples);
 	     tuple != NULL;
 	     tuple = ISC_LIST_NEXT(tuple, link)) {
-		if (tuple->rdata.type != dns_rdatatype_dnskey)
+		if (tuple->rdata.type != dns_rdatatype_dnskey) {
 			continue;
+		}
 
 		result = dns_rdata_tostruct(&tuple->rdata, &dnskey, NULL);
 		RUNTIME_CHECK(result == ISC_R_SUCCESS);
 		if ((dnskey.flags &
 		     (DNS_KEYFLAG_OWNERMASK|DNS_KEYTYPE_NOAUTH))
 			 != DNS_KEYOWNER_ZONE)
+		{
 			continue;
+		}
 
 		dns_rdata_toregion(&tuple->rdata, &r);
 
@@ -17796,8 +17818,10 @@ add_signing_records(dns_db_t *db, dns_rdatatype_t privatetype,
 
 		if (sign_all || tuple->op == DNS_DIFFOP_DEL) {
 			CHECK(rr_exists(db, ver, name, &rdata, &flag));
-			if (flag)
+			if (flag) {
 				continue;
+			}
+
 			CHECK(dns_difftuple_create(diff->mctx, DNS_DIFFOP_ADD,
 						   name, 0, &rdata, &newtuple));
 			CHECK(do_one_tuple(&newtuple, db, ver, diff));
@@ -18097,7 +18121,6 @@ zone_rekey(dns_zone_t *zone) {
 	} else if (result != ISC_R_NOTFOUND)
 		goto failure;
 
-
 	/* Get the CDS rdataset */
 	result = dns_db_findrdataset(db, node, ver, dns_rdatatype_cds,
 				     dns_rdatatype_none, 0, &cdsset, NULL);
@@ -18121,7 +18144,6 @@ zone_rekey(dns_zone_t *zone) {
 	if (result == ISC_R_SUCCESS) {
 		bool check_ksk;
 		check_ksk = DNS_ZONE_OPTION(zone, DNS_ZONEOPT_UPDATECHECKKSK);
-
 		result = dns_dnssec_updatekeys(&dnskeys, &keys, &rmkeys,
 					       &zone->origin, ttl, &diff,
 					       !check_ksk,

From 4af2d5b6d6724521f52084ee55f9a1cfe4246f9c Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Fri, 22 Mar 2019 15:42:10 +0100
Subject: [PATCH 4/6] With update-check-ksk also consider offline keys

The option `update-check-ksk` will look if both KSK and ZSK are
available before signing records.  It will make sure the keys are
active and available.  However, for operational practices keys may
be offline.  This commit relaxes the update-check-ksk check and will
mark a key that is offline to be available when adding signature
tasks.

(cherry picked from commit 3cb8c49c73906b28921012619a3bb87805613b81)
(cherry picked from commit b508cffeee3bfb8bc7dcf39db59ec3782a5d9e4c)
---
 CHANGES        |  4 ++++
 lib/dns/zone.c | 11 ++++-------
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/CHANGES b/CHANGES
index 28df46719b..ae83d90b91 100644
--- a/CHANGES
+++ b/CHANGES
@@ -4,6 +4,10 @@
 			recursion was requested by the client, not on
 			whether recursion was available. [GL #963]
 
+5209.	[bug]		When update-check-ksk is true, add_sigs was not
+			considering offline keys, leaving record sets signed
+			with the incorrect type key. [GL #763]
+
 5208.	[test]		Run valid rdata wire encodings through totext+fromtext
 			and tofmttext+fromtext methods to check these methods.
 			[GL #899]
diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index 57295fc00f..000b42b841 100644
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -8669,9 +8669,6 @@ zone_sign(dns_zone_t *zone) {
 			 */
 			if (!dst_key_isprivate(zone_keys[i]))
 				continue;
-			/*
-			 * Should be redundant.
-			 */
 			if (dst_key_inactive(zone_keys[i]))
 				continue;
 
@@ -8710,11 +8707,11 @@ zone_sign(dns_zone_t *zone) {
 						continue;
 					if (!dst_key_isprivate(zone_keys[j]))
 						continue;
-					/*
-					 * Should be redundant.
+					/* Don't consider inactive keys, however
+					 * the key may be temporary offline, so do
+					 * consider keys which private key files are
+					 * unavailable.
 					 */
-					if (dst_key_inactive(zone_keys[j]))
-						continue;
 					if (REVOKE(zone_keys[j]))
 						continue;
 					if (KSK(zone_keys[j]))

From c5e1bfc6f94f232999ae671728e5518621f71002 Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Fri, 12 Apr 2019 11:31:41 +0200
Subject: [PATCH 5/6] Fix copyrights

---
 bin/tests/system/dnssec/ns2/named.conf.in | 6 +++---
 util/copyrights                           | 1 +
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/bin/tests/system/dnssec/ns2/named.conf.in b/bin/tests/system/dnssec/ns2/named.conf.in
index 22991b34ee..8e0cfbb0e1 100644
--- a/bin/tests/system/dnssec/ns2/named.conf.in
+++ b/bin/tests/system/dnssec/ns2/named.conf.in
@@ -27,12 +27,12 @@ options {
 };
 
 key rndc_key {
-        secret "1234abcd8765";
-        algorithm hmac-sha256;
+	secret "1234abcd8765";
+	algorithm hmac-sha256;
 };
 
 controls {
-        inet 10.53.0.2 port @CONTROLPORT@ allow { any; } keys { rndc_key; };
+	inet 10.53.0.2 port @CONTROLPORT@ allow { any; } keys { rndc_key; };
 };
 
 zone "." {
diff --git a/util/copyrights b/util/copyrights
index 4f7b0050cf..22b7896f69 100644
--- a/util/copyrights
+++ b/util/copyrights
@@ -1095,6 +1095,7 @@
 ./bin/tests/system/dnssec/ns2/rfc2335.example.db	X	2004,2018,2019
 ./bin/tests/system/dnssec/ns2/sign.sh		SH	2000,2001,2002,2003,2004,2006,2007,2008,2009,2010,2011,2012,2014,2015,2016,2018,2019
 ./bin/tests/system/dnssec/ns2/single-nsec3.db.in	ZONE	2010,2016,2018,2019
+./bin/tests/system/dnssec/ns2/template.secure.db.in	ZONE	2019
 ./bin/tests/system/dnssec/ns3/auto-nsec.example.db.in	ZONE	2011,2016,2018,2019
 ./bin/tests/system/dnssec/ns3/auto-nsec3.example.db.in	ZONE	2011,2016,2018,2019
 ./bin/tests/system/dnssec/ns3/bogus.example.db.in	ZONE	2000,2001,2004,2007,2014,2016,2018,2019

From ce3d35d95010bfd72f5821959bd2109b1dbc1941 Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Fri, 12 Apr 2019 15:41:48 +0200
Subject: [PATCH 6/6] Fix dnssec test

The following changes were needed:

* Remove dnskey-sig-validity option (added in 9.12)
* Replace rndccmd, dig_with_opts with export variables
* Remove tests for CDNSKEY and CDS (in 9.11 always signed with ZSK)
---
 bin/tests/system/dnssec/ns2/named.conf.in |   1 -
 bin/tests/system/dnssec/tests.sh          | 119 ++++++++++------------
 2 files changed, 54 insertions(+), 66 deletions(-)

diff --git a/bin/tests/system/dnssec/ns2/named.conf.in b/bin/tests/system/dnssec/ns2/named.conf.in
index 8e0cfbb0e1..6124f4540f 100644
--- a/bin/tests/system/dnssec/ns2/named.conf.in
+++ b/bin/tests/system/dnssec/ns2/named.conf.in
@@ -149,7 +149,6 @@ zone "updatecheck-kskonly.secure" {
 	dnssec-dnskey-kskonly yes;
 	update-check-ksk yes;
 	sig-validity-interval 10;
-	dnskey-sig-validity 40;
 	file "updatecheck-kskonly.secure.db.signed";
 	allow-update { any; };
 };
diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh
index 54e5219b09..67d66fa5ec 100644
--- a/bin/tests/system/dnssec/tests.sh
+++ b/bin/tests/system/dnssec/tests.sh
@@ -39,7 +39,7 @@ dnssec_loadkeys_on() {
 	nsidx=$1
 	zone=$2
 	nextpart ns${nsidx}/named.run > /dev/null
-	rndccmd 10.53.0.${nsidx} loadkeys ${zone} | sed "s/^/ns${nsidx} /" | cat_i
+	$RNDCCMD 10.53.0.${nsidx} loadkeys ${zone} | sed "s/^/ns${nsidx} /" | cat_i
 	wait_for_log "next key event" ns${nsidx}/named.run
 }
 
@@ -3607,23 +3607,20 @@ SECTIONS="+answer +noauthority +noadditional"
 echo_i "testing zone $zone KSK=$KSK_ID ZSK=$ZSK_ID"
 
 # Basic checks to make sure everything is fine before the KSK is made offline.
-for qtype in "DNSKEY" "CDNSKEY" "CDS"
-do
-  echo_i "checking $qtype RRset is signed with KSK only (update-check-ksk, dnssec-ksk-only) ($n)"
-  ret=0
-  dig_with_opts $SECTIONS @10.53.0.2 $qtype $zone > dig.out.test$n
-  lines=$(awk -v qt="$qtype" '$4 == "RRSIG" && $5 == qt {print}' dig.out.test$n | wc -l)
-  test "$lines" -eq 1 || ret=1
-  grep $KSK_ID dig.out.test$n > /dev/null || ret=1
-  grep $ZSK_ID dig.out.test$n > /dev/null && ret=1
-  n=$((n+1))
-  test "$ret" -eq 0 || echo_i "failed"
-  status=$((status+ret))
-done
-
-echo_i "checking SOA RRset is signed with ZSK only (update-check-ksk and dnssec-ksk-only) ($n)"
+echo_i "checking DNSKEY RRset is signed with KSK only (update-check-ksk, dnssec-ksk-only) ($n)"
 ret=0
-dig_with_opts $SECTIONS @10.53.0.2 soa $zone > dig.out.test$n
+$DIG $DIGOPTS $SECTIONS @10.53.0.2 DNSKEY $zone > dig.out.test$n
+lines=$(awk '$4 == "RRSIG" && $5 == "DNSKEY" {print}' dig.out.test$n | wc -l)
+test "$lines" -eq 1 || ret=1
+grep $KSK_ID dig.out.test$n > /dev/null || ret=1
+grep $ZSK_ID dig.out.test$n > /dev/null && ret=1
+n=$((n+1))
+test "$ret" -eq 0 || echo_i "failed"
+status=$((status+ret))
+
+echo_i "checking SOA RRset is signed with ZSK only (update-check-ksk, dnssec-ksk-only) ($n)"
+ret=0
+$DIG $DIGOPTS $SECTIONS @10.53.0.2 soa $zone > dig.out.test$n
 lines=$(awk '$4 == "RRSIG" && $5 == "SOA" {print}' dig.out.test$n | wc -l)
 grep $KSK_ID dig.out.test$n > /dev/null && ret=1
 grep $ZSK_ID dig.out.test$n > /dev/null || ret=1
@@ -3633,8 +3630,9 @@ test "$ret" -eq 0 || echo_i "failed"
 status=$((status+ret))
 
 # Roll the ZSK.
+echo_i "roll ZSK for zone $zone"
 sleep 1
-zsk2=$("$KEYGEN" -q -a "$DEFAULT_ALGORITHM" -b "$DEFAULT_BITS" -K ns2 -n zone "$zone")
+zsk2=`$KEYGEN -q -r $RANDFILE -a RSASHA1 -b 1024 -K ns2 -n zone $zone`
 echo_i "new ZSK $zsk2 created for zone $zone"
 echo "$zsk2" | sed -e 's/.*[+]//' -e 's/^0*//' > ns2/$zone.zsk.id2
 ZSK_ID2=`cat ns2/$zone.zsk.id2`
@@ -3664,26 +3662,23 @@ echo send
 ) | $NSUPDATE
 
 # Redo the tests now that the zone is updated and the KSK is offline.
-for qtype in "DNSKEY" "CDNSKEY" "CDS"
-do
-  echo_i "checking $qtype RRset is signed with KSK only, KSK offline (update-check-ksk, dnssec-ksk-only) ($n)"
-  ret=0
-  dig_with_opts $SECTIONS @10.53.0.2 $qtype $zone > dig.out.test$n
-  lines=$(awk -v qt="$qtype" '$4 == "RRSIG" && $5 == qt {print}' dig.out.test$n | wc -l)
-  test "$lines" -eq 1 || ret=1
-  grep $KSK_ID  dig.out.test$n > /dev/null || ret=1
-  grep $ZSK_ID  dig.out.test$n > /dev/null && ret=1
-  grep $ZSK_ID2 dig.out.test$n > /dev/null && ret=1
-  n=$((n+1))
-  test "$ret" -eq 0 || echo_i "failed"
-  status=$((status+ret))
-done
+echo_i "checking DNSKEY RRset is signed with KSK only, KSK offline (update-check-ksk, dnssec-ksk-only) ($n)"
+ret=0
+$DIG $DIGOPTS $SECTIONS @10.53.0.2 DNSKEY $zone > dig.out.test$n
+lines=$(awk '$4 == "RRSIG" && $5 == "DNSKEY" {print}' dig.out.test$n | wc -l)
+test "$lines" -eq 1 || ret=1
+grep $KSK_ID  dig.out.test$n > /dev/null || ret=1
+grep $ZSK_ID  dig.out.test$n > /dev/null && ret=1
+grep $ZSK_ID2 dig.out.test$n > /dev/null && ret=1
+n=$((n+1))
+test "$ret" -eq 0 || echo_i "failed"
+status=$((status+ret))
 
 for qtype in "SOA" "TXT"
 do
   echo_i "checking $qtype RRset is signed with ZSK only, KSK offline (update-check-ksk and dnssec-ksk-only) ($n)"
   ret=0
-  dig_with_opts $SECTIONS @10.53.0.2 $qtype $zone > dig.out.test$n
+  $DIG $DIGOPTS $SECTIONS @10.53.0.2 $qtype $zone > dig.out.test$n
   lines=$(awk -v qt="$qtype" '$4 == "RRSIG" && $5 == qt {print}' dig.out.test$n | wc -l)
   grep $KSK_ID  dig.out.test$n > /dev/null && ret=1
   grep $ZSK_ID  dig.out.test$n > /dev/null && ret=1
@@ -3702,7 +3697,7 @@ mv ns2/$KSK.private.bak ns2/$KSK.private
 
 # Roll the ZSK again.
 sleep 1
-zsk3=$("$KEYGEN" -q -a "$DEFAULT_ALGORITHM" -b "$DEFAULT_BITS" -K ns2 -n zone "$zone")
+zsk3=`$KEYGEN -q  -r $RANDFILE -a RSASHA1 -b 1024 -K ns2 -n zone $zone`
 echo_i "new ZSK $zsk3 created for zone $zone"
 echo "$zsk3" | sed -e 's/.*[+]//' -e 's/^0*//' > ns2/$zone.zsk.id3
 ZSK_ID3=`cat ns2/$zone.zsk.id3`
@@ -3733,27 +3728,24 @@ echo send
 ) | $NSUPDATE
 
 # Redo the tests now that the ZSK roll has deleted the old key.
-for qtype in "DNSKEY" "CDNSKEY" "CDS"
-do
-  echo_i "checking $qtype RRset is signed with KSK only, old ZSK deleted (update-check-ksk, dnssec-ksk-only) ($n)"
-  ret=0
-  dig_with_opts $SECTIONS @10.53.0.2 $qtype $zone > dig.out.test$n
-  lines=$(awk -v qt="$qtype" '$4 == "RRSIG" && $5 == qt {print}' dig.out.test$n | wc -l)
-  test "$lines" -eq 1 || ret=1
-  grep $KSK_ID  dig.out.test$n > /dev/null || ret=1
-  grep $ZSK_ID  dig.out.test$n > /dev/null && ret=1
-  grep $ZSK_ID2 dig.out.test$n > /dev/null && ret=1
-  grep $ZSK_ID3 dig.out.test$n > /dev/null && ret=1
-  n=$((n+1))
-  test "$ret" -eq 0 || echo_i "failed"
-  status=$((status+ret))
-done
+echo_i "checking DNSKEY RRset is signed with KSK only, old ZSK deleted (update-check-ksk, dnssec-ksk-only) ($n)"
+ret=0
+$DIG $DIGOPTS $SECTIONS @10.53.0.2 DNSKEY $zone > dig.out.test$n
+lines=$(awk '$4 == "RRSIG" && $5 == "DNSKEY" {print}' dig.out.test$n | wc -l)
+test "$lines" -eq 1 || ret=1
+grep $KSK_ID  dig.out.test$n > /dev/null || ret=1
+grep $ZSK_ID  dig.out.test$n > /dev/null && ret=1
+grep $ZSK_ID2 dig.out.test$n > /dev/null && ret=1
+grep $ZSK_ID3 dig.out.test$n > /dev/null && ret=1
+n=$((n+1))
+test "$ret" -eq 0 || echo_i "failed"
+status=$((status+ret))
 
 for qtype in "SOA" "TXT"
 do
   echo_i "checking $qtype RRset is signed with ZSK only, old ZSK deleted (update-check-ksk and dnssec-ksk-only) ($n)"
   ret=0
-  dig_with_opts $SECTIONS @10.53.0.2 $qtype $zone > dig.out.test$n
+  $DIG $DIGOPTS $SECTIONS @10.53.0.2 $qtype $zone > dig.out.test$n
   lines=$(awk -v qt="$qtype" '$4 == "RRSIG" && $5 == qt {print}' dig.out.test$n | wc -l)
   grep $KSK_ID  dig.out.test$n > /dev/null && ret=1
   grep $ZSK_ID  dig.out.test$n > /dev/null && ret=1
@@ -3770,21 +3762,18 @@ echo_i "sleep 6 to make new ZSK $zsk3 active and ZSK $zsk2 inactive"
 sleep 6
 
 # Redo the tests one more time.
-for qtype in "DNSKEY" "CDNSKEY" "CDS"
-do
-  echo_i "checking $qtype RRset is signed with KSK only, new ZSK active (update-check-ksk, dnssec-ksk-only) ($n)"
-  ret=0
-  dig_with_opts $SECTIONS @10.53.0.2 $qtype $zone > dig.out.test$n
-  lines=$(awk -v qt="$qtype" '$4 == "RRSIG" && $5 == qt {print}' dig.out.test$n | wc -l)
-  test "$lines" -eq 1 || ret=1
-  grep $KSK_ID  dig.out.test$n > /dev/null || ret=1
-  grep $ZSK_ID  dig.out.test$n > /dev/null && ret=1
-  grep $ZSK_ID2 dig.out.test$n > /dev/null && ret=1
-  grep $ZSK_ID3 dig.out.test$n > /dev/null && ret=1
-  n=$((n+1))
-  test "$ret" -eq 0 || echo_i "failed"
-  status=$((status+ret))
-done
+echo_i "checking DNSKEY RRset is signed with KSK only, new ZSK active (update-check-ksk, dnssec-ksk-only) ($n)"
+ret=0
+$DIG $DIGOPTS $SECTIONS @10.53.0.2 DNSKEY $zone > dig.out.test$n
+lines=$(awk '$4 == "RRSIG" && $5 == "DNSKEY" {print}' dig.out.test$n | wc -l)
+test "$lines" -eq 1 || ret=1
+grep $KSK_ID  dig.out.test$n > /dev/null || ret=1
+grep $ZSK_ID  dig.out.test$n > /dev/null && ret=1
+grep $ZSK_ID2 dig.out.test$n > /dev/null && ret=1
+grep $ZSK_ID3 dig.out.test$n > /dev/null && ret=1
+n=$((n+1))
+test "$ret" -eq 0 || echo_i "failed"
+status=$((status+ret))
 
 echo_i "exit status: $status"
 [ $status -eq 0 ] || exit 1