From: Colin Vidal <colin@isc.org>
Date: Mon, 8 Sep 2025 12:58:47 +0000 (+0200)
Subject: cfg_aclconfctx_t object is part of named_server
X-Git-Tag: v9.21.14~29^2~4
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=201f62d9efb3cf6ccbac1127768121e899d36ee8;p=thirdparty%2Fbind9.git

cfg_aclconfctx_t object is part of named_server

`named_g_actconfctx` is a global variable holding the ACL configuration
context alive (in particular, to dynamically load zones). However, this
object is build once per configuration (early) and is used only inside
server.c `apply_configuration` flow. (Two exceptions: the shutdown flow,
still in server.c and plugin check flow, which doesn't need it, so it's
NULL in such case).

Instead of leaving this global publicly exposed, it is now part of the
`named_server_t` object. This allows us to clearly see that, when
reconfigureing the server, the new instance of the ACL context is known
only by the newly built object and not currently used by "production"
object; and will help to move move logic before the exclusive mode is
taken.

The other advantage is that the ACL configuration context can now be
built before the exclusive lock as well.
---

diff --git a/bin/named/include/named/globals.h b/bin/named/include/named/globals.h
index 370cdc3914e..126d248f4d7 100644
--- a/bin/named/include/named/globals.h
+++ b/bin/named/include/named/globals.h
@@ -93,8 +93,7 @@ EXTERN const char *named_g_conffile	   INIT(NAMED_SYSCONFDIR "/named.conf");
 EXTERN const char *named_g_defaultbindkeys INIT(NULL);
 EXTERN const char *named_g_keyfile	   INIT(NAMED_SYSCONFDIR "/rndc.key");
 
-EXTERN bool named_g_conffileset		    INIT(false);
-EXTERN cfg_aclconfctx_t *named_g_aclconfctx INIT(NULL);
+EXTERN bool named_g_conffileset INIT(false);
 
 /*
  * Misc.
diff --git a/bin/named/include/named/server.h b/bin/named/include/named/server.h
index 1e6621ae9e6..106bb3502d6 100644
--- a/bin/named/include/named/server.h
+++ b/bin/named/include/named/server.h
@@ -40,6 +40,9 @@
 
 #include <named/types.h>
 
+struct cfg_aclconfctx;
+typedef struct cfg_aclconfctx cfg_aclconfctx_t;
+
 /*%
  * Name server state.  Better here than in lots of separate global variables.
  */
@@ -106,6 +109,8 @@ struct named_server {
 
 	isc_signal_t *sighup;
 	isc_signal_t *sigusr1;
+
+	cfg_aclconfctx_t *aclconfctx;
 };
 
 #define NAMED_SERVER_MAGIC    ISC_MAGIC('S', 'V', 'E', 'R')
@@ -416,5 +421,5 @@ named_server_getmemprof(void);
  */
 isc_result_t
 named_register_one_plugin(const cfg_obj_t *config, const cfg_obj_t *obj,
-			  const char *plugin_path, const char *parameters,
-			  void *callback_data);
+			  cfg_aclconfctx_t *actx, const char *plugin_path,
+			  const char *parameters, void *callback_data);
diff --git a/bin/named/include/named/zoneconf.h b/bin/named/include/named/zoneconf.h
index 47fb9aeaebd..8975c840beb 100644
--- a/bin/named/include/named/zoneconf.h
+++ b/bin/named/include/named/zoneconf.h
@@ -84,7 +84,8 @@ named_zone_templateopts(const cfg_obj_t *config, const cfg_obj_t *zoptions);
 
 isc_result_t
 named_zone_loadplugins(dns_zone_t *zone, const cfg_obj_t *config,
-		       const cfg_obj_t *toptions, const cfg_obj_t *zoptions);
+		       const cfg_obj_t *toptions, const cfg_obj_t *zoptions,
+		       cfg_aclconfctx_t *actx);
 /*%<
  * Load plugins that should run for this specific zone. Take care of cleaning
  * up any pre-existing plugins first, if the zone is re-used.
@@ -95,4 +96,5 @@ named_zone_loadplugins(dns_zone_t *zone, const cfg_obj_t *config,
  * \li	'zoptions' to be a valid zone configuration tree
  * \li	'toptions' to be NULL or valid template configuration tree
  * \li	'zoptions' to be NULL or a valid zone configuration tree
+ * \li  'actx' to be NULL (confcheck case only) or a valid acl conf ctx
  */
