From nobody Fri Dec 3 14:05:02 2021 X-Original-To: freebsd-pf@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 5A69B18A877F for ; Fri, 3 Dec 2021 14:05:10 +0000 (UTC) (envelope-from mike@sentex.net) Received: from smarthost1.sentex.ca (smarthost1.sentex.ca [IPv6:2607:f3e0:0:1::12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "smarthost1.sentex.ca", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4J5F2Z1zLBz4msj; Fri, 3 Dec 2021 14:05:10 +0000 (UTC) (envelope-from mike@sentex.net) Received: from pyroxene2a.sentex.ca (pyroxene19.sentex.ca [199.212.134.19]) by smarthost1.sentex.ca (8.16.1/8.16.1) with ESMTPS id 1B3E53Aw002848 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Fri, 3 Dec 2021 09:05:03 -0500 (EST) (envelope-from mike@sentex.net) Received: from [IPV6:2607:f3e0:0:4::29] ([IPv6:2607:f3e0:0:4:0:0:0:29]) by pyroxene2a.sentex.ca (8.16.1/8.15.2) with ESMTPS id 1B3E52bq002082 (version=TLSv1.3 cipher=TLS_AES_128_GCM_SHA256 bits=128 verify=NO); Fri, 3 Dec 2021 09:05:03 -0500 (EST) (envelope-from mike@sentex.net) Message-ID: <1b5c4e46-7868-9fb9-4259-0c286b071310@sentex.net> Date: Fri, 3 Dec 2021 09:05:02 -0500 List-Id: Technical discussion and general questions about packet filter (pf) List-Archive: https://lists.freebsd.org/archives/freebsd-pf List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-freebsd-pf@freebsd.org X-BeenThere: freebsd-pf@freebsd.org MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.3.2 Subject: Re: git: 7f944794868f - stable/12 - pf: Introduce ridentifier Content-Language: en-US To: Kristof Provost Cc: freebsd-pf@freebsd.org References: <202111261940.1AQJeGLZ022802@gitrepo.freebsd.org> <52E4AB7A-6D27-4B11-ABCD-94BB12D389F4@FreeBSD.org> From: mike tancsa In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 2.84 X-Rspamd-Queue-Id: 4J5F2Z1zLBz4msj X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-ThisMailContainsUnwantedMimeParts: N Also, thanks for all the awesome PF work that you have done!  Its much appreciated     ---Mike On 12/3/2021 1:05 AM, Özkan KIRIK wrote: > Thank you Kristof. The patch works properly! > > On Wed, Dec 1, 2021 at 11:58 PM Kristof Provost wrote: >> 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 >> + >> #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 >