3635. [bug] Signatures were not being removed from a zone with

                        only KSK keys for a algorithm. [RT #24439]

CHANGES

+3635.	[bug]		Signatures were not being removed from a zone with
+			only KSK keys for a algorithm. [RT #24439]
+
 3634.	[func]		Report build-id in rndc status. Report build-id
 			when building from a git repository. [RT #20422]
 

bin/tests/system/autosign/tests.sh

@@ -44,14 +44,18 @@ showprivate () {
 
 # check that signing records are marked as complete
 checkprivate () {
-    ret=0
+    _ret=0
+    expected="${3:-0}"
     x=`showprivate "$@"`
-    echo $x | grep incomplete >&- 2>&- && ret=1
-    [ $ret = 1 ] && {
-        echo "$x"
-        echo "I:failed"
-    }
-    return $ret
+    echo $x | grep incomplete > /dev/null && _ret=1
+
+    if [ $_ret = $expected ]; then
+        return 0
+    fi
+
+    echo "$x"
+    echo "I:failed"
+    return 1
 }
 
 #
@@ -208,6 +212,7 @@ ret=0
 missing=`sed 's/^K.*+007+0*\([0-9]\)/\1/' < missingzsk.key`
 $JOURNALPRINT ns3/nozsk.example.db.jnl | \
    awk '{if ($1 == "del" && $5 == "RRSIG" && $12 == id) {exit 1}} END {exit 0}' id=$missing || ret=1
+n=`expr $n + 1`
 if [ $ret != 0 ]; then echo "I:failed"; fi
 status=`expr $status + $ret`
 
@@ -216,24 +221,23 @@ ret=0
 inactive=`sed 's/^K.*+007+0*\([0-9]\)/\1/' < inactivezsk.key`
 $JOURNALPRINT ns3/inaczsk.example.db.jnl | \
    awk '{if ($1 == "del" && $5 == "RRSIG" && $12 == id) {exit 1}} END {exit 0}' id=$inactive || ret=1
+n=`expr $n + 1`
 if [ $ret != 0 ]; then echo "I:failed"; fi
 status=`expr $status + $ret`
 
-echo "I:checking that non-replaceable RRSIGs are logged only once ($n)"
+echo "I:checking that non-replaceable RRSIGs are logged only once (missing private key) ($n)"
 ret=0
 loglines=`grep "Key nozsk.example/NSEC3RSASHA1/$missing .* retaining signatures" ns3/named.run | wc -l`
 [ "$loglines" -eq 1 ] || ret=1
-loglines=`grep "Key inaczsk.example/NSEC3RSASHA1/$inactive .* retaining signatures" ns3/named.run | wc -l`
-[ "$loglines" -eq 1 ] || ret=1
+n=`expr $n + 1`
 if [ $ret != 0 ]; then echo "I:failed"; fi
 status=`expr $status + $ret`
 
-echo "I:checking serial is not incremented when signatures are unchanged ($n)"
+echo "I:checking that non-replaceable RRSIGs are logged only once (inactive private key) ($n)"
 ret=0
-newserial=`$DIG $DIGOPTS +short soa nozsk.example @10.53.0.3 | awk '$0 !~ /SOA/ {print $3}'`
-[ "$newserial" -eq 2 ] || ret=1
-newserial=`$DIG $DIGOPTS +short soa inaczsk.example @10.53.0.3 | awk '$0 !~ /SOA/ {print $3}'`
-[ "$newserial" -eq 2 ] || ret=1
+loglines=`grep "Key inaczsk.example/NSEC3RSASHA1/$inactive .* retaining signatures" ns3/named.run | wc -l`
+[ "$loglines" -eq 1 ] || ret=1
+n=`expr $n + 1`
 if [ $ret != 0 ]; then echo "I:failed"; fi
 status=`expr $status + $ret`
 
@@ -943,7 +947,7 @@ checkprivate oldsigs.example 10.53.0.3 || ret=1
 checkprivate optout.example 10.53.0.3 || ret=1
 checkprivate optout.nsec3.example 10.53.0.3 || ret=1
 checkprivate optout.optout.example 10.53.0.3 || ret=1
-checkprivate prepub.example 10.53.0.3 || ret=1
+checkprivate prepub.example 10.53.0.3 1 || ret=1
 checkprivate rsasha256.example 10.53.0.3 || ret=1
 checkprivate rsasha512.example 10.53.0.3 || ret=1
 checkprivate secure.example 10.53.0.3 || ret=1

bin/tests/system/inline/clean.sh

@@ -52,6 +52,10 @@ rm -f ns3/expired.db
 rm -f ns3/expired.db.jnl
 rm -f ns3/expired.db.signed
 rm -f ns3/expired.db.signed.jnl
+rm -f ns3/nsec3.db
+rm -f ns3/nsec3.db.jnl
+rm -f ns3/nsec3.db.signed
+rm -f ns3/nsec3.db.signed.jnl
 rm -f ns3/retransfer.bk
 rm -f ns3/retransfer.bk.jnl
 rm -f ns3/retransfer.bk.signed

bin/tests/system/inline/ns1/root.db.in

@@ -47,3 +47,6 @@ ns3.expired.		A	10.53.0.3
 
 retransfer.			NS	ns3.retransfer.
 ns3.retransfer.		A	10.53.0.3
+
+nsec3.			NS	ns3.nsec3.
+ns3.nsec3.		A	10.53.0.3

bin/tests/system/inline/ns3/named.conf

@@ -95,3 +95,11 @@ zone "retransfer" {
 	auto-dnssec maintain;
 	file "retransfer.bk";
 };
+
+zone "nsec3" {
+	type master;
+	inline-signing yes;
+	auto-dnssec maintain;
+	allow-update { any; };
+	file "nsec3.db";
+};

bin/tests/system/inline/ns3/sign.sh

@@ -74,6 +74,12 @@ keyname=`$KEYGEN -q -r $RANDFILE -a RSASHA1 -b 768 -n zone $zone`
 keyname=`$KEYGEN -q -r $RANDFILE -a RSASHA1 -b 1024 -n zone -f KSK $zone`
 $DSFROMKEY -T 1200 $keyname >> ../ns1/root.db
 
+zone=nsec3
+rm -f K${zone}.+*+*.key
+rm -f K${zone}.+*+*.private
+keyname=`$KEYGEN -q -r $RANDFILE -a NSEC3RSASHA1 -b 1024 -n zone -f KSK $zone`
+$DSFROMKEY -T 1200 $keyname >> ../ns1/root.db
+
 for s in a c d h k l m q z
 do
 	zone=test-$s

bin/tests/system/inline/setup.sh

@@ -28,6 +28,7 @@ cp ns3/master.db.in ns3/master.db
 cp ns3/master.db.in ns3/dynamic.db
 cp ns3/master.db.in ns3/updated.db
 cp ns3/master.db.in ns3/expired.db
+cp ns3/master.db.in ns3/nsec3.db
 
 touch ns4/trusted.conf
 cp ns4/noixfr.db.in ns4/noixfr.db

bin/tests/system/inline/tests.sh

@@ -25,6 +25,24 @@ RANDFILE=random.data
 status=0
 n=0
 
+$RNDC -c ../common/rndc.conf -s 10.53.0.3 -p 9953 signing -nsec3param 1 0 0 - nsec3
+
+for i in 1 2 3 4 5 6 7 8 9 0
+do
+	nsec3param=`$DIG +short @10.53.0.3 -p 5300 nsec3param nsec3.`
+	test -n "$nsec3param" && break
+	sleep 1
+done
+
+n=`expr $n + 1`
+echo "I:checking that rrsigs are replaced with ksk only"
+ret=0
+$DIG @10.53.0.3 -p 5300 axfr nsec3. |
+	awk '/RRSIG NSEC3/ {a[$1]++} END { for (i in a) {if (a[i] != 1) exit (1)}}' || ret=1
+#$DIG @10.53.0.3 -p 5300 axfr nsec3. | grep -w NSEC | grep -v "IN.RRSIG.NSEC" 
+if [ $ret != 0 ]; then echo "I:failed"; fi
+status=`expr $status + $ret`
+
 n=`expr $n + 1`
 echo "I:checking that the zone is signed on initial transfer ($n)"
 ret=0

lib/dns/zone.c

@@ -752,8 +752,6 @@ static isc_result_t delete_nsec(dns_db_t *db, dns_dbversion_t *ver,
 				dns_dbnode_t *node, dns_name_t *name,
 				dns_diff_t *diff);
 static void zone_rekey(dns_zone_t *zone);
-static isc_boolean_t delsig_ok(dns_rdata_rrsig_t *rrsig_ptr,
-			       dst_key_t **keys, unsigned int nkeys);
 static isc_result_t zone_send_securedb(dns_zone_t *zone, isc_boolean_t locked,
 				       dns_db_t *db);
 
@@ -5609,19 +5607,39 @@ set_key_expiry_warning(dns_zone_t *zone, isc_stdtime_t when, isc_stdtime_t now)
  * have no new key.
  */
 static isc_boolean_t
-delsig_ok(dns_rdata_rrsig_t *rrsig_ptr, dst_key_t **keys, unsigned int nkeys) {
+delsig_ok(dns_rdata_rrsig_t *rrsig_ptr, dst_key_t **keys, unsigned int nkeys,
+	  isc_boolean_t *warn)
+{
 	unsigned int i = 0;
+	isc_boolean_t have_ksk = ISC_FALSE, have_zsk = ISC_FALSE;
+	isc_boolean_t have_pksk = ISC_FALSE, have_pzsk = ISC_FALSE;
 
-	/*
-	 * It's okay to delete a signature if there is an active ZSK
-	 * with the same algorithm
-	 */
 	for (i = 0; i < nkeys; i++) {
-		if (rrsig_ptr->algorithm == dst_key_alg(keys[i]) &&
-		    (dst_key_isprivate(keys[i])) && !KSK(keys[i]))
-			return (ISC_TRUE);
+		if (rrsig_ptr->algorithm != dst_key_alg(keys[i]))
+			continue;
+		if (dst_key_isprivate(keys[i])) {
+			if (KSK(keys[i]))
+				have_ksk = have_pksk = ISC_TRUE;
+			else
+				have_zsk = have_pzsk = ISC_TRUE;
+		} else {
+			if (KSK(keys[i]))
+				have_ksk = ISC_TRUE;
+			else
+				have_zsk = ISC_TRUE;
+		}
 	}
 
+	if (have_zsk && have_ksk && !have_pzsk)
+		*warn = ISC_TRUE;
+
+	/*
+	 * It's okay to delete a signature if there is an active key
+	 * with the same algorithm to replace it.
+	 */
+	if (have_pksk || have_pzsk)
+		return (ISC_TRUE);
+
 	/*
 	 * Failing that, it is *not* okay to delete a signature
 	 * if the associated public key is still in the DNSKEY RRset
@@ -5689,7 +5707,8 @@ del_sigs(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name,
 		RUNTIME_CHECK(result == ISC_R_SUCCESS);
 
 		if (type != dns_rdatatype_dnskey) {
-			if (delsig_ok(&rrsig, keys, nkeys)) {
+			isc_boolean_t warn = ISC_FALSE, deleted = ISC_FALSE;
+			if (delsig_ok(&rrsig, keys, nkeys, &warn)) {
 				result = update_one_rr(db, ver, zonediff->diff,
 					       DNS_DIFFOP_DELRESIGN, name,
 					       rdataset.ttl, &rdata);
@@ -5697,7 +5716,9 @@ del_sigs(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name,
 					changed = ISC_TRUE;
 				if (result != ISC_R_SUCCESS)
 					break;
-			} else {
+				deleted = ISC_TRUE;
+			}
+			if (warn) {
 				/*
 				 * At this point, we've got an RRSIG,
 				 * which is signed by an inactive key.
@@ -5707,7 +5728,7 @@ del_sigs(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name,
 				 * offline will prevent us spinning waiting
 				 * for the private part.
 				 */
-				if (incremental) {
+				if (incremental && !deleted) {
 					result = offline(db, ver, zonediff,
 							 name, rdataset.ttl,
 							 &rdata);