wgengine/netstack: block 4via6 forwards to host-scoped targets (#20866) (#20926)

The packet filter only sees a via address's outer ULA, so netstack unconditionally dialed whatever IPv4 was embedded in it.

This change refuses TCP, UDP, and ping relays to host-scoped destinations after UnmapVia.

Credit to the Anthropic infrastructure security team for finding and reporting.

Fixes https://github.com/tailscale/corp/issues/46646

Change-Id: I93d8e27a2eddbce7eb3f8c5fa4677f8de3a8ed9e

Change-Id: I8865c0e89474600b0b8254ebebd0523e29412e58

(cherry picked from commit 90ed0bcf4b)

Signed-off-by: Mike Jensen <mikej@tailscale.com>

ipn/ipnlocal/local.go

@@ -7046,6 +7046,15 @@ func (b *LocalBackend) ShouldHandleViaIP(ip netip.Addr) bool {
 	return false
 }
 
+// ShouldForwardToVia reports whether a flow to the 4via6 destination via may
+// be forwarded to the embedded IPv4 target.
+//
+// The packet filter only sees the outer via address of a 4via6 flow, so this
+// is the sole policy check on the embedded target.
+func (b *LocalBackend) ShouldForwardToVia(via netip.Addr) bool {
+	return viaTargetAllowed(tsaddr.UnmapVia(via))
+}
+
 // Logout logs out the current profile, if any, and waits for the logout to
 // complete.
 func (b *LocalBackend) Logout(ctx context.Context, actor ipnauth.Actor) error {

ipn/ipnlocal/via.go

@@ -0,0 +1,32 @@
+// Copyright (c) Tailscale Inc & contributors
+// SPDX-License-Identifier: BSD-3-Clause
+
+package ipnlocal
+
+import (
+	"net/netip"
+
+	"tailscale.com/net/tsaddr"
+)
+
+// v4BroadcastAddr is 255.255.255.255, the limited broadcast address.
+var v4BroadcastAddr = netip.AddrFrom4([4]byte{255, 255, 255, 255})
+
+// viaTargetAllowed reports whether ip may be forwarded to after unmapping a
+// 4via6 destination. The packet filter only sees the outer via address, so
+// this is the sole check on the embedded IPv4 target.
+func viaTargetAllowed(ip netip.Addr) bool {
+	if !ip.Is4() {
+		return false // UnmapVia only returns IPv4
+	} else if ip.IsLoopback() || ip.IsMulticast() || ip.IsUnspecified() || ip == v4BroadcastAddr {
+		return false
+	} else if tsaddr.IsTailscaleIP(ip) {
+		// A CGNAT-range target may route to tailscale0 or to a site LAN
+		// depending on the tailnet and OS (see shouldUseOneCGNATRoute);
+		// it's hard to detect when forwarding would be OK, so deny always
+		return false
+	} else if ip.IsLinkLocalUnicast() {
+		return false
+	}
+	return true
+}

ipn/ipnlocal/via_test.go

@@ -0,0 +1,66 @@
+// Copyright (c) Tailscale Inc & contributors
+// SPDX-License-Identifier: BSD-3-Clause
+
+package ipnlocal
+
+import (
+	"net/netip"
+	"testing"
+)
+
+func TestViaTargetAllowed(t *testing.T) {
+	cases := []struct {
+		ip   string
+		want bool
+	}{
+		{"10.0.0.1", true},
+		{"192.168.1.1", true},
+		{"8.8.8.8", true},
+		{"169.254.169.254", false}, // cloud instance metadata
+		{"169.254.0.1", false},     // other link-local
+		{"127.0.0.1", false},       // loopback
+		{"127.255.0.1", false},
+		{"10.9.4.99", true},      // normal LAN host
+		{"192.168.50.254", true}, // last routable host on a /24 still allowed
+		{"0.0.0.0", false},       // Linux connect() treats as localhost
+		{"255.255.255.255", false},
+		{"192.168.50.128", true},
+		{"10.100.200.5", true},
+		{"10.1.2.255", true}, // last octet 255 is not inherently broadcast
+		{"172.16.0.9", true},
+		{"192.168.50.63", true},
+		{"224.0.0.1", false}, // multicast
+		{"239.255.252.250", false},
+		{"10.20.30.40", true},
+		{"198.51.100.7", true},
+		{"169.254.169.1", false}, // other link-local
+		{"255.0.0.5", true},
+		{"10.9.8.7", true},
+		{"192.168.1.254", true},
+		{"10.0.255.200", true}, // host in a /16 whose last octet isn't 0/255
+		{"224.0.0.251", false},
+		{"100.64.9.8", false},
+		{"192.168.5.10", true},
+		{"172.16.255.254", true}, // last host of a /16 (last octet 254) allowed
+		{"127.1.2.3", false},
+		{"198.18.0.10", true},
+		{"239.255.255.250", false}, // SSDP multicast
+		{"192.168.50.128", true},
+		{"169.254.100.200", false}, // link-local
+		{"10.9.4.255", true},       // last octet 255 is not inherently broadcast
+		{"224.0.1.129", false},
+		{"8.20.30.40", true},
+		{"192.168.50.255", true}, // last octet 255 is not inherently broadcast
+		{"10.11.12.13", true},
+		{"239.1.2.3", false},
+		{"100.64.0.1", false}, // tailnet CGNAT: would proxy as this node
+		{"100.100.100.100", false},
+		{"::1", false},
+	}
+	for _, tc := range cases {
+		ip := netip.MustParseAddr(tc.ip)
+		if got := viaTargetAllowed(ip); got != tc.want {
+			t.Errorf("viaTargetAllowed(%v) = %v, want %v", ip, got, tc.want)
+		}
+	}
+}

wgengine/netstack/netstack.go

@@ -1441,6 +1441,16 @@ func (ns *Impl) injectInbound(p *packet.Parsed, t *tstun.Wrapper, gro *gro.GRO)
 	return filter.DropSilently, gro
 }
 
+// metricViaHostScopedDrop counts refused 4via6 forwards to host-scoped targets.
+var metricViaHostScopedDrop = clientmetric.NewCounter("netstack_via_host_scoped_dropped")
+
+// shouldForwardToVia reports whether a flow to the 4via6 destination via may
+// be forwarded to the embedded IPv4 target. The packet filter only sees the
+// outer via address, so this is the sole check on the embedded target.
+func (ns *Impl) shouldForwardToVia(via netip.Addr) bool {
+	return ns.lb != nil && ns.lb.ShouldForwardToVia(via)
+}
+
 // shouldHandlePing returns whether or not netstack should handle an incoming
 // ICMP echo request packet, and the IP address that should be pinged from this
 // process. The IP address can be different from the destination in the packet
@@ -1474,7 +1484,15 @@ func (ns *Impl) shouldHandlePing(p *packet.Parsed) (_ netip.Addr, ok bool) {
 		// IPv4 and expect to get a useful result. However, in this specific
 		// case things are safe because the 'userPing' function doesn't make
 		// use of the input packet.
-		return tsaddr.UnmapVia(destIP), true
+		unmapped := tsaddr.UnmapVia(destIP)
+		if !ns.shouldForwardToVia(destIP) {
+			// Don't relay pings to host-scoped targets: a relayed reply
+			// would reveal which LAN addresses are reachable from this
+			// node, aiding network reconnaissance.
+			metricViaHostScopedDrop.Add(1)
+			return netip.Addr{}, false
+		}
+		return unmapped, true
 	}
 
 	// If we get here, we don't do anything unless this netstack instance
@@ -1542,8 +1560,11 @@ func (ns *Impl) acceptTCP(r *tcp.ForwarderRequest) {
 
 	dstAddrPort := netip.AddrPortFrom(dialIP, reqDetails.LocalPort)
 
-	if viaRange.Contains(dialIP) {
+	isVia := viaRange.Contains(dialIP)
+	var viaIP netip.Addr
+	if isVia {
 		isTailscaleIP = false
+		viaIP = dialIP
 		dialIP = tsaddr.UnmapVia(dialIP)
 	}
 
@@ -1555,6 +1576,15 @@ func (ns *Impl) acceptTCP(r *tcp.ForwarderRequest) {
 		}
 	}()
 
+	if isVia && !ns.shouldForwardToVia(viaIP) {
+		// Refuse host-scoped 4via6 targets with a RST
+		metricViaHostScopedDrop.Add(1)
+		ns.logf("netstack: rejecting TCP connection to host-scoped 4via6 target %v from %v",
+			netip.AddrPortFrom(dialIP, reqDetails.LocalPort), clientRemoteAddrPort)
+		r.Complete(true) // sends a RST
+		return
+	}
+
 	var wq waiter.Queue
 
 	// We can't actually create the endpoint or complete the inbound
@@ -2047,6 +2077,15 @@ func (ns *Impl) forwardUDP(client *gonet.UDPConn, clientAddr, dstAddr netip.Addr
 		backendListenAddr = &net.UDPAddr{IP: ip, Port: int(srcPort)}
 	} else {
 		if dstIP := dstAddr.Addr(); viaRange.Contains(dstIP) {
+			if !ns.shouldForwardToVia(dstIP) {
+				// Close the client endpoint: the guard is attacker-triggerable
+				// per packet, so a bare return would leak gVisor endpoints.
+				metricViaHostScopedDrop.Add(1)
+				ns.logf("netstack: dropping UDP flow to host-scoped 4via6 target %v from %v",
+					netip.AddrPortFrom(tsaddr.UnmapVia(dstIP), dstAddr.Port()), clientAddr)
+				client.Close()
+				return
+			}
 			dstAddr = netip.AddrPortFrom(tsaddr.UnmapVia(dstIP), dstAddr.Port())
 		}
 		backendRemoteAddr = net.UDPAddrFromAddrPort(dstAddr)

wgengine/netstack/netstack_test.go

@@ -18,8 +18,10 @@
 	"gvisor.dev/gvisor/pkg/tcpip/adapters/gonet"
 	"gvisor.dev/gvisor/pkg/tcpip/header"
 	"gvisor.dev/gvisor/pkg/tcpip/network/ipv4"
+	"gvisor.dev/gvisor/pkg/tcpip/network/ipv6"
 	"gvisor.dev/gvisor/pkg/tcpip/stack"
 	"gvisor.dev/gvisor/pkg/tcpip/transport/udp"
+	"gvisor.dev/gvisor/pkg/waiter"
 	"tailscale.com/envknob"
 	"tailscale.com/ipn"
 	"tailscale.com/ipn/ipnlocal"
@@ -261,6 +263,7 @@ func TestShouldHandlePing(t *testing.T) {
 			impl := makeNetstack(t, func(impl *Impl) {
 				impl.ProcessSubnets = subnets
 			})
+
 			pingDst, ok := impl.shouldHandlePing(pkt)
 
 			// Handled due to being 4via6
@@ -707,6 +710,50 @@ func tcp4syn(tb testing.TB, src, dst netip.Addr, sport, dport uint16) []byte {
 	return ip
 }
 
+// tcp6syn is tcp4syn for IPv6 packets.
+func tcp6syn(tb testing.TB, src, dst netip.Addr, sport, dport uint16) []byte {
+	srcAddr := tcpip.AddrFrom16(src.As16())
+	dstAddr := tcpip.AddrFrom16(dst.As16())
+
+	ip := header.IPv6(make([]byte, header.IPv6MinimumSize+header.TCPMinimumSize))
+	ip.Encode(&header.IPv6Fields{
+		SrcAddr:           srcAddr,
+		DstAddr:           dstAddr,
+		PayloadLength:     header.TCPMinimumSize,
+		TransportProtocol: header.TCPProtocolNumber,
+		HopLimit:          64,
+	})
+
+	tcp := header.TCP(ip[header.IPv6MinimumSize:])
+	tcp.Encode(&header.TCPFields{
+		SrcPort:    sport,
+		DstPort:    dport,
+		SeqNum:     0,
+		DataOffset: header.TCPMinimumSize,
+		Flags:      header.TCPFlagSyn,
+		WindowSize: 65535,
+		Checksum:   0,
+	})
+	xsum := header.PseudoHeaderChecksum(header.TCPProtocolNumber, srcAddr, dstAddr,
+		uint16(header.TCPMinimumSize))
+	tcp.SetChecksum(^tcp.CalculateChecksum(xsum))
+	if !tcp.IsChecksumValid(srcAddr, dstAddr, 0, 0) {
+		tb.Fatal("test broken; packet has incorrect TCP checksum")
+	}
+
+	return ip
+}
+
+// mustVia99 returns the 4via6 address embedding v4 within site 99's /96.
+func mustVia99(tb testing.TB, v4 string) netip.Addr {
+	tb.Helper()
+	p, err := tsaddr.MapVia(99, netip.PrefixFrom(netip.MustParseAddr(v4), 32))
+	if err != nil {
+		tb.Fatal(err)
+	}
+	return p.Addr()
+}
+
 // makeHangDialer returns a dialer that notifies the returned channel when a
 // connection is dialed and then hangs until the test finishes.
 func makeHangDialer(tb testing.TB) (netx.DialFunc, chan struct{}) {
@@ -1328,6 +1375,182 @@ func TestAcceptTCPLoopbackForwardVsRST(t *testing.T) {
 	}
 }
 
+// TestShouldHandlePingViaHostScoped verifies that ping relay for 4via6
+// addresses embedding host-scoped IPv4 destinations is refused.
+func TestShouldHandlePingViaHostScoped(t *testing.T) {
+	srcIP := netip.AddrFrom4([4]byte{1, 2, 3, 4})
+	pingPkt := func(dst netip.Addr) *packet.Parsed {
+		icmph := packet.ICMP6Header{
+			IP6Header: packet.IP6Header{
+				IPProto: ipproto.ICMPv6,
+				Src:     srcIP,
+				Dst:     dst,
+			},
+			Type: packet.ICMP6EchoRequest,
+			Code: packet.ICMP6NoCode,
+		}
+		_, payload := packet.ICMPEchoPayload(nil)
+		pkt := &packet.Parsed{}
+		pkt.Decode(packet.Generate(icmph, payload))
+		return pkt
+	}
+
+	cases := []struct {
+		name   string
+		v4     string
+		wantOK bool
+	}{
+		{"SiteHost", "10.1.1.9", true},
+		{"Metadata", "169.254.169.254", false},
+		{"Loopback", "127.0.0.1", false},
+		{"TailscaleCGNAT", "100.64.1.2", false},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			impl := makeNetstack(t, func(impl *Impl) {
+				impl.ProcessSubnets = true
+			})
+			dst := mustVia99(t, tc.v4)
+			pingDst, ok := impl.shouldHandlePing(pingPkt(dst))
+			if ok != tc.wantOK {
+				t.Fatalf("shouldHandlePing(%v) ok = %v, want %v", dst, ok, tc.wantOK)
+			}
+			if ok && pingDst != netip.MustParseAddr(tc.v4) {
+				t.Errorf("shouldHandlePing(%v) pingDst = %v, want %v", dst, pingDst, tc.v4)
+			}
+		})
+	}
+}
+
+// TestAcceptTCPViaHostScoped is a regression test for the 4via6 filter
+// bypass: the packet filter only sees the outer via address, so acceptTCP
+// must police the unmapped destination itself. SYNs to via addresses
+// embedding host-scoped IPv4 destinations must be RST without invoking the
+// forward dialer; ordinary site destinations must still be forwarded.
+func TestAcceptTCPViaHostScoped(t *testing.T) {
+	viaPrefix := netip.MustParsePrefix("fd7a:115c:a1e0:b1a:0:63::/96") // site 99
+
+	cases := []struct {
+		name string
+		dst  netip.AddrPort
+		// wantForward is if acceptTCP should dial the target or reject the connection with a RST
+		wantForward bool
+	}{
+		{"MetadataBlocked", netip.AddrPortFrom(mustVia99(t, "169.254.169.254"), 80), false},
+		{"LoopbackBlocked", netip.AddrPortFrom(mustVia99(t, "127.0.0.1"), 22), false},
+		{"TailscaleCGNATBlocked", netip.AddrPortFrom(mustVia99(t, "100.64.3.4"), 80), false},
+		{"SiteHostAllowed", netip.AddrPortFrom(mustVia99(t, "10.0.0.1"), 80), true},
+		{"SiteNetworkAllowed", netip.AddrPortFrom(mustVia99(t, "10.0.0.0"), 80), true},
+		{"SiteBroadcastAllowed", netip.AddrPortFrom(mustVia99(t, "10.0.0.255"), 80), true},
+		{"SiteLastHostAllowed", netip.AddrPortFrom(mustVia99(t, "10.0.0.254"), 80), true},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			impl := makeNetstack(t, func(impl *Impl) {
+				impl.ProcessSubnets = true
+			})
+			// Advertise the via prefix so ShouldHandleViaIP accepts the dst
+			prefs := ipn.NewPrefs()
+			prefs.AdvertiseRoutes = []netip.Prefix{viaPrefix}
+			impl.lb.Start(ipn.Options{UpdatePrefs: prefs})
+			impl.atomicIsLocalIPFunc.Store(looksLikeATailscaleSelfAddress)
+
+			dialFn, gotConn := makeHangDialer(t)
+			impl.forwardDialFunc = dialFn
+
+			client := tsaddr.Tailscale4To6(netip.MustParseAddr("100.101.102.103"))
+			pkt := tcp6syn(t, client, tc.dst.Addr(), 1234, tc.dst.Port())
+			var parsed packet.Parsed
+			parsed.Decode(pkt)
+
+			if resp, _ := impl.injectInbound(&parsed, impl.tundev, nil); resp != filter.DropSilently {
+				t.Fatalf("inject for %v: got filter outcome %v, want filter.DropSilently", tc.dst, resp)
+			}
+
+			// Same synchronization as TestAcceptTCPLoopbackForwardVsRST:
+			// the in-flight counter reaching 0 means acceptTCP returned
+			// (RST path); gotConn firing means it called forwardTCP.
+			inFlightZero := make(chan struct{})
+			go func() {
+				for {
+					impl.mu.Lock()
+					n := impl.connsInFlightByClient[client]
+					impl.mu.Unlock()
+					if n == 0 {
+						close(inFlightZero)
+						return
+					}
+					time.Sleep(time.Millisecond)
+				}
+			}()
+
+			select {
+			case <-gotConn:
+				if !tc.wantForward {
+					t.Fatalf("forwardDialFunc was called for %v; acceptTCP forwarded a host-scoped 4via6 target instead of sending a RST", tc.dst)
+				}
+			case <-inFlightZero:
+				if tc.wantForward {
+					t.Fatalf("forwardDialFunc was NOT called for %v; acceptTCP rejected a legitimate 4via6 target", tc.dst)
+				}
+			case <-time.After(5 * time.Second):
+				t.Fatalf("timed out waiting for acceptTCP to dispatch %v SYN", tc.dst)
+			}
+		})
+	}
+}
+
+// TestForwardUDPViaHostScoped is a regression test for the 4via6 filter
+// bypass: the packet filter only sees the outer via address, so forwardUDP
+// must police the unmapped destination itself. UDP flows to via addresses
+// embedding host-scoped IPv4 destinations must be dropped before any backend
+// socket is created, and the drop must close the client endpoint: acceptUDP
+// runs forwardUDP in a goroutine, so nothing else would.
+func TestForwardUDPViaHostScoped(t *testing.T) {
+	tstest.AssertNotParallel(t) // calls clientmetric.ResetForTest
+	clientmetric.ResetForTest(t)
+
+	impl := makeNetstack(t, func(impl *Impl) {
+		impl.atomicIsLocalIPFunc.Store(looksLikeATailscaleSelfAddress)
+	})
+
+	client := tsaddr.Tailscale4To6(netip.MustParseAddr("100.101.102.103"))
+	blocked := []string{
+		"127.0.0.1",
+		"169.254.169.254", // port 80 below: metadata
+		"100.64.1.2",
+		"224.0.0.1",
+		"0.0.0.0",
+		"255.255.255.255",
+	}
+	for i, v4 := range blocked {
+		dst := netip.AddrPortFrom(mustVia99(t, v4), 80)
+
+		// A bare endpoint suffices: the drop path only closes the conn; dialing would need real routes
+		var wq waiter.Queue
+		ep, err := impl.ipstack.NewEndpoint(udp.ProtocolNumber, ipv6.ProtocolNumber, &wq)
+		if err != nil {
+			t.Fatalf("NewEndpoint: %v", err)
+		}
+		conn := gonet.NewUDPConn(&wq, ep)
+
+		// Distinct source port per flow; the guard runs synchronously, so no waiting is needed
+		before := metricViaHostScopedDrop.Value()
+		impl.forwardUDP(conn, netip.AddrPortFrom(client, uint16(1000+i)), dst)
+
+		if got := metricViaHostScopedDrop.Value() - before; got != 1 {
+			t.Errorf("UDP to %v: drop metric delta = %d, want 1 (guard did not fire)", dst, got)
+		}
+		// ep.Read never blocks (no gonet retry loop): ErrClosedForReceive iff the guard closed the conn
+		w := tcpip.SliceWriter(make([]byte, 1))
+		_, rerr := ep.Read(&w, tcpip.ReadOptions{})
+		if _, closed := rerr.(*tcpip.ErrClosedForReceive); !closed {
+			t.Errorf("after UDP to %v: client endpoint Read = %v, want ErrClosedForReceive", dst, rerr)
+		}
+	}
+}
+
 func TestShouldSendToHost(t *testing.T) {
 	var (
 		selfIP4             = netip.MustParseAddr("100.64.1.2")