From 25e258fb0bd812f093850995118a4ca2e825c96b Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Fri, 26 Sep 2025 15:54:51 +0200 Subject: [PATCH 1/2] provide a context structure for plugin_register() This commit introduces a new type, ns_pluginregister_ctx_t, which is passed to plugin_check() and plugin_register() in place of the 'source' parameter. The source value is now just part of the structure, which also holds a pointer to the zone origin if the plugin is loaded at a zone level. This provides more contextual information, enabling the plugin to make specific configuration decisions based on the name of the zone for which it is loaded. It's also flexible if more contextual data are needed in the future: add a new field to ns_pluginregister_ctx_t, and new plugins can use it without affecting compatibility with existing plugins. --- bin/named/server.c | 4 +- bin/named/zoneconf.c | 6 +- bin/plugins/filter-a.c | 6 +- bin/plugins/filter-aaaa.c | 6 +- bin/tests/system/hooks/driver/test-async.c | 8 +-- .../system/hooks/driver/test-syncplugin.c | 55 +++++++++++++++++-- lib/isccfg/check.c | 19 ++++--- lib/ns/hooks.c | 11 ++-- lib/ns/include/ns/hooks.h | 34 ++++++++---- 9 files changed, 109 insertions(+), 40 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index c34fd28a8b..6dedd6a6e0 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -5421,7 +5421,9 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, } if (plugin_list != NULL) { - ns_hook_data_t hookdata = { .source = NS_HOOKSOURCE_VIEW }; + ns_hook_data_t hookdata = { + .pluginregister_ctx = { .source = NS_HOOKSOURCE_VIEW } + }; INSIST(view->hooktable == NULL); ns_hooktable_create(view->mctx, &hookdata.hooktable); diff --git a/bin/named/zoneconf.c b/bin/named/zoneconf.c index 6c58acfbaf..5bf24c1455 100644 --- a/bin/named/zoneconf.c +++ b/bin/named/zoneconf.c @@ -2129,7 +2129,11 @@ named_zone_loadplugins(dns_zone_t *zone, const cfg_obj_t *config, } if (tpluginlist != NULL || zpluginlist != NULL) { - ns_hook_data_t hookdata = { .source = NS_HOOKSOURCE_ZONE }; + ns_hook_data_t hookdata = { + .pluginregister_ctx = { .source = NS_HOOKSOURCE_ZONE, + .origin = dns_zone_getorigin( + zone) } + }; isc_mem_t *zmctx = dns_zone_getmctx(zone); ns_hooktable_create(zmctx, &hookdata.hooktable); diff --git a/bin/plugins/filter-a.c b/bin/plugins/filter-a.c index 74209132d9..bd85a3b008 100644 --- a/bin/plugins/filter-a.c +++ b/bin/plugins/filter-a.c @@ -324,12 +324,12 @@ cleanup: isc_result_t plugin_register(const char *parameters, const void *cfg, const char *cfg_file, unsigned long cfg_line, isc_mem_t *mctx, void *aclctx, - ns_hooktable_t *hooktable, ns_hooksource_t source, + ns_hooktable_t *hooktable, const ns_pluginregister_ctx_t *ctx, void **instp) { filter_instance_t *inst = NULL; isc_result_t result = ISC_R_SUCCESS; - UNUSED(source); + UNUSED(ctx); isc_log_write(NS_LOGCATEGORY_GENERAL, NS_LOGMODULE_HOOKS, ISC_LOG_INFO, "registering 'filter-a' " @@ -366,7 +366,7 @@ 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, - ns_hooksource_t source ISC_ATTR_UNUSED) { + const ns_pluginregister_ctx_t *ctx 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 bf58f557b9..80403455a0 100644 --- a/bin/plugins/filter-aaaa.c +++ b/bin/plugins/filter-aaaa.c @@ -327,12 +327,12 @@ cleanup: isc_result_t plugin_register(const char *parameters, const void *cfg, const char *cfg_file, unsigned long cfg_line, isc_mem_t *mctx, void *aclctx, - ns_hooktable_t *hooktable, ns_hooksource_t source, + ns_hooktable_t *hooktable, const ns_pluginregister_ctx_t *ctx, void **instp) { filter_instance_t *inst = NULL; isc_result_t result = ISC_R_SUCCESS; - UNUSED(source); + UNUSED(ctx); isc_log_write(NS_LOGCATEGORY_GENERAL, NS_LOGMODULE_HOOKS, ISC_LOG_INFO, "registering 'filter-aaaa' " @@ -370,7 +370,7 @@ 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, - ns_hooksource_t source ISC_ATTR_UNUSED) { + const ns_pluginregister_ctx_t *ctx 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 1d86de1e9c..57197a76ae 100644 --- a/bin/tests/system/hooks/driver/test-async.c +++ b/bin/tests/system/hooks/driver/test-async.c @@ -125,14 +125,14 @@ logmsg(const char *fmt, ...) { isc_result_t plugin_register(const char *parameters, const void *cfg, const char *cfg_file, unsigned long cfg_line, isc_mem_t *mctx, void *aclctx, - ns_hooktable_t *hooktable, ns_hooksource_t source, + ns_hooktable_t *hooktable, const ns_pluginregister_ctx_t *ctx, void **instp) { async_instance_t *inst = NULL; - UNUSED(source); UNUSED(parameters); UNUSED(cfg); UNUSED(aclctx); + UNUSED(ctx); isc_log_write(NS_LOGCATEGORY_GENERAL, NS_LOGMODULE_HOOKS, ISC_LOG_INFO, "registering 'test-async' module from %s:%lu", cfg_file, @@ -158,14 +158,14 @@ 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, - ns_hooksource_t source) { + const ns_pluginregister_ctx_t *ctx) { UNUSED(parameters); UNUSED(cfg); UNUSED(cfg_file); UNUSED(cfg_line); UNUSED(mctx); UNUSED(aclctx); - UNUSED(source); + UNUSED(ctx); 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 59f163add4..89009161f8 100644 --- a/bin/tests/system/hooks/driver/test-syncplugin.c +++ b/bin/tests/system/hooks/driver/test-syncplugin.c @@ -113,7 +113,7 @@ syncplugin__parse_rcode(const cfg_obj_t *syncplugincfg, uint8_t *rcode) { isc_result_t plugin_register(const char *parameters, const void *cfg, const char *cfgfile, unsigned long cfgline, isc_mem_t *mctx, void *aclctx, - ns_hooktable_t *hooktable, ns_hooksource_t source, + ns_hooktable_t *hooktable, const ns_pluginregister_ctx_t *ctx, void **instp) { isc_result_t result; cfg_parser_t *parser = NULL; @@ -123,10 +123,13 @@ plugin_register(const char *parameters, const void *cfg, const char *cfgfile, ns_hook_t hook; syncplugin_t *inst = NULL; char *sourcestr = NULL; + dns_name_t example2com; + dns_name_t example3com; + dns_name_t example4com; UNUSED(cfg); UNUSED(aclctx); - UNUSED(source); + UNUSED(ctx); inst = isc_mem_get(mctx, sizeof(*inst)); *inst = (syncplugin_t){ .mctx = mctx }; @@ -155,12 +158,40 @@ plugin_register(const char *parameters, const void *cfg, const char *cfgfile, sourcestr = obj->value.string.base; if (strcmp(sourcestr, "zone") == 0) { - if (source != NS_HOOKSOURCE_ZONE) { + if (ctx->source != NS_HOOKSOURCE_ZONE) { result = ISC_R_FAILURE; goto cleanup; } + if (ctx->origin == NULL) { + result = ISC_R_FAILURE; + goto cleanup; + } + + dns_name_init(&example2com); + dns_name_init(&example3com); + dns_name_init(&example4com); + + result = dns_name_fromstring(&example2com, "example2.com.", + NULL, 0, isc_g_mctx); + result = dns_name_fromstring(&example3com, "example3.com.", + NULL, 0, isc_g_mctx); + result = dns_name_fromstring(&example4com, "example4.com.", + NULL, 0, isc_g_mctx); + + if (!dns_name_equal(ctx->origin, &example2com) && + !dns_name_equal(ctx->origin, &example3com) && + !dns_name_equal(ctx->origin, &example4com)) + { + result = ISC_R_FAILURE; + goto cleanup; + } + } else if (strcmp(sourcestr, "view") == 0) { - if (source != NS_HOOKSOURCE_VIEW) { + if (ctx->source != NS_HOOKSOURCE_VIEW) { + result = ISC_R_FAILURE; + goto cleanup; + } + if (ctx->origin != NULL) { result = ISC_R_FAILURE; goto cleanup; } @@ -173,6 +204,18 @@ plugin_register(const char *parameters, const void *cfg, const char *cfgfile, ns_hook_add(hooktable, mctx, NS_QUERY_NXDOMAIN_BEGIN, &hook); cleanup: + if (DNS_NAME_VALID(&example2com)) { + dns_name_free(&example2com, isc_g_mctx); + } + + if (DNS_NAME_VALID(&example3com)) { + dns_name_free(&example3com, isc_g_mctx); + } + + if (DNS_NAME_VALID(&example4com)) { + dns_name_free(&example4com, isc_g_mctx); + } + if (syncplugincfg != NULL) { cfg_obj_destroy(parser, &syncplugincfg); } @@ -187,14 +230,14 @@ cleanup: isc_result_t plugin_check(const char *parameters, const void *cfg, const char *cfgfile, unsigned long cfgline, isc_mem_t *mctx, void *aclctx, - ns_hooksource_t source) { + const ns_pluginregister_ctx_t *ctx) { UNUSED(parameters); UNUSED(cfg); UNUSED(cfgfile); UNUSED(cfgline); UNUSED(mctx); UNUSED(aclctx); - UNUSED(source); + UNUSED(ctx); return ISC_R_SUCCESS; } diff --git a/lib/isccfg/check.c b/lib/isccfg/check.c index bd2bfb4740..c60315d4a2 100644 --- a/lib/isccfg/check.c +++ b/lib/isccfg/check.c @@ -3063,6 +3063,7 @@ struct check_one_plugin_data { cfg_aclconfctx_t *aclctx; ns_hooksource_t source; isc_result_t *check_result; + const ns_pluginregister_ctx_t *ctx; }; /*% @@ -3094,7 +3095,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->source); + data->mctx, data->aclctx, data->ctx); if (result != ISC_R_SUCCESS) { cfg_obj_log(obj, ISC_LOG_ERROR, "%s: plugin check failed: %s", full_path, isc_result_totext(result)); @@ -3106,14 +3107,20 @@ 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, ns_hooksource_t source, + cfg_aclconfctx_t *aclctx, const dns_name_t *zname, isc_mem_t *mctx) { isc_result_t result = ISC_R_SUCCESS; + ns_pluginregister_ctx_t ctx = { + .source = (zname == NULL) ? NS_HOOKSOURCE_VIEW + : NS_HOOKSOURCE_ZONE, + .origin = zname, + }; struct check_one_plugin_data check_one_plugin_data = { .mctx = mctx, .aclctx = aclctx, - .source = source, + .source = ctx.source, .check_result = &result, + .ctx = &ctx, }; (void)cfg_pluginlist_foreach(config, plugins, aclctx, check_one_plugin, @@ -4132,8 +4139,7 @@ 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, - NS_HOOKSOURCE_ZONE, mctx); + tresult = check_plugins(plugins, config, aclctx, zname, mctx); if (tresult != ISC_R_SUCCESS) { result = tresult; } @@ -5745,8 +5751,7 @@ 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, - NS_HOOKSOURCE_VIEW, mctx); + tresult = check_plugins(plugins, config, aclctx, NULL, mctx); if (tresult != ISC_R_SUCCESS) { result = tresult; } diff --git a/lib/ns/hooks.c b/lib/ns/hooks.c index c8fc2c34f8..2b4fd9e031 100644 --- a/lib/ns/hooks.c +++ b/lib/ns/hooks.c @@ -239,13 +239,14 @@ ns_plugin_register(const char *modpath, const char *parameters, const void *cfg, isc_log_write(NS_LOGCATEGORY_GENERAL, NS_LOGMODULE_HOOKS, ISC_LOG_INFO, "registering plugin '%s'", modpath); - INSIST(hookdata->source != NS_HOOKSOURCE_UNDEFINED); + INSIST(hookdata->pluginregister_ctx.source != NS_HOOKSOURCE_UNDEFINED); CHECK(plugin->check_func(parameters, cfg, cfg_file, cfg_line, mctx, - aclctx, hookdata->source)); + aclctx, &hookdata->pluginregister_ctx)); CHECK(plugin->register_func(parameters, cfg, cfg_file, cfg_line, mctx, aclctx, hookdata->hooktable, - hookdata->source, &plugin->inst)); + &hookdata->pluginregister_ctx, + &plugin->inst)); ISC_LIST_APPEND(*hookdata->plugins, plugin, link); @@ -260,14 +261,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, ns_hooksource_t source) { + void *aclctx, const ns_pluginregister_ctx_t *ctx) { 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, source); + aclctx, ctx); cleanup: if (plugin != NULL) { diff --git a/lib/ns/include/ns/hooks.h b/lib/ns/include/ns/hooks.h index 7bf6eaccec..aac396ccaa 100644 --- a/lib/ns/include/ns/hooks.h +++ b/lib/ns/include/ns/hooks.h @@ -369,12 +369,18 @@ * instance, bad cookie handling), it would be skipped. * * The `plugin_register` function (defined by each plugin and called - * when the plugin is loaded) has a `ns_hooksource_t source` parameter. - * It indicates whether the plugin has been loaded at the zone level + * when the plugin is loaded) has a `ns_pluginregister_ctx_t ctx` parameter. + * This provides to the plugin registering function various contextual + * informations about the plugin. For instance, the `ns_hooksource_t source` + * property indicates whether the plugin has been loaded at the zone level * (`NS_HOOKSOURCE_ZONE`) or at the view level (`NS_HOOKSOURCE_VIEW`). * While this can be ignored if it doesn't matter where the plugin is * loaded, it can also be checked to enforce that the plugin is loaded - * only at the zone or view level. + * only at the zone or view level. Another property is `dns_name_t *origin` + * which indicates the zone name to the plugin if it is loaded at the zone level + * (this property is NULL otherwise). Note that `ns_pluginregister_ctx_t` + * parameter is defined in a parent stack frame, thus, it is valid only during + * `plugin_register` execution. */ /*! @@ -490,10 +496,18 @@ typedef enum { NS_HOOKSOURCE_ZONE } ns_hooksource_t; -typedef struct ns_hook_data { - ns_hooktable_t *hooktable; - ns_plugins_t *plugins; +typedef struct ns_pluginregister_ctx { + /* is this a zone or a view plugin */ ns_hooksource_t source; + + /* origin of the zone if this is a zone plugin, NULL otherwise */ + const dns_name_t *origin; +} ns_pluginregister_ctx_t; + +typedef struct ns_hook_data { + ns_hooktable_t *hooktable; + ns_plugins_t *plugins; + ns_pluginregister_ctx_t pluginregister_ctx; } ns_hook_data_t; /* @@ -512,8 +526,8 @@ typedef struct ns_hook_data { typedef isc_result_t ns_plugin_register_t(const char *parameters, const void *cfg, const char *file, unsigned long line, isc_mem_t *mctx, void *aclctx, - ns_hooktable_t *hooktable, ns_hooksource_t source, - void **instp); + ns_hooktable_t *hooktable, + const ns_pluginregister_ctx_t *ctx, void **instp); /*%< * Called when registering a new plugin. * @@ -538,7 +552,7 @@ 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, - ns_hooksource_t source); + const ns_pluginregister_ctx_t *ctx); /*%< * Check the validity of 'parameters'. */ @@ -605,7 +619,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, ns_hooksource_t source); + void *aclctx, const ns_pluginregister_ctx_t *ctx); /*%< * Open the plugin module at 'modpath' and check the validity of * 'parameters', logging any errors or warnings found, then From b6758cf695b04294f11819c277b6295c14038e9b Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Sun, 28 Sep 2025 23:07:11 +0200 Subject: [PATCH 2/2] unload zone plugin before freeing the zone Make sure all zone's plugin are unloaded before the zone gets freed. This makes passing zone metadata like its origin to the plugin registering function safe, as this garantee that the origin would always be valid from the plugin lifecycle. --- lib/dns/zone.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 901336780a..f918eee208 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -1237,6 +1237,8 @@ zone_free(dns_zone_t *zone) { REQUIRE(zone->timer == NULL); REQUIRE(zone->zmgr == NULL); + dns_zone_unloadplugins(zone); + isc_refcount_destroy(&zone->references); isc_refcount_destroy(&zone->irefs); @@ -1385,7 +1387,6 @@ zone_free(dns_zone_t *zone) { if (zone->gluecachestats != NULL) { isc_stats_detach(&zone->gluecachestats); } - dns_zone_unloadplugins(zone); /* last stuff */ ZONEDB_DESTROYLOCK(&zone->dblock);