From fa2e4e66b04dfdc2fccfdacaa6ab7cbded035586 Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Thu, 3 Dec 2020 08:53:34 +0100
Subject: [PATCH 1/9] Add tests for going from secure to insecure

Add two test zones that will be reconfigured to go insecure, by
setting the 'dnssec-policy' option to 'none'.

One zone was using inline-signing (implicitly through dnssec-policy),
the other is a dynamic zone.

Two tweaks to the kasp system test are required: we need to set
when to except the CDS/CDS Delete Records, and we need to know
when we are dealing with a dynamic zone (because the logs to look for
are slightly different, inline-signing prints "(signed)" after the
zone name, dynamic zones do not).
---
 bin/tests/system/kasp/ns6/named.conf.in  |  14 ++
 bin/tests/system/kasp/ns6/named2.conf.in |  27 +++
 bin/tests/system/kasp/ns6/setup.sh       |  40 ++++
 bin/tests/system/kasp/tests.sh           | 265 ++++++++++++++++++++++-
 4 files changed, 344 insertions(+), 2 deletions(-)

diff --git a/bin/tests/system/kasp/ns6/named.conf.in b/bin/tests/system/kasp/ns6/named.conf.in
index 5053329e4e..6b08497b0a 100644
--- a/bin/tests/system/kasp/ns6/named.conf.in
+++ b/bin/tests/system/kasp/ns6/named.conf.in
@@ -64,6 +64,20 @@ zone "migrate-nomatch-alglen.kasp" {
 	update-check-ksk yes;
 };
 
