Hoist opening pf.os(5) fingerprints '-o' earlier so that it doesn't

need to be unveiled at runtime in the monitor process.

Cleanup the unused internal privsep "getlines" code, we now explictly
fdpass the OS fingerprints file instead.

ok mestre@ kn@

4 files changed, 63 insertions, 116 deletions

diff --git a/usr.sbin/tcpdump/pfctl_osfp.c b/usr.sbin/tcpdump/pfctl_osfp.c
index e4424615698..c337cc5e20c 100644
--- a/usr.sbin/tcpdump/pfctl_osfp.c
+++ b/usr.sbin/tcpdump/pfctl_osfp.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pfctl_osfp.c,v 1.13 2017/05/28 10:06:12 akfaew Exp $ */
+/*	$OpenBSD: pfctl_osfp.c,v 1.14 2018/11/08 14:06:09 brynet Exp $ */
 
 /*
  * Copyright (c) 2003 Mike Frantzen <frantzen@openbsd.org>
@@ -81,17 +81,14 @@ void			 print_name_list(int, struct name_list *, const char *);
 void			 sort_name_list(int, struct name_list *);
 struct name_entry	*lookup_name_list(struct name_list *, const char *);
 
-/* XXX arbitrary */
-#define MAX_FP_LINE 1024
-
 /* Load fingerprints from a file */
 int
 pfctl_file_fingerprints(int dev, int opts, const char *fp_filename)
 {
-	u_char buf[MAX_FP_LINE];
+	FILE *in;
 	u_char *line;
 	size_t len;
-	int i, lineno = 0;
+	int i, fd, lineno = 0;
 	int window, w_mod, ttl, df, psize, p_mod, mss, mss_mod, wscale,
 	    wscale_mod, optcnt, ts0;
 	pf_tcpopts_t packed_tcpopts;
@@ -99,15 +96,22 @@ pfctl_file_fingerprints(int dev, int opts, const char *fp_filename)
 	struct pf_osfp_ioctl fp;
 
 	pfctl_flush_my_fingerprints(&classes);
+	
+	fd = priv_open_pfosfp();
+	if (fd < 0)
+		return (1);
+	
+	if ((in = fdopen(fd, "r")) == NULL) {
+		warn("%s", fp_filename);
+		return (1);
+	}
+
 	class = version = subtype = desc = tcpopts = NULL;
 
 	if ((opts & PF_OPT_NOACTION) == 0)
 		pfctl_clear_fingerprints(dev, opts);
 
-	priv_getlines(FTAB_PFOSFP);
-	while ((len = priv_getline(buf, sizeof(buf))) > 0) {
-		buf[len -1] = '\n';
-		line = buf;
+	while ((line = fgetln(in, &len)) != NULL) {
 		lineno++;
 		free(class);
 		free(version);
diff --git a/usr.sbin/tcpdump/privsep.c b/usr.sbin/tcpdump/privsep.c
index c087773d685..a9329fb6dc4 100644
--- a/usr.sbin/tcpdump/privsep.c
+++ b/usr.sbin/tcpdump/privsep.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: privsep.c,v 1.49 2018/09/28 06:48:59 mestre Exp $	*/
+/*	$OpenBSD: privsep.c,v 1.50 2018/11/08 14:06:09 brynet Exp $	*/
 
 /*
  * Copyright (c) 2003 Can Erkin Acar
@@ -73,12 +73,13 @@ static const int allowed_max[] = {
 	/* INIT */	ALLOW(PRIV_OPEN_BPF) | ALLOW(PRIV_OPEN_DUMP) |
 			ALLOW(PRIV_SETFILTER),
 	/* BPF */	ALLOW(PRIV_SETFILTER),
-	/* FILTER */	ALLOW(PRIV_OPEN_OUTPUT) | ALLOW(PRIV_GETSERVENTRIES) |
+	/* FILTER */	ALLOW(PRIV_OPEN_PFOSFP) | ALLOW(PRIV_OPEN_OUTPUT) |
+			ALLOW(PRIV_GETSERVENTRIES) |
 			ALLOW(PRIV_GETPROTOENTRIES) |
 			ALLOW(PRIV_ETHER_NTOHOST) | ALLOW(PRIV_INIT_DONE),
 	/* RUN */	ALLOW(PRIV_GETHOSTBYADDR) | ALLOW(PRIV_ETHER_NTOHOST) |
-			ALLOW(PRIV_GETRPCBYNUMBER) | ALLOW(PRIV_GETLINES) |
-			ALLOW(PRIV_LOCALTIME) | ALLOW(PRIV_PCAP_STATS),
+			ALLOW(PRIV_GETRPCBYNUMBER) | ALLOW(PRIV_LOCALTIME) |
+			ALLOW(PRIV_PCAP_STATS),
 	/* EXIT */	0
 };
 
@@ -90,21 +91,10 @@ static int allowed_ext[] = {
 	/* INIT */	ALLOW(PRIV_SETFILTER),
 	/* BPF */	ALLOW(PRIV_SETFILTER),
 	/* FILTER */	ALLOW(PRIV_GETSERVENTRIES),
-	/* RUN */	ALLOW(PRIV_GETLINES) | ALLOW(PRIV_LOCALTIME) |
-			ALLOW(PRIV_PCAP_STATS),
+	/* RUN */	ALLOW(PRIV_LOCALTIME) | ALLOW(PRIV_PCAP_STATS),
 	/* EXIT */	0
 };
 
-struct ftab {
-	char *name;
-	int max;
-	int count;
-};
-
-static struct ftab file_table[] = {{PF_OSFP_FILE, 1, 0}};
-
-#define NUM_FILETAB (sizeof(file_table) / sizeof(struct ftab))
-
 int		debug_level = LOG_INFO;
 int		priv_fd = -1;
 volatile	pid_t child_pid = -1;
@@ -114,6 +104,7 @@ extern void	set_slave_signals(void);
 
 static void	impl_open_bpf(int, int *);
 static void	impl_open_dump(int, const char *);
+static void	impl_open_pfosfp(int);
 static void	impl_open_output(int, const char *);
 static void	impl_setfilter(int, char *, int *);
 static void	impl_init_done(int, int *);
@@ -123,7 +114,6 @@ static void	impl_getrpcbynumber(int);
 static void	impl_getserventries(int);
 static void	impl_getprotoentries(int);
 static void	impl_localtime(int fd);
-static void	impl_getlines(int);
 static void	impl_pcap_stats(int, int *);
 
 static void	test_state(int, int);
@@ -277,6 +267,8 @@ priv_exec(int argc, char *argv[])
 		allowed_ext[STATE_RUN] |= ALLOW(PRIV_GETRPCBYNUMBER);
 		allowed_ext[STATE_FILTER] |= ALLOW(PRIV_GETPROTOENTRIES);
 	}
+	if (oflag)
+		allowed_ext[STATE_FILTER] |= ALLOW(PRIV_OPEN_PFOSFP);
 
 	if (infile)
 		cmdbuf = read_infile(infile);
@@ -297,6 +289,10 @@ priv_exec(int argc, char *argv[])
 			test_state(cmd, STATE_BPF);
 			impl_open_dump(sock, RFileName);
 			break;
+		case PRIV_OPEN_PFOSFP:
+			test_state(cmd, STATE_FILTER);
+			impl_open_pfosfp(sock);
+			break;
 		case PRIV_OPEN_OUTPUT:
 			test_state(cmd, STATE_RUN);
 			impl_open_output(sock, WFileName);
@@ -309,10 +305,6 @@ priv_exec(int argc, char *argv[])
 			test_state(cmd, STATE_RUN);
 			impl_init_done(sock, &bpfd);
 
-			if (oflag) {
-				if (unveil("/etc/pf.os", "r") == -1)
-					err(1, "unveil");
-			}
 			if (unveil("/etc/ethers", "r") == -1)
 				err(1, "unveil");
 			if (unveil("/etc/rpc", "r") == -1)
@@ -345,10 +337,6 @@ priv_exec(int argc, char *argv[])
 			test_state(cmd, STATE_RUN);
 			impl_localtime(sock);
 			break;
-		case PRIV_GETLINES:
-			test_state(cmd, STATE_RUN);
-			impl_getlines(sock);
-			break;
 		case PRIV_PCAP_STATS:
 			test_state(cmd, STATE_RUN);
 			impl_pcap_stats(sock, &bpfd);
@@ -416,6 +404,24 @@ impl_open_dump(int fd, const char *RFileName)
 }
 
 static void
