changeset 5525:26b5d9bc2985 draft

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.
author Roy Marples <roy@marples.name>
date Fri, 30 Oct 2020 03:43:51 +0000
parents 8c0e5dc34824
children b1a3d9055662
files src/dhcpcd.c src/dhcpcd.h src/logerr.c src/logerr.h src/privsep-root.c src/privsep.c
diffstat 6 files changed, 118 insertions(+), 25 deletions(-) [+]
line wrap: on
line diff
--- a/src/dhcpcd.c	Mon Oct 26 11:19:54 2020 +0000
+++ b/src/dhcpcd.c	Fri Oct 30 03:43:51 2020 +0000
@@ -1860,7 +1860,7 @@
 	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
--- a/src/dhcpcd.h	Mon Oct 26 11:19:54 2020 +0000
+++ b/src/dhcpcd.h	Fri Oct 30 03:43:51 2020 +0000
@@ -199,6 +199,7 @@
 	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 */
--- a/src/logerr.c	Mon Oct 26 11:19:54 2020 +0000
+++ b/src/logerr.c	Fri Oct 30 03:43:51 2020 +0000
@@ -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 @@
 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 @@
 		 * 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 @@
 	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 @@
 	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 @@
 
 	(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 @@
 	fclose(ctx->log_file);
 	ctx->log_file = NULL;
 #endif
-#if defined(LOGERR_TAG) && defined(__linux__)
+#if defined(__linux__)
 	free(_logprog);
 #endif
 }
--- a/src/logerr.h	Mon Oct 26 11:19:54 2020 +0000
+++ b/src/logerr.h	Fri Oct 30 03:43:51 2020 +0000
@@ -76,6 +76,11 @@
 #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)
--- a/src/privsep-root.c	Mon Oct 26 11:19:54 2020 +0000
+++ b/src/privsep-root.c	Fri Oct 30 03:43:51 2020 +0000
@@ -780,18 +780,34 @@
 		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)
-		return -1;
-	if (ps_setbuf_fdpair(fd) == -1)
+	if (xsocketpair(AF_UNIX, SOCK_DGRAM | SOCK_CXNB, 0, logfd) == -1)
 		return -1;
 #ifdef PRIVSEP_RIGHTS
-	if (ps_rights_limit_fdpair(fd) == -1)
+	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(datafd) == -1)
 		return -1;
 #endif
 
@@ -800,14 +816,22 @@
 	    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;
--- a/src/privsep.c	Mon Oct 26 11:19:54 2020 +0000
+++ b/src/privsep.c	Fri Oct 30 03:43:51 2020 +0000
@@ -542,6 +542,19 @@
 	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;