Re: pf.conf / scrub resulting in invalid checksum

On Sun 09/10/2022 17:10, Björn Ketelaars wrote:
> I'm using mcast-proxy from ports as multicast routing proxy for use with
> my ISP's iptv platform. After some setting up i noticed from
> mcast-proxy's logging that all incoming packets are dropped because of
> IP invalid checksums [0]. At first I believed this was the result of
> hardware checksum offloading. However, after some more digging I found
> that my pf.conf was to blame, specifically:
>
> match inet scrub (max-mss 1460, no-df, random-id)
>
> Removing `no-df` and `random-id` as argument causes mcast-proxy to
> accept all incoming IGMP packets resulting in a working solution.
>
> After grepping sys/net/pf* i think that pf_test() calls pf_scrub(),
> which changes the fragment offset field and/or the identification field
> thus changing the packet header. However, it seems that the checksum of
> a changed packet is not updated.
>
> Can anyone shed a light on the above?
>
>
> [0] https://github.com/Esdenera/mcast-proxy/blob/master/usr.sbin/mcast-proxy/mcast-proxy.c#L539-L549

(reply also send to tech@)

In 2011 henning@ removed fiddling with the ip checksum of normalised
packets in sys/net/pf_norm.c (r1.131). Rationale was that the checksum
is always recalculated in all output paths anyway. In 2016 procter@
reintroduced checksum modification to preserve end-to-end checksums
(r1.189 of sys/net/pf_norm.c). Although I'm not sure, it seems as if
somewhere in that timeslot checksum recalculation of normalised packets
was broken.

Issue got caught as net/mcast-proxy strictly adheres to RFC2236, which
states that "When receiving packets, the checksum MUST be verified
before processing a packet". After scrubbing a packet the checksum
becomes invalid thus failing verification by net/mcast-proxy.

I found two workarounds:
1.) rip out checksum verification from net/mcast-proxy;
2.) don't scrub packets with, e.g., id-random and/or no-df set.

However, proposed solution is to fix this in pf. Diff below fixes the
issue at hand.

Comments/OK?


Index: sys/net/pf.c
===================================================================
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.1140
diff -u -p -r1.1140 pf.c
--- sys/net/pf.c 3 Sep 2022 19:22:19 -0000 1.1140
+++ sys/net/pf.c 10 Oct 2022 03:22:06 -0000
@@ -164,7 +164,7 @@ void pf_add_threshold(struct pf_thres
int pf_check_threshold(struct pf_threshold *);
int pf_check_tcp_cksum(struct mbuf *, int, int,
sa_family_t);
-static __inline void pf_cksum_fixup(u_int16_t *, u_int16_t, u_int16_t,
+__inline void pf_cksum_fixup(u_int16_t *, u_int16_t, u_int16_t,
u_int8_t);
void pf_cksum_fixup_a(u_int16_t *, const struct pf_addr *,
const struct pf_addr *, sa_family_t, u_int8_t);
@@ -1937,7 +1937,7 @@ pf_addr_wrap_neq(struct pf_addr_wrap *aw
* Note: this serves also as a reduction step for at most one add (as the
* trailing mod 2^16 prevents further reductions by destroying carries).
*/
-static __inline void
+__inline void
pf_cksum_fixup(u_int16_t *cksum, u_int16_t was, u_int16_t now,
u_int8_t proto)
{
Index: sys/net/pf_norm.c
===================================================================
RCS file: /cvs/src/sys/net/pf_norm.c,v
retrieving revision 1.224
diff -u -p -r1.224 pf_norm.c
--- sys/net/pf_norm.c 22 Aug 2022 20:35:39 -0000 1.224
+++ sys/net/pf_norm.c 10 Oct 2022 03:22:06 -0000
@@ -1646,14 +1646,21 @@ pf_scrub(struct mbuf *m, u_int16_t flags
#ifdef INET6
struct ip6_hdr *h6 = mtod(m, struct ip6_hdr *);