+impl_open_pfosfp(int fd)
+{
+	int file, err = 0;
+
+	logmsg(LOG_DEBUG, "[priv]: msg PRIV_OPEN_PFOSFP received");
+
+	file = open(PF_OSFP_FILE, O_RDONLY, 0);
+	err = errno;
+	if (file < 0)
+		logmsg(LOG_DEBUG, "[priv]: failed to open %s: %s",
+		    PF_OSFP_FILE, strerror(errno));
+	send_fd(fd, file);
+	must_write(fd, &err, sizeof(int));
+	if (file >= 0)
+		close(file);
+}
+
+static void
 impl_open_output(int fd, const char *WFileName)
 {
 	int file, err;
@@ -577,55 +583,6 @@ impl_localtime(int fd)
 }
 
 static void
-impl_getlines(int fd)
-{
-	FILE *fp;
-	char *buf, *lbuf, *file;
-	size_t len, fid;
-
-	logmsg(LOG_DEBUG, "[priv]: msg PRIV_GETLINES received");
-
-	must_read(fd, &fid, sizeof(size_t));
-	if (fid >= NUM_FILETAB)
-		errx(1, "invalid file id");
-
-	file = file_table[fid].name;
-
-	if (file == NULL)
-		errx(1, "invalid file referenced");
-
-	if (file_table[fid].count >= file_table[fid].max)
-		errx(1, "maximum open count exceeded for %s", file);
-
-	file_table[fid].count++;
-
-	if ((fp = fopen(file, "r")) == NULL) {
-		write_zero(fd);
-		return;
-	}
-
-	lbuf = NULL;
-	while ((buf = fgetln(fp, &len))) {
-		if (buf[len - 1] == '\n')
-			buf[len - 1] = '\0';
-		else {
-			if ((lbuf = malloc(len + 1)) == NULL)
-				err(1, NULL);
-			memcpy(lbuf, buf, len);
-			lbuf[len] = '\0';
-			buf = lbuf;
-		}
-
-		write_string(fd, buf);
-
-		free(lbuf);
-		lbuf = NULL;
-	}
-	write_zero(fd);
-	fclose(fp);
-}
-
-static void
 impl_pcap_stats(int fd, int *bpfd)
 {
 	struct pcap_stat stats;
@@ -786,17 +743,6 @@ priv_localtime(const time_t *t)
 	return &lt;
 }
 
