NRPE uses signals unsafely, hangs, can bring down system

Fix for issues:
http://tracker.nagios.org/view.php?id=377
http://tracker.nagios.org/view.php?id=548 (nagios dies on restart while sending command to cmd file)
http://tracker.nagios.org/view.php?id=592 (send_nsca signal handling is undefined behaviour, causes SEGVs)

Replaced all instances of signal() with sigaction() and blocked all signals during
signal processing using sigfillset(). This should prevent any reentrant problems during
signal handling.

Also replaced as many unsafe function calls with safe ones (not very many, mainly some
printf()s to write()s).

Added check for sigaction() to configure.ac

configure.ac

@@ -620,7 +620,7 @@ AC_TRY_COMPILE([#include <sys/time.h>],
                               AC_DEFINE(NEED_GETTIMEOFDAY,1,[Define if gettimeofday is needed])))
 
 dnl Checks for library functions.
-AC_CHECK_FUNCS(memmove select socket strdup strstr strtol strtoul floor)
+AC_CHECK_FUNCS(memmove select socket strdup strstr strtol strtoul floor sigaction)
 AC_CHECK_FUNCS(poll)
 
 AC_MSG_CHECKING(return type of socket size)

plugins-root/check_icmp.c

@@ -378,6 +378,9 @@ main(int argc, char **argv)
 	int icmp_sockerrno, udp_sockerrno, tcp_sockerrno;
 	int result;
 	struct rta_host *host;
+#ifdef HAVE_SIGACTION
+	struct sigaction sig_action;
+#endif
 
 	setlocale (LC_ALL, "");
 	bindtextdomain (PACKAGE, LOCALEDIR);
@@ -575,10 +578,21 @@ main(int argc, char **argv)
 	if(warn.rta > crit.rta) warn.rta = crit.rta;
 	if(warn_down > crit_down) crit_down = warn_down;
 
+#ifdef HAVE_SIGACTION
+	sig_action.sa_sigaction = NULL;
+	sig_action.sa_handler = finish;
+	sigfillset(&sig_action.sa_mask);
+	sig_action.sa_flags = SA_NODEFER|SA_RESTART;
+	sigaction(SIGINT, &sig_action, NULL);
+	sigaction(SIGHUP, &sig_action, NULL);
+	sigaction(SIGTERM, &sig_action, NULL);
+	sigaction(SIGALRM, &sig_action, NULL);
+#else /* HAVE_SIGACTION */
 	signal(SIGINT, finish);
 	signal(SIGHUP, finish);
 	signal(SIGTERM, finish);
 	signal(SIGALRM, finish);
+#endif /* HAVE_SIGACTION */
 	if(debug) printf("Setting alarm timeout to %u seconds\n", timeout);
 	alarm(timeout);
 
