From f205efcf1856c2338989c5e76e2d23fde24fd532 Mon Sep 17 00:00:00 2001 From: Maisem Ali Date: Sat, 3 Aug 2024 08:57:31 -0700 Subject: [PATCH] net/packet/checksum: fix v6 NAT We were copying 12 out of the 16 bytes which meant that the 1:1 NAT required would only work if the last 4 bytes happened to match between the new and old address, something that our tests accidentally had. Fix it by copying the full 16 bytes and make the tests also verify the addr and use rand addresses. Updates #9511 Signed-off-by: Maisem Ali --- net/packet/checksum/checksum.go | 2 +- net/packet/checksum/checksum_test.go | 35 ++++++++++++++++++++++++---- tailcfg/tailcfg.go | 3 ++- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/net/packet/checksum/checksum.go b/net/packet/checksum/checksum.go index c49ae3626..547ea3a35 100644 --- a/net/packet/checksum/checksum.go +++ b/net/packet/checksum/checksum.go @@ -61,7 +61,7 @@ func UpdateDstAddr(q *packet.Parsed, dst netip.Addr) { b := q.Buffer() if dst.Is6() { v6 := dst.As16() - copy(b[24:36], v6[:]) + copy(b[24:40], v6[:]) updateV6PacketChecksums(q, old, dst) } else { v4 := dst.As4() diff --git a/net/packet/checksum/checksum_test.go b/net/packet/checksum/checksum_test.go index aeb030c1c..bf818743d 100644 --- a/net/packet/checksum/checksum_test.go +++ b/net/packet/checksum/checksum_test.go @@ -5,6 +5,7 @@ package checksum import ( "encoding/binary" + "math/rand/v2" "net/netip" "testing" @@ -94,7 +95,7 @@ func TestHeaderChecksumsV4(t *testing.T) { } func TestNatChecksumsV6UDP(t *testing.T) { - a1, a2 := netip.MustParseAddr("a::1"), netip.MustParseAddr("b::1") + a1, a2 := randV6Addr(), randV6Addr() // Make a fake UDP packet with 32 bytes of zeros as the datagram payload. b := header.IPv6(make([]byte, header.IPv6MinimumSize+header.UDPMinimumSize+32)) @@ -124,25 +125,43 @@ func TestNatChecksumsV6UDP(t *testing.T) { } // Parse the packet. - var p packet.Parsed + var p, p2 packet.Parsed p.Decode(b) t.Log(p.String()) // Update the source address of the packet to be the same as the dest. UpdateSrcAddr(&p, a2) + p2.Decode(p.Buffer()) + if p2.Src.Addr() != a2 { + t.Fatalf("got %v, want %v", p2.Src, a2) + } if !udp.IsChecksumValid(tcpip.AddrFrom16Slice(a2.AsSlice()), tcpip.AddrFrom16Slice(a2.AsSlice()), checksum.Checksum(b.Payload()[header.UDPMinimumSize:], 0)) { t.Fatal("incorrect checksum after updating source address") } // Update the dest address of the packet to be the original source address. UpdateDstAddr(&p, a1) + p2.Decode(p.Buffer()) + if p2.Dst.Addr() != a1 { + t.Fatalf("got %v, want %v", p2.Dst, a1) + } if !udp.IsChecksumValid(tcpip.AddrFrom16Slice(a2.AsSlice()), tcpip.AddrFrom16Slice(a1.AsSlice()), checksum.Checksum(b.Payload()[header.UDPMinimumSize:], 0)) { t.Fatal("incorrect checksum after updating destination address") } } +func randV6Addr() netip.Addr { + a1, a2 := rand.Int64(), rand.Int64() + return netip.AddrFrom16([16]byte{ + byte(a1 >> 56), byte(a1 >> 48), byte(a1 >> 40), byte(a1 >> 32), + byte(a1 >> 24), byte(a1 >> 16), byte(a1 >> 8), byte(a1), + byte(a2 >> 56), byte(a2 >> 48), byte(a2 >> 40), byte(a2 >> 32), + byte(a2 >> 24), byte(a2 >> 16), byte(a2 >> 8), byte(a2), + }) +} + func TestNatChecksumsV6TCP(t *testing.T) { - a1, a2 := netip.MustParseAddr("a::1"), netip.MustParseAddr("b::1") + a1, a2 := randV6Addr(), randV6Addr() // Make a fake TCP packet with no payload. b := header.IPv6(make([]byte, header.IPv6MinimumSize+header.TCPMinimumSize)) @@ -178,18 +197,26 @@ func TestNatChecksumsV6TCP(t *testing.T) { } // Parse the packet. - var p packet.Parsed + var p, p2 packet.Parsed p.Decode(b) t.Log(p.String()) // Update the source address of the packet to be the same as the dest. UpdateSrcAddr(&p, a2) + p2.Decode(p.Buffer()) + if p2.Src.Addr() != a2 { + t.Fatalf("got %v, want %v", p2.Src, a2) + } if !tcp.IsChecksumValid(tcpip.AddrFrom16Slice(a2.AsSlice()), tcpip.AddrFrom16Slice(a2.AsSlice()), 0, 0) { t.Fatal("incorrect checksum after updating source address") } // Update the dest address of the packet to be the original source address. UpdateDstAddr(&p, a1) + p2.Decode(p.Buffer()) + if p2.Dst.Addr() != a1 { + t.Fatalf("got %v, want %v", p2.Dst, a1) + } if !tcp.IsChecksumValid(tcpip.AddrFrom16Slice(a2.AsSlice()), tcpip.AddrFrom16Slice(a1.AsSlice()), 0, 0) { t.Fatal("incorrect checksum after updating destination address") } diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 5d639dd54..5a06c89ff 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -146,7 +146,8 @@ type CapabilityVersion int // - 101: 2024-07-01: Client supports SSH agent forwarding when handling connections with /bin/su // - 102: 2024-07-12: NodeAttrDisableMagicSockCryptoRouting support // - 103: 2024-07-24: Client supports NodeAttrDisableCaptivePortalDetection -const CurrentCapabilityVersion CapabilityVersion = 103 +// - 104: 2024-08-03: SelfNodeV6MasqAddrForThisPeer now works +const CurrentCapabilityVersion CapabilityVersion = 104 type StableID string