From 529fd913db81a093ddb05d438afc1109cd80d5be Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Tue, 26 Aug 2025 15:05:55 +0200 Subject: [PATCH] move keystores handle from the zone to the view The list of keystores is owned by the single server object (named_g_server), but dns_zone_t has a pointer into it in order to preserve encapsulation (lib/dns won't link to bin/named for good reasons). However, getting the keystores from the zone uses the zone lock whereas this is not needed (as the pointer value doesn't depends on the zone, and is initialized only with the same named_g_server->keystores value); also storing an extra pointer per zone is not needed; also, there was a logic based on the zone->secure property which was not needed (as there is only one keystore). The keystores pointer is now accessible and lock-free at view level, it also simplifies a bit the various zone configuration APIs (server.c, zoneconf.c). --- bin/named/include/named/zoneconf.h | 4 +-- bin/named/server.c | 42 ++++++++++++++---------------- bin/named/zoneconf.c | 6 ++--- lib/dns/include/dns/view.h | 25 +++++++++--------- lib/dns/include/dns/zone.h | 23 ---------------- lib/dns/update.c | 2 +- lib/dns/zone.c | 37 +++++--------------------- 7 files changed, 43 insertions(+), 96 deletions(-) diff --git a/bin/named/include/named/zoneconf.h b/bin/named/include/named/zoneconf.h index e96be44d3e..ed0735ec51 100644 --- a/bin/named/include/named/zoneconf.h +++ b/bin/named/include/named/zoneconf.h @@ -25,8 +25,8 @@ isc_result_t named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig, const cfg_obj_t *zconfig, cfg_aclconfctx_t *ac, - dns_kasplist_t *kasplist, dns_keystorelist_t *keystores, - dns_zone_t *zone, dns_zone_t *raw); + dns_kasplist_t *kasplist, dns_zone_t *zone, + dns_zone_t *raw); /*%< * Configure or reconfigure a zone according to the named.conf * data. diff --git a/bin/named/server.c b/bin/named/server.c index b10d015f82..07a1e6d6e3 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -494,8 +494,8 @@ 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, - dns_keystorelist_t *keystores, cfg_aclconfctx_t *aclconf, - bool added, bool old_rpz_ok, bool is_catz_member, bool modify); + cfg_aclconfctx_t *aclconf, bool added, bool old_rpz_ok, + bool is_catz_member, bool modify); static void configure_zone_setviewcommit(isc_result_t result, const cfg_obj_t *zconfig, @@ -2491,8 +2491,7 @@ catz_addmodzone_cb(void *arg) { dns_view_thaw(cz->view); result = configure_zone(cfg->config, zoneobj, cfg->vconfig, cz->view, &cz->cbd->server->viewlist, - &cz->cbd->server->kasplist, - &cz->cbd->server->keystorelist, cfg->actx, true, + &cz->cbd->server->kasplist, cfg->actx, true, false, true, cz->mod); dns_view_freeze(cz->view); isc_loopmgr_resume(); @@ -2767,9 +2766,8 @@ catz_reconfigure(dns_catz_entry_t *entry, void *arg1, void *arg2) { result = configure_zone(data->config, zoneobj, cfg->vconfig, view, &data->cbd->server->viewlist, - &data->cbd->server->kasplist, - &data->cbd->server->keystorelist, cfg->actx, - true, false, true, true); + &data->cbd->server->kasplist, cfg->actx, true, + false, true, true); if (result != ISC_R_SUCCESS) { isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR, @@ -3836,6 +3834,8 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, REQUIRE(DNS_VIEW_VALID(view)); + view->keystores = keystores; + if (config != NULL) { (void)cfg_map_get(config, "options", &options); } @@ -3915,8 +3915,8 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, CFG_LIST_FOREACH(zonelist, element) { const cfg_obj_t *zconfig = cfg_listelt_value(element); CHECK(configure_zone(config, zconfig, vconfig, view, viewlist, - kasplist, keystores, actx, false, - old_rpz_ok, false, false)); + kasplist, actx, false, old_rpz_ok, false, + false)); zone_element_latest = element; } @@ -6228,8 +6228,8 @@ 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, - dns_keystorelist_t *keystores, cfg_aclconfctx_t *aclconf, - bool added, bool old_rpz_ok, bool is_catz_member, bool modify) { + cfg_aclconfctx_t *aclconf, 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 */ dns_zone_t *raw = NULL; /* New or reused raw zone */ @@ -6429,7 +6429,7 @@ configure_zone(const cfg_obj_t *config, const cfg_obj_t *zconfig, dns_zone_setstats(zone, named_g_server->zonestats); } CHECK(named_zone_configure(config, vconfig, zconfig, aclconf, - kasplist, keystores, zone, NULL)); + kasplist, zone, NULL)); dns_zone_attach(zone, &view->redirect); goto cleanup; } @@ -6605,7 +6605,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, - keystores, zone, raw)); + zone, raw)); /* * Add the zone to its view in the new view list. @@ -7499,8 +7499,7 @@ configure_newzones(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, const cfg_obj_t *zconfig = cfg_listelt_value(element); CHECK(configure_zone(config, zconfig, vconfig, view, &named_g_server->viewlist, - &named_g_server->kasplist, - &named_g_server->keystorelist, actx, true, + &named_g_server->kasplist, actx, true, false, false, false)); } @@ -7683,8 +7682,7 @@ configure_newzone(const cfg_obj_t *zconfig, cfg_obj_t *config, cfg_aclconfctx_t *actx) { return configure_zone( config, zconfig, vconfig, view, &named_g_server->viewlist, - &named_g_server->kasplist, &named_g_server->keystorelist, actx, - true, false, false, false); + &named_g_server->kasplist, actx, true, false, false, false); } /*% @@ -13180,9 +13178,8 @@ do_addzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, /* Mark view unfrozen and configure zone */ dns_view_thaw(view); result = configure_zone(cfg->config, zoneobj, cfg->vconfig, view, - &server->viewlist, &server->kasplist, - &server->keystorelist, cfg->actx, true, false, - false, false); + &server->viewlist, &server->kasplist, cfg->actx, + true, false, false, false); dns_view_freeze(view); isc_loopmgr_resume(); @@ -13378,9 +13375,8 @@ do_modzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, /* Reconfigure the zone */ dns_view_thaw(view); result = configure_zone(cfg->config, zoneobj, cfg->vconfig, view, - &server->viewlist, &server->kasplist, - &server->keystorelist, cfg->actx, true, false, - false, true); + &server->viewlist, &server->kasplist, cfg->actx, + true, false, false, true); dns_view_freeze(view); isc_loopmgr_resume(); diff --git a/bin/named/zoneconf.c b/bin/named/zoneconf.c index beba0aba46..2ebbb94843 100644 --- a/bin/named/zoneconf.c +++ b/bin/named/zoneconf.c @@ -870,8 +870,8 @@ process_notifytype(dns_notifytype_t ntype, dns_zonetype_t ztype, isc_result_t named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig, const cfg_obj_t *zconfig, cfg_aclconfctx_t *ac, - dns_kasplist_t *kasplist, dns_keystorelist_t *keystorelist, - dns_zone_t *zone, dns_zone_t *raw) { + dns_kasplist_t *kasplist, dns_zone_t *zone, + dns_zone_t *raw) { isc_result_t result; const char *zname; dns_rdataclass_t zclass; @@ -1591,8 +1591,6 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig, filename = cfg_obj_asstring(obj); dns_zone_setkeydirectory(zone, filename); } - /* Also save a reference to the keystore list. */ - dns_zone_setkeystores(zone, keystorelist); obj = NULL; result = named_config_get(maps, "sig-signing-signatures", &obj); diff --git a/lib/dns/include/dns/view.h b/lib/dns/include/dns/view.h index d536cccbd3..f6b9f71841 100644 --- a/lib/dns/include/dns/view.h +++ b/lib/dns/include/dns/view.h @@ -79,18 +79,19 @@ struct dns_view { /* Unlocked. */ - unsigned int magic; - isc_mem_t *mctx; - dns_rdataclass_t rdclass; - char *name; - dns_zt_t *zonetable; - dns_resolver_t *resolver; - dns_adb_t *adb; - dns_requestmgr_t *requestmgr; - dns_dispatchmgr_t *dispatchmgr; - dns_cache_t *cache; - dns_db_t *cachedb; - dns_db_t *hints; + unsigned int magic; + isc_mem_t *mctx; + dns_rdataclass_t rdclass; + char *name; + dns_zt_t *zonetable; + dns_resolver_t *resolver; + dns_adb_t *adb; + dns_requestmgr_t *requestmgr; + dns_dispatchmgr_t *dispatchmgr; + dns_cache_t *cache; + dns_db_t *cachedb; + dns_db_t *hints; + dns_keystorelist_t *keystores; /* * security roots and negative trust anchors. diff --git a/lib/dns/include/dns/zone.h b/lib/dns/include/dns/zone.h index 39c1183045..0497eb5733 100644 --- a/lib/dns/include/dns/zone.h +++ b/lib/dns/include/dns/zone.h @@ -1633,29 +1633,6 @@ dns_zone_getkeydirectory(dns_zone_t *zone); * Pointer to null-terminated file name, or NULL. */ -void -dns_zone_setkeystores(dns_zone_t *zone, dns_keystorelist_t *keystores); -/*%< - * Sets the keystore list where private keys used for - * online signing or dynamic zones are found. - * - * Require: - *\li 'zone' to be a valid zone. - */ - -dns_keystorelist_t * -dns_zone_getkeystores(dns_zone_t *zone); -/*%< - * Gets the keystore list where private keys used for - * online signing or dynamic zones are found. - * - * Require: - *\li 'zone' to be a valid zone. - * - * Returns: - * Pointer to the keystore list, or NULL. - */ - isc_result_t dns_zone_getdnsseckeys(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, isc_stdtime_t now, dns_dnsseckeylist_t *keys); diff --git a/lib/dns/update.c b/lib/dns/update.c index c5a749e503..911f6a3eba 100644 --- a/lib/dns/update.c +++ b/lib/dns/update.c @@ -1031,7 +1031,7 @@ find_zone_keys(dns_zone_t *zone, isc_mem_t *mctx, unsigned int maxkeys, kasp = dns_zone_getkasp(zone); keydir = dns_zone_getkeydirectory(zone); - keystores = dns_zone_getkeystores(zone); + keystores = dns_zone_getview(zone)->keystores; dns_zone_lock_keyfiles(zone); result = dns_dnssec_findmatchingkeys(dns_zone_getorigin(zone), kasp, diff --git a/lib/dns/zone.c b/lib/dns/zone.c index edd6f5d143..7277d9849e 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -335,7 +335,6 @@ struct dns_zone { isc_stdtime_t log_key_expired_timer; char *keydirectory; dns_keyfileio_t *kfio; - dns_keystorelist_t *keystores; dns_xfrin_t *xfr; uint32_t maxrefresh; @@ -6854,8 +6853,9 @@ dns_zone_getdnsseckeys(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, /* Get keys from private key files. */ dns_zone_lock_keyfiles(zone); - result = dns_dnssec_findmatchingkeys(origin, kasp, dir, zone->keystores, - now, dns_zone_getmctx(zone), keys); + result = dns_dnssec_findmatchingkeys(origin, kasp, dir, + zone->view->keystores, now, + dns_zone_getmctx(zone), keys); dns_zone_unlock_keyfiles(zone); if (result != ISC_R_SUCCESS && result != ISC_R_NOTFOUND) { @@ -16693,7 +16693,7 @@ dns_zone_dnskey_inuse(dns_zone_t *zone, dns_rdata_t *rdata, bool *inuse) { kasp = dns_zone_getkasp(zone); keydir = dns_zone_getkeydirectory(zone); - keystores = dns_zone_getkeystores(zone); + keystores = zone->view->keystores; dns_zone_lock_keyfiles(zone); result = dns_dnssec_findmatchingkeys(dns_zone_getorigin(zone), kasp, @@ -20004,32 +20004,6 @@ dns_zone_getkeydirectory(dns_zone_t *zone) { return zone->keydirectory; } -void -dns_zone_setkeystores(dns_zone_t *zone, dns_keystorelist_t *keystores) { - REQUIRE(DNS_ZONE_VALID(zone)); - - LOCK_ZONE(zone); - zone->keystores = keystores; - UNLOCK_ZONE(zone); -} - -dns_keystorelist_t * -dns_zone_getkeystores(dns_zone_t *zone) { - dns_keystorelist_t *ks = NULL; - - REQUIRE(DNS_ZONE_VALID(zone)); - - LOCK_ZONE(zone); - if (inline_raw(zone) && zone->secure != NULL) { - ks = zone->secure->keystores; - } else { - ks = zone->keystores; - } - UNLOCK_ZONE(zone); - - return ks; -} - unsigned int dns_zonemgr_getcount(dns_zonemgr_t *zmgr, dns_zonestate_t state) { unsigned int count = 0; @@ -22418,7 +22392,8 @@ zone_rekey(dns_zone_t *zone) { dns_zone_lock_keyfiles(zone); result = dns_dnssec_findmatchingkeys(&zone->origin, kasp, dir, - zone->keystores, now, mctx, &keys); + zone->view->keystores, now, mctx, + &keys); dns_zone_unlock_keyfiles(zone); if (result != ISC_R_SUCCESS) {