diff --git a/client/internal/ipcauth/interceptor.go b/client/internal/ipcauth/interceptor.go index bb1b8aebf..e8a437b40 100644 --- a/client/internal/ipcauth/interceptor.go +++ b/client/internal/ipcauth/interceptor.go @@ -72,8 +72,11 @@ func (i *Interceptor) authorize(ctx context.Context, fullMethod string) error { } } - // Per-user / per-target-profile RPCs authorize themselves in the handler. + // These RPCs authorize themselves in the handler (target-profile check) or are + // connection-lifecycle actions any authenticated local user may perform; they + // bypass the active-profile gate. Still audit the C/H ones on allow. if handlerAuthorizedMethods[fullMethod] { + i.auditAllow(id, fullMethod) return nil } diff --git a/client/internal/ipcauth/interceptor_test.go b/client/internal/ipcauth/interceptor_test.go index c3d92eb12..80c7d16fb 100644 --- a/client/internal/ipcauth/interceptor_test.go +++ b/client/internal/ipcauth/interceptor_test.go @@ -42,9 +42,11 @@ func ctxWith(id Identity) context.Context { } const ( - up = servicePath + "Up" - list = servicePath + "ListProfiles" - unkwn = servicePath + "SomeFutureMethod" + up = servicePath + "Up" + list = servicePath + "ListProfiles" + unkwn = servicePath + "SomeFutureMethod" + down = servicePath + "Down" + switchp = servicePath + "SwitchProfile" ) func TestInterceptorAuthorize(t *testing.T) { @@ -65,6 +67,8 @@ func TestInterceptorAuthorize(t *testing.T) { {"uid owner allowed", Ownership{Owners: []string{"uid:1000"}}, nil, ctxWith(Identity{UID: 1000}), up, false}, {"non-owner denied", Ownership{Owners: []string{"uid:1000"}}, nil, ctxWith(Identity{UID: 2000}), up, true}, {"handler-authorized bypass", Ownership{Owners: []string{"uid:1000"}}, nil, ctxWith(Identity{UID: 2000}), list, false}, + {"down allowed while another user's profile active", Ownership{Owners: []string{"uid:1000"}}, nil, ctxWith(Identity{UID: 2000}), down, false}, + {"switch-profile bypasses active-profile gate", Ownership{Owners: []string{"uid:1000"}}, nil, ctxWith(Identity{UID: 2000}), switchp, false}, {"unknown method gated", Ownership{Owners: []string{"uid:1000"}}, nil, ctxWith(Identity{UID: 2000}), unkwn, true}, {"primary gid owner", Ownership{Owners: []string{"gid:5000"}}, nil, ctxWith(Identity{UID: 2000, GID: 5000}), up, false}, {"group-name owner via resolver", Ownership{Owners: []string{"group:admins"}}, diff --git a/client/internal/ipcauth/policy.go b/client/internal/ipcauth/policy.go index 4b0449de1..8e88650f2 100644 --- a/client/internal/ipcauth/policy.go +++ b/client/internal/ipcauth/policy.go @@ -18,15 +18,26 @@ type ProfilePolicy interface { ClaimActiveProfileOwnerIfUnowned(id Identity) (bool, error) } -// handlerAuthorizedMethods bypass the active-profile gate: they are per-user or -// per-target-profile operations whose handler does its own authorization (bound -// to the caller identity). Peer identity is still required to reach them. +// handlerAuthorizedMethods bypass the active-profile gate. Peer identity is +// still required to reach them. Two groups: +// +// - Per-user / per-target-profile operations whose handler does its own +// authorization bound to the caller identity: AddProfile, ListProfiles, +// GetActiveProfile, RemoveProfile, RenameProfile, and SwitchProfile (gated +// on the TARGET profile's ownership via authorizeTargetProfile). +// - Connection-lifecycle operations on the single shared daemon connection +// that any authenticated local user may perform: Down. Gating these on the +// ACTIVE profile's owner would trap a user behind another user's profile — +// they could neither disconnect nor switch to their OWN profile. SwitchProfile +// is bounded by its target check; Down only tears the connection down. var handlerAuthorizedMethods = map[string]bool{ servicePath + "AddProfile": true, servicePath + "ListProfiles": true, servicePath + "GetActiveProfile": true, servicePath + "RemoveProfile": true, servicePath + "RenameProfile": true, + servicePath + "SwitchProfile": true, + servicePath + "Down": true, } // auditMethods are worth an audit log line. Denials are always logged.