Mercurial > hg > dhcpcd

changeset 5454:68ef863871d1 draft

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
dhcpcd: Only manipulate stdin, stdout and stderr when valid UNIX application expect these to exist even if pointed at /dev/null. We cannot change which fd they use, it's always 0, 1 and 2. But if these fd's are not open when dhcpcd is called, they could be assigned to dhcpcd internals. In this instance we should not use the streams in anyway or form.
author Roy Marples <roy@marples.name>
date Sat, 12 Sep 2020 20:14:47 +0100
parents fef58462dea6
children d106ecc43837
files src/dhcpcd.c src/dhcpcd.h src/privsep.c
diffstat 3 files changed, 67 insertions(+), 23 deletions(-) [+]
line wrap: on
line diff
--- a/src/dhcpcd.c	Sat Sep 12 15:58:03 2020 +0100
+++ b/src/dhcpcd.c	Sat Sep 12 20:14:47 2020 +0100
@@ -360,7 +360,7 @@
 		return;
 
 	/* Don't use loginfo because this makes no sense in a log. */
-	if (!(logopts & LOGERR_QUIET))
+	if (!(logopts & LOGERR_QUIET) && ctx->stderr_valid)
 		(void)fprintf(stderr,
 		    "forked to background, child pid %d\n", getpid());
 	i = EXIT_SUCCESS;
@@ -1893,9 +1893,16 @@
 	ctx.ps_inet_fd = ctx.ps_control_fd = -1;
 	TAILQ_INIT(&ctx.ps_processes);
 #endif
-	rt_init(&ctx);
 
