Re: pf.conf / scrub resulting in invalid checksum

Hello,

On Mon, Oct 10, 2022 at 06:52:00AM +0200, Bjorn Ketelaars wrote:
</snip>
>
> (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?

diff reads good to me. change makes sense in my opinion.

OK sashan


>
>
> 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 *);
>