fork+exec model for pflogd(8); move pcap init to the re-exec'd privsep

parent and use 'legit' fdpassing primitives to send the bpf fd to the
unprivileged child process.

Also reduces the pledge(2) promises in the unpriv child to just
   "stdio recvfd"

with help from deraadt, pcap feedback from canacar

ok deraadt@

3 files changed, 133 insertions, 39 deletions

diff --git a/sbin/pflogd/pflogd.c b/sbin/pflogd/pflogd.c
index 429f0d375b9..678421e0779 100644
--- a/sbin/pflogd/pflogd.c
+++ b/sbin/pflogd/pflogd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pflogd.c,v 1.54 2017/07/23 14:28:22 jca Exp $	*/
+/*	$OpenBSD: pflogd.c,v 1.55 2017/09/05 15:41:25 brynet Exp $	*/
 
 /*
  * Copyright (c) 2001 Theo de Raadt
@@ -54,6 +54,7 @@ pcap_t *hpcap;
 static FILE *dpcap;
 
 int Debug = 0;
+static int privchild = 0;
 static int snaplen = DEF_SNAPLEN;
 static int cur_snaplen = DEF_SNAPLEN;
 
@@ -73,7 +74,6 @@ void  dump_packet(u_char *, const struct pcap_pkthdr *, const u_char *);
 void  dump_packet_nobuf(u_char *, const struct pcap_pkthdr *, const u_char *);
 int   flush_buffer(FILE *);
 int   if_exists(char *);
-int   init_pcap(void);
 void  logmsg(int, const char *, ...);
 void  purge_buffer(void);
 int   reset_dump(int);
@@ -215,8 +215,6 @@ init_pcap(void)
 
 	set_pcap_filter();
 
-	cur_snaplen = snaplen = pcap_snapshot(hpcap);
-
 	/* lock */
 	if (ioctl(pcap_fileno(hpcap), BIOCLOCK) < 0) {
 		logmsg(LOG_ERR, "BIOCLOCK: %s", strerror(errno));
@@ -542,9 +540,7 @@ main(int argc, char **argv)
 
 	ret = 0;
 
-	closefrom(STDERR_FILENO + 1);
-
-	while ((ch = getopt(argc, argv, "Dxd:f:i:s:")) != -1) {
+	while ((ch = getopt(argc, argv, "Dxd:f:i:P:s:")) != -1) {
 		switch (ch) {
 		case 'D':
 			Debug = 1;
@@ -560,6 +556,11 @@ main(int argc, char **argv)
 		case 'i':
 			interface = optarg;
 			break;
+		case 'P': /* used internally, exec the parent */
+			privchild = strtonum(optarg, 2, INT_MAX, &errstr);
+			if (errstr)
+				errx(1, "priv child %s: %s", errstr, optarg);
+			break;
 		case 's':
 			snaplen = strtonum(optarg, 0, PFLOGD_MAXSNAPLEN,
 			    &errstr);
@@ -567,6 +568,7 @@ main(int argc, char **argv)
 				snaplen = DEF_SNAPLEN;
 			if (errstr)
 				snaplen = PFLOGD_MAXSNAPLEN;
+			cur_snaplen = snaplen;
 			break;
 		case 'x':
 			Xflag = 1;
@@ -606,23 +608,16 @@ main(int argc, char **argv)
 		if (filter == NULL)
 			logmsg(LOG_NOTICE, "Failed to form filter expression");
 	}
+	argc += optind;
+	argv -= optind;
 
-	/* initialize pcap before dropping privileges */
-	if (init_pcap()) {
-		logmsg(LOG_ERR, "Exiting, init failure");
-		exit(1);
-	}
+	if (privchild > 1)
+		priv_exec(privchild, argc, argv);
 
 	/* Privilege separation begins here */
-	if (priv_init()) {
-		logmsg(LOG_ERR, "unable to privsep");
-		exit(1);
-	}
+	priv_init(argc, argv);
 
-	/*
-	 * XXX needs wpath cpath rpath, for try_reset_dump() ?
-	 */
-	if (pledge("stdio rpath wpath cpath unix recvfd", NULL) == -1)
+	if (pledge("stdio recvfd", NULL) == -1)
 		err(1, "pledge");
 
 	setproctitle("[initializing]");
@@ -634,6 +629,9 @@ main(int argc, char **argv)
 	signal(SIGHUP, sig_hup);
 	alarm(delay);
 
+	if (priv_init_pcap(snaplen))
+		errx(1, "priv_init_pcap failed");
+
 	buffer = malloc(PFLOGD_BUFSIZE);
 
 	if (buffer == NULL) {
diff --git a/sbin/pflogd/pflogd.h b/sbin/pflogd/pflogd.h
index 7bab61167de..655d3f9a988 100644
--- a/sbin/pflogd/pflogd.h
+++ b/sbin/pflogd/pflogd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pflogd.h,v 1.5 2015/10/10 22:36:06 deraadt Exp $ */
+/*	$OpenBSD: pflogd.h,v 1.6 2017/09/05 15:41:25 brynet Exp $ */
 
 /*
  * Copyright (c) 2003 Can Erkin Acar
@@ -34,13 +34,15 @@
 void  logmsg(int priority, const char *message, ...);
 
 /* Privilege separation */
-int	priv_init(void);
+void	priv_init(int, char **);
+__dead void priv_exec(int, int, char **);
+int	priv_init_pcap(int);
 int	priv_set_snaplen(int snaplen);
 int	priv_open_log(void);
 int	priv_move_log(void);
 int	priv_pcap_stats(struct pcap_stat *);
-pcap_t *pcap_open_live_fd(int fd, int snaplen, char *ebuf);
 
+int   init_pcap(void);
 void set_pcap_filter(void);
 /* File descriptor send/recv */
 void send_fd(int, int);
diff --git a/sbin/pflogd/privsep.c b/sbin/pflogd/privsep.c
index ab1a9bb7c1a..d5bde071e8b 100644
--- a/sbin/pflogd/privsep.c
+++ b/sbin/pflogd/privsep.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: privsep.c,v 1.27 2017/08/12 16:31:09 florian Exp $	*/
+/*	$OpenBSD: privsep.c,v 1.28 2017/09/05 15:41:25 brynet Exp $	*/
 
 /*
  * Copyright (c) 2003 Can Erkin Acar
@@ -40,6 +40,7 @@
 #include "pflogd.h"
 
 enum cmd_types {
+	PRIV_INIT_PCAP,		/* init pcap fdpass bpf */
 	PRIV_SET_SNAPLEN,	/* set the snaplength */
 	PRIV_MOVE_LOG,		/* move logfile away */
 	PRIV_OPEN_LOG,		/* open logfile for appending */
@@ -59,18 +60,17 @@ static int  set_snaplen(int snap);
 static int  move_log(const char *name);
 
 extern char *filename;
+extern char *interface;
+extern char errbuf[PCAP_ERRBUF_SIZE];
 extern pcap_t *hpcap;
 
 /* based on syslogd privsep */
-int
-priv_init(void)
+void
+priv_init(int argc, char *argv[])
 {
-	int i, bpfd = -1, socks[2], cmd;
-	int snaplen, ret, olderrno;
+	int i, nargc, socks[2];
 	struct passwd *pw;
-
-	for (i = 1; i < _NSIG; i++)
-		signal(i, SIG_DFL);
+	char childnum[11], **privargv;
 
 	/* Create sockets */
 	if (socketpair(AF_LOCAL, SOCK_STREAM, PF_UNSPEC, socks) == -1)
@@ -103,8 +103,45 @@ priv_init(void)
 			err(1, "setresuid() failed");
 		close(socks[0]);
 		priv_fd = socks[1];
-		return 0;
+		return;
 	}
+	close(socks[1]);
+
+	if (dup2(socks[0], 3) == -1)
+		err(1, "dup2 priv sock failed");
+	closefrom(4);
+
+	snprintf(childnum, sizeof(childnum), "%d", child_pid);
+	if ((privargv = reallocarray(NULL, argc + 3, sizeof(char *))) == NULL)
+		err(1, "alloc priv argv failed");
+	nargc = 0;
+	privargv[nargc++] = argv[0];
+	privargv[nargc++] = "-P";
+	privargv[nargc++] = childnum;
+	for (i = 1; i < argc; i++)
+		privargv[nargc++] = argv[i];
+	privargv[nargc] = NULL;
+	execvp(privargv[0], privargv);
+	err(1, "exec priv '%s' failed", privargv[0]);
+}
+
+__dead void
+priv_exec(int child, int argc, char *argv[])
+{
+	int i, fd = -1, bpfd = -1, sock, cmd;
+	int snaplen, ret, olderrno;
+	unsigned int buflen;
+
+	if (argc <= 2 || strcmp("-P", argv[1]) != 0)
+		errx(1, "exec without priv");
+
+	sock = 3;
+	closefrom(4);
+
+	child_pid = child;
+
+	for (i = 1; i < _NSIG; i++)
+		signal(i, SIG_DFL);
 
 	/* Father */
 	/* Pass ALRM/TERM/HUP/INT/QUIT through to child, and accept CHLD */
@@ -116,7 +153,6 @@ priv_init(void)
 	signal(SIGCHLD, sig_chld);
 
 	setproctitle("[priv]");
-	close(socks[1]);
 
 #if 0
 	/* This needs to do bpf ioctl */
@@ -125,13 +161,31 @@ BROKEN	if (pledge("stdio rpath wpath cpath sendfd proc bpf", NULL) == -1)
 #endif
 
 	while (!gotsig_chld) {
-		if (may_read(socks[0], &cmd, sizeof(int)))
+		if (may_read(sock, &cmd, sizeof(int)))
 			break;
 		switch (cmd) {
+		case PRIV_INIT_PCAP:
+			logmsg(LOG_DEBUG,
+			    "[priv]: msg PRIV_INIT_PCAP received");
+			/* initialize pcap */
+			if (hpcap != NULL || init_pcap()) {
+				logmsg(LOG_ERR, "[priv]: Exiting, init failed");
+				_exit(1);
+			}
+			buflen = hpcap->bufsize; /* BIOCGBLEN for unpriv proc */
+			must_write(sock, &buflen, sizeof(unsigned int));
+			fd = pcap_fileno(hpcap);
+			send_fd(sock, fd);
+			if (fd < 0) {
+				logmsg(LOG_ERR, "[priv]: Exiting, init failed");
+				_exit(1);
+			}
+			break;
+
 		case PRIV_SET_SNAPLEN:
 			logmsg(LOG_DEBUG,
 			    "[priv]: msg PRIV_SET_SNAPLENGTH received");
-			must_read(socks[0], &snaplen, sizeof(int));
+			must_read(sock, &snaplen, sizeof(int));
 
 			ret = set_snaplen(snaplen);
 			if (ret) {
@@ -140,7 +194,7 @@ BROKEN	if (pledge("stdio rpath wpath cpath sendfd proc bpf", NULL) == -1)
 				   snaplen);
 			}
 
-			must_write(socks[0], &ret, sizeof(int));
+			must_write(sock, &ret, sizeof(int));
 			break;
 
 		case PRIV_OPEN_LOG:
@@ -155,7 +209,7 @@ BROKEN	if (pledge("stdio rpath wpath cpath sendfd proc bpf", NULL) == -1)
 			    O_RDWR|O_CREAT|O_APPEND|O_NONBLOCK|O_NOFOLLOW,
 			    0600);
 			olderrno = errno;
-			send_fd(socks[0], bpfd);
+			send_fd(sock, bpfd);
 			if (bpfd < 0)
 				logmsg(LOG_NOTICE,
 				    "[priv]: failed to open %s: %s",
@@ -166,7 +220,7 @@ BROKEN	if (pledge("stdio rpath wpath cpath sendfd proc bpf", NULL) == -1)
 			logmsg(LOG_DEBUG,
 			    "[priv]: msg PRIV_MOVE_LOG received");
 			ret = move_log(filename);
-			must_write(socks[0], &ret, sizeof(int));
+			must_write(sock, &ret, sizeof(int));
 			break;
 
 		default:
@@ -176,7 +230,7 @@ BROKEN	if (pledge("stdio rpath wpath cpath sendfd proc bpf", NULL) == -1)
 		}
 	}
 
-	_exit(1);
+	exit(1);
 }
 
 /* this is called from parent */
@@ -233,6 +287,46 @@ move_log(const char *name)
 	return (0);
 }
 
+
+/* receive bpf fd from privileged process using fdpass and init pcap */
+int
+priv_init_pcap(int snaplen)
+{
+	int cmd, fd;
+	unsigned int buflen;
+
+	if (priv_fd < 0)
+		errx(1, "%s: called from privileged portion", __func__);
+
+	cmd = PRIV_INIT_PCAP;
+
+	must_write(priv_fd, &cmd, sizeof(int));
+	must_read(priv_fd, &buflen, sizeof(unsigned int));
+	fd = receive_fd(priv_fd);
+	if (fd < 0)
+		return (-1);
+
+	/* XXX temporary until pcap_open_live_fd API */
+	hpcap = pcap_create(interface, errbuf);
+	if (hpcap == NULL)
+		return (-1);
+
+	/* XXX copies from pcap_open_live/pcap_activate */
+	hpcap->fd = fd;
+	pcap_set_snaplen(hpcap, snaplen);
+	pcap_set_promisc(hpcap, 1);
+	pcap_set_timeout(hpcap, PCAP_TO_MS);
+	hpcap->oldstyle = 1;
+	hpcap->linktype = DLT_PFLOG;
+	hpcap->bufsize = buflen; /* XXX bpf BIOCGBLEN */
+	hpcap->buffer = malloc(hpcap->bufsize);
+	if (hpcap->buffer == NULL)
+		return (-1);
+	hpcap->activated = 1;
+
+	return (0);
+}
+
 /*
  * send the snaplength to privileged process
  */