From f9340fc152e75a592e645acfa7aa79b75d331fab Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 19 Nov 2025 16:35:31 -0800 Subject: [PATCH 1/3] add a test for allow-recursion/allow-query-cache inheritance allow-recursion is set to "none" in the options block and to "any" in the view. allow-query-cache in the view should inherit the "any", not the "none". (currently this test does not pass.) --- .../system/allow-query/ns3/named5.conf.in | 42 +++++++++++ .../system/allow-query/ns3/named6.conf.in | 41 +++++++++++ .../system/allow-query/ns3/named7.conf.in | 41 +++++++++++ .../system/allow-query/ns3/named8.conf.in | 42 +++++++++++ bin/tests/system/allow-query/tests.sh | 71 +++++++++++++++++++ 5 files changed, 237 insertions(+) create mode 100644 bin/tests/system/allow-query/ns3/named5.conf.in create mode 100644 bin/tests/system/allow-query/ns3/named6.conf.in create mode 100644 bin/tests/system/allow-query/ns3/named7.conf.in create mode 100644 bin/tests/system/allow-query/ns3/named8.conf.in diff --git a/bin/tests/system/allow-query/ns3/named5.conf.in b/bin/tests/system/allow-query/ns3/named5.conf.in new file mode 100644 index 0000000000..81edf10e34 --- /dev/null +++ b/bin/tests/system/allow-query/ns3/named5.conf.in @@ -0,0 +1,42 @@ +/* + * 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. + */ + +options { + port @PORT@; + pid-file "named.pid"; + listen-on { 10.53.0.3; 10.53.1.2; }; + listen-on-v6 { none; }; + recursion no; + allow-recursion { none; }; + dnssec-validation no; +}; + +key rndc_key { + secret "1234abcd8765"; + algorithm @DEFAULT_HMAC@; +}; + +controls { + inet 10.53.0.3 port @CONTROLPORT@ allow { any; } keys { rndc_key; }; +}; + +view internal { + match-destinations { 10.53.0.3; }; + zone "." { + type hint; + file "../../_common/root.hint"; + }; + + recursion yes; + allow-recursion { any; }; +}; diff --git a/bin/tests/system/allow-query/ns3/named6.conf.in b/bin/tests/system/allow-query/ns3/named6.conf.in new file mode 100644 index 0000000000..94e796ba24 --- /dev/null +++ b/bin/tests/system/allow-query/ns3/named6.conf.in @@ -0,0 +1,41 @@ +/* + * 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. + */ + +options { + port @PORT@; + pid-file "named.pid"; + listen-on { 10.53.0.3; }; + listen-on-v6 { none; }; + recursion no; + allow-query-cache { none; }; + dnssec-validation no; +}; + +key rndc_key { + secret "1234abcd8765"; + algorithm @DEFAULT_HMAC@; +}; + +controls { + inet 10.53.0.3 port @CONTROLPORT@ allow { any; } keys { rndc_key; }; +}; + +view internal { + match-destinations { 10.53.0.3; }; + zone "." { + type hint; + file "../../_common/root.hint"; + }; + recursion yes; + allow-recursion{ any; }; +}; diff --git a/bin/tests/system/allow-query/ns3/named7.conf.in b/bin/tests/system/allow-query/ns3/named7.conf.in new file mode 100644 index 0000000000..28a05bf171 --- /dev/null +++ b/bin/tests/system/allow-query/ns3/named7.conf.in @@ -0,0 +1,41 @@ +/* + * 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. + */ + +options { + port @PORT@; + pid-file "named.pid"; + listen-on { 10.53.0.3; }; + listen-on-v6 { none; }; + recursion no; + allow-recursion { none; }; + dnssec-validation no; +}; + +key rndc_key { + secret "1234abcd8765"; + algorithm @DEFAULT_HMAC@; +}; + +controls { + inet 10.53.0.3 port @CONTROLPORT@ allow { any; } keys { rndc_key; }; +}; + +view internal { + match-destinations { 10.53.0.3; }; + zone "." { + type hint; + file "../../_common/root.hint"; + }; + recursion yes; + allow-query{ any; }; +}; diff --git a/bin/tests/system/allow-query/ns3/named8.conf.in b/bin/tests/system/allow-query/ns3/named8.conf.in new file mode 100644 index 0000000000..a248f55ac4 --- /dev/null +++ b/bin/tests/system/allow-query/ns3/named8.conf.in @@ -0,0 +1,42 @@ +/* + * 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. + */ + +options { + port @PORT@; + pid-file "named.pid"; + listen-on { 10.53.0.3; 10.53.0.4; 10.53.1.2; }; + listen-on-v6 { none; }; + recursion no; + allow-query-cache { none; }; + dnssec-validation no; +}; + +key rndc_key { + secret "1234abcd8765"; + algorithm @DEFAULT_HMAC@; +}; + +controls { + inet 10.53.0.3 port @CONTROLPORT@ allow { any; } keys { rndc_key; }; +}; + +view internal { + zone "." { + type hint; + file "../../_common/root.hint"; + }; + + recursion yes; + allow-query-cache { 10.53.0.3; 10.53.0.4; }; + allow-query { 10.53.0.4; }; +}; diff --git a/bin/tests/system/allow-query/tests.sh b/bin/tests/system/allow-query/tests.sh index bfacf76508..f332ce15e1 100644 --- a/bin/tests/system/allow-query/tests.sh +++ b/bin/tests/system/allow-query/tests.sh @@ -734,5 +734,76 @@ nextpart ns3/named.run | grep 'allow-recursion-on did not match' >/dev/null || r if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) +# Test 63 - allow-query-cache inheritance from allow-recursion +n=$((n + 1)) +copy_setports ns3/named5.conf.in ns3/named.conf +rndc_reload ns3 10.53.0.3 + +echo_i "test $n: inheritance of allow-query-cache from allow-recursion" +ret=0 +# this should be allowed +$DIG -p ${PORT} @10.53.0.3 e.normal.example a >dig.out.ns3.1.$n || ret=1 +grep 'ANSWER: 1' dig.out.ns3.1.$n >/dev/null || ret=1 +# this should be prohibited +$DIG -p ${PORT} @10.53.1.2 f.normal.example a >dig.out.ns3.2.$n || ret=1 +grep 'recursion requested but not available' dig.out.ns3.2.$n >/dev/null || ret=1 +grep 'status: REFUSED' dig.out.ns3.2.$n >/dev/null || ret=1 +grep 'EDE: 18 (Prohibited)' dig.out.ns3.2.$n >/dev/null || ret=1 +grep 'EDE: 20' dig.out.ns3.2.$n >/dev/null && ret=1 +if [ $ret != 0 ]; then echo_i "failed"; fi +status=$((status + ret)) + +# Test 64 - allow-query-cache no inheritance from allow-recursion as it is +# defined in the options +n=$((n + 1)) +copy_setports ns3/named6.conf.in ns3/named.conf +rndc_reload ns3 10.53.0.3 + +echo_i "test $n: allow-query-cache defined in options, so it does not inherit from allow-recursion" +ret=0 +$DIG -p ${PORT} @10.53.0.3 f.normal.example a >dig.out.ns3.1.$n || ret=1 +grep 'recursion requested but not available' dig.out.ns3.1.$n >/dev/null || ret=1 +grep 'status: REFUSED' dig.out.ns3.1.$n >/dev/null || ret=1 +grep 'EDE: 18 (Prohibited)' dig.out.ns3.1.$n >/dev/null || ret=1 +grep 'EDE: 20' dig.out.ns3.1.$n >/dev/null || ret=1 +if [ $ret != 0 ]; then echo_i "failed"; fi +status=$((status + ret)) + +# Test 65 - allow-query-cache inherits from allow-recursion before allow-query +n=$((n + 1)) +copy_setports ns3/named7.conf.in ns3/named.conf +rndc_reload ns3 10.53.0.3 + +echo_i "test $n: allow-query-cache inherits from allow-recursion before allow-query" +ret=0 +$DIG -p ${PORT} -b 10.53.0.3 @10.53.0.3 f.normal.example a >dig.out.ns3.1.$n || ret=1 +grep 'recursion requested but not available' dig.out.ns3.1.$n >/dev/null || ret=1 +grep 'status: REFUSED' dig.out.ns3.1.$n >/dev/null || ret=1 +grep 'EDE: 18 (Prohibited)' dig.out.ns3.1.$n >/dev/null || ret=1 +grep 'EDE: 20' dig.out.ns3.1.$n >/dev/null || ret=1 +if [ $ret != 0 ]; then echo_i "failed"; fi +status=$((status + ret)) + +# Test 66 - allow-recursion inheritance from allow-query +n=$((n + 1)) +copy_setports ns3/named8.conf.in ns3/named.conf +rndc_reload ns3 10.53.0.3 + +echo_i "test $n: inheritance of allow-query-cache from allow-recursion" +ret=0 +# this should be prohibited (10.53.1.2 does not have recursion allowed) +$DIG -p ${PORT} -b 10.53.1.2 @10.53.1.2 f.normal.example a >dig.out.ns3.1.$n || ret=1 +grep 'recursion requested but not available' dig.out.ns3.1.$n >/dev/null || ret=1 +grep 'status: REFUSED' dig.out.ns3.1.$n >/dev/null || ret=1 +grep 'EDE: 18 (Prohibited)' dig.out.ns3.1.$n >/dev/null || ret=1 +grep 'EDE: 20' dig.out.ns3.1.$n >/dev/null || ret=1 +# this should be allowed +$DIG -p ${PORT} -b 10.53.0.4 @10.53.0.4 f.normal.example a >dig.out.ns3.2.$n || ret=1 +grep 'ANSWER: 1' dig.out.ns3.2.$n >/dev/null || ret=1 +# this should be allowed +$DIG -p ${PORT} -b 10.53.0.4 @10.53.0.4 e.normal.example a >dig.out.ns3.3.$n || ret=1 +grep 'ANSWER: 1' dig.out.ns3.3.$n >/dev/null || ret=1 +status=$((status + ret)) +if [ $ret != 0 ]; then echo_i "failed"; fi echo_i "exit status: $status" [ $status -eq 0 ] || exit 1 From 1a77ae2a7afaaa24d9c181dc2ca33c6853ec2246 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 19 Nov 2025 17:52:39 -0800 Subject: [PATCH 2/3] fix allow-recursion/allow-query-cache inheritance the merging of options and defaults into the effective configuration broke the mutual inheritance of the allow-recursion, allow-query, and allow-query-cache ACLs, and of the allow-recursion-on and allow-query-cache-on ACLs. this has been corrected by adding a 'cloned' flag to the cfg_obj structure to indicate whether it was configured explicitly or cloned from the defaults during parsing. we can then adjust the ACLs while configuring a view, favoring user-configured values when they're available over cloned defaults. currently the adjustments to the ACLs are done in configure_view(); later they'll be moved into the effective configuration and this special handling can be removed. --- bin/named/server.c | 128 ++++++++++++++++------------ lib/isccfg/include/isccfg/cfg.h | 9 ++ lib/isccfg/include/isccfg/grammar.h | 7 ++ lib/isccfg/namedconf.c | 48 +++++------ lib/isccfg/parser.c | 47 +++++----- tests/isccfg/grammar_test.c | 18 +--- 6 files changed, 140 insertions(+), 117 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 8441148606..20dbb74ed3 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -541,7 +541,7 @@ load_nzf(dns_view_t *view); static isc_result_t configure_view_acl(const cfg_obj_t *vconfig, const cfg_obj_t *config, const char *aclname, const char *acltuplename, - cfg_aclconfctx_t *aclctx, isc_mem_t *mctx, + cfg_aclconfctx_t *aclctx, isc_mem_t *mctx, bool *expp, dns_acl_t **aclp) { isc_result_t result; const cfg_obj_t *maps[4]; @@ -580,6 +580,14 @@ configure_view_acl(const cfg_obj_t *vconfig, const cfg_obj_t *config, aclobj = cfg_tuple_get(aclobj, acltuplename); } + if (expp != NULL) { + /* + * Note whether the ACL was explicitly configured, + * not cloned during the merge process. + */ + *expp = !cfg_obj_iscloned(aclobj); + } + result = cfg_acl_fromconfig(aclobj, config, aclctx, mctx, 0, aclp); return result; @@ -4773,9 +4781,10 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, * can be retrieved.) */ CHECK(configure_view_acl(vconfig, config, "match-clients", NULL, aclctx, - isc_g_mctx, &view->matchclients)); + isc_g_mctx, NULL, &view->matchclients)); CHECK(configure_view_acl(vconfig, config, "match-destinations", NULL, - aclctx, isc_g_mctx, &view->matchdestinations)); + aclctx, isc_g_mctx, NULL, + &view->matchdestinations)); /* * Configure the "match-recursive-only" option. @@ -4860,72 +4869,83 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, view->root_key_sentinel = cfg_obj_asboolean(obj); /* - * Set the "allow-query", "allow-query-cache", "allow-recursion", - * "allow-recursion-on" and "allow-query-cache-on" ACLs if - * configured in named.conf, but NOT from the global defaults. - * This is done by leaving the third argument to configure_view_acl() - * NULL. - * - * We ignore the global defaults here because these ACLs - * can inherit from each other. If any are still unset after - * applying the inheritance rules, we'll look up the defaults at - * that time. + * The "allow-query", "allow-query-cache", "allow-recursion", + * "allow-recursion-on" and "allow-query-cache-on" ACLs can + * inherit from one other, so there's special handling based + * on whether they're explicitly set in named.conf or cloned + * from the defaults. */ - - /* named.conf only */ + bool aq, aqc, ar, aro, aqco; CHECK(configure_view_acl(vconfig, config, "allow-query", NULL, aclctx, - isc_g_mctx, &view->queryacl)); - - /* named.conf only */ - CHECK(configure_view_acl(vconfig, config, "allow-query-cache", NULL, - aclctx, isc_g_mctx, &view->cacheacl)); - /* named.conf only */ - CHECK(configure_view_acl(vconfig, config, "allow-query-cache-on", NULL, - aclctx, isc_g_mctx, &view->cacheonacl)); - + isc_g_mctx, &aq, &view->queryacl)); CHECK(configure_view_acl(vconfig, config, "allow-query-on", NULL, - aclctx, isc_g_mctx, &view->queryonacl)); + aclctx, isc_g_mctx, NULL, &view->queryonacl)); + + CHECK(configure_view_acl(vconfig, config, "allow-query-cache", NULL, + aclctx, isc_g_mctx, &aqc, &view->cacheacl)); + CHECK(configure_view_acl(vconfig, config, "allow-query-cache-on", NULL, + aclctx, isc_g_mctx, &aqco, &view->cacheonacl)); CHECK(configure_view_acl(vconfig, config, "allow-proxy", NULL, aclctx, - isc_g_mctx, &view->proxyacl)); + isc_g_mctx, NULL, &view->proxyacl)); CHECK(configure_view_acl(vconfig, config, "allow-proxy-on", NULL, - aclctx, isc_g_mctx, &view->proxyonacl)); + aclctx, isc_g_mctx, NULL, &view->proxyonacl)); if (strcmp(view->name, "_bind") != 0 && view->rdclass != dns_rdataclass_chaos) { - /* named.conf only */ CHECK(configure_view_acl(vconfig, config, "allow-recursion", - NULL, aclctx, isc_g_mctx, + NULL, aclctx, isc_g_mctx, &ar, &view->recursionacl)); - /* named.conf only */ CHECK(configure_view_acl(vconfig, config, "allow-recursion-on", - NULL, aclctx, isc_g_mctx, + NULL, aclctx, isc_g_mctx, &aro, &view->recursiononacl)); } - if (view->recursion == false) { + if (view->recursion) { /* - * We're not recursive; if the query-cache ACLs haven't - * been set at the options/view level, set them to none. + * "allow-query-cache" inherits from "allow-recursion" if set, + * otherwise from "allow-query" if set. */ - if (view->cacheacl == NULL) { - CHECK(dns_acl_none(mctx, &view->cacheacl)); + if (!aqc) { + if (ar) { + dns_acl_detach(&view->cacheacl); + dns_acl_attach(view->recursionacl, + &view->cacheacl); + } else if (aq) { + dns_acl_detach(&view->cacheacl); + dns_acl_attach(view->queryacl, &view->cacheacl); + } } - if (view->cacheonacl == NULL) { - CHECK(dns_acl_none(mctx, &view->cacheonacl)); - } - } - /* - * Finished setting recursion and query-cache ACLs, so now we - * can get the allow-query default if it wasn't set in named.conf - */ - if (view->queryacl == NULL) { - /* global default only */ - CHECK(configure_view_acl(NULL, NULL, "allow-query", NULL, - aclctx, isc_g_mctx, &view->queryacl)); + /* + * "allow-recursion" inherits from "allow-query-cache" if set, + * otherwise from "allow-query" if set. + */ + if (!ar) { + if (aqc) { + dns_acl_detach(&view->recursionacl); + dns_acl_attach(view->cacheacl, + &view->recursionacl); + } else if (aq) { + dns_acl_detach(&view->recursionacl); + dns_acl_attach(view->queryacl, + &view->recursionacl); + } + } + + /* + * "allow-query-cache-on" inherits from "allow-recursion-on" + * if set, and vice versa. + */ + if (!aqco && aro) { + dns_acl_detach(&view->cacheonacl); + dns_acl_attach(view->recursiononacl, &view->cacheonacl); + } else if (!aro && aqco) { + dns_acl_detach(&view->recursiononacl); + dns_acl_attach(view->cacheonacl, &view->recursiononacl); + } } /* @@ -4934,7 +4954,8 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, * and is needed by some broken clients. */ CHECK(configure_view_acl(vconfig, config, "no-case-compress", NULL, - aclctx, isc_g_mctx, &view->nocasecompress)); + aclctx, isc_g_mctx, NULL, + &view->nocasecompress)); /* * Disable name compression completely, this is a tradeoff @@ -4949,7 +4970,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, * Filter setting on addresses in the answer section. */ CHECK(configure_view_acl(vconfig, config, "deny-answer-addresses", - "acl", aclctx, isc_g_mctx, + "acl", aclctx, isc_g_mctx, NULL, &view->denyansweracl)); CHECK(configure_view_nametable(vconfig, config, "deny-answer-addresses", "except-from", isc_g_mctx, @@ -4971,12 +4992,13 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, */ if (view->transferacl == NULL) { CHECK(configure_view_acl(vconfig, config, "allow-transfer", - NULL, aclctx, isc_g_mctx, + NULL, aclctx, isc_g_mctx, NULL, &view->transferacl)); } if (view->notifyacl == NULL) { CHECK(configure_view_acl(vconfig, config, "allow-notify", NULL, - aclctx, isc_g_mctx, &view->notifyacl)); + aclctx, isc_g_mctx, NULL, + &view->notifyacl)); } obj = NULL; @@ -8120,7 +8142,7 @@ apply_configuration(cfg_obj_t *effectiveconfig, cfg_obj_t *bindkeys, * no default. */ result = configure_view_acl(NULL, effectiveconfig, "blackhole", NULL, - aclctx, isc_g_mctx, + aclctx, isc_g_mctx, NULL, &server->sctx->blackholeacl); if (result != ISC_R_SUCCESS) { goto cleanup_tls; diff --git a/lib/isccfg/include/isccfg/cfg.h b/lib/isccfg/include/isccfg/cfg.h index ec3076f3ce..9e74dc4d7b 100644 --- a/lib/isccfg/include/isccfg/cfg.h +++ b/lib/isccfg/include/isccfg/cfg.h @@ -549,6 +549,12 @@ cfg_obj_istype(const cfg_obj_t *obj, const cfg_type_t *type); * Return true iff 'obj' is of type 'type'. */ +bool +cfg_obj_iscloned(const cfg_obj_t *obj); +/*%< + * Return true iff 'obj' has the 'CFG_OBJ_CLONED' flag. + */ + void cfg_obj_log(const cfg_obj_t *obj, int level, const char *fmt, ...) ISC_FORMAT_PRINTF(3, 4); @@ -578,6 +584,9 @@ const cfg_clausedef_t * cfg_map_nextclause(const cfg_type_t *map, const void **clauses, unsigned int *idx); +const cfg_clausedef_t * +cfg_map_findclause(const cfg_type_t *map, const char *name); + typedef isc_result_t(pluginlist_cb_t)( const cfg_obj_t *config, const cfg_obj_t *obj, cfg_aclconfctx_t *aclctx, const char *plugin_path, const char *parameters, void *callback_data); diff --git a/lib/isccfg/include/isccfg/grammar.h b/lib/isccfg/include/isccfg/grammar.h index e0839e9e64..659a953e2c 100644 --- a/lib/isccfg/include/isccfg/grammar.h +++ b/lib/isccfg/include/isccfg/grammar.h @@ -201,6 +201,13 @@ struct cfg_obj { isc_mem_t *mctx; isc_refcount_t references; + /*% + * Indicates that an object was cloned from the defaults + * or otherwise generated during the configuration merge + * process: + */ + bool cloned; + const cfg_type_t *type; union { uint32_t uint32; diff --git a/lib/isccfg/namedconf.c b/lib/isccfg/namedconf.c index e2d5796ed9..b949a44304 100644 --- a/lib/isccfg/namedconf.c +++ b/lib/isccfg/namedconf.c @@ -1219,6 +1219,16 @@ merge_append(cfg_obj_t *effectiveobj, const cfg_obj_t *defaultobj) { cfg_list_addclone(effectiveobj, defaultobj, false); } +static void +cloneto(cfg_obj_t *options, const cfg_obj_t *obj, const char *clausename) { + isc_result_t result; + const cfg_clausedef_t *clause = cfg_map_findclause(options->type, + clausename); + + result = cfg_map_addclone(options, obj, clause); + INSIST(result == ISC_R_SUCCESS); +} + static void options_merge_defaultacl(cfg_obj_t *effectiveoptions, const cfg_obj_t *defaultoptions, const char *aclname, @@ -1233,9 +1243,7 @@ options_merge_defaultacl(cfg_obj_t *effectiveoptions, result = cfg_map_get(defaultoptions, aclname, &obj); INSIST(result == ISC_R_SUCCESS); - cfg_obj_ref(UNCONST(obj)); - result = cfg_map_add(effectiveoptions, UNCONST(obj), aclname); - INSIST(result == ISC_R_SUCCESS); + cloneto(effectiveoptions, obj, aclname); } static void @@ -1265,19 +1273,13 @@ options_merge(cfg_obj_t *effectiveoptions, const cfg_obj_t *defaultoptions) { if (result != ISC_R_SUCCESS) { result = cfg_map_get(effectiveoptions, "allow-recursion", &obj); if (result == ISC_R_SUCCESS) { - cfg_obj_ref(UNCONST(obj)); - result = cfg_map_add(effectiveoptions, UNCONST(obj), - "allow-query-cache"); - INSIST(result == ISC_R_SUCCESS); + cloneto(effectiveoptions, obj, "allow-query-cache"); } else { result = cfg_map_get(effectiveoptions, "allow-query", &obj); if (result == ISC_R_SUCCESS) { - cfg_obj_ref(UNCONST(obj)); - result = cfg_map_add(effectiveoptions, - UNCONST(obj), - "allow-query-cache"); - INSIST(result == ISC_R_SUCCESS); + cloneto(effectiveoptions, obj, + "allow-query-cache"); } else { noquerycacheacl = true; } @@ -1290,19 +1292,13 @@ options_merge(cfg_obj_t *effectiveoptions, const cfg_obj_t *defaultoptions) { result = cfg_map_get(effectiveoptions, "allow-query-cache", &obj); if (result == ISC_R_SUCCESS) { - cfg_obj_ref(UNCONST(obj)); - result = cfg_map_add(effectiveoptions, UNCONST(obj), - "allow-recursion"); - INSIST(result == ISC_R_SUCCESS); + cloneto(effectiveoptions, obj, "allow-recursion"); } else { result = cfg_map_get(effectiveoptions, "allow-query", &obj); if (result == ISC_R_SUCCESS) { - cfg_obj_ref(UNCONST(obj)); - result = cfg_map_add(effectiveoptions, - UNCONST(obj), - "allow-recursion"); - INSIST(result == ISC_R_SUCCESS); + cloneto(effectiveoptions, obj, + "allow-recursion"); } else { norecursionacl = true; } @@ -1315,10 +1311,7 @@ options_merge(cfg_obj_t *effectiveoptions, const cfg_obj_t *defaultoptions) { result = cfg_map_get(effectiveoptions, "allow-recursion-on", &obj); if (result == ISC_R_SUCCESS) { - cfg_obj_ref(UNCONST(obj)); - result = cfg_map_add(effectiveoptions, UNCONST(obj), - "allow-query-cache-on"); - INSIST(result == ISC_R_SUCCESS); + cloneto(effectiveoptions, obj, "allow-query-cache-on"); } else { noquerycacheonacl = true; } @@ -1330,10 +1323,7 @@ options_merge(cfg_obj_t *effectiveoptions, const cfg_obj_t *defaultoptions) { result = cfg_map_get(effectiveoptions, "allow-query-cache-on", &obj); if (result == ISC_R_SUCCESS) { - cfg_obj_ref(UNCONST(obj)); - result = cfg_map_add(effectiveoptions, UNCONST(obj), - "allow-recursion-on"); - INSIST(result == ISC_R_SUCCESS); + cloneto(effectiveoptions, obj, "allow-recursion-on"); } else { norecursiononacl = true; } diff --git a/lib/isccfg/parser.c b/lib/isccfg/parser.c index 15c2c53e46..69938d7c4a 100644 --- a/lib/isccfg/parser.c +++ b/lib/isccfg/parser.c @@ -175,6 +175,7 @@ cfg_obj_clone(const cfg_obj_t *source, cfg_obj_t **target) { cfg_obj_create(source->mctx, source->file, source->line, source->type, target); + (*target)->cloned = source->cloned; source->type->rep->copy(*target, source); } @@ -2989,6 +2990,23 @@ cfg_map_nextclause(const cfg_type_t *map, const void **clauses, return &(*clauseset)[*idx]; } +const cfg_clausedef_t * +cfg_map_findclause(const cfg_type_t *map, const char *name) { + const cfg_clausedef_t *found = NULL; + const void *clauses = NULL; + unsigned int idx; + + REQUIRE(map != NULL && map->rep == &cfg_rep_map); + REQUIRE(name != NULL); + + found = cfg_map_firstclause(map, &clauses, &idx); + while (name != NULL && strcasecmp(name, found->name)) { + found = cfg_map_nextclause(map, &clauses, &idx); + } + + return ((cfg_clausedef_t *)clauses) + idx; +} + /* Parse an arbitrary token, storing its raw text representation. */ static isc_result_t parse_token(cfg_parser_t *pctx, const cfg_type_t *type ISC_ATTR_UNUSED, @@ -3986,6 +4004,12 @@ cfg_obj_istype(const cfg_obj_t *obj, const cfg_type_t *type) { return obj->type == type; } +bool +cfg_obj_iscloned(const cfg_obj_t *obj) { + REQUIRE(VALID_CFGOBJ(obj)); + return obj->cloned; +} + /* * Destroy 'obj'. */ @@ -4037,24 +4061,6 @@ cfg_print_grammar(const cfg_type_t *type, unsigned int flags, cfg_doc_obj(&pctx, type); } -static const cfg_clausedef_t * -map_lookup_clause(const cfg_obj_t *mapobj, const char *clausename) { - const cfg_map_t *map; - const cfg_clausedef_t *const *clauseset = NULL; - const cfg_clausedef_t *clause = NULL; - - map = &mapobj->value.map; - for (clauseset = map->clausesets; *clauseset != NULL; clauseset++) { - for (clause = *clauseset; clause->name != NULL; clause++) { - if (strcasecmp(clause->name, clausename) == 0) { - return clause; - } - } - } - - return NULL; -} - static isc_result_t map_define(cfg_obj_t *mapobj, cfg_obj_t *obj, const cfg_clausedef_t *clause) { isc_result_t result; @@ -4110,7 +4116,7 @@ cfg_map_add(cfg_obj_t *mapobj, cfg_obj_t *obj, const char *clausename) { REQUIRE(mapobj->type->rep == &cfg_rep_map); REQUIRE(clausename != NULL); - clause = map_lookup_clause(mapobj, clausename); + clause = cfg_map_findclause(mapobj->type, clausename); if (clause == NULL || clause->name == NULL) { return ISC_R_FAILURE; } @@ -4142,6 +4148,8 @@ cfg_map_addclone(cfg_obj_t *map, const cfg_obj_t *obj, elt = cfg_list_first(obj); while (elt != NULL && result == ISC_R_SUCCESS) { cfg_obj_clone(elt->obj, &clone); + clone->cloned = true; + result = map_define(map, clone, clause); elt = cfg_list_next(elt); @@ -4153,6 +4161,7 @@ cfg_map_addclone(cfg_obj_t *map, const cfg_obj_t *obj, } } else { cfg_obj_clone(obj, &clone); + clone->cloned = true; result = map_define(map, clone, clause); } diff --git a/tests/isccfg/grammar_test.c b/tests/isccfg/grammar_test.c index 15610a6d11..59ea824112 100644 --- a/tests/isccfg/grammar_test.c +++ b/tests/isccfg/grammar_test.c @@ -72,29 +72,15 @@ assert_text(const char *text) { memset(gtext, 0, sizeof(gtext)); } -static const cfg_clausedef_t * -find_clause(const cfg_type_t *map, const char *name) { - const cfg_clausedef_t *found = NULL; - const void *clauses = NULL; - unsigned int idx; - - found = cfg_map_firstclause(map, &clauses, &idx); - while (name != NULL && strcasecmp(name, found->name)) { - found = cfg_map_nextclause(map, &clauses, &idx); - } - - return ((cfg_clausedef_t *)clauses) + idx; -} - static void test__querysource(const char *clause_name, const char *name, const char *expected) { const cfg_clausedef_t *options_clause = NULL; - options_clause = find_clause(&cfg_type_namedconf, clause_name); + options_clause = cfg_map_findclause(&cfg_type_namedconf, clause_name); assert_non_null(options_clause); const cfg_clausedef_t *querysource_clause = NULL; - querysource_clause = find_clause(options_clause->type, name); + querysource_clause = cfg_map_findclause(options_clause->type, name); assert_non_null(querysource_clause); querysource_clause->type->doc(&gprinter, querysource_clause->type); assert_text(expected); From f798feda40a29098fda78c4cd3e4e9589cb47d58 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 19 Nov 2025 22:09:53 -0800 Subject: [PATCH 3/3] fix ACL settings when merging views when merging view objects into the effective configuration, add allow-query-cache, allow-recursion, allow-query-cache-on and allow-recursion-on ACLs as needed to reflect the way those options inherit from each other. this means the effective configuration is now correct for each view. ACLs no longer need to be corrected when applying the configuration, and the actual effective ACL values will be displayed in "rndc showconf" and "named-checkconf -pe". --- bin/named/server.c | 96 ++--------- lib/isccfg/include/isccfg/cfg.h | 6 - lib/isccfg/include/isccfg/grammar.h | 3 +- lib/isccfg/namedconf.c | 246 ++++++++++++++++------------ lib/isccfg/parser.c | 6 - 5 files changed, 161 insertions(+), 196 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 20dbb74ed3..f7955a421a 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -541,7 +541,7 @@ load_nzf(dns_view_t *view); static isc_result_t configure_view_acl(const cfg_obj_t *vconfig, const cfg_obj_t *config, const char *aclname, const char *acltuplename, - cfg_aclconfctx_t *aclctx, isc_mem_t *mctx, bool *expp, + cfg_aclconfctx_t *aclctx, isc_mem_t *mctx, dns_acl_t **aclp) { isc_result_t result; const cfg_obj_t *maps[4]; @@ -580,14 +580,6 @@ configure_view_acl(const cfg_obj_t *vconfig, const cfg_obj_t *config, aclobj = cfg_tuple_get(aclobj, acltuplename); } - if (expp != NULL) { - /* - * Note whether the ACL was explicitly configured, - * not cloned during the merge process. - */ - *expp = !cfg_obj_iscloned(aclobj); - } - result = cfg_acl_fromconfig(aclobj, config, aclctx, mctx, 0, aclp); return result; @@ -4781,10 +4773,9 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, * can be retrieved.) */ CHECK(configure_view_acl(vconfig, config, "match-clients", NULL, aclctx, - isc_g_mctx, NULL, &view->matchclients)); + isc_g_mctx, &view->matchclients)); CHECK(configure_view_acl(vconfig, config, "match-destinations", NULL, - aclctx, isc_g_mctx, NULL, - &view->matchdestinations)); + aclctx, isc_g_mctx, &view->matchdestinations)); /* * Configure the "match-recursive-only" option. @@ -4868,94 +4859,40 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, INSIST(result == ISC_R_SUCCESS); view->root_key_sentinel = cfg_obj_asboolean(obj); - /* - * The "allow-query", "allow-query-cache", "allow-recursion", - * "allow-recursion-on" and "allow-query-cache-on" ACLs can - * inherit from one other, so there's special handling based - * on whether they're explicitly set in named.conf or cloned - * from the defaults. - */ - bool aq, aqc, ar, aro, aqco; CHECK(configure_view_acl(vconfig, config, "allow-query", NULL, aclctx, - isc_g_mctx, &aq, &view->queryacl)); + isc_g_mctx, &view->queryacl)); CHECK(configure_view_acl(vconfig, config, "allow-query-on", NULL, - aclctx, isc_g_mctx, NULL, &view->queryonacl)); + aclctx, isc_g_mctx, &view->queryonacl)); CHECK(configure_view_acl(vconfig, config, "allow-query-cache", NULL, - aclctx, isc_g_mctx, &aqc, &view->cacheacl)); + aclctx, isc_g_mctx, &view->cacheacl)); CHECK(configure_view_acl(vconfig, config, "allow-query-cache-on", NULL, - aclctx, isc_g_mctx, &aqco, &view->cacheonacl)); + aclctx, isc_g_mctx, &view->cacheonacl)); CHECK(configure_view_acl(vconfig, config, "allow-proxy", NULL, aclctx, - isc_g_mctx, NULL, &view->proxyacl)); + isc_g_mctx, &view->proxyacl)); CHECK(configure_view_acl(vconfig, config, "allow-proxy-on", NULL, - aclctx, isc_g_mctx, NULL, &view->proxyonacl)); + aclctx, isc_g_mctx, &view->proxyonacl)); if (strcmp(view->name, "_bind") != 0 && view->rdclass != dns_rdataclass_chaos) { CHECK(configure_view_acl(vconfig, config, "allow-recursion", - NULL, aclctx, isc_g_mctx, &ar, + NULL, aclctx, isc_g_mctx, &view->recursionacl)); CHECK(configure_view_acl(vconfig, config, "allow-recursion-on", - NULL, aclctx, isc_g_mctx, &aro, + NULL, aclctx, isc_g_mctx, &view->recursiononacl)); } - if (view->recursion) { - /* - * "allow-query-cache" inherits from "allow-recursion" if set, - * otherwise from "allow-query" if set. - */ - if (!aqc) { - if (ar) { - dns_acl_detach(&view->cacheacl); - dns_acl_attach(view->recursionacl, - &view->cacheacl); - } else if (aq) { - dns_acl_detach(&view->cacheacl); - dns_acl_attach(view->queryacl, &view->cacheacl); - } - } - - /* - * "allow-recursion" inherits from "allow-query-cache" if set, - * otherwise from "allow-query" if set. - */ - if (!ar) { - if (aqc) { - dns_acl_detach(&view->recursionacl); - dns_acl_attach(view->cacheacl, - &view->recursionacl); - } else if (aq) { - dns_acl_detach(&view->recursionacl); - dns_acl_attach(view->queryacl, - &view->recursionacl); - } - } - - /* - * "allow-query-cache-on" inherits from "allow-recursion-on" - * if set, and vice versa. - */ - if (!aqco && aro) { - dns_acl_detach(&view->cacheonacl); - dns_acl_attach(view->recursiononacl, &view->cacheonacl); - } else if (!aro && aqco) { - dns_acl_detach(&view->recursiononacl); - dns_acl_attach(view->cacheonacl, &view->recursiononacl); - } - } - /* * Ignore case when compressing responses to the specified * clients. This causes case not always to be preserved, * and is needed by some broken clients. */ CHECK(configure_view_acl(vconfig, config, "no-case-compress", NULL, - aclctx, isc_g_mctx, NULL, - &view->nocasecompress)); + aclctx, isc_g_mctx, &view->nocasecompress)); /* * Disable name compression completely, this is a tradeoff @@ -4970,7 +4907,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, * Filter setting on addresses in the answer section. */ CHECK(configure_view_acl(vconfig, config, "deny-answer-addresses", - "acl", aclctx, isc_g_mctx, NULL, + "acl", aclctx, isc_g_mctx, &view->denyansweracl)); CHECK(configure_view_nametable(vconfig, config, "deny-answer-addresses", "except-from", isc_g_mctx, @@ -4992,13 +4929,12 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, */ if (view->transferacl == NULL) { CHECK(configure_view_acl(vconfig, config, "allow-transfer", - NULL, aclctx, isc_g_mctx, NULL, + NULL, aclctx, isc_g_mctx, &view->transferacl)); } if (view->notifyacl == NULL) { CHECK(configure_view_acl(vconfig, config, "allow-notify", NULL, - aclctx, isc_g_mctx, NULL, - &view->notifyacl)); + aclctx, isc_g_mctx, &view->notifyacl)); } obj = NULL; @@ -8142,7 +8078,7 @@ apply_configuration(cfg_obj_t *effectiveconfig, cfg_obj_t *bindkeys, * no default. */ result = configure_view_acl(NULL, effectiveconfig, "blackhole", NULL, - aclctx, isc_g_mctx, NULL, + aclctx, isc_g_mctx, &server->sctx->blackholeacl); if (result != ISC_R_SUCCESS) { goto cleanup_tls; diff --git a/lib/isccfg/include/isccfg/cfg.h b/lib/isccfg/include/isccfg/cfg.h index 9e74dc4d7b..882ac2cca1 100644 --- a/lib/isccfg/include/isccfg/cfg.h +++ b/lib/isccfg/include/isccfg/cfg.h @@ -549,12 +549,6 @@ cfg_obj_istype(const cfg_obj_t *obj, const cfg_type_t *type); * Return true iff 'obj' is of type 'type'. */ -bool -cfg_obj_iscloned(const cfg_obj_t *obj); -/*%< - * Return true iff 'obj' has the 'CFG_OBJ_CLONED' flag. - */ - void cfg_obj_log(const cfg_obj_t *obj, int level, const char *fmt, ...) ISC_FORMAT_PRINTF(3, 4); diff --git a/lib/isccfg/include/isccfg/grammar.h b/lib/isccfg/include/isccfg/grammar.h index 659a953e2c..460afb1087 100644 --- a/lib/isccfg/include/isccfg/grammar.h +++ b/lib/isccfg/include/isccfg/grammar.h @@ -133,7 +133,8 @@ struct cfg_printer { * cfg_rep_map, because the allow-query and allow-recursion ACLs have * complex semantics that need to be preserved. */ -typedef void (*cfg_mergefunc_t)(cfg_obj_t *effectiveobj, +typedef void (*cfg_mergefunc_t)(const cfg_obj_t *config, + cfg_obj_t *effectiveobj, const cfg_obj_t *defaultobj); struct cfg_clausedef { diff --git a/lib/isccfg/namedconf.c b/lib/isccfg/namedconf.c index b949a44304..e4421c50c8 100644 --- a/lib/isccfg/namedconf.c +++ b/lib/isccfg/namedconf.c @@ -1148,7 +1148,8 @@ static cfg_type_t cfg_type_fetchesper = { "fetchesper", cfg_parse_tuple, &cfg_rep_tuple, fetchesper_fields }; static void -map_merge(cfg_obj_t *effectivemap, const cfg_obj_t *defaultmap) { +map_merge(const cfg_obj_t *config ISC_ATTR_UNUSED, cfg_obj_t *effectivemap, + const cfg_obj_t *defaultmap) { const void *clauses = NULL; const cfg_clausedef_t *clause = NULL; unsigned int i = 0; @@ -1178,7 +1179,7 @@ map_merge(cfg_obj_t *effectivemap, const cfg_obj_t *defaultmap) { if (effectiveobj != NULL && defaultobj != NULL && clause->merge != NULL) { - clause->merge(effectiveobj, defaultobj); + clause->merge(effectivemap, effectiveobj, defaultobj); continue; } @@ -1202,21 +1203,14 @@ map_merge(cfg_obj_t *effectivemap, const cfg_obj_t *defaultmap) { } /* - * These are used when merging clauses with CFG_CLAUSEFLAG_MULTI, where - * the entries from the user configuration and the default configuration - * are added together, rather than the user configuration overriding the - * default. merge_prepend() puts the default configuration at the - * beginning of the cloned object (for example, for dnssec-policy), and - * merge_append() puts it at the end (for example, for views). + * "dnssec-policy" has CFG_CLAUSEFLAG_MULTI, but unlike most such + * clauses, the entries in the user configuration are appended to the + * default configuration instead of overriding the list. */ static void -merge_prepend(cfg_obj_t *effectiveobj, const cfg_obj_t *defaultobj) { - cfg_list_addclone(effectiveobj, defaultobj, true); -} - -static void -merge_append(cfg_obj_t *effectiveobj, const cfg_obj_t *defaultobj) { - cfg_list_addclone(effectiveobj, defaultobj, false); +policy_merge(const cfg_obj_t *config ISC_ATTR_UNUSED, cfg_obj_t *eff, + const cfg_obj_t *def) { + cfg_list_addclone(eff, def, true); } static void @@ -1230,115 +1224,159 @@ cloneto(cfg_obj_t *options, const cfg_obj_t *obj, const char *clausename) { } static void -options_merge_defaultacl(cfg_obj_t *effectiveoptions, - const cfg_obj_t *defaultoptions, const char *aclname, - bool needsdefault) { +setdefaultacl(cfg_obj_t *options, const cfg_obj_t *defaultoptions, + const char *aclname) { const cfg_obj_t *obj = NULL; isc_result_t result; - if (needsdefault == false) { + result = cfg_map_get(options, aclname, &obj); + if (result == ISC_R_SUCCESS) { return; } + obj = NULL; result = cfg_map_get(defaultoptions, aclname, &obj); INSIST(result == ISC_R_SUCCESS); - cloneto(effectiveoptions, obj, aclname); + cloneto(options, obj, aclname); +} + +static const cfg_obj_t * +aclobj(const cfg_obj_t *o, const cfg_obj_t *v, const char *name) { + const cfg_obj_t *obj = NULL; + + cfg_map_get(v, name, &obj); + if (obj == NULL) { + cfg_map_get(o, name, &obj); + } + return obj; } static void -options_merge(cfg_obj_t *effectiveoptions, const cfg_obj_t *defaultoptions) { - const cfg_obj_t *obj = NULL; - isc_result_t result; - bool noquerycacheacl = false; - bool norecursionacl = false; - bool noquerycacheonacl = false; - bool norecursiononacl = false; +setacls(const cfg_obj_t *config, cfg_obj_t *voptions, + const cfg_obj_t *defaultoptions) { + const cfg_obj_t *options = NULL; + const cfg_obj_t *query = NULL, *cache = NULL, *cacheon = NULL; + const cfg_obj_t *recursion = NULL, *recursionon = NULL; + + cfg_map_get(config, "options", &options); + INSIST(options != NULL); /* - * ACLs allow-query-cache, allow-recursion, allow-query-cache-on and - * allow-recursion-on need to be "merged" at once because there - * are implicit dependency rules between them. After all those - * dependency rules have been applied, the default values are used - * _only_ if they are still undefined in the user configuration. - * - * This need to be done only for the global options, because the views - * and zone ACL initialization code will look in the global options - * as fallback, and they'll be defined there. - * - * This is useless (and shouldn't have any effect) for views with - * recursion=false, but needed for those with recursion=true + * This can be called in two different contexts: from the top-level + * option clause, or from the user-defined views. */ - result = cfg_map_get(effectiveoptions, "allow-query-cache", &obj); - if (result != ISC_R_SUCCESS) { - result = cfg_map_get(effectiveoptions, "allow-recursion", &obj); - if (result == ISC_R_SUCCESS) { - cloneto(effectiveoptions, obj, "allow-query-cache"); - } else { - result = cfg_map_get(effectiveoptions, "allow-query", - &obj); - if (result == ISC_R_SUCCESS) { - cloneto(effectiveoptions, obj, - "allow-query-cache"); - } else { - noquerycacheacl = true; - } + INSIST((options == voptions && defaultoptions != NULL) || + (options != voptions && defaultoptions == NULL)); + + query = aclobj(options, voptions, "allow-query"); + recursion = aclobj(options, voptions, "allow-recursion"); + cache = aclobj(options, voptions, "allow-query-cache"); + + cacheon = aclobj(options, voptions, "allow-query-cache-on"); + recursionon = aclobj(options, voptions, "allow-recursion-on"); + + bool aq = query != NULL && !query->cloned; + bool aqc = cache != NULL && !cache->cloned; + bool ar = recursion != NULL && !recursion->cloned; + + bool aqco = cacheon != NULL && !cacheon->cloned; + bool aro = recursionon != NULL && !recursionon->cloned; + + /* + * "allow-query-cache" inherits from "allow-recursion" if set, + * otherwise from "allow-query" if set. + */ + if (!aqc) { + if (ar) { + cloneto(voptions, recursion, "allow-query-cache"); + } else if (aq) { + cloneto(voptions, query, "allow-query-cache"); } } - obj = NULL; - result = cfg_map_get(effectiveoptions, "allow-recursion", &obj); - if (result != ISC_R_SUCCESS) { - result = cfg_map_get(effectiveoptions, "allow-query-cache", - &obj); - if (result == ISC_R_SUCCESS) { - cloneto(effectiveoptions, obj, "allow-recursion"); - } else { - result = cfg_map_get(effectiveoptions, "allow-query", - &obj); - if (result == ISC_R_SUCCESS) { - cloneto(effectiveoptions, obj, - "allow-recursion"); - } else { - norecursionacl = true; - } + /* + * "allow-recursion" inherits from "allow-query-cache" if set, + * otherwise from "allow-query" if set. + */ + if (!ar) { + if (aqc) { + cloneto(voptions, cache, "allow-recursion"); + } else if (aq) { + cloneto(voptions, query, "allow-recursion"); } } - obj = NULL; - result = cfg_map_get(effectiveoptions, "allow-query-cache-on", &obj); - if (result != ISC_R_SUCCESS) { - result = cfg_map_get(effectiveoptions, "allow-recursion-on", - &obj); - if (result == ISC_R_SUCCESS) { - cloneto(effectiveoptions, obj, "allow-query-cache-on"); - } else { - noquerycacheonacl = true; - } + /* + * "allow-query-cache-on" inherits from "allow-recursion-on" + * if set, and vice versa. + */ + if (!aqco && aro) { + cloneto(voptions, recursionon, "allow-query-cache-on"); + } else if (!aro && aqco) { + cloneto(voptions, cacheon, "allow-recursion-on"); } - obj = NULL; - result = cfg_map_get(effectiveoptions, "allow-recursion-on", &obj); - if (result != ISC_R_SUCCESS) { - result = cfg_map_get(effectiveoptions, "allow-query-cache-on", - &obj); - if (result == ISC_R_SUCCESS) { - cloneto(effectiveoptions, obj, "allow-recursion-on"); - } else { - norecursiononacl = true; - } + if (options == voptions) { + /* + * This is the top-level options clause. This clause gets copies + * of the default ACL if they are not defined. Those will be + * used for user views ACLs too. + */ + setdefaultacl(voptions, defaultoptions, "allow-query-cache"); + setdefaultacl(voptions, defaultoptions, "allow-recursion"); + setdefaultacl(voptions, defaultoptions, "allow-query-cache-on"); + setdefaultacl(voptions, defaultoptions, "allow-recursion-on"); } +} - options_merge_defaultacl(effectiveoptions, defaultoptions, - "allow-query-cache", noquerycacheacl); - options_merge_defaultacl(effectiveoptions, defaultoptions, - "allow-recursion", norecursionacl); - options_merge_defaultacl(effectiveoptions, defaultoptions, - "allow-query-cache-on", noquerycacheonacl); - options_merge_defaultacl(effectiveoptions, defaultoptions, - "allow-recursion-on", norecursiononacl); +static void +options_merge(const cfg_obj_t *config, cfg_obj_t *effectiveoptions, + const cfg_obj_t *defaultoptions) { + /* + * ACLs allow-query-cache, allow-recursion, allow-query-cache-on + * and allow-recursion-on need to be merged with the defaults + * carefully, because there are implicit dependency rules + * between them. + * + * Note: this is similar to the code in view_merge() + * below, but that's only called when views are explicitly + * configured in named.conf, so we need to do this at the + * options level too. + */ + setacls(config, effectiveoptions, defaultoptions); - map_merge(effectiveoptions, defaultoptions); + map_merge(config, effectiveoptions, defaultoptions); +} + +/* + * "view" has CFG_CLAUSEFLAG_MULTI, but unlike most such clauses, the + * entries in the user configuration are *prepended* to the default + * configuration instead of overriding the list. + * + * After all views have been cloned into the effective configuration, + * we correct their ACL settings to take into account the mutual iheritance + * of allow-recursion, allow-query-cache, and allow-query. + */ +static void +view_merge(const cfg_obj_t *config, cfg_obj_t *eff, const cfg_obj_t *def) { + REQUIRE(cfg_obj_islist(eff)); + REQUIRE(cfg_obj_islist(def)); + + cfg_list_addclone(eff, def, false); + CFG_LIST_FOREACH(eff, elt) { + const cfg_obj_t *name = cfg_tuple_get(elt->obj, "name"); + cfg_obj_t *voptions = NULL; + + if (name != NULL && + strcmp(cfg_obj_asstring(name), "_bind") == 0) + { + continue; + } + + voptions = UNCONST(cfg_tuple_get(elt->obj, "options")); + setacls(config, voptions, NULL); + } } /*% @@ -1349,7 +1387,7 @@ static cfg_clausedef_t namedconf_clauses[] = { { "acl", &cfg_type_acl, CFG_CLAUSEFLAG_MULTI }, { "controls", &cfg_type_controls, CFG_CLAUSEFLAG_MULTI }, { "dnssec-policy", &cfg_type_dnssecpolicy, CFG_CLAUSEFLAG_MULTI, - merge_prepend }, + policy_merge }, #if HAVE_LIBNGHTTP2 { "http", &cfg_type_http_description, CFG_CLAUSEFLAG_MULTI | CFG_CLAUSEFLAG_OPTIONAL }, @@ -1380,7 +1418,7 @@ static cfg_clausedef_t namedconf_clauses[] = { CFG_CLAUSEFLAG_MULTI | CFG_CLAUSEFLAG_BUILTINONLY | CFG_CLAUSEFLAG_NODOC }, { "tls", &cfg_type_tlsconf, CFG_CLAUSEFLAG_MULTI }, - { "view", &cfg_type_view, CFG_CLAUSEFLAG_MULTI, merge_append }, + { "view", &cfg_type_view, CFG_CLAUSEFLAG_MULTI, view_merge }, { NULL, NULL, 0 } }; @@ -2221,7 +2259,8 @@ static cfg_type_t cfg_type_staleanswerclienttimeout = { }; static void -prefetch_merge(cfg_obj_t *effectiveobj, const cfg_obj_t *defaultobj) { +prefetch_merge(const cfg_obj_t *config ISC_ATTR_UNUSED, cfg_obj_t *effectiveobj, + const cfg_obj_t *defaultobj) { cfg_obj_t *trigger = NULL; cfg_obj_t *eligible = NULL; @@ -2243,7 +2282,8 @@ prefetch_merge(cfg_obj_t *effectiveobj, const cfg_obj_t *defaultobj) { } static void -checknames_merge(cfg_obj_t *effectiveobj, const cfg_obj_t *defaultobj) { +checknames_merge(const cfg_obj_t *config ISC_ATTR_UNUSED, + cfg_obj_t *effectiveobj, const cfg_obj_t *defaultobj) { /* * Applies only to the top-level option `check-names` statement. * The view and zone-level versions aren't merged into the defaults @@ -4346,7 +4386,7 @@ cfg_effective_config(const cfg_obj_t *userconfig, REQUIRE(userconfig != NULL && userconfig->type == &cfg_type_namedconf); cfg_obj_clone(userconfig, &effective); - map_merge(effective, defaultconfig); + map_merge(effective, effective, defaultconfig); return effective; } diff --git a/lib/isccfg/parser.c b/lib/isccfg/parser.c index 69938d7c4a..199e9aa9fb 100644 --- a/lib/isccfg/parser.c +++ b/lib/isccfg/parser.c @@ -4004,12 +4004,6 @@ cfg_obj_istype(const cfg_obj_t *obj, const cfg_type_t *type) { return obj->type == type; } -bool -cfg_obj_iscloned(const cfg_obj_t *obj) { - REQUIRE(VALID_CFGOBJ(obj)); - return obj->cloned; -} - /* * Destroy 'obj'. */