fix: dev: fix configuration bugs involving global defaults

The configuration code for the `max-cache-size`, `dnssec-validation`, and `response-padding` options were unnecessarily complicated, and in the case of `max-cache-size`, buggy. These have been fixed. The `optionmaps` variable in `configure_view()` is no longer needed and has been removed.

Merge branch 'each-cleanup-defaultconfig' into 'main'

See merge request isc-projects/bind9!11165
This commit is contained in:
Evan Hunt 2025-10-29 18:28:45 +00:00
commit 539cda62a7
3 changed files with 84 additions and 123 deletions

View file

@ -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\
@ -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\

View file

@ -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 };
/*
@ -3797,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;
@ -3854,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));
@ -3863,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.
@ -4040,42 +4000,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) {
@ -4249,27 +4227,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;
@ -4879,11 +4847,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);
@ -5202,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);

View file

@ -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
};
/*%