privsep: Send all log messages to the privileged actioneer

If dhcpcd starts and no syslogd implementation is running then
various syscall filters could be triggered when dhcpcd wants to syslog
and it's already in a chroot.

Not all libc openlog implementations support LOG_NDELAY and
openlog does not return an error code and can also mask errno back to 0.
So we have no way of knowing if we have a syslog connection or not.
This means we cannot cache the connection at startup because syslog itself
will try and open if no connection.

As such, all logging is now directed to the dhcpcd privileged actioneer
process which will handle all the syslog and log file writing actions.

The only downside of this approach (other than an extra fd per process)
is that we no longer know which PID raised the message. While we could
put the correct PID in the logfile as we control the API, we cannot
put it into syslog as we cannot control that API.
As all privsep errors should log which function they came from this
will hopefully not be an issue as on the happy path only the master
process will log stuff.

diff --git a/src/dhcpcd.c b/src/dhcpcd.c
index 9b0a79a45cc4360b4d663b8e38f1543b7466bd48..bd4cb74f77e5839e67eb7a71158a284f171afd67 100644 (file)
--- a/src/dhcpcd.c
+++ b/src/dhcpcd.c
@@ -1860,7 +1860,7 @@ main(int argc, char **argv, char **envp)
        ctx.dhcp6_wfd = -1;
 #endif
 #ifdef PRIVSEP
-       ctx.ps_root_fd = ctx.ps_data_fd = -1;
+       ctx.ps_root_fd = ctx.ps_syslog_fd = ctx.ps_data_fd = -1;
        ctx.ps_inet_fd = ctx.ps_control_fd = -1;
        TAILQ_INIT(&ctx.ps_processes);
 #endif

diff --git a/src/dhcpcd.h b/src/dhcpcd.h
index 8cdd3c191040e8d0a233562245f88e13b8d1d181..b3cdf9419971b389bf9d6b508deb466236c28ea1 100644 (file)
--- a/src/dhcpcd.h
+++ b/src/dhcpcd.h
@@ -199,6 +199,7 @@ struct dhcpcd_ctx {
        struct passwd *ps_user; /* struct passwd for privsep user */
        pid_t ps_root_pid;
        int ps_root_fd;         /* Privileged Actioneer commands */
+       int ps_syslog_fd;       /* syslog(3) wrapper */
        int ps_data_fd;         /* Data from root spawned processes */
        struct eloop *ps_eloop; /* eloop for polling root data */
        struct ps_process_head ps_processes;    /* List of spawned processes */

diff --git a/src/logerr.c b/src/logerr.c
index ad337ba060dcfa014359cb1259e7951ba2249ad4..6d793082cb6042847fb45eedb0b7e21cf41205c4 100644 (file)
--- a/src/logerr.c
+++ b/src/logerr.c
@@ -47,11 +47,15 @@
 #undef LOGERR_TAG
 #endif
 
+/* syslog protocol is 1k message max, RFC 3164 section 4.1 */
+#define LOGERR_SYSLOGBUF       1024 + sizeof(int)
+
 #define UNUSED(a)              (void)(a)
 
 struct logctx {
        char             log_buf[BUFSIZ];
        unsigned int     log_opts;
+       int              log_syslogfd;
 #ifndef SMALL
        FILE            *log_file;
 #ifdef LOGERR_TAG
@@ -63,9 +67,10 @@ struct logctx {
 static struct logctx _logctx = {
        /* syslog style, but without the hostname or tag. */
        .log_opts = LOGERR_LOG | LOGERR_LOG_DATE | LOGERR_LOG_PID,
+       .log_syslogfd = -1,
 };
 
-#if defined(LOGERR_TAG) && defined(__linux__)
+#if defined(__linux__)
 /* Poor man's getprogname(3). */
 static char *_logprog;
 static const char *
@@ -79,9 +84,12 @@ getprogname(void)
                 * so zero the buffer. */
                if ((_logprog = calloc(1, PATH_MAX + 1)) == NULL)
                        return NULL;
+               if (readlink("/proc/self/exe", _logprog, PATH_MAX + 1) == -1) {
+                       free(_logprog);
+                       _logprog = NULL;
+                       return NULL;
+               }
        }
-       if (readlink("/proc/self/exe", _logprog, PATH_MAX + 1) == -1)
-               return NULL;
        if (_logprog[0] == '[')
                return NULL;
        p = strrchr(_logprog, '/');
@@ -198,22 +206,33 @@ vlogmessage(int pri, const char *fmt, va_list args)
        struct logctx *ctx = &_logctx;
        int len = 0;
 
+       if (ctx->log_syslogfd != -1) {
+               char buf[LOGERR_SYSLOGBUF];
+
+               memcpy(buf, &pri, sizeof(pri));
+               len = vsnprintf(buf + sizeof(pri), sizeof(buf) - sizeof(pri),
+                   fmt, args);
+               if (len != -1)
+                       len = (int)write(ctx->log_syslogfd, buf,
+                           ((size_t)++len) + sizeof(pri));
+               return len;
+       }
+
        if (ctx->log_opts & LOGERR_ERR &&
            (pri <= LOG_ERR ||
            (!(ctx->log_opts & LOGERR_QUIET) && pri <= LOG_INFO) ||
            (ctx->log_opts & LOGERR_DEBUG && pri <= LOG_DEBUG)))
                len = vlogprintf_r(ctx, stderr, fmt, args);
 
-       if (!(ctx->log_opts & LOGERR_LOG))
-               return len;
-
 #ifndef SMALL
        if (ctx->log_file != NULL &&
            (pri != LOG_DEBUG || (ctx->log_opts & LOGERR_DEBUG)))
                len = vlogprintf_r(ctx, ctx->log_file, fmt, args);
 #endif
 
-       vsyslog(pri, fmt, args);
+       if (ctx->log_opts & LOGERR_LOG)
+               vsyslog(pri, fmt, args);
+
        return len;
 }
 #if defined(__GNUC__) && (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 5))
@@ -331,6 +350,43 @@ log_errx(const char *fmt, ...)
        va_end(args);
 }
 
