On Thu, Jul 25, 2019 at 10:06:52AM -0400, Bryan Steele wrote:
> On Thu, Jul 18, 2019 at 05:44:21PM -0400, Bryan Steele wrote:
> > On Thu, Jul 18, 2019 at 11:46:46AM -0400, Bryan Steele wrote:
> > > On Thu, Jul 18, 2019 at 04:13:10PM +0200, Alexander Bluhm wrote:
> > > > Hi,
> > > >
> > > > Can we track unveil(2) violators in process accounting lastcomm(1)?
> > > > This makes it easier to find them.
> > > >
> > > > $ lastcomm | grep -e '-[A-Z]U'
> > > > pflogd -FU root __ 0.00 secs Thu Jul 18 14:19
> > > > (2:33:22.00)
> > > >
> > > > Seems that pflogd(8) has to be investigated.
> > >
> > > Interesting.
> > >
> > > This appears to be a false positive, libpcap will first attempt to open
> > > /dev/bpf as O_RDWR and then falls back to O_RDONLY on EACCES. I'm fairly
> > > confident that pflogd(8) does not need write access to bpf. If anything,
> > > unveil(2) appears to have found an old bug here, as before pflogd was
> > > always opening the device with both read/write permissions.
> > >
> > > tcpdump avoids this by internalizing more parts of libpcap, and also
> > > opening /dev/bpf O_RDONLY itself.
> > >
> > > spamlogd appears to be the only other user of pcap_open_live() in base,
> > > unfortunately it calls unveil after, so /dev/bpf is opened O_RDWR. I
> > > don't use spamlogd, though.
> >
> > Here's a potential diff for both pflogd and spamlogd, I've tested it
> > works with pflogd. I only compile tested spamlogd. But it should avoid
> > the unveil accounting errors.
> >
> > This duplicates a lot of code into both, but I don't feel like trying
> > to extent the libpcap API for this.
> >
> > comments? ok?
>
> With bluhm@'s unveil accounting diff, is anyone ok wit this this
> approach to fixing the issue? These are the only pcap_open_live(3)
> consumers in base, but I don't feel comfortable trying to extend
> the libpcap API, and pretty much everything is already poking at
> libpcap internals due to bad API design for privsep.
>
> -Bryan.
Here's a new diff that renames the local function to avoid conflicting
with future attempts at improving the libpcap API. Requested by deraadt@
-Bryan.
Index: pflogd.c
===================================================================
RCS file: /cvs/src/sbin/pflogd/pflogd.c,v
retrieving revision 1.59
diff -u -p -u -r1.59 pflogd.c
--- sbin/pflogd/pflogd.c 26 Aug 2018 18:24:46 -0000 1.59
+++ sbin/pflogd/pflogd.c 25 Jul 2019 14:23:14 -0000
@@ -73,6 +73,7 @@ void dump_packet(u_char *, const struct
void dump_packet_nobuf(u_char *, const struct pcap_pkthdr *, const u_char *);
int flush_buffer(FILE *);
int if_exists(char *);
+pcap_t *pflog_read_live(const char *, int, int, int, char *);
void logmsg(int, const char *, ...);
void purge_buffer(void);
int reset_dump(void);
@@ -194,10 +195,95 @@ if_exists(char *ifname)
return (if_nametoindex(ifname) != 0);
}
+pcap_t *
+pflog_read_live(const char *source, int slen, int promisc, int to_ms,
+ char *ebuf)
+{
+ int fd;
+ struct bpf_version bv;
+ struct ifreq ifr;
+ u_int v, dlt = DLT_PFLOG;
+ pcap_t *p;
+
+ if (source == NULL || slen <= 0)
+ return (NULL);
+
+ p = pcap_create(source, ebuf);
+ if (p == NULL)
+ return (NULL);
+
+ /* Open bpf(4) read only */
+ if ((fd = open("/dev/bpf", O_RDONLY)) == -1)
+ return (NULL);
+
+ if (ioctl(fd, BIOCVERSION, &bv) == -1) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE, "BIOCVERSION: %s",
+ pcap_strerror(errno));
+ goto bad;
+ }
+
+ if (bv.bv_major != BPF_MAJOR_VERSION ||
+ bv.bv_minor < BPF_MINOR_VERSION) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE,
+ "kernel bpf filter out of date");
+ goto bad;
+ }
+
+ strlcpy(ifr.ifr_name, source, sizeof(ifr.ifr_name));
+ if (ioctl(fd, BIOCSETIF, &ifr) == -1) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE, "BIOCSETIF: %s",
+ pcap_strerror(errno));
+ goto bad;
+ }
+
+ if (dlt != (u_int) -1 && ioctl(fd, BIOCSDLT, &dlt)) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE, "BIOCSDLT: %s",
+ pcap_strerror(errno));
+ goto bad;
+ }
+
+ p->fd = fd;
+ p->snapshot = slen;
+ p->linktype = dlt;
+
+ /* set timeout */
+ if (to_ms != 0) {
+ struct timeval to;
+ to.tv_sec = to_ms / 1000;
+ to.tv_usec = (to_ms * 1000) % 1000000;
+ if (ioctl(p->fd, BIOCSRTIMEOUT, &to) == -1) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE, "BIOCSRTIMEOUT: %s",
+ pcap_strerror(errno));
+ }
+ }
+ if (promisc)
+ /* this is allowed to fail */
+ ioctl(fd, BIOCPROMISC, NULL);
+
+ if (ioctl(fd, BIOCGBLEN, &v) == -1) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE, "BIOCGBLEN: %s",
+ pcap_strerror(errno));
+ goto bad;
+ }
+ p->bufsize = v;
+ p->buffer = malloc(p->bufsize);
+ if (p->buffer == NULL) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE, "malloc: %s",
+ pcap_strerror(errno));
+ goto bad;
+ }
+ p->activated = 1;
+ return (p);
+
+bad:
+ pcap_close(p);
+ return (NULL);
+}
+
int
init_pcap(void)
{
- hpcap = pcap_open_live(interface, snaplen, 1, PCAP_TO_MS, errbuf);
+ hpcap = pflog_read_live(interface, snaplen, 1, PCAP_TO_MS, errbuf);
if (hpcap == NULL) {
logmsg(LOG_ERR, "Failed to initialize: %s", errbuf);
return (-1);
Index: spamlogd/Makefile
===================================================================
RCS file: /cvs/src/libexec/spamlogd/Makefile,v
retrieving revision 1.8
diff -u -p -u -r1.8 Makefile
--- libexec/spamlogd/Makefile 28 Jun 2018 02:23:27 -0000 1.8
+++ libexec/spamlogd/Makefile 25 Jul 2019 14:21:57 -0000
@@ -5,6 +5,8 @@ SRCS= spamlogd.c sync.c gdcopy.c
MAN= spamlogd.8
CFLAGS+= -Wall -Wstrict-prototypes -I${.CURDIR}/../spamd
+# for pcap-int.h
+CFLAGS+=-I${.CURDIR}/../../lib/libpcap
LDADD+= -lpcap -lcrypto
DPADD+= ${LIBPCAP} ${LIBCRYPTO}
.PATH: ${.CURDIR}/../spamd
Index: spamlogd/spamlogd.c
===================================================================
RCS file: /cvs/src/libexec/spamlogd/spamlogd.c,v
retrieving revision 1.28
diff -u -p -u -r1.28 spamlogd.c
--- libexec/spamlogd/spamlogd.c 25 Oct 2018 06:41:50 -0000 1.28
+++ libexec/spamlogd/spamlogd.c 25 Jul 2019 14:21:57 -0000
@@ -49,6 +49,7 @@
#include <syslog.h>
#include <string.h>
#include <unistd.h>
+#include <pcap-int.h>
#include <pcap.h>
#include "grey.h"
@@ -107,6 +108,91 @@ sighandler_close(int signal)
pcap_breakloop(hpcap); /* sighdlr safe */
}
+pcap_t *
+pflog_read_live(const char *source, int slen, int promisc, int to_ms,
+ char *ebuf)
+{
+ int fd;
+ struct bpf_version bv;
+ struct ifreq ifr;
+ u_int v, dlt = DLT_PFLOG;
+ pcap_t *p;
+
+ if (source == NULL || slen <= 0)
+ return (NULL);
+
+ p = pcap_create(source, ebuf);
+ if (p == NULL)
+ return (NULL);
+
+ /* Open bpf(4) read only */
+ if ((fd = open("/dev/bpf", O_RDONLY)) == -1)
+ return (NULL);
+
+ if (ioctl(fd, BIOCVERSION, &bv) == -1) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE, "BIOCVERSION: %s",
+ pcap_strerror(errno));
+ goto bad;
+ }
+
+ if (bv.bv_major != BPF_MAJOR_VERSION ||
+ bv.bv_minor < BPF_MINOR_VERSION) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE,
+ "kernel bpf filter out of date");
+ goto bad;
+ }
+
+ strlcpy(ifr.ifr_name, source, sizeof(ifr.ifr_name));
+ if (ioctl(fd, BIOCSETIF, &ifr) == -1) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE, "BIOCSETIF: %s",
+ pcap_strerror(errno));
+ goto bad;
+ }
+
+ if (dlt != (u_int) -1 && ioctl(fd, BIOCSDLT, &dlt)) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE, "BIOCSDLT: %s",
+ pcap_strerror(errno));
+ goto bad;
+ }
+
+ p->fd = fd;
+ p->snapshot = slen;
+ p->linktype = dlt;
+
+ /* set timeout */
+ if (to_ms != 0) {
+ struct timeval to;
+ to.tv_sec = to_ms / 1000;
+ to.tv_usec = (to_ms * 1000) % 1000000;
+ if (ioctl(p->fd, BIOCSRTIMEOUT, &to) == -1) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE, "BIOCSRTIMEOUT: %s",
+ pcap_strerror(errno));
+ }
+ }
+ if (promisc)
+ /* this is allowed to fail */
+ ioctl(fd, BIOCPROMISC, NULL);
+
+ if (ioctl(fd, BIOCGBLEN, &v) == -1) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE, "BIOCGBLEN: %s",
+ pcap_strerror(errno));
+ goto bad;
+ }
+ p->bufsize = v;
+ p->buffer = malloc(p->bufsize);
+ if (p->buffer == NULL) {
+ snprintf(ebuf, PCAP_ERRBUF_SIZE, "malloc: %s",
+ pcap_strerror(errno));
+ goto bad;
+ }
+ p->activated = 1;
+ return (p);
+
+bad:
+ pcap_close(p);
+ return (NULL);
+}
+
int
init_pcap(void)
{
@@ -114,7 +200,7 @@ init_pcap(void)
char filter[PCAPFSIZ] = "ip and port 25 and action pass "
"and tcp[13]&0x12=0x2";
- if ((hpcap = pcap_open_live(pflogif, PCAPSNAP, 1, PCAPTIMO,
+ if ((hpcap = pflog_read_live(pflogif, PCAPSNAP, 1, PCAPTIMO,
errbuf)) == NULL) {
logmsg(LOG_ERR, "Failed to initialize: %s", errbuf);
return (-1);
