diff --git a/cmd/derper/depaware.txt b/cmd/derper/depaware.txt index 362b07882..e20c4e556 100644 --- a/cmd/derper/depaware.txt +++ b/cmd/derper/depaware.txt @@ -164,11 +164,16 @@ tailscale.com/cmd/derper dependencies: (generated by github.com/tailscale/depawa 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 - tailscale.com/util/syspolicy/setting from tailscale.com/util/syspolicy + 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/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/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+ @@ -189,7 +194,7 @@ tailscale.com/cmd/derper dependencies: (generated by github.com/tailscale/depawa golang.org/x/crypto/salsa20/salsa from golang.org/x/crypto/nacl/box+ golang.org/x/crypto/sha3 from crypto/internal/mlkem768+ W 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 @@ -250,7 +255,7 @@ tailscale.com/cmd/derper dependencies: (generated by github.com/tailscale/depawa encoding/pem from crypto/tls+ errors from bufio+ expvar from github.com/prometheus/client_golang/prometheus+ - flag from tailscale.com/cmd/derper + flag from tailscale.com/cmd/derper+ fmt from compress/flate+ go/token from google.golang.org/protobuf/internal/strs hash from crypto+ @@ -284,7 +289,7 @@ tailscale.com/cmd/derper dependencies: (generated by github.com/tailscale/depawa os from crypto/rand+ 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+ diff --git a/cmd/k8s-operator/depaware.txt b/cmd/k8s-operator/depaware.txt index 19d6808d7..2ad3978c9 100644 --- a/cmd/k8s-operator/depaware.txt +++ b/cmd/k8s-operator/depaware.txt @@ -812,8 +812,11 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/ tailscale.com/util/slicesx from tailscale.com/appc+ tailscale.com/util/syspolicy from tailscale.com/control/controlclient+ tailscale.com/util/syspolicy/internal from tailscale.com/util/syspolicy/setting+ - tailscale.com/util/syspolicy/internal/loggerx from tailscale.com/util/syspolicy - tailscale.com/util/syspolicy/setting from tailscale.com/util/syspolicy + 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/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/sysresources from tailscale.com/wgengine/magicsock tailscale.com/util/systemd from tailscale.com/control/controlclient+ tailscale.com/util/testenv from tailscale.com/control/controlclient+ @@ -823,7 +826,7 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/ tailscale.com/util/vizerror from tailscale.com/tailcfg+ 💣 tailscale.com/util/winutil from tailscale.com/clientupdate+ W 💣 tailscale.com/util/winutil/authenticode from tailscale.com/clientupdate+ - W 💣 tailscale.com/util/winutil/gp from tailscale.com/net/dns + W 💣 tailscale.com/util/winutil/gp from tailscale.com/net/dns+ W tailscale.com/util/winutil/policy from tailscale.com/ipn/ipnlocal W 💣 tailscale.com/util/winutil/winenv from tailscale.com/hostinfo+ tailscale.com/util/zstdframe from tailscale.com/control/controlclient+ diff --git a/cmd/tailscale/depaware.txt b/cmd/tailscale/depaware.txt index 765bbc483..cce76a81e 100644 --- a/cmd/tailscale/depaware.txt +++ b/cmd/tailscale/depaware.txt @@ -174,14 +174,18 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep tailscale.com/util/slicesx from tailscale.com/net/dns/recursive+ 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 - tailscale.com/util/syspolicy/setting from tailscale.com/util/syspolicy - tailscale.com/util/testenv from tailscale.com/cmd/tailscale/cli + 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/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/cmd/tailscale/cli+ tailscale.com/util/truncate from tailscale.com/cmd/tailscale/cli tailscale.com/util/usermetric from tailscale.com/health tailscale.com/util/vizerror from tailscale.com/tailcfg+ W 💣 tailscale.com/util/winutil from tailscale.com/clientupdate+ W 💣 tailscale.com/util/winutil/authenticode from tailscale.com/clientupdate + 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/client/web+ tailscale.com/version/distro from tailscale.com/client/web+ diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 26165d659..b3a4aa86f 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -401,8 +401,11 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/util/slicesx from tailscale.com/net/dns/recursive+ tailscale.com/util/syspolicy from tailscale.com/cmd/tailscaled+ tailscale.com/util/syspolicy/internal from tailscale.com/util/syspolicy/setting+ - tailscale.com/util/syspolicy/internal/loggerx from tailscale.com/util/syspolicy - tailscale.com/util/syspolicy/setting from tailscale.com/util/syspolicy + 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/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/sysresources from tailscale.com/wgengine/magicsock tailscale.com/util/systemd from tailscale.com/control/controlclient+ tailscale.com/util/testenv from tailscale.com/ipn/ipnlocal+ @@ -412,7 +415,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/util/vizerror from tailscale.com/tailcfg+ 💣 tailscale.com/util/winutil from tailscale.com/clientupdate+ W 💣 tailscale.com/util/winutil/authenticode from tailscale.com/clientupdate+ - W 💣 tailscale.com/util/winutil/gp from tailscale.com/net/dns + W 💣 tailscale.com/util/winutil/gp from tailscale.com/net/dns+ W tailscale.com/util/winutil/policy from tailscale.com/ipn/ipnlocal W 💣 tailscale.com/util/winutil/winenv from tailscale.com/hostinfo+ tailscale.com/util/zstdframe from tailscale.com/control/controlclient+ diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 9a8fa5e02..5fee5d00e 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -54,6 +54,8 @@ import ( "tailscale.com/util/must" "tailscale.com/util/set" "tailscale.com/util/syspolicy" + "tailscale.com/util/syspolicy/setting" + "tailscale.com/util/syspolicy/source" "tailscale.com/wgengine" "tailscale.com/wgengine/filter" "tailscale.com/wgengine/wgcfg" @@ -1559,94 +1561,6 @@ func dnsResponse(domain, address string) []byte { return must.Get(b.Finish()) } -type errorSyspolicyHandler struct { - t *testing.T - err error - key syspolicy.Key - allowKeys map[syspolicy.Key]*string -} - -func (h *errorSyspolicyHandler) ReadString(key string) (string, error) { - sk := syspolicy.Key(key) - if _, ok := h.allowKeys[sk]; !ok { - h.t.Errorf("ReadString: %q is not in list of permitted keys", h.key) - } - if sk == h.key { - return "", h.err - } - return "", syspolicy.ErrNoSuchKey -} - -func (h *errorSyspolicyHandler) ReadUInt64(key string) (uint64, error) { - h.t.Errorf("ReadUInt64(%q) unexpectedly called", key) - return 0, syspolicy.ErrNoSuchKey -} - -func (h *errorSyspolicyHandler) ReadBoolean(key string) (bool, error) { - h.t.Errorf("ReadBoolean(%q) unexpectedly called", key) - return false, syspolicy.ErrNoSuchKey -} - -func (h *errorSyspolicyHandler) ReadStringArray(key string) ([]string, error) { - h.t.Errorf("ReadStringArray(%q) unexpectedly called", key) - return nil, syspolicy.ErrNoSuchKey -} - -type mockSyspolicyHandler struct { - t *testing.T - // stringPolicies is the collection of policies that we expect to see - // queried by the current test. If the policy is expected but unset, then - // use nil, otherwise use a string equal to the policy's desired value. - stringPolicies map[syspolicy.Key]*string - // stringArrayPolicies is the collection of policies that we expected to see - // queries by the current test, that return policy string arrays. - stringArrayPolicies map[syspolicy.Key][]string - // failUnknownPolicies is set if policies other than those in stringPolicies - // (uint64 or bool policies are not supported by mockSyspolicyHandler yet) - // should be considered a test failure if they are queried. - failUnknownPolicies bool -} - -func (h *mockSyspolicyHandler) ReadString(key string) (string, error) { - if s, ok := h.stringPolicies[syspolicy.Key(key)]; ok { - if s == nil { - return "", syspolicy.ErrNoSuchKey - } - return *s, nil - } - if h.failUnknownPolicies { - h.t.Errorf("ReadString(%q) unexpectedly called", key) - } - return "", syspolicy.ErrNoSuchKey -} - -func (h *mockSyspolicyHandler) ReadUInt64(key string) (uint64, error) { - if h.failUnknownPolicies { - h.t.Errorf("ReadUInt64(%q) unexpectedly called", key) - } - return 0, syspolicy.ErrNoSuchKey -} - -func (h *mockSyspolicyHandler) ReadBoolean(key string) (bool, error) { - if h.failUnknownPolicies { - h.t.Errorf("ReadBoolean(%q) unexpectedly called", key) - } - return false, syspolicy.ErrNoSuchKey -} - -func (h *mockSyspolicyHandler) ReadStringArray(key string) ([]string, error) { - if h.failUnknownPolicies { - h.t.Errorf("ReadStringArray(%q) unexpectedly called", key) - } - if s, ok := h.stringArrayPolicies[syspolicy.Key(key)]; ok { - if s == nil { - return []string{}, syspolicy.ErrNoSuchKey - } - return s, nil - } - return nil, syspolicy.ErrNoSuchKey -} - func TestSetExitNodeIDPolicy(t *testing.T) { pfx := netip.MustParsePrefix tests := []struct { @@ -1856,23 +1770,18 @@ func TestSetExitNodeIDPolicy(t *testing.T) { }, } + syspolicy.RegisterWellKnownSettingsForTest(t) + for _, test := range tests { t.Run(test.name, func(t *testing.T) { b := newTestBackend(t) - msh := &mockSyspolicyHandler{ - t: t, - stringPolicies: map[syspolicy.Key]*string{ - syspolicy.ExitNodeID: nil, - syspolicy.ExitNodeIP: nil, - }, - } - if test.exitNodeIDKey { - msh.stringPolicies[syspolicy.ExitNodeID] = &test.exitNodeID - } - if test.exitNodeIPKey { - msh.stringPolicies[syspolicy.ExitNodeIP] = &test.exitNodeIP - } - syspolicy.SetHandlerForTest(t, msh) + + policyStore := source.NewTestStoreOf(t, + source.TestSettingOf(syspolicy.ExitNodeID, test.exitNodeID), + source.TestSettingOf(syspolicy.ExitNodeIP, test.exitNodeIP), + ) + syspolicy.MustRegisterStoreForTest(t, "TestStore", setting.DeviceScope, policyStore) + if test.nm == nil { test.nm = new(netmap.NetworkMap) } @@ -1994,13 +1903,13 @@ func TestUpdateNetmapDeltaAutoExitNode(t *testing.T) { report: report, }, } - msh := &mockSyspolicyHandler{ - t: t, - stringPolicies: map[syspolicy.Key]*string{ - syspolicy.ExitNodeID: ptr.To("auto:any"), - }, - } - syspolicy.SetHandlerForTest(t, msh) + + syspolicy.RegisterWellKnownSettingsForTest(t) + policyStore := source.NewTestStoreOf(t, source.TestSettingOf( + syspolicy.ExitNodeID, "auto:any", + )) + syspolicy.MustRegisterStoreForTest(t, "TestStore", setting.DeviceScope, policyStore) + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { b := newTestLocalBackend(t) @@ -2049,13 +1958,11 @@ func TestAutoExitNodeSetNetInfoCallback(t *testing.T) { } cc = newClient(t, opts) b.cc = cc - msh := &mockSyspolicyHandler{ - t: t, - stringPolicies: map[syspolicy.Key]*string{ - syspolicy.ExitNodeID: ptr.To("auto:any"), - }, - } - syspolicy.SetHandlerForTest(t, msh) + syspolicy.RegisterWellKnownSettingsForTest(t) + policyStore := source.NewTestStoreOf(t, source.TestSettingOf( + syspolicy.ExitNodeID, "auto:any", + )) + syspolicy.MustRegisterStoreForTest(t, "TestStore", setting.DeviceScope, policyStore) peer1 := makePeer(1, withCap(26), withDERP(3), withSuggest(), withExitRoutes()) peer2 := makePeer(2, withCap(26), withDERP(2), withSuggest(), withExitRoutes()) selfNode := tailcfg.Node{ @@ -2160,13 +2067,11 @@ func TestSetControlClientStatusAutoExitNode(t *testing.T) { DERPMap: derpMap, } b := newTestLocalBackend(t) - msh := &mockSyspolicyHandler{ - t: t, - stringPolicies: map[syspolicy.Key]*string{ - syspolicy.ExitNodeID: ptr.To("auto:any"), - }, - } - syspolicy.SetHandlerForTest(t, msh) + syspolicy.RegisterWellKnownSettingsForTest(t) + policyStore := source.NewTestStoreOf(t, source.TestSettingOf( + syspolicy.ExitNodeID, "auto:any", + )) + syspolicy.MustRegisterStoreForTest(t, "TestStore", setting.DeviceScope, policyStore) b.netMap = nm b.lastSuggestedExitNode = peer1.StableID() b.sys.MagicSock.Get().SetLastNetcheckReportForTest(b.ctx, report) @@ -2400,17 +2305,16 @@ func TestApplySysPolicy(t *testing.T) { }, } + syspolicy.RegisterWellKnownSettingsForTest(t) + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - msh := &mockSyspolicyHandler{ - t: t, - stringPolicies: make(map[syspolicy.Key]*string, len(tt.stringPolicies)), - } + settings := make([]source.TestSetting[string], 0, len(tt.stringPolicies)) for p, v := range tt.stringPolicies { - v := v // construct a unique pointer for each policy value - msh.stringPolicies[p] = &v + settings = append(settings, source.TestSettingOf(p, v)) } - syspolicy.SetHandlerForTest(t, msh) + policyStore := source.NewTestStoreOf(t, settings...) + syspolicy.MustRegisterStoreForTest(t, "TestStore", setting.DeviceScope, policyStore) t.Run("unit", func(t *testing.T) { prefs := tt.prefs.Clone() @@ -2546,35 +2450,19 @@ func TestPreferencePolicyInfo(t *testing.T) { }, } + syspolicy.RegisterWellKnownSettingsForTest(t) + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { for _, pp := range preferencePolicies { t.Run(string(pp.key), func(t *testing.T) { - var h syspolicy.Handler - - allPolicies := make(map[syspolicy.Key]*string, len(preferencePolicies)+1) - allPolicies[syspolicy.ControlURL] = nil - for _, pp := range preferencePolicies { - allPolicies[pp.key] = nil + s := source.TestSetting[string]{ + Key: pp.key, + Error: tt.policyError, + Value: tt.policyValue, } - - if tt.policyError != nil { - h = &errorSyspolicyHandler{ - t: t, - err: tt.policyError, - key: pp.key, - allowKeys: allPolicies, - } - } else { - msh := &mockSyspolicyHandler{ - t: t, - stringPolicies: allPolicies, - failUnknownPolicies: true, - } - msh.stringPolicies[pp.key] = &tt.policyValue - h = msh - } - syspolicy.SetHandlerForTest(t, h) + policyStore := source.NewTestStoreOf(t, s) + syspolicy.MustRegisterStoreForTest(t, "TestStore", setting.DeviceScope, policyStore) prefs := defaultPrefs.AsStruct() pp.set(prefs, tt.initialValue) @@ -3825,15 +3713,16 @@ func TestShouldAutoExitNode(t *testing.T) { expectedBool: false, }, } + + syspolicy.RegisterWellKnownSettingsForTest(t) + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - msh := &mockSyspolicyHandler{ - t: t, - stringPolicies: map[syspolicy.Key]*string{ - syspolicy.ExitNodeID: ptr.To(tt.exitNodeIDPolicyValue), - }, - } - syspolicy.SetHandlerForTest(t, msh) + policyStore := source.NewTestStoreOf(t, source.TestSettingOf( + syspolicy.ExitNodeID, tt.exitNodeIDPolicyValue, + )) + syspolicy.MustRegisterStoreForTest(t, "TestStore", setting.DeviceScope, policyStore) + got := shouldAutoExitNode() if got != tt.expectedBool { t.Fatalf("expected %v got %v for %v policy value", tt.expectedBool, got, tt.exitNodeIDPolicyValue) @@ -3971,17 +3860,13 @@ func TestFillAllowedSuggestions(t *testing.T) { want: []tailcfg.StableNodeID{"ABC", "def", "gHiJ"}, }, } + syspolicy.RegisterWellKnownSettingsForTest(t) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - mh := mockSyspolicyHandler{ - t: t, - } - if tt.allowPolicy != nil { - mh.stringArrayPolicies = map[syspolicy.Key][]string{ - syspolicy.AllowedSuggestedExitNodes: tt.allowPolicy, - } - } - syspolicy.SetHandlerForTest(t, &mh) + policyStore := source.NewTestStoreOf(t, source.TestSettingOf( + syspolicy.AllowedSuggestedExitNodes, tt.allowPolicy, + )) + syspolicy.MustRegisterStoreForTest(t, "TestStore", setting.DeviceScope, policyStore) got := fillAllowedSuggestions() if got == nil { diff --git a/util/syspolicy/caching_handler.go b/util/syspolicy/caching_handler.go deleted file mode 100644 index 5192958bc..000000000 --- a/util/syspolicy/caching_handler.go +++ /dev/null @@ -1,122 +0,0 @@ -// Copyright (c) Tailscale Inc & AUTHORS -// SPDX-License-Identifier: BSD-3-Clause - -package syspolicy - -import ( - "errors" - "sync" -) - -// CachingHandler is a handler that reads policies from an underlying handler the first time each key is requested -// and permanently caches the result unless there is an error. If there is an ErrNoSuchKey error, that result is cached, -// otherwise the actual error is returned and the next read for that key will retry using the handler. -type CachingHandler struct { - mu sync.Mutex - strings map[string]string - uint64s map[string]uint64 - bools map[string]bool - strArrs map[string][]string - notFound map[string]bool - handler Handler -} - -// NewCachingHandler creates a CachingHandler given a handler. -func NewCachingHandler(handler Handler) *CachingHandler { - return &CachingHandler{ - handler: handler, - strings: make(map[string]string), - uint64s: make(map[string]uint64), - bools: make(map[string]bool), - strArrs: make(map[string][]string), - notFound: make(map[string]bool), - } -} - -// ReadString reads the policy settings value string given the key. -// ReadString first reads from the handler's cache before resorting to using the handler. -func (ch *CachingHandler) ReadString(key string) (string, error) { - ch.mu.Lock() - defer ch.mu.Unlock() - if val, ok := ch.strings[key]; ok { - return val, nil - } - if notFound := ch.notFound[key]; notFound { - return "", ErrNoSuchKey - } - val, err := ch.handler.ReadString(key) - if errors.Is(err, ErrNoSuchKey) { - ch.notFound[key] = true - return "", err - } else if err != nil { - return "", err - } - ch.strings[key] = val - return val, nil -} - -// ReadUInt64 reads the policy settings uint64 value given the key. -// ReadUInt64 first reads from the handler's cache before resorting to using the handler. -func (ch *CachingHandler) ReadUInt64(key string) (uint64, error) { - ch.mu.Lock() - defer ch.mu.Unlock() - if val, ok := ch.uint64s[key]; ok { - return val, nil - } - if notFound := ch.notFound[key]; notFound { - return 0, ErrNoSuchKey - } - val, err := ch.handler.ReadUInt64(key) - if errors.Is(err, ErrNoSuchKey) { - ch.notFound[key] = true - return 0, err - } else if err != nil { - return 0, err - } - ch.uint64s[key] = val - return val, nil -} - -// ReadBoolean reads the policy settings boolean value given the key. -// ReadBoolean first reads from the handler's cache before resorting to using the handler. -func (ch *CachingHandler) ReadBoolean(key string) (bool, error) { - ch.mu.Lock() - defer ch.mu.Unlock() - if val, ok := ch.bools[key]; ok { - return val, nil - } - if notFound := ch.notFound[key]; notFound { - return false, ErrNoSuchKey - } - val, err := ch.handler.ReadBoolean(key) - if errors.Is(err, ErrNoSuchKey) { - ch.notFound[key] = true - return false, err - } else if err != nil { - return false, err - } - ch.bools[key] = val - return val, nil -} - -// ReadBoolean reads the policy settings boolean value given the key. -// ReadBoolean first reads from the handler's cache before resorting to using the handler. -func (ch *CachingHandler) ReadStringArray(key string) ([]string, error) { - ch.mu.Lock() - defer ch.mu.Unlock() - if val, ok := ch.strArrs[key]; ok { - return val, nil - } - if notFound := ch.notFound[key]; notFound { - return nil, ErrNoSuchKey - } - val, err := ch.handler.ReadStringArray(key) - if errors.Is(err, ErrNoSuchKey) { - ch.notFound[key] = true - return nil, err - } else if err != nil { - return nil, err - } - ch.strArrs[key] = val - return val, nil -} diff --git a/util/syspolicy/caching_handler_test.go b/util/syspolicy/caching_handler_test.go deleted file mode 100644 index 881f6ff83..000000000 --- a/util/syspolicy/caching_handler_test.go +++ /dev/null @@ -1,262 +0,0 @@ -// Copyright (c) Tailscale Inc & AUTHORS -// SPDX-License-Identifier: BSD-3-Clause - -package syspolicy - -import ( - "testing" -) - -func TestHandlerReadString(t *testing.T) { - tests := []struct { - name string - key string - handlerKey Key - handlerValue string - handlerError error - preserveHandler bool - wantValue string - wantErr error - strings map[string]string - expectedCalls int - }{ - { - name: "read existing cached values", - key: "test", - handlerKey: "do not read", - strings: map[string]string{"test": "foo"}, - wantValue: "foo", - expectedCalls: 0, - }, - { - name: "read existing values not cached", - key: "test", - handlerKey: "test", - handlerValue: "foo", - wantValue: "foo", - expectedCalls: 1, - }, - { - name: "error no such key", - key: "test", - handlerKey: "test", - handlerError: ErrNoSuchKey, - wantErr: ErrNoSuchKey, - expectedCalls: 1, - }, - { - name: "other error", - key: "test", - handlerKey: "test", - handlerError: someOtherError, - wantErr: someOtherError, - preserveHandler: true, - expectedCalls: 2, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - testHandler := &testHandler{ - t: t, - key: tt.handlerKey, - s: tt.handlerValue, - err: tt.handlerError, - } - cache := NewCachingHandler(testHandler) - if tt.strings != nil { - cache.strings = tt.strings - } - got, err := cache.ReadString(tt.key) - if err != tt.wantErr { - t.Errorf("err=%v want %v", err, tt.wantErr) - } - if got != tt.wantValue { - t.Errorf("got %v want %v", got, cache.strings[tt.key]) - } - if !tt.preserveHandler { - testHandler.key, testHandler.s, testHandler.err = "do not read", "", nil - } - got, err = cache.ReadString(tt.key) - if err != tt.wantErr { - t.Errorf("repeat err=%v want %v", err, tt.wantErr) - } - if got != tt.wantValue { - t.Errorf("repeat got %v want %v", got, cache.strings[tt.key]) - } - if testHandler.calls != tt.expectedCalls { - t.Errorf("calls=%v want %v", testHandler.calls, tt.expectedCalls) - } - }) - } -} - -func TestHandlerReadUint64(t *testing.T) { - tests := []struct { - name string - key string - handlerKey Key - handlerValue uint64 - handlerError error - preserveHandler bool - wantValue uint64 - wantErr error - uint64s map[string]uint64 - expectedCalls int - }{ - { - name: "read existing cached values", - key: "test", - handlerKey: "do not read", - uint64s: map[string]uint64{"test": 1}, - wantValue: 1, - expectedCalls: 0, - }, - { - name: "read existing values not cached", - key: "test", - handlerKey: "test", - handlerValue: 1, - wantValue: 1, - expectedCalls: 1, - }, - { - name: "error no such key", - key: "test", - handlerKey: "test", - handlerError: ErrNoSuchKey, - wantErr: ErrNoSuchKey, - expectedCalls: 1, - }, - { - name: "other error", - key: "test", - handlerKey: "test", - handlerError: someOtherError, - wantErr: someOtherError, - preserveHandler: true, - expectedCalls: 2, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - testHandler := &testHandler{ - t: t, - key: tt.handlerKey, - u64: tt.handlerValue, - err: tt.handlerError, - } - cache := NewCachingHandler(testHandler) - if tt.uint64s != nil { - cache.uint64s = tt.uint64s - } - got, err := cache.ReadUInt64(tt.key) - if err != tt.wantErr { - t.Errorf("err=%v want %v", err, tt.wantErr) - } - if got != tt.wantValue { - t.Errorf("got %v want %v", got, cache.strings[tt.key]) - } - if !tt.preserveHandler { - testHandler.key, testHandler.s, testHandler.err = "do not read", "", nil - } - got, err = cache.ReadUInt64(tt.key) - if err != tt.wantErr { - t.Errorf("repeat err=%v want %v", err, tt.wantErr) - } - if got != tt.wantValue { - t.Errorf("repeat got %v want %v", got, cache.strings[tt.key]) - } - if testHandler.calls != tt.expectedCalls { - t.Errorf("calls=%v want %v", testHandler.calls, tt.expectedCalls) - } - }) - } - -} - -func TestHandlerReadBool(t *testing.T) { - tests := []struct { - name string - key string - handlerKey Key - handlerValue bool - handlerError error - preserveHandler bool - wantValue bool - wantErr error - bools map[string]bool - expectedCalls int - }{ - { - name: "read existing cached values", - key: "test", - handlerKey: "do not read", - bools: map[string]bool{"test": true}, - wantValue: true, - expectedCalls: 0, - }, - { - name: "read existing values not cached", - key: "test", - handlerKey: "test", - handlerValue: true, - wantValue: true, - expectedCalls: 1, - }, - { - name: "error no such key", - key: "test", - handlerKey: "test", - handlerError: ErrNoSuchKey, - wantErr: ErrNoSuchKey, - expectedCalls: 1, - }, - { - name: "other error", - key: "test", - handlerKey: "test", - handlerError: someOtherError, - wantErr: someOtherError, - preserveHandler: true, - expectedCalls: 2, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - testHandler := &testHandler{ - t: t, - key: tt.handlerKey, - b: tt.handlerValue, - err: tt.handlerError, - } - cache := NewCachingHandler(testHandler) - if tt.bools != nil { - cache.bools = tt.bools - } - got, err := cache.ReadBoolean(tt.key) - if err != tt.wantErr { - t.Errorf("err=%v want %v", err, tt.wantErr) - } - if got != tt.wantValue { - t.Errorf("got %v want %v", got, cache.strings[tt.key]) - } - if !tt.preserveHandler { - testHandler.key, testHandler.s, testHandler.err = "do not read", "", nil - } - got, err = cache.ReadBoolean(tt.key) - if err != tt.wantErr { - t.Errorf("repeat err=%v want %v", err, tt.wantErr) - } - if got != tt.wantValue { - t.Errorf("repeat got %v want %v", got, cache.strings[tt.key]) - } - if testHandler.calls != tt.expectedCalls { - t.Errorf("calls=%v want %v", testHandler.calls, tt.expectedCalls) - } - }) - } - -} diff --git a/util/syspolicy/handler.go b/util/syspolicy/handler.go index f1fad9770..f511f0a56 100644 --- a/util/syspolicy/handler.go +++ b/util/syspolicy/handler.go @@ -4,16 +4,17 @@ package syspolicy import ( - "errors" - "sync/atomic" + "tailscale.com/util/syspolicy/internal" + "tailscale.com/util/syspolicy/rsop" + "tailscale.com/util/syspolicy/setting" + "tailscale.com/util/syspolicy/source" ) -var ( - handlerUsed atomic.Bool - handler Handler = defaultHandler{} -) +// TODO(nickkhyl): delete this file once other repos are updated. // Handler reads system policies from OS-specific storage. +// +// Deprecated: implementing a [source.Store] should be preferred. type Handler interface { // ReadString reads the policy setting's string value for the given key. // It should return ErrNoSuchKey if the key does not have a value set. @@ -29,55 +30,88 @@ type Handler interface { ReadStringArray(key string) ([]string, error) } -// ErrNoSuchKey is returned by a Handler when the specified key does not have a -// value set. -var ErrNoSuchKey = errors.New("no such key") - -// defaultHandler is the catch all syspolicy type for anything that isn't windows or apple. -type defaultHandler struct{} - -func (defaultHandler) ReadString(_ string) (string, error) { - return "", ErrNoSuchKey -} - -func (defaultHandler) ReadUInt64(_ string) (uint64, error) { - return 0, ErrNoSuchKey -} - -func (defaultHandler) ReadBoolean(_ string) (bool, error) { - return false, ErrNoSuchKey -} - -func (defaultHandler) ReadStringArray(_ string) ([]string, error) { - return nil, ErrNoSuchKey -} - -// markHandlerInUse is called before handler methods are called. -func markHandlerInUse() { - handlerUsed.Store(true) -} - -// RegisterHandler initializes the policy handler and ensures registration will happen once. +// RegisterHandler wraps and registers the specified handler as the device's +// policy [source.Store] for the program's lifetime. +// +// Deprecated: using [RegisterStore] should be preferred. func RegisterHandler(h Handler) { - // Technically this assignment is not concurrency safe, but in the - // event that there was any risk of a data race, we will panic due to - // the CompareAndSwap failing. - handler = h - if !handlerUsed.CompareAndSwap(false, true) { - panic("handler was already used before registration") - } + rsop.RegisterStore("DeviceHandler", setting.DeviceScope, WrapHandler(h)) } // TB is a subset of testing.TB that we use to set up test helpers. // It's defined here to avoid pulling in the testing package. -type TB interface { - Helper() - Cleanup(func()) +type TB = internal.TB + +// SetHandlerForTest wraps and sets the specified handler as the device's policy +// [source.Store] for the duration of tb. +// +// Deprecated: using [MustRegisterStoreForTest] should be preferred. +func SetHandlerForTest(tb TB, h Handler) { + RegisterWellKnownSettingsForTest(tb) + MustRegisterStoreForTest(tb, "DeviceHandler-TestOnly", setting.DefaultScope(), WrapHandler(h)) } -func SetHandlerForTest(tb TB, h Handler) { - tb.Helper() - oldHandler := handler - handler = h - tb.Cleanup(func() { handler = oldHandler }) +var _ source.Store = (*handlerStore)(nil) + +// handlerStore is a [source.Store] that calls the underlying [Handler]. +// +// TODO(nickkhyl): remove it when the corp and android repos are updated. +type handlerStore struct { + h Handler +} + +// WrapHandler returns a [source.Store] that wraps the specified [Handler]. +func WrapHandler(h Handler) source.Store { + return handlerStore{h} +} + +// Lock implements [source.Lockable]. +func (s handlerStore) Lock() error { + if lockable, ok := s.h.(source.Lockable); ok { + return lockable.Lock() + } + return nil +} + +// Unlock implements [source.Lockable]. +func (s handlerStore) Unlock() { + if lockable, ok := s.h.(source.Lockable); ok { + lockable.Unlock() + } +} + +// RegisterChangeCallback implements [source.Changeable]. +func (s handlerStore) RegisterChangeCallback(callback func()) (unregister func(), err error) { + if changeable, ok := s.h.(source.Changeable); ok { + return changeable.RegisterChangeCallback(callback) + } + return func() {}, nil +} + +// ReadString implements [source.Store]. +func (s handlerStore) ReadString(key setting.Key) (string, error) { + return s.h.ReadString(string(key)) +} + +// ReadUInt64 implements [source.Store]. +func (s handlerStore) ReadUInt64(key setting.Key) (uint64, error) { + return s.h.ReadUInt64(string(key)) +} + +// ReadBoolean implements [source.Store]. +func (s handlerStore) ReadBoolean(key setting.Key) (bool, error) { + return s.h.ReadBoolean(string(key)) +} + +// ReadStringArray implements [source.Store]. +func (s handlerStore) ReadStringArray(key setting.Key) ([]string, error) { + return s.h.ReadStringArray(string(key)) +} + +// Done implements [source.Expirable]. +func (s handlerStore) Done() <-chan struct{} { + if expirable, ok := s.h.(source.Expirable); ok { + return expirable.Done() + } + return nil } diff --git a/util/syspolicy/handler_test.go b/util/syspolicy/handler_test.go deleted file mode 100644 index 39b18936f..000000000 --- a/util/syspolicy/handler_test.go +++ /dev/null @@ -1,19 +0,0 @@ -// Copyright (c) Tailscale Inc & AUTHORS -// SPDX-License-Identifier: BSD-3-Clause - -package syspolicy - -import "testing" - -func TestDefaultHandlerReadValues(t *testing.T) { - var h defaultHandler - - got, err := h.ReadString(string(AdminConsoleVisibility)) - if got != "" || err != ErrNoSuchKey { - t.Fatalf("got %v err %v", got, err) - } - result, err := h.ReadUInt64(string(LogSCMInteractions)) - if result != 0 || err != ErrNoSuchKey { - t.Fatalf("got %v err %v", result, err) - } -} diff --git a/util/syspolicy/handler_windows.go b/util/syspolicy/handler_windows.go deleted file mode 100644 index 661853ead..000000000 --- a/util/syspolicy/handler_windows.go +++ /dev/null @@ -1,105 +0,0 @@ -// Copyright (c) Tailscale Inc & AUTHORS -// SPDX-License-Identifier: BSD-3-Clause - -package syspolicy - -import ( - "errors" - "fmt" - - "tailscale.com/util/clientmetric" - "tailscale.com/util/winutil" -) - -var ( - windowsErrors = clientmetric.NewCounter("windows_syspolicy_errors") - windowsAny = clientmetric.NewGauge("windows_syspolicy_any") -) - -type windowsHandler struct{} - -func init() { - RegisterHandler(NewCachingHandler(windowsHandler{})) - - keyList := []struct { - isSet func(Key) bool - keys []Key - }{ - { - isSet: func(k Key) bool { - _, err := handler.ReadString(string(k)) - return err == nil - }, - keys: stringKeys, - }, - { - isSet: func(k Key) bool { - _, err := handler.ReadBoolean(string(k)) - return err == nil - }, - keys: boolKeys, - }, - { - isSet: func(k Key) bool { - _, err := handler.ReadUInt64(string(k)) - return err == nil - }, - keys: uint64Keys, - }, - } - - var anySet bool - for _, l := range keyList { - for _, k := range l.keys { - if !l.isSet(k) { - continue - } - clientmetric.NewGauge(fmt.Sprintf("windows_syspolicy_%s", k)).Set(1) - anySet = true - } - } - if anySet { - windowsAny.Set(1) - } -} - -func (windowsHandler) ReadString(key string) (string, error) { - s, err := winutil.GetPolicyString(key) - if errors.Is(err, winutil.ErrNoValue) { - err = ErrNoSuchKey - } else if err != nil { - windowsErrors.Add(1) - } - - return s, err -} - -func (windowsHandler) ReadUInt64(key string) (uint64, error) { - value, err := winutil.GetPolicyInteger(key) - if errors.Is(err, winutil.ErrNoValue) { - err = ErrNoSuchKey - } else if err != nil { - windowsErrors.Add(1) - } - return value, err -} - -func (windowsHandler) ReadBoolean(key string) (bool, error) { - value, err := winutil.GetPolicyInteger(key) - if errors.Is(err, winutil.ErrNoValue) { - err = ErrNoSuchKey - } else if err != nil { - windowsErrors.Add(1) - } - return value != 0, err -} - -func (windowsHandler) ReadStringArray(key string) ([]string, error) { - value, err := winutil.GetPolicyStringArray(key) - if errors.Is(err, winutil.ErrNoValue) { - err = ErrNoSuchKey - } else if err != nil { - windowsErrors.Add(1) - } - return value, err -} diff --git a/util/syspolicy/policy_keys.go b/util/syspolicy/policy_keys.go index ec0556a94..162885b27 100644 --- a/util/syspolicy/policy_keys.go +++ b/util/syspolicy/policy_keys.go @@ -3,10 +3,24 @@ package syspolicy -import "tailscale.com/util/syspolicy/setting" +import ( + "tailscale.com/types/lazy" + "tailscale.com/util/syspolicy/internal" + "tailscale.com/util/syspolicy/setting" + "tailscale.com/util/testenv" +) +// Key is a string that uniquely identifies a policy and must remain unchanged +// once established and documented for a given policy setting. It may contain +// alphanumeric characters and zero or more [KeyPathSeparator]s to group +// individual policy settings into categories. type Key = setting.Key +// The const block below lists known policy keys. +// When adding a key to this list, remember to add a corresponding +// [setting.Definition] to [implicitDefinitions] below. +// Otherwise, the [TestKnownKeysRegistered] test will fail as a reminder. + const ( // Keys with a string value ControlURL Key = "LoginURL" // default ""; if blank, ipn uses ipn.DefaultControlURL. @@ -110,3 +124,90 @@ const ( // AllowedSuggestedExitNodes's string array value is a list of exit node IDs that restricts which exit nodes are considered when generating suggestions for exit nodes. AllowedSuggestedExitNodes Key = "AllowedSuggestedExitNodes" ) + +// implicitDefinitions is a list of [setting.Definition] that will be registered +// automatically when the policy setting definitions are first used by the syspolicy package hierarchy. +// This includes the first time a policy needs to be read from any source. +var implicitDefinitions = []*setting.Definition{ + // Device policy settings (can only be configured on a per-device basis): + setting.NewDefinition(AllowedSuggestedExitNodes, setting.DeviceSetting, setting.StringListValue), + setting.NewDefinition(ApplyUpdates, setting.DeviceSetting, setting.PreferenceOptionValue), + setting.NewDefinition(AuthKey, setting.DeviceSetting, setting.StringValue), + setting.NewDefinition(CheckUpdates, setting.DeviceSetting, setting.PreferenceOptionValue), + setting.NewDefinition(ControlURL, setting.DeviceSetting, setting.StringValue), + setting.NewDefinition(DeviceSerialNumber, setting.DeviceSetting, setting.StringValue), + setting.NewDefinition(EnableIncomingConnections, setting.DeviceSetting, setting.PreferenceOptionValue), + setting.NewDefinition(EnableRunExitNode, setting.DeviceSetting, setting.PreferenceOptionValue), + setting.NewDefinition(EnableServerMode, setting.DeviceSetting, setting.PreferenceOptionValue), + setting.NewDefinition(EnableTailscaleDNS, setting.DeviceSetting, setting.PreferenceOptionValue), + setting.NewDefinition(EnableTailscaleSubnets, setting.DeviceSetting, setting.PreferenceOptionValue), + setting.NewDefinition(ExitNodeAllowLANAccess, setting.DeviceSetting, setting.PreferenceOptionValue), + setting.NewDefinition(ExitNodeID, setting.DeviceSetting, setting.StringValue), + setting.NewDefinition(ExitNodeIP, setting.DeviceSetting, setting.StringValue), + setting.NewDefinition(FlushDNSOnSessionUnlock, setting.DeviceSetting, setting.BooleanValue), + setting.NewDefinition(LogSCMInteractions, setting.DeviceSetting, setting.BooleanValue), + setting.NewDefinition(LogTarget, setting.DeviceSetting, setting.StringValue), + setting.NewDefinition(MachineCertificateSubject, setting.DeviceSetting, setting.StringValue), + setting.NewDefinition(PostureChecking, setting.DeviceSetting, setting.PreferenceOptionValue), + setting.NewDefinition(Tailnet, setting.DeviceSetting, setting.StringValue), + + // User policy settings (can be configured on a user- or device-basis): + setting.NewDefinition(AdminConsoleVisibility, setting.UserSetting, setting.VisibilityValue), + setting.NewDefinition(AutoUpdateVisibility, setting.UserSetting, setting.VisibilityValue), + setting.NewDefinition(ExitNodeMenuVisibility, setting.UserSetting, setting.VisibilityValue), + setting.NewDefinition(KeyExpirationNoticeTime, setting.UserSetting, setting.DurationValue), + setting.NewDefinition(ManagedByCaption, setting.UserSetting, setting.StringValue), + setting.NewDefinition(ManagedByOrganizationName, setting.UserSetting, setting.StringValue), + setting.NewDefinition(ManagedByURL, setting.UserSetting, setting.StringValue), + setting.NewDefinition(NetworkDevicesVisibility, setting.UserSetting, setting.VisibilityValue), + setting.NewDefinition(PreferencesMenuVisibility, setting.UserSetting, setting.VisibilityValue), + setting.NewDefinition(ResetToDefaultsVisibility, setting.UserSetting, setting.VisibilityValue), + setting.NewDefinition(RunExitNodeVisibility, setting.UserSetting, setting.VisibilityValue), + setting.NewDefinition(SuggestedExitNodeVisibility, setting.UserSetting, setting.VisibilityValue), + setting.NewDefinition(TestMenuVisibility, setting.UserSetting, setting.VisibilityValue), + setting.NewDefinition(UpdateMenuVisibility, setting.UserSetting, setting.VisibilityValue), +} + +func init() { + internal.Init.MustDefer(func() error { + // Avoid implicit [setting.Definition] registration during tests. + // Each test should control which policy settings to register. + // Use [setting.SetDefinitionsForTest] to specify necessary definitions, + // or [setWellKnownSettingsForTest] to set implicit definitions for the test duration. + if testenv.InTest() { + return nil + } + for _, d := range implicitDefinitions { + setting.RegisterDefinition(d) + } + return nil + }) +} + +var implicitDefinitionMap lazy.SyncValue[setting.DefinitionMap] + +// WellKnownSettingDefinition returns a well-known, implicit setting definition by its key, +// or an [ErrNoSuchKey] if a policy setting with the specified key does not exist +// among implicit policy definitions. +func WellKnownSettingDefinition(k Key) (*setting.Definition, error) { + m, err := implicitDefinitionMap.GetErr(func() (setting.DefinitionMap, error) { + return setting.DefinitionMapOf(implicitDefinitions) + }) + if err != nil { + return nil, err + } + if d, ok := m[k]; ok { + return d, nil + } + return nil, ErrNoSuchKey +} + +// RegisterWellKnownSettingsForTest registers all implicit setting definitions +// for the duration of the test. +func RegisterWellKnownSettingsForTest(tb TB) { + tb.Helper() + err := setting.SetDefinitionsForTest(tb, implicitDefinitions...) + if err != nil { + tb.Fatalf("Failed to register well-known settings: %v", err) + } +} diff --git a/util/syspolicy/policy_keys_test.go b/util/syspolicy/policy_keys_test.go new file mode 100644 index 000000000..4d3260f3e --- /dev/null +++ b/util/syspolicy/policy_keys_test.go @@ -0,0 +1,95 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +package syspolicy + +import ( + "fmt" + "go/ast" + "go/parser" + "go/token" + "go/types" + "os" + "reflect" + "strconv" + "testing" + + "tailscale.com/util/syspolicy/setting" +) + +func TestKnownKeysRegistered(t *testing.T) { + keyConsts, err := listStringConsts[Key]("policy_keys.go") + if err != nil { + t.Fatalf("listStringConsts failed: %v", err) + } + + m, err := setting.DefinitionMapOf(implicitDefinitions) + if err != nil { + t.Fatalf("definitionMapOf failed: %v", err) + } + + for _, key := range keyConsts { + t.Run(string(key), func(t *testing.T) { + d := m[key] + if d == nil { + t.Fatalf("%q was not registered", key) + } + if d.Key() != key { + t.Fatalf("d.Key got: %s, want %s", d.Key(), key) + } + }) + } +} + +func TestNotAWellKnownSetting(t *testing.T) { + d, err := WellKnownSettingDefinition("TestSettingDoesNotExist") + if d != nil || err == nil { + t.Fatalf("got %v, %v; want nil, %v", d, err, ErrNoSuchKey) + } +} + +func listStringConsts[T ~string](filename string) (map[string]T, error) { + fset := token.NewFileSet() + src, err := os.ReadFile(filename) + if err != nil { + return nil, err + } + + f, err := parser.ParseFile(fset, filename, src, 0) + if err != nil { + return nil, err + } + + consts := make(map[string]T) + typeName := reflect.TypeFor[T]().Name() + for _, d := range f.Decls { + g, ok := d.(*ast.GenDecl) + if !ok || g.Tok != token.CONST { + continue + } + + for _, s := range g.Specs { + vs, ok := s.(*ast.ValueSpec) + if !ok || len(vs.Names) != len(vs.Values) { + continue + } + if typ, ok := vs.Type.(*ast.Ident); !ok || typ.Name != typeName { + continue + } + + for i, n := range vs.Names { + lit, ok := vs.Values[i].(*ast.BasicLit) + if !ok { + return nil, fmt.Errorf("unexpected string literal: %v = %v", n.Name, types.ExprString(vs.Values[i])) + } + val, err := strconv.Unquote(lit.Value) + if err != nil { + return nil, fmt.Errorf("unexpected string literal: %v = %v", n.Name, lit.Value) + } + consts[n.Name] = T(val) + } + } + } + + return consts, nil +} diff --git a/util/syspolicy/policy_keys_windows.go b/util/syspolicy/policy_keys_windows.go deleted file mode 100644 index 5e9a71695..000000000 --- a/util/syspolicy/policy_keys_windows.go +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright (c) Tailscale Inc & AUTHORS -// SPDX-License-Identifier: BSD-3-Clause - -package syspolicy - -var stringKeys = []Key{ - ControlURL, - LogTarget, - Tailnet, - ExitNodeID, - ExitNodeIP, - EnableIncomingConnections, - EnableServerMode, - ExitNodeAllowLANAccess, - EnableTailscaleDNS, - EnableTailscaleSubnets, - AdminConsoleVisibility, - NetworkDevicesVisibility, - TestMenuVisibility, - UpdateMenuVisibility, - RunExitNodeVisibility, - PreferencesMenuVisibility, - ExitNodeMenuVisibility, - AutoUpdateVisibility, - ResetToDefaultsVisibility, - KeyExpirationNoticeTime, - PostureChecking, - ManagedByOrganizationName, - ManagedByCaption, - ManagedByURL, -} - -var boolKeys = []Key{ - LogSCMInteractions, - FlushDNSOnSessionUnlock, -} - -var uint64Keys = []Key{} diff --git a/util/syspolicy/syspolicy.go b/util/syspolicy/syspolicy.go index abe42ed90..d925731c3 100644 --- a/util/syspolicy/syspolicy.go +++ b/util/syspolicy/syspolicy.go @@ -1,51 +1,82 @@ // Copyright (c) Tailscale Inc & AUTHORS // SPDX-License-Identifier: BSD-3-Clause -// Package syspolicy provides functions to retrieve system settings of a device. +// 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 import ( "errors" + "fmt" + "reflect" "time" "tailscale.com/util/syspolicy/internal/loggerx" + "tailscale.com/util/syspolicy/rsop" "tailscale.com/util/syspolicy/setting" + "tailscale.com/util/syspolicy/source" ) +var ( + // ErrNotConfigured is returned when the requested policy setting is not configured. + ErrNotConfigured = setting.ErrNotConfigured + // ErrTypeMismatch is returned when there's a type mismatch between the actual type + // of the setting value and the expected type. + ErrTypeMismatch = setting.ErrTypeMismatch + // ErrNoSuchKey is returned by [setting.DefinitionOf] when no policy setting + // has been registered with the specified key. + // + // This error is also returned by a (now deprecated) [Handler] when the specified + // key does not have a value set. While the package maintains compatibility with this + // usage of ErrNoSuchKey, it is recommended to return [ErrNotConfigured] from newer + // [source.Store] implementations. + ErrNoSuchKey = setting.ErrNoSuchKey +) + +// RegisterStore registers a new policy [source.Store] with the specified name and [setting.PolicyScope]. +// +// It is a shorthand for [rsop.RegisterStore]. +func RegisterStore(name string, scope setting.PolicyScope, store source.Store) (*rsop.StoreRegistration, error) { + return rsop.RegisterStore(name, scope, store) +} + +// MustRegisterStoreForTest is like [rsop.RegisterStoreForTest], but it fails the test if the store could not be registered. +func MustRegisterStoreForTest(tb TB, name string, scope setting.PolicyScope, store source.Store) *rsop.StoreRegistration { + tb.Helper() + reg, err := rsop.RegisterStoreForTest(tb, name, scope, store) + if err != nil { + tb.Fatalf("Failed to register policy store %q as a %v policy source: %v", name, scope, err) + } + return reg +} + +// GetString returns a string policy setting with the specified key, +// or defaultValue if it does not exist. func GetString(key Key, defaultValue string) (string, error) { - markHandlerInUse() - v, err := handler.ReadString(string(key)) - if errors.Is(err, ErrNoSuchKey) { - return defaultValue, nil - } - return v, err + return getCurrentPolicySettingValue(key, defaultValue) } +// GetUint64 returns a numeric policy setting with the specified key, +// or defaultValue if it does not exist. func GetUint64(key Key, defaultValue uint64) (uint64, error) { - markHandlerInUse() - v, err := handler.ReadUInt64(string(key)) - if errors.Is(err, ErrNoSuchKey) { - return defaultValue, nil - } - return v, err + return getCurrentPolicySettingValue(key, defaultValue) } +// GetBoolean returns a boolean policy setting with the specified key, +// or defaultValue if it does not exist. func GetBoolean(key Key, defaultValue bool) (bool, error) { - markHandlerInUse() - v, err := handler.ReadBoolean(string(key)) - if errors.Is(err, ErrNoSuchKey) { - return defaultValue, nil - } - return v, err + return getCurrentPolicySettingValue(key, defaultValue) } +// GetStringArray returns a multi-string policy setting with the specified key, +// or defaultValue if it does not exist. func GetStringArray(key Key, defaultValue []string) ([]string, error) { - markHandlerInUse() - v, err := handler.ReadStringArray(string(key)) - if errors.Is(err, ErrNoSuchKey) { - return defaultValue, nil - } - return v, err + return getCurrentPolicySettingValue(key, defaultValue) } // GetPreferenceOption loads a policy from the registry that can be @@ -55,13 +86,7 @@ func GetStringArray(key Key, defaultValue []string) ([]string, error) { // "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 Key) (setting.PreferenceOption, error) { - s, err := GetString(name, "user-decides") - if err != nil { - return setting.ShowChoiceByPolicy, err - } - var opt setting.PreferenceOption - err = opt.UnmarshalText([]byte(s)) - return opt, err + return getCurrentPolicySettingValue(name, setting.ShowChoiceByPolicy) } // GetVisibility loads a policy from the registry that can be managed @@ -70,13 +95,7 @@ func GetPreferenceOption(name Key) (setting.PreferenceOption, error) { // true) or "hide" (return true). If not present or set to a different value, // "show" (return false) is the default. func GetVisibility(name Key) (setting.Visibility, error) { - s, err := GetString(name, "show") - if err != nil { - return setting.VisibleByPolicy, err - } - var visibility setting.Visibility - visibility.UnmarshalText([]byte(s)) - return visibility, nil + return getCurrentPolicySettingValue(name, setting.VisibleByPolicy) } // GetDuration loads a policy from the registry that can be managed @@ -85,15 +104,58 @@ func GetVisibility(name Key) (setting.Visibility, error) { // understands. If the registry value is "" or can not be processed, // defaultValue is returned instead. func GetDuration(name Key, defaultValue time.Duration) (time.Duration, error) { - opt, err := GetString(name, "") - if opt == "" || err != nil { - return defaultValue, err + d, err := getCurrentPolicySettingValue(name, defaultValue) + if err != nil { + return d, err } - v, err := time.ParseDuration(opt) - if err != nil || v < 0 { + if d < 0 { return defaultValue, nil } - return v, nil + return d, nil +} + +// 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) { + effective, err := rsop.PolicyFor(setting.DefaultScope()) + if err != nil { + return nil, err + } + return effective.RegisterChangeCallback(cb), nil +} + +// getCurrentPolicySettingValue returns the value of the policy setting +// specified by its key from the [rsop.Policy] of the [setting.DefaultScope]. It +// returns def if the policy setting is not configured, or an error if it has +// an error or could not be converted to the specified type T. +func getCurrentPolicySettingValue[T setting.ValueType](key Key, def T) (T, error) { + effective, err := rsop.PolicyFor(setting.DefaultScope()) + if err != nil { + return def, err + } + value, err := effective.Get().GetErr(key) + if err != nil { + if errors.Is(err, setting.ErrNotConfigured) || errors.Is(err, setting.ErrNoSuchKey) { + return def, nil + } + return def, err + } + if res, ok := value.(T); ok { + return res, nil + } + return convertPolicySettingValueTo(value, def) +} + +func convertPolicySettingValueTo[T setting.ValueType](value any, def T) (T, error) { + // Convert [PreferenceOption], [Visibility], or [time.Duration] back to a string + // if someone requests a string instead of the actual setting's value. + // TODO(nickkhyl): check if this behavior is relied upon anywhere besides the old tests. + if reflect.TypeFor[T]().Kind() == reflect.String { + if str, ok := value.(fmt.Stringer); ok { + return any(str.String()).(T), nil + } + } + return def, fmt.Errorf("%w: got %T, want %T", setting.ErrTypeMismatch, value, def) } // SelectControlURL returns the ControlURL to use based on a value in diff --git a/util/syspolicy/syspolicy_test.go b/util/syspolicy/syspolicy_test.go index 8280aa1df..a70a49d39 100644 --- a/util/syspolicy/syspolicy_test.go +++ b/util/syspolicy/syspolicy_test.go @@ -9,57 +9,15 @@ import ( "testing" "time" + "tailscale.com/types/logger" + "tailscale.com/util/syspolicy/internal/loggerx" + "tailscale.com/util/syspolicy/internal/metrics" "tailscale.com/util/syspolicy/setting" + "tailscale.com/util/syspolicy/source" ) -// testHandler encompasses all data types returned when testing any of the syspolicy -// methods that involve getting a policy value. -// For keys and the corresponding values, check policy_keys.go. -type testHandler struct { - t *testing.T - key Key - s string - u64 uint64 - b bool - sArr []string - err error - calls int // used for testing reads from cache vs. handler -} - var someOtherError = errors.New("error other than not found") -func (th *testHandler) ReadString(key string) (string, error) { - if key != string(th.key) { - th.t.Errorf("ReadString(%q) want %q", key, th.key) - } - th.calls++ - return th.s, th.err -} - -func (th *testHandler) ReadUInt64(key string) (uint64, error) { - if key != string(th.key) { - th.t.Errorf("ReadUint64(%q) want %q", key, th.key) - } - th.calls++ - return th.u64, th.err -} - -func (th *testHandler) ReadBoolean(key string) (bool, error) { - if key != string(th.key) { - th.t.Errorf("ReadBool(%q) want %q", key, th.key) - } - th.calls++ - return th.b, th.err -} - -func (th *testHandler) ReadStringArray(key string) ([]string, error) { - if key != string(th.key) { - th.t.Errorf("ReadStringArray(%q) want %q", key, th.key) - } - th.calls++ - return th.sArr, th.err -} - func TestGetString(t *testing.T) { tests := []struct { name string @@ -69,23 +27,28 @@ func TestGetString(t *testing.T) { defaultValue string wantValue string wantError error + wantMetrics []metrics.TestState }{ { name: "read existing value", key: AdminConsoleVisibility, handlerValue: "hide", wantValue: "hide", + wantMetrics: []metrics.TestState{ + {Name: "$os_syspolicy_any", Value: 1}, + {Name: "$os_syspolicy_AdminConsole", Value: 1}, + }, }, { name: "read non-existing value", key: EnableServerMode, - handlerError: ErrNoSuchKey, + handlerError: ErrNotConfigured, wantError: nil, }, { name: "read non-existing value, non-blank default", key: EnableServerMode, - handlerError: ErrNoSuchKey, + handlerError: ErrNotConfigured, defaultValue: "test", wantValue: "test", wantError: nil, @@ -95,24 +58,43 @@ func TestGetString(t *testing.T) { key: NetworkDevicesVisibility, handlerError: someOtherError, wantError: someOtherError, + wantMetrics: []metrics.TestState{ + {Name: "$os_syspolicy_errors", Value: 1}, + {Name: "$os_syspolicy_NetworkDevices_error", Value: 1}, + }, }, } + RegisterWellKnownSettingsForTest(t) + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - SetHandlerForTest(t, &testHandler{ - t: t, - key: tt.key, - s: tt.handlerValue, - err: tt.handlerError, - }) + h := metrics.NewTestHandler(t) + metrics.SetHooksForTest(t, h.AddMetric, h.SetMetric) + + s := source.TestSetting[string]{ + Key: tt.key, + Value: tt.handlerValue, + Error: tt.handlerError, + } + registerSingleSettingStoreForTest(t, s) + value, err := GetString(tt.key, tt.defaultValue) - if err != tt.wantError { + if !errorsMatchForTest(err, tt.wantError) { t.Errorf("err=%q, want %q", err, tt.wantError) } if value != tt.wantValue { t.Errorf("value=%v, want %v", value, tt.wantValue) } + wantMetrics := tt.wantMetrics + if !metrics.ShouldReport() { + // Check that metrics are not reported on platforms + // where they shouldn't be reported. + // As of 2024-09-04, syspolicy only reports metrics + // on Windows and Android. + wantMetrics = nil + } + h.MustEqual(wantMetrics...) }) } } @@ -129,7 +111,7 @@ func TestGetUint64(t *testing.T) { }{ { name: "read existing value", - key: KeyExpirationNoticeTime, + key: LogSCMInteractions, handlerValue: 1, wantValue: 1, }, @@ -137,14 +119,14 @@ func TestGetUint64(t *testing.T) { name: "read non-existing value", key: LogSCMInteractions, handlerValue: 0, - handlerError: ErrNoSuchKey, + handlerError: ErrNotConfigured, wantValue: 0, }, { name: "read non-existing value, non-zero default", key: LogSCMInteractions, defaultValue: 2, - handlerError: ErrNoSuchKey, + handlerError: ErrNotConfigured, wantValue: 2, }, { @@ -157,14 +139,23 @@ func TestGetUint64(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - SetHandlerForTest(t, &testHandler{ - t: t, - key: tt.key, - u64: tt.handlerValue, - err: tt.handlerError, - }) + // None of the policy settings tested here are integers. + // In fact, we don't have any integer policies as of 2024-10-08. + // However, we can register each of them as an integer policy setting + // for the duration of the test, providing us with something to test against. + if err := setting.SetDefinitionsForTest(t, setting.NewDefinition(tt.key, setting.DeviceSetting, setting.IntegerValue)); err != nil { + t.Fatalf("SetDefinitionsForTest failed: %v", err) + } + + s := source.TestSetting[uint64]{ + Key: tt.key, + Value: tt.handlerValue, + Error: tt.handlerError, + } + registerSingleSettingStoreForTest(t, s) + value, err := GetUint64(tt.key, tt.defaultValue) - if err != tt.wantError { + if !errorsMatchForTest(err, tt.wantError) { t.Errorf("err=%q, want %q", err, tt.wantError) } if value != tt.wantValue { @@ -183,45 +174,69 @@ func TestGetBoolean(t *testing.T) { defaultValue bool wantValue bool wantError error + wantMetrics []metrics.TestState }{ { name: "read existing value", key: FlushDNSOnSessionUnlock, handlerValue: true, wantValue: true, + wantMetrics: []metrics.TestState{ + {Name: "$os_syspolicy_any", Value: 1}, + {Name: "$os_syspolicy_FlushDNSOnSessionUnlock", Value: 1}, + }, }, { name: "read non-existing value", key: LogSCMInteractions, handlerValue: false, - handlerError: ErrNoSuchKey, + handlerError: ErrNotConfigured, wantValue: false, }, { name: "reading value returns other error", key: FlushDNSOnSessionUnlock, handlerError: someOtherError, - wantError: someOtherError, + wantError: someOtherError, // expect error... defaultValue: true, - wantValue: false, + wantValue: true, // ...AND default value if the handler fails. + wantMetrics: []metrics.TestState{ + {Name: "$os_syspolicy_errors", Value: 1}, + {Name: "$os_syspolicy_FlushDNSOnSessionUnlock_error", Value: 1}, + }, }, } + RegisterWellKnownSettingsForTest(t) + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - SetHandlerForTest(t, &testHandler{ - t: t, - key: tt.key, - b: tt.handlerValue, - err: tt.handlerError, - }) + h := metrics.NewTestHandler(t) + metrics.SetHooksForTest(t, h.AddMetric, h.SetMetric) + + s := source.TestSetting[bool]{ + Key: tt.key, + Value: tt.handlerValue, + Error: tt.handlerError, + } + registerSingleSettingStoreForTest(t, s) + value, err := GetBoolean(tt.key, tt.defaultValue) - if err != tt.wantError { + if !errorsMatchForTest(err, tt.wantError) { t.Errorf("err=%q, want %q", err, tt.wantError) } if value != tt.wantValue { t.Errorf("value=%v, want %v", value, tt.wantValue) } + wantMetrics := tt.wantMetrics + if !metrics.ShouldReport() { + // Check that metrics are not reported on platforms + // where they shouldn't be reported. + // As of 2024-09-04, syspolicy only reports metrics + // on Windows and Android. + wantMetrics = nil + } + h.MustEqual(wantMetrics...) }) } } @@ -234,29 +249,42 @@ func TestGetPreferenceOption(t *testing.T) { handlerError error wantValue setting.PreferenceOption wantError error + wantMetrics []metrics.TestState }{ { name: "always by policy", key: EnableIncomingConnections, handlerValue: "always", wantValue: setting.AlwaysByPolicy, + wantMetrics: []metrics.TestState{ + {Name: "$os_syspolicy_any", Value: 1}, + {Name: "$os_syspolicy_AllowIncomingConnections", Value: 1}, + }, }, { name: "never by policy", key: EnableIncomingConnections, handlerValue: "never", wantValue: setting.NeverByPolicy, + wantMetrics: []metrics.TestState{ + {Name: "$os_syspolicy_any", Value: 1}, + {Name: "$os_syspolicy_AllowIncomingConnections", Value: 1}, + }, }, { name: "use default", key: EnableIncomingConnections, handlerValue: "", wantValue: setting.ShowChoiceByPolicy, + wantMetrics: []metrics.TestState{ + {Name: "$os_syspolicy_any", Value: 1}, + {Name: "$os_syspolicy_AllowIncomingConnections", Value: 1}, + }, }, { name: "read non-existing value", key: EnableIncomingConnections, - handlerError: ErrNoSuchKey, + handlerError: ErrNotConfigured, wantValue: setting.ShowChoiceByPolicy, }, { @@ -265,24 +293,43 @@ func TestGetPreferenceOption(t *testing.T) { handlerError: someOtherError, wantValue: setting.ShowChoiceByPolicy, wantError: someOtherError, + wantMetrics: []metrics.TestState{ + {Name: "$os_syspolicy_errors", Value: 1}, + {Name: "$os_syspolicy_AllowIncomingConnections_error", Value: 1}, + }, }, } + RegisterWellKnownSettingsForTest(t) + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - SetHandlerForTest(t, &testHandler{ - t: t, - key: tt.key, - s: tt.handlerValue, - err: tt.handlerError, - }) + h := metrics.NewTestHandler(t) + metrics.SetHooksForTest(t, h.AddMetric, h.SetMetric) + + s := source.TestSetting[string]{ + Key: tt.key, + Value: tt.handlerValue, + Error: tt.handlerError, + } + registerSingleSettingStoreForTest(t, s) + option, err := GetPreferenceOption(tt.key) - if err != tt.wantError { + if !errorsMatchForTest(err, tt.wantError) { t.Errorf("err=%q, want %q", err, tt.wantError) } if option != tt.wantValue { t.Errorf("option=%v, want %v", option, tt.wantValue) } + wantMetrics := tt.wantMetrics + if !metrics.ShouldReport() { + // Check that metrics are not reported on platforms + // where they shouldn't be reported. + // As of 2024-09-04, syspolicy only reports metrics + // on Windows and Android. + wantMetrics = nil + } + h.MustEqual(wantMetrics...) }) } } @@ -295,24 +342,33 @@ func TestGetVisibility(t *testing.T) { handlerError error wantValue setting.Visibility wantError error + wantMetrics []metrics.TestState }{ { name: "hidden by policy", key: AdminConsoleVisibility, handlerValue: "hide", wantValue: setting.HiddenByPolicy, + wantMetrics: []metrics.TestState{ + {Name: "$os_syspolicy_any", Value: 1}, + {Name: "$os_syspolicy_AdminConsole", Value: 1}, + }, }, { name: "visibility default", key: AdminConsoleVisibility, handlerValue: "show", wantValue: setting.VisibleByPolicy, + wantMetrics: []metrics.TestState{ + {Name: "$os_syspolicy_any", Value: 1}, + {Name: "$os_syspolicy_AdminConsole", Value: 1}, + }, }, { name: "read non-existing value", key: AdminConsoleVisibility, handlerValue: "show", - handlerError: ErrNoSuchKey, + handlerError: ErrNotConfigured, wantValue: setting.VisibleByPolicy, }, { @@ -322,24 +378,43 @@ func TestGetVisibility(t *testing.T) { handlerError: someOtherError, wantValue: setting.VisibleByPolicy, wantError: someOtherError, + wantMetrics: []metrics.TestState{ + {Name: "$os_syspolicy_errors", Value: 1}, + {Name: "$os_syspolicy_AdminConsole_error", Value: 1}, + }, }, } + RegisterWellKnownSettingsForTest(t) + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - SetHandlerForTest(t, &testHandler{ - t: t, - key: tt.key, - s: tt.handlerValue, - err: tt.handlerError, - }) + h := metrics.NewTestHandler(t) + metrics.SetHooksForTest(t, h.AddMetric, h.SetMetric) + + s := source.TestSetting[string]{ + Key: tt.key, + Value: tt.handlerValue, + Error: tt.handlerError, + } + registerSingleSettingStoreForTest(t, s) + visibility, err := GetVisibility(tt.key) - if err != tt.wantError { + if !errorsMatchForTest(err, tt.wantError) { t.Errorf("err=%q, want %q", err, tt.wantError) } if visibility != tt.wantValue { t.Errorf("visibility=%v, want %v", visibility, tt.wantValue) } + wantMetrics := tt.wantMetrics + if !metrics.ShouldReport() { + // Check that metrics are not reported on platforms + // where they shouldn't be reported. + // As of 2024-09-04, syspolicy only reports metrics + // on Windows and Android. + wantMetrics = nil + } + h.MustEqual(wantMetrics...) }) } } @@ -353,6 +428,7 @@ func TestGetDuration(t *testing.T) { defaultValue time.Duration wantValue time.Duration wantError error + wantMetrics []metrics.TestState }{ { name: "read existing value", @@ -360,25 +436,34 @@ func TestGetDuration(t *testing.T) { handlerValue: "2h", wantValue: 2 * time.Hour, defaultValue: 24 * time.Hour, + wantMetrics: []metrics.TestState{ + {Name: "$os_syspolicy_any", Value: 1}, + {Name: "$os_syspolicy_KeyExpirationNotice", Value: 1}, + }, }, { name: "invalid duration value", key: KeyExpirationNoticeTime, handlerValue: "-20", wantValue: 24 * time.Hour, + wantError: errors.New(`time: missing unit in duration "-20"`), defaultValue: 24 * time.Hour, + wantMetrics: []metrics.TestState{ + {Name: "$os_syspolicy_errors", Value: 1}, + {Name: "$os_syspolicy_KeyExpirationNotice_error", Value: 1}, + }, }, { name: "read non-existing value", key: KeyExpirationNoticeTime, - handlerError: ErrNoSuchKey, + handlerError: ErrNotConfigured, wantValue: 24 * time.Hour, defaultValue: 24 * time.Hour, }, { name: "read non-existing value different default", key: KeyExpirationNoticeTime, - handlerError: ErrNoSuchKey, + handlerError: ErrNotConfigured, wantValue: 0 * time.Second, defaultValue: 0 * time.Second, }, @@ -389,24 +474,43 @@ func TestGetDuration(t *testing.T) { wantValue: 24 * time.Hour, wantError: someOtherError, defaultValue: 24 * time.Hour, + wantMetrics: []metrics.TestState{ + {Name: "$os_syspolicy_errors", Value: 1}, + {Name: "$os_syspolicy_KeyExpirationNotice_error", Value: 1}, + }, }, } + RegisterWellKnownSettingsForTest(t) + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - SetHandlerForTest(t, &testHandler{ - t: t, - key: tt.key, - s: tt.handlerValue, - err: tt.handlerError, - }) + h := metrics.NewTestHandler(t) + metrics.SetHooksForTest(t, h.AddMetric, h.SetMetric) + + s := source.TestSetting[string]{ + Key: tt.key, + Value: tt.handlerValue, + Error: tt.handlerError, + } + registerSingleSettingStoreForTest(t, s) + duration, err := GetDuration(tt.key, tt.defaultValue) - if err != tt.wantError { + if !errorsMatchForTest(err, tt.wantError) { t.Errorf("err=%q, want %q", err, tt.wantError) } if duration != tt.wantValue { t.Errorf("duration=%v, want %v", duration, tt.wantValue) } + wantMetrics := tt.wantMetrics + if !metrics.ShouldReport() { + // Check that metrics are not reported on platforms + // where they shouldn't be reported. + // As of 2024-09-04, syspolicy only reports metrics + // on Windows and Android. + wantMetrics = nil + } + h.MustEqual(wantMetrics...) }) } } @@ -420,23 +524,28 @@ func TestGetStringArray(t *testing.T) { defaultValue []string wantValue []string wantError error + wantMetrics []metrics.TestState }{ { name: "read existing value", key: AllowedSuggestedExitNodes, handlerValue: []string{"foo", "bar"}, wantValue: []string{"foo", "bar"}, + wantMetrics: []metrics.TestState{ + {Name: "$os_syspolicy_any", Value: 1}, + {Name: "$os_syspolicy_AllowedSuggestedExitNodes", Value: 1}, + }, }, { name: "read non-existing value", key: AllowedSuggestedExitNodes, - handlerError: ErrNoSuchKey, + handlerError: ErrNotConfigured, wantError: nil, }, { name: "read non-existing value, non nil default", key: AllowedSuggestedExitNodes, - handlerError: ErrNoSuchKey, + handlerError: ErrNotConfigured, defaultValue: []string{"foo", "bar"}, wantValue: []string{"foo", "bar"}, wantError: nil, @@ -446,28 +555,68 @@ func TestGetStringArray(t *testing.T) { key: AllowedSuggestedExitNodes, handlerError: someOtherError, wantError: someOtherError, + wantMetrics: []metrics.TestState{ + {Name: "$os_syspolicy_errors", Value: 1}, + {Name: "$os_syspolicy_AllowedSuggestedExitNodes_error", Value: 1}, + }, }, } + RegisterWellKnownSettingsForTest(t) + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - SetHandlerForTest(t, &testHandler{ - t: t, - key: tt.key, - sArr: tt.handlerValue, - err: tt.handlerError, - }) + h := metrics.NewTestHandler(t) + metrics.SetHooksForTest(t, h.AddMetric, h.SetMetric) + + s := source.TestSetting[[]string]{ + Key: tt.key, + Value: tt.handlerValue, + Error: tt.handlerError, + } + registerSingleSettingStoreForTest(t, s) + value, err := GetStringArray(tt.key, tt.defaultValue) - if err != tt.wantError { + if !errorsMatchForTest(err, tt.wantError) { t.Errorf("err=%q, want %q", err, tt.wantError) } if !slices.Equal(tt.wantValue, value) { t.Errorf("value=%v, want %v", value, tt.wantValue) } + wantMetrics := tt.wantMetrics + if !metrics.ShouldReport() { + // Check that metrics are not reported on platforms + // where they shouldn't be reported. + // As of 2024-09-04, syspolicy only reports metrics + // on Windows and Android. + wantMetrics = nil + } + h.MustEqual(wantMetrics...) }) } } +func registerSingleSettingStoreForTest[T source.TestValueType](tb TB, s source.TestSetting[T]) { + policyStore := source.NewTestStoreOf(tb, s) + MustRegisterStoreForTest(tb, "TestStore", setting.DeviceScope, policyStore) +} + +func BenchmarkGetString(b *testing.B) { + loggerx.SetForTest(b, logger.Discard, logger.Discard) + RegisterWellKnownSettingsForTest(b) + + wantControlURL := "https://login.tailscale.com" + registerSingleSettingStoreForTest(b, source.TestSettingOf(ControlURL, wantControlURL)) + + b.ResetTimer() + for i := 0; i < b.N; i++ { + gotControlURL, _ := GetString(ControlURL, "https://controlplane.tailscale.com") + if gotControlURL != wantControlURL { + b.Fatalf("got %v; want %v", gotControlURL, wantControlURL) + } + } +} + func TestSelectControlURL(t *testing.T) { tests := []struct { reg, disk, want string @@ -499,3 +648,13 @@ func TestSelectControlURL(t *testing.T) { } } } + +func errorsMatchForTest(got, want error) bool { + if got == nil && want == nil { + return true + } + if got == nil || want == nil { + return false + } + return errors.Is(got, want) || got.Error() == want.Error() +} diff --git a/util/syspolicy/syspolicy_windows.go b/util/syspolicy/syspolicy_windows.go new file mode 100644 index 000000000..9d57e249e --- /dev/null +++ b/util/syspolicy/syspolicy_windows.go @@ -0,0 +1,92 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +package syspolicy + +import ( + "errors" + "fmt" + "os/user" + + "tailscale.com/util/syspolicy/internal" + "tailscale.com/util/syspolicy/rsop" + "tailscale.com/util/syspolicy/setting" + "tailscale.com/util/syspolicy/source" + "tailscale.com/util/testenv" +) + +func init() { + // On Windows, we should automatically register the Registry-based policy + // store for the device. If we are running in a user's security context + // (e.g., we're the GUI), we should also register the Registry policy store for + // the user. In the future, we should register (and unregister) user policy + // stores whenever a user connects to (or disconnects from) the local backend. + // This ensures the backend is aware of the user's policy settings and can send + // them to the GUI/CLI/Web clients on demand or whenever they change. + // + // Other platforms, such as macOS, iOS and Android, should register their + // platform-specific policy stores via [RegisterStore] + // (or [RegisterHandler] until they implement the [source.Store] interface). + // + // External code, such as the ipnlocal package, may choose to register + // additional policy stores, such as config files and policies received from + // the control plane. + internal.Init.MustDefer(func() error { + // Do not register or use default policy stores during tests. + // Each test should set up its own necessary configurations. + if testenv.InTest() { + return nil + } + return configureSyspolicy(nil) + }) +} + +// configureSyspolicy configures syspolicy for use on Windows, +// either in test or regular builds depending on whether tb has a non-nil value. +func configureSyspolicy(tb internal.TB) error { + const localSystemSID = "S-1-5-18" + // Always create and register a machine policy store that reads + // policy settings from the HKEY_LOCAL_MACHINE registry hive. + machineStore, err := source.NewMachinePlatformPolicyStore() + if err != nil { + return fmt.Errorf("failed to create the machine policy store: %v", err) + } + if tb == nil { + _, err = rsop.RegisterStore("Platform", setting.DeviceScope, machineStore) + } else { + _, err = rsop.RegisterStoreForTest(tb, "Platform", setting.DeviceScope, machineStore) + } + if err != nil { + return err + } + // Check whether the current process is running as Local System or not. + u, err := user.Current() + if err != nil { + return err + } + if u.Uid == localSystemSID { + return nil + } + // If it's not a Local System's process (e.g., it's the GUI rather than the tailscaled service), + // we should create and use a policy store for the current user that reads + // policy settings from that user's registry hive (HKEY_CURRENT_USER). + userStore, err := source.NewUserPlatformPolicyStore(0) + if err != nil { + return fmt.Errorf("failed to create the current user's policy store: %v", err) + } + if tb == nil { + _, err = rsop.RegisterStore("Platform", setting.CurrentUserScope, userStore) + } else { + _, err = rsop.RegisterStoreForTest(tb, "Platform", setting.CurrentUserScope, userStore) + } + if err != nil { + return err + } + // And also set [setting.CurrentUserScope] as the [setting.DefaultScope], so [GetString], + // [GetVisibility] and similar functions would be returning a merged result + // of the machine's and user's policies. + if !setting.SetDefaultScope(setting.CurrentUserScope) { + return errors.New("current scope already set") + } + return nil +}