Privilege escalation via overly broad annotation scopes

PoC (live-API path is deployment-specific): Reproduce with a Grafana deployment configured with annotation.max-scope-count = 3. Attempt to create an annotation with four scopes. Expect a 400 Bad Request and the annotation not being persisted. Example REST-path approach (adjust to your deployment): 1) Set the limit to 3 in conf/defaults.ini or via environment/config mechanism and restart Grafana. 2) Attempt to create an annotation with four scopes via the backend API: curl -X POST "http://grafana-host/api/annotations" \ -H "Authorization: Bearer <TOKEN>" \ -H "Content-Type: application/json" \ -d '{"name":"test-anno","namespace":"org-1","spec":{"text":"hello","time":1000,"scopes":["scope-1","scope-2","scope-3","scope-4"]}}' Expected: HTTP 400 Bad Request with a message containing "too many scopes" and the annotation is not created. Alternative (Kubernetes CRD backend): If the annotation store is backed by a Kubernetes CRD, apply a resource with 4 scopes under the grafana annotation CRD. The API server should reject with a similar error indicating the max scope count. 3) In a unit/integration test context (as in the repo): - Set adapter.maxScopeCount = 3 - Attempt to Create an annotation whose Spec.Scopes has length 4 - Expect an error and verify the resource is not persisted.

