From 789ce76dd14be41ebe5f69c361d859cd810f736d Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Wed, 1 Mar 2023 12:47:25 +0000 Subject: [PATCH 1/3] Add a catz system test check for [GL #3911] The trick is to configure a duplicate zone, which comes after the catalog zone, where the duplicate zone is an existing member zone. In that scenario, all the zones which come before the "faulty" zone in the configuration file will fail to be reverted to the previous version of the view after a reconfiguration error, and in this particular case that will result in an assertion failure when the catalog zone update is initiated, because it will be still tied to the new version of the view, which was dismissed. (cherry picked from commit 93c4f382f4a8a4d049cdcae5971412667a7bade9) --- bin/tests/system/catz/ns2/named1.conf.in | 11 ++++++++- bin/tests/system/catz/tests.sh | 31 ++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/bin/tests/system/catz/ns2/named1.conf.in b/bin/tests/system/catz/ns2/named1.conf.in index 5a7babdbfe..b1677e4dc1 100644 --- a/bin/tests/system/catz/ns2/named1.conf.in +++ b/bin/tests/system/catz/ns2/named1.conf.in @@ -68,7 +68,7 @@ view "default" { }; # A faulty dlz configuration to check if named and catz survive a certain class - # of failed configuration attempts (see GL#3060). + # of failed configuration attempts (see GL #3060). # We use "dlz" because the dlz processing code is located in an ideal place in # the view configuration function for the test to cover the view reverting code. #T3 dlz "bad-dlz" { @@ -116,6 +116,15 @@ view "default" { primaries { 10.53.0.1; }; }; + # When the following zone configuration is enabled, "dom3.example" should + # already exist as a member of "catalog1.example", and named should be able + # to deal with that situation (see GL #3911). Make sure that this duplicate + # zone comes after the the "catalog1.example" zone in the configuration file. +#T4 zone "dom3.example" { +#T4 type secondary; +#T4 file "dom2.example.db"; +#T4 }; + # No "version" property zone "catalog-bad1.example" { type secondary; diff --git a/bin/tests/system/catz/tests.sh b/bin/tests/system/catz/tests.sh index 4919dd5261..eacb39c078 100644 --- a/bin/tests/system/catz/tests.sh +++ b/bin/tests/system/catz/tests.sh @@ -656,6 +656,7 @@ status=$((status+ret)) nextpart ns2/named.run >/dev/null +# GL #3060 n=$((n+1)) echo_i "reconfiguring secondary - checking if catz survives a certain class of failed reconfiguration attempts ($n)" ret=0 @@ -682,6 +683,36 @@ status=$((status+ret)) nextpart ns2/named.run >/dev/null +# GL #3911 +n=$((n+1)) +echo_i "reconfiguring secondary - checking if catz survives another type of failed reconfiguration attempts ($n)" +ret=0 +sed -e "s/^#T4//" < ns2/named1.conf.in > ns2/named.conf.tmp +copy_setports ns2/named.conf.tmp ns2/named.conf +$RNDC -c ../common/rndc.conf -s 10.53.0.2 -p "${CONTROLPORT}" reconfig > /dev/null 2>&1 && ret=1 +if [ $ret -ne 0 ]; then echo_i "failed"; fi +status=$((status+ret)) + +# catalog zone update can be deferred +sleep 2 + +n=$((n+1)) +echo_i "checking again that dom3.example. is served by secondary ($n)" +ret=0 +wait_for_soa @10.53.0.2 dom3.example. dig.out.test$n || ret=1 +if [ $ret -ne 0 ]; then echo_i "failed"; fi +status=$((status+ret)) + +n=$((n+1)) +echo_i "reconfiguring secondary - reverting the bad configuration ($n)" +ret=0 +copy_setports ns2/named1.conf.in ns2/named.conf +rndccmd 10.53.0.2 reconfig || ret=1 +if [ $ret -ne 0 ]; then echo_i "failed"; fi +status=$((status+ret)) + +nextpart ns2/named.run >/dev/null + n=$((n+1)) echo_i "adding a domain dom-existing.example. to primary via RNDC ($n)" ret=0 From 78f1d256a3091aa471df26c2c4388f0eb634ac2b Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Wed, 1 Mar 2023 12:30:46 +0000 Subject: [PATCH 2/3] Fix view's zones reverting bug during reconfiguration During reconfiguration, the configure_view() function reverts the configured zones to the previous view in case if there is an error. It uses the 'zones_configured' boolean variable to decide whether it is required to revert the zones, i.e. the error happened after all the zones were successfully configured. The problem is that it does not account for the case when an error happens during the configuration of one of the zones (not the first), in which case there are zones that are already configured for the new view (and they need to be reverted), and there are zones that are not (starting from the failed one). Since 'zones_configured' remains 'false', the configured zones are not reverted. Replace the 'zones_configured' variable with a pointer to the latest successfully configured zone configuration element, and when reverting, revert up to and including that zone. (cherry picked from commit 84c235a4b0477a34c0ac2054af98b39efc5b0df5) --- bin/named/server.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 6b6f020017..dd4d017d7d 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -4074,7 +4074,8 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, const cfg_obj_t *dyndb_list, *plugin_list; const cfg_obj_t *disabled; const cfg_obj_t *obj, *obj2; - const cfg_listelt_t *element; + const cfg_listelt_t *element = NULL; + const cfg_listelt_t *zone_element_latest = NULL; in_port_t port; dns_cache_t *cache = NULL; isc_result_t result; @@ -4092,7 +4093,6 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, dns_dispatch_t *dispatch6 = NULL; bool rpz_configured = false; bool catz_configured = false; - bool zones_configured = false; bool shared_cache = false; int i = 0, j = 0, k = 0; const char *str; @@ -4203,8 +4203,8 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, CHECK(configure_zone(config, zconfig, vconfig, mctx, view, viewlist, kasplist, actx, false, old_rpz_ok, false)); + zone_element_latest = element; } - zones_configured = true; /* * Check that a primary or secondary zone was found for each @@ -6075,7 +6075,7 @@ cleanup: dns_view_detach(&pview); } - if (zones_configured) { + if (zone_element_latest != NULL) { for (element = cfg_list_first(zonelist); element != NULL; element = cfg_list_next(element)) { @@ -6083,6 +6083,13 @@ cleanup: cfg_listelt_value(element); configure_zone_setviewcommit(result, zconfig, view); + if (element == zone_element_latest) { + /* + * This was the latest element that was + * successfully configured earlier. + */ + break; + } } } } From 4a5156f132a2a2a0c94eb392b7e6fbfa827d8474 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Wed, 1 Mar 2023 13:45:03 +0000 Subject: [PATCH 3/3] Add CHANGES and release notes for [GL #3911] (cherry picked from commit e1036253db90efbd17db0eef3b17e37d2db86176) --- CHANGES | 5 +++++ doc/notes/notes-current.rst | 4 +++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index a487144934..f0ce6b7d6d 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,8 @@ +6119. [bug] Make sure to revert the reconfigured zones to the + previous version of the view, when the new view + reconfiguration fails during the configuration of + one of the configured zones. [GL #3911] + 6116. [bug] Fix error path cleanup issues in dns_catz_new_zones() and dns_catz_new_zone() functions. [GL #3900] diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index d4593b8044..deb913fe5b 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -65,7 +65,9 @@ Feature Changes Bug Fixes ~~~~~~~~~ -- None. +- :iscman:`named` could crash with an assertion failure when adding a new zone + into the configuration file for a name, which is already configured as a + member zone for a catalog zone. This has been fixed. :gl:`#3911` Known Issues ~~~~~~~~~~~~