From nobody Wed Dec 1 20:58:09 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 BDF9518C3D8B for ; Wed, 1 Dec 2021 20:58:12 +0000 (UTC) (envelope-from kp@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4J4BJ44HjLz4rTw; Wed, 1 Dec 2021 20:58:12 +0000 (UTC) (envelope-from kp@FreeBSD.org) Received: from venus.codepro.be (venus.codepro.be [5.9.86.228]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mx1.codepro.be", Issuer "R3" (verified OK)) (Authenticated sender: kp) by smtp.freebsd.org (Postfix) with ESMTPSA id 4696B24E1D; Wed, 1 Dec 2021 20:58:12 +0000 (UTC) (envelope-from kp@FreeBSD.org) Received: by venus.codepro.be (Postfix, authenticated sender kp) id 2CCD3FE0A; Wed, 1 Dec 2021 21:58:10 +0100 (CET) From: "Kristof Provost" To: "=?utf-8?q?=C3=96zkan?= KIRIK" Cc: freebsd-pf@freebsd.org Subject: Re: git: 7f944794868f - stable/12 - pf: Introduce ridentifier Date: Wed, 01 Dec 2021 21:58:09 +0100 X-Mailer: MailMate (1.13.2r5673) Message-ID: <52E4AB7A-6D27-4B11-ABCD-94BB12D389F4@FreeBSD.org> In-Reply-To: References: <202111261940.1AQJeGLZ022802@gitrepo.freebsd.org> 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 Content-Type: multipart/alternative; boundary="=_MailMate_EB7371D3-1194-4826-AE1E-7EC07629AD1A_=" Content-Transfer-Encoding: 8bit ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1638392292; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=CUxJtr/aJ+Jl45x9dyQZCIYsUytvZ3oVSMDWI9sgq5A=; b=J6Jl6sOa19i0/rEOiwZ2m/TH0LUgcfjf+UbYIX+tGPHbEF4jjyvA/ZvV5ien6YY/esOaFY 7b+mKiv1wOiyXdrxkvHTuzf1bwlJ8d3EclHCNu9llydV4ZCFtwiPmGIBLSqM1y8XnC8eQZ RusiJdH6l4QHIxR52GVzspmyPvMrTbJkvvZo3o1MRfeyi3d/cwiUmMvTohIGRhmCuYSPTf vWbH1wGGFq8hXxHcWwa502znmROYTbOKEhJVfh4FZyBmNgXn4b44749T1H8+jSWlbQFOTe MZCRRh7Vh/MXFG2cBcqBzVkr4E6CUtTznTOtGPiuAuLUNQqJWL6Xr37eh/r7qA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1638392292; a=rsa-sha256; cv=none; b=e2bCHOCkJLhvhBmfpCLNw4E3nOPUWbW3m7Ym60i7flhg2gLOc5J6pMQ5p67wl+yNzZeWHC oX+eaiVUcxSH5kByQ5kdN6v4DUy/f3aE0sZxDR7GUs+bo/5UraLklSbjpHnTfesVPkP54L 1N6nzdV86e0EngLWw96cSwUxlQtN3Q39Jz9yf1njpwxVDTrXXt+Q/nWuM9/7XVTW8oZsPH y8nS/qdpdPRfIEL18TF72Y3JK1dpKpIEZURBuOeG+TqfcI9OGnOSDILUz8gde/NcW+xbjs XW2huAj3Pwy8KNEPjm13m6zwdSa+9qAHc9bt/R6UKH00+bJaZyN4l0BE8NzTaQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: Y --=_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 + #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_=--