changeset 5297:477edd06fea7 draft

privsep: harden process handling If eloop is exited, only allow explicit re-entry. Only exit on read/write error if a forked process and not root. If the root process fails to read/write to a sub-process, stop the sub-process.
author Roy Marples <roy@marples.name>
date Tue, 02 Jun 2020 15:50:17 +0100
parents 3fd9a92dd852
children 95d69db26ac5
files src/eloop.c src/eloop.h src/privsep-root.c src/privsep.c
diffstat 4 files changed, 41 insertions(+), 13 deletions(-) [+]
line wrap: on
line diff
--- a/src/eloop.c	Tue Jun 02 14:51:20 2020 +0100
+++ b/src/eloop.c	Tue Jun 02 15:50:17 2020 +0100
@@ -727,6 +727,13 @@
 	eloop->exitnow = 1;
 }
 
+void
+eloop_enter(struct eloop *eloop)
+{
+
+	eloop->exitnow = 0;
+}
+
 #if defined(HAVE_KQUEUE) || defined(HAVE_EPOLL)
 static int
 eloop_open(struct eloop *eloop)
@@ -1010,7 +1017,6 @@
 
 	assert(eloop != NULL);
 
-	eloop->exitnow = 0;
 	for (;;) {
 		if (eloop->exitnow)
 			break;
--- a/src/eloop.h	Tue Jun 02 14:51:20 2020 +0100
+++ b/src/eloop.h	Tue Jun 02 15:50:17 2020 +0100
@@ -92,6 +92,7 @@
 void eloop_clear(struct eloop *);
 void eloop_free(struct eloop *);
 void eloop_exit(struct eloop *, int);
+void eloop_enter(struct eloop *);
 int eloop_start(struct eloop *, sigset_t *);
 
 #endif
--- a/src/privsep-root.c	Tue Jun 02 14:51:20 2020 +0100
+++ b/src/privsep-root.c	Tue Jun 02 15:50:17 2020 +0100
@@ -124,6 +124,7 @@
 	    ps_root_readerrorcb, &psr_ctx) == -1)
 		return -1;
 
+	eloop_enter(ctx->ps_eloop);
 	eloop_start(ctx->ps_eloop, &ctx->sigset);
 
 	errno = psr_ctx.psr_error.psr_errno;
@@ -181,6 +182,7 @@
 	    ps_root_mreaderrorcb, &psr_ctx) == -1)
 		return -1;
 
+	eloop_enter(ctx->ps_eloop);
 	eloop_start(ctx->ps_eloop, &ctx->sigset);
 
 	errno = psr_ctx.psr_error.psr_errno;
@@ -446,9 +448,21 @@
 
 			ps_freeprocess(psp);
 			return ret;
-		} else if (!(psm->ps_cmd & PS_START))
-			return ps_sendpsmmsg(ctx, psp->psp_fd, psm, msg);
-		/* Process has already started .... */
+		} else if (psm->ps_cmd & PS_START) {
+			/* Process has already started .... */
+			return 0;
+		}
+
+		err = ps_sendpsmmsg(ctx, psp->psp_fd, psm, msg);
+		if (err == -1) {
+			logerr("%s: failed to send message to pid %d",
+			    __func__, psp->psp_pid);
+			shutdown(psp->psp_fd, SHUT_RDWR);
+			close(psp->psp_fd);
+			psp->psp_fd = -1;
+			ps_dostop(ctx, &psp->psp_pid, &psp->psp_fd);
+			ps_freeprocess(psp);
+		}
 		return 0;
 	}
 
--- a/src/privsep.c	Tue Jun 02 14:51:20 2020 +0100
+++ b/src/privsep.c	Tue Jun 02 15:50:17 2020 +0100
@@ -288,6 +288,7 @@
 		(void)ps_sendcmd(ctx, *priv_fd, PS_STOP, 0, NULL, 0);
 	shutdown(*priv_fd, SHUT_RDWR);
 	*priv_fd = -1;
+	eloop_exit(ctx->eloop, EXIT_FAILURE);
 	return -1;
 }
 
@@ -301,6 +302,9 @@
 #endif
 	if (*pid == 0)
 		return 0;
+	if (*fd == -1)
+		goto wait;
+
 	eloop_event_delete(ctx->eloop, *fd);
 	if (ps_sendcmd(ctx, *fd, PS_STOP, 0, NULL, 0) == -1 &&
 	    errno != ECONNRESET)
@@ -309,11 +313,8 @@
 		logerr(__func__);
 	close(*fd);
 	*fd = -1;
-	/* We won't have permission for all processes .... */
-#if 0
-	if (kill(*pid, SIGTERM) == -1)
-		logerr(__func__);
-#endif
+
+wait:
 	status = 0;
 
 #ifdef HAVE_CAPSICUM
@@ -544,7 +545,8 @@
 #ifdef PRIVSEP_DEBUG
 	logdebugx("%s: %zd", __func__, len);
 #endif
-	if ((len == -1 || len == 0) && ctx->options & DHCPCD_FORKED)
+	if ((len == -1 || len == 0) && ctx->options & DHCPCD_FORKED &&
+	    !(ctx->options & DHCPCD_PRIVSEPROOT))
 		eloop_exit(ctx->eloop, len == 0 ? EXIT_SUCCESS : EXIT_FAILURE);
 	return len;
 }
@@ -687,8 +689,12 @@
 #ifdef PRIVSEP_DEBUG
 	logdebugx("%s: recv fd %d, %zd bytes", __func__, rfd, len);
 #endif
-	if ((len == -1 || len == 0) && ctx->options & DHCPCD_FORKED) {
-		eloop_exit(ctx->eloop, len == 0 ? EXIT_SUCCESS : EXIT_FAILURE);
+
+	if (len == -1 || len == 0) {
+		if (ctx->options & DHCPCD_FORKED &&
+		    !(ctx->options & DHCPCD_PRIVSEPROOT))
+			eloop_exit(ctx->eloop,
+			    len == 0 ? EXIT_SUCCESS : EXIT_FAILURE);
 		return len;
 	}
 
@@ -697,7 +703,8 @@
 #ifdef PRIVSEP_DEBUG
 	logdebugx("%s: send fd %d, %zu bytes", __func__, wfd, len);
 #endif
-	if ((len == -1 || len == 0) && ctx->options & DHCPCD_FORKED)
+	if ((len == -1 || len == 0) && ctx->options & DHCPCD_FORKED &&
+	    !(ctx->options & DHCPCD_PRIVSEPROOT))
 		eloop_exit(ctx->eloop, len == 0 ? EXIT_SUCCESS : EXIT_FAILURE);
 	return len;
 }