summaryrefslogtreecommitdiffhomepage
path: root/wgengine/magicsock
diff options
context:
space:
mode:
Diffstat (limited to 'wgengine/magicsock')
-rw-r--r--wgengine/magicsock/endpoint.go5
-rw-r--r--wgengine/magicsock/magicsock.go35
-rw-r--r--wgengine/magicsock/magicsock_test.go221
3 files changed, 97 insertions, 164 deletions
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 1f6e89591..906b59688 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
@@ -456,19 +455,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
@@ -647,7 +633,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
@@ -4268,16 +4253,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 {
@@ -4285,9 +4264,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
@@ -4295,11 +4271,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 a6510a57f..bb4aedaa9 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.
@@ -3935,60 +3943,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",
@@ -4002,19 +4005,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",
@@ -4026,11 +4027,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",
@@ -4042,17 +4042,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()
@@ -4065,9 +4063,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)
@@ -4122,10 +4117,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:
@@ -4177,34 +4168,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 {
@@ -4217,19 +4203,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
@@ -4245,13 +4219,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)
- }
})
}
}
Copyright © 2025 - 2026 Wayne Cole. WTFPL. Attribution appreciated. No warranty. Not liable for bodily damages.