diff --git a/util/linuxfw/iptables_runner.go b/util/linuxfw/iptables_runner.go index 868114ac8..52b9a29fb 100644 --- a/util/linuxfw/iptables_runner.go +++ b/util/linuxfw/iptables_runner.go @@ -322,7 +322,16 @@ func (i *iptablesRunner) DNATWithLoadBalancer(origDst netip.Addr, dsts []netip.A func (i *iptablesRunner) ClampMSSToPMTU(tun string, addr netip.Addr) error { table := i.getIPTByAddr(addr) - return table.Append("mangle", "FORWARD", "-o", tun, "-p", "tcp", "--tcp-flags", "SYN,RST", "SYN", "-j", "TCPMSS", "--clamp-mss-to-pmtu") + // Clamp MSS on forwarded TCP handshakes in both directions: the SYN + // leaving via tun towards the tailnet peer, and the SYN-ACK arriving on + // tun and being forwarded back out towards the originating endpoint. A + // single -o tun rule only clamps one side of the handshake, leaving the + // endpoint on the other side advertising an MSS that is too large for the + // tun MTU, which black-holes large segments when PMTU discovery is broken. + if err := table.Append("mangle", "FORWARD", "-o", tun, "-p", "tcp", "--tcp-flags", "SYN,RST", "SYN", "-j", "TCPMSS", "--clamp-mss-to-pmtu"); err != nil { + return err + } + return table.Append("mangle", "FORWARD", "-i", tun, "-p", "tcp", "--tcp-flags", "SYN,RST", "SYN", "-j", "TCPMSS", "--clamp-mss-to-pmtu") } // addBase6 adds some basic IPv6 processing rules to be diff --git a/util/linuxfw/nftables_runner.go b/util/linuxfw/nftables_runner.go index 107eab951..dd0961883 100644 --- a/util/linuxfw/nftables_runner.go +++ b/util/linuxfw/nftables_runner.go @@ -248,11 +248,16 @@ func (n *nftablesRunner) EnsureSNATForDst(src, dst netip.Addr) error { // ClampMSSToPMTU ensures that all packets with TCP flags (SYN, ACK, RST) set // being forwarded via the given interface (tun) have MSS set to - 40 (IP and TCP headers). This can be useful if this tailscale -// instance is expected to run as a forwarding proxy, forwarding packets from an -// endpoint with higher MTU in an environment where path MTU discovery is -// expected to not work (such as the proxies created by the Tailscale Kubernetes -// operator). ClamMSSToPMTU creates a new base-chain ts-clamp in the filter +// interface> - 40 (IP and TCP headers). It clamps both directions of a +// forwarded TCP handshake: the SYN leaving via tun and the SYN-ACK arriving on +// tun. Clamping only the output direction would leave the endpoint on the other +// side of the proxy advertising an MSS that is too large for the tun MTU, +// black-holing large segments when path MTU discovery is broken. This can be +// useful if this tailscale instance is expected to run as a forwarding proxy, +// forwarding packets from an endpoint with higher MTU in an environment where +// path MTU discovery is expected to not work (such as the proxies created by +// the Tailscale Kubernetes operator). ClampMSSToPMTU creates a new base-chain +// ts-clamp in the filter // table with accept policy and priority -150. In practice, this means that for // SYN packets the clamp rule in this chain will likely run first and accept the // packet. This is fine because 1) nftables run ALL chains with the same hook @@ -289,61 +294,75 @@ func (n *nftablesRunner) ClampMSSToPMTU(tun string, addr netip.Addr) error { return fmt.Errorf("error ensuring forward chain: %w", err) } - clampRule := &nftables.Rule{ - Table: filterTable, - Chain: fwChain, - Exprs: []expr.Any{ - &expr.Meta{Key: expr.MetaKeyOIFNAME, Register: 1}, - &expr.Cmp{ - Op: expr.CmpOpEq, - Register: 1, - Data: []byte(tun), + // clampRuleForIface builds a rule that clamps the MSS of forwarded TCP + // handshake packets matching tun on the given interface-name meta key. + // ifaceKey is either expr.MetaKeyOIFNAME (packets leaving via tun) or + // expr.MetaKeyIIFNAME (packets arriving on tun). + clampRuleForIface := func(ifaceKey expr.MetaKey) *nftables.Rule { + return &nftables.Rule{ + Table: filterTable, + Chain: fwChain, + Exprs: []expr.Any{ + &expr.Meta{Key: ifaceKey, Register: 1}, + &expr.Cmp{ + Op: expr.CmpOpEq, + Register: 1, + Data: []byte(tun), + }, + &expr.Meta{Key: expr.MetaKeyL4PROTO, Register: 1}, + &expr.Cmp{ + Op: expr.CmpOpEq, + Register: 1, + Data: []byte{unix.IPPROTO_TCP}, + }, + &expr.Payload{ + DestRegister: 1, + Base: expr.PayloadBaseTransportHeader, + Offset: 13, + Len: 1, + }, + &expr.Bitwise{ + DestRegister: 1, + SourceRegister: 1, + Len: 1, + Mask: []byte{0x02}, + Xor: []byte{0x00}, + }, + &expr.Cmp{ + Op: expr.CmpOpNeq, // match any packet with a TCP flag set (SYN, ACK, RST) + Register: 1, + Data: []byte{0x00}, + }, + &expr.Rt{ + Register: 1, + Key: expr.RtTCPMSS, + }, + &expr.Byteorder{ + DestRegister: 1, + SourceRegister: 1, + Op: expr.ByteorderHton, + Len: 2, + Size: 2, + }, + &expr.Exthdr{ + SourceRegister: 1, + Type: 2, + Offset: 2, + Len: 2, + Op: expr.ExthdrOpTcpopt, + }, }, - &expr.Meta{Key: expr.MetaKeyL4PROTO, Register: 1}, - &expr.Cmp{ - Op: expr.CmpOpEq, - Register: 1, - Data: []byte{unix.IPPROTO_TCP}, - }, - &expr.Payload{ - DestRegister: 1, - Base: expr.PayloadBaseTransportHeader, - Offset: 13, - Len: 1, - }, - &expr.Bitwise{ - DestRegister: 1, - SourceRegister: 1, - Len: 1, - Mask: []byte{0x02}, - Xor: []byte{0x00}, - }, - &expr.Cmp{ - Op: expr.CmpOpNeq, // match any packet with a TCP flag set (SYN, ACK, RST) - Register: 1, - Data: []byte{0x00}, - }, - &expr.Rt{ - Register: 1, - Key: expr.RtTCPMSS, - }, - &expr.Byteorder{ - DestRegister: 1, - SourceRegister: 1, - Op: expr.ByteorderHton, - Len: 2, - Size: 2, - }, - &expr.Exthdr{ - SourceRegister: 1, - Type: 2, - Offset: 2, - Len: 2, - Op: expr.ExthdrOpTcpopt, - }, - }, + } } - n.conn.AddRule(clampRule) + + // Clamp both directions of the forwarded handshake: the SYN leaving via + // tun towards the tailnet peer, and the SYN-ACK arriving on tun and being + // forwarded back out towards the originating endpoint. Matching only the + // output interface leaves the endpoint on the other side advertising an MSS + // that is too large for the tun MTU, which black-holes large segments when + // PMTU discovery is broken. + n.conn.AddRule(clampRuleForIface(expr.MetaKeyOIFNAME)) + n.conn.AddRule(clampRuleForIface(expr.MetaKeyIIFNAME)) return n.conn.Flush() } @@ -584,8 +603,9 @@ type NetfilterRunner interface { DeleteSvc(svc, tun string, targetIPs []netip.Addr, pm []PortMap) error - // ClampMSSToPMTU adds a rule to the mangle/FORWARD chain to clamp MSS for - // traffic destined for the provided tun interface. + // ClampMSSToPMTU adds rules to clamp the MSS of forwarded TCP handshake + // packets in both directions (entering and leaving the provided tun + // interface), so both endpoints negotiate an MSS that fits the tun MTU. ClampMSSToPMTU(tun string, addr netip.Addr) error // AddMagicsockPortRule adds a rule to the ts-input chain to accept