From abfdcd0f70b8a48dd9f12e24da6d592e544542f5 Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@tailscale.com>
Date: Tue, 8 Nov 2022 07:47:22 -0800
Subject: [PATCH] wgengine/netstack: fix shouldProcessInbound peerapi non-SYN
 handling

It was eating TCP packets to peerapi ports to subnet routers.  Some of
the TCP flow's packets went onward, some got eaten.  So some TCP flows
to subnet routers, if they used an unfortunate TCP port number, got
broken.

Change-Id: Ifea036119ccfb081f4dfa18b892373416a5239f8
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
---
 wgengine/netstack/netstack.go | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/wgengine/netstack/netstack.go b/wgengine/netstack/netstack.go
index 0b7ffbf5f..aa79a275c 100644
--- a/wgengine/netstack/netstack.go
+++ b/wgengine/netstack/netstack.go
@@ -535,23 +535,30 @@ var viaRange = tsaddr.TailscaleViaRange()
 // WireGuard peer) should be handled by netstack.
 func (ns *Impl) shouldProcessInbound(p *packet.Parsed, t *tstun.Wrapper) bool {
 	// Handle incoming peerapi connections in netstack.
-	if ns.lb != nil && p.IPProto == ipproto.TCP {
+	dstIP := p.Dst.Addr()
+	isLocal := ns.isLocalIP(dstIP)
+
+	// Handle TCP connection to the Tailscale IP(s) in some cases:
+	if ns.lb != nil && p.IPProto == ipproto.TCP && isLocal {
 		var peerAPIPort uint16
-		dstIP := p.Dst.Addr()
-		if p.TCPFlags&packet.TCPSynAck == packet.TCPSyn && ns.isLocalIP(dstIP) {
-			if port, ok := ns.lb.GetPeerAPIPort(p.Dst.Addr()); ok {
+
+		if p.TCPFlags&packet.TCPSynAck == packet.TCPSyn {
+			if port, ok := ns.lb.GetPeerAPIPort(dstIP); ok {
 				peerAPIPort = port
 				atomic.StoreUint32(ns.peerAPIPortAtomic(dstIP), uint32(port))
 			}
 		} else {
 			peerAPIPort = uint16(atomic.LoadUint32(ns.peerAPIPortAtomic(dstIP)))
 		}
-		if p.IPProto == ipproto.TCP && p.Dst.Port() == peerAPIPort {
+		dport := p.Dst.Port()
+		if dport == peerAPIPort {
+			return true
+		}
+
+		// Also handle SSH connections, if enabled.
+		if dport == 22 && ns.lb.ShouldRunSSH() {
 			return true
 		}
-	}
-	if ns.isInboundTSSH(p) && ns.processSSH() {
-		return true
 	}
 	if p.IPVersion == 6 && viaRange.Contains(p.Dst.Addr()) {
 		return ns.lb != nil && ns.lb.ShouldHandleViaIP(p.Dst.Addr())
@@ -561,7 +568,6 @@ func (ns *Impl) shouldProcessInbound(p *packet.Parsed, t *tstun.Wrapper) bool {
 		// netstack isn't used at all; don't even do an isLocalIP lookup.
 		return false
 	}
-	isLocal := ns.isLocalIP(p.Dst.Addr())
 	if ns.ProcessLocalIPs && isLocal {
 		return true
 	}
@@ -647,12 +653,6 @@ func (ns *Impl) userPing(dstIP netip.Addr, pingResPkt []byte) {
 	}
 }
 
-func (ns *Impl) isInboundTSSH(p *packet.Parsed) bool {
-	return p.IPProto == ipproto.TCP &&
-		p.Dst.Port() == 22 &&
-		ns.isLocalIP(p.Dst.Addr())
-}
-
 // injectInbound is installed as a packet hook on the 'inbound' (from a
 // WireGuard peer) path. Returning filter.Accept releases the packet to
 // continue normally (typically being delivered to the host networking stack),