Improve exit status handling, merged from sslsplit

NEWS.md

@@ -1,6 +1,8 @@
 
 ### SSLsplit develop
 
+-   Propagate the exit status of the privsep child process to the parent
+    process and use 128+signal convention (issue #252).
 -   Minor bugfixes and improvements.
 
 
@@ -80,9 +82,9 @@ This release includes work sponsored by HackerOne.
 -   Fix data processing when EOF is received before all incoming data has been
     processed.
 -   Fix multiple signal handling issues in the privilege separation parent
-    which led to the parent process being killed ungracefully (SIGTERM) or
-    being stuck in wait() while still having signals (SIGQUIT etc) queued up
-    for forwarding to the child process (issue #137).
+    which led to the parent process being killed ungracefully or being stuck
+    in wait() while still having signals queued up for forwarding to the child
+    process (issue #137).
 -   No longer assume an out of memory condition when a certificate contains
     neither a CN nor a subjectAltName extension.
 -   Fix parallel make build (-j) for the test target (issue #140).

log.c

@@ -69,7 +69,7 @@ log_exceptcb(void)
 		return;
 	}
 	if (proxy_ctx) {
-		proxy_loopbreak(proxy_ctx);
+		proxy_loopbreak(proxy_ctx, -1);
 		proxy_ctx = NULL;
 	}
 }

main.c

@@ -896,7 +896,7 @@ main(int argc, char *argv[])
 	 * remaining slots are passed down to log subsystem. */
 	int clisock[6];
 	if (privsep_fork(global, clisock,
-	                 sizeof(clisock)/sizeof(clisock[0])) != 0) {
+	                 sizeof(clisock)/sizeof(clisock[0]), &rv) != 0) {
 		/* parent has exited the monitor loop after waiting for child,
 		 * or an error occurred */
 		if (global->pidfile) {
@@ -947,9 +947,24 @@ main(int argc, char *argv[])
 		log_err_level_printf(LOG_CRIT, "Failed to init NAT state table lookup.\n");
 		goto out_nat_failed;
 	}
-	rv = EXIT_SUCCESS;
 
-	proxy_run(proxy);
+	int proxy_rv = proxy_run(proxy);
+	if (proxy_rv == 0) {
+		rv = EXIT_SUCCESS;
+	} else if (proxy_rv > 0) {
+		/*
+		 * We terminated because of receiving a signal.  For our normal
+		 * termination signals as documented in the man page, we want
+		 * to return with EXIT_SUCCESS.  For other signals, which
+		 * should be considered abnormal terminations, we want to
+		 * return an exit status of 128 + signal number.
+		 */
+		if (proxy_rv == SIGTERM || proxy_rv == SIGINT) {
+			rv = EXIT_SUCCESS;
+		} else {
+			rv = 128 + proxy_rv;
+		}
+	}
 
 #ifdef DEBUG_PROXY
 	log_dbg_level_printf(LOG_DBG_MODE_FINEST, "main: EXIT closing privsep clisock=%d\n", clisock[0]);

privsep.c

@@ -1119,7 +1119,7 @@ privsep_client_update_atime(int clisock, const userdbkeys_t *keys)
  * will not be touched.
  */
 int
-privsep_fork(global_t *global, int clisock[], size_t nclisock)
+privsep_fork(global_t *global, int clisock[], size_t nclisock, int *parent_rv)
 {
 	int selfpipev[2]; /* self-pipe trick: signal handler -> select */
 	int chldpipev[2]; /* el cheapo interprocess sync early after fork */
@@ -1269,21 +1269,30 @@ privsep_fork(global_t *global, int clisock[], size_t nclisock)
 	close(selfpipev[1]);
 
 	int status;
-	wait(&status);
+	pid_t wpid;
+	wpid = wait(&status);
+	if (wpid != pid) {
+		/* should never happen, warn if it does anyway */
+		log_err_printf("Child pid %lld != expected %lld from wait(2)\n",
+		               (long long)wpid, (long long)pid);
+	}
 	if (WIFEXITED(status)) {
 		if (WEXITSTATUS(status) != 0) {
-			log_err_level_printf(LOG_CRIT, "Child proc %lld exited with status %d\n",
-			               (long long)pid, WEXITSTATUS(status));
+			log_err_level_printf(LOG_CRIT, "Child pid %lld exited with status %d\n",
+			               (long long)wpid, WEXITSTATUS(status));
 		} else {
-			log_dbg_printf("Child proc %lld exited with status %d\n",
-			               (long long)pid, WEXITSTATUS(status));
+			log_dbg_printf("Child pid %lld exited with status %d\n",
+			               (long long)wpid, WEXITSTATUS(status));
 		}
+		*parent_rv = WEXITSTATUS(status);
 	} else if (WIFSIGNALED(status)) {
-		log_err_level_printf(LOG_CRIT, "Child proc %lld killed by signal %d\n",
-		               (long long)pid, WTERMSIG(status));
+		log_err_level_printf(LOG_CRIT, "Child pid %lld killed by signal %d\n",
+		               (long long)wpid, WTERMSIG(status));
+		*parent_rv = 128 + WTERMSIG(status);
 	} else {
-		log_err_level_printf(LOG_CRIT, "Child proc %lld neither exited nor killed\n",
-		               (long long)pid);
+		/* can only happen with WUNTRACED option or active tracing */
+		log_err_level_printf(LOG_CRIT, "Child pid %lld neither exited nor killed\n",
+		               (long long)wpid);
 	}
 
 	return 1;

privsep.h

@@ -32,7 +32,7 @@
 #include "attrib.h"
 #include "opts.h"
 
-int privsep_fork(global_t *, int[], size_t);
+int privsep_fork(global_t *, int[], size_t, int *);
 
 int privsep_client_openfile(int, const char *, int);
 int privsep_client_opensock(int, const proxyspec_t *spec);

proxy.c

@@ -66,6 +66,7 @@ struct proxy_ctx {
 	struct event *gcev;
 	struct proxy_listener_ctx *lctx;
 	global_t *global;
+	int loopbreak_reason;
 };
 
 static proxy_listener_ctx_t *
@@ -213,7 +214,7 @@ proxy_signal_cb(evutil_socket_t fd, UNUSED short what, void *arg)
 	case SIGTERM:
 	case SIGQUIT:
 	case SIGINT:
-		proxy_loopbreak(ctx);
+		proxy_loopbreak(ctx, fd);
 		break;
 	case SIGHUP:
 	case SIGUSR1:
@@ -381,10 +382,11 @@ leave0:
 }
 
 /*
- * Run the event loop.  Returns when the event loop is canceled by a signal
- * or on failure.
+ * Run the event loop.
+ * Returns 0 on non-signal termination, signal number when the event loop was
+ * canceled by a signal, or -1 on failure.
  */
-void
+int
 proxy_run(proxy_ctx_t *ctx)
 {
 	if (ctx->global->detach) {
@@ -397,23 +399,27 @@ proxy_run(proxy_ctx_t *ctx)
 #endif /* PURIFY */
 	if (pxy_thrmgr_run(ctx->thrmgr) == -1) {
 		log_err_level_printf(LOG_CRIT, "Failed to start thread manager\n");
-		return;
+		return -1;
 	}
 	if (OPTS_DEBUG(ctx->global)) {
 		log_dbg_printf("Starting main event loop.\n");
 	}
 	event_base_dispatch(ctx->evbase);
 	if (OPTS_DEBUG(ctx->global)) {
-		log_dbg_printf("Main event loop stopped.\n");
+		log_dbg_printf("Main event loop stopped (reason=%i).\n",
+		               ctx->loopbreak_reason);
 	}
+	return ctx->loopbreak_reason;
 }
 
 /*
- * Break the loop of the proxy, causing the proxy_run to return.
+ * Break the loop of the proxy, causing the proxy_run to return, returning
+ * the reason given in reason (signal number, 0 for success, -1 for error).
  */
 void
-proxy_loopbreak(proxy_ctx_t *ctx)
+proxy_loopbreak(proxy_ctx_t *ctx, int reason)
 {
+	ctx->loopbreak_reason = reason;
 	event_base_loopbreak(ctx->evbase);
 }
 

proxy.h

@@ -50,8 +50,8 @@ typedef struct proxy_listener_ctx {
 } proxy_listener_ctx_t;
 
 proxy_ctx_t * proxy_new(global_t *, int) NONNULL(1) MALLOC;
-void proxy_run(proxy_ctx_t *) NONNULL(1);
-void proxy_loopbreak(proxy_ctx_t *) NONNULL(1);
+int proxy_run(proxy_ctx_t *) NONNULL(1);
+void proxy_loopbreak(proxy_ctx_t *, int) NONNULL(1);
 void proxy_free(proxy_ctx_t *) NONNULL(1);
 void proxy_listener_errorcb(struct evconnlistener *, UNUSED void *);
 

sslproxy.1

@@ -569,15 +569,6 @@ Address that the program should return packets to.  This is the address where
 SSLproxy is listening for returned packets from the program.  This address is 
 inserted into the SSLproxy header line along with the dynamically assigned port
 number.  If not specified, defaults to 127.0.0.1.
-.SH SIGNALS
-A running \fBsslproxy\fP accepts SIGINT and SIGQUIT for a clean shutdown, and
-SIGHUP and SIGUSR1 to re-open the single-file log files (such as \fB-l\fP,
-\fB-L\fP and \fB-X\fP).  The canonical way to rotate or post-process logs is to
-rename the active log file, send SIGHUP or SIGUSR1 to the PID in the PID file
-given by \fB-p\fP, give SSLproxy some time to flush buffers after closing the
-old file, and then post-process the renamed log file.
-Per-connection log files (such as \fB-S\fP and \fB-F\fP) are not re-opened
-because their filename is specific to the connection.
 .SH "LOG SPECIFICATIONS"
 Log specifications are composed of zero or more printf-style directives;
 ordinary characters are included directly in the output path.
@@ -774,6 +765,20 @@ Fully supported, including IPv6.
 Note that return path filtering (rp_filter) also needs to be disabled on
 interfaces which handle TPROXY redirected traffic.
 .RE
+.SH SIGNALS
+A running \fBsslproxy\fP accepts SIGINT and SIGTERM for a clean shutdown and
+SIGUSR1 to re-open the single-file log files (such as \fB-l\fP, \fB-L\fP and
+\fB-X\fP).  The canonical way to rotate or post-process logs is to rename the
+active log file, send SIGUSR1 to the PID in the PID file given by \fB-p\fP,
+give SSLproxy some time to flush buffers after closing the old file, and then
+post-process the renamed log file.
+Per-connection log files (such as \fB-S\fP and \fB-F\fP) are not re-opened
+because their filename is specific to the connection.
+.SH "EXIT STATUS"
+The \fBsslproxy\fP process will exit with 0 on regular shutdown
+(SIGINT, SIGTERM), and 128 + signal number on controlled shutdown based on
+receiving a different signal such as SIGHUP.  Exit status in the range 1..127
+indicates error conditions.
 .SH EXAMPLES
 With configuration similar to the above NAT engine samples, intercept HTTPS and 
 POP3S over IPv4 using forged certificates with CA private key \fBca.key\fP and