Merge branch '1256-jitter-dynamically-updated-signatures' into 'master'

Resolve "Signature Expiration Jitter not working for dynamic NSEC3 zones"

Closes #1256

See merge request !2451

CHANGES

+5315.	[bug]		Apply the inital RRSIG expiration spread fixed
+			to all dynamically created records in the zone
+			including NSEC3. Also fix the signature clusters
+			when the server has been offline for prolonged
+			period of times. [GL #1256]
+
 5314.	[func]		Added a new statistics variable "tcp-highwater"
 			that reports the maximum number of simultaneous TCP
 			clients BIND has handled while running. [GL #1206]

bin/tests/system/autosign/clean.sh

@@ -39,6 +39,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/jitter.nsec3.example.db
 rm -f ns3/kg.out ns3/s.out ns3/st.out
 rm -f ns3/kskonly.example.db
 rm -f ns3/nozsk.example.db ns3/inaczsk.example.db

bin/tests/system/autosign/ns3/jitter.nsec3.example.db.in

+; 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. . (
+				2000042407 ; 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
+

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

@@ -52,6 +52,21 @@ ksk=`$KEYGEN -q -a RSASHA1 -3 -fk $zone 2> kg.out` || dumpit kg.out
 $KEYGEN -q -a RSASHA1 -3 $zone > kg.out 2>&1 || dumpit kg.out
 $DSFROMKEY $ksk.key > dsset-${zone}$TP
 
+#
+#  Jitter/NSEC3 test zone
+#
+setup jitter.nsec3.example
+cp $infile $zonefile
+count=1
+while [ $count -le 100 ]
+do
+    echo "label${count} IN TXT label${count}" >> $zonefile
+    count=`expr $count + 1`
+done
+# Don't create keys just yet, because the scenario we want to test
+# is an unsigned zone that has a NSEC3PARAM record added with
+# dynamic update before the keys are generated.
+
 #
 #  OPTOUT/NSEC3 test zone
 #
@@ -150,9 +165,16 @@ $DSFROMKEY $ksk.key > dsset-${zone}$TP
 #
 setup oldsigs.example
 cp $infile $zonefile
+count=1
+while [ $count -le 100 ]
+do
+    echo "label${count} IN TXT label${count}" >> $zonefile
+    count=`expr $count + 1`
+done
 $KEYGEN -q -a RSASHA1 -fk $zone > kg.out 2>&1 || dumpit kg.out
 $KEYGEN -q -a RSASHA1 $zone > kg.out 2>&1 || dumpit kg.out
-$SIGNER -PS -s now-1y -e now-6mo -o $zone -f $zonefile $infile > s.out || dumpit s.out
+$SIGNER -PS -s now-1y -e now-6mo -o $zone -f $zonefile.signed $zonefile > s.out || dumpit s.out
+mv $zonefile.signed $zonefile
 
 #
 # NSEC3->NSEC transition test zone.

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

@@ -95,6 +95,14 @@ zone "nsec3.nsec3.example" {
 	auto-dnssec maintain;
 };
 
+zone "jitter.nsec3.example" {
+	type master;
+	file "jitter.nsec3.example.db";
+	allow-update { any; };
+	auto-dnssec maintain;
+	sig-validity-interval 10 2;
+};
+
 zone "secure.nsec3.example" {
 	type master;
 	file "secure.nsec3.example.db";
@@ -178,6 +186,7 @@ zone "oldsigs.example" {
 	file "oldsigs.example.db";
 	allow-update { any; };
 	auto-dnssec maintain;
+	sig-validity-interval 10 2;
 };
 
 zone "prepub.example" {

bin/tests/system/autosign/tests.sh

@@ -50,6 +50,43 @@ checkprivate () {
     return 1
 }
 
+# Check the signatures expiration times.  First check how many signatures
+# there are in total ($rrsigs).  Then see what the distribution of signature
+# expiration times is ($expiretimes).  Ignore the time part for a better
+# modelled distribution.
+checkjitter () {
+	_file=$1
+	_ret=0
+
+	cat $_file | awk '$4 == "RRSIG" {print substr($9,1,8)}' | sort | uniq -c | cat_i
+	_rrsigs=$(cat $_file | awk '$4 == "RRSIG" {print $4}' | cat_i | wc -l)
+	_expiretimes=$(cat $_file | awk '$4 == "RRSIG" {print substr($9,1,8)}' | sort | uniq -c | awk '{print $1}')
+	_count=0
+	_total=0
+	for _num in $_expiretimes
+	do
+		_total=$(($_total + $_num))
+	done
+	# Make sure the total number of numbers matches the number of RRSIGs.
+	test $_total -eq $_rrsigs || _ret=1
+	# Calculate mean: The number of signatures divided over 8 days.
+	_mean=$(($_total / 8))
+	# We expect the number of signatures not to exceed twice the mean.
+	_limit=$(($_mean * 2))
+	# Add an additional margin.
+	_limit=$(($_limit + 10))
+	# Find outliers.
+	for _num in $_expiretimes
+	do
+		if [ $_num -gt $_limit ]; then
+			echo_i "error: too many RRSIG records ($_num) with the same expiration time"
+			_ret=1
+		fi
+	done
+
+	return $_ret
+}
+
 #
 #  The NSEC record at the apex of the zone and its RRSIG records are
 #  added as part of the last step in signing a zone.  We wait for the
@@ -334,6 +371,15 @@ do
 	sleep 1
 done
 n=`expr $n + 1`
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=`expr $status + $ret`
+# Check jitter distribution.
+echo_i "checking expired signatures were jittered correctly ($n)"
+ret=0
+$DIG $DIGOPTS axfr oldsigs.example @10.53.0.3 > dig.out.ns3.test$n || ret=1
+checkjitter dig.out.ns3.test$n || ret=1
+n=`expr $n + 1`
+if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
 echo_i "checking NSEC->NSEC3 conversion succeeded ($n)"
@@ -938,6 +984,36 @@ n=`expr $n + 1`
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
+echo_i "checking jitter in a newly signed NSEC3 zone ($n)"
+ret=0
+# Use DNS UPDATE to add an NSEC3PARAM record into the zone.
+$NSUPDATE > nsupdate.out.test$n 2>&1 <<END || ret=1
+server 10.53.0.3 ${PORT}
+zone jitter.nsec3.example.
+update add jitter.nsec3.example. 3600 NSEC3PARAM 1 0 10 BEEF
+send
+END
+[ $ret != 0 ] && echo_i "error: dynamic update add NSEC3PARAM failed"
+# Create DNSSEC keys in the zone directory.
+$KEYGEN -a rsasha1 -3 -q -K ns3 jitter.nsec3.example > /dev/null
+# Trigger zone signing.
+$RNDCCMD 10.53.0.3 sign jitter.nsec3.example. 2>&1 | sed 's/^/ns3 /' | cat_i
+# Wait until zone has been signed.
+for i in 0 1 2 3 4 5 6 7 8 9; do
+	failed=0
+	$DIG $DIGOPTS axfr jitter.nsec3.example @10.53.0.3 > dig.out.ns3.test$n || failed=1
+	grep "NSEC3PARAM" dig.out.ns3.test$n > /dev/null || failed=1
+	[ $failed -eq 0 ] && break
+	echo_i "waiting ... ($i)"
+	sleep 2
+done
+[ $failed != 0 ] && echo_i "error: no NSEC3PARAM found in AXFR" && ret=1
+# Check jitter distribution.
+checkjitter dig.out.ns3.test$n || ret=1
+n=`expr $n + 1`
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=`expr $status + $ret`
+
 echo_i "checking that serial number and RRSIGs are both updated (rt21045) ($n)"
 ret=0
 oldserial=`$DIG $DIGOPTS +short soa prepub.example @10.53.0.3 | awk '$0 !~ /SOA/ {print $3}'`

lib/dns/update.c

@@ -19,6 +19,7 @@
 #include <isc/netaddr.h>
 #include <isc/platform.h>
 #include <isc/print.h>
+#include <isc/random.h>
 #include <isc/serial.h>
 #include <isc/stats.h>
 #include <isc/stdtime.h>
@@ -1389,6 +1390,25 @@ struct dns_update_state {
 	       sign_nsec, update_nsec3, process_nsec3, sign_nsec3 } state;
 };
 
+static uint32_t
+dns__jitter_expire(dns_zone_t *zone, uint32_t sigvalidityinterval) {
+	/* Spread out signatures over time */
+	if (sigvalidityinterval >= 3600U) {
+		uint32_t expiryinterval = dns_zone_getsigresigninginterval(zone);
+
+		if (sigvalidityinterval < 7200U) {
+			expiryinterval = 1200;
+		} else if (expiryinterval > sigvalidityinterval) {
+			expiryinterval = sigvalidityinterval;
+		} else {
+			expiryinterval = sigvalidityinterval - expiryinterval;
+		}
+		uint32_t jitter = isc_random_uniform(expiryinterval);
+		sigvalidityinterval -= jitter;
+	}
+	return (sigvalidityinterval);
+}
+
 isc_result_t
 dns_update_signaturesinc(dns_update_log_t *log, dns_zone_t *zone, dns_db_t *db,
 			 dns_dbversion_t *oldver, dns_dbversion_t *newver,
@@ -1440,7 +1460,7 @@ dns_update_signaturesinc(dns_update_log_t *log, dns_zone_t *zone, dns_db_t *db,
 
 		isc_stdtime_get(&now);
 		state->inception = now - 3600; /* Allow for some clock skew. */
-		state->expire = now + sigvalidityinterval;
+		state->expire = now + dns__jitter_expire(zone, sigvalidityinterval);
 		state->keyexpire = dns_zone_getkeyvalidityinterval(zone);
 		if (state->keyexpire == 0) {
 			state->keyexpire = state->expire;

lib/dns/zone.c

@@ -6613,8 +6613,8 @@ zone_resigninc(dns_zone_t *zone) {
 	dst_key_t *zone_keys[DNS_MAXZONEKEYS];
 	bool check_ksk, keyset_kskonly = false;
 	isc_result_t result;
-	isc_stdtime_t now, inception, soaexpire, expire, stop;
-	uint32_t jitter, sigvalidityinterval;
+	isc_stdtime_t now, inception, soaexpire, expire, fullexpire, stop;
+	uint32_t sigvalidityinterval, expiryinterval;
 	unsigned int i;
 	unsigned int nkeys = 0;
 	unsigned int resign;
@@ -6662,20 +6662,34 @@ zone_resigninc(dns_zone_t *zone) {
 	sigvalidityinterval = zone->sigvalidityinterval;
 	inception = now - 3600;	/* Allow for clock skew. */
 	soaexpire = now + sigvalidityinterval;
+	expiryinterval = dns_zone_getsigresigninginterval(zone);
+	if (expiryinterval > sigvalidityinterval) {
+		expiryinterval = sigvalidityinterval;
+	} else {
+		expiryinterval = sigvalidityinterval - expiryinterval;
+	}
+
 	/*
 	 * Spread out signatures over time if they happen to be
 	 * clumped.  We don't do this for each add_sigs() call as
-	 * we still want some clustering to occur.
+	 * we still want some clustering to occur.  In normal operations
+	 * the records should be re-signed as they fall due and they should
+	 * already be spread out.  However if the server is off for a
+	 * period we need to ensure that the clusters don't become
+	 * synchronised by using the full jitter range.
 	 */
 	if (sigvalidityinterval >= 3600U) {
+		uint32_t normaljitter, fulljitter;
 		if (sigvalidityinterval > 7200U) {
-			jitter = isc_random_uniform(3600);
+			normaljitter = isc_random_uniform(3600);
+			fulljitter = isc_random_uniform(expiryinterval);
 		} else {
-			jitter = isc_random_uniform(1200);
+			normaljitter = fulljitter = isc_random_uniform(1200);
 		}
-		expire = soaexpire - jitter - 1;
+		expire = soaexpire - normaljitter - 1;
+		fullexpire = soaexpire - fulljitter - 1;
 	} else {
-		expire = soaexpire - 1;
+		expire = fullexpire = soaexpire - 1;
 	}
 	stop = now + 5;
 
@@ -6715,9 +6729,17 @@ zone_resigninc(dns_zone_t *zone) {
 			break;
 		}
 
+		/*
+		 * If re-signing is over 5 minutes late use 'fullexpire'
+		 * to redistribute the signature over the complete
+		 * re-signing window, otherwise only add a small amount
+		 * of jitter.
+		 */
 		result = add_sigs(db, version, name, zone, covers,
 				  zonediff.diff, zone_keys, nkeys, zone->mctx,
-				  inception, expire, check_ksk, keyset_kskonly);
+				  inception,
+				  resign > (now - 300) ? expire : fullexpire,
+				  check_ksk, keyset_kskonly);
 		if (result != ISC_R_SUCCESS) {
 			dns_zone_log(zone, ISC_LOG_ERROR,
 				     "zone_resigninc:add_sigs -> %s",
@@ -7677,7 +7699,7 @@ zone_nsec3chain(dns_zone_t *zone) {
 	bool first;
 	isc_result_t result;
 	isc_stdtime_t now, inception, soaexpire, expire;
-	uint32_t jitter, sigvalidityinterval;
+	uint32_t jitter, sigvalidityinterval, expiryinterval;
 	unsigned int i;
 	unsigned int nkeys = 0;
 	uint32_t nodes;
@@ -7749,6 +7771,12 @@ zone_nsec3chain(dns_zone_t *zone) {
 	sigvalidityinterval = dns_zone_getsigvalidityinterval(zone);
 	inception = now - 3600;	/* Allow for clock skew. */
 	soaexpire = now + sigvalidityinterval;
+	expiryinterval = dns_zone_getsigresigninginterval(zone);
+	if (expiryinterval > sigvalidityinterval) {
+		expiryinterval = sigvalidityinterval;
+	} else {
+		expiryinterval = sigvalidityinterval - expiryinterval;
+	}
 
 	/*
 	 * Spread out signatures over time if they happen to be
@@ -7757,7 +7785,7 @@ zone_nsec3chain(dns_zone_t *zone) {
 	 */
 	if (sigvalidityinterval >= 3600U) {
 		if (sigvalidityinterval > 7200U) {
-			jitter = isc_random_uniform(3600);
+			jitter = isc_random_uniform(expiryinterval);
 		} else {
 			jitter = isc_random_uniform(1200);
 		}

lib/ns/update.c

@@ -3181,14 +3181,14 @@ update_action(isc_task_t *task, isc_event_t *event) {
 			 CHECK(dns_nsec3param_deletechains(db, ver, zone,
 							   true, &diff));
 		} else if (has_dnskey && isdnssec(db, ver, privatetype)) {
-			uint32_t interval;
 			dns_update_log_t log;
+			uint32_t interval = dns_zone_getsigvalidityinterval(zone);
 
-			interval = dns_zone_getsigvalidityinterval(zone);
 			log.func = update_log_cb;
 			log.arg = client;
 			result = dns_update_signatures(&log, zone, db, oldver,
-						       ver, &diff, interval);
+						       ver, &diff,
+						       interval);
 
 			if (result != ISC_R_SUCCESS) {
 				update_log(client, zone,