diff --git a/bin/named/server.c b/bin/named/server.c
index ea955267ac7..993e8f218f6 100644
--- a/bin/named/server.c
+++ b/bin/named/server.c
@@ -2986,7 +2986,8 @@ cleanup:
 	} while (0)
 
 static isc_result_t
-configure_rrl(dns_view_t *view, const cfg_obj_t *config, const cfg_obj_t *map) {
+configure_rrl(dns_view_t *view, const cfg_obj_t *config, const cfg_obj_t *map,
+	      cfg_aclconfctx_t *actx) {
 	const cfg_obj_t *obj;
 	dns_rrl_t *rrl;
 	isc_result_t result;
@@ -3097,8 +3098,8 @@ configure_rrl(dns_view_t *view, const cfg_obj_t *config, const cfg_obj_t *map) {
 	obj = NULL;
 	result = cfg_map_get(map, "exempt-clients", &obj);
 	if (result == ISC_R_SUCCESS) {
-		result = cfg_acl_fromconfig(obj, config, named_g_aclconfctx,
-					    isc_g_mctx, 0, &rrl->exempt);
+		result = cfg_acl_fromconfig(obj, config, actx, isc_g_mctx, 0,
+					    &rrl->exempt);
 		CHECK_RRL(result == ISC_R_SUCCESS, "invalid %s%s",
 			  "address match list", "");
 	}
@@ -3714,8 +3715,8 @@ create_mapped_acl(void) {
 
 isc_result_t
 named_register_one_plugin(const cfg_obj_t *config, const cfg_obj_t *obj,
-			  const char *plugin_path, const char *parameters,
-			  void *callback_data) {
+			  cfg_aclconfctx_t *actx, const char *plugin_path,
+			  const char *parameters, void *callback_data) {
 	char full_path[PATH_MAX];
 	isc_result_t result;
 	ns_hook_data_t *hookdata = callback_data;
@@ -3733,7 +3734,7 @@ named_register_one_plugin(const cfg_obj_t *config, const cfg_obj_t *obj,
 
 	result = ns_plugin_register(full_path, parameters, config,
 				    cfg_obj_file(obj), cfg_obj_line(obj),
-				    isc_g_mctx, named_g_aclconfctx, hookdata);
+				    isc_g_mctx, actx, hookdata);
 	if (result != ISC_R_SUCCESS) {
 		isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER,
 			      ISC_LOG_ERROR,
@@ -5430,7 +5431,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config,
 		view->plugins = hookdata.plugins;
 		view->plugins_free = ns_plugins_free;
 
-		CHECK(cfg_pluginlist_foreach(config, plugin_list,
+		CHECK(cfg_pluginlist_foreach(config, plugin_list, actx,
 					     named_register_one_plugin,
 					     &hookdata));
 	}
@@ -5687,7 +5688,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config,
 	obj = NULL;
 	result = named_config_get(maps, "rate-limit", &obj);
 	if (result == ISC_R_SUCCESS) {
-		result = configure_rrl(view, config, obj);
+		result = configure_rrl(view, config, obj, actx);
 		if (result != ISC_R_SUCCESS) {
 			goto cleanup;
 		}
@@ -6255,7 +6256,7 @@ static isc_result_t
 configure_zone(const cfg_obj_t *config, const cfg_obj_t *zconfig,
 	       const cfg_obj_t *vconfig, dns_view_t *view,
 	       dns_viewlist_t *viewlist, dns_kasplist_t *kasplist,
-	       cfg_aclconfctx_t *aclconf, bool added, bool old_rpz_ok,
+	       cfg_aclconfctx_t *actx, bool added, bool old_rpz_ok,
 	       bool is_catz_member, bool modify) {
 	dns_view_t *pview = NULL; /* Production view */
 	dns_zone_t *zone = NULL;  /* New or reused zone */
@@ -6455,7 +6456,7 @@ configure_zone(const cfg_obj_t *config, const cfg_obj_t *zconfig,
 						     zone));
 			dns_zone_setstats(zone, named_g_server->zonestats);
 		}
-		CHECK(named_zone_configure(config, vconfig, zconfig, aclconf,
+		CHECK(named_zone_configure(config, vconfig, zconfig, actx,
 					   kasplist, zone, NULL));
 		dns_zone_attach(zone, &view->redirect);
 		goto cleanup;
@@ -6631,7 +6632,7 @@ configure_zone(const cfg_obj_t *config, const cfg_obj_t *zconfig,
 	/*
 	 * Configure the zone.
 	 */
-	CHECK(named_zone_configure(config, vconfig, zconfig, aclconf, kasplist,
+	CHECK(named_zone_configure(config, vconfig, zconfig, actx, kasplist,
 				   zone, raw));
 
 	/*
@@ -6662,7 +6663,7 @@ configure_zone(const cfg_obj_t *config, const cfg_obj_t *zconfig,
 		dns_zone_rekey(zone, fullsign, false);
 	}
 
-	result = named_zone_loadplugins(zone, config, toptions, zoptions);
+	result = named_zone_loadplugins(zone, config, toptions, zoptions, actx);
 
 cleanup:
 	if (zone != NULL) {
@@ -8126,6 +8127,7 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config,
 	bool exclusive = true;
 	dns_aclenv_t *env =
 		ns_interfacemgr_getaclenv(named_g_server->interfacemgr);
+	cfg_aclconfctx_t *tmpaclconfctx, *aclconfctx = NULL;
 
 	isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER,
 		      ISC_LOG_DEBUG(1), "apply_configuration");
@@ -8153,18 +8155,15 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config,
 	maps[i++] = named_g_defaultoptions;
 	maps[i] = NULL;
 
-	/* Ensure exclusive access to configuration data. */
-	isc_loopmgr_pause();
-
 	/* Create the ACL configuration context */
-	if (named_g_aclconfctx != NULL) {
-		cfg_aclconfctx_detach(&named_g_aclconfctx);
-	}
-	result = cfg_aclconfctx_create(isc_g_mctx, &named_g_aclconfctx);
+	result = cfg_aclconfctx_create(isc_g_mctx, &aclconfctx);
 	if (result != ISC_R_SUCCESS) {
-		goto cleanup_exclusive;
+		goto cleanup_aclconfctx;
 	}
 
+	/* Ensure exclusive access to configuration data. */
+	isc_loopmgr_pause();
+
 	/*
 	 * Shut down all dyndb instances.
 	 */
@@ -8282,7 +8281,7 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config,
 		char *dir = UNCONST(cfg_obj_asstring(obj));
 		named_geoip_load(dir);
 	}
-	named_g_aclconfctx->geoip = named_g_geoip;
+	aclconfctx->geoip = named_g_geoip;
 #endif /* HAVE_GEOIP2 */
 
 	/*
@@ -8320,7 +8319,7 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config,
 	result = named_config_get(maps, "sig0checks-quota-exempt", &obj);
 	if (result == ISC_R_SUCCESS) {
 		result = cfg_acl_fromconfig(
-			obj, config, named_g_aclconfctx, isc_g_mctx, 0,
+			obj, config, aclconfctx, isc_g_mctx, 0,
 			&server->sctx->sig0checksquota_exempt);
 		INSIST(result == ISC_R_SUCCESS);
 	}
@@ -8330,7 +8329,7 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config,
 	 * no default.
 	 */
 	result = configure_view_acl(NULL, config, NULL, "blackhole", NULL,
-				    named_g_aclconfctx, isc_g_mctx,
+				    aclconfctx, isc_g_mctx,
 				    &server->sctx->blackholeacl);
 	if (result != ISC_R_SUCCESS) {
 		goto cleanup_bindkeys_parser;
@@ -8632,8 +8631,8 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config,
 			goto cleanup_portsets;
 		}
 		result = listenlist_fromconfig(
-			clistenon, config, named_g_aclconfctx, isc_g_mctx,
-			AF_INET, server->tlsctx_server_cache, &listenon);
+			clistenon, config, aclconfctx, isc_g_mctx, AF_INET,
+			server->tlsctx_server_cache, &listenon);
 		if (result != ISC_R_SUCCESS) {
 			goto cleanup_portsets;
 		}
@@ -8656,8 +8655,8 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config,
 			goto cleanup_portsets;
 		}
 		result = listenlist_fromconfig(
-			clistenon, config, named_g_aclconfctx, isc_g_mctx,
-			AF_INET6, server->tlsctx_server_cache, &listenon);
+			clistenon, config, aclconfctx, isc_g_mctx, AF_INET6,
+			server->tlsctx_server_cache, &listenon);
 		if (result != ISC_R_SUCCESS) {
 			goto cleanup_portsets;
 		}
@@ -8782,15 +8781,13 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config,
 		goto cleanup_kasplist;
 	}
 
-	result = create_views(config, configparser, named_g_aclconfctx,
-			      &viewlist);
+	result = create_views(config, configparser, aclconfctx, &viewlist);
 	if (result != ISC_R_SUCCESS) {
 		goto cleanup_viewlist;
 	}
 
-	result = configure_views(config, bindkeys, named_g_aclconfctx,
-				 &viewlist, &cachelist, &kasplist, server,
-				 first_time);
+	result = configure_views(config, bindkeys, aclconfctx, &viewlist,
+				 &cachelist, &kasplist, server, first_time);
 	if (result != ISC_R_SUCCESS) {
 		goto cleanup_cachelist;
 	}
@@ -9183,6 +9180,13 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config,
 	server->kasplist = kasplist;
 	kasplist = tmpkasplist;
 
+	/*
+	 * Swap server aclconfctx
+	 */
+	tmpaclconfctx = server->aclconfctx;
+	server->aclconfctx = aclconfctx;
+	aclconfctx = tmpaclconfctx;
+
 	(void)named_server_loadnta(server);
 
 	/*
@@ -9200,7 +9204,7 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config,
 
 	/* Configure the statistics channel(s) */
 	result = named_statschannels_configure(named_g_server, config,
-					       named_g_aclconfctx);
+					       server->aclconfctx);
 	if (result != ISC_R_SUCCESS) {
 		isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER,
 			      ISC_LOG_ERROR,
@@ -9213,7 +9217,7 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config,
 	 * Bind the control port(s).
 	 */
 	result = named_controls_configure(named_g_server->controls, config,
-					  named_g_aclconfctx);
+					  server->aclconfctx);
 	if (result != ISC_R_SUCCESS) {
 		isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER,
 			      ISC_LOG_ERROR, "binding control channel(s): %s",
@@ -9221,6 +9225,7 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config,
 		goto cleanup_altsecrets;
 	}
 
+
 	(void)ns_interfacemgr_scan(server->interfacemgr, true, true);
 
 	/*
@@ -9288,11 +9293,15 @@ cleanup_bindkeys_parser:
 		cfg_parser_destroy(&bindkeys_parser);
 	}
 
-cleanup_exclusive:
 	if (exclusive) {
 		isc_loopmgr_resume();
 	}
 
+cleanup_aclconfctx:
+	if (aclconfctx != NULL) {
+		cfg_aclconfctx_detach(&aclconfctx);
+	}
+
 	isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER,
 		      ISC_LOG_DEBUG(1), "apply_configuration: %s",
 		      isc_result_totext(result));
@@ -9562,8 +9571,8 @@ shutdown_server(void *arg) {
 
 	cleanup_session_key(server, server->mctx);
 
-	if (named_g_aclconfctx != NULL) {
-		cfg_aclconfctx_detach(&named_g_aclconfctx);
+	if (server->aclconfctx != NULL) {
+		cfg_aclconfctx_detach(&server->aclconfctx);
 	}
 
 	cfg_obj_destroy(named_g_parser, &named_g_defaultconfig);
diff --git a/bin/named/zoneconf.c b/bin/named/zoneconf.c
index e4a4cf4b429..592c65a0b7d 100644
--- a/bin/named/zoneconf.c
+++ b/bin/named/zoneconf.c
@@ -2101,7 +2101,8 @@ named_zone_templateopts(const cfg_obj_t *config, const cfg_obj_t *zoptions) {
 
 isc_result_t
 named_zone_loadplugins(dns_zone_t *zone, const cfg_obj_t *config,
-		       const cfg_obj_t *toptions, const cfg_obj_t *zoptions) {
+		       const cfg_obj_t *toptions, const cfg_obj_t *zoptions,
+		       cfg_aclconfctx_t *actx) {
 	isc_result_t result = ISC_R_SUCCESS;
 	const cfg_obj_t *zpluginlist = NULL;
 	const cfg_obj_t *tpluginlist = NULL;
@@ -2135,14 +2136,14 @@ named_zone_loadplugins(dns_zone_t *zone, const cfg_obj_t *config,
 		ns_plugins_create(zmctx, &hookdata.plugins);
 		dns_zone_setplugins(zone, hookdata.plugins, ns_plugins_free);
 
-		result = cfg_pluginlist_foreach(config, tpluginlist,
+		result = cfg_pluginlist_foreach(config, tpluginlist, actx,
 						named_register_one_plugin,
 						&hookdata);
 		if (result != ISC_R_SUCCESS) {
 			return result;
 		}
 
-		result = cfg_pluginlist_foreach(config, zpluginlist,
+		result = cfg_pluginlist_foreach(config, zpluginlist, actx,
 						named_register_one_plugin,
 						&hookdata);
 	}
diff --git a/lib/isccfg/check.c b/lib/isccfg/check.c
index 1d06e1e303d..3d03d1e10cc 100644
--- a/lib/isccfg/check.c
+++ b/lib/isccfg/check.c
@@ -2940,12 +2940,14 @@ struct check_one_plugin_data {
  */
 static isc_result_t
 check_one_plugin(const cfg_obj_t *config, const cfg_obj_t *obj,
-		 const char *plugin_path, const char *parameters,
-		 void *callback_data) {
+		 cfg_aclconfctx_t *actx, const char *plugin_path,
+		 const char *parameters, void *callback_data) {
 	struct check_one_plugin_data *data = callback_data;
 	char full_path[PATH_MAX];
 	isc_result_t result = ISC_R_SUCCESS;
 
+	UNUSED(actx);
+
 	result = ns_plugin_expandpath(plugin_path, full_path,
 				      sizeof(full_path));
 	if (result != ISC_R_SUCCESS) {
@@ -2978,7 +2980,7 @@ check_plugins(const cfg_obj_t *plugins, const cfg_obj_t *config,
 		.check_result = &result,
 	};
 
-	(void)cfg_pluginlist_foreach(config, plugins, check_one_plugin,
+	(void)cfg_pluginlist_foreach(config, plugins, actx, check_one_plugin,
 				     &check_one_plugin_data);
 
 	return result;
diff --git a/lib/isccfg/include/isccfg/cfg.h b/lib/isccfg/include/isccfg/cfg.h
index 260654073b2..ba49b1c5b74 100644
--- a/lib/isccfg/include/isccfg/cfg.h
+++ b/lib/isccfg/include/isccfg/cfg.h
@@ -39,6 +39,8 @@
  *** Types
  ***/
 
+typedef struct cfg_aclconfctx cfg_aclconfctx_t;
+
 /*%
  * A configuration parser.
  */
@@ -586,11 +588,9 @@ const char *
 cfg_map_nextclause(const cfg_type_t *map, const void **clauses,
 		   unsigned int *idx);
 
-typedef isc_result_t(pluginlist_cb_t)(const cfg_obj_t *config,
-				      const cfg_obj_t *obj,
-				      const char      *plugin_path,
-				      const char      *parameters,
-				      void	      *callback_data);
+typedef isc_result_t(pluginlist_cb_t)(
+	const cfg_obj_t *config, const cfg_obj_t *obj, cfg_aclconfctx_t *actx,
+	const char *plugin_path, const char *parameters, void *callback_data);
 /*%<
  * Function prototype for the callback used with cfg_pluginlist_foreach().
  * Called once for each element of the list passed to cfg_pluginlist_foreach().
@@ -606,7 +606,8 @@ typedef isc_result_t(pluginlist_cb_t)(const cfg_obj_t *config,
 
 isc_result_t
 cfg_pluginlist_foreach(const cfg_obj_t *config, const cfg_obj_t *list,
-		       pluginlist_cb_t *callback, void *callback_data);
+		       cfg_aclconfctx_t *actx, pluginlist_cb_t *callback,
+		       void *callback_data);
 /*%<
  * For every "plugin" stanza present in 'list' (which in turn is a part of
  * 'config'), invoke the given 'callback', passing 'callback_data' to it along
diff --git a/lib/isccfg/parser.c b/lib/isccfg/parser.c
index 46a8012dbb5..c8e411f8def 100644
--- a/lib/isccfg/parser.c
+++ b/lib/isccfg/parser.c
@@ -3945,7 +3945,8 @@ cleanup:
 
 isc_result_t
 cfg_pluginlist_foreach(const cfg_obj_t *config, const cfg_obj_t *list,
-		       pluginlist_cb_t *callback, void *callback_data) {
+		       cfg_aclconfctx_t *actx, pluginlist_cb_t *callback,
+		       void *callback_data) {
 	isc_result_t result = ISC_R_SUCCESS;
 
 	REQUIRE(config != NULL);
@@ -3975,7 +3976,7 @@ cfg_pluginlist_foreach(const cfg_obj_t *config, const cfg_obj_t *list,
 			parameters = cfg_obj_asstring(obj);
 		}
 
-		result = callback(config, obj, library, parameters,
+		result = callback(config, obj, actx, library, parameters,
 				  callback_data);
 		if (result != ISC_R_SUCCESS) {
 			break;