+int
+loggetsyslogfd(void)
+{
+       struct logctx *ctx = &_logctx;
+
+       return ctx->log_syslogfd;
+}
+
+void
+logsetsyslogfd(int fd)
+{
+       struct logctx *ctx = &_logctx;
+
+       ctx->log_syslogfd = fd;
+}
+
+int
+loghandlesyslogfd(int fd)
+{
+       char buf[LOGERR_SYSLOGBUF];
+       int len, pri;
+
+       len = (int)read(fd, buf, sizeof(buf));
+       if (len == -1)
+               return -1;
+
+       /* Ensure we have pri and a terminator */
+       if (len < (int)sizeof(pri) + 1 || buf[len - 1] != '\0') {
+               errno = EINVAL;
+               return -1;
+       }
+
+       memcpy(&pri, buf, sizeof(pri));
+       logmessage(pri, "%s", buf + sizeof(pri));
+       return len;
+}
+
 unsigned int
 loggetopts(void)
 {
@@ -373,15 +429,9 @@ logopen(const char *path)
 
        (void)setvbuf(stderr, ctx->log_buf, _IOLBF, sizeof(ctx->log_buf));
 
-       if (!(ctx->log_opts & LOGERR_LOG))
-               return 1;
-
-#ifdef LOG_NDELAY
-       opts |= LOG_NDELAY;
-#endif
        if (ctx->log_opts & LOGERR_LOG_PID)
                opts |= LOG_PID;
-       openlog(NULL, opts, LOGERR_SYSLOG_FACILITY);
+       openlog(getprogname(), opts, LOGERR_SYSLOG_FACILITY);
        if (path == NULL)
                return 1;
 
@@ -410,7 +460,7 @@ logclose(void)
        fclose(ctx->log_file);
        ctx->log_file = NULL;
 #endif
-#if defined(LOGERR_TAG) && defined(__linux__)
+#if defined(__linux__)
        free(_logprog);
 #endif
 }

diff --git a/src/logerr.h b/src/logerr.h
index 4b4d6dc42566418e531b4eceff52594eed5d3e8e..c1b80153766220ed82d93326772f2fbbe8964046 100644 (file)
--- a/src/logerr.h
+++ b/src/logerr.h
@@ -76,6 +76,11 @@ __printflike(2, 3) void logerrmessage(int pri, const char *fmt, ...);
 #define logerr(...)    log_err(__VA_ARGS__)
 #define logerrx(...)   log_errx(__VA_ARGS__)
 
