Re: tcpdump: revisiting some old diffs, cleanup unused functions
OK
Re: tcpdump: revisiting some old diffs, cleanup unused functions
Still works as advertised, ok mestre@ On 19:32 Wed 07 Nov , Bryan Steele wrote: > On Wed, Nov 07, 2018 at 07:06:09PM -0500, Bryan Steele wrote: > > I'm revisiting some old tcpdump diffs, now that mestre@ has added proper > > unveil(2) support! :-) > > > > Refresher: https://marc.info/?l=openbsd-tech&m=150535073209723&w=2 > > > > This hoists opening pf.os(5) fingerprints '-o' from the 'RUN' state to > > the 'FILTER' state, this will allow for a reduced pledge(2) at runtime > > in the (currently root) monitor process. > > This was a bit of copy & paste, sorry. This moves the opening of pf.os > earlier and avoids the unveil later on. Of course, reducing the runtime > pledge(2) promises will come later! :-) > > > > > This still works as well as it already has. :-) > > > > ( ... ) [tcp sum ok] (src OS: OpenBSD 6.1) 3311509932:3311509932(0) win > > 16384 > > (DF) (ttl 64, id 41239, len 64) > > > > The only potential difference is that if /etc/pf.os is replaced at > > runtime, tcpdump won't reopen it. > > > > I don't think that's a problem.. > > > > ok? > > > > -Bryan. > > Remove the now unused internal privsep "getline" code, which passed > lines over a socket, replaced with explicit fdpassing of /etc/pf.os. > > This depends on the previous diff.. > > ok? > > -Bryan. > > Index: privsep.c > === > RCS file: /cvs/src/usr.sbin/tcpdump/privsep.c,v > retrieving revision 1.49 > diff -u -p -u -r1.49 privsep.c > --- privsep.c 28 Sep 2018 06:48:59 - 1.49 > +++ privsep.c 8 Nov 2018 00:19:47 - > @@ -77,8 +77,8 @@ static const int allowed_max[] = { > 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 +90,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; > @@ -123,7 +112,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); > @@ -345,10 +333,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); > @@ -577,55 +561,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); > - > -
Re: tcpdump: revisiting some old diffs, cleanup unused functions
On Wed, Nov 07, 2018 at 07:06:09PM -0500, Bryan Steele wrote: > I'm revisiting some old tcpdump diffs, now that mestre@ has added proper > unveil(2) support! :-) > > Refresher: https://marc.info/?l=openbsd-tech&m=150535073209723&w=2 > > This hoists opening pf.os(5) fingerprints '-o' from the 'RUN' state to > the 'FILTER' state, this will allow for a reduced pledge(2) at runtime > in the (currently root) monitor process. This was a bit of copy & paste, sorry. This moves the opening of pf.os earlier and avoids the unveil later on. Of course, reducing the runtime pledge(2) promises will come later! :-) > > This still works as well as it already has. :-) > > ( ... ) [tcp sum ok] (src OS: OpenBSD 6.1) 3311509932:3311509932(0) win > 16384 > (DF) (ttl 64, id 41239, len 64) > > The only potential difference is that if /etc/pf.os is replaced at > runtime, tcpdump won't reopen it. > > I don't think that's a problem.. > > ok? > > -Bryan. Remove the now unused internal privsep "getline" code, which passed lines over a socket, replaced with explicit fdpassing of /etc/pf.os. This depends on the previous diff.. ok? -Bryan. Index: privsep.c === RCS file: /cvs/src/usr.sbin/tcpdump/privsep.c,v retrieving revision 1.49 diff -u -p -u -r1.49 privsep.c --- privsep.c 28 Sep 2018 06:48:59 - 1.49 +++ privsep.c 8 Nov 2018 00:19:47 - @@ -77,8 +77,8 @@ static const int allowed_max[] = { 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 +90,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)) - intdebug_level = LOG_INFO; intpriv_fd = -1; volatile pid_t child_pid = -1; @@ -123,7 +112,6 @@ static void impl_getrpcbynumber(int); static voidimpl_getserventries(int); static voidimpl_getprotoentries(int); static voidimpl_localtime(int fd); -static voidimpl_getlines(int); static voidimpl_pcap_stats(int, int *); static voidtest_state(int, int); @@ -345,10 +333,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); @@ -577,55 +561,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 +721,6 @@ priv_localtime(const time_t *t