On Thu, Sep 07, 2017 at 12:27:18AM -0400, Bryan Steele wrote: > Hi, > > This turned out easier then pflogd thanks to the existing privsep design > work done by deraadt@ and canacar@ many years ago. While tcpdump isn't a
Small correction for the record: done by otto@ and cancacar@ while being prodded almost gently by deraadt@ -Otto > daemon in the traditional sense, it isn't so uncommon for people to have > long running sessions. At least on OpenBSD, this is even safe thanks to > privsep and very strict pledge(2) promises. > > This does shuffle things around a bit internally, but I don't think I've > totally broken anything. > > Seems to work with some light testing, live captures, -w/-r offline pcap > files. > > Comments? > > -Bryan. > > Index: privsep.c > =================================================================== > RCS file: /cvs/src/usr.sbin/tcpdump/privsep.c,v > retrieving revision 1.45 > diff -u -p -u -r1.45 privsep.c > --- usr.sbin/tcpdump/privsep.c 14 Jun 2017 20:48:54 -0000 1.45 > +++ usr.sbin/tcpdump/privsep.c 7 Sep 2017 03:19:26 -0000 > @@ -33,6 +33,7 @@ > #include <err.h> > #include <errno.h> > #include <fcntl.h> > +#include <limits.h> > #include <netdb.h> > #include <paths.h> > #include <pwd.h> > @@ -132,13 +133,10 @@ static void logmsg(int, const char *, .. > int > priv_init(int argc, char **argv) > { > - int bpfd = -1; > - int i, socks[2], cmd, nflag = 0; > + int i, nargc, socks[2]; > struct passwd *pw; > - char *cmdbuf, *infile = NULL; > - char *RFileName = NULL; > - char *WFileName = NULL; > sigset_t allsigs, oset; > + char childnum[11], **privargv; > > closefrom(STDERR_FILENO + 1); > for (i = 1; i < _NSIG; i++) > @@ -155,7 +153,7 @@ priv_init(int argc, char **argv) > if (child_pid < 0) > err(1, "fork() failed"); > > - if (child_pid) { > + if (!child_pid) { > close(socks[0]); > priv_fd = socks[1]; > > @@ -188,14 +186,48 @@ priv_init(int argc, char **argv) > > return (0); > } > + 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 argc, char *argv[]) > +{ > + int bpfd = -1; > + int i, sock, cmd, nflag = 0; > + char *cmdbuf, *infile = NULL; > + char *RFileName = NULL; > + char *WFileName = NULL; > + const char *errstr; > + > + sock = 3; > + > + closefrom(4); > + for (i = 1; i < _NSIG; i++) > + signal(i, SIG_DFL); > > - sigprocmask(SIG_SETMASK, &oset, NULL); > signal(SIGINT, SIG_IGN); > > /* parse the arguments for required options */ > opterr = 0; > while ((i = getopt(argc, argv, > - "ac:D:deE:fF:i:lLnNOopqr:s:StT:vw:xXy:Y")) != -1) { > + "ac:D:deE:fF:i:lLnNOopP:qr:s:StT:vw:xXy:Y")) != -1) { > switch (i) { > case 'n': > nflag++; > @@ -213,12 +245,21 @@ priv_init(int argc, char **argv) > infile = optarg; > break; > > + case 'P': > + child_pid = strtonum(optarg, 2, INT_MAX, &errstr); > + if (errstr) > + errx(1, "priv child %s: %s", errstr, optarg); > + break; > + > default: > /* nothing */ > break; > } > } > > + if (child_pid < 2) > + errx(1, "exec without priv"); > + > if (RFileName != NULL) { > if (strcmp(RFileName, "-") != 0) > allowed_ext[STATE_INIT] |= ALLOW(PRIV_OPEN_DUMP); > @@ -245,31 +286,30 @@ priv_init(int argc, char **argv) > cmdbuf = copy_argv(&argv[optind]); > > setproctitle("[priv]"); > - close(socks[1]); > > for (;;) { > - if (may_read(socks[0], &cmd, sizeof(int))) > + if (may_read(sock, &cmd, sizeof(int))) > break; > switch (cmd) { > case PRIV_OPEN_BPF: > test_state(cmd, STATE_BPF); > - impl_open_bpf(socks[0], &bpfd); > + impl_open_bpf(sock, &bpfd); > break; > case PRIV_OPEN_DUMP: > test_state(cmd, STATE_BPF); > - impl_open_dump(socks[0], RFileName); > + impl_open_dump(sock, RFileName); > break; > case PRIV_OPEN_OUTPUT: > test_state(cmd, STATE_RUN); > - impl_open_output(socks[0], WFileName); > + impl_open_output(sock, WFileName); > break; > case PRIV_SETFILTER: > test_state(cmd, STATE_FILTER); > - impl_setfilter(socks[0], cmdbuf, &bpfd); > + impl_setfilter(sock, cmdbuf, &bpfd); > break; > case PRIV_INIT_DONE: > test_state(cmd, STATE_RUN); > - impl_init_done(socks[0], &bpfd); > + impl_init_done(sock, &bpfd); > > if (pledge("stdio rpath inet unix dns recvfd bpf", > NULL) == -1) > err(1, "pledge"); > @@ -277,45 +317,45 @@ priv_init(int argc, char **argv) > break; > case PRIV_GETHOSTBYADDR: > test_state(cmd, STATE_RUN); > - impl_gethostbyaddr(socks[0]); > + impl_gethostbyaddr(sock); > break; > case PRIV_ETHER_NTOHOST: > test_state(cmd, cur_state); > - impl_ether_ntohost(socks[0]); > + impl_ether_ntohost(sock); > break; > case PRIV_GETRPCBYNUMBER: > test_state(cmd, STATE_RUN); > - impl_getrpcbynumber(socks[0]); > + impl_getrpcbynumber(sock); > break; > case PRIV_GETSERVENTRIES: > test_state(cmd, STATE_FILTER); > - impl_getserventries(socks[0]); > + impl_getserventries(sock); > break; > case PRIV_GETPROTOENTRIES: > test_state(cmd, STATE_FILTER); > - impl_getprotoentries(socks[0]); > + impl_getprotoentries(sock); > break; > case PRIV_LOCALTIME: > test_state(cmd, STATE_RUN); > - impl_localtime(socks[0]); > + impl_localtime(sock); > break; > case PRIV_GETLINES: > test_state(cmd, STATE_RUN); > - impl_getlines(socks[0]); > + impl_getlines(sock); > break; > case PRIV_PCAP_STATS: > test_state(cmd, STATE_RUN); > - impl_pcap_stats(socks[0], &bpfd); > + impl_pcap_stats(sock, &bpfd); > break; > default: > logmsg(LOG_ERR, "[priv]: unknown command %d", cmd); > - _exit(1); > + exit(1); > /* NOTREACHED */ > } > } > > /* NOTREACHED */ > - _exit(0); > + exit(0); > } > > static void > Index: privsep.h > =================================================================== > RCS file: /cvs/src/usr.sbin/tcpdump/privsep.h,v > retrieving revision 1.9 > diff -u -p -u -r1.9 privsep.h > --- usr.sbin/tcpdump/privsep.h 14 Jun 2017 20:48:54 -0000 1.9 > +++ usr.sbin/tcpdump/privsep.h 7 Sep 2017 03:19:26 -0000 > @@ -44,6 +44,7 @@ struct ether_addr; > > /* Privilege separation */ > int priv_init(int, char **); > +__dead void priv_exec(int, char **); > void priv_init_done(void); > > int setfilter(int, int, char *); > Index: setsignal.c > =================================================================== > RCS file: /cvs/src/usr.sbin/tcpdump/setsignal.c,v > retrieving revision 1.6 > diff -u -p -u -r1.6 setsignal.c > --- usr.sbin/tcpdump/setsignal.c 14 Oct 2015 04:55:17 -0000 1.6 > +++ usr.sbin/tcpdump/setsignal.c 7 Sep 2017 03:19:26 -0000 > @@ -36,8 +36,6 @@ setsignal(int sig, void (*func)(int)) > if (sigaction(sig, NULL, &sa) == 0 && sa.sa_handler != SIG_IGN) { > sa.sa_handler = func; > sa.sa_flags = SA_RESTART; > - if (sig == SIGCHLD) > - sa.sa_flags |= SA_NOCLDSTOP; > sigemptyset(&sa.sa_mask); > sigaction(sig, &sa, NULL); > } > Index: tcpdump.c > =================================================================== > RCS file: /cvs/src/usr.sbin/tcpdump/tcpdump.c,v > retrieving revision 1.79 > diff -u -p -u -r1.79 tcpdump.c > --- usr.sbin/tcpdump/tcpdump.c 16 Nov 2016 13:47:27 -0000 1.79 > +++ usr.sbin/tcpdump/tcpdump.c 7 Sep 2017 03:19:26 -0000 > @@ -85,15 +85,12 @@ char *device = NULL; > > int32_t thiszone; /* seconds offset from gmt to local time */ > > -extern volatile pid_t child_pid; > - > /* Externs */ > extern void bpf_dump(struct bpf_program *, int); > extern int esp_init(char *); > > /* Forwards */ > void cleanup(int); > -void gotchld(int); > extern __dead void usage(void); > > /* Length of saved portion of packet. */ > @@ -218,6 +215,10 @@ main(int argc, char **argv) > else > program_name = argv[0]; > > + /* '-P' used internally, exec the parent */ > + if (argc >= 2 && strcmp("-P", argv[1]) == 0) > + priv_exec(argc, argv); > + > if (priv_init(argc, argv)) > error("Failed to setup privsep"); > > @@ -533,25 +534,6 @@ cleanup(int signo) > _exit(0); > } > > -void > -gotchld(int signo) > -{ > - pid_t pid; > - int status; > - int save_err = errno; > - > - do { > - pid = waitpid(child_pid, &status, WNOHANG); > - if (pid > 0 && (WIFEXITED(status) || WIFSIGNALED(status))) > - cleanup(0); > - } while (pid == -1 && errno == EINTR); > - > - if (pid == -1) > - _exit(1); > - > - errno = save_err; > -} > - > /* dump the buffer in `emacs-hexl' style */ > void > default_print_hexl(const u_char *cp, unsigned int length) > @@ -682,7 +664,6 @@ set_slave_signals(void) > { > setsignal(SIGTERM, cleanup); > setsignal(SIGINT, cleanup); > - setsignal(SIGCHLD, gotchld); > setsignal(SIGHUP, cleanup); > } >