ipn/ipnlocal: add additional syspolicy enforcement

This adds support for enforcing exit node LAN access, DNS and subnet
routes.

Adding new preference policies was getting repetitive, so this turns
some of the boilerplate into a table.

Updates tailscale/corp#15585
Updates ENG-2240

Change-Id: Iabd3c42b0ae120b3145fac066c5caa7fc4d67824
Signed-off-by: Adrian Dewhurst <adrian@tailscale.com>

ipn/ipnlocal/local.go

@@ -1230,6 +1230,42 @@ func (b *LocalBackend) SetControlClientStatus(c controlclient.Client, st control
 	b.authReconfig()
 }
 
+type preferencePolicyInfo struct {
+	key syspolicy.Key
+	get func(ipn.PrefsView) bool
+	set func(*ipn.Prefs, bool)
+}
+
+var preferencePolicies = []preferencePolicyInfo{
+	{
+		key: syspolicy.EnableIncomingConnections,
+		// Allow Incoming (used by the UI) is the negation of ShieldsUp (used by the
+		// backend), so this has to convert between the two conventions.
+		get: func(p ipn.PrefsView) bool { return !p.ShieldsUp() },
+		set: func(p *ipn.Prefs, v bool) { p.ShieldsUp = !v },
+	},
+	{
+		key: syspolicy.EnableServerMode,
+		get: func(p ipn.PrefsView) bool { return p.ForceDaemon() },
+		set: func(p *ipn.Prefs, v bool) { p.ForceDaemon = v },
+	},
+	{
+		key: syspolicy.ExitNodeAllowLANAccess,
+		get: func(p ipn.PrefsView) bool { return p.ExitNodeAllowLANAccess() },
+		set: func(p *ipn.Prefs, v bool) { p.ExitNodeAllowLANAccess = v },
+	},
+	{
+		key: syspolicy.EnableTailscaleDNS,
+		get: func(p ipn.PrefsView) bool { return p.CorpDNS() },
+		set: func(p *ipn.Prefs, v bool) { p.CorpDNS = v },
+	},
+	{
+		key: syspolicy.EnableTailscaleSubnets,
+		get: func(p ipn.PrefsView) bool { return p.RouteAll() },
+		set: func(p *ipn.Prefs, v bool) { p.RouteAll = v },
+	},
+}
+
 // applySysPolicy overwrites configured preferences with policies that may be
 // configured by the system administrator in an OS-specific way.
 func applySysPolicy(prefs *ipn.Prefs) (anyChange bool) {
@@ -1238,21 +1274,14 @@ func applySysPolicy(prefs *ipn.Prefs) (anyChange bool) {
 		anyChange = true
 	}
 
-	// Allow Incoming (used by the UI) is the negation of ShieldsUp (used by the
+	for _, opt := range preferencePolicies {
-	// backend), so this has to convert between the two conventions.
+		if po, err := syspolicy.GetPreferenceOption(opt.key); err == nil {
-	if shieldsUp, err := syspolicy.GetPreferenceOption(syspolicy.EnableIncomingConnections); err == nil {
+			curVal := opt.get(prefs.View())
-		newVal := !shieldsUp.ShouldEnable(!prefs.ShieldsUp)
+			newVal := po.ShouldEnable(curVal)
-		if prefs.ShieldsUp != newVal {
+			if curVal != newVal {
-			prefs.ShieldsUp = newVal
+				opt.set(prefs, newVal)
-			anyChange = true
+				anyChange = true
-		}
+			}
-	}
-
-	if forceDaemon, err := syspolicy.GetPreferenceOption(syspolicy.EnableServerMode); err == nil {
-		newVal := forceDaemon.ShouldEnable(prefs.ForceDaemon)
-		if prefs.ForceDaemon != newVal {
-			prefs.ForceDaemon = newVal
-			anyChange = true
 		}
 	}
 

ipn/ipnlocal/local_test.go

@@ -5,6 +5,7 @@ package ipnlocal
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net"
 	"net/http"
@@ -1332,6 +1333,34 @@ func (rc *routeCollector) AdvertiseRoute(pfx netip.Prefix) error {
 	return nil
 }
 
+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
+}
+
 type mockSyspolicyHandler struct {
 	t *testing.T
 	// stringPolicies is the collection of policies that we expect to see
@@ -1625,28 +1654,40 @@ func TestApplySysPolicy(t *testing.T) {
 		{
 			name: "prefs set without policies",
 			prefs: ipn.Prefs{
-				ControlURL:  "1",
+				ControlURL:             "1",
-				ShieldsUp:   true,
+				ShieldsUp:              true,
-				ForceDaemon: true,
+				ForceDaemon:            true,
+				ExitNodeAllowLANAccess: true,
+				CorpDNS:                true,
+				RouteAll:               true,
 			},
 			wantPrefs: ipn.Prefs{
-				ControlURL:  "1",
+				ControlURL:             "1",
-				ShieldsUp:   true,
+				ShieldsUp:              true,
-				ForceDaemon: true,
+				ForceDaemon:            true,
+				ExitNodeAllowLANAccess: true,
+				CorpDNS:                true,
+				RouteAll:               true,
 			},
 		},
 		{
 			name: "empty prefs with policies",
 			wantPrefs: ipn.Prefs{
-				ControlURL:  "1",
+				ControlURL:             "1",
-				ShieldsUp:   true,
+				ShieldsUp:              true,
-				ForceDaemon: true,
+				ForceDaemon:            true,
+				ExitNodeAllowLANAccess: true,
+				CorpDNS:                true,
+				RouteAll:               true,
 			},
 			wantAnyChange: true,
 			stringPolicies: map[syspolicy.Key]string{
 				syspolicy.ControlURL:                "1",
 				syspolicy.EnableIncomingConnections: "never",
 				syspolicy.EnableServerMode:          "always",
+				syspolicy.ExitNodeAllowLANAccess:    "always",
+				syspolicy.EnableTailscaleDNS:        "always",
+				syspolicy.EnableTailscaleSubnets:    "always",
 			},
 		},
 		{
@@ -1665,42 +1706,63 @@ func TestApplySysPolicy(t *testing.T) {
 				syspolicy.ControlURL:                "1",
 				syspolicy.EnableIncomingConnections: "never",
 				syspolicy.EnableServerMode:          "always",
+				syspolicy.ExitNodeAllowLANAccess:    "never",
+				syspolicy.EnableTailscaleDNS:        "never",
+				syspolicy.EnableTailscaleSubnets:    "never",
 			},
 		},
 		{
 			name: "prefs set with conflicting policies",
 			prefs: ipn.Prefs{
-				ControlURL:  "1",
+				ControlURL:             "1",
-				ShieldsUp:   true,
+				ShieldsUp:              true,
-				ForceDaemon: true,
+				ForceDaemon:            true,
+				ExitNodeAllowLANAccess: false,
+				CorpDNS:                true,
+				RouteAll:               false,
 			},
 			wantPrefs: ipn.Prefs{
-				ControlURL:  "2",
+				ControlURL:             "2",
-				ShieldsUp:   false,
+				ShieldsUp:              false,
-				ForceDaemon: false,
+				ForceDaemon:            false,
+				ExitNodeAllowLANAccess: true,
+				CorpDNS:                false,
+				RouteAll:               true,
 			},
 			wantAnyChange: true,
 			stringPolicies: map[syspolicy.Key]string{
 				syspolicy.ControlURL:                "2",
 				syspolicy.EnableIncomingConnections: "always",
 				syspolicy.EnableServerMode:          "never",
+				syspolicy.ExitNodeAllowLANAccess:    "always",
+				syspolicy.EnableTailscaleDNS:        "never",
+				syspolicy.EnableTailscaleSubnets:    "always",
 			},
 		},
 		{
 			name: "prefs set with neutral policies",
 			prefs: ipn.Prefs{
-				ControlURL:  "1",
+				ControlURL:             "1",
-				ShieldsUp:   true,
+				ShieldsUp:              true,
-				ForceDaemon: true,
+				ForceDaemon:            true,
+				ExitNodeAllowLANAccess: false,
+				CorpDNS:                true,
+				RouteAll:               true,
 			},
 			wantPrefs: ipn.Prefs{
-				ControlURL:  "1",
+				ControlURL:             "1",
-				ShieldsUp:   true,
+				ShieldsUp:              true,
-				ForceDaemon: true,
+				ForceDaemon:            true,
+				ExitNodeAllowLANAccess: false,
+				CorpDNS:                true,
+				RouteAll:               true,
 			},
 			stringPolicies: map[syspolicy.Key]string{
 				syspolicy.EnableIncomingConnections: "user-decides",
 				syspolicy.EnableServerMode:          "user-decides",
+				syspolicy.ExitNodeAllowLANAccess:    "user-decides",
+				syspolicy.EnableTailscaleDNS:        "user-decides",
+				syspolicy.EnableTailscaleSubnets:    "user-decides",
 			},
 		},
 		{
@@ -1713,78 +1775,21 @@ func TestApplySysPolicy(t *testing.T) {
 				syspolicy.ControlURL: "set",
 			},
 		},
-		{
-			name: "always incoming connections",
-			prefs: ipn.Prefs{
-				ShieldsUp: true,
-			},
-			wantPrefs: ipn.Prefs{
-				ShieldsUp: false,
-			},
-			wantAnyChange: true,
-			stringPolicies: map[syspolicy.Key]string{
-				syspolicy.EnableIncomingConnections: "always",
-			},
-		},
-		{
-			name: "never incoming connections",
-			prefs: ipn.Prefs{
-				ShieldsUp: false,
-			},
-			wantPrefs: ipn.Prefs{
-				ShieldsUp: true,
-			},
-			wantAnyChange: true,
-			stringPolicies: map[syspolicy.Key]string{
-				syspolicy.EnableIncomingConnections: "never",
-			},
-		},
-		{
-			name: "always server mode",
-			prefs: ipn.Prefs{
-				ForceDaemon: false,
-			},
-			wantPrefs: ipn.Prefs{
-				ForceDaemon: true,
-			},
-			wantAnyChange: true,
-			stringPolicies: map[syspolicy.Key]string{
-				syspolicy.EnableServerMode: "always",
-			},
-		},
-		{
-			name: "never server mode",
-			prefs: ipn.Prefs{
-				ForceDaemon: true,
-			},
-			wantPrefs: ipn.Prefs{
-				ForceDaemon: false,
-			},
-			wantAnyChange: true,
-			stringPolicies: map[syspolicy.Key]string{
-				syspolicy.EnableServerMode: "never",
-			},
-		},
 	}
+
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			t.Run("unit", func(t *testing.T) {
+			msh := &mockSyspolicyHandler{
-				// Be strict when testing the func directly.
+				t:              t,
-				msh := &mockSyspolicyHandler{
+				stringPolicies: make(map[syspolicy.Key]*string, len(tt.stringPolicies)),
-					t: t,
+			}
-					stringPolicies: map[syspolicy.Key]*string{
+			for p, v := range tt.stringPolicies {
-						syspolicy.ControlURL:                nil,
+				v := v // construct a unique pointer for each policy value
-						syspolicy.EnableIncomingConnections: nil,
+				msh.stringPolicies[p] = &v
-						syspolicy.EnableServerMode:          nil,
+			}
-					},
+			syspolicy.SetHandlerForTest(t, msh)
-					failUnknownPolicies: true,
-				}
-				for p, v := range tt.stringPolicies {
-					v := v // construct a unique pointer for each policy value
-					msh.stringPolicies[p] = &v
-				}
-				syspolicy.SetHandlerForTest(t, msh)
 
+			t.Run("unit", func(t *testing.T) {
 				prefs := tt.prefs.Clone()
 
 				gotAnyChange := applySysPolicy(prefs)
@@ -1802,16 +1807,6 @@ func TestApplySysPolicy(t *testing.T) {
 					t.Errorf("prefs=%v, want %v", prefs.Pretty(), tt.wantPrefs.Pretty())
 				}
 			})
-			// Create a more relaxed syspolicy for integration tests.
-			msh := &mockSyspolicyHandler{
-				t:              t,
-				stringPolicies: make(map[syspolicy.Key]*string, len(tt.stringPolicies)),
-			}
-			for p, v := range tt.stringPolicies {
-				v := v // construct a unique pointer for each policy value
-				msh.stringPolicies[p] = &v
-			}
-			syspolicy.SetHandlerForTest(t, msh)
 
 			t.Run("set prefs", func(t *testing.T) {
 
@@ -1851,3 +1846,136 @@ func TestApplySysPolicy(t *testing.T) {
 		})
 	}
 }
+
+func TestPreferencePolicyInfo(t *testing.T) {
+	tests := []struct {
+		name         string
+		initialValue bool
+		wantValue    bool
+		wantChange   bool
+		policyValue  string
+		policyError  error
+	}{
+		{
+			name:         "force enable modify",
+			initialValue: false,
+			wantValue:    true,
+			wantChange:   true,
+			policyValue:  "always",
+		},
+		{
+			name:         "force enable unchanged",
+			initialValue: true,
+			wantValue:    true,
+			policyValue:  "always",
+		},
+		{
+			name:         "force disable modify",
+			initialValue: true,
+			wantValue:    false,
+			wantChange:   true,
+			policyValue:  "never",
+		},
+		{
+			name:         "force disable unchanged",
+			initialValue: false,
+			wantValue:    false,
+			policyValue:  "never",
+		},
+		{
+			name:         "unforced enabled",
+			initialValue: true,
+			wantValue:    true,
+			policyValue:  "user-decides",
+		},
+		{
+			name:         "unforced disabled",
+			initialValue: false,
+			wantValue:    false,
+			policyValue:  "user-decides",
+		},
+		{
+			name:         "blank enabled",
+			initialValue: true,
+			wantValue:    true,
+			policyValue:  "",
+		},
+		{
+			name:         "blank disabled",
+			initialValue: false,
+			wantValue:    false,
+			policyValue:  "",
+		},
+		{
+			name:         "unset enabled",
+			initialValue: true,
+			wantValue:    true,
+			policyError:  syspolicy.ErrNoSuchKey,
+		},
+		{
+			name:         "unset disabled",
+			initialValue: false,
+			wantValue:    false,
+			policyError:  syspolicy.ErrNoSuchKey,
+		},
+		{
+			name:         "error enabled",
+			initialValue: true,
+			wantValue:    true,
+			policyError:  errors.New("test error"),
+		},
+		{
+			name:         "error disabled",
+			initialValue: false,
+			wantValue:    false,
+			policyError:  errors.New("test error"),
+		},
+	}
+
+	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
+					}
+
+					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)
+
+					prefs := defaultPrefs.AsStruct()
+					pp.set(prefs, tt.initialValue)
+
+					gotAnyChange := applySysPolicy(prefs)
+
+					if gotAnyChange != tt.wantChange {
+						t.Errorf("anyChange=%v, want %v", gotAnyChange, tt.wantChange)
+					}
+					got := pp.get(prefs.View())
+					if got != tt.wantValue {
+						t.Errorf("pref=%v, want %v", got, tt.wantValue)
+					}
+				})
+			}
+		})
+	}
+}