unveil in process accounting and lastcomm

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

unveil in process accounting and lastcomm

Alexander Bluhm
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.

Also we keep record about programs that may be exploited and do
something illegal.  We have the same mechanism for pledge(2).

Not sure if we want it for both EACCES and ENOENT cases.  If it
creates false positives, we can change that later to EACCES only.

ok?

bluhm

Index: kern/kern_unveil.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_unveil.c,v
retrieving revision 1.27
diff -u -p -r1.27 kern_unveil.c
--- kern/kern_unveil.c 14 Jul 2019 03:26:02 -0000 1.27
+++ kern/kern_unveil.c 18 Jul 2019 12:01:24 -0000
@@ -18,6 +18,7 @@

 #include <sys/param.h>

+#include <sys/acct.h>
 #include <sys/mount.h>
 #include <sys/filedesc.h>
 #include <sys/proc.h>
@@ -823,6 +824,7 @@ unveil_check_final(struct proc *p, struc
     " vnode %p\n",
     p->p_p->ps_comm, p->p_p->ps_pid, ni->ni_vp);
 #endif
+ p->p_p->ps_acflag |= AUNVEIL;
  if (uv->uv_flags & UNVEIL_USERSET)
  return EACCES;
  else
@@ -865,10 +867,11 @@ unveil_check_final(struct proc *p, struc
  * EACCESS. Otherwise, use any covering match
  * that we found above this dir.
  */
- if (uv->uv_flags & UNVEIL_USERSET)
+ if (uv->uv_flags & UNVEIL_USERSET) {
+ p->p_p->ps_acflag |= AUNVEIL;
  return EACCES;
- else
- goto done;
+ }
+ goto done;
  }
  /* directory flags match, update match */
  if (uv->uv_flags & UNVEIL_USERSET)
@@ -881,6 +884,7 @@ unveil_check_final(struct proc *p, struc
  printf("unveil: %s(%d) flag mismatch for terminal '%s'\n",
     p->p_p->ps_comm, p->p_p->ps_pid, tname->un_name);
 #endif
+ p->p_p->ps_acflag |= AUNVEIL;
  return EACCES;
  }
  /* name and flags match in this dir. update match*/
@@ -903,8 +907,10 @@ done:
     p->p_p->ps_comm, p->p_p->ps_pid, ni->ni_cnd.cn_nameptr,
     ni->ni_unveil_match->uv_vp);
 #endif
+ p->p_p->ps_acflag |= AUNVEIL;
  return EACCES;
  }
+ p->p_p->ps_acflag |= AUNVEIL;
  return ENOENT;
 }

Index: sys/acct.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/sys/acct.h,v
retrieving revision 1.7
diff -u -p -r1.7 acct.h
--- sys/acct.h 8 Jun 2017 17:14:02 -0000 1.7
+++ sys/acct.h 18 Jul 2019 11:37:27 -0000
@@ -63,6 +63,7 @@ struct acct {
 #define AXSIG 0x10 /* killed by a signal */
 #define APLEDGE 0x20 /* killed due to pledge violation */
 #define ATRAP 0x40 /* memory access violation */
+#define AUNVEIL 0x80 /* unveil access violation */
  u_int8_t  ac_flag; /* accounting flags */
 };

Reply | Threaded
Open this post in threaded view
|

Re: unveil in process accounting and lastcomm

Bryan Steele-2
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.

> Also we keep record about programs that may be exploited and do
> something illegal.  We have the same mechanism for pledge(2).
>
> Not sure if we want it for both EACCES and ENOENT cases.  If it
> creates false positives, we can change that later to EACCES only.

With all that said, I do feel like false positives are incredibly
likely for unveil. For example, I suspect in ports you'll find
software that checks for Linux-isms (/proc, /sys) and and then
fallback. Maybe just EACCES? I don't know.

