From 4f08beb1de55865d450ef91ed2c5d7527d859964 Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Tue, 24 Aug 2021 09:27:38 +0200
Subject: [PATCH 1/7] Add stats unit test

Add a simple stats unit test that tests the existing library functions
isc_stats_ncounters, isc_stats_increment, isc_stats_decrement,
isc_stats_set, and isc_stats_update_if_greater.

(manually picked from commit 0bac9c7c5ca73a5a81c6d28ad9e6160591517ced)
---
 lib/isc/tests/Makefile.in  |  12 +++-
 lib/isc/tests/stats_test.c | 122 +++++++++++++++++++++++++++++++++++++
 util/copyrights            |   1 +
 3 files changed, 132 insertions(+), 3 deletions(-)
 create mode 100644 lib/isc/tests/stats_test.c

diff --git a/lib/isc/tests/Makefile.in b/lib/isc/tests/Makefile.in
index 3e0949b927..4637f8de60 100644
--- a/lib/isc/tests/Makefile.in
+++ b/lib/isc/tests/Makefile.in
@@ -36,7 +36,7 @@ SRCS =		isctest.c aes_test.c buffer_test.c \
 		parse_test.c pool_test.c \
 		quota_test.c radix_test.c random_test.c \
 		regex_test.c result_test.c safe_test.c siphash_test.c sockaddr_test.c \
-		socket_test.c symtab_test.c task_test.c \
+		socket_test.c stats_test.c symtab_test.c task_test.c \
 		taskpool_test.c time_test.c timer_test.c
 
 SUBDIRS =
@@ -51,8 +51,9 @@ TARGETS =	aes_test@EXEEXT@ buffer_test@EXEEXT@ \
 		quota_test@EXEEXT@ radix_test@EXEEXT@ \
 		random_test@EXEEXT@ regex_test@EXEEXT@ result_test@EXEEXT@ \
 		safe_test@EXEEXT@ siphash_test@EXEEXT@ sockaddr_test@EXEEXT@ \
-		socket_test@EXEEXT@ symtab_test@EXEEXT@ task_test@EXEEXT@ \
-		taskpool_test@EXEEXT@ time_test@EXEEXT@ timer_test@EXEEXT@
+		socket_test@EXEEXT@ stats_test@EXEEXT@ symtab_test@EXEEXT@ \
+		task_test@EXEEXT@ taskpool_test@EXEEXT@ time_test@EXEEXT@ \
+		timer_test@EXEEXT@
 
 @BIND9_MAKE_RULES@
 
@@ -181,6 +182,11 @@ sockaddr_test@EXEEXT@: sockaddr_test.@O@ isctest.@O@ ${ISCDEPLIBS}
 		${LDFLAGS} -o $@ sockaddr_test.@O@ isctest.@O@ \
 		${ISCLIBS} ${LIBS}
 
+stats_test@EXEEXT@: stats_test.@O@ isctest.@O@ ${ISCDEPLIBS}
+	${LIBTOOL_MODE_LINK} ${PURIFY} ${CC} ${CFLAGS} \
+		${LDFLAGS} -o $@ stats_test.@O@ isctest.@O@ \
+		${ISCLIBS} ${LIBS}
+
 symtab_test@EXEEXT@: symtab_test.@O@ isctest.@O@ ${ISCDEPLIBS}
 	${LIBTOOL_MODE_LINK} ${PURIFY} ${CC} ${CFLAGS} \
 		${LDFLAGS} -o $@ symtab_test.@O@ isctest.@O@ \
