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 *);
> #endif /* INET6 */
> + u_int16_t old;
>
> /* Clear IP_DF if no-df was requested */
> - if (flags & PFSTATE_NODF && af == AF_INET && h->ip_off & htons(IP_DF))
> + if (flags & PFSTATE_NODF && af == AF_INET && h->ip_off & htons(IP_DF)) {
> + old = h->ip_off;
> h->ip_off &= htons(~IP_DF);
> + pf_cksum_fixup(&h->ip_sum, old, h->ip_off, 0);
> + }
>
> /* Enforce a minimum ttl, may cause endless packet loops */
> - if (min_ttl && af == AF_INET && h->ip_ttl < min_ttl)
> + if (min_ttl && af == AF_INET && h->ip_ttl < min_ttl) {
> + old = h->ip_ttl;
> h->ip_ttl = min_ttl;
> + pf_cksum_fixup(&h->ip_sum, old, h->ip_off, 0);
> + }
> #ifdef INET6
> if (min_ttl && af == AF_INET6 && h6->ip6_hlim < min_ttl)
> h6->ip6_hlim = min_ttl;
> @@ -1661,8 +1668,11 @@ pf_scrub(struct mbuf *m, u_int16_t flags
>
> /* Enforce tos */
> if (flags & PFSTATE_SETTOS) {
> - if (af == AF_INET)
> + if (af == AF_INET) {
> + old = *(u_int16_t *)h;
> h->ip_tos = tos | (h->ip_tos & IPTOS_ECN_MASK);
> + pf_cksum_fixup(&h->ip_sum, old, *(u_int16_t *)h, 0);
> + }
> #ifdef INET6
> if (af == AF_INET6) {
> /* drugs are unable to explain such idiocy */
> @@ -1674,6 +1684,9 @@ pf_scrub(struct mbuf *m, u_int16_t flags
>
> /* random-id, but not for fragments */
> if (flags & PFSTATE_RANDOMID && af == AF_INET &&
> - !(h->ip_off & ~htons(IP_DF)))
> + !(h->ip_off & ~htons(IP_DF))) {
> + old = h->ip_id;
> h->ip_id = htons(ip_randomid());
> + pf_cksum_fixup(&h->ip_sum, old, h->ip_id, 0);
> + }
> }
> Index: sys/net/pfvar.h
> ===================================================================
> RCS file: /cvs/src/sys/net/pfvar.h,v
> retrieving revision 1.510
> diff -u -p -r1.510 pfvar.h
> --- sys/net/pfvar.h 3 Sep 2022 14:57:54 -0000 1.510
> +++ sys/net/pfvar.h 10 Oct 2022 03:22:06 -0000
> @@ -1745,6 +1745,8 @@ extern void pf_print_state(struct
> pf_
> extern void pf_print_flags(u_int8_t);
> extern void pf_addrcpy(struct pf_addr *, struct pf_addr *,
> sa_family_t);
> +extern void pf_cksum_fixup(u_int16_t *, u_int16_t,
> + u_int16_t, u_int8_t);
> void pf_rm_rule(struct pf_rulequeue *,
> struct pf_rule *);
> void pf_purge_rule(struct pf_rule *);
>
