mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-14 22:17:36 +00:00
fix(gateway): admit operator.provision keys on tenants.create and ten… (#1584)
* fix(gateway): admit operator.provision keys on tenants.create and tenants.users.add The CVE #866 fail-closed hardening regressed operator.provision: the role-only router check maps provision-only API keys to viewer and rejects tenants.create / tenants.users.add with 'requires admin role', even though the tenant handlers explicitly admit ScopeProvision on exactly those two methods (issue #1524). Restore the intended least-privilege provisioning path without any role promotion: the router now allows credentials carrying ScopeProvision on exactly the two tenant-provisioning RPCs (permissions.IsProvisionMethod). Every other admin/write surface stays denied, and viewers without the provision scope gain nothing. Regression tests (internal/gateway/router_test.go) prove: - provision-only succeeds on tenants.create and tenants.users.add - provision-only stays denied on tenants.update and agents.create - plain viewers stay denied on the provisioning methods - unauthenticated clients stay denied * refactor(gateway): route provisionScopeAllowed through permissions.HasProvisionScope Address review feedback on #1584: HasProvisionScope was exported and tested but had no production caller — the router used Client.HasScope directly. Wire the router to the permissions helper so the provision scope check has a single source of truth.
This commit is contained in:
1 parent
cbb72ccd7e
commit
a1913f813f
4 files changed
+163
-1
No files matched your search
@@ -67,7 +67,16 @@ func (r *MethodRouter) Handle(ctx context.Context, client *Client, req *protocol
|
||||
// Permission check: skip for connect, health, and browser pairing status (used by unauthenticated clients)
|
||||
if req.Method != protocol.MethodConnect && req.Method != protocol.MethodHealth && req.Method != protocol.MethodBrowserPairingStatus {
|
||||
if pe := r.server.policyEngine; pe != nil {
|
||||
if !pe.CanAccess(client.role, req.Method) {
|
||||
// provisionScopeAllowed implements the narrow method-scoped
|
||||
// exception for operator.provision credentials (issue #1524, a
|
||||
// regression from the CVE #866 fail-closed hardening). The tenant
|
||||
// handlers already admit ScopeProvision on tenants.create and
|
||||
// tenants.users.add, but the role-only check below maps
|
||||
// provision-only keys to viewer and rejects them before the
|
||||
// handler runs. The exception grants exactly those two methods to
|
||||
// credentials carrying ScopeProvision — no role promotion, and
|
||||
// every other admin/write surface stays denied.
|
||||
if !pe.CanAccess(client.role, req.Method) && !provisionScopeAllowed(client, req.Method) {
|
||||
required := permissions.MethodRole(req.Method)
|
||||
slog.Warn("security.permission_denied",
|
||||
"method", req.Method,
|
||||
@@ -121,6 +130,13 @@ func (r *MethodRouter) registerDefaults() {
|
||||
r.Register(protocol.MethodStatus, r.handleStatus)
|
||||
}
|
||||
|
||||
// provisionScopeAllowed reports whether a client carrying the operator.provision
|
||||
// scope may call the given method. True only for the two tenant-provisioning
|
||||
// RPCs — see permissions.IsProvisionMethod.
|
||||
func provisionScopeAllowed(c *Client, method string) bool {
|
||||
return permissions.HasProvisionScope(c.scopes) && permissions.IsProvisionMethod(method)
|
||||
}
|
||||
|
||||
// --- Built-in handlers ---
|
||||
|
||||
func (r *MethodRouter) handleConnect(ctx context.Context, client *Client, req *protocol.RequestFrame) {
|
||||
|
||||
@@ -6,6 +6,8 @@ import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/google/uuid"
|
||||
|
||||
"github.com/nextlevelbuilder/goclaw/internal/config"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/permissions"
|
||||
"github.com/nextlevelbuilder/goclaw/pkg/protocol"
|
||||
@@ -62,3 +64,90 @@ func TestHandleConnectAllowsExplicitInsecureNoTokenOptIn(t *testing.T) {
|
||||
t.Fatalf("role = %q, want operator", client.role)
|
||||
}
|
||||
}
|
||||
|
||||
// callMethod dispatches method through the router and returns the response frame.
|
||||
func callMethod(t *testing.T, server *Server, client *Client, method string) *protocol.ResponseFrame {
|
||||
t.Helper()
|
||||
req := &protocol.RequestFrame{ID: "req-" + method, Method: method}
|
||||
server.router.Handle(context.Background(), client, req)
|
||||
select {
|
||||
case raw := <-client.send:
|
||||
var resp protocol.ResponseFrame
|
||||
if err := json.Unmarshal(raw, &resp); err != nil {
|
||||
t.Fatalf("unmarshal response: %v", err)
|
||||
}
|
||||
return &resp
|
||||
case <-time.After(500 * time.Millisecond):
|
||||
t.Fatalf("expected a response frame for %s", method)
|
||||
return nil
|
||||
}
|
||||
}
|
||||
|
||||
// TestRouterProvisionScopeTenantMethods covers the narrow method-scoped
|
||||
// exception for operator.provision credentials (issue #1524): provision-only
|
||||
// API keys are admitted on exactly tenants.create and tenants.users.add —
|
||||
// the two methods the tenant handlers already gate on ScopeProvision — while
|
||||
// every other admin/write surface stays denied and plain viewers gain nothing.
|
||||
func TestRouterProvisionScopeTenantMethods(t *testing.T) {
|
||||
cfg := config.Default()
|
||||
cfg.Gateway.Host = "127.0.0.1"
|
||||
cfg.Gateway.Token = "test-token"
|
||||
|
||||
server := NewServer(cfg, nil, nil, nil)
|
||||
server.SetPolicyEngine(permissions.NewPolicyEngine(nil))
|
||||
|
||||
reached := map[string]bool{}
|
||||
for _, method := range []string{
|
||||
protocol.MethodTenantsCreate,
|
||||
protocol.MethodTenantsUsersAdd,
|
||||
protocol.MethodTenantsUpdate,
|
||||
protocol.MethodAgentsCreate,
|
||||
} {
|
||||
m := method
|
||||
server.router.Register(m, func(ctx context.Context, c *Client, req *protocol.RequestFrame) {
|
||||
reached[m] = true
|
||||
c.SendResponse(protocol.NewOKResponse(req.ID, map[string]any{"ok": true}))
|
||||
})
|
||||
}
|
||||
|
||||
// Provision-only API key: RoleFromScopes maps it to viewer; scopes carry
|
||||
// operator.provision (as the WS connect path would set them).
|
||||
prov, _ := NewCapturingTestClient(permissions.RoleViewer, uuid.Nil, "provisioner", 8)
|
||||
prov.scopes = []permissions.Scope{permissions.ScopeProvision}
|
||||
|
||||
// 1) Provision-only succeeds on the two tenant-provisioning methods.
|
||||
for _, method := range []string{protocol.MethodTenantsCreate, protocol.MethodTenantsUsersAdd} {
|
||||
resp := callMethod(t, server, prov, method)
|
||||
if resp.Error != nil {
|
||||
t.Fatalf("provision-only %s: unexpected error %v (code %s)", method, resp.Error.Message, resp.Error.Code)
|
||||
}
|
||||
if !reached[method] {
|
||||
t.Fatalf("provision-only %s: handler never invoked", method)
|
||||
}
|
||||
}
|
||||
|
||||
// 2) Provision-only remains denied on other admin/write surfaces.
|
||||
for _, method := range []string{protocol.MethodTenantsUpdate, protocol.MethodAgentsCreate} {
|
||||
resp := callMethod(t, server, prov, method)
|
||||
if resp.Error == nil || resp.Error.Code != protocol.ErrUnauthorized {
|
||||
t.Fatalf("provision-only %s: want unauthorized, got error=%v reached=%v", method, resp.Error, reached[method])
|
||||
}
|
||||
}
|
||||
|
||||
// 3) A viewer WITHOUT the provision scope still cannot reach the two
|
||||
// tenant-provisioning methods.
|
||||
viewer, _ := NewCapturingTestClient(permissions.RoleViewer, uuid.Nil, "viewer", 8)
|
||||
for _, method := range []string{protocol.MethodTenantsCreate, protocol.MethodTenantsUsersAdd} {
|
||||
resp := callMethod(t, server, viewer, method)
|
||||
if resp.Error == nil || resp.Error.Code != protocol.ErrUnauthorized {
|
||||
t.Fatalf("plain viewer %s: want unauthorized, got error=%v reached=%v", method, resp.Error, reached[method])
|
||||
}
|
||||
}
|
||||
|
||||
// 4) An unauthenticated client stays denied.
|
||||
anon, _ := NewCapturingTestClient("", uuid.Nil, "", 8)
|
||||
resp := callMethod(t, server, anon, protocol.MethodTenantsCreate)
|
||||
if resp.Error == nil || resp.Error.Code != protocol.ErrUnauthorized {
|
||||
t.Fatalf("unauthenticated tenants.create: want unauthorized, got error=%v reached=%v", resp.Error, reached[protocol.MethodTenantsCreate])
|
||||
}
|
||||
}
|
||||
@@ -168,6 +168,24 @@ func RoleFromScopes(scopes []Scope) Role {
|
||||
return RoleViewer
|
||||
}
|
||||
|
||||
// IsProvisionMethod reports whether method is one of the tenant-provisioning
|
||||
// RPCs that ScopeProvision exists to grant (issue #1524). ScopeProvision is a
|
||||
// least-privilege scope for automated tenant onboarding: it admits exactly
|
||||
// these two methods and nothing else — never a role promotion or a broad
|
||||
// admin bypass.
|
||||
func IsProvisionMethod(method string) bool {
|
||||
switch method {
|
||||
case protocol.MethodTenantsCreate, protocol.MethodTenantsUsersAdd:
|
||||
return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// HasProvisionScope reports whether scopes include ScopeProvision.
|
||||
func HasProvisionScope(scopes []Scope) bool {
|
||||
return slices.Contains(scopes, ScopeProvision)
|
||||
}
|
||||
|
||||
// MethodRole returns the minimum role required for a given RPC method.
|
||||
//
|
||||
// Policy is fail-closed (default-deny): methods absent from every allowlist
|
||||
|
||||
@@ -91,6 +91,45 @@ func TestRoleFromScopes(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestIsProvisionMethod covers the ScopeProvision method allowlist (issue #1524):
|
||||
// exactly tenants.create and tenants.users.add, nothing else.
|
||||
func TestIsProvisionMethod(t *testing.T) {
|
||||
allowed := []string{protocol.MethodTenantsCreate, protocol.MethodTenantsUsersAdd}
|
||||
for _, method := range allowed {
|
||||
if !IsProvisionMethod(method) {
|
||||
t.Fatalf("IsProvisionMethod(%q) = false, want true", method)
|
||||
}
|
||||
}
|
||||
denied := []string{
|
||||
protocol.MethodTenantsUpdate,
|
||||
protocol.MethodTenantsUsersRemove,
|
||||
protocol.MethodAgentsCreate,
|
||||
protocol.MethodAPIKeysCreate,
|
||||
protocol.MethodConfigApply,
|
||||
"tenants.nonexistent",
|
||||
}
|
||||
for _, method := range denied {
|
||||
if IsProvisionMethod(method) {
|
||||
t.Fatalf("IsProvisionMethod(%q) = true, want false", method)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestHasProvisionScope(t *testing.T) {
|
||||
if !HasProvisionScope([]Scope{ScopeProvision}) {
|
||||
t.Fatal("HasProvisionScope([operator.provision]) = false, want true")
|
||||
}
|
||||
if !HasProvisionScope([]Scope{ScopeRead, ScopeProvision}) {
|
||||
t.Fatal("HasProvisionScope([operator.read operator.provision]) = false, want true")
|
||||
}
|
||||
if HasProvisionScope(nil) {
|
||||
t.Fatal("HasProvisionScope(nil) = true, want false")
|
||||
}
|
||||
if HasProvisionScope([]Scope{ScopeAdmin}) {
|
||||
t.Fatal("HasProvisionScope([operator.admin]) = true, want false")
|
||||
}
|
||||
}
|
||||
|
||||
// --- CanAccess: role-based method access ---
|
||||
|
||||
func TestCanAccess_AdminMethods(t *testing.T) {
|
||||
|
||||
Reference in new issue
Block a user