From 6b5f714e53857195f404b11ab27f60febe825fd0 Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Mon, 17 Nov 2025 15:23:58 +0100 Subject: [PATCH 1/2] remove need_hints parameters to configure_view The `configure_view()` `need_hints` is removed as it this function was always called with the value `true`. The `need_hints` wasn't even used in the function. The only thing it was actually used was to throw a warning which can be done simply in an `else` condition branch. Moreoever, in the case of catalog zones and response-policy, it fixes a possible bug that would affect root zones, as those wouldn't be reverted back to their previous version in case of the view fails to load (during a server reconfiguration). --- bin/named/server.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 93211d7f1a..ce1237e007 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -3716,8 +3716,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, named_cachelist_t *oldcachelist, dns_kasplist_t *kasplist, const cfg_obj_t *bindkeys, isc_mem_t *mctx, cfg_aclconfctx_t *aclctx, - isc_tlsctx_cache_t *tlsctx_client_cache, bool need_hints, - bool first_time) { + isc_tlsctx_cache_t *tlsctx_client_cache, bool first_time) { const cfg_obj_t *maps[4] = { 0 }; const cfg_obj_t *cfgmaps[3] = { 0 }; const cfg_obj_t *options = NULL; @@ -3813,7 +3812,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, * is used for real lookups and so cares about hints. */ obj = NULL; - if (view->rdclass == dns_rdataclass_in && need_hints && + if (view->rdclass == dns_rdataclass_in && named_config_get(maps, "response-policy", &obj) == ISC_R_SUCCESS) { CHECK(configure_rpz(view, NULL, obj, &old_rpz_ok, first_time)); @@ -3821,7 +3820,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, } obj = NULL; - if (view->rdclass != dns_rdataclass_in && need_hints && + if (view->rdclass != dns_rdataclass_in && named_config_get(maps, "catalog-zones", &obj) == ISC_R_SUCCESS) { cfg_obj_log(obj, ISC_LOG_WARNING, @@ -3830,7 +3829,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, } obj = NULL; - if (view->rdclass == dns_rdataclass_in && need_hints && + if (view->rdclass == dns_rdataclass_in && named_config_get(maps, "catalog-zones", &obj) == ISC_R_SUCCESS) { CHECK(configure_catz(view, NULL, config, obj)); @@ -4662,9 +4661,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, &rootzone); if (rootzone != NULL) { dns_zone_detach(&rootzone); - need_hints = false; - } - if (need_hints) { + } else { isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_WARNING, "no root hints for view '%s'", @@ -5558,7 +5555,7 @@ cleanup: obj = NULL; if (rpz_configured && - pview->rdclass == dns_rdataclass_in && need_hints && + pview->rdclass == dns_rdataclass_in && named_config_get(maps, "response-policy", &obj) == ISC_R_SUCCESS) { @@ -5584,7 +5581,7 @@ cleanup: obj = NULL; if (catz_configured && - pview->rdclass == dns_rdataclass_in && need_hints && + pview->rdclass == dns_rdataclass_in && named_config_get(maps, "catalog-zones", &obj) == ISC_R_SUCCESS) { @@ -7791,7 +7788,7 @@ configure_views(cfg_obj_t *config, const cfg_obj_t *bindkeys, result = configure_view(view, viewlist, config, vconfig, cachelist, &server->cachelist, kasplist, bindkeys, isc_g_mctx, aclctx, - tlsctx_client_cache, true, first_time); + tlsctx_client_cache, first_time); if (result != ISC_R_SUCCESS) { dns_view_detach(&view); return result; @@ -7820,7 +7817,7 @@ configure_views(cfg_obj_t *config, const cfg_obj_t *bindkeys, result = configure_view(view, viewlist, config, NULL, cachelist, &server->cachelist, kasplist, bindkeys, isc_g_mctx, aclctx, tlsctx_client_cache, - true, first_time); + first_time); if (result != ISC_R_SUCCESS) { dns_view_detach(&view); return result; From fd49c950703663b0c14e0ec917621714d270af30 Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Mon, 17 Nov 2025 17:00:27 +0100 Subject: [PATCH 2/2] enforces that catalog-zone can't be used in non IN views Catalog-zones can't be used in view which are not from the IN class. This is now enforced as the server won't load (instead of loading without the catalog-zone). This configuration error is now also caught by `named-checkconf`. --- bin/named/server.c | 13 +-------- bin/tests/system/catz/ns2/named1.conf.in | 17 ------------ bin/tests/system/catz/ns2/named2.conf.in | 14 ---------- bin/tests/system/catz/tests.sh | 9 ------- .../system/checkconf/bad-catz-class.conf | 23 ++++++++++++++++ .../system/checkconf/bad-chaos-catz.conf | 27 +++++++++++++++++++ lib/isccfg/check.c | 20 ++++++++++---- 7 files changed, 66 insertions(+), 57 deletions(-) create mode 100644 bin/tests/system/checkconf/bad-catz-class.conf create mode 100644 bin/tests/system/checkconf/bad-chaos-catz.conf diff --git a/bin/named/server.c b/bin/named/server.c index ce1237e007..f06f95f731 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -3820,18 +3820,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, } obj = NULL; - if (view->rdclass != dns_rdataclass_in && - named_config_get(maps, "catalog-zones", &obj) == ISC_R_SUCCESS) - { - cfg_obj_log(obj, ISC_LOG_WARNING, - "'catalog-zones' option is only supported " - "for views with class IN"); - } - - obj = NULL; - if (view->rdclass == dns_rdataclass_in && - named_config_get(maps, "catalog-zones", &obj) == ISC_R_SUCCESS) - { + if (named_config_get(maps, "catalog-zones", &obj) == ISC_R_SUCCESS) { CHECK(configure_catz(view, NULL, config, obj)); catz_configured = true; } diff --git a/bin/tests/system/catz/ns2/named1.conf.in b/bin/tests/system/catz/ns2/named1.conf.in index 6860153a31..e35ba79174 100644 --- a/bin/tests/system/catz/ns2/named1.conf.in +++ b/bin/tests/system/catz/ns2/named1.conf.in @@ -186,23 +186,6 @@ view "default" { }; -view "ch" ch { - - catalog-zones { - zone "catalog-bad5.example" - default-masters { 10.53.0.1; } - in-memory yes; - }; - - # Non-IN class catalog zone - zone "catalog-bad5.example" ch { - type secondary; - file "catalog-bad5.example.db"; - primaries { 10.53.0.1; }; - }; - -}; - key tsig_key. { secret "LSAnCU+Z"; algorithm @DEFAULT_HMAC@; diff --git a/bin/tests/system/catz/ns2/named2.conf.in b/bin/tests/system/catz/ns2/named2.conf.in index 385adf4deb..82621cac1c 100644 --- a/bin/tests/system/catz/ns2/named2.conf.in +++ b/bin/tests/system/catz/ns2/named2.conf.in @@ -121,20 +121,6 @@ view "default" { }; -view "ch" ch { - - # Removed catalog-zone option, otherwise this is - # identical to named1.conf.in - - # Non-IN class catalog zone - zone "catalog-bad5.example" ch { - type secondary; - file "catalog-bad5.example.db"; - primaries { 10.53.0.1; }; - }; - -}; - key tsig_key. { secret "LSAnCU+Z"; algorithm @DEFAULT_HMAC@; diff --git a/bin/tests/system/catz/tests.sh b/bin/tests/system/catz/tests.sh index 3ac389cf88..be3e3f4be9 100644 --- a/bin/tests/system/catz/tests.sh +++ b/bin/tests/system/catz/tests.sh @@ -116,15 +116,6 @@ wait_for_message ns2/named.run "catz: invalid record in catalog zone - version.c if [ $ret -ne 0 ]; then echo_i "failed"; fi status=$((status + ret)) -n=$((n + 1)) -echo_i "checking that catalog-bad5.example (non-IN class) has failed to load ($n)" -ret=0 -wait_for_message ns2/named.run "'catalog-zones' option is only supported for views with class IN" \ - && wait_for_message ns2/named.run "all zones loaded" || ret=1 -grep -F "catz: dns_catz_zone_add catalog-bad5.example" ns2/named.run && ret=1 -if [ $ret -ne 0 ]; then echo_i "failed"; fi -status=$((status + ret)) - nextpart ns2/named.run >/dev/null ########################################################################## diff --git a/bin/tests/system/checkconf/bad-catz-class.conf b/bin/tests/system/checkconf/bad-catz-class.conf new file mode 100644 index 0000000000..af33b07a3c --- /dev/null +++ b/bin/tests/system/checkconf/bad-catz-class.conf @@ -0,0 +1,23 @@ +/* + * Copyright (C) Internet Systems Consortium, Inc. ("ISC") + * + * SPDX-License-Identifier: MPL-2.0 + * + * 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 https://mozilla.org/MPL/2.0/. + * + * See the COPYRIGHT file distributed with this work for additional + * information regarding copyright ownership. + */ + +view bar ch { + catalog-zones { + zone "foo"; + }; + + zone "foo" { + type primary; + file "foo"; + }; +}; diff --git a/bin/tests/system/checkconf/bad-chaos-catz.conf b/bin/tests/system/checkconf/bad-chaos-catz.conf new file mode 100644 index 0000000000..6923a3190c --- /dev/null +++ b/bin/tests/system/checkconf/bad-chaos-catz.conf @@ -0,0 +1,27 @@ +/* + * Copyright (C) Internet Systems Consortium, Inc. ("ISC") + * + * SPDX-License-Identifier: MPL-2.0 + * + * 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 https://mozilla.org/MPL/2.0/. + * + * See the COPYRIGHT file distributed with this work for additional + * information regarding copyright ownership. + */ + +view "ch" ch { + catalog-zones { + zone "catalog-bad5.example" default-masters { 10.53.0.1; } + in-memory yes; + }; + + # Non-IN class catalog zone + zone "catalog-bad5.example" ch { + type secondary; + file "catalog-bad5.example.db"; + primaries { 10.53.0.1; }; + }; +}; + diff --git a/lib/isccfg/check.c b/lib/isccfg/check.c index c49afa9a5f..fb1e1d92b9 100644 --- a/lib/isccfg/check.c +++ b/lib/isccfg/check.c @@ -5539,11 +5539,21 @@ check_viewconf(const cfg_obj_t *config, const cfg_obj_t *voptions, */ if (opts != NULL) { obj = NULL; - if ((cfg_map_get(opts, "catalog-zones", &obj) == - ISC_R_SUCCESS) && - (check_catz(obj, viewname, mctx) != ISC_R_SUCCESS)) - { - result = ISC_R_FAILURE; + if (cfg_map_get(opts, "catalog-zones", &obj) == ISC_R_SUCCESS) { + if (vclass != dns_rdataclass_in) { + cfg_obj_log( + obj, ISC_LOG_ERROR, + "'catalog-zones' option is only " + "supported for views with class IN"); + + if (result == ISC_R_SUCCESS) { + result = ISC_R_FAILURE; + } + } + + if (check_catz(obj, viewname, mctx) != ISC_R_SUCCESS) { + result = ISC_R_FAILURE; + } } }