From: Colin Vidal <colin@isc.org>
Date: Thu, 16 Oct 2025 14:26:06 +0000 (+0200)
Subject: effective config: specific options/acl cases
X-Git-Tag: v9.21.15~22^2~10
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a99573d64846cc05d8d87453bc1afed23c15ce14;p=thirdparty%2Fbind9.git

effective config: specific options/acl cases

Implement the specific rules of ACL inheritance when buiding the
effective configuration. As those rules are directly implemented in the
configuration tree, they are removed from `apply_configuation`.
---

diff --git a/bin/named/server.c b/bin/named/server.c
index 49e7143bdd3..653c915ee21 100644
--- a/bin/named/server.c
+++ b/bin/named/server.c
@@ -553,9 +553,9 @@ load_nzf(dns_view_t *view, ns_cfgctx_t *nzcfg);
  */
 static isc_result_t
 configure_view_acl(const cfg_obj_t *vconfig, const cfg_obj_t *config,
-		   const cfg_obj_t *gconfig, const char *aclname,
-		   const char *acltuplename, cfg_aclconfctx_t *aclctx,
-		   isc_mem_t *mctx, dns_acl_t **aclp) {
+		   const char *aclname, const char *acltuplename,
+		   cfg_aclconfctx_t *aclctx, isc_mem_t *mctx,
+		   dns_acl_t **aclp) {
 	isc_result_t result;
 	const cfg_obj_t *maps[4];
 	const cfg_obj_t *aclobj = NULL;
@@ -574,13 +574,6 @@ configure_view_acl(const cfg_obj_t *vconfig, const cfg_obj_t *config,
 			maps[i++] = options;
 		}
 	}
-	if (gconfig != NULL) {
-		const cfg_obj_t *options = NULL;
-		(void)cfg_map_get(gconfig, "options", &options);
-		if (options != NULL) {
-			maps[i++] = options;
-		}
-	}
 	maps[i] = NULL;
 
 	(void)named_config_get(maps, aclname, &aclobj);
@@ -4825,11 +4818,10 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config,
 	 * must be passed so that named ACLs defined at the global level
 	 * can be retrieved.)
 	 */
-	CHECK(configure_view_acl(vconfig, config, NULL, "match-clients", NULL,
-				 aclctx, isc_g_mctx, &view->matchclients));
-	CHECK(configure_view_acl(vconfig, config, NULL, "match-destinations",
-				 NULL, aclctx, isc_g_mctx,
-				 &view->matchdestinations));
+	CHECK(configure_view_acl(vconfig, config, "match-clients", NULL, aclctx,
+				 isc_g_mctx, &view->matchclients));
+	CHECK(configure_view_acl(vconfig, config, "match-destinations", NULL,
+				 aclctx, isc_g_mctx, &view->matchdestinations));
 
 	/*
 	 * Configure the "match-recursive-only" option.
@@ -4927,125 +4919,39 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config,
 	 */
 
 	/* named.conf only */
-	CHECK(configure_view_acl(vconfig, config, NULL, "allow-query", NULL,
-				 aclctx, isc_g_mctx, &view->queryacl));
+	CHECK(configure_view_acl(vconfig, config, "allow-query", NULL, aclctx,
+				 isc_g_mctx, &view->queryacl));
 
 	/* named.conf only */
-	CHECK(configure_view_acl(vconfig, config, NULL, "allow-query-cache",
-				 NULL, aclctx, isc_g_mctx, &view->cacheacl));
+	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, NULL, "allow-query-cache-on",
-				 NULL, aclctx, isc_g_mctx, &view->cacheonacl));
+	CHECK(configure_view_acl(vconfig, config, "allow-query-cache-on", NULL,
+				 aclctx, isc_g_mctx, &view->cacheonacl));
 