+/* These zones are going insecure. */
+zone "step1.going-insecure.kasp" {
+	type master;
+	file "step1.going-insecure.kasp.db";
+	dnssec-policy "migrate";
+};
+
+zone "step1.going-insecure-dynamic.kasp" {
+	type master;
+	file "step1.going-insecure-dynamic.kasp.db";
+	dnssec-policy "migrate";
+	allow-update { any; };
+};
+
 /* These are alorithm rollover test zones. */
 zone "step1.algorithm-roll.kasp" {
 	type primary;
diff --git a/bin/tests/system/kasp/ns6/named2.conf.in b/bin/tests/system/kasp/ns6/named2.conf.in
index d07ee9eb6e..f0edd67b2a 100644
--- a/bin/tests/system/kasp/ns6/named2.conf.in
+++ b/bin/tests/system/kasp/ns6/named2.conf.in
@@ -57,6 +57,33 @@ zone "migrate-nomatch-alglen.kasp" {
 	dnssec-policy "migrate-nomatch-alglen";
 };
 
+/* Zones for testing going insecure. */
+zone "step1.going-insecure.kasp" {
+        type master;
+        file "step1.going-insecure.kasp.db";
+        dnssec-policy "none";
+};
+
+zone "step2.going-insecure.kasp" {
+        type master;
+        file "step2.going-insecure.kasp.db";
+        dnssec-policy "none";
+};
+
+zone "step1.going-insecure-dynamic.kasp" {
+        type master;
+        file "step1.going-insecure-dynamic.kasp.db";
+        dnssec-policy "none";
+	allow-update { any; };
+};
+
+zone "step2.going-insecure-dynamic.kasp" {
+        type master;
+        file "step2.going-insecure-dynamic.kasp.db";
+        dnssec-policy "none";
+	allow-update { any; };
+};
+
 /*
  * Zones for testing KSK/ZSK algorithm roll.
  */
diff --git a/bin/tests/system/kasp/ns6/setup.sh b/bin/tests/system/kasp/ns6/setup.sh
index 7ae089f8d9..99d842eaa5 100644
--- a/bin/tests/system/kasp/ns6/setup.sh
+++ b/bin/tests/system/kasp/ns6/setup.sh
@@ -86,6 +86,46 @@ private_type_record $zone 5 "$KSK" >> "$infile"
 private_type_record $zone 5 "$ZSK" >> "$infile"
 $SIGNER -S -x -s now-1h -e now+2w -o $zone -O full -f $zonefile $infile > signer.out.$zone.1 2>&1
 
+# The child zones (step1, step2) beneath these zones represent the various
+# steps of unsigning a zone.
+for zn in going-insecure.kasp going-insecure-dynamic.kasp
+do
+	# Step 1:
+	# Set up a zone with dnssec-policy that is going insecure.
+	setup step1.$zn
+	echo "$zone" >> zones
+	T="now-10d"
+	ksktimes="-P $T -A $T -P sync $T"
+	zsktimes="-P $T -A $T"
+	KSK=$($KEYGEN -a $DEFAULT_ALGORITHM -L 7200 -f KSK $ksktimes $zone 2> keygen.out.$zone.1)
+	ZSK=$($KEYGEN -a $DEFAULT_ALGORITHM -L 7200        $zsktimes $zone 2> keygen.out.$zone.2)
+	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"
+	$SIGNER -S -x -s now-1h -e now+2w -o $zone -O full -f $zonefile $infile > signer.out.$zone.1 2>&1
+
+	# Step 2:
+	# Set up a zone with dnssec-policy that is going insecure. Don't add
+	# this zone to the zones file, because this zone is no longer expected
+	# to be fully signed.
+	setup step2.$zn
+	# The DS was withdrawn from the parent zone 26 hours ago.
+	Trem="now-26h"
+	ksktimes="-P $T -A $T -P sync $T"
+	zsktimes="-P $T -A $T"
+	KSK=$($KEYGEN -a $DEFAULT_ALGORITHM -L 7200 -f KSK $ksktimes $zone 2> keygen.out.$zone.1)
+	ZSK=$($KEYGEN -a $DEFAULT_ALGORITHM -L 7200        $zsktimes $zone 2> keygen.out.$zone.2)
+	$SETTIME -s -g $H -k $O $T -r $O $T -d $U $Trem -D ds $Trem "$KSK" > settime.out.$zone.1 2>&1
+	$SETTIME -s -g $H -k $O $T -z $O $T                         "$ZSK" > settime.out.$zone.2 2>&1
+	# Fake lifetime of old algorithm keys.
+	echo "Lifetime: 0" >> "${KSK}.state"
+	echo "Lifetime: 5184000" >> "${ZSK}.state"
+	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"
+	$SIGNER -S -x -s now-1h -e now+2w -o $zone -O full -f $zonefile $infile > signer.out.$zone.1 2>&1
+done
+
 #
 # The zones at algorithm-roll.kasp represent the various steps of a ZSK/KSK
 # algorithm rollover.
diff --git a/bin/tests/system/kasp/tests.sh b/bin/tests/system/kasp/tests.sh
index b8f623b2cf..d22ec2660b 100644
--- a/bin/tests/system/kasp/tests.sh
+++ b/bin/tests/system/kasp/tests.sh
@@ -172,12 +172,26 @@ set_server() {
 # Set zone name for testing keys.
 set_zone() {
 	ZONE=$1
+	DYNAMIC="no"
 }
+# By default zones are considered static.
+# When testing dynamic zones, call 'set_dynamic' after 'set_zone'.
+set_dynamic() {
+	DYNAMIC="yes"
+}
+
 # Set policy settings (name $1, number of keys $2, dnskey ttl $3) for testing keys.
 set_policy() {
 	POLICY=$1
 	NUM_KEYS=$2
 	DNSKEY_TTL=$3
+	CDS_DELETE="no"
+}
+# By default policies are considered to be secure.
+# If a zone sets its policy to "none", call 'set_cdsdelete' to tell the system
+# test to expect a CDS and CDNSKEY Delete record.
+set_cdsdelete() {
+	CDS_DELETE="yes"
 }
 
 # Set key properties for testing keys.
@@ -1044,6 +1058,14 @@ check_cds() {
 	dig_with_opts "$ZONE" "@${SERVER}" "CDNSKEY" > "dig.out.$DIR.test$n.cdnskey" || log_error "dig ${ZONE} CDNSKEY failed"
 	grep "status: NOERROR" "dig.out.$DIR.test$n.cdnskey" > /dev/null || log_error "mismatch status in DNS response"
 
+	if [ "$CDS_DELETE" = "no" ]; then
+		grep "CDS.*0 0 0 00" "dig.out.$DIR.test$n.cds" > /dev/null && log_error "unexpected CDS DELETE record in DNS response"
+		grep "CDNSKEY.*0 3 0 AA==" "dig.out.$DIR.test$n.cdnskey" > /dev/null && log_error "unexpected CDNSKEY DELETE record in DNS response"
+	else
+		grep "CDS.*0 0 0 00" "dig.out.$DIR.test$n.cds" > /dev/null || log_error "missing CDS DELETE record in DNS response"
+		grep "CDNSKEY.*0 3 0 AA==" "dig.out.$DIR.test$n.cdnskey" > /dev/null || log_error "missing CDNSKEY DELETE record in DNS response"
+	fi
+
 	if [ "$(key_get KEY1 STATE_DS)" = "rumoured" ] || [ "$(key_get KEY1 STATE_DS)" = "omnipresent" ]; then
 		response_has_cds_for_key KEY1 "dig.out.$DIR.test$n.cds" || log_error "missing CDS record in response for key $(key_get KEY1 ID)"
 		check_signatures "CDS" "dig.out.$DIR.test$n.cds" "KSK"
@@ -1210,7 +1232,13 @@ _loadkeys_on() {
 
 	nextpart $_dir/named.run > /dev/null
 	rndccmd $_server loadkeys $_zone in $_view > rndc.dnssec.loadkeys.out.$_zone.$n
-	wait_for_log 20 "zone ${_zone}/IN (signed): next key event" $_dir/named.run || return 1
+
+	if [ "${DYNAMIC}" = "yes" ]; then
+		wait_for_log 20 "zone ${_zone}/IN: next key event" $_dir/named.run || return 1
+	else
+		# inline-signing zone adds "(signed)"
+		wait_for_log 20 "zone ${_zone}/IN (signed): next key event" $_dir/named.run || return 1
+	fi
 }
 
 # Tell named that the DS for the key in given zone has been seen in the
@@ -1346,6 +1374,7 @@ status=$((status+ret))
 # Zone: dynamic.kasp
 #
 set_zone "dynamic.kasp"
+set_dynamic
 set_policy "default" "1" "3600"
 set_server "ns3" "10.53.0.3"
 # Key properties, timings and states same as above.
@@ -1378,6 +1407,7 @@ status=$((status+ret))
 # Zone: dynamic-inline-signing.kasp
 #
 set_zone "dynamic-inline-signing.kasp"
+set_dynamic
 set_policy "default" "1" "3600"
 set_server "ns3" "10.53.0.3"
 # Key properties, timings and states same as above.
@@ -2937,7 +2967,12 @@ check_next_key_event() {
 	grep "zone ${ZONE}.*: next key event in .* seconds" "${DIR}/named.run" > "keyevent.out.$ZONE.test$n" || log_error "no next key event for zone ${ZONE}"
 
 	# Get the latest next key event.
-	_time=$(awk '{print $10}' < "keyevent.out.$ZONE.test$n" | tail -1)
+	if [ "${DYNAMIC}" = "yes" ]; then
+		_time=$(awk '{print $9}' < "keyevent.out.$ZONE.test$n" | tail -1)
+	else
+		# inline-signing zone adds "(signed)"
+		_time=$(awk '{print $10}' < "keyevent.out.$ZONE.test$n" | tail -1)
+	fi
 
 	# The next key event time must within threshold of the
 	# expected time.
@@ -4452,6 +4487,90 @@ dnssec_verify
 _migratenomatch_alglen_ksk=$(key_get KEY1 ID)
 _migratenomatch_alglen_zsk=$(key_get KEY2 ID)
 
+#
+# Testing going insecure.
+#
+
+#
+# Zone step1.going-insecure.kasp
+#
+set_zone "step1.going-insecure.kasp"
+set_policy "migrate" "2" "7200"
+set_server "ns6" "10.53.0.6"
+
+# Policy parameters.
+# Lksk:      0
+# Lzsk:      60 days (5184000 seconds)
+# Iret(KSK): DS TTL (1d) + DprpP (1h) + retire-safety (1h)
+# Iret(KSK): 1d2h (93600 seconds)
+# Iret(ZSK): RRSIG TTL (1d) + Dprp (5m) + Dsgn (9d) + retire-safety (1h)
+# Iret(ZSK): 10d1h5m (867900 seconds)
+Lksk=0
+Lzsk=5184000
+IretKSK=93600
+IretZSK=867900
+
+init_migration_insecure() {
+	key_clear        "KEY1"
+	set_keyrole      "KEY1" "ksk"
+	set_keylifetime  "KEY1" "${Lksk}"
+	set_keyalgorithm "KEY1" "$DEFAULT_ALGORITHM_NUMBER" "$DEFAULT_ALGORITHM" "$DEFAULT_BITS"
+	set_keysigning   "KEY1" "yes"
+	set_zonesigning  "KEY1" "no"
+
+	set_keystate "KEY1" "GOAL"         "omnipresent"
+	set_keystate "KEY1" "STATE_DNSKEY" "omnipresent"
+	set_keystate "KEY1" "STATE_KRRSIG" "omnipresent"
+	set_keystate "KEY1" "STATE_DS"     "omnipresent"
+
+	key_clear        "KEY2"
+	set_keyrole      "KEY2" "zsk"
+	set_keylifetime  "KEY2" "${Lzsk}"
+	set_keyalgorithm "KEY2" "$DEFAULT_ALGORITHM_NUMBER" "$DEFAULT_ALGORITHM" "$DEFAULT_BITS"
+	set_keysigning   "KEY2" "no"
+	set_zonesigning  "KEY2" "yes"
+
+	set_keystate "KEY2" "GOAL"         "omnipresent"
+	set_keystate "KEY2" "STATE_DNSKEY" "omnipresent"
+	set_keystate "KEY2" "STATE_ZRRSIG" "omnipresent"
+
+	key_clear "KEY3"
+	key_clear "KEY4"
+}
+init_migration_insecure
+
+# Various signing policy checks.
+check_keys
+check_dnssecstatus "$SERVER" "$POLICY" "$ZONE"
+
+# We have set the timing metadata to now - 10 days (864000 seconds).
+rollover_predecessor_keytimes -864000
+check_keytimes
+check_apex
+check_subdomain
+dnssec_verify
+
+#
+# Zone step1.going-insecure-dynamic.kasp
+#
+
+set_zone "step1.going-insecure-dynamic.kasp"
+set_dynamic
+set_policy "migrate" "2" "7200"
+set_server "ns6" "10.53.0.6"
+init_migration_insecure
+
+# Various signing policy checks.
+check_keys
+check_dnssecstatus "$SERVER" "$POLICY" "$ZONE"
+
+# We have set the timing metadata to now - 10 days (864000 seconds).
+rollover_predecessor_keytimes -864000
+check_keytimes
+check_apex
+check_subdomain
+dnssec_verify
+
 # Reconfig dnssec-policy (triggering algorithm roll and other dnssec-policy
 # changes).
 echo_i "reconfig dnssec-policy to trigger algorithm rollover"
@@ -4501,6 +4620,148 @@ wait_for_done_signing() {
 	status=$((status+ret))
 }
 
+#
+# Testing going insecure.
+#
+
+#
+# Zone: step1.going-insecure.kasp
+#
+set_zone "step1.going-insecure.kasp"
+set_policy "none" "2" "7200"
+set_server "ns6" "10.53.0.6"
+# Expect a CDS/CDNSKEY Delete Record.
+set_cdsdelete
+
+# Key goal states should be HIDDEN.
+init_migration_insecure
+set_keystate "KEY1" "GOAL" "hidden"
+set_keystate "KEY2" "GOAL" "hidden"
+# The DS may be removed if we are going insecure.
+set_keystate "KEY1" "STATE_DS" "unretentive"
+
+# Various signing policy checks.
+check_keys
+wait_for_done_signing
+check_dnssecstatus "$SERVER" "$POLICY" "$ZONE"
+check_apex
+check_subdomain
+dnssec_verify
+
+# Tell named that the DS has been removed.
+rndc_checkds "$SERVER" "$DIR" "KEY1" "now" "withdrawn" "$ZONE"
+wait_for_done_signing
+check_dnssecstatus "$SERVER" "$POLICY" "$ZONE"
+check_apex
+check_subdomain
+dnssec_verify
+
+# Next key event is when the DS becomes HIDDEN. This happens after the
+# parent propagation delay, retire safety delay, and DS TTL:
+# 1h + 1h + 1d = 26h = 93600 seconds.
+check_next_key_event 93600
+
+#
+# Zone: step2.going-insecure.kasp
+#
+set_zone "step2.going-insecure.kasp"
+set_policy "none" "2" "7200"
+set_server "ns6" "10.53.0.6"
+# Expect a CDS/CDNSKEY Delete Record.
+set_cdsdelete
+
+# The DS is long enough removed from the zone to be considered HIDDEN.
+# This means the DNSKEY and the KSK signatures can be removed.
+set_keystate     "KEY1" "STATE_DS"     "hidden"
+set_keystate     "KEY1" "STATE_DNSKEY" "unretentive"
+set_keystate     "KEY1" "STATE_KRRSIG" "unretentive"
+set_keysigning   "KEY1" "no"
+
+set_keystate     "KEY2" "STATE_DNSKEY" "unretentive"
+set_keystate     "KEY2" "STATE_ZRRSIG" "unretentive"
+set_zonesigning  "KEY2" "no"
+
+# Various signing policy checks.
+check_keys
+check_dnssecstatus "$SERVER" "$POLICY" "$ZONE"
+check_apex
+check_subdomain
+
+# Next key event is when the DNSKEY becomes HIDDEN. This happens after the
+# propagation delay, plus DNSKEY TTL:
+# 5m + 2h = 125m =  7500 seconds.
+check_next_key_event 7500
+
+#
+# Zone: step1.going-insecure-dynamic.kasp
+#
+set_zone "step1.going-insecure-dynamic.kasp"
+set_dynamic
+set_policy "none" "2" "7200"
+set_server "ns6" "10.53.0.6"
+# Expect a CDS/CDNSKEY Delete Record.
+set_cdsdelete
+
+# Key goal states should be HIDDEN.
+init_migration_insecure
+set_keystate "KEY1" "GOAL" "hidden"
+set_keystate "KEY2" "GOAL" "hidden"
+# The DS may be removed if we are going insecure.
+set_keystate "KEY1" "STATE_DS" "unretentive"
+
+# Various signing policy checks.
+check_keys
+wait_for_done_signing
+check_dnssecstatus "$SERVER" "$POLICY" "$ZONE"
+check_apex
+check_subdomain
+dnssec_verify
+
+# Tell named that the DS has been removed.
+rndc_checkds "$SERVER" "$DIR" "KEY1" "now" "withdrawn" "$ZONE"
+wait_for_done_signing
+check_dnssecstatus "$SERVER" "$POLICY" "$ZONE"
+check_apex
+check_subdomain
+dnssec_verify
+
+# Next key event is when the DS becomes HIDDEN. This happens after the
+# parent propagation delay, retire safety delay, and DS TTL:
+# 1h + 1h + 1d = 26h = 93600 seconds.
+check_next_key_event 93600
+
+#
+# Zone: step2.going-insecure-dynamic.kasp
+#
+set_zone "step2.going-insecure-dynamic.kasp"
+set_dynamic
+set_policy "none" "2" "7200"
+set_server "ns6" "10.53.0.6"
+# Expect a CDS/CDNSKEY Delete Record.
+set_cdsdelete
+
+# The DS is long enough removed from the zone to be considered HIDDEN.
+# This means the DNSKEY and the KSK signatures can be removed.
+set_keystate     "KEY1" "STATE_DS"     "hidden"
+set_keystate     "KEY1" "STATE_DNSKEY" "unretentive"
+set_keystate     "KEY1" "STATE_KRRSIG" "unretentive"
+set_keysigning   "KEY1" "no"
+
+set_keystate     "KEY2" "STATE_DNSKEY" "unretentive"
+set_keystate     "KEY2" "STATE_ZRRSIG" "unretentive"
+set_zonesigning  "KEY2" "no"
+
+# Various signing policy checks.
+check_keys
+check_dnssecstatus "$SERVER" "$POLICY" "$ZONE"
+check_apex
+check_subdomain
+
+# Next key event is when the DNSKEY becomes HIDDEN. This happens after the
+# propagation delay, plus DNSKEY TTL:
+# 5m + 2h = 125m =  7500 seconds.
+check_next_key_event 7500
+
 #
 # Testing migration.
 #

From 756674f6d1e62e4f5eabbb71e80a25974a32783c Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Thu, 3 Dec 2020 09:03:21 +0100
Subject: [PATCH 2/9] Small adjustments to kasp rndc_checkds function

Slightly better test output, and only call 'load keys' if the
'rndc checkds' call succeeded.
---
 bin/tests/system/kasp/tests.sh | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/bin/tests/system/kasp/tests.sh b/bin/tests/system/kasp/tests.sh
index d22ec2660b..12b3fa9e0e 100644
--- a/bin/tests/system/kasp/tests.sh
+++ b/bin/tests/system/kasp/tests.sh
@@ -1256,21 +1256,23 @@ rndc_checkds() {
 	_keycmd=""
 	if [ "${_key}" != "-" ]; then
 		_keyid=$(key_get $_key ID)
-		_keycmd="-key ${_keyid}"
+		_keycmd=" -key ${_keyid}"
 	fi
 
 	_whencmd=""
 	if [ "${_when}" != "now" ]; then
-		_whencmd="-when ${_when}"
+		_whencmd=" -when ${_when}"
 	fi
 
 	n=$((n+1))
-	echo_i "calling rndc dnssec -checkds ${_keycmd} ${_whencmd} ${_what} zone ${_zone} ($n)"
+	echo_i "calling rndc dnssec -checkds${_keycmd}${_whencmd} ${_what} zone ${_zone} in ${_view} ($n)"
 	ret=0
 
-	rndccmd $_server dnssec -checkds $_keycmd $_whencmd $_what $_zone in $_view > rndc.dnssec.checkds.out.$_zone.$n || log_error "rndc dnssec -checkds (${_keycmd} ${_whencmd} ${_what} zone ${_zone} failed"
+	rndccmd $_server dnssec -checkds $_keycmd $_whencmd $_what $_zone in $_view > rndc.dnssec.checkds.out.$_zone.$n || log_error "rndc dnssec -checkds${_keycmd}${_whencmd} ${_what} zone ${_zone} failed"
 
-	_loadkeys_on $_server $_dir $_zone || log_error "loadkeys zone ${_zone} failed ($n)"
+	if [ "$ret" -eq 0 ]; then
+		 _loadkeys_on $_server $_dir $_zone || log_error "loadkeys zone ${_zone} failed ($n)"
+	fi
 
 	test "$ret" -eq 0 || echo_i "failed"
 	status=$((status+ret))

From 8f2c5e45da47394c812f5499b2766b13387c7bbc Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Thu, 3 Dec 2020 10:19:38 +0100
Subject: [PATCH 3/9] Add function to see if dst key uses kasp

For purposes of zones transitioning back to insecure mode, it is
practical to see if related keys have a state file associated.
---
 lib/dns/dst_api.c           | 15 +++++++++++++--
 lib/dns/dst_internal.h      |  1 +
 lib/dns/include/dst/dst.h   |  9 +++++++++
 lib/dns/win32/libdns.def.in |  1 +
 4 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/lib/dns/dst_api.c b/lib/dns/dst_api.c
index 215a133a69..a501d0be11 100644
--- a/lib/dns/dst_api.c
+++ b/lib/dns/dst_api.c
@@ -649,12 +649,14 @@ dst_key_fromnamedfile(const char *filename, const char *dirname, int type,
 				   filename, ".state");
 		INSIST(result == ISC_R_SUCCESS);
 
+		key->kasp = false;
 		result = dst_key_read_state(newfilename, mctx, &key);
-		if (result == ISC_R_FILENOTFOUND) {
+		if (result == ISC_R_SUCCESS) {
+			key->kasp = true;
+		} else if (result == ISC_R_FILENOTFOUND) {
 			/* Having no state is valid. */
 			result = ISC_R_SUCCESS;
 		}
-
 		isc_mem_put(mctx, newfilename, newfilenamelen);
 		newfilename = NULL;
 		RETERR(result);
@@ -2600,6 +2602,8 @@ dst_key_goal(dst_key_t *key) {
 	dst_key_state_t state;
 	isc_result_t result;
 
+	REQUIRE(VALID_KEY(key));
+
 	result = dst_key_getstate(key, DST_KEY_GOAL, &state);
 	if (result == ISC_R_SUCCESS) {
 		return (state);
@@ -2607,6 +2611,13 @@ dst_key_goal(dst_key_t *key) {
 	return (DST_KEY_STATE_HIDDEN);
 }
 
+bool
+dst_key_haskasp(dst_key_t *key) {
+	REQUIRE(VALID_KEY(key));
+
+	return (key->kasp);
+}
+
 void
 dst_key_copy_metadata(dst_key_t *to, dst_key_t *from) {
 	dst_key_state_t state;
diff --git a/lib/dns/dst_internal.h b/lib/dns/dst_internal.h
index b40107880d..43ab84ab15 100644
--- a/lib/dns/dst_internal.h
+++ b/lib/dns/dst_internal.h
@@ -123,6 +123,7 @@ struct dst_key {
 	bool keystateset[DST_MAX_KEYSTATES + 1];	  /*%< data
 							   * set? */
 
+	bool kasp;     /*%< key has kasp state */
 	bool inactive; /*%< private key not present as it is
 			* inactive */
 	bool external; /*%< external key */
diff --git a/lib/dns/include/dst/dst.h b/lib/dns/include/dst/dst.h
index 5ae0f87be9..010bc99a39 100644
--- a/lib/dns/include/dst/dst.h
+++ b/lib/dns/include/dst/dst.h
@@ -1103,6 +1103,15 @@ dst_key_isexternal(dst_key_t *key);
  *	'key' to be valid.
  */
 
+bool
+dst_key_haskasp(dst_key_t *key);
+/*%<
+ * Check if this key has state (and thus uses KASP).
+ *
+ * Requires:
+ *	'key' to be valid.
+ */
+
 bool
 dst_key_is_unused(dst_key_t *key);
 /*%<
diff --git a/lib/dns/win32/libdns.def.in b/lib/dns/win32/libdns.def.in
index 4f88cbb98f..6e723fe2b8 100644
--- a/lib/dns/win32/libdns.def.in
+++ b/lib/dns/win32/libdns.def.in
@@ -1452,6 +1452,7 @@ dst_key_getstate
 dst_key_gettime
 dst_key_getttl
 dst_key_goal
+dst_key_haskasp
 dst_key_id
 dst_key_is_active
 dst_key_is_published

From cf420b2af0d45693d0f5f34d9113ea411b5f2225 Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Thu, 3 Dec 2020 15:01:42 +0100
Subject: [PATCH 4/9] Treat dnssec-policy "none" as a builtin zone

Configure "none" as a builtin policy. Change the 'cfg_kasp_fromconfig'
api so that the 'name' will determine what policy needs to be
configured.

When transitioning a zone from secure to insecure, there will be
cases when a zone with no DNSSEC policy (dnssec-policy none) should
be using KASP. When there are key state files available, this is an
indication that the zone once was DNSSEC signed but is reconfigured
to become insecure.

If we would not run the keymgr, named would abruptly remove the
DNSSEC records from the zone, making the zone bogus. Therefore,
change the code such that a zone will use kasp if there is a valid
dnssec-policy configured, or if there are state files available.
---
 bin/dnssec/dnssec-keygen.c           |   8 +-
 bin/named/server.c                   |  21 ++--
 bin/named/zoneconf.c                 | 132 +++++++++++++------------
 bin/tests/system/kasp/tests.sh       |  28 +++---
 lib/bind9/check.c                    |   6 +-
 lib/dns/include/dns/zone.h           |  26 +++++
 lib/dns/update.c                     |   7 +-
 lib/dns/win32/libdns.def.in          |   2 +
 lib/dns/zone.c                       | 138 ++++++++++++++++++++++-----
 lib/isccfg/include/isccfg/kaspconf.h |  17 ++--
 lib/isccfg/kaspconf.c                |  32 ++++---
 11 files changed, 277 insertions(+), 140 deletions(-)

diff --git a/bin/dnssec/dnssec-keygen.c b/bin/dnssec/dnssec-keygen.c
index f16d98257b..349aa00f43 100644
--- a/bin/dnssec/dnssec-keygen.c
+++ b/bin/dnssec/dnssec-keygen.c
@@ -269,8 +269,8 @@ kasp_from_conf(cfg_obj_t *config, isc_mem_t *mctx, const char *name,
 			continue;
 		}
 
-		result = cfg_kasp_fromconfig(kconfig, mctx, lctx, &kasplist,
-					     &kasp);
+		result = cfg_kasp_fromconfig(kconfig, NULL, mctx, lctx,
+					     &kasplist, &kasp);
 		if (result != ISC_R_SUCCESS) {
 			fatal("failed to configure dnssec-policy '%s': %s",
 			      cfg_obj_asstring(cfg_tuple_get(kconfig, "name")),
@@ -284,7 +284,7 @@ kasp_from_conf(cfg_obj_t *config, isc_mem_t *mctx, const char *name,
 	*kaspp = kasp;
 
 	/*
-	 * Same cleanup for kasp list.
+	 * Cleanup kasp list.
 	 */
 	for (kasp = ISC_LIST_HEAD(kasplist); kasp != NULL; kasp = kasp_next) {
 		kasp_next = ISC_LIST_NEXT(kasp, link);
@@ -782,7 +782,7 @@ keygen(keygen_ctx_t *ctx, isc_mem_t *mctx, int argc, char **argv) {
 		}
 
 		/* Set dnssec-policy related metadata */
-		if (ctx->policy) {
+		if (ctx->policy != NULL) {
 			dst_key_setnum(key, DST_NUM_LIFETIME, ctx->lifetime);
 			dst_key_setbool(key, DST_BOOL_KSK, ctx->ksk);
 			dst_key_setbool(key, DST_BOOL_ZSK, ctx->zsk);
diff --git a/bin/named/server.c b/bin/named/server.c
index 5ce695b5ee..839438161c 100644
--- a/bin/named/server.c
+++ b/bin/named/server.c
@@ -9121,18 +9121,25 @@ load_configuration(const char *filename, named_server_t *server,
 	{
 		cfg_obj_t *kconfig = cfg_listelt_value(element);
 		kasp = NULL;
-		CHECK(cfg_kasp_fromconfig(kconfig, named_g_mctx, named_g_lctx,
-					  &kasplist, &kasp));
+		CHECK(cfg_kasp_fromconfig(kconfig, NULL, named_g_mctx,
+					  named_g_lctx, &kasplist, &kasp));
 		INSIST(kasp != NULL);
 		dns_kasp_freeze(kasp);
 		dns_kasp_detach(&kasp);
 	}
 	/*
-	 * Create the default kasp.
+	 * Create the built-in kasp policies ("default", "none").
 	 */
 	kasp = NULL;
-	CHECK(cfg_kasp_fromconfig(NULL, named_g_mctx, named_g_lctx, &kasplist,
-				  &kasp));
+	CHECK(cfg_kasp_fromconfig(NULL, "default", named_g_mctx, named_g_lctx,
+				  &kasplist, &kasp));
+	INSIST(kasp != NULL);
+	dns_kasp_freeze(kasp);
+	dns_kasp_detach(&kasp);
+
+	kasp = NULL;
+	CHECK(cfg_kasp_fromconfig(NULL, "none", named_g_mctx, named_g_lctx,
+				  &kasplist, &kasp));
 	INSIST(kasp != NULL);
 	dns_kasp_freeze(kasp);
 	dns_kasp_detach(&kasp);
@@ -14528,7 +14535,6 @@ named_server_signing(named_server_t *server, isc_lex_t *lex,
 		     isc_buffer_t **text) {
 	isc_result_t result = ISC_R_SUCCESS;
 	dns_zone_t *zone = NULL;
-	dns_kasp_t *kasp = NULL;
 	dns_name_t *origin;
 	dns_db_t *db = NULL;
 	dns_dbnode_t *node = NULL;
@@ -14647,8 +14653,7 @@ named_server_signing(named_server_t *server, isc_lex_t *lex,
 		CHECK(ISC_R_UNEXPECTEDEND);
 	}
 
-	kasp = dns_zone_getkasp(zone);
-	if (kasp != NULL) {
+	if (dns_zone_use_kasp(zone)) {
 		(void)putstr(text, "zone uses dnssec-policy, use rndc dnssec "
 				   "command instead");
 		(void)putnull(text);
diff --git a/bin/named/zoneconf.c b/bin/named/zoneconf.c
index 9102367470..2765b77e69 100644
--- a/bin/named/zoneconf.c
+++ b/bin/named/zoneconf.c
@@ -913,6 +913,7 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig,
 	bool check = false, fail = false;
 	bool warn = false, ignore = false;
 	bool ixfrdiff;
+	bool use_kasp = false;
 	dns_masterformat_t masterformat;
 	const dns_master_style_t *masterstyle = &dns_master_style_default;
 	isc_stats_t *zoneqrystats;
@@ -1248,19 +1249,15 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig,
 		result = named_config_get(maps, "dnssec-policy", &obj);
 		if (result == ISC_R_SUCCESS) {
 			kaspname = cfg_obj_asstring(obj);
-			if (strcmp(kaspname, "none") != 0) {
-				result = dns_kasplist_find(kasplist, kaspname,
-							   &kasp);
-				if (result != ISC_R_SUCCESS) {
-					cfg_obj_log(obj, named_g_lctx,
-						    ISC_LOG_ERROR,
-						    "'dnssec-policy '%s' not "
-						    "found ",
-						    kaspname);
-					RETERR(result);
-				}
-				dns_zone_setkasp(zone, kasp);
+			result = dns_kasplist_find(kasplist, kaspname, &kasp);
+			if (result != ISC_R_SUCCESS) {
+				cfg_obj_log(obj, named_g_lctx, ISC_LOG_ERROR,
+					    "'dnssec-policy '%s' not found ",
+					    kaspname);
+				RETERR(result);
 			}
+			dns_zone_setkasp(zone, kasp);
+			use_kasp = dns_zone_use_kasp(zone);
 		}
 
 		obj = NULL;
@@ -1561,7 +1558,7 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig,
 		bool allow = false, maint = false;
 		bool sigvalinsecs;
 
-		if (kasp != NULL) {
+		if (use_kasp) {
 			if (dns_kasp_nsec3(kasp)) {
 				result = dns_zone_setnsec3param(
 					zone, 1, dns_kasp_nsec3flags(kasp),
@@ -1575,7 +1572,7 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig,
 			INSIST(result == ISC_R_SUCCESS);
 		}
 
-		if (kasp != NULL) {
+		if (use_kasp) {
 			seconds = (uint32_t)dns_kasp_sigvalidity_dnskey(kasp);
 		} else {
 			obj = NULL;
@@ -1586,7 +1583,7 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig,
 		}
 		dns_zone_setkeyvalidityinterval(zone, seconds);
 
-		if (kasp != NULL) {
+		if (use_kasp) {
 			seconds = (uint32_t)dns_kasp_sigvalidity(kasp);
 			dns_zone_setsigvalidityinterval(zone, seconds);
 			seconds = (uint32_t)dns_kasp_sigrefresh(kasp);
@@ -1673,7 +1670,8 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig,
 
 		obj = NULL;
 		result = cfg_map_get(zoptions, "auto-dnssec", &obj);
-		if (dns_zone_getkasp(zone) != NULL) {
+		if (kasp != NULL && strcmp(dns_kasp_getname(kasp), "none") != 0)
+		{
 			dns_zone_setkeyopt(zone, DNS_ZONEKEY_ALLOW, true);
 			dns_zone_setkeyopt(zone, DNS_ZONEKEY_CREATE, true);
 			dns_zone_setkeyopt(zone, DNS_ZONEKEY_MAINTAIN, true);
@@ -1692,6 +1690,11 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig,
 			dns_zone_setkeyopt(zone, DNS_ZONEKEY_ALLOW, allow);
 			dns_zone_setkeyopt(zone, DNS_ZONEKEY_CREATE, false);
 			dns_zone_setkeyopt(zone, DNS_ZONEKEY_MAINTAIN, maint);
+		} else {
+			bool s2i = dns_zone_secure_to_insecure(zone, false);
+			dns_zone_setkeyopt(zone, DNS_ZONEKEY_ALLOW, s2i);
+			dns_zone_setkeyopt(zone, DNS_ZONEKEY_CREATE, false);
+			dns_zone_setkeyopt(zone, DNS_ZONEKEY_MAINTAIN, s2i);
 		}
 	}
 
@@ -2139,11 +2142,15 @@ named_zone_reusable(dns_zone_t *zone, const cfg_obj_t *zconfig,
 bool
 named_zone_inlinesigning(dns_zone_t *zone, const cfg_obj_t *zconfig,
 			 const cfg_obj_t *vconfig, const cfg_obj_t *config) {
+	isc_result_t res;
 	const cfg_obj_t *zoptions = NULL;
 	const cfg_obj_t *voptions = NULL;
 	const cfg_obj_t *options = NULL;
 	const cfg_obj_t *signing = NULL;
-	bool inline_signing;
+	const cfg_obj_t *allowupdate = NULL;
+	const cfg_obj_t *updatepolicy = NULL;
+	bool zone_is_dynamic = false;
+	bool inline_signing = false;
 
 	(void)cfg_map_get(config, "options", &options);
 
@@ -2155,6 +2162,38 @@ named_zone_inlinesigning(dns_zone_t *zone, const cfg_obj_t *zconfig,
 	inline_signing = (cfg_map_get(zoptions, "inline-signing", &signing) ==
 				  ISC_R_SUCCESS &&
 			  cfg_obj_asboolean(signing));
+	if (inline_signing) {
+		return (true);
+	}
+
+	if (cfg_map_get(zoptions, "update-policy", &updatepolicy) ==
+	    ISC_R_SUCCESS) {
+		zone_is_dynamic = true;
+	} else {
+		res = cfg_map_get(zoptions, "allow-update", &allowupdate);
+		if (res != ISC_R_SUCCESS && voptions != NULL) {
+			res = cfg_map_get(voptions, "allow-update",
+					  &allowupdate);
+		}
+		if (res != ISC_R_SUCCESS && options != NULL) {
+			res = cfg_map_get(options, "allow-update",
+					  &allowupdate);
+		}
+		if (res == ISC_R_SUCCESS) {
+			dns_acl_t *acl = NULL;
+			cfg_aclconfctx_t *actx = NULL;
+			res = cfg_acl_fromconfig(
+				allowupdate, config, named_g_lctx, actx,
+				dns_zone_getmctx(zone), 0, &acl);
+			if (res == ISC_R_SUCCESS && acl != NULL &&
+			    !dns_acl_isnone(acl)) {
+				zone_is_dynamic = true;
+			}
+			if (acl != NULL) {
+				dns_acl_detach(&acl);
+			}
+		}
+	}
 
 	/*
 	 * If inline-signing is not set, perhaps implictly through a
@@ -2164,51 +2203,24 @@ named_zone_inlinesigning(dns_zone_t *zone, const cfg_obj_t *zconfig,
 	 * inline-signing.
 	 */
 	signing = NULL;
-	if (!inline_signing &&
+	if (!inline_signing && !zone_is_dynamic &&
 	    cfg_map_get(zoptions, "dnssec-policy", &signing) == ISC_R_SUCCESS &&
-	    signing != NULL && strcmp(cfg_obj_asstring(signing), "none") != 0)
+	    signing != NULL)
 	{
-		{
-			isc_result_t res;
-			bool zone_is_dynamic = false;
-			const cfg_obj_t *au = NULL;
-			const cfg_obj_t *up = NULL;
-
-			if (cfg_map_get(zoptions, "update-policy", &up) ==
-			    ISC_R_SUCCESS) {
-				zone_is_dynamic = true;
-			} else {
-				res = cfg_map_get(zoptions, "allow-update",
-						  &au);
-				if (res != ISC_R_SUCCESS && voptions != NULL) {
-					res = cfg_map_get(voptions,
-							  "allow-update", &au);
-				}
-				if (res != ISC_R_SUCCESS && options != NULL) {
-					res = cfg_map_get(options,
-							  "allow-update", &au);
-				}
-				if (res == ISC_R_SUCCESS) {
-					dns_acl_t *acl = NULL;
-					cfg_aclconfctx_t *actx = NULL;
-					res = cfg_acl_fromconfig(
-						au, config, named_g_lctx, actx,
-						dns_zone_getmctx(zone), 0,
-						&acl);
-					if (res == ISC_R_SUCCESS &&
-					    acl != NULL && !dns_acl_isnone(acl))
-					{
-						zone_is_dynamic = true;
-					}
-					if (acl != NULL) {
-						dns_acl_detach(&acl);
-					}
-				}
-			}
-
-			if (!zone_is_dynamic) {
-				inline_signing = true;
-			}
+		if (strcmp(cfg_obj_asstring(signing), "none") != 0) {
+			inline_signing = true;
+			dns_zone_log(
+				zone, ISC_LOG_DEBUG(1), "inline-signing: %s",
+				inline_signing
+					? "implicitly through dnssec-policy"
+					: "no");
+		} else {
+			inline_signing = dns_zone_secure_to_insecure(zone,
+								     true);
+			dns_zone_log(
+				zone, ISC_LOG_DEBUG(1), "inline-signing: %s",
+				inline_signing ? "transitioning to insecure"
+					       : "no");
 		}
 	}
 
diff --git a/bin/tests/system/kasp/tests.sh b/bin/tests/system/kasp/tests.sh
index 12b3fa9e0e..4519717fa6 100644
--- a/bin/tests/system/kasp/tests.sh
+++ b/bin/tests/system/kasp/tests.sh
@@ -960,22 +960,18 @@ check_dnssecstatus() {
 
 	rndccmd $_server dnssec -status $_zone in $_view > rndc.dnssec.status.out.$_zone.$n || log_error "rndc dnssec -status zone ${_zone} failed"
 
-	if [ "$_policy" = "none" ]; then
-		grep "Zone does not have dnssec-policy" rndc.dnssec.status.out.$_zone.$n > /dev/null || log_error "bad dnssec status for unsigned zone ${_zone}"
-	else
-		grep "dnssec-policy: ${_policy}" rndc.dnssec.status.out.$_zone.$n > /dev/null || log_error "bad dnssec status for signed zone ${_zone}"
-		if [ "$(key_get KEY1 EXPECT)" = "yes" ]; then
-			grep "key: $(key_get KEY1 ID)" rndc.dnssec.status.out.$_zone.$n > /dev/null || log_error "missing key $(key_get KEY1 ID) from dnssec status"
-		fi
-		if [ "$(key_get KEY2 EXPECT)" = "yes" ]; then
-			grep "key: $(key_get KEY2 ID)" rndc.dnssec.status.out.$_zone.$n > /dev/null || log_error "missing key $(key_get KEY2 ID) from dnssec status"
-		fi
-		if [ "$(key_get KEY3 EXPECT)" = "yes" ]; then
-			grep "key: $(key_get KEY3 ID)" rndc.dnssec.status.out.$_zone.$n > /dev/null || log_error "missing key $(key_get KEY3 ID) from dnssec status"
-		fi
-		if [ "$(key_get KEY4 EXPECT)" = "yes" ]; then
-			grep "key: $(key_get KEY4 ID)" rndc.dnssec.status.out.$_zone.$n > /dev/null || log_error "missing key $(key_get KEY4 ID) from dnssec status"
-		fi
+	grep "dnssec-policy: ${_policy}" rndc.dnssec.status.out.$_zone.$n > /dev/null || log_error "bad dnssec status for signed zone ${_zone}"
+	if [ "$(key_get KEY1 EXPECT)" = "yes" ]; then
+		grep "key: $(key_get KEY1 ID)" rndc.dnssec.status.out.$_zone.$n > /dev/null || log_error "missing key $(key_get KEY1 ID) from dnssec status"
+	fi
+	if [ "$(key_get KEY2 EXPECT)" = "yes" ]; then
+		grep "key: $(key_get KEY2 ID)" rndc.dnssec.status.out.$_zone.$n > /dev/null || log_error "missing key $(key_get KEY2 ID) from dnssec status"
+	fi
+	if [ "$(key_get KEY3 EXPECT)" = "yes" ]; then
+		grep "key: $(key_get KEY3 ID)" rndc.dnssec.status.out.$_zone.$n > /dev/null || log_error "missing key $(key_get KEY3 ID) from dnssec status"
+	fi
+	if [ "$(key_get KEY4 EXPECT)" = "yes" ]; then
+		grep "key: $(key_get KEY4 ID)" rndc.dnssec.status.out.$_zone.$n > /dev/null || log_error "missing key $(key_get KEY4 ID) from dnssec status"
 	fi
 
 	test "$ret" -eq 0 || echo_i "failed"
diff --git a/lib/bind9/check.c b/lib/bind9/check.c
index 3e0b341d39..1473b5385b 100644
--- a/lib/bind9/check.c
+++ b/lib/bind9/check.c
@@ -1015,9 +1015,9 @@ check_options(const cfg_obj_t *options, isc_log_t *logctx, isc_mem_t *mctx,
 						continue;
 					}
 
-					ret = cfg_kasp_fromconfig(kconfig, mctx,
-								  logctx, &list,
-								  &kasp);
+					ret = cfg_kasp_fromconfig(kconfig, NULL,
+								  mctx, logctx,
+								  &list, &kasp);
 					if (ret != ISC_R_SUCCESS) {
 						if (result == ISC_R_SUCCESS) {
 							result = ret;
diff --git a/lib/dns/include/dns/zone.h b/lib/dns/include/dns/zone.h
index af7208f56b..8cee35515f 100644
--- a/lib/dns/include/dns/zone.h
+++ b/lib/dns/include/dns/zone.h
@@ -694,6 +694,32 @@ dns_zone_getkasp(dns_zone_t *zone);
  *\li	'zone' to be a valid zone.
  */
 
+bool
+dns_zone_secure_to_insecure(dns_zone_t *zone, bool reconfig);
+/*%<
+ *	Returns true if the zone is transitioning to insecure.
+ *	Only can happen if a zone previously used a dnssec-policy,
+ *	but changed the value to "none" (or removed the configuration
+ *	option). If 'reconfig' is true, only check the key files,
+ *	because the zone structure is not yet updated with the
+ *	newest configuration.
+ *
+ * Require:
+ *\li	'zone' to be a valid zone.
+ */
+
+bool
+dns_zone_use_kasp(dns_zone_t *zone);
+/*%<
+ *	Check if zone needs to use kasp.
+ *	True if there is a policy that is not "none",
+ *	or if there are state files associated with the keys
+ *	related to this zone.
+ *
+ * Require:
+ *\li	'zone' to be a valid zone.
+ */
+
 void
 dns_zone_setkasp(dns_zone_t *zone, dns_kasp_t *kasp);
 /*%<
diff --git a/lib/dns/update.c b/lib/dns/update.c
index 43dad7035a..808a636a81 100644
--- a/lib/dns/update.c
+++ b/lib/dns/update.c
@@ -1087,7 +1087,6 @@ add_sigs(dns_update_log_t *log, dns_zone_t *zone, dns_db_t *db,
 	 bool keyset_kskonly) {
 	isc_result_t result;
 	dns_dbnode_t *node = NULL;
-	dns_kasp_t *kasp = dns_zone_getkasp(zone);
 	dns_rdataset_t rdataset;
 	dns_rdata_t sig_rdata = DNS_RDATA_INIT;
 	dns_stats_t *dnssecsignstats = dns_zone_getdnssecsignstats(zone);
@@ -1095,11 +1094,13 @@ add_sigs(dns_update_log_t *log, dns_zone_t *zone, dns_db_t *db,
 	unsigned char data[1024]; /* XXX */
 	unsigned int i, j;
 	bool added_sig = false;
+	bool use_kasp = false;
 	isc_mem_t *mctx = diff->mctx;
 
-	if (kasp != NULL) {
+	if (dns_zone_use_kasp(zone)) {
 		check_ksk = false;
 		keyset_kskonly = true;
+		use_kasp = true;
 	}
 
 	dns_rdataset_init(&rdataset);
@@ -1174,7 +1175,7 @@ add_sigs(dns_update_log_t *log, dns_zone_t *zone, dns_db_t *db,
 			}
 		}
 
-		if (kasp != NULL) {
+		if (use_kasp) {
 			/*
 			 * A dnssec-policy is found. Check what RRsets this
 			 * key should sign.
diff --git a/lib/dns/win32/libdns.def.in b/lib/dns/win32/libdns.def.in
index 6e723fe2b8..4747fbdfbd 100644
--- a/lib/dns/win32/libdns.def.in
+++ b/lib/dns/win32/libdns.def.in
@@ -1278,6 +1278,7 @@ dns_zone_rekey
 dns_zone_replacedb
 dns_zone_rpz_enable
 dns_zone_rpz_enable_db
+dns_zone_secure_to_insecure
 dns_zone_set_parentcatz
 dns_zone_setadded
 dns_zone_setalsonotify
@@ -1366,6 +1367,7 @@ dns_zone_setzeronosoattl
 dns_zone_signwithkey
 dns_zone_synckeyzone
 dns_zone_unload
+dns_zone_use_kasp
 dns_zone_verifydb
 dns_zonekey_iszonekey
 dns_zonemgr_attach
diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index a6a3e0b683..80d43ce71f 100644
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -1830,7 +1830,7 @@ dns_zone_isdynamic(dns_zone_t *zone, bool ignore_freeze) {
 	}
 
 	/* Kasp zones are always dynamic. */
-	if (dns_zone_getkasp(zone) != NULL) {
+	if (dns_zone_use_kasp(zone)) {
 		return (true);
 	}
 
@@ -5833,6 +5833,82 @@ dns_zone_getkasp(dns_zone_t *zone) {
 	return (zone->kasp);
 }
 
+static bool
+statefile_exist(dns_zone_t *zone) {
+	isc_result_t ret;
+	dns_dnsseckeylist_t keys;
+	dns_dnsseckey_t *key = NULL;
+	isc_stdtime_t now;
+	isc_time_t timenow;
+	bool found = false;
+
+	TIME_NOW(&timenow);
+	now = isc_time_seconds(&timenow);
+
+	ISC_LIST_INIT(keys);
+
+	ret = dns_dnssec_findmatchingkeys(dns_zone_getorigin(zone),
+					  dns_zone_getkeydirectory(zone), now,
+					  dns_zone_getmctx(zone), &keys);
+	if (ret == ISC_R_SUCCESS) {
+		for (key = ISC_LIST_HEAD(keys); key != NULL;
+		     key = ISC_LIST_NEXT(key, link)) {
+			if (dst_key_haskasp(key->key)) {
+				found = true;
+				break;
+			}
+		}
+	}
+
+	/* Clean up keys */
+	while (!ISC_LIST_EMPTY(keys)) {
+		key = ISC_LIST_HEAD(keys);
+		ISC_LIST_UNLINK(keys, key, link);
+		dns_dnsseckey_destroy(dns_zone_getmctx(zone), &key);
+	}
+
+	return (found);
+}
+
+bool
+dns_zone_secure_to_insecure(dns_zone_t *zone, bool reconfig) {
+	REQUIRE(DNS_ZONE_VALID(zone));
+
+	/*
+	 * If checking during reconfig, the zone is not yet updated
+	 * with the new kasp configuration, so only check the key
+	 * files.
+	 */
+	if (reconfig) {
+		return (statefile_exist(zone));
+	}
+
+	if (zone->kasp == NULL) {
+		return (false);
+	}
+	if (strcmp(dns_kasp_getname(zone->kasp), "none") != 0) {
+		return (false);
+	}
+	/*
+	 * "dnssec-policy none", but if there are key state files
+	 * this zone used to be secure but is transitioning back to
+	 * insecure.
+	 */
+	return (statefile_exist(zone));
+}
+
+bool
+dns_zone_use_kasp(dns_zone_t *zone) {
+	dns_kasp_t *kasp = dns_zone_getkasp(zone);
+
+	if (kasp == NULL) {
+		return (false);
+	} else if (strcmp(dns_kasp_getname(kasp), "none") != 0) {
+		return (true);
+	}
+	return dns_zone_secure_to_insecure(zone, false);
+}
+
 void
 dns_zone_setoption(dns_zone_t *zone, dns_zoneopt_t option, bool value) {
 	REQUIRE(DNS_ZONE_VALID(zone));
@@ -6784,17 +6860,18 @@ add_sigs(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name, dns_zone_t *zone,
 	 isc_stdtime_t expire, bool check_ksk, bool keyset_kskonly) {
 	isc_result_t result;
 	dns_dbnode_t *node = NULL;
-	dns_kasp_t *kasp = dns_zone_getkasp(zone);
 	dns_stats_t *dnssecsignstats;
 	dns_rdataset_t rdataset;
 	dns_rdata_t sig_rdata = DNS_RDATA_INIT;
 	unsigned char data[1024]; /* XXX */
 	isc_buffer_t buffer;
 	unsigned int i, j;
+	bool use_kasp = false;
 
-	if (kasp != NULL) {
+	if (dns_zone_use_kasp(zone)) {
 		check_ksk = false;
 		keyset_kskonly = true;
+		use_kasp = true;
 	}
 
 	dns_rdataset_init(&rdataset);
@@ -6872,8 +6949,7 @@ add_sigs(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name, dns_zone_t *zone,
 				}
 			}
 		}
-
-		if (kasp != NULL) {
+		if (use_kasp) {
 			/*
 			 * A dnssec-policy is found. Check what RRsets this
 			 * key should sign.
@@ -7308,7 +7384,7 @@ signed_with_good_key(dns_zone_t *zone, dns_db_t *db, dns_dbnode_t *node,
 		dns_rdata_reset(&rdata);
 	}
 
-	if (kasp) {
+	if (dns_zone_use_kasp(zone)) {
 		dns_kasp_key_t *kkey;
 		int zsk_count = 0;
 		bool approved;
@@ -7425,7 +7501,6 @@ sign_a_node(dns_db_t *db, dns_zone_t *zone, dns_name_t *name,
 	    bool is_zsk, bool keyset_kskonly, bool is_bottom_of_zone,
 	    dns_diff_t *diff, int32_t *signatures, isc_mem_t *mctx) {
 	isc_result_t result;
-	dns_kasp_t *kasp = dns_zone_getkasp(zone);
 	dns_rdatasetiter_t *iterator = NULL;
 	dns_rdataset_t rdataset;
 	dns_rdata_t rdata = DNS_RDATA_INIT;
@@ -7521,7 +7596,7 @@ sign_a_node(dns_db_t *db, dns_zone_t *zone, dns_name_t *name,
 		} else if (is_zsk && !dst_key_is_signing(key, DST_BOOL_ZSK,
 							 inception, &when)) {
 			/* Only applies to dnssec-policy. */
-			if (kasp != NULL) {
+			if (dns_zone_use_kasp(zone)) {
 				goto next_rdataset;
 			}
 		}
@@ -9165,6 +9240,7 @@ zone_sign(dns_zone_t *zone) {
 	bool is_bottom_of_zone;
 	bool build_nsec = false;
 	bool build_nsec3 = false;
+	bool use_kasp = false;
 	bool first;
 	isc_result_t result;
 	isc_stdtime_t now, inception, soaexpire, expire;
@@ -9221,7 +9297,6 @@ zone_sign(dns_zone_t *zone) {
 	}
 
 	kasp = dns_zone_getkasp(zone);
-
 	sigvalidityinterval = dns_zone_getsigvalidityinterval(zone);
 	inception = now - 3600; /* Allow for clock skew. */
 	soaexpire = now + sigvalidityinterval;
@@ -9258,16 +9333,20 @@ zone_sign(dns_zone_t *zone) {
 	signing = ISC_LIST_HEAD(zone->signing);
 	first = true;
 
-	check_ksk = (kasp != NULL)
-			    ? false
-			    : DNS_ZONE_OPTION(zone, DNS_ZONEOPT_UPDATECHECKKSK);
-	keyset_kskonly = (kasp != NULL)
-				 ? true
-				 : DNS_ZONE_OPTION(zone,
-						   DNS_ZONEOPT_DNSKEYKSKONLY);
+	if (dns_zone_use_kasp(zone)) {
+		check_ksk = false;
+		keyset_kskonly = true;
+		use_kasp = true;
+	} else {
+		check_ksk = DNS_ZONE_OPTION(zone, DNS_ZONEOPT_UPDATECHECKKSK);
+		keyset_kskonly = DNS_ZONE_OPTION(zone,
+						 DNS_ZONEOPT_DNSKEYKSKONLY);
+	}
+	dnssec_log(zone, ISC_LOG_DEBUG(3), "zone_sign:use kasp -> %s",
+		   use_kasp ? "yes" : "no");
 
 	/* Determine which type of chain to build */
-	if (kasp != NULL) {
+	if (use_kasp) {
 		build_nsec3 = dns_kasp_nsec3(kasp);
 		build_nsec = !build_nsec3;
 	} else {
@@ -9452,7 +9531,7 @@ zone_sign(dns_zone_t *zone) {
 					}
 				}
 			}
-			if (kasp != NULL) {
+			if (use_kasp) {
 				/*
 				 * A dnssec-policy is found. Check what
 				 * RRsets this key can sign.
@@ -16462,7 +16541,7 @@ copy_non_dnssec_records(dns_zone_t *zone, dns_db_t *db, dns_db_t *version,
 			 * Allow DNSSEC records with dnssec-policy.
 			 * WMM: Perhaps add config option for it.
 			 */
-			if (dns_zone_getkasp(zone) == NULL) {
+			if (!dns_zone_use_kasp(zone)) {
 				dns_rdataset_disassociate(&rdataset);
 				continue;
 			}
@@ -19747,7 +19826,7 @@ zone_rekey(dns_zone_t *zone) {
 	dns__zonediff_t zonediff;
 	bool commit = false, newactive = false;
 	bool newalg = false;
-	bool fullsign;
+	bool fullsign, use_kasp;
 	dns_ttl_t ttl = 3600;
 	const char *dir = NULL;
 	isc_mem_t *mctx = NULL;
@@ -19818,9 +19897,10 @@ zone_rekey(dns_zone_t *zone) {
 	 * True when called from "rndc sign".  Indicates the zone should be
 	 * fully signed now.
 	 */
-	kasp = dns_zone_getkasp(zone);
 	fullsign = DNS_ZONEKEY_OPTION(zone, DNS_ZONEKEY_FULLSIGN);
 
+	kasp = dns_zone_getkasp(zone);
+	use_kasp = dns_zone_use_kasp(zone);
 	if (kasp != NULL) {
 		LOCK(&kasp->lock);
 	}
@@ -19833,9 +19913,7 @@ zone_rekey(dns_zone_t *zone) {
 			   isc_result_totext(result));
 	}
 
-	if (kasp != NULL &&
-	    (result == ISC_R_SUCCESS || result == ISC_R_NOTFOUND)) {
-		ttl = dns_kasp_dnskeyttl(kasp);
+	if (use_kasp && (result == ISC_R_SUCCESS || result == ISC_R_NOTFOUND)) {
 		result = dns_keymgr_run(&zone->origin, zone->rdclass, dir, mctx,
 					&keys, kasp, now, &nexttime);
 		if (result != ISC_R_SUCCESS) {
@@ -19851,6 +19929,16 @@ zone_rekey(dns_zone_t *zone) {
 	}
 
 	if (result == ISC_R_SUCCESS) {
+		bool insecure = dns_zone_secure_to_insecure(zone, false);
+
+		/*
+		 * Only update DNSKEY TTL if we have a policy.
+		 */
+		if (kasp != NULL && strcmp(dns_kasp_getname(kasp), "none") != 0)
+		{
+			ttl = dns_kasp_dnskeyttl(kasp);
+		}
+
 		result = dns_dnssec_updatekeys(&dnskeys, &keys, &rmkeys,
 					       &zone->origin, ttl, &diff, mctx,
 					       dnssec_report);
@@ -20063,7 +20151,7 @@ zone_rekey(dns_zone_t *zone) {
 	/*
 	 * If keymgr provided a next time, use the calculated next rekey time.
 	 */
-	if (kasp != NULL) {
+	if (use_kasp) {
 		isc_time_t timenext;
 		uint32_t nexttime_seconds;
 
diff --git a/lib/isccfg/include/isccfg/kaspconf.h b/lib/isccfg/include/isccfg/kaspconf.h
index 0f8ac1dd7e..43f6e690a7 100644
--- a/lib/isccfg/include/isccfg/kaspconf.h
+++ b/lib/isccfg/include/isccfg/kaspconf.h
@@ -25,22 +25,23 @@
 ISC_LANG_BEGINDECLS
 
 isc_result_t
-cfg_kasp_fromconfig(const cfg_obj_t *config, isc_mem_t *mctx, isc_log_t *logctx,
-		    dns_kasplist_t *kasplist, dns_kasp_t **kaspp);
+cfg_kasp_fromconfig(const cfg_obj_t *config, const char *name, isc_mem_t *mctx,
+		    isc_log_t *logctx, dns_kasplist_t *kasplist,
+		    dns_kasp_t **kaspp);
 /*%<
- * Create and configure a KASP. If 'config' is NULL, the default configuration
- * is used. If a 'kasplist' is provided, a lookup happens and if a KASP
- * already exists with the same name, no new KASP is created, and no attach to
- * 'kaspp' happens.
+ * Create and configure a KASP. If 'config' is NULL, a built-in configuration
+ * is used, referred to by 'name'. If a 'kasplist' is provided, a lookup
+ * happens and if a KASP already exists with the same name, no new KASP is
+ * created, and no attach to 'kaspp' happens.
  *
  * Requires:
  *
+ *\li  'name' is either NULL, or a valid C string.
+ *
  *\li  'mctx' is a valid memory context.
  *
  *\li  'logctx' is a valid logging context.
  *
- *\li  'name' is a valid C string.
- *
  *\li  kaspp != NULL && *kaspp == NULL
  *
  * Returns:
diff --git a/lib/isccfg/kaspconf.c b/lib/isccfg/kaspconf.c
index 9a006c8963..3e0e0874f4 100644
--- a/lib/isccfg/kaspconf.c
+++ b/lib/isccfg/kaspconf.c
@@ -253,8 +253,9 @@ cfg_nsec3param_fromconfig(const cfg_obj_t *config, dns_kasp_t *kasp,
 }
 
 isc_result_t
-cfg_kasp_fromconfig(const cfg_obj_t *config, isc_mem_t *mctx, isc_log_t *logctx,
-		    dns_kasplist_t *kasplist, dns_kasp_t **kaspp) {
+cfg_kasp_fromconfig(const cfg_obj_t *config, const char *name, isc_mem_t *mctx,
+		    isc_log_t *logctx, dns_kasplist_t *kasplist,
+		    dns_kasp_t **kaspp) {
 	isc_result_t result;
 	const cfg_obj_t *maps[2];
 	const cfg_obj_t *koptions = NULL;
@@ -267,11 +268,10 @@ cfg_kasp_fromconfig(const cfg_obj_t *config, isc_mem_t *mctx, isc_log_t *logctx,
 
 	REQUIRE(kaspp != NULL && *kaspp == NULL);
 
-	kaspname = (config != NULL)
+	kaspname = (name == NULL)
 			   ? cfg_obj_asstring(cfg_tuple_get(config, "name"))
-			   : "default";
-
-	REQUIRE(strcmp(kaspname, "none") != 0);
+			   : name;
+	INSIST(kaspname != NULL);
 
 	result = dns_kasplist_find(kasplist, kaspname, &kasp);
 
@@ -317,12 +317,7 @@ cfg_kasp_fromconfig(const cfg_obj_t *config, isc_mem_t *mctx, isc_log_t *logctx,
 						    DNS_KASP_RETIRE_SAFETY));
 
 	(void)confget(maps, "keys", &keys);
-	if (keys == NULL) {
-		result = cfg_kaspkey_fromconfig(NULL, kasp, logctx);
-		if (result != ISC_R_SUCCESS) {
-			goto cleanup;
-		}
-	} else {
+	if (keys != NULL) {
 		for (element = cfg_list_first(keys); element != NULL;
 		     element = cfg_list_next(element))
 		{
@@ -332,8 +327,19 @@ cfg_kasp_fromconfig(const cfg_obj_t *config, isc_mem_t *mctx, isc_log_t *logctx,
 				goto cleanup;
 			}
 		}
+		INSIST(!(dns_kasp_keylist_empty(kasp)));
+	} else if (strcmp(kaspname, "none") == 0) {
+		/* "dnssec-policy none": key list must be empty */
+		INSIST(strcmp(kaspname, "none") == 0);
+		INSIST(dns_kasp_keylist_empty(kasp));
+	} else {
+		/* No keys clause configured, use the "default". */
+		result = cfg_kaspkey_fromconfig(NULL, kasp, logctx);
+		if (result != ISC_R_SUCCESS) {
+			goto cleanup;
+		}
+		INSIST(!(dns_kasp_keylist_empty(kasp)));
 	}
-	INSIST(!(dns_kasp_keylist_empty(kasp)));
 
 	/* Configuration: NSEC3 */
 	(void)confget(maps, "nsec3param", &nsec3);

From 68d715a229c586ba427fed9ec3af87770a647509 Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Thu, 3 Dec 2020 15:33:45 +0100
Subject: [PATCH 5/9] Publish CDS/CDNSKEY Delete Records

Check if zone is transitioning from secure to insecure. If so,
delete the CDS/CDNSKEY records, otherwise make sure they are not
part of the RRset.
---
 lib/dns/dnssec.c             | 154 +++++++++++++++++++++++++++++++----
 lib/dns/include/dns/dnssec.h |  19 +++++
 lib/dns/win32/libdns.def.in  |   2 +-
 lib/dns/zone.c               |  11 +++
 4 files changed, 168 insertions(+), 18 deletions(-)

diff --git a/lib/dns/dnssec.c b/lib/dns/dnssec.c
index 81c541fee7..54bbd1fd6a 100644
--- a/lib/dns/dnssec.c
+++ b/lib/dns/dnssec.c
@@ -2010,24 +2010,50 @@ dns_dnssec_syncupdate(dns_dnsseckeylist_t *keys, dns_dnsseckeylist_t *rmkeys,
 			}
 		}
 
-		if (dns_rdataset_isassociated(cds) && syncdelete(key->key, now))
-		{
-			/* Delete both SHA-1 and SHA-256 */
-			if (exists(cds, &cds_sha1)) {
-				RETERR(delrdata(&cds_sha1, diff, origin,
-						cds->ttl, mctx));
-			}
-			if (exists(cds, &cds_sha256)) {
-				RETERR(delrdata(&cds_sha256, diff, origin,
-						cds->ttl, mctx));
-			}
-		}
+		if (syncdelete(key->key, now)) {
+			char keystr[DST_KEY_FORMATSIZE];
+			dst_key_format(key->key, keystr, sizeof(keystr));
 
-		if (dns_rdataset_isassociated(cdnskey) &&
-		    syncdelete(key->key, now)) {
-			if (exists(cdnskey, &cdnskeyrdata)) {
-				RETERR(delrdata(&cdnskeyrdata, diff, origin,
-						cdnskey->ttl, mctx));
+			if (dns_rdataset_isassociated(cds)) {
+				/* Delete both SHA-1 and SHA-256 */
+				if (exists(cds, &cds_sha1)) {
+					isc_log_write(dns_lctx,
+						      DNS_LOGCATEGORY_GENERAL,
+						      DNS_LOGMODULE_DNSSEC,
+						      ISC_LOG_INFO,
+						      "CDS (SHA-1) for key %s "
+						      "is now deleted",
+						      keystr);
+					RETERR(delrdata(&cds_sha1, diff, origin,
+							cds->ttl, mctx));
+				}
+				if (exists(cds, &cds_sha256)) {
+					isc_log_write(dns_lctx,
+						      DNS_LOGCATEGORY_GENERAL,
+						      DNS_LOGMODULE_DNSSEC,
+						      ISC_LOG_INFO,
+						      "CDS (SHA-256) for key "
+						      "%s is now deleted",
+						      keystr);
+					RETERR(delrdata(&cds_sha256, diff,
+							origin, cds->ttl,
+							mctx));
+				}
+			}
+
+			if (dns_rdataset_isassociated(cdnskey)) {
+				if (exists(cdnskey, &cdnskeyrdata)) {
+					isc_log_write(dns_lctx,
+						      DNS_LOGCATEGORY_GENERAL,
+						      DNS_LOGMODULE_DNSSEC,
+						      ISC_LOG_INFO,
+						      "CDNSKEY for key %s is "
+						      "now deleted",
+						      keystr);
+					RETERR(delrdata(&cdnskeyrdata, diff,
+							origin, cdnskey->ttl,
+							mctx));
+				}
 			}
 		}
 	}
@@ -2047,6 +2073,9 @@ dns_dnssec_syncupdate(dns_dnsseckeylist_t *keys, dns_dnsseckeylist_t *rmkeys,
 		dns_rdata_t cdnskeyrdata = DNS_RDATA_INIT;
 		dns_name_t *origin = dst_key_name(key->key);
 
+		char keystr[DST_KEY_FORMATSIZE];
+		dst_key_format(key->key, keystr, sizeof(keystr));
+
 		RETERR(make_dnskey(key->key, keybuf, sizeof(keybuf),
 				   &cdnskeyrdata));
 
@@ -2058,10 +2087,21 @@ dns_dnssec_syncupdate(dns_dnsseckeylist_t *keys, dns_dnsseckeylist_t *rmkeys,
 						 DNS_DSDIGEST_SHA256, dsbuf2,
 						 &cds_sha256));
 			if (exists(cds, &cds_sha1)) {
+				isc_log_write(
+					dns_lctx, DNS_LOGCATEGORY_GENERAL,
+					DNS_LOGMODULE_DNSSEC, ISC_LOG_INFO,
+					"CDS (SHA-1) for key %s is now deleted",
+					keystr);
 				RETERR(delrdata(&cds_sha1, diff, origin,
 						cds->ttl, mctx));
 			}
 			if (exists(cds, &cds_sha256)) {
+				isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL,
+					      DNS_LOGMODULE_DNSSEC,
+					      ISC_LOG_INFO,
+					      "CDS (SHA-256) for key %s is now "
+					      "deleted",
+					      keystr);
 				RETERR(delrdata(&cds_sha256, diff, origin,
 						cds->ttl, mctx));
 			}
@@ -2069,6 +2109,11 @@ dns_dnssec_syncupdate(dns_dnsseckeylist_t *keys, dns_dnsseckeylist_t *rmkeys,
 
 		if (dns_rdataset_isassociated(cdnskey)) {
 			if (exists(cdnskey, &cdnskeyrdata)) {
+				isc_log_write(
+					dns_lctx, DNS_LOGCATEGORY_GENERAL,
+					DNS_LOGMODULE_DNSSEC, ISC_LOG_INFO,
+					"CDNSKEY for key %s is now deleted",
+					keystr);
 				RETERR(delrdata(&cdnskeyrdata, diff, origin,
 						cdnskey->ttl, mctx));
 			}
@@ -2081,6 +2126,81 @@ failure:
 	return (result);
 }
 
+isc_result_t
+dns_dnssec_syncdelete(dns_rdataset_t *cds, dns_rdataset_t *cdnskey,
+		      dns_name_t *origin, dns_rdataclass_t zclass,
+		      dns_ttl_t ttl, dns_diff_t *diff, isc_mem_t *mctx,
+		      bool dnssec_insecure) {
+	unsigned char dsbuf[5] = { 0, 0, 0, 0, 0 };  /* CDS DELETE rdata */
+	unsigned char keybuf[5] = { 0, 0, 3, 0, 0 }; /* CDNSKEY DELETE rdata */
+	char namebuf[DNS_NAME_FORMATSIZE];
+	dns_rdata_t cds_delete = DNS_RDATA_INIT;
+	dns_rdata_t cdnskey_delete = DNS_RDATA_INIT;
+	isc_region_t r;
+	isc_result_t result;
+
+	r.base = keybuf;
+	r.length = sizeof(keybuf);
+	dns_rdata_fromregion(&cdnskey_delete, zclass, dns_rdatatype_cdnskey,
+			     &r);
+
+	r.base = dsbuf;
+	r.length = sizeof(dsbuf);
+	dns_rdata_fromregion(&cds_delete, zclass, dns_rdatatype_cds, &r);
+
+	dns_name_format(origin, namebuf, sizeof(namebuf));
+
+	if (dnssec_insecure) {
+		if (!dns_rdataset_isassociated(cdnskey) ||
+		    !exists(cdnskey, &cdnskey_delete)) {
+			isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL,
+				      DNS_LOGMODULE_DNSSEC, ISC_LOG_INFO,
+				      "CDNSKEY (DELETE) for zone %s is now "
+				      "published",
+				      namebuf);
+			RETERR(addrdata(&cdnskey_delete, diff, origin, ttl,
+					mctx));
+		}
+
+		if (!dns_rdataset_isassociated(cds) ||
+		    !exists(cds, &cds_delete)) {
+			isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL,
+				      DNS_LOGMODULE_DNSSEC, ISC_LOG_INFO,
+				      "CDS (DELETE) for zone %s is now "
+				      "published",
+				      namebuf);
+			RETERR(addrdata(&cds_delete, diff, origin, ttl, mctx));
+		}
+	} else {
+		if (dns_rdataset_isassociated(cdnskey) &&
+		    exists(cdnskey, &cdnskey_delete)) {
+			isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL,
+				      DNS_LOGMODULE_DNSSEC, ISC_LOG_INFO,
+				      "CDNSKEY (DELETE) for zone %s is now "
+				      "deleted",
+				      namebuf);
+			RETERR(delrdata(&cdnskey_delete, diff, origin,
+					cdnskey->ttl, mctx));
+		}
+
+		if (dns_rdataset_isassociated(cds) && exists(cds, &cds_delete))
+		{
+			isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL,
+				      DNS_LOGMODULE_DNSSEC, ISC_LOG_INFO,
+				      "CDS (DELETE) for zone %s is now "
+				      "deleted",
+				      namebuf);
+			RETERR(delrdata(&cds_delete, diff, origin, cds->ttl,
+					mctx));
+		}
+	}
+
+	result = ISC_R_SUCCESS;
+
+failure:
+	return (result);
+}
+
 /*
  * Update 'keys' with information from 'newkeys'.
  *
diff --git a/lib/dns/include/dns/dnssec.h b/lib/dns/include/dns/dnssec.h
index 6e951c7267..c51b2c9aad 100644
--- a/lib/dns/include/dns/dnssec.h
+++ b/lib/dns/include/dns/dnssec.h
@@ -357,6 +357,25 @@ dns_dnssec_syncupdate(dns_dnsseckeylist_t *keys, dns_dnsseckeylist_t *rmkeys,
 		      isc_mem_t *mctx);
 /*%<
  * Update the CDS and CDNSKEY RRsets, adding and removing keys as needed.
+ *
+ * Returns:
+ *\li   ISC_R_SUCCESS
+ *\li   Other values indicate error
+ */
+
+isc_result_t
+dns_dnssec_syncdelete(dns_rdataset_t *cds, dns_rdataset_t *cdnskey,
+		      dns_name_t *origin, dns_rdataclass_t zclass,
+		      dns_ttl_t ttl, dns_diff_t *diff, isc_mem_t *mctx,
+		      bool dnssec_insecure);
+/*%<
+ * Add or remove the CDS DELETE record and the CDNSKEY DELETE record.
+ * If 'dnssec_insecure' is true, the DELETE records should be present.
+ * Otherwise, the DELETE records must be removed from the RRsets (if present).
+ *
+ * Returns:
+ *\li   ISC_R_SUCCESS
+ *\li   Other values indicate error
  */
 
 isc_result_t
diff --git a/lib/dns/win32/libdns.def.in b/lib/dns/win32/libdns.def.in
index 4747fbdfbd..c9b4e1fe80 100644
--- a/lib/dns/win32/libdns.def.in
+++ b/lib/dns/win32/libdns.def.in
@@ -332,7 +332,7 @@ dns_dnssec_selfsigns
 dns_dnssec_sign
 dns_dnssec_signmessage
 dns_dnssec_signs
-dns_dnssec_syncupdate
+dns_dnssec_syncdelete
 dns_dnssec_syncupdate
 dns_dnssec_updatekeys
 dns_dnssec_verify
diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index 80d43ce71f..967603ddcf 100644
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -19966,6 +19966,17 @@ zone_rekey(dns_zone_t *zone) {
 			goto failure;
 		}
 
+		result = dns_dnssec_syncdelete(&cdsset, &cdnskeyset,
+					       &zone->origin, zone->rdclass,
+					       ttl, &diff, mctx, insecure);
+		if (result != ISC_R_SUCCESS) {
+			dnssec_log(zone, ISC_LOG_ERROR,
+				   "zone_rekey:couldn't update CDS/CDNSKEY "
+				   "DELETE records: %s",
+				   isc_result_totext(result));
+			goto failure;
+		}
+
 		/*
 		 * See if any pre-existing keys have newly become active;
 		 * also, see if any new key is for a new algorithm, as in that

From 913410006912984b49f9e8efa74e7c1f274cbe4d Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Thu, 3 Dec 2020 16:04:28 +0100
Subject: [PATCH 6/9] Update keymgr to allow transition to insecure mode

The keymgr prevented zones from going to insecure mode. If we
have a policy with an empty key list this is a signal that the zone
wants to go back to insecure mode. In this case allow one extra state
transition to be valid when checking for DNSSEC safety.
---
 lib/dns/keymgr.c | 36 ++++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/lib/dns/keymgr.c b/lib/dns/keymgr.c
index 2518b48739..bfd8009c3a 100644
--- a/lib/dns/keymgr.c
+++ b/lib/dns/keymgr.c
@@ -777,7 +777,7 @@ keymgr_dnskey_hidden_or_chained(dns_dnsseckeylist_t *keyring,
  */
 static bool
 keymgr_have_ds(dns_dnsseckeylist_t *keyring, dns_dnsseckey_t *key, int type,
-	       dst_key_state_t next_state) {
+	       dst_key_state_t next_state, bool secure_to_insecure) {
 	dst_key_state_t states[2][4] = {
 		/* DNSKEY, ZRRSIG, KRRSIG, DS */
 		{ NA, NA, NA, OMNIPRESENT }, /* DS present */
@@ -792,7 +792,10 @@ keymgr_have_ds(dns_dnsseckeylist_t *keyring, dns_dnsseckey_t *key, int type,
 	return (keymgr_key_exists_with_state(keyring, key, type, next_state,
 					     states[0], na, false, false) ||
 		keymgr_key_exists_with_state(keyring, key, type, next_state,
-					     states[1], na, false, false));
+					     states[1], na, false, false) ||
+		(secure_to_insecure &&
+		 keymgr_key_exists_with_state(keyring, key, type, next_state,
+					      na, na, false, false)));
 }
 
 /*
@@ -1022,14 +1025,17 @@ keymgr_policy_approval(dns_dnsseckeylist_t *keyring, dns_dnsseckey_t *key,
  */
 static bool
 keymgr_transition_allowed(dns_dnsseckeylist_t *keyring, dns_dnsseckey_t *key,
-			  int type, dst_key_state_t next_state) {
+			  int type, dst_key_state_t next_state,
+			  bool secure_to_insecure) {
 	/* Debug logging. */
 	if (isc_log_wouldlog(dns_lctx, ISC_LOG_DEBUG(1))) {
 		bool rule1a, rule1b, rule2a, rule2b, rule3a, rule3b;
 		char keystr[DST_KEY_FORMATSIZE];
 		dst_key_format(key->key, keystr, sizeof(keystr));
-		rule1a = keymgr_have_ds(keyring, key, type, NA);
-		rule1b = keymgr_have_ds(keyring, key, type, next_state);
+		rule1a = keymgr_have_ds(keyring, key, type, NA,
+					secure_to_insecure);
+		rule1b = keymgr_have_ds(keyring, key, type, next_state,
+					secure_to_insecure);
 		rule2a = keymgr_have_dnskey(keyring, key, type, NA);
 		rule2b = keymgr_have_dnskey(keyring, key, type, next_state);
 		rule3a = keymgr_have_rrsig(keyring, key, type, NA);
@@ -1054,8 +1060,9 @@ keymgr_transition_allowed(dns_dnsseckeylist_t *keyring, dns_dnsseckey_t *key,
 		 * invalid state.  If the rule check passes, also check if
 		 * the next state is also still a valid situation.
 		 */
-		(!keymgr_have_ds(keyring, key, type, NA) ||
-		 keymgr_have_ds(keyring, key, type, next_state)) &&
+		(!keymgr_have_ds(keyring, key, type, NA, secure_to_insecure) ||
+		 keymgr_have_ds(keyring, key, type, next_state,
+				secure_to_insecure)) &&
 		/*
 		 * Rule 2: There must be a DNSKEY at all times.  Again, first
 		 * check the current situation, then assess the next state.
@@ -1246,7 +1253,7 @@ keymgr_transition_time(dns_dnsseckey_t *key, int type,
  */
 static isc_result_t
 keymgr_update(dns_dnsseckeylist_t *keyring, dns_kasp_t *kasp, isc_stdtime_t now,
-	      isc_stdtime_t *nexttime) {
+	      isc_stdtime_t *nexttime, bool secure_to_insecure) {
 	bool changed;
 
 	/* Repeat until nothing changed. */
@@ -1328,7 +1335,9 @@ transition:
 
 			/* Is the transition DNSSEC safe? */
 			if (!keymgr_transition_allowed(keyring, dkey, i,
-						       next_state)) {
+						       next_state,
+						       secure_to_insecure))
+			{
 				/* No, this would make the zone bogus. */
 				isc_log_write(
 					dns_lctx, DNS_LOGCATEGORY_DNSSEC,
@@ -1677,6 +1686,7 @@ dns_keymgr_run(const dns_name_t *origin, dns_rdataclass_t rdclass,
 	dns_dnsseckey_t *newkey = NULL;
 	isc_dir_t dir;
 	bool dir_open = false;
+	bool secure_to_insecure = false;
 	int options = (DST_TYPE_PRIVATE | DST_TYPE_PUBLIC | DST_TYPE_STATE);
 	char keystr[DST_KEY_FORMATSIZE];
 
@@ -1832,8 +1842,14 @@ dns_keymgr_run(const dns_name_t *origin, dns_rdataclass_t rdclass,
 		ISC_LIST_APPENDLIST(*keyring, newkeys, link);
 	}
 
+	/*
+	 * If the policy has an empty key list, this means the zone is going
+	 * back to unsigned.
+	 */
+	secure_to_insecure = dns_kasp_keylist_empty(kasp);
+
 	/* Read to update key states. */
-	keymgr_update(keyring, kasp, now, nexttime);
+	keymgr_update(keyring, kasp, now, nexttime, secure_to_insecure);
 
 	/* Store key states and update hints. */
 	for (dns_dnsseckey_t *dkey = ISC_LIST_HEAD(*keyring); dkey != NULL;

From 2fc42b598b954f8cbd8a5fc99cc990b15b235b40 Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Mon, 7 Dec 2020 14:37:23 +0100
Subject: [PATCH 7/9] Fix a quirky mkeys test failure

The mkeys system test started to fail after introducing support for
zones transitioning to unsigned without going bogus. This is because
there was actually a bug in the code: if you reconfigure a zone and
remove the "auto-dnssec" option, the zone is actually still DNSSEC
maintained. This is because in zoneconf.c there is no call
to 'dns_zone_setkeyopt()' if the configuration option is not used
(cfg_map_get(zoptions, "auto-dnssec", &obj) will return an error).

The mkeys system test implicitly relied on this bug: initially the
root zone is being DNSSEC maintained, then at some point it needs to
reset the root zone in order to prepare for some tests with bad
signatures. Because it needs to inject a bad signature, 'auto-dnssec'
is removed from the configuration.

The test pass but for the wrong reasons:

I:mkeys:reset the root server
I:mkeys:reinitialize trust anchors
I:mkeys:check positive validation (18)

The 'check positive validation' test works because the zone is still
DNSSEC maintained: The DNSSEC records in the signed root zone file on
disk are being ignored.

After fixing the bug/introducing graceful transition to insecure,
the root zone is no longer DNSSEC maintained after the reconfig.

The zone now explicitly needs to be reloaded because otherwise the
'check positive validation' test works against an old version of the
zone (the one with all the revoked keys), and the test will obviously
fail.
---
 bin/tests/system/mkeys/tests.sh | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/bin/tests/system/mkeys/tests.sh b/bin/tests/system/mkeys/tests.sh
index 31b3b64ad8..2debe51e14 100644
--- a/bin/tests/system/mkeys/tests.sh
+++ b/bin/tests/system/mkeys/tests.sh
@@ -462,7 +462,9 @@ grep "example..*.RRSIG..*TXT" dig.out.ns2.test$n > /dev/null || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=$((status+ret))
 
-echo_i "reset the root server"
+n=$((n+1))
+echo_i "reset the root server ($n)"
+ret=0
 $SETTIME -D none -R none -K ns1 "$original" > /dev/null
 $SETTIME -D now -K ns1 "$standby1" > /dev/null
 $SETTIME -D now -K ns1 "$standby2" > /dev/null
@@ -470,6 +472,9 @@ $SIGNER -Sg -K ns1 -N unixtime -o . ns1/root.db > /dev/null 2>/dev/null
 copy_setports ns1/named2.conf.in ns1/named.conf
 rm -f ns1/root.db.signed.jnl
 mkeys_reconfig_on 1 || ret=1
+mkeys_reload_on 1 || ret=1
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=$((status+ret))
 
 echo_i "reinitialize trust anchors"
 stop_server --use-rndc --port "${CONTROLPORT}" mkeys ns2

From 7825d8f916bcfb0e725f0db5402035fd5c48a432 Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Tue, 8 Dec 2020 09:42:51 +0100
Subject: [PATCH 8/9] Add documentation and notes for [#1750]

---
 CHANGES                     | 9 +++++++++
 doc/misc/rfc-compliance     | 1 +
 doc/notes/notes-current.rst | 6 +++++-
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/CHANGES b/CHANGES
index 367a14341a..c5b665997f 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,12 @@
+5553.	[bug]		When reconfiguring named, removing "auto-dnssec"
+			actually did not turn off DNSSEC maintenance.
+			This has been fixed. [GL #2341]
+
+5552.	[func]		When switching to "dnssec-policy none;", named
+			now permits a safe transition to insecure mode
+			and publishes the CDS and CDNSKEY DELETE
+			records, as described in RFC 8078. [GL #1750]
+
 5551.	[bug]		Only assign threads to CPUs in the CPU affinity set.
 			Thanks to Ole Bjørn Hessen. [GL #2245]
 
diff --git a/doc/misc/rfc-compliance b/doc/misc/rfc-compliance
index 07e9735377..c1694c9b16 100644
--- a/doc/misc/rfc-compliance
+++ b/doc/misc/rfc-compliance
@@ -98,6 +98,7 @@ or Best Current Practice (BCP) documents.  The list is non exhaustive.
   RFC7793
   RFC7830 [15]
   RFC7929
+  RFC8078 [20]
   RFC8080
   RFC8880
 
diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst
index c26e11c919..8f697929a3 100644
--- a/doc/notes/notes-current.rst
+++ b/doc/notes/notes-current.rst
@@ -34,7 +34,11 @@ Removed Features
 Feature Changes
 ~~~~~~~~~~~~~~~
 
-- ``ipv4only.arpa`` is now served when ``dns64`` is configured. [GL #385]
+- It is now possible to transition a zone from secure to insecure mode
+  without making it bogus in the process: changing to ``dnssec-policy
+  none;`` also causes CDS and CDNSKEY DELETE records to be published, to
+  signal that the entire DS RRset at the parent must be removed, as
+  described in RFC 8078. [GL #1750]
 
 - When using the ``unixtime`` or ``date`` method to update the SOA
   serial number, ``named`` and ``dnssec-signzone`` silently fell back to

From 08b6e8c2c9218928537a6b6a4f72cccc5e1faea6 Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Tue, 8 Dec 2020 09:55:36 +0100
Subject: [PATCH 9/9] Add notes for [#2341]

Mention the bugfix in the release.
---
 CHANGES                     | 2 +-
 doc/notes/notes-current.rst | 5 ++---
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/CHANGES b/CHANGES
index c5b665997f..464f770b1d 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,5 +1,5 @@
 5553.	[bug]		When reconfiguring named, removing "auto-dnssec"
-			actually did not turn off DNSSEC maintenance.
+			did not actually turn off DNSSEC maintenance.
 			This has been fixed. [GL #2341]
 
 5552.	[func]		When switching to "dnssec-policy none;", named
diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst
index 8f697929a3..79b09fd595 100644
--- a/doc/notes/notes-current.rst
+++ b/doc/notes/notes-current.rst
@@ -50,6 +50,5 @@ Feature Changes
 Bug Fixes
 ~~~~~~~~~
 
-- Only assign threads to CPUs in the CPU affinity set, so that ``named`` no
-  longer attempts to run threads on CPUs outside the affinity set. Thanks to
-  Ole Bjørn Hessen. [GL #2245]
+- When reconfiguring ``named``, removing ``auto-dnssec`` did actually not turn
+  off DNSSEC maintenance. This has been fixed. [GL #2341]