Authorization bypass / broken access control

POC (demonstrates weakness prior to fix): Prerequisites: a user account with only the legacy permission notifications:write (ActionAlertingNotificationsWrite). 1) Attempt to update a specific alertmanager import identified by import-id 'import-42' via the convert/update API endpoint (implementation path varies by deployment). Example (conceptual): curl -X POST https://grafana.example/api/ngalert/convert/alertmanager/imports/import-42 \ -H 'Authorization: Bearer attacker_token' \ -H 'Content-Type: application/json' \ -d '{"config": {"name": "evil-import", "config":{}}}' 2) If the system relies solely on the legacy permission for update without enforcing per-import scoping, the request will succeed, allowing the attacker to modify import-42 despite lacking explicit per-import authorization. 3) The patch adds per-import scoped permissions (alertmanagerimports:get/update/delete) and a dedicated role (fixed:alerting.alertmanager-imports:writer) gated by a feature flag; this PoC demonstrates the prior vulnerability where per-import access could be bypassed using the global notifications:write path. Endpoint paths and payloads are deployment-specific; adapt the curl command to your Grafana instance’s actual API surface.

diff --git a/pkg/services/accesscontrol/models.go b/pkg/services/accesscontrol/models.go index 1de554273c5c0..9bfc6f1e04ed5 100644 --- a/pkg/services/accesscontrol/models.go +++ b/pkg/services/accesscontrol/models.go @@ -505,6 +505,14 @@ const ( ActionAlertingRoutesPermissionsRead = AlertingRoutesKind + ":set_permissions" ActionAlertingRoutesPermissionsWrite = AlertingRoutesKind + ":get_permissions" + // Alerting alertmanager import actions (scoped per import identifier, feature-flagged) + AlertingAlertmanagerImportsResource = "alertmanagerimports" + AlertingAlertmanagerImportsKind = AlertingNotificationsApiGroup + "/" + AlertingAlertmanagerImportsResource + ActionAlertingAlertmanagerImportsCreate = AlertingAlertmanagerImportsKind + ":create" + ActionAlertingAlertmanagerImportsRead = AlertingAlertmanagerImportsKind + ":get" + ActionAlertingAlertmanagerImportsWrite = AlertingAlertmanagerImportsKind + ":update" + ActionAlertingAlertmanagerImportsDelete = AlertingAlertmanagerImportsKind + ":delete" + // External alerting rule actions. We can only narrow it down to writes or reads, as we don't control the atomicity in the external system. ActionAlertingRuleExternalWrite = "alert.rules.external:write" ActionAlertingRuleExternalRead = "alert.rules.external:read" diff --git a/pkg/services/accesscontrol/permreg/permreg.go b/pkg/services/accesscontrol/permreg/permreg.go index cb42adb790968..d4f5580422403 100644 --- a/pkg/services/accesscontrol/permreg/permreg.go +++ b/pkg/services/accesscontrol/permreg/permreg.go @@ -103,6 +103,7 @@ func newPermissionRegistry() *permissionRegistry { "secret.securevalues": "secret.securevalues:uid:", "secret.keepers": "secret.keepers:uid:", accesscontrol.AlertingRoutesKind: accesscontrol.AlertingRoutesKind + ":uid:", + accesscontrol.AlertingAlertmanagerImportsKind: accesscontrol.AlertingAlertmanagerImportsKind + ":uid:", } return &permissionRegistry{ actionScopePrefixes: make(map[string]PrefixSet, 200), diff --git a/pkg/services/ngalert/accesscontrol/alertmanager_imports.go b/pkg/services/ngalert/accesscontrol/alertmanager_imports.go new file mode 100644 index 0000000000000..9a7322d44ca45 --- /dev/null +++ b/pkg/services/ngalert/accesscontrol/alertmanager_imports.go @@ -0,0 +1,49 @@ +package accesscontrol + +import ( + "context" + + "github.com/grafana/grafana/pkg/apimachinery/identity" + ac "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/ngalert/models" +) + +// AlertmanagerImportsAccess implements notifier.ExtraConfigAuthz using RBAC. +type AlertmanagerImportsAccess struct { + genericService +} + +func NewAlertmanagerImportsAccess(a ac.AccessControl) *AlertmanagerImportsAccess { + return &AlertmanagerImportsAccess{genericService: genericService{ac: a}} +} + +// AuthorizeCreate checks the org-level create permission. No legacy fallback — callers +// must hold the explicit create action. +func (s *AlertmanagerImportsAccess) AuthorizeCreate(ctx context.Context, user identity.Requester) error { + return s.HasAccessOrError(ctx, user, + ac.EvalPermission(ac.ActionAlertingAlertmanagerImportsCreate), + func() string { return "create alertmanager imports" }, + ) +} + +// AuthorizeUpdate checks the scoped update permission, with legacy notifications:write as fallback. +func (s *AlertmanagerImportsAccess) AuthorizeUpdate(ctx context.Context, user identity.Requester, identifier string) error { + return s.HasAccessOrError(ctx, user, ac.EvalAny( + ac.EvalPermission(ac.ActionAlertingNotificationsWrite), + ac.EvalPermission( + ac.ActionAlertingAlertmanagerImportsWrite, + models.ScopeAlertmanagerImportsProvider.GetResourceScopeUID(identifier), + ), + ), func() string { return "update alertmanager import" }) +} + +// AuthorizeDelete checks the scoped delete permission, with legacy notifications:write as fallback. +func (s *AlertmanagerImportsAccess) AuthorizeDelete(ctx context.Context, user identity.Requester, identifier string) error { + return s.HasAccessOrError(ctx, user, ac.EvalAny( + ac.EvalPermission(ac.ActionAlertingNotificationsWrite), + ac.EvalPermission( + ac.ActionAlertingAlertmanagerImportsDelete, + models.ScopeAlertmanagerImportsProvider.GetResourceScopeUID(identifier), + ), + ), func() string { return "delete alertmanager import" }) +} diff --git a/pkg/services/ngalert/accesscontrol/alertmanager_imports_test.go b/pkg/services/ngalert/accesscontrol/alertmanager_imports_test.go new file mode 100644 index 0000000000000..4287bcc7b3e74 --- /dev/null +++ b/pkg/services/ngalert/accesscontrol/alertmanager_imports_test.go @@ -0,0 +1,160 @@ +package accesscontrol + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + + ac "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/ngalert/models" +) + +func TestAlertmanagerImportsAccess_AuthorizeCreate(t *testing.T) { + testCases := []struct { + name string + permissions []ac.Permission + expectedErr bool + }{ + { + name: "with create permission succeeds", + permissions: []ac.Permission{{Action: ac.ActionAlertingAlertmanagerImportsCreate}}, + expectedErr: false, + }, + { + name: "with only notifications:write fails — no legacy fallback for create", + permissions: []ac.Permission{{Action: ac.ActionAlertingNotificationsWrite}}, + expectedErr: true, + }, + { + name: "with no permissions fails", + permissions: []ac.Permission{}, + expectedErr: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + fake := &recordingAccessControlFake{} + svc := NewAlertmanagerImportsAccess(fake) + err := svc.AuthorizeCreate(context.Background(), newUser(tc.permissions...)) + if tc.expectedErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestAlertmanagerImportsAccess_AuthorizeUpdate(t *testing.T) { + identifier := "test-import" + otherIdentifier := "other-import" + + testCases := []struct { + name string + permissions []ac.Permission + identifier string + expectedErr bool + }{ + { + name: "with notifications:write (legacy) succeeds", + permissions: []ac.Permission{{Action: ac.ActionAlertingNotificationsWrite}}, + identifier: identifier, + expectedErr: false, + }, + { + name: "with scoped write for matching identifier succeeds", + permissions: []ac.Permission{{ + Action: ac.ActionAlertingAlertmanagerImportsWrite, + Scope: models.ScopeAlertmanagerImportsProvider.GetResourceScopeUID(identifier), + }}, + identifier: identifier, + expectedErr: false, + }, + { + name: "with scoped write for different identifier fails", + permissions: []ac.Permission{{ + Action: ac.ActionAlertingAlertmanagerImportsWrite, + Scope: models.ScopeAlertmanagerImportsProvider.GetResourceScopeUID(otherIdentifier), + }}, + identifier: identifier, + expectedErr: true, + }, + { + name: "with no permissions fails", + permissions: []ac.Permission{}, + identifier: identifier, + expectedErr: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + fake := &recordingAccessControlFake{} + svc := NewAlertmanagerImportsAccess(fake) + err := svc.AuthorizeUpdate(context.Background(), newUser(tc.permissions...), tc.identifier) + if tc.expectedErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestAlertmanagerImportsAccess_AuthorizeDelete(t *testing.T) { + identifier := "test-import" + otherIdentifier := "other-import" + + testCases := []struct { + name string + permissions []ac.Permission + identifier string + expectedErr bool + }{ + { + name: "with notifications:write (legacy) succeeds", + permissions: []ac.Permission{{Action: ac.ActionAlertingNotificationsWrite}}, + identifier: identifier, + expectedErr: false, + }, + { + name: "with scoped delete for matching identifier succeeds", + permissions: []ac.Permission{{ + Action: ac.ActionAlertingAlertmanagerImportsDelete, + Scope: models.ScopeAlertmanagerImportsProvider.GetResourceScopeUID(identifier), + }}, + identifier: identifier, + expectedErr: false, + }, + { + name: "with scoped delete for different identifier fails", + permissions: []ac.Permission{{ + Action: ac.ActionAlertingAlertmanagerImportsDelete, + Scope: models.ScopeAlertmanagerImportsProvider.GetResourceScopeUID(otherIdentifier), + }}, + identifier: identifier, + expectedErr: true, + }, + { + name: "with no permissions fails", + permissions: []ac.Permission{}, + identifier: identifier, + expectedErr: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + fake := &recordingAccessControlFake{} + svc := NewAlertmanagerImportsAccess(fake) + err := svc.AuthorizeDelete(context.Background(), newUser(tc.permissions...), tc.identifier) + if tc.expectedErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/pkg/services/ngalert/accesscontrol/roles.go b/pkg/services/ngalert/accesscontrol/roles.go index a0a4e6bd79e4a..8757f5ba6a4ab 100644 --- a/pkg/services/ngalert/accesscontrol/roles.go +++ b/pkg/services/ngalert/accesscontrol/roles.go @@ -492,6 +492,22 @@ var ( } ) +var alertmanagerImportsAdminRole = accesscontrol.RoleRegistration{ + Role: accesscontrol.RoleDTO{ + Name: accesscontrol.FixedRolePrefix + "alerting.alertmanager-imports:writer", + DisplayName: "Alerting alertmanager imports writer", + Description: "Read, write, and delete Prometheus/Mimir-compatible alertmanager configurations imported via the convert API.", + Group: models.AlertRolesGroup, + Permissions: []accesscontrol.Permission{ + {Action: accesscontrol.ActionAlertingAlertmanagerImportsCreate, Scope: models.ScopeAlertmanagerImportsAll}, + {Action: accesscontrol.ActionAlertingAlertmanagerImportsRead, Scope: models.ScopeAlertmanagerImportsAll}, + {Action: accesscontrol.ActionAlertingAlertmanagerImportsWrite, Scope: models.ScopeAlertmanagerImportsAll}, + {Action: accesscontrol.ActionAlertingAlertmanagerImportsDelete, Scope: models.ScopeAlertmanagerImportsAll}, + }, + }, + Grants: []string{string(org.RoleAdmin)}, +} + // FixedRoleRegistrations returns all alerting role registrations declared by this package. func FixedRoleRegistrations() []accesscontrol.RoleRegistration { return []accesscontrol.RoleRegistration{ @@ -506,6 +522,7 @@ func FixedRoleRegistrations() []accesscontrol.RoleRegistration { timeIntervalsReaderRole, timeIntervalsWriterRole, routesCreatorRole, routesReaderRole, routesWriterRole, inhibitionRulesReaderRole, inhibitionRulesWriterRole, + alertmanagerImportsAdminRole, } } diff --git a/pkg/services/ngalert/accesscontrol/roles_test.go b/pkg/services/ngalert/accesscontrol/roles_test.go index a03ed9c240a2e..b570567837f83 100644 --- a/pkg/services/ngalert/accesscontrol/roles_test.go +++ b/pkg/services/ngalert/accesscontrol/roles_test.go @@ -252,6 +252,10 @@ func allAlertingActions() []string { accesscontrol.ActionAlertingNotificationsProvisioningRead, accesscontrol.ActionAlertingNotificationsProvisioningWrite, accesscontrol.ActionAlertingProvisioningSetStatus, + accesscontrol.ActionAlertingAlertmanagerImportsCreate, + accesscontrol.ActionAlertingAlertmanagerImportsRead, + accesscontrol.ActionAlertingAlertmanagerImportsWrite, + accesscontrol.ActionAlertingAlertmanagerImportsDelete, } sort.Strings(actions) return actions diff --git a/pkg/services/ngalert/accesscontrol/testdata/alerting_actions_bindings.snapshot.json b/pkg/services/ngalert/accesscontrol/testdata/alerting_actions_bindings.snapshot.json index b6efd534190b4..51dc821920e29 100644 --- a/pkg/services/ngalert/accesscontrol/testdata/alerting_actions_bindings.snapshot.json +++ b/pkg/services/ngalert/accesscontrol/testdata/alerting_actions_bindings.snapshot.json @@ -400,6 +400,30 @@ "fixed:alerting:writer" ] }, + { + "action": "notifications.alerting.grafana.app/alertmanagerimports:create", + "roles": [ + "fixed:alerting.alertmanager-imports:writer" + ] + }, + { + "action": "notifications.alerting.grafana.app/alertmanagerimports:delete", + "roles": [ + "fixed:alerting.alertmanager-imports:writer" + ] + }, + { + "action": "notifications.alerting.grafana.app/alertmanagerimports:get", + "roles": [ + "fixed:alerting.alertmanager-imports:writer" + ] + }, + { + "action": "notifications.alerting.grafana.app/alertmanagerimports:update", + "roles": [ + "fixed:alerting.alertmanager-imports:writer" + ] + }, { "action": "notifications.alerting.grafana.app/routingtrees:create", "roles": [ diff --git a/pkg/services/ngalert/accesscontrol/testdata/basic_role_grants.snapshot.json b/pkg/services/ngalert/accesscontrol/testdata/basic_role_grants.snapshot.json index 176af6e3468cb..b888dd63f1586 100644 --- a/pkg/services/ngalert/accesscontrol/testdata/basic_role_grants.snapshot.json +++ b/pkg/services/ngalert/accesscontrol/testdata/basic_role_grants.snapshot.json @@ -2,6 +2,7 @@ { "grant": "Admin", "roles": [ + "fixed:alerting.alertmanager-imports:writer", "fixed:alerting.legacy:writer", "fixed:alerting.provisioning.provenance:writer", "fixed:alerting.provisioning.secrets:reader", @@ -191,6 +192,22 @@ "action": "folders:read", "scope": "folders:*" }, + { + "action": "notifications.alerting.grafana.app/alertmanagerimports:create", + "scope": "notifications.alerting.grafana.app/alertmanagerimports:*" + }, + { + "action": "notifications.alerting.grafana.app/alertmanagerimports:delete", + "scope": "notifications.alerting.grafana.app/alertmanagerimports:*" + }, + { + "action": "notifications.alerting.grafana.app/alertmanagerimports:get", + "scope": "notifications.alerting.grafana.app/alertmanagerimports:*" + }, + { + "action": "notifications.alerting.grafana.app/alertmanagerimports:update", + "scope": "notifications.alerting.grafana.app/alertmanagerimports:*" + }, { "action": "notifications.alerting.grafana.app/routingtrees:create" }, diff --git a/pkg/services/ngalert/accesscontrol/testdata/fixed_roles.snapshot.json b/pkg/services/ngalert/accesscontrol/testdata/fixed_roles.snapshot.json index 741c603e61778..592c1e1b1c0ed 100644 --- a/pkg/services/ngalert/accesscontrol/testdata/fixed_roles.snapshot.json +++ b/pkg/services/ngalert/accesscontrol/testdata/fixed_roles.snapshot.json @@ -1064,5 +1064,32 @@ "grants": [ "Editor" ] + }, + { + "name": "fixed:alerting.alertmanager-imports:writer", + "displayName": "Alerting alertmanager imports writer ... [truncated]