From f343b496c3efef16b1d0dd4183a3d7624daba944 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 15 Apr 2026 00:49:12 +0000 Subject: [PATCH] wgengine, all: remove LazyWG, use wireguard-go callback API for on-demand peers Replace the UAPI text protocol-based wireguard configuration with wireguard-go's new direct callback API (SetPeerLookupFunc, SetPeerByIPPacketFunc, RemoveMatchingPeers, SetPrivateKey). Instead of computing a trimmed wireguard config ahead of time upon control plane updates and pushing it via UAPI, install callbacks so wireguard-go creates peers on demand when packets arrive. This removes all the LazyWG trimming machinery: idle peer tracking, activity maps, noteRecvActivity callbacks, the KeepFullWGConfig control knob, and the ts_omit_lazywg build tag. For incoming packets, PeerLookupFunc answers wireguard-go's questions about unknown public keys by looking up the peer in the full config. For outgoing packets, PeerByIPPacketFunc (installed from LocalBackend.lookupPeerByIP) maps destination IPs to node public keys using the existing nodeByAddr index. Updates tailscale/corp#12345 Change-Id: I4cba80979ac49a1231d00a01fdba5f0c2af95dd8 Signed-off-by: Brad Fitzpatrick --- control/controlknobs/controlknobs.go | 7 - .../buildfeatures/feature_lazywg_disabled.go | 13 - .../buildfeatures/feature_lazywg_enabled.go | 13 - feature/featuretags/featuretags.go | 1 - flake.nix | 2 +- flakehashes.json | 4 +- go.mod | 2 +- go.sum | 4 +- ipn/ipnlocal/local.go | 23 + ipn/ipnlocal/state_test.go | 2 + net/tstun/wrap.go | 28 +- shell.nix | 2 +- types/key/node.go | 5 + wgengine/magicsock/endpoint.go | 5 - wgengine/magicsock/magicsock.go | 35 +- wgengine/magicsock/magicsock_test.go | 221 ++++---- wgengine/userspace.go | 483 ++++-------------- wgengine/userspace_test.go | 196 ------- wgengine/watchdog.go | 4 + wgengine/wgcfg/config.go | 8 +- wgengine/wgcfg/config_test.go | 2 +- wgengine/wgcfg/device.go | 91 ++-- wgengine/wgcfg/device_test.go | 200 ++------ wgengine/wgcfg/parser.go | 186 ------- wgengine/wgcfg/parser_test.go | 95 ---- wgengine/wgcfg/wgcfg_clone.go | 1 - wgengine/wgcfg/writer.go | 154 ------ wgengine/wgengine.go | 4 + 28 files changed, 354 insertions(+), 1437 deletions(-) delete mode 100644 feature/buildfeatures/feature_lazywg_disabled.go delete mode 100644 feature/buildfeatures/feature_lazywg_enabled.go delete mode 100644 wgengine/wgcfg/parser.go delete mode 100644 wgengine/wgcfg/parser_test.go delete mode 100644 wgengine/wgcfg/writer.go diff --git a/control/controlknobs/controlknobs.go b/control/controlknobs/controlknobs.go index 0e8051210..77a496349 100644 --- a/control/controlknobs/controlknobs.go +++ b/control/controlknobs/controlknobs.go @@ -21,11 +21,6 @@ type Knobs struct { // DisableUPnP indicates whether to attempt UPnP mapping. DisableUPnP atomic.Bool - // KeepFullWGConfig is whether we should disable the lazy wireguard - // programming and instead give WireGuard the full netmap always, even for - // idle peers. - KeepFullWGConfig atomic.Bool - // RandomizeClientPort is whether control says we should randomize // the client port. RandomizeClientPort atomic.Bool @@ -125,7 +120,6 @@ func (k *Knobs) UpdateFromNodeAttributes(capMap tailcfg.NodeCapMap) { } has := capMap.Contains var ( - keepFullWG = has(tailcfg.NodeAttrDebugDisableWGTrim) disableUPnP = has(tailcfg.NodeAttrDisableUPnP) randomizeClientPort = has(tailcfg.NodeAttrRandomizeClientPort) disableDeltaUpdates = has(tailcfg.NodeAttrDisableDeltaUpdates) @@ -153,7 +147,6 @@ func (k *Knobs) UpdateFromNodeAttributes(capMap tailcfg.NodeCapMap) { oneCGNAT.Set(false) } - k.KeepFullWGConfig.Store(keepFullWG) k.DisableUPnP.Store(disableUPnP) k.RandomizeClientPort.Store(randomizeClientPort) k.OneCGNAT.Store(oneCGNAT) diff --git a/feature/buildfeatures/feature_lazywg_disabled.go b/feature/buildfeatures/feature_lazywg_disabled.go deleted file mode 100644 index af1ad388c..000000000 --- a/feature/buildfeatures/feature_lazywg_disabled.go +++ /dev/null @@ -1,13 +0,0 @@ -// Copyright (c) Tailscale Inc & contributors -// SPDX-License-Identifier: BSD-3-Clause - -// Code generated by gen.go; DO NOT EDIT. - -//go:build ts_omit_lazywg - -package buildfeatures - -// HasLazyWG is whether the binary was built with support for modular feature "Lazy WireGuard configuration for memory-constrained devices with large netmaps". -// Specifically, it's whether the binary was NOT built with the "ts_omit_lazywg" build tag. -// It's a const so it can be used for dead code elimination. -const HasLazyWG = false diff --git a/feature/buildfeatures/feature_lazywg_enabled.go b/feature/buildfeatures/feature_lazywg_enabled.go deleted file mode 100644 index f2d6a10f8..000000000 --- a/feature/buildfeatures/feature_lazywg_enabled.go +++ /dev/null @@ -1,13 +0,0 @@ -// Copyright (c) Tailscale Inc & contributors -// SPDX-License-Identifier: BSD-3-Clause - -// Code generated by gen.go; DO NOT EDIT. - -//go:build !ts_omit_lazywg - -package buildfeatures - -// HasLazyWG is whether the binary was built with support for modular feature "Lazy WireGuard configuration for memory-constrained devices with large netmaps". -// Specifically, it's whether the binary was NOT built with the "ts_omit_lazywg" build tag. -// It's a const so it can be used for dead code elimination. -const HasLazyWG = true diff --git a/feature/featuretags/featuretags.go b/feature/featuretags/featuretags.go index 4f34acbe8..e44a4f592 100644 --- a/feature/featuretags/featuretags.go +++ b/feature/featuretags/featuretags.go @@ -171,7 +171,6 @@ var Features = map[FeatureTag]FeatureMeta{ "ipnbus": {Sym: "IPNBus", Desc: "IPN notification bus (watch-ipn-bus) support, used by GUIs, debugging, and nicer 'tailscale up' support"}, "iptables": {Sym: "IPTables", Desc: "Linux iptables support"}, "kube": {Sym: "Kube", Desc: "Kubernetes integration"}, - "lazywg": {Sym: "LazyWG", Desc: "Lazy WireGuard configuration for memory-constrained devices with large netmaps"}, "linuxdnsfight": {Sym: "LinuxDNSFight", Desc: "Linux support for detecting DNS fights (inotify watching of /etc/resolv.conf)"}, "linkspeed": { Sym: "LinkSpeed", diff --git a/flake.nix b/flake.nix index a887b2fbe..2a5880c10 100644 --- a/flake.nix +++ b/flake.nix @@ -164,4 +164,4 @@ }); }; } -# nix-direnv cache busting line: sha256-1jbM+hcFOtKCTEIGSqdBMTiDoBkmOCuVK5Tjzi0lJAA= +# nix-direnv cache busting line: sha256-5zxCDQ12bu8dvJ51RCQk/m07oM2qNNrTB5cbb1Za/sc= diff --git a/flakehashes.json b/flakehashes.json index e93aee793..a90e77cf1 100644 --- a/flakehashes.json +++ b/flakehashes.json @@ -4,7 +4,7 @@ "sri": "sha256-pCvFNTFuvhSBb5O+PPuilaowP4tXcCOP1NgYUDJTcJU=" }, "vendor": { - "goModSum": "sha256-mqDXN3lDP0C7G0Kyp4/CTVQ1fagOz40fNKLgPVfgSBw=", - "sri": "sha256-1jbM+hcFOtKCTEIGSqdBMTiDoBkmOCuVK5Tjzi0lJAA=" + "goModSum": "sha256-xjPeSzdlDw247JtuZ9gI/OXh0IYvQV3qN1WNRbSlir8=", + "sri": "sha256-5zxCDQ12bu8dvJ51RCQk/m07oM2qNNrTB5cbb1Za/sc=" } } diff --git a/go.mod b/go.mod index cddae58e8..8adb1f076 100644 --- a/go.mod +++ b/go.mod @@ -104,7 +104,7 @@ require ( github.com/tailscale/ts-gokrazy v0.0.0-20260429180033-fe741c6deb44 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-20260304043104-4184faf59e56 + github.com/tailscale/wireguard-go v0.0.0-20260427181203-e3ac4a0afb4e 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 8fed57905..2caac328d 100644 --- a/go.sum +++ b/go.sum @@ -1157,8 +1157,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-20260304043104-4184faf59e56 h1:/R1vu+eNhg1eKstmVPEKvsJgkh4TUyb+J+Eadwv+d/I= -github.com/tailscale/wireguard-go v0.0.0-20260304043104-4184faf59e56/go.mod h1:zvaAPQrjUBWufXgqpSQ1/BYu9ZFOKnsNWLFQe+E78cM= +github.com/tailscale/wireguard-go v0.0.0-20260427181203-e3ac4a0afb4e h1:GexFR7ak1iz26fxg8HWCpOEqAOL8UEZJ7J3JxeCalDs= +github.com/tailscale/wireguard-go v0.0.0-20260427181203-e3ac4a0afb4e/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 87564954a..891558a48 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -540,6 +540,8 @@ func NewLocalBackend(logf logger.Logf, logID logid.PublicID, sys *tsd.System, lo b.currentNodeAtomic.Store(nb) nb.ready() + e.SetPeerByIPPacketFunc(b.lookupPeerByIP) + if sys.InitialConfig != nil { if err := b.initPrefsFromConfig(sys.InitialConfig); err != nil { return nil, err @@ -5121,6 +5123,27 @@ func (b *LocalBackend) NetMap() *netmap.NetworkMap { return b.currentNode().NetMap() } +// lookupPeerByIP returns the node public key for the peer that owns the +// given IP address. It is the fast path for [Engine.SetPeerByIPPacketFunc], +// handling exact-IP matches against node addresses; subnet routes and exit +// nodes are handled by a BART-based fallback in userspaceEngine that uses +// the wireguard-filtered peer list (see lastCfgFull). +// +// It is called by wireguard-go on every outbound packet (not cached), so +// it must be fast. +func (b *LocalBackend) lookupPeerByIP(ip netip.Addr) (key.NodePublic, bool) { + nb := b.currentNode() + nid, ok := nb.NodeByAddr(ip) + if !ok { + return key.NodePublic{}, false + } + peer, ok := nb.NodeByID(nid) + if !ok { + return key.NodePublic{}, false + } + return peer.Key(), true +} + func (b *LocalBackend) isEngineBlocked() bool { b.mu.Lock() defer b.mu.Unlock() diff --git a/ipn/ipnlocal/state_test.go b/ipn/ipnlocal/state_test.go index 7646ed764..104c29a3f 100644 --- a/ipn/ipnlocal/state_test.go +++ b/ipn/ipnlocal/state_test.go @@ -1938,6 +1938,8 @@ func (e *mockEngine) Ping(ip netip.Addr, pingType tailcfg.PingType, size int, cb func (e *mockEngine) InstallCaptureHook(packet.CaptureCallback) {} +func (e *mockEngine) SetPeerByIPPacketFunc(func(netip.Addr) (_ key.NodePublic, ok bool)) {} + func (e *mockEngine) Close() { e.mu.Lock() defer e.mu.Unlock() diff --git a/net/tstun/wrap.go b/net/tstun/wrap.go index 1b28eb157..cd75aff5c 100644 --- a/net/tstun/wrap.go +++ b/net/tstun/wrap.go @@ -111,8 +111,7 @@ type Wrapper struct { // you might need to add an align64 field here. lastActivityAtomic mono.Time // time of last send or receive - destIPActivity syncs.AtomicValue[map[netip.Addr]func()] - discoKey syncs.AtomicValue[key.DiscoPublic] + discoKey syncs.AtomicValue[key.DiscoPublic] // timeNow, if non-nil, will be used to obtain the current time. timeNow func() time.Time @@ -340,16 +339,6 @@ func (t *Wrapper) now() time.Time { return time.Now() } -// SetDestIPActivityFuncs sets a map of funcs to run per packet -// destination (the map keys). -// -// The map ownership passes to the Wrapper. It must be non-nil. -func (t *Wrapper) SetDestIPActivityFuncs(m map[netip.Addr]func()) { - if buildfeatures.HasLazyWG { - t.destIPActivity.Store(m) - } -} - // SetDiscoKey sets the current discovery key. // // It is only used for filtering out bogus traffic when network @@ -997,13 +986,6 @@ func (t *Wrapper) Read(buffs [][]byte, sizes []int, offset int) (int, error) { for _, data := range res.data { p.Decode(data[res.dataOffset:]) - if buildfeatures.HasLazyWG { - if m := t.destIPActivity.Load(); m != nil { - if fn := m[p.Dst.Addr()]; fn != nil { - fn() - } - } - } if buildfeatures.HasCapture && captHook != nil { captHook(packet.FromLocal, t.now(), p.Buffer(), p.CaptureMeta) } @@ -1136,14 +1118,6 @@ func (t *Wrapper) injectedRead(res tunInjectedRead, outBuffs [][]byte, sizes []i pc.snat(p) invertGSOChecksum(pkt, gso) - if buildfeatures.HasLazyWG { - if m := t.destIPActivity.Load(); m != nil { - if fn := m[p.Dst.Addr()]; fn != nil { - fn() - } - } - } - if res.packet != nil { var gsoOptions tun.GSOOptions gsoOptions, err = stackGSOToTunGSO(pkt, gso) diff --git a/shell.nix b/shell.nix index 098090285..6c36f7706 100644 --- a/shell.nix +++ b/shell.nix @@ -16,4 +16,4 @@ ) { src = ./.; }).shellNix -# nix-direnv cache busting line: sha256-1jbM+hcFOtKCTEIGSqdBMTiDoBkmOCuVK5Tjzi0lJAA= +# nix-direnv cache busting line: sha256-5zxCDQ12bu8dvJ51RCQk/m07oM2qNNrTB5cbb1Za/sc= diff --git a/types/key/node.go b/types/key/node.go index 98f72c719..a1d8e47ba 100644 --- a/types/key/node.go +++ b/types/key/node.go @@ -65,6 +65,11 @@ func NewNode() NodePrivate { // Raw32 returns k as 32 raw bytes. func (k NodePrivate) Raw32() [32]byte { return k.k } +// NodePrivateAs returns a NodePrivate as a named fixed-size array of bytes. +// It's intended for interoperability with wireguard-go's +// device.NoisePrivateKey type. +func NodePrivateAs[T ~[32]byte](k NodePrivate) T { return k.k } + // NodePrivateFromRaw32 parses a 32-byte raw value as a NodePrivate. // // Deprecated: only needed to cast from legacy node private key types, diff --git a/wgengine/magicsock/endpoint.go b/wgengine/magicsock/endpoint.go index 510d0d315..71edfe9a1 100644 --- a/wgengine/magicsock/endpoint.go +++ b/wgengine/magicsock/endpoint.go @@ -530,11 +530,6 @@ func (de *endpoint) noteRecvActivity(src epAddr, now mono.Time) bool { elapsed := now.Sub(de.lastRecvWG.LoadAtomic()) if elapsed > 10*time.Second { de.lastRecvWG.StoreAtomic(now) - - if de.c.noteRecvActivity == nil { - return false - } - de.c.noteRecvActivity(de.publicKey) return true } return false diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index 2596e1d12..9720f57cd 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -164,7 +164,6 @@ type Conn struct { derpActiveFunc func() idleFunc func() time.Duration // nil means unknown testOnlyPacketListener nettype.PacketListener - noteRecvActivity func(key.NodePublic) // or nil, see Options.NoteRecvActivity onDERPRecv func(int, key.NodePublic, []byte) bool // or nil, see Options.OnDERPRecv netMon *netmon.Monitor // must be non-nil health *health.Tracker // or nil @@ -457,19 +456,6 @@ type Options struct { // Only used by tests. TestOnlyPacketListener nettype.PacketListener - // NoteRecvActivity, if provided, is a func for magicsock to call - // whenever it receives a packet from a a peer if it's been more - // than ~10 seconds since the last one. (10 seconds is somewhat - // arbitrary; the sole user, lazy WireGuard configuration, - // just doesn't need or want it called on - // every packet, just every minute or two for WireGuard timeouts, - // and 10 seconds seems like a good trade-off between often enough - // and not too often.) - // The provided func is likely to call back into - // Conn.ParseEndpoint, which acquires Conn.mu. As such, you should - // not hold Conn.mu while calling it. - NoteRecvActivity func(key.NodePublic) - // NetMon is the network monitor to use. // It must be non-nil. NetMon *netmon.Monitor @@ -648,7 +634,6 @@ func NewConn(opts Options) (*Conn, error) { c.derpActiveFunc = opts.derpActiveFunc() c.idleFunc = opts.IdleFunc c.testOnlyPacketListener = opts.TestOnlyPacketListener - c.noteRecvActivity = opts.NoteRecvActivity c.onDERPRecv = opts.OnDERPRecv // Set up publishers and subscribers. Subscribe calls must return before @@ -4270,16 +4255,10 @@ var _ conn.Endpoint = (*lazyEndpoint)(nil) // InitiationMessagePublicKey implements [conn.InitiationAwareEndpoint]. // wireguard-go calls us here if we passed it a [*lazyEndpoint] for an -// initiation message, for which it might not have the relevant peer configured, -// enabling us to just-in-time configure it and note its activity via -// [*endpoint.noteRecvActivity], before it performs peer lookup and attempts -// decryption. +// initiation message, for which it might not have the relevant peer configured. +// Wireguard-go's PeerLookupFunc handles on-demand peer creation. // -// Reception of all other WireGuard message types implies pre-existing knowledge -// of the peer by wireguard-go for it to do useful work. See -// [userspaceEngine.maybeReconfigWireguardLocked] & -// [userspaceEngine.noteRecvActivity] for more details around just-in-time -// wireguard-go peer (de)configuration. +// We still update endpoint activity tracking for bestAddr management. func (le *lazyEndpoint) InitiationMessagePublicKey(peerPublicKey [32]byte) { pubKey := key.NodePublicFromRaw32(mem.B(peerPublicKey[:])) if le.maybeEP != nil && pubKey.Compare(le.maybeEP.publicKey) == 0 { @@ -4287,9 +4266,6 @@ func (le *lazyEndpoint) InitiationMessagePublicKey(peerPublicKey [32]byte) { } le.c.mu.Lock() ep, ok := le.c.peerMap.endpointForNodeKey(pubKey) - // [Conn.mu] must not be held while [Conn.noteRecvActivity] is called, which - // [endpoint.noteRecvActivity] can end up calling. See - // [Options.NoteRecvActivity] docs. le.c.mu.Unlock() if !ok { return @@ -4297,11 +4273,6 @@ func (le *lazyEndpoint) InitiationMessagePublicKey(peerPublicKey [32]byte) { now := mono.Now() ep.lastRecvUDPAny.StoreAtomic(now) ep.noteRecvActivity(le.src, now) - // [ep.noteRecvActivity] may end up JIT configuring the peer, but we don't - // update [peerMap] as wireguard-go hasn't decrypted the initiation - // message yet. wireguard-go will call us below in [lazyEndpoint.FromPeer] - // if it successfully decrypts the message, at which point it's safe to - // insert le.src into the [peerMap] for ep. } func (le *lazyEndpoint) ClearSrc() {} diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go index df9e57351..3552ecc19 100644 --- a/wgengine/magicsock/magicsock_test.go +++ b/wgengine/magicsock/magicsock_test.go @@ -242,6 +242,25 @@ func newMagicStackWithKey(t testing.TB, logf logger.Logf, ln nettype.PacketListe func (s *magicStack) Reconfig(cfg *wgcfg.Config) error { s.tsTun.SetWGConfig(cfg) s.wgLogger.SetPeers(cfg.Peers) + + // In production, LocalBackend installs a PeerByIPPacketFunc via + // Engine.SetPeerByIPPacketFunc. Tests that bypass LocalBackend need + // to install one here for outbound packet routing. + ipToPeer := make(map[netip.Addr]device.NoisePublicKey, len(cfg.Peers)) + for _, p := range cfg.Peers { + pk := p.PublicKey.Raw32() + for _, pfx := range p.AllowedIPs { + if pfx.IsSingleIP() { + ipToPeer[pfx.Addr()] = pk + } + } + } + s.dev.SetPeerByIPPacketFunc(func(_, dst netip.Addr, _ []byte) (device.NoisePublicKey, bool) { + pk, ok := ipToPeer[dst] + return pk, ok + }) + + s.dev.SetPrivateKey(key.NodePrivateAs[device.NoisePrivateKey](cfg.PrivateKey)) return wgcfg.ReconfigDevice(s.dev, cfg, s.conn.logf) } @@ -1442,13 +1461,8 @@ func TestDiscoStringLogRace(t *testing.T) { } func Test32bitAlignment(t *testing.T) { - // Need an associated conn with non-nil noteRecvActivity to - // trigger interesting work on the atomics in endpoint. - called := 0 de := endpoint{ - c: &Conn{ - noteRecvActivity: func(key.NodePublic) { called++ }, - }, + c: &Conn{}, } if off := unsafe.Offsetof(de.lastRecvWG); off%8 != 0 { @@ -1456,13 +1470,7 @@ func Test32bitAlignment(t *testing.T) { } de.noteRecvActivity(epAddr{}, mono.Now()) // verify this doesn't panic on 32-bit - if called != 1 { - t.Fatal("expected call to noteRecvActivity") - } - de.noteRecvActivity(epAddr{}, mono.Now()) - if called != 1 { - t.Error("expected no second call to noteRecvActivity") - } + de.noteRecvActivity(epAddr{}, mono.Now()) // second call should be throttled } // newTestConn returns a new Conn. @@ -3957,60 +3965,55 @@ func TestConn_receiveIP(t *testing.T) { // If [*endpoint] then we expect 'got' to be the same [*endpoint]. If // [*lazyEndpoint] and [*lazyEndpoint.maybeEP] is non-nil, we expect // got.maybeEP to also be non-nil. Must not be reused across tests. - wantEndpointType wgconn.Endpoint - wantSize int - wantIsGeneveEncap bool - wantOk bool - wantMetricInc *clientmetric.Metric - wantNoteRecvActivityCalled bool + wantEndpointType wgconn.Endpoint + wantSize int + wantIsGeneveEncap bool + wantOk bool + wantMetricInc *clientmetric.Metric }{ { - name: "naked-disco", - b: looksLikeNakedDisco, - ipp: netip.MustParseAddrPort("127.0.0.1:7777"), - cache: &epAddrEndpointCache{}, - wantEndpointType: nil, - wantSize: 0, - wantIsGeneveEncap: false, - wantOk: false, - wantMetricInc: metricRecvDiscoBadPeer, - wantNoteRecvActivityCalled: false, + name: "naked-disco", + b: looksLikeNakedDisco, + ipp: netip.MustParseAddrPort("127.0.0.1:7777"), + cache: &epAddrEndpointCache{}, + wantEndpointType: nil, + wantSize: 0, + wantIsGeneveEncap: false, + wantOk: false, + wantMetricInc: metricRecvDiscoBadPeer, }, { - name: "geneve-encap-disco", - b: looksLikeGeneveDisco, - ipp: netip.MustParseAddrPort("127.0.0.1:7777"), - cache: &epAddrEndpointCache{}, - wantEndpointType: nil, - wantSize: 0, - wantIsGeneveEncap: false, - wantOk: false, - wantMetricInc: metricRecvDiscoBadPeer, - wantNoteRecvActivityCalled: false, + name: "geneve-encap-disco", + b: looksLikeGeneveDisco, + ipp: netip.MustParseAddrPort("127.0.0.1:7777"), + cache: &epAddrEndpointCache{}, + wantEndpointType: nil, + wantSize: 0, + wantIsGeneveEncap: false, + wantOk: false, + wantMetricInc: metricRecvDiscoBadPeer, }, { - name: "STUN-binding", - b: looksLikeSTUNBinding, - ipp: netip.MustParseAddrPort("127.0.0.1:7777"), - cache: &epAddrEndpointCache{}, - wantEndpointType: nil, - wantSize: 0, - wantIsGeneveEncap: false, - wantOk: false, - wantMetricInc: findMetricByName("netcheck_stun_recv_ipv4"), - wantNoteRecvActivityCalled: false, + name: "STUN-binding", + b: looksLikeSTUNBinding, + ipp: netip.MustParseAddrPort("127.0.0.1:7777"), + cache: &epAddrEndpointCache{}, + wantEndpointType: nil, + wantSize: 0, + wantIsGeneveEncap: false, + wantOk: false, + wantMetricInc: findMetricByName("netcheck_stun_recv_ipv4"), }, { - name: "naked-WireGuard-init-lazyEndpoint-empty-peerMap", - b: looksLikeNakedWireGuardInit, - ipp: netip.MustParseAddrPort("127.0.0.1:7777"), - cache: &epAddrEndpointCache{}, - wantEndpointType: &lazyEndpoint{}, - wantSize: len(looksLikeNakedWireGuardInit), - wantIsGeneveEncap: false, - wantOk: true, - wantMetricInc: nil, - wantNoteRecvActivityCalled: false, + name: "naked-WireGuard-init-lazyEndpoint-empty-peerMap", + b: looksLikeNakedWireGuardInit, + ipp: netip.MustParseAddrPort("127.0.0.1:7777"), + cache: &epAddrEndpointCache{}, + wantEndpointType: &lazyEndpoint{}, + wantSize: len(looksLikeNakedWireGuardInit), + wantIsGeneveEncap: false, + wantOk: true, + wantMetricInc: nil, }, { name: "naked-WireGuard-init-endpoint-matching-peerMap-entry", @@ -4024,19 +4027,17 @@ func TestConn_receiveIP(t *testing.T) { wantIsGeneveEncap: false, wantOk: true, wantMetricInc: nil, - wantNoteRecvActivityCalled: true, }, { - name: "geneve-WireGuard-init-lazyEndpoint-empty-peerMap", - b: looksLikeGeneveWireGuardInit, - ipp: netip.MustParseAddrPort("127.0.0.1:7777"), - cache: &epAddrEndpointCache{}, - wantEndpointType: &lazyEndpoint{}, - wantSize: len(looksLikeGeneveWireGuardInit) - packet.GeneveFixedHeaderLength, - wantIsGeneveEncap: true, - wantOk: true, - wantMetricInc: nil, - wantNoteRecvActivityCalled: false, + name: "geneve-WireGuard-init-lazyEndpoint-empty-peerMap", + b: looksLikeGeneveWireGuardInit, + ipp: netip.MustParseAddrPort("127.0.0.1:7777"), + cache: &epAddrEndpointCache{}, + wantEndpointType: &lazyEndpoint{}, + wantSize: len(looksLikeGeneveWireGuardInit) - packet.GeneveFixedHeaderLength, + wantIsGeneveEncap: true, + wantOk: true, + wantMetricInc: nil, }, { name: "geneve-WireGuard-init-lazyEndpoint-matching-peerMap-activity-noted", @@ -4048,11 +4049,10 @@ func TestConn_receiveIP(t *testing.T) { wantEndpointType: &lazyEndpoint{ maybeEP: newPeerMapInsertableEndpoint(0), }, - wantSize: len(looksLikeGeneveWireGuardInit) - packet.GeneveFixedHeaderLength, - wantIsGeneveEncap: true, - wantOk: true, - wantMetricInc: nil, - wantNoteRecvActivityCalled: true, + wantSize: len(looksLikeGeneveWireGuardInit) - packet.GeneveFixedHeaderLength, + wantIsGeneveEncap: true, + wantOk: true, + wantMetricInc: nil, }, { name: "geneve-WireGuard-init-lazyEndpoint-matching-peerMap-no-activity-noted", @@ -4064,17 +4064,15 @@ func TestConn_receiveIP(t *testing.T) { wantEndpointType: &lazyEndpoint{ maybeEP: newPeerMapInsertableEndpoint(mono.Now().Add(time.Hour * 24)), }, - wantSize: len(looksLikeGeneveWireGuardInit) - packet.GeneveFixedHeaderLength, - wantIsGeneveEncap: true, - wantOk: true, - wantMetricInc: nil, - wantNoteRecvActivityCalled: false, + wantSize: len(looksLikeGeneveWireGuardInit) - packet.GeneveFixedHeaderLength, + wantIsGeneveEncap: true, + wantOk: true, + wantMetricInc: nil, }, // TODO(jwhited): verify cache.de is used when conditions permit } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - noteRecvActivityCalled := false metricBefore := int64(0) if tt.wantMetricInc != nil { metricBefore = tt.wantMetricInc.Value() @@ -4087,9 +4085,6 @@ func TestConn_receiveIP(t *testing.T) { peerMap: newPeerMap(), } c.havePrivateKey.Store(true) - c.noteRecvActivity = func(public key.NodePublic) { - noteRecvActivityCalled = true - } var counts netlogtype.CountsByConnection c.SetConnectionCounter(counts.Add) @@ -4144,10 +4139,6 @@ func TestConn_receiveIP(t *testing.T) { if tt.wantMetricInc != nil && tt.wantMetricInc.Value() != metricBefore+1 { t.Errorf("receiveIP() metric %v not incremented", tt.wantMetricInc.Name()) } - if tt.wantNoteRecvActivityCalled != noteRecvActivityCalled { - t.Errorf("receiveIP() noteRecvActivityCalled = %v, want %v", noteRecvActivityCalled, tt.wantNoteRecvActivityCalled) - } - if tt.cache.de != nil { switch ep := got.(type) { case *endpoint: @@ -4199,34 +4190,29 @@ func TestConn_receiveIP(t *testing.T) { func Test_lazyEndpoint_InitiationMessagePublicKey(t *testing.T) { tests := []struct { - name string - callWithPeerMapKey bool - maybeEPMatchingKey bool - wantNoteRecvActivityCalled bool + name string + callWithPeerMapKey bool + maybeEPMatchingKey bool }{ { - name: "noteRecvActivity-called", - callWithPeerMapKey: true, - maybeEPMatchingKey: false, - wantNoteRecvActivityCalled: true, + name: "noteRecvActivity-called", + callWithPeerMapKey: true, + maybeEPMatchingKey: false, }, { - name: "maybeEP-early-return", - callWithPeerMapKey: true, - maybeEPMatchingKey: true, - wantNoteRecvActivityCalled: false, + name: "maybeEP-early-return", + callWithPeerMapKey: true, + maybeEPMatchingKey: true, }, { - name: "not-in-peerMap-early-return", - callWithPeerMapKey: false, - maybeEPMatchingKey: false, - wantNoteRecvActivityCalled: false, + name: "not-in-peerMap-early-return", + callWithPeerMapKey: false, + maybeEPMatchingKey: false, }, { - name: "not-in-peerMap-maybeEP-early-return", - callWithPeerMapKey: false, - maybeEPMatchingKey: true, - wantNoteRecvActivityCalled: false, + name: "not-in-peerMap-maybeEP-early-return", + callWithPeerMapKey: false, + maybeEPMatchingKey: true, }, } for _, tt := range tests { @@ -4239,19 +4225,7 @@ func Test_lazyEndpoint_InitiationMessagePublicKey(t *testing.T) { key: key.NewDisco().Public(), }) - var noteRecvActivityCalledFor key.NodePublic conn := newConn(t.Logf) - conn.noteRecvActivity = func(public key.NodePublic) { - // wireguard-go will call into ParseEndpoint if the "real" - // noteRecvActivity ends up JIT configuring the peer. Mimic that - // to ensure there are no deadlocks around conn.mu. - // See tailscale/tailscale#16651 & http://go/corp#30836 - _, err := conn.ParseEndpoint(ep.publicKey.UntypedHexString()) - if err != nil { - t.Fatalf("ParseEndpoint() err: %v", err) - } - noteRecvActivityCalledFor = public - } ep.c = conn var pubKey [32]byte @@ -4267,13 +4241,6 @@ func Test_lazyEndpoint_InitiationMessagePublicKey(t *testing.T) { le.maybeEP = ep } le.InitiationMessagePublicKey(pubKey) - want := key.NodePublic{} - if tt.wantNoteRecvActivityCalled { - want = ep.publicKey - } - if noteRecvActivityCalledFor.Compare(want) != 0 { - t.Fatalf("noteRecvActivityCalledFor = %v, want %v", noteRecvActivityCalledFor, want) - } }) } } diff --git a/wgengine/userspace.go b/wgengine/userspace.go index c2ac0c7dc..23edf30b3 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -4,22 +4,21 @@ package wgengine import ( - "bufio" "context" crand "crypto/rand" "crypto/x509" "errors" "fmt" "io" - "maps" "math" "net/netip" "runtime" "slices" - "strings" "sync" + "sync/atomic" "time" + "github.com/gaissmai/bart" "github.com/tailscale/wireguard-go/device" "github.com/tailscale/wireguard-go/tun" "tailscale.com/control/controlknobs" @@ -69,29 +68,6 @@ import ( "tailscale.com/wgengine/wglog" ) -// Lazy wireguard-go configuration parameters. -const ( - // lazyPeerIdleThreshold is the idle duration after - // which we remove a peer from the wireguard configuration. - // (This includes peers that have never been idle, which - // effectively have infinite idleness) - lazyPeerIdleThreshold = 5 * time.Minute - - // packetSendTimeUpdateFrequency controls how often we record - // the time that we wrote a packet to an IP address. - packetSendTimeUpdateFrequency = 10 * time.Second - - // packetSendRecheckWireguardThreshold controls how long we can go - // between packet sends to an IP before checking to see - // whether this IP address needs to be added back to the - // WireGuard peer oconfig. - packetSendRecheckWireguardThreshold = 1 * time.Minute -) - -// statusPollInterval is how often we ask wireguard-go for its engine -// status (as long as there's activity). See docs on its use below. -const statusPollInterval = 1 * time.Minute - // networkLoggerUploadTimeout is the maximum timeout to wait when // shutting down the network logger as it uploads the last network log messages. const networkLoggerUploadTimeout = 5 * time.Second @@ -133,21 +109,27 @@ type userspaceEngine struct { // is being routed over Tailscale. isDNSIPOverTailscale syncs.AtomicValue[func(netip.Addr) bool] - wgLock sync.Mutex // serializes all wgdev operations; see lock order comment below - lastCfgFull wgcfg.Config - lastNMinPeers int - lastRouter *router.Config - lastEngineFull *wgcfg.Config // of full wireguard config, not trimmed - lastEngineInputs *maybeReconfigInputs - lastDNSConfig dns.ConfigView // or invalid if none - lastIsSubnetRouter bool // was the node a primary subnet router in the last run. - recvActivityAt map[key.NodePublic]mono.Time - trimmedNodes map[key.NodePublic]bool // set of node keys of peers currently excluded from wireguard config - sentActivityAt map[netip.Addr]*mono.Time // value is accessed atomically - destIPActivityFuncs map[netip.Addr]func() - lastStatusPollTime mono.Time // last time we polled the engine status - reconfigureVPN func() error // or nil - conn25PacketHooks Conn25PacketHooks // or nil + wgLock sync.Mutex // serializes all wgdev operations; see lock order comment below + + // peerByIPRoute is a longest-prefix-match table built from + // lastCfgFull.Peers AllowedIPs. It's the slow path for + // SetPeerByIPPacketFunc, used when LocalBackend's exact-IP fast path + // (nodeByAddr) misses — i.e. for subnet routes and exit-node default + // routes. Built from lastCfgFull (the wireguard-filtered peer list) + // rather than the netmap so that exit-node selection is honored: the + // netmap has 0.0.0.0/0 in AllowedIPs for every exit-capable peer, but + // lastCfgFull only has it for the currently-selected exit node. + // + // Replaced (not mutated) by maybeReconfigWireguardLocked. Read by + // the per-packet wgdev callback without locking. + peerByIPRoute atomic.Pointer[bart.Table[key.NodePublic]] + + lastCfgFull wgcfg.Config + lastRouter *router.Config + lastDNSConfig dns.ConfigView // or invalid if none + lastIsSubnetRouter bool // was the node a primary subnet router in the last run. + reconfigureVPN func() error // or nil + conn25PacketHooks Conn25PacketHooks // or nil mu sync.Mutex // guards following; see lock order comment below netMap *netmap.NetworkMap // or nil @@ -461,10 +443,6 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error) ForceDiscoKey: conf.ForceDiscoKey, OnDERPRecv: conf.OnDERPRecv, } - if buildfeatures.HasLazyWG { - magicsockOpts.NoteRecvActivity = e.noteRecvActivity - } - var err error e.magicConn, err = magicsock.NewConn(magicsockOpts) if err != nil { @@ -532,6 +510,16 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error) e.logf("Creating WireGuard device...") e.wgdev = wgcfg.NewDevice(e.tundev, e.magicConn.Bind(), e.wgLogger.DeviceLogger) closePool.addFunc(e.wgdev.Close) + + // Install a default outbound-packet peer lookup callback. It uses only + // the engine's BART table, which is rebuilt from the wireguard-filtered + // peer list on every Reconfig. Consumers (e.g. LocalBackend) may later + // call SetPeerByIPPacketFunc to additionally install a fast path for + // exact node-address matches; the BART remains the slow-path fallback. + // Without this default, callers that don't run a LocalBackend would + // have no way to route outbound packets to peers, since peers are + // created lazily from inbound packets only via SetPeerLookupFunc. + e.SetPeerByIPPacketFunc(nil) closePool.addFunc(func() { if err := e.magicConn.Close(); err != nil { e.logf("error closing magicconn: %v", err) @@ -691,163 +679,11 @@ func (e *userspaceEngine) handleLocalPackets(p *packet.Parsed, t *tstun.Wrapper) return filter.Accept } -var debugTrimWireguard = envknob.RegisterOptBool("TS_DEBUG_TRIM_WIREGUARD") - -// forceFullWireguardConfig reports whether we should give wireguard our full -// network map, even for inactive peers. -// -// TODO(bradfitz): remove this at some point. We had a TODO to do it before 1.0 -// but it's still there as of 1.30. Really we should not do this wireguard lazy -// peer config at all and just fix wireguard-go to not have so much extra memory -// usage per peer. That would simplify a lot of Tailscale code. OTOH, we have 50 -// MB of memory on iOS now instead of 15 MB, so the other option is to just give -// up on lazy wireguard config and blow the memory and hope for the best on iOS. -// That's sad too. Or we get rid of these knobs (lazy wireguard config has been -// stable!) but I'm worried that a future regression would be easier to debug -// with these knobs in place. -func (e *userspaceEngine) forceFullWireguardConfig(numPeers int) bool { - // Did the user explicitly enable trimming via the environment variable knob? - if b, ok := debugTrimWireguard().Get(); ok { - return !b - } - return e.controlKnobs != nil && e.controlKnobs.KeepFullWGConfig.Load() -} - -// isTrimmablePeer reports whether p is a peer that we can trim out of the -// network map. -// -// For implementation simplicity, we can only trim peers that have -// only non-subnet AllowedIPs (an IPv4 /32 or IPv6 /128), which is the -// common case for most peers. Subnet router nodes will just always be -// created in the wireguard-go config. -func (e *userspaceEngine) isTrimmablePeer(p *wgcfg.Peer, numPeers int) bool { - if e.forceFullWireguardConfig(numPeers) { - return false - } - - // AllowedIPs must all be single IPs, not subnets. - for _, aip := range p.AllowedIPs { - if !aip.IsSingleIP() { - return false - } - } - return true -} - -// noteRecvActivity is called by magicsock when a packet has been -// received for the peer with node key nk. Magicsock calls this no -// more than every 10 seconds for a given peer. -func (e *userspaceEngine) noteRecvActivity(nk key.NodePublic) { - e.wgLock.Lock() - defer e.wgLock.Unlock() - - if _, ok := e.recvActivityAt[nk]; !ok { - // Not a trimmable peer we care about tracking. (See isTrimmablePeer) - if e.trimmedNodes[nk] { - e.logf("wgengine: [unexpected] noteReceiveActivity called on idle node %v that's not in recvActivityAt", nk.ShortString()) - } - return - } - now := e.timeNow() - e.recvActivityAt[nk] = now - - // As long as there's activity, periodically poll the engine to get - // stats for the far away side effect of - // ipn/ipnlocal.LocalBackend.parseWgStatusLocked to log activity, for - // use in various admin dashboards. - // This particularly matters on platforms without a connected GUI, as - // the GUIs generally poll this enough to cause that logging. But - // tailscaled alone did not, hence this. - if e.lastStatusPollTime.IsZero() || now.Sub(e.lastStatusPollTime) >= statusPollInterval { - e.lastStatusPollTime = now - go e.RequestStatus() - } - - // If the last activity time jumped a bunch (say, at least - // half the idle timeout) then see if we need to reprogram - // WireGuard. This could probably be just - // lazyPeerIdleThreshold without the divide by 2, but - // maybeReconfigWireguardLocked is cheap enough to call every - // couple minutes (just not on every packet). - if e.trimmedNodes[nk] { - e.logf("wgengine: idle peer %v now active, reconfiguring WireGuard", nk.ShortString()) - e.maybeReconfigWireguardLocked(nil) - } -} - -// isActiveSinceLocked reports whether the peer identified by (nk, ip) -// has had a packet sent to or received from it since t. +// maybeReconfigWireguardLocked reconfigures wireguard-go with the current +// full config, installing a PeerLookupFunc for on-demand peer creation. // // e.wgLock must be held. -func (e *userspaceEngine) isActiveSinceLocked(nk key.NodePublic, ip netip.Addr, t mono.Time) bool { - if e.recvActivityAt[nk].After(t) { - return true - } - timePtr, ok := e.sentActivityAt[ip] - if !ok { - return false - } - return timePtr.LoadAtomic().After(t) -} - -// maybeReconfigInputs holds the inputs to the maybeReconfigWireguardLocked -// function. If these things don't change between calls, there's nothing to do. -// -// If you add a field, update Equal and Clone, and add a case to -// TestMaybeReconfigInputsEqual. -type maybeReconfigInputs struct { - WGConfig *wgcfg.Config - TrimmedNodes map[key.NodePublic]bool - - // TrackNodes and TrackIPs are built in full.Peers iteration order, - // which is sorted by NodeID (via sortedPeers -> WGCfg). Equal uses - // order-dependent comparison, so any change to that ordering - // invariant must update the comparison logic. - TrackNodes views.Slice[key.NodePublic] - TrackIPs views.Slice[netip.Addr] -} - -func (i *maybeReconfigInputs) Equal(o *maybeReconfigInputs) bool { - if i == o { - return true - } - if i == nil || o == nil { - return false - } - if !i.WGConfig.Equal(o.WGConfig) { - return false - } - if len(i.TrimmedNodes) != len(o.TrimmedNodes) { - return false - } - for k := range i.TrimmedNodes { - if !o.TrimmedNodes[k] { - return false - } - } - if !views.SliceEqual(i.TrackNodes, o.TrackNodes) { - return false - } - return views.SliceEqual(i.TrackIPs, o.TrackIPs) -} - -func (i *maybeReconfigInputs) Clone() *maybeReconfigInputs { - if i == nil { - return nil - } - v := *i - v.WGConfig = i.WGConfig.Clone() - v.TrimmedNodes = maps.Clone(i.TrimmedNodes) - return &v -} - -// discoChanged are the set of peers whose disco keys have changed, implying they've restarted. -// If a peer is in this set and was previously in the live wireguard config, -// it needs to be first removed and then re-added to flush out its wireguard session key. -// If discoChanged is nil or empty, this extra removal step isn't done. -// -// e.wgLock must be held. -func (e *userspaceEngine) maybeReconfigWireguardLocked(discoChanged map[key.NodePublic]bool) error { +func (e *userspaceEngine) maybeReconfigWireguardLocked() error { if hook := e.testMaybeReconfigHook; hook != nil { hook() return nil @@ -856,181 +692,49 @@ func (e *userspaceEngine) maybeReconfigWireguardLocked(discoChanged map[key.Node full := e.lastCfgFull e.wgLogger.SetPeers(full.Peers) - // Compute a minimal config to pass to wireguard-go - // based on the full config. Prune off all the peers - // and only add the active ones back. - min := full - min.Peers = make([]wgcfg.Peer, 0, e.lastNMinPeers) - - // We'll only keep a peer around if it's been active in - // the past 5 minutes. That's more than WireGuard's key - // rotation time anyway so it's no harm if we remove it - // later if it's been inactive. - var activeCutoff mono.Time - if buildfeatures.HasLazyWG { - activeCutoff = e.timeNow().Add(-lazyPeerIdleThreshold) - } - - // Not all peers can be trimmed from the network map (see - // isTrimmablePeer). For those that are trimmable, keep track of - // their NodeKey and Tailscale IPs. These are the ones we'll need - // to install tracking hooks for to watch their send/receive - // activity. - // - // trackNodes and trackIPs are appended in full.Peers order (sorted - // by NodeID). maybeReconfigInputs.Equal depends on this ordering; - // see the struct comment. - var trackNodes []key.NodePublic - var trackIPs []netip.Addr - if buildfeatures.HasLazyWG { - trackNodes = make([]key.NodePublic, 0, len(full.Peers)) - trackIPs = make([]netip.Addr, 0, len(full.Peers)) - } - - // Don't re-alloc the map; the Go compiler optimizes map clears as of - // Go 1.11, so we can re-use the existing + allocated map. - if e.trimmedNodes != nil { - clear(e.trimmedNodes) - } else { - e.trimmedNodes = make(map[key.NodePublic]bool) - } - - needRemoveStep := false - for i := range full.Peers { - p := &full.Peers[i] - nk := p.PublicKey - if !buildfeatures.HasLazyWG || !e.isTrimmablePeer(p, len(full.Peers)) { - min.Peers = append(min.Peers, *p) - if discoChanged[nk] { - needRemoveStep = true - } - continue - } - trackNodes = append(trackNodes, nk) - recentlyActive := false - for _, cidr := range p.AllowedIPs { - trackIPs = append(trackIPs, cidr.Addr()) - recentlyActive = recentlyActive || e.isActiveSinceLocked(nk, cidr.Addr(), activeCutoff) - } - if recentlyActive { - min.Peers = append(min.Peers, *p) - if discoChanged[nk] { - needRemoveStep = true - } - } else { - e.trimmedNodes[nk] = true + // Rebuild the prefix-match peer routing table from the current + // (wireguard-filtered) peer list and publish it atomically. + rt := &bart.Table[key.NodePublic]{} + for _, p := range full.Peers { + for _, pfx := range p.AllowedIPs { + rt.Insert(pfx, p.PublicKey) } } - e.lastNMinPeers = len(min.Peers) + e.peerByIPRoute.Store(rt) - if changed := checkchange.Update(&e.lastEngineInputs, &maybeReconfigInputs{ - WGConfig: &min, - TrimmedNodes: e.trimmedNodes, - TrackNodes: views.SliceOf(trackNodes), - TrackIPs: views.SliceOf(trackIPs), - }); !changed { - return nil - } - - if buildfeatures.HasLazyWG { - e.updateActivityMapsLocked(trackNodes, trackIPs) - } - - if needRemoveStep { - minner := min - minner.Peers = nil - numRemove := 0 - for _, p := range min.Peers { - if discoChanged[p.PublicKey] { - numRemove++ - continue - } - minner.Peers = append(minner.Peers, p) - } - if numRemove > 0 { - e.logf("wgengine: Reconfig: removing session keys for %d peers", numRemove) - if err := wgcfg.ReconfigDevice(e.wgdev, &minner, e.logf); err != nil { - e.logf("wgdev.Reconfig: %v", err) - return err - } - } - } - - e.logf("wgengine: Reconfig: configuring userspace WireGuard config (with %d/%d peers)", len(min.Peers), len(full.Peers)) - if err := wgcfg.ReconfigDevice(e.wgdev, &min, e.logf); err != nil { + e.logf("wgengine: Reconfig: configuring userspace WireGuard config (with %d peers)", len(full.Peers)) + if err := wgcfg.ReconfigDevice(e.wgdev, &full, e.logf); err != nil { e.logf("wgdev.Reconfig: %v", err) return err } return nil } -// updateActivityMapsLocked updates the data structures used for tracking the activity -// of wireguard peers that we might add/remove dynamically from the real config -// as given to wireguard-go. +// SetPeerByIPPacketFunc installs a callback used by wireguard-go to look up +// which peer should handle an outbound packet by destination IP. // -// e.wgLock must be held. -func (e *userspaceEngine) updateActivityMapsLocked(trackNodes []key.NodePublic, trackIPs []netip.Addr) { - if !buildfeatures.HasLazyWG { - return - } - // Generate the new map of which nodekeys we want to track - // receive times for. - mr := map[key.NodePublic]mono.Time{} // TODO: only recreate this if set of keys changed - for _, nk := range trackNodes { - // Preserve old times in the new map, but also - // populate map entries for new trackNodes values with - // time.Time{} zero values. (Only entries in this map - // are tracked, so the Time zero values allow it to be - // tracked later) - mr[nk] = e.recvActivityAt[nk] - } - e.recvActivityAt = mr - - oldTime := e.sentActivityAt - e.sentActivityAt = make(map[netip.Addr]*mono.Time, len(oldTime)) - oldFunc := e.destIPActivityFuncs - e.destIPActivityFuncs = make(map[netip.Addr]func(), len(oldFunc)) - - updateFn := func(timePtr *mono.Time) func() { - return func() { - now := e.timeNow() - old := timePtr.LoadAtomic() - - // How long's it been since we last sent a packet? - elapsed := now.Sub(old) - if old == 0 { - // For our first packet, old is 0, which has indeterminate meaning. - // Set elapsed to a big number (four score and seven years). - elapsed = 762642 * time.Hour - } - - if elapsed >= packetSendTimeUpdateFrequency { - timePtr.StoreAtomic(now) - } - // On a big jump, assume we might no longer be in the wireguard - // config and go check. - if elapsed >= packetSendRecheckWireguardThreshold { - e.wgLock.Lock() - defer e.wgLock.Unlock() - e.maybeReconfigWireguardLocked(nil) +// fn is an optional fast path for exact node-address matches (e.g. dst is a +// Tailscale IP). On miss (or if fn is nil), the engine's own BART table +// ([userspaceEngine.peerByIPRoute], built from the wireguard-filtered peer +// list) is consulted to handle subnet routes and exit-node default routes. +// +// [NewUserspaceEngine] installs a BART-only default at engine creation time, +// so callers that don't call SetPeerByIPPacketFunc (e.g. those not running +// a LocalBackend) still get working outbound packet routing. +func (e *userspaceEngine) SetPeerByIPPacketFunc(fn func(netip.Addr) (_ key.NodePublic, ok bool)) { + e.wgdev.SetPeerByIPPacketFunc(func(_, dst netip.Addr, _ []byte) (device.NoisePublicKey, bool) { + if fn != nil { + if pk, ok := fn(dst); ok { + return pk.Raw32(), true } } - } - - for _, ip := range trackIPs { - timePtr := oldTime[ip] - if timePtr == nil { - timePtr = new(mono.Time) + if rt := e.peerByIPRoute.Load(); rt != nil { + if pk, ok := rt.Lookup(dst); ok { + return pk.Raw32(), true + } } - e.sentActivityAt[ip] = timePtr - - fn := oldFunc[ip] - if fn == nil { - fn = updateFn(timePtr) - } - e.destIPActivityFuncs[ip] = fn - } - e.tundev.SetDestIPActivityFuncs(e.destIPActivityFuncs) + return device.NoisePublicKey{}, false + }) } // hasOverlap checks if there is a IPPrefix which is common amongst the two @@ -1119,7 +823,7 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config, } isSubnetRouterChanged := buildfeatures.HasAdvertiseRoutes && isSubnetRouter != e.lastIsSubnetRouter - engineChanged := checkchange.Update(&e.lastEngineFull, cfg) + engineChanged := !e.lastCfgFull.Equal(cfg) routerChanged := checkchange.Update(&e.lastRouter, routerCfg) dnsChanged := buildfeatures.HasDNS && !e.lastDNSConfig.Equal(dnsCfg.View()) if dnsChanged { @@ -1151,11 +855,10 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config, } // See if any peers have changed disco keys, which means they've restarted. - // If so, we need to update the wireguard-go/device.Device in two phases: - // once without the node which has restarted, to clear its wireguard session key, - // and a second time with it. + // If so, remove the peer from wireguard-go to flush its session key, + // then let the PeerLookupFunc re-create it on demand. discoChanged := make(map[key.NodePublic]bool) - { + if engineChanged { prevEP := make(map[key.NodePublic]key.DiscoPublic) for i := range e.lastCfgFull.Peers { if p := &e.lastCfgFull.Peers[i]; !p.DiscoKey.IsZero() { @@ -1168,7 +871,6 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config, continue } - // If the key changed, mark the connection for reconfiguration. pub := p.PublicKey if old, ok := prevEP[pub]; ok && old != p.DiscoKey { @@ -1209,21 +911,36 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config, e.testDiscoChangedHook(discoChanged) } + if !e.lastCfgFull.PrivateKey.Equal(cfg.PrivateKey) { + // Tell magicsock about the new (or initial) private key + // (which is needed by DERP) before wgdev gets it, as wgdev + // will start trying to handshake, which we want to be able to + // go over DERP. + if err := e.magicConn.SetPrivateKey(cfg.PrivateKey); err != nil { + e.logf("wgengine: Reconfig: SetPrivateKey: %v", err) + } + + if err := e.wgdev.SetPrivateKey(key.NodePrivateAs[device.NoisePrivateKey](cfg.PrivateKey)); err != nil { + e.logf("wgengine: Reconfig: wgdev.SetPrivateKey: %v", err) + } + } + e.lastCfgFull = *cfg.Clone() - // Tell magicsock about the new (or initial) private key - // (which is needed by DERP) before wgdev gets it, as wgdev - // will start trying to handshake, which we want to be able to - // go over DERP. - if err := e.magicConn.SetPrivateKey(cfg.PrivateKey); err != nil { - e.logf("wgengine: Reconfig: SetPrivateKey: %v", err) - } e.magicConn.UpdatePeers(peerSet) e.magicConn.SetPreferredPort(listenPort) e.magicConn.UpdatePMTUD() - if err := e.maybeReconfigWireguardLocked(discoChanged); err != nil { - return err + if engineChanged { + if err := e.maybeReconfigWireguardLocked(); err != nil { + return err + } + // Now that we've reconfigured wireguard-go, remove any peers with + // changed disco keys to flush their session keys, and let them be + // re-created on demand by the PeerLookupFunc. + for pub := range discoChanged { + e.wgdev.RemovePeer(pub.Raw32()) + } } // Cleanup map of tsmp marks for peers that no longer exists in config. @@ -1368,8 +1085,14 @@ func (e *userspaceEngine) PeerByKey(pubKey key.NodePublic) (_ wgint.Peer, ok boo if dev == nil { return wgint.Peer{}, false } - peer := dev.LookupPeer(pubKey.Raw32()) - if peer == nil { + // Use LookupActivePeer (not LookupPeer) to avoid triggering on-demand + // peer creation via PeerLookupFunc. PeerByKey is called from status + // polling paths (getStatus, getPeerStatusLite) which iterate every peer + // in the netmap; using LookupPeer would lazily create a wireguard-go + // peer for every single netmap peer on each status poll, leaking + // memory via per-peer queues and goroutines. + peer, ok := dev.LookupActivePeer(pubKey.Raw32()) + if !ok { return wgint.Peer{}, false } return wgint.PeerOf(peer), true @@ -1465,8 +1188,6 @@ func (e *userspaceEngine) Close() { e.closing = true e.mu.Unlock() - r := bufio.NewReader(strings.NewReader("")) - e.wgdev.IpcSetOperation(r) e.magicConn.Close() if e.netMonOwned { e.netMon.Close() diff --git a/wgengine/userspace_test.go b/wgengine/userspace_test.go index c874a778a..918c466c1 100644 --- a/wgengine/userspace_test.go +++ b/wgengine/userspace_test.go @@ -8,7 +8,6 @@ import ( "math/rand" "net/netip" "os" - "reflect" "runtime" "testing" @@ -19,81 +18,16 @@ import ( "tailscale.com/health" "tailscale.com/net/dns" "tailscale.com/net/netaddr" - "tailscale.com/net/tstun" "tailscale.com/tailcfg" - "tailscale.com/tstest" - "tailscale.com/tstime/mono" "tailscale.com/types/key" "tailscale.com/types/netmap" "tailscale.com/types/opt" - "tailscale.com/types/views" "tailscale.com/util/eventbus/eventbustest" "tailscale.com/util/usermetric" "tailscale.com/wgengine/router" "tailscale.com/wgengine/wgcfg" ) -func TestNoteReceiveActivity(t *testing.T) { - now := mono.Time(123456) - var logBuf tstest.MemLogger - - confc := make(chan bool, 1) - gotConf := func() bool { - select { - case <-confc: - return true - default: - return false - } - } - e := &userspaceEngine{ - timeNow: func() mono.Time { return now }, - recvActivityAt: map[key.NodePublic]mono.Time{}, - logf: logBuf.Logf, - tundev: new(tstun.Wrapper), - testMaybeReconfigHook: func() { confc <- true }, - trimmedNodes: map[key.NodePublic]bool{}, - } - ra := e.recvActivityAt - - nk := key.NewNode().Public() - - // Activity on an untracked key should do nothing. - e.noteRecvActivity(nk) - if len(ra) != 0 { - t.Fatalf("unexpected growth in map: now has %d keys; want 0", len(ra)) - } - if logBuf.Len() != 0 { - t.Fatalf("unexpected log write (and thus activity): %s", logBuf.Bytes()) - } - - // Now track it, but don't mark it trimmed, so shouldn't update. - ra[nk] = 0 - e.noteRecvActivity(nk) - if len(ra) != 1 { - t.Fatalf("unexpected growth in map: now has %d keys; want 1", len(ra)) - } - if got := ra[nk]; got != now { - t.Fatalf("time in map = %v; want %v", got, now) - } - if gotConf() { - t.Fatalf("unexpected reconfig") - } - - // Now mark it trimmed and expect an update. - e.trimmedNodes[nk] = true - e.noteRecvActivity(nk) - if len(ra) != 1 { - t.Fatalf("unexpected growth in map: now has %d keys; want 1", len(ra)) - } - if got := ra[nk]; got != now { - t.Fatalf("time in map = %v; want %v", got, now) - } - if !gotConf() { - t.Fatalf("didn't get expected reconfig") - } -} - func nodeViews(v []*tailcfg.Node) []tailcfg.NodeView { nv := make([]tailcfg.NodeView, len(v)) for i, n := range v { @@ -112,7 +46,6 @@ func TestUserspaceEngineReconfig(t *testing.T) { t.Fatal(err) } t.Cleanup(e.Close) - ue := e.(*userspaceEngine) routerCfg := &router.Config{} @@ -148,20 +81,6 @@ func TestUserspaceEngineReconfig(t *testing.T) { if err != nil { t.Fatal(err) } - - wantRecvAt := map[key.NodePublic]mono.Time{ - nkFromHex(nodeHex): 0, - } - if got := ue.recvActivityAt; !reflect.DeepEqual(got, wantRecvAt) { - t.Errorf("wrong recvActivityAt\n got: %v\nwant: %v\n", got, wantRecvAt) - } - - wantTrimmedNodes := map[key.NodePublic]bool{ - nkFromHex(nodeHex): true, - } - if got := ue.trimmedNodes; !reflect.DeepEqual(got, wantTrimmedNodes) { - t.Errorf("wrong wantTrimmedNodes\n got: %v\nwant: %v\n", got, wantTrimmedNodes) - } } } @@ -557,121 +476,6 @@ func nkFromHex(hex string) key.NodePublic { return k } -// makeMaybeReconfigInputs builds a maybeReconfigInputs with n peers, -// each with a unique key, disco key, and AllowedIPs entry. -func makeMaybeReconfigInputs(n int) *maybeReconfigInputs { - peers := make([]wgcfg.Peer, n) - trimmed := make(map[key.NodePublic]bool, n) - trackNodes := make([]key.NodePublic, n) - trackIPs := make([]netip.Addr, n) - - for i := range n { - nk := key.NewNode() - pub := nk.Public() - peers[i] = wgcfg.Peer{ - PublicKey: pub, - DiscoKey: key.NewDisco().Public(), - AllowedIPs: []netip.Prefix{netip.PrefixFrom(netip.AddrFrom4([4]byte{100, 64, byte(i >> 8), byte(i)}), 32)}, - } - trimmed[pub] = true - trackNodes[i] = pub - trackIPs[i] = netip.AddrFrom4([4]byte{100, 64, byte(i >> 8), byte(i)}) - } - - return &maybeReconfigInputs{ - WGConfig: &wgcfg.Config{ - PrivateKey: key.NewNode(), - Peers: peers, - MTU: 1280, - }, - TrimmedNodes: trimmed, - TrackNodes: views.SliceOf(trackNodes), - TrackIPs: views.SliceOf(trackIPs), - } -} - -func TestMaybeReconfigInputsEqual(t *testing.T) { - a := makeMaybeReconfigInputs(100) - b := a.Clone() - - // nil cases - if !(*maybeReconfigInputs)(nil).Equal(nil) { - t.Error("nil.Equal(nil) should be true") - } - if a.Equal(nil) { - t.Error("non-nil.Equal(nil) should be false") - } - if (*maybeReconfigInputs)(nil).Equal(a) { - t.Error("nil.Equal(non-nil) should be false") - } - - // same pointer - if !a.Equal(a) { - t.Error("a.Equal(a) should be true") - } - - // cloned equal value - if !a.Equal(b) { - t.Error("a.Equal(clone) should be true") - } - - // Verify that every field in the struct is covered by Equal. - // Each entry mutates exactly one field of a clone and expects - // Equal to return false. If a new field is added to - // maybeReconfigInputs without a corresponding entry here, the - // field count check below will fail. - type mutator struct { - field string - fn func(c *maybeReconfigInputs) - } - mutators := []mutator{ - {"WGConfig", func(c *maybeReconfigInputs) { - c.WGConfig.MTU = 9999 - }}, - {"TrimmedNodes", func(c *maybeReconfigInputs) { - c.TrimmedNodes[key.NewNode().Public()] = true - }}, - {"TrackNodes", func(c *maybeReconfigInputs) { - ns := c.TrackNodes.AsSlice() - ns[0] = key.NewNode().Public() - c.TrackNodes = views.SliceOf(ns) - }}, - {"TrackIPs", func(c *maybeReconfigInputs) { - ips := c.TrackIPs.AsSlice() - ips[0] = netip.MustParseAddr("1.2.3.4") - c.TrackIPs = views.SliceOf(ips) - }}, - } - - // Ensure we have a mutator for every field. - numFields := reflect.TypeOf(maybeReconfigInputs{}).NumField() - if len(mutators) != numFields { - t.Fatalf("maybeReconfigInputs has %d fields but test covers %d; update the mutators table", numFields, len(mutators)) - } - - for _, m := range mutators { - c := a.Clone() - m.fn(c) - if a.Equal(c) { - t.Errorf("Equal did not detect change in field %s", m.field) - } - } -} - -func BenchmarkMaybeReconfigInputsEqual(b *testing.B) { - for _, n := range []int{10, 100, 1000, 5000} { - b.Run(fmt.Sprintf("peers=%d", n), func(b *testing.B) { - a := makeMaybeReconfigInputs(n) - o := a.Clone() - b.ReportAllocs() - b.ResetTimer() - for range b.N { - a.Equal(o) - } - }) - } -} - // an experiment to see if genLocalAddrFunc was worth it. As of Go // 1.16, it still very much is. (30-40x faster) func BenchmarkGenLocalAddrFunc(b *testing.B) { diff --git a/wgengine/watchdog.go b/wgengine/watchdog.go index 4bb320b4b..6aa1c1bd6 100644 --- a/wgengine/watchdog.go +++ b/wgengine/watchdog.go @@ -215,6 +215,10 @@ func (e *watchdogEngine) SetNetworkMap(nm *netmap.NetworkMap) { e.watchdog(SetNetworkMap, func() { e.wrap.SetNetworkMap(nm) }) } +func (e *watchdogEngine) SetPeerByIPPacketFunc(fn func(netip.Addr) (_ key.NodePublic, ok bool)) { + e.wrap.SetPeerByIPPacketFunc(fn) +} + func (e *watchdogEngine) Ping(ip netip.Addr, pingType tailcfg.PingType, size int, cb func(*ipnstate.PingResult)) { e.watchdog(Ping, func() { e.wrap.Ping(ip, pingType, size, cb) }) } diff --git a/wgengine/wgcfg/config.go b/wgengine/wgcfg/config.go index 782812139..5510b65b2 100644 --- a/wgengine/wgcfg/config.go +++ b/wgengine/wgcfg/config.go @@ -53,11 +53,6 @@ type Peer struct { V6MasqAddr *netip.Addr // if non-nil, masquerade IPv6 traffic to this peer using this address IsJailed bool // if true, this peer is jailed and cannot initiate connections PersistentKeepalive uint16 // in seconds between keep-alives; 0 to disable - // wireguard-go's endpoint for this peer. It should always equal Peer.PublicKey. - // We represent it explicitly so that we can detect if they diverge and recover. - // There is no need to set WGEndpoint explicitly when constructing a Peer by hand. - // It is only populated when reading Peers from wireguard-go. - WGEndpoint key.NodePublic } func addrPtrEq(a, b *netip.Addr) bool { @@ -74,8 +69,7 @@ func (p Peer) Equal(o Peer) bool { p.IsJailed == o.IsJailed && p.PersistentKeepalive == o.PersistentKeepalive && addrPtrEq(p.V4MasqAddr, o.V4MasqAddr) && - addrPtrEq(p.V6MasqAddr, o.V6MasqAddr) && - p.WGEndpoint == o.WGEndpoint + addrPtrEq(p.V6MasqAddr, o.V6MasqAddr) } // PeerWithKey returns the Peer with key k and reports whether it was found. diff --git a/wgengine/wgcfg/config_test.go b/wgengine/wgcfg/config_test.go index 7059b17b2..013d3a4b4 100644 --- a/wgengine/wgcfg/config_test.go +++ b/wgengine/wgcfg/config_test.go @@ -30,7 +30,7 @@ func TestPeerEqual(t *testing.T) { for sf := range rt.Fields() { switch sf.Name { case "PublicKey", "DiscoKey", "AllowedIPs", "IsJailed", - "PersistentKeepalive", "V4MasqAddr", "V6MasqAddr", "WGEndpoint": + "PersistentKeepalive", "V4MasqAddr", "V6MasqAddr": // These are compared in [Peer.Equal]. default: t.Errorf("Have you added field %q to Peer.Equal? Do so if not, and then update TestPeerEqual", sf.Name) diff --git a/wgengine/wgcfg/device.go b/wgengine/wgcfg/device.go index ba29cfbdc..ed32f8337 100644 --- a/wgengine/wgcfg/device.go +++ b/wgengine/wgcfg/device.go @@ -4,9 +4,8 @@ package wgcfg import ( - "errors" - "io" - "sort" + "fmt" + "net/netip" "github.com/tailscale/wireguard-go/conn" "github.com/tailscale/wireguard-go/device" @@ -21,27 +20,15 @@ func NewDevice(tunDev tun.Device, bind conn.Bind, logger *device.Logger) *device return ret } -func DeviceConfig(d *device.Device) (*Config, error) { - r, w := io.Pipe() - errc := make(chan error, 1) - go func() { - errc <- d.IpcGetOperation(w) - w.Close() - }() - cfg, fromErr := FromUAPI(r) - r.Close() - getErr := <-errc - err := errors.Join(getErr, fromErr) - if err != nil { - return nil, err - } - sort.Slice(cfg.Peers, func(i, j int) bool { - return cfg.Peers[i].PublicKey.Less(cfg.Peers[j].PublicKey) - }) - return cfg, nil -} - // ReconfigDevice replaces the existing device configuration with cfg. +// +// Instead of using the UAPI text protocol, it uses the wireguard-go direct API +// to install a [device.PeerLookupFunc] callback that creates peers on demand. +// +// The caller is responsible for: +// - calling [device.Device.SetPrivateKey] when the key changes +// - installing a [device.PeerByIPPacketFunc] on the device for outbound +// packet routing (e.g. via [tailscale.com/wgengine.Engine.SetPeerByIPPacketFunc]) func ReconfigDevice(d *device.Device, cfg *Config, logf logger.Logf) (err error) { defer func() { if err != nil { @@ -49,20 +36,52 @@ func ReconfigDevice(d *device.Device, cfg *Config, logf logger.Logf) (err error) } }() - prev, err := DeviceConfig(d) - if err != nil { - return err + // Build peer map: public key → allowed IPs. + peers := make(map[device.NoisePublicKey][]netip.Prefix, len(cfg.Peers)) + for _, p := range cfg.Peers { + peers[p.PublicKey.Raw32()] = p.AllowedIPs } - r, w := io.Pipe() - errc := make(chan error, 1) - go func() { - errc <- d.IpcSetOperation(r) - r.Close() - }() + // Remove peers not in the new config. + d.RemoveMatchingPeers(func(pk device.NoisePublicKey) bool { + _, exists := peers[pk] + return !exists + }) - toErr := cfg.ToUAPI(logf, w, prev) - w.Close() - setErr := <-errc - return errors.Join(setErr, toErr) + // Update AllowedIPs on any already-active peers whose config may have + // changed. Peers that don't exist yet will get the correct AllowedIPs + // from PeerLookupFunc when they are lazily created. + for pk, allowedIPs := range peers { + if peer, ok := d.LookupActivePeer(pk); ok { + peer.SetAllowedIPs(allowedIPs) + } + } + + // Install callback for lazy peer creation (incoming packets). + bind := d.Bind() + d.SetPeerLookupFunc(func(pubk device.NoisePublicKey) (_ *device.NewPeerConfig, ok bool) { + allowedIPs, ok := peers[pubk] + if !ok { + return nil, false + } + ep, err := bind.ParseEndpoint(fmt.Sprintf("%x", pubk[:])) + if err != nil { + logf("wgcfg: failed to parse endpoint for peer %x: %v", pubk[:8], err) + return nil, false + } + return &device.NewPeerConfig{ + AllowedIPs: allowedIPs, + Endpoint: ep, + }, true + }) + + // RemoveMatchingPeers _again_, now that SetPeerLookupFunc is installed, + // lest any removed peers got re-created before the new SetPeerLookupFunc + // func was installed. + d.RemoveMatchingPeers(func(pk device.NoisePublicKey) bool { + _, exists := peers[pk] + return !exists + }) + + return nil } diff --git a/wgengine/wgcfg/device_test.go b/wgengine/wgcfg/device_test.go index 507f22311..07eb41adb 100644 --- a/wgengine/wgcfg/device_test.go +++ b/wgengine/wgcfg/device_test.go @@ -4,33 +4,22 @@ package wgcfg import ( - "bufio" - "bytes" "io" "net/netip" "os" - "sort" - "strings" - "sync" "testing" "github.com/tailscale/wireguard-go/conn" "github.com/tailscale/wireguard-go/device" "github.com/tailscale/wireguard-go/tun" - "go4.org/mem" "tailscale.com/types/key" ) -func TestDeviceConfig(t *testing.T) { - newK := func() (key.NodePublic, key.NodePrivate) { - t.Helper() - k := key.NewNode() - return k.Public(), k - } +func TestReconfigDevice(t *testing.T) { k1, pk1 := newK() ip1 := netip.MustParsePrefix("10.0.0.1/32") - k2, pk2 := newK() + k2, _ := newK() ip2 := netip.MustParsePrefix("10.0.0.2/32") k3, _ := newK() @@ -38,165 +27,80 @@ func TestDeviceConfig(t *testing.T) { cfg1 := &Config{ PrivateKey: pk1, - Peers: []Peer{{ - PublicKey: k2, - AllowedIPs: []netip.Prefix{ip2}, - }}, + Peers: []Peer{ + {PublicKey: k2, AllowedIPs: []netip.Prefix{ip2}}, + }, } - cfg2 := &Config{ - PrivateKey: pk2, - Peers: []Peer{{ - PublicKey: k1, - AllowedIPs: []netip.Prefix{ip1}, - PersistentKeepalive: 5, - }}, - } + dev := NewDevice(newNilTun(), new(noopBind), device.NewLogger(device.LogLevelError, "test")) + defer dev.Close() - device1 := NewDevice(newNilTun(), new(noopBind), device.NewLogger(device.LogLevelError, "device1")) - device2 := NewDevice(newNilTun(), new(noopBind), device.NewLogger(device.LogLevelError, "device2")) - defer device1.Close() - defer device2.Close() - - cmp := func(t *testing.T, d *device.Device, want *Config) { - t.Helper() - got, err := DeviceConfig(d) - if err != nil { + t.Run("initial-config", func(t *testing.T) { + if err := ReconfigDevice(dev, cfg1, t.Logf); err != nil { t.Fatal(err) } - prev := new(Config) - gotbuf := new(strings.Builder) - err = got.ToUAPI(t.Logf, gotbuf, prev) - gotStr := gotbuf.String() - if err != nil { - t.Errorf("got.ToUAPI(): error: %v", err) - return + // Peer should be creatable on demand via LookupPeer. + peer := dev.LookupPeer(k2.Raw32()) + if peer == nil { + t.Fatal("expected peer k2 to exist via LookupPeer") } - wantbuf := new(strings.Builder) - err = want.ToUAPI(t.Logf, wantbuf, prev) - wantStr := wantbuf.String() - if err != nil { - t.Errorf("want.ToUAPI(): error: %v", err) - return + // Unknown peer should not be found. + peer = dev.LookupPeer(k3.Raw32()) + if peer != nil { + t.Fatal("expected unknown peer k3 to not exist") } - if gotStr != wantStr { - buf := new(bytes.Buffer) - w := bufio.NewWriter(buf) - if err := d.IpcGetOperation(w); err != nil { - t.Errorf("on error, could not IpcGetOperation: %v", err) - } - w.Flush() - t.Errorf("config mismatch:\n---- got:\n%s\n---- want:\n%s\n---- uapi:\n%s", gotStr, wantStr, buf.String()) - } - } - - t.Run("device1-config", func(t *testing.T) { - if err := ReconfigDevice(device1, cfg1, t.Logf); err != nil { - t.Fatal(err) - } - cmp(t, device1, cfg1) }) - t.Run("device2-config", func(t *testing.T) { - if err := ReconfigDevice(device2, cfg2, t.Logf); err != nil { - t.Fatal(err) - } - cmp(t, device2, cfg2) - }) - - // This is only to test that Config and Reconfig are properly synchronized. - t.Run("device2-config-reconfig", func(t *testing.T) { - var wg sync.WaitGroup - wg.Add(2) - - go func() { - ReconfigDevice(device2, cfg2, t.Logf) - wg.Done() - }() - - go func() { - DeviceConfig(device2) - wg.Done() - }() - - wg.Wait() - }) - - t.Run("device1-modify-peer", func(t *testing.T) { - cfg1.Peers[0].DiscoKey = key.DiscoPublicFromRaw32(mem.B([]byte{0: 1, 31: 0})) - if err := ReconfigDevice(device1, cfg1, t.Logf); err != nil { - t.Fatal(err) - } - cmp(t, device1, cfg1) - }) - - t.Run("device1-replace-endpoint", func(t *testing.T) { - cfg1.Peers[0].DiscoKey = key.DiscoPublicFromRaw32(mem.B([]byte{0: 2, 31: 0})) - if err := ReconfigDevice(device1, cfg1, t.Logf); err != nil { - t.Fatal(err) - } - cmp(t, device1, cfg1) - }) - - t.Run("device1-add-new-peer", func(t *testing.T) { + t.Run("add-peer", func(t *testing.T) { cfg1.Peers = append(cfg1.Peers, Peer{ PublicKey: k3, AllowedIPs: []netip.Prefix{ip3}, }) - sort.Slice(cfg1.Peers, func(i, j int) bool { - return cfg1.Peers[i].PublicKey.Less(cfg1.Peers[j].PublicKey) - }) - - origCfg, err := DeviceConfig(device1) - if err != nil { + if err := ReconfigDevice(dev, cfg1, t.Logf); err != nil { t.Fatal(err) } - - if err := ReconfigDevice(device1, cfg1, t.Logf); err != nil { - t.Fatal(err) + // Both peers should now be discoverable. + if p := dev.LookupPeer(k2.Raw32()); p == nil { + t.Fatal("expected peer k2 to exist") } - cmp(t, device1, cfg1) - - newCfg, err := DeviceConfig(device1) - if err != nil { - t.Fatal(err) - } - - peer0 := func(cfg *Config) Peer { - p, ok := cfg.PeerWithKey(k2) - if !ok { - t.Helper() - t.Fatal("failed to look up peer 2") - } - return p - } - peersEqual := func(p, q Peer) bool { - return p.PublicKey == q.PublicKey && p.DiscoKey == q.DiscoKey && p.PersistentKeepalive == q.PersistentKeepalive && cidrsEqual(p.AllowedIPs, q.AllowedIPs) - } - if !peersEqual(peer0(origCfg), peer0(newCfg)) { - t.Error("reconfig modified old peer") + if p := dev.LookupPeer(k3.Raw32()); p == nil { + t.Fatal("expected peer k3 to exist") } }) - t.Run("device1-remove-peer", func(t *testing.T) { - removeKey := cfg1.Peers[len(cfg1.Peers)-1].PublicKey - cfg1.Peers = cfg1.Peers[:len(cfg1.Peers)-1] - - if err := ReconfigDevice(device1, cfg1, t.Logf); err != nil { + t.Run("remove-peer", func(t *testing.T) { + cfg2 := &Config{ + PrivateKey: pk1, + Peers: []Peer{ + {PublicKey: k2, AllowedIPs: []netip.Prefix{ip2}}, + }, + } + if err := ReconfigDevice(dev, cfg2, t.Logf); err != nil { t.Fatal(err) } - cmp(t, device1, cfg1) - - newCfg, err := DeviceConfig(device1) - if err != nil { - t.Fatal(err) + // k2 should still be discoverable. + if p := dev.LookupPeer(k2.Raw32()); p == nil { + t.Fatal("expected peer k2 to exist") } - - _, ok := newCfg.PeerWithKey(removeKey) - if ok { - t.Error("reconfig failed to remove peer") + // k3 should no longer be discoverable. + if p := dev.LookupPeer(k3.Raw32()); p != nil { + t.Fatal("expected peer k3 to not exist after removal") } }) + + t.Run("self-key-not-peer", func(t *testing.T) { + // The device's own key should not be a peer. + if p := dev.LookupPeer(k1.Raw32()); p != nil { + t.Fatal("expected own key to not be a peer") + } + }) + + _ = ip1 // suppress unused +} + +func newK() (key.NodePublic, key.NodePrivate) { + k := key.NewNode() + return k.Public(), k } // TODO: replace with a loopback tunnel diff --git a/wgengine/wgcfg/parser.go b/wgengine/wgcfg/parser.go deleted file mode 100644 index 8fb921409..000000000 --- a/wgengine/wgcfg/parser.go +++ /dev/null @@ -1,186 +0,0 @@ -// Copyright (c) Tailscale Inc & contributors -// SPDX-License-Identifier: BSD-3-Clause - -package wgcfg - -import ( - "bufio" - "fmt" - "io" - "net" - "net/netip" - "strconv" - "strings" - - "go4.org/mem" - "tailscale.com/types/key" -) - -type ParseError struct { - why string - offender string -} - -func (e *ParseError) Error() string { - return fmt.Sprintf("%s: %q", e.why, e.offender) -} - -func parseEndpoint(s string) (host string, port uint16, err error) { - i := strings.LastIndexByte(s, ':') - if i < 0 { - return "", 0, &ParseError{"Missing port from endpoint", s} - } - host, portStr := s[:i], s[i+1:] - if len(host) < 1 { - return "", 0, &ParseError{"Invalid endpoint host", host} - } - uport, err := strconv.ParseUint(portStr, 10, 16) - if err != nil { - return "", 0, err - } - hostColon := strings.IndexByte(host, ':') - if host[0] == '[' || host[len(host)-1] == ']' || hostColon > 0 { - err := &ParseError{"Brackets must contain an IPv6 address", host} - if len(host) > 3 && host[0] == '[' && host[len(host)-1] == ']' && hostColon > 0 { - maybeV6 := net.ParseIP(host[1 : len(host)-1]) - if maybeV6 == nil || len(maybeV6) != net.IPv6len { - return "", 0, err - } - } else { - return "", 0, err - } - host = host[1 : len(host)-1] - } - return host, uint16(uport), nil -} - -// memROCut separates a mem.RO at the separator if it exists, otherwise -// it returns two empty ROs and reports that it was not found. -func memROCut(s mem.RO, sep byte) (before, after mem.RO, found bool) { - if i := mem.IndexByte(s, sep); i >= 0 { - return s.SliceTo(i), s.SliceFrom(i + 1), true - } - found = false - return -} - -// FromUAPI generates a Config from r. -// r should be generated by calling device.IpcGetOperation; -// it is not compatible with other uapi streams. -func FromUAPI(r io.Reader) (*Config, error) { - cfg := new(Config) - var peer *Peer // current peer being operated on - deviceConfig := true - - scanner := bufio.NewScanner(r) - for scanner.Scan() { - line := mem.B(scanner.Bytes()) - if line.Len() == 0 { - continue - } - key, value, ok := memROCut(line, '=') - if !ok { - return nil, fmt.Errorf("failed to cut line %q on =", line.StringCopy()) - } - valueBytes := scanner.Bytes()[key.Len()+1:] - - if key.EqualString("public_key") { - if deviceConfig { - deviceConfig = false - } - // Load/create the peer we are now configuring. - var err error - peer, err = cfg.handlePublicKeyLine(valueBytes) - if err != nil { - return nil, err - } - continue - } - - var err error - if deviceConfig { - err = cfg.handleDeviceLine(key, value, valueBytes) - } else { - err = cfg.handlePeerLine(peer, key, value, valueBytes) - } - if err != nil { - return nil, err - } - } - - if err := scanner.Err(); err != nil { - return nil, err - } - - return cfg, nil -} - -func (cfg *Config) handleDeviceLine(k, value mem.RO, valueBytes []byte) error { - switch { - case k.EqualString("private_key"): - // wireguard-go guarantees not to send zero value; private keys are already clamped. - var err error - cfg.PrivateKey, err = key.ParseNodePrivateUntyped(value) - if err != nil { - return err - } - case k.EqualString("listen_port") || k.EqualString("fwmark"): - // ignore - default: - return fmt.Errorf("unexpected IpcGetOperation key: %q", k.StringCopy()) - } - return nil -} - -func (cfg *Config) handlePublicKeyLine(valueBytes []byte) (*Peer, error) { - p := Peer{} - var err error - p.PublicKey, err = key.ParseNodePublicUntyped(mem.B(valueBytes)) - if err != nil { - return nil, err - } - cfg.Peers = append(cfg.Peers, p) - return &cfg.Peers[len(cfg.Peers)-1], nil -} - -func (cfg *Config) handlePeerLine(peer *Peer, k, value mem.RO, valueBytes []byte) error { - switch { - case k.EqualString("endpoint"): - nk, err := key.ParseNodePublicUntyped(value) - if err != nil { - return fmt.Errorf("invalid endpoint %q for peer %q, expected a hex public key", value.StringCopy(), peer.PublicKey.ShortString()) - } - // nk ought to equal peer.PublicKey. - // Under some rare circumstances, it might not. See corp issue #3016. - // Even if that happens, don't stop early, so that we can recover from it. - // Instead, note the value of nk so we can fix as needed. - peer.WGEndpoint = nk - case k.EqualString("persistent_keepalive_interval"): - n, err := mem.ParseUint(value, 10, 16) - if err != nil { - return err - } - peer.PersistentKeepalive = uint16(n) - case k.EqualString("allowed_ip"): - ipp := netip.Prefix{} - err := ipp.UnmarshalText(valueBytes) - if err != nil { - return err - } - peer.AllowedIPs = append(peer.AllowedIPs, ipp) - case k.EqualString("protocol_version"): - if !value.EqualString("1") { - return fmt.Errorf("invalid protocol version: %q", value.StringCopy()) - } - case k.EqualString("replace_allowed_ips") || - k.EqualString("preshared_key") || - k.EqualString("last_handshake_time_sec") || - k.EqualString("last_handshake_time_nsec") || - k.EqualString("tx_bytes") || - k.EqualString("rx_bytes"): - // ignore - default: - return fmt.Errorf("unexpected IpcGetOperation key: %q", k.StringCopy()) - } - return nil -} diff --git a/wgengine/wgcfg/parser_test.go b/wgengine/wgcfg/parser_test.go deleted file mode 100644 index 8c38ec025..000000000 --- a/wgengine/wgcfg/parser_test.go +++ /dev/null @@ -1,95 +0,0 @@ -// Copyright (c) Tailscale Inc & contributors -// SPDX-License-Identifier: BSD-3-Clause - -package wgcfg - -import ( - "bufio" - "bytes" - "io" - "net/netip" - "reflect" - "runtime" - "testing" - - "tailscale.com/types/key" -) - -func noError(t *testing.T, err error) bool { - if err == nil { - return true - } - _, fn, line, _ := runtime.Caller(1) - t.Errorf("Error at %s:%d: %#v", fn, line, err) - return false -} - -func equal(t *testing.T, expected, actual any) bool { - if reflect.DeepEqual(expected, actual) { - return true - } - _, fn, line, _ := runtime.Caller(1) - t.Errorf("Failed equals at %s:%d\nactual %#v\nexpected %#v", fn, line, actual, expected) - return false -} - -func TestParseEndpoint(t *testing.T) { - _, _, err := parseEndpoint("[192.168.42.0:]:51880") - if err == nil { - t.Error("Error was expected") - } - host, port, err := parseEndpoint("192.168.42.0:51880") - if noError(t, err) { - equal(t, "192.168.42.0", host) - equal(t, uint16(51880), port) - } - host, port, err = parseEndpoint("test.wireguard.com:18981") - if noError(t, err) { - equal(t, "test.wireguard.com", host) - equal(t, uint16(18981), port) - } - host, port, err = parseEndpoint("[2607:5300:60:6b0::c05f:543]:2468") - if noError(t, err) { - equal(t, "2607:5300:60:6b0::c05f:543", host) - equal(t, uint16(2468), port) - } - _, _, err = parseEndpoint("[::::::invalid:18981") - if err == nil { - t.Error("Error was expected") - } -} - -func BenchmarkFromUAPI(b *testing.B) { - newK := func() (key.NodePublic, key.NodePrivate) { - b.Helper() - k := key.NewNode() - return k.Public(), k - } - k1, pk1 := newK() - ip1 := netip.MustParsePrefix("10.0.0.1/32") - - peer := Peer{ - PublicKey: k1, - AllowedIPs: []netip.Prefix{ip1}, - } - cfg1 := &Config{ - PrivateKey: pk1, - Peers: []Peer{peer, peer, peer, peer}, - } - - buf := new(bytes.Buffer) - w := bufio.NewWriter(buf) - if err := cfg1.ToUAPI(b.Logf, w, &Config{}); err != nil { - b.Fatal(err) - } - w.Flush() - r := bytes.NewReader(buf.Bytes()) - b.ReportAllocs() - for range b.N { - r.Seek(0, io.SeekStart) - _, err := FromUAPI(r) - if err != nil { - b.Errorf("failed from UAPI: %v", err) - } - } -} diff --git a/wgengine/wgcfg/wgcfg_clone.go b/wgengine/wgcfg/wgcfg_clone.go index 9e8de7b6f..a8a212267 100644 --- a/wgengine/wgcfg/wgcfg_clone.go +++ b/wgengine/wgcfg/wgcfg_clone.go @@ -72,5 +72,4 @@ var _PeerCloneNeedsRegeneration = Peer(struct { V6MasqAddr *netip.Addr IsJailed bool PersistentKeepalive uint16 - WGEndpoint key.NodePublic }{}) diff --git a/wgengine/wgcfg/writer.go b/wgengine/wgcfg/writer.go deleted file mode 100644 index f4981e3e9..000000000 --- a/wgengine/wgcfg/writer.go +++ /dev/null @@ -1,154 +0,0 @@ -// Copyright (c) Tailscale Inc & contributors -// SPDX-License-Identifier: BSD-3-Clause - -package wgcfg - -import ( - "fmt" - "io" - "net/netip" - "strconv" - - "tailscale.com/types/key" - "tailscale.com/types/logger" -) - -// ToUAPI writes cfg in UAPI format to w. -// Prev is the previous device Config. -// -// Prev is required so that we can remove now-defunct peers without having to -// remove and re-add all peers, and so that we can avoid writing information -// about peers that have not changed since the previous time we wrote our -// Config. -func (cfg *Config) ToUAPI(logf logger.Logf, w io.Writer, prev *Config) error { - var stickyErr error - set := func(key, value string) { - if stickyErr != nil { - return - } - _, err := fmt.Fprintf(w, "%s=%s\n", key, value) - if err != nil { - stickyErr = err - } - } - setUint16 := func(key string, value uint16) { - set(key, strconv.FormatUint(uint64(value), 10)) - } - setPeer := func(peer Peer) { - set("public_key", peer.PublicKey.UntypedHexString()) - } - - // Device config. - if !prev.PrivateKey.Equal(cfg.PrivateKey) { - set("private_key", cfg.PrivateKey.UntypedHexString()) - } - - old := make(map[key.NodePublic]Peer) - for _, p := range prev.Peers { - old[p.PublicKey] = p - } - - // Add/configure all new peers. - for _, p := range cfg.Peers { - oldPeer, wasPresent := old[p.PublicKey] - - // We only want to write the peer header/version if we're about - // to change something about that peer, or if it's a new peer. - // Figure out up-front whether we'll need to do anything for - // this peer, and skip doing anything if not. - // - // If the peer was not present in the previous config, this - // implies that this is a new peer; set all of these to 'true' - // to ensure that we're writing the full peer configuration. - willSetEndpoint := oldPeer.WGEndpoint != p.PublicKey || !wasPresent - willChangeIPs := !cidrsEqual(oldPeer.AllowedIPs, p.AllowedIPs) || !wasPresent - willChangeKeepalive := oldPeer.PersistentKeepalive != p.PersistentKeepalive // if not wasPresent, no need to redundantly set zero (default) - - if !willSetEndpoint && !willChangeIPs && !willChangeKeepalive { - // It's safe to skip doing anything here; wireguard-go - // will not remove a peer if it's unspecified unless we - // tell it to (which we do below if necessary). - continue - } - - setPeer(p) - set("protocol_version", "1") - - // Avoid setting endpoints if the correct one is already known - // to WireGuard, because doing so generates a bit more work in - // calling magicsock's ParseEndpoint for effectively a no-op. - if willSetEndpoint { - if wasPresent { - // We had an endpoint, and it was wrong. - // By construction, this should not happen. - // If it does, keep going so that we can recover from it, - // but log so that we know about it, - // because it is an indicator of other failed invariants. - // See corp issue 3016. - logf("[unexpected] endpoint changed from %s to %s", oldPeer.WGEndpoint, p.PublicKey) - } - set("endpoint", p.PublicKey.UntypedHexString()) - } - - // TODO: replace_allowed_ips is expensive. - // If p.AllowedIPs is a strict superset of oldPeer.AllowedIPs, - // then skip replace_allowed_ips and instead add only - // the new ipps with allowed_ip. - if willChangeIPs { - set("replace_allowed_ips", "true") - for _, ipp := range p.AllowedIPs { - set("allowed_ip", ipp.String()) - } - } - - // Set PersistentKeepalive after the peer is otherwise configured, - // because it can trigger handshake packets. - if willChangeKeepalive { - setUint16("persistent_keepalive_interval", p.PersistentKeepalive) - } - } - - // Remove peers that were present but should no longer be. - for _, p := range cfg.Peers { - delete(old, p.PublicKey) - } - for _, p := range old { - setPeer(p) - set("remove", "true") - } - - if stickyErr != nil { - stickyErr = fmt.Errorf("ToUAPI: %w", stickyErr) - } - return stickyErr -} - -func cidrsEqual(x, y []netip.Prefix) bool { - // TODO: re-implement using netaddr.IPSet.Equal. - if len(x) != len(y) { - return false - } - // First see if they're equal in order, without allocating. - exact := true - for i := range x { - if x[i] != y[i] { - exact = false - break - } - } - if exact { - return true - } - - // Otherwise, see if they're the same, but out of order. - m := make(map[netip.Prefix]bool) - for _, v := range x { - m[v] = true - } - for _, v := range y { - if !m[v] { - return false - } - } - return true -} diff --git a/wgengine/wgengine.go b/wgengine/wgengine.go index 9dd782e4a..5ca4b75cf 100644 --- a/wgengine/wgengine.go +++ b/wgengine/wgengine.go @@ -137,4 +137,8 @@ type Engine interface { // packets traversing the data path. The hook can be uninstalled by // calling this function with a nil value. InstallCaptureHook(packet.CaptureCallback) + + // SetPeerByIPPacketFunc installs a callback used by wireguard-go to + // look up which peer should handle an outbound packet by destination IP. + SetPeerByIPPacketFunc(func(netip.Addr) (_ key.NodePublic, ok bool)) }