feature/conn25: send TSMP message to client for no IP mapping on connector

When a connector receives a packet from a client on a transit IP that it
can't find a real IP mapping for, it drops the packet. This commit
starts notifying the client of this dropping over TSMP, so the client
can tell the connector to re-establish the transit IP-real IP binding.

Updates tailscale/corp#34256.

Signed-off-by: Naman Sood <mail@nsood.in>

feature/conn25/conn25.go

@@ -195,11 +195,11 @@ func (e *extension) installHooks(dph *datapathHandler) error {
 		}
 		return dph.HandlePacketFromTunDevice(p)
 	}
-	tun.PostFilterPacketInboundFromWireGuardAppConnector = func(p *packet.Parsed, _ *tstun.Wrapper) filter.Response {
+	tun.PostFilterPacketInboundFromWireGuardAppConnector = func(p *packet.Parsed, tun *tstun.Wrapper) filter.Response {
 		if !e.conn25.isConfigured() {
 			return filter.Accept
 		}
-		return dph.HandlePacketFromWireGuard(p)
+		return dph.HandlePacketFromWireGuard(p, tun)
 	}
 
 	// Manage how we react to changes to the current node,

feature/conn25/datapath.go

@@ -11,6 +11,7 @@ import (
 	"tailscale.com/net/flowtrack"
 	"tailscale.com/net/packet"
 	"tailscale.com/net/packet/checksum"
+	"tailscale.com/net/tstun"
 	"tailscale.com/types/ipproto"
 	"tailscale.com/types/logger"
 	"tailscale.com/wgengine/filter"
@@ -100,7 +101,7 @@ func newDatapathHandler(ipMapper IPMapper, logf logger.Logf) *datapathHandler {
 // 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 WireGuard, on both connectors, and clients of connectors.
-func (dh *datapathHandler) HandlePacketFromWireGuard(p *packet.Parsed) filter.Response {
+func (dh *datapathHandler) HandlePacketFromWireGuard(p *packet.Parsed, tun *tstun.Wrapper) filter.Response {
 	// TODO(tailscale/corp#38764): Support other protocols, like ICMP for error messages.
 	if p.IPProto != ipproto.TCP && p.IPProto != ipproto.UDP {
 		return filter.Accept
@@ -130,7 +131,17 @@ func (dh *datapathHandler) HandlePacketFromWireGuard(p *packet.Parsed) filter.Re
 	realIP, err := dh.ipMapper.ConnectorRealIPForTransitIPConnection(p.Src.Addr(), transitIP)
 	if err != nil {
 		if errors.Is(err, ErrUnmappedSrcAndTransitIP) {
-			// TODO(tailscale/corp#34256): This path should deliver an ICMP error to the client.
+			rj := packet.TailscaleRejectedHeader{
+				IPSrc:  p.Dst.Addr(),
+				IPDst:  p.Src.Addr(),
+				Src:    p.Src,
+				Dst:    p.Dst,
+				Proto:  p.IPProto,
+				Reason: packet.RejectedDueToUnknownAppConnectorTransitIP,
+			}
+			if err := tun.InjectOutbound(packet.Generate(rj, nil)); err != nil {
+				dh.debugLogf("error sending TSMP flow rejection packet: %v", err)
+			}
 			return filter.Drop
 		}
 		dh.debugLogf("error mapping src and transit IP, passing packet unmodified: %v", err)

feature/conn25/datapath_test.go

@@ -4,13 +4,20 @@
 package conn25
 
 import (
+	"bytes"
 	"errors"
 	"net/netip"
 	"testing"
 
+	"go4.org/netipx"
 	"tailscale.com/net/packet"
+	"tailscale.com/net/tstun"
 	"tailscale.com/types/ipproto"
+	"tailscale.com/types/views"
+	"tailscale.com/util/eventbus/eventbustest"
+	"tailscale.com/util/usermetric"
 	"tailscale.com/wgengine/filter"
+	"tailscale.com/wgengine/filter/filtertype"
 )
 
 type testConn25 struct {
@@ -131,6 +138,39 @@ func TestHandlePacketFromTunDevice(t *testing.T) {
 	}
 }
 
+func newFakeTUN(t *testing.T) *tstun.Wrapper {
+	t.Helper()
+
+	// Create a TUN device and wrap it.
+	fake := tstun.NewFake()
+	reg := new(usermetric.Registry)
+	bus := eventbustest.NewBus(t)
+	tun := tstun.Wrap(t.Logf, fake, reg, bus)
+
+	// Create a packet filter. We're not testing the filter, so just make
+	// one that allows everything through.
+	protos := views.SliceOf([]ipproto.Proto{
+		ipproto.TCP,
+		ipproto.UDP,
+	})
+	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))
+
+	// Start the TUN device.
+	tun.Start()
+	return tun
+}
+
 func TestHandlePacketFromWireGuard(t *testing.T) {
 	clientSrcIP := netip.MustParseAddr("100.70.0.1")
 	unknownSrcIP := netip.MustParseAddr("100.99.99.99")
@@ -147,6 +187,7 @@ func TestHandlePacketFromWireGuard(t *testing.T) {
 		expectedSrc            netip.AddrPort
 		expectedDst            netip.AddrPort
 		expectedFilterResponse filter.Response
+		expectedInjectedPkt    []byte
 	}{
 		{
 			description: "accept-and-nat-new-connector-flow-mapped-src-and-transit-ip",
@@ -167,6 +208,14 @@ func TestHandlePacketFromWireGuard(t *testing.T) {
 			expectedSrc:            netip.AddrPortFrom(unknownSrcIP, clientPort),
 			expectedDst:            netip.AddrPortFrom(transitIP, serverPort),
 			expectedFilterResponse: filter.Drop,
+			expectedInjectedPkt: packet.Generate(packet.TailscaleRejectedHeader{
+				IPSrc:  transitIP,
+				IPDst:  unknownSrcIP,
+				Proto:  ipproto.UDP,
+				Src:    netip.AddrPortFrom(unknownSrcIP, clientPort),
+				Dst:    netip.AddrPortFrom(transitIP, serverPort),
+				Reason: packet.RejectedDueToUnknownAppConnectorTransitIP,
+			}, nil),
 		},
 		{
 			description: "accept-dont-nat-other-mapping-error",
@@ -218,12 +267,14 @@ func TestHandlePacketFromWireGuard(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.HandlePacketFromWireGuard(tt.p); want != got {
+			if want, got := tt.expectedFilterResponse, dph.HandlePacketFromWireGuard(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 {
@@ -232,6 +283,22 @@ func TestHandlePacketFromWireGuard(t *testing.T) {
 			if want, got := tt.expectedDst, tt.p.Dst; want != got {
 				t.Errorf("unexpected packet dst: want %v, got %v", want, got)
 			}
+			if tt.expectedInjectedPkt != nil {
+				var buf [tstun.MaxPacketSize]byte
+				bufs := [][]byte{buf[:]}
+				sizes := []int{0}
+				n, err := tun.Read(bufs, sizes, 0)
+				if err != nil {
+					t.Errorf("error reading injected packet: %v", err)
+				}
+				if n != 1 {
+					t.Errorf("expected to read 1 packet, got %d", n)
+				}
+				if want, got := tt.expectedInjectedPkt, buf[:sizes[0]]; !bytes.Equal(want, got) {
+					t.Errorf("unexpected contents of injected packet: want %+x, got %+x", want, got)
+
+				}
+			}
 		})
 	}
 }
@@ -290,7 +357,10 @@ func TestClientFlowCache(t *testing.T) {
 	}
 	incoming.StuffForTesting(40)
 
-	if dph.HandlePacketFromWireGuard(incoming) != filter.Accept {
+	tun := newFakeTUN(t)
+	defer tun.Close()
+
+	if dph.HandlePacketFromWireGuard(incoming, tun) != filter.Accept {
 		t.Errorf("call to HandlePacketFromWireGuard was not accepted")
 	}
 	if want, got := netip.AddrPortFrom(magicIP, serverPort), incoming.Src; want != got {
@@ -326,8 +396,11 @@ func TestConnectorFlowCache(t *testing.T) {
 	}
 	outgoing.StuffForTesting(40)
 
+	tun := newFakeTUN(t)
+	defer tun.Close()
+
 	o1 := outgoing
-	if dph.HandlePacketFromWireGuard(&o1) != filter.Accept {
+	if dph.HandlePacketFromWireGuard(&o1, tun) != filter.Accept {
 		t.Errorf("first call to HandlePacketFromWireGuard was not accepted")
 	}
 	if want, got := netip.AddrPortFrom(realIP, serverPort), o1.Dst; want != got {
@@ -335,7 +408,7 @@ func TestConnectorFlowCache(t *testing.T) {
 	}
 	// The second call should use the cache.
 	o2 := outgoing
-	if dph.HandlePacketFromWireGuard(&o2) != filter.Accept {
+	if dph.HandlePacketFromWireGuard(&o2, tun) != filter.Accept {
 		t.Errorf("second call to HandlePacketFromWireGuard was not accepted")
 	}
 	if want, got := netip.AddrPortFrom(realIP, serverPort), o2.Dst; want != got {

net/packet/tsmp.go

@@ -29,7 +29,7 @@ const minTSMPSize = 7 // the rejected body is 7 bytes
 // On the wire, after the IP header, it's currently 7 or 8 bytes:
 //   - '!'
 //   - IPProto byte (IANA protocol number: TCP or UDP)
-//   - 'A' or 'S' (RejectedDueToACLs, RejectedDueToShieldsUp)
+//   - byte stating rejection reason (see [TailscaleRejectReason] for valid values)
 //   - srcPort big endian uint16
 //   - dstPort big endian uint16
 //   - [optional] byte of flag bits:
@@ -101,6 +101,11 @@ const (
 	// RejectedDueToHostFirewall means that the target host's
 	// firewall is blocking the traffic.
 	RejectedDueToHostFirewall TailscaleRejectReason = 'W'
+
+	// RejectedDueToUnknownAppConnectorTransitIP means that the connector host has no real IP
+	// mapping that matches the provided transit IP for this client, so the
+	// connector has no destination to forward the connection to.
+	RejectedDueToUnknownAppConnectorTransitIP TailscaleRejectReason = 'T'
 )
 
 func (r TailscaleRejectReason) String() string {
@@ -113,6 +118,8 @@ func (r TailscaleRejectReason) String() string {
 		return "host-ip-forwarding-unavailable"
 	case RejectedDueToHostFirewall:
 		return "host-firewall"
+	case RejectedDueToUnknownAppConnectorTransitIP:
+		return "app-connector-transit-ip-unknown"
 	}
 	return fmt.Sprintf("0x%02x", byte(r))
 }