-/* start getting lines from a file */
-void
-priv_getlines(size_t sz)
-{
-	if (priv_fd < 0)
-		errx(1, "%s called from privileged portion", __func__);
-
-	write_command(priv_fd, PRIV_GETLINES);
-	must_write(priv_fd, &sz, sizeof(size_t));
-}
-
 int
 priv_pcap_stats(struct pcap_stat *ps)
 {
@@ -808,16 +754,20 @@ priv_pcap_stats(struct pcap_stat *ps)
 	return (0);
 }
 
-/* retrieve a line from a file, should be called repeatedly after calling
-   priv_getlines(), until it returns zero. */
-size_t
-priv_getline(char *line, size_t line_len)
+int
+priv_open_pfosfp(void)
 {
-	if (priv_fd < 0)
-		errx(1, "%s called from privileged portion", __func__);
+	int fd, err = 0;
+	write_command(priv_fd, PRIV_OPEN_PFOSFP);
+
+	fd = receive_fd(priv_fd);
+	must_read(priv_fd, &err, sizeof(int));
+	if (fd < 0) {
+		warnc(err, "%s", PF_OSFP_FILE);
+		return (-1);
+	}
 
-	/* read the line */
-	return (read_string(priv_fd, line, line_len, __func__));
+	return (fd);
 }
 
 /* Read all data or return 1 for error. */