> ok?
>
> bluhm
>
> Index: kern/kern_unveil.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_unveil.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 kern_unveil.c
> --- kern/kern_unveil.c 14 Jul 2019 03:26:02 -0000 1.27
> +++ kern/kern_unveil.c 18 Jul 2019 12:01:24 -0000
> @@ -18,6 +18,7 @@
>
>  #include <sys/param.h>
>
> +#include <sys/acct.h>
>  #include <sys/mount.h>
>  #include <sys/filedesc.h>
>  #include <sys/proc.h>
> @@ -823,6 +824,7 @@ unveil_check_final(struct proc *p, struc
>      " vnode %p\n",
>      p->p_p->ps_comm, p->p_p->ps_pid, ni->ni_vp);
>  #endif
> + p->p_p->ps_acflag |= AUNVEIL;
>   if (uv->uv_flags & UNVEIL_USERSET)
>   return EACCES;
>   else
> @@ -865,10 +867,11 @@ unveil_check_final(struct proc *p, struc
>   * EACCESS. Otherwise, use any covering match
>   * that we found above this dir.
>   */
> - if (uv->uv_flags & UNVEIL_USERSET)
> + if (uv->uv_flags & UNVEIL_USERSET) {
> + p->p_p->ps_acflag |= AUNVEIL;
>   return EACCES;
> - else
> - goto done;
> + }
> + goto done;
>   }
>   /* directory flags match, update match */
>   if (uv->uv_flags & UNVEIL_USERSET)
> @@ -881,6 +884,7 @@ unveil_check_final(struct proc *p, struc
>   printf("unveil: %s(%d) flag mismatch for terminal '%s'\n",
>      p->p_p->ps_comm, p->p_p->ps_pid, tname->un_name);
>  #endif
> + p->p_p->ps_acflag |= AUNVEIL;
>   return EACCES;
>   }
>   /* name and flags match in this dir. update match*/
> @@ -903,8 +907,10 @@ done:
>      p->p_p->ps_comm, p->p_p->ps_pid, ni->ni_cnd.cn_nameptr,
>      ni->ni_unveil_match->uv_vp);
>  #endif
> + p->p_p->ps_acflag |= AUNVEIL;
>   return EACCES;
>   }
> + p->p_p->ps_acflag |= AUNVEIL;
>   return ENOENT;
>  }
>
> Index: sys/acct.h
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/sys/acct.h,v
> retrieving revision 1.7
> diff -u -p -r1.7 acct.h
> --- sys/acct.h 8 Jun 2017 17:14:02 -0000 1.7
> +++ sys/acct.h 18 Jul 2019 11:37:27 -0000
> @@ -63,6 +63,7 @@ struct acct {
>  #define AXSIG 0x10 /* killed by a signal */
>  #define APLEDGE 0x20 /* killed due to pledge violation */
>  #define ATRAP 0x40 /* memory access violation */
> +#define AUNVEIL 0x80 /* unveil access violation */
>   u_int8_t  ac_flag; /* accounting flags */
>  };
>
>

Reply | Threaded
Open this post in threaded view
|

Re: unveil in process accounting and lastcomm

Bryan Steele-2
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?

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 18 Jul 2019 21:30:39 -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 *pcap_live(const char *, int, int, int, char *);
 void  logmsg(int, const char *, ...);
 void  purge_buffer(void);
 int   reset_dump(void);
@@ -194,10 +195,97 @@ if_exists(char *ifname)
  return (if_nametoindex(ifname) != 0);
 }
 
