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) } }