Merge branch '159-improve-handling-of-inline-signed-zones-with-missing-signing-keys' into 'v9_10'

Improve handling of inline signed zones with missing signing keys

Closes #159

See merge request isc-projects/bind9!133

CHANGES

@@ -1,3 +1,12 @@
+4933.	[bug]		Not creating signing keys for an inline signed zone
+			prevented changes applied to the raw zone from being
+			reflected in the secure zone until signing keys were
+			made available. [GL #159]
+
+4932.	[bug]		Bumped signed serial of an inline signed zone was
+			logged even when an error occurred while updating
+			signatures. [GL #159]
+
 4918.	[bug]		Fix double free after keygen error in dnssec-keygen
 			when OpenSSL >= 1.1.0 is used and RSA_generate_key_ex
 			fails. [GL #109]

bin/tests/system/inline/clean.sh

@@ -15,6 +15,7 @@
 rm -f */named.conf
 rm -f */named.memstats
 rm -f */named.run
+rm -f */named.run.prev
 rm -f */trusted.conf
 rm -f ns1/K*
 rm -f ns1/dsset-*
@@ -28,6 +29,10 @@ rm -f ns2/inactiveksk.db
 rm -f ns2/inactiveksk.db.jnl
 rm -f ns2/inactivezsk.db
 rm -f ns2/inactivezsk.db.jnl
+rm -f ns2/nokeys.db
+rm -f ns2/nokeys.db.jnl
+rm -f ns2/removedkeys-secondary.db
+rm -f ns2/removedkeys-secondary.db.jnl
 rm -f ns2/retransfer.db
 rm -f ns2/retransfer.db.jnl
 rm -f ns2/retransfer3.db
@@ -65,10 +70,22 @@ rm -f ns3/inactivezsk.bk
 rm -f ns3/inactivezsk.bk.jnl
 rm -f ns3/inactivezsk.bk.signed
 rm -f ns3/inactivezsk.bk.signed.jnl
+rm -f ns3/nokeys.bk
+rm -f ns3/nokeys.bk.jnl
+rm -f ns3/nokeys.bk.signed
+rm -f ns3/nokeys.bk.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/removedkeys-primary.db
+rm -f ns3/removedkeys-primary.db.jnl
+rm -f ns3/removedkeys-primary.db.signed
+rm -f ns3/removedkeys-primary.db.signed.jnl
+rm -f ns3/removedkeys-secondary.bk
+rm -f ns3/removedkeys-secondary.bk.jnl
+rm -f ns3/removedkeys-secondary.bk.signed
+rm -f ns3/removedkeys-secondary.bk.signed.jnl
 rm -f ns3/retransfer.bk
 rm -f ns3/retransfer.bk.jnl
 rm -f ns3/retransfer.bk.signed
@@ -106,3 +123,4 @@ rm -f checkgost checkdsa checkecdsa
 rm -f ns3/a-file
 rm -f dig.out.*
 rm -f rndc.out.ns*
+rm -rf ns3/removedkeys

bin/tests/system/inline/ns2/named.conf.in

@@ -73,3 +73,15 @@ zone "inactivezsk" {
 	file "inactivezsk.db";
 	allow-update { any; };
 };
+
+zone "nokeys" {
+	type master;
+	file "nokeys.db";
+	allow-update { any; };
+};
+
+zone "removedkeys-secondary" {
+	type master;
+	file "removedkeys-secondary.db";
+	allow-update { any; };
+};

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

@@ -137,3 +137,28 @@ zone "inactivezsk" {
 	auto-dnssec maintain;
 	file "inactivezsk.bk";
 };
+
+zone "nokeys" {
+	type slave;
+	masters { 10.53.0.2; };
+	inline-signing yes;
+	auto-dnssec maintain;
+	file "nokeys.bk";
+};
+
+zone "removedkeys-primary" {
+	type master;
+	inline-signing yes;
+	auto-dnssec maintain;
+	allow-update { any; };
+	also-notify { 10.53.0.2; };
+	file "removedkeys-primary.db";
+};
+
+zone "removedkeys-secondary" {
+	type slave;
+	masters { 10.53.0.2; };
+	inline-signing yes;
+	auto-dnssec maintain;
+	file "removedkeys-secondary.bk";
+};

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

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

bin/tests/system/inline/setup.sh

@@ -26,6 +26,8 @@ touch ns2/trusted.conf
 cp ns2/bits.db.in ns2/bits.db
 cp ns2/bits.db.in ns2/inactiveksk.db
 cp ns2/bits.db.in ns2/inactivezsk.db
+cp ns2/bits.db.in ns2/nokeys.db
+cp ns2/bits.db.in ns2/removedkeys-secondary.db
 cp ns2/bits.db.in ns2/retransfer.db
 cp ns2/bits.db.in ns2/retransfer3.db
 rm -f ns2/bits.db.jnl
@@ -36,6 +38,9 @@ cp ns3/master.db.in ns3/updated.db
 cp ns3/master.db.in ns3/expired.db
 cp ns3/master.db.in ns3/nsec3.db
 cp ns3/master.db.in ns3/externalkey.db
+cp ns3/master.db.in ns3/removedkeys-primary.db
+
+mkdir ns3/removedkeys
 
 touch ns4/trusted.conf
 cp ns4/noixfr.db.in ns4/noixfr.db

bin/tests/system/inline/tests.sh

@@ -1003,14 +1003,173 @@ grep "DNSKEY 8 1 [0-9]* [0-9]* [0-9]* ${kskid} " dig.out.ns3.test$n > /dev/null
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
+# Wait until an update to the raw part of a given inline signed zone is fully
+# processed.  As waiting for a fixed amount of time is suboptimal and there is
+# no single message that would signify both a successful modification and an
+# error in a race-free manner, instead wait until either notifies are sent
+# (which means the secure zone was modified) or a receive_secure_serial() error
+# is logged (which means the zone was not modified and will not be modified any
+# further in response to the relevant raw zone update).
+wait_until_raw_zone_update_is_processed() {
+	zone="$1"
+	for i in 1 2 3 4 5 6 7 8 9 10
+	do
+		if nextpart ns3/named.run | egrep "zone ${zone}.*(sending notifies|receive_secure_serial)" > /dev/null; then
+			return
+		fi
+		sleep 1
+	done
+}
+
+n=`expr $n + 1`
+echo_i "checking that changes to raw zone are applied to a previously unsigned secure zone ($n)"
+ret=0
+# Query for bar.nokeys/A and ensure the response is negative.  As this zone
+# does not have any signing keys set up, the response must be unsigned.
+$DIG $DIGOPTS @10.53.0.3 bar.nokeys. A > dig.out.ns3.pre.test$n 2>&1 || ret=1
+grep "status: NOERROR" dig.out.ns3.pre.test$n > /dev/null && ret=1
+grep "RRSIG" dig.out.ns3.pre.test$n > /dev/null && ret=1
+# Ensure the wait_until_raw_zone_update_is_processed() call below will ignore
+# log messages generated before the raw zone is updated.
+nextpart ns3/named.run > /dev/null
+# Add a record to the raw zone on the master.
+$NSUPDATE << EOF || ret=1
+zone nokeys.
+server 10.53.0.2 ${PORT}
+update add bar.nokeys. 0 A 127.0.0.1
+send
+EOF
+wait_until_raw_zone_update_is_processed "nokeys"
+# Query for bar.nokeys/A again and ensure the signer now returns a positive,
+# yet still unsigned response.
+$DIG $DIGOPTS @10.53.0.3 bar.nokeys. A > dig.out.ns3.post.test$n 2>&1
+grep "status: NOERROR" dig.out.ns3.post.test$n > /dev/null || ret=1
+grep "RRSIG" dig.out.ns3.pre.test$n > /dev/null && ret=1
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=`expr $status + $ret`
+
+n=`expr $n + 1`
+echo_i "checking that changes to raw zone are not applied to a previously signed secure zone with no keys available (primary) ($n)"
+ret=0
+# Query for bar.removedkeys-primary/A and ensure the response is negative.  As
+# this zone has signing keys set up, the response must be signed.
+$DIG $DIGOPTS @10.53.0.3 bar.removedkeys-primary. A > dig.out.ns3.pre.test$n 2>&1 || ret=1
+grep "status: NOERROR" dig.out.ns3.pre.test$n > /dev/null && ret=1
+grep "RRSIG" dig.out.ns3.pre.test$n > /dev/null || ret=1
+# Remove the signing keys for this zone.
+mv -f ns3/Kremovedkeys-primary* ns3/removedkeys
+# Ensure the wait_until_raw_zone_update_is_processed() call below will ignore
+# log messages generated before the raw zone is updated.
+nextpart ns3/named.run > /dev/null
+# Add a record to the raw zone on the master.
+$NSUPDATE << EOF || ret=1
+zone removedkeys-primary.
+server 10.53.0.3 ${PORT}
+update add bar.removedkeys-primary. 0 A 127.0.0.1
+send
+EOF
+wait_until_raw_zone_update_is_processed "removedkeys-primary"
+# Query for bar.removedkeys-primary/A again and ensure the signer still returns
+# a negative, signed response.
+$DIG $DIGOPTS @10.53.0.3 bar.removedkeys-primary. A > dig.out.ns3.post.test$n 2>&1
+grep "status: NOERROR" dig.out.ns3.post.test$n > /dev/null && ret=1
+grep "RRSIG" dig.out.ns3.pre.test$n > /dev/null || ret=1
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=`expr $status + $ret`
+
+n=`expr $n + 1`
+echo_i "checking that backlogged changes to raw zone are applied after keys become available (primary) ($n)"
+ret=0
+# Restore the signing keys for this zone.
+mv ns3/removedkeys/Kremovedkeys-primary* ns3
+$RNDCCMD 10.53.0.3 loadkeys removedkeys-primary > /dev/null 2>&1
+# Determine what a SOA record with a bumped serial number should look like.
+BUMPED_SOA=`sed -n 's/.*\(add removedkeys-primary.*IN.*SOA\)/\1/p;' ns3/named.run | tail -1 | awk '{$8 += 1; print $0}'`
+# Ensure the wait_until_raw_zone_update_is_processed() call below will ignore
+# log messages generated before the raw zone is updated.
+nextpart ns3/named.run > /dev/null
+# Bump the SOA serial number of the raw zone.
+$NSUPDATE << EOF || ret=1
+zone removedkeys-primary.
+server 10.53.0.3 ${PORT}
+update del removedkeys-primary. SOA
+update ${BUMPED_SOA}
+send
+EOF
+wait_until_raw_zone_update_is_processed "removedkeys-primary"
+# Query for bar.removedkeys-primary/A again and ensure the signer now returns a
+# positive, signed response.
+$DIG $DIGOPTS @10.53.0.3 bar.removedkeys-primary. A > dig.out.ns3.test$n 2>&1
+grep "status: NOERROR" dig.out.ns3.test$n > /dev/null || ret=1
+grep "RRSIG" dig.out.ns3.test$n > /dev/null || ret=1
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=`expr $status + $ret`
+
+n=`expr $n + 1`
+echo_i "checking that changes to raw zone are not applied to a previously signed secure zone with no keys available (secondary) ($n)"
+ret=0
+# Query for bar.removedkeys-secondary/A and ensure the response is negative.  As this
+# zone does have signing keys set up, the response must be signed.
+$DIG $DIGOPTS @10.53.0.3 bar.removedkeys-secondary. A > dig.out.ns3.pre.test$n 2>&1 || ret=1
+grep "status: NOERROR" dig.out.ns3.pre.test$n > /dev/null && ret=1
+grep "RRSIG" dig.out.ns3.pre.test$n > /dev/null || ret=1
+# Remove the signing keys for this zone.
+mv -f ns3/Kremovedkeys-secondary* ns3/removedkeys
+# Ensure the wait_until_raw_zone_update_is_processed() call below will ignore
+# log messages generated before the raw zone is updated.
+nextpart ns3/named.run > /dev/null
+# Add a record to the raw zone on the master.
+$NSUPDATE << EOF || ret=1
+zone removedkeys-secondary.
+server 10.53.0.2 ${PORT}
+update add bar.removedkeys-secondary. 0 A 127.0.0.1
+send
+EOF
+wait_until_raw_zone_update_is_processed "removedkeys-secondary"
+# Query for bar.removedkeys-secondary/A again and ensure the signer still returns a
+# negative, signed response.
+$DIG $DIGOPTS @10.53.0.3 bar.removedkeys-secondary. A > dig.out.ns3.post.test$n 2>&1
+grep "status: NOERROR" dig.out.ns3.post.test$n > /dev/null && ret=1
+grep "RRSIG" dig.out.ns3.pre.test$n > /dev/null || ret=1
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=`expr $status + $ret`
+
+n=`expr $n + 1`
+echo_i "checking that backlogged changes to raw zone are applied after keys become available (secondary) ($n)"
+ret=0
+# Restore the signing keys for this zone.
+mv ns3/removedkeys/Kremovedkeys-secondary* ns3
+$RNDCCMD 10.53.0.3 loadkeys removedkeys-secondary > /dev/null 2>&1
+# Determine what a SOA record with a bumped serial number should look like.
+BUMPED_SOA=`sed -n 's/.*\(add removedkeys-secondary.*IN.*SOA\)/\1/p;' ns2/named.run | tail -1 | awk '{$8 += 1; print $0}'`
+# Ensure the wait_until_raw_zone_update_is_processed() call below will ignore
+# log messages generated before the raw zone is updated.
+nextpart ns3/named.run > /dev/null
+# Bump the SOA serial number of the raw zone on the master.
+$NSUPDATE << EOF || ret=1
+zone removedkeys-secondary.
+server 10.53.0.2 ${PORT}
+update del removedkeys-secondary. SOA
+update ${BUMPED_SOA}
+send
+EOF
+wait_until_raw_zone_update_is_processed "removedkeys-secondary"
+# Query for bar.removedkeys-secondary/A again and ensure the signer now returns
+# a positive, signed response.
+$DIG $DIGOPTS @10.53.0.3 bar.removedkeys-secondary. A > dig.out.ns3.test$n 2>&1
+grep "status: NOERROR" dig.out.ns3.test$n > /dev/null || ret=1
+grep "RRSIG" dig.out.ns3.test$n > /dev/null || ret=1
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=`expr $status + $ret`
+
 n=`expr $n + 1`
 echo_i "check that zonestatus reports 'type: master' for a inline master zone ($n)"
 ret=0
 $RNDCCMD 10.53.0.3 zonestatus master > rndc.out.ns3.test$n
 grep "type: master" rndc.out.ns3.test$n > /dev/null || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
-
 status=`expr $status + $ret`
+
 n=`expr $n + 1`
 echo_i "check that zonestatus reports 'type: slave' for a inline slave zone ($n)"
 ret=0

lib/dns/zone.c

@@ -13870,6 +13870,7 @@ receive_secure_serial(isc_task_t *task, isc_event_t *event) {
 	dns_zone_t *zone;
 	dns_difftuple_t *tuple = NULL, *soatuple = NULL;
 	dns_update_log_t log = { update_log_cb, NULL };
+	isc_uint32_t newserial = 0, desired = 0;
 	isc_time_t timenow;
 
 	UNUSED(task);
@@ -13977,7 +13978,7 @@ receive_secure_serial(isc_task_t *task, isc_event_t *event) {
 				     zone->rss_newver));
 
 		if (soatuple != NULL) {
-			isc_uint32_t oldserial, newserial, desired;
+			isc_uint32_t oldserial;
 
 			CHECK(dns_db_createsoatuple(zone->rss_db,
 						    zone->rss_oldver,
@@ -13996,9 +13997,6 @@ receive_secure_serial(isc_task_t *task, isc_event_t *event) {
 					   zone->rss_newver, &zone->rss_diff));
 			CHECK(do_one_tuple(&soatuple, zone->rss_db,
 					   zone->rss_newver, &zone->rss_diff));
-			dns_zone_log(zone, ISC_LOG_INFO,
-				     "serial %u (unsigned %u)",
-				     newserial, desired);
 		} else
 			CHECK(update_soa_serial(zone->rss_db, zone->rss_newver,
 						&zone->rss_diff, zone->mctx,
@@ -14017,8 +14015,17 @@ receive_secure_serial(isc_task_t *task, isc_event_t *event) {
 		fprintf(stderr, "looping on dns_update_signaturesinc\n");
 		return;
 	}
-	if (result != ISC_R_SUCCESS)
+	/*
+	 * If something went wrong while trying to update the secure zone and
+	 * the latter was already signed before, do not apply raw zone deltas
+	 * to it as that would break existing DNSSEC signatures.  However, if
+	 * the secure zone was not yet signed (e.g. because no signing keys
+	 * were created for it), commence applying raw zone deltas to it so
+	 * that contents of the raw zone and the secure zone are kept in sync.
+	 */
+	if (result != ISC_R_SUCCESS && dns_db_issecure(zone->rss_db)) {
 		goto failure;
+	}
 
 	if (rjournal == NULL)
 		CHECK(dns_journal_open(zone->rss_raw->mctx,
@@ -14044,6 +14051,11 @@ receive_secure_serial(isc_task_t *task, isc_event_t *event) {
 	dns_db_closeversion(zone->rss_db, &zone->rss_oldver, ISC_FALSE);
 	dns_db_closeversion(zone->rss_db, &zone->rss_newver, ISC_TRUE);
 
+	if (newserial != 0) {
+		dns_zone_log(zone, ISC_LOG_INFO, "serial %u (unsigned %u)",
+			     newserial, desired);
+	}
+
  failure:
 	isc_event_free(&zone->rss_event);
 	event = ISC_LIST_HEAD(zone->rss_events);