+pcap_t *
+pcap_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) < 0) {
+ 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) < 0) {
+ 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:
+ if (fd >= 0)
+ close(fd);
+ free(p);
+ return (NULL);
+}
+
 int
 init_pcap(void)
 {
- hpcap = pcap_open_live(interface, snaplen, 1, PCAP_TO_MS, errbuf);
+ hpcap = pcap_live(interface, snaplen, 1, PCAP_TO_MS, errbuf);
  if (hpcap == NULL) {
  logmsg(LOG_ERR, "Failed to initialize: %s", errbuf);
  return (-1);
Index: 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 18 Jul 2019 21:37:48 -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.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 18 Jul 2019 21:37:48 -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,93 @@ sighandler_close(int signal)
  pcap_breakloop(hpcap); /* sighdlr safe */
 }
 
+pcap_t *
+pcap_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) < 0) {
+ 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) < 0) {
+ 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:
+ if (fd >= 0)
+ close(fd);
+ free(p);
+ return (NULL);
+}
+
 int
 init_pcap(void)
 {
@@ -114,7 +202,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 = pcap_live(pflogif, PCAPSNAP, 1, PCAPTIMO,
     errbuf)) == NULL) {
  logmsg(LOG_ERR, "Failed to initialize: %s", errbuf);
  return (-1);

Reply | Threaded
Open this post in threaded view
|

Re: unveil in process accounting and lastcomm

Alexander Bluhm
In reply to this post by Alexander Bluhm
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.

Could I put that in?  Process accounting is cheap and does not hurt.

I have added it localy to my daily mail like pledge.  Then I will
notice how many bugs or false positives we have.

bluhm

> $ 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.
>
> Also we keep record about programs that may be exploited and do
> something illegal.  We have the same mechanism for pledge(2).
>
> Not sure if we want it for both EACCES and ENOENT cases.  If it
> creates false positives, we can change that later to EACCES only.
>
> ok?
>
> bluhm
>
> Index: kern/kern_unveil.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_unveil.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 kern_unveil.c
> --- kern/kern_unveil.c 14 Jul 2019 03:26:02 -0000 1.27
> +++ kern/kern_unveil.c 18 Jul 2019 12:01:24 -0000
> @@ -18,6 +18,7 @@
>
>  #include <sys/param.h>
>
> +#include <sys/acct.h>
>  #include <sys/mount.h>
>  #include <sys/filedesc.h>
>  #include <sys/proc.h>
> @@ -823,6 +824,7 @@ unveil_check_final(struct proc *p, struc
>      " vnode %p\n",
>      p->p_p->ps_comm, p->p_p->ps_pid, ni->ni_vp);
>  #endif
> + p->p_p->ps_acflag |= AUNVEIL;
>   if (uv->uv_flags & UNVEIL_USERSET)
>   return EACCES;
>   else
> @@ -865,10 +867,11 @@ unveil_check_final(struct proc *p, struc
>   * EACCESS. Otherwise, use any covering match
>   * that we found above this dir.
>   */
> - if (uv->uv_flags & UNVEIL_USERSET)
> + if (uv->uv_flags & UNVEIL_USERSET) {
> + p->p_p->ps_acflag |= AUNVEIL;
>   return EACCES;
> - else
> - goto done;
> + }
> + goto done;
>   }
>   /* directory flags match, update match */
>   if (uv->uv_flags & UNVEIL_USERSET)
> @@ -881,6 +884,7 @@ unveil_check_final(struct proc *p, struc
>   printf("unveil: %s(%d) flag mismatch for terminal '%s'\n",
>      p->p_p->ps_comm, p->p_p->ps_pid, tname->un_name);
>  #endif
> + p->p_p->ps_acflag |= AUNVEIL;
>   return EACCES;
>   }
>   /* name and flags match in this dir. update match*/
> @@ -903,8 +907,10 @@ done:
>      p->p_p->ps_comm, p->p_p->ps_pid, ni->ni_cnd.cn_nameptr,
>      ni->ni_unveil_match->uv_vp);
>  #endif
> + p->p_p->ps_acflag |= AUNVEIL;
>   return EACCES;
>   }
> + p->p_p->ps_acflag |= AUNVEIL;
>   return ENOENT;
>  }
>
> Index: sys/acct.h
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/sys/acct.h,v
> retrieving revision 1.7
> diff -u -p -r1.7 acct.h
> --- sys/acct.h 8 Jun 2017 17:14:02 -0000 1.7
> +++ sys/acct.h 18 Jul 2019 11:37:27 -0000
> @@ -63,6 +63,7 @@ struct acct {
>  #define AXSIG 0x10 /* killed by a signal */
>  #define APLEDGE 0x20 /* killed due to pledge violation */
>  #define ATRAP 0x40 /* memory access violation */
> +#define AUNVEIL 0x80 /* unveil access violation */
>   u_int8_t  ac_flag; /* accounting flags */
>  };

Reply | Threaded
Open this post in threaded view
|

Re: unveil in process accounting and lastcomm

Theo de Raadt-2
I have worried about secret unveil failures, and I am happy with
this approach.

