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 77da039d7113b6e117f22158c12ebbdb69bd3b93..74209132d9c2a2eed35673036c82da608c23de76 100644 (file)
--- 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, &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;

diff --git a/bin/plugins/filter-aaaa.c b/bin/plugins/filter-aaaa.c
index 6d70b4d75b81abb1dff32fea26f586bc4185753b..bf58f557b957fb8d58d0b2652e55dbf114d74adf 100644 (file)
--- 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, &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;

diff --git a/bin/tests/system/hooks/driver/test-async.c b/bin/tests/system/hooks/driver/test-async.c
index 7872e7df7fb9111964c8ae019b134181033691e8..1d86de1e9ceea71256042f1a9f731a6a9bcee164 100644 (file)
--- 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 c087f0c36bedf37039202ff9446c39c98cb692a8..59f163add492b601abd43e94f706412d18ae2bc0 100644 (file)
--- 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 18ea746dae8a45616c22c3a5beb1bf188631b9ef..bd2bfb4740cb2369ced54cef8508bd2a215d80fa 100644 (file)
--- 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 ea01abaae5a6a206d2e85efc25324889af06b764..c8fc2c34f8cffe1ea84e5684b7d79ea29c1a3ac2 100644 (file)
--- 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 f02641b87248174ec47e09193225eb0cc2675844..7bf6eaccec0b813eb074f6d1a64fbecfe289f800 100644 (file)
--- 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