From a56e58c244994d81411de5caf92772695f9dab1e Mon Sep 17 00:00:00 2001
From: Claire Wang <clairew@users.noreply.github.com>
Date: Fri, 29 Sep 2023 21:27:04 -0400
Subject: [PATCH] util/syspolicy: add read boolean setting (#9592)

---
 cmd/tailscaled/depaware.txt                   |  1 +
 cmd/tailscaled/tailscaled_windows.go          |  5 +-
 .../tailscaled_deps_test_windows.go           |  1 +
 util/syspolicy/handler.go                     |  6 ++
 util/syspolicy/handler_windows.go             |  8 +++
 util/syspolicy/syspolicy.go                   |  9 +++
 util/syspolicy/syspolicy_test.go              | 60 +++++++++++++++++++
 7 files changed, 88 insertions(+), 2 deletions(-)

diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt
index e4397dc74..b5944ebd1 100644
--- a/cmd/tailscaled/depaware.txt
+++ b/cmd/tailscaled/depaware.txt
@@ -352,6 +352,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/util/set                                       from tailscale.com/health+
         tailscale.com/util/singleflight                              from tailscale.com/control/controlclient+
         tailscale.com/util/slicesx                                   from tailscale.com/net/dnscache+
+   W    tailscale.com/util/syspolicy                                 from tailscale.com/cmd/tailscaled
         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+
diff --git a/cmd/tailscaled/tailscaled_windows.go b/cmd/tailscaled/tailscaled_windows.go
index d383bf8c3..e3e08f28a 100644
--- a/cmd/tailscaled/tailscaled_windows.go
+++ b/cmd/tailscaled/tailscaled_windows.go
@@ -51,6 +51,7 @@ import (
 	"tailscale.com/types/logger"
 	"tailscale.com/types/logid"
 	"tailscale.com/util/osdiag"
+	"tailscale.com/util/syspolicy"
 	"tailscale.com/util/winutil"
 	"tailscale.com/version"
 	"tailscale.com/wf"
@@ -131,7 +132,7 @@ func runWindowsService(pol *logpolicy.Policy) error {
 		osdiag.LogSupportInfo(logger.WithPrefix(log.Printf, "Support Info: "), osdiag.LogSupportInfoReasonStartup)
 	}()
 
-	if logSCMInteractions, _ := winutil.GetPolicyInteger("LogSCMInteractions"); logSCMInteractions != 0 {
+	if logSCMInteractions, _ := syspolicy.GetBoolean(syspolicy.LogSCMInteractions, false); logSCMInteractions {
 		syslog, err := eventlog.Open(serviceName)
 		if err == nil {
 			syslogf = func(format string, args ...any) {
@@ -158,7 +159,7 @@ func (service *ipnService) Execute(args []string, r <-chan svc.ChangeRequest, ch
 	syslogf("Service start pending")
 
 	svcAccepts := svc.AcceptStop
-	if flushDNSOnSessionUnlock, _ := winutil.GetPolicyInteger("FlushDNSOnSessionUnlock"); flushDNSOnSessionUnlock != 0 {
+	if flushDNSOnSessionUnlock, _ := syspolicy.GetBoolean(syspolicy.FlushDNSOnSessionUnlock, false); flushDNSOnSessionUnlock {
 		svcAccepts |= svc.AcceptSessionChange
 	}
 
diff --git a/tstest/integration/tailscaled_deps_test_windows.go b/tstest/integration/tailscaled_deps_test_windows.go
index 08c18bb7b..cea8e5749 100644
--- a/tstest/integration/tailscaled_deps_test_windows.go
+++ b/tstest/integration/tailscaled_deps_test_windows.go
@@ -53,6 +53,7 @@ import (
 	_ "tailscale.com/util/multierr"
 	_ "tailscale.com/util/osdiag"
 	_ "tailscale.com/util/osshare"
+	_ "tailscale.com/util/syspolicy"
 	_ "tailscale.com/util/winutil"
 	_ "tailscale.com/version"
 	_ "tailscale.com/version/distro"
diff --git a/util/syspolicy/handler.go b/util/syspolicy/handler.go
index 3457f2426..68ba09176 100644
--- a/util/syspolicy/handler.go
+++ b/util/syspolicy/handler.go
@@ -19,6 +19,8 @@ type Handler interface {
 	ReadString(key string) (string, error)
 	// ReadUInt64 reads the policy settings uint64 value given the key.
 	ReadUInt64(key string) (uint64, error)
+	// ReadBool reads the policy setting's boolean value, given the key.
+	ReadBoolean(key string) (bool, error)
 }
 
 // ErrNoSuchKey is returned when the specified key does not have a value set.
@@ -35,6 +37,10 @@ func (defaultHandler) ReadUInt64(_ string) (uint64, error) {
 	return 0, ErrNoSuchKey
 }
 
+func (defaultHandler) ReadBoolean(_ string) (bool, error) {
+	return false, ErrNoSuchKey
+}
+
 // markHandlerInUse is called before handler methods are called.
 func markHandlerInUse() {
 	handlerUsed.Store(true)
diff --git a/util/syspolicy/handler_windows.go b/util/syspolicy/handler_windows.go
index 612e60b77..c12a21fdf 100644
--- a/util/syspolicy/handler_windows.go
+++ b/util/syspolicy/handler_windows.go
@@ -30,3 +30,11 @@ func (windowsHandler) ReadUInt64(key string) (uint64, error) {
 	}
 	return value, err
 }
+
+func (windowsHandler) ReadBoolean(key string) (bool, error) {
+	value, err := winutil.GetPolicyInteger(key)
+	if errors.Is(err, winutil.ErrNoValue) {
+		err = ErrNoSuchKey
+	}
+	return value != 0, err
+}
diff --git a/util/syspolicy/syspolicy.go b/util/syspolicy/syspolicy.go
index 656fb50b0..2f8cf9dcb 100644
--- a/util/syspolicy/syspolicy.go
+++ b/util/syspolicy/syspolicy.go
@@ -27,6 +27,15 @@ func GetUint64(key Key, defaultValue uint64) (uint64, error) {
 	return v, err
 }
 
+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
+}
+
 // PreferenceOption is a policy that governs whether a boolean variable
 // is forcibly assigned an administrator-defined value, or allowed to receive
 // a user-defined value.
diff --git a/util/syspolicy/syspolicy_test.go b/util/syspolicy/syspolicy_test.go
index 2ff4249af..859843431 100644
--- a/util/syspolicy/syspolicy_test.go
+++ b/util/syspolicy/syspolicy_test.go
@@ -17,6 +17,7 @@ type testHandler struct {
 	key Key
 	s   string
 	u64 uint64
+	b   bool
 	err error
 }
 
@@ -43,6 +44,13 @@ func (th *testHandler) ReadUInt64(key string) (uint64, error) {
 	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)
+	}
+	return th.b, th.err
+}
+
 func TestGetString(t *testing.T) {
 	tests := []struct {
 		name         string
@@ -157,6 +165,58 @@ func TestGetUint64(t *testing.T) {
 	}
 }
 
+func TestGetBoolean(t *testing.T) {
+	tests := []struct {
+		name         string
+		key          Key
+		handlerValue bool
+		handlerError error
+		defaultValue bool
+		wantValue    bool
+		wantError    error
+	}{
+		{
+			name:         "read existing value",
+			key:          FlushDNSOnSessionUnlock,
+			handlerValue: true,
+			wantValue:    true,
+		},
+		{
+			name:         "read non-existing value",
+			key:          LogSCMInteractions,
+			handlerValue: false,
+			handlerError: ErrNoSuchKey,
+			wantValue:    false,
+		},
+		{
+			name:         "reading value returns other error",
+			key:          FlushDNSOnSessionUnlock,
+			handlerError: someOtherError,
+			wantError:    someOtherError,
+			defaultValue: true,
+			wantValue:    false,
+		},
+	}
+
+	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,
+			})
+			value, err := GetBoolean(tt.key, tt.defaultValue)
+			if 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)
+			}
+		})
+	}
+}
+
 func TestGetPreferenceOption(t *testing.T) {
 	tests := []struct {
 		name         string