From 5cb0a00b11f94596d544ef208ffad55de5f7ea69 Mon Sep 17 00:00:00 2001 From: salman Date: Tue, 27 Jun 2023 18:26:32 +0000 Subject: [PATCH] WIP wgengine: inject ICMP PTB for oversize packets --- net/packet/icmp4.go | 3 +- net/packet/icmp6.go | 1 + wgengine/magicsock/magicsock.go | 6 ++++ wgengine/userspace.go | 56 +++++++++++++++++++++++++++++++++ 4 files changed, 65 insertions(+), 1 deletion(-) diff --git a/net/packet/icmp4.go b/net/packet/icmp4.go index 730239abf..9375aebd6 100644 --- a/net/packet/icmp4.go +++ b/net/packet/icmp4.go @@ -45,7 +45,8 @@ func (t ICMP4Type) String() string { type ICMP4Code uint8 const ( - ICMP4NoCode ICMP4Code = 0 + ICMP4NoCode ICMP4Code = 0x00 + ICMP4FragmentationNeeded = 0x04 ) // ICMP4Header is an IPv4+ICMPv4 header. diff --git a/net/packet/icmp6.go b/net/packet/icmp6.go index 518746b55..d74667c6d 100644 --- a/net/packet/icmp6.go +++ b/net/packet/icmp6.go @@ -20,6 +20,7 @@ type ICMP6Type uint8 const ( ICMP6Unreachable ICMP6Type = 1 + ICMP6PacketTooBig ICMP6Type = 2 ICMP6TimeExceeded ICMP6Type = 3 ICMP6EchoRequest ICMP6Type = 128 ICMP6EchoReply ICMP6Type = 129 diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index 5874b9f35..8cbe35a15 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -3247,6 +3247,12 @@ func (c *Conn) shouldDoPeriodicReSTUNLocked() bool { return true } +// PathMTU returns the path MTU to the peer at dst (tailscale address) +// TODO unstub +func (c *Conn) PathMTU(dst netip.Addr) int { + return 1280 +} + func (c *Conn) onPortMapChanged() { c.ReSTUN("portmap-changed") } // ReSTUN triggers an address discovery. diff --git a/wgengine/userspace.go b/wgengine/userspace.go index e29e20dae..2bc9fcb23 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -7,6 +7,7 @@ import ( "bufio" "context" crand "crypto/rand" + "encoding/binary" "errors" "fmt" "io" @@ -476,6 +477,61 @@ func (e *userspaceEngine) handleLocalPackets(p *packet.Parsed, t *tstun.Wrapper) } } + // TODO IPv4 is 20 bytes but IPv6 is 40 - move this into magicsock where we know + // which we're using. + // TODO consts to avoid numbers. + const tailscaleOverhead = 40 + 8 + 32 // IP + UDP + WireGuard + pmtu := e.magicConn.PathMTU(p.Dst.Addr()) + if len(p.Buffer())+tailscaleOverhead > pmtu { + var icmph packet.Header + var payload []byte + if p.Src.Addr().Is4() { + // From https://www.ietf.org/rfc/rfc1191.txt + // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + // | Type = 3 | Code = 4 | Checksum | + // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + // | unused = 0 | Next-Hop MTU | + // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + // | Internet Header + 64 bits of Original Datagram Data | + // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + icmph = packet.ICMP4Header{ + IP4Header: packet.IP4Header{ + IPProto: ipproto.ICMPv4, + Src: p.Dst.Addr(), + Dst: p.Src.Addr(), + }, + Type: packet.ICMP4Unreachable, + Code: packet.ICMP4FragmentationNeeded, + } + payload = make([]byte, 4+20+8) + binary.BigEndian.PutUint32(payload, uint32(pmtu)) + copy(payload[4:], p.Buffer()[:len(payload)-4]) + } else { + // https://www.rfc-editor.org/rfc/rfc4443.html#section-3.2 + // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + // | Type | Code | Checksum | + // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + // | MTU | + // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + // | As much of invoking packet | + // + as possible without the ICMPv6 packet + + // | exceeding the minimum IPv6 MTU [IPv6] | + icmph = packet.ICMP6Header{ + IP6Header: packet.IP6Header{ + IPProto: ipproto.ICMPv6, + Src: p.Dst.Addr(), + Dst: p.Src.Addr(), + }, + Type: packet.ICMP6PacketTooBig, + Code: packet.ICMP6NoCode, + } + payload = make([]byte, 4+40+8) // says as much of invoking packet, but headers are enough. + binary.BigEndian.PutUint32(payload, uint32(pmtu)) + copy(payload[4:], p.Buffer()[:len(payload)-4]) + } + e.tundev.InjectInboundCopy(packet.Generate(icmph, payload)) // TODO use InjectInboundDirect + return filter.Drop + } return filter.Accept }