ipn/{ipnauth,ipnlocal,ipnserver}: move the AlwaysOn policy check from ipnserver to ipnauth

In this PR, we move the code that checks the AlwaysOn policy from ipnserver.actor to ipnauth.
It is intended to be used by ipnauth.Actor implementations, and we temporarily make it exported
while these implementations reside in ipnserver and in corp. We'll unexport it later.

We also update [ipnauth.Actor.CheckProfileAccess] to accept an auditLogger, which is called
to write details about the action to the audit log when required by the policy, and update
LocalBackend.EditPrefsAs to use an auditLogger that writes to the regular backend log.

Updates tailscale/corp#26146

Signed-off-by: Nick Khyl <[email protected]>

ipn/ipnauth/actor.go

@@ -10,6 +10,11 @@ import (
 	"tailscale.com/ipn"
 )
 
+// AuditLogFunc is any function that can be used to log audit actions performed by an [Actor].
+//
+// TODO(nickkhyl,barnstar): define a named string type for the action (in tailcfg?) and use it here.
+type AuditLogFunc func(action, details string)
+
 // Actor is any actor using the [ipnlocal.LocalBackend].
 //
 // It typically represents a specific OS user, indicating that an operation
@@ -30,7 +35,10 @@ type Actor interface {
 	// CheckProfileAccess checks whether the actor has the necessary access rights
 	// to perform a given action on the specified Tailscale profile.
 	// It returns an error if access is denied.
-	CheckProfileAccess(profile ipn.LoginProfileView, requestedAccess ProfileAccess) error
+	//
+	// If the auditLogger is non-nil, it is used to write details about the action
+	// to the audit log when required by the policy.
+	CheckProfileAccess(profile ipn.LoginProfileView, requestedAccess ProfileAccess, auditLogger AuditLogFunc) error
 
 	// IsLocalSystem reports whether the actor is the Windows' Local System account.
 	//

ipn/ipnauth/policy.go

@@ -0,0 +1,46 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+package ipnauth
+
+import (
+	"errors"
+	"fmt"
+
+	"tailscale.com/ipn"
+	"tailscale.com/util/syspolicy"
+)
+
+// CheckDisconnectPolicy checks if the policy allows the specified actor to disconnect
+// Tailscale with the given optional reason. It returns nil if the operation is allowed,
+// or an error if it is not. If auditLogger is non-nil, it is called to log the action
+// when required by the policy.
+//
+// Note: this function only checks the policy and does not check whether the actor has
+// the necessary access rights to the device or profile. It is intended to be used by
+// [Actor] implementations on platforms where [syspolicy] is supported.
+//
+// TODO(nickkhyl): unexport it when we move [ipn.Actor] implementations from [ipnserver]
+// and corp to this package.
+func CheckDisconnectPolicy(actor Actor, profile ipn.LoginProfileView, reason string, auditLogger AuditLogFunc) error {
+	if alwaysOn, _ := syspolicy.GetBoolean(syspolicy.AlwaysOn, false); !alwaysOn {
+		return nil
+	}
+	if allowWithReason, _ := syspolicy.GetBoolean(syspolicy.AlwaysOnOverrideWithReason, false); !allowWithReason {
+		return errors.New("disconnect not allowed: always-on mode is enabled")
+	}
+	if reason == "" {
+		return errors.New("disconnect not allowed: reason required")
+	}
+	if auditLogger != nil {
+		var details string
+		if username, _ := actor.Username(); username != "" { // best-effort; we don't have it on all platforms
+			details = fmt.Sprintf("%q is being disconnected by %q: %v", profile.Name(), username, reason)
+		} else {
+			details = fmt.Sprintf("%q is being disconnected: %v", profile.Name(), reason)
+		}
+		// TODO(nickkhyl,barnstar): use a const for DISCONNECT_NODE.
+		auditLogger("DISCONNECT_NODE", details)
+	}
+	return nil
+}

ipn/ipnauth/self.go

@@ -28,7 +28,7 @@ func (u unrestricted) Username() (string, error) { return "", nil }
 func (u unrestricted) ClientID() (_ ClientID, ok bool) { return NoClientID, false }
 
 // CheckProfileAccess implements [Actor].
-func (u unrestricted) CheckProfileAccess(_ ipn.LoginProfileView, _ ProfileAccess) error {
+func (u unrestricted) CheckProfileAccess(_ ipn.LoginProfileView, _ ProfileAccess, _ AuditLogFunc) error {
 	// Unrestricted access to all profiles.
 	return nil
 }

ipn/ipnauth/test_actor.go

@@ -31,7 +31,7 @@ func (a *TestActor) Username() (string, error) { return a.Name, a.NameErr }
 func (a *TestActor) ClientID() (_ ClientID, ok bool) { return a.CID, a.CID != NoClientID }
 
 // CheckProfileAccess implements [Actor].
-func (a *TestActor) CheckProfileAccess(profile ipn.LoginProfileView, _ ProfileAccess) error {
+func (a *TestActor) CheckProfileAccess(profile ipn.LoginProfileView, _ ProfileAccess, _ AuditLogFunc) error {
 	return errors.New("profile access denied")
 }
 

ipn/ipnlocal/local.go

@@ -4058,7 +4058,9 @@ func (b *LocalBackend) EditPrefsAs(mp *ipn.MaskedPrefs, actor ipnauth.Actor) (ip
 	unlock := b.lockAndGetUnlock()
 	defer unlock()
 	if mp.WantRunningSet && !mp.WantRunning && b.pm.CurrentPrefs().WantRunning() {
-		if err := actor.CheckProfileAccess(b.pm.CurrentProfile(), ipnauth.Disconnect); err != nil {
+		// TODO(barnstar,nickkhyl): replace loggerFn with the actual audit logger.
+		loggerFn := func(action, details string) { b.logf("[audit]: %s: %s", action, details) }
+		if err := actor.CheckProfileAccess(b.pm.CurrentProfile(), ipnauth.Disconnect, loggerFn); err != nil {
 			return ipn.PrefsView{}, err
 		}
 

ipn/ipnserver/actor.go

@@ -17,7 +17,6 @@ import (
 	"tailscale.com/types/logger"
 	"tailscale.com/util/ctxkey"
 	"tailscale.com/util/osuser"
-	"tailscale.com/util/syspolicy"
 	"tailscale.com/version"
 )
 
@@ -80,7 +79,7 @@ func actorWithAccessOverride(baseActor *actor, reason string) *actor {
 }
 
 // CheckProfileAccess implements [ipnauth.Actor].
-func (a *actor) CheckProfileAccess(profile ipn.LoginProfileView, requestedAccess ipnauth.ProfileAccess) error {
+func (a *actor) CheckProfileAccess(profile ipn.LoginProfileView, requestedAccess ipnauth.ProfileAccess, auditLogger ipnauth.AuditLogFunc) error {
 	// TODO(nickkhyl): return errors of more specific types and have them
 	// translated to the appropriate HTTP status codes in the API handler.
 	if profile.LocalUserID() != a.UserID() {
@@ -88,18 +87,8 @@ func (a *actor) CheckProfileAccess(profile ipn.LoginProfileView, requestedAccess
 	}
 	switch requestedAccess {
 	case ipnauth.Disconnect:
-		if alwaysOn, _ := syspolicy.GetBoolean(syspolicy.AlwaysOn, false); alwaysOn {
-			if allowWithReason, _ := syspolicy.GetBoolean(syspolicy.AlwaysOnOverrideWithReason, false); !allowWithReason {
-				return errors.New("disconnect not allowed: always-on mode is enabled")
-			}
-			if a.accessOverrideReason == "" {
-				return errors.New("disconnect not allowed: reason required")
-			}
-			maybeUsername, _ := a.Username() // best-effort
-			a.logf("Tailscale (%q) is being disconnected by %q: %v", profile.Name(), maybeUsername, a.accessOverrideReason)
-			// TODO(nickkhyl): Log the reason to the audit log once we have one.
-		}
-		return nil // disconnect is allowed
+		// Disconnect is allowed if a user owns the profile and the policy permits it.
+		return ipnauth.CheckDisconnectPolicy(a, profile, a.accessOverrideReason, auditLogger)
 	default:
 		return errors.New("the requested operation is not allowed")
 	}