diff --git a/NEWS.adoc b/NEWS.adoc index 51c221fc52..f27663a4dc 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -762,6 +762,13 @@ https://github.com/networkupstools/nut/milestone/13 codebase layout in the library, and this in turn caused some fallout. Fixed `libupsclient` builds with OpenSSL or NSS when Clang treats declarations after executable statements as errors. [issue #3761, PR #3762] + * Fixed `upsmon` losing the global `CERTPATH`, `CERTFILE`, `CERTIDENT`, + `CERTVERIFY` and `FORCESSL` settings from `upsmon.conf` when no + `nutauth.conf` file was found, and OpenSSL builds of `libupsclient` + never loading a `CERTPATH` which names a PEM file with CA certificates + (only a directory worked). Also fixed `upsmon` writing into freed + memory when it reloaded `upsmon.conf` with such settings after a + `nutauth.conf` file was read. [PR #3797] * The Perl `UPS::Nut` module now decodes quoted values and backslash escapes in query/list replies and `nutauth.conf` entries, and quotes authentication arguments. Single quotes in configuration are literal. diff --git a/clients/authconf.c b/clients/authconf.c index 1d00acceee..e47fc4143b 100644 --- a/clients/authconf.c +++ b/clients/authconf.c @@ -1026,9 +1026,6 @@ int upscli_read_authconf_file(const char *filename, int fatal_errors, int debug_ { char fn[NUT_PATH_MAX + 1]; - /* Ensure we start fresh if called multiple times */ - upscli_free_authconf_list(); - if (!filename) { /* Select a starting point - whichever default expected file exists; * it may INCLUDE further files as wanted by user or site sysadmin. @@ -1105,6 +1102,12 @@ int upscli_read_authconf_file(const char *filename, int fatal_errors, int debug_ } } + /* Ensure we start fresh if called multiple times; but only once + * there is a file to read: callers like upsmon may have populated + * the list (e.g. global defaults from legacy CERT* keywords) before + * a best-effort attempt which finds no file */ + upscli_free_authconf_list(); + return parse_authconf_file(filename, fatal_errors, 1); } @@ -1212,6 +1215,15 @@ upscli_authconf_t *upscli_get_authconf_item(const char *user, const char *host, upsdebugx(2, "%s: starting for [%s]@[%s]:[%s]", __func__, NUT_STRARG(user), NUT_STRARG(host), NUT_STRARG(port)); + if (!host && !port && !user && !global_defaults && add_to_list) { + /* Global section requested before one was read from a file, + * e.g. by upsmon for legacy CERT* keywords in upsmon.conf: + * create it, so host-specific items inherit its settings */ + upsdebugx(3, "%s: create new item for global defaults", __func__); + global_defaults = upscli_add_authconf_item(NULL); + return global_defaults; + } + /* We want this parsed always, so we can know if there * is a fixed user, or assign the section name, at least */ if (upscli_normalize_authconf_section_parts( diff --git a/clients/authconf.h b/clients/authconf.h index 82b9fababa..3d062c2a63 100644 --- a/clients/authconf.h +++ b/clients/authconf.h @@ -59,6 +59,9 @@ void upscli_free_authconf_list(void); * (whichever is found first); then one can follow `INCLUDE` trail if needed. * Ultimate error, if file was not found, is posted at specified debug_level>=0 * or logged as LOG_WARNING if negative. + * The current list of authentication configurations is replaced by the + * file contents. With a NULL filename, it is replaced only if a default + * file was found; otherwise it is kept as it was. * Returns -1 on error, 1 on success. */ int upscli_read_authconf_file(const char *filename, int fatal_errors, int debug_level); @@ -99,7 +102,8 @@ upscli_authconf_t *upscli_find_authconf_item(const char *user, const char *host, * not on the list (can adapt to changes in higher levels but must be freed by * caller), or will be edited on or added to the list (subsequent calls would * likely not add anything new, but memory management is easier, data is cached). - * if all args are NULL, return the global section or NULL if none such in the list. + * if all args are NULL, return the global section or NULL if none such in the list + * (with `add_to_list`, an empty global section is created and added if missing). */ upscli_authconf_t *upscli_get_authconf_item(const char *user, const char *host, const char *port, int add_to_list); diff --git a/clients/upsclient.c b/clients/upsclient.c index 8fa03762fe..27267d302f 100644 --- a/clients/upsclient.c +++ b/clients/upsclient.c @@ -1187,22 +1187,25 @@ void *upscli_get_or_create_ssl_context(int certverify, const char *certpath, break; } - ret = SSL_CTX_load_verify_locations(cfg->ssl_ctx, NULL, certpath); + /* Try a specific PEM file first: loading a directory (CApath) + * succeeds for any string, as its lookups only happen later + * during verification, so a file would never be tried after it */ + ret = SSL_CTX_load_verify_locations(cfg->ssl_ctx, certpath, NULL); if (ret != 1) { ssl_debug(); - upsdebugx(1, "%s: Failed to load CA certificate(s) from directory %s", __func__, certpath); + upsdebugx(1, "%s: Failed to load CA certificate(s) from file %s", __func__, certpath); - /* Can it be a specific PEM file? */ - if ((ret = SSL_CTX_load_verify_locations(cfg->ssl_ctx, certpath, NULL)) != 1) { + /* Can it be a directory with (hashed) CA certificate files? */ + if ((ret = SSL_CTX_load_verify_locations(cfg->ssl_ctx, NULL, certpath)) != 1) { ssl_debug(); - upslogx(LOG_ERR, "Failed to load CA certificate(s) from directory or file %s", certpath); + upslogx(LOG_ERR, "Failed to load CA certificate(s) from file or directory %s", certpath); upscli_ssl_context_config_free(cfg); return NULL; } else { - upsdebugx(1, "%s: ...but succeeded to load CA certificate(s) from file %s", __func__, certpath); + upsdebugx(1, "%s: ...but succeeded to load CA certificate(s) from directory %s", __func__, certpath); } } else { - upsdebugx(1, "%s: Succeeded to load CA certificate(s) from directory %s", __func__, certpath); + upsdebugx(1, "%s: Succeeded to load CA certificate(s) from file %s", __func__, certpath); } /* Adapted from https://linux.die.net/man/3/ssl_set_verify man page example */ diff --git a/clients/upsmon.c b/clients/upsmon.c index a10a2236ff..2331930822 100644 --- a/clients/upsmon.c +++ b/clients/upsmon.c @@ -2829,6 +2829,16 @@ static void loadconfig(void) upsdebugx(1, "Using best-effort auth config detection"); upscli_read_authconf_file(NULL, 0, 1); } + + /* A file that was read replaced the list, including the + * global section which the legacy keywords had populated + * (FIXME: merge the file into it instead, as documented): + * point to the current one (or create it if the file had + * none), so a reload does not edit a freed entry */ + ac_default = upscli_get_authconf_item(NULL, NULL, NULL, 1); + if (!ac_default) { + fatalx(EXIT_FAILURE, "Fatal error: unable to get default authconf entry"); + } } /* FIXME: Per legacy behavior, we silently went on.