Fix reloading inline-signed zones

While "rndc reload" causes dns_zone_asyncload() to be called for the
signed version of an inline-signed zone, the subsequent zone_load() call
causes the raw version to be reloaded from storage.  This means that
DNS_ZONEFLG_LOADPENDING gets set for the signed version of the zone by
dns_zone_asyncload() before the reload is attempted, but zone_postload()
is only called for the raw version and thus DNS_ZONEFLG_LOADPENDING is
cleared for the raw version, but not for the signed version.  This in
turn prevents zone maintenance from happening for the signed version of
the zone.

Until commit 29b7efdd9f8ce9b49371809498744c4b076dc3ee, this problem
remained dormant because DNS_ZONEFLG_LOADPENDING was previously
immediately, unconditionally cleared after zone loading was started
(whereas it should only be cleared when zone loading is finished or an
error occurs).  This behavior caused other issues [1] and thus had to be
changed.

Fix reloading inline-signed zones by clearing DNS_ZONEFLG_LOADPENDING
for the signed version of the zone once the raw version reload
completes.  Take care not to clear it prematurely during initial zone
load.  Also make sure that DNS_ZONEFLG_LOADPENDING gets cleared when
zone_postload() encounters an error or returns early, to prevent other
scenarios from resulting in the same problem.  Add comments aiming to
help explain code flow.

[1] see RT #47076

bin/tests/system/inline/ns3/master5.db.in

@@ -0,0 +1,22 @@
+; 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	ns3 . (
+				2000042411 ; serial
+				20         ; refresh (20 seconds)
+				20         ; retry (20 seconds)
+				1814400    ; expire (3 weeks)
+				3600       ; minimum (1 hour)
+				)
+			NS	ns3
+ns3			A	10.53.0.3
+
+c			A	10.0.0.3
+e			A	10.0.0.5

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

