Merge pull request #3497 from jimklimov/issue-3454

Try to avoid losing client outputs if exit clean-up crashes; try to avoid (Open)SSL-related crashes there

Author: jimklimov

NEWS.adoc

@@ -162,6 +162,11 @@ https://github.com/networkupstools/nut/milestone/13
     * The `libupsclient` (C) and `libnutclient` (C++) API were updated to
       report the ability to check `CERTIDENT` information. [#3331]
 
+ - Various clients:
+    * Flush standard output and error buffers before handling clean exit
+      rituals, to avoid losing the result of client's work in case that
+      clean-up crashes (e.g. due to third-party code involved). [#3454]
+
  - `upsimage.cgi` client updates:
     * Adjust scaling for small (single-digit) value ranges to avoid division
       by zero and not rendering anything. [issue #3469]

clients/upsc.c

@@ -381,6 +381,11 @@ static void list_clients(const char *devname)
 
 static void clean_exit(void)
 {
+	/* Flush *our* output before possibly failing in third-party code
+	 * (e.g. SSL libs), so client consumers have a chance to see it */
+	fflush(stdout);
+	fflush(stderr);
+
 	if (ups) {
 		upscli_disconnect(ups);
 	}

clients/upsclient.c

@@ -173,7 +173,7 @@ static struct timeval upscli_default_connect_timeout = {0, 0};
 static int upscli_default_connect_timeout_initialized = 0;
 
 #ifdef WITH_OPENSSL
-static SSL_CTX	*ssl_ctx;
+static SSL_CTX	*ssl_ctx = NULL;
 #endif	/* WITH_OPENSSL */
 
 #ifdef WITH_NSS
@@ -832,6 +832,7 @@ int upscli_init2(int certverify, const char *certpath,
 	ret = SSL_CTX_set_min_proto_version(ssl_ctx, TLS1_VERSION);
 	if (ret != 1) {
 		upslogx(LOG_ERR, "Can not set minimum protocol to TLSv1");
+		upscli_cleanup();
 		return -1;
 	}
 # endif
@@ -841,6 +842,7 @@ int upscli_init2(int certverify, const char *certpath,
 			upslogx(LOG_ERR, "Can not verify certificate if any is specified: no CERTPATH was given");
 			/* Failed: checking the server cert is mandatory, but no
 			 * collection of trusted CA/server cert files was given */
+			upscli_cleanup();
 			return -1;
 		}
 	} else {
@@ -863,6 +865,7 @@ int upscli_init2(int certverify, const char *certpath,
 			if ((ret = SSL_CTX_load_verify_locations(ssl_ctx, certpath, NULL)) != 1) {
 				ssl_debug();
 				upslogx(LOG_ERR, "Failed to load CA certificate(s) from directory or file %s", certpath);
+				upscli_cleanup();
 				return -1;
 			} else {
 				upsdebugx(1, "%s: ...but succeeded to load CA certificate(s) from file %s", __func__, certpath);
@@ -918,6 +921,7 @@ int upscli_init2(int certverify, const char *certpath,
 #   else	/* Not SSL_* methods either */
 
 		upslogx(LOG_ERR, "Private key password support not implemented for OpenSSL < ~0.9.6..~1.1 yet");
+		upscli_cleanup();
 		return -1;
 #   endif
 #  endif	/* ...SET_DEFAULT_PASSWD_CB */
@@ -930,16 +934,19 @@ int upscli_init2(int certverify, const char *certpath,
 		if ((ssl_ret = SSL_CTX_use_certificate_chain_file(ssl_ctx, certfile)) != 1) {
 			upslogx(LOG_ERR, "Failed to load client certificate from %s", certfile);
 			ssl_debug();
+			upscli_cleanup();
 			return -1;
 		}
 		if ((ssl_ret = SSL_CTX_use_PrivateKey_file(ssl_ctx, certfile, SSL_FILETYPE_PEM)) != 1) {
 			upslogx(LOG_ERR, "Failed to load client private key from %s", certfile);
 			ssl_debug();
+			upscli_cleanup();
 			return -1;
 		}
 		if ((ssl_ret = SSL_CTX_check_private_key(ssl_ctx)) != 1) {
 			upslogx(LOG_ERR, "Failed to check client private key from %s", certfile);
 			ssl_debug();
+			upscli_cleanup();
 			return -1;
 		}
 
@@ -976,6 +983,7 @@ int upscli_init2(int certverify, const char *certpath,
 							OPENSSL_free(subject);
 						}
 						upslogx(LOG_ERR, "Unexpected certificate provided");
+						upscli_cleanup();
 						return -1;
 					} else {
 						upsdebugx(2, "Certificate subject verified against CERTIDENT subject name (%s)", sslcertname);
@@ -986,12 +994,14 @@ int upscli_init2(int certverify, const char *certpath,
 			}
 #  else	/* Missing X509 methods wanted above */
 			upslogx(LOG_ERR, "Can not verify CERTIDENT '%s': not supported in this OpenSSL build (too old)", sslcertname);
+			upscli_cleanup();
 			return -1;
 #  endif	/* Got ways to check CERTIDENT? */
 		}	/* else: CERTIDENT did not pass a name, nothing to check */
 	} else {
 		if (sslcertname && *sslcertname) {
 			upslogx(LOG_ERR, "Can not verify CERTIDENT '%s': no CERTFILE was provided", sslcertname);
+			upscli_cleanup();
 			return -1;
 		}
 	}
@@ -1024,13 +1034,15 @@ int upscli_init2(int certverify, const char *certpath,
 	if (status != SECSuccess) {
 		upslogx(LOG_ERR, "Can not initialize SSL context");
 		nss_error("upscli_init / NSS_[NoDB]_Init");
+		upscli_cleanup();
 		return -1;
 	}
 
 	status = NSS_SetDomesticPolicy();
 	if (status != SECSuccess) {
 		upslogx(LOG_ERR, "Can not initialize SSL policy");
 		nss_error("upscli_init / NSS_SetDomesticPolicy");
+		upscli_cleanup();
 		return -1;
 	}
 
@@ -1040,25 +1052,29 @@ int upscli_init2(int certverify, const char *certpath,
 	if (status != SECSuccess) {
 		upslogx(LOG_ERR, "Can not enable SSLv3");
 		nss_error("upscli_init / SSL_OptionSetDefault(SSL_ENABLE_SSL3)");
+		upscli_cleanup();
 		return -1;
 	}
 	status = SSL_OptionSetDefault(SSL_ENABLE_TLS, PR_TRUE);
 	if (status != SECSuccess) {
 		upslogx(LOG_ERR, "Can not enable TLSv1");
 		nss_error("upscli_init / SSL_OptionSetDefault(SSL_ENABLE_TLS)");
+		upscli_cleanup();
 		return -1;
 	}
 	status = SSL_OptionSetDefault(SSL_V2_COMPATIBLE_HELLO, PR_FALSE);
 	if (status != SECSuccess) {
 		upslogx(LOG_ERR, "Can not disable SSLv2 hello compatibility");
 		nss_error("upscli_init / SSL_OptionSetDefault(SSL_V2_COMPATIBLE_HELLO)");
+		upscli_cleanup();
 		return -1;
 	}
 	verify_certificate = certverify;
 #else
 	/* Note: historically we do not return with error here,
 	 * and nowadays have the default timeout handling etc.,
 	 * just fall through to below and treat as initialized.
+	 * There's nothing to retry to change that state anyway.
 	 */
 	if (certverify || certpath || certname || certpasswd || certfile) {
 		upslogx(LOG_ERR, "upscli_init called but SSL wasn't compiled in");
@@ -1118,7 +1134,6 @@ int upscli_cleanup(void)
 		SSL_CTX_free(ssl_ctx);
 		ssl_ctx = NULL;
 	}
-
 #endif /* WITH_OPENSSL */
 
 #ifdef WITH_NSS

clients/upscmd.c

@@ -299,6 +299,11 @@ static void do_cmd(char **argv, const int argc)
 
 static void clean_exit(void)
 {
+	/* Flush *our* output before possibly failing in third-party code
+	 * (e.g. SSL libs), so client consumers have a chance to see it */
+	fflush(stdout);
+	fflush(stderr);
+
 	if (ups) {
 		upscli_disconnect(ups);
 	}

clients/upsimage.c

@@ -656,6 +656,11 @@ static int get_var(const char *var, char *buf, size_t buflen)
 
 static void clean_exit(void)
 {
+	/* Flush *our* output before possibly failing in third-party code
+	 * (e.g. SSL libs), so client consumers have a chance to see it */
+	fflush(stdout);
+	fflush(stderr);
+
 	upscli_cleanup();
 
 	upsdebugx(1, "%s: finished, exiting", __func__);

clients/upslog.c

@@ -1065,6 +1065,11 @@ int main(int argc, char **argv)
 	upslogx(LOG_INFO, "Signal %d: exiting", exit_flag);
 	upsnotify(NOTIFY_STATE_STOPPING, "Signal %d: exiting", exit_flag);
 
+	/* Flush *our* output before possibly failing in third-party code
+	 * (e.g. SSL libs), so client consumers have a chance to see it */
+	fflush(stdout);
+	fflush(stderr);
+
 	for (
 		monhost_ups_current = monhost_ups_anchor;
 		monhost_ups_current != NULL;
@@ -1087,6 +1092,9 @@ int main(int argc, char **argv)
 		logformat = NULL;
 	}
 
+	fflush(stdout);
+	fflush(stderr);
+
 	upscli_cleanup();
 	exit(EXIT_SUCCESS);
 }

clients/upsmon.c

@@ -2777,6 +2777,11 @@ static void upsmon_cleanup(void)
 	int	i;
 	utype_t	*utmp, *unext;
 
+	/* Flush *our* output before possibly failing in third-party code
+	 * (e.g. SSL libs), so client consumers have a chance to see it */
+	fflush(stdout);
+	fflush(stderr);
+
 	/* close all fds */
 	utmp = firstups;
 
@@ -2799,6 +2804,9 @@ static void upsmon_cleanup(void)
 		free(notifylist[i].msg);
 	}
 
+	fflush(stdout);
+	fflush(stderr);
+
 	upscli_cleanup();
 
 #ifdef WIN32

clients/upsrw.c

@@ -91,6 +91,11 @@ static void help(const char *prog)
 
 static void clean_exit(void)
 {
+	/* Flush *our* output before possibly failing in third-party code
+	 * (e.g. SSL libs), so client consumers have a chance to see it */
+	fflush(stdout);
+	fflush(stderr);
+
 	if (ups) {
 		upscli_disconnect(ups);
 	}

clients/upssched.c

@@ -2270,5 +2270,11 @@ int main(int argc, char **argv)
 	checkconf();
 
 	upsdebugx(1, "Exiting upssched (CLI process)");
+
+	/* Flush *our* output before possibly failing in third-party code
+	 * (e.g. SSL libs), so client consumers have a chance to see it */
+	fflush(stdout);
+	fflush(stderr);
+
 	exit(EXIT_SUCCESS);
 }

clients/upsset.c

@@ -1117,6 +1117,11 @@ static void check_conf(void)
 
 static void clean_exit(void)
 {
+	/* Flush *our* output before possibly failing in third-party code
+	 * (e.g. SSL libs), so client consumers have a chance to see it */
+	fflush(stdout);
+	fflush(stderr);
+
 	upscli_cleanup();
 
 	upsdebugx(1, "%s: finished, exiting", __func__);