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 749b3cacfc781122c94f2f183b9ee6dcbbb9864d, 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

(cherry picked from commit 5431583971bd5bea89e47356f28e7718814593c0)

diff --git a/bin/tests/system/inline/ns3/master5.db.in b/bin/tests/system/inline/ns3/master5.db.in
new file mode 100644 (file)
index 0000000..9bad591
--- /dev/null
+++ b/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

diff --git a/bin/tests/system/inline/ns3/named.conf.in b/bin/tests/system/inline/ns3/named.conf.in
index 5c5cd1966e51328e5cdbc44f5d7141f547ea838b..1dff0b5679a229705f75ce8fb273208b20ea8b28 100644 (file)
--- a/bin/tests/system/inline/ns3/named.conf.in
+++ b/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" {

diff --git a/bin/tests/system/inline/tests.sh b/bin/tests/system/inline/tests.sh
index 75d104e18befe26d4bab655610b58793a5b7d2f7..35b42686b18a55ac88a85ae98b741d0e0ba5da94 100755 (executable)
--- a/bin/tests/system/inline/tests.sh
+++ b/bin/tests/system/inline/tests.sh
@@ -898,6 +898,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

diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index 0de29cca1139dadeada71a855daad37d34db51b2..a25d0cc158c21723c3b71bec20f56337de235bad 100644 (file)
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -1927,6 +1927,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;
@@ -1943,6 +1948,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)
@@ -4668,7 +4695,7 @@ zone_postload(dns_zone_t *zone, dns_db_t *db, isc_time_t loadtime,
                                        dns_zone_log(zone, ISC_LOG_INFO,
                                                     "ixfr-from-differences: "
                                                     "unchanged");
-                                       return(ISC_R_SUCCESS);
+                                       goto done;
                                }
 
                                serialmin = (oldserial + 1) & 0xffffffffU;
@@ -4891,8 +4918,7 @@ zone_postload(dns_zone_t *zone, dns_db_t *db, isc_time_t loadtime,
                             dns_db_issecure(db) ? " (DNSSEC signed)" : "");
 
        zone->loadtime = loadtime;
-       DNS_ZONE_CLRFLAG(zone, DNS_ZONEFLG_LOADPENDING);
-       return (result);
+       goto done;
 
  cleanup:
        for (inc = ISC_LIST_HEAD(zone->newincludes);
@@ -4929,6 +4955,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);
 }
 

diff --git a/util/copyrights b/util/copyrights
index 468916df366eb5d7bc78518187d5fb923cccc969..decbd6b27984bf2ef20326ed8701796ca6c9cf7c 100644 (file)
--- a/util/copyrights
+++ b/util/copyrights
@@ -1460,6 +1460,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  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  2018