path: root/src/send.c
diff options
authorAndrejs Hanins <ahanins@gmail.com>2018-10-27 03:16:00 +0200
committerJason A. Donenfeld <Jason@zx2c4.com>2018-10-27 14:20:00 +0200
commit724ef27d132a7a6a8ea017e597cccd3997561696 (patch)
treecfaa87b9c0a410bd0930f69e882592192c02feb6 /src/send.c
parentreceive: assume all levels have been checksumed, not just outer (diff)
send: calculate inner checksums for all protocols
I'm using GRE tunnel (transparent Ethernet bridging flavor) over WireGuard interface to be able to bridge L2 network segments. The typical protocol chain looks like this IP->GRE->EthernetHeader->IP->UDP. UDP here is the packet sent from the L2 network segment which is tunneled using GRE over Wireguard. Indeed, there is a checksum inside UDP header which is, as a rule, kept partially calculated while packet travels through network stack and outer protocols are added until the packet reaches WG device which exposes NETIF_F_HW_CSUM feature meaning it can handle checksum offload for all protocols. But the problem here is that skb_checksum_setup called from encrypt_packet handles only TCP/UDP protocols under top level IP, but in my case there is a GRE protocol there, so skb_checksum_help is not called and packet continues its life with unfinished (broken) checksum and gets encrypted as-is. When such packet is received by other side and reaches L2 networks it's seen there with a broken checksum inside the UDP header. The fact that Wireguard on the receiving side sets skb->ip_summed to CHECKSUM_UNNECESSARY partially mitigates the problem by telling network stack on the receiving side that validation of the checksum is not necessary, so local TCP stack, for example, works fine. But it doesn't help in situations when packet needs to be forwarded further (sent out from the box). In this case there is no way we can tell next hop that checksum verification for this packet is not necessary, we just send it out with bad checksum and packet gets dropped on the next hop box. I think the issue of the original code was the wrong usage of skb_checksum_setup, simply because it's not needed in this case. Instead, we can just rely on ip_summed skb field to see if partial checksum needs to be finalized or not. Note that many other drivers in kernel follow this approach. In summary: - skb_checksum_setup can only handle TCP/UDP protocols under top level IP header, packets with other protocols (like GRE) are sent out by Wireguard with unfinished partial checksums which causes problems on receiving side (bad checksums). - encrypt_packet gets skb prepared by network stack, so there is no need to setup the checksum from scratch, but just perform hw checksum offload using software helper skb_checksum_help for packet which explicitly require it as denoted by CHECKSUM_PARTIAL. Signed-off-by: Andrejs Hanins <ahanins@gmail.com>
Diffstat (limited to '')
1 files changed, 4 insertions, 5 deletions
diff --git a/src/send.c b/src/send.c
index 06996f0..dc466ae 100644
--- a/src/send.c
+++ b/src/send.c
@@ -187,11 +187,10 @@ static bool encrypt_packet(struct sk_buff *skb, struct noise_keypair *keypair,
if (unlikely(skb_cow_head(skb, DATA_PACKET_HEAD_ROOM) < 0))
return false;
- /* We have to remember to add the checksum to the innerpacket, in case
- * the receiver forwards it.
- */
- if (likely(!skb_checksum_setup(skb, true)))
- skb_checksum_help(skb);
+ /* Finalize checksum calculation for the inner packet, if required. */
+ if (unlikely(skb->ip_summed == CHECKSUM_PARTIAL &&
+ skb_checksum_help(skb)))
+ return false;
/* Only after checksumming can we safely add on the padding at the end
* and the header.