diff --git a/lib/isc/tests/stats_test.c b/lib/isc/tests/stats_test.c
new file mode 100644
index 0000000000..a212613143
--- /dev/null
+++ b/lib/isc/tests/stats_test.c
@@ -0,0 +1,122 @@
+/*
+ * 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 https://mozilla.org/MPL/2.0/.
+ *
+ * See the COPYRIGHT file distributed with this work for additional
+ * information regarding copyright ownership.
+ */
+
+#if HAVE_CMOCKA
+
+#include <sched.h> /* IWYU pragma: keep */
+#include <setjmp.h>
+#include <stdarg.h>
+#include <stddef.h>
+#include <stdlib.h>
+#include <string.h>
+
+#define UNIT_TESTING
+#include <cmocka.h>
+
+#include <isc/mem.h>
+#include <isc/result.h>
+#include <isc/stats.h>
+#include <isc/util.h>
+
+#include "isctest.h"
+
+static int
+_setup(void **state) {
+	isc_result_t result;
+
+	UNUSED(state);
+
+	result = isc_test_begin(NULL, true, 0);
+	assert_int_equal(result, ISC_R_SUCCESS);
+
+	return (0);
+}
+
+static int
+_teardown(void **state) {
+	UNUSED(state);
+
+	isc_test_end();
+
+	return (0);
+}
+
+/* test stats */
+static void
+isc_stats_basic_test(void **state) {
+	isc_stats_t *stats = NULL;
+	isc_result_t result;
+
+	UNUSED(state);
+
+	result = isc_stats_create(test_mctx, &stats, 4);
+	assert_int_equal(result, ISC_R_SUCCESS);
+	assert_int_equal(isc_stats_ncounters(stats), 4);
+
+	/* Default all 0. */
+	for (int i = 0; i < isc_stats_ncounters(stats); i++) {
+		assert_int_equal(isc_stats_get_counter(stats, i), 0);
+	}
+
+	/* Test increment. */
+	for (int i = 0; i < isc_stats_ncounters(stats); i++) {
+		isc_stats_increment(stats, i);
+		assert_int_equal(isc_stats_get_counter(stats, i), 1);
+		isc_stats_increment(stats, i);
+		assert_int_equal(isc_stats_get_counter(stats, i), 2);
+	}
+
+	/* Test decrement. */
+	for (int i = 0; i < isc_stats_ncounters(stats); i++) {
+		isc_stats_decrement(stats, i);
+		assert_int_equal(isc_stats_get_counter(stats, i), 1);
+		isc_stats_decrement(stats, i);
+		assert_int_equal(isc_stats_get_counter(stats, i), 0);
+	}
+
+	/* Test set. */
+	for (int i = 0; i < isc_stats_ncounters(stats); i++) {
+		isc_stats_set(stats, i, i);
+		assert_int_equal(isc_stats_get_counter(stats, i), i);
+	}
+
+	/* Test update if greater. */
+	for (int i = 0; i < isc_stats_ncounters(stats); i++) {
+		isc_stats_update_if_greater(stats, i, i);
+		assert_int_equal(isc_stats_get_counter(stats, i), i);
+		isc_stats_update_if_greater(stats, i, i + 1);
+		assert_int_equal(isc_stats_get_counter(stats, i), i + 1);
+	}
+
+	isc_stats_detach(&stats);
+}
+
+int
+main(void) {
+	const struct CMUnitTest tests[] = {
+		cmocka_unit_test_setup_teardown(isc_stats_basic_test, _setup,
+						_teardown),
+	};
+
+	return (cmocka_run_group_tests(tests, NULL, NULL));
+}
+
+#else /* HAVE_CMOCKA */
+
+#include <stdio.h>
+
+int
+main(void) {
+	printf("1..0 # Skipped: cmocka not available\n");
+	return (SKIPPED_TEST_EXIT_CODE);
+}
+
+#endif /* if HAVE_CMOCKA */
diff --git a/util/copyrights b/util/copyrights
index cb3c1283bb..ad4ecb0ea7 100644
--- a/util/copyrights
+++ b/util/copyrights
@@ -2275,6 +2275,7 @@
 ./lib/isc/tests/siphash_test.c			C	2019,2020,2021
 ./lib/isc/tests/sockaddr_test.c			C	2012,2015,2016,2017,2018,2019,2020,2021
 ./lib/isc/tests/socket_test.c			C	2011,2012,2013,2014,2015,2016,2017,2018,2019,2020,2021
+./lib/isc/tests/stats_test.c			C	2021
 ./lib/isc/tests/symtab_test.c			C	2011,2012,2013,2016,2018,2019,2020,2021
 ./lib/isc/tests/task_test.c			C	2011,2012,2016,2017,2018,2019,2020,2021
 ./lib/isc/tests/taskpool_test.c			C	2011,2012,2016,2018,2019,2020,2021

From 4a1987a380190d1da2d7f475faa210a480c82193 Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Thu, 19 Aug 2021 13:38:51 +0200
Subject: [PATCH 2/7] Add a function isc_stats_resize

Add a new function to resize the number of counters in a statistics
counter structure. This will be needed when we keep track of DNSSEC
sign statistics and new keys are introduced due to a rollover.

(cherry picked from commit 9acce8a82abec4b88630c1d05a18a44ebb051ecb)
---
 lib/isc/include/isc/stats.h | 11 +++++++++++
 lib/isc/stats.c             | 32 ++++++++++++++++++++++++++++++++
 lib/isc/tests/stats_test.c  | 17 +++++++++++++++++
 lib/isc/win32/libisc.def.in |  1 +
 4 files changed, 61 insertions(+)

diff --git a/lib/isc/include/isc/stats.h b/lib/isc/include/isc/stats.h
index c2ab4e6c03..f57aa1a96f 100644
--- a/lib/isc/include/isc/stats.h
+++ b/lib/isc/include/isc/stats.h
@@ -238,6 +238,17 @@ isc_stats_get_counter(isc_stats_t *stats, isc_statscounter_t counter);
  *	on creation.
  */
 
+void
+isc_stats_resize(isc_stats_t **stats, int ncounters);
+/*%<
+ * Resize a statistics counter structure of general type. The new set of
+ * counters are indexed by an ID between 0 and ncounters -1.
+ *
+ * Requires:
+ *\li	'stats' is a valid isc_stats_t.
+ *\li	'ncounters' is a non-zero positive number.
+ */
+
 ISC_LANG_ENDDECLS
 
 #endif /* ISC_STATS_H */
diff --git a/lib/isc/stats.c b/lib/isc/stats.c
index 6084310a91..7d5dc1a725 100644
--- a/lib/isc/stats.c
+++ b/lib/isc/stats.c
@@ -167,3 +167,35 @@ isc_stats_get_counter(isc_stats_t *stats, isc_statscounter_t counter) {
 
 	return (atomic_load_acquire(&stats->counters[counter]));
 }
