From d05e6dc09e7a36e2b6082ce259e33eb3eecd0c0c Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 1 Sep 2025 08:04:17 -0700 Subject: [PATCH] util/syspolicy/policyclient: add policyclient.Client interface, start plumbing This is step 2 of ~4, breaking up #14720 into reviewable chunks, with the aim to make syspolicy be a build-time configurable feature. Step 1 was #16984. In this second step, the util/syspolicy/policyclient package is added with the policyclient.Client interface. This is the interface that's always present (regardless of build tags), and is what code around the tree uses to ask syspolicy/MDM questions. There are two implementations of policyclient.Client for now: 1) NoPolicyClient, which only returns default values. 2) the unexported, temporary 'globalSyspolicy', which is implemented in terms of the global functions we wish to later eliminate. This then starts to plumb around the policyclient.Client to most callers. Future changes will plumb it more. When the last of the global func callers are gone, then we can unexport the global functions and make a proper policyclient.Client type and constructor in the syspolicy package, removing the globalSyspolicy impl out of tsd. The final change will sprinkle build tags in a few more places and lock it in with dependency tests to make sure the dependencies don't later creep back in. Updates #16998 Updates #12614 Change-Id: Ib2c93d15c15c1f2b981464099177cd492d50391c Signed-off-by: Brad Fitzpatrick --- cmd/derper/depaware.txt | 1 + cmd/k8s-operator/depaware.txt | 3 +- cmd/tailscale/depaware.txt | 1 + cmd/tailscaled/depaware.txt | 1 + cmd/tsidp/depaware.txt | 5 +- control/controlclient/direct.go | 10 ++- control/controlclient/sign_supported.go | 10 +-- control/controlclient/sign_unsupported.go | 3 +- ipn/ipnlocal/c2n.go | 2 +- ipn/ipnlocal/local.go | 9 ++- net/dns/manager_windows.go | 4 +- posture/serialnumber_macos.go | 3 +- posture/serialnumber_macos_test.go | 3 +- posture/serialnumber_notmacos.go | 3 +- posture/serialnumber_notmacos_test.go | 3 +- posture/serialnumber_stub.go | 3 +- posture/serialnumber_syspolicy.go | 6 +- posture/serialnumber_test.go | 3 +- tsd/syspolicy_off.go | 12 ++++ tsd/syspolicy_on.go | 41 ++++++++++++ tsd/tsd.go | 7 +++ tsnet/depaware.txt | 5 +- util/syspolicy/policyclient/policyclient.go | 66 ++++++++++++++++++++ util/syspolicy/rsop/change_callbacks.go | 3 +- util/syspolicy/rsop/resultant_policy_test.go | 13 ++-- 25 files changed, 184 insertions(+), 36 deletions(-) create mode 100644 tsd/syspolicy_off.go create mode 100644 tsd/syspolicy_on.go create mode 100644 util/syspolicy/policyclient/policyclient.go diff --git a/cmd/derper/depaware.txt b/cmd/derper/depaware.txt index ccea25a8a..0597d5d1f 100644 --- a/cmd/derper/depaware.txt +++ b/cmd/derper/depaware.txt @@ -175,6 +175,7 @@ tailscale.com/cmd/derper dependencies: (generated by github.com/tailscale/depawa 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/pkey from tailscale.com/ipn+ + tailscale.com/util/syspolicy/policyclient from tailscale.com/util/syspolicy/rsop 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+ diff --git a/cmd/k8s-operator/depaware.txt b/cmd/k8s-operator/depaware.txt index a0214575b..40c8abb08 100644 --- a/cmd/k8s-operator/depaware.txt +++ b/cmd/k8s-operator/depaware.txt @@ -951,11 +951,12 @@ 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/control/controlclient+ + 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/pkey from tailscale.com/control/controlclient+ + tailscale.com/util/syspolicy/policyclient from tailscale.com/control/controlclient+ 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+ diff --git a/cmd/tailscale/depaware.txt b/cmd/tailscale/depaware.txt index 7f09be33f..cf1691c71 100644 --- a/cmd/tailscale/depaware.txt +++ b/cmd/tailscale/depaware.txt @@ -196,6 +196,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep 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/pkey from tailscale.com/ipn+ + tailscale.com/util/syspolicy/policyclient from tailscale.com/util/syspolicy/rsop 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+ diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 46efa5b21..f08601f81 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -433,6 +433,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de 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/pkey from tailscale.com/cmd/tailscaled+ + tailscale.com/util/syspolicy/policyclient from tailscale.com/control/controlclient+ 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+ diff --git a/cmd/tsidp/depaware.txt b/cmd/tsidp/depaware.txt index f1e22efbf..743492904 100644 --- a/cmd/tsidp/depaware.txt +++ b/cmd/tsidp/depaware.txt @@ -380,12 +380,13 @@ 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/control/controlclient+ + tailscale.com/util/syspolicy from tailscale.com/ipn+ 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 tailscale.com/util/syspolicy/pkey from tailscale.com/control/controlclient+ - tailscale.com/util/syspolicy/rsop from tailscale.com/ipn/ipnlocal+ + tailscale.com/util/syspolicy/policyclient from tailscale.com/control/controlclient+ + tailscale.com/util/syspolicy/rsop from tailscale.com/ipn/localapi+ tailscale.com/util/syspolicy/setting from tailscale.com/client/local+ tailscale.com/util/syspolicy/source from tailscale.com/util/syspolicy+ tailscale.com/util/systemd from tailscale.com/control/controlclient+ diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index cee938779..47283a673 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -6,6 +6,7 @@ package controlclient import ( "bufio" "bytes" + "cmp" "context" "encoding/binary" "encoding/json" @@ -53,8 +54,8 @@ import ( "tailscale.com/util/clientmetric" "tailscale.com/util/multierr" "tailscale.com/util/singleflight" - "tailscale.com/util/syspolicy" "tailscale.com/util/syspolicy/pkey" + "tailscale.com/util/syspolicy/policyclient" "tailscale.com/util/systemd" "tailscale.com/util/testenv" "tailscale.com/util/zstdframe" @@ -77,6 +78,7 @@ type Direct struct { debugFlags []string skipIPForwardingCheck bool pinger Pinger + polc policyclient.Client // always non-nil popBrowser func(url string) // or nil c2nHandler http.Handler // or nil onClientVersion func(*tailcfg.ClientVersion) // or nil @@ -125,6 +127,7 @@ type Options struct { Clock tstime.Clock Hostinfo *tailcfg.Hostinfo // non-nil passes ownership, nil means to use default using os.Hostname, etc DiscoPublicKey key.DiscoPublic + PolicyClient policyclient.Client // or nil for none Logf logger.Logf HTTPTestClient *http.Client // optional HTTP client to use (for tests only) NoiseTestClient *http.Client // optional HTTP client to use for noise RPCs (tests only) @@ -299,6 +302,7 @@ func NewDirect(opts Options) (*Direct, error) { health: opts.HealthTracker, skipIPForwardingCheck: opts.SkipIPForwardingCheck, pinger: opts.Pinger, + polc: cmp.Or(opts.PolicyClient, policyclient.Client(policyclient.NoPolicyClient{})), popBrowser: opts.PopBrowserURL, onClientVersion: opts.OnClientVersion, onTailnetDefaultAutoUpdate: opts.OnTailnetDefaultAutoUpdate, @@ -617,7 +621,7 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new return regen, opt.URL, nil, err } - tailnet, err := syspolicy.GetString(pkey.Tailnet, "") + tailnet, err := c.polc.GetString(pkey.Tailnet, "") if err != nil { c.logf("unable to provide Tailnet field in register request. err: %v", err) } @@ -647,7 +651,7 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new AuthKey: authKey, } } - err = signRegisterRequest(&request, c.serverURL, c.serverLegacyKey, machinePrivKey.Public()) + err = signRegisterRequest(c.polc, &request, c.serverURL, c.serverLegacyKey, machinePrivKey.Public()) if err != nil { // If signing failed, clear all related fields request.SignatureType = tailcfg.SignatureNone diff --git a/control/controlclient/sign_supported.go b/control/controlclient/sign_supported.go index fab7cd16b..439e6d36b 100644 --- a/control/controlclient/sign_supported.go +++ b/control/controlclient/sign_supported.go @@ -18,8 +18,8 @@ import ( "github.com/tailscale/certstore" "tailscale.com/tailcfg" "tailscale.com/types/key" - "tailscale.com/util/syspolicy" "tailscale.com/util/syspolicy/pkey" + "tailscale.com/util/syspolicy/policyclient" ) // getMachineCertificateSubject returns the exact name of a Subject that needs @@ -31,8 +31,8 @@ import ( // each RegisterRequest will be unsigned. // // Example: "CN=Tailscale Inc Test Root CA,OU=Tailscale Inc Test Certificate Authority,O=Tailscale Inc,ST=ON,C=CA" -func getMachineCertificateSubject() string { - machineCertSubject, _ := syspolicy.GetString(pkey.MachineCertificateSubject, "") +func getMachineCertificateSubject(polc policyclient.Client) string { + machineCertSubject, _ := polc.GetString(pkey.MachineCertificateSubject, "") return machineCertSubject } @@ -137,7 +137,7 @@ func findIdentity(subject string, st certstore.Store) (certstore.Identity, []*x5 // using that identity's public key. In addition to the signature, the full // certificate chain is included so that the control server can validate the // certificate from a copy of the root CA's certificate. -func signRegisterRequest(req *tailcfg.RegisterRequest, serverURL string, serverPubKey, machinePubKey key.MachinePublic) (err error) { +func signRegisterRequest(polc policyclient.Client, req *tailcfg.RegisterRequest, serverURL string, serverPubKey, machinePubKey key.MachinePublic) (err error) { defer func() { if err != nil { err = fmt.Errorf("signRegisterRequest: %w", err) @@ -148,7 +148,7 @@ func signRegisterRequest(req *tailcfg.RegisterRequest, serverURL string, serverP return errBadRequest } - machineCertificateSubject := getMachineCertificateSubject() + machineCertificateSubject := getMachineCertificateSubject(polc) if machineCertificateSubject == "" { return errCertificateNotConfigured } diff --git a/control/controlclient/sign_unsupported.go b/control/controlclient/sign_unsupported.go index 5e161dcbc..f6c4ddc62 100644 --- a/control/controlclient/sign_unsupported.go +++ b/control/controlclient/sign_unsupported.go @@ -8,9 +8,10 @@ package controlclient import ( "tailscale.com/tailcfg" "tailscale.com/types/key" + "tailscale.com/util/syspolicy/policyclient" ) // signRegisterRequest on non-supported platforms always returns errNoCertStore. -func signRegisterRequest(req *tailcfg.RegisterRequest, serverURL string, serverPubKey, machinePubKey key.MachinePublic) error { +func signRegisterRequest(polc policyclient.Client, req *tailcfg.RegisterRequest, serverURL string, serverPubKey, machinePubKey key.MachinePublic) error { return errNoCertStore } diff --git a/ipn/ipnlocal/c2n.go b/ipn/ipnlocal/c2n.go index 8c3bf7b26..b1a780cc1 100644 --- a/ipn/ipnlocal/c2n.go +++ b/ipn/ipnlocal/c2n.go @@ -353,7 +353,7 @@ func handleC2NPostureIdentityGet(b *LocalBackend, w http.ResponseWriter, r *http } if choice.ShouldEnable(b.Prefs().PostureChecking()) { - res.SerialNumbers, err = posture.GetSerialNumbers(b.logf) + res.SerialNumbers, err = posture.GetSerialNumbers(b.polc, b.logf) if err != nil { b.logf("c2n: GetSerialNumbers returned error: %v", err) } diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index bcfb99b09..61bde31e4 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -109,7 +109,7 @@ import ( "tailscale.com/util/slicesx" "tailscale.com/util/syspolicy" "tailscale.com/util/syspolicy/pkey" - "tailscale.com/util/syspolicy/rsop" + "tailscale.com/util/syspolicy/policyclient" "tailscale.com/util/systemd" "tailscale.com/util/testenv" "tailscale.com/util/usermetric" @@ -203,7 +203,8 @@ type LocalBackend struct { keyLogf logger.Logf // for printing list of peers on change statsLogf logger.Logf // for printing peers stats on change sys *tsd.System - health *health.Tracker // always non-nil + health *health.Tracker // always non-nil + polc policyclient.Client // always non-nil metrics metrics e wgengine.Engine // non-nil; TODO(bradfitz): remove; use sys store ipn.StateStore // non-nil; TODO(bradfitz): remove; use sys @@ -515,6 +516,7 @@ func NewLocalBackend(logf logger.Logf, logID logid.PublicID, sys *tsd.System, lo keyLogf: logger.LogOnChange(logf, 5*time.Minute, clock.Now), statsLogf: logger.LogOnChange(logf, 5*time.Minute, clock.Now), sys: sys, + polc: sys.PolicyClientOrDefault(), health: sys.HealthTracker(), metrics: m, e: e, @@ -1970,7 +1972,7 @@ func (b *LocalBackend) reconcilePrefs() (_ ipn.PrefsView, anyChange bool) { // sysPolicyChanged is a callback triggered by syspolicy when it detects // a change in one or more syspolicy settings. -func (b *LocalBackend) sysPolicyChanged(policy *rsop.PolicyChange) { +func (b *LocalBackend) sysPolicyChanged(policy policyclient.PolicyChange) { if policy.HasChangedAnyOf(pkey.AlwaysOn, pkey.AlwaysOnOverrideWithReason) { // If the AlwaysOn or the AlwaysOnOverrideWithReason policy has changed, // we should reset the overrideAlwaysOn flag, as the override might @@ -2468,6 +2470,7 @@ func (b *LocalBackend) Start(opts ipn.Options) error { DiscoPublicKey: discoPublic, DebugFlags: debugFlags, HealthTracker: b.health, + PolicyClient: b.sys.PolicyClientOrDefault(), Pinger: b, PopBrowserURL: b.tellClientToBrowseToURL, OnClientVersion: b.onClientVersion, diff --git a/net/dns/manager_windows.go b/net/dns/manager_windows.go index d1cec2a00..901ab6dd0 100644 --- a/net/dns/manager_windows.go +++ b/net/dns/manager_windows.go @@ -31,7 +31,7 @@ import ( "tailscale.com/util/dnsname" "tailscale.com/util/syspolicy" "tailscale.com/util/syspolicy/pkey" - "tailscale.com/util/syspolicy/rsop" + "tailscale.com/util/syspolicy/policyclient" "tailscale.com/util/syspolicy/setting" "tailscale.com/util/winutil" ) @@ -508,7 +508,7 @@ func (m *windowsManager) Close() error { // sysPolicyChanged is a callback triggered by [syspolicy] when it detects // a change in one or more syspolicy settings. -func (m *windowsManager) sysPolicyChanged(policy *rsop.PolicyChange) { +func (m *windowsManager) sysPolicyChanged(policy policyclient.PolicyChange) { if policy.HasChanged(pkey.EnableDNSRegistration) { m.reconfigureDNSRegistration() } diff --git a/posture/serialnumber_macos.go b/posture/serialnumber_macos.go index 48355d313..18c929107 100644 --- a/posture/serialnumber_macos.go +++ b/posture/serialnumber_macos.go @@ -59,10 +59,11 @@ import ( "strings" "tailscale.com/types/logger" + "tailscale.com/util/syspolicy/policyclient" ) // GetSerialNumber returns the platform serial sumber as reported by IOKit. -func GetSerialNumbers(_ logger.Logf) ([]string, error) { +func GetSerialNumbers(policyclient.Client, logger.Logf) ([]string, error) { csn := C.getSerialNumber() serialNumber := C.GoString(csn) diff --git a/posture/serialnumber_macos_test.go b/posture/serialnumber_macos_test.go index 9f0ce1c6a..9d9b9f578 100644 --- a/posture/serialnumber_macos_test.go +++ b/posture/serialnumber_macos_test.go @@ -11,6 +11,7 @@ import ( "tailscale.com/types/logger" "tailscale.com/util/cibuild" + "tailscale.com/util/syspolicy/policyclient" ) func TestGetSerialNumberMac(t *testing.T) { @@ -20,7 +21,7 @@ func TestGetSerialNumberMac(t *testing.T) { t.Skip() } - sns, err := GetSerialNumbers(logger.Discard) + sns, err := GetSerialNumbers(policyclient.NoPolicyClient{}, logger.Discard) if err != nil { t.Fatalf("failed to get serial number: %s", err) } diff --git a/posture/serialnumber_notmacos.go b/posture/serialnumber_notmacos.go index 8b91738b0..132fa08f6 100644 --- a/posture/serialnumber_notmacos.go +++ b/posture/serialnumber_notmacos.go @@ -13,6 +13,7 @@ import ( "github.com/digitalocean/go-smbios/smbios" "tailscale.com/types/logger" + "tailscale.com/util/syspolicy/policyclient" ) // getByteFromSmbiosStructure retrieves a 8-bit unsigned integer at the given specOffset. @@ -71,7 +72,7 @@ func init() { numOfTables = len(validTables) } -func GetSerialNumbers(logf logger.Logf) ([]string, error) { +func GetSerialNumbers(polc policyclient.Client, logf logger.Logf) ([]string, error) { // Find SMBIOS data in operating system-specific location. rc, _, err := smbios.Stream() if err != nil { diff --git a/posture/serialnumber_notmacos_test.go b/posture/serialnumber_notmacos_test.go index f2a15e037..da5aada85 100644 --- a/posture/serialnumber_notmacos_test.go +++ b/posture/serialnumber_notmacos_test.go @@ -12,6 +12,7 @@ import ( "testing" "tailscale.com/types/logger" + "tailscale.com/util/syspolicy/policyclient" ) func TestGetSerialNumberNotMac(t *testing.T) { @@ -21,7 +22,7 @@ func TestGetSerialNumberNotMac(t *testing.T) { // Comment out skip for local testing. t.Skip() - sns, err := GetSerialNumbers(logger.Discard) + sns, err := GetSerialNumbers(policyclient.NoPolicyClient{}, logger.Discard) if err != nil { t.Fatalf("failed to get serial number: %s", err) } diff --git a/posture/serialnumber_stub.go b/posture/serialnumber_stub.go index 4cc84fa13..854a0014b 100644 --- a/posture/serialnumber_stub.go +++ b/posture/serialnumber_stub.go @@ -14,9 +14,10 @@ import ( "errors" "tailscale.com/types/logger" + "tailscale.com/util/syspolicy/policyclient" ) // GetSerialNumber returns client machine serial number(s). -func GetSerialNumbers(_ logger.Logf) ([]string, error) { +func GetSerialNumbers(polc policyclient.Client, _ logger.Logf) ([]string, error) { return nil, errors.New("not implemented") } diff --git a/posture/serialnumber_syspolicy.go b/posture/serialnumber_syspolicy.go index 5123d561d..64a154a2c 100644 --- a/posture/serialnumber_syspolicy.go +++ b/posture/serialnumber_syspolicy.go @@ -9,15 +9,15 @@ import ( "fmt" "tailscale.com/types/logger" - "tailscale.com/util/syspolicy" "tailscale.com/util/syspolicy/pkey" + "tailscale.com/util/syspolicy/policyclient" ) // GetSerialNumbers returns the serial number of the device as reported by an // MDM solution. It requires configuration via the DeviceSerialNumber system policy. // This is the only way to gather serial numbers on iOS, tvOS and Android. -func GetSerialNumbers(_ logger.Logf) ([]string, error) { - s, err := syspolicy.GetString(pkey.DeviceSerialNumber, "") +func GetSerialNumbers(polc policyclient.Client, _ logger.Logf) ([]string, error) { + s, err := polc.GetString(pkey.DeviceSerialNumber, "") if err != nil { return nil, fmt.Errorf("failed to get serial number from MDM: %v", err) } diff --git a/posture/serialnumber_test.go b/posture/serialnumber_test.go index fac4392fa..6db3651e2 100644 --- a/posture/serialnumber_test.go +++ b/posture/serialnumber_test.go @@ -7,10 +7,11 @@ import ( "testing" "tailscale.com/types/logger" + "tailscale.com/util/syspolicy/policyclient" ) func TestGetSerialNumber(t *testing.T) { // ensure GetSerialNumbers is implemented // or covered by a stub on a given platform. - _, _ = GetSerialNumbers(logger.Discard) + _, _ = GetSerialNumbers(policyclient.NoPolicyClient{}, logger.Discard) } diff --git a/tsd/syspolicy_off.go b/tsd/syspolicy_off.go new file mode 100644 index 000000000..221b8f223 --- /dev/null +++ b/tsd/syspolicy_off.go @@ -0,0 +1,12 @@ +// 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{} } diff --git a/tsd/syspolicy_on.go b/tsd/syspolicy_on.go new file mode 100644 index 000000000..8d7762bd9 --- /dev/null +++ b/tsd/syspolicy_on.go @@ -0,0 +1,41 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +//go:build !ts_omit_syspolicy + +package tsd + +import ( + "tailscale.com/util/syspolicy" + "tailscale.com/util/syspolicy/pkey" + "tailscale.com/util/syspolicy/policyclient" +) + +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) RegisterChangeCallback(cb func(policyclient.PolicyChange)) (unregister func(), err error) { + return syspolicy.RegisterChangeCallback(cb) +} diff --git a/tsd/tsd.go b/tsd/tsd.go index ccd804f81..b7194a3d7 100644 --- a/tsd/tsd.go +++ b/tsd/tsd.go @@ -33,6 +33,7 @@ import ( "tailscale.com/proxymap" "tailscale.com/types/netmap" "tailscale.com/util/eventbus" + "tailscale.com/util/syspolicy/policyclient" "tailscale.com/util/usermetric" "tailscale.com/wgengine" "tailscale.com/wgengine/magicsock" @@ -165,6 +166,12 @@ func (s *System) UserMetricsRegistry() *usermetric.Registry { return &s.userMetricsRegistry } +// PolicyClientOrDefault returns the policy client if set or a no-op default +// otherwise. It always returns a non-nil value. +func (s *System) PolicyClientOrDefault() policyclient.Client { + return getPolicyClient() +} + // SubSystem represents some subsystem of the Tailscale node daemon. // // A subsystem can be set to a value, and then later retrieved. A subsystem diff --git a/tsnet/depaware.txt b/tsnet/depaware.txt index bdf90c9a8..f4b0dc775 100644 --- a/tsnet/depaware.txt +++ b/tsnet/depaware.txt @@ -375,12 +375,13 @@ 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/control/controlclient+ + tailscale.com/util/syspolicy from tailscale.com/ipn+ 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 tailscale.com/util/syspolicy/pkey from tailscale.com/control/controlclient+ - tailscale.com/util/syspolicy/rsop from tailscale.com/ipn/ipnlocal+ + tailscale.com/util/syspolicy/policyclient from tailscale.com/control/controlclient+ + tailscale.com/util/syspolicy/rsop from tailscale.com/ipn/localapi+ tailscale.com/util/syspolicy/setting from tailscale.com/client/local+ tailscale.com/util/syspolicy/source from tailscale.com/util/syspolicy+ tailscale.com/util/systemd from tailscale.com/control/controlclient+ diff --git a/util/syspolicy/policyclient/policyclient.go b/util/syspolicy/policyclient/policyclient.go new file mode 100644 index 000000000..0b15599c1 --- /dev/null +++ b/util/syspolicy/policyclient/policyclient.go @@ -0,0 +1,66 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +// Package policyclient contains the minimal syspolicy interface as needed by +// client code using syspolicy. It's the part that's always linked in, even if the rest +// of syspolicy is omitted from the build. +package policyclient + +import "tailscale.com/util/syspolicy/pkey" + +// Client is the interface between code making questions about the system policy +// and the actual implementation. +type Client interface { + // GetString returns a string policy setting with the specified key, + // or defaultValue (and a nil error) if it does not exist. + GetString(key pkey.Key, defaultValue string) (string, error) + + // GetStringArray returns a string array policy setting with the specified key, + // or defaultValue (and a nil error) if it does not exist. + GetStringArray(key pkey.Key, defaultValue []string) ([]string, error) + + // GetBoolean returns a boolean policy setting with the specified key, + // or defaultValue (and a nil error) if it does not exist. + GetBoolean(key pkey.Key, defaultValue bool) (bool, error) + + // SetDebugLoggingEnabled enables or disables debug logging for the policy client. + SetDebugLoggingEnabled(enabled bool) + + // RegisterChangeCallback registers a callback function that will be called + // whenever a policy change is detected. It returns a function to unregister + // the callback and an error if the registration fails. + RegisterChangeCallback(cb func(PolicyChange)) (unregister func(), err error) +} + +// PolicyChange is the interface representing a change in policy settings. +type PolicyChange interface { + // HasChanged reports whether the policy setting identified by the given key + // has changed. + HasChanged(pkey.Key) bool + + // HasChangedAnyOf reports whether any of the provided policy settings + // changed in this change. + HasChangedAnyOf(keys ...pkey.Key) bool +} + +// NoPolicyClient is a no-op implementation of [Client] that only +// returns default values. +type NoPolicyClient struct{} + +func (NoPolicyClient) GetBoolean(key pkey.Key, defaultValue bool) (bool, error) { + return defaultValue, nil +} + +func (NoPolicyClient) GetString(key pkey.Key, defaultValue string) (string, error) { + return defaultValue, nil +} + +func (NoPolicyClient) GetStringArray(key pkey.Key, defaultValue []string) ([]string, error) { + return defaultValue, nil +} + +func (NoPolicyClient) SetDebugLoggingEnabled(enabled bool) {} + +func (NoPolicyClient) RegisterChangeCallback(cb func(PolicyChange)) (unregister func(), err error) { + return func() {}, nil +} diff --git a/util/syspolicy/rsop/change_callbacks.go b/util/syspolicy/rsop/change_callbacks.go index 59dba07c6..4e71f683a 100644 --- a/util/syspolicy/rsop/change_callbacks.go +++ b/util/syspolicy/rsop/change_callbacks.go @@ -12,6 +12,7 @@ import ( "tailscale.com/util/set" "tailscale.com/util/syspolicy/internal/loggerx" "tailscale.com/util/syspolicy/pkey" + "tailscale.com/util/syspolicy/policyclient" "tailscale.com/util/syspolicy/setting" ) @@ -21,7 +22,7 @@ type Change[T any] struct { } // PolicyChangeCallback is a function called whenever a policy changes. -type PolicyChangeCallback func(*PolicyChange) +type PolicyChangeCallback func(policyclient.PolicyChange) // PolicyChange describes a policy change. type PolicyChange struct { diff --git a/util/syspolicy/rsop/resultant_policy_test.go b/util/syspolicy/rsop/resultant_policy_test.go index 2da46a8ca..3ff142119 100644 --- a/util/syspolicy/rsop/resultant_policy_test.go +++ b/util/syspolicy/rsop/resultant_policy_test.go @@ -16,6 +16,7 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" "tailscale.com/tstest" "tailscale.com/util/syspolicy/pkey" + "tailscale.com/util/syspolicy/policyclient" "tailscale.com/util/syspolicy/setting" "tailscale.com/util/syspolicy/source" @@ -602,8 +603,8 @@ func TestChangePolicySetting(t *testing.T) { } // Subscribe to the policy change callback... - policyChanged := make(chan *PolicyChange) - unregister := policy.RegisterChangeCallback(func(pc *PolicyChange) { policyChanged <- pc }) + policyChanged := make(chan policyclient.PolicyChange) + unregister := policy.RegisterChangeCallback(func(pc policyclient.PolicyChange) { policyChanged <- pc }) t.Cleanup(unregister) // ...make the change, and measure the time between initiating the change @@ -611,7 +612,7 @@ func TestChangePolicySetting(t *testing.T) { start := time.Now() const wantValueA = "TestValueA" store.SetStrings(source.TestSettingOf(settingA.Key(), wantValueA)) - change := <-policyChanged + change := (<-policyChanged).(*PolicyChange) gotDelay := time.Since(start) // Ensure there is at least a [policyReloadMinDelay] delay between @@ -653,7 +654,7 @@ func TestChangePolicySetting(t *testing.T) { // The callback should be invoked only once, even though the policy setting // has changed N times. - change = <-policyChanged + change = (<-policyChanged).(*PolicyChange) gotDelay = time.Since(start) gotCallbacks := 1 drain: @@ -853,8 +854,8 @@ func TestReplacePolicySource(t *testing.T) { } // Subscribe to the policy change callback. - policyChanged := make(chan *PolicyChange, 1) - unregister := policy.RegisterChangeCallback(func(pc *PolicyChange) { policyChanged <- pc }) + policyChanged := make(chan policyclient.PolicyChange, 1) + unregister := policy.RegisterChangeCallback(func(pc policyclient.PolicyChange) { policyChanged <- pc }) t.Cleanup(unregister) // Now, let's replace the initial store with the new store.