From 1f51bb68915eec6ba7e44a4fb184ff68b069d9d2 Mon Sep 17 00:00:00 2001 From: Maisem Ali Date: Tue, 14 May 2024 13:23:34 -0400 Subject: [PATCH] net/tstun: do SNAT after filterPacketOutboundToWireGuard In a configuration where the local node (ip1) has a different IP (ip2) that it uses to communicate with a peer (ip3) we would do UDP flow tracking on the `ip2->ip3` tuple. When we receive the response from the peer `ip3->ip2` we would dnat it back to `ip3->ip1` which would then not match the flow track state and the packet would get dropped. To fix this, we should do flow tracking on the `ip1->ip3` tuple instead of `ip2->ip3` which requires doing SNAT after the running filterPacketOutboundToWireGuard. Updates tailscale/corp#19971, tailscale/corp#8020 Signed-off-by: Maisem Ali --- net/tstun/wrap.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/net/tstun/wrap.go b/net/tstun/wrap.go index ebf06527a..06296815d 100644 --- a/net/tstun/wrap.go +++ b/net/tstun/wrap.go @@ -915,8 +915,6 @@ func (t *Wrapper) Read(buffs [][]byte, sizes []int, offset int) (int, error) { for _, data := range res.data { p.Decode(data[res.dataOffset:]) - pc.snat(p) - if m := t.destIPActivity.Load(); m != nil { if fn := m[p.Dst.Addr()]; fn != nil { fn() @@ -932,6 +930,10 @@ func (t *Wrapper) Read(buffs [][]byte, sizes []int, offset int) (int, error) { continue } } + + // Make sure to do SNAT after filtering, so that any flow tracking in + // the filter sees the original source address. See #12133. + pc.snat(p) n := copy(buffs[buffsPos][offset:], p.Buffer()) if n != len(data)-res.dataOffset { panic(fmt.Sprintf("short copy: %d != %d", n, len(data)-res.dataOffset))