+
+void
+isc_stats_resize(isc_stats_t **statsp, int ncounters) {
+	isc_stats_t *stats;
+	size_t counters_alloc_size;
+	isc__atomic_statcounter_t *newcounters;
+
+	REQUIRE(statsp != NULL && *statsp != NULL);
+	REQUIRE(ISC_STATS_VALID(*statsp));
+	REQUIRE(ncounters > 0);
+
+	stats = *statsp;
+	if (stats->ncounters >= ncounters) {
+		/* We already have enough counters. */
+		return;
+	}
+
+	/* Grow number of counters. */
+	counters_alloc_size = sizeof(isc__atomic_statcounter_t) * ncounters;
+	newcounters = isc_mem_get(stats->mctx, counters_alloc_size);
+	for (int i = 0; i < ncounters; i++) {
+		atomic_init(&newcounters[i], 0);
+	}
+	for (int i = 0; i < stats->ncounters; i++) {
+		uint32_t counter = atomic_load_acquire(&stats->counters[i]);
+		atomic_store_release(&newcounters[i], counter);
+	}
+	isc_mem_put(stats->mctx, stats->counters,
+		    sizeof(isc__atomic_statcounter_t) * stats->ncounters);
+	stats->counters = newcounters;
+	stats->ncounters = ncounters;
+}
diff --git a/lib/isc/tests/stats_test.c b/lib/isc/tests/stats_test.c
index a212613143..7b7476d3ad 100644
--- a/lib/isc/tests/stats_test.c
+++ b/lib/isc/tests/stats_test.c
@@ -96,6 +96,23 @@ isc_stats_basic_test(void **state) {
 		assert_int_equal(isc_stats_get_counter(stats, i), i + 1);
 	}
 
+	/* Test resize. */
+	isc_stats_resize(&stats, 3);
+	assert_int_equal(isc_stats_ncounters(stats), 4);
+	isc_stats_resize(&stats, 4);
+	assert_int_equal(isc_stats_ncounters(stats), 4);
+	isc_stats_resize(&stats, 5);
+	assert_int_equal(isc_stats_ncounters(stats), 5);
+
+	/* Existing counters are retained */
+	for (int i = 0; i < isc_stats_ncounters(stats); i++) {
+		uint32_t expect = i + 1;
+		if (i == 4) {
+			expect = 0;
+		}
+		assert_int_equal(isc_stats_get_counter(stats, i), expect);
+	}
+
 	isc_stats_detach(&stats);
 }
 
diff --git a/lib/isc/win32/libisc.def.in b/lib/isc/win32/libisc.def.in
index 1faeb3504a..a32a908b25 100644
--- a/lib/isc/win32/libisc.def.in
+++ b/lib/isc/win32/libisc.def.in
@@ -609,6 +609,7 @@ isc_stats_dump
 isc_stats_get_counter
 isc_stats_increment
 isc_stats_ncounters
+isc_stats_resize
 isc_stats_set
 isc_stats_update_if_greater
 isc_stdio_close

From df6fb95621cae29586d0fcbf649e6ccf4ed83863 Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Fri, 20 Aug 2021 11:14:49 +0200
Subject: [PATCH 3/7] Grow dnssec-sign statistics instead of rotating

We have introduced dnssec-sign statistics to the zone statistics. This
introduced an operational issue because when using zone-statistics
full, the memory usage was going through the roof. We fixed this by
by allocating just four key slots per zone. If a zone exceeds the
number of keys for example through a key rollover, the keys will be
rotated out on a FIFO basis.

This works for most cases, and fixes the immediate problem of high
memory usage, but if you sign your zone with many, many keys, or are
sign with a ZSK/KSK double algorithm strategy you may experience weird
statistics. A better strategy is to grow the number of key slots per
zone on key rollover events.

That is what this commit is doing: instead of rotating the four slots
to track sign statistics, named now grows the number of key slots
during a key rollover (or via some other method that introduces new
keys).

(cherry picked from commit d9cca81d508ae0f84974b89ce48af17d5096186d)
---
 lib/dns/stats.c | 39 ++++++++++++++-------------------------
 1 file changed, 14 insertions(+), 25 deletions(-)

diff --git a/lib/dns/stats.c b/lib/dns/stats.c
index 011889e77a..1ca0fb563a 100644
--- a/lib/dns/stats.c
+++ b/lib/dns/stats.c
@@ -103,7 +103,7 @@ typedef enum {
  */
 
 /* Maximum number of keys to keep track of for DNSSEC signing statistics. */
-static int dnssecsign_max_keys = 4;
+static int dnssecsign_num_keys = 4;
 static int dnssecsign_block_size = 3;
 /* Key id mask */
 #define DNSSECSIGNSTATS_KEY_ID_MASK 0x0000FFFF
@@ -245,7 +245,8 @@ dns_dnssecsignstats_create(isc_mem_t *mctx, dns_stats_t **statsp) {
 	 * the actual counters for creating and refreshing signatures.
 	 */
 	return (create_stats(mctx, dns_statstype_dnssec,
-			     dnssecsign_max_keys * 3, statsp));
+			     dnssecsign_num_keys * dnssecsign_block_size,
+			     statsp));
 }
 
 /*%
@@ -361,6 +362,8 @@ void
 dns_dnssecsignstats_increment(dns_stats_t *stats, dns_keytag_t id, uint8_t alg,
 			      dnssecsignstats_type_t operation) {
 	uint32_t kval;
+	int num_keys = isc_stats_ncounters(stats->counters) /
+		       dnssecsign_block_size;
 
 	REQUIRE(DNS_STATS_VALID(stats) && stats->type == dns_statstype_dnssec);
 
@@ -368,7 +371,7 @@ dns_dnssecsignstats_increment(dns_stats_t *stats, dns_keytag_t id, uint8_t alg,
 	kval = (uint32_t)(alg << 16 | id);
 
 	/* Look up correct counter. */
