From 695be761b0028b75f799dabd5743a9d42491918f Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Thu, 22 Feb 2024 15:18:30 +0100
Subject: [PATCH] Test dnssec-ksr sign

Add test cases for the 'sign' command. Reuse the earlier generated KSR
files.

Also update dnssec-ksr.c to have better cleanup.
---
 bin/dnssec/dnssec-ksr.c       |  46 ++++++-
 bin/tests/system/ksr/setup.sh |   3 +
 bin/tests/system/ksr/tests.sh | 232 ++++++++++++++++++++++++++++++++--
 3 files changed, 268 insertions(+), 13 deletions(-)

diff --git a/bin/dnssec/dnssec-ksr.c b/bin/dnssec/dnssec-ksr.c
index a5ebc28d65..1fb449bd14 100644
--- a/bin/dnssec/dnssec-ksr.c
+++ b/bin/dnssec/dnssec-ksr.c
@@ -520,6 +520,8 @@ print_dnskeys(dns_kasp_key_t *kaspkey, dns_ttl_t ttl, dns_dnsseckeylist_t *keys,
 	char timestr[26]; /* Minimal buf as per ctime_r() spec. */
 	dns_rdatalist_t *rdatalist = NULL;
 	dns_rdataset_t rdataset = DNS_RDATASET_INIT;
+	isc_bufferlist_t cleanup = ISC_LIST_INITIALIZER;
+	isc_buffer_t *cbuf = NULL;
 	isc_result_t ret = ISC_R_SUCCESS;
 	isc_stdtime_t next_bundle = next_inception;
 
@@ -583,6 +585,8 @@ print_dnskeys(dns_kasp_key_t *kaspkey, dns_ttl_t ttl, dns_dnsseckeylist_t *keys,
 		dns_rdata_fromregion(rdata, dns_rdataclass_in,
 				     dns_rdatatype_dnskey, &r);
 		ISC_LIST_APPEND(rdatalist->rdata, rdata, link);
+		ISC_LIST_APPEND(cleanup, newbuf, link);
+		isc_buffer_clear(newbuf);
 	}
 	/* Error if no key pair found. */
 	if (ISC_LIST_EMPTY(rdatalist->rdata)) {
@@ -595,6 +599,17 @@ print_dnskeys(dns_kasp_key_t *kaspkey, dns_ttl_t ttl, dns_dnsseckeylist_t *keys,
 	print_rdata(&rdataset);
 
 fail:
+	/* Cleanup */
+	freerrset(&rdataset);
+
+	cbuf = ISC_LIST_HEAD(cleanup);
+	while (cbuf != NULL) {
+		isc_buffer_t *nbuf = ISC_LIST_NEXT(cbuf, link);
+		ISC_LIST_UNLINK(cleanup, cbuf, link);
+		isc_buffer_free(&cbuf);
+		cbuf = nbuf;
+	}
+
 	if (ret != ISC_R_SUCCESS) {
 		fatal("failed to print %s/%s %s key pair found for bundle %s",
 		      namestr, algstr, rolestr, timestr);
@@ -610,6 +625,8 @@ sign_rrset(ksr_ctx_t *ksr, isc_stdtime_t inception, isc_stdtime_t expiration,
 	char utc[sizeof("YYYYMMDDHHSSMM")];
 	dns_rdatalist_t *rrsiglist = NULL;
 	dns_rdataset_t rrsigset = DNS_RDATASET_INIT;
+	isc_bufferlist_t cleanup = ISC_LIST_INITIALIZER;
+	isc_buffer_t *cbuf = NULL;
 	isc_buffer_t timebuf;
 	isc_buffer_t b;
 	isc_region_t r;
@@ -644,19 +661,17 @@ sign_rrset(ksr_ctx_t *ksr, isc_stdtime_t inception, isc_stdtime_t expiration,
 	{
 		isc_buffer_t buf;
 		isc_buffer_t *newbuf = NULL;
-		dns_rdata_t *rdata = NULL;
+		dns_rdata_t rdata = DNS_RDATA_INIT;
 		dns_rdata_t *rrsig = NULL;
 		isc_region_t rs;
 		unsigned char rdatabuf[SIG_FORMATSIZE];
 		isc_stdtime_t clockskew = inception - 3600;
 
-		rdata = isc_mem_get(mctx, sizeof(*rdata));
 		rrsig = isc_mem_get(mctx, sizeof(*rrsig));
-		dns_rdata_init(rdata);
 		dns_rdata_init(rrsig);
 		isc_buffer_init(&buf, rdatabuf, sizeof(rdatabuf));
 		ret = dns_dnssec_sign(name, rrset, dk->key, &clockskew,
-				      &expiration, mctx, &buf, rdata);
+				      &expiration, mctx, &buf, &rdata);
 		if (ret != ISC_R_SUCCESS) {
 			fatal("failed to sign KSR");
 		}
@@ -667,11 +682,20 @@ sign_rrset(ksr_ctx_t *ksr, isc_stdtime_t inception, isc_stdtime_t expiration,
 		dns_rdata_fromregion(rrsig, dns_rdataclass_in,
 				     dns_rdatatype_rrsig, &rs);
 		ISC_LIST_APPEND(rrsiglist->rdata, rrsig, link);
+		ISC_LIST_APPEND(cleanup, newbuf, link);
 		isc_buffer_clear(newbuf);
 	}
 	dns_rdatalist_tordataset(rrsiglist, &rrsigset);
 	print_rdata(&rrsigset);
 	freerrset(&rrsigset);
+
+	cbuf = ISC_LIST_HEAD(cleanup);
+	while (cbuf != NULL) {
+		isc_buffer_t *nbuf = ISC_LIST_NEXT(cbuf, link);
+		ISC_LIST_UNLINK(cleanup, cbuf, link);
+		isc_buffer_free(&cbuf);
+		cbuf = nbuf;
+	}
 }
 
 static void
@@ -871,6 +895,8 @@ sign(ksr_ctx_t *ksr) {
 	dns_dnsseckeylist_t keys;
 	dns_kasp_t *kasp = NULL;
 	dns_rdatalist_t *rdatalist = NULL;
+	isc_bufferlist_t cleanup_list = ISC_LIST_INITIALIZER;
+	isc_buffer_t *cbuf = NULL;
 	isc_result_t ret;
 	isc_stdtime_t inception;
 	isc_lex_t *lex = NULL;
@@ -985,6 +1011,8 @@ sign(ksr_ctx_t *ksr) {
 			isc_region_t r;
 			u_char rdatabuf[DST_KEY_MAXSIZE];
 
+			INSIST(rdatalist != NULL);
+
 			rdata = isc_mem_get(mctx, sizeof(*rdata));
 			dns_rdata_init(rdata);
 			isc_buffer_init(&buf, rdatabuf, sizeof(rdatabuf));
@@ -1005,6 +1033,8 @@ sign(ksr_ctx_t *ksr) {
 			}
 
 			ISC_LIST_APPEND(rdatalist->rdata, rdata, link);
+			ISC_LIST_APPEND(cleanup_list, newbuf, link);
+			isc_buffer_clear(newbuf);
 		}
 	}
 
@@ -1028,6 +1058,14 @@ sign(ksr_ctx_t *ksr) {
 
 fail:
 	/* Clean up */
+	cbuf = ISC_LIST_HEAD(cleanup_list);
+	while (cbuf != NULL) {
+		isc_buffer_t *nbuf = ISC_LIST_NEXT(cbuf, link);
+		ISC_LIST_UNLINK(cleanup_list, cbuf, link);
+		isc_buffer_free(&cbuf);
+		cbuf = nbuf;
+	}
+
 	isc_lex_destroy(&lex);
 	cleanup(&keys, kasp);
 }
diff --git a/bin/tests/system/ksr/setup.sh b/bin/tests/system/ksr/setup.sh
index ae272a2049..e4cc852b23 100644
--- a/bin/tests/system/ksr/setup.sh
+++ b/bin/tests/system/ksr/setup.sh
@@ -19,6 +19,7 @@ set -e
 $SHELL clean.sh
 
 mkdir keydir
+mkdir offline
 
 copy_setports named.conf.in named.conf
 
@@ -30,6 +31,8 @@ create_ksk () {
 	do
 		num=$(($num+1))
 		cat "${ksk}.key" | grep -v ";.*" > "$1.ksk$num"
+		cp "${ksk}.key" offline/
+		cp "${ksk}.private" offline/
 	done
 }
 create_ksk common.test common
diff --git a/bin/tests/system/ksr/tests.sh b/bin/tests/system/ksr/tests.sh
index 06171ca971..8eff4af0af 100644
--- a/bin/tests/system/ksr/tests.sh
+++ b/bin/tests/system/ksr/tests.sh
@@ -42,8 +42,8 @@ EOF
         fi
 }
 
-# Check keys that were created. The keys created are listed in the latest ksr output
-# file, ksr.keygen.out.$n.
+# Check keys that were created. The keys created are listed in the latest ksr
+# output file, ksr.keygen.out.$n.
 # $1: zone name
 # $2: key directory
 check_keys () (
@@ -161,7 +161,7 @@ n=$((n+1))
 echo_i "check that 'dnssec-ksr keygen' selects pregenerated keys for the same time bundle ($n)"
 ret=0
 ksr common -e +1y keygen common.test > ksr.keygen.out.$n 2>&1 || ret=1
-diff ksr.keygen.out.expect ksr.keygen.out.$n > /dev/null|| ret=1
+diff -w ksr.keygen.out.expect ksr.keygen.out.$n > /dev/null|| ret=1
 for key in $(cat ksr.keygen.out.$n)
 do
 	# Ensure the files are not modified.
@@ -208,11 +208,168 @@ cp ksr.request.expect.$n ksr.request.expect.base
 grep ";; KeySigningRequest generated at" ksr.request.out.$n > footer.$n || ret=1
 cat footer.$n >> ksr.request.expect.$n
 # Check if request output is the same as expected.
-diff ksr.request.out.$n ksr.request.expect.$n > /dev/null || ret=1
+diff -w ksr.request.out.$n ksr.request.expect.$n > /dev/null || ret=1
+# Save request for ksr sign operation.
 cp ksr.request.expect.$n ksr.request.expect
 test "$ret" -eq 0 || echo_i "failed"
 status=$((status+ret))
 
+# Sign request: common
+n=$((n+1))
+echo_i "check that 'dnssec-ksr sign' errors on missing KSR file ($n)"
+ret=0
+ksr common -i $now -e +1y sign common.test > ksr.sign.out.$n 2>&1 && ret=1
+grep "dnssec-ksr: fatal: 'sign' requires a KSR file" ksr.sign.out.$n > /dev/null || ret=1
+test "$ret" -eq 0 || echo_i "failed"
+status=$((status+ret))
+
+n=$((n+1))
+echo_i "check that 'dnssec-ksr sign' creates correct SKR in the common case ($n)"
+ret=0
+ksr common -i $now -e +1y -K offline -f ksr.request.expect sign common.test > ksr.sign.out.$n 2>&1 || ret=1
+
+
+_update_expected_zsks() {
+    zsk=$((zsk+1))
+    next=$((next+1))
+    inception=$rollover_done
+    if [ "$next" -le "$numzsks" ]; then
+        key1="${zone}.${DEFAULT_ALGORITHM_NUMBER}.zsk${zsk}"
+        key2="${zone}.${DEFAULT_ALGORITHM_NUMBER}.zsk${next}"
+        zsk1=$(cat $key1.id)
+        zsk2=$(cat $key2.id)
+        rollover_start=$(cat $zsk2.state | grep "Published" | awk '{print $2}')
+        rollover_done=$(cat $zsk1.state | grep "Removed" | awk '{print $2}')
+    else
+        # No more expected rollovers.
+        key1="${zone}.${DEFAULT_ALGORITHM_NUMBER}.zsk${zsk}"
+        zsk1=$(cat $key1.id)
+        rollover_start=$((end+1))
+        rollover_done=$((end+1))
+    fi
+}
+
+check_ksr() {
+	_ret=0
+	zone=$1
+	file=$2
+	start=$3
+	end=$4
+	numzsks=$5
+
+	echo_i "check ksr: zone $1 file $2 from $3 to $4 num-zsk $5"
+
+	# Initial state: not in a rollover, expect a SignedKeyResponse header
+	# on the first line, start with the first ZSK (set zsk=0 so when we
+	# call _update_expected_zsks, zsk is set to 1.
+	rollover=0
+	expect="header"
+	zsk=0
+	next=1
+	rollover_done=$start
+	_update_expected_zsks
+
+	echo_i "check ksr: inception $inception rollover-start $rollover_start rollover-done $rollover_done"
+
+	lineno=0
+	while IFS= read -r line
+	do
+	    # A single signed key response will consist of:
+	    # ;; SignedKeyResponse (header)
+	    # ;; DNSKEY 257 (ksk)
+	    # ;; one or two (during rollover) DNSKEY 256 (zsk1, zsk2)
+	    # ;; RRSIG (rrsig)
+	    err=0
+	    lineno=$((lineno+1))
+
+	    # skip empty lines
+	    if [ -z "$line" ]; then
+	        continue
+	    fi
+
+	    if [ "$expect" = "header" ]; then
+		expected=";; SignedKeyResponse 1.0 $inception"
+	        echo "$(echo $line | tr -s ' ')" | grep "$expected" > /dev/null || err=1
+		next_inception=$(addtime $inception 777600)
+	        expect="ksk"
+	    elif [ "$expect" = "ksk" ]; then
+		expected="$(cat ${zone}.ksk1)"
+	        echo "$(echo $line | tr -s ' ')" | grep "$expected" > /dev/null || err=1
+	        expect="zsk1"
+	    elif [ "$expect" = "zsk1" ]; then
+		expected="$(cat $key1)"
+	        echo "$(echo $line | tr -s ' ')" | grep "$expected" > /dev/null || err=1
+	        expect="rrsig"
+	        [ "$rollover" -eq 1 ] && expect="zsk2"
+	    elif [ "$expect" = "zsk2" ]; then
+		expected="$(cat $key2)"
+	        echo "$(echo $line | tr -s ' ')" | grep "$expected" > /dev/null || err=1
+	        expect="rrsig"
+	    elif [ "$expect" = "rrsig" ]; then
+		expiration=$(addtime $inception 1209600) # signature-validity 14 days
+		inception=$(addtime $inception -3600) # adjust for one hour clock skew
+		expected="${zone}. 3600 IN RRSIG DNSKEY 13 2 3600 $expiration $inception"
+	        echo "$(echo $line | tr -s ' ')" | grep "$expected" > /dev/null || err=1
+
+	        inception=$next_inception
+		expect="header"
+
+		# Update rollover status if required.
+	        if [ "$inception" -ge "$end" ]; then
+	            expect="footer"
+	        elif [ "$inception" -ge "$rollover_done" ]; then
+		    [ "$rollover" -eq 1 ] && inception=$rollover_done
+		    rollover=0
+		    _update_expected_zsks
+		elif [ "$inception" -ge "$rollover_start" ]; then
+		    [ "$rollover" -eq 0 ] && inception=$rollover_start
+		    rollover=1
+		    # Keys will be sorted, so during a rollover a key with a
+		    # lower keytag will be printed first. Update key1/key2 and
+		    # zsk1/zsk2 accordingly.
+		    id1=$(keyfile_to_key_id "$zsk1")
+		    id2=$(keyfile_to_key_id "$zsk2")
+		    if [ $id1 -gt $id2 ]; then
+		        key1="${zone}.${DEFAULT_ALGORITHM_NUMBER}.zsk${next}"
+		        key2="${zone}.${DEFAULT_ALGORITHM_NUMBER}.zsk${zsk}"
+		        zsk1=$(cat $key1.id)
+		        zsk2=$(cat $key2.id)
+		    fi
+		fi
+	    elif [ "$expect" = "footer" ]; then
+	        expected=";; SignedKeyResponse 1.0 generated at"
+	        echo "$(echo $line | tr -s ' ')" | grep "$expected" > /dev/null || err=1
+
+	        expect="eof"
+	    elif [ "$expect" = "eof" ]; then
+		expected="EOF"
+		echo_i "failed: expected EOF"
+		err=1
+	    else
+	        echo_i "failed: bad expect value $expect"
+	        err=1
+	    fi
+
+	    echo "$(echo $line | tr -s ' ')" | grep "$expected" > /dev/null || err=1
+	    if [ "$err" -ne 0 ]; then
+	        echo_i "unexpected data on line $lineno:"
+	        echo_i "line:     $(echo $line | tr -s ' ')"
+	        echo_i "expected: $expected"
+	    fi
+
+	    _ret=$((_ret+err))
+	done < $file
+
+	return $_ret
+}
+
+zsk1=$(cat common.test.$DEFAULT_ALGORITHM_NUMBER.zsk1.id)
+start=$(cat $zsk1.state | grep "Generated" | awk '{print $2}')
+end=$(addtime $start 31536000) # one year
+check_ksr "common.test" "ksr.sign.out.$n" $start $end 2 || ret=1
+test "$ret" -eq 0 || echo_i "failed"
+status=$((status+ret))
+
 # Key generation: common (2)
 n=$((n+1))
 echo_i "check that 'dnssec-ksr keygen' pregenerates keys in the given key-directory ($n)"
@@ -261,7 +418,7 @@ ksr common -i $now -e +1y request common.test > ksr.request.out.$n 2>&1 || ret=1
 cp ksr.request.expect.base ksr.request.expect.$n
 grep ";; KeySigningRequest generated at" ksr.request.out.$n > footer.$n || ret=1
 cat footer.$n >> ksr.request.expect.$n
-diff ksr.request.out.$n ksr.request.expect.$n > /dev/null || ret=1
+diff -w ksr.request.out.$n ksr.request.expect.$n > /dev/null || ret=1
 test "$ret" -eq 0 || echo_i "failed"
 status=$((status+ret))
 
@@ -298,7 +455,9 @@ cat common.test.$DEFAULT_ALGORITHM_NUMBER.zsk4 >> ksr.request.expect.$n
 cp ksr.request.expect.$n ksr.request.expect.base
 grep ";; KeySigningRequest generated at" ksr.request.out.$n > footer.$n || ret=1
 cat footer.$n >> ksr.request.expect.$n
-diff ksr.request.out.$n ksr.request.expect.$n > /dev/null || ret=1
+diff -w ksr.request.out.$n ksr.request.expect.$n > /dev/null || ret=1
+# Save request for ksr sign operation.
+cp ksr.request.expect.$n ksr.request.expect
 test "$ret" -eq 0 || echo_i "failed"
 status=$((status+ret))
 
@@ -310,6 +469,17 @@ grep "dnssec-ksr: fatal: no common.test/ECDSAP256SHA256 zsk key pair found for b
 test "$ret" -eq 0 || echo_i "failed"
 status=$((status+ret))
 
+# Sign request: common (2)
+n=$((n+1))
+echo_i "check that 'dnssec-ksr sign' creates correct SKR with the new interval ($n)"
+ret=0
+ksr common -i $now -e +2y -K offline -f ksr.request.expect sign common.test > ksr.sign.out.$n 2>&1 || ret=1
+start=$(cat $zsk1.state | grep "Generated" | awk '{print $2}')
+end=$(addtime $start 63072000) # two years
+check_ksr "common.test" "ksr.sign.out.$n" $start $end 4 || ret=1
+test "$ret" -eq 0 || echo_i "failed"
+status=$((status+ret))
+
 # Key generation: csk
 n=$((n+1))
 echo_i "check that 'dnssec-ksr keygen' creates no keys for policy with csk ($n)"
@@ -342,6 +512,7 @@ grep "Active: $active" ${key}.state > /dev/null || ret=1
 grep "Retired:" ${key}.state > /dev/null && ret=1
 grep "Removed:" ${key}.state > /dev/null && ret=1
 cat ${key}.key | grep -v ";.*" > unlimited.test.$DEFAULT_ALGORITHM_NUMBER.zsk1
+echo $key > "unlimited.test.${DEFAULT_ALGORITHM_NUMBER}.zsk1.id"
 test "$ret" -eq 0 || echo_i "failed"
 status=$((status+ret))
 
@@ -349,7 +520,7 @@ status=$((status+ret))
 n=$((n+1))
 echo_i "check that 'dnssec-ksr request' creates correct KSR with unlimited zsk ($n)"
 ret=0
-ksr unlimited -i $created -e +10y request unlimited.test > ksr.request.out.$n 2>&1 || ret=1
+ksr unlimited -i $created -e +4y request unlimited.test > ksr.request.out.$n 2>&1 || ret=1
 # Only one bundle: KSK + ZSK
 inception=$(cat $key.state | grep "Generated" | cut -d' ' -f 2-)
 echo ";; KeySigningRequest 1.0 $inception" > ksr.request.expect.$n
@@ -358,7 +529,20 @@ cat unlimited.test.$DEFAULT_ALGORITHM_NUMBER.zsk1 >> ksr.request.expect.$n
 # Footer
 grep ";; KeySigningRequest generated at" ksr.request.out.$n > footer.$n || ret=1
 cat footer.$n >> ksr.request.expect.$n
-diff ksr.request.out.$n ksr.request.expect.$n > /dev/null || ret=1
+diff -w ksr.request.out.$n ksr.request.expect.$n > /dev/null || ret=1
+# Save request for ksr sign operation.
+cp ksr.request.expect.$n ksr.request.expect
+test "$ret" -eq 0 || echo_i "failed"
+status=$((status+ret))
+
+# Sign request: unlimited
+n=$((n+1))
+echo_i "check that 'dnssec-ksr sign' creates correct SKR with unlimited zsk ($n)"
+ret=0
+ksr unlimited -i $created -e +4y -K offline -f ksr.request.expect sign unlimited.test > ksr.sign.out.$n 2>&1 || ret=1
+start=$(cat $key.state | grep "Generated" | awk '{print $2}')
+end=$(addtime $start 126144000) # four years
+check_ksr "unlimited.test" "ksr.sign.out.$n" $start $end 1 || ret=1
 test "$ret" -eq 0 || echo_i "failed"
 status=$((status+ret))
 
@@ -446,10 +630,40 @@ cat two-tone.test.$ALTERNATIVE_ALGORITHM_NUMBER.zsk2 >> ksr.request.expect.$n
 grep ";; KeySigningRequest generated at" ksr.request.out.$n > footer.$n || ret=1
 cat footer.$n >> ksr.request.expect.$n
 # Check the KSR request against the expected request.
-diff ksr.request.out.$n ksr.request.expect.$n > /dev/null || ret=1
+diff -w ksr.request.out.$n ksr.request.expect.$n > /dev/null || ret=1
+# Save request for ksr sign operation.
+cp ksr.request.expect.$n ksr.request.expect
 test "$ret" -eq 0 || echo_i "failed"
 status=$((status+ret))
 
+# Sign request: two-tone
+n=$((n+1))
+echo_i "check that 'dnssec-ksr sign' creates correct SKR with multiple algorithms ($n)"
+ret=0
+ksr two-tone -i $created -e +6mo -K offline -f ksr.request.expect sign two-tone.test > ksr.sign.out.$n 2>&1 || ret=1
+# Weak testing:
+zone="two-tone.test"
+# expect 24 headers (including the footer)
+lines=$(grep ";; SignedKeyResponse 1.0" ksr.sign.out.$n | wc -l)
+test "$lines" -eq 24 || ret=1
+# expect 23 KSKs (for each header one)
+lines=$(grep "DNSKEY.*257 3 8" ksr.sign.out.$n | wc -l)
+test "$lines" -eq 23 || ret=1
+lines=$(grep "DNSKEY.*257 3 13" ksr.sign.out.$n | wc -l)
+test "$lines" -eq 23 || ret=1
+# and thus 23 signatures
+lines=$(grep "RRSIG.*DNSKEY 8" ksr.sign.out.$n | wc -l)
+test "$lines" -eq 23 || ret=1
+lines=$(grep "RRSIG.*DNSKEY 13" ksr.sign.out.$n | wc -l)
+test "$lines" -eq 23 || ret=1
+# expect 25 ZSK (two more for double keys during the rollover)
+lines=$(grep "DNSKEY.*256 3 8" ksr.sign.out.$n | wc -l)
+test "$lines" -eq 25 || ret=1
+lines=$(grep "DNSKEY.*256 3 13" ksr.sign.out.$n | wc -l)
+test "$lines" -eq 25 || ret=1
+
+test "$ret" -eq 0 || echo_i "failed"
+status=$((status+ret))
 
 echo_i "exit status: $status"
 [ $status -eq 0 ] || exit 1