From: Evan Hunt Date: Sat, 27 Sep 2025 03:57:52 +0000 (-0700) Subject: check plugin config before registering X-Git-Tag: v9.21.14~15^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=92cefc52bc3942503a50e0b0f176ba68bd7f5cfb;p=thirdparty%2Fbind9.git check plugin config before registering In named_config_parsefile(), when checking the validity of named.conf, the checking of plugin correctness was deliberately postponed until the plugin is loaded and registered. However, when the plugin was registered, the checking was never actually done: the plugin_register() implementation was called, but plugin_check() was not. This made it necessary to duplicate the correctness checking in both functions, so that both named-checkconf and named could catch errors. That should not be required. ns_plugin_register() now calls the check function before the register function, and aborts if either one fails. ns_plugin_check() calls only the check function. ns_plugin_check() is used by named-checkconf, and ns_plugin_register() is used by named. (Note: this design has a side effect that a call to ns_plugin_register() will result in the plugin parameters being parsed twice at registration time.) ns_plugin_check() now takes an additional argument for the hook source: zone or view. --- diff --git a/bin/plugins/filter-a.c b/bin/plugins/filter-a.c index 77da039d711..74209132d9c 100644 --- a/bin/plugins/filter-a.c +++ b/bin/plugins/filter-a.c @@ -286,8 +286,6 @@ parse_parameters(filter_instance_t *inst, const char *parameters, CHECK(cfg_parse_buffer(parser, &b, cfg_file, cfg_line, &cfg_type_parameters, 0, ¶m_obj)); - CHECK(check_syntax(param_obj, cfg, mctx, aclctx)); - CHECK(parse_filter_a_on(param_obj, "filter-a-on-v6", &inst->v6_a)); CHECK(parse_filter_a_on(param_obj, "filter-a-on-v4", &inst->v4_a)); @@ -367,7 +365,8 @@ cleanup: isc_result_t plugin_check(const char *parameters, const void *cfg, const char *cfg_file, - unsigned long cfg_line, isc_mem_t *mctx, void *aclctx) { + unsigned long cfg_line, isc_mem_t *mctx, void *aclctx, + ns_hooksource_t source ISC_ATTR_UNUSED) { isc_result_t result = ISC_R_SUCCESS; cfg_parser_t *parser = NULL; cfg_obj_t *param_obj = NULL; diff --git a/bin/plugins/filter-aaaa.c b/bin/plugins/filter-aaaa.c index 6d70b4d75b8..bf58f557b95 100644 --- a/bin/plugins/filter-aaaa.c +++ b/bin/plugins/filter-aaaa.c @@ -287,8 +287,6 @@ parse_parameters(filter_instance_t *inst, const char *parameters, CHECK(cfg_parse_buffer(parser, &b, cfg_file, cfg_line, &cfg_type_parameters, 0, ¶m_obj)); - CHECK(check_syntax(param_obj, cfg, mctx, aclctx)); - CHECK(parse_filter_aaaa_on(param_obj, "filter-aaaa-on-v4", &inst->v4_aaaa)); CHECK(parse_filter_aaaa_on(param_obj, "filter-aaaa-on-v6", @@ -371,7 +369,8 @@ cleanup: isc_result_t plugin_check(const char *parameters, const void *cfg, const char *cfg_file, - unsigned long cfg_line, isc_mem_t *mctx, void *aclctx) { + unsigned long cfg_line, isc_mem_t *mctx, void *aclctx, + ns_hooksource_t source ISC_ATTR_UNUSED) { isc_result_t result = ISC_R_SUCCESS; cfg_parser_t *parser = NULL; cfg_obj_t *param_obj = NULL; diff --git a/bin/tests/system/hooks/driver/test-async.c b/bin/tests/system/hooks/driver/test-async.c index 7872e7df7fb..1d86de1e9ce 100644 --- a/bin/tests/system/hooks/driver/test-async.c +++ b/bin/tests/system/hooks/driver/test-async.c @@ -157,13 +157,15 @@ plugin_register(const char *parameters, const void *cfg, const char *cfg_file, isc_result_t plugin_check(const char *parameters, const void *cfg, const char *cfg_file, - unsigned long cfg_line, isc_mem_t *mctx, void *aclctx) { + unsigned long cfg_line, isc_mem_t *mctx, void *aclctx, + ns_hooksource_t source) { UNUSED(parameters); UNUSED(cfg); UNUSED(cfg_file); UNUSED(cfg_line); UNUSED(mctx); UNUSED(aclctx); + UNUSED(source); return ISC_R_SUCCESS; } diff --git a/bin/tests/system/hooks/driver/test-syncplugin.c b/bin/tests/system/hooks/driver/test-syncplugin.c index c087f0c36be..59f163add49 100644 --- a/bin/tests/system/hooks/driver/test-syncplugin.c +++ b/bin/tests/system/hooks/driver/test-syncplugin.c @@ -186,13 +186,15 @@ cleanup: isc_result_t plugin_check(const char *parameters, const void *cfg, const char *cfgfile, - unsigned long cfgline, isc_mem_t *mctx, void *aclctx) { + unsigned long cfgline, isc_mem_t *mctx, void *aclctx, + ns_hooksource_t source) { UNUSED(parameters); UNUSED(cfg); UNUSED(cfgfile); UNUSED(cfgline); UNUSED(mctx); UNUSED(aclctx); + UNUSED(source); return ISC_R_SUCCESS; } diff --git a/lib/isccfg/check.c b/lib/isccfg/check.c index 18ea746dae8..bd2bfb4740c 100644 --- a/lib/isccfg/check.c +++ b/lib/isccfg/check.c @@ -3061,6 +3061,7 @@ check: struct check_one_plugin_data { isc_mem_t *mctx; cfg_aclconfctx_t *aclctx; + ns_hooksource_t source; isc_result_t *check_result; }; @@ -3093,7 +3094,7 @@ check_one_plugin(const cfg_obj_t *config, const cfg_obj_t *obj, result = ns_plugin_check(full_path, parameters, config, cfg_obj_file(obj), cfg_obj_line(obj), - data->mctx, data->aclctx); + data->mctx, data->aclctx, data->source); if (result != ISC_R_SUCCESS) { cfg_obj_log(obj, ISC_LOG_ERROR, "%s: plugin check failed: %s", full_path, isc_result_totext(result)); @@ -3105,11 +3106,13 @@ check_one_plugin(const cfg_obj_t *config, const cfg_obj_t *obj, static isc_result_t check_plugins(const cfg_obj_t *plugins, const cfg_obj_t *config, - cfg_aclconfctx_t *aclctx, isc_mem_t *mctx) { + cfg_aclconfctx_t *aclctx, ns_hooksource_t source, + isc_mem_t *mctx) { isc_result_t result = ISC_R_SUCCESS; struct check_one_plugin_data check_one_plugin_data = { .mctx = mctx, .aclctx = aclctx, + .source = source, .check_result = &result, }; @@ -4129,7 +4132,8 @@ isccfg_check_zoneconf(const cfg_obj_t *zconfig, const cfg_obj_t *voptions, const cfg_obj_t *plugins = NULL; (void)cfg_map_get(zoptions, "plugin", &plugins); - tresult = check_plugins(plugins, config, aclctx, mctx); + tresult = check_plugins(plugins, config, aclctx, + NS_HOOKSOURCE_ZONE, mctx); if (tresult != ISC_R_SUCCESS) { result = tresult; } @@ -5741,7 +5745,8 @@ check_viewconf(const cfg_obj_t *config, const cfg_obj_t *voptions, (void)cfg_map_get(config, "plugin", &plugins); } - tresult = check_plugins(plugins, config, aclctx, mctx); + tresult = check_plugins(plugins, config, aclctx, + NS_HOOKSOURCE_VIEW, mctx); if (tresult != ISC_R_SUCCESS) { result = tresult; } diff --git a/lib/ns/hooks.c b/lib/ns/hooks.c index ea01abaae5a..c8fc2c34f8c 100644 --- a/lib/ns/hooks.c +++ b/lib/ns/hooks.c @@ -240,6 +240,9 @@ ns_plugin_register(const char *modpath, const char *parameters, const void *cfg, "registering plugin '%s'", modpath); INSIST(hookdata->source != NS_HOOKSOURCE_UNDEFINED); + + CHECK(plugin->check_func(parameters, cfg, cfg_file, cfg_line, mctx, + aclctx, hookdata->source)); CHECK(plugin->register_func(parameters, cfg, cfg_file, cfg_line, mctx, aclctx, hookdata->hooktable, hookdata->source, &plugin->inst)); @@ -257,14 +260,14 @@ cleanup: isc_result_t ns_plugin_check(const char *modpath, const char *parameters, const void *cfg, const char *cfg_file, unsigned long cfg_line, isc_mem_t *mctx, - void *aclctx) { + void *aclctx, ns_hooksource_t source) { isc_result_t result; ns_plugin_t *plugin = NULL; CHECK(load_plugin(mctx, modpath, &plugin)); result = plugin->check_func(parameters, cfg, cfg_file, cfg_line, mctx, - aclctx); + aclctx, source); cleanup: if (plugin != NULL) { diff --git a/lib/ns/include/ns/hooks.h b/lib/ns/include/ns/hooks.h index f02641b8724..7bf6eaccec0 100644 --- a/lib/ns/include/ns/hooks.h +++ b/lib/ns/include/ns/hooks.h @@ -505,7 +505,7 @@ typedef struct ns_hook_data { * as well; if not, set NS_PLUGIN_AGE to 0. */ #ifndef NS_PLUGIN_VERSION -#define NS_PLUGIN_VERSION 2 +#define NS_PLUGIN_VERSION 3 #define NS_PLUGIN_AGE 0 #endif /* ifndef NS_PLUGIN_VERSION */ @@ -537,7 +537,8 @@ ns_plugin_destroy_t(void **instp); typedef isc_result_t ns_plugin_check_t(const char *parameters, const void *cfg, const char *file, - unsigned long line, isc_mem_t *mctx, void *aclctx); + unsigned long line, isc_mem_t *mctx, void *aclctx, + ns_hooksource_t source); /*%< * Check the validity of 'parameters'. */ @@ -604,7 +605,7 @@ ns_plugin_register(const char *modpath, const char *parameters, const void *cfg, isc_result_t ns_plugin_check(const char *modpath, const char *parameters, const void *cfg, const char *cfg_file, unsigned long cfg_line, isc_mem_t *mctx, - void *aclctx); + void *aclctx, ns_hooksource_t source); /*%< * Open the plugin module at 'modpath' and check the validity of * 'parameters', logging any errors or warnings found, then