Merge pull request #2548 from jimklimov/issue-2463

Refactor and fix check for process name by PID; update docs for ASCII in config files

Author: jimklimov

clients/upsclient.c

@@ -1025,7 +1025,7 @@ int upscli_tryconnect(UPSCONN_t *ups, const char *host, uint16_t port, int flags
 	ups->fd = -1;
 
 	if (!host) {
-		upslogx(LOG_WARNING, "%s: Host not found: '%s'", __func__, NUT_STRARG(host));
+		upslogx(LOG_WARNING, "%s: Host not specified", __func__);
 		ups->upserror = UPSCLI_ERR_NOSUCHHOST;
 		return -1;
 	}

common/common.c

@@ -919,10 +919,26 @@ size_t parseprogbasename(char *buf, size_t buflen, const char *progname, size_t
 	return progbasenamelen;
 }
 
-int checkprocname(pid_t pid, const char *progname)
+int checkprocname_ignored(const char *caller)
 {
-	/* If we can determine the binary path name of the specified "pid",
+	char	*s = NULL;
+
+	if ((s = getenv("NUT_IGNORE_CHECKPROCNAME"))) {
+		/* FIXME: Make server/conf.c::parse_boolean() reusable */
+		if ( (!strcasecmp(s, "true")) || (!strcasecmp(s, "on")) || (!strcasecmp(s, "yes")) || (!strcasecmp(s, "1"))) {
+			upsdebugx(1, "%s for %s: skipping because caller set NUT_IGNORE_CHECKPROCNAME", __func__, NUT_STRARG(caller));
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
+int compareprocname(pid_t pid, const char *procname, const char *progname)
+{
+	/* Given the binary path name of (presumably) a running process,
 	 * check if it matches the assumed name of the current program.
+	 * The "pid" value is used in log reporting.
 	 * Returns:
 	 *	-3	Skipped because NUT_IGNORE_CHECKPROCNAME is set
 	 *	-2	Could not parse a program name (ok to proceed,
@@ -932,9 +948,9 @@ int checkprocname(pid_t pid, const char *progname)
 	 *	0	Process name identified, does not seem to match
 	 *	1+	Process name identified, and seems to match with
 	 *		varying precision
-	 * Generally speaking, if (checkprocname(...)) then ok to proceed
+	 * Generally speaking, if (compareprocname(...)) then ok to proceed
 	 */
-	char	*procname = NULL, *s;
+
 	int	ret = -127;
 	size_t	procbasenamelen = 0, progbasenamelen = 0;
 	/* Track where the last dot is in the basename; 0 means none */
@@ -945,16 +961,11 @@ int checkprocname(pid_t pid, const char *progname)
 	char	procbasename[PATH_MAX], progbasename[PATH_MAX];
 #endif
 
-	if ((s = getenv("NUT_IGNORE_CHECKPROCNAME"))) {
-		/* FIXME: Make server/conf.c::parse_boolean() reusable */
-		if ( (!strcasecmp(s, "true")) || (!strcasecmp(s, "on")) || (!strcasecmp(s, "yes")) || (!strcasecmp(s, "1"))) {
-			upsdebugx(1, "%s: skipping because caller set NUT_IGNORE_CHECKPROCNAME", __func__);
-			ret = -3;
-			goto finish;
-		}
+	if (checkprocname_ignored(__func__)) {
+		ret = -3;
+		goto finish;
 	}
 
-	procname = getprocname(pid);
 	if (!procname || !progname) {
 		ret = -1;
 		goto finish;
@@ -975,7 +986,7 @@ int checkprocname(pid_t pid, const char *progname)
 	}
 
 	/* First quickly try for an exact hit of base names */
-	if (progbasenamelen == procbasenamelen && progbasenamedot == procbasenamedot && !strcmp(procname, progname)) {
+	if (progbasenamelen == procbasenamelen && progbasenamedot == procbasenamedot && !strcmp(procbasename, progbasename)) {
 		ret = 2;
 		goto finish;
 	}
@@ -1091,6 +1102,42 @@ int checkprocname(pid_t pid, const char *progname)
 	return ret;
 }
 
+int checkprocname(pid_t pid, const char *progname)
+{
+	/* If we can determine the binary path name of the specified "pid",
+	 * check if it matches the assumed name of the current program.
+	 * Returns: same as compareprocname()
+	 * Generally speaking, if (checkprocname(...)) then ok to proceed
+	 */
+	char	*procname = NULL;
+	int	ret = 0;
+
+	/* Quick skip before drilling into getprocname() */
+	if (checkprocname_ignored(__func__)) {
+		ret = -3;
+		goto finish;
+	}
+
+	if (!progname) {
+		ret = -1;
+		goto finish;
+	}
+
+	procname = getprocname(pid);
+	if (!procname) {
+		ret = -1;
+		goto finish;
+	}
+
+	ret = compareprocname(pid, procname, progname);
+
+finish:
+	if (procname)
+		free(procname);
+
+	return ret;
+}
+
 #ifdef WIN32
 /* In WIN32 all non binaries files (namely configuration and PID files)
    are retrieved relative to the path of the binary itself.
@@ -1154,7 +1201,7 @@ int sendsignalpid(pid_t pid, int sig, const char *progname, int check_current_pr
 {
 #ifndef WIN32
 	int	ret, cpn1 = -10, cpn2 = -10;
-	char	*current_progname = NULL;
+	char	*current_progname = NULL, *procname = NULL;
 
 	/* TOTHINK: What about containers where a NUT daemon *is* the only process
 	 * and is the PID=1 of the container (recycle if dead)? */
@@ -1167,9 +1214,12 @@ int sendsignalpid(pid_t pid, int sig, const char *progname, int check_current_pr
 	}
 
 	ret = 0;
-	if (progname) {
+	if (!checkprocname_ignored(__func__))
+		procname = getprocname(pid);
+
+	if (procname && progname) {
 		/* Check against some expected (often built-in) name */
-		if (!(cpn1 = checkprocname(pid, progname))) {
+		if (!(cpn1 = compareprocname(pid, procname, progname))) {
 			/* Did not match expected (often built-in) name */
 			ret = -1;
 		} else {
@@ -1182,13 +1232,13 @@ int sendsignalpid(pid_t pid, int sig, const char *progname, int check_current_pr
 	}
 	/* if (cpn1 == -3) => NUT_IGNORE_CHECKPROCNAME=true */
 	/* if (cpn1 == -1) => could not determine name of PID... retry just in case? */
-	if (ret <= 0 && check_current_progname && cpn1 != -3) {
+	if (procname && ret <= 0 && check_current_progname && cpn1 != -3) {
 		/* NOTE: This could be optimized a bit by pre-finding the procname
 		 * of "pid" and re-using it, but this is not a hot enough code path
 		 * to bother much.
 		 */
 		current_progname = getprocname(getpid());
-		if (current_progname && (cpn2 = checkprocname(pid, current_progname))) {
+		if (current_progname && (cpn2 = compareprocname(pid, procname, current_progname))) {
 			if (cpn2 > 0) {
 				/* Matched current process as asked, ok to proceed */
 				ret = 2;
@@ -1206,17 +1256,23 @@ int sendsignalpid(pid_t pid, int sig, const char *progname, int check_current_pr
 	}
 
 	/* if ret == 0, ok to proceed - not asked for any sanity checks;
-	 * if ret > 0 we had some definitive match above
+	 * if ret > 0, ok to proceed - we had some definitive match above;
+	 * if ret < 0, NOT OK to proceed - we had some definitive fault above
 	 */
 	if (ret < 0) {
 		upsdebugx(1,
 			"%s: ran at least one check, and all such checks "
 			"found a process name for PID %" PRIuMAX " and "
-			"failed to match: expected progname='%s' (res=%d), "
-			"current progname='%s' (res=%d)",
+			"failed to match: "
+			"found procname='%s', "
+			"expected progname='%s' (res=%d%s), "
+			"current progname='%s' (res=%d%s)",
 			__func__, (uintmax_t)pid,
+			NUT_STRARG(procname),
 			NUT_STRARG(progname), cpn1,
-			NUT_STRARG(current_progname), cpn2);
+			(cpn1 == -10 ? ": did not check" : ""),
+			NUT_STRARG(current_progname), cpn2,
+			(cpn2 == -10 ? ": did not check" : ""));
 
 		if (nut_debug_level > 0 || nut_sendsignal_debug_level > 1) {
 			switch (ret) {
@@ -1267,14 +1323,25 @@ int sendsignalpid(pid_t pid, int sig, const char *progname, int check_current_pr
 			current_progname = NULL;
 		}
 
+		if (procname) {
+			free(procname);
+			procname = NULL;
+		}
+
 		/* Logged or not, sanity-check was requested and failed */
 		return -1;
 	}
+
 	if (current_progname) {
 		free(current_progname);
 		current_progname = NULL;
 	}
 
+	if (procname) {
+		free(procname);
+		procname = NULL;
+	}
+
 	/* see if this is going to work first - does the process exist,
 	 * and do we have permissions to signal it? */
 	ret = kill(pid, 0);

conf/hosts.conf.sample

@@ -3,6 +3,9 @@
 # This file is used to control the CGI programs.  If you have not
 # installed them, you may safely ignore or delete this file.
 #
+# NOTE: Contents of this file should be pure ASCII (character codes
+# not in range would be ignored with a warning message).
+#
 # -----------------------------------------------------------------------
 #
 # upsstats will use the list of MONITOR entries when displaying the

conf/nut.conf.sample

@@ -14,6 +14,8 @@
 #  You MUST NOT use spaces around the equal sign!
 #  Practical support for this file and its settings currently varies between
 #  various OS packages and NUT sample scripts, but should converge over time.
+#  Contents of this file should be pure ASCII (character codes not in range
+#  would be ignored with a warning message).
 #
 # See also: `man nut.conf` (usually in Manual pages Section 5,
 #           for Configuration files)

conf/ups.conf.sample

@@ -9,6 +9,9 @@
 #
 # ---
 #
+# NOTE: Contents of this file should be pure ASCII (character codes
+# not in range would be ignored with a warning message).
+#
 # This is where you configure all the UPSes that this system will be
 # monitoring directly.  These are usually attached to serial ports, but
 # USB devices and SNMP devices are also supported.

conf/upsd.conf.sample

@@ -7,6 +7,9 @@
 # Each entry below provides usage and default value.
 #
 # For more information, refer to upsd.conf manual page.
+#
+# NOTE: Contents of this file should be pure ASCII (character codes
+# not in range would be ignored with a warning message).
 
 # =======================================================================
 # MAXAGE <seconds>

conf/upsd.users.sample

@@ -4,6 +4,9 @@
 # Users are defined here, are given passwords, and their privileges are
 # controlled here too.  Since this file will contain passwords, keep it
 # secure, with only enough permissions for upsd to read it.
+#
+# NOTE: Contents of this file should be pure ASCII (character codes
+# not in range would be ignored with a warning message).
 
 # --------------------------------------------------------------------------
 

conf/upsmon.conf.sample.in

@@ -2,6 +2,9 @@
 #
 # This file contains passwords, so keep it secure.
 #
+# NOTE: Contents of this file should be pure ASCII (character codes
+# not in range would be ignored with a warning message).
+#
 # A minimal configuration should include at least one MONITOR instruction,
 # MINSUPPLIES (may be 0 if this system is only monitoring other NUT servers),
 # and a POWERDOWNFLAG if this machine is a "primary" system connected to

conf/upssched.conf.sample.in

@@ -1,5 +1,8 @@
 # Network UPS Tools - upssched.conf sample file
 #
+# NOTE: Contents of this file should be pure ASCII (character codes
+# not in range would be ignored with a warning message).
+#
 # ============================================================================
 #
 # CMDSCRIPT <scriptname>

conf/upsset.conf.sample

@@ -5,6 +5,9 @@
 # the upsset.cgi program from running until you have assured it that you
 # have secured your web server's CGI directory.
 #
+# NOTE: Contents of this file should be pure ASCII (character codes
+# not in range would be ignored with a warning message).
+#
 # By default, your web server will probably let anyone access upsset.cgi
 # once it is installed.  This means that anyone could attempt to crack
 # upsd logins since they would appear to be coming from your web server,