From 5431583971bd5bea89e47356f28e7718814593c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Wed, 22 Aug 2018 11:28:54 +0200 Subject: [PATCH] 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 | 22 ++++++++++ bin/tests/system/inline/ns3/named.conf.in | 4 ++ bin/tests/system/inline/tests.sh | 25 ++++++++++++ lib/dns/zone.c | 49 +++++++++++++++++++++-- util/copyrights | 1 + 5 files changed, 98 insertions(+), 3 deletions(-) create mode 100644 bin/tests/system/inline/ns3/master5.db.in diff --git a/bin/tests/system/inline/ns3/master5.db.in b/bin/tests/system/inline/ns3/master5.db.in new file mode 100644 index 0000000000..9bad59178c --- /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 5c5cd1966e..1dff0b5679 100644 --- 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 a6e053e4c0..73ab2aeeb5 100755 --- a/bin/tests/system/inline/tests.sh +++ b/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 diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 9fde18728e..623d728251 100644 --- a/lib/dns/zone.c +++ b/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); } diff --git a/util/copyrights b/util/copyrights index 29a22faac5..b66f04dcb3 100644 --- a/util/copyrights +++ b/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