util/syspolicy: finish plumbing policyclient, add feature/syspolicy, move global impl

This is step 4 of making syspolicy a build-time feature.

This adds a policyclient.Get() accessor to return the correct
implementation to use: either the real one, or the no-op one. (A third
type, a static one for testing, also exists, so in general a
policyclient.Client should be plumbed around and not always fetched
via policyclient.Get whenever possible, especially if tests need to use
alternate syspolicy)

Updates #16998
Updates #12614

Change-Id: Iaf19670744a596d5918acfa744f5db4564272978
Signed-off-by: Brad Fitzpatrick <[email protected]>

client/web/auth.go

@@ -192,7 +192,7 @@ func (s *Server) controlSupportsCheckMode(ctx context.Context) bool {
 	if err != nil {
 		return true
 	}
-	controlURL, err := url.Parse(prefs.ControlURLOrDefault())
+	controlURL, err := url.Parse(prefs.ControlURLOrDefault(s.polc))
 	if err != nil {
 		return true
 	}

client/web/web.go

@@ -5,6 +5,7 @@
 package web
 
 import (
+	"cmp"
 	"context"
 	"encoding/json"
 	"errors"
@@ -36,6 +37,7 @@ import (
 	"tailscale.com/types/logger"
 	"tailscale.com/types/views"
 	"tailscale.com/util/httpm"
+	"tailscale.com/util/syspolicy/policyclient"
 	"tailscale.com/version"
 	"tailscale.com/version/distro"
 )
@@ -49,6 +51,7 @@ type Server struct {
 	mode ServerMode
 
 	logf    logger.Logf
+	polc    policyclient.Client // must be non-nil
 	lc      *local.Client
 	timeNow func() time.Time
 
@@ -139,9 +142,13 @@ type ServerOpts struct {
 	TimeNow func() time.Time
 
 	// Logf optionally provides a logger function.
-	// log.Printf is used as default.
+	// If nil, log.Printf is used as default.
 	Logf logger.Logf
 
+	// PolicyClient, if non-nil, will be used to fetch policy settings.
+	// If nil, the default policy client will be used.
+	PolicyClient policyclient.Client
+
 	// The following two fields are required and used exclusively
 	// in ManageServerMode to facilitate the control server login
 	// check step for authorizing browser sessions.
@@ -178,6 +185,7 @@ func NewServer(opts ServerOpts) (s *Server, err error) {
 	}
 	s = &Server{
 		mode:           opts.Mode,
+		polc:           cmp.Or(opts.PolicyClient, policyclient.Get()),
 		logf:           opts.Logf,
 		devMode:        envknob.Bool("TS_DEBUG_WEB_CLIENT_DEV"),
 		lc:             opts.LocalClient,
@@ -950,7 +958,7 @@ func (s *Server) serveGetNodeData(w http.ResponseWriter, r *http.Request) {
 		UnraidToken:      os.Getenv("UNRAID_CSRF_TOKEN"),
 		RunningSSHServer: prefs.RunSSH,
 		URLPrefix:        strings.TrimSuffix(s.pathPrefix, "/"),
-		ControlAdminURL:  prefs.AdminPageURL(),
+		ControlAdminURL:  prefs.AdminPageURL(s.polc),
 		LicensesURL:      licenses.LicensesURL(),
 		Features:         availableFeatures(),
 

client/web/web_test.go

@@ -28,6 +28,7 @@ import (
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/views"
 	"tailscale.com/util/httpm"
+	"tailscale.com/util/syspolicy/policyclient"
 )
 
 func TestQnapAuthnURL(t *testing.T) {
@@ -576,6 +577,7 @@ func TestServeAuth(t *testing.T) {
 		timeNow:     func() time.Time { return timeNow },
 		newAuthURL:  mockNewAuthURL,
 		waitAuthURL: mockWaitAuthURL,
+		polc:        policyclient.NoPolicyClient{},
 	}
 
 	successCookie := "ts-cookie-success"

cmd/derper/depaware.txt

@@ -170,21 +170,15 @@ tailscale.com/cmd/derper dependencies: (generated by github.com/tailscale/depawa
         tailscale.com/util/set                                       from tailscale.com/derp+
         tailscale.com/util/singleflight                              from tailscale.com/net/dnscache
         tailscale.com/util/slicesx                                   from tailscale.com/cmd/derper+
-        tailscale.com/util/syspolicy                                 from tailscale.com/ipn
-        tailscale.com/util/syspolicy/internal                        from tailscale.com/util/syspolicy/setting+
-        tailscale.com/util/syspolicy/internal/loggerx                from tailscale.com/util/syspolicy/internal/metrics+
-        tailscale.com/util/syspolicy/internal/metrics                from tailscale.com/util/syspolicy/source
+        tailscale.com/util/syspolicy/internal                        from tailscale.com/util/syspolicy/setting
         tailscale.com/util/syspolicy/pkey                            from tailscale.com/ipn+
-        tailscale.com/util/syspolicy/policyclient                    from tailscale.com/util/syspolicy/rsop
-        tailscale.com/util/syspolicy/ptype                           from tailscale.com/util/syspolicy+
-        tailscale.com/util/syspolicy/rsop                            from tailscale.com/util/syspolicy
-        tailscale.com/util/syspolicy/setting                         from tailscale.com/util/syspolicy+
-        tailscale.com/util/syspolicy/source                          from tailscale.com/util/syspolicy+
-        tailscale.com/util/testenv                                   from tailscale.com/util/syspolicy+
+        tailscale.com/util/syspolicy/policyclient                    from tailscale.com/ipn
+        tailscale.com/util/syspolicy/ptype                           from tailscale.com/util/syspolicy/policyclient+
+        tailscale.com/util/syspolicy/setting                         from tailscale.com/client/local
+        tailscale.com/util/testenv                                   from tailscale.com/net/bakedroots+
         tailscale.com/util/usermetric                                from tailscale.com/health
         tailscale.com/util/vizerror                                  from tailscale.com/tailcfg+
    W 💣 tailscale.com/util/winutil                                   from tailscale.com/hostinfo+
-   W 💣 tailscale.com/util/winutil/gp                                from tailscale.com/util/syspolicy/source
    W 💣 tailscale.com/util/winutil/winenv                            from tailscale.com/hostinfo+
         tailscale.com/version                                        from tailscale.com/derp+
         tailscale.com/version/distro                                 from tailscale.com/envknob+
@@ -205,7 +199,7 @@ tailscale.com/cmd/derper dependencies: (generated by github.com/tailscale/depawa
         golang.org/x/crypto/nacl/secretbox                           from golang.org/x/crypto/nacl/box
         golang.org/x/crypto/salsa20/salsa                            from golang.org/x/crypto/nacl/box+
         golang.org/x/exp/constraints                                 from tailscale.com/util/winutil+
-        golang.org/x/exp/maps                                        from tailscale.com/util/syspolicy/setting+
+        golang.org/x/exp/maps                                        from tailscale.com/util/syspolicy/setting
    L    golang.org/x/net/bpf                                         from github.com/mdlayher/netlink+
         golang.org/x/net/dns/dnsmessage                              from net+
         golang.org/x/net/http/httpguts                               from net/http+
@@ -393,7 +387,7 @@ tailscale.com/cmd/derper dependencies: (generated by github.com/tailscale/depawa
         os                                                           from crypto/internal/sysrand+
         os/exec                                                      from github.com/coreos/go-iptables/iptables+
         os/signal                                                    from tailscale.com/cmd/derper
-   W    os/user                                                      from tailscale.com/util/winutil+
+   W    os/user                                                      from tailscale.com/util/winutil
         path                                                         from github.com/prometheus/client_golang/prometheus/internal+
         path/filepath                                                from crypto/x509+
         reflect                                                      from crypto/x509+

cmd/k8s-operator/depaware.txt

@@ -798,6 +798,7 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/
         tailscale.com/envknob                                        from tailscale.com/client/local+
         tailscale.com/envknob/featureknob                            from tailscale.com/client/web+
         tailscale.com/feature                                        from tailscale.com/ipn/ipnext+
+        tailscale.com/feature/syspolicy                              from tailscale.com/logpolicy
         tailscale.com/health                                         from tailscale.com/control/controlclient+
         tailscale.com/health/healthmsg                               from tailscale.com/ipn/ipnlocal+
         tailscale.com/hostinfo                                       from tailscale.com/client/web+
@@ -951,7 +952,7 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/
         tailscale.com/util/set                                       from tailscale.com/cmd/k8s-operator+
         tailscale.com/util/singleflight                              from tailscale.com/control/controlclient+
         tailscale.com/util/slicesx                                   from tailscale.com/appc+
-        tailscale.com/util/syspolicy                                 from tailscale.com/ipn+
+        tailscale.com/util/syspolicy                                 from tailscale.com/feature/syspolicy
         tailscale.com/util/syspolicy/internal                        from tailscale.com/util/syspolicy/setting+
         tailscale.com/util/syspolicy/internal/loggerx                from tailscale.com/util/syspolicy/internal/metrics+
         tailscale.com/util/syspolicy/internal/metrics                from tailscale.com/util/syspolicy/source

cmd/tailscale/cli/maybe_syspolicy.go

@@ -0,0 +1,8 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+//go:build !ts_omit_syspolicy
+
+package cli
+
+import _ "tailscale.com/feature/syspolicy"

cmd/tailscale/cli/up.go

@@ -39,6 +39,7 @@ import (
 	"tailscale.com/types/preftype"
 	"tailscale.com/types/views"
 	"tailscale.com/util/dnsname"
+	"tailscale.com/util/syspolicy/policyclient"
 	"tailscale.com/version/distro"
 )
 
@@ -609,7 +610,7 @@ func runUp(ctx context.Context, cmd string, args []string, upArgs upArgsT) (retE
 					if env.upArgs.json {
 						printUpDoneJSON(ipn.NeedsMachineAuth, "")
 					} else {
-						fmt.Fprintf(Stderr, "\nTo approve your machine, visit (as admin):\n\n\t%s\n\n", prefs.AdminPageURL())
+						fmt.Fprintf(Stderr, "\nTo approve your machine, visit (as admin):\n\n\t%s\n\n", prefs.AdminPageURL(policyclient.Get()))
 					}
 				case ipn.Running:
 					// Done full authentication process

cmd/tailscale/depaware.txt

@@ -106,6 +106,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep
         tailscale.com/envknob/featureknob                            from tailscale.com/client/web
         tailscale.com/feature                                        from tailscale.com/tsweb
         tailscale.com/feature/capture/dissector                      from tailscale.com/cmd/tailscale/cli
+        tailscale.com/feature/syspolicy                              from tailscale.com/cmd/tailscale/cli
         tailscale.com/health                                         from tailscale.com/net/tlsdial+
         tailscale.com/health/healthmsg                               from tailscale.com/cmd/tailscale/cli
         tailscale.com/hostinfo                                       from tailscale.com/client/web+
@@ -191,15 +192,15 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep
         tailscale.com/util/singleflight                              from tailscale.com/net/dnscache+
         tailscale.com/util/slicesx                                   from tailscale.com/net/dns/recursive+
    L    tailscale.com/util/stringsx                                  from tailscale.com/client/systray
-        tailscale.com/util/syspolicy                                 from tailscale.com/ipn
+        tailscale.com/util/syspolicy                                 from tailscale.com/feature/syspolicy
         tailscale.com/util/syspolicy/internal                        from tailscale.com/util/syspolicy/setting+
-        tailscale.com/util/syspolicy/internal/loggerx                from tailscale.com/util/syspolicy/internal/metrics+
+        tailscale.com/util/syspolicy/internal/loggerx                from tailscale.com/util/syspolicy+
         tailscale.com/util/syspolicy/internal/metrics                from tailscale.com/util/syspolicy/source
         tailscale.com/util/syspolicy/pkey                            from tailscale.com/ipn+
-        tailscale.com/util/syspolicy/policyclient                    from tailscale.com/util/syspolicy/rsop
-        tailscale.com/util/syspolicy/ptype                           from tailscale.com/util/syspolicy+
+        tailscale.com/util/syspolicy/policyclient                    from tailscale.com/client/web+
+        tailscale.com/util/syspolicy/ptype                           from tailscale.com/util/syspolicy/policyclient+
         tailscale.com/util/syspolicy/rsop                            from tailscale.com/util/syspolicy
-        tailscale.com/util/syspolicy/setting                         from tailscale.com/util/syspolicy+
+        tailscale.com/util/syspolicy/setting                         from tailscale.com/client/local+
         tailscale.com/util/syspolicy/source                          from tailscale.com/util/syspolicy+
         tailscale.com/util/testenv                                   from tailscale.com/cmd/tailscale/cli+
         tailscale.com/util/truncate                                  from tailscale.com/cmd/tailscale/cli
@@ -228,7 +229,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep
         golang.org/x/crypto/pbkdf2                                   from software.sslmate.com/src/go-pkcs12
         golang.org/x/crypto/salsa20/salsa                            from golang.org/x/crypto/nacl/box+
         golang.org/x/exp/constraints                                 from github.com/dblohm7/wingoes/pe+
-        golang.org/x/exp/maps                                        from tailscale.com/util/syspolicy/internal/metrics+
+        golang.org/x/exp/maps                                        from tailscale.com/util/syspolicy/setting+
    L    golang.org/x/image/draw                                      from github.com/fogleman/gg
    L    golang.org/x/image/font                                      from github.com/fogleman/gg+
    L    golang.org/x/image/font/basicfont                            from github.com/fogleman/gg

cmd/tailscaled/depaware.txt

@@ -276,6 +276,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/feature/capture                                from tailscale.com/feature/condregister
         tailscale.com/feature/condregister                           from tailscale.com/cmd/tailscaled
         tailscale.com/feature/relayserver                            from tailscale.com/feature/condregister
+        tailscale.com/feature/syspolicy                              from tailscale.com/feature/condregister+
         tailscale.com/feature/taildrop                               from tailscale.com/feature/condregister
    L    tailscale.com/feature/tap                                    from tailscale.com/feature/condregister
         tailscale.com/feature/tpm                                    from tailscale.com/feature/condregister
@@ -428,7 +429,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/util/set                                       from tailscale.com/derp+
         tailscale.com/util/singleflight                              from tailscale.com/control/controlclient+
         tailscale.com/util/slicesx                                   from tailscale.com/net/dns/recursive+
-        tailscale.com/util/syspolicy                                 from tailscale.com/cmd/tailscaled+
+        tailscale.com/util/syspolicy                                 from tailscale.com/feature/syspolicy
         tailscale.com/util/syspolicy/internal                        from tailscale.com/util/syspolicy/setting+
         tailscale.com/util/syspolicy/internal/loggerx                from tailscale.com/util/syspolicy/internal/metrics+
         tailscale.com/util/syspolicy/internal/metrics                from tailscale.com/util/syspolicy/source

cmd/tailscaled/tailscaled.go

@@ -64,8 +64,8 @@ import (
 	"tailscale.com/util/clientmetric"
 	"tailscale.com/util/multierr"
 	"tailscale.com/util/osshare"
-	"tailscale.com/util/syspolicy"
 	"tailscale.com/util/syspolicy/pkey"
+	"tailscale.com/util/syspolicy/policyclient"
 	"tailscale.com/version"
 	"tailscale.com/version/distro"
 	"tailscale.com/wgengine"
@@ -773,7 +773,7 @@ func tryEngine(logf logger.Logf, sys *tsd.System, name string) (onlyNetstack boo
 			// configuration being unavailable (from the noop
 			// manager). More in Issue 4017.
 			// TODO(bradfitz): add a Synology-specific DNS manager.
-			conf.DNS, err = dns.NewOSConfigurator(logf, sys.HealthTracker(), sys.ControlKnobs(), "") // empty interface name
+			conf.DNS, err = dns.NewOSConfigurator(logf, sys.HealthTracker(), sys.PolicyClientOrDefault(), sys.ControlKnobs(), "") // empty interface name
 			if err != nil {
 				return false, fmt.Errorf("dns.NewOSConfigurator: %w", err)
 			}
@@ -807,7 +807,7 @@ func tryEngine(logf logger.Logf, sys *tsd.System, name string) (onlyNetstack boo
 			return false, fmt.Errorf("creating router: %w", err)
 		}
 
-		d, err := dns.NewOSConfigurator(logf, sys.HealthTracker(), sys.ControlKnobs(), devName)
+		d, err := dns.NewOSConfigurator(logf, sys.HealthTracker(), sys.PolicyClientOrDefault(), sys.ControlKnobs(), devName)
 		if err != nil {
 			dev.Close()
 			r.Close()
@@ -1012,6 +1012,6 @@ func defaultEncryptState() bool {
 		// (plan9/FreeBSD/etc).
 		return false
 	}
-	v, _ := syspolicy.GetBoolean(pkey.EncryptState, false)
+	v, _ := policyclient.Get().GetBoolean(pkey.EncryptState, false)
 	return v
 }

cmd/tailscaled/tailscaled_windows.go

@@ -55,8 +55,8 @@ import (
 	"tailscale.com/types/logger"
 	"tailscale.com/types/logid"
 	"tailscale.com/util/osdiag"
-	"tailscale.com/util/syspolicy"
 	"tailscale.com/util/syspolicy/pkey"
+	"tailscale.com/util/syspolicy/policyclient"
 	"tailscale.com/util/winutil"
 	"tailscale.com/util/winutil/gp"
 	"tailscale.com/version"
@@ -156,7 +156,7 @@ func runWindowsService(pol *logpolicy.Policy) error {
 
 	if syslog, err := eventlog.Open(serviceName); err == nil {
 		syslogf = func(format string, args ...any) {
-			if logSCMInteractions, _ := syspolicy.GetBoolean(pkey.LogSCMInteractions, false); logSCMInteractions {
+			if logSCMInteractions, _ := policyclient.Get().GetBoolean(pkey.LogSCMInteractions, false); logSCMInteractions {
 				syslog.Info(0, fmt.Sprintf(format, args...))
 			}
 		}
@@ -390,7 +390,7 @@ func handleSessionChange(chgRequest svc.ChangeRequest) {
 	if chgRequest.Cmd != svc.SessionChange || chgRequest.EventType != windows.WTS_SESSION_UNLOCK {
 		return
 	}
-	if flushDNSOnSessionUnlock, _ := syspolicy.GetBoolean(pkey.FlushDNSOnSessionUnlock, false); flushDNSOnSessionUnlock {
+	if flushDNSOnSessionUnlock, _ := policyclient.Get().GetBoolean(pkey.FlushDNSOnSessionUnlock, false); flushDNSOnSessionUnlock {
 		log.Printf("Received WTS_SESSION_UNLOCK event, initiating DNS flush.")
 		go func() {
 			err := dns.Flush()

cmd/tsidp/depaware.txt

@@ -240,6 +240,7 @@ tailscale.com/cmd/tsidp dependencies: (generated by github.com/tailscale/depawar
         tailscale.com/envknob                                        from tailscale.com/client/local+
         tailscale.com/envknob/featureknob                            from tailscale.com/client/web+
         tailscale.com/feature                                        from tailscale.com/ipn/ipnext+
+        tailscale.com/feature/syspolicy                              from tailscale.com/logpolicy
         tailscale.com/health                                         from tailscale.com/control/controlclient+
         tailscale.com/health/healthmsg                               from tailscale.com/ipn/ipnlocal+
         tailscale.com/hostinfo                                       from tailscale.com/client/web+
@@ -380,7 +381,7 @@ tailscale.com/cmd/tsidp dependencies: (generated by github.com/tailscale/depawar
         tailscale.com/util/set                                       from tailscale.com/control/controlclient+
         tailscale.com/util/singleflight                              from tailscale.com/control/controlclient+
         tailscale.com/util/slicesx                                   from tailscale.com/appc+
-        tailscale.com/util/syspolicy                                 from tailscale.com/ipn+
+        tailscale.com/util/syspolicy                                 from tailscale.com/feature/syspolicy
         tailscale.com/util/syspolicy/internal                        from tailscale.com/util/syspolicy+
         tailscale.com/util/syspolicy/internal/loggerx                from tailscale.com/util/syspolicy+
         tailscale.com/util/syspolicy/internal/metrics                from tailscale.com/util/syspolicy/source

feature/condregister/maybe_syspolicy.go

@@ -0,0 +1,8 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+//go:build !ts_omit_syspolicy
+
+package condregister
+
+import _ "tailscale.com/feature/syspolicy"

feature/syspolicy/syspolicy.go

@@ -0,0 +1,7 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+// Package syspolicy provides an interface for system-wide policy management.
+package syspolicy
+
+import _ "tailscale.com/util/syspolicy" // for its registration side effects

ipn/desktop/extension.go

@@ -18,8 +18,8 @@ import (
 	"tailscale.com/ipn"
 	"tailscale.com/ipn/ipnext"
 	"tailscale.com/types/logger"
-	"tailscale.com/util/syspolicy"
 	"tailscale.com/util/syspolicy/pkey"
+	"tailscale.com/util/syspolicy/policyclient"
 )
 
 // featureName is the name of the feature implemented by this package.
@@ -136,7 +136,7 @@ func (e *desktopSessionsExt) getBackgroundProfile(profiles ipnext.ProfileStore)
 	e.mu.Lock()
 	defer e.mu.Unlock()
 
-	if alwaysOn, _ := syspolicy.GetBoolean(pkey.AlwaysOn, false); !alwaysOn {
+	if alwaysOn, _ := policyclient.Get().GetBoolean(pkey.AlwaysOn, false); !alwaysOn {
 		// If the Always-On mode is disabled, there's no background profile
 		// as far as the desktop session extension is concerned.
 		return ipn.LoginProfileView{}

ipn/ipnauth/policy.go

@@ -10,8 +10,8 @@ import (
 	"tailscale.com/client/tailscale/apitype"
 	"tailscale.com/ipn"
 	"tailscale.com/tailcfg"
-	"tailscale.com/util/syspolicy"
 	"tailscale.com/util/syspolicy/pkey"
+	"tailscale.com/util/syspolicy/policyclient"
 )
 
 type actorWithPolicyChecks struct{ Actor }
@@ -51,10 +51,10 @@ func (a actorWithPolicyChecks) CheckProfileAccess(profile ipn.LoginProfileView,
 // TODO(nickkhyl): unexport it when we move [ipn.Actor] implementations from [ipnserver]
 // and corp to this package.
 func CheckDisconnectPolicy(actor Actor, profile ipn.LoginProfileView, reason string, auditFn AuditLogFunc) error {
-	if alwaysOn, _ := syspolicy.GetBoolean(pkey.AlwaysOn, false); !alwaysOn {
+	if alwaysOn, _ := policyclient.Get().GetBoolean(pkey.AlwaysOn, false); !alwaysOn {
 		return nil
 	}
-	if allowWithReason, _ := syspolicy.GetBoolean(pkey.AlwaysOnOverrideWithReason, false); !allowWithReason {
+	if allowWithReason, _ := policyclient.Get().GetBoolean(pkey.AlwaysOnOverrideWithReason, false); !allowWithReason {
 		return errors.New("disconnect not allowed: always-on mode is enabled")
 	}
 	if reason == "" {

ipn/ipnlocal/c2n.go

@@ -30,6 +30,7 @@ import (
 	"tailscale.com/util/goroutines"
 	"tailscale.com/util/set"
 	"tailscale.com/util/syspolicy/pkey"
+	"tailscale.com/util/syspolicy/ptype"
 	"tailscale.com/version"
 	"tailscale.com/version/distro"
 )
@@ -342,7 +343,7 @@ func handleC2NPostureIdentityGet(b *LocalBackend, w http.ResponseWriter, r *http
 	// this will first check syspolicy, MDM settings like Registry
 	// on Windows or defaults on macOS. If they are not set, it falls
 	// back to the cli-flag, `--posture-checking`.
-	choice, err := b.polc.GetPreferenceOption(pkey.PostureChecking)
+	choice, err := b.polc.GetPreferenceOption(pkey.PostureChecking, ptype.ShowChoiceByPolicy)
 	if err != nil {
 		b.logf(
 			"c2n: failed to read PostureChecking from syspolicy, returning default from CLI: %s; got error: %s",

ipn/ipnlocal/local.go

@@ -109,6 +109,7 @@ import (
 	"tailscale.com/util/slicesx"
 	"tailscale.com/util/syspolicy/pkey"
 	"tailscale.com/util/syspolicy/policyclient"
+	"tailscale.com/util/syspolicy/ptype"
 	"tailscale.com/util/systemd"
 	"tailscale.com/util/testenv"
 	"tailscale.com/util/usermetric"
@@ -1610,7 +1611,7 @@ func (b *LocalBackend) SetControlClientStatus(c controlclient.Client, st control
 		// future "tailscale up" to start checking for
 		// implicit setting reverts, which it doesn't do when
 		// ControlURL is blank.
-		prefs.ControlURL = prefs.ControlURLOrDefault()
+		prefs.ControlURL = prefs.ControlURLOrDefault(b.polc)
 		prefsChanged = true
 	}
 	if st.Persist.Valid() {
@@ -1870,7 +1871,7 @@ func (b *LocalBackend) applySysPolicyLocked(prefs *ipn.Prefs) (anyChange bool) {
 	}
 
 	for _, opt := range preferencePolicies {
-		if po, err := b.polc.GetPreferenceOption(opt.key); err == nil {
+		if po, err := b.polc.GetPreferenceOption(opt.key, ptype.ShowChoiceByPolicy); err == nil {
 			curVal := opt.get(prefs.View())
 			newVal := po.ShouldEnable(curVal)
 			if curVal != newVal {
@@ -2425,7 +2426,7 @@ func (b *LocalBackend) Start(opts ipn.Options) error {
 
 	loggedOut := prefs.LoggedOut()
 
-	serverURL := prefs.ControlURLOrDefault()
+	serverURL := prefs.ControlURLOrDefault(b.polc)
 	if inServerMode := prefs.ForceDaemon(); inServerMode || runtime.GOOS == "windows" {
 		b.logf("Start: serverMode=%v", inServerMode)
 	}
@@ -3498,7 +3499,7 @@ func (b *LocalBackend) validPopBrowserURLLocked(urlStr string) bool {
 	if err != nil {
 		return false
 	}
-	serverURL := b.sanitizedPrefsLocked().ControlURLOrDefault()
+	serverURL := b.sanitizedPrefsLocked().ControlURLOrDefault(b.polc)
 	if ipn.IsLoginServerSynonym(serverURL) {
 		// When connected to the official Tailscale control plane, only allow
 		// URLs from tailscale.com or its subdomains.
@@ -4049,7 +4050,7 @@ func (b *LocalBackend) SwitchToBestProfile(reason string) {
 // but b.mu must held on entry. It is released on exit.
 func (b *LocalBackend) switchToBestProfileLockedOnEntry(reason string, unlock unlockOnce) {
 	defer unlock()
-	oldControlURL := b.pm.CurrentPrefs().ControlURLOrDefault()
+	oldControlURL := b.pm.CurrentPrefs().ControlURLOrDefault(b.polc)
 	profile, background := b.resolveBestProfileLocked()
 	cp, switched, err := b.pm.SwitchToProfile(profile)
 	switch {
@@ -4076,7 +4077,7 @@ func (b *LocalBackend) switchToBestProfileLockedOnEntry(reason string, unlock un
 		return
 	}
 	// As an optimization, only reset the dialPlan if the control URL changed.
-	if newControlURL := b.pm.CurrentPrefs().ControlURLOrDefault(); oldControlURL != newControlURL {
+	if newControlURL := b.pm.CurrentPrefs().ControlURLOrDefault(b.polc); oldControlURL != newControlURL {
 		b.resetDialPlan()
 	}
 	if err := b.resetForProfileChangeLockedOnEntry(unlock); err != nil {
@@ -4250,7 +4251,7 @@ func (b *LocalBackend) isDefaultServerLocked() bool {
 	if !prefs.Valid() {
 		return true // assume true until set otherwise
 	}
-	return prefs.ControlURLOrDefault() == ipn.DefaultControlURL
+	return prefs.ControlURLOrDefault(b.polc) == ipn.DefaultControlURL
 }
 
 var exitNodeMisconfigurationWarnable = health.Register(&health.Warnable{
@@ -5687,7 +5688,7 @@ func (b *LocalBackend) enterStateLockedOnEntry(newState ipn.State, unlock unlock
 	// Some temporary (2024-05-05) debugging code to help us catch
 	// https://github.com/tailscale/tailscale/issues/11962 in the act.
 	if prefs.WantRunning() &&
-		prefs.ControlURLOrDefault() == ipn.DefaultControlURL &&
+		prefs.ControlURLOrDefault(b.polc) == ipn.DefaultControlURL &&
 		envknob.Bool("TS_PANIC_IF_HIT_MAIN_CONTROL") {
 		panic("[unexpected] use of main control server in integration test")
 	}
@@ -7288,13 +7289,13 @@ func (b *LocalBackend) SwitchProfile(profile ipn.ProfileID) error {
 	unlock := b.lockAndGetUnlock()
 	defer unlock()
 
-	oldControlURL := b.pm.CurrentPrefs().ControlURLOrDefault()
+	oldControlURL := b.pm.CurrentPrefs().ControlURLOrDefault(b.polc)
 	if _, changed, err := b.pm.SwitchToProfileByID(profile); !changed || err != nil {
 		return err // nil if we're already on the target profile
 	}
 
 	// As an optimization, only reset the dialPlan if the control URL changed.
-	if newControlURL := b.pm.CurrentPrefs().ControlURLOrDefault(); oldControlURL != newControlURL {
+	if newControlURL := b.pm.CurrentPrefs().ControlURLOrDefault(b.polc); oldControlURL != newControlURL {
 		b.resetDialPlan()
 	}
 

ipn/prefs.go

@@ -28,8 +28,8 @@ import (
 	"tailscale.com/types/preftype"
 	"tailscale.com/types/views"
 	"tailscale.com/util/dnsname"
-	"tailscale.com/util/syspolicy"
 	"tailscale.com/util/syspolicy/pkey"
+	"tailscale.com/util/syspolicy/policyclient"
 	"tailscale.com/version"
 )
 
@@ -718,16 +718,16 @@ func NewPrefs() *Prefs {
 //
 // If not configured, or if the configured value is a legacy name equivalent to
 // the default, then DefaultControlURL is returned instead.
-func (p PrefsView) ControlURLOrDefault() string {
-	return p.ж.ControlURLOrDefault()
+func (p PrefsView) ControlURLOrDefault(polc policyclient.Client) string {
+	return p.ж.ControlURLOrDefault(polc)
 }
 
 // ControlURLOrDefault returns the coordination server's URL base.
 //
 // If not configured, or if the configured value is a legacy name equivalent to
 // the default, then DefaultControlURL is returned instead.
-func (p *Prefs) ControlURLOrDefault() string {
-	controlURL, err := syspolicy.GetString(pkey.ControlURL, p.ControlURL)
+func (p *Prefs) ControlURLOrDefault(polc policyclient.Client) string {
+	controlURL, err := polc.GetString(pkey.ControlURL, p.ControlURL)
 	if err != nil {
 		controlURL = p.ControlURL
 	}
@@ -756,11 +756,11 @@ func (p *Prefs) DefaultRouteAll(goos string) bool {
 }
 
 // AdminPageURL returns the admin web site URL for the current ControlURL.
-func (p PrefsView) AdminPageURL() string { return p.ж.AdminPageURL() }
+func (p PrefsView) AdminPageURL(polc policyclient.Client) string { return p.ж.AdminPageURL(polc) }
 
 // AdminPageURL returns the admin web site URL for the current ControlURL.
-func (p *Prefs) AdminPageURL() string {
-	url := p.ControlURLOrDefault()
+func (p *Prefs) AdminPageURL(polc policyclient.Client) string {
+	url := p.ControlURLOrDefault(polc)
 	if IsLoginServerSynonym(url) {
 		// TODO(crawshaw): In future release, make this https://console.tailscale.com
 		url = "https://login.tailscale.com"

ipn/prefs_test.go

@@ -23,6 +23,7 @@ import (
 	"tailscale.com/types/opt"
 	"tailscale.com/types/persist"
 	"tailscale.com/types/preftype"
+	"tailscale.com/util/syspolicy/policyclient"
 )
 
 func fieldsOf(t reflect.Type) (fields []string) {
@@ -1032,15 +1033,16 @@ func TestExitNodeIPOfArg(t *testing.T) {
 
 func TestControlURLOrDefault(t *testing.T) {
 	var p Prefs
-	if got, want := p.ControlURLOrDefault(), DefaultControlURL; got != want {
+	polc := policyclient.NoPolicyClient{}
+	if got, want := p.ControlURLOrDefault(polc), DefaultControlURL; got != want {
 		t.Errorf("got %q; want %q", got, want)
 	}
 	p.ControlURL = "http://foo.bar"
-	if got, want := p.ControlURLOrDefault(), "http://foo.bar"; got != want {
+	if got, want := p.ControlURLOrDefault(polc), "http://foo.bar"; got != want {
 		t.Errorf("got %q; want %q", got, want)
 	}
 	p.ControlURL = "https://login.tailscale.com"
-	if got, want := p.ControlURLOrDefault(), DefaultControlURL; got != want {
+	if got, want := p.ControlURLOrDefault(polc), DefaultControlURL; got != want {
 		t.Errorf("got %q; want %q", got, want)
 	}
 }

logpolicy/logpolicy.go

@@ -51,8 +51,8 @@ import (
 	"tailscale.com/util/clientmetric"
 	"tailscale.com/util/must"
 	"tailscale.com/util/racebuild"
-	"tailscale.com/util/syspolicy"
 	"tailscale.com/util/syspolicy/pkey"
+	"tailscale.com/util/syspolicy/policyclient"
 	"tailscale.com/util/testenv"
 	"tailscale.com/version"
 	"tailscale.com/version/distro"
@@ -66,7 +66,7 @@ var getLogTargetOnce struct {
 func getLogTarget() string {
 	getLogTargetOnce.Do(func() {
 		envTarget, _ := os.LookupEnv("TS_LOG_TARGET")
-		getLogTargetOnce.v, _ = syspolicy.GetString(pkey.LogTarget, envTarget)
+		getLogTargetOnce.v, _ = policyclient.Get().GetString(pkey.LogTarget, envTarget)
 	})
 
 	return getLogTargetOnce.v

logpolicy/maybe_syspolicy.go

@@ -0,0 +1,8 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+//go:build !ts_omit_syspolicy
+
+package logpolicy
+
+import _ "tailscale.com/feature/syspolicy"

net/dns/manager.go

@@ -30,6 +30,7 @@ import (
 	"tailscale.com/util/clientmetric"
 	"tailscale.com/util/dnsname"
 	"tailscale.com/util/slicesx"
+	"tailscale.com/util/syspolicy/policyclient"
 )
 
 var (
@@ -576,7 +577,7 @@ func (m *Manager) FlushCaches() error {
 //
 // health must not be nil
 func CleanUp(logf logger.Logf, netMon *netmon.Monitor, health *health.Tracker, interfaceName string) {
-	oscfg, err := NewOSConfigurator(logf, nil, nil, interfaceName)
+	oscfg, err := NewOSConfigurator(logf, health, policyclient.Get(), nil, interfaceName)
 	if err != nil {
 		logf("creating dns cleanup: %v", err)
 		return

net/dns/manager_darwin.go

@@ -14,12 +14,13 @@ import (
 	"tailscale.com/net/tsaddr"
 	"tailscale.com/types/logger"
 	"tailscale.com/util/mak"
+	"tailscale.com/util/syspolicy/policyclient"
 )
 
 // NewOSConfigurator creates a new OS configurator.
 //
 // The health tracker and the knobs may be nil and are ignored on this platform.
-func NewOSConfigurator(logf logger.Logf, _ *health.Tracker, _ *controlknobs.Knobs, ifName string) (OSConfigurator, error) {
+func NewOSConfigurator(logf logger.Logf, _ *health.Tracker, _ policyclient.Client, _ *controlknobs.Knobs, ifName string) (OSConfigurator, error) {
 	return &darwinConfigurator{logf: logf, ifName: ifName}, nil
 }
 

net/dns/manager_default.go

@@ -9,11 +9,12 @@ import (
 	"tailscale.com/control/controlknobs"
 	"tailscale.com/health"
 	"tailscale.com/types/logger"
+	"tailscale.com/util/syspolicy/policyclient"
 )
 
 // NewOSConfigurator creates a new OS configurator.
 //
 // The health tracker and the knobs may be nil and are ignored on this platform.
-func NewOSConfigurator(logger.Logf, *health.Tracker, *controlknobs.Knobs, string) (OSConfigurator, error) {
+func NewOSConfigurator(logger.Logf, *health.Tracker, policyclient.Client, *controlknobs.Knobs, string) (OSConfigurator, error) {
 	return NewNoopManager()
 }

net/dns/manager_freebsd.go

@@ -10,12 +10,13 @@ import (
 	"tailscale.com/control/controlknobs"
 	"tailscale.com/health"
 	"tailscale.com/types/logger"
+	"tailscale.com/util/syspolicy/policyclient"
 )
 
 // NewOSConfigurator creates a new OS configurator.
 //
 // The health tracker may be nil; the knobs may be nil and are ignored on this platform.
-func NewOSConfigurator(logf logger.Logf, health *health.Tracker, _ *controlknobs.Knobs, _ string) (OSConfigurator, error) {
+func NewOSConfigurator(logf logger.Logf, health *health.Tracker, _ policyclient.Client, _ *controlknobs.Knobs, _ string) (OSConfigurator, error) {
 	bs, err := os.ReadFile("/etc/resolv.conf")
 	if os.IsNotExist(err) {
 		return newDirectManager(logf, health), nil

net/dns/manager_linux.go

@@ -22,6 +22,7 @@ import (
 	"tailscale.com/types/logger"
 	"tailscale.com/util/clientmetric"
 	"tailscale.com/util/cmpver"
+	"tailscale.com/util/syspolicy/policyclient"
 	"tailscale.com/version/distro"
 )
 
@@ -38,7 +39,7 @@ var publishOnce sync.Once
 // NewOSConfigurator created a new OS configurator.
 //
 // The health tracker may be nil; the knobs may be nil and are ignored on this platform.
-func NewOSConfigurator(logf logger.Logf, health *health.Tracker, _ *controlknobs.Knobs, interfaceName string) (ret OSConfigurator, err error) {
+func NewOSConfigurator(logf logger.Logf, health *health.Tracker, _ policyclient.Client, _ *controlknobs.Knobs, interfaceName string) (ret OSConfigurator, err error) {
 	if distro.Get() == distro.JetKVM {
 		return NewNoopManager()
 	}

net/dns/manager_openbsd.go

@@ -11,6 +11,7 @@ import (
 	"tailscale.com/control/controlknobs"
 	"tailscale.com/health"
 	"tailscale.com/types/logger"
+	"tailscale.com/util/syspolicy/policyclient"
 )
 
 type kv struct {
@@ -24,7 +25,7 @@ func (kv kv) String() string {
 // NewOSConfigurator created a new OS configurator.
 //
 // The health tracker may be nil; the knobs may be nil and are ignored on this platform.
-func NewOSConfigurator(logf logger.Logf, health *health.Tracker, _ *controlknobs.Knobs, interfaceName string) (OSConfigurator, error) {
+func NewOSConfigurator(logf logger.Logf, health *health.Tracker, _ policyclient.Client, _ *controlknobs.Knobs, interfaceName string) (OSConfigurator, error) {
 	return newOSConfigurator(logf, health, interfaceName,
 		newOSConfigEnv{
 			rcIsResolvd: rcIsResolvd,

net/dns/manager_plan9.go

@@ -21,9 +21,10 @@ import (
 	"tailscale.com/health"
 	"tailscale.com/types/logger"
 	"tailscale.com/util/set"
+	"tailscale.com/util/syspolicy/policyclient"
 )
 
-func NewOSConfigurator(logf logger.Logf, ht *health.Tracker, knobs *controlknobs.Knobs, interfaceName string) (OSConfigurator, error) {
+func NewOSConfigurator(logf logger.Logf, ht *health.Tracker, _ policyclient.Client, knobs *controlknobs.Knobs, interfaceName string) (OSConfigurator, error) {
 	return &plan9DNSManager{
 		logf:  logf,
 		ht:    ht,

net/dns/manager_solaris.go

@@ -7,8 +7,9 @@ import (
 	"tailscale.com/control/controlknobs"
 	"tailscale.com/health"
 	"tailscale.com/types/logger"
+	"tailscale.com/util/syspolicy/policyclient"
 )
 
-func NewOSConfigurator(logf logger.Logf, health *health.Tracker, _ *controlknobs.Knobs, iface string) (OSConfigurator, error) {
+func NewOSConfigurator(logf logger.Logf, health *health.Tracker, _ policyclient.Client, _ *controlknobs.Knobs, iface string) (OSConfigurator, error) {
 	return newDirectManager(logf, health), nil
 }

net/dns/manager_windows.go

@@ -29,7 +29,6 @@ import (
 	"tailscale.com/health"
 	"tailscale.com/types/logger"
 	"tailscale.com/util/dnsname"
-	"tailscale.com/util/syspolicy"
 	"tailscale.com/util/syspolicy/pkey"
 	"tailscale.com/util/syspolicy/policyclient"
 	"tailscale.com/util/syspolicy/ptype"
@@ -48,6 +47,7 @@ type windowsManager struct {
 	knobs      *controlknobs.Knobs // or nil
 	nrptDB     *nrptRuleDatabase
 	wslManager *wslManager
+	polc       policyclient.Client
 
 	unregisterPolicyChangeCb func() // called when the manager is closing
 
@@ -58,11 +58,15 @@ type windowsManager struct {
 // NewOSConfigurator created a new OS configurator.
 //
 // The health tracker and the knobs may be nil.
-func NewOSConfigurator(logf logger.Logf, health *health.Tracker, knobs *controlknobs.Knobs, interfaceName string) (OSConfigurator, error) {
+func NewOSConfigurator(logf logger.Logf, health *health.Tracker, polc policyclient.Client, knobs *controlknobs.Knobs, interfaceName string) (OSConfigurator, error) {
+	if polc == nil {
+		panic("nil policyclient.Client")
+	}
 	ret := &windowsManager{
 		logf:       logf,
 		guid:       interfaceName,
 		knobs:      knobs,
+		polc:       polc,
 		wslManager: newWSLManager(logf, health),
 	}
 
@@ -71,7 +75,7 @@ func NewOSConfigurator(logf logger.Logf, health *health.Tracker, knobs *controlk
 	}
 
 	var err error
-	if ret.unregisterPolicyChangeCb, err = syspolicy.RegisterChangeCallback(ret.sysPolicyChanged); err != nil {
+	if ret.unregisterPolicyChangeCb, err = polc.RegisterChangeCallback(ret.sysPolicyChanged); err != nil {
 		logf("error registering policy change callback: %v", err) // non-fatal
 	}
 
@@ -521,7 +525,7 @@ func (m *windowsManager) reconfigureDNSRegistration() {
 	// Disable DNS registration by default (if the policy setting is not configured).
 	// This is primarily for historical reasons and to avoid breaking existing
 	// setups that rely on this behavior.
-	enableDNSRegistration, err := syspolicy.GetPreferenceOptionOrDefault(pkey.EnableDNSRegistration, ptype.NeverByPolicy)
+	enableDNSRegistration, err := m.polc.GetPreferenceOption(pkey.EnableDNSRegistration, ptype.NeverByPolicy)
 	if err != nil {
 		m.logf("error getting DNSRegistration policy setting: %v", err) // non-fatal; we'll use the default
 	}

net/dns/manager_windows_test.go

@@ -17,6 +17,7 @@ import (
 	"golang.org/x/sys/windows/registry"
 	"tailscale.com/types/logger"
 	"tailscale.com/util/dnsname"
+	"tailscale.com/util/syspolicy/policyclient"
 	"tailscale.com/util/winutil"
 	"tailscale.com/util/winutil/gp"
 )
@@ -133,7 +134,7 @@ func TestManagerWindowsGPCopy(t *testing.T) {
 	}
 	defer delIfKey()
 
-	cfg, err := NewOSConfigurator(logf, nil, nil, fakeInterface.String())
+	cfg, err := NewOSConfigurator(logf, nil, policyclient.NoPolicyClient{}, nil, fakeInterface.String())
 	if err != nil {
 		t.Fatalf("NewOSConfigurator: %v\n", err)
 	}
@@ -262,7 +263,7 @@ func runTest(t *testing.T, isLocal bool) {
 	}
 	defer delIfKey()
 
-	cfg, err := NewOSConfigurator(logf, nil, nil, fakeInterface.String())
+	cfg, err := NewOSConfigurator(logf, nil, policyclient.NoPolicyClient{}, nil, fakeInterface.String())
 	if err != nil {
 		t.Fatalf("NewOSConfigurator: %v\n", err)
 	}

tsd/syspolicy_off.go

@@ -1,12 +0,0 @@
-// Copyright (c) Tailscale Inc & AUTHORS
-// SPDX-License-Identifier: BSD-3-Clause
-
-//go:build ts_omit_syspolicy
-
-package tsd
-
-import (
-	"tailscale.com/util/syspolicy/policyclient"
-)
-
-func getPolicyClient() policyclient.Client { return policyclient.NoPolicyClient{} }

tsd/syspolicy_on.go

@@ -1,64 +0,0 @@
-// Copyright (c) Tailscale Inc & AUTHORS
-// SPDX-License-Identifier: BSD-3-Clause
-
-//go:build !ts_omit_syspolicy
-
-package tsd
-
-import (
-	"time"
-
-	"tailscale.com/util/syspolicy"
-	"tailscale.com/util/syspolicy/pkey"
-	"tailscale.com/util/syspolicy/policyclient"
-	"tailscale.com/util/syspolicy/ptype"
-)
-
-func getPolicyClient() policyclient.Client { return globalSyspolicy{} }
-
-// globalSyspolicy implements [policyclient.Client] using the syspolicy global
-// functions and global registrations.
-//
-// TODO: de-global-ify. This implementation using the old global functions
-// is an intermediate stage while changing policyclient to be modular.
-type globalSyspolicy struct{}
-
-func (globalSyspolicy) GetBoolean(key pkey.Key, defaultValue bool) (bool, error) {
-	return syspolicy.GetBoolean(key, defaultValue)
-}
-
-func (globalSyspolicy) GetString(key pkey.Key, defaultValue string) (string, error) {
-	return syspolicy.GetString(key, defaultValue)
-}
-
-func (globalSyspolicy) GetStringArray(key pkey.Key, defaultValue []string) ([]string, error) {
-	return syspolicy.GetStringArray(key, defaultValue)
-}
-
-func (globalSyspolicy) SetDebugLoggingEnabled(enabled bool) {
-	syspolicy.SetDebugLoggingEnabled(enabled)
-}
-
-func (globalSyspolicy) GetUint64(key pkey.Key, defaultValue uint64) (uint64, error) {
-	return syspolicy.GetUint64(key, defaultValue)
-}
-
-func (globalSyspolicy) GetDuration(name pkey.Key, defaultValue time.Duration) (time.Duration, error) {
-	return syspolicy.GetDuration(name, defaultValue)
-}
-
-func (globalSyspolicy) GetPreferenceOption(name pkey.Key) (ptype.PreferenceOption, error) {
-	return syspolicy.GetPreferenceOption(name)
-}
-
-func (globalSyspolicy) GetVisibility(name pkey.Key) (ptype.Visibility, error) {
-	return syspolicy.GetVisibility(name)
-}
-
-func (globalSyspolicy) HasAnyOf(keys ...pkey.Key) (bool, error) {
-	return syspolicy.HasAnyOf(keys...)
-}
-
-func (globalSyspolicy) RegisterChangeCallback(cb func(policyclient.PolicyChange)) (unregister func(), err error) {
-	return syspolicy.RegisterChangeCallback(cb)
-}

tsd/tsd.go

@@ -175,7 +175,7 @@ func (s *System) PolicyClientOrDefault() policyclient.Client {
 	if client, ok := s.PolicyClient.GetOK(); ok {
 		return client
 	}
-	return getPolicyClient()
+	return policyclient.Get()
 }
 
 // SubSystem represents some subsystem of the Tailscale node daemon.

tsnet/depaware.txt

@@ -236,6 +236,7 @@ tailscale.com/tsnet dependencies: (generated by github.com/tailscale/depaware)
         tailscale.com/envknob                                        from tailscale.com/client/local+
         tailscale.com/envknob/featureknob                            from tailscale.com/client/web+
         tailscale.com/feature                                        from tailscale.com/ipn/ipnext+
+        tailscale.com/feature/syspolicy                              from tailscale.com/logpolicy
         tailscale.com/health                                         from tailscale.com/control/controlclient+
         tailscale.com/health/healthmsg                               from tailscale.com/ipn/ipnlocal+
         tailscale.com/hostinfo                                       from tailscale.com/client/web+
@@ -375,7 +376,7 @@ tailscale.com/tsnet dependencies: (generated by github.com/tailscale/depaware)
         tailscale.com/util/set                                       from tailscale.com/control/controlclient+
         tailscale.com/util/singleflight                              from tailscale.com/control/controlclient+
         tailscale.com/util/slicesx                                   from tailscale.com/appc+
-        tailscale.com/util/syspolicy                                 from tailscale.com/ipn+
+        tailscale.com/util/syspolicy                                 from tailscale.com/feature/syspolicy
         tailscale.com/util/syspolicy/internal                        from tailscale.com/util/syspolicy+
         tailscale.com/util/syspolicy/internal/loggerx                from tailscale.com/util/syspolicy+
         tailscale.com/util/syspolicy/internal/metrics                from tailscale.com/util/syspolicy/source

tstest/integration/tailscaled_deps_test_darwin.go

@@ -51,8 +51,8 @@ import (
 	_ "tailscale.com/util/eventbus"
 	_ "tailscale.com/util/multierr"
 	_ "tailscale.com/util/osshare"
-	_ "tailscale.com/util/syspolicy"
 	_ "tailscale.com/util/syspolicy/pkey"
+	_ "tailscale.com/util/syspolicy/policyclient"
 	_ "tailscale.com/version"
 	_ "tailscale.com/version/distro"
 	_ "tailscale.com/wgengine"

tstest/integration/tailscaled_deps_test_freebsd.go

@@ -51,8 +51,8 @@ import (
 	_ "tailscale.com/util/eventbus"
 	_ "tailscale.com/util/multierr"
 	_ "tailscale.com/util/osshare"
-	_ "tailscale.com/util/syspolicy"
 	_ "tailscale.com/util/syspolicy/pkey"
+	_ "tailscale.com/util/syspolicy/policyclient"
 	_ "tailscale.com/version"
 	_ "tailscale.com/version/distro"
 	_ "tailscale.com/wgengine"

tstest/integration/tailscaled_deps_test_linux.go

@@ -51,8 +51,8 @@ import (
 	_ "tailscale.com/util/eventbus"
 	_ "tailscale.com/util/multierr"
 	_ "tailscale.com/util/osshare"
-	_ "tailscale.com/util/syspolicy"
 	_ "tailscale.com/util/syspolicy/pkey"
+	_ "tailscale.com/util/syspolicy/policyclient"
 	_ "tailscale.com/version"
 	_ "tailscale.com/version/distro"
 	_ "tailscale.com/wgengine"

tstest/integration/tailscaled_deps_test_openbsd.go

@@ -51,8 +51,8 @@ import (
 	_ "tailscale.com/util/eventbus"
 	_ "tailscale.com/util/multierr"
 	_ "tailscale.com/util/osshare"
-	_ "tailscale.com/util/syspolicy"
 	_ "tailscale.com/util/syspolicy/pkey"
+	_ "tailscale.com/util/syspolicy/policyclient"
 	_ "tailscale.com/version"
 	_ "tailscale.com/version/distro"
 	_ "tailscale.com/wgengine"

tstest/integration/tailscaled_deps_test_windows.go

@@ -62,8 +62,8 @@ import (
 	_ "tailscale.com/util/multierr"
 	_ "tailscale.com/util/osdiag"
 	_ "tailscale.com/util/osshare"
-	_ "tailscale.com/util/syspolicy"
 	_ "tailscale.com/util/syspolicy/pkey"
+	_ "tailscale.com/util/syspolicy/policyclient"
 	_ "tailscale.com/util/winutil"
 	_ "tailscale.com/util/winutil/gp"
 	_ "tailscale.com/version"

util/syspolicy/policyclient/policyclient.go

@@ -44,8 +44,8 @@ type Client interface {
 	// overrides of users' choices in a way that we do not want tailcontrol to have
 	// the authority to set. It describes user-decides/always/never options, where
 	// "always" and "never" remove the user's ability to make a selection. If not
-	// present or set to a different value, "user-decides" is the default.
-	GetPreferenceOption(key pkey.Key) (ptype.PreferenceOption, error)
+	// present or set to a different value, defaultValue (and a nil error) is returned.
+	GetPreferenceOption(key pkey.Key, defaultValue ptype.PreferenceOption) (ptype.PreferenceOption, error)
 
 	// GetVisibility returns whether a UI element should be visible based on
 	// the system's configuration.
@@ -66,6 +66,21 @@ type Client interface {
 	RegisterChangeCallback(cb func(PolicyChange)) (unregister func(), err error)
 }
 
+// Get returns a non-nil [Client] implementation as a function of the
+// build tags. It returns a no-op implementation if the full syspolicy
+// package is omitted from the build.
+func Get() Client {
+	return client
+}
+
+// RegisterClientImpl registers a [Client] implementation to be returned by
+// [Get].
+func RegisterClientImpl(c Client) {
+	client = c
+}
+
+var client Client = NoPolicyClient{}
+
 // PolicyChange is the interface representing a change in policy settings.
 type PolicyChange interface {
 	// HasChanged reports whether the policy setting identified by the given key
@@ -81,6 +96,8 @@ type PolicyChange interface {
 // returns default values.
 type NoPolicyClient struct{}
 
+var _ Client = NoPolicyClient{}
+
 func (NoPolicyClient) GetBoolean(key pkey.Key, defaultValue bool) (bool, error) {
 	return defaultValue, nil
 }
@@ -101,8 +118,8 @@ func (NoPolicyClient) GetDuration(name pkey.Key, defaultValue time.Duration) (ti
 	return defaultValue, nil
 }
 
-func (NoPolicyClient) GetPreferenceOption(name pkey.Key) (ptype.PreferenceOption, error) {
-	return ptype.ShowChoiceByPolicy, nil
+func (NoPolicyClient) GetPreferenceOption(name pkey.Key, defaultValue ptype.PreferenceOption) (ptype.PreferenceOption, error) {
+	return defaultValue, nil
 }
 
 func (NoPolicyClient) GetVisibility(name pkey.Key) (ptype.Visibility, error) {

util/syspolicy/syspolicy.go

@@ -1,13 +1,9 @@
 // Copyright (c) Tailscale Inc & AUTHORS
 // SPDX-License-Identifier: BSD-3-Clause
 
-// Package syspolicy facilitates retrieval of the current policy settings
-// applied to the device or user and receiving notifications when the policy
-// changes.
-//
-// It provides functions that return specific policy settings by their unique
-// [setting.Key]s, such as [GetBoolean], [GetUint64], [GetString],
-// [GetStringArray], [GetPreferenceOption], [GetVisibility] and [GetDuration].
+// Package syspolicy contains the implementation of system policy management.
+// Calling code should use the client interface in
+// tailscale.com/util/syspolicy/policyclient.
 package syspolicy
 
 import (
@@ -18,6 +14,7 @@ import (
 
 	"tailscale.com/util/syspolicy/internal/loggerx"
 	"tailscale.com/util/syspolicy/pkey"
+	"tailscale.com/util/syspolicy/policyclient"
 	"tailscale.com/util/syspolicy/ptype"
 	"tailscale.com/util/syspolicy/rsop"
 	"tailscale.com/util/syspolicy/setting"
@@ -58,9 +55,9 @@ func MustRegisterStoreForTest(tb testenv.TB, name string, scope setting.PolicySc
 	return reg
 }
 
-// HasAnyOf returns whether at least one of the specified policy settings is configured,
+// hasAnyOf returns whether at least one of the specified policy settings is configured,
 // or an error if no keys are provided or the check fails.
-func HasAnyOf(keys ...pkey.Key) (bool, error) {
+func hasAnyOf(keys ...pkey.Key) (bool, error) {
 	if len(keys) == 0 {
 		return false, errors.New("at least one key must be specified")
 	}
@@ -82,62 +79,55 @@ func HasAnyOf(keys ...pkey.Key) (bool, error) {
 	return false, nil
 }
 
-// GetString returns a string policy setting with the specified key,
+// getString returns a string policy setting with the specified key,
 // or defaultValue if it does not exist.
-func GetString(key pkey.Key, defaultValue string) (string, error) {
+func getString(key pkey.Key, defaultValue string) (string, error) {
 	return getCurrentPolicySettingValue(key, defaultValue)
 }
 
-// GetUint64 returns a numeric policy setting with the specified key,
+// getUint64 returns a numeric policy setting with the specified key,
 // or defaultValue if it does not exist.
-func GetUint64(key pkey.Key, defaultValue uint64) (uint64, error) {
+func getUint64(key pkey.Key, defaultValue uint64) (uint64, error) {
 	return getCurrentPolicySettingValue(key, defaultValue)
 }
 
-// GetBoolean returns a boolean policy setting with the specified key,
+// getBoolean returns a boolean policy setting with the specified key,
 // or defaultValue if it does not exist.
-func GetBoolean(key pkey.Key, defaultValue bool) (bool, error) {
+func getBoolean(key pkey.Key, defaultValue bool) (bool, error) {
 	return getCurrentPolicySettingValue(key, defaultValue)
 }
 
-// GetStringArray returns a multi-string policy setting with the specified key,
+// getStringArray returns a multi-string policy setting with the specified key,
 // or defaultValue if it does not exist.
-func GetStringArray(key pkey.Key, defaultValue []string) ([]string, error) {
+func getStringArray(key pkey.Key, defaultValue []string) ([]string, error) {
 	return getCurrentPolicySettingValue(key, defaultValue)
 }
 
-// GetPreferenceOption loads a policy from the registry that can be
+// getPreferenceOption loads a policy from the registry that can be
 // managed by an enterprise policy management system and allows administrative
 // overrides of users' choices in a way that we do not want tailcontrol to have
 // the authority to set. It describes user-decides/always/never options, where
 // "always" and "never" remove the user's ability to make a selection. If not
-// present or set to a different value, "user-decides" is the default.
-func GetPreferenceOption(name pkey.Key) (ptype.PreferenceOption, error) {
-	return getCurrentPolicySettingValue(name, ptype.ShowChoiceByPolicy)
-}
-
-// GetPreferenceOptionOrDefault is like [GetPreferenceOption], but allows
-// specifying a default value to return if the policy setting is not configured.
-// It can be used in situations where "user-decides" is not the default.
-func GetPreferenceOptionOrDefault(name pkey.Key, defaultValue ptype.PreferenceOption) (ptype.PreferenceOption, error) {
+// present or set to a different value, defaultValue (and a nil error) is returned.
+func getPreferenceOption(name pkey.Key, defaultValue ptype.PreferenceOption) (ptype.PreferenceOption, error) {
 	return getCurrentPolicySettingValue(name, defaultValue)
 }
 
-// GetVisibility loads a policy from the registry that can be managed
+// getVisibility loads a policy from the registry that can be managed
 // by an enterprise policy management system and describes show/hide decisions
 // for UI elements. The registry value should be a string set to "show" (return
 // true) or "hide" (return true). If not present or set to a different value,
 // "show" (return false) is the default.
-func GetVisibility(name pkey.Key) (ptype.Visibility, error) {
+func getVisibility(name pkey.Key) (ptype.Visibility, error) {
 	return getCurrentPolicySettingValue(name, ptype.VisibleByPolicy)
 }
 
-// GetDuration loads a policy from the registry that can be managed
+// getDuration loads a policy from the registry that can be managed
 // by an enterprise policy management system and describes a duration for some
 // action. The registry value should be a string that time.ParseDuration
 // understands. If the registry value is "" or can not be processed,
 // defaultValue is returned instead.
-func GetDuration(name pkey.Key, defaultValue time.Duration) (time.Duration, error) {
+func getDuration(name pkey.Key, defaultValue time.Duration) (time.Duration, error) {
 	d, err := getCurrentPolicySettingValue(name, defaultValue)
 	if err != nil {
 		return d, err
@@ -148,9 +138,9 @@ func GetDuration(name pkey.Key, defaultValue time.Duration) (time.Duration, erro
 	return d, nil
 }
 
-// RegisterChangeCallback adds a function that will be called whenever the effective policy
+// registerChangeCallback adds a function that will be called whenever the effective policy
 // for the default scope changes. The returned function can be used to unregister the callback.
-func RegisterChangeCallback(cb rsop.PolicyChangeCallback) (unregister func(), err error) {
+func registerChangeCallback(cb rsop.PolicyChangeCallback) (unregister func(), err error) {
 	effective, err := rsop.PolicyFor(setting.DefaultScope())
 	if err != nil {
 		return nil, err
@@ -233,7 +223,53 @@ func SelectControlURL(reg, disk string) string {
 	return def
 }
 
-// SetDebugLoggingEnabled controls whether spammy debug logging is enabled.
-func SetDebugLoggingEnabled(v bool) {
-	loggerx.SetDebugLoggingEnabled(v)
+func init() {
+	policyclient.RegisterClientImpl(globalSyspolicy{})
+}
+
+// globalSyspolicy implements [policyclient.Client] using the syspolicy global
+// functions and global registrations.
+//
+// TODO: de-global-ify. This implementation using the old global functions
+// is an intermediate stage while changing policyclient to be modular.
+type globalSyspolicy struct{}
+
+func (globalSyspolicy) GetBoolean(key pkey.Key, defaultValue bool) (bool, error) {
+	return getBoolean(key, defaultValue)
+}
+
+func (globalSyspolicy) GetString(key pkey.Key, defaultValue string) (string, error) {
+	return getString(key, defaultValue)
+}
+
+func (globalSyspolicy) GetStringArray(key pkey.Key, defaultValue []string) ([]string, error) {
+	return getStringArray(key, defaultValue)
+}
+
+func (globalSyspolicy) SetDebugLoggingEnabled(enabled bool) {
+	loggerx.SetDebugLoggingEnabled(enabled)
+}
+
+func (globalSyspolicy) GetUint64(key pkey.Key, defaultValue uint64) (uint64, error) {
+	return getUint64(key, defaultValue)
+}
+
+func (globalSyspolicy) GetDuration(name pkey.Key, defaultValue time.Duration) (time.Duration, error) {
+	return getDuration(name, defaultValue)
+}
+
+func (globalSyspolicy) GetPreferenceOption(name pkey.Key, defaultValue ptype.PreferenceOption) (ptype.PreferenceOption, error) {
+	return getPreferenceOption(name, defaultValue)
+}
+
+func (globalSyspolicy) GetVisibility(name pkey.Key) (ptype.Visibility, error) {
+	return getVisibility(name)
+}
+
+func (globalSyspolicy) HasAnyOf(keys ...pkey.Key) (bool, error) {
+	return hasAnyOf(keys...)
+}
+
+func (globalSyspolicy) RegisterChangeCallback(cb func(policyclient.PolicyChange)) (unregister func(), err error) {
+	return registerChangeCallback(cb)
 }

util/syspolicy/syspolicy_test.go

@@ -82,7 +82,7 @@ func TestGetString(t *testing.T) {
 			}
 			registerSingleSettingStoreForTest(t, s)
 
-			value, err := GetString(tt.key, tt.defaultValue)
+			value, err := getString(tt.key, tt.defaultValue)
 			if !errorsMatchForTest(err, tt.wantError) {
 				t.Errorf("err=%q, want %q", err, tt.wantError)
 			}
@@ -157,7 +157,7 @@ func TestGetUint64(t *testing.T) {
 			}
 			registerSingleSettingStoreForTest(t, s)
 
-			value, err := GetUint64(tt.key, tt.defaultValue)
+			value, err := getUint64(tt.key, tt.defaultValue)
 			if !errorsMatchForTest(err, tt.wantError) {
 				t.Errorf("err=%q, want %q", err, tt.wantError)
 			}
@@ -224,7 +224,7 @@ func TestGetBoolean(t *testing.T) {
 			}
 			registerSingleSettingStoreForTest(t, s)
 
-			value, err := GetBoolean(tt.key, tt.defaultValue)
+			value, err := getBoolean(tt.key, tt.defaultValue)
 			if !errorsMatchForTest(err, tt.wantError) {
 				t.Errorf("err=%q, want %q", err, tt.wantError)
 			}
@@ -317,7 +317,7 @@ func TestGetPreferenceOption(t *testing.T) {
 			}
 			registerSingleSettingStoreForTest(t, s)
 
-			option, err := GetPreferenceOption(tt.key)
+			option, err := getPreferenceOption(tt.key, ptype.ShowChoiceByPolicy)
 			if !errorsMatchForTest(err, tt.wantError) {
 				t.Errorf("err=%q, want %q", err, tt.wantError)
 			}
@@ -402,7 +402,7 @@ func TestGetVisibility(t *testing.T) {
 			}
 			registerSingleSettingStoreForTest(t, s)
 
-			visibility, err := GetVisibility(tt.key)
+			visibility, err := getVisibility(tt.key)
 			if !errorsMatchForTest(err, tt.wantError) {
 				t.Errorf("err=%q, want %q", err, tt.wantError)
 			}
@@ -498,7 +498,7 @@ func TestGetDuration(t *testing.T) {
 			}
 			registerSingleSettingStoreForTest(t, s)
 
-			duration, err := GetDuration(tt.key, tt.defaultValue)
+			duration, err := getDuration(tt.key, tt.defaultValue)
 			if !errorsMatchForTest(err, tt.wantError) {
 				t.Errorf("err=%q, want %q", err, tt.wantError)
 			}
@@ -579,7 +579,7 @@ func TestGetStringArray(t *testing.T) {
 			}
 			registerSingleSettingStoreForTest(t, s)
 
-			value, err := GetStringArray(tt.key, tt.defaultValue)
+			value, err := getStringArray(tt.key, tt.defaultValue)
 			if !errorsMatchForTest(err, tt.wantError) {
 				t.Errorf("err=%q, want %q", err, tt.wantError)
 			}
@@ -613,7 +613,7 @@ func BenchmarkGetString(b *testing.B) {
 
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
-		gotControlURL, _ := GetString(pkey.ControlURL, "https://controlplane.tailscale.com")
+		gotControlURL, _ := getString(pkey.ControlURL, "https://controlplane.tailscale.com")
 		if gotControlURL != wantControlURL {
 			b.Fatalf("got %v; want %v", gotControlURL, wantControlURL)
 		}