From 97fec31c7d2ea184b1123bea6139f325ecf9de9f Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Wed, 23 Oct 2024 12:58:33 +0200 Subject: [PATCH] Pro-actively set cookie_secret_file value Right after config is read, so no wrapper is needed anymore to determine the value. --- configparser.y | 12 ++++- nsd-checkconf.c | 26 +++++----- options.c | 6 +++ options.h | 11 +---- remote.c | 16 ++++--- simdzone | 2 +- util.c | 125 +++++++++++++++++++++++++++--------------------- 7 files changed, 112 insertions(+), 86 deletions(-) diff --git a/configparser.y b/configparser.y index edee6639..9204bb7f 100644 --- a/configparser.y +++ b/configparser.y @@ -541,7 +541,17 @@ server_option: } } | VAR_COOKIE_SECRET_FILE STRING - { cfg_parser->opt->cookie_secret_file = region_strdup(cfg_parser->opt->region, $2); } + { + /* Empty filename means explicitly disabled cookies from file, internally + * represented as NULL. + * Note that after parsing, if no value was configured, then + * cookie_secret_file_is_default is still 1, then the default cookie + * secret file value will be assigned to cookie_secret_file. + */ + if(*$2) cfg_parser->opt->cookie_secret_file = region_strdup(cfg_parser->opt->region, $2); + cfg_parser->opt->cookie_secret_file_is_default = 0; + } + | VAR_XFRD_TCP_MAX number { cfg_parser->opt->xfrd_tcp_max = (int)$2; } | VAR_XFRD_TCP_PIPELINE number diff --git a/nsd-checkconf.c b/nsd-checkconf.c index 76c00ccb..f4a2801d 100644 --- a/nsd-checkconf.c +++ b/nsd-checkconf.c @@ -166,23 +166,13 @@ usage(void) } static void -print_string_var_default(const char* varname, const char* value, - const char* default_value) +print_string_var(const char* varname, const char* value) { - if (value) { - printf("\t%s \"%s\"\n", varname, value); - } else if (default_value) { - printf("\t#%s \"%s\"\n", varname, default_value); - } else { + if (!value) { printf("\t#%s\n", varname); + } else { + printf("\t%s \"%s\"\n", varname, value); } - -} - -static void -print_string_var(const char* varname, const char* value) -{ - print_string_var_default(varname, value, NULL); } static void @@ -731,7 +721,13 @@ config_test_print_server(nsd_options_type* opt) printf("\tanswer-cookie: %s\n", opt->answer_cookie?"yes":"no"); print_string_var("cookie-secret:", opt->cookie_secret); print_string_var("cookie-staging-secret:", opt->cookie_staging_secret); - print_string_var_default("cookie-secret-file:", opt->cookie_secret_file, COOKIESECRETSFILE); + if(opt->cookie_secret_file_is_default) { + print_string_var("#cookie-secret-file:", opt->cookie_secret_file); + } else if(opt->cookie_secret_file) { + print_string_var("cookie-secret-file:", opt->cookie_secret_file); + } else { + print_string_var("cookie-secret-file:", ""); + } if(opt->proxy_protocol_port) { struct proxy_protocol_port_list* p; for(p = opt->proxy_protocol_port; p; p = p->next) diff --git a/options.c b/options.c index 53190f82..dcae0934 100644 --- a/options.c +++ b/options.c @@ -154,6 +154,7 @@ nsd_options_create(region_type* region) opt->cookie_secret = NULL; opt->cookie_staging_secret = NULL; opt->cookie_secret_file = NULL; + opt->cookie_secret_file_is_default = 1; opt->control_enable = 0; opt->control_interface = NULL; opt->control_port = NSD_CONTROL_PORT; @@ -258,6 +259,11 @@ parse_options_file(struct nsd_options* opt, const char* file, opt->configfile = region_strdup(opt->region, file); + /* Set default cookie_secret_file value */ + if(opt->cookie_secret_file_is_default && !opt->cookie_secret_file) { + opt->cookie_secret_file = + region_strdup(opt->region, COOKIESECRETSFILE); + } /* Semantic errors */ if(opt->cookie_staging_secret && !opt->cookie_secret) { c_error("a cookie-staging-secret cannot be configured without " diff --git a/options.h b/options.h index 78ba7a5c..2567a62d 100644 --- a/options.h +++ b/options.h @@ -213,6 +213,8 @@ struct nsd_options { char *cookie_staging_secret; /** path to cookie secret store */ char *cookie_secret_file; + /** set when the cookie_secret_file whas not explicitely configured */ + uint8_t cookie_secret_file_is_default; /** enable verify */ int verify_enable; /** list of ip addresses used to serve zones for verification */ @@ -493,15 +495,6 @@ int nsd_options_insert_zone(struct nsd_options* opt, struct zone_options* zone); int nsd_options_insert_pattern(struct nsd_options* opt, struct pattern_options* pat); -/* return the configured cookie secrets filename or NULL if disabled */ -static inline const char* cookie_secret_file(struct nsd_options* opt) -{ - /* NULL means the default of COOKIESECRETSFILE, "" means disabled */ - return opt->cookie_secret_file - ? ( *opt->cookie_secret_file ? opt->cookie_secret_file : NULL ) - : COOKIESECRETSFILE; -} - /* parses options file. Returns false on failure. callback, if nonNULL, * gets called with error strings, default prints. */ int parse_options_file(struct nsd_options* opt, const char* file, diff --git a/remote.c b/remote.c index af5fa8f5..83d79195 100644 --- a/remote.c +++ b/remote.c @@ -1981,6 +1981,8 @@ repat_cookie_options_changed(struct nsd_options* old, struct nsd_options* new) , new->cookie_secret) || opt_str_changed( old->cookie_staging_secret , new->cookie_staging_secret) + || old->cookie_secret_file_is_default != + new->cookie_secret_file_is_default || opt_str_changed( old->cookie_secret_file , new->cookie_secret_file); } @@ -2011,6 +2013,8 @@ repat_options(xfrd_state_type* xfrd, struct nsd_options* newopt) region_str_replace( oldopt->region , &oldopt->cookie_staging_secret , newopt->cookie_staging_secret); + oldopt->cookie_secret_file_is_default = + newopt->cookie_secret_file_is_default; region_str_replace( oldopt->region , &oldopt->cookie_secret_file , newopt->cookie_secret_file); @@ -2385,7 +2389,7 @@ do_del_tsig(RES* ssl, xfrd_state_type* xfrd, char* arg) { static int can_dump_cookie_secrets(RES* ssl, nsd_type* const nsd) { - if(!cookie_secret_file(nsd->options)) + if(!nsd->options->cookie_secret_file) (void)ssl_printf(ssl, "error: empty cookie-secret-file\n"); else if(nsd->cookie_secrets_source == COOKIE_SECRETS_FROM_CONFIG) @@ -2408,14 +2412,14 @@ cookie_secret_file_dump_and_reload(RES* ssl, nsd_type* const nsd) { size_t i; /* open write only and truncate */ - if(!cookie_secret_file(nsd->options)) { + if(!nsd->options->cookie_secret_file) { (void)ssl_printf(ssl, "cookie-secret-file empty\n"); return 0; } - else if((f = fopen(cookie_secret_file(nsd->options), "w")) == NULL ) { + else if((f = fopen(nsd->options->cookie_secret_file, "w")) == NULL ) { (void)ssl_printf( ssl , "unable to open cookie secret file %s: %s\n" - , cookie_secret_file(nsd->options) + , nsd->options->cookie_secret_file , strerror(errno)); return 0; } @@ -2432,7 +2436,7 @@ cookie_secret_file_dump_and_reload(RES* ssl, nsd_type* const nsd) { fclose(f); nsd->cookie_secrets_source = COOKIE_SECRETS_FROM_FILE; region_str_replace(nsd->region, &nsd->cookie_secrets_filename - , cookie_secret_file(nsd->options)); + , nsd->options->cookie_secret_file); task_new_cookies(xfrd->nsd->task[xfrd->nsd->mytask], xfrd->last_task, nsd->do_answer_cookie, nsd->cookie_count, nsd->cookie_secrets); xfrd_set_reload_now(xfrd); @@ -2533,7 +2537,7 @@ do_add_cookie_secret(RES* ssl, xfrd_state_type* xrfd, char* arg) { if(!cookie_secret_file_dump_and_reload(ssl, nsd)) { explicit_bzero(arg, strlen(arg)); (void)ssl_printf(ssl, "error: writing to cookie secret file: \"%s\"\n" - , cookie_secret_file(nsd->options)); + , nsd->options->cookie_secret_file); memcpy( nsd->cookie_secrets, backup_cookie_secrets , sizeof(cookie_secrets_type)); nsd->cookie_count = backup_cookie_count; diff --git a/simdzone b/simdzone index 68356bd1..fa98e45d 160000 --- a/simdzone +++ b/simdzone @@ -1 +1 @@ -Subproject commit 68356bd1949342493aae7437bee153efbab05a2f +Subproject commit fa98e45d149c90ee04cdac93a438bc01463ec95e diff --git a/util.c b/util.c index 6ad6c1e8..e598695b 100644 --- a/util.c +++ b/util.c @@ -1175,30 +1175,69 @@ void drop_cookie_secret(struct nsd* nsd) nsd->cookie_count -= 1; } -static -int cookie_secret_file_read(nsd_type* nsd) { +void reconfig_cookies(struct nsd* nsd, struct nsd_options* options) +{ cookie_secret_type cookie_secrets[NSD_COOKIE_HISTORY_SIZE]; char secret[NSD_COOKIE_SECRET_SIZE * 2 + 2/*'\n' and '\0'*/]; - FILE* f; + FILE* f = NULL; size_t count = 0; const char* fn; + size_t i, j; - if(!(fn = cookie_secret_file(nsd->options))) - return 1; /* Explicitely disabled with empty filename */ + nsd->do_answer_cookie = options->answer_cookie; + + /* Cookie secrets in the configuration file take precedence */ + if(options->cookie_secret) { + ssize_t len = hex_pton(options->cookie_secret, + nsd->cookie_secrets[0].cookie_secret, + NSD_COOKIE_SECRET_SIZE); + + /* Cookie length guaranteed in configparser.y */ + assert(len == NSD_COOKIE_SECRET_SIZE); + nsd->cookie_count = 1; + if(options->cookie_staging_secret) { + len = hex_pton(options->cookie_staging_secret, + nsd->cookie_secrets[1].cookie_secret, + NSD_COOKIE_SECRET_SIZE); + /* Cookie length guaranteed in configparser.y */ + assert(len == NSD_COOKIE_SECRET_SIZE); + nsd->cookie_count = 2; + } + /*************************************************************/ + nsd->cookie_secrets_source = COOKIE_SECRETS_FROM_CONFIG; + return; + /*************************************************************/ + } + /* Are cookies from file explicitly disabled? */ + if(!(fn = nsd->options->cookie_secret_file)) + goto generate_cookie_secrets; else if((f = fopen(fn, "r")) != NULL) ; /* pass */ - /* a non-existing cookie file is not an error */ + /* a non-existing cookie file is not necessarily an error */ else if(errno != ENOENT) { log_msg( LOG_ERR , "error reading cookie secret file \"%s\": \"%s\"" , fn, strerror(errno)); - return 0; - } - else if(nsd->options->cookie_secret_file != NULL /* explicit name */ - ||!(f = fopen((fn = CONFIGDIR"/nsd_cookiesecrets.txt"), "r"))) - return 1; + goto generate_cookie_secrets; + + /* Only at startup cookie_secrets_source == COOKIE_SECRETS_NONE. + * Only then the previous default file location will be tried + * when the current default file location didn't exist. + */ + } else if(nsd->cookie_secrets_source == COOKIE_SECRETS_NONE + && nsd->options->cookie_secret_file_is_default + && (f = fopen((fn = CONFIGDIR"/nsd_cookiesecrets.txt"),"r"))) + ; /* pass */ + + else if(errno != ENOENT) { + log_msg( LOG_ERR + , "error reading cookie secret file \"%s\": \"%s\"" + , fn, strerror(errno)); + goto generate_cookie_secrets; + } else + goto generate_cookie_secrets; /* cookie secret file exists and is readable */ for( count = 0; count < NSD_COOKIE_HISTORY_SIZE; count++ ) { @@ -1216,7 +1255,7 @@ int cookie_secret_file_read(nsd_type* nsd) { , fn); explicit_bzero(cookie_secrets, sizeof(cookie_secrets)); explicit_bzero(secret, sizeof(secret)); - return 0; + goto generate_cookie_secrets; } /* needed for `hex_pton`; stripping potential `\n` */ secret[secret_len] = '\0'; @@ -1229,61 +1268,39 @@ int cookie_secret_file_read(nsd_type* nsd) { , fn); explicit_bzero(cookie_secrets, sizeof(cookie_secrets)); explicit_bzero(secret, sizeof(secret)); - return 0; - explicit_bzero(secret, sizeof(secret)); + goto generate_cookie_secrets; } + explicit_bzero(secret, sizeof(secret)); } fclose(f); - if(count && nsd->cookie_secrets_source != COOKIE_SECRETS_FROM_FILE) { + if(count) { nsd->cookie_count = count; memcpy(nsd->cookie_secrets, cookie_secrets, sizeof(cookie_secrets)); - nsd->cookie_secrets_source = COOKIE_SECRETS_FROM_FILE; region_str_replace( nsd->region , &nsd->cookie_secrets_filename, fn ); + explicit_bzero(cookie_secrets, sizeof(cookie_secrets)); + /*************************************************************/ + nsd->cookie_secrets_source = COOKIE_SECRETS_FROM_FILE; + return; + /*************************************************************/ } explicit_bzero(cookie_secrets, sizeof(cookie_secrets)); - return 1; -} - - -void reconfig_cookies(struct nsd* nsd, struct nsd_options* options) -{ - nsd->do_answer_cookie = options->answer_cookie; - if(options->cookie_secret) { - ssize_t len = hex_pton(options->cookie_secret, - nsd->cookie_secrets[0].cookie_secret, - NSD_COOKIE_SECRET_SIZE); - /* Cookie length guaranteed in configparser.y */ - assert(len == NSD_COOKIE_SECRET_SIZE); - nsd->cookie_count = 1; - nsd->cookie_secrets_source = COOKIE_SECRETS_FROM_CONFIG; - if(options->cookie_staging_secret) { - len = hex_pton(options->cookie_staging_secret, - nsd->cookie_secrets[1].cookie_secret, - NSD_COOKIE_SECRET_SIZE); - /* Cookie length guaranteed in configparser.y */ - assert(len == NSD_COOKIE_SECRET_SIZE); - nsd->cookie_count = 2; - } - } else { - size_t i, j; - - /* Calculate a new random secret */ - srandom(getpid() ^ time(NULL)); +generate_cookie_secrets: + /* Calculate a new random secret */ + srandom(getpid() ^ time(NULL)); - for( j = 0; j < NSD_COOKIE_HISTORY_SIZE; j++) { + for( j = 0; j < NSD_COOKIE_HISTORY_SIZE; j++) { #if defined(HAVE_SSL) - if (!RAND_status() - || !RAND_bytes(nsd->cookie_secrets[j].cookie_secret, NSD_COOKIE_SECRET_SIZE)) + if (!RAND_status() + || !RAND_bytes(nsd->cookie_secrets[j].cookie_secret, NSD_COOKIE_SECRET_SIZE)) #endif - for (i = 0; i < NSD_COOKIE_SECRET_SIZE; i++) - nsd->cookie_secrets[j].cookie_secret[i] = random_generate(256); - } - nsd->cookie_count = 1; - nsd->cookie_secrets_source = COOKIE_SECRETS_GENERATED; - if(cookie_secret_file(nsd->options)) - cookie_secret_file_read(nsd); + for (i = 0; i < NSD_COOKIE_SECRET_SIZE; i++) + nsd->cookie_secrets[j].cookie_secret[i] = random_generate(256); } + nsd->cookie_count = 1; + /*********************************************************************/ + nsd->cookie_secrets_source = COOKIE_SECRETS_GENERATED; + /*********************************************************************/ }