Re: NPF: broken checksums

[Maxime Villard <max%m00nbsd.net@localhost>]

Le 03/04/2018 à 07:12, Maxime Villard a écrit :
> I'm having trouble understanding what's going on here:
>
> 188     if (npf_fetch_tcpopts(npc, &maxmss, &wscale) &&
> 189         nbuf_cksum_barrier(npc->npc_nbuf, mi->mi_di)) {
> 190         th = npc->npc_l4.tcp;
> 191         cksum = npf_fixup16_cksum(th->th_sum, mss, maxmss);
> 192         th->th_sum = cksum;
> 193     }
>
> Isn't there an inverted logic? We would like to manually fix up when there is
> no offloading, that is to say when !nbuf_cksum_barrier.
>
> In fact, if you look at nbuf_cksum_barrier, you can see that it always returns
> false for an inbound packet, and therefore we never update the checksum, so it
> is always wrong.
>
> As a result of that a "max-mss" procedure on "pass in" just doesn't work,
> because the packets systematically get kicked in tcp_input (the checksum
> validation fails).

While I'm around, there appears to be an unexpected behavior in
npf_fetch_tcpopts:

272 		if (mss) {
273 			if (*mss) {
274 				memcpy(nptr + 2, mss, sizeof(uint16_t));
275 			} else {
276 				memcpy(mss, nptr + 2, sizeof(uint16_t));
277 			}
278 		}

The function gets called with *mss=0 when we want to _read_ the mss in the
packet. We set *mss=value_on_the_packet, which is fine. However, if the packet
contains another TCPOPT_MAXSEG option, we come back here, and this time we
have *mss!=0; so we end up _patching_ the second option in the packet, with
the content of the first one.