tcpdump's privsep monitor process handles any privileged operations on
behalf of the unprivileged "packet parser" process. After this, it
enters its final runtime state, which:

* Performs DNS and other "numbers to names" lookups, sending results
back over a pipe/socketpair.
* Displays the final packet statistics on ^C.

We can now go a step further by dropping root privileges here, as bpf
BIOCGSTATS is still permitted by non-root on open descriptors after it
has been permanently locked with BIOCLOCK. This provides some additional
protection, to go along with the already tight unveil(2) and pledge(2)
restrictions.

This also fixes a small corner case with '-w' mode, as the monitor
process may not always finish with PRIV_INIT_DONE. I don't really
think this was a problem (in this case no DNS lookups are performed,
only ^C statistics).

comments or ok? :-)

-Bryan.

Index: privsep.c
===================================================================
RCS file: /cvs/src/usr.sbin/tcpdump/privsep.c,v
retrieving revision 1.51
diff -u -p -u -r1.51 privsep.c
--- usr.sbin/tcpdump/privsep.c  9 Nov 2018 18:39:34 -0000       1.51
+++ usr.sbin/tcpdump/privsep.c  16 Nov 2018 17:27:37 -0000
@@ -102,6 +102,8 @@ static volatile     sig_atomic_t cur_state =
 
 extern void    set_slave_signals(void);
 
+static void    drop_privs(int);
+
 static void    impl_open_bpf(int, int *);
 static void    impl_open_dump(int, const char *);
 static void    impl_open_pfosfp(int);
@@ -119,11 +121,42 @@ static void       impl_pcap_stats(int, int *);
 static void    test_state(int, int);
 static void    logmsg(int, const char *, ...);
 
+static void
+drop_privs(int nochroot)
+{
+       struct passwd *pw;
+
+       /*
+        * If run as regular user, then tcpdump will rely on
+        * pledge(2). If we are root, we want to chroot also..
+        */
+       if (getuid() != 0)
+               return;
+
+       pw = getpwnam("_tcpdump");
+       if (pw == NULL)
+               errx(1, "unknown user _tcpdump");
+
+       if (!nochroot) {
+               if (chroot(pw->pw_dir) == -1)
+                       err(1, "unable to chroot");
+               if (chdir("/") == -1)
+                       err(1, "unable to chdir");
+       }
+
+       /* drop to _tcpdump */
+       if (setgroups(1, &pw->pw_gid) == -1)
+               err(1, "setgroups() failed");
+       if (setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) == -1)
+               err(1, "setresgid() failed");
+       if (setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid) == -1)
+               err(1, "setresuid() failed");
+}
+
 int
 priv_init(int argc, char **argv)
 {
        int i, nargc, socks[2];
-       struct passwd *pw;
        sigset_t allsigs, oset;
        char **privargv;
 
@@ -149,29 +182,7 @@ priv_init(int argc, char **argv)
                set_slave_signals();
                sigprocmask(SIG_SETMASK, &oset, NULL);
 
-               /*
-                * If run as regular user, packet parser will rely on
-                * pledge(2). If we are root, we want to chroot also..
-                */
-               if (getuid() != 0)
-                       return (0);
-
-               pw = getpwnam("_tcpdump");
-               if (pw == NULL)
-                       errx(1, "unknown user _tcpdump");
-
-               if (chroot(pw->pw_dir) == -1)
-                       err(1, "unable to chroot");
-               if (chdir("/") == -1)
-                       err(1, "unable to chdir");
-
-               /* drop to _tcpdump */
-               if (setgroups(1, &pw->pw_gid) == -1)
-                       err(1, "setgroups() failed");
-               if (setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) == -1)
-                       err(1, "setresgid() failed");
-               if (setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid) == -1)
-                       err(1, "setresuid() failed");
+               drop_privs(0);
 
                return (0);
        }
@@ -256,10 +267,8 @@ priv_exec(int argc, char *argv[])
        if (WFileName != NULL) {
                if (strcmp(WFileName, "-") != 0)
                        allowed_ext[STATE_FILTER] |= ALLOW(PRIV_OPEN_OUTPUT);
-               else
-                       allowed_ext[STATE_FILTER] |= ALLOW(PRIV_INIT_DONE);
-       } else
-               allowed_ext[STATE_FILTER] |= ALLOW(PRIV_INIT_DONE);
+       }
+       allowed_ext[STATE_FILTER] |= ALLOW(PRIV_INIT_DONE);
        if (!nflag) {
                allowed_ext[STATE_RUN] |= ALLOW(PRIV_GETHOSTBYADDR);
                allowed_ext[STATE_FILTER] |= ALLOW(PRIV_ETHER_NTOHOST);
@@ -294,7 +303,7 @@ priv_exec(int argc, char *argv[])
                        impl_open_pfosfp(sock);
                        break;
                case PRIV_OPEN_OUTPUT:
-                       test_state(cmd, STATE_RUN);
+                       test_state(cmd, STATE_FILTER);
                        impl_open_output(sock, WFileName);
                        break;
                case PRIV_SETFILTER:
@@ -305,6 +314,7 @@ priv_exec(int argc, char *argv[])
                        test_state(cmd, STATE_RUN);
                        impl_init_done(sock, &bpfd);
 
+                       drop_privs(1);
                        if (unveil("/etc/ethers", "r") == -1)
                                err(1, "unveil");
                        if (unveil("/etc/rpc", "r") == -1)
Index: privsep_pcap.c
===================================================================
RCS file: /cvs/src/usr.sbin/tcpdump/privsep_pcap.c,v
retrieving revision 1.22
diff -u -p -u -r1.22 privsep_pcap.c
--- usr.sbin/tcpdump/privsep_pcap.c     19 Apr 2017 05:36:13 -0000      1.22
+++ usr.sbin/tcpdump/privsep_pcap.c     16 Nov 2018 17:27:37 -0000
@@ -479,10 +479,9 @@ priv_pcap_dump_open(pcap_t *p, char *fna
        if (priv_fd < 0)
                errx(1, "%s: called from privileged portion", __func__);
 
-       if (fname[0] == '-' && fname[1] == '\0') {
+       if (fname[0] == '-' && fname[1] == '\0')
                f = stdout;
-               priv_init_done();
-       } else {
+       else {
                write_command(priv_fd, PRIV_OPEN_OUTPUT);
                fd = receive_fd(priv_fd);
                must_read(priv_fd, &err, sizeof(err));
@@ -500,6 +499,7 @@ priv_pcap_dump_open(pcap_t *p, char *fna
                        return (NULL);
                }
        }
+       priv_init_done();
 
        (void)sf_write_header(f, p->linktype, p->tzoff, p->snapshot);
        return ((pcap_dumper_t *)f);

Reply via email to