Authorization bypass / Information disclosure via initial watch backfill in Unified Storage

PoC outline to reproduce the vulnerability before the fix: 1) Set up a Watch on a resource group where the caller has read access to a subset of items (e.g., allowed-item) but not to others (e.g., denied-item). 2) Seed the backend with two resources: one the caller is allowed to see and one they are not (e.g., playlists in different folders). 3) Initiate a Watch request with SendInitialEvents set to true so the initial backfill is emitted. 4) Observe the initial events stream. If the stream includes the denied-item (even though the caller lacks per-item access), information about that resource is disclosed. Expected pre-fix behavior (vulnerability): The initial-events backfill would emit both items, enabling information disclosure and potential authorization bypass during watch initialization. Expected post-fix behavior (after this commit): The initial-events backfill is filtered by the ItemChecker, so only allowed items are emitted in the initial stream, preventing leakage of denied resources. Concrete steps you can reproduce in a test environment modeled after Grafana’s tests: - Create two resources: allowed-item and denied-item, assigning the denied-item to a folder/name the checker will deny for the test user. - Configure an AccessClient (per-item checker) that denies reads for the denied-item name (verb GET). - Start a Watch with SendInitialEvents=true for the resource group. - Drain initial events and verify that only allowed-item is present; the denied-item should be absent. Code sketch (conceptual, aligns with the patch in TestWatchInitialEventsRespectsItemChecker): - Implement an AccessClient.Check that returns Allowed for all but the denied-item path. - Use a test server with AccessClient, seed two resources, and run a Watch with SendInitialEvents=true. - Collect and inspect initial ADDED events; ensure denied-item does not appear.