diff --git a/conf/defaults.ini b/conf/defaults.ini index 23bd1d5fd7281..be0b3fd6d9802 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -1803,6 +1803,10 @@ retention_ttl = 2160h # Generate and persist grafana.app/legacyID labels for legacy API compatibility. enable_legacy_id = false +# Maximum number of scopes that can be attached to a single annotation. +# Set to 0 to disallow scopes entirely. Negative values are rejected. +max_scope_count = 5 + # gRPC server address for remote annotation storage (only used when store_backend = "grpc") grpc_address = localhost:9090 diff --git a/conf/sample.ini b/conf/sample.ini index a54138fb23c90..1d0d96ff0d3b5 100644 --- a/conf/sample.ini +++ b/conf/sample.ini @@ -1734,6 +1734,10 @@ default_datasource_uid = # Generate and persist grafana.app/legacyID labels for legacy API compatibility. ;enable_legacy_id = false +# Maximum number of scopes that can be attached to a single annotation. +# Set to 0 to disallow scopes entirely. Negative values are rejected. +;max_scope_count = 5 + # gRPC server address for remote annotation storage (only used when store_backend = "grpc") ;grpc_address = localhost:9090 diff --git a/pkg/registry/apps/annotation/config.go b/pkg/registry/apps/annotation/config.go index 452630c5f8a1a..150019201751b 100644 --- a/pkg/registry/apps/annotation/config.go +++ b/pkg/registry/apps/annotation/config.go @@ -15,6 +15,9 @@ const ( // defaultRetentionTTL is the default retention period for annotations // TODO: determine appropriate default TTL defaultRetentionTTL = 90 * 24 * time.Hour + // defaultMaxScopeCount caps how many scopes can be attached to a single + // annotation. 0 means no scopes are allowed. + defaultMaxScopeCount = 5 ) // Config holds the store backend configuration for the annotation app. @@ -42,6 +45,11 @@ type Config struct { // for new annotations and persisted in the store. EnableLegacyID bool + // MaxScopeCount caps how many scopes can be attached to a single + // annotation. 0 means no scopes are allowed. Negative values are + // rejected by the settings loader. + MaxScopeCount int + // CleanupSettings configures annotation pruning for the SQL backend's LifecycleManager. // Zero value (all limits unset) disables cleanup. Not used by memory or gRPC backends. CleanupSettings annotations.CleanupSettings @@ -55,6 +63,8 @@ func (c *Config) AddFlags(flags *pflag.FlagSet) { // General lifecycle flags flags.DurationVar(&c.RetentionTTL, "annotation.retention-ttl", defaultRetentionTTL, "Retention TTL for annotations (old data will be cleaned up)") + flags.IntVar(&c.MaxScopeCount, "annotation.max-scope-count", defaultMaxScopeCount, "Maximum number of scopes that can be attached to a single annotation") + // gRPC flags flags.StringVar(&c.GRPCAddress, "annotation.grpc-address", "", "gRPC server address for the annotation store") flags.BoolVar(&c.GRPCUseTLS, "annotation.grpc-use-tls", false, "Enable TLS for the annotation gRPC connection") @@ -80,6 +90,7 @@ func newConfigFromSettings(cfg *setting.Cfg) Config { StoreBackend: cfg.AnnotationAppPlatform.StoreBackend, RetentionTTL: retentionTTL, EnableLegacyID: cfg.AnnotationAppPlatform.EnableLegacyID, + MaxScopeCount: cfg.AnnotationAppPlatform.MaxScopeCount, GRPCAddress: cfg.AnnotationAppPlatform.GRPCAddress, GRPCUseTLS: cfg.AnnotationAppPlatform.GRPCUseTLS, diff --git a/pkg/registry/apps/annotation/k8s_adapter.go b/pkg/registry/apps/annotation/k8s_adapter.go index 394d618658306..6e3024cdee67c 100644 --- a/pkg/registry/apps/annotation/k8s_adapter.go +++ b/pkg/registry/apps/annotation/k8s_adapter.go @@ -78,6 +78,11 @@ type k8sRESTAdapter struct { snowflakeNode *snowflake.Node + // maxScopeCount caps how many scopes may be attached to a single + // annotation. 0 means no scopes are allowed. Negative values are + // rejected by the settings loader. + maxScopeCount int + tracer trace.Tracer metrics *Metrics logger log.Logger @@ -226,6 +231,10 @@ func (s *k8sRESTAdapter) Create(ctx context.Context, annotation.Name = annotation.GenerateName + util.GenerateShortUID() } + if err := s.validateScopeCount(annotation); err != nil { + return nil, err + } + user, err := identity.GetRequester(ctx) if err != nil { return nil, apierrors.NewUnauthorized("failed to get requester from context") @@ -287,6 +296,10 @@ func (s *k8sRESTAdapter) Update(ctx context.Context, return nil, false, apierrors.NewBadRequest("namespace in URL does not match namespace in body") } + if err := s.validateScopeCount(resource); err != nil { + return nil, false, err + } + // Check authz on both existing and new body: prevents privilege escalation via scope changes. allowed, err := canAccessAnnotation(ctx, s.accessClient, s.folderResolver, namespace, existing, utils.VerbUpdate) if err != nil { @@ -389,3 +402,11 @@ func parseFieldSelector(fs fields.Selector, opts *ListOptions) error { } return nil } + +func (s *k8sRESTAdapter) validateScopeCount(a *annotationV0.Annotation) error { + if len(a.Spec.Scopes) > s.maxScopeCount { + return apierrors.NewBadRequest(fmt.Sprintf( + "too many scopes: %d (max allowed %d)", len(a.Spec.Scopes), s.maxScopeCount)) + } + return nil +} diff --git a/pkg/registry/apps/annotation/k8s_adapter_test.go b/pkg/registry/apps/annotation/k8s_adapter_test.go index 81408537e188c..2cf8c99c2d983 100644 --- a/pkg/registry/apps/annotation/k8s_adapter_test.go +++ b/pkg/registry/apps/annotation/k8s_adapter_test.go @@ -316,6 +316,92 @@ func TestK8sAdapter_List(t *testing.T) { }) } +// TestK8sAdapter_MaxScopeCount pins the contract for Spec.Scopes cardinality +// on both Create and Update: +// - len(Scopes) <= maxScopeCount succeeds; +// - over the limit returns 400 BadRequest and the annotation is not persisted/mutated. +// maxScopeCount = 0 is the configured "no scopes allowed" mode. +func TestK8sAdapter_MaxScopeCount(t *testing.T) { + ns := "org-1" + allowAll := &fakeAccessClient{fn: func(_ authtypes.BatchCheckItem) bool { return true }} + + buildScopes := func(n int) []string { + s := make([]string, n) + for i := range n { + s[i] = fmt.Sprintf("scope-%d", i) + } + return s + } + + cases := []struct { + name string + maxScopeCount int + scopeCount int + expectErr bool + }{ + {"at limit succeeds", 3, 3, false}, + {"over limit rejected", 3, 4, true}, + {"zero allows no scopes", 0, 0, false}, + {"zero rejects any scopes", 0, 1, true}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + store := NewMemoryStore() + adapter := newTestAdapter(store, allowAll) + adapter.maxScopeCount = tc.maxScopeCount + ctx := k8srequest.WithNamespace(identity.WithServiceIdentityContext(t.Context(), 1), ns) + + name := "anno" + obj := &annotationV0.Annotation{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: ns}, + Spec: annotationV0.AnnotationSpec{Text: "hello", Time: 1000, Scopes: buildScopes(tc.scopeCount)}, + } + _, err := adapter.Create(ctx, obj, nil, &metav1.CreateOptions{}) + + if !tc.expectErr { + require.NoError(t, err) + return + } + + require.Error(t, err) + assert.True(t, apierrors.IsBadRequest(err), "expected 400 BadRequest, got %v", err) + assert.Contains(t, err.Error(), "max allowed") + + _, getErr := store.Get(ctx, ns, name) + assert.ErrorIs(t, getErr, ErrNotFound, "annotation should not have been persisted") + }) + } + + t.Run("update over limit rejected", func(t *testing.T) { + store := NewMemoryStore() + adapter := newTestAdapter(store, allowAll) + adapter.maxScopeCount = 2 + ctx := k8srequest.WithNamespace(identity.WithServiceIdentityContext(t.Context(), 1), ns) + + name := "original" + orig := &annotationV0.Annotation{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: ns}, + Spec: annotationV0.AnnotationSpec{Text: "hello", Time: 1000, Scopes: buildScopes(1)}, + } + _, err := adapter.Create(ctx, orig, nil, &metav1.CreateOptions{}) + require.NoError(t, err) + + updated := &annotationV0.Annotation{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: ns}, + Spec: annotationV0.AnnotationSpec{Text: "hello", Time: 1000, Scopes: buildScopes(3)}, + } + _, _, err = adapter.Update(ctx, name, &updatedObjectInfo{obj: updated}, nil, nil, false, &metav1.UpdateOptions{}) + require.Error(t, err) + assert.True(t, apierrors.IsBadRequest(err), "expected 400 BadRequest, got %v", err) + assert.Contains(t, err.Error(), "max allowed") + + stored, getErr := store.Get(ctx, ns, name) + require.NoError(t, getErr, "original annotation must still exist") + assert.Len(t, stored.Spec.Scopes, 1, "stored annotation must not have been mutated") + }) +} + // compile-time assertion that errStore implements Store var _ Store = (*errStore)(nil) diff --git a/pkg/registry/apps/annotation/register.go b/pkg/registry/apps/annotation/register.go index fd9dfecb2efb2..2f1f902e84276 100644 --- a/pkg/registry/apps/annotation/register.go +++ b/pkg/registry/apps/annotation/register.go @@ -124,6 +124,7 @@ func NewAppInstaller( folderResolver: folderResolver, installer: installer, snowflakeNode: sfNode, + maxScopeCount: cfg.MaxScopeCount, tracer: installer.tracer, metrics: installer.metrics, logger: logger, diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index 830b432cab89b..be3c5c8847ed7 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -1067,7 +1067,11 @@ func (cfg *Cfg) readAnnotationSettings() error { section := cfg.Raw.Section("annotations") cfg.AnnotationCleanupJobBatchSize = section.Key("cleanupjob_batchsize").MustInt64(100) cfg.AnnotationMaximumTagsLength = section.Key("tags_length").MustInt64(500) - cfg.AnnotationAppPlatform = loadAnnotationAppPlatformSettings(cfg.Raw) + annotationAppPlatformSettings, err := loadAnnotationAppPlatformSettings(cfg.Raw) + if err != nil { + return err + } + cfg.AnnotationAppPlatform = annotationAppPlatformSettings switch { case cfg.AnnotationMaximumTagsLength > 4096: @@ -1151,15 +1155,21 @@ type AnnotationAppPlatformSettings struct { // EnableLegacyID controls whether a grafana.app/legacyID label is generated // for new annotations. EnableLegacyID bool + + // MaxScopeCount caps how many scopes may be attached to a single + // annotation. 0 means no scopes are allowed. Negative values are + // rejected at load time. Default 5. + MaxScopeCount int } -func loadAnnotationAppPlatformSettings(cfg *ini.File) AnnotationAppPlatformSettings { +func loadAnnotationAppPlatformSettings(cfg *ini.File) (AnnotationAppPlatformSettings, error) { appPlatformSection := cfg.Section("annotations.app_platform") - return AnnotationAppPlatformSettings{ + settings := AnnotationAppPlatformSettings{ Enabled: appPlatformSection.Key("enabled").MustBool(false), StoreBackend: appPlatformSection.Key("store_backend").MustString("legacy-sql"), RetentionTTL: appPlatformSection.Key("retention_ttl").MustDuration(2160 * time.Hour), EnableLegacyID: appPlatformSection.Key("enable_legacy_id").MustBool(false), + MaxScopeCount: appPlatformSection.Key("max_scope_count").MustInt(5), GRPCAddress: appPlatformSection.Key("grpc_address").MustString("localhost:9090"), GRPCUseTLS: appPlatformSection.Key("grpc_use_tls").MustBool(false), @@ -1174,6 +1184,12 @@ func loadAnnotationAppPlatformSettings(cfg *ini.File) AnnotationAppPlatformSetti PostgresTagCacheTTL: appPlatformSection.Key("postgres_tag_cache_ttl").MustDuration(60 * time.Second), PostgresTagCacheSize: appPlatformSection.Key("postgres_tag_cache_size").MustInt(1000), } + + if settings.MaxScopeCount < 0 { + return AnnotationAppPlatformSettings{}, fmt.Errorf("[annotations.app_platform.max_scope_count] must not be negative") + } + + return settings, nil } // envNameFromIniName converts an ini-style name (section or key) to the diff --git a/pkg/setting/setting_annotations_test.go b/pkg/setting/setting_annotations_test.go new file mode 100644 index 0000000000000..1554a8a7316be --- /dev/null +++ b/pkg/setting/setting_annotations_test.go @@ -0,0 +1,45 @@ +package setting + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gopkg.in/ini.v1" +) + +func TestLoadAnnotationAppPlatformSettings_MaxScopeCount(t *testing.T) { + cases := []struct { + name string + iniValue *string // nil means no key set + expectedMaxScopeCount int + expectErr bool + }{ + {name: "default when key absent", expectedMaxScopeCount: 5}, + {name: "explicit positive", iniValue: new("10"), expectedMaxScopeCount: 10}, + {name: "zero is accepted", iniValue: new("0"), expectedMaxScopeCount: 0}, + {name: "negative is rejected", iniValue: new("-1"), expectErr: true}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + iniFile := ini.Empty() + if tc.iniValue != nil { + section, err := iniFile.NewSection("annotations.app_platform") + require.NoError(t, err) + + _, err = section.NewKey("max_scope_count", *tc.iniValue) + require.NoError(t, err) + } + + settings, err := loadAnnotationAppPlatformSettings(iniFile) + if tc.expectErr { + assert.Error(t, err) + return + } + + require.NoError(t, err) + assert.Equal(t, tc.expectedMaxScopeCount, settings.MaxScopeCount) + }) + } +}