Alexander Bluhm <[hidden email]> 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.
>
> Could I put that in?  Process accounting is cheap and does not hurt.
>
> I have added it localy to my daily mail like pledge.  Then I will
> notice how many bugs or false positives we have.
>
> bluhm
>
> > $ 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.
> >
> > Also we keep record about programs that may be exploited and do
> > something illegal.  We have the same mechanism for pledge(2).
> >
> > Not sure if we want it for both EACCES and ENOENT cases.  If it
> > creates false positives, we can change that later to EACCES only.
> >
> > ok?
> >
> > bluhm
> >
> > Index: kern/kern_unveil.c
> > ===================================================================
> > RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_unveil.c,v
> > retrieving revision 1.27
> > diff -u -p -r1.27 kern_unveil.c
> > --- kern/kern_unveil.c 14 Jul 2019 03:26:02 -0000 1.27
> > +++ kern/kern_unveil.c 18 Jul 2019 12:01:24 -0000
> > @@ -18,6 +18,7 @@
> >
> >  #include <sys/param.h>
> >
> > +#include <sys/acct.h>
> >  #include <sys/mount.h>
> >  #include <sys/filedesc.h>
> >  #include <sys/proc.h>
> > @@ -823,6 +824,7 @@ unveil_check_final(struct proc *p, struc
> >      " vnode %p\n",
> >      p->p_p->ps_comm, p->p_p->ps_pid, ni->ni_vp);
> >  #endif
> > + p->p_p->ps_acflag |= AUNVEIL;
> >   if (uv->uv_flags & UNVEIL_USERSET)
> >   return EACCES;
> >   else
> > @@ -865,10 +867,11 @@ unveil_check_final(struct proc *p, struc
> >   * EACCESS. Otherwise, use any covering match
> >   * that we found above this dir.
> >   */
> > - if (uv->uv_flags & UNVEIL_USERSET)
> > + if (uv->uv_flags & UNVEIL_USERSET) {
> > + p->p_p->ps_acflag |= AUNVEIL;
> >   return EACCES;
> > - else
> > - goto done;
> > + }
> > + goto done;
> >   }
> >   /* directory flags match, update match */
> >   if (uv->uv_flags & UNVEIL_USERSET)
> > @@ -881,6 +884,7 @@ unveil_check_final(struct proc *p, struc
> >   printf("unveil: %s(%d) flag mismatch for terminal '%s'\n",
> >      p->p_p->ps_comm, p->p_p->ps_pid, tname->un_name);
> >  #endif
> > + p->p_p->ps_acflag |= AUNVEIL;
> >   return EACCES;
> >   }
> >   /* name and flags match in this dir. update match*/
> > @@ -903,8 +907,10 @@ done:
> >      p->p_p->ps_comm, p->p_p->ps_pid, ni->ni_cnd.cn_nameptr,
> >      ni->ni_unveil_match->uv_vp);
> >  #endif
> > + p->p_p->ps_acflag |= AUNVEIL;
> >   return EACCES;
> >   }
> > + p->p_p->ps_acflag |= AUNVEIL;
> >   return ENOENT;
> >  }
> >
> > Index: sys/acct.h
> > ===================================================================
> > RCS file: /data/mirror/openbsd/cvs/src/sys/sys/acct.h,v
> > retrieving revision 1.7
> > diff -u -p -r1.7 acct.h
> > --- sys/acct.h 8 Jun 2017 17:14:02 -0000 1.7
> > +++ sys/acct.h 18 Jul 2019 11:37:27 -0000
> > @@ -63,6 +63,7 @@ struct acct {
> >  #define AXSIG 0x10 /* killed by a signal */
> >  #define APLEDGE 0x20 /* killed due to pledge violation */
> >  #define ATRAP 0x40 /* memory access violation */
> > +#define AUNVEIL 0x80 /* unveil access violation */
> >   u_int8_t  ac_flag; /* accounting flags */
> >  };
>

Reply | Threaded
Open this post in threaded view
|

Re: unveil in process accounting and lastcomm

Alexander Bluhm
In reply to this post by Alexander Bluhm
On Thu, Jul 18, 2019 at 04:13:10PM +0200, Alexander Bluhm wrote:
> $ lastcomm | grep -e '-[A-Z]U'
> pflogd     -FU     root    __         0.00 secs Thu Jul 18 14:19 (2:33:22.00)

