From cdb924f87b757696dcc821deea58456629800c02 Mon Sep 17 00:00:00 2001
From: Maisem Ali <maisem@tailscale.com>
Date: Fri, 2 Dec 2022 13:54:32 +0500
Subject: [PATCH] ipn/ipnlocal: sanitize prefs in more notify code paths
Signed-off-by: Maisem Ali <maisem@tailscale.com>
---
ipn/ipnlocal/local.go | 33 ++++++++++++---------------------
1 file changed, 12 insertions(+), 21 deletions(-)
diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go
index dad0a15f9..dca383ba7 100644
--- a/ipn/ipnlocal/local.go
+++ b/ipn/ipnlocal/local.go
@@ -535,10 +535,10 @@ func stripKeysFromPrefs(p ipn.PrefsView) ipn.PrefsView {
func (b *LocalBackend) Prefs() ipn.PrefsView {
b.mu.Lock()
defer b.mu.Unlock()
- return b.prefsLocked()
+ return b.sanitizedPrefsLocked()
}
-func (b *LocalBackend) prefsLocked() ipn.PrefsView {
+func (b *LocalBackend) sanitizedPrefsLocked() ipn.PrefsView {
return stripKeysFromPrefs(b.pm.CurrentPrefs())
}
@@ -912,7 +912,7 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) {
// Now complete the lock-free parts of what we started while locked.
if prefsChanged {
- b.notifyPrefs(prefs.View())
+ b.send(ipn.Notify{Prefs: ptr.To(prefs.View())})
}
if st.NetMap != nil {
@@ -927,8 +927,7 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) {
b.logf("Failed to save new controlclient state: %v", err)
}
b.mu.Unlock()
- np := stripKeysFromPrefs(p)
- b.send(ipn.Notify{ErrMessage: &msg, Prefs: &np})
+ b.send(ipn.Notify{ErrMessage: &msg, Prefs: &p})
return
}
if netMap != nil {
@@ -1336,7 +1335,7 @@ func (b *LocalBackend) Start(opts ipn.Options) error {
blid := b.backendLogID
b.logf("Backend: logs: be:%v fe:%v", blid, opts.FrontendLogID)
b.send(ipn.Notify{BackendLogID: &blid})
- b.notifyPrefs(prefs)
+ b.send(ipn.Notify{Prefs: &prefs})
if !loggedOut && b.hasNodeKey() {
// Even if !WantRunning, we should verify our key, if there
@@ -1348,19 +1347,6 @@ func (b *LocalBackend) Start(opts ipn.Options) error {
return nil
}
-// notifyPrefs delivers prefs to the connected frontend and any API watchers
-// from LocalBackend.WatchNotifications (via the LocalAPI).
-// It strips keys and other sensitive data prior to sending.
-//
-// If no frontend is connected or API watchers are backed up, the notification
-// is dropped without being delivered.
-//
-// b.mu must not be held.
-func (b *LocalBackend) notifyPrefs(p ipn.PrefsView) {
- np := stripKeysFromPrefs(p)
- b.send(ipn.Notify{Prefs: &np})
-}
-
var warnInvalidUnsignedNodes = health.NewWarnable()
// updateFilterLocked updates the packet filter in wgengine based on the
@@ -1754,7 +1740,7 @@ func (b *LocalBackend) WatchNotifications(ctx context.Context, mask ipn.NotifyWa
}
}
if mask&ipn.NotifyInitialPrefs != 0 {
- ini.Prefs = ptr.To(b.prefsLocked())
+ ini.Prefs = ptr.To(b.sanitizedPrefsLocked())
}
if mask&ipn.NotifyInitialNetMap != 0 {
ini.NetMap = b.netMap
@@ -1833,8 +1819,13 @@ func (b *LocalBackend) DebugNotify(n ipn.Notify) {
// If no frontend is connected or API watchers are backed up, the notification
// is dropped without being delivered.
//
+// If n contains Prefs, those will be sanitized before being delivered.
+//
// b.mu must not be held.
func (b *LocalBackend) send(n ipn.Notify) {
+ if n.Prefs != nil {
+ n.Prefs = ptr.To(stripKeysFromPrefs(*n.Prefs))
+ }
if n.Version == "" {
n.Version = version.Long
}
@@ -2594,7 +2585,7 @@ func (b *LocalBackend) setPrefsLockedOnEntry(caller string, newp *ipn.Prefs) ipn
b.authReconfig()
}
- b.notifyPrefs(prefs)
+ b.send(ipn.Notify{Prefs: &prefs})
return prefs
}