Hi Richard, Thanks for the patch. I had problems parsing some terminology in your description, so I thought I'd lay out my understanding of the matter, and you can let me know whether or not this corresponds with what you had in mind:
- On egress, we must compute the packet checksum, because it may well be forwarded by the receiving end after decapsulation. That doesn't concern this patch, however. - On ingress, we've already checked the poly1305 sum, so we have no doubt that the packet has arrived without corruption. - Therefore, it's not necessary to check the IP checksum on ingress because: * If the packet originated on the peer that did the encapsulation, there's no chance for corruption; * If the packet did not originate on the peer that did the encapsulation, it was that peer's responsibility to drop it if the checksum was wrong; * If the packet does have an incorrect checksum, because the originating peer did not check it, and we forward it along, the machine we forward it to will drop it. It seemed like from your message that you had a case in mind in which it actually would be necessary to check the IP checksum on ingress, but I didn't quite divine what you had in mind. Jason On Fri, Jun 26, 2020 at 10:03 PM <richard.n.proc...@gmail.com> wrote: > > Hi, > > On its receive path, wg(4) uses the same mbuf for both the encrypted > capsule and its encapsulated packet, which it passes up to the stack. We > must therefore clear this mbuf's checksum status flags, as although the > capsule may have been subject to hardware offload, its encapsulated packet > was not. > > This ensures that the transport checksums of packets bound for local > delivery are verified. That is necessary because, although the tunnel > provides stronger integrity checks, the tunnel endpoints and the > transport endpoints needn't coincide. > > However, as the network and tunnel endpoints _do_ conincide, it remains > unncessary to check the per-hop IPv4 checksum. > > ok? > > Index: net/if_wg.c > =================================================================== > RCS file: /cvs/src/sys/net/if_wg.c,v > retrieving revision 1.7 > diff -u -p -u -p -r1.7 if_wg.c > --- net/if_wg.c 23 Jun 2020 10:03:49 -0000 1.7 > +++ net/if_wg.c 27 Jun 2020 02:48:37 -0000 > @@ -1660,14 +1660,10 @@ wg_decap(struct wg_softc *sc, struct mbu > goto error; > } > > - /* > - * We can mark incoming packet csum OK. We mark all flags OK > - * irrespective to the packet type. > - */ > - m->m_pkthdr.csum_flags |= (M_IPV4_CSUM_IN_OK | M_TCP_CSUM_IN_OK | > - M_UDP_CSUM_IN_OK | M_ICMP_CSUM_IN_OK); > - m->m_pkthdr.csum_flags &= ~(M_IPV4_CSUM_IN_BAD | M_TCP_CSUM_IN_BAD | > - M_UDP_CSUM_IN_BAD | M_ICMP_CSUM_IN_BAD); > + /* tunneled packet was not offloaded */ > + m->m_pkthdr.csum_flags = 0; > + /* optimise: the tunnel provided a stronger integrity check */ > + m->m_pkthdr.csum_flags |= M_IPV4_CSUM_IN_OK; > > m->m_pkthdr.ph_ifidx = sc->sc_if.if_index; > m->m_pkthdr.ph_rtableid = sc->sc_if.if_rdomain;
