From nobody Fri Dec 3 06:05:46 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 86A7E18B2F3F for ; Fri, 3 Dec 2021 06:05:58 +0000 (UTC) (envelope-from ozkan.kirik@gmail.com) Received: from mail-ua1-x935.google.com (mail-ua1-x935.google.com [IPv6:2607:f8b0:4864:20::935]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4J52Pd50mNz4TwN; Fri, 3 Dec 2021 06:05:57 +0000 (UTC) (envelope-from ozkan.kirik@gmail.com) Received: by mail-ua1-x935.google.com with SMTP id p2so3435341uad.11; Thu, 02 Dec 2021 22:05:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=dd70BXIQB2x8p4AEVGmZcZZyewl/RwBp60Rc3DYdVD4=; b=XGKkDJO1DIT9OynCDGWK+LDDWHcUyqDda8vCdVRm1VJ3TGO8RczTlY8Jp/yruSHYSe PNNjqvRLBFGQAyOMZml/4Zsmn6WeyTHDKoCsqXpapZSnPP4bQcoHZ12YXkW6m08robUo +ZWoztK33PfdWg1R9pk2SuvputVBQpSa82RULLcs0bzjSgON4v93GnuBPX14thLeVlby Bt4lSB1tWwkiVNcUNGW116mRmJecxBNC+0yZQbjFKFAN2vlP6RFk/ir8iRBMEPA3temr 0qvxRQJCJEPM5AYgqYGOCB1wgXWWohmAmnXMkY1Qmwoab378O92KKoI6SvOaa88EPQVh vuAw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=dd70BXIQB2x8p4AEVGmZcZZyewl/RwBp60Rc3DYdVD4=; b=vsQcI9v7LK9duZyP/jLhRW3PCVc3VvVbvYhWoAiTWq9oVlfhg8uY7ka1aIti6gCxhm gyfXV6bvOUiPP0xbrcvEXQTtSwNym6AAr6WN3NV7voZR9aCLpb6FzJF/4uWov1oO3HRM v4wRHfe/PlszHq/vNI/mkfecr1y1Jz2klEjY1Aocr7UW55aAvECh51QckCxfxDHn/Zme g4lUIRHl0THqr+NBGlb0Ht7snp8Ho9w+sG2TlqrLfdiChWkU3LUFenbUwZ218PF2ztCk dkpQnETe1RzmT09qWsEFqAZ2UtD69zXsJS1a+7PSYdngzMKAro0RliclD7IA7blzVU/v wdLQ== X-Gm-Message-State: AOAM530injoA42mwXu3NXgqX9V5narkRJsC8H8xeENYV5kF/DfdJ5hLO CzS0EiEkQzTHAwLIa+ekIWrlcHy4VcGWFlVoAbybSuOH X-Google-Smtp-Source: ABdhPJwzDhLMGw8JpTqClbjhuo10pnG1bnPLP1XIUR8uTpFmPqpRP1CTjQO2c1/JL6U+5fKF0AERzKFo3lyPMkt9ias= X-Received: by 2002:ab0:6eca:: with SMTP id c10mr20905314uav.118.1638511557072; Thu, 02 Dec 2021 22:05:57 -0800 (PST) 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 References: <202111261940.1AQJeGLZ022802@gitrepo.freebsd.org> <52E4AB7A-6D27-4B11-ABCD-94BB12D389F4@FreeBSD.org> In-Reply-To: <52E4AB7A-6D27-4B11-ABCD-94BB12D389F4@FreeBSD.org> From: =?UTF-8?B?w5Z6a2FuIEtJUklL?= Date: Fri, 3 Dec 2021 09:05:46 +0300 Message-ID: Subject: Re: git: 7f944794868f - stable/12 - pf: Introduce ridentifier To: Kristof Provost Cc: freebsd-pf@freebsd.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 4J52Pd50mNz4TwN X-Spamd-Bar: / Authentication-Results: mx1.freebsd.org; dkim=pass header.d=gmail.com header.s=20210112 header.b=XGKkDJO1; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (mx1.freebsd.org: domain of ozkankirik@gmail.com designates 2607:f8b0:4864:20::935 as permitted sender) smtp.mailfrom=ozkankirik@gmail.com X-Spamd-Result: default: False [-0.19 / 15.00]; DWL_DNSWL_NONE(0.00)[gmail.com:dkim]; ARC_NA(0.00)[]; R_DKIM_ALLOW(-0.20)[gmail.com:s=20210112]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; FREEMAIL_FROM(0.00)[gmail.com]; TO_MATCH_ENVRCPT_ALL(0.00)[]; MIME_GOOD(-0.10)[text/plain]; R_SPF_ALLOW(-0.20)[+ip6:2607:f8b0:4000::/36]; NEURAL_HAM_LONG(-1.00)[-1.000]; NEURAL_SPAM_MEDIUM(0.81)[0.810]; NEURAL_SPAM_SHORT(1.00)[0.997]; MID_RHS_MATCH_FROMTLD(0.00)[]; DKIM_TRACE(0.00)[gmail.com:+]; RCPT_COUNT_TWO(0.00)[2]; DMARC_POLICY_ALLOW(-0.50)[gmail.com,none]; RCVD_IN_DNSWL_NONE(0.00)[2607:f8b0:4864:20::935:from]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; FREEMAIL_ENVFROM(0.00)[gmail.com]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US]; TAGGED_FROM(0.00)[]; RCVD_TLS_ALL(0.00)[]; RCVD_COUNT_TWO(0.00)[2] X-ThisMailContainsUnwantedMimeParts: N 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, =C3=96zkan KIRIK wrote: > > Because tshark/wireshark don't know this header change yet. > > I=E2=80=99ve looked at the Wireshark parser code, and .. well, it=E2=80= =99s wrong. It arbitrarily adds 3 bytes to the header length, and that=E2= =80=99s not the correct solution. I=E2=80=99m 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 W= ireshark. */ > + u_int8_t pad2[3]; > }; > > -#define PFLOG_HDRLEN sizeof(struct pfloghdr) > +#define PFLOG_HDRLEN BPF_WORDALIGN(offsetof(struct pfl= oghdr, 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 =3D PFLOG_HDRLEN; > + hdr.length =3D PFLOG_REAL_HDRLEN; > hdr.af =3D af; > hdr.action =3D rm->action; > hdr.reason =3D reason; > > It looks like I had assumed that the header was expected to be aligned to= 4 bytes, but it=E2=80=99s actually expected to be aligned to sizeof(long).= This should fix that. > > I=E2=80=99ve gone one further and added a dummy field to arrange the upda= ted struct so that Wireshark will work, essentially retaining its incorrect= assumption. It really should be fixed as well though. > > Kristof