Date: Wed, 1 Dec 2021 07:59:42 +0300 From: =?UTF-8?B?w5Z6a2FuIEtJUklL?= <ozkan.kirik@gmail.com> To: Kristof Provost <kp@freebsd.org> Cc: freebsd-pf@freebsd.org Subject: Re: git: 7f944794868f - stable/12 - pf: Introduce ridentifier Message-ID: <CAAcX-AFshuFUo7g9q_S3%2B5iS3Ko_Y%2BD0M5_nNEMPWOeMp66T3Q@mail.gmail.com> In-Reply-To: <202111261940.1AQJeGLZ022802@gitrepo.freebsd.org> References: <202111261940.1AQJeGLZ022802@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Is it possible to make this feature optional (like sysctl).
Because tshark/wireshark don't know this header change yet.
And even though tcpdump has been patched by this commit, it still
cannot parse the packet properly also.
I think that because of the pflog header growed, all the tcpdump &
libpcap like applications use old pflog header structures so, the
other headers are shifted.
I'm using stable/12 pulled at 28th November (900ca3c03a4e).
The outputs are below:
I'm started ping request from client to the pf host.
# pfctl -ss -vvvv
all icmp 192.168.33.10:12703 <- 192.168.33.1:12703 0:0
age 00:00:04, expires in 00:00:09, 4:4 pkts, 336:336 bytes, anchor 61, r=
ule 0
id: ecf8a66100000001 creatorid: 47132d55 gateway: 0.0.0.0
origif: em1
# tcpdump -tttt -lveqni pflog10
tcpdump: listening on pflog10, link-type PFLOG (OpenBSD pflog file),
capture size 262144 bytes
2021-12-01 07:49:09.295319 rule 61.final.0/0(match): pass in on em1: IP0
^C
1 packet captured
1 packet received by filter
0 packets dropped by kernel
# tshark -Tjson -ni pflog10
[Capturing on 'pflog10'
{
"_index": "packets-2021-12-01",
"_type": "doc",
"_score": null,
"_source": {
"layers": {
"frame": {
"frame.interface_id": "0",
"frame.interface_id_tree": {
"frame.interface_name": "pflog10"
},
"frame.encap_type": "39",
"frame.time": "Dec 1, 2021 07:49:36.782656168 +03",
"frame.offset_shift": "0.000000000",
"frame.time_epoch": "1638334176.782656168",
"frame.time_delta": "0.000000000",
"frame.time_delta_displayed": "0.000000000",
"frame.time_relative": "0.000000000",
"frame.number": "1",
"frame.len": "152",
"frame.cap_len": "152",
"frame.marked": "0",
"frame.ignored": "0",
"frame.protocols": "pflog:ip"
},
"pflog": {
"pflog.length": "68",
"pflog.af": "2",
"pflog.action": "0",
"pflog.reason": "0",
"pflog.ifname": "em1",
"pflog.ruleset": "final,
"pflog.rulenr": "61",
"pflog.subrulenr": "0",
"pflog.uid": "-1",
"pflog.pid": "-1601830656",
"pflog.rule_uid": "0",
"pflog.rule_pid": "-468385792",
"pflog.dir": "1",
"pflog.pad": "00:00:00"
},
"ip": {
"ip.version": "5",
"ip.version_tree": {
"_ws.expert": {
"ip.bogus_ip_version": "",
"_ws.expert.message": "Bogus IP version",
"_ws.expert.severity": "8388608",
"_ws.expert.group": "150994944"
}
}
}
}
}
^C1 packet captured
}
]
Best regards,
=C3=96zkan
On Fri, Nov 26, 2021 at 10:40 PM Kristof Provost <kp@freebsd.org> wrote:
>
> The branch stable/12 has been updated by kp:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=3D7f944794868f49c59449086a37=
55d72e7f747e41
>
> commit 7f944794868f49c59449086a3755d72e7f747e41
> Author: Kristof Provost <kp@FreeBSD.org>
> AuthorDate: 2021-10-29 15:40:53 +0000
> Commit: Kristof Provost <kp@FreeBSD.org>
> CommitDate: 2021-11-26 03:49:02 +0000
>
> pf: Introduce ridentifier
>
> Allow users to set a number on rules which will be exposed as part of
> the pflog header.
> The intent behind this is to allow users to correlate rules across
> updates (remember that pf rules continue to exist and match existing
> states, even if they're removed from the active ruleset) and pflog.
>
> Obtained from: pfSense
> MFC after: 3 weeks
> Sponsored by: Rubicon Communications, LLC ("Netgate")
> Differential Revision: https://reviews.freebsd.org/D32750
>
> (cherry picked from commit 76c5eecc3490d89a9a3492ed2354802b69d69602)
> ---
> contrib/tcpdump/print-pflog.c | 7 ++++++-
> lib/libpfctl/libpfctl.c | 2 ++
> lib/libpfctl/libpfctl.h | 1 +
> sbin/pfctl/parse.y | 14 ++++++++++++++
> sbin/pfctl/pfctl_parser.c | 2 ++
> share/man/man4/pflog.4 | 3 ++-
> share/man/man5/pf.conf.5 | 7 ++++++-
> sys/net/if_pflog.h | 1 +
> sys/net/pfvar.h | 1 +
> sys/netpfil/ipfw/nat64/nat64clat.c | 2 +-
> sys/netpfil/ipfw/nat64/nat64lsn.c | 2 +-
> sys/netpfil/ipfw/nat64/nat64stl.c | 2 +-
> sys/netpfil/pf/if_pflog.c | 3 ++-
> sys/netpfil/pf/pf_nv.c | 2 ++
> 14 files changed, 42 insertions(+), 7 deletions(-)
>
> diff --git a/contrib/tcpdump/print-pflog.c b/contrib/tcpdump/print-pflog.=
c
> index 38201c55ee3f..49994507e728 100644
> --- a/contrib/tcpdump/print-pflog.c
> +++ b/contrib/tcpdump/print-pflog.c
> @@ -88,10 +88,12 @@ static const struct tok pf_directions[] =3D {
> static void
> pflog_print(netdissect_options *ndo, const struct pfloghdr *hdr)
> {
> - uint32_t rulenr, subrulenr;
> + uint32_t rulenr, subrulenr, ridentifier;
>
> rulenr =3D EXTRACT_32BITS(&hdr->rulenr);
> subrulenr =3D EXTRACT_32BITS(&hdr->subrulenr);
> + ridentifier =3D EXTRACT_32BITS(&hdr->ridentifier);
> +
> if (subrulenr =3D=3D (uint32_t)-1)
> ND_PRINT((ndo, "rule %u/", rulenr));
> else
> @@ -102,6 +104,9 @@ pflog_print(netdissect_options *ndo, const struct pfl=
oghdr *hdr)
> if (hdr->uid !=3D UID_MAX)
> ND_PRINT((ndo, " [uid %u]", (unsigned)hdr->uid));
>
> + if (ridentifier !=3D 0)
> + ND_PRINT((ndo, " [ridentifier %u]", ridentifier));
> +
> ND_PRINT((ndo, ": %s %s on %s: ",
> tok2str(pf_actions, "unkn(%u)", hdr->action),
> tok2str(pf_directions, "unkn(%u)", hdr->dir),
> diff --git a/lib/libpfctl/libpfctl.c b/lib/libpfctl/libpfctl.c
> index c2d57d8136ca..e41f970e7696 100644
> --- a/lib/libpfctl/libpfctl.c
> +++ b/lib/libpfctl/libpfctl.c
> @@ -455,6 +455,7 @@ pf_nvrule_to_rule(const nvlist_t *nvl, struct pfctl_r=
ule *rule)
> assert(labelcount <=3D PF_RULE_MAX_LABEL_COUNT);
> for (size_t i =3D 0; i < labelcount; i++)
> strlcpy(rule->label[i], labels[i], PF_RULE_LABEL_SIZE);
> + rule->ridentifier =3D nvlist_get_number(nvl, "ridentifier");
> strlcpy(rule->ifname, nvlist_get_string(nvl, "ifname"), IFNAMSIZ)=
;
> strlcpy(rule->qname, nvlist_get_string(nvl, "qname"), PF_QNAME_SI=
ZE);
> strlcpy(rule->pqname, nvlist_get_string(nvl, "pqname"), PF_QNAME_=
SIZE);
> @@ -566,6 +567,7 @@ pfctl_add_rule(int dev, const struct pfctl_rule *r, c=
onst char *anchor,
> r->label[labelcount]);
> labelcount++;
> }
> + nvlist_add_number(nvlr, "ridentifier", r->ridentifier);
>
> nvlist_add_string(nvlr, "ifname", r->ifname);
> nvlist_add_string(nvlr, "qname", r->qname);
> diff --git a/lib/libpfctl/libpfctl.h b/lib/libpfctl/libpfctl.h
> index 70c144772c02..ac239d7cdcb1 100644
> --- a/lib/libpfctl/libpfctl.h
> +++ b/lib/libpfctl/libpfctl.h
> @@ -81,6 +81,7 @@ struct pfctl_rule {
> struct pf_rule_addr dst;
> union pf_rule_ptr skip[PF_SKIP_COUNT];
> char label[PF_RULE_MAX_LABEL_COUNT][PF_RULE_L=
ABEL_SIZE];
> + u_int32_t ridentifier;
> char ifname[IFNAMSIZ];
> char qname[PF_QNAME_SIZE];
> char pqname[PF_QNAME_SIZE];
> diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y
> index 2dd0e6b6ff43..f06462bda864 100644
> --- a/sbin/pfctl/parse.y
> +++ b/sbin/pfctl/parse.y
> @@ -236,6 +236,7 @@ static struct filter_opts {
> struct node_icmp *icmpspec;
> u_int32_t tos;
> u_int32_t prob;
> + u_int32_t ridentifier;
> struct {
> int action;
> struct node_state_opt *options;
> @@ -260,6 +261,7 @@ static struct filter_opts {
> static struct antispoof_opts {
> char *label[PF_RULE_MAX_LABEL_COUNT];
> int labelcount;
> + u_int32_t ridentifier;
> u_int rtableid;
> } antispoof_opts;
>
> @@ -468,6 +470,7 @@ int parseport(char *, struct range *r, int);
> %token BITMASK RANDOM SOURCEHASH ROUNDROBIN STATICPORT PROBABILITY MAPEP=
ORTSET
> %token ALTQ CBQ CODEL PRIQ HFSC FAIRQ BANDWIDTH TBRSIZE LINKSHARE REALTI=
ME
> %token UPPERLIMIT QUEUE PRIORITY QLIMIT HOGS BUCKETS RTABLE TARGET INTER=
VAL
> +%token RIDENTIFIER
> %token LOAD RULESET_OPTIMIZATION PRIO
> %token STICKYADDRESS MAXSRCSTATES MAXSRCNODES SOURCETRACK GLOBAL RULE
> %token MAXSRCCONN MAXSRCCONNRATE OVERLOAD FLUSH SLOPPY
> @@ -915,6 +918,7 @@ anchorrule : ANCHOR anchorname dir quick interface a=
f proto fromto
> r.af =3D $6;
> r.prob =3D $9.prob;
> r.rtableid =3D $9.rtableid;
> + r.ridentifier =3D $9.ridentifier;
>
> if ($9.tag)
> if (strlcpy(r.tagname, $9.tag,
> @@ -1314,6 +1318,7 @@ antispoof : ANTISPOOF logquick antispoof_ifspc af a=
ntispoof_opts {
> r.logif =3D $2.logif;
> r.quick =3D $2.quick;
> r.af =3D $4;
> + r.ridentifier =3D $5.ridentifier;
> if (rule_label(&r, $5.label))
> YYERROR;
> r.rtableid =3D $5.rtableid;
> @@ -1366,6 +1371,7 @@ antispoof : ANTISPOOF logquick antispoof_ifspc af a=
ntispoof_opts {
> r.logif =3D $2.logif;
> r.quick =3D $2.quick;
> r.af =3D $4;
> + r.ridentifier =3D $5.ridentifier;
> if (rule_label(&r, $5.label))
> YYERROR;
> r.rtableid =3D $5.rtableid;
> @@ -1428,6 +1434,9 @@ antispoof_opt : label {
> }
> antispoof_opts.label[antispoof_opts.labelcount++]=
=3D $1;
> }
> + | RIDENTIFIER number {
> + antispoof_opts.ridentifier =3D $2;
> + }
> | RTABLE NUMBER {
> if ($2 < 0 || $2 > rt_tableid_max()) {
> yyerror("invalid rtable id");
> @@ -2143,6 +2152,7 @@ pfrule : action dir logquick interface r=
oute af proto fromto
> YYERROR;
> for (int i =3D 0; i < PF_RULE_MAX_LABEL_COUNT; i+=
+)
> free($9.label[i]);
> + r.ridentifier =3D $9.ridentifier;
> r.flags =3D $9.flags.b1;
> r.flagset =3D $9.flags.b2;
> if (($9.flags.b1 & $9.flags.b2) !=3D $9.flags.b1)=
{
> @@ -2573,6 +2583,9 @@ filter_opt : USER uids {
> filter_opts.keep.action =3D $1.action;
> filter_opts.keep.options =3D $1.options;
> }
> + | RIDENTIFIER number {
> + filter_opts.ridentifier =3D $2;
> + }
> | FRAGMENT {
> filter_opts.fragment =3D 1;
> }
> @@ -5687,6 +5700,7 @@ lookup(char *s)
> { "return-icmp", RETURNICMP},
> { "return-icmp6", RETURNICMP6},
> { "return-rst", RETURNRST},
> + { "ridentifier", RIDENTIFIER},
> { "round-robin", ROUNDROBIN},
> { "route", ROUTE},
> { "route-to", ROUTETO},
> diff --git a/sbin/pfctl/pfctl_parser.c b/sbin/pfctl/pfctl_parser.c
> index dc4a2254d733..adf9255f0c84 100644
> --- a/sbin/pfctl/pfctl_parser.c
> +++ b/sbin/pfctl/pfctl_parser.c
> @@ -1019,6 +1019,8 @@ print_rule(struct pfctl_rule *r, const char *anchor=
_call, int verbose, int numer
> i =3D 0;
> while (r->label[i][0])
> printf(" label \"%s\"", r->label[i++]);
> + if (r->ridentifier)
> + printf(" ridentifier %u", r->ridentifier);
> if (r->qname[0] && r->pqname[0])
> printf(" queue(%s, %s)", r->qname, r->pqname);
> else if (r->qname[0])
> diff --git a/share/man/man4/pflog.4 b/share/man/man4/pflog.4
> index 428bb5bd7f26..6269644bc312 100644
> --- a/share/man/man4/pflog.4
> +++ b/share/man/man4/pflog.4
> @@ -25,7 +25,7 @@
> .\"
> .\" $FreeBSD$
> .\"
> -.Dd May 31, 2007
> +.Dd October 29, 2021
> .Dt PFLOG 4
> .Os
> .Sh NAME
> @@ -79,6 +79,7 @@ struct pfloghdr {
> pid_t rule_pid;
> u_int8_t dir;
> u_int8_t pad[3];
> + u_int32_t ridentifier;
> };
> .Ed
> .Sh EXAMPLES
> diff --git a/share/man/man5/pf.conf.5 b/share/man/man5/pf.conf.5
> index f75edb6fcc17..e9ec3467da54 100644
> --- a/share/man/man5/pf.conf.5
> +++ b/share/man/man5/pf.conf.5
> @@ -1868,6 +1868,9 @@ pass in inet proto tcp from any to 1.2.3.5 \e
> The macro expansion for the
> .Ar label
> directive occurs only at configuration file parse time, not during runti=
me.
> +.It Ar ridentifier Aq Ar number
> +Add an identifier (number) to the rule, which can be used to correlate t=
he rule
> +to pflog entries, even after ruleset updates.
> .It Xo Ar queue Aq Ar queue
> .No \*(Ba ( Aq Ar queue ,
> .Aq Ar queue )
> @@ -2970,7 +2973,8 @@ filteropt =3D user | group | flags | icmp-type=
| icmp6-type | "tos" tos |
> "label" string | "tag" string | [ ! ] "tagged" string |
> "set prio" ( number | "(" number [ [ "," ] number ] ")"=
) |
> "queue" ( string | "(" string [ [ "," ] string ] ")" ) =
|
> - "rtable" number | "probability" number"%" | "prio" numb=
er
> + "rtable" number | "probability" number"%" | "prio" numb=
er |
> + "ridentifier" number
>
> nat-rule =3D [ "no" ] "nat" [ "pass" [ "log" [ "(" logopts ")" ] ]=
]
> [ "on" ifspec ] [ af ]
> @@ -2994,6 +2998,7 @@ rdr-rule =3D [ "no" ] "rdr" [ "pass" [ "log" =
[ "(" logopts ")" ] ] ]
>
> antispoof-rule =3D "antispoof" [ "log" ] [ "quick" ]
> "for" ifspec [ af ] [ "label" string ]
> + [ "ridentifier" number ]
>
> table-rule =3D "table" "\*(Lt" string "\*(Gt" [ tableopts-list ]
> tableopts-list =3D tableopts-list tableopts | tableopts
> diff --git a/sys/net/if_pflog.h b/sys/net/if_pflog.h
> index 5ed341a85d86..c77d8da1440a 100644
> --- a/sys/net/if_pflog.h
> +++ b/sys/net/if_pflog.h
> @@ -50,6 +50,7 @@ struct pfloghdr {
> pid_t rule_pid;
> u_int8_t dir;
> u_int8_t pad[3];
> + u_int32_t ridentifier;
> };
>
> #define PFLOG_HDRLEN sizeof(struct pfloghdr)
> diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
> index 6f8d79b27133..4c4fc7c65015 100644
> --- a/sys/net/pfvar.h
> +++ b/sys/net/pfvar.h
> @@ -572,6 +572,7 @@ struct pf_krule {
> struct pf_rule_addr dst;
> union pf_krule_ptr skip[PF_SKIP_COUNT];
> char label[PF_RULE_MAX_LABEL_COUNT][PF_RULE_L=
ABEL_SIZE];
> + uint32_t ridentifier;
> char ifname[IFNAMSIZ];
> char qname[PF_QNAME_SIZE];
> char pqname[PF_QNAME_SIZE];
> diff --git a/sys/netpfil/ipfw/nat64/nat64clat.c b/sys/netpfil/ipfw/nat64/=
nat64clat.c
> index fcc922726d02..c48c68183e08 100644
> --- a/sys/netpfil/ipfw/nat64/nat64clat.c
> +++ b/sys/netpfil/ipfw/nat64/nat64clat.c
> @@ -71,7 +71,7 @@ nat64clat_log(struct pfloghdr *plog, struct mbuf *m, sa=
_family_t family,
> static uint32_t pktid =3D 0;
>
> memset(plog, 0, sizeof(*plog));
> - plog->length =3D PFLOG_REAL_HDRLEN;
> + plog->length =3D PFLOG_HDRLEN;
> plog->af =3D family;
> plog->action =3D PF_NAT;
> plog->dir =3D PF_IN;
> diff --git a/sys/netpfil/ipfw/nat64/nat64lsn.c b/sys/netpfil/ipfw/nat64/n=
at64lsn.c
> index ad1b62b07a92..ab77a071bcdb 100644
> --- a/sys/netpfil/ipfw/nat64/nat64lsn.c
> +++ b/sys/netpfil/ipfw/nat64/nat64lsn.c
> @@ -181,7 +181,7 @@ nat64lsn_log(struct pfloghdr *plog, struct mbuf *m, s=
a_family_t family,
> {
>
> memset(plog, 0, sizeof(*plog));
> - plog->length =3D PFLOG_REAL_HDRLEN;
> + plog->length =3D PFLOG_HDRLEN;
> plog->af =3D family;
> plog->action =3D PF_NAT;
> plog->dir =3D PF_IN;
> diff --git a/sys/netpfil/ipfw/nat64/nat64stl.c b/sys/netpfil/ipfw/nat64/n=
at64stl.c
> index a150322d1a44..fa7afee44be7 100644
> --- a/sys/netpfil/ipfw/nat64/nat64stl.c
> +++ b/sys/netpfil/ipfw/nat64/nat64stl.c
> @@ -70,7 +70,7 @@ nat64stl_log(struct pfloghdr *plog, struct mbuf *m, sa_=
family_t family,
> static uint32_t pktid =3D 0;
>
> memset(plog, 0, sizeof(*plog));
> - plog->length =3D PFLOG_REAL_HDRLEN;
> + plog->length =3D PFLOG_HDRLEN;
> plog->af =3D family;
> plog->action =3D PF_NAT;
> plog->dir =3D PF_IN;
> diff --git a/sys/netpfil/pf/if_pflog.c b/sys/netpfil/pf/if_pflog.c
> index 9eb168b9a74f..4853c1301d6f 100644
> --- a/sys/netpfil/pf/if_pflog.c
> +++ b/sys/netpfil/pf/if_pflog.c
> @@ -215,7 +215,7 @@ pflog_packet(struct pfi_kkif *kif, struct mbuf *m, sa=
_family_t af, u_int8_t dir,
> return (0);
>
> bzero(&hdr, sizeof(hdr));
> - hdr.length =3D PFLOG_REAL_HDRLEN;
> + hdr.length =3D PFLOG_HDRLEN;
> hdr.af =3D af;
> hdr.action =3D rm->action;
> hdr.reason =3D reason;
> @@ -231,6 +231,7 @@ pflog_packet(struct pfi_kkif *kif, struct mbuf *m, sa=
_family_t af, u_int8_t dir,
> strlcpy(hdr.ruleset, ruleset->anchor->name,
> sizeof(hdr.ruleset));
> }
> + hdr.ridentifier =3D htonl(rm->ridentifier);
> /*
> * XXXGL: we avoid pf_socket_lookup() when we are holding
> * state lock, since this leads to unsafe LOR.
> diff --git a/sys/netpfil/pf/pf_nv.c b/sys/netpfil/pf/pf_nv.c
> index d53c6fe4b84e..b6676be645d7 100644
> --- a/sys/netpfil/pf/pf_nv.c
> +++ b/sys/netpfil/pf/pf_nv.c
> @@ -531,6 +531,7 @@ pf_nvrule_to_krule(const nvlist_t *nvl, struct pf_kru=
le *rule)
> }
> }
>
> + PFNV_CHK(pf_nvuint32_opt(nvl, "ridentifier", &rule->ridentifier, =
0));
> PFNV_CHK(pf_nvstring(nvl, "ifname", rule->ifname,
> sizeof(rule->ifname)));
> PFNV_CHK(pf_nvstring(nvl, "qname", rule->qname, sizeof(rule->qnam=
e)));
> @@ -693,6 +694,7 @@ pf_krule_to_nvrule(struct pf_krule *rule)
> nvlist_append_string_array(nvl, "labels", rule->label[i])=
;
> }
> nvlist_add_string(nvl, "label", rule->label[0]);
> + nvlist_add_number(nvl, "ridentifier", rule->ridentifier);
> nvlist_add_string(nvl, "ifname", rule->ifname);
> nvlist_add_string(nvl, "qname", rule->qname);
> nvlist_add_string(nvl, "pqname", rule->pqname);
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAAcX-AFshuFUo7g9q_S3%2B5iS3Ko_Y%2BD0M5_nNEMPWOeMp66T3Q>