ipn/ipnlocal: update ipn.Prefs when there's a change in syspolicy settings

In this PR, we update ipnlocal.NewLocalBackend to subscribe to policy change notifications
and reapply syspolicy settings to the current profile's ipn.Prefs whenever a change occurs.

Updates #12687

Signed-off-by: Nick Khyl <nickk@tailscale.com>

ipn/ipnlocal/local.go

@@ -106,6 +106,7 @@
 	"tailscale.com/util/rands"
 	"tailscale.com/util/set"
 	"tailscale.com/util/syspolicy"
+	"tailscale.com/util/syspolicy/rsop"
 	"tailscale.com/util/systemd"
 	"tailscale.com/util/testenv"
 	"tailscale.com/util/uniq"
@@ -178,27 +179,28 @@ type watchSession struct {
 // state machine generates events back out to zero or more components.
 type LocalBackend struct {
 	// Elements that are thread-safe or constant after construction.
-	ctx                   context.Context    // canceled by Close
+	ctx                      context.Context    // canceled by Close
-	ctxCancel             context.CancelFunc // cancels ctx
+	ctxCancel                context.CancelFunc // cancels ctx
-	logf                  logger.Logf        // general logging
+	logf                     logger.Logf        // general logging
-	keyLogf               logger.Logf        // for printing list of peers on change
+	keyLogf                  logger.Logf        // for printing list of peers on change
-	statsLogf             logger.Logf        // for printing peers stats on change
+	statsLogf                logger.Logf        // for printing peers stats on change
-	sys                   *tsd.System
+	sys                      *tsd.System
-	health                *health.Tracker // always non-nil
+	health                   *health.Tracker // always non-nil
-	metrics               metrics
+	metrics                  metrics
-	e                     wgengine.Engine // non-nil; TODO(bradfitz): remove; use sys
+	e                        wgengine.Engine // non-nil; TODO(bradfitz): remove; use sys
-	store                 ipn.StateStore  // non-nil; TODO(bradfitz): remove; use sys
+	store                    ipn.StateStore  // non-nil; TODO(bradfitz): remove; use sys
-	dialer                *tsdial.Dialer  // non-nil; TODO(bradfitz): remove; use sys
+	dialer                   *tsdial.Dialer  // non-nil; TODO(bradfitz): remove; use sys
-	pushDeviceToken       syncs.AtomicValue[string]
+	pushDeviceToken          syncs.AtomicValue[string]
-	backendLogID          logid.PublicID
+	backendLogID             logid.PublicID
-	unregisterNetMon      func()
+	unregisterNetMon         func()
-	unregisterHealthWatch func()
+	unregisterHealthWatch    func()
-	portpoll              *portlist.Poller // may be nil
+	unregisterSysPolicyWatch func()
-	portpollOnce          sync.Once        // guards starting readPoller
+	portpoll                 *portlist.Poller // may be nil
-	varRoot               string           // or empty if SetVarRoot never called
+	portpollOnce             sync.Once        // guards starting readPoller
-	logFlushFunc          func()           // or nil if SetLogFlusher wasn't called
+	varRoot                  string           // or empty if SetVarRoot never called
-	em                    *expiryManager   // non-nil
+	logFlushFunc             func()           // or nil if SetLogFlusher wasn't called
-	sshAtomicBool         atomic.Bool
+	em                       *expiryManager   // non-nil
+	sshAtomicBool            atomic.Bool
 	// webClientAtomicBool controls whether the web client is running. This should
 	// be true unless the disable-web-client node attribute has been set.
 	webClientAtomicBool atomic.Bool
@@ -410,7 +412,7 @@ type metrics struct {
 // but is not actually running.
 //
 // If dialer is nil, a new one is made.
-func NewLocalBackend(logf logger.Logf, logID logid.PublicID, sys *tsd.System, loginFlags controlclient.LoginFlags) (*LocalBackend, error) {
+func NewLocalBackend(logf logger.Logf, logID logid.PublicID, sys *tsd.System, loginFlags controlclient.LoginFlags) (_ *LocalBackend, err error) {
 	e := sys.Engine.Get()
 	store := sys.StateStore.Get()
 	dialer := sys.Dialer.Get()
@@ -485,6 +487,15 @@ func NewLocalBackend(logf logger.Logf, logID logid.PublicID, sys *tsd.System, lo
 		}
 	}
 
+	if b.unregisterSysPolicyWatch, err = b.registerSysPolicyWatch(); err != nil {
+		return nil, err
+	}
+	defer func() {
+		if err != nil {
+			b.unregisterSysPolicyWatch()
+		}
+	}()
+
 	netMon := sys.NetMon.Get()
 	b.sockstatLogger, err = sockstatlog.NewLogger(logpolicy.LogsDir(logf), logf, logID, netMon, sys.HealthTracker())
 	if err != nil {
@@ -981,6 +992,7 @@ func (b *LocalBackend) Shutdown() {
 
 	b.unregisterNetMon()
 	b.unregisterHealthWatch()
+	b.unregisterSysPolicyWatch()
 	if cc != nil {
 		cc.Shutdown()
 	}
@@ -1703,6 +1715,40 @@ func applySysPolicy(prefs *ipn.Prefs, lastSuggestedExitNode tailcfg.StableNodeID
 	return anyChange
 }
 
+// registerSysPolicyWatch subscribes to syspolicy change notifications
+// and immediately applies the effective syspolicy settings to the current profile.
+func (b *LocalBackend) registerSysPolicyWatch() (unregister func(), err error) {
+	if unregister, err = syspolicy.RegisterChangeCallback(b.sysPolicyChanged); err != nil {
+		return nil, fmt.Errorf("syspolicy: LocalBacked failed to register policy change callback: %v", err)
+	}
+	if prefs, anyChange := b.applySysPolicy(); anyChange {
+		b.logf("syspolicy: changed initial profile prefs: %v", prefs.Pretty())
+	}
+	return unregister, nil
+}
+
+// applySysPolicy overwrites the current profile's preferences with policies
+// that may be configured by the system administrator in an OS-specific way.
+//
+// b.mu must not be held.
+func (b *LocalBackend) applySysPolicy() (_ ipn.PrefsView, anyChange bool) {
+	unlock := b.lockAndGetUnlock()
+	prefs := b.pm.CurrentPrefs().AsStruct()
+	if !applySysPolicy(prefs, b.lastSuggestedExitNode) {
+		unlock.UnlockEarly()
+		return prefs.View(), false
+	}
+	return b.setPrefsLockedOnEntry(prefs, unlock), true
+}
+
+// sysPolicyChanged is a callback triggered by syspolicy when it detects
+// a change in one or more syspolicy settings.
+func (b *LocalBackend) sysPolicyChanged(*rsop.PolicyChange) {
+	if prefs, anyChange := b.applySysPolicy(); anyChange {
+		b.logf("syspolicy: changed profile prefs: %v", prefs.Pretty())
+	}
+}
+
 var _ controlclient.NetmapDeltaUpdater = (*LocalBackend)(nil)
 
 // UpdateNetmapDelta implements controlclient.NetmapDeltaUpdater.
@@ -3889,10 +3935,14 @@ func (b *LocalBackend) setPrefsLockedOnEntry(newp *ipn.Prefs, unlock unlockOnce)
 	}
 
 	prefs := newp.View()
-	if err := b.pm.SetPrefs(prefs, ipn.NetworkProfile{
+	np := b.pm.CurrentProfile().NetworkProfile
-		MagicDNSName: b.netMap.MagicDNSSuffix(),
+	if netMap != nil {
-		DomainName:   b.netMap.DomainName(),
+		np = ipn.NetworkProfile{
-	}); err != nil {
+			MagicDNSName: b.netMap.MagicDNSSuffix(),
+			DomainName:   b.netMap.DomainName(),
+		}
+	}
+	if err := b.pm.SetPrefs(prefs, np); err != nil {
 		b.logf("failed to save new controlclient state: %v", err)
 	}
 

ipn/ipnlocal/local_test.go

@@ -4562,3 +4562,126 @@ func TestGetVIPServices(t *testing.T) {
 		})
 	}
 }
+
+func TestUpdatePrefsOnSysPolicyChange(t *testing.T) {
+	const enableLogging = false
+
+	type fieldChange struct {
+		name string
+		want any
+	}
+
+	wantPrefsChanges := func(want ...fieldChange) *wantedNotification {
+		return &wantedNotification{
+			name: "Prefs",
+			cond: func(t testing.TB, actor ipnauth.Actor, n *ipn.Notify) bool {
+				if n.Prefs != nil {
+					prefs := reflect.Indirect(reflect.ValueOf(n.Prefs.AsStruct()))
+					for _, f := range want {
+						got := prefs.FieldByName(f.name).Interface()
+						if !reflect.DeepEqual(got, f.want) {
+							t.Errorf("%v: got %v; want %v", f.name, got, f.want)
+						}
+					}
+				}
+				return n.Prefs != nil
+			},
+		}
+	}
+
+	unexpectedPrefsChange := func(t testing.TB, _ ipnauth.Actor, n *ipn.Notify) bool {
+		if n.Prefs != nil {
+			t.Errorf("Unexpected Prefs: %v", n.Prefs.Pretty())
+			return true
+		}
+		return false
+	}
+
+	tests := []struct {
+		name           string
+		initialPrefs   *ipn.Prefs
+		stringSettings []source.TestSetting[string]
+		want           *wantedNotification
+	}{
+		{
+			name:           "ShieldsUp/True",
+			stringSettings: []source.TestSetting[string]{source.TestSettingOf(syspolicy.EnableIncomingConnections, "never")},
+			want:           wantPrefsChanges(fieldChange{"ShieldsUp", true}),
+		},
+		{
+			name:           "ShieldsUp/False",
+			initialPrefs:   &ipn.Prefs{ShieldsUp: true},
+			stringSettings: []source.TestSetting[string]{source.TestSettingOf(syspolicy.EnableIncomingConnections, "always")},
+			want:           wantPrefsChanges(fieldChange{"ShieldsUp", false}),
+		},
+		{
+			name:           "ExitNodeID",
+			stringSettings: []source.TestSetting[string]{source.TestSettingOf(syspolicy.ExitNodeID, "foo")},
+			want:           wantPrefsChanges(fieldChange{"ExitNodeID", tailcfg.StableNodeID("foo")}),
+		},
+		{
+			name:           "EnableRunExitNode",
+			stringSettings: []source.TestSetting[string]{source.TestSettingOf(syspolicy.EnableRunExitNode, "always")},
+			want:           wantPrefsChanges(fieldChange{"AdvertiseRoutes", []netip.Prefix{tsaddr.AllIPv4(), tsaddr.AllIPv6()}}),
+		},
+		{
+			name: "Multiple",
+			initialPrefs: &ipn.Prefs{
+				ExitNodeAllowLANAccess: true,
+			},
+			stringSettings: []source.TestSetting[string]{
+				source.TestSettingOf(syspolicy.EnableServerMode, "always"),
+				source.TestSettingOf(syspolicy.ExitNodeAllowLANAccess, "never"),
+				source.TestSettingOf(syspolicy.ExitNodeIP, "127.0.0.1"),
+			},
+			want: wantPrefsChanges(
+				fieldChange{"ForceDaemon", true},
+				fieldChange{"ExitNodeAllowLANAccess", false},
+				fieldChange{"ExitNodeIP", netip.MustParseAddr("127.0.0.1")},
+			),
+		},
+		{
+			name: "NoChange",
+			initialPrefs: &ipn.Prefs{
+				CorpDNS:         true,
+				ExitNodeID:      "foo",
+				AdvertiseRoutes: []netip.Prefix{tsaddr.AllIPv4(), tsaddr.AllIPv6()},
+			},
+			stringSettings: []source.TestSetting[string]{
+				source.TestSettingOf(syspolicy.EnableTailscaleDNS, "always"),
+				source.TestSettingOf(syspolicy.ExitNodeID, "foo"),
+				source.TestSettingOf(syspolicy.EnableRunExitNode, "always"),
+			},
+			want: nil, // syspolicy settings match the preferences; no change notification is expected.
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			syspolicy.RegisterWellKnownSettingsForTest(t)
+			store := source.NewTestStoreOf[string](t)
+			syspolicy.MustRegisterStoreForTest(t, "TestSource", setting.DeviceScope, store)
+
+			lb := newLocalBackendWithTestControl(t, enableLogging, func(tb testing.TB, opts controlclient.Options) controlclient.Client {
+				return newClient(tb, opts)
+			})
+			if tt.initialPrefs != nil {
+				lb.SetPrefsForTest(tt.initialPrefs)
+			}
+			if err := lb.Start(ipn.Options{}); err != nil {
+				t.Fatalf("(*LocalBackend).Start(): %v", err)
+			}
+
+			nw := newNotificationWatcher(t, lb, &ipnauth.TestActor{})
+			if tt.want != nil {
+				nw.watch(0, []wantedNotification{*tt.want})
+			} else {
+				nw.watch(0, nil, unexpectedPrefsChange)
+			}
+
+			store.SetStrings(tt.stringSettings...)
+
+			nw.check()
+		})
+	}
+}