From cd921cc7ef97404de941c9ac2f9fc14d63b75ef1 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 28 Oct 2025 18:28:35 -0700 Subject: [PATCH 1/3] fix a "max-cache-size" configuration bug "max-cache-size default;" is allowed, according to the documentation and the parser, but when it's configured, named crashes due to an INSIST that the only legal string value is "unlimited". this has been fied. the configuration has also been simplified. previously, we checked for max-cache-size in view and options, then determined whether to look in the global default options based on whether the view had recursion set. the default value set there was only applicable to views with recursion. now, the default is an explicit "default", which affects views with and without recursion in different ways. the cfg type for "max-cache-size" has been changed from cfg_type_sizeorpercent to cfg_type_maxcachesize. --- bin/named/config.c | 2 +- bin/named/server.c | 119 +++++++++++++++++------------------------ lib/isccfg/namedconf.c | 18 +++---- 3 files changed, 58 insertions(+), 81 deletions(-) diff --git a/bin/named/config.c b/bin/named/config.c index 00006a8522..9870698402 100644 --- a/bin/named/config.c +++ b/bin/named/config.c @@ -168,7 +168,7 @@ options {\n\ #ifdef HAVE_LMDB " lmdb-mapsize 32M;\n" #endif /* ifdef HAVE_LMDB */ - " max-cache-size 90%;\n\ + " max-cache-size default;\n\ max-cache-ttl 604800; /* 1 week */\n\ max-clients-per-query 100;\n\ max-ncache-ttl 10800; /* 3 hours */\n\ diff --git a/bin/named/server.c b/bin/named/server.c index 5d7f74cb19..c3dca714b2 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -3742,42 +3742,6 @@ named_register_one_plugin(const cfg_obj_t *config, const cfg_obj_t *obj, return result; } -/* - * Determine if a minimal-sized cache can be used for a given view, according - * to 'maps' (implicit defaults, global options, view options) and 'optionmaps' - * (global options, view options). This is only allowed for views which have - * recursion disabled and do not have "max-cache-size" set explicitly. Using - * minimal-sized caches prevents a situation in which all explicitly configured - * and built-in views inherit the default "max-cache-size 90%;" setting, which - * could lead to memory exhaustion with multiple views configured. - */ -static bool -minimal_cache_allowed(const cfg_obj_t *maps[4], - const cfg_obj_t *optionmaps[3]) { - const cfg_obj_t *obj; - - /* - * Do not use a minimal-sized cache for a view with recursion enabled. - */ - obj = NULL; - (void)named_config_get(maps, "recursion", &obj); - INSIST(obj != NULL); - if (cfg_obj_asboolean(obj)) { - return false; - } - - /* - * Do not use a minimal-sized cache if a specific size was requested. - */ - obj = NULL; - (void)named_config_get(optionmaps, "max-cache-size", &obj); - if (obj != NULL) { - return false; - } - - return true; -} - static const char *const response_synonyms[] = { "response", NULL }; /* @@ -4040,42 +4004,60 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, * we can reuse/share an existing cache. */ obj = NULL; - result = named_config_get(maps, "max-cache-size", &obj); + result = named_config_get(maps, "recursion", &obj); INSIST(result == ISC_R_SUCCESS); - /* - * If "-T maxcachesize=..." is in effect, it overrides any other - * "max-cache-size" setting found in configuration, either implicit or - * explicit. For simplicity, the value passed to that command line - * option is always treated as the number of bytes to set - * "max-cache-size" to. - */ + view->recursion = cfg_obj_asboolean(obj); + if (named_g_maxcachesize != 0) { - max_cache_size = named_g_maxcachesize; - } else if (minimal_cache_allowed(maps, optionmaps)) { /* - * dns_cache_setcachesize() will adjust this to the smallest - * allowed value. + * If "-T maxcachesize=..." is in effect, it overrides any + * other "max-cache-size" setting found in configuration, + * either implicit or explicit. For simplicity, the value + * passed to that command line option is always treated as + * the number of bytes to set "max-cache-size" to. */ - max_cache_size = 1; - } else if (cfg_obj_isstring(obj)) { - str = cfg_obj_asstring(obj); - INSIST(strcasecmp(str, "unlimited") == 0); - max_cache_size = 0; - } else if (cfg_obj_ispercentage(obj)) { - max_cache_size = SIZE_AS_PERCENT; - max_cache_size_percent = cfg_obj_aspercentage(obj); + max_cache_size = named_g_maxcachesize; } else { - uint64_t value = cfg_obj_asuint64(obj); - if (value > SIZE_MAX) { - cfg_obj_log(obj, ISC_LOG_WARNING, - "'max-cache-size " - "%" PRIu64 "' " - "is too large for this " - "system; reducing to %lu", - value, (unsigned long)SIZE_MAX); - value = SIZE_MAX; + obj = NULL; + result = named_config_get(maps, "max-cache-size", &obj); + INSIST(result == ISC_R_SUCCESS); + if (cfg_obj_isstring(obj) && + strcasecmp(cfg_obj_asstring(obj), "default") == 0) + { + /* + * The default for a view with recursion + * is 90% of memory. With no recursion, + * it's the minimum cache size allowed by + * dns_cache_setcachesize(). + */ + if (view->recursion) { + max_cache_size = SIZE_AS_PERCENT; + max_cache_size_percent = 90; + } else { + max_cache_size = 1; + } + } else if (cfg_obj_isstring(obj)) { + str = cfg_obj_asstring(obj); + INSIST(strcasecmp(str, "unlimited") == 0); + max_cache_size = 0; + } else if (cfg_obj_ispercentage(obj)) { + max_cache_size = SIZE_AS_PERCENT; + max_cache_size_percent = cfg_obj_aspercentage(obj); + } else if (cfg_obj_isuint64(obj)) { + uint64_t value = cfg_obj_asuint64(obj); + if (value > SIZE_MAX) { + cfg_obj_log(obj, ISC_LOG_WARNING, + "'max-cache-size " + "%" PRIu64 "' " + "is too large for this " + "system; reducing to %lu", + value, (unsigned long)SIZE_MAX); + value = SIZE_MAX; + } + max_cache_size = (size_t)value; + } else { + UNREACHABLE(); } - max_cache_size = (size_t)value; } if (max_cache_size == SIZE_AS_PERCENT) { @@ -4879,11 +4861,6 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, /* * Configure other configurable data. */ - obj = NULL; - result = named_config_get(maps, "recursion", &obj); - INSIST(result == ISC_R_SUCCESS); - view->recursion = cfg_obj_asboolean(obj); - obj = NULL; result = named_config_get(maps, "qname-minimization", &obj); INSIST(result == ISC_R_SUCCESS); diff --git a/lib/isccfg/namedconf.c b/lib/isccfg/namedconf.c index 5d0117ac5a..4b7a203ddb 100644 --- a/lib/isccfg/namedconf.c +++ b/lib/isccfg/namedconf.c @@ -111,6 +111,7 @@ static cfg_type_t cfg_type_logseverity; static cfg_type_t cfg_type_logsuffix; static cfg_type_t cfg_type_logversions; static cfg_type_t cfg_type_remoteselement; +static cfg_type_t cfg_type_maxcachesize; static cfg_type_t cfg_type_maxduration; static cfg_type_t cfg_type_minimal; static cfg_type_t cfg_type_nameportiplist; @@ -138,7 +139,6 @@ static cfg_type_t cfg_type_server; static cfg_type_t cfg_type_server_key_kludge; static cfg_type_t cfg_type_size; static cfg_type_t cfg_type_sizenodefault; -static cfg_type_t cfg_type_sizeorpercent; static cfg_type_t cfg_type_sizeval; static cfg_type_t cfg_type_sockaddr4wild; static cfg_type_t cfg_type_sockaddr6wild; @@ -2091,7 +2091,7 @@ static cfg_clausedef_t view_clauses[] = { { "lmdb-mapsize", &cfg_type_sizeval, CFG_CLAUSEFLAG_NOTCONFIGURED }, #endif /* ifdef HAVE_LMDB */ { "max-acache-size", NULL, CFG_CLAUSEFLAG_ANCIENT }, - { "max-cache-size", &cfg_type_sizeorpercent, 0 }, + { "max-cache-size", &cfg_type_maxcachesize, 0 }, { "max-cache-ttl", &cfg_type_duration, 0 }, { "max-clients-per-query", &cfg_type_uint32, 0 }, { "max-ncache-ttl", &cfg_type_duration, 0 }, @@ -2932,14 +2932,14 @@ static cfg_type_t cfg_type_sizeval_percent = { */ static isc_result_t -parse_size_or_percent(cfg_parser_t *pctx, const cfg_type_t *type, - cfg_obj_t **ret) { +parse_maxcachesize(cfg_parser_t *pctx, const cfg_type_t *type, + cfg_obj_t **ret) { return cfg_parse_enum_or_other(pctx, type, &cfg_type_sizeval_percent, ret); } static void -doc_parse_size_or_percent(cfg_printer_t *pctx, const cfg_type_t *type) { +doc_maxcachesize(cfg_printer_t *pctx, const cfg_type_t *type) { UNUSED(type); cfg_print_cstr(pctx, "( default | unlimited | "); cfg_doc_terminal(pctx, &cfg_type_sizeval); @@ -2948,10 +2948,10 @@ doc_parse_size_or_percent(cfg_printer_t *pctx, const cfg_type_t *type) { cfg_print_cstr(pctx, " )"); } -static const char *sizeorpercent_enums[] = { "default", "unlimited", NULL }; -static cfg_type_t cfg_type_sizeorpercent = { - "size_or_percent", parse_size_or_percent, cfg_print_ustring, - doc_parse_size_or_percent, &cfg_rep_string, sizeorpercent_enums +static const char *maxcachesize_enums[] = { "default", "unlimited", NULL }; +static cfg_type_t cfg_type_maxcachesize = { + "maxcachesize", parse_maxcachesize, cfg_print_ustring, + doc_maxcachesize, &cfg_rep_string, maxcachesize_enums }; /*% From fffae65e27e906c452294cd7b96a8fc2db550acd Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 28 Oct 2025 18:34:55 -0700 Subject: [PATCH 2/3] simplify "dnssec-validation" configuration In the past, "dnssec-validation" was not looked up in the global defaults unless "dnssec-enable" was true. "dnssec-enable" has been obsolete for several years, but dnssec-validation was still being configured in two steps. This commit removes the vestigial bits of the old logic. --- bin/named/server.c | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index c3dca714b2..1f1fdc8876 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -4231,27 +4231,17 @@ 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_defaultoptions yet */ - (void)named_config_get(optionmaps, "dnssec-validation", &obj); - if (obj == NULL) { + result = named_config_get(maps, "dnssec-validation", &obj); + INSIST(result == ISC_R_SUCCESS); + if (cfg_obj_isboolean(obj)) { + view->enablevalidation = cfg_obj_asboolean(obj); + } else { /* - * Default to VALIDATION_DEFAULT as set in config.c. + * If dnssec-validation is set but not boolean, + * then it must be "auto" */ - (void)cfg_map_get(named_g_defaultoptions, "dnssec-validation", - &obj); - INSIST(obj != NULL); - } - if (obj != NULL) { - if (cfg_obj_isboolean(obj)) { - view->enablevalidation = cfg_obj_asboolean(obj); - } else { - /* - * If dnssec-validation is set but not boolean, - * then it must be "auto" - */ - view->enablevalidation = true; - auto_root = true; - } + view->enablevalidation = true; + auto_root = true; } obj = NULL; From cf409e814f13c932d92fae5d85ddd16ac7ebfdc0 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 28 Oct 2025 18:45:25 -0700 Subject: [PATCH 3/3] fix "response-padding" configuration and remove optionmaps Add a default "response-padding" option in the global defaults, to disable the option, and simplify the configuration code so that looks at the global defaults if the option is not set in named.conf. This enables us to remove the 'optionmaps' variable in configure_view(), which was used for options that only look in named.conf. --- bin/named/config.c | 1 + bin/named/server.c | 39 ++++++++++++++++----------------------- 2 files changed, 17 insertions(+), 23 deletions(-) diff --git a/bin/named/config.c b/bin/named/config.c index 9870698402..daaaafc525 100644 --- a/bin/named/config.c +++ b/bin/named/config.c @@ -188,6 +188,7 @@ options {\n\ parental-source *;\n\ parental-source-v6 *;\n\ provide-ixfr true;\n\ + response-padding { none; } block-size 0;\n\ qname-minimization relaxed;\n\ query-source address *;\n\ query-source-v6 address *;\n\ diff --git a/bin/named/server.c b/bin/named/server.c index 1f1fdc8876..03ec0874eb 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -3761,7 +3761,6 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, bool first_time) { const cfg_obj_t *maps[4]; const cfg_obj_t *cfgmaps[3]; - const cfg_obj_t *optionmaps[3]; const cfg_obj_t *options = NULL; const cfg_obj_t *voptions = NULL; const cfg_obj_t *forwardtype = NULL; @@ -3818,6 +3817,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, const char *qminmode = NULL; dns_adb_t *adb = NULL; bool oldcache = false; + uint32_t padding; REQUIRE(DNS_VIEW_VALID(view)); @@ -3827,27 +3827,23 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, /* * maps: view options, options, defaults - * cfgmaps: view options, config - * optionmaps: view options, options + * cfgmaps: view options, top-level config */ if (vconfig != NULL) { voptions = cfg_tuple_get(vconfig, "options"); maps[i++] = voptions; - optionmaps[j++] = voptions; cfgmaps[k++] = voptions; } if (options != NULL) { maps[i++] = options; - optionmaps[j++] = options; } - maps[i++] = named_g_defaultoptions; maps[i] = NULL; - optionmaps[j] = NULL; + if (config != NULL) { - cfgmaps[k++] = config; + cfgmaps[j++] = config; } - cfgmaps[k] = NULL; + cfgmaps[j] = NULL; /* * Set the view's port number for outgoing queries. @@ -5169,22 +5165,19 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, if (view->pad_acl != NULL) { dns_acl_detach(&view->pad_acl); } - result = named_config_get(optionmaps, "response-padding", &obj); - if (result == ISC_R_SUCCESS) { - const cfg_obj_t *padobj = cfg_tuple_get(obj, "block-size"); - const cfg_obj_t *aclobj = cfg_tuple_get(obj, "acl"); - uint32_t padding = cfg_obj_asuint32(padobj); + result = named_config_get(maps, "response-padding", &obj); + INSIST(result == ISC_R_SUCCESS); + padding = cfg_obj_asuint32(cfg_tuple_get(obj, "block-size")); - if (padding > 512U) { - cfg_obj_log(obj, ISC_LOG_WARNING, - "response-padding block-size cannot " - "exceed 512: lowering"); - padding = 512U; - } - view->padding = (uint16_t)padding; - CHECK(cfg_acl_fromconfig(aclobj, config, aclctx, isc_g_mctx, 0, - &view->pad_acl)); + if (padding > 512U) { + cfg_obj_log(obj, ISC_LOG_WARNING, + "response-padding block-size cannot " + "exceed 512: lowering"); + padding = 512U; } + view->padding = (uint16_t)padding; + CHECK(cfg_acl_fromconfig(cfg_tuple_get(obj, "acl"), config, aclctx, + isc_g_mctx, 0, &view->pad_acl)); obj = NULL; result = named_config_get(maps, "require-server-cookie", &obj);