Hi tech@
I've commited this, please test it as much as possible by applying the diff
right now or just wait for the next snapshot.
Let it run for a long time and let me know of any problems as soon as you get
any. For this your ktrace, pcap and coredump files will be VERY important for
further analysis of the issue. As noted in sysctl(8)'s man in order to generate
the dumps in a safe place please do the following:
# mkdir -m 700 /var/crash/tcpdump
# sysctl kern.nosuidcoredump=3
If someone has weird networks then even better for testing it since mine is
pretty much vanilla and might not have been enough to see flaws with the diff.
I can't stress this enough, please test it and report it ASAP either here or in
private.
On 18:08 Wed 26 Sep , Ricardo Mestre wrote:
> I'm not too worried about the crash, I was playing with removing rpath
> from pledge in the case that /etc/ethers wasn't need which sure enough
> it is. pledge was most likely the reason that made it crash.
>
> But brynet@ pointed out that while he was working on tcpdump last year
> he saw that it also needs to open /etc/rpc, and yes it calls
> getrpcbynumber(3) so an updated diff is below.
>
> Index: privsep.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/tcpdump/privsep.c,v
> retrieving revision 1.48
> diff -u -p -u -r1.48 privsep.c
> --- privsep.c 8 Aug 2018 22:57:12 -0000 1.48
> +++ privsep.c 26 Sep 2018 17:04:25 -0000
> @@ -207,7 +207,7 @@ __dead void
> priv_exec(int argc, char *argv[])
> {
> int bpfd = -1;
> - int i, sock, cmd, nflag = 0, Pflag = 0;
> + int i, sock, cmd, nflag = 0, oflag = 0, Pflag = 0;
> char *cmdbuf, *infile = NULL;
> char *RFileName = NULL;
> char *WFileName = NULL;
> @@ -229,6 +229,10 @@ priv_exec(int argc, char *argv[])
> nflag++;
> break;
>
> + case 'o':
> + oflag = 1;
> + break;
> +
> case 'r':
> RFileName = optarg;
> break;
> @@ -305,6 +309,14 @@ priv_exec(int argc, char *argv[])
> test_state(cmd, STATE_RUN);
> impl_init_done(sock, &bpfd);
>
> + if (oflag) {
> + if (unveil("/etc/pf.os", "r") == -1)
> + err(1, "unveil");
> + }
> + if (unveil("/etc/ethers", "r") == -1)
> + err(1, "unveil");
> + if (unveil("/etc/rpc", "r") == -1)
> + err(1, "unveil");
> if (pledge("stdio rpath inet dns recvfd bpf", NULL) ==
> -1)
> err(1, "pledge");
>
>
> On 08:58 Wed 26 Sep , Theo de Raadt wrote:
> > Ricardo Mestre <ser...@helheim.mooo.com> wrote:
> >
> > > Hi,
> > >
> > > This has been shown internally for some time, but deraadt@ asked me to
> > > show it
> > > to a bigger audience now so here it is!
> > >
> > > If we want OS fingerprinting by using -o flag then we can unveil
> > > /etc/pf.os in
> > > read mode, nevertheless in order to do this we need to inform the privsep
> > > proc
> > > that we are using -o so I added it to priv_exec().
> >
> > looks right
> >
> > > The other file needed to be unveiled is /etc/ethers in read mode, which I
> > > tried
> > > to make it conditional but after several successful tests I bumped into a
> > > packet which made tcpdump crash after some time. Unfortunately I don't
> > > have the
> > > core nor the pcap files to investigate what happen so for now the unveil
> > > of
> > > this file will be kept unconditional regardless of the flags or expression
> > > used.
> >
> > It is very likely that a protocol parser will find a MAC address nested
> > inside, and try to parse it via /etc/ethers. So makes sense, and there
> > is little risk in exposing ethers
> >
> > There is far more risk that some other file is required, and will fail to
> > open silently, and tcpdump willbehave differently
> >
> > You know well: unveil requires a different audit approach than pledge
> >
> > But it is rare for a missing file via unveil to crash a program, so
> > something
> > is fishy. A crash isn't good, perhaps try to reproduce and collect a
> > corefile
> > using the sysctl kern.nosuidcoredump=3 method.
> >
>