@@ -976,10 +990,10 @@ finish(int sig)
 
 	host = list;
 	while(host) {
-		if(debug) puts("");
+		if(debug) write(STDOUT_FILENO, "\n", 1);
 		if(i) {
-			if(i < targets) printf(" :: ");
-			else printf("\n");
+			if(i < targets) write(STDOUT_FILENO, " :: ", 4);
+			else write(STDOUT_FILENO, "\n", 1);
 		}
 		i++;
 		if(!host->icmp_recv) {
@@ -1008,7 +1022,7 @@ finish(int sig)
 	i = 0;
 	host = list;
 	while(host) {
-		if(debug) puts("");
+		if(debug) write(STDOUT_FILENO, "\n", 1);
 		printf("%srta=%0.3fms;%0.3f;%0.3f;0; %spl=%u%%;%u;%u;; %srtmax=%0.3fms;;;; %srtmin=%0.3fms;;;; ",
 			   (targets > 1) ? host->name : "",
 			   host->rta / 1000, (float)warn.rta / 1000, (float)crit.rta / 1000,

plugins/check_smtp.c

@@ -131,7 +131,17 @@ main (int argc, char **argv)
 	struct timeval tv;
 
 	/* Catch pipe errors in read/write - sometimes occurs when writing QUIT */
+#ifdef HAVE_SIGACTION
+	struct sigaction sig_action;
+
+	sig_action.sa_sigaction = NULL;
+	sig_action.sa_handler = SIG_IGN;
+	sigemptyset(&sig_action.sa_mask);
+	sig_action.sa_flags = 0;
+	sigaction(SIGPIPE, &sig_action, NULL);
+#else /* HAVE_SIGACTION */
 	(void) signal (SIGPIPE, SIG_IGN);
+#endif /* HAVE_SIGACTION */
 
 	setlocale (LC_ALL, "");
 	bindtextdomain (PACKAGE, LOCALEDIR);

plugins/netutils.c

@@ -42,10 +42,31 @@ int address_family = AF_INET;
 void
 socket_timeout_alarm_handler (int sig)
 {
+	const char msg1[] = " - Socket timeout";
+	const char msg2[] = " - Abnormal timeout";
+	switch(timeout_state) {
+		case STATE_OK:
+			write(STDOUT_FILENO, "OK", 2);
+			break;
+		case STATE_WARNING:
+			write(STDOUT_FILENO, "WARNING", 7);
+			break;
+		case STATE_CRITICAL:
+			write(STDOUT_FILENO, "CRITICAL", 8);
+			break;
+		case STATE_DEPENDENT:
+			write(STDOUT_FILENO, "DEPENDENT", 9);
+			break;
+		default:
+			write(STDOUT_FILENO, "UNKNOWN", 7);
+			break;
+	}
 	if (sig == SIGALRM)
-		printf (_("%s - Socket timeout after %d seconds\n"), state_text(timeout_state),  timeout_interval);
+		write(STDOUT_FILENO, msg1, sizeof(msg1) - 1);
+/*		printf (_("%s - Socket timeout after %d seconds\n"), state_text(timeout_state),  timeout_interval); */
 	else
-		printf (_("%s - Abnormal timeout after %d seconds\n"), state_text(timeout_state), timeout_interval);
+		write(STDOUT_FILENO, msg2, sizeof(msg2) - 1);
+/*		printf (_("%s - Abnormal timeout after %d seconds\n"), state_text(timeout_state), timeout_interval); */
 
 	exit (timeout_state);
 }

plugins/popen.c

@@ -295,16 +295,20 @@ RETSIGTYPE
 popen_timeout_alarm_handler (int signo)
 {
 	int fh;
+	const char msg1[] = "CRITICAL - Plugin timed out\n";
+	const char msg2[] = "CRITICAL - popen timeout received, but no child process\n";
 	if (signo == SIGALRM) {
 		if (child_process != NULL) {
 			fh=fileno (child_process);
 			if(fh >= 0){
 				kill (childpid[fh], SIGKILL);
 			}
-			printf (_("CRITICAL - Plugin timed out after %d seconds\n"),
-						timeout_interval);
+			/* printf (_("CRITICAL - Plugin timed out after %d seconds\n"),
+						timeout_interval); */
+			write(STDOUT_FILENO, msg1, sizeof(msg1));
 		} else {
-			printf ("%s\n", _("CRITICAL - popen timeout received, but no child process"));
+			/* printf ("%s\n", _("CRITICAL - popen timeout received, but no child process")); */
+			write(STDOUT_FILENO, msg2, sizeof(msg2));
 		}
 		exit (STATE_CRITICAL);
 	}

plugins/runcmd.c

@@ -259,9 +259,29 @@ void
 runcmd_timeout_alarm_handler (int signo)
 {
 	size_t i;
-
-	if (signo == SIGALRM)
-		printf("%s - Plugin timed out while executing system call\n", state_text(timeout_state));
+	const char msg[] = " - Plugin timed out while executing system call\n";
+
+	if (signo == SIGALRM) {
+		/* printf("%s - Plugin timed out while executing system call\n", state_text(timeout_state)); */
+		switch(timeout_state) {
+			case STATE_OK:
+				write(STDOUT_FILENO, "OK", 2);
+				break;
+			case STATE_WARNING:
+				write(STDOUT_FILENO, "WARNING", 7);
+				break;
+			case STATE_CRITICAL:
+				write(STDOUT_FILENO, "CRITICAL", 8);
+				break;
+			case STATE_DEPENDENT:
+				write(STDOUT_FILENO, "DEPENDENT", 9);
+				break;
+			default:
+				write(STDOUT_FILENO, "UNKNOWN", 7);
+				break;
+		}
+		write(STDOUT_FILENO, msg, sizeof(msg) - 1);
+	}
 
 	if(np_pids) for(i = 0; i < maxfd; i++) {
 		if(np_pids[i] != 0) kill(np_pids[i], SIGKILL);

plugins/sslutils.c

@@ -48,7 +48,7 @@ int np_net_ssl_init_with_hostname_and_version(int sd, char *host_name, int versi
 }
 
 int np_net_ssl_init_with_hostname_version_and_cert(int sd, char *host_name, int version, char *cert, char *privkey) {
-	SSL_METHOD *method = NULL;
+	const SSL_METHOD *method = NULL;
 	long ssl_options = NULL;
 
 	switch (version) {

plugins/utils.c

@@ -170,9 +170,28 @@ state_text (int result)
 void
 timeout_alarm_handler (int signo)
 {
+	const char msg[] = " - Plugin timed out\n";
 	if (signo == SIGALRM) {
-		printf (_("%s - Plugin timed out after %d seconds\n"),
-						state_text(timeout_state), timeout_interval);
+/*		printf (_("%s - Plugin timed out after %d seconds\n"),
+						state_text(timeout_state), timeout_interval); */
+		switch(timeout_state) {
+			case STATE_OK:
+				write(STDOUT_FILENO, "OK", 2);
+				break;
+			case STATE_WARNING:
+				write(STDOUT_FILENO, "WARNING", 7);
+				break;
+			case STATE_CRITICAL:
+				write(STDOUT_FILENO, "CRITICAL", 8);
+				break;
+			case STATE_DEPENDENT:
+				write(STDOUT_FILENO, "DEPENDENT", 9);
+				break;
+			default:
+				write(STDOUT_FILENO, "UNKNOWN", 7);
+				break;
+		}
+		write(STDOUT_FILENO, msg, sizeof(msg) - 1);
 		exit (timeout_state);
 	}
 }