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);
> }
>
