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=150535073209723=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, );
> @@ -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, , 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, ))) {
> - 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=150535073209723=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, );
@@ -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, , 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, ))) {
- 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)
return