Oops, I have forgotten to show the userland part of my diff.

Do we want unveil violators in the daily mail?  We can turn it off
if we get too many false positives.

ok?

bluhm

Index: etc/daily
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/etc/daily,v
retrieving revision 1.91
diff -u -p -r1.91 daily
--- etc/daily 6 Feb 2018 19:57:37 -0000 1.91
+++ etc/daily 25 Jul 2019 09:56:20 -0000
@@ -74,7 +74,7 @@ if [ -f /var/account/acct ]; then
  mv -f /var/account/acct.0 /var/account/acct.1
  cp -f /var/account/acct /var/account/acct.0
  sa -sq
- lastcomm -f /var/account/acct.0 | grep -e ' -[A-Z]*[PT]'
+ lastcomm -f /var/account/acct.0 | grep -e ' -[A-Z]*[PTU]'
 fi

 # If ROOTBACKUP is set to 1 in the environment, and
Index: usr.bin/lastcomm/lastcomm.1
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.bin/lastcomm/lastcomm.1,v
retrieving revision 1.19
diff -u -p -r1.19 lastcomm.1
--- usr.bin/lastcomm/lastcomm.1 27 Feb 2018 07:58:29 -0000 1.19
+++ usr.bin/lastcomm/lastcomm.1 25 Jul 2019 09:42:15 -0000
@@ -115,10 +115,13 @@ indicates the command was terminated wit
 .Sq P
 indicates the command was terminated due to a
 .Xr pledge 2
-violation, and
+violation,
 .Sq T
 indicates the command did a memory access violation detected by a
-processor trap.
+processor trap, and
+.Sq U
+indicates the command tried a file access that was prevented by
+.Xr unveil 2 .
 .Sh FILES
 .Bl -tag -width /var/account/acct -compact
 .It Pa /var/account/acct
Index: usr.bin/lastcomm/lastcomm.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.bin/lastcomm/lastcomm.c,v
retrieving revision 1.27
diff -u -p -r1.27 lastcomm.c
--- usr.bin/lastcomm/lastcomm.c 27 Feb 2018 07:58:29 -0000 1.27
+++ usr.bin/lastcomm/lastcomm.c 25 Jul 2019 09:41:34 -0000
@@ -174,6 +174,7 @@ flagbits(int f)
  BIT(AXSIG, 'X');
  BIT(APLEDGE, 'P');
  BIT(ATRAP, 'T');
+ BIT(AUNVEIL, 'U');
  *p = '\0';
  return (flags);
 }

Reply | Threaded
Open this post in threaded view
|

Re: unveil in process accounting and lastcomm

Alexander Bluhm
On Thu, Jul 25, 2019 at 12:00:48PM +0200, Alexander Bluhm wrote:
> Do we want unveil violators in the daily mail?  We can turn it off
> if we get too many false positives.

Janne Johansson recommend to mention lastcomm(1) in unveil(2) man
page.  Diff for daily, lastcomm(1), unveil(2).  Kernel has been
commited already.

bluhm

Index: etc/daily
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/etc/daily,v
retrieving revision 1.91
diff -u -p -r1.91 daily
--- etc/daily 6 Feb 2018 19:57:37 -0000 1.91
+++ etc/daily 25 Jul 2019 09:56:20 -0000
@@ -74,7 +74,7 @@ if [ -f /var/account/acct ]; then
  mv -f /var/account/acct.0 /var/account/acct.1
  cp -f /var/account/acct /var/account/acct.0
  sa -sq
- lastcomm -f /var/account/acct.0 | grep -e ' -[A-Z]*[PT]'
+ lastcomm -f /var/account/acct.0 | grep -e ' -[A-Z]*[PTU]'
 fi

 # If ROOTBACKUP is set to 1 in the environment, and
Index: usr.bin/lastcomm/lastcomm.1
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.bin/lastcomm/lastcomm.1,v
retrieving revision 1.19
diff -u -p -r1.19 lastcomm.1
--- usr.bin/lastcomm/lastcomm.1 27 Feb 2018 07:58:29 -0000 1.19
+++ usr.bin/lastcomm/lastcomm.1 25 Jul 2019 09:42:15 -0000
@@ -115,10 +115,13 @@ indicates the command was terminated wit
 .Sq P
 indicates the command was terminated due to a
 .Xr pledge 2
