fix: dev: 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,
the checking was never actually done: the `plugin_register()`
implementation was called, but `plugin_check()` was not.

`ns_plugin_register()` (used by `named`) now calls the check function
before the register function, and aborts if either one fails.
`ns_plugin_check()` (used by `named-checkconf`) calls only
the check function.

Merge branch 'each-check-plugin-named' into 'main'

See merge request isc-projects/bind9!11031
This commit is contained in:
Colin Vidal 2025-10-01 11:10:40 +02:00
commit 0e575d150f
7 changed files with 28 additions and 17 deletions

View file

@ -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, &param_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;

View file

@ -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, &param_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;

View file

@ -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;
}

View file

@ -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;
}

View file

@ -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;
}

View file

@ -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) {

View file

@ -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