diff --git a/pkg/extensions/enterprise_imports.go b/pkg/extensions/enterprise_imports.go index 30ca114055639..8a4dca1a0f1f5 100644 --- a/pkg/extensions/enterprise_imports.go +++ b/pkg/extensions/enterprise_imports.go @@ -109,6 +109,8 @@ import ( _ "github.com/grafana/grafana/apps/advisor/pkg/app/checks/datasourcecheck" _ "github.com/grafana/grafana/apps/advisor/pkg/app/checks/plugincheck" _ "github.com/grafana/grafana/apps/alerting/alertenrichment/pkg/apis/alertenrichment/v1beta1" + _ "github.com/grafana/grafana/apps/alerting/historian/pkg/apis" + _ "github.com/grafana/grafana/apps/alerting/historian/pkg/app" _ "github.com/grafana/grafana/apps/alerting/historian/pkg/app/config" _ "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1" _ "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v2" @@ -299,6 +301,7 @@ import ( _ "github.com/grafana/grafana/pkg/registry/fieldselectors" _ "github.com/grafana/grafana/pkg/registry/usagestatssvcs" _ "github.com/grafana/grafana/pkg/semconv" + _ "github.com/grafana/grafana/pkg/server" _ "github.com/grafana/grafana/pkg/services/accesscontrol" _ "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" _ "github.com/grafana/grafana/pkg/services/accesscontrol/actest" diff --git a/pkg/storage/unified/resource/server.go b/pkg/storage/unified/resource/server.go index a22398ed800d9..c364ca1617fad 100644 --- a/pkg/storage/unified/resource/server.go +++ b/pkg/storage/unified/resource/server.go @@ -1753,6 +1753,9 @@ func (s *server) Watch(req *resourcepb.WatchRequest, srv resourcepb.ResourceStor if err := iter.Error(); err != nil { return err } + if !checker(iter.Name(), iter.Folder()) { + continue + } if err := srv.Send(&resourcepb.WatchEvent{ Type: resourcepb.WatchEvent_ADDED, Resource: &resourcepb.WatchEvent_Resource{ diff --git a/pkg/storage/unified/resource/server_test.go b/pkg/storage/unified/resource/server_test.go index 60df4b6a4e817..d71af5b41a23b 100644 --- a/pkg/storage/unified/resource/server_test.go +++ b/pkg/storage/unified/resource/server_test.go @@ -1104,6 +1104,7 @@ func newWatchTestUser() *identity.StaticRequester { type watchTestServerOpts struct { BookmarkFrequency time.Duration StorageMetrics *StorageMetrics + AccessClient authlib.AccessClient } func newWatchTestServer(t *testing.T, opts watchTestServerOpts) *server { @@ -1124,6 +1125,7 @@ func newWatchTestServer(t *testing.T, opts watchTestServerOpts) *server { Backend: store, BookmarkFrequency: opts.BookmarkFrequency, StorageMetrics: opts.StorageMetrics, + AccessClient: opts.AccessClient, }) require.NoError(t, err) t.Cleanup(func() { @@ -1383,7 +1385,94 @@ func TestWatchEventMetricsWithSinceRV(t *testing.T) { "WatchEventLatency should only observe events that arrived after the subscription started") } +// TestWatchInitialEventsRespectsItemChecker tests that checker is used for +// initial-events when SendInitialEvents=true. +func TestWatchInitialEventsRespectsItemChecker(t *testing.T) { + const ( + allowedFolder = "folder-a" + deniedFolder = "folder-b" + allowedName = "allowed-playlist" + deniedName = "denied-playlist" + ) + + ac := &callbackAccessClient{ + fn: func(req authlib.CheckRequest, folder string) (authlib.CheckResponse, error) { + // Only fail reads, allow other verbs to seed test resources + if req.Verb == utils.VerbGet && folder != allowedFolder { + return deny() + } + return allow() + }, + } + srv := newWatchTestServer(t, watchTestServerOpts{AccessClient: ac}) + + user := newWatchTestUser() + ctx, cancel := context.WithCancel(authlib.WithAuthInfo(t.Context(), user)) + defer cancel() + + for _, item := range []struct { + name, folder string + }{ + {allowedName, allowedFolder}, + {deniedName, deniedFolder}, + } { + value := []byte(`{"apiVersion":"playlist.grafana.app/v0alpha1","kind":"Playlist","metadata":{"name":"` + item.name + `","uid":"uid-` + item.name + `","namespace":"` + watchTestNamespace + `","annotations":{"grafana.app/folder":"` + item.folder + `"}},"spec":{"title":"t","interval":"5m","items":[]}}`) + created, err := srv.Create(ctx, &resourcepb.CreateRequest{ + Key: &resourcepb.ResourceKey{ + Group: watchTestGroup, + Resource: watchTestResource, + Namespace: watchTestNamespace, + Name: item.name, + }, + Value: value, + }) + require.NoError(t, err) + require.Nil(t, created.Error, "creating seed resource %q", item.name) + } + + mock := newMockWatchServer(ctx) + var eg errgroup.Group + eg.Go(func() error { + return srv.Watch(&resourcepb.WatchRequest{ + Options: &resourcepb.ListOptions{ + Key: &resourcepb.ResourceKey{ + Group: watchTestGroup, + Resource: watchTestResource, + Namespace: watchTestNamespace, + }, + }, + SendInitialEvents: true, + }, mock) + }) + + // Drain events + var added []*resourcepb.WatchEvent + deadline := time.After(2 * time.Second) +drain: + for { + select { + case evt := <-mock.events: + if evt.Type == resourcepb.WatchEvent_ADDED { + added = append(added, evt) + } + case <-deadline: + break drain + } + } + + cancel() + require.NoError(t, eg.Wait()) + + for _, evt := range added { + require.NotContains(t, string(evt.Resource.Value), `"name":"`+deniedName+`"`) + } + require.Len(t, added, 1) + require.Contains(t, string(added[0].Resource.Value), `"name":"`+allowedName+`"`) +} + // callbackAccessClient is a test helper whose Check behavior can be swapped between calls. +// Compile returns an ItemChecker that delegates to the same fn (with Verb=get), +// so a single callback drives both Check and per-item authorization. type callbackAccessClient struct { fn func(req authlib.CheckRequest, folder string) (authlib.CheckResponse, error) } @@ -1392,8 +1481,20 @@ func (c *callbackAccessClient) Check(_ context.Context, _ authlib.AuthInfo, req return c.fn(req, folder) } -func (c *callbackAccessClient) Compile(_ context.Context, _ authlib.AuthInfo, _ authlib.ListRequest) (authlib.ItemChecker, authlib.Zookie, error) { - return func(_, _ string) bool { return true }, authlib.NoopZookie{}, nil +func (c *callbackAccessClient) Compile(_ context.Context, _ authlib.AuthInfo, req authlib.ListRequest) (authlib.ItemChecker, authlib.Zookie, error) { + return func(name, folder string) bool { + resp, err := c.fn(authlib.CheckRequest{ + Verb: utils.VerbGet, + Group: req.Group, + Resource: req.Resource, + Namespace: req.Namespace, + Name: name, + }, folder) + if err != nil { + return false + } + return resp.Allowed + }, authlib.NoopZookie{}, nil } func (c *callbackAccessClient) BatchCheck(_ context.Context, _ authlib.AuthInfo, req authlib.BatchCheckRequest) (authlib.BatchCheckResponse, error) {