-	logopts = LOGERR_ERR|LOGERR_LOG|LOGERR_LOG_DATE|LOGERR_LOG_PID;
+	/* Check our streams for validity */
+	ctx.stdin_valid =  fcntl(STDIN_FILENO,  F_GETFD) != -1;
+	ctx.stdout_valid = fcntl(STDOUT_FILENO, F_GETFD) != -1;
+	ctx.stderr_valid = fcntl(STDERR_FILENO, F_GETFD) != -1;
+
+	logopts = LOGERR_LOG | LOGERR_LOG_DATE | LOGERR_LOG_PID;
+	if (ctx.stderr_valid)
+		logopts |= LOGERR_ERR;
+
 	i = 0;
 	while ((opt = getopt_long(argc, argv,
 	    ctx.options & DHCPCD_PRINT_PIDFILE ? NOERR_IF_OPTS : IF_OPTS,
@@ -1979,6 +1986,8 @@
 	ctx.ifc = argc - optind;
 	ctx.ifv = argv + optind;
 
+	rt_init(&ctx);
+
 	ifo = read_config(&ctx, NULL, NULL, NULL);
 	if (ifo == NULL) {
 		if (ctx.options & DHCPCD_PRINT_PIDFILE)
@@ -2256,12 +2265,13 @@
 	}
 
 	loginfox(PACKAGE "-" VERSION " starting");
-	if (freopen(_PATH_DEVNULL, "r", stdin) == NULL)
-		logerr("%s: freopen stdin", __func__);
+	if (ctx.stdin_valid && freopen(_PATH_DEVNULL, "w", stdin) == NULL)
+		logwarn("freopen stdin");
 
 #if defined(USE_SIGNALS) && !defined(THERE_IS_NO_FORK)
 	if (xsocketpair(AF_UNIX, SOCK_DGRAM | SOCK_CXNB, 0, fork_fd) == -1 ||
-	    xsocketpair(AF_UNIX, SOCK_DGRAM | SOCK_CXNB, 0, stderr_fd) == -1)
+	    (ctx.stderr_valid &&
+	    xsocketpair(AF_UNIX, SOCK_DGRAM | SOCK_CXNB, 0, stderr_fd) == -1))
 	{
 		logerr("socketpair");
 		goto exit_failure;
@@ -2284,15 +2294,18 @@
 		 * Redirect stdout as well.
 		 * dhcpcd doesn't output via stdout, but something in
 		 * a called script might.
-		 *
-		 * Do NOT rights limit this fd as it will affect scripts.
-		 * For example, cmp reports insufficient caps on FreeBSD.
 		 */
-		if (dup2(stderr_fd[1], STDERR_FILENO) == -1 ||
-		    dup2(stderr_fd[1], STDOUT_FILENO) == -1)
-			logerr("dup2");
-		close(stderr_fd[0]);
-		close(stderr_fd[1]);
+		if (ctx.stderr_valid) {
+			if (dup2(stderr_fd[1], STDERR_FILENO) == -1 ||
+			    (ctx.stdout_valid &&
+			    dup2(stderr_fd[1], STDOUT_FILENO) == -1))
+				logerr("dup2");
+			close(stderr_fd[0]);
+			close(stderr_fd[1]);
+		} else if (ctx.stdout_valid) {
+			if (freopen(_PATH_DEVNULL, "w", stdout) == NULL)
+				logerr("freopen stdout");
+		}
 		if (setsid() == -1) {
 			logerr("%s: setsid", __func__);
 			goto exit_failure;
@@ -2312,10 +2325,9 @@
 		break;
 	default:
 		ctx.options |= DHCPCD_FORKED; /* A lie */
+		setproctitle("[launcher]");
 		ctx.fork_fd = fork_fd[0];
 		close(fork_fd[1]);
-		ctx.stderr_fd = stderr_fd[0];
-		close(stderr_fd[1]);
 #ifdef PRIVSEP_RIGHTS
 		if (ps_rights_limit_fd(fork_fd[0]) == -1 ||
 		    ps_rights_limit_fd(stderr_fd[0]) == 1)
@@ -2324,10 +2336,21 @@
 			goto exit_failure;
 		}
 #endif
-		setproctitle("[launcher]");
 		eloop_event_add(ctx.eloop, ctx.fork_fd, dhcpcd_fork_cb, &ctx);
-		eloop_event_add(ctx.eloop, ctx.stderr_fd, dhcpcd_stderr_cb,
-		    &ctx);
+
+		if (ctx.stderr_valid) {
+			ctx.stderr_fd = stderr_fd[0];
+			close(stderr_fd[1]);
+#ifdef PRIVSEP_RIGHTS
+			if (ps_rights_limit_fd(stderr_fd[0]) == 1) {
+				logerr("ps_rights_limit_fdpair");
+				goto exit_failure;
+			}
+#endif
+			if (ctx.stderr_valid)
+				eloop_event_add(ctx.eloop, ctx.stderr_fd,
+				    dhcpcd_stderr_cb, &ctx);
+		}
 		goto run_loop;
 	}
 
--- a/src/dhcpcd.h	Sat Sep 12 15:58:03 2020 +0100
+++ b/src/dhcpcd.h	Sat Sep 12 20:14:47 2020 +0100
@@ -117,8 +117,11 @@
 struct dhcpcd_ctx {
 	char pidfile[sizeof(PIDFILE) + IF_NAMESIZE + 1];
 	char vendor[256];
+	bool stdin_valid;	/* It's possible stdin, stdout and stderr */
+	bool stdout_valid;	/* could be closed when dhcpcd starts. */
+	bool stderr_valid;
+	int stderr_fd;	/* FD for logging to stderr */
 	int fork_fd;	/* FD for the fork init signal pipe */
-	int stderr_fd;	/* FD for logging to stderr */
 	const char *cffile;
 	unsigned long long options;
 	char *logfile;
--- a/src/privsep.c	Sat Sep 12 15:58:03 2020 +0100
+++ b/src/privsep.c	Sat Sep 12 20:14:47 2020 +0100
@@ -76,7 +76,6 @@
 #ifdef HAVE_CAPSICUM
 #include <sys/capsicum.h>
 #include <capsicum_helpers.h>
-#define ps_rights_limit_stdio caph_limit_stdio
 #endif
 #ifdef HAVE_UTIL_H
 #include <util.h>
@@ -278,6 +277,25 @@
 		return -1;
 	return 0;
 }
+
+static int
+ps_rights_limit_stdio(struct dhcpcd_ctx *ctx)
+{
+	const int iebadf = CAPH_IGNORE_EBADF;
+	int error = 0;
+
+	if (ctx->stdin_valid &&
+	    caph_limit_stream(STDIN_FILENO, CAPH_READ | iebadf) == -1)
+		error = -1;
+	if (ctx->stdout_valid &&
+	    caph_limit_stream(STDOUT_FILENO, CAPH_WRITE | iebadf) == -1)
+		error = -1;
+	if (ctx->stderr_valid &&
+	    caph_limit_stream(STDERR_FILENO, CAPH_WRITE | iebadf) == -1)
+		error = -1;
+
+	return error;
+}
 #endif
 
 pid_t
@@ -346,7 +364,7 @@
 
 #ifdef PRIVSEP_RIGHTS
 		/* We cannot limit the root process in any way. */
-		if (ps_rights_limit_stdio() == -1) {
+		if (ps_rights_limit_stdio(ctx) == -1) {
 			logerr("ps_rights_limit_stdio");
 			goto errexit;
 		}
@@ -484,7 +502,7 @@
 	if ((ctx->pf_inet_fd != -1 &&
 	    ps_rights_limit_ioctl(ctx->pf_inet_fd) == -1) ||
 	    (ctx->link_fd != -1 && ps_rights_limit_fd(ctx->link_fd) == -1) ||
-	     ps_rights_limit_stdio() == -1)
+	     ps_rights_limit_stdio(ctx) == -1)
 	{
 		logerr("%s: cap_rights_limit", __func__);
 		return -1;