ipn,wgengine: only intercept TailFS traffic on quad 100

This fixes a regression introduced with 993acf4 and released in
v1.60.0.

The regression caused us to intercept all userspace traffic to port
8080 which prevented users from exposing their own services to their
tailnet at port 8080.

Now, we only intercept traffic to port 8080 if it's bound for
100.100.100.100 or fd7a:115c:a1e0::53.

Fixes #11283

Signed-off-by: Percy Wegmann <[email protected]>
(cherry picked from commit 17cd0626f35dbc7948a78665d06a5862fc3dfdab)

ipn/ipnlocal/local.go

@@ -3329,13 +3329,24 @@ var (
 // TCPHandlerForDst returns a TCP handler for connections to dst, or nil if
 // no handler is needed. It also returns a list of TCP socket options to
 // apply to the socket before calling the handler.
+// TCPHandlerForDst is called both for connections to our node's local IP
+// as well as to the service IP (quad 100).
 func (b *LocalBackend) TCPHandlerForDst(src, dst netip.AddrPort) (handler func(c net.Conn) error, opts []tcpip.SettableSocketOption) {
-	if dst.Port() == 80 && (dst.Addr() == magicDNSIP || dst.Addr() == magicDNSIPv6) {
-		if b.ShouldRunWebClient() {
-			return b.handleWebClientConn, opts
+	// First handle internal connections to the service IP
+	hittingServiceIP := dst.Addr() == magicDNSIP || dst.Addr() == magicDNSIPv6
+	if hittingServiceIP {
+		switch dst.Port() {
+		case 80:
+			if b.ShouldRunWebClient() {
+				return b.handleWebClientConn, opts
+			}
+			return b.HandleQuad100Port80Conn, opts
+		case TailFSLocalPort:
+			return b.handleTailFSConn, opts
 		}
-		return b.HandleQuad100Port80Conn, opts
 	}
+
+	// Then handle external connections to the local IP.
 	if !b.isLocalIP(dst.Addr()) {
 		return nil, nil
 	}
@@ -3353,18 +3364,6 @@ func (b *LocalBackend) TCPHandlerForDst(src, dst netip.AddrPort) (handler func(c
 	if dst.Port() == webClientPort && b.ShouldRunWebClient() {
 		return b.handleWebClientConn, opts
 	}
-	if dst.Port() == TailFSLocalPort {
-		fs, ok := b.sys.TailFSForLocal.GetOK()
-		if ok {
-			return func(conn net.Conn) error {
-				if !b.TailFSAccessEnabled() {
-					conn.Close()
-					return nil
-				}
-				return fs.HandleConn(conn, conn.RemoteAddr())
-			}, opts
-		}
-	}
 	if port, ok := b.GetPeerAPIPort(dst.Addr()); ok && dst.Port() == port {
 		return func(c net.Conn) error {
 			b.handlePeerAPIConn(src, dst, c)
@@ -3377,6 +3376,15 @@ func (b *LocalBackend) TCPHandlerForDst(src, dst netip.AddrPort) (handler func(c
 	return nil, nil
 }
 
+func (b *LocalBackend) handleTailFSConn(conn net.Conn) error {
+	fs, ok := b.sys.TailFSForLocal.GetOK()
+	if !ok || !b.TailFSAccessEnabled() {
+		conn.Close()
+		return nil
+	}
+	return fs.HandleConn(conn, conn.RemoteAddr())
+}
+
 func (b *LocalBackend) peerAPIServicesLocked() (ret []tailcfg.Service) {
 	for _, pln := range b.peerAPIListeners {
 		proto := tailcfg.PeerAPI4

ipn/ipnlocal/local_test.go

@@ -2173,3 +2173,72 @@ func TestOnTailnetDefaultAutoUpdate(t *testing.T) {
 		})
 	}
 }
+
+func TestTCPHandlerForDst(t *testing.T) {
+	b := newTestBackend(t)
+
+	tests := []struct {
+		desc      string
+		dst       string
+		intercept bool
+	}{
+		{
+			desc:      "intercept port 80 (Web UI) on quad100 IPv4",
+			dst:       "100.100.100.100:80",
+			intercept: true,
+		},
+		{
+			desc:      "intercept port 80 (Web UI) on quad100 IPv6",
+			dst:       "[fd7a:115c:a1e0::53]:80",
+			intercept: true,
+		},
+		{
+			desc:      "don't intercept port 80 on local ip",
+			dst:       "100.100.103.100:80",
+			intercept: false,
+		},
+		{
+			desc:      "intercept port 8080 (TailFS) on quad100 IPv4",
+			dst:       "100.100.100.100:8080",
+			intercept: true,
+		},
+		{
+			desc:      "intercept port 8080 (TailFS) on quad100 IPv6",
+			dst:       "[fd7a:115c:a1e0::53]:8080",
+			intercept: true,
+		},
+		{
+			desc:      "don't intercept port 8080 on local ip",
+			dst:       "100.100.103.100:8080",
+			intercept: false,
+		},
+		{
+			desc:      "don't intercept port 9080 on quad100 IPv4",
+			dst:       "100.100.100.100:9080",
+			intercept: false,
+		},
+		{
+			desc:      "don't intercept port 9080 on quad100 IPv6",
+			dst:       "[fd7a:115c:a1e0::53]:9080",
+			intercept: false,
+		},
+		{
+			desc:      "don't intercept port 9080 on local ip",
+			dst:       "100.100.103.100:9080",
+			intercept: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.dst, func(t *testing.T) {
+			t.Log(tt.desc)
+			src := netip.MustParseAddrPort("100.100.102.100:51234")
+			h, _ := b.TCPHandlerForDst(src, netip.MustParseAddrPort(tt.dst))
+			if !tt.intercept && h != nil {
+				t.Error("intercepted traffic we shouldn't have")
+			} else if tt.intercept && h == nil {
+				t.Error("failed to intercept traffic we should have")
+			}
+		})
+	}
+}

ipn/ipnlocal/serve_test.go

@@ -684,7 +684,8 @@ func newTestBackend(t *testing.T) *LocalBackend {
 
 	b.netMap = &netmap.NetworkMap{
 		SelfNode: (&tailcfg.Node{
-			Name: "example.ts.net",
+			Name:         "example.ts.net",
+			Capabilities: []tailcfg.NodeCapability{tailcfg.NodeAttrsTailFSAccess},
 		}).View(),
 		UserProfiles: map[tailcfg.UserID]tailcfg.UserProfile{
 			tailcfg.UserID(1): {

wgengine/netstack/netstack.go

@@ -1131,25 +1131,13 @@ func (ns *Impl) acceptTCP(r *tcp.ForwarderRequest) {
 	// Local Services (DNS and WebDAV)
 	hittingServiceIP := dialIP == serviceIP || dialIP == serviceIPv6
 	hittingDNS := hittingServiceIP && reqDetails.LocalPort == 53
-	hittingTailFS := hittingServiceIP && ns.tailFSForLocal != nil && reqDetails.LocalPort == ipnlocal.TailFSLocalPort
-	if hittingDNS || hittingTailFS {
+	if hittingDNS {
 		c := getConnOrReset()
 		if c == nil {
 			return
 		}
 		addrPort := netip.AddrPortFrom(clientRemoteIP, reqDetails.RemotePort)
-		if hittingDNS {
-			go ns.dns.HandleTCPConn(c, addrPort)
-		} else if hittingTailFS {
-			if !ns.lb.TailFSAccessEnabled() {
-				c.Close()
-				return
-			}
-			err := ns.tailFSForLocal.HandleConn(c, net.TCPAddrFromAddrPort(addrPort))
-			if err != nil {
-				ns.logf("netstack: tailfs.HandleConn: %v", err)
-			}
-		}
+		go ns.dns.HandleTCPConn(c, addrPort)
 		return
 	}