From 1a4d423328df021a85da3a52fc7cfa2d838930af Mon Sep 17 00:00:00 2001 From: Adrian Dewhurst Date: Tue, 5 Dec 2023 17:16:34 -0500 Subject: [PATCH] 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 --- ipn/ipnlocal/local.go | 59 +++++-- ipn/ipnlocal/local_test.go | 326 ++++++++++++++++++++++++++----------- 2 files changed, 271 insertions(+), 114 deletions(-) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 0bfef49bc..26869fc3d 100644 --- a/ipn/ipnlocal/local.go +++ b/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 - // backend), so this has to convert between the two conventions. - if shieldsUp, err := syspolicy.GetPreferenceOption(syspolicy.EnableIncomingConnections); err == nil { - newVal := !shieldsUp.ShouldEnable(!prefs.ShieldsUp) - if prefs.ShieldsUp != newVal { - prefs.ShieldsUp = newVal - 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 + for _, opt := range preferencePolicies { + if po, err := syspolicy.GetPreferenceOption(opt.key); err == nil { + curVal := opt.get(prefs.View()) + newVal := po.ShouldEnable(curVal) + if curVal != newVal { + opt.set(prefs, newVal) + anyChange = true + } } } diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 18166b65a..a6d62c760 100644 --- a/ipn/ipnlocal/local_test.go +++ b/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", - ShieldsUp: true, - ForceDaemon: true, + ControlURL: "1", + ShieldsUp: true, + ForceDaemon: true, + ExitNodeAllowLANAccess: true, + CorpDNS: true, + RouteAll: true, }, wantPrefs: ipn.Prefs{ - ControlURL: "1", - ShieldsUp: true, - ForceDaemon: true, + ControlURL: "1", + ShieldsUp: true, + ForceDaemon: true, + ExitNodeAllowLANAccess: true, + CorpDNS: true, + RouteAll: true, }, }, { name: "empty prefs with policies", wantPrefs: ipn.Prefs{ - ControlURL: "1", - ShieldsUp: true, - ForceDaemon: true, + ControlURL: "1", + ShieldsUp: 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", - ShieldsUp: true, - ForceDaemon: true, + ControlURL: "1", + ShieldsUp: true, + ForceDaemon: true, + ExitNodeAllowLANAccess: false, + CorpDNS: true, + RouteAll: false, }, wantPrefs: ipn.Prefs{ - ControlURL: "2", - ShieldsUp: false, - ForceDaemon: false, + ControlURL: "2", + ShieldsUp: 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", - ShieldsUp: true, - ForceDaemon: true, + ControlURL: "1", + ShieldsUp: true, + ForceDaemon: true, + ExitNodeAllowLANAccess: false, + CorpDNS: true, + RouteAll: true, }, wantPrefs: ipn.Prefs{ - ControlURL: "1", - ShieldsUp: true, - ForceDaemon: true, + ControlURL: "1", + ShieldsUp: 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) { - // Be strict when testing the func directly. - msh := &mockSyspolicyHandler{ - t: t, - stringPolicies: map[syspolicy.Key]*string{ - syspolicy.ControlURL: nil, - syspolicy.EnableIncomingConnections: nil, - syspolicy.EnableServerMode: nil, - }, - 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) + 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("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) + } + }) + } + }) + } +}