From 838b094528bedae720d039b0f2cc2ad859df4680 Mon Sep 17 00:00:00 2001 From: nmbro Date: Fri, 23 Oct 2026 09:45:43 +0200 Subject: [PATCH 1/4] clients/authconf.c: keep upsmon.conf CERT* defaults upsmon parses the legacy global CERTPATH, CERTFILE, CERTIDENT, CERTVERIFY and FORCESSL keywords from upsmon.conf into the entry which it gets with upscli_get_authconf_item(NULL, NULL, NULL, 1). Two issues lost those settings, so e.g. the CA from CERTPATH was never loaded and FORCESSL/CERTVERIFY were ignored: * With all-NULL arguments and an empty list, that call did not create the global section: it created a "@localhost:3493" item (which upsmon then renamed to a NULL section), not known as global defaults, so the items later made for the monitored hosts did not inherit from it. Now such a call creates (and adds) the global section, as documented for all-NULL arguments. * Without an AUTHCONF line, upsmon then calls the best-effort upscli_read_authconf_file(NULL, 0, 1), which freed the current list before looking for a file. With no nutauth.conf on the system, the settings were discarded (and upsmon kept a pointer to the freed entry). Now the list is freed only once a file was found to be read. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Ju3WEqpswgy9dN7EQB3Fjn Signed-off-by: nmbro --- clients/authconf.c | 18 +++++++++++++++--- clients/authconf.h | 5 ++++- 2 files changed, 19 insertions(+), 4 deletions(-) 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..c5a68ef607 100644 --- a/clients/authconf.h +++ b/clients/authconf.h @@ -59,6 +59,8 @@ 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 only + * if a file was found to read; 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 +101,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); From ad3924f5f20b1ce93fb3afd4fe38fe69d6211a98 Mon Sep 17 00:00:00 2001 From: nmbro Date: Fri, 23 Oct 2026 09:45:47 +0200 Subject: [PATCH 2/4] clients/upsclient.c: load a CERTPATH file as a CA bundle The OpenSSL client documents CERTPATH as either a directory of (hashed) CA certificates or a PEM file with a collection of CA certificates, and upscli_get_or_create_ssl_context() tried SSL_CTX_load_verify_locations() with CERTPATH as a directory (CApath) first, then as a file (CAfile). But loading a CApath succeeds for any string, as OpenSSL only looks up the directory later, during verification. So a CERTPATH file was never loaded, and verification of any server certificate failed (e.g. with "certificate verify failed", verify error num=19) even with the right CA in that file. Try CERTPATH as a file first, then as a directory, so both forms work; the NSS code path is not changed. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Ju3WEqpswgy9dN7EQB3Fjn Signed-off-by: nmbro --- clients/upsclient.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) 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 */ From 9df69aa9c3bc2013873980259c0e56e078424b2e Mon Sep 17 00:00:00 2001 From: nmbro Date: Fri, 23 Oct 2026 09:47:58 +0200 Subject: [PATCH 3/4] NEWS.adoc: note upsmon CERT* defaults and CERTPATH file fixes [#3797] Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Ju3WEqpswgy9dN7EQB3Fjn Signed-off-by: nmbro --- NEWS.adoc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/NEWS.adoc b/NEWS.adoc index 95925e9291..536eed9316 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -762,6 +762,11 @@ 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). [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. From 6a204c6a06c0adf3866d8f33944b6036f4153ceb Mon Sep 17 00:00:00 2001 From: nmbro Date: Sat, 24 Oct 2026 10:44:25 +0200 Subject: [PATCH 4/4] clients/upsmon.c: re-fetch default authconf entry upsmon keeps a pointer to the global authconf section, which the legacy CERTPATH, CERTFILE, CERTIDENT, CERTVERIFY and FORCESSL keywords in upsmon.conf populate. If a nutauth.conf file is then read (named by AUTHCONF or found in a default location), upscli_read_authconf_file() replaces the whole list and that pointer is left dangling, so on a reload (SIGHUP or "upsmon -c reload") such keywords wrote into freed memory (AddressSanitizer reports a heap-use-after-free in parse_conf_arg()). Fetch the default entry again after reading the file (creating it if the file had no global section). This does not yet merge the file into the settings from upsmon.conf as UPGRADING.adoc and upsmon.conf(5) describe; that is left for a separate change. Also clarify in authconf.h that an explicitly named file always replaces the list; only the search for a default file keeps it when nothing is found. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Ju3WEqpswgy9dN7EQB3Fjn Signed-off-by: nmbro --- NEWS.adoc | 4 +++- clients/authconf.h | 5 +++-- clients/upsmon.c | 10 ++++++++++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/NEWS.adoc b/NEWS.adoc index 92119fa957..f27663a4dc 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -766,7 +766,9 @@ https://github.com/networkupstools/nut/milestone/13 `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). [PR #3797] + (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.h b/clients/authconf.h index c5a68ef607..3d062c2a63 100644 --- a/clients/authconf.h +++ b/clients/authconf.h @@ -59,8 +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 only - * if a file was found to read; otherwise it is kept as it was. + * 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); 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.