-	for (int i = 0; i < dnssecsign_max_keys; i++) {
+	for (int i = 0; i < num_keys; i++) {
 		int idx = i * dnssecsign_block_size;
 		uint32_t counter = isc_stats_get_counter(stats->counters, idx);
 		if (counter == kval) {
@@ -379,7 +382,7 @@ dns_dnssecsignstats_increment(dns_stats_t *stats, dns_keytag_t id, uint8_t alg,
 	}
 
 	/* No match found. Store key in unused slot. */
-	for (int i = 0; i < dnssecsign_max_keys; i++) {
+	for (int i = 0; i < num_keys; i++) {
 		int idx = i * dnssecsign_block_size;
 		uint32_t counter = isc_stats_get_counter(stats->counters, idx);
 		if (counter == 0) {
@@ -389,27 +392,12 @@ dns_dnssecsignstats_increment(dns_stats_t *stats, dns_keytag_t id, uint8_t alg,
 		}
 	}
 
-	/* No room, rotate keys. */
-	for (int i = 1; i < dnssecsign_max_keys; i++) {
-		int gidx = i * dnssecsign_block_size; /* Get key (get index,
-							 gidx) */
-		uint32_t keyv = isc_stats_get_counter(stats->counters, gidx);
-		uint32_t sign = isc_stats_get_counter(
-			stats->counters, (gidx + dns_dnssecsignstats_sign));
-		uint32_t refr = isc_stats_get_counter(
-			stats->counters, (gidx + dns_dnssecsignstats_refresh));
-
-		int sidx = (i - 1) * dnssecsign_block_size; /* Set key, (set
-							       index, sidx) */
-		isc_stats_set(stats->counters, keyv, sidx);
-		isc_stats_set(stats->counters, sign,
-			      (sidx + dns_dnssecsignstats_sign));
-		isc_stats_set(stats->counters, refr,
-			      (sidx + dns_dnssecsignstats_refresh));
-	}
+	/* No room, grow stats storage. */
+	isc_stats_resize(&stats->counters,
+			 (num_keys * dnssecsign_block_size * 2));
 
 	/* Reset counters for new key (new index, nidx). */
-	int nidx = (dnssecsign_max_keys - 1) * dnssecsign_block_size;
+	int nidx = num_keys * dnssecsign_block_size;
 	isc_stats_set(stats->counters, kval, nidx);
 	isc_stats_set(stats->counters, 0, (nidx + dns_dnssecsignstats_sign));
 	isc_stats_set(stats->counters, 0, (nidx + dns_dnssecsignstats_refresh));
@@ -525,9 +513,10 @@ dnssec_dumpcb(isc_statscounter_t counter, uint64_t value, void *arg) {
 static void
 dnssec_statsdump(isc_stats_t *stats, dnssecsignstats_type_t operation,
 		 isc_stats_dumper_t dump_fn, void *arg, unsigned int options) {
-	int i;
+	int i, num_keys;
 
-	for (i = 0; i < dnssecsign_max_keys; i++) {
+	num_keys = isc_stats_ncounters(stats) / dnssecsign_block_size;
+	for (i = 0; i < num_keys; i++) {
 		int idx = dnssecsign_block_size * i;
 		uint32_t kval, val;
 		dns_keytag_t id;

From 7e90ef8f8c47bb4f42313145144bf75ac72b13fc Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Fri, 20 Aug 2021 11:19:28 +0200
Subject: [PATCH 4/7] Add back the statschannel manykeys test case

Add a test case that has more than four keys (the initial number of
key slots that are created for dnssec-sign statistics). We shouldn't
be expecting weird values.

This fixes some errors in the manykeys zone configuration (keys
were created for algorithm RSASHA256, but the policy expected RSASHA1,
and the zone was not allowing dynamic updates).

This also fixes an error in the calls to 'zones-json.pl': The perl
script excepts an index number where the zone can be found, rather
than the zone name.

(cherry picked from commit 019a52a1844c843b9e5dff2f7a2f5adea8400c2f)
---
 .../system/statschannel/ns2/named.conf.in     |  9 +-
 bin/tests/system/statschannel/tests.sh        | 96 ++++++++++++++++---
 bin/tests/system/statschannel/zones-json.pl   |  1 -
 3 files changed, 89 insertions(+), 17 deletions(-)

diff --git a/bin/tests/system/statschannel/ns2/named.conf.in b/bin/tests/system/statschannel/ns2/named.conf.in
index 68367989cc..c4ec68db3f 100644
--- a/bin/tests/system/statschannel/ns2/named.conf.in
+++ b/bin/tests/system/statschannel/ns2/named.conf.in
@@ -36,8 +36,8 @@ controls {
 
 dnssec-policy "manykeys" {
 	keys {
-		ksk lifetime unlimited algorithm 5;
-		zsk lifetime unlimited algorithm 5;
+		ksk lifetime unlimited algorithm 8;
+		zsk lifetime unlimited algorithm 8;
 		ksk lifetime unlimited algorithm 13;
 		zsk lifetime unlimited algorithm 13;
 		ksk lifetime unlimited algorithm 14;
@@ -62,8 +62,9 @@ zone "dnssec" {
 };
 
 zone "manykeys" {
-        type primary;
-        file "manykeys.db.signed";
+	type primary;
+	file "manykeys.db.signed";
+	allow-update { any; };
 	zone-statistics full;
 	dnssec-policy "manykeys";
 };
diff --git a/bin/tests/system/statschannel/tests.sh b/bin/tests/system/statschannel/tests.sh
index 5be676553a..46ed3a4acb 100644
--- a/bin/tests/system/statschannel/tests.sh
+++ b/bin/tests/system/statschannel/tests.sh
@@ -185,14 +185,11 @@ refresh_prefix="dnssec-refresh operations"
 ksk_id=`cat ns2/$zone.ksk.id`
 zsk_id=`cat ns2/$zone.zsk.id`
 
-# 1. Test sign operations for scheduled resigning.
+# Test sign operations for scheduled resigning.
 ret=0
 # The dnssec zone has 10 RRsets to sign (including NSEC) with the ZSK and one
 # RRset (DNSKEY) with the KSK. So starting named with signatures that expire
 # almost right away, this should trigger 10 zsk and 1 ksk sign operations.
-# However, the DNSSEC maintenance assumes when we see the SOA record we have
-# walked the whole zone, since the SOA record should always have the most
-# recent signature.
 echo "${refresh_prefix} ${zsk_id}: 10" > zones.expect
 echo "${refresh_prefix} ${ksk_id}: 1" >> zones.expect
 echo "${sign_prefix} ${zsk_id}: 10" >> zones.expect
@@ -200,20 +197,20 @@ echo "${sign_prefix} ${ksk_id}: 1" >> zones.expect
 cat zones.expect | sort > zones.expect.$n
 rm -f zones.expect
 # Fetch and check the dnssec sign statistics.
-echo_i "fetching zone stats data after zone maintenance at startup ($n)"
+echo_i "fetching zone '$zone' stats data after zone maintenance at startup ($n)"
 if [ $PERL_XML ]; then
     getzones xml $zone x$n || ret=1
     cmp zones.out.x$n zones.expect.$n || ret=1
 fi
 if [ $PERL_JSON ]; then
-    getzones json $zone j$n || ret=1
+    getzones json 0 j$n || ret=1
     cmp zones.out.j$n zones.expect.$n || ret=1
 fi
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 n=`expr $n + 1`
 
-# 2. Test sign operations after dynamic update.
+# Test sign operations after dynamic update.
 ret=0
 (
 # Update dnssec zone to trigger signature creation.
@@ -230,22 +227,22 @@ echo "${sign_prefix} ${ksk_id}: 1" >> zones.expect
 cat zones.expect | sort > zones.expect.$n
 rm -f zones.expect
 # Fetch and check the dnssec sign statistics.
-echo_i "fetching zone stats data after dynamic update ($n)"
+echo_i "fetching zone '$zone' stats data after dynamic update ($n)"
 if [ $PERL_XML ]; then
     getzones xml $zone x$n || ret=1
     cmp zones.out.x$n zones.expect.$n || ret=1
 fi
 if [ $PERL_JSON ]; then
-    getzones json $zone j$n || ret=1
+    getzones json 0 j$n || ret=1
     cmp zones.out.j$n zones.expect.$n || ret=1
 fi
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 n=`expr $n + 1`
 
-# 3. Test sign operations of KSK.
+# Test sign operations of KSK.
 ret=0
-echo_i "fetch zone stats data after updating DNSKEY RRset ($n)"
+echo_i "fetch zone '$zone' stats data after updating DNSKEY RRset ($n)"
 # Add a standby DNSKEY, this triggers resigning the DNSKEY RRset.
 zsk=$("$KEYGEN" -K ns2 -q -a "$DEFAULT_ALGORITHM" -b "$DEFAULT_BITS" "$zone")
 $SETTIME -K ns2 -P now -A never $zsk.key > /dev/null
@@ -263,13 +260,88 @@ if [ $PERL_XML ]; then
     cmp zones.out.x$n zones.expect.$n || ret=1
 fi
 if [ $PERL_JSON ]; then
-    getzones json $zone j$n || ret=1
+    getzones json 0 j$n || ret=1
     cmp zones.out.j$n zones.expect.$n || ret=1
 fi
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 n=`expr $n + 1`
 
+# Test sign operations for scheduled resigning (many keys).
+ret=0
+zone="manykeys"
+ksk8_id=`cat ns2/$zone.ksk8.id`
+zsk8_id=`cat ns2/$zone.zsk8.id`
+ksk13_id=`cat ns2/$zone.ksk13.id`
+zsk13_id=`cat ns2/$zone.zsk13.id`
+ksk14_id=`cat ns2/$zone.ksk14.id`
+zsk14_id=`cat ns2/$zone.zsk14.id`
+# The dnssec zone has 10 RRsets to sign (including NSEC) with the ZSKs and one
+# RRset (DNSKEY) with the KSKs. So starting named with signatures that expire
+# almost right away, this should trigger 10 zsk and 1 ksk sign operations per
+# key.
+echo "${refresh_prefix} ${zsk8_id}: 10" > zones.expect
+echo "${refresh_prefix} ${zsk13_id}: 10" >> zones.expect
+echo "${refresh_prefix} ${zsk14_id}: 10" >> zones.expect
+echo "${refresh_prefix} ${ksk8_id}: 1" >> zones.expect
+echo "${refresh_prefix} ${ksk13_id}: 1" >> zones.expect
+echo "${refresh_prefix} ${ksk14_id}: 1" >> zones.expect
+echo "${sign_prefix} ${zsk8_id}: 10" >> zones.expect
+echo "${sign_prefix} ${zsk13_id}: 10" >> zones.expect
+echo "${sign_prefix} ${zsk14_id}: 10" >> zones.expect
+echo "${sign_prefix} ${ksk8_id}: 1" >> zones.expect
+echo "${sign_prefix} ${ksk13_id}: 1" >> zones.expect
+echo "${sign_prefix} ${ksk14_id}: 1" >> zones.expect
+cat zones.expect | sort > zones.expect.$n
+rm -f zones.expect
+# Fetch and check the dnssec sign statistics.
+echo_i "fetching zone '$zone' stats data after zone maintenance at startup ($n)"
+if [ $PERL_XML ]; then
+    getzones xml $zone x$n || ret=1
+    cmp zones.out.x$n zones.expect.$n || ret=1
+fi
+if [ $PERL_JSON ]; then
+    getzones json 2 j$n || ret=1
+    cmp zones.out.j$n zones.expect.$n || ret=1
+fi
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=`expr $status + $ret`
+n=`expr $n + 1`
+
+# Test sign operations after dynamic update (many keys).
+ret=0
+(
+# Update dnssec zone to trigger signature creation.
+echo zone $zone
+echo server 10.53.0.2 "$PORT"
+echo update add $zone. 300 in txt "nsupdate added me"
+echo send
+) | $NSUPDATE
+# This should trigger the resign of SOA, TXT and NSEC (+3 zsk).
+echo "${refresh_prefix} ${zsk8_id}: 10" > zones.expect
+echo "${refresh_prefix} ${zsk13_id}: 10" >> zones.expect
+echo "${refresh_prefix} ${zsk14_id}: 10" >> zones.expect
+echo "${refresh_prefix} ${ksk8_id}: 1" >> zones.expect
+echo "${refresh_prefix} ${ksk13_id}: 1" >> zones.expect
+echo "${refresh_prefix} ${ksk14_id}: 1" >> zones.expect
+echo "${sign_prefix} ${zsk8_id}: 13" >> zones.expect
+echo "${sign_prefix} ${zsk13_id}: 13" >> zones.expect
+echo "${sign_prefix} ${zsk14_id}: 13" >> zones.expect
+echo "${sign_prefix} ${ksk8_id}: 1" >> zones.expect
+echo "${sign_prefix} ${ksk13_id}: 1" >> zones.expect
+echo "${sign_prefix} ${ksk14_id}: 1" >> zones.expect
+cat zones.expect | sort > zones.expect.$n
+rm -f zones.expect
+# Fetch and check the dnssec sign statistics.
+echo_i "fetching zone '$zone' stats data after dynamic update ($n)"
+if [ $PERL_XML ]; then
+    getzones xml $zone x$n || ret=1
+    cmp zones.out.x$n zones.expect.$n || ret=1
+fi
+if [ $PERL_JSON ]; then
+    getzones json 2 j$n || ret=1
+    cmp zones.out.j$n zones.expect.$n || ret=1
+fi
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 n=`expr $n + 1`
diff --git a/bin/tests/system/statschannel/zones-json.pl b/bin/tests/system/statschannel/zones-json.pl
index 9ccaf0eaf8..5da65324a2 100644
--- a/bin/tests/system/statschannel/zones-json.pl
+++ b/bin/tests/system/statschannel/zones-json.pl
@@ -23,7 +23,6 @@ close(INPUT);
 
 my $ref = decode_json($text);
 
-
 my $dnssecsign = $ref->{views}->{_default}->{zones}[$zone]->{"dnssec-sign"};
 my $type = "dnssec-sign operations ";
 foreach $key (keys %{$dnssecsign}) {

From c499478321548d585bb024dfae629d77ed129f0d Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Fri, 20 Aug 2021 15:06:13 +0200
Subject: [PATCH 5/7] Clear dnssec-sign stats for removed keys

Clear the key slots for dnssec-sign statistics for keys that are
removed. This way, the number of slots will stabilize to the maximum
key usage in a zone and will not grow every time a key rollover is
triggered.

(cherry picked from commit de15e07800f695ac2a344463cbe56fe9c189f95d)
---
 lib/dns/include/dns/stats.h | 13 +++++++++++--
 lib/dns/stats.c             | 27 +++++++++++++++++++++++++++
 lib/dns/win32/libdns.def.in |  1 +
 lib/dns/zone.c              | 18 ++++++++++++++++++
 4 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/lib/dns/include/dns/stats.h b/lib/dns/include/dns/stats.h
index 25d3f0bc9e..eb7c4602ec 100644
--- a/lib/dns/include/dns/stats.h
+++ b/lib/dns/include/dns/stats.h
@@ -698,8 +698,17 @@ void
 dns_dnssecsignstats_increment(dns_stats_t *stats, dns_keytag_t id, uint8_t alg,
 			      dnssecsignstats_type_t operation);
 /*%<
- * Increment the statistics counter for the DNSKEY 'id'. The 'operation'
- * determines what counter is incremented.
+ * Increment the statistics counter for the DNSKEY 'id' with algorithm 'alg'.
+ * The 'operation' determines what counter is incremented.
+ *
+ * Requires:
+ *\li	'stats' is a valid dns_stats_t created by dns_dnssecsignstats_create().
+ */
+
+void
+dns_dnssecsignstats_clear(dns_stats_t *stats, dns_keytag_t id, uint8_t alg);
+/*%<
+ * Clear the statistics counter for the DNSKEY 'id' with algorithm 'alg'.
  *
  * Requires:
  *\li	'stats' is a valid dns_stats_t created by dns_dnssecsignstats_create().
diff --git a/lib/dns/stats.c b/lib/dns/stats.c
index 1ca0fb563a..6a84c9c11f 100644
--- a/lib/dns/stats.c
+++ b/lib/dns/stats.c
@@ -406,6 +406,33 @@ dns_dnssecsignstats_increment(dns_stats_t *stats, dns_keytag_t id, uint8_t alg,
 	isc_stats_increment(stats->counters, (nidx + operation));
 }
 
+void
+dns_dnssecsignstats_clear(dns_stats_t *stats, dns_keytag_t id, uint8_t alg) {
+	uint32_t kval;
+	int num_keys = isc_stats_ncounters(stats->counters) /
+		       dnssecsign_block_size;
+
+	REQUIRE(DNS_STATS_VALID(stats) && stats->type == dns_statstype_dnssec);
+
+	/* Shift algorithm in front of key tag, which is 16 bits */
+	kval = (uint32_t)(alg << 16 | id);
+
+	/* Look up correct counter. */
+	for (int i = 0; i < num_keys; i++) {
+		int idx = i * dnssecsign_block_size;
+		uint32_t counter = isc_stats_get_counter(stats->counters, idx);
+		if (counter == kval) {
+			/* Match */
+			isc_stats_set(stats->counters, 0, idx);
+			isc_stats_set(stats->counters, 0,
+				      (idx + dns_dnssecsignstats_sign));
+			isc_stats_set(stats->counters, 0,
+				      (idx + dns_dnssecsignstats_refresh));
+			return;
+		}
+	}
+}
+
 /*%
  * Dump methods
  */
diff --git a/lib/dns/win32/libdns.def.in b/lib/dns/win32/libdns.def.in
index d23500508b..31f511103f 100644
--- a/lib/dns/win32/libdns.def.in
+++ b/lib/dns/win32/libdns.def.in
@@ -339,6 +339,7 @@ dns_dnssec_verify
 dns_dnssec_verifymessage
 dns_dnsseckey_create
 dns_dnsseckey_destroy
+dns_dnssecsignstats_clear
 dns_dnssecsignstats_create
 dns_dnssecsignstats_dump
 dns_dnssecsignstats_increment
diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index fc9157da6a..d563f2a95d 100644
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -21560,6 +21560,8 @@ zone_rekey(dns_zone_t *zone) {
 
 	if (commit) {
 		dns_difftuple_t *tuple;
+		dns_stats_t *dnssecsignstats =
+			dns_zone_getdnssecsignstats(zone);
 
 		DNS_ZONE_SETFLAG(zone, DNS_ZONEFLG_NEEDNOTIFY);
 
@@ -21580,6 +21582,22 @@ zone_rekey(dns_zone_t *zone) {
 						   "%s",
 						   dns_result_totext(result));
 				}
+
+				/* Clear DNSSEC sign statistics. */
+				if (dnssecsignstats != NULL) {
+					dns_dnssecsignstats_clear(
+						dnssecsignstats,
+						dst_key_id(key->key),
+						dst_key_alg(key->key));
+					/*
+					 * Also clear the dnssec-sign
+					 * statistics of the revoked key id.
+					 */
+					dns_dnssecsignstats_clear(
+						dnssecsignstats,
+						dst_key_rid(key->key),
+						dst_key_alg(key->key));
+				}
 			}
 		}
 

From 229bc4ee959360d614be2df7952c29ad14f6bbe3 Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Fri, 20 Aug 2021 15:08:29 +0200
Subject: [PATCH 6/7] Add statschannel test case for key removal

Add a statschannel test case to confirm that when keys are removed
(in this case because of a dnssec-policy change), the corresponding
dnssec-sign stats are cleared and are no longer shown in the
statistics.

(cherry picked from commit 1a3c82f765f622ed96f0b1e09923b94aed56b3d0)
---
 .../system/statschannel/ns2/named2.conf.in    | 66 +++++++++++++++++++
 bin/tests/system/statschannel/tests.sh        | 29 ++++++++
 2 files changed, 95 insertions(+)
 create mode 100644 bin/tests/system/statschannel/ns2/named2.conf.in

diff --git a/bin/tests/system/statschannel/ns2/named2.conf.in b/bin/tests/system/statschannel/ns2/named2.conf.in
new file mode 100644
index 0000000000..15c52d4972
--- /dev/null
+++ b/bin/tests/system/statschannel/ns2/named2.conf.in
@@ -0,0 +1,66 @@
+/*
+ * 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.
+ */
+
+options {
+	query-source address 10.53.0.2;
+	notify-source 10.53.0.2;
+	transfer-source 10.53.0.2;
+	port @PORT@;
+	pid-file "named.pid";
+	listen-on { 10.53.0.2; };
+	listen-on-v6 { none; };
+	recursion no;
+	notify no;
+	minimal-responses no;
+	version none;  // make statistics independent of the version number
+};
+
+statistics-channels { inet 10.53.0.2 port @EXTRAPORT1@ allow { localhost; }; };
+
+key rndc_key {
+	secret "1234abcd8765";
+	algorithm hmac-sha256;
+};
+
+controls {
+	inet 10.53.0.2 port @CONTROLPORT@ allow { any; } keys { rndc_key; };
+};
+
+dnssec-policy "manykeys" {
+	keys {
+		ksk lifetime unlimited algorithm 8;
+		zsk lifetime unlimited algorithm 8;
+	};
+};
+
+zone "example" {
+	type primary;
+	file "example.db";
+	allow-transfer { any; };
+};
+
+zone "dnssec" {
+	type primary;
+	file "dnssec.db.signed";
+	auto-dnssec maintain;
+	allow-update { any; };
+	zone-statistics full;
+	dnssec-dnskey-kskonly yes;
+	update-check-ksk yes;
+};
+
+zone "manykeys" {
+	type primary;
+	file "manykeys.db.signed";
+	allow-update { any; };
+	zone-statistics full;
+	dnssec-policy "manykeys";
+};
diff --git a/bin/tests/system/statschannel/tests.sh b/bin/tests/system/statschannel/tests.sh
index 46ed3a4acb..f460882950 100644
--- a/bin/tests/system/statschannel/tests.sh
+++ b/bin/tests/system/statschannel/tests.sh
@@ -346,5 +346,34 @@ if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 n=`expr $n + 1`
 
+# Test sign operations after dnssec-policy change (removing keys).
+ret=0
+copy_setports ns2/named2.conf.in ns2/named.conf
+$RNDCCMD 10.53.0.2 reload 2>&1 | sed 's/^/I:ns2 /'
+# This should trigger the resign of DNSKEY (+1 ksk), and SOA, NSEC,
+# TYPE65534 (+3 zsk). The dnssec-sign statistics for the removed keys should
+# be cleared and thus no longer visible. But NSEC and SOA are (mistakenly)
+# counted double, one time because of zone_resigninc and one time because of
+# zone_nsec3chain. So +5 zsk in total.
+echo "${refresh_prefix} ${zsk8_id}: 15" > zones.expect
+echo "${refresh_prefix} ${ksk8_id}: 2" >> zones.expect
+echo "${sign_prefix} ${zsk8_id}: 18" >> zones.expect
+echo "${sign_prefix} ${ksk8_id}: 2" >> zones.expect
+cat zones.expect | sort > zones.expect.$n
+rm -f zones.expect
+# Fetch and check the dnssec sign statistics.
+echo_i "fetching zone '$zone' stats data after dnssec-policy change ($n)"
+if [ $PERL_XML ]; then
+    getzones xml $zone x$n || ret=1
+    cmp zones.out.x$n zones.expect.$n || ret=1
+fi
+if [ $PERL_JSON ]; then
+    getzones json 2 j$n || ret=1
+    cmp zones.out.j$n zones.expect.$n || ret=1
+fi
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=`expr $status + $ret`
+n=`expr $n + 1`
+
 echo_i "exit status: $status"
 [ $status -eq 0 ] || exit 1

From 7505703a43c552ba8ec12f27b02880de170fbe29 Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Fri, 20 Aug 2021 15:10:42 +0200
Subject: [PATCH 7/7] Add CHANGES for [GL #1721]

(cherry picked from commit 8224dc8e351b6d1b36eca66498402d822b986565)
---
 CHANGES | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/CHANGES b/CHANGES
index 70b4b0e312..d128cedec5 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,6 @@
+5699.	[func]		Grow and shrink dnssec-sign statistics on key rollover
+			events. [GL #1721]
+
 5698.	[bug]		Migrate a single key to CSK when reconfiguring a zone
 			to use 'dnssec-policy'. [GL #2857]