-	CHECK(configure_view_acl(vconfig, config, named_g_defaultconfig,
-				 "allow-query-on", NULL, aclctx, isc_g_mctx,
-				 &view->queryonacl));
+	CHECK(configure_view_acl(vconfig, config, "allow-query-on", NULL,
+				 aclctx, isc_g_mctx, &view->queryonacl));
 
-	CHECK(configure_view_acl(vconfig, config, named_g_defaultconfig,
-				 "allow-proxy", NULL, aclctx, isc_g_mctx,
-				 &view->proxyacl));
+	CHECK(configure_view_acl(vconfig, config, "allow-proxy", NULL, aclctx,
+				 isc_g_mctx, &view->proxyacl));
 
-	CHECK(configure_view_acl(vconfig, config, named_g_defaultconfig,
-				 "allow-proxy-on", NULL, aclctx, isc_g_mctx,
-				 &view->proxyonacl));
+	CHECK(configure_view_acl(vconfig, config, "allow-proxy-on", NULL,
+				 aclctx, isc_g_mctx, &view->proxyonacl));
 
 	if (strcmp(view->name, "_bind") != 0 &&
 	    view->rdclass != dns_rdataclass_chaos)
 	{
 		/* named.conf only */
-		CHECK(configure_view_acl(vconfig, config, NULL,
-					 "allow-recursion", NULL, aclctx,
-					 isc_g_mctx, &view->recursionacl));
+		CHECK(configure_view_acl(vconfig, config, "allow-recursion",
+					 NULL, aclctx, isc_g_mctx,
+					 &view->recursionacl));
 		/* named.conf only */
-		CHECK(configure_view_acl(vconfig, config, NULL,
-					 "allow-recursion-on", NULL, aclctx,
-					 isc_g_mctx, &view->recursiononacl));
+		CHECK(configure_view_acl(vconfig, config, "allow-recursion-on",
+					 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 (view->cacheacl == NULL) {
-			if (view->recursionacl != NULL) {
-				dns_acl_attach(view->recursionacl,
-					       &view->cacheacl);
-			} else if (view->queryacl != NULL) {
-				dns_acl_attach(view->queryacl, &view->cacheacl);
-			}
-		}
-
-		/*
-		 * "allow-recursion" inherits from "allow-query-cache" if set,
-		 * otherwise from "allow-query" if set.
-		 */
-		if (view->recursionacl == NULL) {
-			if (view->cacheacl != NULL) {
-				dns_acl_attach(view->cacheacl,
-					       &view->recursionacl);
-			} else if (view->queryacl != NULL) {
-				dns_acl_attach(view->queryacl,
-					       &view->recursionacl);
-			}
-		}
-
-		/*
-		 * "allow-query-cache-on" inherits from "allow-recursion-on"
-		 * if set.
-		 */
-		if (view->cacheonacl == NULL) {
-			if (view->recursiononacl != NULL) {
-				dns_acl_attach(view->recursiononacl,
-					       &view->cacheonacl);
-			}
-		}
-
-		/*
-		 * "allow-recursion-on" inherits from "allow-query-cache-on"
-		 * if set.
-		 */
-		if (view->recursiononacl == NULL) {
-			if (view->cacheonacl != NULL) {
-				dns_acl_attach(view->cacheonacl,
-					       &view->recursiononacl);
-			}
-		}
-
-		/*
-		 * If any are still unset at this point, we now get default
-		 * values for from the global config.
-		 */
-
-		if (view->recursionacl == NULL) {
-			/* global default only */
-			CHECK(configure_view_acl(
-				NULL, NULL, named_g_defaultconfig,
-				"allow-recursion", NULL, aclctx, isc_g_mctx,
-				&view->recursionacl));
-		}
-		if (view->recursiononacl == NULL) {
-			/* global default only */
-			CHECK(configure_view_acl(
-				NULL, NULL, named_g_defaultconfig,
-				"allow-recursion-on", NULL, aclctx, isc_g_mctx,
-				&view->recursiononacl));
-		}
-		if (view->cacheacl == NULL) {
-			/* global default only */
-			CHECK(configure_view_acl(
-				NULL, NULL, named_g_defaultconfig,
-				"allow-query-cache", NULL, aclctx, isc_g_mctx,
-				&view->cacheacl));
-		}
-		if (view->cacheonacl == NULL) {
-			/* global default only */
-			CHECK(configure_view_acl(
-				NULL, NULL, named_g_defaultconfig,
-				"allow-query-cache-on", NULL, aclctx,
-				isc_g_mctx, &view->cacheonacl));
-		}
-	} else {
+	if (view->recursion == false) {
 		/*
 		 * We're not recursive; if the query-cache ACLs haven't
 		 * been set at the options/view level, set them to none.
@@ -5064,9 +4970,8 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config,
 	 */
 	if (view->queryacl == NULL) {
 		/* global default only */
-		CHECK(configure_view_acl(NULL, NULL, named_g_defaultconfig,
-					 "allow-query", NULL, aclctx,
-					 isc_g_mctx, &view->queryacl));
+		CHECK(configure_view_acl(NULL, NULL, "allow-query", NULL,
+					 aclctx, isc_g_mctx, &view->queryacl));
 	}
 
 	/*
@@ -5074,9 +4979,8 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config,
 	 * clients. This causes case not always to be preserved,
 	 * and is needed by some broken clients.
 	 */
-	CHECK(configure_view_acl(vconfig, config, named_g_defaultconfig,
-				 "no-case-compress", NULL, aclctx, isc_g_mctx,
-				 &view->nocasecompress));
+	CHECK(configure_view_acl(vconfig, config, "no-case-compress", NULL,
+				 aclctx, isc_g_mctx, &view->nocasecompress));
 
 	/*
 	 * Disable name compression completely, this is a tradeoff
@@ -5090,9 +4994,9 @@ 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, named_g_defaultconfig,
-				 "deny-answer-addresses", "acl", aclctx,
-				 isc_g_mctx, &view->denyansweracl));
+	CHECK(configure_view_acl(vconfig, config, "deny-answer-addresses",
+				 "acl", aclctx, isc_g_mctx,
+				 &view->denyansweracl));
 	CHECK(configure_view_nametable(vconfig, config, "deny-answer-addresses",
 				       "except-from", isc_g_mctx,
 				       &view->answeracl_exclude));
@@ -5107,36 +5011,18 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config,
 				       "except-from", isc_g_mctx,
 				       &view->answernames_exclude));
 
-	/*
-	 * Configure default allow-update and allow-update-forwarding ACLs,
-	 * so they can be inherited by zones. (XXX: These are not
-	 * read from the options/view level here. However, they may be
-	 * read from there in zoneconf.c:configure_zone_acl() later.)
-	 */
-	if (view->updateacl == NULL) {
-		CHECK(configure_view_acl(NULL, NULL, named_g_defaultconfig,
-					 "allow-update", NULL, aclctx,
-					 isc_g_mctx, &view->updateacl));
-	}
-	if (view->upfwdacl == NULL) {
-		CHECK(configure_view_acl(NULL, NULL, named_g_defaultconfig,
-					 "allow-update-forwarding", NULL,
-					 aclctx, isc_g_mctx, &view->upfwdacl));
-	}
-
 	/*
 	 * Configure default allow-transfer and allow-notify ACLs so they
 	 * can be inherited by zones.
 	 */
 	if (view->transferacl == NULL) {
-		CHECK(configure_view_acl(vconfig, config, named_g_defaultconfig,
-					 "allow-transfer", NULL, aclctx,
-					 isc_g_mctx, &view->transferacl));
+		CHECK(configure_view_acl(vconfig, config, "allow-transfer",
+					 NULL, aclctx, isc_g_mctx,
+					 &view->transferacl));
 	}
 	if (view->notifyacl == NULL) {
-		CHECK(configure_view_acl(vconfig, config, named_g_defaultconfig,
-					 "allow-notify", NULL, aclctx,
-					 isc_g_mctx, &view->notifyacl));
+		CHECK(configure_view_acl(vconfig, config, "allow-notify", NULL,
+					 aclctx, isc_g_mctx, &view->notifyacl));
 	}
 
 	obj = NULL;
@@ -8200,9 +8086,8 @@ apply_configuration(cfg_obj_t *config, cfg_obj_t *bindkeys,
 	 * Set "blackhole". Only legal at options level; there is
 	 * no default.
 	 */
-	result = configure_view_acl(NULL, config, NULL, "blackhole", NULL,
-				    aclctx, isc_g_mctx,
-				    &server->sctx->blackholeacl);
+	result = configure_view_acl(NULL, config, "blackhole", NULL, aclctx,
+				    isc_g_mctx, &server->sctx->blackholeacl);
 	if (result != ISC_R_SUCCESS) {
 		goto cleanup_tls;
 	}
diff --git a/lib/isccfg/namedconf.c b/lib/isccfg/namedconf.c
index d28387549c9..85ea405a01a 100644
--- a/lib/isccfg/namedconf.c
+++ b/lib/isccfg/namedconf.c
@@ -1196,6 +1196,138 @@ map_merge(cfg_obj_t *effectivemap, const cfg_obj_t *defaultmap) {
 	}
 }
 
+static void
+options_merge_defaultacl(cfg_obj_t *effectiveoptions,
+			 const cfg_obj_t *defaultoptions, const char *aclname,
+			 bool needsdefault) {
+	const cfg_obj_t *obj = NULL;
+	isc_result_t result;
+
+	if (needsdefault == false) {
+		return;
+	}
+
+	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);
+}
+
+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;
+
+	/*
+	 * 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
+	 */
+	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) {
+			cfg_obj_ref(UNCONST(obj));
+			result = cfg_map_add(effectiveoptions, UNCONST(obj),
+					     "allow-query-cache");
+			INSIST(result == ISC_R_SUCCESS);
+		} 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);
+			} else {
+				noquerycacheacl = true;
+			}
+		}
+	}
+
+	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) {
+			cfg_obj_ref(UNCONST(obj));
+			result = cfg_map_add(effectiveoptions, UNCONST(obj),
+					     "allow-recursion");
+			INSIST(result == ISC_R_SUCCESS);
+		} 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);
+			} else {
+				norecursionacl = true;
+			}
+		}
+	}
+
+	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) {
+			cfg_obj_ref(UNCONST(obj));
+			result = cfg_map_add(effectiveoptions, UNCONST(obj),
+					     "allow-query-cache-on");
+			INSIST(result == ISC_R_SUCCESS);
+		} else {
+			noquerycacheonacl = true;
+		}
+	}
+
+	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) {
+			cfg_obj_ref(UNCONST(obj));
+			result = cfg_map_add(effectiveoptions, UNCONST(obj),
+					     "allow-recursion-on");
+			INSIST(result == ISC_R_SUCCESS);
+		} else {
+			norecursiononacl = true;
+		}
+	}
+
+	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);
+
+	map_merge(effectiveoptions, defaultoptions);
+}
+
 /*%
  * Clauses that can be found within the top level of the named.conf
  * file only.
@@ -1216,7 +1348,7 @@ static cfg_clausedef_t namedconf_clauses[] = {
 	{ "lwres", NULL, CFG_CLAUSEFLAG_MULTI | CFG_CLAUSEFLAG_ANCIENT },
 	{ "masters", &cfg_type_serverlist,
 	  CFG_CLAUSEFLAG_MULTI | CFG_CLAUSEFLAG_NODOC },
-	{ "options", &cfg_type_options, 0 },
+	{ "options", &cfg_type_options, 0, options_merge },
 	{ "parental-agents", &cfg_type_serverlist,
 	  CFG_CLAUSEFLAG_MULTI | CFG_CLAUSEFLAG_NODOC },
 	{ "primaries", &cfg_type_serverlist,