@@ -58,6 +58,10 @@ zone "master" {
 	inline-signing yes;
 	auto-dnssec maintain;
 	file "master.db";
+	notify explicit;
+	also-notify {
+		10.53.0.3;
+	};
 };
 
 zone "dynamic" {

bin/tests/system/inline/tests.sh

@@ -897,6 +897,31 @@ done
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
+n=`expr $n + 1`
+echo_i "check that reloading all zones does not cause zone maintenance to cease for inline-signed zones ($n)"
+ret=1
+# Ensure "rndc reload" attempts to load ns3/master.db by waiting 1 second so
+# that the master file modification time has no possibility of being equal to
+# the one stored during server startup.
+sleep 1
+nextpart ns3/named.run > /dev/null
+cp ns3/master5.db.in ns3/master.db
+$RNDCCMD 10.53.0.3 reload 2>&1 | sed 's/^/ns3 /' | cat_i
+for i in 1 2 3 4 5 6 7 8 9 10
+do
+	if nextpart ns3/named.run | grep "zone master.*sending notifies" > /dev/null; then
+		ret=0
+		break
+	fi
+	sleep 1
+done
+# Sanity check: master file updates should be reflected in the signed zone,
+# i.e. SOA RNAME should no longer be set to "hostmaster".
+$DIG $DIGOPTS @10.53.0.3 master SOA > dig.out.ns3.test$n || ret=1
+grep "hostmaster" 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 "test add/del zone combinations ($n)"
 ret=0

lib/dns/zone.c

@@ -1882,6 +1882,11 @@ zone_touched(dns_zone_t *zone) {
 	return (false);
 }
 
+/*
+ * Note: when dealing with inline-signed zones, external callers will always
+ * call zone_load() for the secure zone; zone_load() calls itself recursively
+ * in order to load the raw zone.
+ */
 static isc_result_t
 zone_load(dns_zone_t *zone, unsigned int flags, bool locked) {
 	isc_result_t result;
@@ -1898,6 +1903,28 @@ zone_load(dns_zone_t *zone, unsigned int flags, bool locked) {
 	INSIST(zone != zone->raw);
 	hasraw = inline_secure(zone);
 	if (hasraw) {
+		/*
+		 * We are trying to load an inline-signed zone.  First call
+		 * self recursively to try loading the raw version of the zone.
+		 * Assuming the raw zone file is readable, there are two
+		 * possibilities:
+		 *
+		 *  a) the raw zone was not yet loaded and thus it will be
+		 *     loaded now, synchronously; if this succeeds, a
+		 *     subsequent attempt to load the signed zone file will
+		 *     take place and thus zone_postload() will be called
+		 *     twice: first for the raw zone and then for the secure
+		 *     zone; the latter call will take care of syncing the raw
+		 *     version with the secure version,
+		 *
+		 *  b) the raw zone was already loaded and we are trying to
+		 *     reload it, which will happen asynchronously; this means
+		 *     zone_postload() will only be called for the raw zone
+		 *     because "result" returned by the zone_load() call below
+		 *     will not be ISC_R_SUCCESS but rather DNS_R_CONTINUE;
+		 *     zone_postload() called for the raw zone will take care
+		 *     of syncing the raw version with the secure version.
+		 */
 		result = zone_load(zone->raw, flags, false);
 		if (result != ISC_R_SUCCESS) {
 			if (!locked)
@@ -4661,7 +4688,7 @@ zone_postload(dns_zone_t *zone, dns_db_t *db, isc_time_t loadtime,
 						      ISC_LOG_INFO,
 						     "ixfr-from-differences: "
 						     "unchanged");
-					return(ISC_R_SUCCESS);
+					goto done;
 				}
 
 				serialmin = (oldserial + 1) & 0xffffffffU;
@@ -4880,8 +4907,7 @@ zone_postload(dns_zone_t *zone, dns_db_t *db, isc_time_t loadtime,
 	}
 
 	zone->loadtime = loadtime;
-	DNS_ZONE_CLRFLAG(zone, DNS_ZONEFLG_LOADPENDING);
-	return (result);
+	goto done;
 
  cleanup:
 	if (zone->type == dns_zone_key && result != ISC_R_SUCCESS) {
@@ -4927,6 +4953,23 @@ zone_postload(dns_zone_t *zone, dns_db_t *db, isc_time_t loadtime,
 			result = ISC_R_SUCCESS;
 	}
 
+ done:
+	DNS_ZONE_CLRFLAG(zone, DNS_ZONEFLG_LOADPENDING);
+	/*
+	 * If this is an inline-signed zone and we were called for the raw
+	 * zone, we need to clear DNS_ZONEFLG_LOADPENDING for the secure zone
+	 * as well, but only if this is a reload, not an initial zone load: in
+	 * the former case, zone_postload() will not be run for the secure
+	 * zone; in the latter case, it will be.  Check which case we are
+	 * dealing with by consulting the DNS_ZONEFLG_LOADED flag for the
+	 * secure zone: if it is set, this must be a reload.
+	 */
+	if (inline_raw(zone) &&
+	    DNS_ZONE_FLAG(zone->secure, DNS_ZONEFLG_LOADED))
+	{
+		DNS_ZONE_CLRFLAG(zone->secure, DNS_ZONEFLG_LOADPENDING);
+	}
+
 	return (result);
 }
 

util/copyrights

@@ -1373,6 +1373,7 @@
 ./bin/tests/system/inline/ns3/master2.db.in	ZONE	2011,2012,2016,2018
 ./bin/tests/system/inline/ns3/master3.db.in	ZONE	2012,2016,2018
 ./bin/tests/system/inline/ns3/master4.db.in	ZONE	2012,2016,2018
+./bin/tests/system/inline/ns3/master5.db.in	ZONE	2018
 ./bin/tests/system/inline/ns3/named.conf.in	CONF-C	2011,2012,2013,2015,2016,2017,2018
 ./bin/tests/system/inline/ns3/sign.sh		SH	2011,2012,2013,2014,2016,2017,2018
 ./bin/tests/system/inline/ns4/named.conf.in	CONF-C	2011,2016,2018