Date: Mon, 28 Aug 2023 09:15:44 +0200 From: Kristof Provost <kp@FreeBSD.org> To: FreeBSD PF List <freebsd-pf@freebsd.org>, FreeBSD Ports ML <freebsd-ports@freebsd.org> Subject: Re: pf userspace API changes Message-ID: <F339AA38-3089-4C6B-9997-670EEB87170F@FreeBSD.org> In-Reply-To: <0E45DD6F-81E3-45DB-9FB2-E47B8F26FD00@FreeBSD.org> References: <0E45DD6F-81E3-45DB-9FB2-E47B8F26FD00@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--=_MailMate_1767B026-9A02-4D14-939B-D23767FA7B10_= Content-Type: text/plain; charset=UTF-8; format=flowed; markup=markdown Content-Transfer-Encoding: 8bit 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 --=_MailMate_1767B026-9A02-4D14-939B-D23767FA7B10_= Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable <!DOCTYPE html> <html> <head> <meta http-equiv=3D"Content-Type" content=3D"text/xhtml; charset=3Dutf-8"= > </head> <body><div style=3D"font-family: sans-serif;"><div class=3D"markdown" sty= le=3D"white-space: normal;"> <p dir=3D"auto">This is going to land soon, in 15. So 14 will keep the ol= d ioctls, but they=E2=80=99ll disappear in 15.</p> <p dir=3D"auto">Best regards,<br> Kristof</p> <p dir=3D"auto">On 6 Apr 2023, at 16:21, Kristof Provost wrote:</p> <blockquote style=3D"margin: 0 0 5px; padding-left: 5px; border-left: 2px= solid #136BCE; color: #136BCE;"> <p dir=3D"auto">Hi,</p> <p dir=3D"auto">Quick heads up that there are going to be breaking change= s to the pf API towards userspace for 14.0. (That is, the ioctl interface= presented by /dev/pf).</p> <p dir=3D"auto">I=E2=80=99ve been rewriting several types of calls to sta= rt using nvlists, because otherwise it=E2=80=99s basically unmanageable t= o extend calls (which we pretty much have to do to add new features).<br>= The old struct-based ioctls have been left in place so far, but as that=E2= =80=99s a substantial amount of (now untested!) code I=E2=80=99m very kee= n to be able to remove.</p> <p dir=3D"auto">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 bre= ak stable/12 or stable/13 or 13.2 or =E2=80=A6</p> <p dir=3D"auto">The initial breaking change is <a href=3D"https://reviews= =2Efreebsd.org/D30056">https://reviews.freebsd.org/D30056</a>; . That remo= ves DIOCCLRSTATES and DIOCKILLSTATES, which are now DIOCCLRSTATESNV and D= IOCKILLSTATESNV, based on nvlists.</p> <p dir=3D"auto">To make that all easier for userspace to manage there=E2=80= =99s libpfctl, which wraps all of the API details. That port will be avai= lable for all supported platforms (when <a href=3D"https://reviews.freebs= d.org/D39360">https://reviews.freebsd.org/D39360</a>; lands, soon).</p> <p dir=3D"auto">There are likely to be more changes in the future, so I=E2= =80=99d strongly encourage all API users to migrate to using libpfctl rat= her than trying to roll their own implementations.</p> <p dir=3D"auto">Here=E2=80=99s an example of how security/snortsam needed= to be changed to cope with the above:</p> <pre style=3D"margin-left: 15px; margin-right: 15px; padding: 5px; border= : thin solid gray; overflow-x: auto; max-width: 90vw; background-color: #= E4E4E4;"><code style=3D"padding: 0 0.25em; background-color: #E4E4E4;">co= mmit 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=3D snortsam PORTVERSION=3D 2.70 -PORTREVISION=3D 1 +PORTREVISION=3D 2 CATEGORIES=3D security MASTER_SITES=3D http://www.snortsam.net/files/snortsam/ DISTNAME=3D ${PORTNAME}-src-${PORTVERSION} @@ -16,6 +16,8 @@ SAMTOOL_DESC=3D install samtool .include <bsd.port.pre.mk> +LIB_DEPENDS=3D libpfctl.so:net/libpfctl + USE_RC_SUBR=3D snortsam SUB_FILES=3D pkg-message \ pkg-install diff --git a/security/snortsam/files/patch-src_Makefile b/security/snorts= am/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 =3D samtool + PROG =3D snortsam + SAMTOOL =3D samtool +-CFLAGS =3D -O2 -D$(SYSTYPE) $(DEBUG) +-LDFLAGS =3D ++CFLAGS =3D -O2 -D$(SYSTYPE) $(DEBUG) -I/usr/local/include ++LDFLAGS =3D -L/usr/local/lib -lpfctl + SYSTYPE =3D `uname` + + # OS specific flags diff --git a/security/snortsam/files/patch-src__ssp_pf2.c b/security/snor= tsam/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 =3D TRUE; + unsigned int PF2val_count =3D 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=3D0, killed_src=3D0, killed_dst=3D0; ++ unsigned int kcount; + + bzero(&pfa, sizeof(pfa)); +- bzero(&psk, sizeof(psk)); ++ bzero(&k, sizeof(k)); + + if (ipsrc =3D=3D NULL || !ipsrc[0]) + return (-1); + + if (inet_pton(AF_INET, ipsrc, &pfa.v4) =3D=3D 1) +- psk.psk_af =3D saf =3D AF_INET; ++ k.af =3D AF_INET; + else if (inet_pton(AF_INET6, ipsrc, &pfa.v6) =3D=3D 1) +- psk.psk_af =3D saf =3D AF_INET6; ++ k.af =3D 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_s= rc.addr.v.a.addr)); +- memset(&psk.psk_src.addr.v.a.mask, 0xff, sizeof(psk.psk_src.a= ddr.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 fa= iled (%s)", strerror(errno)); + logmessage(1, msg, "pf2", 0); + } + else { +-#if OpenBSD >=3D 200811 /* since OpenBSD4_4 killed states returned i= n psk_killed */ +- killed_src +=3D psk.psk_killed; +-#else +- killed_src +=3D psk.psk_af; +-#endif ++ killed_src +=3D kcount; + #ifdef FWSAMDEBUG + printf("Debug: [pf2] killed %lu (tin) states for host %s= \n", killed_src, ipsrc); + #endif + } +- psk.psk_af =3D 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 ad= dress field (set before for incomming) */ +- memcpy(&psk.psk_dst.addr.v.a.addr, &pfa, sizeof(psk.psk_d= st.addr.v.a.addr)); +- memset(&psk.psk_dst.addr.v.a.mask, 0xff, sizeof(psk.psk_dst.a= ddr.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 fa= iled (%s)", strerror(errno)); + logmessage(1, msg, "pf2", 0); + } + else { +-#if OpenBSD >=3D 200811 /* since OpenBSD4_4 killed states returned i= n psk_killed */ +- killed_dst +=3D psk.psk_killed; +-#else +- killed_dst +=3D psk.psk_af; +-#endif ++ killed_dst +=3D kcount; + #ifdef FWSAMDEBUG + printf("Debug: [pf2] killed %lu (tout) states for host %= s\n", killed_dst, ipsrc); + #endif </code></pre> <p dir=3D"auto">Tl;dr If you maintain a port that uses /dev/pf you=E2=80=99= re going to have to start using net/libpfctl.</p> <p dir=3D"auto">Best regards,<br> Kristof</p> </blockquote> </div> </div> </body> </html> --=_MailMate_1767B026-9A02-4D14-939B-D23767FA7B10_=--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?F339AA38-3089-4C6B-9997-670EEB87170F>