Merge branch 'michal-befix-squashed-v9_11' into 'v9_11'

Fix bottom of zone determination.

See merge request isc-projects/bind9!1200

CHANGES

@@ -1,3 +1,7 @@
+5108.	[bug]		Named could fail to determine bottom of zone when
+			removing out of date keys leading to invalid NSEC
+			and NSEC3 records being added to the zone. [GL #771]
+
 5107.	[bug]		'host -U' did not work.	[GL #769]
 
 5104.	[cleanup]	Log clearer informational message when a catz zone

bin/tests/system/autosign/clean.sh

@@ -16,7 +16,7 @@ rm -f */named.memstats
 rm -f */named.run
 rm -f */named.conf
 rm -f activate-now-publish-1day.key
-rm -f active.key inact.key del.key unpub.key standby.key rev.key
+rm -f active.key inact.key del.key delzsk.key unpub.key standby.key rev.key
 rm -f delayksk.key delayzsk.key autoksk.key autozsk.key
 rm -f dig.out.*
 rm -f digcomp.out.test*
@@ -35,6 +35,7 @@ rm -f ns3/inacksk2.example.db
 rm -f ns3/inacksk3.example.db
 rm -f ns3/inaczsk2.example.db
 rm -f ns3/inaczsk3.example.db
+rm -f ns3/delzsk.example.db
 rm -f ns3/kg.out ns3/s.out ns3/st.out
 rm -f ns3/nozsk.example.db ns3/inaczsk.example.db
 rm -f ns3/nsec.example.db

bin/tests/system/autosign/ns3/delzsk.example.db.in

@@ -0,0 +1,23 @@
+; Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+;
+; This Source Code Form is subject to the terms of the Mozilla Public
+; License, v. 2.0. If a copy of the MPL was not distributed with this
+; file, You can obtain one at http://mozilla.org/MPL/2.0/.
+;
+; See the COPYRIGHT file distributed with this work for additional
+; information regarding copyright ownership.
+
+$TTL 300	; 5 minutes
+@			IN SOA	mname1. . (
+				2000010101 ; serial
+				20         ; refresh (20 seconds)
+				20         ; retry (20 seconds)
+				1814400    ; expire (3 weeks)
+				3600       ; minimum (1 hour)
+				)
+			NS	ns
+ns			A	10.53.0.3
+
+sub			NS	ns.sub
+			DS	12345 8 1 0000000000000000000000000000000000000000
+ns.sub			A	10.53.0.3

bin/tests/system/autosign/ns3/keygen.sh

@@ -305,3 +305,14 @@ ksk=`$KEYGEN -a NSEC3RSASHA1 -b 1024 -3 -q -r $RANDFILE -fk $zone 2> kg.out` ||
 $KEYGEN -a NSEC3RSASHA1 -b 1024 -3 -q -r $RANDFILE $zone > kg.out 2>&1 || dumpit kg.out
 $KEYGEN -a NSEC3RSASHA1 -b 1024 -3 -q -r $RANDFILE -P now -A now+3600 $zone > kg.out 2>&1 || dumpit kg.out
 $DSFROMKEY $ksk.key > dsset-${zone}$TP
+
+#
+# A zone that starts with an active KSK + ZSK and an inactive ZSK, with the
+# latter getting deleted during the test.
+#
+setup delzsk.example
+cp $infile $zonefile
+ksk=`$KEYGEN -a NSEC3RSASHA1 -b 1024 -3 -q -fk $zone 2> kg.out` || dumpit kg.out
+$KEYGEN -a NSEC3RSASHA1 -b 1024 -3 -q $zone > kg.out 2>&1 || dumpit kg.out
+zsk=`$KEYGEN -a NSEC3RSASHA1 -b 1024 -3 -q -I now-1w $zone 2>kg.out` || dumpit kg.out
+echo $zsk > ../delzsk.key

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

@@ -274,4 +274,11 @@ zone "inaczsk3.example" {
 	auto-dnssec maintain;
 };
 
+zone "delzsk.example." {
+	type master;
+	file "delzsk.example.db";
+	allow-update { any; };
+	auto-dnssec maintain;
+};
+
 include "trusted.conf";

bin/tests/system/autosign/tests.sh

@@ -1370,5 +1370,59 @@ n=`expr $n + 1`
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
+echo_i "checking for out-of-zone NSEC3 records after ZSK removal ($n)"
+ret=0
+# Switch the zone over to NSEC3 and wait until the transition is complete.
+$RNDCCMD 10.53.0.3 signing -nsec3param 1 1 10 12345678 delzsk.example. > signing.out.1.test$n 2>&1 || ret=1
+for i in 0 1 2 3 4 5 6 7 8 9; do
+	_ret=1
+	$DIG $DIGOPTS delzsk.example NSEC3PARAM @10.53.0.3 > dig.out.ns3.1.test$n 2>&1 || ret=1
+	grep "NSEC3PARAM.*12345678" dig.out.ns3.1.test$n > /dev/null 2>&1
+	if [ $? -eq 0 ]; then
+		_ret=0
+		break
+	fi
+	sleep 1
+done
+if [ $_ret -ne 0 ]; then
+	echo_i "timed out waiting for NSEC3 chain creation"
+	ret=1
+fi
+# Mark the inactive ZSK as pending removal.
+file="ns3/`cat delzsk.key`.key"
+$SETTIME -D now-1h $file > settime.out.test$n 2>&1 || ret=1
+# Trigger removal of the inactive ZSK and wait until its completion.
+$RNDCCMD 10.53.0.3 loadkeys delzsk.example 2>&1 | sed 's/^/ns3 /' | cat_i
+for i in 0 1 2 3 4 5 6 7 8 9; do
+	_ret=1
+	$RNDCCMD 10.53.0.3 signing -list delzsk.example > signing.out.2.test$n 2>&1
+	grep "Signing " signing.out.2.test$n > /dev/null 2>&1
+	if [ $? -ne 0 ]; then
+		if [ `cat signing.out.2.test$n | wc -l` -eq 2 ]; then
+			_ret=0
+			break
+		fi
+	fi
+	sleep 1
+done
+if [ $_ret -ne 0 ]; then
+	echo_i "timed out waiting for key removal"
+	ret=1
+fi
+# Check whether key removal caused NSEC3 records to be erroneously created for
+# glue records due to a secure delegation already being signed by the active key
+# (i.e. a key other than the one being removed but using the same algorithm).
+#
+# For reference:
+#
+#     $ nsec3hash 12345678 1 10 ns.sub.delzsk.example.
+#     589R358VSPJUFVAJU949JPVF74D9PTGH (salt=12345678, hash=1, iterations=10)
+#
+$DIG $DIGOPTS delzsk.example AXFR @10.53.0.3 > dig.out.ns3.3.test$n || ret=1
+grep "589R358VSPJUFVAJU949JPVF74D9PTGH" dig.out.ns3.3.test$n > /dev/null 2>&1 && ret=1
+n=`expr $n + 1`
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=`expr $status + $ret`
+
 echo_i "exit status: $status"
 [ $status -eq 0 ] || exit 1

doc/arm/notes.xml

@@ -92,6 +92,20 @@
 	  remote queries. This flaw is disclosed in CVE-2018-5738. [GL #309]
 	</para>
       </listitem>
+      <listitem>
+	<para>
+	  Code change #4964, intended to prevent double signatures
+	  when deleting an inactive zone DNSKEY in some situations,
+	  introduced a new problem during zone processing in which
+	  some delegation glue RRsets are incorrectly identified
+	  as needing RRSIGs, which are then created for them using
+	  the current active ZSK for the zone. In some, but not all
+	  cases, the newly-signed RRsets are added to the zone's
+	  NSEC/NSEC3 chain, but incompletely -- this can result in
+	  a broken chain, affecting validation of proof of nonexistence
+	  for records in the zone. [GL #771]
+	</para>
+      </listitem>
     </itemizedlist>
   </section>
 

lib/dns/zone.c

@@ -6806,13 +6806,64 @@ add_nsec(dns_db_t *db, dns_dbversion_t *version, dns_name_t *name,
 	return (result);
 }
 
+static isc_result_t
+check_if_bottom_of_zone(dns_db_t *db, dns_dbnode_t *node,
+			dns_dbversion_t *version, bool *is_bottom_of_zone)
+{
+	isc_result_t result;
+	dns_rdatasetiter_t *iterator = NULL;
+	dns_rdataset_t rdataset;
+	bool seen_soa = false, seen_ns = false, seen_dname = false;
+
+	REQUIRE(is_bottom_of_zone != NULL);
+
+	result = dns_db_allrdatasets(db, node, version, 0, &iterator);
+	if (result != ISC_R_SUCCESS) {
+		if (result == ISC_R_NOTFOUND) {
+			result = ISC_R_SUCCESS;
+		}
+		return (result);
+	}
+
+	dns_rdataset_init(&rdataset);
+	for (result = dns_rdatasetiter_first(iterator);
+	     result == ISC_R_SUCCESS;
+	     result = dns_rdatasetiter_next(iterator)) {
+		dns_rdatasetiter_current(iterator, &rdataset);
+		switch (rdataset.type) {
+		case dns_rdatatype_soa:
+			seen_soa = true;
+			break;
+		case dns_rdatatype_ns:
+			seen_ns = true;
+			break;
+		case dns_rdatatype_dname:
+			seen_dname = true;
+			break;
+		}
+		dns_rdataset_disassociate(&rdataset);
+	}
+	if (result != ISC_R_NOMORE) {
+		goto failure;
+	}
+	if ((seen_ns && !seen_soa) || seen_dname) {
+		*is_bottom_of_zone = true;
+	}
+	result = ISC_R_SUCCESS;
+
+ failure:
+	dns_rdatasetiter_destroy(&iterator);
+
+	return (result);
+}
+
 static isc_result_t
 sign_a_node(dns_db_t *db, dns_name_t *name, dns_dbnode_t *node,
 	    dns_dbversion_t *version, bool build_nsec3,
 	    bool build_nsec, dst_key_t *key,
 	    isc_stdtime_t inception, isc_stdtime_t expire,
 	    unsigned int minimum, bool is_ksk,
-	    bool keyset_kskonly, bool *delegation,
+	    bool keyset_kskonly, bool is_bottom_of_zone,
 	    dns_diff_t *diff, int32_t *signatures, isc_mem_t *mctx)
 {
 	isc_result_t result;
@@ -6823,7 +6874,6 @@ sign_a_node(dns_db_t *db, dns_name_t *name, dns_dbnode_t *node,
 	unsigned char data[1024];
 	bool seen_soa, seen_ns, seen_rr, seen_dname, seen_nsec,
 		      seen_nsec3, seen_ds;
-	bool bottom;
 
 	result = dns_db_allrdatasets(db, node, version, 0, &iterator);
 	if (result != ISC_R_SUCCESS) {
@@ -6858,8 +6908,6 @@ sign_a_node(dns_db_t *db, dns_name_t *name, dns_dbnode_t *node,
 	}
 	if (result != ISC_R_NOMORE)
 		goto failure;
-	if (seen_ns && !seen_soa)
-		*delegation = true;
 	/*
 	 * Going from insecure to NSEC3.
 	 * Don't generate NSEC3 records for NSEC3 records.
@@ -6875,14 +6923,12 @@ sign_a_node(dns_db_t *db, dns_name_t *name, dns_dbnode_t *node,
 	 * Don't generate NSEC records for NSEC3 records.
 	 */
 	if (build_nsec && !seen_nsec3 && !seen_nsec && seen_rr) {
-		/* Build and add NSEC. */
-		bottom = (seen_ns && !seen_soa) || seen_dname;
 		/*
 		 * Build a NSEC record except at the origin.
 		 */
 		if (!dns_name_equal(name, dns_db_origin(db))) {
 			CHECK(add_nsec(db, version, name, node, minimum,
-				       bottom, diff));
+				       is_bottom_of_zone, diff));
 			/* Count a NSEC generation as a signature generation. */
 			(*signatures)--;
 		}
@@ -6904,7 +6950,7 @@ sign_a_node(dns_db_t *db, dns_name_t *name, dns_dbnode_t *node,
 			    rdataset.type != dns_rdatatype_cdnskey)
 				goto next_rdataset;
 		}
-		if (*delegation &&
+		if (seen_ns && !seen_soa &&
 		    rdataset.type != dns_rdatatype_ds &&
 		    rdataset.type != dns_rdatatype_nsec)
 			goto next_rdataset;
@@ -6926,8 +6972,6 @@ sign_a_node(dns_db_t *db, dns_name_t *name, dns_dbnode_t *node,
 	}
 	if (result == ISC_R_NOMORE)
 		result = ISC_R_SUCCESS;
-	if (seen_dname)
-		*delegation = true;
  failure:
 	if (dns_rdataset_isassociated(&rdataset))
 		dns_rdataset_disassociate(&rdataset);
@@ -8408,7 +8452,7 @@ zone_sign(dns_zone_t *zone) {
 	bool check_ksk, keyset_kskonly, is_ksk;
 	bool with_ksk, with_zsk;
 	bool commit = false;
-	bool delegation;
+	bool is_bottom_of_zone;
 	bool build_nsec = false;
 	bool build_nsec3 = false;
 	bool first;
@@ -8534,7 +8578,7 @@ zone_sign(dns_zone_t *zone) {
 		if (signing->db != db)
 			goto next_signing;
 
-		delegation = false;
+		is_bottom_of_zone = false;
 
 		if (first && signing->deleteit) {
 			/*
@@ -8589,7 +8633,7 @@ zone_sign(dns_zone_t *zone) {
 				 * we skip all obscured names.
 				 */
 				dns_name_copy(found, name, NULL);
-				delegation = true;
+				is_bottom_of_zone = true;
 				goto next_node;
 			}
 		}
@@ -8600,6 +8644,10 @@ zone_sign(dns_zone_t *zone) {
 		with_ksk = false;
 		with_zsk = false;
 		dns_dbiterator_pause(signing->dbiterator);
+
+		CHECK(check_if_bottom_of_zone(db, node, version,
+					      &is_bottom_of_zone));
+
 		for (i = 0; !has_alg && i < nkeys; i++) {
 			bool both = false;
 
@@ -8687,7 +8735,7 @@ zone_sign(dns_zone_t *zone) {
 					  build_nsec, zone_keys[i], inception,
 					  expire, zone->minimum, is_ksk,
 					  (both && keyset_kskonly),
-					  &delegation, zonediff.diff,
+					  is_bottom_of_zone, zonediff.diff,
 					  &signatures, zone->mctx));
 			/*
 			 * If we are adding we are done.  Look for other keys
@@ -8756,7 +8804,7 @@ zone_sign(dns_zone_t *zone) {
 					"zone_sign:dns_dbiterator_next -> %s",
 					     dns_result_totext(result));
 				goto failure;
-			} else if (delegation) {
+			} else if (is_bottom_of_zone) {
 				dns_dbiterator_current(signing->dbiterator,
 						       &node, nextname);
 				dns_db_detachnode(db, &node);

util/copyrights

@@ -608,6 +608,7 @@
 ./bin/tests/system/autosign/ns2/private.secure.example.db.in	ZONE	2009,2016,2018
 ./bin/tests/system/autosign/ns3/autonsec3.example.db.in	ZONE	2011,2016,2018
 ./bin/tests/system/autosign/ns3/delay.example.db	ZONE	2011,2016,2018
+./bin/tests/system/autosign/ns3/delzsk.example.db.in	ZONE	2018
 ./bin/tests/system/autosign/ns3/inacksk2.example.db.in	ZONE	2017,2018
 ./bin/tests/system/autosign/ns3/inacksk3.example.db.in	ZONE	2017,2018
 ./bin/tests/system/autosign/ns3/inaczsk.example.db.in	ZONE	2011,2016,2018