From 9c773af04cb980dbfa709df236f7ae6290f36ce4 Mon Sep 17 00:00:00 2001 From: Tom DNetto Date: Sun, 4 Dec 2022 20:07:03 -0800 Subject: [PATCH] ipn/ipnlocal: fix use of stale profile while processing netmap Signed-off-by: Tom DNetto --- ipn/ipnlocal/local.go | 37 ++++++++++++++++++++++-------------- ipn/ipnlocal/network-lock.go | 2 ++ 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 39090f8fa..4158ca68b 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -829,7 +829,9 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) { if err := b.pm.DeleteProfile(b.pm.CurrentProfile().ID); err != nil { b.logf("error deleting profile: %v", err) } - b.resetForProfileChangeLockedOnEntry() + if err := b.resetForProfileChangeLockedOnEntry(); err != nil { + b.logf("resetForProfileChangeLockedOnEntry err: %v", err) + } return } @@ -851,9 +853,6 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) { if !prefs.Persist.View().Equals(*st.Persist) { prefsChanged = true prefs.Persist = st.Persist.AsStruct() - if err := b.initTKALocked(); err != nil { - b.logf("initTKALocked: %v", err) - } } } if st.URL != "" { @@ -873,7 +872,26 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) { if findExitNodeIDLocked(prefs, st.NetMap) { prefsChanged = true } - // Prefs will be written out; this is not safe unless locked or cloned. + + // Perform all mutations of prefs based on the netmap here. + if st.NetMap != nil { + if b.updatePersistFromNetMapLocked(st.NetMap, prefs) { + prefsChanged = true + } + } + // Prefs will be written out if stale; this is not safe unless locked or cloned. + if prefsChanged { + if err := b.pm.SetPrefs(prefs.View()); err != nil { + b.logf("Failed to save new controlclient state: %v", err) + } + } + // initTKALocked is dependent on CurrentProfile.ID, which is initialized + // (for new profiles) on the first call to b.pm.SetPrefs. + if err := b.initTKALocked(); err != nil { + b.logf("initTKALocked: %v", err) + } + + // Perform all reconfiguration based on the netmap here. if st.NetMap != nil { b.capTailnetLock = hasCapability(st.NetMap, tailcfg.CapabilityTailnetLockAlpha) @@ -896,18 +914,9 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) { if !envknob.TKASkipSignatureCheck() { b.tkaFilterNetmapLocked(st.NetMap) } - if b.updatePersistFromNetMapLocked(st.NetMap, prefs) { - prefsChanged = true - } b.setNetMapLocked(st.NetMap) b.updateFilterLocked(st.NetMap, prefs.View()) } - - if prefsChanged { - if err := b.pm.SetPrefs(prefs.View()); err != nil { - b.logf("Failed to save new controlclient state: %v", err) - } - } b.mu.Unlock() // Now complete the lock-free parts of what we started while locked. diff --git a/ipn/ipnlocal/network-lock.go b/ipn/ipnlocal/network-lock.go index 612e8bc21..7fb4dd77c 100644 --- a/ipn/ipnlocal/network-lock.go +++ b/ipn/ipnlocal/network-lock.go @@ -300,6 +300,8 @@ func (b *LocalBackend) tkaApplyDisablementLocked(secret []byte) error { // chonkPathLocked returns the absolute path to the directory in which TKA // state (the 'tailchonk') is stored. +// +// b.mu must be held. func (b *LocalBackend) chonkPathLocked() string { return filepath.Join(b.TailscaleVarRoot(), "tka-profiles", string(b.pm.CurrentProfile().ID)) }