diff --git a/wgengine/netstack/netstack.go b/wgengine/netstack/netstack.go index 9740a5da6..7a8475843 100644 --- a/wgengine/netstack/netstack.go +++ b/wgengine/netstack/netstack.go @@ -664,7 +664,11 @@ func (ns *Impl) injectInbound(p *packet.Parsed, t *tstun.Wrapper) filter.Respons } destIP := p.Dst.Addr() - if p.IsEchoRequest() && ns.ProcessSubnets && !tsaddr.IsTailscaleIP(destIP) { + + // If this is an echo request and we're a subnet router, handle pings + // ourselves instead of forwarding the packet on. + pingIP, handlePing := ns.shouldHandlePing(p) + if handlePing { var pong []byte // the reply to the ping, if our relayed ping works if destIP.Is4() { h := p.ICMP4Header() @@ -675,7 +679,7 @@ func (ns *Impl) injectInbound(p *packet.Parsed, t *tstun.Wrapper) filter.Respons h.ToResponse() pong = packet.Generate(&h, p.Payload()) } - go ns.userPing(destIP, pong) + go ns.userPing(pingIP, pong) return filter.DropSilently } @@ -704,6 +708,43 @@ func (ns *Impl) injectInbound(p *packet.Parsed, t *tstun.Wrapper) filter.Respons return filter.DropSilently } +// 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 +// if the destination is a 4via6 address. +func (ns *Impl) shouldHandlePing(p *packet.Parsed) (_ netip.Addr, ok bool) { + if !p.IsEchoRequest() { + return netip.Addr{}, false + } + if !ns.ProcessSubnets { + return netip.Addr{}, false + } + + destIP := p.Dst.Addr() + + // For non-4via6 addresses, we don't handle pings if they're destined + // for a Tailscale IP. + if !viaRange.Contains(destIP) { + if tsaddr.IsTailscaleIP(destIP) { + return netip.Addr{}, false + } + + return destIP, true + } + + // The input echo request was to a 4via6 address, which we cannot + // simply ping as-is from this process. Translate the destination to an + // IPv4 address, so that our relayed ping (in userPing) is pinging the + // underlying destination IP. + // + // ICMPv4 and ICMPv6 are different protocols with different on-the-wire + // representations, so normally you can't send an ICMPv6 message over + // 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 +} + func netaddrIPFromNetstackIP(s tcpip.Address) netip.Addr { switch len(s) { case 4: diff --git a/wgengine/netstack/netstack_test.go b/wgengine/netstack/netstack_test.go index 626808306..1d21f378b 100644 --- a/wgengine/netstack/netstack_test.go +++ b/wgengine/netstack/netstack_test.go @@ -13,6 +13,7 @@ import ( "tailscale.com/net/packet" "tailscale.com/net/tsdial" "tailscale.com/net/tstun" + "tailscale.com/types/ipproto" "tailscale.com/wgengine" "tailscale.com/wgengine/filter" ) @@ -84,3 +85,167 @@ func TestNetstackLeakMode(t *testing.T) { t.Fatalf("refs.leakMode is 0, want a non-zero value") } } + +func makeNetstack(t *testing.T, config func(*Impl)) *Impl { + tunDev := tstun.NewFake() + dialer := new(tsdial.Dialer) + logf := func(format string, args ...any) { + if !t.Failed() { + t.Helper() + t.Logf(format, args...) + } + } + eng, err := wgengine.NewUserspaceEngine(logf, wgengine.Config{ + Tun: tunDev, + Dialer: dialer, + }) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { eng.Close() }) + ig, ok := eng.(wgengine.InternalsGetter) + if !ok { + t.Fatal("not an InternalsGetter") + } + tunWrap, magicSock, dns, ok := ig.GetInternals() + if !ok { + t.Fatal("failed to get internals") + } + + ns, err := Create(logf, tunWrap, eng, magicSock, dialer, dns) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { ns.Close() }) + + ns.atomicIsLocalIPFunc.Store(func(netip.Addr) bool { return true }) + config(ns) + + if err := ns.Start(); err != nil { + t.Fatalf("Start: %v", err) + } + return ns +} + +func TestShouldHandlePing(t *testing.T) { + srcIP := netip.AddrFrom4([4]byte{1, 2, 3, 4}) + + t.Run("ICMP4", func(t *testing.T) { + dst := netip.MustParseAddr("5.6.7.8") + icmph := packet.ICMP4Header{ + IP4Header: packet.IP4Header{ + IPProto: ipproto.ICMPv4, + Src: srcIP, + Dst: dst, + }, + Type: packet.ICMP4EchoRequest, + Code: packet.ICMP4NoCode, + } + _, payload := packet.ICMPEchoPayload(nil) + icmpPing := packet.Generate(icmph, payload) + pkt := &packet.Parsed{} + pkt.Decode(icmpPing) + + impl := makeNetstack(t, func(impl *Impl) { + impl.ProcessSubnets = true + }) + pingDst, ok := impl.shouldHandlePing(pkt) + if !ok { + t.Errorf("expected shouldHandlePing==true") + } + if pingDst != dst { + t.Errorf("got dst %s; want %s", pingDst, dst) + } + }) + + t.Run("ICMP6-no-via", func(t *testing.T) { + dst := netip.MustParseAddr("2a09:8280:1::4169") + icmph := packet.ICMP6Header{ + IP6Header: packet.IP6Header{ + IPProto: ipproto.ICMPv6, + Src: srcIP, + Dst: dst, + }, + Type: packet.ICMP6EchoRequest, + Code: packet.ICMP6NoCode, + } + _, payload := packet.ICMPEchoPayload(nil) + icmpPing := packet.Generate(icmph, payload) + pkt := &packet.Parsed{} + pkt.Decode(icmpPing) + + impl := makeNetstack(t, func(impl *Impl) { + impl.ProcessSubnets = true + }) + pingDst, ok := impl.shouldHandlePing(pkt) + + // Expect that we handle this since it's going out onto the + // network. + if !ok { + t.Errorf("expected shouldHandlePing==true") + } + if pingDst != dst { + t.Errorf("got dst %s; want %s", pingDst, dst) + } + }) + + t.Run("ICMP6-tailscale-addr", func(t *testing.T) { + dst := netip.MustParseAddr("fd7a:115c:a1e0:ab12::1") + icmph := packet.ICMP6Header{ + IP6Header: packet.IP6Header{ + IPProto: ipproto.ICMPv6, + Src: srcIP, + Dst: dst, + }, + Type: packet.ICMP6EchoRequest, + Code: packet.ICMP6NoCode, + } + _, payload := packet.ICMPEchoPayload(nil) + icmpPing := packet.Generate(icmph, payload) + pkt := &packet.Parsed{} + pkt.Decode(icmpPing) + + impl := makeNetstack(t, func(impl *Impl) { + impl.ProcessSubnets = true + }) + _, ok := impl.shouldHandlePing(pkt) + + // We don't handle this because it's a Tailscale IP and not 4via6 + if ok { + t.Errorf("expected shouldHandlePing==false") + } + }) + + t.Run("ICMP6-4via6", func(t *testing.T) { + // The 4via6 route 10.1.1.0/24 siteid 7, and then the IP + // 10.1.1.9 within that route. + dst := netip.MustParseAddr("fd7a:115c:a1e0:b1a:0:7:a01:109") + expectedPingDst := netip.MustParseAddr("10.1.1.9") + icmph := packet.ICMP6Header{ + IP6Header: packet.IP6Header{ + IPProto: ipproto.ICMPv6, + Src: srcIP, + Dst: dst, + }, + Type: packet.ICMP6EchoRequest, + Code: packet.ICMP6NoCode, + } + _, payload := packet.ICMPEchoPayload(nil) + icmpPing := packet.Generate(icmph, payload) + pkt := &packet.Parsed{} + pkt.Decode(icmpPing) + + impl := makeNetstack(t, func(impl *Impl) { + impl.ProcessSubnets = true + }) + pingDst, ok := impl.shouldHandlePing(pkt) + + // Handled due to being 4via6 + if !ok { + t.Errorf("expected shouldHandlePing==true") + } + if pingDst != expectedPingDst { + t.Errorf("got dst %s; want %s", pingDst, expectedPingDst) + } + }) +}