mirrors/tailscale
1
0
Fork 0
mirror of https://github.com/tailscale/tailscale.git synced 2025-08-06 22:27:36 +02:00
Code Issues Packages Projects Releases Wiki Activity

ipn/{ipnauth,ipnlocal,localapi}: make EditPrefs return an error if changing exit node is restricted by policy

Browse Source 
We extract checkEditPrefsAccessLocked, adjustEditPrefsLocked, and onEditPrefsLocked from the EditPrefs
execution path, defining when each step is performed and what behavior is allowed at each stage.

Currently, this is primarily used to support Always On mode, to handle the Exit Node enablement toggle,
and to report prefs edit metrics.

We then use it to enforce Exit Node policy settings by preventing users from setting an exit node
and making EditPrefs return an error when an exit node is restricted by policy. This enforcement is also
extended to the Exit Node toggle.

These changes prepare for supporting Exit Node overrides when permitted by policy and preventing logout
while Always On mode is enabled.

In the future, implementation of these methods can be delegated to ipnext extensions via the feature hooks.

Updates tailscale/corp#29969
Updates tailscale/corp#26249

Signed-off-by: Nick Khyl <nickk@tailscale.com>
This commit is contained in:
Nick Khyl 2025-07-07 17:04:07 -05:00 committed by Nick Khyl
parent a6f6478129
commit f1c7b463cd
5 changed files with 191 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
ipn/ipnauth/self.go
View File

@ -13,6 +13,11 @@
// has unlimited access.
var Self Actor = unrestricted{}
// TODO is a caller identity used when the operation is performed on behalf of a user,
// rather than by tailscaled itself, but the surrounding function is not yet extended
// to accept an [Actor] parameter. It grants the same unrestricted access as [Self].
var TODO Actor = unrestricted{}
// unrestricted is an [Actor] that has unlimited access to the currently running
// tailscaled instance. It's typically used for operations performed by tailscaled
// on its own, or upon a request from the control plane, rather on behalf of a user.
@ -49,3 +54,10 @@ func (unrestricted) IsLocalSystem() bool { return false }
// Deprecated: this method exists for compatibility with the current (as of 2025-01-28)
// permission model and will be removed as we progress on tailscale/corp#18342.
func (unrestricted) IsLocalAdmin(operatorUID string) bool { return false }
// IsTailscaled reports whether the given Actor represents Tailscaled itself,
// such as [Self] or a [TODO] placeholder actor.
func IsTailscaled(a Actor) bool {
_, ok := a.(unrestricted)
return ok
}

168
ipn/ipnlocal/local.go
View File

