Merge pull request #3527 from jimklimov/issue-3525

`upssched` and other NUT daemons: fix Windows state pipe permissions

Author: jimklimov

NEWS.adoc

@@ -133,6 +133,7 @@ https://github.com/networkupstools/nut/milestone/13
       `connect()` to ensure a timeout if something does not respond. [PR #3456]
     * Windows driver state named pipes now use explicit security attributes
       instead of relying on default settings from the launch context. [PR #3479]
+    * Likewise for named pipes in other NUT daemons and `upssched`. [PR #3527]
     * Drivers with libusb-1.0 code path, and `nutdrv_qx` in particular: revise
       to fix per-close `libusb_exit()` deadlock, and escalate a persistent
       `LIBUSB_ERROR_OVERFLOW` state to USB reset handling. [PR #3448]
@@ -207,6 +208,10 @@ https://github.com/networkupstools/nut/milestone/13
       and wake-up, or NTP/RTC clock changes during boot, or severe delays
       on a stressed system (to name a few practical cases). [issue #3405]
 
+ - `upssched` client/tool updates:
+    * Fixed handling of `NOTIFYMSG` from command line if other arguments are
+      present (e.g. debugging with `-DDDDDD`). [issues #3105, #3525, PR #3527]
+
  - `NUT-Monitor` Python GUI client:
     * Fixed Qt tray tooltips to render in plain text, not rich text which
       ended up as literal HTML on KDE Plasma 6; now that rich text formatting

clients/upssched.c

@@ -763,19 +763,24 @@ static TYPE_FD open_sock(void)
 	set_close_on_exec(fd);
 
 #else /* WIN32 */
+	SECURITY_ATTRIBUTES	pipe_sa;
+	SECURITY_DESCRIPTOR	pipe_sd;
+
+	init_pipe_security(&pipe_sa, &pipe_sd);
 
 	fd = CreateNamedPipe(
-			pipefn, /* pipe name */
-			PIPE_ACCESS_DUPLEX | /* read/write access */
-			FILE_FLAG_OVERLAPPED, /* async IO */
-			PIPE_TYPE_BYTE |
-			PIPE_READMODE_BYTE |
-			PIPE_WAIT,
-			PIPE_UNLIMITED_INSTANCES, /* max. instances */
-			BUF_LEN, /* output buffer size */
-			BUF_LEN, /* input buffer size */
-			0, /* client time-out */
-			NULL); /* FIXME: default security attributes */
+			pipefn,		/* pipe name */
+			PIPE_ACCESS_DUPLEX	/* read/write access */
+			| FILE_FLAG_OVERLAPPED,	/* async IO */
+			PIPE_TYPE_BYTE
+			| PIPE_READMODE_BYTE
+			| PIPE_REJECT_REMOTE_CLIENTS	/* local host only */
+			| PIPE_WAIT,
+			PIPE_UNLIMITED_INSTANCES,	/* max. instances */
+			BUF_LEN,	/* output buffer size */
+			BUF_LEN,	/* input buffer size */
+			0,		/* client time-out */
+			&pipe_sa);	/* default security attributes */
 
 	if (INVALID_FD(fd)) {
 		fatal_with_errno(EXIT_FAILURE,
@@ -998,25 +1003,30 @@ static TYPE_FD conn_add(TYPE_FD sockfd)
 #else /* WIN32 */
 
 	conn_t	*conn, *tmp, *last;
+	SECURITY_ATTRIBUTES	pipe_sa;
+	SECURITY_DESCRIPTOR	pipe_sd;
 
 	/* We have detected a connection on the opened pipe. So we start
 	 * by saving its handle and creating a new pipe for future connection */
 	conn = xcalloc(1, sizeof(*conn));
 	conn->fd = sockfd;
 
+	init_pipe_security(&pipe_sa, &pipe_sd);
+
 	/* sock is the handle of the connection pending pipe */
 	acc = CreateNamedPipe(
-			pipefn, /* pipe name */
-			PIPE_ACCESS_DUPLEX |  /* read/write access */
-			FILE_FLAG_OVERLAPPED, /* async IO */
-			PIPE_TYPE_BYTE |
-			PIPE_READMODE_BYTE |
-			PIPE_WAIT,
-			PIPE_UNLIMITED_INSTANCES, /* max. instances */
-			BUF_LEN, /* output buffer size */
-			BUF_LEN, /* input buffer size */
-			0, /* client time-out */
-			NULL); /* FIXME: default security attribute */
+			pipefn,		/* pipe name */
+			PIPE_ACCESS_DUPLEX	/* read/write access */
+			| FILE_FLAG_OVERLAPPED,	/* async IO */
+			PIPE_TYPE_BYTE
+			| PIPE_READMODE_BYTE
+			| PIPE_REJECT_REMOTE_CLIENTS	/* local host only */
+			| PIPE_WAIT,
+			PIPE_UNLIMITED_INSTANCES,	/* max. instances */
+			BUF_LEN,	/* output buffer size */
+			BUF_LEN,	/* input buffer size */
+			0,		/* client time-out */
+			&pipe_sa);	/* default security attribute */
 
 	if (INVALID_FD(acc)) {
 		fatal_with_errno(EXIT_FAILURE,
@@ -2352,20 +2362,19 @@ static void help(const char *arg_progname)
 
 	nut_report_config_flags();
 
-	printf("\n%s", suggest_doc_links(arg_progname, "upsmon.conf"));
+	printf("\n%s", suggest_doc_links(arg_progname, "upssched.conf"));
 
 	exit(EXIT_SUCCESS);
 }
 
 int main(int argc, char **argv)
 {
-	int	opt_ret, argn = 0;
+	int	opt_ret;
 
 	/* Here this is a global variable, used also in start_daemon() */
 	prog = getprogname_argv0_default(argc > 0 ? argv[0] : NULL, "upssched");
 
 	while ((opt_ret = getopt(argc, argv, optstring)) != -1) {
-		argn++;
 		switch (opt_ret) {
 			case 'D':
 				nut_debug_level_args++;
@@ -2411,9 +2420,9 @@ int main(int argc, char **argv)
 
 	ups_name = getenv("UPSNAME");
 	notify_type = getenv("NOTIFYTYPE");
-	upsdebugx(2, "Remaining argn=%d of argc=%d", argn, argc);
-	if (argc > argn + 1 && *argv[argn + 1])
-		notify_msg = argv[argn + 1];
+	upsdebugx(2, "Handled optind=%d CLI tokens of argc=%d", optind - 1, argc);
+	if (argc > optind && *argv[optind])
+		notify_msg = argv[optind];
 
 	if ((!list_timers) && ((!ups_name) || (!notify_type))) {
 		printf("Error: environment variables UPSNAME and NOTIFYTYPE must be set.\n");

common/wincompat.c

@@ -454,7 +454,7 @@ void syslog(int priority, const char *fmt, ...)
 	/* At least while this is not configurable, can be static to speed up;
 	 * see https://github.com/networkupstools/nut/issues/3375
 	 */
-	static char	pipe_full_name[] = "\\\\.\\pipe\\"EVENTLOG_PIPE_NAME;
+	static char	pipe_full_name[] = "\\\\.\\pipe\\"EVENTLOG_PIPE_NAME, reported_no_pipe = 0, reentered = 0;
 	char	buf1[LARGEBUF + sizeof(DWORD)];
 	char	buf2[LARGEBUF];
 	va_list	ap;
@@ -465,6 +465,13 @@ void syslog(int priority, const char *fmt, ...)
 		return;
 	}
 
+	/* Could an upsdebugx() below cause re-syslog?.. */
+	if (reentered) {
+		return;
+	}
+
+	reentered = 1;
+
 	/* Format message */
 	va_start(ap, fmt);
 	vsnprintf(buf1, sizeof(buf1), fmt, ap);
@@ -490,20 +497,29 @@ void syslog(int priority, const char *fmt, ...)
 		NULL);			/* no template file */
 
 	if (pipe == INVALID_HANDLE_VALUE) {
-		upsdebug_with_errno(1,
+		upsdebug_with_errno(reported_no_pipe ? 7 : 1,
 			"%s: SKIP: can't open existing event log NAMED_PIPE: '%s'",
 			__func__, pipe_full_name);
 
+		reported_no_pipe = 1;
+		reentered = 0;
 		return;
 	}
 
+	if (reported_no_pipe)
+		upsdebugx(1, "%s: opened existing event log NAMED_PIPE which we failed to use earlier: '%s'",
+			__func__, pipe_full_name);
+	reported_no_pipe = 0;
+
 	WriteFile(pipe, buf1, strlen(buf2) + sizeof(DWORD), &bytesWritten, NULL);
 
 	/* testing result is useless. If we have an error and try to report it,
 	 * this will probably lead to a call to this function and an infinite
 	 * loop */
 	upsdebugx(6, "%s: closing event log NAMED_PIPE", __func__);
 	CloseHandle(pipe);
+
+	reentered = 0;
 }
 
 /* Signal emulation via NamedPipe */
@@ -515,10 +531,27 @@ static const char	*named_pipe_name=NULL;
 OVERLAPPED		pipe_connection_overlapped;
 pipe_conn_t		*pipe_connhead = NULL;
 
+void init_pipe_security(SECURITY_ATTRIBUTES *sa, SECURITY_DESCRIPTOR *sd)
+{
+	if (!InitializeSecurityDescriptor(sd, SECURITY_DESCRIPTOR_REVISION)) {
+		fatal_with_errno(EXIT_FAILURE, "InitializeSecurityDescriptor failed");
+	}
+
+	if (!SetSecurityDescriptorDacl(sd, TRUE, NULL, FALSE)) {
+		fatal_with_errno(EXIT_FAILURE, "SetSecurityDescriptorDacl failed");
+	}
+
+	sa->nLength = sizeof(*sa);
+	sa->lpSecurityDescriptor = sd;
+	sa->bInheritHandle = FALSE;
+}
+
 void pipe_create(const char * pipe_name)
 {
 	BOOL	ret;
 	char	pipe_full_name[NUT_PATH_MAX + 1];
+	SECURITY_ATTRIBUTES	pipe_sa;
+	SECURITY_DESCRIPTOR	pipe_sd;
 
 	/* save pipe name for further use in pipe_connect */
 	if (pipe_name == NULL) {
@@ -537,19 +570,22 @@ void pipe_create(const char * pipe_name)
 		CloseHandle(pipe_connection_overlapped.hEvent);
 	}
 	memset(&pipe_connection_overlapped, 0, sizeof(pipe_connection_overlapped));
+	init_pipe_security(&pipe_sa, &pipe_sd);
+
 	upsdebugx(2, "%s: creating NAMED_PIPE (listener): '%s'", __func__, pipe_full_name);
 	pipe_connection_handle = CreateNamedPipe(
 		pipe_full_name,
 		PIPE_ACCESS_INBOUND	/* to server only */
 		| FILE_FLAG_OVERLAPPED,	/* async IO */
 		PIPE_TYPE_MESSAGE
 		| PIPE_READMODE_MESSAGE
+		| PIPE_REJECT_REMOTE_CLIENTS	/* local host only */
 		| PIPE_WAIT,
 		PIPE_UNLIMITED_INSTANCES,	/* max. instances */
 		LARGEBUF,		/* output buffer size */
 		LARGEBUF,		/* input buffer size */
 		0,			/* client time-out */
-		NULL);			/* FIXME: default security attribute */
+		&pipe_sa);		/* default security attribute */
 
 	if (pipe_connection_handle == INVALID_HANDLE_VALUE) {
 		upslogx(LOG_ERR, "Error creating named pipe");

drivers/dstate.c

@@ -66,23 +66,6 @@
 	double			previous_battery_charge_value = -1.0;
 	st_tree_timespec_t	previous_battery_charge_timestamp;
 
-#ifdef WIN32
-static void init_pipe_security(SECURITY_ATTRIBUTES *sa, SECURITY_DESCRIPTOR *sd)
-{
-	if (!InitializeSecurityDescriptor(sd, SECURITY_DESCRIPTOR_REVISION)) {
-		fatal_with_errno(EXIT_FAILURE, "InitializeSecurityDescriptor failed");
-	}
-
-	if (!SetSecurityDescriptorDacl(sd, TRUE, NULL, FALSE)) {
-		fatal_with_errno(EXIT_FAILURE, "SetSecurityDescriptorDacl failed");
-	}
-
-	sa->nLength = sizeof(*sa);
-	sa->lpSecurityDescriptor = sd;
-	sa->bInheritHandle = FALSE;
-}
-#endif	/* WIN32 */
-
 #ifndef WIN32
 /* this may be a frequent stumbling point for new users, so be verbose here */
 static void sock_fail(const char *fn)
@@ -223,6 +206,7 @@ static TYPE_FD sock_open(const char *fn)
 		| FILE_FLAG_OVERLAPPED,	/* async IO */
 		PIPE_TYPE_BYTE
 		| PIPE_READMODE_BYTE
+		| PIPE_REJECT_REMOTE_CLIENTS	/* local host only */
 		| PIPE_WAIT,
 		PIPE_UNLIMITED_INSTANCES,	/* max. instances */
 		ST_SOCK_BUF_LEN,	/* output buffer size */
@@ -681,6 +665,7 @@ static void sock_connect(TYPE_FD sock)
 		| FILE_FLAG_OVERLAPPED,	/* async IO */
 		PIPE_TYPE_BYTE
 		| PIPE_READMODE_BYTE
+		| PIPE_REJECT_REMOTE_CLIENTS	/* local host only */
 		| PIPE_WAIT,
 		PIPE_UNLIMITED_INSTANCES,	/* max. instances */
 		ST_SOCK_BUF_LEN,	/* output buffer size */

include/wincompat.h

@@ -1,6 +1,3 @@
-#ifndef NUT_WINCOMPAT_H
-#define NUT_WINCOMPAT_H 1
-
 /*
    Copyright (C)
 	2001	Andrew Delpha (delpha@computer.org)
@@ -29,6 +26,9 @@
    with the NUT code.
 */
 
+#ifndef NUT_WINCOMPAT_H
+#define NUT_WINCOMPAT_H 1
+
 #include "common.h"
 #include <limits.h>
 #include "nut_stdint.h"
@@ -157,6 +157,7 @@ typedef struct pipe_conn_s {
 extern pipe_conn_t *pipe_connhead;
 extern OVERLAPPED pipe_connection_overlapped;
 void pipe_create(const char * pipe_name);
+void init_pipe_security(SECURITY_ATTRIBUTES *sa, SECURITY_DESCRIPTOR *sd);
 void pipe_connect();
 void pipe_disconnect(pipe_conn_t *conn);
 int pipe_ready(pipe_conn_t *conn);