+/* For syslog in a chroot */
+int loggetsyslogfd(void);
+void logsetsyslogfd(int);
+int loghandlesyslogfd(int);
+
 unsigned int loggetopts(void);
 void logsetopts(unsigned int);
 #define        LOGERR_DEBUG    (1U << 6)

diff --git a/src/privsep-root.c b/src/privsep-root.c
index 770dd953477affdd55dc007365af73f93228ba6d..209f2624f38d3c6e1973eb0e369a72d620538e97 100644 (file)
--- a/src/privsep-root.c
+++ b/src/privsep-root.c
@@ -780,18 +780,34 @@ ps_root_dispatch(void *arg)
                logerr(__func__);
 }
 
+static void
+ps_root_syslog(void *arg)
+{
+       struct dhcpcd_ctx *ctx = arg;
+
+       if (loghandlesyslogfd(ctx->ps_syslog_fd) == -1)
+               logerr(__func__);
+}
+
 pid_t
 ps_root_start(struct dhcpcd_ctx *ctx)
 {
-       int fd[2];
+       int logfd[2], datafd[2];
        pid_t pid;
 
-       if (socketpair(AF_UNIX, SOCK_DGRAM | SOCK_CXNB, 0, fd) == -1)
+       if (xsocketpair(AF_UNIX, SOCK_DGRAM | SOCK_CXNB, 0, logfd) == -1)
                return -1;
-       if (ps_setbuf_fdpair(fd) == -1)
+#ifdef PRIVSEP_RIGHTS
+       if (ps_rights_limit_fdpair(logfd) == -1)
+               return -1;
+#endif
+
+       if (socketpair(AF_UNIX, SOCK_DGRAM | SOCK_CXNB, 0, datafd) == -1)
+               return -1;
+       if (ps_setbuf_fdpair(datafd) == -1)
                return -1;
 #ifdef PRIVSEP_RIGHTS
-       if (ps_rights_limit_fdpair(fd) == -1)
+       if (ps_rights_limit_fdpair(datafd) == -1)
                return -1;
 #endif
 
@@ -800,14 +816,22 @@ ps_root_start(struct dhcpcd_ctx *ctx)
            ps_root_startcb, ps_root_signalcb, 0);
 
        if (pid == 0) {
-               ctx->ps_data_fd = fd[1];
-               close(fd[0]);
+               ctx->ps_syslog_fd = logfd[1];
+               if (eloop_event_add(ctx->eloop, ctx->ps_syslog_fd,
+                   ps_root_syslog, ctx) == -1)
+                       return -1;
+               close(logfd[0]);
+               ctx->ps_data_fd = datafd[1];
+               close(datafd[0]);
                return 0;
        } else if (pid == -1)
                return -1;
 
-       ctx->ps_data_fd = fd[0];
-       close(fd[1]);
+       logsetsyslogfd(logfd[0]);
+       close(logfd[1]);
+
+       ctx->ps_data_fd = datafd[0];
+       close(datafd[1]);
        if (eloop_event_add(ctx->eloop, ctx->ps_data_fd,
            ps_root_dispatch, ctx) == -1)
                return -1;

diff --git a/src/privsep.c b/src/privsep.c
index e9fa463fed6240cb0fcd39b2f99004aa40140e58..6e76902afc2855a693f4aabed06792665d16aedb 100644 (file)
--- a/src/privsep.c
+++ b/src/privsep.c
@@ -542,6 +542,19 @@ ps_mastersandbox(struct dhcpcd_ctx *ctx, const char *_pledge)
        dropped = ps_dropprivs(ctx);
        if (forked)
                ctx->options |= DHCPCD_FORKED;
+
+       /*
+        * If we don't have a root process, we cannot use syslog.
+        * If it cannot be opened before chrooting then syslog(3) will fail.
+        * openlog(3) does not return an error which doubly sucks.
+        */
+       if (ctx->ps_root_fd == -1) {
+               unsigned int logopts = loggetopts();
+
+               logopts &= ~LOGERR_LOG;
+               logsetopts(logopts);
+       }
+
        if (dropped == -1) {
                logerr("%s: ps_dropprivs", __func__);
                return -1;