diff --git a/feature/conn25/conn25.go b/feature/conn25/conn25.go index e5433cd54..587d3eb97 100644 --- a/feature/conn25/conn25.go +++ b/feature/conn25/conn25.go @@ -203,11 +203,11 @@ func (e *extension) installHooks(dph *datapathHandler) error { // Intercept packets from the tun device and from WireGuard // to perform DNAT and SNAT. - tun.PreFilterPacketOutboundToWireGuardAppConnectorIntercept = func(p *packet.Parsed, _ *tstun.Wrapper) filter.Response { + tun.PreFilterPacketOutboundToWireGuardAppConnectorIntercept = func(p *packet.Parsed, tun *tstun.Wrapper) filter.Response { if !e.conn25.isConfigured() { return filter.Accept } - return dph.HandlePacketFromTunDevice(p) + return dph.HandlePacketFromTunDevice(p, tun) } tun.PostFilterPacketInboundFromWireGuardAppConnector = func(p *packet.Parsed, tun *tstun.Wrapper) filter.Response { if !e.conn25.isConfigured() { diff --git a/feature/conn25/datapath.go b/feature/conn25/datapath.go index 6eb8f897d..63bf248aa 100644 --- a/feature/conn25/datapath.go +++ b/feature/conn25/datapath.go @@ -204,7 +204,7 @@ func (dh *datapathHandler) HandlePacketFromWireGuard(p *packet.Parsed, tun *tstu // that the packet should pass through subsequent stages of the datapath pipeline. // Returning [filter.Drop] signals the packet should be dropped. This method handles all // packets coming from the tun device, on both connectors, and clients of connectors. -func (dh *datapathHandler) HandlePacketFromTunDevice(p *packet.Parsed) filter.Response { +func (dh *datapathHandler) HandlePacketFromTunDevice(p *packet.Parsed, tun *tstun.Wrapper) filter.Response { if !isSupportedProtocol(p.IPProto) { return filter.Accept } @@ -233,7 +233,11 @@ func (dh *datapathHandler) HandlePacketFromTunDevice(p *packet.Parsed) filter.Re transitIP, err := dh.conn25.ClientTransitIPForMagicIP(magicIP) if err != nil { if errors.Is(err, ErrUnmappedMagicIP) { - // TODO(tailscale/corp#34257): This path should deliver an ICMP error to the client. + // The destination is a Magic IP within a configured range, but + // there's no active Transit IP mapping for it (never assigned, or + // expired). Tell the local application the host is unreachable so + // it recovers quickly (e.g. re-resolves) instead of blackholing. + dh.sendICMPHostUnreachable(p, tun) return filter.Drop } dh.debugLogf("error mapping magic IP, passing packet unmodified: %v", err) @@ -273,6 +277,55 @@ func (dh *datapathHandler) HandlePacketFromTunDevice(p *packet.Parsed) filter.Re return filter.Accept } +// sendICMPHostUnreachable injects an ICMP "host unreachable" error (or its IPv6 +// equivalent, "address unreachable") back to the local host in response to the +// tun-device packet p, whose destination Magic IP has no active Transit IP +// mapping. The error is delivered inbound so the local application that sent p +// sees it, prompting it to recover (e.g. re-resolve) rather than the packet +// being silently blackholed. +func (dh *datapathHandler) sendICMPHostUnreachable(p *packet.Parsed, tun *tstun.Wrapper) { + dst := p.Dst.Addr() + src := p.Src.Addr() + + // The ICMP error payload carries the offending packet's IP header plus the + // first 8 bytes of its transport header, which is enough for the OS to match + // the error to the originating socket. The transport section begins right + // after the IP header, so its offset is the IP header length. + buf := p.Buffer() + ipHeaderLen := len(buf) - len(p.Transport()) + payload := buf[:min(ipHeaderLen+8, len(buf))] + + var errPkt []byte + switch { + case dst.Is4(): + errPkt = packet.Generate(packet.ICMP4Header{ + IP4Header: packet.IP4Header{ + // The error appears to come from the unreachable Magic IP, + // addressed back to the sender. + Src: dst, + Dst: src, + }, + Type: packet.ICMP4Unreachable, + Code: packet.ICMP4HostUnreachable, + }, payload) + case dst.Is6(): + errPkt = packet.Generate(packet.ICMP6Header{ + IP6Header: packet.IP6Header{ + Src: dst, + Dst: src, + }, + Type: packet.ICMP6Unreachable, + Code: packet.ICMP6AddressUnreachable, + }, payload) + default: + return + } + + if err := tun.InjectInboundCopy(errPkt); err != nil { + dh.debugLogf("error injecting ICMP host unreachable packet: %v", err) + } +} + func (dh *datapathHandler) dnatAction(to netip.Addr) PacketAction { return PacketAction(func(p *packet.Parsed) { checksum.UpdateDstAddr(p, to) }) } diff --git a/feature/conn25/datapath_test.go b/feature/conn25/datapath_test.go index 897d29f95..1b8be8490 100644 --- a/feature/conn25/datapath_test.go +++ b/feature/conn25/datapath_test.go @@ -8,7 +8,9 @@ "errors" "net/netip" "testing" + "time" + "github.com/tailscale/wireguard-go/tun/tuntest" "go4.org/netipx" "tailscale.com/net/packet" "tailscale.com/net/tstun" @@ -123,12 +125,14 @@ func TestHandlePacketFromTunDevice(t *testing.T) { return netip.Addr{}, nil } dph := newDatapathHandler(mock, t.Logf) + tun := newFakeTUN(t) + defer tun.Close() tt.p.IPProto = ipproto.UDP tt.p.IPVersion = 4 tt.p.StuffForTesting(40) - if want, got := tt.expectedFilterResponse, dph.HandlePacketFromTunDevice(tt.p); want != got { + if want, got := tt.expectedFilterResponse, dph.HandlePacketFromTunDevice(tt.p, tun); want != got { t.Errorf("unexpected filter response: want %v, got %v", want, got) } if want, got := tt.expectedSrc, tt.p.Src; want != got { @@ -141,6 +145,69 @@ func TestHandlePacketFromTunDevice(t *testing.T) { } } +// TestUnmappedMagicIPICMPUnreachable verifies that a packet to a Magic IP with +// no active Transit IP mapping is dropped and an ICMP host-unreachable error is +// injected back toward the local host, sourced from the Magic IP and addressed +// to the original sender. +func TestUnmappedMagicIPICMPUnreachable(t *testing.T) { + clientSrcIP := netip.MustParseAddr("100.70.0.1") + unusedMagicIP := netip.MustParseAddr("10.64.0.2") + clientPort := uint16(1234) + serverPort := uint16(80) + + mock := &testConn25{} + mock.clientTransitIPForMagicIPFn = func(netip.Addr) (netip.Addr, error) { + return netip.Addr{}, ErrUnmappedMagicIP + } + dph := newDatapathHandler(mock, t.Logf) + chtun, tun := newChannelTUN(t) + defer tun.Close() + + // Build a real UDP/IPv4 packet so the parsed transport offset is populated + // and the ICMP error payload extraction is exercised as in production. + raw := packet.Generate(packet.UDP4Header{ + IP4Header: packet.IP4Header{Src: clientSrcIP, Dst: unusedMagicIP}, + SrcPort: clientPort, + DstPort: serverPort, + }, []byte("hello")) + var p packet.Parsed + p.Decode(raw) + + // InjectInboundCopy blocks until the injected packet is read from the + // channel TUN's Inbound queue, so drain it concurrently. + got := make(chan []byte, 1) + go func() { got <- <-chtun.Inbound }() + + if want, got := filter.Drop, dph.HandlePacketFromTunDevice(&p, tun); want != got { + t.Fatalf("unexpected filter response: want %v, got %v", want, got) + } + + var injected packet.Parsed + select { + case b := <-got: + injected.Decode(b) + case <-time.After(10 * time.Second): + t.Fatal("timed out waiting for injected ICMP packet") + } + + if got := injected.IPProto; got != ipproto.ICMPv4 { + t.Errorf("injected packet proto: want ICMPv4, got %v", got) + } + if !injected.IsError() { + t.Errorf("injected packet is not an ICMP error") + } + if h := injected.ICMP4Header(); h.Type != packet.ICMP4Unreachable || h.Code != packet.ICMP4HostUnreachable { + t.Errorf("injected ICMP type/code: want %v/%v, got %v/%v", + packet.ICMP4Unreachable, packet.ICMP4HostUnreachable, h.Type, h.Code) + } + if want, got := unusedMagicIP, injected.Src.Addr(); want != got { + t.Errorf("injected packet src: want %v, got %v", want, got) + } + if want, got := clientSrcIP, injected.Dst.Addr(); want != got { + t.Errorf("injected packet dst: want %v, got %v", want, got) + } +} + func newFakeTUN(t *testing.T) *tstun.Wrapper { t.Helper() @@ -174,6 +241,40 @@ func newFakeTUN(t *testing.T) *tstun.Wrapper { return tun } +// newChannelTUN is like newFakeTUN, but backed by a channel-based TUN device +// whose Inbound queue captures packets injected toward the local host (e.g. via +// InjectInboundCopy), so tests can observe them. +func newChannelTUN(t *testing.T) (*tuntest.ChannelTUN, *tstun.Wrapper) { + t.Helper() + + chtun := tuntest.NewChannelTUN() + reg := new(usermetric.Registry) + bus := eventbustest.NewBus(t) + tun := tstun.Wrap(t.Logf, chtun.TUN(), reg, bus) + + protos := views.SliceOf([]ipproto.Proto{ + ipproto.TCP, + ipproto.UDP, + ipproto.ICMPv4, + ipproto.ICMPv6, + }) + allIPs := netip.MustParsePrefix("0.0.0.0/0") + matches := []filter.Match{ + { + IPProto: protos, + Srcs: []netip.Prefix{allIPs}, + Dsts: []filtertype.NetPortRange{{Net: allIPs, Ports: filtertype.AllPorts}}, + }, + } + var sb netipx.IPSetBuilder + sb.AddPrefix(allIPs) + ipSet, _ := sb.IPSet() + tun.SetFilter(filter.New(matches, nil, ipSet, ipSet, nil, t.Logf)) + + tun.Start() + return chtun, tun +} + func TestHandlePacketFromWireGuard(t *testing.T) { clientSrcIP := netip.MustParseAddr("100.70.0.1") unknownSrcIP := netip.MustParseAddr("100.99.99.99") @@ -325,6 +426,8 @@ func TestClientFlowCache(t *testing.T) { return transitIP, nil } dph := newDatapathHandler(mock, t.Logf) + tun := newFakeTUN(t) + defer tun.Close() outgoing := packet.Parsed{ IPProto: ipproto.UDP, @@ -335,7 +438,7 @@ func TestClientFlowCache(t *testing.T) { outgoing.StuffForTesting(40) o1 := outgoing - if dph.HandlePacketFromTunDevice(&o1) != filter.Accept { + if dph.HandlePacketFromTunDevice(&o1, tun) != filter.Accept { t.Errorf("first call to HandlePacketFromTunDevice was not accepted") } if want, got := netip.AddrPortFrom(transitIP, serverPort), o1.Dst; want != got { @@ -343,7 +446,7 @@ func TestClientFlowCache(t *testing.T) { } // The second call should use the cache. o2 := outgoing - if dph.HandlePacketFromTunDevice(&o2) != filter.Accept { + if dph.HandlePacketFromTunDevice(&o2, tun) != filter.Accept { t.Errorf("second call to HandlePacketFromTunDevice was not accepted") } if want, got := netip.AddrPortFrom(transitIP, serverPort), o2.Dst; want != got { @@ -360,9 +463,6 @@ func TestClientFlowCache(t *testing.T) { } incoming.StuffForTesting(40) - tun := newFakeTUN(t) - defer tun.Close() - if dph.HandlePacketFromWireGuard(incoming, tun) != filter.Accept { t.Errorf("call to HandlePacketFromWireGuard was not accepted") } @@ -428,7 +528,7 @@ func TestConnectorFlowCache(t *testing.T) { } incoming.StuffForTesting(40) - if dph.HandlePacketFromTunDevice(incoming) != filter.Accept { + if dph.HandlePacketFromTunDevice(incoming, tun) != filter.Accept { t.Errorf("call to HandlePacketFromTunDevice was not accepted") } if want, got := netip.AddrPortFrom(transitIP, serverPort), incoming.Src; want != got { diff --git a/net/packet/icmp4.go b/net/packet/icmp4.go index 492a0e9df..8749e91ab 100644 --- a/net/packet/icmp4.go +++ b/net/packet/icmp4.go @@ -49,6 +49,10 @@ func (t ICMP4Type) String() string { const ( ICMP4NoCode ICMP4Code = 0 + + // ICMP4HostUnreachable is the code used with ICMP4Unreachable to + // indicate that the destination host could not be reached. + ICMP4HostUnreachable ICMP4Code = 1 ) // ICMP4Header is an IPv4+ICMPv4 header. diff --git a/net/packet/icmp6.go b/net/packet/icmp6.go index a91db53c9..faa45c853 100644 --- a/net/packet/icmp6.go +++ b/net/packet/icmp6.go @@ -52,6 +52,11 @@ func (t ICMP6Type) String() string { const ( ICMP6NoCode ICMP6Code = 0 + + // ICMP6AddressUnreachable is the code used with ICMP6Unreachable to + // indicate that the destination address could not be reached. It is + // the IPv6 equivalent of ICMP4HostUnreachable. + ICMP6AddressUnreachable ICMP6Code = 3 ) // ICMP6Header is an IPv4+ICMPv4 header.