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.
This commit is contained in:
Evan Hunt 2025-09-26 20:57:52 -07:00
parent 7fa4cbedc5
commit 92cefc52bc
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