From 36c74c58c15b55ee169e6c5b4e73d400123fa25e Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Tue, 26 Aug 2025 12:35:56 +0200 Subject: [PATCH 01/10] refactor view creation/config in apply_configuration In order to help splitting apply_configuration, the inline loops and bit of logic around it for views creation and configuration, each of those are now in a dedicatated function. --- bin/named/server.c | 314 ++++++++++++++++++++++++--------------------- 1 file changed, 168 insertions(+), 146 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index ca39a4be5a..704cf3facf 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -7842,12 +7842,170 @@ cleanup: #endif /* HAVE_LMDB */ +static isc_result_t +create_views(cfg_obj_t *config, cfg_parser_t *parser, + cfg_aclconfctx_t *aclconfctx, dns_viewlist_t *viewlist) { + isc_result_t result = ISC_R_SUCCESS; + const cfg_obj_t *views = NULL; + + (void)cfg_map_get(config, "view", &views); + CFG_LIST_FOREACH(views, element) { + cfg_obj_t *vconfig = cfg_listelt_value(element); + dns_view_t *view = NULL; + + result = create_view(vconfig, viewlist, &view); + if (result != ISC_R_SUCCESS) { + return result; + } + INSIST(view != NULL); + + result = setup_newzones(view, config, vconfig, parser, + aclconfctx); + dns_view_detach(&view); + + if (result != ISC_R_SUCCESS) { + return result; + } + } + + /* + * If there were no explicit views then we do the default + * view here. + */ + if (views == NULL) { + dns_view_t *view = NULL; + + result = create_view(NULL, viewlist, &view); + if (result != ISC_R_SUCCESS) { + return result; + } + INSIST(view != NULL); + + result = setup_newzones(view, config, NULL, parser, aclconfctx); + + dns_view_detach(&view); + } + + return result; +} + +static isc_result_t +configure_views(cfg_obj_t *config, const cfg_obj_t *bindkeys, + cfg_aclconfctx_t *aclconfctx, dns_viewlist_t *viewlist, + dns_viewlist_t *builtin_viewlist, named_cachelist_t *cachelist, + named_server_t *server, bool first_time) { + isc_result_t result = ISC_R_SUCCESS; + const cfg_obj_t *views = NULL; + dns_viewlist_t tmpviewlist; + + /* + * Configure and freeze all explicit views. Explicit + * views that have zones were already created at parsing + * time, but views with no zones must be created here. + */ + (void)cfg_map_get(config, "view", &views); + CFG_LIST_FOREACH(views, element) { + cfg_obj_t *vconfig = cfg_listelt_value(element); + dns_view_t *view = NULL; + + result = find_view(vconfig, viewlist, &view); + if (result != ISC_R_SUCCESS) { + return result; + } + + result = configure_view(view, viewlist, config, vconfig, + cachelist, &server->cachelist, + &server->kasplist, bindkeys, isc_g_mctx, + aclconfctx, true, first_time); + if (result != ISC_R_SUCCESS) { + dns_view_detach(&view); + return result; + } + dns_view_freeze(view); + dns_view_detach(&view); + } + + /* + * Make sure we have a default view if and only if there + * were no explicit views. + */ + if (views == NULL) { + dns_view_t *view = NULL; + result = find_view(NULL, viewlist, &view); + if (result != ISC_R_SUCCESS) { + return result; + } + result = configure_view(view, viewlist, config, NULL, cachelist, + &server->cachelist, &server->kasplist, + bindkeys, isc_g_mctx, aclconfctx, true, + first_time); + if (result != ISC_R_SUCCESS) { + dns_view_detach(&view); + return result; + } + dns_view_freeze(view); + dns_view_detach(&view); + } + + /* + * Create (or recreate) the built-in views. + */ + views = NULL; + RUNTIME_CHECK(cfg_map_get(named_g_defaultconfig, "view", &views) == + ISC_R_SUCCESS); + CFG_LIST_FOREACH(views, element) { + cfg_obj_t *vconfig = cfg_listelt_value(element); + dns_view_t *view = NULL; + + result = create_view(vconfig, builtin_viewlist, &view); + if (result != ISC_R_SUCCESS) { + return result; + } + + result = configure_view(view, viewlist, config, vconfig, + cachelist, &server->cachelist, + &server->kasplist, bindkeys, isc_g_mctx, + aclconfctx, false, first_time); + if (result != ISC_R_SUCCESS) { + dns_view_detach(&view); + return result; + } + dns_view_freeze(view); + dns_view_detach(&view); + } + + /* Now combine the two viewlists into one */ + ISC_LIST_APPENDLIST(*viewlist, *builtin_viewlist, link); + + /* + * Commit any dns_zone_setview() calls on all zones in the new + * view. + */ + ISC_LIST_FOREACH(*viewlist, view, link) { + dns_view_setviewcommit(view); + } + + /* + * Save the new view list. The old "production" one will be cleared by + * the caller + */ + tmpviewlist = server->viewlist; + server->viewlist = *viewlist; + *viewlist = tmpviewlist; + + /* Make the view list available to each of the views */ + ISC_LIST_FOREACH(server->viewlist, view, link) { + view->viewlist = &server->viewlist; + } + + return result; +} + static isc_result_t apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, named_server_t *server, bool first_time) { cfg_obj_t *bindkeys = NULL; cfg_parser_t *bindkeys_parser = NULL; - const cfg_obj_t *builtin_views = NULL; const cfg_obj_t *maps[3]; const cfg_obj_t *obj = NULL; const cfg_obj_t *options = NULL; @@ -7856,8 +8014,6 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, dns_kasp_t *default_kasp = NULL; dns_kasplist_t tmpkasplist, kasplist; dns_keystorelist_t tmpkeystorelist, keystorelist; - const cfg_obj_t *views = NULL; - dns_viewlist_t tmpviewlist; dns_viewlist_t viewlist, builtin_viewlist; in_port_t listen_port, udpport_low, udpport_high; int i, backlog; @@ -8609,151 +8765,17 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, server->kasplist = kasplist; kasplist = tmpkasplist; - /* - * Configure the views. - */ - views = NULL; - (void)cfg_map_get(config, "view", &views); - - /* - * Create the views. - */ - CFG_LIST_FOREACH(views, element) { - cfg_obj_t *vconfig = cfg_listelt_value(element); - dns_view_t *view = NULL; - - result = create_view(vconfig, &viewlist, &view); - if (result != ISC_R_SUCCESS) { - goto cleanup_viewlist; - } - INSIST(view != NULL); - - result = setup_newzones(view, config, vconfig, configparser, - named_g_aclconfctx); - dns_view_detach(&view); - - if (result != ISC_R_SUCCESS) { - goto cleanup_viewlist; - } + result = create_views(config, configparser, named_g_aclconfctx, + &viewlist); + if (result != ISC_R_SUCCESS) { + goto cleanup_viewlist; } - /* - * If there were no explicit views then we do the default - * view here. - */ - if (views == NULL) { - dns_view_t *view = NULL; - - result = create_view(NULL, &viewlist, &view); - if (result != ISC_R_SUCCESS) { - goto cleanup_viewlist; - } - INSIST(view != NULL); - - result = setup_newzones(view, config, NULL, configparser, - named_g_aclconfctx); - - dns_view_detach(&view); - if (result != ISC_R_SUCCESS) { - goto cleanup_viewlist; - } - } - - /* - * Configure and freeze all explicit views. Explicit - * views that have zones were already created at parsing - * time, but views with no zones must be created here. - */ - CFG_LIST_FOREACH(views, element) { - cfg_obj_t *vconfig = cfg_listelt_value(element); - dns_view_t *view = NULL; - - view = NULL; - result = find_view(vconfig, &viewlist, &view); - if (result != ISC_R_SUCCESS) { - goto cleanup_cachelist; - } - - result = configure_view(view, &viewlist, config, vconfig, - &cachelist, &server->cachelist, - &server->kasplist, bindkeys, isc_g_mctx, - named_g_aclconfctx, true, first_time); - if (result != ISC_R_SUCCESS) { - dns_view_detach(&view); - goto cleanup_cachelist; - } - dns_view_freeze(view); - dns_view_detach(&view); - } - - /* - * Make sure we have a default view if and only if there - * were no explicit views. - */ - if (views == NULL) { - dns_view_t *view = NULL; - result = find_view(NULL, &viewlist, &view); - if (result != ISC_R_SUCCESS) { - goto cleanup_cachelist; - } - result = configure_view(view, &viewlist, config, NULL, - &cachelist, &server->cachelist, - &server->kasplist, bindkeys, isc_g_mctx, - named_g_aclconfctx, true, first_time); - if (result != ISC_R_SUCCESS) { - dns_view_detach(&view); - goto cleanup_cachelist; - } - dns_view_freeze(view); - dns_view_detach(&view); - } - - /* - * Create (or recreate) the built-in views. - */ - builtin_views = NULL; - RUNTIME_CHECK(cfg_map_get(named_g_defaultconfig, "view", - &builtin_views) == ISC_R_SUCCESS); - CFG_LIST_FOREACH(builtin_views, element) { - cfg_obj_t *vconfig = cfg_listelt_value(element); - dns_view_t *view = NULL; - - result = create_view(vconfig, &builtin_viewlist, &view); - if (result != ISC_R_SUCCESS) { - goto cleanup_cachelist; - } - - result = configure_view(view, &viewlist, config, vconfig, - &cachelist, &server->cachelist, - &server->kasplist, bindkeys, isc_g_mctx, - named_g_aclconfctx, false, first_time); - if (result != ISC_R_SUCCESS) { - dns_view_detach(&view); - goto cleanup_cachelist; - } - dns_view_freeze(view); - dns_view_detach(&view); - } - - /* Now combine the two viewlists into one */ - ISC_LIST_APPENDLIST(viewlist, builtin_viewlist, link); - - /* - * Commit any dns_zone_setview() calls on all zones in the new - * view. - */ - ISC_LIST_FOREACH(viewlist, view, link) { - dns_view_setviewcommit(view); - } - - /* Swap our new view list with the production one. */ - tmpviewlist = server->viewlist; - server->viewlist = viewlist; - viewlist = tmpviewlist; - - /* Make the view list available to each of the views */ - ISC_LIST_FOREACH(server->viewlist, view, link) { - view->viewlist = &server->viewlist; + result = configure_views(config, bindkeys, named_g_aclconfctx, + &viewlist, &builtin_viewlist, &cachelist, + server, first_time); + if (result != ISC_R_SUCCESS) { + goto cleanup_cachelist; } /* Swap our new cache list with the production one. */ From 0fb6c9ae74e7c14a6c8845939a90234960fdb679 Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Tue, 26 Aug 2025 13:19:10 +0200 Subject: [PATCH 02/10] apply_configuration: remove builtin_viewlist When creating/configuring the view, the user-defined views are built and set into the viewlist, then builtin-view inside the builtin_viewlist. But there is no seperate logic applied to those two lists, and they are immediately merged into viewlist right after. This commit removes this intermediate list and add builtin-views directly into the main viewlist instead. --- bin/named/server.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 704cf3facf..e2253fad18 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -7892,8 +7892,8 @@ create_views(cfg_obj_t *config, cfg_parser_t *parser, static isc_result_t configure_views(cfg_obj_t *config, const cfg_obj_t *bindkeys, cfg_aclconfctx_t *aclconfctx, dns_viewlist_t *viewlist, - dns_viewlist_t *builtin_viewlist, named_cachelist_t *cachelist, - named_server_t *server, bool first_time) { + named_cachelist_t *cachelist, named_server_t *server, + bool first_time) { isc_result_t result = ISC_R_SUCCESS; const cfg_obj_t *views = NULL; dns_viewlist_t tmpviewlist; @@ -7957,7 +7957,7 @@ configure_views(cfg_obj_t *config, const cfg_obj_t *bindkeys, cfg_obj_t *vconfig = cfg_listelt_value(element); dns_view_t *view = NULL; - result = create_view(vconfig, builtin_viewlist, &view); + result = create_view(vconfig, viewlist, &view); if (result != ISC_R_SUCCESS) { return result; } @@ -7974,9 +7974,6 @@ configure_views(cfg_obj_t *config, const cfg_obj_t *bindkeys, dns_view_detach(&view); } - /* Now combine the two viewlists into one */ - ISC_LIST_APPENDLIST(*viewlist, *builtin_viewlist, link); - /* * Commit any dns_zone_setview() calls on all zones in the new * view. @@ -8014,7 +8011,7 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, dns_kasp_t *default_kasp = NULL; dns_kasplist_t tmpkasplist, kasplist; dns_keystorelist_t tmpkeystorelist, keystorelist; - dns_viewlist_t viewlist, builtin_viewlist; + dns_viewlist_t viewlist; in_port_t listen_port, udpport_low, udpport_high; int i, backlog; isc_interval_t interval; @@ -8051,7 +8048,6 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, ISC_LIST_INIT(kasplist); ISC_LIST_INIT(keystorelist); ISC_LIST_INIT(viewlist); - ISC_LIST_INIT(builtin_viewlist); ISC_LIST_INIT(cachelist); ISC_LIST_INIT(altsecrets); @@ -8772,8 +8768,7 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, } result = configure_views(config, bindkeys, named_g_aclconfctx, - &viewlist, &builtin_viewlist, &cachelist, - server, first_time); + &viewlist, &cachelist, server, first_time); if (result != ISC_R_SUCCESS) { goto cleanup_cachelist; } @@ -9224,8 +9219,6 @@ cleanup_cachelist: isc_mem_put(server->mctx, nsc, sizeof(*nsc)); } - ISC_LIST_APPENDLIST(viewlist, builtin_viewlist, link); - cleanup_viewlist: ISC_LIST_FOREACH(viewlist, view, link) { ISC_LIST_UNLINK(viewlist, view, link); From c97be6a7f59055eef797ad4bb24627df8996e1aa Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Thu, 28 Aug 2025 17:29:23 +0200 Subject: [PATCH 03/10] apply_configuration: add configure_kasplist The kasplist (dnssec-policy defined in the builtin and global configuration options) was built inside apply_configuration. This commit extracts this logic into its separate function. In order to make the view configuration independent of the global `server` object, the newly built kasplist is now passed as parameter. (This eventually will help to be able to configure the views outside of the exclusive mode by limiting its dependency to the global `server`/`named_g_server`). --- bin/named/server.c | 188 +++++++++++++++++++++++++-------------------- 1 file changed, 103 insertions(+), 85 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index e2253fad18..35d182158f 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -503,7 +503,7 @@ configure_zone_setviewcommit(isc_result_t result, const cfg_obj_t *zconfig, static isc_result_t configure_newzones(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, - cfg_aclconfctx_t *actx); + cfg_aclconfctx_t *actx, dns_kasplist_t *kasplist); static const cfg_obj_t * find_maplist(const cfg_obj_t *config, const char *listname, const char *name); @@ -3972,7 +3972,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, * from the newzone file for zones that were added during previous * runs. */ - CHECK(configure_newzones(view, config, vconfig, actx)); + CHECK(configure_newzones(view, config, vconfig, actx, kasplist)); /* * Create Dynamically Loadable Zone driver. @@ -7507,7 +7507,7 @@ configure_zone_setviewcommit(isc_result_t result, const cfg_obj_t *zconfig, static isc_result_t configure_newzones(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, - cfg_aclconfctx_t *actx) { + cfg_aclconfctx_t *actx, dns_kasplist_t *kasplist) { isc_result_t result; ns_cfgctx_t *nzctx = NULL; const cfg_obj_t *zonelist = NULL; @@ -7527,9 +7527,8 @@ configure_newzones(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, CFG_LIST_FOREACH(zonelist, element) { const cfg_obj_t *zconfig = cfg_listelt_value(element); CHECK(configure_zone(config, zconfig, vconfig, view, - &named_g_server->viewlist, - &named_g_server->kasplist, actx, true, - false, false, false)); + &named_g_server->viewlist, kasplist, actx, + true, false, false, false)); } result = ISC_R_SUCCESS; @@ -7622,7 +7621,8 @@ cleanup: typedef isc_result_t (*newzone_cfg_cb_t)(const cfg_obj_t *zconfig, cfg_obj_t *config, cfg_obj_t *vconfig, dns_view_t *view, - cfg_aclconfctx_t *actx); + cfg_aclconfctx_t *actx, + dns_kasplist_t *kasplist); /*% * For each zone found in a NZD opened by the caller, create an object @@ -7638,7 +7638,8 @@ typedef isc_result_t (*newzone_cfg_cb_t)(const cfg_obj_t *zconfig, static isc_result_t for_all_newzone_cfgs(newzone_cfg_cb_t callback, cfg_obj_t *config, cfg_obj_t *vconfig, dns_view_t *view, - cfg_aclconfctx_t *actx, MDB_txn *txn, MDB_dbi dbi) { + cfg_aclconfctx_t *actx, dns_kasplist_t *kasplist, + MDB_txn *txn, MDB_dbi dbi) { const cfg_obj_t *zconfig, *zlist; isc_result_t result = ISC_R_SUCCESS; cfg_obj_t *zconfigobj = NULL; @@ -7680,7 +7681,8 @@ for_all_newzone_cfgs(newzone_cfg_cb_t callback, cfg_obj_t *config, /* * Invoke callback. */ - result = callback(zconfig, config, vconfig, view, actx); + result = callback(zconfig, config, vconfig, view, actx, + kasplist); if (result != ISC_R_SUCCESS) { break; } @@ -7707,11 +7709,11 @@ for_all_newzone_cfgs(newzone_cfg_cb_t callback, cfg_obj_t *config, */ static isc_result_t configure_newzone(const cfg_obj_t *zconfig, cfg_obj_t *config, - cfg_obj_t *vconfig, dns_view_t *view, - cfg_aclconfctx_t *actx) { - return configure_zone( - config, zconfig, vconfig, view, &named_g_server->viewlist, - &named_g_server->kasplist, actx, true, false, false, false); + cfg_obj_t *vconfig, dns_view_t *view, cfg_aclconfctx_t *actx, + dns_kasplist_t *kasplist) { + return configure_zone(config, zconfig, vconfig, view, + &named_g_server->viewlist, kasplist, actx, true, + false, false, false); } /*% @@ -7720,10 +7722,11 @@ configure_newzone(const cfg_obj_t *zconfig, cfg_obj_t *config, static isc_result_t configure_newzone_revert(const cfg_obj_t *zconfig, cfg_obj_t *config, cfg_obj_t *vconfig, dns_view_t *view, - cfg_aclconfctx_t *actx) { + cfg_aclconfctx_t *actx, dns_kasplist_t *kasplist) { UNUSED(config); UNUSED(vconfig); UNUSED(actx); + UNUSED(kasplist); configure_zone_setviewcommit(ISC_R_FAILURE, zconfig, view); @@ -7732,7 +7735,7 @@ configure_newzone_revert(const cfg_obj_t *zconfig, cfg_obj_t *config, static isc_result_t configure_newzones(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, - cfg_aclconfctx_t *actx) { + cfg_aclconfctx_t *actx, dns_kasplist_t *kasplist) { isc_result_t result; MDB_txn *txn = NULL; MDB_dbi dbi; @@ -7756,7 +7759,7 @@ configure_newzones(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, view->new_zone_db, view->name); result = for_all_newzone_cfgs(configure_newzone, config, vconfig, view, - actx, txn, dbi); + actx, kasplist, txn, dbi); if (result != ISC_R_SUCCESS) { /* * An error was encountered while attempting to configure zones @@ -7767,7 +7770,8 @@ configure_newzones(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, * terms of trying to make things right. */ (void)for_all_newzone_cfgs(configure_newzone_revert, config, - vconfig, view, actx, txn, dbi); + vconfig, view, actx, kasplist, txn, + dbi); } (void)nzd_close(&txn, false); @@ -7892,8 +7896,8 @@ create_views(cfg_obj_t *config, cfg_parser_t *parser, static isc_result_t configure_views(cfg_obj_t *config, const cfg_obj_t *bindkeys, cfg_aclconfctx_t *aclconfctx, dns_viewlist_t *viewlist, - named_cachelist_t *cachelist, named_server_t *server, - bool first_time) { + named_cachelist_t *cachelist, dns_kasplist_t *kasplist, + named_server_t *server, bool first_time) { isc_result_t result = ISC_R_SUCCESS; const cfg_obj_t *views = NULL; dns_viewlist_t tmpviewlist; @@ -7914,9 +7918,9 @@ configure_views(cfg_obj_t *config, const cfg_obj_t *bindkeys, } result = configure_view(view, viewlist, config, vconfig, - cachelist, &server->cachelist, - &server->kasplist, bindkeys, isc_g_mctx, - aclconfctx, true, first_time); + cachelist, &server->cachelist, kasplist, + bindkeys, isc_g_mctx, aclconfctx, true, + first_time); if (result != ISC_R_SUCCESS) { dns_view_detach(&view); return result; @@ -7936,8 +7940,8 @@ configure_views(cfg_obj_t *config, const cfg_obj_t *bindkeys, return result; } result = configure_view(view, viewlist, config, NULL, cachelist, - &server->cachelist, &server->kasplist, - bindkeys, isc_g_mctx, aclconfctx, true, + &server->cachelist, kasplist, bindkeys, + isc_g_mctx, aclconfctx, true, first_time); if (result != ISC_R_SUCCESS) { dns_view_detach(&view); @@ -7963,9 +7967,9 @@ configure_views(cfg_obj_t *config, const cfg_obj_t *bindkeys, } result = configure_view(view, viewlist, config, vconfig, - cachelist, &server->cachelist, - &server->kasplist, bindkeys, isc_g_mctx, - aclconfctx, false, first_time); + cachelist, &server->cachelist, kasplist, + bindkeys, isc_g_mctx, aclconfctx, false, + first_time); if (result != ISC_R_SUCCESS) { dns_view_detach(&view); return result; @@ -7998,6 +8002,64 @@ configure_views(cfg_obj_t *config, const cfg_obj_t *bindkeys, return result; } +static isc_result_t +configure_kasplist(const cfg_obj_t *config, dns_kasplist_t *kasplist, + dns_keystorelist_t *keystorelist) { + isc_result_t result = ISC_R_SUCCESS; + dns_kasp_t *default_kasp = NULL; + const cfg_obj_t *kasps = NULL; + + /* + * Create the built-in kasp policies ("default", "insecure"). + */ + (void)cfg_map_get(named_g_defaultconfig, "dnssec-policy", &kasps); + CFG_LIST_FOREACH(kasps, element) { + cfg_obj_t *kconfig = cfg_listelt_value(element); + dns_kasp_t *kasp = NULL; + + result = cfg_kasp_fromconfig(kconfig, default_kasp, true, + isc_g_mctx, keystorelist, kasplist, + &kasp); + if (result != ISC_R_SUCCESS) { + return result; + } + INSIST(kasp != NULL); + dns_kasp_freeze(kasp); + + /* Insist that the first built-in policy is the default one. */ + if (default_kasp == NULL) { + INSIST(strcmp(dns_kasp_getname(kasp), "default") == 0); + dns_kasp_attach(kasp, &default_kasp); + } + + dns_kasp_detach(&kasp); + } + INSIST(default_kasp != NULL); + + /* + * Create the DNSSEC key and signing policies (KASP). + */ + kasps = NULL; + (void)cfg_map_get(config, "dnssec-policy", &kasps); + CFG_LIST_FOREACH(kasps, element) { + cfg_obj_t *kconfig = cfg_listelt_value(element); + dns_kasp_t *kasp = NULL; + + result = cfg_kasp_fromconfig(kconfig, default_kasp, true, + isc_g_mctx, keystorelist, kasplist, + &kasp); + if (result != ISC_R_SUCCESS) { + return result; + } + INSIST(kasp != NULL); + dns_kasp_freeze(kasp); + dns_kasp_detach(&kasp); + } + dns_kasp_detach(&default_kasp); + + return result; +} + static isc_result_t apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, named_server_t *server, bool first_time) { @@ -8006,9 +8068,7 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, const cfg_obj_t *maps[3]; const cfg_obj_t *obj = NULL; const cfg_obj_t *options = NULL; - const cfg_obj_t *kasps = NULL; const cfg_obj_t *keystores = NULL; - dns_kasp_t *default_kasp = NULL; dns_kasplist_t tmpkasplist, kasplist; dns_keystorelist_t tmpkeystorelist, keystorelist; dns_viewlist_t viewlist; @@ -8705,61 +8765,10 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, } } - /* - * Create the built-in kasp policies ("default", "insecure"). - */ - kasps = NULL; - (void)cfg_map_get(named_g_defaultconfig, "dnssec-policy", &kasps); - CFG_LIST_FOREACH(kasps, element) { - cfg_obj_t *kconfig = cfg_listelt_value(element); - dns_kasp_t *kasp = NULL; - - result = cfg_kasp_fromconfig(kconfig, default_kasp, true, - isc_g_mctx, &keystorelist, - &kasplist, &kasp); - if (result != ISC_R_SUCCESS) { - goto cleanup_kasplist; - } - INSIST(kasp != NULL); - dns_kasp_freeze(kasp); - - /* Insist that the first built-in policy is the default one. */ - if (default_kasp == NULL) { - INSIST(strcmp(dns_kasp_getname(kasp), "default") == 0); - dns_kasp_attach(kasp, &default_kasp); - } - - dns_kasp_detach(&kasp); + result = configure_kasplist(config, &kasplist, &keystorelist); + if (result != ISC_R_SUCCESS) { + goto cleanup_kasplist; } - INSIST(default_kasp != NULL); - - /* - * Create the DNSSEC key and signing policies (KASP). - */ - kasps = NULL; - (void)cfg_map_get(config, "dnssec-policy", &kasps); - CFG_LIST_FOREACH(kasps, element) { - cfg_obj_t *kconfig = cfg_listelt_value(element); - dns_kasp_t *kasp = NULL; - - result = cfg_kasp_fromconfig(kconfig, default_kasp, true, - isc_g_mctx, &keystorelist, - &kasplist, &kasp); - if (result != ISC_R_SUCCESS) { - goto cleanup_kasplist; - } - INSIST(kasp != NULL); - dns_kasp_freeze(kasp); - dns_kasp_detach(&kasp); - } - dns_kasp_detach(&default_kasp); - - /* - * Save kasp list. - */ - tmpkasplist = server->kasplist; - server->kasplist = kasplist; - kasplist = tmpkasplist; result = create_views(config, configparser, named_g_aclconfctx, &viewlist); @@ -8768,7 +8777,8 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, } result = configure_views(config, bindkeys, named_g_aclconfctx, - &viewlist, &cachelist, server, first_time); + &viewlist, &cachelist, &kasplist, server, + first_time); if (result != ISC_R_SUCCESS) { goto cleanup_cachelist; } @@ -9149,6 +9159,14 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, &server->keystorelist); } + /* + * Save the new kasp list with the old one (so the new one will be used + * and the old one will be cleared). + */ + tmpkasplist = server->kasplist; + server->kasplist = kasplist; + kasplist = tmpkasplist; + (void)named_server_loadnta(server); /* From de11150e478bae52d69819dabb22a83439a6a958 Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Mon, 8 Sep 2025 10:46:11 +0200 Subject: [PATCH 04/10] apply_configuation: add configure_keystores The keystores list build logic was inlined in apply_configuration, this commit extracts it into its own function. --- bin/named/server.c | 56 +++++++++++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 20 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 35d182158f..a7602e95fa 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -8002,6 +8002,37 @@ configure_views(cfg_obj_t *config, const cfg_obj_t *bindkeys, return result; } +static isc_result_t +configure_keystores(const cfg_obj_t *config, dns_keystorelist_t *keystorelist) { + isc_result_t result = ISC_R_SUCCESS; + const cfg_obj_t *keystores = NULL; + + /* + * Create the built-in key store ("key-directory"). + */ + result = cfg_keystore_fromconfig(NULL, isc_g_mctx, keystorelist, NULL); + if (result != ISC_R_SUCCESS) { + return result; + } + + /* + * Create the DNSSEC key stores. + */ + keystores = NULL; + (void)cfg_map_get(config, "key-store", &keystores); + CFG_LIST_FOREACH(keystores, element) { + cfg_obj_t *kconfig = cfg_listelt_value(element); + + result = cfg_keystore_fromconfig(kconfig, isc_g_mctx, + keystorelist, NULL); + if (result != ISC_R_SUCCESS) { + return result; + } + } + + return result; +} + static isc_result_t configure_kasplist(const cfg_obj_t *config, dns_kasplist_t *kasplist, dns_keystorelist_t *keystorelist) { @@ -8068,7 +8099,6 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, const cfg_obj_t *maps[3]; const cfg_obj_t *obj = NULL; const cfg_obj_t *options = NULL; - const cfg_obj_t *keystores = NULL; dns_kasplist_t tmpkasplist, kasplist; dns_keystorelist_t tmpkeystorelist, keystorelist; dns_viewlist_t viewlist; @@ -8742,29 +8772,11 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, */ (void)configure_session_key(maps, server, isc_g_mctx, first_time); - /* - * Create the built-in key store ("key-directory"). - */ - result = cfg_keystore_fromconfig(NULL, isc_g_mctx, &keystorelist, NULL); + result = configure_keystores(config, &keystorelist); if (result != ISC_R_SUCCESS) { goto cleanup_keystorelist; } - /* - * Create the DNSSEC key stores. - */ - keystores = NULL; - (void)cfg_map_get(config, "key-store", &keystores); - CFG_LIST_FOREACH(keystores, element) { - cfg_obj_t *kconfig = cfg_listelt_value(element); - - result = cfg_keystore_fromconfig(kconfig, isc_g_mctx, - &keystorelist, NULL); - if (result != ISC_R_SUCCESS) { - goto cleanup_keystorelist; - } - } - result = configure_kasplist(config, &kasplist, &keystorelist); if (result != ISC_R_SUCCESS) { goto cleanup_kasplist; @@ -9150,6 +9162,10 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, /* * Swap the new keystores list with the old one (so the new one will be * used and old one will be cleared). + * + * If this is the initial server setup, store the address + * `&server->keystorelist` in the zone manager, so the zones can reach + * the list during runtime whenever needed. */ tmpkeystorelist = server->keystorelist; server->keystorelist = keystorelist; From 4523852ded5c3a263ab6fc40295007beda911786 Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Mon, 8 Sep 2025 11:07:29 +0200 Subject: [PATCH 05/10] apply_configuration: bump config map before exclusive mode Moving the config map building outside of the exclusive mode, and this is local data only and no runtime object uses it. --- bin/named/server.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index a7602e95fa..ea955267ac 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -8141,6 +8141,18 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, ISC_LIST_INIT(cachelist); ISC_LIST_INIT(altsecrets); + /* + * Fill in the maps array, used for resolving defaults. + */ + i = 0; + options = NULL; + result = cfg_map_get(config, "options", &options); + if (result == ISC_R_SUCCESS) { + maps[i++] = options; + } + maps[i++] = named_g_defaultoptions; + maps[i] = NULL; + /* Ensure exclusive access to configuration data. */ isc_loopmgr_pause(); @@ -8187,18 +8199,6 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, dns_zonemgr_set_tlsctx_cache(server->zonemgr, server->tlsctx_client_cache); - /* - * Fill in the maps array, used for resolving defaults. - */ - i = 0; - options = NULL; - result = cfg_map_get(config, "options", &options); - if (result == ISC_R_SUCCESS) { - maps[i++] = options; - } - maps[i++] = named_g_defaultoptions; - maps[i] = NULL; - #if HAVE_LIBNGHTTP2 obj = NULL; result = named_config_get(maps, "http-port", &obj); From 201f62d9efb3cf6ccbac1127768121e899d36ee8 Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Mon, 8 Sep 2025 14:58:47 +0200 Subject: [PATCH 06/10] cfg_aclconfctx_t object is part of named_server `named_g_actconfctx` is a global variable holding the ACL configuration context alive (in particular, to dynamically load zones). However, this object is build once per configuration (early) and is used only inside server.c `apply_configuration` flow. (Two exceptions: the shutdown flow, still in server.c and plugin check flow, which doesn't need it, so it's NULL in such case). Instead of leaving this global publicly exposed, it is now part of the `named_server_t` object. This allows us to clearly see that, when reconfigureing the server, the new instance of the ACL context is known only by the newly built object and not currently used by "production" object; and will help to move move logic before the exclusive mode is taken. The other advantage is that the ACL configuration context can now be built before the exclusive lock as well. --- bin/named/include/named/globals.h | 3 +- bin/named/include/named/server.h | 9 +++- bin/named/include/named/zoneconf.h | 4 +- bin/named/server.c | 85 +++++++++++++++++------------- bin/named/zoneconf.c | 7 +-- lib/isccfg/check.c | 8 +-- lib/isccfg/include/isccfg/cfg.h | 13 ++--- lib/isccfg/parser.c | 5 +- 8 files changed, 77 insertions(+), 57 deletions(-) diff --git a/bin/named/include/named/globals.h b/bin/named/include/named/globals.h index 370cdc3914..126d248f4d 100644 --- a/bin/named/include/named/globals.h +++ b/bin/named/include/named/globals.h @@ -93,8 +93,7 @@ EXTERN const char *named_g_conffile INIT(NAMED_SYSCONFDIR "/named.conf"); EXTERN const char *named_g_defaultbindkeys INIT(NULL); EXTERN const char *named_g_keyfile INIT(NAMED_SYSCONFDIR "/rndc.key"); -EXTERN bool named_g_conffileset INIT(false); -EXTERN cfg_aclconfctx_t *named_g_aclconfctx INIT(NULL); +EXTERN bool named_g_conffileset INIT(false); /* * Misc. diff --git a/bin/named/include/named/server.h b/bin/named/include/named/server.h index 1e6621ae9e..106bb3502d 100644 --- a/bin/named/include/named/server.h +++ b/bin/named/include/named/server.h @@ -40,6 +40,9 @@ #include +struct cfg_aclconfctx; +typedef struct cfg_aclconfctx cfg_aclconfctx_t; + /*% * Name server state. Better here than in lots of separate global variables. */ @@ -106,6 +109,8 @@ struct named_server { isc_signal_t *sighup; isc_signal_t *sigusr1; + + cfg_aclconfctx_t *aclconfctx; }; #define NAMED_SERVER_MAGIC ISC_MAGIC('S', 'V', 'E', 'R') @@ -416,5 +421,5 @@ named_server_getmemprof(void); */ isc_result_t named_register_one_plugin(const cfg_obj_t *config, const cfg_obj_t *obj, - const char *plugin_path, const char *parameters, - void *callback_data); + cfg_aclconfctx_t *actx, const char *plugin_path, + const char *parameters, void *callback_data); diff --git a/bin/named/include/named/zoneconf.h b/bin/named/include/named/zoneconf.h index 47fb9aeaeb..8975c840be 100644 --- a/bin/named/include/named/zoneconf.h +++ b/bin/named/include/named/zoneconf.h @@ -84,7 +84,8 @@ named_zone_templateopts(const cfg_obj_t *config, const cfg_obj_t *zoptions); isc_result_t named_zone_loadplugins(dns_zone_t *zone, const cfg_obj_t *config, - const cfg_obj_t *toptions, const cfg_obj_t *zoptions); + const cfg_obj_t *toptions, const cfg_obj_t *zoptions, + cfg_aclconfctx_t *actx); /*%< * Load plugins that should run for this specific zone. Take care of cleaning * up any pre-existing plugins first, if the zone is re-used. @@ -95,4 +96,5 @@ named_zone_loadplugins(dns_zone_t *zone, const cfg_obj_t *config, * \li 'zoptions' to be a valid zone configuration tree * \li 'toptions' to be NULL or valid template configuration tree * \li 'zoptions' to be NULL or a valid zone configuration tree + * \li 'actx' to be NULL (confcheck case only) or a valid acl conf ctx */ diff --git a/bin/named/server.c b/bin/named/server.c index ea955267ac..993e8f218f 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -2986,7 +2986,8 @@ cleanup: } while (0) static isc_result_t -configure_rrl(dns_view_t *view, const cfg_obj_t *config, const cfg_obj_t *map) { +configure_rrl(dns_view_t *view, const cfg_obj_t *config, const cfg_obj_t *map, + cfg_aclconfctx_t *actx) { const cfg_obj_t *obj; dns_rrl_t *rrl; isc_result_t result; @@ -3097,8 +3098,8 @@ configure_rrl(dns_view_t *view, const cfg_obj_t *config, const cfg_obj_t *map) { obj = NULL; result = cfg_map_get(map, "exempt-clients", &obj); if (result == ISC_R_SUCCESS) { - result = cfg_acl_fromconfig(obj, config, named_g_aclconfctx, - isc_g_mctx, 0, &rrl->exempt); + result = cfg_acl_fromconfig(obj, config, actx, isc_g_mctx, 0, + &rrl->exempt); CHECK_RRL(result == ISC_R_SUCCESS, "invalid %s%s", "address match list", ""); } @@ -3714,8 +3715,8 @@ create_mapped_acl(void) { isc_result_t named_register_one_plugin(const cfg_obj_t *config, const cfg_obj_t *obj, - const char *plugin_path, const char *parameters, - void *callback_data) { + cfg_aclconfctx_t *actx, const char *plugin_path, + const char *parameters, void *callback_data) { char full_path[PATH_MAX]; isc_result_t result; ns_hook_data_t *hookdata = callback_data; @@ -3733,7 +3734,7 @@ named_register_one_plugin(const cfg_obj_t *config, const cfg_obj_t *obj, result = ns_plugin_register(full_path, parameters, config, cfg_obj_file(obj), cfg_obj_line(obj), - isc_g_mctx, named_g_aclconfctx, hookdata); + isc_g_mctx, actx, hookdata); if (result != ISC_R_SUCCESS) { isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR, @@ -5430,7 +5431,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, view->plugins = hookdata.plugins; view->plugins_free = ns_plugins_free; - CHECK(cfg_pluginlist_foreach(config, plugin_list, + CHECK(cfg_pluginlist_foreach(config, plugin_list, actx, named_register_one_plugin, &hookdata)); } @@ -5687,7 +5688,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, obj = NULL; result = named_config_get(maps, "rate-limit", &obj); if (result == ISC_R_SUCCESS) { - result = configure_rrl(view, config, obj); + result = configure_rrl(view, config, obj, actx); if (result != ISC_R_SUCCESS) { goto cleanup; } @@ -6255,7 +6256,7 @@ static isc_result_t configure_zone(const cfg_obj_t *config, const cfg_obj_t *zconfig, const cfg_obj_t *vconfig, dns_view_t *view, dns_viewlist_t *viewlist, dns_kasplist_t *kasplist, - cfg_aclconfctx_t *aclconf, bool added, bool old_rpz_ok, + cfg_aclconfctx_t *actx, bool added, bool old_rpz_ok, bool is_catz_member, bool modify) { dns_view_t *pview = NULL; /* Production view */ dns_zone_t *zone = NULL; /* New or reused zone */ @@ -6455,7 +6456,7 @@ configure_zone(const cfg_obj_t *config, const cfg_obj_t *zconfig, zone)); dns_zone_setstats(zone, named_g_server->zonestats); } - CHECK(named_zone_configure(config, vconfig, zconfig, aclconf, + CHECK(named_zone_configure(config, vconfig, zconfig, actx, kasplist, zone, NULL)); dns_zone_attach(zone, &view->redirect); goto cleanup; @@ -6631,7 +6632,7 @@ configure_zone(const cfg_obj_t *config, const cfg_obj_t *zconfig, /* * Configure the zone. */ - CHECK(named_zone_configure(config, vconfig, zconfig, aclconf, kasplist, + CHECK(named_zone_configure(config, vconfig, zconfig, actx, kasplist, zone, raw)); /* @@ -6662,7 +6663,7 @@ configure_zone(const cfg_obj_t *config, const cfg_obj_t *zconfig, dns_zone_rekey(zone, fullsign, false); } - result = named_zone_loadplugins(zone, config, toptions, zoptions); + result = named_zone_loadplugins(zone, config, toptions, zoptions, actx); cleanup: if (zone != NULL) { @@ -8126,6 +8127,7 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, bool exclusive = true; dns_aclenv_t *env = ns_interfacemgr_getaclenv(named_g_server->interfacemgr); + cfg_aclconfctx_t *tmpaclconfctx, *aclconfctx = NULL; isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_DEBUG(1), "apply_configuration"); @@ -8153,18 +8155,15 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, maps[i++] = named_g_defaultoptions; maps[i] = NULL; + /* Create the ACL configuration context */ + result = cfg_aclconfctx_create(isc_g_mctx, &aclconfctx); + if (result != ISC_R_SUCCESS) { + goto cleanup_aclconfctx; + } + /* Ensure exclusive access to configuration data. */ isc_loopmgr_pause(); - /* Create the ACL configuration context */ - if (named_g_aclconfctx != NULL) { - cfg_aclconfctx_detach(&named_g_aclconfctx); - } - result = cfg_aclconfctx_create(isc_g_mctx, &named_g_aclconfctx); - if (result != ISC_R_SUCCESS) { - goto cleanup_exclusive; - } - /* * Shut down all dyndb instances. */ @@ -8282,7 +8281,7 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, char *dir = UNCONST(cfg_obj_asstring(obj)); named_geoip_load(dir); } - named_g_aclconfctx->geoip = named_g_geoip; + aclconfctx->geoip = named_g_geoip; #endif /* HAVE_GEOIP2 */ /* @@ -8320,7 +8319,7 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, result = named_config_get(maps, "sig0checks-quota-exempt", &obj); if (result == ISC_R_SUCCESS) { result = cfg_acl_fromconfig( - obj, config, named_g_aclconfctx, isc_g_mctx, 0, + obj, config, aclconfctx, isc_g_mctx, 0, &server->sctx->sig0checksquota_exempt); INSIST(result == ISC_R_SUCCESS); } @@ -8330,7 +8329,7 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, * no default. */ result = configure_view_acl(NULL, config, NULL, "blackhole", NULL, - named_g_aclconfctx, isc_g_mctx, + aclconfctx, isc_g_mctx, &server->sctx->blackholeacl); if (result != ISC_R_SUCCESS) { goto cleanup_bindkeys_parser; @@ -8632,8 +8631,8 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, goto cleanup_portsets; } result = listenlist_fromconfig( - clistenon, config, named_g_aclconfctx, isc_g_mctx, - AF_INET, server->tlsctx_server_cache, &listenon); + clistenon, config, aclconfctx, isc_g_mctx, AF_INET, + server->tlsctx_server_cache, &listenon); if (result != ISC_R_SUCCESS) { goto cleanup_portsets; } @@ -8656,8 +8655,8 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, goto cleanup_portsets; } result = listenlist_fromconfig( - clistenon, config, named_g_aclconfctx, isc_g_mctx, - AF_INET6, server->tlsctx_server_cache, &listenon); + clistenon, config, aclconfctx, isc_g_mctx, AF_INET6, + server->tlsctx_server_cache, &listenon); if (result != ISC_R_SUCCESS) { goto cleanup_portsets; } @@ -8782,15 +8781,13 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, goto cleanup_kasplist; } - result = create_views(config, configparser, named_g_aclconfctx, - &viewlist); + result = create_views(config, configparser, aclconfctx, &viewlist); if (result != ISC_R_SUCCESS) { goto cleanup_viewlist; } - result = configure_views(config, bindkeys, named_g_aclconfctx, - &viewlist, &cachelist, &kasplist, server, - first_time); + result = configure_views(config, bindkeys, aclconfctx, &viewlist, + &cachelist, &kasplist, server, first_time); if (result != ISC_R_SUCCESS) { goto cleanup_cachelist; } @@ -9183,6 +9180,13 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, server->kasplist = kasplist; kasplist = tmpkasplist; + /* + * Swap server aclconfctx + */ + tmpaclconfctx = server->aclconfctx; + server->aclconfctx = aclconfctx; + aclconfctx = tmpaclconfctx; + (void)named_server_loadnta(server); /* @@ -9200,7 +9204,7 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, /* Configure the statistics channel(s) */ result = named_statschannels_configure(named_g_server, config, - named_g_aclconfctx); + server->aclconfctx); if (result != ISC_R_SUCCESS) { isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR, @@ -9213,7 +9217,7 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, * Bind the control port(s). */ result = named_controls_configure(named_g_server->controls, config, - named_g_aclconfctx); + server->aclconfctx); if (result != ISC_R_SUCCESS) { isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR, "binding control channel(s): %s", @@ -9221,6 +9225,7 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, goto cleanup_altsecrets; } + (void)ns_interfacemgr_scan(server->interfacemgr, true, true); /* @@ -9288,11 +9293,15 @@ cleanup_bindkeys_parser: cfg_parser_destroy(&bindkeys_parser); } -cleanup_exclusive: if (exclusive) { isc_loopmgr_resume(); } +cleanup_aclconfctx: + if (aclconfctx != NULL) { + cfg_aclconfctx_detach(&aclconfctx); + } + isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_DEBUG(1), "apply_configuration: %s", isc_result_totext(result)); @@ -9562,8 +9571,8 @@ shutdown_server(void *arg) { cleanup_session_key(server, server->mctx); - if (named_g_aclconfctx != NULL) { - cfg_aclconfctx_detach(&named_g_aclconfctx); + if (server->aclconfctx != NULL) { + cfg_aclconfctx_detach(&server->aclconfctx); } cfg_obj_destroy(named_g_parser, &named_g_defaultconfig); diff --git a/bin/named/zoneconf.c b/bin/named/zoneconf.c index e4a4cf4b42..592c65a0b7 100644 --- a/bin/named/zoneconf.c +++ b/bin/named/zoneconf.c @@ -2101,7 +2101,8 @@ named_zone_templateopts(const cfg_obj_t *config, const cfg_obj_t *zoptions) { isc_result_t named_zone_loadplugins(dns_zone_t *zone, const cfg_obj_t *config, - const cfg_obj_t *toptions, const cfg_obj_t *zoptions) { + const cfg_obj_t *toptions, const cfg_obj_t *zoptions, + cfg_aclconfctx_t *actx) { isc_result_t result = ISC_R_SUCCESS; const cfg_obj_t *zpluginlist = NULL; const cfg_obj_t *tpluginlist = NULL; @@ -2135,14 +2136,14 @@ named_zone_loadplugins(dns_zone_t *zone, const cfg_obj_t *config, ns_plugins_create(zmctx, &hookdata.plugins); dns_zone_setplugins(zone, hookdata.plugins, ns_plugins_free); - result = cfg_pluginlist_foreach(config, tpluginlist, + result = cfg_pluginlist_foreach(config, tpluginlist, actx, named_register_one_plugin, &hookdata); if (result != ISC_R_SUCCESS) { return result; } - result = cfg_pluginlist_foreach(config, zpluginlist, + result = cfg_pluginlist_foreach(config, zpluginlist, actx, named_register_one_plugin, &hookdata); } diff --git a/lib/isccfg/check.c b/lib/isccfg/check.c index 1d06e1e303..3d03d1e10c 100644 --- a/lib/isccfg/check.c +++ b/lib/isccfg/check.c @@ -2940,12 +2940,14 @@ struct check_one_plugin_data { */ static isc_result_t check_one_plugin(const cfg_obj_t *config, const cfg_obj_t *obj, - const char *plugin_path, const char *parameters, - void *callback_data) { + cfg_aclconfctx_t *actx, const char *plugin_path, + const char *parameters, void *callback_data) { struct check_one_plugin_data *data = callback_data; char full_path[PATH_MAX]; isc_result_t result = ISC_R_SUCCESS; + UNUSED(actx); + result = ns_plugin_expandpath(plugin_path, full_path, sizeof(full_path)); if (result != ISC_R_SUCCESS) { @@ -2978,7 +2980,7 @@ check_plugins(const cfg_obj_t *plugins, const cfg_obj_t *config, .check_result = &result, }; - (void)cfg_pluginlist_foreach(config, plugins, check_one_plugin, + (void)cfg_pluginlist_foreach(config, plugins, actx, check_one_plugin, &check_one_plugin_data); return result; diff --git a/lib/isccfg/include/isccfg/cfg.h b/lib/isccfg/include/isccfg/cfg.h index 260654073b..ba49b1c5b7 100644 --- a/lib/isccfg/include/isccfg/cfg.h +++ b/lib/isccfg/include/isccfg/cfg.h @@ -39,6 +39,8 @@ *** Types ***/ +typedef struct cfg_aclconfctx cfg_aclconfctx_t; + /*% * A configuration parser. */ @@ -586,11 +588,9 @@ const char * cfg_map_nextclause(const cfg_type_t *map, const void **clauses, unsigned int *idx); -typedef isc_result_t(pluginlist_cb_t)(const cfg_obj_t *config, - const cfg_obj_t *obj, - const char *plugin_path, - const char *parameters, - void *callback_data); +typedef isc_result_t(pluginlist_cb_t)( + const cfg_obj_t *config, const cfg_obj_t *obj, cfg_aclconfctx_t *actx, + const char *plugin_path, const char *parameters, void *callback_data); /*%< * Function prototype for the callback used with cfg_pluginlist_foreach(). * Called once for each element of the list passed to cfg_pluginlist_foreach(). @@ -606,7 +606,8 @@ typedef isc_result_t(pluginlist_cb_t)(const cfg_obj_t *config, isc_result_t cfg_pluginlist_foreach(const cfg_obj_t *config, const cfg_obj_t *list, - pluginlist_cb_t *callback, void *callback_data); + cfg_aclconfctx_t *actx, pluginlist_cb_t *callback, + void *callback_data); /*%< * For every "plugin" stanza present in 'list' (which in turn is a part of * 'config'), invoke the given 'callback', passing 'callback_data' to it along diff --git a/lib/isccfg/parser.c b/lib/isccfg/parser.c index 46a8012dbb..c8e411f8de 100644 --- a/lib/isccfg/parser.c +++ b/lib/isccfg/parser.c @@ -3945,7 +3945,8 @@ cleanup: isc_result_t cfg_pluginlist_foreach(const cfg_obj_t *config, const cfg_obj_t *list, - pluginlist_cb_t *callback, void *callback_data) { + cfg_aclconfctx_t *actx, pluginlist_cb_t *callback, + void *callback_data) { isc_result_t result = ISC_R_SUCCESS; REQUIRE(config != NULL); @@ -3975,7 +3976,7 @@ cfg_pluginlist_foreach(const cfg_obj_t *config, const cfg_obj_t *list, parameters = cfg_obj_asstring(obj); } - result = callback(config, obj, library, parameters, + result = callback(config, obj, actx, library, parameters, callback_data); if (result != ISC_R_SUCCESS) { break; From e1be2be4efdf13eabee8fa454a5bfbd63bb5c98d Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Mon, 8 Sep 2025 15:57:47 +0200 Subject: [PATCH 07/10] move creation of keystores, kasp list and view outside of exclusive mode The keystores initialization, the KASP list initialization as well as the initialization of the view no longer depends of any data shared by running "production" objects during re-configuration of the server. This allows us to move those outside (before) the exclusive mode is taken. --- bin/named/server.c | 62 +++++++++++++++++++++++----------------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 993e8f218f..37cc1dea6a 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -8161,6 +8161,21 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, goto cleanup_aclconfctx; } + result = configure_keystores(config, &keystorelist); + if (result != ISC_R_SUCCESS) { + goto cleanup_keystorelist; + } + + result = configure_kasplist(config, &kasplist, &keystorelist); + if (result != ISC_R_SUCCESS) { + goto cleanup_kasplist; + } + + result = create_views(config, configparser, aclconfctx, &viewlist); + if (result != ISC_R_SUCCESS) { + goto cleanup_viewlist; + } + /* Ensure exclusive access to configuration data. */ isc_loopmgr_pause(); @@ -8771,21 +8786,6 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, */ (void)configure_session_key(maps, server, isc_g_mctx, first_time); - result = configure_keystores(config, &keystorelist); - if (result != ISC_R_SUCCESS) { - goto cleanup_keystorelist; - } - - result = configure_kasplist(config, &kasplist, &keystorelist); - if (result != ISC_R_SUCCESS) { - goto cleanup_kasplist; - } - - result = create_views(config, configparser, aclconfctx, &viewlist); - if (result != ISC_R_SUCCESS) { - goto cleanup_viewlist; - } - result = configure_views(config, bindkeys, aclconfctx, &viewlist, &cachelist, &kasplist, server, first_time); if (result != ISC_R_SUCCESS) { @@ -9258,6 +9258,22 @@ cleanup_cachelist: isc_mem_put(server->mctx, nsc, sizeof(*nsc)); } +cleanup_portsets: + isc_portset_destroy(isc_g_mctx, &v6portset); + isc_portset_destroy(isc_g_mctx, &v4portset); + +cleanup_bindkeys_parser: + if (bindkeys_parser != NULL) { + if (bindkeys != NULL) { + cfg_obj_destroy(bindkeys_parser, &bindkeys); + } + cfg_parser_destroy(&bindkeys_parser); + } + + if (exclusive) { + isc_loopmgr_resume(); + } + cleanup_viewlist: ISC_LIST_FOREACH(viewlist, view, link) { ISC_LIST_UNLINK(viewlist, view, link); @@ -9281,22 +9297,6 @@ cleanup_keystorelist: dns_keystore_detach(&keystore); } -cleanup_portsets: - isc_portset_destroy(isc_g_mctx, &v6portset); - isc_portset_destroy(isc_g_mctx, &v4portset); - -cleanup_bindkeys_parser: - if (bindkeys_parser != NULL) { - if (bindkeys != NULL) { - cfg_obj_destroy(bindkeys_parser, &bindkeys); - } - cfg_parser_destroy(&bindkeys_parser); - } - - if (exclusive) { - isc_loopmgr_resume(); - } - cleanup_aclconfctx: if (aclconfctx != NULL) { cfg_aclconfctx_detach(&aclconfctx); From c225ba17c29b5f072fa43a33fd888fcc0cfd4b83 Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Tue, 9 Sep 2025 15:41:17 +0200 Subject: [PATCH 08/10] creation of client TLS ctx before exclusive mode When the server is configured (inside `apply_configuration`) a client TLS context cache is created and attached to the global server object. It is then used by `configure_view` flow (and also during runtime though the zone manager). It is now created before the exclusive mode, and the swap of the previous TLS cache ctx is done at the end of the exclusive mode, if everything went well. This allows us (among other follow-up changes) to move the `configure_views` function outside of the exclusive mode. --- bin/named/server.c | 64 ++++++++++++++++++++++++++++------------------ 1 file changed, 39 insertions(+), 25 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 37cc1dea6a..ef0a7451f5 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -3795,7 +3795,8 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, cfg_obj_t *vconfig, named_cachelist_t *cachelist, named_cachelist_t *oldcachelist, dns_kasplist_t *kasplist, const cfg_obj_t *bindkeys, isc_mem_t *mctx, - cfg_aclconfctx_t *actx, bool need_hints, bool first_time) { + cfg_aclconfctx_t *actx, isc_tlsctx_cache_t *tlsctx_client_cache, + bool need_hints, bool first_time) { const cfg_obj_t *maps[4]; const cfg_obj_t *cfgmaps[3]; const cfg_obj_t *optionmaps[3]; @@ -4506,8 +4507,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, goto cleanup; } - CHECK(dns_view_createresolver(view, resopts, - named_g_server->tlsctx_client_cache, + CHECK(dns_view_createresolver(view, resopts, tlsctx_client_cache, dispatch4, dispatch6)); if (resstats == NULL) { @@ -7896,9 +7896,11 @@ create_views(cfg_obj_t *config, cfg_parser_t *parser, static isc_result_t configure_views(cfg_obj_t *config, const cfg_obj_t *bindkeys, - cfg_aclconfctx_t *aclconfctx, dns_viewlist_t *viewlist, - named_cachelist_t *cachelist, dns_kasplist_t *kasplist, - named_server_t *server, bool first_time) { + cfg_aclconfctx_t *aclconfctx, + isc_tlsctx_cache_t *tlsctx_client_cache, + dns_viewlist_t *viewlist, named_cachelist_t *cachelist, + dns_kasplist_t *kasplist, named_server_t *server, + bool first_time) { isc_result_t result = ISC_R_SUCCESS; const cfg_obj_t *views = NULL; dns_viewlist_t tmpviewlist; @@ -7920,8 +7922,8 @@ configure_views(cfg_obj_t *config, const cfg_obj_t *bindkeys, result = configure_view(view, viewlist, config, vconfig, cachelist, &server->cachelist, kasplist, - bindkeys, isc_g_mctx, aclconfctx, true, - first_time); + bindkeys, isc_g_mctx, aclconfctx, + tlsctx_client_cache, true, first_time); if (result != ISC_R_SUCCESS) { dns_view_detach(&view); return result; @@ -7942,8 +7944,8 @@ configure_views(cfg_obj_t *config, const cfg_obj_t *bindkeys, } result = configure_view(view, viewlist, config, NULL, cachelist, &server->cachelist, kasplist, bindkeys, - isc_g_mctx, aclconfctx, true, - first_time); + isc_g_mctx, aclconfctx, + tlsctx_client_cache, true, first_time); if (result != ISC_R_SUCCESS) { dns_view_detach(&view); return result; @@ -7969,8 +7971,8 @@ configure_views(cfg_obj_t *config, const cfg_obj_t *bindkeys, result = configure_view(view, viewlist, config, vconfig, cachelist, &server->cachelist, kasplist, - bindkeys, isc_g_mctx, aclconfctx, false, - first_time); + bindkeys, isc_g_mctx, aclconfctx, + tlsctx_client_cache, false, first_time); if (result != ISC_R_SUCCESS) { dns_view_detach(&view); return result; @@ -8128,6 +8130,7 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, dns_aclenv_t *env = ns_interfacemgr_getaclenv(named_g_server->interfacemgr); cfg_aclconfctx_t *tmpaclconfctx, *aclconfctx = NULL; + isc_tlsctx_cache_t *tlsctx_client_cache = NULL; isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_DEBUG(1), "apply_configuration"); @@ -8176,6 +8179,9 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, goto cleanup_viewlist; } + /* Create a new client TLS context cache */ + isc_tlsctx_cache_create(isc_g_mctx, &tlsctx_client_cache); + /* Ensure exclusive access to configuration data. */ isc_loopmgr_pause(); @@ -8197,22 +8203,13 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, } } - /* Let's recreate the TLS context cache */ + /* Let's recreate the server TLS context cache */ if (server->tlsctx_server_cache != NULL) { isc_tlsctx_cache_detach(&server->tlsctx_server_cache); } isc_tlsctx_cache_create(isc_g_mctx, &server->tlsctx_server_cache); - if (server->tlsctx_client_cache != NULL) { - isc_tlsctx_cache_detach(&server->tlsctx_client_cache); - } - - isc_tlsctx_cache_create(isc_g_mctx, &server->tlsctx_client_cache); - - dns_zonemgr_set_tlsctx_cache(server->zonemgr, - server->tlsctx_client_cache); - #if HAVE_LIBNGHTTP2 obj = NULL; result = named_config_get(maps, "http-port", &obj); @@ -8786,8 +8783,9 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, */ (void)configure_session_key(maps, server, isc_g_mctx, first_time); - result = configure_views(config, bindkeys, aclconfctx, &viewlist, - &cachelist, &kasplist, server, first_time); + result = configure_views(config, bindkeys, aclconfctx, + tlsctx_client_cache, &viewlist, &cachelist, + &kasplist, server, first_time); if (result != ISC_R_SUCCESS) { goto cleanup_cachelist; } @@ -9187,6 +9185,17 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, server->aclconfctx = aclconfctx; aclconfctx = tmpaclconfctx; + /* + * Swap client TLS context + */ + if (server->tlsctx_client_cache != NULL) { + isc_tlsctx_cache_detach(&server->tlsctx_client_cache); + } + + isc_tlsctx_cache_attach(tlsctx_client_cache, + &server->tlsctx_client_cache); + dns_zonemgr_set_tlsctx_cache(server->zonemgr, tlsctx_client_cache); + (void)named_server_loadnta(server); /* @@ -9225,7 +9234,6 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, goto cleanup_altsecrets; } - (void)ns_interfacemgr_scan(server->interfacemgr, true, true); /* @@ -9274,6 +9282,12 @@ cleanup_bindkeys_parser: isc_loopmgr_resume(); } + /* + * Detach the TLS client context (whether the one created at the + * begining of this function, or the previous running one) + */ + isc_tlsctx_cache_detach(&tlsctx_client_cache); + cleanup_viewlist: ISC_LIST_FOREACH(viewlist, view, link) { ISC_LIST_UNLINK(viewlist, view, link); From 3fe239e5cfa859557ed63fa2d08ffb9a484e265c Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Wed, 10 Sep 2025 15:17:11 +0200 Subject: [PATCH 09/10] apply_configuration: log subroutines for tests In order to have a (minimal) test ensuring we don't move back `apply_configuration` subroutines which can be done before the exclusive lock is taken, `APPLY_CONFIGURATION_SUBROUTINE_LOG` macro is added and used for the few subroutines already extracted from the exclusive mode. Those expected logs are added in `configloading` system test checks. --- bin/named/server.c | 12 ++++++++++++ .../system/configloading/tests_configloading.py | 8 ++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index ef0a7451f5..fad8716f80 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -7847,12 +7847,18 @@ cleanup: #endif /* HAVE_LMDB */ +#define APPLY_CONFIGURATION_SUBROUTINE_LOG \ + isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, \ + ISC_LOG_DEBUG(1), "apply_configuration: %s", __func__); + static isc_result_t create_views(cfg_obj_t *config, cfg_parser_t *parser, cfg_aclconfctx_t *aclconfctx, dns_viewlist_t *viewlist) { isc_result_t result = ISC_R_SUCCESS; const cfg_obj_t *views = NULL; + APPLY_CONFIGURATION_SUBROUTINE_LOG; + (void)cfg_map_get(config, "view", &views); CFG_LIST_FOREACH(views, element) { cfg_obj_t *vconfig = cfg_listelt_value(element); @@ -7905,6 +7911,8 @@ configure_views(cfg_obj_t *config, const cfg_obj_t *bindkeys, const cfg_obj_t *views = NULL; dns_viewlist_t tmpviewlist; + APPLY_CONFIGURATION_SUBROUTINE_LOG; + /* * Configure and freeze all explicit views. Explicit * views that have zones were already created at parsing @@ -8010,6 +8018,8 @@ configure_keystores(const cfg_obj_t *config, dns_keystorelist_t *keystorelist) { isc_result_t result = ISC_R_SUCCESS; const cfg_obj_t *keystores = NULL; + APPLY_CONFIGURATION_SUBROUTINE_LOG; + /* * Create the built-in key store ("key-directory"). */ @@ -8043,6 +8053,8 @@ configure_kasplist(const cfg_obj_t *config, dns_kasplist_t *kasplist, dns_kasp_t *default_kasp = NULL; const cfg_obj_t *kasps = NULL; + APPLY_CONFIGURATION_SUBROUTINE_LOG; + /* * Create the built-in kasp policies ("default", "insecure"). */ diff --git a/bin/tests/system/configloading/tests_configloading.py b/bin/tests/system/configloading/tests_configloading.py index f0a136cc36..92a1907741 100644 --- a/bin/tests/system/configloading/tests_configloading.py +++ b/bin/tests/system/configloading/tests_configloading.py @@ -13,8 +13,9 @@ def test_configloading_log(ns1): """ This test is a "guard/warning" to make sure the named.conf loading - (parsing) is done outside of the exclusive mode (so, named is still able to - answer queries and operating normally in case of configuration reload). It + (parsing), keystore building, kasplist building and view creation is done + outside of the exclusive mode (so, named is still able to answer queries + and operating normally in case of configuration reload). It is currently based on logging, so it's quite brittle. """ @@ -22,6 +23,9 @@ def test_configloading_log(ns1): "load_configuration", "parsing user configuration from ", "apply_configuration", + "apply_configuration: configure_keystores", + "apply_configuration: configure_kasplist", + "apply_configuration: create_views", "loop exclusive mode: starting", ] From 17a2cbcbc5a8ca4b9b8168fb0dd3dd8ae39e4128 Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Tue, 16 Sep 2025 15:49:42 +0200 Subject: [PATCH 10/10] comment about ifs scan twice the first time Add comment message about why we're scanning interfaces twice during the initial configuration (FreeBSD compatibility). See #3583 --- bin/named/server.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/bin/named/server.c b/bin/named/server.c index fad8716f80..bb46d6e48c 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -8696,6 +8696,12 @@ apply_configuration(cfg_parser_t *configparser, cfg_obj_t *config, * Rescan the interface list to pick up changes in the * listen-on option. This requires the loopmgr to be * temporarily resumed. + * + * The reason we're doing this the first time (instead of having + * only one scan later) is because we're are dropping root + * privileges shortly after and FreeBSD doesn't have Linux + * capabilities so can't listen to a privileged port without + * being root. */ isc_loopmgr_resume(); result = ns_interfacemgr_scan(server->interfacemgr, true, true);