Date: Wed, 01 Dec 2021 21:58:09 +0100 From: "Kristof Provost" <kp@FreeBSD.org> To: "=?utf-8?q?=C3=96zkan?= KIRIK" <ozkan.kirik@gmail.com> Cc: freebsd-pf@freebsd.org Subject: Re: git: 7f944794868f - stable/12 - pf: Introduce ridentifier Message-ID: <52E4AB7A-6D27-4B11-ABCD-94BB12D389F4@FreeBSD.org> In-Reply-To: <CAAcX-AFshuFUo7g9q_S3%2B5iS3Ko_Y%2BD0M5_nNEMPWOeMp66T3Q@mail.gmail.com> References: <202111261940.1AQJeGLZ022802@gitrepo.freebsd.org> <CAAcX-AFshuFUo7g9q_S3%2B5iS3Ko_Y%2BD0M5_nNEMPWOeMp66T3Q@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--=_MailMate_EB7371D3-1194-4826-AE1E-7EC07629AD1A_=
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
On 1 Dec 2021, at 5:59, Özkan KIRIK wrote:
> Because tshark/wireshark don't know this header change yet.
>
I’ve looked at the Wireshark parser code, and .. well, it’s wrong.
It arbitrarily adds 3 bytes to the header length, and that’s not the
correct solution. I’m not going to implement kernel workarounds for
application bugs.
> And even though tcpdump has been patched by this commit, it still
> cannot parse the packet properly also.
>
Try this patch:
diff --git a/sys/net/if_pflog.h b/sys/net/if_pflog.h
index c77d8da1440a..93a69a2bb3a5 100644
--- a/sys/net/if_pflog.h
+++ b/sys/net/if_pflog.h
@@ -31,6 +31,8 @@
#ifndef _NET_IF_PFLOG_H_
#define _NET_IF_PFLOG_H_
+#include <net/bpf.h>
+
#define PFLOGIFS_MAX 16
#define PFLOG_RULESET_NAME_SIZE 16
@@ -51,11 +53,13 @@ struct pfloghdr {
u_int8_t dir;
u_int8_t pad[3];
u_int32_t ridentifier;
+ u_int8_t reserve; /* Appease broken software like
Wireshark. */
+ u_int8_t pad2[3];
};
-#define PFLOG_HDRLEN sizeof(struct pfloghdr)
+#define PFLOG_HDRLEN BPF_WORDALIGN(offsetof(struct
pfloghdr, pad2))
/* minus pad, also used as a signature */
-#define PFLOG_REAL_HDRLEN offsetof(struct pfloghdr, pad)
+#define PFLOG_REAL_HDRLEN offsetof(struct pfloghdr, pad2)
#ifdef _KERNEL
struct pf_rule;
diff --git a/sys/netpfil/pf/if_pflog.c b/sys/netpfil/pf/if_pflog.c
index 4853c1301d6f..5ccdf3a7dd45 100644
--- a/sys/netpfil/pf/if_pflog.c
+++ b/sys/netpfil/pf/if_pflog.c
@@ -215,7 +215,8 @@ 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 = PFLOG_HDRLEN;
+ hdr.length = PFLOG_REAL_HDRLEN;
hdr.af = af;
hdr.action = rm->action;
hdr.reason = reason;
It looks like I had assumed that the header was expected to be aligned
to 4 bytes, but it’s actually expected to be aligned to sizeof(long).
This should fix that.
I’ve gone one further and added a dummy field to arrange the updated
struct so that Wireshark will work, essentially retaining its incorrect
assumption. It really should be fixed as well though.
Kristof
--=_MailMate_EB7371D3-1194-4826-AE1E-7EC07629AD1A_=--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?52E4AB7A-6D27-4B11-ABCD-94BB12D389F4>