From 0667bf7ae71e4ae5203c85901fdfe3ec0e070038 Mon Sep 17 00:00:00 2001
From: Mark Andrews <marka@isc.org>
Date: Thu, 3 May 2018 16:43:15 +1000
Subject: [PATCH 1/4] only sign with other keys when deleting a key if there
 are not already existing signature for the deleted algorithm

---
 lib/dns/zone.c | 79 +++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 71 insertions(+), 8 deletions(-)

diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index 227b9de56a..22ba83bad1 100644
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -805,7 +805,8 @@ static void zone_maintenance(dns_zone_t *zone);
 static void zone_notify(dns_zone_t *zone, isc_time_t *now);
 static void dump_done(void *arg, isc_result_t result);
 static isc_result_t zone_signwithkey(dns_zone_t *zone, dns_secalg_t algorithm,
-				     isc_uint16_t keyid, isc_boolean_t deleteit);
+				     isc_uint16_t keyid,
+				     isc_boolean_t deleteit);
 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);
@@ -8209,15 +8210,26 @@ zone_nsec3chain(dns_zone_t *zone) {
 	INSIST(version == NULL);
 }
 
+/*%
+ * Delete all RRSIG records with the given algorithm and keyid.
+ * Remove the NSEC record and RRSIGs if nkeys is zero.
+ * If all remaining RRsets are signed with the given algorithm
+ * set *has_algp to ISC_TRUE.
+ */
 static isc_result_t
 del_sig(dns_db_t *db, dns_dbversion_t *version, dns_name_t *name,
 	dns_dbnode_t *node, unsigned int nkeys, dns_secalg_t algorithm,
-	isc_uint16_t keyid, dns_diff_t *diff)
+	isc_uint16_t keyid, isc_boolean_t *has_algp, dns_diff_t *diff)
 {
 	dns_rdata_rrsig_t rrsig;
 	dns_rdataset_t rdataset;
 	dns_rdatasetiter_t *iterator = NULL;
 	isc_result_t result;
+	isc_boolean_t alg_missed = ISC_FALSE;
+	isc_boolean_t alg_found = ISC_FALSE;
+
+	char namebuf[DNS_NAME_FORMATSIZE];
+	dns_name_format(name, namebuf, sizeof(namebuf));
 
 	result = dns_db_allrdatasets(db, node, version, 0, &iterator);
 	if (result != ISC_R_SUCCESS) {
@@ -8230,6 +8242,7 @@ del_sig(dns_db_t *db, dns_dbversion_t *version, dns_name_t *name,
 	for (result = dns_rdatasetiter_first(iterator);
 	     result == ISC_R_SUCCESS;
 	     result = dns_rdatasetiter_next(iterator)) {
+		isc_boolean_t has_alg = ISC_FALSE;
 		dns_rdatasetiter_current(iterator, &rdataset);
 		if (nkeys == 0 && rdataset.type == dns_rdatatype_nsec) {
 			for (result = dns_rdataset_first(&rdataset);
@@ -8252,13 +8265,20 @@ del_sig(dns_db_t *db, dns_dbversion_t *version, dns_name_t *name,
 		}
 		for (result = dns_rdataset_first(&rdataset);
 		     result == ISC_R_SUCCESS;
-		     result = dns_rdataset_next(&rdataset)) {
+		     result = dns_rdataset_next(&rdataset))
+		{
 			dns_rdata_t rdata = DNS_RDATA_INIT;
 			dns_rdataset_current(&rdataset, &rdata);
 			CHECK(dns_rdata_tostruct(&rdata, &rrsig, NULL));
-			if (rrsig.algorithm != algorithm ||
-			    rrsig.keyid != keyid)
+			if (nkeys != 0 &&
+			    (rrsig.algorithm != algorithm ||
+			     rrsig.keyid != keyid))
+			{
+				if (rrsig.algorithm == algorithm) {
+					has_alg = ISC_TRUE;
+				}
 				continue;
+			}
 			CHECK(update_one_rr(db, version, diff,
 					    DNS_DIFFOP_DELRESIGN, name,
 					    rdataset.ttl, &rdata));
@@ -8266,9 +8286,25 @@ del_sig(dns_db_t *db, dns_dbversion_t *version, dns_name_t *name,
 		dns_rdataset_disassociate(&rdataset);
 		if (result != ISC_R_NOMORE)
 			break;
+
+		/*
+		 * After deleting, if there's still a signature for
+		 * 'algorithm', set alg_found; if not, set alg_missed.
+		 */
+		if (has_alg) {
+			alg_found = ISC_TRUE;
+		} else {
+			alg_missed = ISC_TRUE;
+		}
 	}
 	if (result == ISC_R_NOMORE)
 		result = ISC_R_SUCCESS;
+
+	/*
+	 * Set `has_algp` if the algorithm was found in every RRset:
+	 * i.e., found in at least one, and not missing from any.
+	 */
+	*has_algp = ISC_TF(alg_found && !alg_missed);
  failure:
 	if (dns_rdataset_isassociated(&rdataset))
 		dns_rdataset_disassociate(&rdataset);
@@ -8298,6 +8334,7 @@ zone_sign(dns_zone_t *zone) {
 	dst_key_t *zone_keys[DNS_MAXZONEKEYS];
 	isc_int32_t signatures;
 	isc_boolean_t check_ksk, keyset_kskonly, is_ksk;
+	isc_boolean_t with_ksk, with_zsk;
 	isc_boolean_t commit = ISC_FALSE;
 	isc_boolean_t delegation;
 	isc_boolean_t build_nsec = ISC_FALSE;
@@ -8387,6 +8424,7 @@ zone_sign(dns_zone_t *zone) {
 		build_nsec = ISC_TRUE;
 
 	while (signing != NULL && nodes-- > 0 && signatures > 0) {
+		isc_boolean_t has_alg = ISC_FALSE;
 		nextsigning = ISC_LIST_NEXT(signing, link);
 
 		ZONEDB_LOCK(&zone->dblock, isc_rwlocktype_read);
@@ -8426,6 +8464,9 @@ zone_sign(dns_zone_t *zone) {
 				zone_keys[j] = zone_keys[i];
 				j++;
 			}
+			for (i = j; i < nkeys; i++) {
+				zone_keys[i] = NULL;
+			}
 			nkeys = j;
 		}
 
@@ -8435,7 +8476,7 @@ zone_sign(dns_zone_t *zone) {
 			dns_dbiterator_pause(signing->dbiterator);
 			CHECK(del_sig(db, version, name, node, nkeys,
 				      signing->algorithm, signing->keyid,
-				      zonediff.diff));
+				      &has_alg, zonediff.diff));
 		}
 
 		/*
@@ -8466,8 +8507,10 @@ zone_sign(dns_zone_t *zone) {
 		/*
 		 * Process one node.
 		 */
+		with_ksk = ISC_FALSE;
+		with_zsk = ISC_FALSE;
 		dns_dbiterator_pause(signing->dbiterator);
-		for (i = 0; i < nkeys; i++) {
+		for (i = 0; !has_alg && i < nkeys; i++) {
 			isc_boolean_t both = ISC_FALSE;
 
 			/*
@@ -8537,6 +8580,19 @@ zone_sign(dns_zone_t *zone) {
 			else
 				is_ksk = ISC_FALSE;
 
+			/*
+			 * If deleting signatures, we need to ensure that
+			 * the RRset is still signed at least once by a
+			 * KSK and a ZSK.
+			 */
+			if (signing->deleteit && !is_ksk && with_zsk) {
+				continue;
+			}
+
+			if (signing->deleteit && is_ksk && with_ksk) {
+				continue;
+			}
+
 			CHECK(sign_a_node(db, name, node, version, build_nsec3,
 					  build_nsec, zone_keys[i], inception,
 					  expire, zone->minimum, is_ksk,
@@ -8547,8 +8603,15 @@ zone_sign(dns_zone_t *zone) {
 			 * If we are adding we are done.  Look for other keys
 			 * of the same algorithm if deleting.
 			 */
-			if (!signing->deleteit)
+			if (!signing->deleteit) {
 				break;
+			}
+			if (!is_ksk) {
+				with_zsk = ISC_TRUE;
+			}
+			if (KSK(zone_keys[i])) {
+				with_ksk = ISC_TRUE;
+			}
 		}
 
 		/*

From 87a3dc8ab930ce4b3f338905903ffa08e4113159 Mon Sep 17 00:00:00 2001
From: Mark Andrews <marka@isc.org>
Date: Thu, 3 May 2018 16:43:15 +1000
Subject: [PATCH 2/4] add support -T sigvalinsecs

---
 bin/named/main.c           |  5 ++++
 bin/named/zoneconf.c       | 41 ++++++++++++++++++---------------
 lib/dns/zone.c             | 47 ++++++++++++++++++++++++++++++++------
 lib/ns/include/ns/server.h |  1 +
 4 files changed, 69 insertions(+), 25 deletions(-)

diff --git a/bin/named/main.c b/bin/named/main.c
index 341a8c794d..6838d3c4c4 100644
--- a/bin/named/main.c
+++ b/bin/named/main.c
@@ -129,6 +129,7 @@ static unsigned int delay = 0;
 static isc_boolean_t nonearest = ISC_FALSE;
 static isc_boolean_t notcp = ISC_FALSE;
 static isc_boolean_t fixedlocal = ISC_FALSE;
+static isc_boolean_t sigvalinsecs = ISC_FALSE;
 
 /*
  * -4 and -6
@@ -541,6 +542,8 @@ parse_T_opt(char *option) {
 		if (dns_zone_mkey_month < dns_zone_mkey_day) {
 			named_main_earlyfatal("bad mkeytimer");
 		}
+	} else if (!strcmp(option, "sigvalinsecs")) {
+		sigvalinsecs = ISC_TRUE;
 	} else if (!strncmp(option, "tat=", 4)) {
 		named_g_tat_interval = atoi(option + 4);
 	} else {
@@ -1111,6 +1114,8 @@ setup(void) {
 		ns_server_setoption(sctx, NS_SERVER_DISABLE4, ISC_TRUE);
 	if (disable6)
 		ns_server_setoption(sctx, NS_SERVER_DISABLE6, ISC_TRUE);
+	if (sigvalinsecs)
+		ns_server_setoption(sctx, NS_SERVER_SIGVALINSECS, ISC_TRUE);
 
 	named_g_server->sctx->delay = delay;
 }
diff --git a/bin/named/zoneconf.c b/bin/named/zoneconf.c
index a6a59c3231..81040e5e24 100644
--- a/bin/named/zoneconf.c
+++ b/bin/named/zoneconf.c
@@ -1439,7 +1439,9 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig,
 	}
 
 	if (ztype == dns_zone_master || raw != NULL) {
+		const cfg_obj_t *validity, *resign;
 		isc_boolean_t allow = ISC_FALSE, maint = ISC_FALSE;
+		isc_boolean_t sigvalinsecs;
 
 		obj = NULL;
 		result = named_config_get(maps, "dnskey-sig-validity", &obj);
@@ -1450,26 +1452,29 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig,
 		obj = NULL;
 		result = named_config_get(maps, "sig-validity-interval", &obj);
 		INSIST(result == ISC_R_SUCCESS && obj != NULL);
-		{
-			const cfg_obj_t *validity, *resign;
 
-			validity = cfg_tuple_get(obj, "validity");
-			seconds = cfg_obj_asuint32(validity) * 86400;
-			dns_zone_setsigvalidityinterval(zone, seconds);
-
-			resign = cfg_tuple_get(obj, "re-sign");
-			if (cfg_obj_isvoid(resign)) {
-				seconds /= 4;
-			} else {
-				if (seconds > 7 * 86400)
-					seconds = cfg_obj_asuint32(resign) *
-							86400;
-				else
-					seconds = cfg_obj_asuint32(resign) *
-							3600;
-			}
-			dns_zone_setsigresigninginterval(zone, seconds);
+		sigvalinsecs = ns_server_getoption(named_g_server->sctx,
+						   NS_SERVER_SIGVALINSECS);
+		validity = cfg_tuple_get(obj, "validity");
+		seconds = cfg_obj_asuint32(validity);
+		if (!sigvalinsecs) {
+			seconds *= 86400;
 		}
+		dns_zone_setsigvalidityinterval(zone, seconds);
+
+		resign = cfg_tuple_get(obj, "re-sign");
+		if (cfg_obj_isvoid(resign)) {
+			seconds /= 4;
+		} else if (!sigvalinsecs) {
+			if (seconds > 7 * 86400) {
+				seconds = cfg_obj_asuint32(resign) * 86400;
+			} else {
+				seconds = cfg_obj_asuint32(resign) * 3600;
+			}
+		} else {
+			seconds = cfg_obj_asuint32(resign);
+		}
+		dns_zone_setsigresigninginterval(zone, seconds);
 
 		obj = NULL;
 		result = named_config_get(maps, "key-directory", &obj);
diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index 22ba83bad1..adb76a5971 100644
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -6412,6 +6412,7 @@ zone_resigninc(dns_zone_t *zone) {
 	isc_boolean_t check_ksk, keyset_kskonly = ISC_FALSE;
 	isc_result_t result;
 	isc_stdtime_t now, inception, soaexpire, expire, stop;
+	isc_uint32_t jitter, sigvalidityinterval;
 	unsigned int i;
 	unsigned int nkeys = 0;
 	unsigned int resign;
@@ -6456,14 +6457,24 @@ zone_resigninc(dns_zone_t *zone) {
 		goto failure;
 	}
 
+	sigvalidityinterval = zone->sigvalidityinterval;
 	inception = now - 3600;	/* Allow for clock skew. */
-	soaexpire = now + dns_zone_getsigvalidityinterval(zone);
+	soaexpire = now + sigvalidityinterval;
 	/*
 	 * 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.
 	 */
-	expire = soaexpire - isc_random_uniform(3600) - 1;
+	if (sigvalidityinterval >= 3600U) {
+		if (sigvalidityinterval > 7200U) {
+			jitter = isc_random_uniform(3600);
+		} else {
+			jitter = isc_random_uniform(1200);
+		}
+		expire = soaexpire - jitter - 1;
+	} else {
+		expire = soaexpire - 1;
+	}
 	stop = now + 5;
 
 	check_ksk = DNS_ZONE_OPTION(zone, DNS_ZONEOPT_UPDATECHECKKSK);
@@ -7406,6 +7417,7 @@ zone_nsec3chain(dns_zone_t *zone) {
 	isc_boolean_t first;
 	isc_result_t result;
 	isc_stdtime_t now, inception, soaexpire, expire;
+	isc_uint32_t jitter, sigvalidityinterval;
 	unsigned int i;
 	unsigned int nkeys = 0;
 	isc_uint32_t nodes;
@@ -7474,15 +7486,25 @@ zone_nsec3chain(dns_zone_t *zone) {
 		goto failure;
 	}
 
+	sigvalidityinterval = dns_zone_getsigvalidityinterval(zone);
 	inception = now - 3600;	/* Allow for clock skew. */
-	soaexpire = now + dns_zone_getsigvalidityinterval(zone);
+	soaexpire = now + sigvalidityinterval;
 
 	/*
 	 * 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.
 	 */
-	expire = soaexpire - isc_random_uniform(3600);
+	if (sigvalidityinterval >= 3600U) {
+		if (sigvalidityinterval > 7200U) {
+			jitter = isc_random_uniform(3600);
+		} else {
+			jitter = isc_random_uniform(1200);
+		}
+		expire = soaexpire - jitter - 1;
+	} else {
+		expire = soaexpire - 1;
+	}
 
 	check_ksk = DNS_ZONE_OPTION(zone, DNS_ZONEOPT_UPDATECHECKKSK);
 	keyset_kskonly = DNS_ZONE_OPTION(zone, DNS_ZONEOPT_DNSKEYKSKONLY);
@@ -8342,6 +8364,7 @@ zone_sign(dns_zone_t *zone) {
 	isc_boolean_t first;
 	isc_result_t result;
 	isc_stdtime_t now, inception, soaexpire, expire;
+	isc_uint32_t jitter, sigvalidityinterval;
 	unsigned int i, j;
 	unsigned int nkeys = 0;
 	isc_uint32_t nodes;
@@ -8392,15 +8415,25 @@ zone_sign(dns_zone_t *zone) {
 		goto failure;
 	}
 
+	sigvalidityinterval = dns_zone_getsigvalidityinterval(zone);
 	inception = now - 3600;	/* Allow for clock skew. */
-	soaexpire = now + dns_zone_getsigvalidityinterval(zone);
+	soaexpire = now + sigvalidityinterval;
 
 	/*
 	 * 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.
 	 */
-	expire = soaexpire - isc_random_uniform(3600);
+	if (sigvalidityinterval >= 3600U) {
+		if (sigvalidityinterval > 7200U) {
+			jitter = isc_random_uniform(3600);
+		} else {
+			jitter = isc_random_uniform(1200);
+		}
+		expire = soaexpire - jitter - 1;
+	} else {
+		expire = soaexpire - 1;
+	}
 
 	/*
 	 * We keep pulling nodes off each iterator in turn until
@@ -17633,7 +17666,7 @@ sign_apex(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver,
 
 	keyexpire = dns_zone_getkeyvalidityinterval(zone);
 	if (keyexpire == 0) {
-		keyexpire = soaexpire;
+		keyexpire = soaexpire - 1;
 	} else {
 		keyexpire += now;
 	}
diff --git a/lib/ns/include/ns/server.h b/lib/ns/include/ns/server.h
index 880de7dad2..bcfb69a92c 100644
--- a/lib/ns/include/ns/server.h
+++ b/lib/ns/include/ns/server.h
@@ -40,6 +40,7 @@
 #define NS_SERVER_DISABLE4	0x00000100U	/*%< -6 */
 #define NS_SERVER_DISABLE6	0x00000200U	/*%< -4 */
 #define NS_SERVER_FIXEDLOCAL	0x00000400U	/*%< -T fixedlocal */
+#define NS_SERVER_SIGVALINSECS	0x00000800U	/*%< -T sigvalinsecs */
 
 /*%
  * Type for callback function to get hostname.

From 0db5b087ed2d5bedfe5ef09ad9ea0fa347eeb18b Mon Sep 17 00:00:00 2001
From: Mark Andrews <marka@isc.org>
Date: Thu, 3 May 2018 16:50:32 +1000
Subject: [PATCH 3/4] add duplicate signature test

---
 bin/tests/system/conf.sh.in                   |   4 +-
 bin/tests/system/conf.sh.win32                |   3 +-
 bin/tests/system/dupsigs/check_journal.pl     | 209 ++++++++++++++++++
 bin/tests/system/dupsigs/clean.sh             |  19 ++
 bin/tests/system/dupsigs/ns1/named.args       |   1 +
 bin/tests/system/dupsigs/ns1/named.conf.in    |  31 +++
 bin/tests/system/dupsigs/ns1/reset_keys.sh    |  97 ++++++++
 .../system/dupsigs/ns1/signing.test.db.in     |  16 ++
 bin/tests/system/dupsigs/prereq.sh            |  15 ++
 bin/tests/system/dupsigs/setup.sh             |  20 ++
 bin/tests/system/dupsigs/tests.sh             |  35 +++
 util/copyrights                               |   9 +
 12 files changed, 456 insertions(+), 3 deletions(-)
 create mode 100644 bin/tests/system/dupsigs/check_journal.pl
 create mode 100644 bin/tests/system/dupsigs/clean.sh
 create mode 100644 bin/tests/system/dupsigs/ns1/named.args
 create mode 100644 bin/tests/system/dupsigs/ns1/named.conf.in
 create mode 100644 bin/tests/system/dupsigs/ns1/reset_keys.sh
 create mode 100644 bin/tests/system/dupsigs/ns1/signing.test.db.in
 create mode 100644 bin/tests/system/dupsigs/prereq.sh
 create mode 100644 bin/tests/system/dupsigs/setup.sh
 create mode 100644 bin/tests/system/dupsigs/tests.sh

diff --git a/bin/tests/system/conf.sh.in b/bin/tests/system/conf.sh.in
index 5518c7918f..e4d60eaa18 100644
--- a/bin/tests/system/conf.sh.in
+++ b/bin/tests/system/conf.sh.in
@@ -71,8 +71,8 @@ KRB5_CONFIG=/dev/null
 
 # The "stress" test is not run by default since it creates enough
 # load on the machine to make it unusable to other users.
-# The "dialup" and "delzone" tests are also not run by default because
-# they take a very long time to complete.
+# The "dialup", "delzone", and "dupsigs" tests are also not run by
+# default because they take a very long time to complete.
 #
 # List of tests hard-coded to use ports 5300 and 9953. For this
 # reason, these must be run sequentially.
diff --git a/bin/tests/system/conf.sh.win32 b/bin/tests/system/conf.sh.win32
index 4da318fca8..6bfac1357c 100644
--- a/bin/tests/system/conf.sh.win32
+++ b/bin/tests/system/conf.sh.win32
@@ -77,7 +77,8 @@ KRB5_CONFIG=NUL
 
 # The "stress" test is not run by default since it creates enough
 # load on the machine to make it unusable to other users.
-# v6synth
+# The "dialup", "delzone", and "dupsigs" tests are also not run by
+# default because they take a very long time to complete.
 #
 # List of tests that use ports 5300 and 9953.  For this reason, these must
 # be run sequentially.
diff --git a/bin/tests/system/dupsigs/check_journal.pl b/bin/tests/system/dupsigs/check_journal.pl
new file mode 100644
index 0000000000..972ea8b008
--- /dev/null
+++ b/bin/tests/system/dupsigs/check_journal.pl
@@ -0,0 +1,209 @@
+#!/usr/bin/env perl
+#
+# 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.
+
+use strict;
+use warnings;
+
+sub process_changeset;
+
+my @changeset;
+
+while( my $line = <> ) {
+    chomp $line;
+
+    if( $line =~ /^(?<op>add|del) (?<label>\S+)\s+(?<ttl>\d+)\s+IN\s+(?<rrtype>\S+)\s+(?<rdata>.*)/ ) {
+        my $change = {
+            op     => $+{op},
+            label  => $+{label},
+            ttl    => $+{ttl},
+            rrtype => $+{rrtype},
+            rdata  => $+{rdata},
+        };
+
+        if( $change->{op} eq 'del' and $change->{rrtype} eq 'SOA' ) {
+            if( @changeset ) {
+                process_changeset( @changeset );
+                @changeset = ();
+            }
+        }
+
+        push @changeset, $change;
+    }
+    else {
+        die "error parsing journal data";
+    }
+}
+
+if( @changeset ) {
+    process_changeset( @changeset );
+}
+
+{
+    my %rrsig_db;
+    my %keys;
+    my $apex;
+
+    sub process_changeset {
+        my @changeset = @_;
+
+        if( not $apex ) {
+            # the first record of the first changeset is guaranteed to be the apex
+            $apex = $changeset[0]{label};
+        }
+
+        my $newserial;
+        my %touched_rrsigs;
+        my %touched_keys;
+
+        foreach my $change( @changeset ) {
+            if( $change->{rrtype} eq 'SOA' ) {
+                if( $change->{op} eq 'add' ) {
+                    if( $change->{rdata} !~ /^\S+ \S+ (?<serial>\d+)/ ) {
+                        die "unable to parse SOA";
+                    }
+
+                    $newserial = $+{serial};
+                }
+            }
+            elsif( $change->{rrtype} eq 'NSEC' ) {
+                ; # do nothing
+            }
+            elsif( $change->{rrtype} eq 'DNSKEY' ) {
+                ; # ignore for now
+            }
+            elsif( $change->{rrtype} eq 'TYPE65534' and $change->{label} eq $apex ) {
+                # key status
+                if( $change->{rdata} !~ /^\\# (?<datasize>\d+) (?<data>[0-9A-F]+)$/ ) {
+                    die "unable to parse key status record";
+                }
+
+                my $datasize = $+{datasize};
+                my $data = $+{data};
+
+                if( $datasize == 5 ) {
+                    my( $alg, $id, $flag_del, $flag_done ) = unpack 'CnCC', pack( 'H10', $data );
+
+                    if( $change->{op} eq 'add' ) {
+                        if( not exists $keys{$id} ) {
+                            $touched_keys{$id} //= 1;
+
+                            $keys{$id} = {
+                                $data        => 1,
+                                rrs          => 1,
+                                done_signing => $flag_done,
+                                deleting     => $flag_del,
+                            };
+                        }
+                        else {
+                            if( not exists $keys{$id}{$data} ) {
+                                my $keydata = $keys{$id};
+                                $touched_keys{$id} = { %$keydata };
+
+                                $keydata->{rrs}++;
+                                $keydata->{$data} = 1;
+                                $keydata->{done_signing} += $flag_done;
+                                $keydata->{deleting} += $flag_del;
+                            }
+                        }
+                    }
+                    else {
+                        # this logic relies upon the convention that there won't
+                        # ever be multiple records with the same flag set
+                        if( exists $keys{$id} ) {
+                            my $keydata = $keys{$id};
+
+                            if( exists $keydata->{$data} ) {
+                                $touched_keys{$id} = { %$keydata };
+
+                                $keydata->{rrs}--;
+                                delete $keydata->{$data};
+                                $keydata->{done_signing} -= $flag_done;
+                                $keydata->{deleting} -= $flag_del;
+
+                                if( $keydata->{rrs} == 0 ) {
+                                    delete $keys{$id};
+                                }
+                            }
+                        }
+                    }
+                }
+                else {
+                    die "unexpected key status record content";
+                }
+            }
+            elsif( $change->{rrtype} eq 'RRSIG' ) {
+                if( $change->{rdata} !~ /^(?<covers>\S+) \d+ \d+ \d+ (?<validity_end>\d+) (?<validity_start>\d+) (?<signing_key>\d+)/ ) {
+                    die "unable to parse RRSIG rdata";
+                }
+
+                $change->{covers} = $+{covers};
+                $change->{validity_end} = $+{validity_end};
+                $change->{validity_start} = $+{validity_start};
+                $change->{signing_key} = $+{signing_key};
+
+                my $db_key = $change->{label} . ':' . $change->{covers};
+
+                $rrsig_db{$db_key} //= {};
+                $touched_rrsigs{$db_key} = 1;
+
+                if( $change->{op} eq 'add' ) {
+                    $rrsig_db{$db_key}{ $change->{signing_key} } = 1;
+                }
+                else {
+                    # del
+                    delete $rrsig_db{$db_key}{ $change->{signing_key} };
+                }
+            }
+        }
+
+        foreach my $key_id( sort keys %touched_keys ) {
+            my $old_data;
+            my $new_data;
+
+            if( ref $touched_keys{$key_id} ) {
+                $old_data = $touched_keys{$key_id};
+            }
+
+            if( exists $keys{$key_id} ) {
+                $new_data = $keys{$key_id};
+            }
+
+            if( $old_data ) {
+                if( $new_data ) {
+                    print "at serial $newserial key $key_id status changed from ($old_data->{deleting},$old_data->{done_signing}) to ($new_data->{deleting},$new_data->{done_signing})\n";
+                }
+                else {
+                    print "at serial $newserial key $key_id status removed from zone\n";
+                }
+            }
+            else {
+                print "at serial $newserial key $key_id status added with flags ($new_data->{deleting},$new_data->{done_signing})\n";
+            }
+        }
+
+        foreach my $rrsig_id( sort keys %touched_rrsigs ) {
+            my $n_signing_keys = keys %{ $rrsig_db{$rrsig_id} };
+
+            if( $n_signing_keys == 0 ) {
+                print "at serial $newserial $rrsig_id went unsigned\n";
+            }
+            elsif( $rrsig_id =~ /:DNSKEY$/ ) {
+                if( $n_signing_keys != 2 ) {
+                    print "at serial $newserial $rrsig_id was signed $n_signing_keys time(s) when it should have been signed twice\n";
+                }
+            }
+            elsif( $n_signing_keys > 1 ) {
+                my @signing_keys = sort { $a <=> $b } keys %{ $rrsig_db{$rrsig_id} };
+                print "at serial $newserial $rrsig_id was signed too many times, keys (@signing_keys)\n";
+            }
+        }
+    }
+}
diff --git a/bin/tests/system/dupsigs/clean.sh b/bin/tests/system/dupsigs/clean.sh
new file mode 100644
index 0000000000..7a6d78e760
--- /dev/null
+++ b/bin/tests/system/dupsigs/clean.sh
@@ -0,0 +1,19 @@
+# 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.
+
+rm -f ns1/named.conf
+rm -f ns1/named.lock
+rm -f ns1/named.memstats
+rm -f ns1/named.run
+rm -f ns1/signing.test.db
+rm -f ns1/signing.test.db.jbk
+rm -f ns1/signing.test.db.signed
+rm -f ns1/signing.test.db.signed.jnl
+rm -f ns1/keys/signing.test/K*
+rm -f ns1/managed-keys.bind*
diff --git a/bin/tests/system/dupsigs/ns1/named.args b/bin/tests/system/dupsigs/ns1/named.args
new file mode 100644
index 0000000000..c7cab8aa5b
--- /dev/null
+++ b/bin/tests/system/dupsigs/ns1/named.args
@@ -0,0 +1 @@
+-D dupsigs-ns1 -X named.lock -m record,size,mctx -T clienttest -c named.conf -d 99 -g -U 4 -T sigvalinsecs
diff --git a/bin/tests/system/dupsigs/ns1/named.conf.in b/bin/tests/system/dupsigs/ns1/named.conf.in
new file mode 100644
index 0000000000..296ad0ecc1
--- /dev/null
+++ b/bin/tests/system/dupsigs/ns1/named.conf.in
@@ -0,0 +1,31 @@
+/*
+ * 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 {
+	recursion no;
+	max-journal-size unlimited;
+	port @PORT@;
+	listen-on { 10.53.0.1; };
+	listen-on-v6 { none; };
+	pid-file "named.pid";
+};
+
+zone "signing.test" {
+	type master;
+	masterfile-format text;
+	allow-update { any; };
+	file "signing.test.db";
+	update-check-ksk yes;
+	key-directory "keys/signing.test";
+	inline-signing yes;
+	auto-dnssec maintain;
+	sig-validity-interval 120 30;
+};
diff --git a/bin/tests/system/dupsigs/ns1/reset_keys.sh b/bin/tests/system/dupsigs/ns1/reset_keys.sh
new file mode 100644
index 0000000000..7e0f5a421d
--- /dev/null
+++ b/bin/tests/system/dupsigs/ns1/reset_keys.sh
@@ -0,0 +1,97 @@
+#!/bin/sh
+#
+# 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.
+
+SYSTEMTESTTOP=../..
+. $SYSTEMTESTTOP/conf.sh
+
+zone=signing.test
+rm -rf keys/signing.test
+mkdir -p keys/signing.test
+
+timetodnssec() {
+    $PERL -e 'my ($S,$M,$H,$d,$m,$y,$x) = gmtime(@ARGV[0]);
+	      printf("%04u%02u%02u%02u%02u%02u\n", $y+1900,$m+1,$d,$H,$M,$S);' ${1}
+}
+
+KEYDIR=keys/signing.test
+KSK=`$KEYGEN -a RSASHA256 -K $KEYDIR -q -f KSK $zone`
+
+ZSK0=`$KEYGEN -a RSASHA256 -K $KEYDIR -q $zone`
+ZSK1=`$KEYGEN -a RSASHA256 -K $KEYDIR -q $zone`
+ZSK2=`$KEYGEN -a RSASHA256 -K $KEYDIR -q $zone`
+ZSK3=`$KEYGEN -a RSASHA256 -K $KEYDIR -q $zone`
+ZSK4=`$KEYGEN -a RSASHA256 -K $KEYDIR -q $zone`
+ZSK5=`$KEYGEN -a RSASHA256 -K $KEYDIR -q $zone`
+ZSK6=`$KEYGEN -a RSASHA256 -K $KEYDIR -q $zone`
+ZSK7=`$KEYGEN -a RSASHA256 -K $KEYDIR -q $zone`
+ZSK8=`$KEYGEN -a RSASHA256 -K $KEYDIR -q $zone`
+ZSK9=`$KEYGEN -a RSASHA256 -K $KEYDIR -q $zone`
+
+# clear all times on all keys
+for FILEN in keys/signing.test/*.key
+do
+    $SETTIME -P none -A none -R none -I none -D none $FILEN
+done
+
+BASE=`date +%s`
+BASET=`timetodnssec $BASE`
+
+# reset the publish and activation time on the KSK
+$SETTIME -P $BASET -A $BASET $KEYDIR/$KSK
+
+# reset the publish and activation time on the first ZSK
+$SETTIME -P $BASET -A $BASET $KEYDIR/$ZSK0
+
+# schedule the first roll
+R1=`expr $BASE + 300`
+R1T=`timetodnssec $R1`
+
+$SETTIME -I $R1T $KEYDIR/$ZSK0
+$SETTIME -P $BASET -A $R1T $KEYDIR/$ZSK1
+
+# schedule the second roll (which includes the delete of the first key)
+R2=`expr $R1 + 300`
+R2T=`timetodnssec $R2`
+DT=$R2
+DTT=`timetodnssec $DT`
+
+$SETTIME -D $DTT $KEYDIR/$ZSK0
+$SETTIME -I $R2T $KEYDIR/$ZSK1
+$SETTIME -P $R1T -A $R2T $KEYDIR/$ZSK2
+
+# schedule the third roll
+# this isn't long enough for the signing to complete
+R3=`expr $R2 + 60`
+R3T=`timetodnssec $R3`
+
+$SETTIME -D $R3T $KEYDIR/$ZSK1
+$SETTIME -I $R3T $KEYDIR/$ZSK2
+$SETTIME -P $R2T -A $R3T $KEYDIR/$ZSK3
+
+$SETTIME -P $R3T $KEYDIR/$ZSK4
+
+echo KSK=$KSK
+echo ZSK0=$ZSK0
+echo ZSK1=$ZSK1
+echo ZSK2=$ZSK2
+echo ZSK3=$ZSK3
+echo ZSK4=$ZSK4
+
+exit
+
+# schedule the fourth roll
+# this isn't long enough for the signing to complete
+R4=`expr $R3 + 30`
+R4T=`timetodnssec $R4`
+
+$SETTIME -D $R4T $KEYDIR/$ZSK2
+$SETTIME -I $R4T $KEYDIR/$ZSK3
+$SETTIME -P $R3T -A $R4T $KEYDIR/$ZSK4
diff --git a/bin/tests/system/dupsigs/ns1/signing.test.db.in b/bin/tests/system/dupsigs/ns1/signing.test.db.in
new file mode 100644
index 0000000000..072efe16b3
--- /dev/null
+++ b/bin/tests/system/dupsigs/ns1/signing.test.db.in
@@ -0,0 +1,16 @@
+; 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 3600
+@ IN SOA  ns root.ns 1996072700 3600 1800 86400 60
+@    NS   ns
+ns   A    127.0.0.1
+ns   AAAA ::1
+
+$GENERATE 0-1999 a${0,4,d} AAAA ::$
diff --git a/bin/tests/system/dupsigs/prereq.sh b/bin/tests/system/dupsigs/prereq.sh
new file mode 100644
index 0000000000..a0d4e9ce14
--- /dev/null
+++ b/bin/tests/system/dupsigs/prereq.sh
@@ -0,0 +1,15 @@
+#!/bin/sh
+#
+# 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.
+
+SYSTEMTESTTOP=..
+. $SYSTEMTESTTOP/conf.sh
+
+exec $SHELL ../testcrypto.sh
diff --git a/bin/tests/system/dupsigs/setup.sh b/bin/tests/system/dupsigs/setup.sh
new file mode 100644
index 0000000000..0c37928a85
--- /dev/null
+++ b/bin/tests/system/dupsigs/setup.sh
@@ -0,0 +1,20 @@
+# 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.
+
+SYSTEMTESTTOP=..
+. $SYSTEMTESTTOP/conf.sh
+
+test -f clean.sh && $SHELL clean.sh
+
+test -r $RANDFILE || $GENRANDOM 800 $RANDFILE
+
+copy_setports ns1/named.conf.in ns1/named.conf
+
+cp -f ns1/signing.test.db.in ns1/signing.test.db
+(cd ns1; $SHELL ./reset_keys.sh)
diff --git a/bin/tests/system/dupsigs/tests.sh b/bin/tests/system/dupsigs/tests.sh
new file mode 100644
index 0000000000..80ddaeaf11
--- /dev/null
+++ b/bin/tests/system/dupsigs/tests.sh
@@ -0,0 +1,35 @@
+# 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.
+
+SYSTEMTESTTOP=..
+. $SYSTEMTESTTOP/conf.sh
+
+status=0
+start=`date +%s`
+end=`expr $start + 1200`
+now=$start
+while test $now -lt $end
+do
+	et=`expr $now - $start`
+	echo "=============== $et ============"
+	$JOURNALPRINT ns1/signing.test.db.signed.jnl | $PERL check_journal.pl
+	$DIG axfr signing.test -p 5300 @10.53.0.1 > dig.out.at$et
+	awk '$4 == "RRSIG" { print $11 }' dig.out.at$et | sort | uniq -c
+	lines=`awk '$4 == "RRSIG" { print}' dig.out.at$et | wc -l`
+	if [ ${et} -ne 0 -a ${lines} -ne 4009 ]
+	then
+		echo_i "failed"
+		status=`expr $status + 1`
+	fi
+	sleep 20
+	now=`date +%s`
+done
+
+echo_i "exit status: $status"
+[ $status -eq 0 ] || exit 1
diff --git a/util/copyrights b/util/copyrights
index 48443d8476..bcd7156aa0 100644
--- a/util/copyrights
+++ b/util/copyrights
@@ -1184,6 +1184,15 @@
 ./bin/tests/system/dsdigest/prereq.sh		SH	2012,2014,2016,2018
 ./bin/tests/system/dsdigest/setup.sh		SH	2012,2014,2016,2017,2018
 ./bin/tests/system/dsdigest/tests.sh		SH	2012,2016,2018
+./bin/tests/system/dupsigs/check_journal.pl	PERL	2018
+./bin/tests/system/dupsigs/clean.sh		SH	2018
+./bin/tests/system/dupsigs/ns1/named.args	X	2018
+./bin/tests/system/dupsigs/ns1/named.conf.in	CONF-C	2018
+./bin/tests/system/dupsigs/ns1/reset_keys.sh	SH	2018
+./bin/tests/system/dupsigs/ns1/signing.test.db.in	ZONE	2018
+./bin/tests/system/dupsigs/prereq.sh		SH	2018
+./bin/tests/system/dupsigs/setup.sh		SH	2018
+./bin/tests/system/dupsigs/tests.sh		SH	2018
 ./bin/tests/system/dyndb/Makefile.in		MAKE	2015,2016,2018
 ./bin/tests/system/dyndb/clean.sh		SH	2015,2016,2018
 ./bin/tests/system/dyndb/driver/.gitignore	X	2015,2018

From ba7a3431562b41b14ae47d7588a03f8e4419e858 Mon Sep 17 00:00:00 2001
From: Mark Andrews <marka@isc.org>
Date: Thu, 10 May 2018 15:04:50 +1000
Subject: [PATCH 4/4] add CHANGES note

---
 CHANGES | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/CHANGES b/CHANGES
index 9bde887fa6..4497dba1cb 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,7 @@
+4964.	[bug]		Reduce the probabilty of double signature when deleting
+			a DNSKEY by checking if the node is otherwise signed
+			by the algorithm of the key to be deleted. [GL #240]
+
 4963.	[test]		ifconfig.sh now uses "ip" instead of "ifconfig",
 			if available, to configure the test interfaces on
 			linux.  [GL #302]