diff --git a/disco/disco.go b/disco/disco.go index 8badad2dc895f..338381eb43f87 100644 --- a/disco/disco.go +++ b/disco/disco.go @@ -28,6 +28,7 @@ import ( "go4.org/mem" "golang.org/x/crypto/nacl/box" + "tailscale.com/envknob" "tailscale.com/types/key" ) @@ -52,15 +53,26 @@ const v0 = byte(0) // v1 Ping and Pong are padded as follows. CallMeMaybe is still on v0 and unpadded. const v1 = byte(1) -// paddedPayloadLen is the desired length we want to pad Ping and Pong payloads -// to so that they are the maximum size of a Wireguard packet we would -// subsequently send. This ensures that any UDP paths we discover will actually -// support the packet sizes the net stack will send over those paths. Any peers -// behind a small-MTU link will have to depend on DERP. +// paddedPayloadLen returns the desired Ping/Pong payload size. The probe is +// padded to the largest WireGuard packet the configured inner MTU can produce, +// so a discovered UDP path can also carry subsequent tailnet traffic. // c.f. https://github.com/coder/coder/issues/15523 -// Our inner IP packets can be up to 1280 bytes, with the Wireguard header of -// 30 bytes, that is 1310. The final 2 is the inner payload header's type and version. -const paddedPayloadLen = 1310 - len(Magic) - keyLen - NonceLen - box.Overhead - 2 +func paddedPayloadLen() int { + const ( + defaultInnerMTU = 1280 + wireGuardOverhead = 30 + wrapperLen = len(Magic) + keyLen + NonceLen + box.Overhead + 2 + minPayloadLen = 12 + keyLen // Ping TxID plus optional node key. + maxUDPPayloadLen = 65507 + ) + + innerMTU := uint(defaultInnerMTU) + if configuredMTU, ok := envknob.LookupUintSized("TS_DEBUG_MTU", 10, 32); ok { + innerMTU = configuredMTU + } + packetLen := min(int(innerMTU)+wireGuardOverhead, maxUDPPayloadLen) + return max(packetLen-wrapperLen, minPayloadLen) +} var errShort = errors.New("short message") @@ -135,7 +147,7 @@ type Ping struct { func (m *Ping) AppendMarshal(b []byte) []byte { hasKey := !m.NodeKey.IsZero() - ret, d := appendMsgHeader(b, TypePing, v1, paddedPayloadLen) + ret, d := appendMsgHeader(b, TypePing, v1, paddedPayloadLen()) n := copy(d, m.TxID[:]) if hasKey { m.NodeKey.AppendTo(d[:n]) @@ -227,7 +239,7 @@ type Pong struct { const pongLen = 12 + 16 + 2 func (m *Pong) AppendMarshal(b []byte) []byte { - ret, d := appendMsgHeader(b, TypePong, v1, paddedPayloadLen) + ret, d := appendMsgHeader(b, TypePong, v1, paddedPayloadLen()) d = d[copy(d, m.TxID[:]):] ip16 := m.Src.Addr().As16() d = d[copy(d, ip16[:]):] diff --git a/disco/disco_test.go b/disco/disco_test.go index 475203e0aa1d2..b131352ba112b 100644 --- a/disco/disco_test.go +++ b/disco/disco_test.go @@ -76,8 +76,8 @@ func TestMarshalAndParse(t *testing.T) { if !ok { t.Fatalf("didn't start with foo: got %q", got) } - // CODER: 1310 is max size of a Wireguard packet we will send. - expectedLen := 1310 - len(Magic) - keyLen - NonceLen - box.Overhead + // CODER: probe size follows the configured inner MTU. + expectedLen := paddedPayloadLen() + 2 switch tt.m.(type) { case *Ping: if len(got) != expectedLen { @@ -106,6 +106,31 @@ func TestMarshalAndParse(t *testing.T) { } } +func TestPaddedPayloadLenFollowsDebugMTU(t *testing.T) { + const ( + wireGuardOverhead = 30 + wrapperLen = len(Magic) + keyLen + NonceLen + box.Overhead + 2 + ) + tests := []struct { + name string + mtu string + want int + }{ + {name: "default", want: 1280 + wireGuardOverhead}, + {name: "nested path", mtu: "1182", want: 1182 + wireGuardOverhead}, + {name: "larger MTU", mtu: "1420", want: 1420 + wireGuardOverhead}, + {name: "cap at maximum UDP payload", mtu: "65536", want: 65507}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("TS_DEBUG_MTU", tt.mtu) + if got := paddedPayloadLen() + wrapperLen; got != tt.want { + t.Fatalf("padded packet length = %d, want %d", got, tt.want) + } + }) + } +} + func TestParsePingPongV0(t *testing.T) { tests := []struct { name string diff --git a/wgengine/netstack/debug_mtu.go b/wgengine/netstack/debug_mtu.go new file mode 100644 index 0000000000000..f62b21fae2eca --- /dev/null +++ b/wgengine/netstack/debug_mtu.go @@ -0,0 +1,96 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +package netstack + +import ( + "encoding/binary" + + "gvisor.dev/gvisor/pkg/tcpip/header" + "gvisor.dev/gvisor/pkg/tcpip/stack" + "gvisor.dev/gvisor/pkg/tcpip/transport/tcp" + "tailscale.com/net/tstun" +) + +func debugTCPMSS() uint16 { + mtu := int(tstun.DefaultMTU()) + if mtu >= int(netstackLinkMTU()) { + return 0 + } + return tcpMSSForMTU(mtu) +} + +func tcpMSSForMTU(mtu int) uint16 { + const ipv6AndTCPHeaderLen = 40 + header.TCPMinimumSize + mss := mtu - ipv6AndTCPHeaderLen + if mss < header.TCPMinimumMSS || mss > int(^uint16(0)) { + return 0 + } + return uint16(mss) +} + +// clampDebugTCPMSS rewrites the MSS advertised by outbound TCP SYN and SYN-ACK +// packets when TS_DEBUG_MTU is below the IPv6 link minimum. This keeps the +// logical link standards-compliant while preventing TCP from producing inner +// packets larger than the explicitly configured nested-path budget. +func clampDebugTCPMSS(pkt *stack.PacketBuffer) { + if pkt.TransportProtocolNumber != tcp.ProtocolNumber { + return + } + mss := debugTCPMSS() + if mss == 0 { + return + } + clampTCPMSSOption(pkt.TransportHeader().Slice(), mss) +} + +func clampTCPMSSOption(tcpHeader []byte, maxMSS uint16) bool { + if len(tcpHeader) < header.TCPMinimumSize || tcpHeader[13]&byte(header.TCPFlagSyn) == 0 { + return false + } + + headerLen := int(tcpHeader[12]>>4) * 4 + if headerLen < header.TCPMinimumSize || headerLen > len(tcpHeader) { + return false + } + + for i := header.TCPMinimumSize; i < headerLen; { + switch tcpHeader[i] { + case header.TCPOptionEOL: + return false + case header.TCPOptionNOP: + i++ + continue + } + + if i+1 >= headerLen { + return false + } + optionLen := int(tcpHeader[i+1]) + if optionLen < 2 || i+optionLen > headerLen { + return false + } + if tcpHeader[i] == header.TCPOptionMSS && optionLen == header.TCPOptionMSSLength { + oldMSS := binary.BigEndian.Uint16(tcpHeader[i+2 : i+4]) + if oldMSS <= maxMSS { + return false + } + + oldChecksum := binary.BigEndian.Uint16(tcpHeader[16:18]) + binary.BigEndian.PutUint16(tcpHeader[i+2:i+4], maxMSS) + binary.BigEndian.PutUint16(tcpHeader[16:18], updateChecksumWord(oldChecksum, oldMSS, maxMSS)) + return true + } + i += optionLen + } + return false +} + +// updateChecksumWord applies RFC 1624's incremental one's-complement checksum +// update for a single aligned 16-bit field. +func updateChecksumWord(checksum, old, new uint16) uint16 { + sum := uint32(^checksum) + uint32(^old) + uint32(new) + sum = (sum & 0xffff) + (sum >> 16) + sum = (sum & 0xffff) + (sum >> 16) + return ^uint16(sum) +} diff --git a/wgengine/netstack/debug_mtu_test.go b/wgengine/netstack/debug_mtu_test.go new file mode 100644 index 0000000000000..4764029128175 --- /dev/null +++ b/wgengine/netstack/debug_mtu_test.go @@ -0,0 +1,81 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +package netstack + +import ( + "encoding/binary" + "testing" + + "gvisor.dev/gvisor/pkg/tcpip/checksum" + "gvisor.dev/gvisor/pkg/tcpip/header" +) + +func TestTCPMSSForMTU(t *testing.T) { + if got, want := tcpMSSForMTU(1182), uint16(1122); got != want { + t.Fatalf("tcpMSSForMTU(1182) = %d, want %d", got, want) + } + if got := tcpMSSForMTU(0); got != 0 { + t.Fatalf("tcpMSSForMTU(0) = %d, want disabled", got) + } +} + +func TestDebugTCPMSS(t *testing.T) { + tests := []struct { + name string + mtu string + want uint16 + }{ + {name: "default"}, + {name: "nested path", mtu: "1182", want: 1122}, + {name: "IPv6 minimum", mtu: "1280"}, + {name: "larger MTU", mtu: "1420"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("TS_DEBUG_MTU", tt.mtu) + if got := debugTCPMSS(); got != tt.want { + t.Fatalf("debugTCPMSS() = %d, want %d", got, tt.want) + } + }) + } +} + +func TestClampTCPMSSOption(t *testing.T) { + const pseudoHeaderChecksum = uint16(0x2345) + tcpHeader := make([]byte, header.TCPMinimumSize+header.TCPOptionMSSLength) + binary.BigEndian.PutUint16(tcpHeader[0:2], 12345) + binary.BigEndian.PutUint16(tcpHeader[2:4], 22) + tcpHeader[12] = byte(len(tcpHeader)/4) << 4 + tcpHeader[13] = byte(header.TCPFlagSyn) + tcpHeader[20] = header.TCPOptionMSS + tcpHeader[21] = header.TCPOptionMSSLength + binary.BigEndian.PutUint16(tcpHeader[22:24], 1220) + binary.BigEndian.PutUint16(tcpHeader[16:18], ^checksum.Checksum(tcpHeader, pseudoHeaderChecksum)) + + if changed := clampTCPMSSOption(tcpHeader, 1122); !changed { + t.Fatal("clampTCPMSSOption did not change an oversized MSS") + } + if got, want := binary.BigEndian.Uint16(tcpHeader[22:24]), uint16(1122); got != want { + t.Fatalf("MSS = %d, want %d", got, want) + } + if got := checksum.Checksum(tcpHeader, pseudoHeaderChecksum); got != 0xffff { + t.Fatalf("updated TCP checksum sum = %#04x, want 0xffff", got) + } + if changed := clampTCPMSSOption(tcpHeader, 1122); changed { + t.Fatal("clampTCPMSSOption changed an already safe MSS") + } +} + +func TestClampTCPMSSOptionIgnoresNonSYN(t *testing.T) { + tcpHeader := make([]byte, header.TCPMinimumSize+header.TCPOptionMSSLength) + tcpHeader[12] = byte(len(tcpHeader)/4) << 4 + tcpHeader[13] = byte(header.TCPFlagAck) + tcpHeader[20] = header.TCPOptionMSS + tcpHeader[21] = header.TCPOptionMSSLength + binary.BigEndian.PutUint16(tcpHeader[22:24], 1220) + + if changed := clampTCPMSSOption(tcpHeader, 1122); changed { + t.Fatal("clampTCPMSSOption changed a non-SYN packet") + } +} diff --git a/wgengine/netstack/mtu_test.go b/wgengine/netstack/mtu_test.go new file mode 100644 index 0000000000000..3de4d1def0655 --- /dev/null +++ b/wgengine/netstack/mtu_test.go @@ -0,0 +1,27 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +package netstack + +import "testing" + +func TestNetstackLinkMTU(t *testing.T) { + tests := []struct { + name string + mtu string + want uint32 + }{ + {name: "default", want: minimumIPv6LinkMTU}, + {name: "below IPv6 minimum", mtu: "1182", want: minimumIPv6LinkMTU}, + {name: "IPv6 minimum", mtu: "1280", want: minimumIPv6LinkMTU}, + {name: "above IPv6 minimum", mtu: "1420", want: 1420}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("TS_DEBUG_MTU", tt.mtu) + if got := netstackLinkMTU(); got != tt.want { + t.Fatalf("netstackLinkMTU() = %d, want %d", got, tt.want) + } + }) + } +} diff --git a/wgengine/netstack/netstack.go b/wgengine/netstack/netstack.go index d17d9b7ccb7fe..c7d61aa0195f2 100644 --- a/wgengine/netstack/netstack.go +++ b/wgengine/netstack/netstack.go @@ -148,6 +148,15 @@ const nicID = 1 // one day making the MTU more dynamic. const maxUDPPacketSize = 1500 +// minimumIPv6LinkMTU is the minimum link MTU permitted by RFC 8200. +// Netstack always enables IPv6, so its link endpoint must not inherit a lower +// TS_DEBUG_MTU value from the underlying TUN configuration. +const minimumIPv6LinkMTU = 1280 + +func netstackLinkMTU() uint32 { + return max(tstun.DefaultMTU(), minimumIPv6LinkMTU) +} + const ( megabytes = 1024 * 1024 // recvBufSize is the size in bytes for TCP receive buffers. 6MiB is the usual maximum in @@ -241,7 +250,7 @@ func Create(logf logger.Logf, tundev *tstun.Wrapper, e wgengine.Engine, mc *magi return nil, fmt.Errorf("could not set max retries: %v", tcpipErr) } - linkEP := NewEndpoint(512, tstun.DefaultMTU(), "") + linkEP := NewEndpoint(512, netstackLinkMTU(), "") if tcpipProblem := ipstack.CreateNIC(nicID, linkEP); tcpipProblem != nil { return nil, fmt.Errorf("could not create netstack NIC: %v", tcpipProblem) } @@ -542,6 +551,8 @@ func (ns *Impl) inject() { continue } + clampDebugTCPMSS(pkt) + if debugPackets { ns.logf("[v2] packet Write out: % x", stack.PayloadSince(pkt.NetworkHeader())) }