-violation, and
+violation,
 .Sq T
 indicates the command did a memory access violation detected by a
-processor trap.
+processor trap, and
+.Sq U
+indicates the command tried a file access that was prevented by
+.Xr unveil 2 .
 .Sh FILES
 .Bl -tag -width /var/account/acct -compact
 .It Pa /var/account/acct
Index: usr.bin/lastcomm/lastcomm.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.bin/lastcomm/lastcomm.c,v
retrieving revision 1.27
diff -u -p -r1.27 lastcomm.c
--- usr.bin/lastcomm/lastcomm.c 27 Feb 2018 07:58:29 -0000 1.27
+++ usr.bin/lastcomm/lastcomm.c 25 Jul 2019 09:41:34 -0000
@@ -174,6 +174,7 @@ flagbits(int f)
  BIT(AXSIG, 'X');
  BIT(APLEDGE, 'P');
  BIT(ATRAP, 'T');
+ BIT(AUNVEIL, 'U');
  *p = '\0';
  return (flags);
 }
Index: lib/libc/sys/unveil.2
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/lib/libc/sys/unveil.2,v
retrieving revision 1.17
diff -u -p -r1.17 unveil.2
--- lib/libc/sys/unveil.2 24 Mar 2019 19:55:31 -0000 1.17
+++ lib/libc/sys/unveil.2 25 Jul 2019 11:12:15 -0000
@@ -132,6 +132,12 @@ use can be tricky because programs misbe
 unexpectedly disappear.
 In many cases it is easier to unveil the directories in which an
 application makes use of files.
+After a process has terminated,
+.Xr lastcomm 1
+will mark it with the
+.Sq U
+flag if file access was prevented by
+.Nm unveil .
 .Sh RETURN VALUES
 .Rv -std
 .Sh ERRORS

Reply | Threaded
Open this post in threaded view
|

Re: unveil in process accounting and lastcomm

Todd C. Miller-3
In reply to this post by Alexander Bluhm
On Thu, 25 Jul 2019 12:00:48 +0200, Alexander Bluhm wrote:

> Do we want unveil violators in the daily mail?  We can turn it off
> if we get too many false positives.

I think so.  If it becomes annoying we can turn it off by default.

 - todd

Reply | Threaded
Open this post in threaded view
|

Re: unveil in process accounting and lastcomm

Theo de Raadt-2
Todd C. Miller <[hidden email]> wrote:

> On Thu, 25 Jul 2019 12:00:48 +0200, Alexander Bluhm wrote:
>
> > Do we want unveil violators in the daily mail?  We can turn it off
> > if we get too many false positives.
>
> I think so.  If it becomes annoying we can turn it off by default.

I'm not sure how any of these could be false positives.  They will
all show that the software is trying to access something it shouldn't.

Reply | Threaded
Open this post in threaded view
|

Re: unveil in process accounting and lastcomm

Bryan Steele-2
In reply to this post by Bryan Steele-2
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.

I'd appreciate if someone could test spamlogd(8) still works.

-Bryan.

Index: pflogd.c
===================================================================
RCS file: /cvs/src/sbin/pflogd/pflogd.c,v
retrieving revision 1.59
diff -u -p -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:00:02 -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 *pcap_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 *
+pcap_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 = pcap_live(interface, snaplen, 1, PCAP_TO_MS, errbuf);
  if (hpcap == NULL) {
  logmsg(LOG_ERR, "Failed to initialize: %s", errbuf);
  return (-1);
Index: 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:01:25 -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.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:01:25 -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 *
+pcap_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 = pcap_live(pflogif, PCAPSNAP, 1, PCAPTIMO,
     errbuf)) == NULL) {
  logmsg(LOG_ERR, "Failed to initialize: %s", errbuf);
  return (-1);

Reply | Threaded
Open this post in threaded view
|

Re: unveil in process accounting and lastcomm

Bryan Steele-2
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);