From 8685ebdb9920dfc2a5612d874cf2040d3592a0b1 Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Tue, 22 Apr 2025 13:46:47 +0200 Subject: [PATCH 1/5] renamed named_g_config/defaults For better clarity, the global variables named_g_config and named_g_defaults have been changed to named_g_defaultconfig and named_g_defaultoptions. --- bin/named/config.c | 2 +- bin/named/include/named/globals.h | 32 +++++------ bin/named/server.c | 91 ++++++++++++++++--------------- bin/named/statschannel.c | 4 +- bin/named/zoneconf.c | 6 +- 5 files changed, 70 insertions(+), 65 deletions(-) diff --git a/bin/named/config.c b/bin/named/config.c index cc36edad98..59ca2883cb 100644 --- a/bin/named/config.c +++ b/bin/named/config.c @@ -890,7 +890,7 @@ named_config_getport(const cfg_obj_t *config, const char *type, if (options != NULL) { maps[i++] = options; } - maps[i++] = named_g_defaults; + maps[i++] = named_g_defaultoptions; maps[i] = NULL; result = named_config_get(maps, type, &portobj); diff --git a/bin/named/include/named/globals.h b/bin/named/include/named/globals.h index 9713859fff..328928e78b 100644 --- a/bin/named/include/named/globals.h +++ b/bin/named/include/named/globals.h @@ -64,19 +64,19 @@ EXTERN bool named_g_run_done INIT(false); * for really short timers, another for client timers, and one * for zone timers. */ -EXTERN isc_nm_t *named_g_netmgr INIT(NULL); -EXTERN cfg_parser_t *named_g_parser INIT(NULL); -EXTERN cfg_parser_t *named_g_addparser INIT(NULL); -EXTERN const char *named_g_version INIT(PACKAGE_VERSION); -EXTERN const char *named_g_product INIT(PACKAGE_NAME); -EXTERN const char *named_g_description INIT(PACKAGE_DESCRIPTION); -EXTERN const char *named_g_srcid INIT(PACKAGE_SRCID); -EXTERN const char *named_g_configargs INIT(PACKAGE_CONFIGARGS); -EXTERN const char *named_g_builder INIT(PACKAGE_BUILDER); -EXTERN in_port_t named_g_port INIT(0); -EXTERN in_port_t named_g_tlsport INIT(0); -EXTERN in_port_t named_g_httpsport INIT(0); -EXTERN in_port_t named_g_httpport INIT(0); +EXTERN isc_nm_t *named_g_netmgr INIT(NULL); +EXTERN cfg_parser_t *named_g_parser INIT(NULL); +EXTERN cfg_parser_t *named_g_addparser INIT(NULL); +EXTERN const char *named_g_version INIT(PACKAGE_VERSION); +EXTERN const char *named_g_product INIT(PACKAGE_NAME); +EXTERN const char *named_g_description INIT(PACKAGE_DESCRIPTION); +EXTERN const char *named_g_srcid INIT(PACKAGE_SRCID); +EXTERN const char *named_g_defaultconfigargs INIT(PACKAGE_CONFIGARGS); +EXTERN const char *named_g_builder INIT(PACKAGE_BUILDER); +EXTERN in_port_t named_g_port INIT(0); +EXTERN in_port_t named_g_tlsport INIT(0); +EXTERN in_port_t named_g_httpsport INIT(0); +EXTERN in_port_t named_g_httpport INIT(0); EXTERN in_port_t named_g_http_listener_clients INIT(0); EXTERN in_port_t named_g_http_streams_per_conn INIT(0); @@ -91,8 +91,8 @@ EXTERN unsigned int named_g_debuglevel INIT(0); /* * Current configuration information. */ -EXTERN cfg_obj_t *named_g_config INIT(NULL); -EXTERN const cfg_obj_t *named_g_defaults INIT(NULL); +EXTERN cfg_obj_t *named_g_defaultconfig INIT(NULL); +EXTERN const cfg_obj_t *named_g_defaultoptions INIT(NULL); 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"); @@ -126,7 +126,7 @@ EXTERN const char *named_g_defaultpidfile INIT(NAMED_LOCALSTATEDIR "/run/" EXTERN const char *named_g_username INIT(NULL); EXTERN isc_time_t named_g_boottime; -EXTERN isc_time_t named_g_configtime; +EXTERN isc_time_t named_g_defaultconfigtime; EXTERN bool named_g_memstatistics INIT(false); EXTERN bool named_g_keepstderr INIT(false); diff --git a/bin/named/server.c b/bin/named/server.c index 6cbfc245c8..9831f24c68 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -1131,7 +1131,7 @@ configure_view_dnsseckeys(dns_view_t *view, const cfg_obj_t *vconfig, } } - maps[i++] = named_g_defaults; + maps[i++] = named_g_defaultoptions; maps[i] = NULL; dns_view_initsecroots(view); @@ -1142,7 +1142,7 @@ configure_view_dnsseckeys(dns_view_t *view, const cfg_obj_t *vconfig, /* * If bind.keys exists and is populated, it overrides - * the trust-anchors clause hard-coded in named_g_config. + * the trust-anchors clause hard-coded in named_g_defaultconfig. */ if (bindkeys != NULL) { isc_log_write(DNS_LOGCATEGORY_SECURITY, @@ -1170,8 +1170,8 @@ configure_view_dnsseckeys(dns_view_t *view, const cfg_obj_t *vconfig, "using built-in root key for view %s", view->name); - (void)cfg_map_get(named_g_config, "trust-anchors", - &builtin_keys); + (void)cfg_map_get(named_g_defaultconfig, + "trust-anchors", &builtin_keys); } if (builtin_keys != NULL) { @@ -3839,7 +3839,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, optionmaps[j++] = options; } - maps[i++] = named_g_defaults; + maps[i++] = named_g_defaultoptions; maps[i] = NULL; optionmaps[j] = NULL; if (config != NULL) { @@ -4211,13 +4211,14 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, view->acceptexpired = cfg_obj_asboolean(obj); obj = NULL; - /* 'optionmaps', not 'maps': don't check named_g_defaults yet */ + /* 'optionmaps', not 'maps': don't check named_g_defaultoptions yet */ (void)named_config_get(optionmaps, "dnssec-validation", &obj); if (obj == NULL) { /* * Default to VALIDATION_DEFAULT as set in config.c. */ - (void)cfg_map_get(named_g_defaults, "dnssec-validation", &obj); + (void)cfg_map_get(named_g_defaultoptions, "dnssec-validation", + &obj); INSIST(obj != NULL); } if (obj != NULL) { @@ -4936,14 +4937,15 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, CHECK(configure_view_acl(vconfig, config, NULL, "allow-query-cache-on", NULL, actx, named_g_mctx, &view->cacheonacl)); - CHECK(configure_view_acl(vconfig, config, named_g_config, + CHECK(configure_view_acl(vconfig, config, named_g_defaultconfig, "allow-query-on", NULL, actx, named_g_mctx, &view->queryonacl)); - CHECK(configure_view_acl(vconfig, config, named_g_config, "allow-proxy", - NULL, actx, named_g_mctx, &view->proxyacl)); + CHECK(configure_view_acl(vconfig, config, named_g_defaultconfig, + "allow-proxy", NULL, actx, named_g_mctx, + &view->proxyacl)); - CHECK(configure_view_acl(vconfig, config, named_g_config, + CHECK(configure_view_acl(vconfig, config, named_g_defaultconfig, "allow-proxy-on", NULL, actx, named_g_mctx, &view->proxyonacl)); @@ -5018,28 +5020,30 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, if (view->recursionacl == NULL) { /* global default only */ CHECK(configure_view_acl( - NULL, NULL, named_g_config, "allow-recursion", - NULL, actx, named_g_mctx, &view->recursionacl)); + NULL, NULL, named_g_defaultconfig, + "allow-recursion", NULL, actx, named_g_mctx, + &view->recursionacl)); } if (view->recursiononacl == NULL) { /* global default only */ - CHECK(configure_view_acl(NULL, NULL, named_g_config, - "allow-recursion-on", NULL, - actx, named_g_mctx, - &view->recursiononacl)); + CHECK(configure_view_acl( + NULL, NULL, named_g_defaultconfig, + "allow-recursion-on", NULL, actx, named_g_mctx, + &view->recursiononacl)); } if (view->cacheacl == NULL) { /* global default only */ CHECK(configure_view_acl( - NULL, NULL, named_g_config, "allow-query-cache", - NULL, actx, named_g_mctx, &view->cacheacl)); + NULL, NULL, named_g_defaultconfig, + "allow-query-cache", NULL, actx, named_g_mctx, + &view->cacheacl)); } if (view->cacheonacl == NULL) { /* global default only */ - CHECK(configure_view_acl(NULL, NULL, named_g_config, - "allow-query-cache-on", NULL, - actx, named_g_mctx, - &view->cacheonacl)); + CHECK(configure_view_acl( + NULL, NULL, named_g_defaultconfig, + "allow-query-cache-on", NULL, actx, + named_g_mctx, &view->cacheonacl)); } } else { /* @@ -5060,7 +5064,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, */ if (view->queryacl == NULL) { /* global default only */ - CHECK(configure_view_acl(NULL, NULL, named_g_config, + CHECK(configure_view_acl(NULL, NULL, named_g_defaultconfig, "allow-query", NULL, actx, named_g_mctx, &view->queryacl)); } @@ -5070,7 +5074,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, * clients. This causes case not always to be preserved, * and is needed by some broken clients. */ - CHECK(configure_view_acl(vconfig, config, named_g_config, + CHECK(configure_view_acl(vconfig, config, named_g_defaultconfig, "no-case-compress", NULL, actx, named_g_mctx, &view->nocasecompress)); @@ -5086,7 +5090,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, /* * Filter setting on addresses in the answer section. */ - CHECK(configure_view_acl(vconfig, config, named_g_config, + CHECK(configure_view_acl(vconfig, config, named_g_defaultconfig, "deny-answer-addresses", "acl", actx, named_g_mctx, &view->denyansweracl)); CHECK(configure_view_nametable(vconfig, config, "deny-answer-addresses", @@ -5110,12 +5114,12 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, * read from there in zoneconf.c:configure_zone_acl() later.) */ if (view->updateacl == NULL) { - CHECK(configure_view_acl(NULL, NULL, named_g_config, + CHECK(configure_view_acl(NULL, NULL, named_g_defaultconfig, "allow-update", NULL, actx, named_g_mctx, &view->updateacl)); } if (view->upfwdacl == NULL) { - CHECK(configure_view_acl(NULL, NULL, named_g_config, + CHECK(configure_view_acl(NULL, NULL, named_g_defaultconfig, "allow-update-forwarding", NULL, actx, named_g_mctx, &view->upfwdacl)); } @@ -5125,12 +5129,12 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, * can be inherited by zones. */ if (view->transferacl == NULL) { - CHECK(configure_view_acl(vconfig, config, named_g_config, + CHECK(configure_view_acl(vconfig, config, named_g_defaultconfig, "allow-transfer", NULL, actx, named_g_mctx, &view->transferacl)); } if (view->notifyacl == NULL) { - CHECK(configure_view_acl(vconfig, config, named_g_config, + CHECK(configure_view_acl(vconfig, config, named_g_defaultconfig, "allow-notify", NULL, actx, named_g_mctx, &view->notifyacl)); } @@ -7357,7 +7361,7 @@ setup_newzones(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, if (result == ISC_R_SUCCESS) { maps[i++] = options; } - maps[i++] = named_g_defaults; + maps[i++] = named_g_defaultoptions; maps[i] = NULL; result = named_config_get(maps, "allow-new-zones", &nz); @@ -7921,14 +7925,15 @@ load_configuration(const char *filename, named_server_t *server, */ if (first_time) { result = named_config_parsedefaults(named_g_parser, - &named_g_config); + &named_g_defaultconfig); if (result != ISC_R_SUCCESS) { named_main_earlyfatal("unable to load " "internal defaults: %s", isc_result_totext(result)); } - RUNTIME_CHECK(cfg_map_get(named_g_config, "options", - &named_g_defaults) == ISC_R_SUCCESS); + RUNTIME_CHECK(cfg_map_get(named_g_defaultconfig, "options", + &named_g_defaultoptions) == + ISC_R_SUCCESS); } /* @@ -8001,7 +8006,7 @@ load_configuration(const char *filename, named_server_t *server, if (result == ISC_R_SUCCESS) { maps[i++] = options; } - maps[i++] = named_g_defaults; + maps[i++] = named_g_defaultoptions; maps[i] = NULL; #if HAVE_LIBNGHTTP2 @@ -8609,7 +8614,7 @@ load_configuration(const char *filename, named_server_t *server, * Create the built-in kasp policies ("default", "insecure"). */ kasps = NULL; - (void)cfg_map_get(named_g_config, "dnssec-policy", &kasps); + (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; @@ -8770,8 +8775,8 @@ load_configuration(const char *filename, named_server_t *server, * Create (or recreate) the built-in views. */ builtin_views = NULL; - RUNTIME_CHECK(cfg_map_get(named_g_config, "view", &builtin_views) == - ISC_R_SUCCESS); + 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; @@ -9184,7 +9189,7 @@ load_configuration(const char *filename, named_server_t *server, /* * Record the time of most recent configuration */ - named_g_configtime = isc_time_now(); + named_g_defaultconfigtime = isc_time_now(); isc_loopmgr_resume(named_g_loopmgr); exclusive = false; @@ -9535,7 +9540,7 @@ shutdown_server(void *arg) { cfg_aclconfctx_detach(&named_g_aclconfctx); } - cfg_obj_destroy(named_g_parser, &named_g_config); + cfg_obj_destroy(named_g_parser, &named_g_defaultconfig); cfg_parser_destroy(&named_g_parser); cfg_parser_destroy(&named_g_addparser); @@ -11880,7 +11885,7 @@ named_server_status(named_server_t *server, isc_buffer_t **text) { isc_time_formathttptimestamp(&named_g_boottime, boottime, sizeof(boottime)); - isc_time_formathttptimestamp(&named_g_configtime, configtime, + isc_time_formathttptimestamp(&named_g_defaultconfigtime, configtime, sizeof(configtime)); snprintf(line, sizeof(line), "version: %s (%s) %s%s%s\n", @@ -13623,7 +13628,7 @@ named_server_changezone(named_server_t *server, char *command, addzone ? NAMED_COMMAND_ADDZONE : NAMED_COMMAND_MODZONE); /* Changing a zone counts as reconfiguration */ - named_g_configtime = isc_time_now(); + named_g_defaultconfigtime = isc_time_now(); cleanup: if (isc_buffer_usedlength(*text) > 0) { @@ -13934,7 +13939,7 @@ named_server_delzone(named_server_t *server, isc_lex_t *lex, zonename); /* Removing a zone counts as reconfiguration */ - named_g_configtime = isc_time_now(); + named_g_defaultconfigtime = isc_time_now(); result = ISC_R_SUCCESS; diff --git a/bin/named/statschannel.c b/bin/named/statschannel.c index 6edfcc6f56..e177c6cbab 100644 --- a/bin/named/statschannel.c +++ b/bin/named/statschannel.c @@ -1779,7 +1779,7 @@ generatexml(named_server_t *server, uint32_t flags, int *buflen, isc_result_t result; isc_time_formatISO8601ms(&named_g_boottime, boottime, sizeof boottime); - isc_time_formatISO8601ms(&named_g_configtime, configtime, + isc_time_formatISO8601ms(&named_g_defaultconfigtime, configtime, sizeof configtime); isc_time_formatISO8601ms(&now, nowstr, sizeof nowstr); @@ -2856,7 +2856,7 @@ generatejson(named_server_t *server, size_t *msglen, const char **msg, now = isc_time_now(); isc_time_formatISO8601ms(&named_g_boottime, boottime, sizeof(boottime)); - isc_time_formatISO8601ms(&named_g_configtime, configtime, + isc_time_formatISO8601ms(&named_g_defaultconfigtime, configtime, sizeof configtime); isc_time_formatISO8601ms(&now, nowstr, sizeof(nowstr)); diff --git a/bin/named/zoneconf.c b/bin/named/zoneconf.c index a0fe793c6f..d923318ca4 100644 --- a/bin/named/zoneconf.c +++ b/bin/named/zoneconf.c @@ -165,7 +165,7 @@ configure_zone_acl(const cfg_obj_t *zconfig, const cfg_obj_t *vconfig, maps[i++] = options; } } - maps[i++] = named_g_defaults; + maps[i++] = named_g_defaultoptions; maps[i] = NULL; (void)named_config_get(maps, aclname, &aclobj); @@ -946,7 +946,7 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig, } nodefault[i] = NULL; - maps[i++] = named_g_defaults; + maps[i++] = named_g_defaultoptions; maps[i] = NULL; if (vconfig != NULL) { @@ -1808,7 +1808,7 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig, dns_name_equal(dns_zone_getorigin(zone), dns_rootname)) { result = named_config_getremotesdef( - named_g_config, "remote-servers", + named_g_defaultconfig, "remote-servers", DEFAULT_IANA_ROOT_ZONE_PRIMARIES, &obj); CHECK(result); } From d7de2ba08477c97e64c00bfffe49be24df077860 Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Tue, 22 Apr 2025 13:46:47 +0200 Subject: [PATCH 2/5] parse user configuration before exclusive mode The configuration file was parsed when named was in exclusive (i.e. single-threaded) mode and unable to answer queries. Because the parsing is a self-contained operation, it is now done before named enters exclusive mode. This reduces the amount of time named can't answer queries when reloading the configuration when the configuration file is large. Note that exclusive mode is still used for applying the configuration changes to the server. Also, simplify the configuration logic by parsing the built-in configuration only once at server start time. --- bin/named/config.c | 95 ++++++++++++++++ bin/named/include/named/config.h | 3 + bin/named/server.c | 180 ++++++++++--------------------- 3 files changed, 156 insertions(+), 122 deletions(-) diff --git a/bin/named/config.c b/bin/named/config.c index 59ca2883cb..de180cf14d 100644 --- a/bin/named/config.c +++ b/bin/named/config.c @@ -18,6 +18,8 @@ #include #include +#include +#include #include #include #include @@ -38,6 +40,7 @@ #include +#include #include #include @@ -369,6 +372,98 @@ named_config_parsedefaults(cfg_parser_t *parser, cfg_obj_t **conf) { conf); } +/* + * This function is called as soon as the 'directory' statement has been + * parsed. This can be extended to support other options if necessary. + */ +static isc_result_t +directory_callback(const char *clausename, const cfg_obj_t *obj, void *arg) { + isc_result_t result; + const char *directory; + + REQUIRE(strcasecmp("directory", clausename) == 0); + + UNUSED(arg); + UNUSED(clausename); + + /* + * Change directory. + */ + directory = cfg_obj_asstring(obj); + + if (!isc_file_ischdiridempotent(directory)) { + cfg_obj_log(obj, ISC_LOG_WARNING, + "option 'directory' contains relative path '%s'", + directory); + } + + if (!isc_file_isdirwritable(directory)) { + isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, + ISC_LOG_ERROR, "directory '%s' is not writable", + directory); + return ISC_R_NOPERM; + } + + result = isc_dir_chdir(directory); + if (result != ISC_R_SUCCESS) { + cfg_obj_log(obj, ISC_LOG_ERROR, + "change directory to '%s' failed: %s", directory, + isc_result_totext(result)); + return result; + } + + char cwd[PATH_MAX]; + if (getcwd(cwd, sizeof(cwd)) == cwd) { + isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, + ISC_LOG_INFO, "the working directory is now '%s'", + cwd); + } + + return ISC_R_SUCCESS; +} + +isc_result_t +named_config_parsefile(cfg_parser_t *parser, cfg_obj_t **conf) { + isc_result_t result; + + REQUIRE(parser); + REQUIRE(conf && *conf == NULL); + + isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, + ISC_LOG_INFO, "parsing user configuration from '%s'", + named_g_conffile); + + cfg_parser_setcallback(parser, directory_callback, NULL); + result = cfg_parse_file(parser, named_g_conffile, &cfg_type_namedconf, + conf); + if (result != ISC_R_SUCCESS) { + goto cleanup; + } + + /* + * Check the validity of the configuration. + * + * (Ignore plugin parameters for now; they will be + * checked later when the modules are actually loaded and + * registered.) + */ + result = isccfg_check_namedconf(*conf, BIND_CHECK_ALGORITHMS, + named_g_mctx); + if (result != ISC_R_SUCCESS) { + goto cleanup; + } + + goto out; + +cleanup: + if (*conf) { + cfg_obj_destroy(parser, conf); + } + +out: + return result; +} + const char * named_config_getdefault(void) { return defaultconf; diff --git a/bin/named/include/named/config.h b/bin/named/include/named/config.h index 62f0e3c815..744f583423 100644 --- a/bin/named/include/named/config.h +++ b/bin/named/include/named/config.h @@ -27,6 +27,9 @@ isc_result_t named_config_parsedefaults(cfg_parser_t *parser, cfg_obj_t **conf); +isc_result_t +named_config_parsefile(cfg_parser_t *parser, cfg_obj_t **conf); + const char * named_config_getdefault(void); diff --git a/bin/named/server.c b/bin/named/server.c index 9831f24c68..d5be63179a 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -1142,7 +1142,8 @@ configure_view_dnsseckeys(dns_view_t *view, const cfg_obj_t *vconfig, /* * If bind.keys exists and is populated, it overrides - * the trust-anchors clause hard-coded in named_g_defaultconfig. + * the trust-anchors clause hard-coded in + * named_g_defaultconfig. */ if (bindkeys != NULL) { isc_log_write(DNS_LOGCATEGORY_SECURITY, @@ -6732,56 +6733,6 @@ configure_server_quota(const cfg_obj_t **maps, const char *name, isc_quota_max(quota, cfg_obj_asuint32(obj)); } -/* - * This function is called as soon as the 'directory' statement has been - * parsed. This can be extended to support other options if necessary. - */ -static isc_result_t -directory_callback(const char *clausename, const cfg_obj_t *obj, void *arg) { - isc_result_t result; - const char *directory; - - REQUIRE(strcasecmp("directory", clausename) == 0); - - UNUSED(arg); - UNUSED(clausename); - - /* - * Change directory. - */ - directory = cfg_obj_asstring(obj); - - if (!isc_file_ischdiridempotent(directory)) { - cfg_obj_log(obj, ISC_LOG_WARNING, - "option 'directory' contains relative path '%s'", - directory); - } - - if (!isc_file_isdirwritable(directory)) { - isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, - ISC_LOG_ERROR, "directory '%s' is not writable", - directory); - return ISC_R_NOPERM; - } - - result = isc_dir_chdir(directory); - if (result != ISC_R_SUCCESS) { - cfg_obj_log(obj, ISC_LOG_ERROR, - "change directory to '%s' failed: %s", directory, - isc_result_totext(result)); - return result; - } - - char cwd[PATH_MAX]; - if (getcwd(cwd, sizeof(cwd)) == cwd) { - isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, - ISC_LOG_INFO, "the working directory is now '%s'", - cwd); - } - - return ISC_R_SUCCESS; -} - /* * This event callback is invoked to do periodic network interface * scanning. @@ -7336,7 +7287,7 @@ cleanup: static isc_result_t setup_newzones(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, - cfg_parser_t *conf_parser, cfg_aclconfctx_t *actx) { + cfg_parser_t *config_parser, cfg_aclconfctx_t *actx) { isc_result_t result = ISC_R_SUCCESS; bool allow = false; ns_cfgctx_t *nzcfg = NULL; @@ -7439,12 +7390,11 @@ setup_newzones(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, *nzcfg = (ns_cfgctx_t){ 0 }; /* - * We attach the parser that was used for config as well - * as the one that will be used for added zones, to avoid + * We attach the parser that will be used for added zones to avoid * a shutdown race later. */ isc_mem_attach(view->mctx, &nzcfg->mctx); - cfg_parser_attach(conf_parser, &nzcfg->conf_parser); + cfg_parser_attach(config_parser, &nzcfg->conf_parser); cfg_parser_attach(named_g_addparser, &nzcfg->add_parser); cfg_aclconfctx_attach(actx, &nzcfg->actx); @@ -7452,8 +7402,8 @@ setup_newzones(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, mapsize); if (result != ISC_R_SUCCESS) { cfg_aclconfctx_detach(&nzcfg->actx); - cfg_parser_destroy(&nzcfg->add_parser); cfg_parser_destroy(&nzcfg->conf_parser); + cfg_parser_destroy(&nzcfg->add_parser); isc_mem_putanddetach(&nzcfg->mctx, nzcfg, sizeof(*nzcfg)); dns_view_setnewzones(view, false, NULL, NULL, 0ULL); return result; @@ -7850,10 +7800,10 @@ cleanup: #endif /* HAVE_LMDB */ static isc_result_t -load_configuration(const char *filename, named_server_t *server, - bool first_time) { - cfg_obj_t *config = NULL, *bindkeys = NULL; - cfg_parser_t *conf_parser = NULL, *bindkeys_parser = NULL; +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; @@ -7891,6 +7841,9 @@ load_configuration(const char *filename, named_server_t *server, dns_aclenv_t *env = ns_interfacemgr_getaclenv(named_g_server->interfacemgr); + isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, + ISC_LOG_DEBUG(1), "apply_configuration"); + /* * Require the reconfiguration to happen always on the main loop */ @@ -7920,22 +7873,6 @@ load_configuration(const char *filename, named_server_t *server, */ dns_dyndb_cleanup(); - /* - * Parse the global default pseudo-config file. - */ - if (first_time) { - result = named_config_parsedefaults(named_g_parser, - &named_g_defaultconfig); - if (result != ISC_R_SUCCESS) { - named_main_earlyfatal("unable to load " - "internal defaults: %s", - isc_result_totext(result)); - } - RUNTIME_CHECK(cfg_map_get(named_g_defaultconfig, "options", - &named_g_defaultoptions) == - ISC_R_SUCCESS); - } - /* * Log the current working directory. */ @@ -7949,38 +7886,6 @@ load_configuration(const char *filename, named_server_t *server, } } - /* - * Parse the configuration file using the new config code. - */ - config = NULL; - isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, - ISC_LOG_INFO, "loading configuration from '%s'", - filename); - result = cfg_parser_create(named_g_mctx, &conf_parser); - if (result != ISC_R_SUCCESS) { - goto cleanup_exclusive; - } - - cfg_parser_setcallback(conf_parser, directory_callback, NULL); - result = cfg_parse_file(conf_parser, filename, &cfg_type_namedconf, - &config); - if (result != ISC_R_SUCCESS) { - goto cleanup_conf_parser; - } - - /* - * Check the validity of the configuration. - * - * (Ignore plugin parameters for now; they will be - * checked later when the modules are actually loaded and - * registered.) - */ - result = isccfg_check_namedconf(config, BIND_CHECK_ALGORITHMS, - named_g_mctx); - if (result != ISC_R_SUCCESS) { - goto cleanup_config; - } - /* Let's recreate the TLS context cache */ if (server->tlsctx_server_cache != NULL) { isc_tlsctx_cache_detach(&server->tlsctx_server_cache); @@ -8052,7 +7957,7 @@ load_configuration(const char *filename, named_server_t *server, result = cfg_parser_create(named_g_mctx, &bindkeys_parser); if (result != ISC_R_SUCCESS) { - goto cleanup_config; + goto cleanup_bindkeys_parser; } result = cfg_parse_file(bindkeys_parser, @@ -8689,7 +8594,7 @@ load_configuration(const char *filename, named_server_t *server, } INSIST(view != NULL); - result = setup_newzones(view, config, vconfig, conf_parser, + result = setup_newzones(view, config, vconfig, configparser, named_g_aclconfctx); dns_view_detach(&view); @@ -8711,7 +8616,7 @@ load_configuration(const char *filename, named_server_t *server, } INSIST(view != NULL); - result = setup_newzones(view, config, NULL, conf_parser, + result = setup_newzones(view, config, NULL, configparser, named_g_aclconfctx); dns_view_detach(&view); @@ -9284,7 +9189,6 @@ cleanup_portsets: isc_portset_destroy(named_g_mctx, &v4portset); cleanup_bindkeys_parser: - if (bindkeys_parser != NULL) { if (bindkeys != NULL) { cfg_obj_destroy(bindkeys_parser, &bindkeys); @@ -9292,24 +9196,49 @@ cleanup_bindkeys_parser: cfg_parser_destroy(&bindkeys_parser); } -cleanup_config: - cfg_obj_destroy(conf_parser, &config); - -cleanup_conf_parser: - cfg_parser_destroy(&conf_parser); - cleanup_exclusive: if (exclusive) { isc_loopmgr_resume(named_g_loopmgr); } isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, - ISC_LOG_DEBUG(1), "load_configuration: %s", + ISC_LOG_DEBUG(1), "apply_configuration: %s", isc_result_totext(result)); return result; } +static isc_result_t +load_configuration(named_server_t *server, bool first_time) { + isc_result_t result; + cfg_parser_t *parser = NULL; + cfg_obj_t *config = NULL; + + isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, + ISC_LOG_DEBUG(1), "load_configuration"); + + result = cfg_parser_create(named_g_mctx, &parser); + if (result != ISC_R_SUCCESS) { + goto out; + } + + result = named_config_parsefile(parser, &config); + if (result != ISC_R_SUCCESS) { + goto cleanup; + } + + result = apply_configuration(parser, config, server, first_time); + +cleanup: + if (config) { + cfg_obj_destroy(parser, &config); + } + cfg_parser_destroy(&parser); + +out: + return result; +} + static isc_result_t view_loaded(void *arg) { isc_result_t result; @@ -9485,11 +9414,18 @@ run_server(void *arg) { CHECKFATAL(cfg_parser_create(named_g_mctx, &named_g_parser), "creating default configuration parser"); + CHECKFATAL(named_config_parsedefaults(named_g_parser, + &named_g_defaultconfig), + "unable to parse defaults config"); + + CHECKFATAL(cfg_map_get(named_g_defaultconfig, "options", + &named_g_defaultoptions), + "missing 'options' in default config"); + CHECKFATAL(cfg_parser_create(named_g_mctx, &named_g_addparser), "creating additional configuration parser"); - CHECKFATAL(load_configuration(named_g_conffile, server, true), - "loading configuration"); + CHECKFATAL(load_configuration(server, true), "loading configuration"); CHECKFATAL(load_zones(server, false), "loading zones"); #ifdef ENABLE_AFL @@ -9969,7 +9905,7 @@ fatal(const char *msg, isc_result_t result) { static isc_result_t loadconfig(named_server_t *server) { isc_result_t result; - result = load_configuration(named_g_conffile, server, false); + result = load_configuration(server, false); if (result == ISC_R_SUCCESS) { isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_INFO, From d7416bb472b14dc317cd0f2ee0a8fc3d4082623e Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Tue, 22 Apr 2025 15:41:39 +0200 Subject: [PATCH 3/5] ignore reload request if in a reload process Ignore an 'rndc reload' or 'rndc reconfig' command if received by named while the server is currently reloading itself. --- bin/named/control.c | 2 +- bin/named/include/named/server.h | 2 +- bin/named/server.c | 36 ++++++++++++++++++++++++++++---- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/bin/named/control.c b/bin/named/control.c index 10a4e00fc4..b5f7c49a80 100644 --- a/bin/named/control.c +++ b/bin/named/control.c @@ -252,7 +252,7 @@ named_control_docommand(isccc_sexpr_t *message, bool readonly, "query logging", NS_SERVER_LOGQUERIES, lex); } else if (command_compare(command, NAMED_COMMAND_RECONFIG)) { - result = named_server_reconfigcommand(named_g_server); + result = named_server_reconfigcommand(named_g_server, *text); } else if (command_compare(command, NAMED_COMMAND_RECURSING)) { result = named_server_dumprecursing(named_g_server); } else if (command_compare(command, NAMED_COMMAND_REFRESH)) { diff --git a/bin/named/include/named/server.h b/bin/named/include/named/server.h index a3aff6df94..7be62eecc7 100644 --- a/bin/named/include/named/server.h +++ b/bin/named/include/named/server.h @@ -161,7 +161,7 @@ named_server_resetstatscommand(named_server_t *server, isc_lex_t *lex, */ isc_result_t -named_server_reconfigcommand(named_server_t *server); +named_server_reconfigcommand(named_server_t *server, isc_buffer_t *text); /*%< * Act on a "reconfig" command from the command channel. */ diff --git a/bin/named/server.c b/bin/named/server.c index d5be63179a..ec9a1aea35 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -9924,8 +9924,16 @@ loadconfig(named_server_t *server) { static isc_result_t reload(named_server_t *server) { isc_result_t result; + int reloadstatus = atomic_exchange(&server->reload_status, + NAMED_RELOAD_IN_PROGRESS); - atomic_store(&server->reload_status, NAMED_RELOAD_IN_PROGRESS); + if (reloadstatus == NAMED_RELOAD_IN_PROGRESS) { + isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, + ISC_LOG_WARNING, + "reload request ignored: already running"); + result = ISC_R_ALREADYRUNNING; + goto cleanup; + } named_os_notify_systemd("RELOADING=1\n" "MONOTONIC_USEC=%" PRIu64 "\n" @@ -10325,8 +10333,16 @@ named_server_reloadcommand(named_server_t *server, isc_lex_t *lex, } if (zone == NULL) { result = reload(server); - if (result == ISC_R_SUCCESS) { + switch (result) { + case ISC_R_SUCCESS: msg = "server reload successful"; + break; + case ISC_R_ALREADYRUNNING: + msg = "reload request ignored as the server is " + "currently being reloaded or reconfigured"; + break; + default: + break; } } else { type = dns_zone_gettype(zone); @@ -10416,9 +10432,21 @@ named_server_resetstatscommand(named_server_t *server, isc_lex_t *lex, * Act on a "reconfig" command from the command channel. */ isc_result_t -named_server_reconfigcommand(named_server_t *server) { +named_server_reconfigcommand(named_server_t *server, isc_buffer_t *text) { isc_result_t result; - atomic_store(&server->reload_status, NAMED_RELOAD_IN_PROGRESS); + int reloadstatus = atomic_exchange(&server->reload_status, + NAMED_RELOAD_IN_PROGRESS); + + if (reloadstatus == NAMED_RELOAD_IN_PROGRESS) { + isc_log_write(NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, + ISC_LOG_WARNING, + "reconfig request ignored: already running"); + result = ISC_R_ALREADYRUNNING; + isc_buffer_printf(text, + "reconfig request ignored as the server is " + "currently being reloaded or reconfigured"); + goto cleanup; + } named_os_notify_systemd("RELOADING=1\n" "MONOTONIC_USEC=%" PRIu64 "\n" From 209b30d563c1cce3f263d514438f3aa13a5dd98f Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Thu, 5 Jun 2025 18:28:22 +0200 Subject: [PATCH 4/5] log-based test for load/apply config Add a new system test which checks named output when starting, reconfiguring and reloading the server. It checks that the steps where configuration is loaded, when named enters exclusive mode, and when the configuration is applied are all logged, and that they occur in the correct order. This adds a guard/warning to keep the parsing of the named.conf outside of the exclusive mode. --- bin/tests/system/configloading/README | 15 ++++++ bin/tests/system/configloading/ns1/example.db | 21 +++++++++ .../system/configloading/ns1/named.conf.in | 38 +++++++++++++++ bin/tests/system/configloading/setup.sh | 19 ++++++++ .../configloading/tests_sh_configloading.py | 47 +++++++++++++++++++ 5 files changed, 140 insertions(+) create mode 100644 bin/tests/system/configloading/README create mode 100644 bin/tests/system/configloading/ns1/example.db create mode 100644 bin/tests/system/configloading/ns1/named.conf.in create mode 100644 bin/tests/system/configloading/setup.sh create mode 100644 bin/tests/system/configloading/tests_sh_configloading.py diff --git a/bin/tests/system/configloading/README b/bin/tests/system/configloading/README new file mode 100644 index 0000000000..4721751eec --- /dev/null +++ b/bin/tests/system/configloading/README @@ -0,0 +1,15 @@ +Copyright (C) Internet Systems Consortium, Inc. ("ISC") + +SPDX-License-Identifier: MPL-2.0 + +This Source Code Form is subject to the terms of the Mozilla Public +License, v. 2.0. If a copy of the MPL was not distributed with this +file, you can obtain one at https://mozilla.org/MPL/2.0/. + +See the COPYRIGHT file distributed with this work for additional +information regarding copyright ownership. + +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 is currently based +on logging, so it's quite brittle. diff --git a/bin/tests/system/configloading/ns1/example.db b/bin/tests/system/configloading/ns1/example.db new file mode 100644 index 0000000000..abad6ab748 --- /dev/null +++ b/bin/tests/system/configloading/ns1/example.db @@ -0,0 +1,21 @@ +; Copyright (C) Internet Systems Consortium, Inc. ("ISC") +; +; SPDX-License-Identifier: MPL-2.0 +; +; This Source Code Form is subject to the terms of the Mozilla Public +; License, v. 2.0. If a copy of the MPL was not distributed with this +; file, you can obtain one at https://mozilla.org/MPL/2.0/. +; +; See the COPYRIGHT file distributed with this work for additional +; information regarding copyright ownership. + +example. IN SOA mname1. . ( + 2 ; serial + 20 ; refresh (20 seconds) + 20 ; retry (20 seconds) + 1814400 ; expire (3 weeks) + 3600 ; minimum (1 hour) + ) + IN NS ns.example. +ns IN A 10.0.0.1 +a IN A 10.0.0.2 diff --git a/bin/tests/system/configloading/ns1/named.conf.in b/bin/tests/system/configloading/ns1/named.conf.in new file mode 100644 index 0000000000..6c85c1df5e --- /dev/null +++ b/bin/tests/system/configloading/ns1/named.conf.in @@ -0,0 +1,38 @@ +/* + * Copyright (C) Internet Systems Consortium, Inc. ("ISC") + * + * SPDX-License-Identifier: MPL-2.0 + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * See the COPYRIGHT file distributed with this work for additional + * information regarding copyright ownership. + */ + +// NS2 + +options { + port @PORT@; + pid-file "named.pid"; + listen-on { 10.53.0.1; }; + listen-on-v6 { none; }; + recursion no; +}; + +key rndc_key { + secret "1234abcd8765"; + algorithm @DEFAULT_HMAC@; +}; + +controls { + inet 10.53.0.1 port @CONTROLPORT@ allow { any; } keys { rndc_key; }; +}; + + +zone "example." { + type primary; + file "example.db"; +}; + diff --git a/bin/tests/system/configloading/setup.sh b/bin/tests/system/configloading/setup.sh new file mode 100644 index 0000000000..7c9de7a6e7 --- /dev/null +++ b/bin/tests/system/configloading/setup.sh @@ -0,0 +1,19 @@ +#!/bin/sh -e + +# Copyright (C) Internet Systems Consortium, Inc. ("ISC") +# +# SPDX-License-Identifier: MPL-2.0 +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, you can obtain one at https://mozilla.org/MPL/2.0/. +# +# See the COPYRIGHT file distributed with this work for additional +# information regarding copyright ownership. + +# shellcheck source=conf.sh +. ../conf.sh + +set -e + +copy_setports ns1/named.conf.in ns1/named.conf diff --git a/bin/tests/system/configloading/tests_sh_configloading.py b/bin/tests/system/configloading/tests_sh_configloading.py new file mode 100644 index 0000000000..2aeb17a0fa --- /dev/null +++ b/bin/tests/system/configloading/tests_sh_configloading.py @@ -0,0 +1,47 @@ +# Copyright (C) Internet Systems Consortium, Inc. ("ISC") +# +# SPDX-License-Identifier: MPL-2.0 +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, you can obtain one at https://mozilla.org/MPL/2.0/. +# +# See the COPYRIGHT file distributed with this work for additional +# information regarding copyright ownership. + +import pytest + +pytestmark = pytest.mark.extra_artifacts( + [ + "ns1/managed-keys.bind.jnl", + ] +) + + +def assert_log_sequence(server, fnname, scopefn): + triggers = { + "load_configuration": 0, + "parsing user configuration from ": 1, + "apply_configuration": 2, + "loop exclusive mode: starting": 3, + } + fn = getattr(server, fnname) + for i in range(len(triggers.items())): + with fn() as watcher: + scopefn() + assert watcher.wait_for_lines(dict(list(triggers.items())[i:])) == i + + +def test_configloading_loading(servers): + server = servers["ns1"] + assert_log_sequence(server, "watch_log_from_start", lambda: ()) + + +def test_configloading_reconfig(servers): + server = servers["ns1"] + assert_log_sequence(server, "watch_log_from_here", lambda: server.rndc("reconfig")) + + +def test_configloading_reload(servers): + server = servers["ns1"] + assert_log_sequence(server, "watch_log_from_here", lambda: server.rndc("reload")) From 349cc060f4120848a6678105320b90edb7a4105c Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Mon, 23 Jun 2025 21:54:43 +0200 Subject: [PATCH 5/5] wait for reload completed in emptyzones system test The emptyzones system test ran two consecutive "rndc reload" commands without waiting for the first one to complete. It used to work because the commands were serialized, but now an rndc reconfig/reload command is ignored if another one is already running, so the emptyzones test is more likely to fail. Fix this problem by waiting for the log message indicating that all the zones are loaded before attempting the next reload. --- bin/tests/system/emptyzones/tests_emptyzones.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bin/tests/system/emptyzones/tests_emptyzones.py b/bin/tests/system/emptyzones/tests_emptyzones.py index 7a8d3966bd..20911cc9bd 100644 --- a/bin/tests/system/emptyzones/tests_emptyzones.py +++ b/bin/tests/system/emptyzones/tests_emptyzones.py @@ -17,7 +17,9 @@ import isctest def test_emptyzones(servers, templates): # check that switching to automatic empty zones works ns1 = servers["ns1"] - ns1.rndc("reload") + with ns1.watch_log_from_here() as watcher: + ns1.rndc("reload") + watcher.wait_for_line("all zones loaded") templates.render("ns1/named.conf", {"automatic_empty_zones": True}) ns1.rndc("reload") msg = dns.message.make_query("version.bind", "TXT", "CH")