diff --git a/usr.sbin/tcpdump/privsep.h b/usr.sbin/tcpdump/privsep.h
index 725d61d0fa8..017b100f9f8 100644
--- a/usr.sbin/tcpdump/privsep.h
+++ b/usr.sbin/tcpdump/privsep.h
@@ -21,12 +21,10 @@
 
 #define TCPDUMP_MAGIC 0xa1b2c3d4
 
-/* file ids used by priv_getlines */
-#define FTAB_PFOSFP	0
-
 enum cmd_types {
 	PRIV_OPEN_BPF,		/* open a bpf descriptor */
 	PRIV_OPEN_DUMP,		/* open dump file for reading */
+	PRIV_OPEN_PFOSFP,	/* open pf.os(5) fingerprint db for reading */
 	PRIV_OPEN_OUTPUT,	/* open output file */
 	PRIV_SETFILTER,		/* set a bpf read filter */
 	PRIV_GETHOSTBYADDR,	/* resolve numeric address into hostname */
@@ -35,7 +33,6 @@ enum cmd_types {
 	PRIV_GETSERVENTRIES,	/* get the service entries table */
 	PRIV_GETPROTOENTRIES,	/* get the ip protocol entries table */
 	PRIV_LOCALTIME,		/* return localtime */
-	PRIV_GETLINES,		/* get lines from a file */
 	PRIV_INIT_DONE,		/* signal that the initialization is done */
 	PRIV_PCAP_STATS		/* get pcap_stats() results */
 };
@@ -75,12 +72,8 @@ void	priv_getprotoentries(void);
    calling priv_getprotoentries() until it returns zero */
 size_t	priv_getprotoentry(char *, size_t, int *);
 
-/* Start getting lines from a file */
-void	priv_getlines(size_t);
-
-/* Retrieve a single line from a file, should be called repeatedly after
-   calling priv_getlines() until it returns zero */
-size_t	priv_getline(char *, size_t);
+/* Retrieve pf.os(5) fingerprints file descriptor */
+int	priv_open_pfosfp();
 
 /* Return the pcap statistics upon completion */
 int	priv_pcap_stats(struct pcap_stat *);
diff --git a/usr.sbin/tcpdump/tcpdump.c b/usr.sbin/tcpdump/tcpdump.c
index aa389a8243f..b4818a38491 100644
--- a/usr.sbin/tcpdump/tcpdump.c
+++ b/usr.sbin/tcpdump/tcpdump.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: tcpdump.c,v 1.87 2018/07/06 07:13:21 dlg Exp $	*/
+/*	$OpenBSD: tcpdump.c,v 1.88 2018/11/08 14:06:09 brynet Exp $	*/
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1991, 1992, 1993, 1994, 1995, 1996, 1997
@@ -471,6 +471,8 @@ main(int argc, char **argv)
 		bpf_dump(fcode, dflag);
 		exit(0);
 	}
+	if (oflag)
+		oflag = init_pfosfp();
 	init_addrtoname(localnet, netmask);
 
 	if (WFileName) {
@@ -500,8 +502,6 @@ main(int argc, char **argv)
 		(void)fflush(stderr);
 	}
 
-	if (oflag)
-		oflag = init_pfosfp();
 	if (tflag > 0)
 		thiszone = gmt2local(0);