Re: pf userspace API changes
- In reply to: Kristof Provost : "pf userspace API changes"
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Mon, 28 Aug 2023 07:15:44 UTC
This is going to land soon, in 15. So 14 will keep the old ioctls, but
they’ll disappear in 15.
Best regards,
Kristof
On 6 Apr 2023, at 16:21, Kristof Provost wrote:
> Hi,
>
> Quick heads up that there are going to be breaking changes to the pf
> API towards userspace for 14.0. (That is, the ioctl interface
> presented by /dev/pf).
>
> I’ve been rewriting several types of calls to start using nvlists,
> because otherwise it’s basically unmanageable to extend calls (which
> we pretty much have to do to add new features).
> The old struct-based ioctls have been left in place so far, but as
> that’s a substantial amount of (now untested!) code I’m very keen
> to be able to remove.
>
> To be very explicit: removal of old ioctls will only ever affect new
> major versions. That is, 14.0 at the earliest. I will not break
> stable/12 or stable/13 or 13.2 or …
>
> The initial breaking change is https://reviews.freebsd.org/D30056 .
> That removes DIOCCLRSTATES and DIOCKILLSTATES, which are now
> DIOCCLRSTATESNV and DIOCKILLSTATESNV, based on nvlists.
>
> To make that all easier for userspace to manage there’s libpfctl,
> which wraps all of the API details. That port will be available for
> all supported platforms (when https://reviews.freebsd.org/D39360
> lands, soon).
>
> There are likely to be more changes in the future, so I’d strongly
> encourage all API users to migrate to using libpfctl rather than
> trying to roll their own implementations.
>
> Here’s an example of how security/snortsam needed to be changed to
> cope with the above:
>
> commit 1136cf1ef66dc93397455818dfce0794d4e65170 (HEAD ->
> freebsd_current/libpfctl)
> Author: Kristof Provost <kp@FreeBSD.org>
> Date: Sun Apr 2 07:01:06 2023 +0200
>
> security/snortsam: use libpfctl
>
> FreeBSD main will remove DIOCKILLSTATES soon. We can use
> libpfctl to
> accomplish the same task though.
>
> Sponsored by: Rubicon Communications, LLC ("Netgate")
>
> diff --git a/security/snortsam/Makefile
> b/security/snortsam/Makefile
> index fbd106774..18ad44448 100644
> --- a/security/snortsam/Makefile
> +++ b/security/snortsam/Makefile
> @@ -1,6 +1,6 @@
> PORTNAME= snortsam
> PORTVERSION= 2.70
> -PORTREVISION= 1
> +PORTREVISION= 2
> CATEGORIES= security
> MASTER_SITES= http://www.snortsam.net/files/snortsam/
> DISTNAME= ${PORTNAME}-src-${PORTVERSION}
> @@ -16,6 +16,8 @@ SAMTOOL_DESC= install samtool
>
> .include <bsd.port.pre.mk>
>
> +LIB_DEPENDS= libpfctl.so:net/libpfctl
> +
> USE_RC_SUBR= snortsam
> SUB_FILES= pkg-message \
> pkg-install
> diff --git a/security/snortsam/files/patch-src_Makefile
> b/security/snortsam/files/patch-src_Makefile
> new file mode 100644
> index 000000000..59936d3d1
> --- /dev/null
> +++ b/security/snortsam/files/patch-src_Makefile
> @@ -0,0 +1,12 @@
> +--- src/Makefile.orig 2010-03-29 20:57:55 UTC
> ++++ src/Makefile
> +@@ -36,9 +36,9 @@ SAMTOOL = samtool
> + PROG = snortsam
> + SAMTOOL = samtool
> +-CFLAGS = -O2 -D$(SYSTYPE) $(DEBUG)
> +-LDFLAGS =
> ++CFLAGS = -O2 -D$(SYSTYPE) $(DEBUG) -I/usr/local/include
> ++LDFLAGS = -L/usr/local/lib -lpfctl
> + SYSTYPE = `uname`
> +
> + # OS specific flags
> diff --git a/security/snortsam/files/patch-src__ssp_pf2.c
> b/security/snortsam/files/patch-src__ssp_pf2.c
> index 81ce7d93e..00327f19c 100644
> --- a/security/snortsam/files/patch-src__ssp_pf2.c
> +++ b/security/snortsam/files/patch-src__ssp_pf2.c
> @@ -1,6 +1,14 @@
> ---- ./src/ssp_pf2.c.orig 2009-11-27 02:39:40.000000000
> +0100
> -+++ ./src/ssp_pf2.c 2014-01-20 19:03:47.000000000 +0100
> -@@ -95,7 +95,7 @@
> +--- src/ssp_pf2.c.orig 2009-11-27 01:39:40 UTC
> ++++ src/ssp_pf2.c
> +@@ -48,6 +48,7 @@
> +
> + #include "snortsam.h"
> + #include "ssp_pf2.h"
> ++#include <libpfctl.h>
> +
> + unsigned int PF2use_anchor = TRUE;
> + unsigned int PF2val_count = 0;
> +@@ -95,7 +96,7 @@ int parse_opts(char *line, opt_pf2 *opt, char
> *sep, ch
> }
> }
>
> @@ -9,3 +17,79 @@
> }
>
>
> +@@ -393,20 +394,21 @@ pf2_kill_states(int pfdev, const char
> *ipsrc, int tin,
> + {
> + char msg[STRBUFSIZE + 2];
> + struct pf_addr pfa;
> +- struct pfioc_state_kill psk;
> ++ struct pfctl_kill k;
> + sa_family_t saf; /* stafe AF_INET family */
> + unsigned long killed=0, killed_src=0, killed_dst=0;
> ++ unsigned int kcount;
> +
> + bzero(&pfa, sizeof(pfa));
> +- bzero(&psk, sizeof(psk));
> ++ bzero(&k, sizeof(k));
> +
> + if (ipsrc == NULL || !ipsrc[0])
> + return (-1);
> +
> + if (inet_pton(AF_INET, ipsrc, &pfa.v4) == 1)
> +- psk.psk_af = saf = AF_INET;
> ++ k.af = AF_INET;
> + else if (inet_pton(AF_INET6, ipsrc, &pfa.v6) == 1)
> +- psk.psk_af = saf = AF_INET6;
> ++ k.af = AF_INET6;
> + else {
> + snprintf(msg, sizeof(msg) - 1, "invalid ipsrc");
> + logmessage(3, msg, "pf2", 0);
> +@@ -415,40 +417,31 @@ pf2_kill_states(int pfdev, const char
> *ipsrc, int tin,
> +
> + /* Kill all states from pfa */
> + if (tin || PF2_KILL_STATE_ALL) {
> +- memcpy(&psk.psk_src.addr.v.a.addr, &pfa,
> sizeof(psk.psk_src.addr.v.a.addr));
> +- memset(&psk.psk_src.addr.v.a.mask, 0xff,
> sizeof(psk.psk_src.addr.v.a.mask));
> +- if (ioctl(pfdev, DIOCKILLSTATES, &psk)) {
> ++ memcpy(&k.src.addr.v.a.addr, &pfa,
> sizeof(k.src.addr.v.a.addr));
> ++ memset(&k.src.addr.v.a.mask, 0xff,
> sizeof(k.src.addr.v.a.mask));
> ++ if (pfctl_kill_states(pfdev, &k, &kcount)) {
> + snprintf(msg, sizeof(msg) - 1, "Error: DIOCKILLSTATES
> failed (%s)", strerror(errno));
> + logmessage(1, msg, "pf2", 0);
> + }
> + else {
> +-#if OpenBSD >= 200811 /* since OpenBSD4_4 killed states returned
> in psk_killed */
> +- killed_src += psk.psk_killed;
> +-#else
> +- killed_src += psk.psk_af;
> +-#endif
> ++ killed_src += kcount;
> + #ifdef FWSAMDEBUG
> + printf("Debug: [pf2] killed %lu (tin) states for host
> %s\n", killed_src, ipsrc);
> + #endif
> + }
> +- psk.psk_af = saf; /* restore AF_INET */
> + }
> +
> + /* Kill all states to pfa */
> + if (tout || PF2_KILL_STATE_ALL) {
> +- bzero(&psk.psk_src, sizeof(psk.psk_src)); /* clear source
> address field (set before for incomming) */
> +- memcpy(&psk.psk_dst.addr.v.a.addr, &pfa,
> sizeof(psk.psk_dst.addr.v.a.addr));
> +- memset(&psk.psk_dst.addr.v.a.mask, 0xff,
> sizeof(psk.psk_dst.addr.v.a.mask));
> +- if (ioctl(pfdev, DIOCKILLSTATES, &psk)) {
> ++ bzero(&k.src, sizeof(k.src)); /* clear source address
> field (set before for incomming) */
> ++ memcpy(&k.dst.addr.v.a.addr, &pfa,
> sizeof(k.dst.addr.v.a.addr));
> ++ memset(&k.dst.addr.v.a.mask, 0xff,
> sizeof(k.dst.addr.v.a.mask));
> ++ if (pfctl_kill_states(pfdev, &k, &kcount)) {
> + snprintf(msg, sizeof(msg) - 1, "Error: DIOCKILLSTATES
> failed (%s)", strerror(errno));
> + logmessage(1, msg, "pf2", 0);
> + }
> + else {
> +-#if OpenBSD >= 200811 /* since OpenBSD4_4 killed states returned
> in psk_killed */
> +- killed_dst += psk.psk_killed;
> +-#else
> +- killed_dst += psk.psk_af;
> +-#endif
> ++ killed_dst += kcount;
> + #ifdef FWSAMDEBUG
> + printf("Debug: [pf2] killed %lu (tout) states for host
> %s\n", killed_dst, ipsrc);
> + #endif
>
> Tl;dr If you maintain a port that uses /dev/pf you’re going to have
> to start using net/libpfctl.
>
> Best regards,
> Kristof