@ -178,6 +178,10 @@ type watchSession struct {
// It is used as a context cancellation cause for the old context
// and can be returned when an operation is performed on it.
errNodeContextChanged = errors.New("profile changed")
// errManagedByPolicy indicates the operation is blocked
// because the target state is managed by a GP/MDM policy.
errManagedByPolicy = errors.New("managed by policy")
)
// LocalBackend is the glue between the major pieces of the Tailscale
@ -3477,12 +3481,14 @@ func (b *LocalBackend) onTailnetDefaultAutoUpdate(au bool) {
b.logf("using tailnet default auto-update setting: %v", au)
prefsClone := prefs.AsStruct()
prefsClone.AutoUpdate.Apply = opt.NewBool(au)
_, err := b.editPrefsLockedOnEntry(&ipn.MaskedPrefs{
Prefs: *prefsClone,
AutoUpdateSet: ipn.AutoUpdatePrefsMask{
ApplySet: true,
},
}, unlock)
_, err := b.editPrefsLockedOnEntry(
ipnauth.Self,
&ipn.MaskedPrefs{
Prefs: *prefsClone,
AutoUpdateSet: ipn.AutoUpdatePrefsMask{
ApplySet: true,
},
}, unlock)
if err != nil {
b.logf("failed to apply tailnet-wide default for auto-updates (%v): %v", au, err)
return
@ -4224,7 +4230,7 @@ func (b *LocalBackend) checkAutoUpdatePrefsLocked(p *ipn.Prefs) error {
// On success, it returns the resulting prefs (or current prefs, in the case of no change).
// Setting the value to false when use of an exit node is already false is not an error,
// nor is true when the exit node is already in use.
func (b *LocalBackend) SetUseExitNodeEnabled(v bool) (ipn.PrefsView, error) {
func (b *LocalBackend) SetUseExitNodeEnabled(actor ipnauth.Actor, v bool) (ipn.PrefsView, error) {
unlock := b.lockAndGetUnlock()
defer unlock()
@ -4267,7 +4273,7 @@ func (b *LocalBackend) SetUseExitNodeEnabled(v bool) (ipn.PrefsView, error) {
mp.InternalExitNodePrior = p0.ExitNodeID()
}
}
return b.editPrefsLockedOnEntry(mp, unlock)
return b.editPrefsLockedOnEntry(actor, mp, unlock)
}
// MaybeClearAppConnector clears the routes from any AppConnector if
@ -4296,30 +4302,83 @@ func (b *LocalBackend) EditPrefsAs(mp *ipn.MaskedPrefs, actor ipnauth.Actor) (ip
return ipn.PrefsView{}, errors.New("can't set Internal fields")
}
// Zeroing the ExitNodeId via localAPI must also zero the prior exit node.
if mp.ExitNodeIDSet && mp.ExitNodeID == "" {
return b.editPrefsLockedOnEntry(actor, mp, b.lockAndGetUnlock())
}
// checkEditPrefsAccessLocked checks whether the current user has access
// to apply the prefs changes in mp.
//
// It returns an error if the user is not allowed, or nil otherwise.
//
// b.mu must be held.
func (b *LocalBackend) checkEditPrefsAccessLocked(actor ipnauth.Actor, mp *ipn.MaskedPrefs) error {
var errs []error
if mp.RunSSHSet && mp.RunSSH && !envknob.CanSSHD() {
errs = append(errs, errors.New("Tailscale SSH server administratively disabled"))
}
// Check if the user is allowed to disconnect Tailscale.
if mp.WantRunningSet && !mp.WantRunning && b.pm.CurrentPrefs().WantRunning() {
if err := actor.CheckProfileAccess(b.pm.CurrentProfile(), ipnauth.Disconnect, b.extHost.AuditLogger()); err != nil {
errs = append(errs, err)
}
}
// Prevent users from changing exit node preferences
// when exit node usage is managed by policy.
if mp.ExitNodeIDSet || mp.ExitNodeIPSet || mp.AutoExitNodeSet {
// TODO(nickkhyl): Allow users to override ExitNode policy settings
// if the ExitNode.AllowUserOverride policy permits it.
// (Policy setting name and details are TBD. See tailscale/corp#29969)
isManaged, err := syspolicy.HasAnyOf(syspolicy.ExitNodeID, syspolicy.ExitNodeIP)
if err != nil {
err = fmt.Errorf("policy check failed: %w", err)
} else if isManaged {
err = errManagedByPolicy
}
if err != nil {
errs = append(errs, fmt.Errorf("exit node cannot be changed: %w", err))
}
}
return multierr.New(errs...)
}
// adjustEditPrefsLocked applies additional changes to mp if necessary,
// such as zeroing out mutually exclusive fields.
//
// It must not assume that the changes in mp will actually be applied.
//
// b.mu must be held.
func (b *LocalBackend) adjustEditPrefsLocked(_ ipnauth.Actor, mp *ipn.MaskedPrefs) {
// Zeroing the ExitNodeID via localAPI must also zero the prior exit node.
if mp.ExitNodeIDSet && mp.ExitNodeID == "" && !mp.InternalExitNodePriorSet {
mp.InternalExitNodePrior = ""
mp.InternalExitNodePriorSet = true
}
// Disable automatic exit node selection if the user explicitly sets
// ExitNodeID or ExitNodeIP.
if mp.ExitNodeIDSet || mp.ExitNodeIPSet {
if (mp.ExitNodeIDSet || mp.ExitNodeIPSet) && !mp.AutoExitNodeSet {
mp.AutoExitNodeSet = true
mp.AutoExitNode = ""
}
}
// Acquire the lock before checking the profile access to prevent
// TOCTOU issues caused by the current profile changing between the
// check and the actual edit.
unlock := b.lockAndGetUnlock()
defer unlock()
if mp.WantRunningSet && !mp.WantRunning && b.pm.CurrentPrefs().WantRunning() {
if err := actor.CheckProfileAccess(b.pm.CurrentProfile(), ipnauth.Disconnect, b.extHost.AuditLogger()); err != nil {
b.logf("check profile access failed: %v", err)
return ipn.PrefsView{}, err
}
// onEditPrefsLocked is called when prefs are edited (typically, via LocalAPI),
// just before the changes in newPrefs are set for the current profile.
//
// The changes in mp have been allowed, but the resulting [ipn.Prefs]
// have not yet been applied and may be subject to reconciliation
// by [LocalBackend.reconcilePrefsLocked], either before or after being set.
//
// This method handles preference edits, typically initiated by the user,
// as opposed to reconfiguring the backend when the final prefs are set.
//
// b.mu must be held; mp must not be mutated by this method.
func (b *LocalBackend) onEditPrefsLocked(_ ipnauth.Actor, mp *ipn.MaskedPrefs, oldPrefs, newPrefs ipn.PrefsView) {
if mp.WantRunningSet && !mp.WantRunning && oldPrefs.WantRunning() {
// If a user has enough rights to disconnect, such as when [syspolicy.AlwaysOn]
// is disabled, or [syspolicy.AlwaysOnOverrideWithReason] is also set and the user
// provides a reason for disconnecting, then we should not force the "always on"
@ -4331,7 +4390,18 @@ func (b *LocalBackend) EditPrefsAs(mp *ipn.MaskedPrefs, actor ipnauth.Actor) (ip
}
}
return b.editPrefsLockedOnEntry(mp, unlock)
// This is recorded here in the EditPrefs path, not the setPrefs path on purpose.
// recordForEdit records metrics related to edits and changes, not the final state.
// If, in the future, we want to record gauge-metrics related to the state of prefs,
// that should be done in the setPrefs path.
e := prefsMetricsEditEvent{
change: mp,
pNew: newPrefs,
pOld: oldPrefs,
node: b.currentNode(),
lastSuggestedExitNode: b.lastSuggestedExitNode,
}
e.record()
}
// startReconnectTimerLocked sets a timer to automatically set WantRunning to true
@ -4368,7 +4438,7 @@ func (b *LocalBackend) startReconnectTimerLocked(d time.Duration) {
}
mp := &ipn.MaskedPrefs{WantRunningSet: true, Prefs: ipn.Prefs{WantRunning: true}}
if _, err := b.editPrefsLockedOnEntry(mp, unlock); err != nil {
if _, err := b.editPrefsLockedOnEntry(ipnauth.Self, mp, unlock); err != nil {
b.logf("failed to automatically reconnect as %q after %v: %v", cp.Name(), d, err)
} else {
b.logf("automatically reconnected as %q after %v", cp.Name(), d)
@ -4399,9 +4469,19 @@ func (b *LocalBackend) stopReconnectTimerLocked() {
// Warning: b.mu must be held on entry, but it unlocks it on the way out.
// TODO(bradfitz): redo the locking on all these weird methods like this.
func (b *LocalBackend) editPrefsLockedOnEntry(mp *ipn.MaskedPrefs, unlock unlockOnce) (ipn.PrefsView, error) {
func (b *LocalBackend) editPrefsLockedOnEntry(actor ipnauth.Actor, mp *ipn.MaskedPrefs, unlock unlockOnce) (ipn.PrefsView, error) {
defer unlock() // for error paths
// Check if the changes in mp are allowed.
if err := b.checkEditPrefsAccessLocked(actor, mp); err != nil {
b.logf("EditPrefs(%v): %v", mp.Pretty(), err)
return ipn.PrefsView{}, err
}
// Apply additional changes to mp if necessary,
// such as clearing mutually exclusive fields.
b.adjustEditPrefsLocked(actor, mp)
if mp.EggSet {
mp.EggSet = false
b.egg = true
@ -4416,29 +4496,18 @@ func (b *LocalBackend) editPrefsLockedOnEntry(mp *ipn.MaskedPrefs, unlock unlock
b.logf("EditPrefs check error: %v", err)
return ipn.PrefsView{}, err
}
if p1.RunSSH && !envknob.CanSSHD() {
b.logf("EditPrefs requests SSH, but disabled by envknob; returning error")
return ipn.PrefsView{}, errors.New("Tailscale SSH server administratively disabled.")
}
if p1.View().Equals(p0) {
return stripKeysFromPrefs(p0), nil
}
b.logf("EditPrefs: %v", mp.Pretty())
newPrefs := b.setPrefsLockedOnEntry(p1, unlock)
// This is recorded here in the EditPrefs path, not the setPrefs path on purpose.
// recordForEdit records metrics related to edits and changes, not the final state.
// If, in the future, we want to record gauge-metrics related to the state of prefs,
// that should be done in the setPrefs path.
e := prefsMetricsEditEvent{
change: mp,
pNew: p1.View(),
pOld: p0,
node: b.currentNode(),
lastSuggestedExitNode: b.lastSuggestedExitNode,
}
e.record()
// Perform any actions required when prefs are edited (typically by a user),
// before the modified prefs are actually set for the current profile.
b.onEditPrefsLocked(actor, mp, p0, p1.View())
newPrefs := b.setPrefsLockedOnEntry(p1, unlock)
// Note: don't perform any actions for the new prefs here. Not
// every prefs change goes through EditPrefs. Put your actions
@ -5829,11 +5898,16 @@ func (b *LocalBackend) Logout(ctx context.Context) error {
// delete it later.
profile := b.pm.CurrentProfile()
_, err := b.editPrefsLockedOnEntry(&ipn.MaskedPrefs{
WantRunningSet: true,
LoggedOutSet: true,
Prefs: ipn.Prefs{WantRunning: false, LoggedOut: true},
}, unlock)
// TODO(nickkhyl): change [LocalBackend.Logout] to accept an [ipnauth.Actor].
// This will allow enforcing Always On mode when a user tries to log out
// while logged in and connected. See tailscale/corp#26249.
_, err := b.editPrefsLockedOnEntry(
ipnauth.TODO,
&ipn.MaskedPrefs{
WantRunningSet: true,
LoggedOutSet: true,
Prefs: ipn.Prefs{WantRunning: false, LoggedOut: true},
}, unlock)
if err != nil {
return err
}

83
ipn/ipnlocal/local_test.go
View File

@ -501,29 +501,30 @@ func TestLazyMachineKeyGeneration(t *testing.T) {
func TestZeroExitNodeViaLocalAPI(t *testing.T) {
lb := newTestLocalBackend(t)
user := &ipnauth.TestActor{}
// Give it an initial exit node in use.
if _, err := lb.EditPrefs(&ipn.MaskedPrefs{
if _, err := lb.EditPrefsAs(&ipn.MaskedPrefs{
ExitNodeIDSet: true,
Prefs: ipn.Prefs{
ExitNodeID: "foo",
},
}); err != nil {
}, user); err != nil {
t.Fatalf("enabling first exit node: %v", err)
}
// SetUseExitNodeEnabled(false) "remembers" the prior exit node.
if _, err := lb.SetUseExitNodeEnabled(false); err != nil {
if _, err := lb.SetUseExitNodeEnabled(user, false); err != nil {
t.Fatal("expected failure")
}
// Zero the exit node
pv, err := lb.EditPrefs(&ipn.MaskedPrefs{
pv, err := lb.EditPrefsAs(&ipn.MaskedPrefs{
ExitNodeIDSet: true,
Prefs: ipn.Prefs{
ExitNodeID: "",
},
})
}, user)
if err != nil {
t.Fatalf("enabling first exit node: %v", err)
@ -539,29 +540,30 @@ func TestZeroExitNodeViaLocalAPI(t *testing.T) {
func TestSetUseExitNodeEnabled(t *testing.T) {
lb := newTestLocalBackend(t)
user := &ipnauth.TestActor{}
// Can't turn it on if it never had an old value.
if _, err := lb.SetUseExitNodeEnabled(true); err == nil {
if _, err := lb.SetUseExitNodeEnabled(user, true); err == nil {
t.Fatal("expected success")
}
// But we can turn it off when it's already off.
if _, err := lb.SetUseExitNodeEnabled(false); err != nil {
if _, err := lb.SetUseExitNodeEnabled(user, false); err != nil {
t.Fatal("expected failure")
}
// Give it an initial exit node in use.
if _, err := lb.EditPrefs(&ipn.MaskedPrefs{
if _, err := lb.EditPrefsAs(&ipn.MaskedPrefs{
ExitNodeIDSet: true,
Prefs: ipn.Prefs{
ExitNodeID: "foo",
},
}); err != nil {
}, user); err != nil {
t.Fatalf("enabling first exit node: %v", err)
}
// Now turn off that exit node.
if prefs, err := lb.SetUseExitNodeEnabled(false); err != nil {
if prefs, err := lb.SetUseExitNodeEnabled(user, false); err != nil {
t.Fatal("expected failure")
} else {
if g, w := prefs.ExitNodeID(), tailcfg.StableNodeID(""); g != w {
@ -573,7 +575,7 @@ func TestSetUseExitNodeEnabled(t *testing.T) {
}
// And turn it back on.
if prefs, err := lb.SetUseExitNodeEnabled(true); err != nil {
if prefs, err := lb.SetUseExitNodeEnabled(user, true); err != nil {
t.Fatal("expected failure")
} else {
if g, w := prefs.ExitNodeID(), tailcfg.StableNodeID("foo"); g != w {
@ -585,9 +587,9 @@ func TestSetUseExitNodeEnabled(t *testing.T) {
}
// Verify we block setting an Internal field.
if _, err := lb.EditPrefs(&ipn.MaskedPrefs{
if _, err := lb.EditPrefsAs(&ipn.MaskedPrefs{
InternalExitNodePriorSet: true,
}); err == nil {
}, user); err == nil {
t.Fatalf("unexpected success; want an error trying to set an internal field")
}
}
@ -612,16 +614,18 @@ func TestConfigureExitNode(t *testing.T) {
}
tests := []struct {
name string
prefs ipn.Prefs
netMap *netmap.NetworkMap
report *netcheck.Report
changePrefs *ipn.MaskedPrefs
useExitNodeEnabled *bool
exitNodeIDPolicy *tailcfg.StableNodeID
exitNodeIPPolicy *netip.Addr
exitNodeAllowedIDs []tailcfg.StableNodeID // nil if all IDs are allowed for auto exit nodes
wantPrefs ipn.Prefs
name string
prefs ipn.Prefs
netMap *netmap.NetworkMap
report *netcheck.Report
changePrefs *ipn.MaskedPrefs
useExitNodeEnabled *bool
exitNodeIDPolicy *tailcfg.StableNodeID
exitNodeIPPolicy *netip.Addr
exitNodeAllowedIDs []tailcfg.StableNodeID // nil if all IDs are allowed for auto exit nodes
wantChangePrefsErr error // if non-nil, the error we expect from [LocalBackend.EditPrefsAs]
wantPrefs ipn.Prefs
wantExitNodeToggleErr error // if non-nil, the error we expect from [LocalBackend.SetUseExitNodeEnabled]
}{
{
name: "exit-node-id-via-prefs", // set exit node ID via prefs
@ -804,6 +808,7 @@ func TestConfigureExitNode(t *testing.T) {
ControlURL: controlURL,
ExitNodeID: exitNode1.StableID(),
},
wantChangePrefsErr: errManagedByPolicy,
},
{
name: "id-via-policy/cannot-override-via-prefs/by-ip", // syspolicy should take precedence over prefs
@ -822,6 +827,7 @@ func TestConfigureExitNode(t *testing.T) {
ControlURL: controlURL,
ExitNodeID: exitNode1.StableID(),
},
wantChangePrefsErr: errManagedByPolicy,
},
{
name: "id-via-policy/cannot-override-via-prefs/by-auto-expr", // syspolicy should take precedence over prefs
@ -840,6 +846,7 @@ func TestConfigureExitNode(t *testing.T) {
ControlURL: controlURL,
ExitNodeID: exitNode1.StableID(),
},
wantChangePrefsErr: errManagedByPolicy,
},
{
name: "ip-via-policy", // set exit node IP via syspolicy (should be resolved to an ID)
@ -999,15 +1006,16 @@ func TestConfigureExitNode(t *testing.T) {
prefs: ipn.Prefs{
ControlURL: controlURL,
},
netMap: clientNetmap,
report: report,
exitNodeIDPolicy: ptr.To(tailcfg.StableNodeID("auto:any")),
useExitNodeEnabled: ptr.To(false), // should be ignored
netMap: clientNetmap,
report: report,
exitNodeIDPolicy: ptr.To(tailcfg.StableNodeID("auto:any")),
useExitNodeEnabled: ptr.To(false), // should fail with an error
wantExitNodeToggleErr: errManagedByPolicy,
wantPrefs: ipn.Prefs{
ControlURL: controlURL,
ExitNodeID: exitNode1.StableID(), // still enforced by the policy setting
AutoExitNode: "any",
InternalExitNodePrior: "auto:any",
InternalExitNodePrior: "",
},
},
}
@ -1046,14 +1054,17 @@ func TestConfigureExitNode(t *testing.T) {
lb.SetControlClientStatus(lb.cc, controlclient.Status{NetMap: tt.netMap})
}
user := &ipnauth.TestActor{}
// If we have a changePrefs, apply it.
if tt.changePrefs != nil {
lb.EditPrefs(tt.changePrefs)
_, err := lb.EditPrefsAs(tt.changePrefs, user)
checkError(t, err, tt.wantChangePrefsErr, true)
}
// If we need to flip exit node toggle on or off, do it.
if tt.useExitNodeEnabled != nil {
lb.SetUseExitNodeEnabled(*tt.useExitNodeEnabled)
_, err := lb.SetUseExitNodeEnabled(user, *tt.useExitNodeEnabled)
checkError(t, err, tt.wantExitNodeToggleErr, true)
}
// Now check the prefs.
@ -6218,6 +6229,18 @@ type test struct {
}
}
func checkError(tb testing.TB, got, want error, fatal bool) {
tb.Helper()
f := tb.Errorf
if fatal {
f = tb.Fatalf
}
if (want == nil) != (got == nil) ||
(want != nil && got != nil && want.Error() != got.Error() && !errors.Is(got, want)) {
f("gotErr: %v; wantErr: %v", got, want)
}
}
func toStrings[T ~string](in []T) []string {
out := make([]string, len(in))
for i, v := range in {

2
ipn/localapi/localapi.go
View File

@ -1910,7 +1910,7 @@ func (h *Handler) serveSetUseExitNodeEnabled(w http.ResponseWriter, r *http.Requ
http.Error(w, "invalid 'enabled' parameter", http.StatusBadRequest)
return
}
prefs, err := h.b.SetUseExitNodeEnabled(v)
prefs, err := h.b.SetUseExitNodeEnabled(h.Actor, v)
if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
return

5
util/syspolicy/syspolicy.go
View File

@ -72,7 +72,10 @@ func HasAnyOf(keys ...Key) (bool, error) {
if errors.Is(err, setting.ErrNotConfigured) || errors.Is(err, setting.ErrNoSuchKey) {
continue
}
return err == nil, err // err may be nil or non-nil
if err != nil {
return false, err
}
return true, nil
}
return false, nil
}