From 47333e9487c1747174232f20db60d48799465ee6 Mon Sep 17 00:00:00 2001 From: Naman Sood Date: Wed, 17 Jun 2026 13:50:51 -0400 Subject: [PATCH] feature/conn25: recreate transit IP mappings when connector loses them Mappings from transit IPs to real IPs are stored ephemerally in the connector, so they're lost on restart. When we send a packet to the connector with a transit IP it does not recognize, it sends us a TSMP message saying so (see #19883). If we (the client) know of such a mapping, we now re-send it to the connector so that a connection can proceed. Fixes tailscale/corp#34256. Signed-off-by: Naman Sood --- feature/conn25/conn25.go | 64 ++++++++++++++++++++++++----------- feature/conn25/conn25_test.go | 46 ++++++++++++++----------- net/tstun/wrap.go | 11 ++++++ wgengine/pendopen.go | 9 ++++- 4 files changed, 91 insertions(+), 39 deletions(-) diff --git a/feature/conn25/conn25.go b/feature/conn25/conn25.go index 56b3affa7..73b25fcd9 100644 --- a/feature/conn25/conn25.go +++ b/feature/conn25/conn25.go @@ -202,6 +202,12 @@ func (e *extension) installHooks(dph *datapathHandler) error { } return dph.HandlePacketFromWireGuard(p, tun) } + tun.OnUnmappedTransitIPMessage = func(pkt packet.TailscaleRejectedHeader) { + if !e.conn25.isConfigured() { + return + } + e.conn25.client.resendTransitIPMapping(pkt.Dst.Addr()) + } // Manage how we react to changes to the current node, // including property changes (e.g. HostInfo, Capabilities, CapMap). @@ -384,21 +390,25 @@ func (c *Conn25) isConfigured() bool { func newConn25(logf logger.Logf) *Conn25 { c := &Conn25{ - logf: logf, - connector: &connector{logf: logf}, + logf: logf, + } + getIPSets := func() ipSets { + cfg, ok := c.getConfig() + if !ok { + return emptyIPSets() + } + return cfg.ipSets } c.config.Store(&config{}) // initialize with empty to avoid nil checks c.client = &client{ logf: logf, addrsCh: make(chan addrs, 64), assignments: addrAssignments{clock: tstime.StdClock{}}, - getIPSets: func() ipSets { - cfg, ok := c.getConfig() - if !ok { - return emptyIPSets() - } - return cfg.ipSets - }, + getIPSets: getIPSets, + } + c.connector = &connector{ + logf: logf, + getIPSets: getIPSets, } return c } @@ -1264,7 +1274,8 @@ func (c *client) rewriteDNSResponse(appName string, hdr dnsmessage.Header, quest } type connector struct { - logf logger.Logf + logf logger.Logf + getIPSets func() ipSets mu sync.Mutex // protects the fields below // transitIPs is a map of connector client peer IP -> client transitIPs that we update as connector client peers instruct us to, and then use to route traffic to its destination on behalf of connector clients. @@ -1286,13 +1297,16 @@ func (c *connector) realIPForTransitIPConnection(srcIP netip.Addr, transitIP net const packetFilterAllowReason = "app connector transit IP" -// packetFilterAllow returns true if the provided packet has a Src that maps to a peer -// that has a transit IP with us that is the packet Dst, and false otherwise. +// packetFilterAllow returns true if the provided packet has a Src that is in +// the configured transit IP range for this connector, false otherwise. func (c *connector) packetFilterAllow(p packet.Parsed) (bool, string) { c.mu.Lock() defer c.mu.Unlock() - _, ok := c.lookupBySrcIPAndTransitIP(p.Src.Addr(), p.Dst.Addr()) - if ok { + ipSets := c.getIPSets() + if ipSets.v4Transit != nil && ipSets.v4Transit.Contains(p.Dst.Addr()) { + return true, packetFilterAllowReason + } + if ipSets.v6Transit != nil && ipSets.v6Transit.Contains(p.Dst.Addr()) { return true, packetFilterAllowReason } return false, "" @@ -1338,11 +1352,7 @@ func (c *client) insertTransitConnMapping(tip netip.Addr, connKey key.NodePublic ctips, ok := c.byConnKey[connKey] tipp := netip.PrefixFrom(tip, tip.BitLen()) - if ok { - if ctips.Contains(tipp) { - return errors.New("byConnKey already contains transit") - } - } else { + if !ok { ctips.Make() mak.Set(&c.byConnKey, connKey, ctips) } @@ -1360,3 +1370,19 @@ func (c *client) lookupTransitIPsByConnKey(k key.NodePublic) ([]netip.Prefix, bo } return s.Slice(), true } + +// resendTransitIPMapping enqueues a request to re-establish an existing +// transit IP-real IP mapping after a connector tells the client that the +// mapping does not exist on its end. If a mapping is not found on the client +// either, this is a no-op. +func (c *client) resendTransitIPMapping(transitIP netip.Addr) { + mapping, ok := c.assignments.lookupByTransitIP(transitIP) + if !ok { + // We have no mappings for this transit IP, so nothing to resend. + return + } + err := c.enqueueAddressAssignment(mapping) + if err != nil { + c.logf("error enqueueing address assignment for resend: %v", err) + } +} diff --git a/feature/conn25/conn25_test.go b/feature/conn25/conn25_test.go index a1063de74..082dcbc78 100644 --- a/feature/conn25/conn25_test.go +++ b/feature/conn25/conn25_test.go @@ -2233,9 +2233,10 @@ func TestTransitIPConnMapping(t *testing.T) { if err := conn25.client.addTransitIPForConnector(as.transit, connectorPeers[1]); err != nil { t.Errorf("unexpected error for first time add: %v", err) } - // But doing it again should fail - if err := conn25.client.addTransitIPForConnector(as.transit, connectorPeers[1]); err == nil { - t.Error("adding a duplicate transitIP for a connector should fail") + // And doing it again shouldn't fail (this is done when resending mappings + // to a restarted connector) + if err := conn25.client.addTransitIPForConnector(as.transit, connectorPeers[1]); err != nil { + t.Errorf("error adding duplicate transitIP for a connector: %v", err) } } @@ -2407,9 +2408,13 @@ func TestIsKnownTransitIP(t *testing.T) { unknownTip := netip.MustParseAddr("100.64.0.42") c := newConn25(t.Logf) - c.client.assignments.insert(&addrs{ + err := c.client.assignments.insert(&addrs{ transit: knownTip, }) + if err != nil { + t.Errorf("error inserting address assignment: %v", err) + return + } if !c.client.isKnownTransitIP(knownTip) { t.Fatal("knownTip: should have been known") @@ -2423,9 +2428,12 @@ func TestLinkLocalAllow(t *testing.T) { knownTip := netip.MustParseAddr("100.64.0.41") c := newConn25(t.Logf) - c.client.assignments.insert(&addrs{ + err := c.client.assignments.insert(&addrs{ transit: knownTip, }) + if err != nil { + t.Fatalf("error inserting address assignment: %v", err) + } if allow, _ := c.client.linkLocalAllow(packet.Parsed{ Dst: netip.AddrPortFrom(knownTip, 1234), @@ -2441,31 +2449,31 @@ func TestLinkLocalAllow(t *testing.T) { } func TestConnectorPacketFilterAllow(t *testing.T) { - knownTip := netip.MustParseAddr("100.64.0.41") - knownSrc := netip.MustParseAddr("100.64.0.1") + src := netip.MustParseAddr("100.64.0.1") + knownTip := netip.MustParseAddr("192.0.2.1") unknownTip := netip.MustParseAddr("100.64.0.42") - unknownSrc := netip.MustParseAddr("100.64.0.42") + + v4TransitIPsBuilder := netipx.IPSetBuilder{} + v4TransitIPsBuilder.AddPrefix(netip.MustParsePrefix("192.0.2.0/24")) + v4TransitIPs := must.Get(v4TransitIPsBuilder.IPSet()) c := newConn25(t.Logf) - c.connector.transitIPs = map[netip.Addr]map[netip.Addr]appAddr{} - c.connector.transitIPs[knownSrc] = map[netip.Addr]appAddr{} - c.connector.transitIPs[knownSrc][knownTip] = appAddr{} + c.reconfig(&config{ + isConfigured: true, + ipSets: ipSets{ + v4Transit: v4TransitIPs, + }, + }) if allow, _ := c.connector.packetFilterAllow(packet.Parsed{ - Src: netip.AddrPortFrom(knownSrc, 1234), + Src: netip.AddrPortFrom(src, 1234), Dst: netip.AddrPortFrom(knownTip, 1234), }); !allow { t.Fatal("knownTip: should have been allowed") } if allow, _ := c.connector.packetFilterAllow(packet.Parsed{ - Src: netip.AddrPortFrom(unknownSrc, 1234), - Dst: netip.AddrPortFrom(knownTip, 1234), - }); allow { - t.Fatal("unknownSrc: should not have been allowed") - } - if allow, _ := c.connector.packetFilterAllow(packet.Parsed{ - Src: netip.AddrPortFrom(knownSrc, 1234), + Src: netip.AddrPortFrom(src, 1234), Dst: netip.AddrPortFrom(unknownTip, 1234), }); allow { t.Fatal("unknownTip: should not have been allowed") diff --git a/net/tstun/wrap.go b/net/tstun/wrap.go index ef4c98fd2..7fef73b2e 100644 --- a/net/tstun/wrap.go +++ b/net/tstun/wrap.go @@ -203,6 +203,11 @@ type Wrapper struct { // false otherwise. OnICMPEchoResponseReceived func(*packet.Parsed) bool + // OnUnmappedTransitIPMessage, if non-nil, is called when a TSMP message is + // received indicating that a packet was rejected by a connector due to a + // missing transit IP->real IP mapping. + OnUnmappedTransitIPMessage func(packet.TailscaleRejectedHeader) + // PeerAPIPort, if non-nil, returns the peerapi port that's // running for the given IP address. PeerAPIPort func(netip.Addr) (port uint16, ok bool) @@ -1171,6 +1176,12 @@ func (t *Wrapper) filterPacketInboundFromWireGuard(p *packet.Parsed, captHook pa if f := t.OnTSMPPongReceived; f != nil { f(data) } + } else if data, ok := p.AsTailscaleRejectedHeader(); ok { + if data.Reason == packet.RejectedDueToUnknownAppConnectorTransitIP { + if f := t.OnUnmappedTransitIPMessage; f != nil { + f(data) + } + } } } diff --git a/wgengine/pendopen.go b/wgengine/pendopen.go index e816506de..4c830bf68 100644 --- a/wgengine/pendopen.go +++ b/wgengine/pendopen.go @@ -68,7 +68,6 @@ func (e *userspaceEngine) trackOpenPreFilterIn(pp *packet.Parsed, t *tstun.Wrapp res = filter.Accept // always if pp.IPProto == ipproto.TSMP { - res = filter.DropSilently rh, ok := pp.AsTailscaleRejectedHeader() if !ok { return @@ -78,6 +77,14 @@ func (e *userspaceEngine) trackOpenPreFilterIn(pp *packet.Parsed, t *tstun.Wrapp } else if f := tsRejectFlow(rh); e.removeFlow(f) { e.logf("open-conn-track: flow %v %v > %v rejected due to %v", rh.Proto, rh.Src, rh.Dst, rh.Reason) } + switch rh.Reason { + case packet.RejectedDueToUnknownAppConnectorTransitIP: + // Keep res = filter.Accept, don't drop this packet because it will + // be used later for further communication between app connector + // and client. + default: + res = filter.DropSilently + } return }