From 88aa2c6bb2adf2da203f5ec5bb8a03020c680609 Mon Sep 17 00:00:00 2001 From: Mike Jensen Date: Wed, 19 Aug 2026 11:25:42 -0600 Subject: [PATCH] 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 90ed0bcf4bc227a81e39e686dc52cf24f17b0c63) Signed-off-by: Mike Jensen --- ipn/ipnlocal/local.go | 9 ++ ipn/ipnlocal/via.go | 32 +++++ ipn/ipnlocal/via_test.go | 66 +++++++++ wgengine/netstack/netstack.go | 43 +++++- wgengine/netstack/netstack_test.go | 223 +++++++++++++++++++++++++++++ 5 files changed, 371 insertions(+), 2 deletions(-) create mode 100644 ipn/ipnlocal/via.go create mode 100644 ipn/ipnlocal/via_test.go diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 5170b9ee7..d5d8d8fbc 100644 --- a/ipn/ipnlocal/local.go +++ b/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 { diff --git a/ipn/ipnlocal/via.go b/ipn/ipnlocal/via.go new file mode 100644 index 000000000..1b4ba713f --- /dev/null +++ b/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 +} diff --git a/ipn/ipnlocal/via_test.go b/ipn/ipnlocal/via_test.go new file mode 100644 index 000000000..486112f30 --- /dev/null +++ b/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) + } + } +} diff --git a/wgengine/netstack/netstack.go b/wgengine/netstack/netstack.go index 1ed42df91..17ba3dfa6 100644 --- a/wgengine/netstack/netstack.go +++ b/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) diff --git a/wgengine/netstack/netstack_test.go b/wgengine/netstack/netstack_test.go index f0e1b04b1..395de0bc7 100644 --- a/wgengine/netstack/netstack_test.go +++ b/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")