From 65fe958033cc0bc71bfb34955c77c730be97aa94 Mon Sep 17 00:00:00 2001 From: Jordan Whited Date: Tue, 7 Jul 2026 15:54:03 -0700 Subject: [PATCH] ipn/ipnlocal,wgengine: implement wg-go SetPriorityMessageOnEstablishmentFunc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements a way to send TSMPDiscoAdverts based on a trigger from wireguard-go when a rekey happens. This lets us distribute disco keys consistently, but also sets us up for a minimal message that can be distributed to other clients. A benchmark is implemented to make it easier to keep the call cheap and to avoid locking up anything in wireguard-go. Updates #20081 Co-authored-by: Jordan Whited Signed-off-by: Claus Lensbøl --- flake.nix | 2 +- flakehashes.json | 4 +- go.mod | 2 +- go.sum | 4 +- ipn/ipnlocal/local.go | 1 + ipn/ipnlocal/state_test.go | 2 + shell.nix | 2 +- tstest/natlab/vmtest/vmtest_test.go | 5 +- wgengine/magicsock/endpoint.go | 1 - wgengine/magicsock/magicsock.go | 94 ++++++------ wgengine/magicsock/magicsock_test.go | 222 ++++++++------------------- wgengine/userspace.go | 48 ++---- wgengine/userspace_test.go | 58 ------- wgengine/wgengine.go | 2 + 14 files changed, 136 insertions(+), 311 deletions(-) diff --git a/flake.nix b/flake.nix index 4f0dc908e..74c2cae75 100644 --- a/flake.nix +++ b/flake.nix @@ -169,4 +169,4 @@ }); }; } -# nix-direnv cache busting line: sha256-sWU4abv9Oz7P21ivL5zgdYNGiJSXamnQR0VmRGKoIrI= +# nix-direnv cache busting line: sha256-hALPmyYeWNhA5J3AKF7Pcs35DuWMBovdVeGHu3VRD48= diff --git a/flakehashes.json b/flakehashes.json index f4095d4d6..983c074a4 100644 --- a/flakehashes.json +++ b/flakehashes.json @@ -4,7 +4,7 @@ "sri": "sha256-cY5yryX+p/xtoTv+WZEKFagiIl0OREHnJY1Bk5VpVVc=" }, "vendor": { - "goModSum": "sha256-gFAlZFTtqRlHzg0hHneMjNpkJ/CLuHEmeEeKT6fHRm0=", - "sri": "sha256-sWU4abv9Oz7P21ivL5zgdYNGiJSXamnQR0VmRGKoIrI=" + "goModSum": "sha256-KTSwQm4eaIgdfZK+aUsyOfuiErh8EsMwL9NGkLkQRBo=", + "sri": "sha256-hALPmyYeWNhA5J3AKF7Pcs35DuWMBovdVeGHu3VRD48=" } } diff --git a/go.mod b/go.mod index 16f0673d8..98e30be6f 100644 --- a/go.mod +++ b/go.mod @@ -112,7 +112,7 @@ require ( github.com/tailscale/ts-gokrazy v0.0.0-20260630224145-b83088f2e52e github.com/tailscale/web-client-prebuilt v0.0.0-20250124233751-d4cd19a26976 github.com/tailscale/wf v0.0.0-20240214030419-6fbb0a674ee6 - github.com/tailscale/wireguard-go v0.0.0-20260622164646-ae172d45f0f7 + github.com/tailscale/wireguard-go v0.0.0-20260707224101-b3d9faf49fdc github.com/tailscale/xnet v0.0.0-20240729143630-8497ac4dab2e github.com/tc-hib/winres v0.2.1 github.com/tcnksm/go-httpstat v0.2.0 diff --git a/go.sum b/go.sum index c1dd32133..0a3fda818 100644 --- a/go.sum +++ b/go.sum @@ -1213,8 +1213,8 @@ github.com/tailscale/web-client-prebuilt v0.0.0-20250124233751-d4cd19a26976 h1:U github.com/tailscale/web-client-prebuilt v0.0.0-20250124233751-d4cd19a26976/go.mod h1:agQPE6y6ldqCOui2gkIh7ZMztTkIQKH049tv8siLuNQ= github.com/tailscale/wf v0.0.0-20240214030419-6fbb0a674ee6 h1:l10Gi6w9jxvinoiq15g8OToDdASBni4CyJOdHY1Hr8M= github.com/tailscale/wf v0.0.0-20240214030419-6fbb0a674ee6/go.mod h1:ZXRML051h7o4OcI0d3AaILDIad/Xw0IkXaHM17dic1Y= -github.com/tailscale/wireguard-go v0.0.0-20260622164646-ae172d45f0f7 h1:u/nUghv4ajOqL+HelhnXtYifN4NHN3NuvQPeZRAds3s= -github.com/tailscale/wireguard-go v0.0.0-20260622164646-ae172d45f0f7/go.mod h1:6SerzcvHWQchKO2BfNdmquA77CHSECZuFl+D9fp4RnI= +github.com/tailscale/wireguard-go v0.0.0-20260707224101-b3d9faf49fdc h1:mbpILtghRLOKuhLlEO8iii7px3gWH1Wtl/OBfS8snrk= +github.com/tailscale/wireguard-go v0.0.0-20260707224101-b3d9faf49fdc/go.mod h1:6SerzcvHWQchKO2BfNdmquA77CHSECZuFl+D9fp4RnI= github.com/tailscale/xnet v0.0.0-20240729143630-8497ac4dab2e h1:zOGKqN5D5hHhiYUp091JqK7DPCqSARyUfduhGUY8Bek= github.com/tailscale/xnet v0.0.0-20240729143630-8497ac4dab2e/go.mod h1:orPd6JZXXRyuDusYilywte7k094d7dycXXU5YnWsrwg= github.com/tc-hib/winres v0.2.1 h1:YDE0FiP0VmtRaDn7+aaChp1KiF4owBiJa5l964l5ujA= diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 5fafc9043..37fdf3b7e 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -636,6 +636,7 @@ func NewLocalBackend(logf logger.Logf, logID logid.PublicID, sys *tsd.System, lo e.SetPeerByIPPacketFunc(b.lookupPeerByIP) e.SetPeerForIPFunc(b.peerForIP) e.SetPeerSessionStateFunc(b.onPeerWireGuardState) + e.SetPeerPriorityMessageOnEstablishmentFunc(b.MagicConn().PriorityMessageForPeer) e.SetNetLogNodeSource(netLogNodeSource{b}) e.SetWGPeerLookup(b.lookupPeerWireGuardString) b.dialer.SetResolveMagicDNS(b.resolveMagicDNS) diff --git a/ipn/ipnlocal/state_test.go b/ipn/ipnlocal/state_test.go index 7c1c9a23b..c17272709 100644 --- a/ipn/ipnlocal/state_test.go +++ b/ipn/ipnlocal/state_test.go @@ -1989,6 +1989,8 @@ func (e *mockEngine) PeerKeyForIP(netip.Addr) (_ key.NodePublic, _ netip.Prefix, } func (e *mockEngine) SetPeerSessionStateFunc(func(key.NodePublic, wgengine.PeerWireGuardState)) { } +func (e *mockEngine) SetPeerPriorityMessageOnEstablishmentFunc(fn func(key.NodePublic) (msg []byte)) { +} func (e *mockEngine) SetNetLogNodeSource(netlog.NodeSource) {} func (e *mockEngine) SetWGPeerLookup(func(wgString string) (tsString string, ok bool)) {} func (e *mockEngine) ProbeLocks() {} diff --git a/shell.nix b/shell.nix index 65bcd1105..4fef3cba4 100644 --- a/shell.nix +++ b/shell.nix @@ -16,4 +16,4 @@ ) { src = ./.; }).shellNix -# nix-direnv cache busting line: sha256-sWU4abv9Oz7P21ivL5zgdYNGiJSXamnQR0VmRGKoIrI= +# nix-direnv cache busting line: sha256-hALPmyYeWNhA5J3AKF7Pcs35DuWMBovdVeGHu3VRD48= diff --git a/tstest/natlab/vmtest/vmtest_test.go b/tstest/natlab/vmtest/vmtest_test.go index f69fcf7f4..d5a7abaf7 100644 --- a/tstest/natlab/vmtest/vmtest_test.go +++ b/tstest/natlab/vmtest/vmtest_test.go @@ -1165,7 +1165,7 @@ func TestCachedNetmapAfterRestart(t *testing.T) { func TestDirectConnectionWithCachedNetmapOnOneNode(t *testing.T) { for _, testPingFrom := range []string{"offline", "online"} { t.Run(fmt.Sprintf("ping_from_%s", testPingFrom), func(t *testing.T) { - if testPingFrom == "online" { + if testPingFrom == "online" && false { t.Skip("https://github.com/tailscale/tailscale/issues/19843") } env := vmtest.New(t) @@ -1257,7 +1257,6 @@ func TestDirectConnectionWithCachedNetmapOnOneNode(t *testing.T) { // however not be sent if there is not endpoints on the online node which is // possible in the event where a node has registered but not finished STUN. func TestDirectConnectionWithCachedNetmapOnTwoNodes(t *testing.T) { - t.Skip("https://github.com/tailscale/tailscale/issues/19843") env := vmtest.New(t) aNet := env.AddNetwork("1.0.0.1", "192.168.1.1/24", vnet.EasyNAT) @@ -1284,6 +1283,7 @@ func TestDirectConnectionWithCachedNetmapOnTwoNodes(t *testing.T) { checkClientMetrics(t, "Node A", env.ClientMetrics(a), map[string]int64{ "magicsock_cached_peer_contact_derp": 0, "magicsock_cached_peer_contact_direct": 0, + "magicsock_tsmp_disco_key_advertisement_sent": 0, }) checkInitialMetrics.End(nil) @@ -1319,6 +1319,7 @@ func TestDirectConnectionWithCachedNetmapOnTwoNodes(t *testing.T) { checkFinalMetrics.Begin() checkClientMetrics(t, "Node A", env.ClientMetrics(a), map[string]int64{ "magicsock_cached_peer_contact_direct": 1, + "magicsock_tsmp_disco_key_advertisement_sent": 1, }) checkFinalMetrics.End(nil) } diff --git a/wgengine/magicsock/endpoint.go b/wgengine/magicsock/endpoint.go index 2f144d0ea..aa8fcbdb5 100644 --- a/wgengine/magicsock/endpoint.go +++ b/wgengine/magicsock/endpoint.go @@ -85,7 +85,6 @@ type endpoint struct { lastSendAny mono.Time // last time there were outgoing packets sent this peer from any trigger, internal or external to magicsock lastFullPing mono.Time // last time we pinged all disco or wireguard only endpoints lastUDPRelayPathDiscovery mono.Time // last time we ran UDP relay path discovery - lastDiscoKeyAdvertisement mono.Time // last time we sent a TSMPDiscoAdvertisement or not to this endpoint derpAddr netip.AddrPort // fallback/bootstrap path, if non-zero (non-zero for well-behaved clients) bestAddr addrQuality // best non-DERP path; zero if none; mutate via setBestAddrLocked() diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index 8e4c6bba7..65eb5671e 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -180,10 +180,9 @@ type Conn struct { connCtxCancel func() // closes connCtx donec <-chan struct{} // connCtx.Done()'s to avoid context.cancelCtx.Done()'s mutex per call - allocRelayEndpointPub *eventbus.Publisher[UDPRelayAllocReq] - portUpdatePub *eventbus.Publisher[router.PortUpdate] - tsmpDiscoKeyAvailablePub *eventbus.Publisher[NewDiscoKeyAvailable] - homeDERPChangedPub *eventbus.Publisher[HomeDERPChanged] + allocRelayEndpointPub *eventbus.Publisher[UDPRelayAllocReq] + portUpdatePub *eventbus.Publisher[router.PortUpdate] + homeDERPChangedPub *eventbus.Publisher[HomeDERPChanged] // pconn4 and pconn6 are the underlying UDP sockets used to // send/receive packets for wireguard and other magicsock @@ -679,7 +678,6 @@ func NewConn(opts Options) (*Conn, error) { c.eventClient = ec c.allocRelayEndpointPub = eventbus.Publish[UDPRelayAllocReq](ec) c.portUpdatePub = eventbus.Publish[router.PortUpdate](ec) - c.tsmpDiscoKeyAvailablePub = eventbus.Publish[NewDiscoKeyAvailable](ec) c.homeDERPChangedPub = eventbus.Publish[HomeDERPChanged](ec) eventbus.SubscribeFunc(ec, c.onPortMapChanged) eventbus.SubscribeFunc(ec, c.onUDPRelayAllocResp) @@ -1254,8 +1252,7 @@ func (c *Conn) DiscoPublicKey() key.DiscoPublic { // RotateDiscoKey generates a new discovery key pair and updates the connection // to use it. This invalidates all existing disco sessions and will cause peers -// to re-establish discovery sessions with the new key. Addtionally, the -// lastTSMPDiscoAdvertisement on all endpoints is reset to 0. +// to re-establish discovery sessions with the new key. // // This is primarily for debugging and testing purposes, a future enhancement // should provide a mechanism for seamless rotation by supporting short term use @@ -1269,11 +1266,6 @@ func (c *Conn) RotateDiscoKey() { newShort := c.discoAtomic.Short() c.discoInfo = make(map[key.DiscoPublic]*discoInfo) connCtx := c.connCtx - for _, endpoint := range c.peerMap.byEpAddr { - endpoint.ep.mu.Lock() - endpoint.ep.lastDiscoKeyAdvertisement = 0 - endpoint.ep.mu.Unlock() - } c.mu.Unlock() c.logf("magicsock: rotated disco key from %v to %v", oldShort, newShort) @@ -2678,8 +2670,6 @@ func (c *Conn) enqueueCallMeMaybe(derpAddr netip.AddrPort, de *endpoint) { return } - c.maybeSendTSMPDiscoAdvert(de) - eps := make([]netip.AddrPort, 0, len(c.lastEndpoints)) for _, ep := range c.lastEndpoints { eps = append(eps, ep.Addr) @@ -4320,6 +4310,8 @@ type discoInfo struct { metricTSMPDiscoKeyAdvertisementReceived = clientmetric.NewCounter("magicsock_tsmp_disco_key_advertisement_received") metricTSMPDiscoKeyAdvertisementApplied = clientmetric.NewCounter("magicsock_tsmp_disco_key_advertisement_applied") metricTSMPDiscoKeyAdvertisementUnchanged = clientmetric.NewCounter("magicsock_tsmp_disco_key_advertisement_unchanged") + metricTSMPDiscoKeyAdvertisementSent = clientmetric.NewCounter("magicsock_tsmp_disco_key_advertisement_sent") + metricTSMPDiscoKeyAdvertisementError = clientmetric.NewCounter("magicsock_tsmp_disco_key_advertisement_error") // Counters for peer contacts established using cached network map data. metricCachedPeerContactDERP = clientmetric.NewCounter("magicsock_cached_peer_contact_derp") @@ -4552,46 +4544,54 @@ type NewDiscoKeyAvailable struct { NodeID tailcfg.NodeID } -// maybeSendTSMPDiscoAdvert conditionally emits an event indicating that we -// should send our DiscoKey to the first node address of the magicksock endpoint. -// -// The event is suppressed if we are communicating over a non-DERP path, or -// less than [discoKeyAdvertisementInterval] has passed since the last DiscoKey -// was sent, or netmap caching is disabled on this node. -// -// We do not need the Conn to be locked, but the endpoint should be. -func (c *Conn) maybeSendTSMPDiscoAdvert(de *endpoint) { +func (c *Conn) PriorityMessageForPeer(nodeKey key.NodePublic) []byte { if !buildfeatures.HasCacheNetMap || !envknob.BoolDefaultTrue("TS_USE_CACHED_NETMAP") { - return + return nil } - // Disable TSMP disco advert by default, unless network map caching is - // enabled for the local node. Caching network maps on the remote node is - // what really matters in terms of handling a TSMP disco advert and applying - // it in a useful way, but the TSMP disco advert implementation as it exists - // here has pathological behaviors. Therefore, it should be disabled for - // almost all tailnets, and we lean on the network map caching control knob - // for this purpose. See #20081. - if c.controlKnobs == nil || !c.controlKnobs.CacheNetworkMaps.Load() { - return + disco := c.DiscoPublicKey() + if disco.IsZero() { + metricTSMPDiscoKeyAdvertisementError.Add(1) + return nil } - de.mu.Lock() - defer de.mu.Unlock() - - if !de.nodeAddr.IsValid() { - return + c.mu.Lock() + self := c.self + ep, ok := c.peerMap.endpointForNodeKey(nodeKey) + c.mu.Unlock() + if !ok || !self.Valid() { + metricTSMPDiscoKeyAdvertisementError.Add(1) + return nil } - now := mono.Now() - if now.Sub(de.lastDiscoKeyAdvertisement) <= discoKeyAdvertisementInterval || - (!de.lastDiscoKeyAdvertisement.IsZero() && !de.bestAddr.isZero()) { - return + ep.mu.Lock() + dst := ep.nodeAddr + ep.mu.Unlock() + + // resolve our own Tailscale address in the same family as dst + src := selfIPMatchingFamily(self, dst) + if !src.IsValid() { + metricTSMPDiscoKeyAdvertisementError.Add(1) + return nil } - de.lastDiscoKeyAdvertisement = now - c.tsmpDiscoKeyAvailablePub.Publish(NewDiscoKeyAvailable{ - NodeFirstAddr: de.nodeAddr, - NodeID: de.nodeID, - }) + tdka := packet.TSMPDiscoKeyAdvertisement{Src: src, Dst: dst, Key: disco} + payload, err := tdka.Marshal() + if err != nil { + metricTSMPDiscoKeyAdvertisementError.Add(1) + return nil + } + metricTSMPDiscoKeyAdvertisementSent.Add(1) + return payload +} + +// selfIPMatchingFamily returns self's first single-IP Tailscale address whose +// family matches want, or the zero Addr. self must be Valid. +func selfIPMatchingFamily(self tailcfg.NodeView, want netip.Addr) netip.Addr { + for _, p := range self.Addresses().All() { + if p.IsSingleIP() && p.Addr().BitLen() == want.BitLen() { + return p.Addr() + } + } + return netip.Addr{} } diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go index 84b4e999e..b8fcf0f20 100644 --- a/wgengine/magicsock/magicsock_test.go +++ b/wgengine/magicsock/magicsock_test.go @@ -4511,172 +4511,80 @@ func TestReceiveTSMPDiscoKeyAdvertisement(t *testing.T) { } } -func TestSendingTSMPDiscoTimer(t *testing.T) { - conn := newTestConn(t) - tw := eventbustest.NewWatcher(t, conn.eventBus) - t.Cleanup(func() { conn.Close() }) - - // maybeSendTSMPDiscoAdvert only advertises when netmap caching is enabled. - conn.controlKnobs = new(controlknobs.Knobs) - conn.controlKnobs.CacheNetworkMaps.Store(true) - - peerKey := key.NewNode().Public() - ep := &endpoint{ - nodeID: 1, - publicKey: peerKey, - nodeAddr: netip.MustParseAddr("100.64.0.1"), - } - discoKey := key.NewDisco().Public() - ep.disco.Store(&endpointDisco{ - key: discoKey, - short: discoKey.ShortString(), - }) - ep.c = conn - conn.mu.Lock() - nodeView := (&tailcfg.Node{ - Key: ep.publicKey, - Addresses: []netip.Prefix{ - netip.MustParsePrefix("100.64.0.1/32"), - }, - }).View() - conn.peersByID = map[tailcfg.NodeID]tailcfg.NodeView{nodeView.ID(): nodeView} - conn.mu.Unlock() - - conn.peerMap.upsertEndpoint(ep, key.DiscoPublic{}) - - if ep.discoShort() != discoKey.ShortString() { - t.Errorf("Original disco key %s, does not match %s", discoKey.ShortString(), ep.discoShort()) - } - - // Only one gets through, second is rate limited. - conn.maybeSendTSMPDiscoAdvert(ep) - conn.maybeSendTSMPDiscoAdvert(ep) - if err := eventbustest.ExpectExactly(tw, eventbustest.Type[NewDiscoKeyAvailable]()); err != nil { - t.Errorf("expected only one event, got: %s", err) - } - - // Reset to get the event firing again. - ep.mu.Lock() - ep.lastDiscoKeyAdvertisement = 0 - ep.mu.Unlock() - conn.maybeSendTSMPDiscoAdvert(ep) - if err := eventbustest.Expect(tw, eventbustest.Type[NewDiscoKeyAvailable]()); err != nil { - t.Errorf("expected only one event, got: %s", err) - } - - // With a direct bestAddr and a non-zero lastDiscoKeyAdvertisement past the - // rate-limit interval. No advert should be sent due to the active bestAddr. - ep.mu.Lock() - ep.lastDiscoKeyAdvertisement = mono.Now().Add(-discoKeyAdvertisementInterval - time.Second) - ep.bestAddr = addrQuality{epAddr: epAddr{ap: netip.MustParseAddrPort("1.2.3.4:567")}} - ep.mu.Unlock() - conn.maybeSendTSMPDiscoAdvert(ep) - - // Simulating restart should send an advert. - ep.mu.Lock() - ep.lastDiscoKeyAdvertisement = 0 - ep.mu.Unlock() - conn.maybeSendTSMPDiscoAdvert(ep) - if err := eventbustest.ExpectExactly(tw, eventbustest.Type[NewDiscoKeyAvailable]()); err != nil { - t.Errorf("expected only one event, got: %s", err) - } -} - -// TestSendingTSMPDiscoCachingDisabled verifies that maybeSendTSMPDiscoAdvert -// early-returns (sends no advert) when netmap caching is not enabled via the -// CacheNetworkMaps control knob, including when no knobs are present at all. -func TestSendingTSMPDiscoCachingDisabled(t *testing.T) { +func BenchmarkPriorityMessageForPeer(b *testing.B) { tests := []struct { - name string - knobs *controlknobs.Knobs + name string + networkSize int }{ - {name: "no-knobs", knobs: nil}, - // Knobs present but CacheNetworkMaps left at its false default. - {name: "caching-disabled", knobs: new(controlknobs.Knobs)}, + {"10 nodes", 10}, + {"10000 nodes", 10000}, } + for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - conn := newTestConn(t) - t.Cleanup(func() { conn.Close() }) - conn.controlKnobs = tt.knobs + b.Run(tt.name, func(b *testing.B) { + conn := newTestConn(b) + conn.SetPrivateKey(key.NewNode()) + peersByID := make(map[tailcfg.NodeID]tailcfg.NodeView, tt.networkSize) + var targetKey key.NodePublic - ep := &endpoint{ - nodeID: 1, - publicKey: key.NewNode().Public(), - nodeAddr: netip.MustParseAddr("100.64.0.1"), + selfNode := (&tailcfg.Node{ + ID: 0, + Addresses: []netip.Prefix{ + netip.MustParsePrefix("100.64.0.0/32"), + netip.MustParsePrefix("fd7a:115c:a1e0::/128"), + }, + }).View() + conn.mu.Lock() + conn.self = selfNode + conn.mu.Unlock() + + for i := range tt.networkSize { + nodeID := tailcfg.NodeID(i + 1) + nodeKey := key.NewNode().Public() + if i == 0 { + targetKey = nodeKey + } + + addrIdx := i + 1 + ip4 := netip.AddrFrom4([4]byte{100, 64, byte(addrIdx >> 8), byte(addrIdx)}) + ip6 := netip.AddrFrom16([16]byte{ + 0xfd, 0x7a, 0x11, 0x5c, 0xa1, 0xe0, + 0, 0, 0, 0, 0, 0, 0, 0, byte(addrIdx >> 8), byte(addrIdx), + }) + ep := &endpoint{ + nodeID: nodeID, + publicKey: nodeKey, + nodeAddr: ip4, + } + + discoKey := key.NewDisco().Public() + ep.disco.Store(&endpointDisco{ + key: discoKey, + short: discoKey.ShortString(), + }) + + ep.c = conn + nodeView := (&tailcfg.Node{ + ID: 1, + Key: ep.publicKey, + Addresses: []netip.Prefix{ + netip.PrefixFrom(ip4, 32), + netip.PrefixFrom(ip6, 128), + }, + }).View() + peersByID[nodeID] = nodeView + conn.mu.Lock() + conn.peerMap.upsertEndpoint(ep, key.DiscoPublic{}) + conn.mu.Unlock() } - ep.c = conn - // A fresh endpoint with a zero lastDiscoKeyAdvertisement and no - // direct bestAddr would otherwise advertise; the only thing - // suppressing it here is the disabled caching knob. On early - // return the timestamp is left untouched (zero). - conn.maybeSendTSMPDiscoAdvert(ep) + conn.mu.Lock() + conn.peersByID = peersByID + conn.mu.Unlock() - ep.mu.Lock() - defer ep.mu.Unlock() - if !ep.lastDiscoKeyAdvertisement.IsZero() { - t.Errorf("lastDiscoKeyAdvertisement = %v; want zero (advert should have been suppressed)", ep.lastDiscoKeyAdvertisement) + for b.Loop() { + conn.PriorityMessageForPeer(targetKey) } }) } } - -// TestSendingTSMPDiscoPeerRelaySuppressed verifies that maybeSendTSMPDiscoAdvert -// suppresses the advert when the bestAddr is a peer relay path (a non-zero -// addrQuality whose epAddr has a VNI set), even though such a path is not -// direct. Suppression is observed via lastDiscoKeyAdvertisement remaining -// unchanged, since a fired advert would overwrite it with the current time. -func TestSendingTSMPDiscoPeerRelaySuppressed(t *testing.T) { - conn := newTestConn(t) - t.Cleanup(func() { conn.Close() }) - - // maybeSendTSMPDiscoAdvert only advertises when netmap caching is enabled. - conn.controlKnobs = new(controlknobs.Knobs) - conn.controlKnobs.CacheNetworkMaps.Store(true) - - peerKey := key.NewNode().Public() - ep := &endpoint{ - nodeID: 1, - publicKey: peerKey, - nodeAddr: netip.MustParseAddr("100.64.0.1"), - } - discoKey := key.NewDisco().Public() - ep.disco.Store(&endpointDisco{ - key: discoKey, - short: discoKey.ShortString(), - }) - ep.c = conn - conn.mu.Lock() - nodeView := (&tailcfg.Node{ - Key: ep.publicKey, - Addresses: []netip.Prefix{ - netip.MustParsePrefix("100.64.0.1/32"), - }, - }).View() - conn.peersByID = map[tailcfg.NodeID]tailcfg.NodeView{nodeView.ID(): nodeView} - conn.mu.Unlock() - - conn.peerMap.upsertEndpoint(ep, key.DiscoPublic{}) - - // A peer relay bestAddr: an epAddr with a VNI set. It is past the - // rate-limit interval with a non-zero lastDiscoKeyAdvertisement, so the - // only thing suppressing the advert is the active (non-zero) bestAddr. - var vni packet.VirtualNetworkID - vni.Set(7) - lastAdvert := mono.Now().Add(-discoKeyAdvertisementInterval - time.Second) - ep.mu.Lock() - ep.lastDiscoKeyAdvertisement = lastAdvert - ep.bestAddr = addrQuality{epAddr: epAddr{ap: netip.MustParseAddrPort("1.2.3.4:567"), vni: vni}} - ep.mu.Unlock() - - conn.maybeSendTSMPDiscoAdvert(ep) - - // A fired advert would have overwritten lastDiscoKeyAdvertisement with the - // current time; confirm it was left untouched, indicating suppression. - ep.mu.Lock() - defer ep.mu.Unlock() - if ep.lastDiscoKeyAdvertisement != lastAdvert { - t.Errorf("lastDiscoKeyAdvertisement = %v; want unchanged %v (advert should have been suppressed)", ep.lastDiscoKeyAdvertisement, lastAdvert) - } -} diff --git a/wgengine/userspace.go b/wgengine/userspace.go index 9b21ef612..599f64c04 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -52,7 +52,6 @@ "tailscale.com/util/execqueue" "tailscale.com/util/mak" "tailscale.com/util/set" - "tailscale.com/util/singleflight" "tailscale.com/util/testenv" "tailscale.com/util/usermetric" "tailscale.com/version" @@ -620,17 +619,6 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error) } e.magicConn.HandleDiscoKeyAdvertisement(peer.Node, pkt) }) - var tsmpRequestGroup singleflight.Group[netip.Addr, struct{}] - eventbus.SubscribeFunc(ec, func(req magicsock.NewDiscoKeyAvailable) { - if !req.NodeFirstAddr.IsValid() { - return - } - go tsmpRequestGroup.Do(req.NodeFirstAddr, func() (struct{}, error) { - e.sendTSMPDiscoAdvertisement(req.NodeFirstAddr) - e.logf("wgengine: sending TSMP disco key advertisement to %v", req.NodeFirstAddr) - return struct{}{}, nil - }) - }) e.eventClient = ec e.logf("Engine created.") return e, nil @@ -758,6 +746,15 @@ func (e *userspaceEngine) SetPeerSessionStateFunc(fn func(key.NodePublic, PeerWi }) } +func (e *userspaceEngine) SetPeerPriorityMessageOnEstablishmentFunc(fn func(key.NodePublic) (msg []byte)) { + e.wgdev.SetPriorityMessageOnEstablishmentFunc(func(pk device.NoisePublicKey) (msg []byte) { + if fn != nil { + return fn(key.NodePublicFromRaw32(mem.B(pk[:]))) + } + return nil + }) +} + // SetNetLogNodeSource installs the [netlog.NodeSource] used by the engine's // network logger. func (e *userspaceEngine) SetNetLogNodeSource(src netlog.NodeSource) { @@ -1403,7 +1400,6 @@ func (e *userspaceEngine) Ping(ip netip.Addr, pingType tailcfg.PingType, size in e.magicConn.Ping(peer, res, size, cb) case "TSMP": e.sendTSMPPing(ip, peer, res, cb) - e.sendTSMPDiscoAdvertisement(ip) case "ICMP": e.sendICMPEchoRequest(ip, peer, res, cb) } @@ -1524,29 +1520,6 @@ func (e *userspaceEngine) sendTSMPPing(ip netip.Addr, peer tailcfg.NodeView, res e.tundev.InjectOutbound(tsmpPing) } -func (e *userspaceEngine) sendTSMPDiscoAdvertisement(ip netip.Addr) { - srcIP, err := e.mySelfIPMatchingFamily(ip) - if err != nil { - e.logf("getting matching node: %s", err) - return - } - tdka := packet.TSMPDiscoKeyAdvertisement{ - Src: srcIP, - Dst: ip, - Key: e.magicConn.DiscoPublicKey(), - } - payload, err := tdka.Marshal() - if err != nil { - e.logf("error generating TSMP Advertisement: %s", err) - metricTSMPDiscoKeyAdvertisementError.Add(1) - } else if err := e.tundev.InjectOutbound(payload); err != nil { - e.logf("error sending TSMP Advertisement: %s", err) - metricTSMPDiscoKeyAdvertisementError.Add(1) - } else { - metricTSMPDiscoKeyAdvertisementSent.Add(1) - } -} - func (e *userspaceEngine) setTSMPPongCallback(data [8]byte, cb func(packet.TSMPPongReply)) { e.mu.Lock() defer e.mu.Unlock() @@ -1698,9 +1671,6 @@ func (ls fwdDNSLinkSelector) PickLink(ip netip.Addr) (linkName string) { metricNumMajorChanges = clientmetric.NewCounter("wgengine_major_changes") metricNumMinorChanges = clientmetric.NewCounter("wgengine_minor_changes") - metricTSMPDiscoKeyAdvertisementSent = clientmetric.NewCounter("magicsock_tsmp_disco_key_advertisement_sent") - metricTSMPDiscoKeyAdvertisementError = clientmetric.NewCounter("magicsock_tsmp_disco_key_advertisement_error") - metricTSMPLearnedKeyMismatch = clientmetric.NewCounter("magicsock_tsmp_learned_key_mismatch") ) diff --git a/wgengine/userspace_test.go b/wgengine/userspace_test.go index 95fbf5a75..61a5cd233 100644 --- a/wgengine/userspace_test.go +++ b/wgengine/userspace_test.go @@ -448,64 +448,6 @@ func TestUserspaceEnginePeerMTUReconfig(t *testing.T) { } } -func TestTSMPKeyAdvertisement(t *testing.T) { - var knobs controlknobs.Knobs - - bus := eventbustest.NewBus(t) - ht := health.NewTracker(bus) - reg := new(usermetric.Registry) - e, err := NewFakeUserspaceEngine(t.Logf, 0, &knobs, ht, reg, bus) - if err != nil { - t.Fatal(err) - } - t.Cleanup(e.Close) - ue := e.(*userspaceEngine) - routerCfg := &router.Config{} - nodeKey := nkFromHex("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb") - nm := &netmap.NetworkMap{ - Peers: nodeViews([]*tailcfg.Node{ - { - ID: 1, - Key: nodeKey, - }, - }), - SelfNode: (&tailcfg.Node{ - StableID: "TESTCTRL00000001", - Name: "test-node.test.ts.net", - Addresses: []netip.Prefix{netip.MustParsePrefix("100.64.0.1/32"), netip.MustParsePrefix("fd7a:115c:a1e0:ab12:4843:cd96:0:1/128")}, - }).View(), - } - cfg := &wgcfg.Config{ - Peers: []wgcfg.Peer{ - { - PublicKey: nodeKey, - AllowedIPs: []netip.Prefix{ - netip.PrefixFrom(netaddr.IPv4(100, 100, 99, 1), 32), - }, - }, - }, - } - - ue.SetSelfNode(nm.SelfNode) - err = ue.Reconfig(cfg, routerCfg, &dns.Config{}) - if err != nil { - t.Fatal(err) - } - - addr := netip.MustParseAddr("100.100.99.1") - previousValue := metricTSMPDiscoKeyAdvertisementSent.Value() - ue.sendTSMPDiscoAdvertisement(addr) - if val := metricTSMPDiscoKeyAdvertisementSent.Value(); val <= previousValue { - errs := metricTSMPDiscoKeyAdvertisementError.Value() - t.Errorf("Expected 1 disco key advert, got %d, errors %d", val, errs) - } - // Remove config to have the engine shut down more consistently - err = ue.Reconfig(&wgcfg.Config{}, &router.Config{}, &dns.Config{}) - if err != nil { - t.Fatal(err) - } -} - func nkFromHex(hex string) key.NodePublic { if len(hex) != 64 { panic(fmt.Sprintf("%q is len %d; want 64", hex, len(hex))) diff --git a/wgengine/wgengine.go b/wgengine/wgengine.go index 6b996fe81..1bbe9032a 100644 --- a/wgengine/wgengine.go +++ b/wgengine/wgengine.go @@ -231,6 +231,8 @@ type Engine interface { // LocalBackend construction. SetPeerSessionStateFunc(func(key.NodePublic, PeerWireGuardState)) + SetPeerPriorityMessageOnEstablishmentFunc(fn func(key.NodePublic) (msg []byte)) + // ProbeLocks acquires and releases the engine's internal locks so // that [ipnlocal.LocalBackend]'s watchdog can detect deadlocks in // the engine. It is otherwise a no-op.