From 898870b439b6b45186f5ae1b0d21b1bad864efc0 Mon Sep 17 00:00:00 2001 From: lr00rl Date: Sun, 26 Jul 2026 01:27:28 -0700 Subject: [PATCH] Deny RPC calls before revealing service state Ungranted plugin callers could distinguish missing, inactive, and active RPC services because Call resolved lifecycle before checking the directed grant. The registry now checks the grant first and reveals service, owner lifecycle, and method details only after authorization; operator dispatch remains unchanged. Constraint: TASK-0008 is limited to internal/plugin and needs zeus security ack before merge Rejected: Return a uniform not-found error for all callers | authorized callers and operator dispatch still need actionable service and lifecycle errors Confidence: high Scope-risk: narrow Tested: gofmt -l internal/plugin/broker.go internal/plugin/rpc.go internal/plugin/rpc_test.go; sh scripts/check-docker-defaults.sh; go vet ./...; go test -race -cover ./internal/plugin (77.1%); go test -race -cover ./... Not-tested: Live plugin-to-plugin deployment path --- internal/plugin/broker.go | 4 +-- internal/plugin/rpc.go | 49 ++++++++++++++++++------------- internal/plugin/rpc_test.go | 57 ++++++++++++++++++++++++++++++++++--- 3 files changed, 85 insertions(+), 25 deletions(-) diff --git a/internal/plugin/broker.go b/internal/plugin/broker.go index c2cc99a..9dd3ce5 100644 --- a/internal/plugin/broker.go +++ b/internal/plugin/broker.go @@ -617,8 +617,8 @@ func (b *Broker) Log(ctx context.Context, level, message string, fields map[stri // registry can authorize the directed edge; a plugin cannot impersonate another // caller. Registry-level denials (ErrRPCDenied) are additionally recorded as a // deny event for audit visibility (the capability check itself already recorded -// an allow). Service/method-not-found are client errors, not security denials, -// so they are returned without an extra deny event. +// an allow). Service/method-not-found are client errors once the directed edge is +// authorized, so they are returned without an extra deny event. func (b *Broker) RPCCall(ctx context.Context, service, method string, request []byte) ([]byte, error) { if err := b.require(ctx, "rpc.call", capRPCCall); err != nil { return nil, err diff --git a/internal/plugin/rpc.go b/internal/plugin/rpc.go index 6b7ac32..71b6e06 100644 --- a/internal/plugin/rpc.go +++ b/internal/plugin/rpc.go @@ -81,9 +81,10 @@ func (r *RPCRegistry) SetOwnerActive(fn OwnerActiveFunc) { r.mu.Unlock() } -// serviceIfActive resolves a service and refuses it when its owning plugin is not -// active. Returns ErrRPCNoService when unregistered so a disabled plugin and an -// absent one are indistinguishable to a caller probing for services. +// serviceIfActive resolves a service for operator or already-authorized dispatch +// and refuses it when its owning plugin is not active. It intentionally returns +// distinct absent and inactive errors; plugin-to-plugin Call checks grants before +// exposing those lifecycle details. func (r *RPCRegistry) serviceIfActive(service string) (*rpcService, error) { r.mu.RLock() svc := r.services[service] @@ -209,20 +210,15 @@ func (r *RPCRegistry) CallOperator(ctx context.Context, service, method string, return svc.handler(ctx, method, request) } -// Call implements RPCHost: resolve the service, enforce the directed allow-list -// (the owner may always self-call), check the method, then dispatch to the -// handler OUTSIDE the lock so a slow or re-entrant handler cannot block the bus. +// Call implements RPCHost: enforce the directed allow-list (the owner may always +// self-call), then reveal service/lifecycle/method details only to authorized +// callers. The handler runs OUTSIDE the lock so a slow or re-entrant handler +// cannot block the bus. func (r *RPCRegistry) Call(ctx context.Context, caller, service, method string, request []byte) ([]byte, error) { - // A granted edge does not outlive its provider: a consumer holding rpc:call on a - // disabled plugin's service is refused here, not served by a backend that only - // looks alive because core registered it at boot. - svc, err := r.serviceIfActive(service) - if err != nil { - return nil, err - } - r.mu.RLock() - allowed := caller == svc.owner + svc := r.services[service] + active := r.ownerActive + allowed := svc != nil && caller == svc.owner if !allowed { if methods := r.grants[service][caller]; methods != nil { _, wildcard := methods["*"] @@ -230,13 +226,28 @@ func (r *RPCRegistry) Call(ctx context.Context, caller, service, method string, allowed = wildcard || exact } } - r.mu.RUnlock() - if !allowed { + r.mu.RUnlock() return nil, fmt.Errorf("%w: %s -> %s", ErrRPCDenied, caller, service) } - if _, ok := svc.methods[method]; !ok { + + if svc == nil { + r.mu.RUnlock() + return nil, fmt.Errorf("%w: %s", ErrRPCNoService, service) + } + owner := svc.owner + methods := svc.methods + handler := svc.handler + r.mu.RUnlock() + + // A granted edge does not outlive its provider: a consumer holding rpc:call on a + // disabled plugin's service is refused here, not served by a backend that only + // looks alive because core registered it at boot. + if active != nil && !active(owner) { + return nil, fmt.Errorf("%w: %s (owner %s)", ErrRPCOwnerInactive, service, owner) + } + if _, ok := methods[method]; !ok { return nil, fmt.Errorf("%w: %s/%s", ErrRPCNoMethod, service, method) } - return svc.handler(ctx, method, request) + return handler(ctx, method, request) } diff --git a/internal/plugin/rpc_test.go b/internal/plugin/rpc_test.go index cc7824b..ef2113a 100644 --- a/internal/plugin/rpc_test.go +++ b/internal/plugin/rpc_test.go @@ -96,8 +96,9 @@ func TestRPCRegistryCallAuthorization(t *testing.T) { } // unknown service / method - if _, err := r.Call(context.Background(), "owner.plugin", "missing/svc", "export", nil); !errors.Is(err, ErrRPCNoService) { - t.Fatalf("missing service: want ErrRPCNoService, got %v", err) + r.Allow("caller.plugin", "missing/svc") + if _, err := r.Call(context.Background(), "caller.plugin", "missing/svc", "export", nil); !errors.Is(err, ErrRPCNoService) { + t.Fatalf("granted missing service: want ErrRPCNoService, got %v", err) } if _, err := r.Call(context.Background(), "owner.plugin", "owner.plugin/nodes", "nope", nil); !errors.Is(err, ErrRPCNoMethod) { t.Fatalf("missing method: want ErrRPCNoMethod, got %v", err) @@ -105,8 +106,56 @@ func TestRPCRegistryCallAuthorization(t *testing.T) { // unregister r.Unregister("owner.plugin/nodes") - if _, err := r.Call(context.Background(), "owner.plugin", "owner.plugin/nodes", "export", nil); !errors.Is(err, ErrRPCNoService) { - t.Fatalf("after unregister: want ErrRPCNoService, got %v", err) + if _, err := r.Call(context.Background(), "owner.plugin", "owner.plugin/nodes", "export", nil); !errors.Is(err, ErrRPCDenied) { + t.Fatalf("after unregister without grant: want ErrRPCDenied, got %v", err) + } +} + +func TestRPCRegistryCallDeniesBeforeRevealingServiceState(t *testing.T) { + r := NewRPCRegistry() + var calls int + handler := func(context.Context, string, []byte) ([]byte, error) { + calls++ + return []byte("ok"), nil + } + if err := r.Register("active.owner", "active.owner/items", "v1", []string{"list"}, handler); err != nil { + t.Fatal(err) + } + if err := r.Register("disabled.owner", "disabled.owner/items", "v1", []string{"list"}, handler); err != nil { + t.Fatal(err) + } + r.SetOwnerActive(func(pluginID string) bool { + return pluginID == "active.owner" + }) + + for _, service := range []string{ + "missing.owner/items", + "active.owner/items", + "disabled.owner/items", + } { + if _, err := r.Call(context.Background(), "caller.plugin", service, "list", nil); !errors.Is(err, ErrRPCDenied) { + t.Fatalf("ungranted call to %s: want ErrRPCDenied, got %v", service, err) + } + } + if calls != 0 { + t.Fatalf("ungranted calls reached handler %d times", calls) + } +} + +func TestRPCRegistryAuthorizedCallStillChecksLifecycle(t *testing.T) { + r := NewRPCRegistry() + if err := r.Register("disabled.owner", "disabled.owner/items", "v1", []string{"list"}, + func(context.Context, string, []byte) ([]byte, error) { return []byte("ok"), nil }); err != nil { + t.Fatal(err) + } + r.SetOwnerActive(func(string) bool { return false }) + r.Allow("caller.plugin", "disabled.owner/items") + + if _, err := r.Call(context.Background(), "caller.plugin", "disabled.owner/items", "list", nil); !errors.Is(err, ErrRPCOwnerInactive) { + t.Fatalf("authorized inactive call: want ErrRPCOwnerInactive, got %v", err) + } + if _, err := r.CallOperator(context.Background(), "disabled.owner/items", "list", nil); !errors.Is(err, ErrRPCOwnerInactive) { + t.Fatalf("operator inactive call: want ErrRPCOwnerInactive, got %v", err) } }