NRPE uses signals unsafely, hangs, can bring down system

Fix for issues:
http://tracker.nagios.org/view.php?id=592
http://tracker.nagios.org/view.php?id=377 (send_nsca signal handling is undefined behaviour, causes SEGVs

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

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

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

Renamed configure.in to configure.ac and added check for sigaction().

.gitignore

@@ -13,3 +13,5 @@ src/Makefile
 src/check_nrpe
 src/nrpe
 subst
+autom4te.cache/
+nbproject/

Changelog

@@ -9,6 +9,8 @@ ENHANCEMENTS
 
 FIXES
 - Added ifdefs for complete_SSL_shutdown to compile without SSL. (Matthew L. Daniel)
+- Renamed configure.in to configure.ac and added check for sigaction (John Frickson)
+- Replaced all instances of signal() with sigaction() + blocking (John Frickson)
 
 2.15 - 09/06/2013
 -----------------

configure.in → configure.ac

@@ -169,7 +169,7 @@ AC_CHECK_LIB(wrap,main,[
 	AC_DEFINE(HAVE_LIBWRAP,[1],[Have the TCP wrappers library])
 	])
 AC_SUBST(LIBWRAPLIBS)
-AC_CHECK_FUNCS(strdup strstr strtoul initgroups closesocket)
+AC_CHECK_FUNCS(strdup strstr strtoul initgroups closesocket sigaction)
 
 dnl socklen_t check - from curl
 AC_CHECK_TYPE([socklen_t], ,[

include/config.h.in

@@ -48,6 +48,7 @@
 #undef HAVE_STRTOUL 
 #undef HAVE_INITGROUPS
 #undef HAVE_CLOSESOCKET
+#undef HAVE_SIGACTION
 
 #undef SIZEOF_INT
 #undef SIZEOF_SHORT

src/check_nrpe.c

@@ -70,6 +70,9 @@ int main(int argc, char **argv){
 	packet receive_packet;
 	int bytes_to_send;
 	int bytes_to_recv;
+#ifdef HAVE_SIGACTION
+	struct sigaction sig_action;
+#endif
 
 	result=process_arguments(argc,argv);
 
@@ -147,10 +150,18 @@ int main(int argc, char **argv){
 #endif
 
 	/* initialize alarm signal handling */
-	signal(SIGALRM,alarm_handler);
+#ifdef HAVE_SIGACTION
+	sig_action.sa_sigaction = NULL;
+	sig_action.sa_handler = alarm_handler;
+	sigfillset(&sig_action.sa_mask);
+	sig_action.sa_flags = SA_NODEFER|SA_RESTART;
+	sigaction(SIGALRM, &sig_action, NULL);
+#else
+	signal(SIGALRM, alarm_handler);
+#endif /* HAVE_SIGACTION */
 
 	/* set socket timeout */
-	alarm(socket_timeout);
+//	alarm(socket_timeout);
 
 	/* try to connect to the host at the given port number */
 	if((sd=my_connect(server_name, &hostaddr, server_port, address_family, 
@@ -449,8 +460,9 @@ int process_arguments(int argc, char **argv){
 
 
 void alarm_handler(int sig){
-
-	printf("CHECK_NRPE: Socket timeout after %d seconds.\n",socket_timeout);
+	const char msg[] = "CHECK_NRPE: Socket timeout";
+	/* printf("CHECK_NRPE: Socket timeout after %d seconds.\n",socket_timeout); */
+	write(STDOUT_FILENO, msg, sizeof(msg) - 1);
 
 	exit(timeout_return_code);
         }

src/nrpe.c

@@ -115,6 +115,9 @@ int main(int argc, char **argv){
 	char seedfile[FILENAME_MAX];
 	int i,c;
 #endif
+#ifdef HAVE_SIGACTION
+	struct sigaction sig_action;
+#endif
 
 	/* set some environment variables */
 	asprintf(&env_string,"NRPE_MULTILINESUPPORT=1");
@@ -312,9 +315,19 @@ int main(int argc, char **argv){
 		/*umask(0);*/
 
 		/* handle signals */
+#ifdef HAVE_SIGACTION
+		sig_action.sa_sigaction = NULL;
+		sig_action.sa_handler = sighandler;
+		sigfillset(&sig_action.sa_mask);
+		sig_action.sa_flags = SA_NODEFER|SA_RESTART;
+		sigaction(SIGQUIT, &sig_action, NULL);
+		sigaction(SIGTERM, &sig_action, NULL);
+		sigaction(SIGHUP, &sig_action, NULL);
+#else /* HAVE_SIGACTION */
 		signal(SIGQUIT,sighandler);
 		signal(SIGTERM,sighandler);
 		signal(SIGHUP,sighandler);
+#endif /* HAVE_SIGACTION */
 
 		/* log info to syslog facility */
 		syslog(LOG_NOTICE,"Starting up daemon");
@@ -382,9 +395,19 @@ int main(int argc, char **argv){
 		/*umask(0);*/
 
 		/* handle signals */
+#ifdef HAVE_SIGACTION
+		sig_action.sa_sigaction = NULL;
+		sig_action.sa_handler = sighandler;
+		sigfillset(&sig_action.sa_mask);
+		sig_action.sa_flags = SA_NODEFER|SA_RESTART;
+		sigaction(SIGQUIT, &sig_action, NULL);
+		sigaction(SIGTERM, &sig_action, NULL);
+		sigaction(SIGHUP, &sig_action, NULL);
+#else /* HAVE_SIGACTION */
 		signal(SIGQUIT,sighandler);
 		signal(SIGTERM,sighandler);
 		signal(SIGHUP,sighandler);
+#endif /* HAVE_SIGACTION */
 
 		/* log info to syslog facility */
 		syslog(LOG_NOTICE,"Starting up daemon");
@@ -903,6 +926,9 @@ void wait_for_connections(void){
 #ifdef HAVE_LIBWRAP
 	struct request_info req;
 #endif
+#ifdef HAVE_SIGACTION
+	struct sigaction sig_action;
+#endif
 
 	add_listen_addr(&listen_addrs, address_family, 
 			(strcmp(server_address, "") == 0) ? NULL : server_address, 
@@ -1012,9 +1038,19 @@ void wait_for_connections(void){
 						}
 
 					/* handle signals */
+#ifdef HAVE_SIGACTION
+					sig_action.sa_sigaction = NULL;
+					sig_action.sa_handler = child_sighandler;
+					sigfillset(&sig_action.sa_mask);
+					sig_action.sa_flags = SA_NODEFER|SA_RESTART;
+					sigaction(SIGQUIT, &sig_action, NULL);
+					sigaction(SIGTERM, &sig_action, NULL);
+					sigaction(SIGHUP, &sig_action, NULL);
+#else /* HAVE_SIGACTION */
 					signal(SIGQUIT,child_sighandler);
 					signal(SIGTERM,child_sighandler);
 					signal(SIGHUP,child_sighandler);
+#endif /* HAVE_SIGACTION */
 
 					/* grandchild does not need to listen for connections */
 					close_listen_socks();
@@ -1206,6 +1242,9 @@ void handle_connection(int sock){
 #ifdef HAVE_SSL
 	SSL *ssl=NULL;
 #endif
+#ifdef HAVE_SIGACTION
+	struct sigaction sig_action;
+#endif
 
 
 	/* log info to syslog facility */
@@ -1218,7 +1257,15 @@ void handle_connection(int sock){
 #endif
 
 	/* set connection handler */
-	signal(SIGALRM,my_connection_sighandler);
+#ifdef HAVE_SIGACTION
+	sig_action.sa_sigaction = NULL;
+	sig_action.sa_handler = my_connection_sighandler;
+	sigfillset(&sig_action.sa_mask);
+	sig_action.sa_flags = SA_NODEFER|SA_RESTART;
+	sigaction(SIGALRM, &sig_action, NULL);
+#else
+	signal(SIGALRM, my_connection_sighandler);
+#endif /* HAVE_SIGACTION */
 	alarm(connection_timeout);
 
 #ifdef HAVE_SSL
@@ -1496,6 +1543,9 @@ int my_system(char *command,int timeout,int *early_timeout,char *output,int outp
 	FILE *fp;
 	int bytes_read=0;
 	time_t start_time,end_time;
+#ifdef HAVE_SIGACTION
+	struct sigaction sig_action;
+#endif
 
 	/* initialize return variables */
 	if(output!=NULL)
@@ -1547,7 +1597,15 @@ int my_system(char *command,int timeout,int *early_timeout,char *output,int outp
 		setpgid(0,0);
 
 		/* trap commands that timeout */
-		signal(SIGALRM,my_system_sighandler);
+#ifdef HAVE_SIGACTION
+		sig_action.sa_sigaction = NULL;
+		sig_action.sa_handler = my_system_sighandler;
+		sigfillset(&sig_action.sa_mask);
+		sig_action.sa_flags = SA_NODEFER|SA_RESTART;
+		sigaction(SIGALRM, &sig_action, NULL);
+#else
+		signal(SIGALRM, my_system_sighandler);
+#endif /* HAVE_SIGACTION */
 		alarm(timeout);
 
 		/* run the command */
@@ -1897,7 +1955,7 @@ void sighandler(int sig){
 void child_sighandler(int sig){
 
 	/* free all memory we allocated */
-	free_memory();
+	/* free_memory(); */
 
 	/* terminate */
 	exit(0);