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