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.

diff --git a/src/dhcpcd.c b/src/dhcpcd.c
index f80d293ac3afab081f40fb9eea1616c2e00613ce..1c863b5e58f766caacc11f31abbde2d696d37d51 100644 (file)
--- a/src/dhcpcd.c
+++ b/src/dhcpcd.c
@@ -360,7 +360,7 @@ dhcpcd_daemonise(struct dhcpcd_ctx *ctx)
                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 @@ main(int argc, char **argv)
        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 @@ main(int argc, char **argv)
        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 @@ printpidfile:
        }
 
        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 @@ printpidfile:
                 * 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 @@ printpidfile:
                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 @@ printpidfile:
                        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;
        }
 

diff --git a/src/dhcpcd.h b/src/dhcpcd.h
index 95b7e07370bd812f9cc4611ee4d5d302899279a2..fdff88a175a046559bbd098fbf628ced98a206ac 100644 (file)
--- a/src/dhcpcd.h
+++ b/src/dhcpcd.h
@@ -117,8 +117,11 @@ struct passwd;
 struct dhcpcd_ctx {
        char pidfile[sizeof(PIDFILE) + IF_NAMESIZE + 1];
        char vendor[256];
-       int fork_fd;    /* FD for the fork init signal pipe */
+       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 */
        const char *cffile;
        unsigned long long options;
        char *logfile;

diff --git a/src/privsep.c b/src/privsep.c
index 3f1dc2d6e06d106a080ae02a3798312a3054a31f..f92ef45b2eaa1a714071d2496ec39c0fddd10323 100644 (file)
--- a/src/privsep.c
+++ b/src/privsep.c
@@ -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 @@ ps_rights_limit_fdpair(int fd[])
                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 @@ ps_dostart(struct dhcpcd_ctx *ctx,
 
 #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 @@ ps_mastersandbox(struct dhcpcd_ctx *ctx)
        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;