Privilege Escalation / Authorization bypass

Proof-of-concept (PoC) demonstrating the authorization bypass path prior to this fix: Assumptions: - A user account U has only limited permissions, e.g., ActionAlertingReceiversCreate, but not write permissions for templates, time intervals, etc. - The attacker can call the alertmanager import promotion endpoint (RouteConvertPrometheusPostAlertmanagerConfig) with header X-Grafana-Alerting-Promote set to true to promote a merged configuration. - The system processes a promotion that adds multiple resource types in a single merge (e.g., a new receiver and a new template). PoC steps (conceptual, executable in a test env): 1) Authenticate as user U with limited permissions (e.g., only receivers:create). 2) Prepare a merged configuration payload that includes multiple resource additions, for example: - AddedReceivers: ["evil_receiver"] - AddedTemplates: ["evil_template"] - AddedTimeIntervals: ["evil_interval"] - AddedRoute: "evil_route" 3) Send a promote request to the alertmanager import endpoint, e.g. (endpoint path is provider-specific): curl -X POST https://grafana.example.com/api/alerts/ngalert/convert-prometheus-config \ -H "Authorization: Bearer <token-for-user-U>" \ -H "X-Grafana-Alerting-Promote: true" \ -H "Content-Type: application/json" \ -d '{ "Identifier": "promo-test-1", "AddedReceivers": ["evil_receiver"], "AddedTemplates": ["evil_template"], "AddedTimeIntervals": ["evil_interval"], "AddedInhibitionRules": [] }' Expected outcome before the fix (vulnerability demonstration): If the server lacks per-resource promote authorization checks, promotion could proceed since the user has at least one required left-per-resource permission (e.g., receivers:create). The merged configuration would be merged into the main Grafana Alertmanager config, potentially altering alert routing and notification behavior without granting all necessary resource permissions. Expected outcome after the fix (what should happen now): The server will invoke AuthorizePromote and detect that the user does not hold all required permissions for the resources included in the promotion (e.g., missing templatesWrite, timeIntervalsWrite, etc.). The request will be rejected with an authorization error (e.g., 403 Forbidden), preventing unauthorized promotion of the configuration. This PoC demonstrates the attack surface and how the new per-resource authorization checks mitigate it.

diff --git a/pkg/services/ngalert/accesscontrol/alertmanager_imports.go b/pkg/services/ngalert/accesscontrol/alertmanager_imports.go index 9a7322d44ca45..d9225968fadb9 100644 --- a/pkg/services/ngalert/accesscontrol/alertmanager_imports.go +++ b/pkg/services/ngalert/accesscontrol/alertmanager_imports.go @@ -6,6 +6,7 @@ import ( "github.com/grafana/grafana/pkg/apimachinery/identity" ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/ngalert/models" + "github.com/grafana/grafana/pkg/services/ngalert/notifier/merge" ) // AlertmanagerImportsAccess implements notifier.ExtraConfigAuthz using RBAC. @@ -47,3 +48,28 @@ func (s *AlertmanagerImportsAccess) AuthorizeDelete(ctx context.Context, user id ), ), func() string { return "delete alertmanager import" }) } + +// AuthorizePromote checks create/write permissions for every notification resource type +// present in the import. Only the types that are actually present are checked. +func (s *AlertmanagerImportsAccess) AuthorizePromote(ctx context.Context, user identity.Requester, result merge.MergeResult) error { + var evals []ac.Evaluator + if len(result.AddedReceivers) > 0 { + evals = append(evals, ac.EvalPermission(ac.ActionAlertingReceiversCreate)) + } + if result.AddedRoute != "" { + evals = append(evals, ac.EvalPermission(ac.ActionAlertingManagedRoutesCreate)) + } + if len(result.AddedTemplates) > 0 { + evals = append(evals, ac.EvalPermission(ac.ActionAlertingNotificationsTemplatesWrite)) + } + if len(result.AddedTimeIntervals) > 0 { + evals = append(evals, ac.EvalPermission(ac.ActionAlertingNotificationsTimeIntervalsWrite)) + } + if len(result.AddedInhibitionRules) > 0 { + evals = append(evals, ac.EvalPermission(ac.ActionAlertingNotificationsInhibitionRulesWrite)) + } + if len(evals) == 0 { + return nil + } + return s.HasAccessOrError(ctx, user, ac.EvalAll(evals...), func() string { return "promote alertmanager import" }) +} diff --git a/pkg/services/ngalert/accesscontrol/alertmanager_imports_test.go b/pkg/services/ngalert/accesscontrol/alertmanager_imports_test.go index 4287bcc7b3e74..818cc73145389 100644 --- a/pkg/services/ngalert/accesscontrol/alertmanager_imports_test.go +++ b/pkg/services/ngalert/accesscontrol/alertmanager_imports_test.go @@ -8,6 +8,7 @@ import ( ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/ngalert/models" + "github.com/grafana/grafana/pkg/services/ngalert/notifier/merge" ) func TestAlertmanagerImportsAccess_AuthorizeCreate(t *testing.T) { @@ -103,6 +104,112 @@ func TestAlertmanagerImportsAccess_AuthorizeUpdate(t *testing.T) { } } +func TestAlertmanagerImportsAccess_AuthorizePromote(t *testing.T) { + testCases := []struct { + name string + permissions []ac.Permission + result merge.MergeResult + expectedErr bool + }{ + { + name: "no resources succeeds without any permissions", + result: merge.MergeResult{}, + expectedErr: false, + }, + { + name: "HasReceivers with receivers:create succeeds", + permissions: []ac.Permission{{Action: ac.ActionAlertingReceiversCreate}}, + result: merge.MergeResult{AddedReceivers: []string{"x"}}, + expectedErr: false, + }, + { + name: "HasReceivers without receivers:create fails", + result: merge.MergeResult{AddedReceivers: []string{"x"}}, + expectedErr: true, + }, + { + name: "HasRoutes with routes:create succeeds", + permissions: []ac.Permission{{Action: ac.ActionAlertingManagedRoutesCreate}}, + result: merge.MergeResult{AddedRoute: "x"}, + expectedErr: false, + }, + { + name: "HasRoutes without routes:create fails", + result: merge.MergeResult{AddedRoute: "x"}, + expectedErr: true, + }, + { + name: "HasTemplates with templates:write succeeds", + permissions: []ac.Permission{{Action: ac.ActionAlertingNotificationsTemplatesWrite}}, + result: merge.MergeResult{AddedTemplates: []string{"x"}}, + expectedErr: false, + }, + { + name: "HasTemplates without templates:write fails", + result: merge.MergeResult{AddedTemplates: []string{"x"}}, + expectedErr: true, + }, + { + name: "HasTimeIntervals with time-intervals:write succeeds", + permissions: []ac.Permission{{Action: ac.ActionAlertingNotificationsTimeIntervalsWrite}}, + result: merge.MergeResult{AddedTimeIntervals: []string{"x"}}, + expectedErr: false, + }, + { + name: "HasTimeIntervals without time-intervals:write fails", + result: merge.MergeResult{AddedTimeIntervals: []string{"x"}}, + expectedErr: true, + }, + { + name: "HasInhibitionRules with inhibition-rules:write succeeds", + permissions: []ac.Permission{{Action: ac.ActionAlertingNotificationsInhibitionRulesWrite}}, + result: merge.MergeResult{AddedInhibitionRules: []string{"x"}}, + expectedErr: false, + }, + { + name: "HasInhibitionRules without inhibition-rules:write fails", + result: merge.MergeResult{AddedInhibitionRules: []string{"x"}}, + expectedErr: true, + }, + { + name: "all resources with all permissions succeeds", + permissions: []ac.Permission{ + {Action: ac.ActionAlertingReceiversCreate}, + {Action: ac.ActionAlertingManagedRoutesCreate}, + {Action: ac.ActionAlertingNotificationsTemplatesWrite}, + {Action: ac.ActionAlertingNotificationsTimeIntervalsWrite}, + {Action: ac.ActionAlertingNotificationsInhibitionRulesWrite}, + }, + result: merge.MergeResult{AddedReceivers: []string{"x"}, AddedRoute: "x", AddedTemplates: []string{"x"}, AddedTimeIntervals: []string{"x"}, AddedInhibitionRules: []string{"x"}}, + expectedErr: false, + }, + { + name: "all resources with one permission missing fails", + permissions: []ac.Permission{ + {Action: ac.ActionAlertingReceiversCreate}, + {Action: ac.ActionAlertingManagedRoutesCreate}, + {Action: ac.ActionAlertingNotificationsTemplatesWrite}, + {Action: ac.ActionAlertingNotificationsTimeIntervalsWrite}, + }, + result: merge.MergeResult{AddedReceivers: []string{"x"}, AddedRoute: "x", AddedTemplates: []string{"x"}, AddedTimeIntervals: []string{"x"}, AddedInhibitionRules: []string{"x"}}, + expectedErr: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + fake := &recordingAccessControlFake{} + svc := NewAlertmanagerImportsAccess(fake) + err := svc.AuthorizePromote(context.Background(), newUser(tc.permissions...), tc.result) + if tc.expectedErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +} + func TestAlertmanagerImportsAccess_AuthorizeDelete(t *testing.T) { identifier := "test-import" otherIdentifier := "other-import" diff --git a/pkg/services/ngalert/api/api_convert_prometheus.go b/pkg/services/ngalert/api/api_convert_prometheus.go index bb5ad55cbf26d..76c2f7b45185c 100644 --- a/pkg/services/ngalert/api/api_convert_prometheus.go +++ b/pkg/services/ngalert/api/api_convert_prometheus.go @@ -67,6 +67,8 @@ const ( configForceReplaceHeader = "X-Grafana-Alerting-Config-Force-Replace" // dryRunHeader if specified, will validate the configuration without saving it dryRunHeader = "X-Grafana-Alerting-Dry-Run" + // promoteHeader if specified, will promote the merge imported configuration into Grafana and save. This is a one-off operation + promoteHeader = "X-Grafana-Alerting-Promote" // versionMessageHeader is the header that specifies an optional message for rule versions. versionMessageHeader = "X-Grafana-Alerting-Version-Message" @@ -142,7 +144,7 @@ type ConvertPrometheusSrv struct { type Alertmanager interface { DeleteExtraConfiguration(ctx context.Context, org int64, user identity.Requester, authz notifier.ExtraConfigAuthz, identifier string) error - SaveAndApplyExtraConfiguration(ctx context.Context, org int64, user identity.Requester, authz notifier.ExtraConfigAuthz, extraConfig v1.ExtraConfiguration, replace bool, dryRun bool) (merge.MergeResult, error) + SaveAndApplyExtraConfiguration(ctx context.Context, org int64, user identity.Requester, authz notifier.ExtraConfigAuthz, extraConfig v1.ExtraConfiguration, replace, dryRun, promote bool) (merge.MergeResult, error) GetAlertmanagerConfiguration(ctx context.Context, org int64, withAutogen bool) (apimodels.GettableUserConfig, error) IsExternalAMSyncConfiguredForOrg(ctx context.Context, orgID int64) (bool, error) } @@ -621,11 +623,18 @@ func (srv *ConvertPrometheusSrv) RouteConvertPrometheusPostAlertmanagerConfig(c return errorToResponse(err) } + promote, err := parseBooleanHeader(c.Req.Header.Get(promoteHeader), promoteHeader) + if err != nil { + logger.Error("Failed to parse promote header", "error", err) + return errorToResponse(err) + } + identifier, err := parseConfigIdentifierHeader(c) if err != nil { logger.Error("Failed to parse config identifier header", "error", err) return errorToResponse(err) } + logger = logger.New("identifier", identifier) ec := v1.ExtraConfiguration{ Identifier: identifier, @@ -634,7 +643,7 @@ func (srv *ConvertPrometheusSrv) RouteConvertPrometheusPostAlertmanagerConfig(c } err = ec.Validate() if err != nil { - logger.Error("Invalid alertmanager configuration", "error", err, "identifier", identifier) + logger.Error("Invalid alertmanager configuration", "error", err) return errorToResponse(err) } @@ -644,20 +653,24 @@ func (srv *ConvertPrometheusSrv) RouteConvertPrometheusPostAlertmanagerConfig(c return errorToResponse(err) } - result, err := srv.am.SaveAndApplyExtraConfiguration(c.Req.Context(), c.GetOrgID(), c.SignedInUser, srv.importsAuthz, ec, replace, dryRun) + result, err := srv.am.SaveAndApplyExtraConfiguration(c.Req.Context(), c.GetOrgID(), c.SignedInUser, srv.importsAuthz, ec, replace, dryRun, promote) if err != nil { - logger.Error("Failed to save alertmanager configuration", "error", err, "identifier", identifier) + logger.Error("Failed to save alertmanager configuration", "error", err) return errorToResponse(fmt.Errorf("failed to save alertmanager configuration: %w", err)) } apiResp := buildConvertResponse(result) + logCtx := append(result.LogContext(), "replace", replace) if dryRun { - logger.Info("Dry run: alertmanager configuration validated successfully", "identifier", identifier, "replace", replace) + logger.Debug("Dry run: alertmanager configuration validated successfully", logCtx...) return response.JSON(http.StatusOK, apiResp) } - - logger.Info("Successfully updated alertmanager configuration with imported Prometheus config", "identifier", identifier, "replace", replace) + if promote { + logger.Info("Successfully imported and promoted alertmanager configuration", logCtx...) + } else { + logger.Info("Successfully updated alertmanager configuration with imported Prometheus config", logCtx...) + } return response.JSON(http.StatusAccepted, apiResp) } diff --git a/pkg/services/ngalert/api/api_convert_prometheus_test.go b/pkg/services/ngalert/api/api_convert_prometheus_test.go index 9ab816224f056..4984103d4f445 100644 --- a/pkg/services/ngalert/api/api_convert_prometheus_test.go +++ b/pkg/services/ngalert/api/api_convert_prometheus_test.go @@ -1964,8 +1964,8 @@ type mockAlertmanager struct { mock.Mock } -func (m *mockAlertmanager) SaveAndApplyExtraConfiguration(ctx context.Context, org int64, user identity.Requester, authz notifier.ExtraConfigAuthz, extraConfig v1.ExtraConfiguration, replace bool, dryRun bool) (merge.MergeResult, error) { - args := m.Called(ctx, org, user, authz, extraConfig, replace, dryRun) +func (m *mockAlertmanager) SaveAndApplyExtraConfiguration(ctx context.Context, org int64, user identity.Requester, authz notifier.ExtraConfigAuthz, extraConfig v1.ExtraConfiguration, replace, dryRun, promote bool) (merge.MergeResult, error) { + args := m.Called(ctx, org, user, authz, extraConfig, replace, dryRun, promote) return args.Get(0).(merge.MergeResult), args.Error(1) } @@ -1997,7 +1997,7 @@ func TestRouteConvertPrometheusPostAlertmanagerConfig(t *testing.T) { return extraConfig.Identifier == identifier && len(extraConfig.TemplateFiles) == 1 && extraConfig.TemplateFiles["test.tmpl"] == "{{ define \"test\" }}Hello{{ end }}" - }), false, false).Return(merge.MergeResult{}, nil).Once() + }), false, false, false).Return(merge.MergeResult{}, nil).Once() rc := createRequestCtx() rc.Req.Header.Set(configIdentifierHeader, identifier) @@ -2030,7 +2030,7 @@ func TestRouteConvertPrometheusPostAlertmanagerConfig(t *testing.T) { mockAM.On("IsExternalAMSyncConfiguredForOrg", mock.Anything, int64(1)).Return(false, nil).Maybe() mockAM.On("SaveAndApplyExtraConfiguration", mock.Anything, int64(1), mock.Anything, mock.Anything, mock.MatchedBy(func(extraConfig v1.ExtraConfiguration) bool { return extraConfig.Identifier == defaultConfigIdentifier - }), false, false).Return(merge.MergeResult{}, nil) + }), false, false, false).Return(merge.MergeResult{}, nil) ft := featuremgmt.WithFeatures(featuremgmt.FlagAlertingMultiplePolicies, featuremgmt.FlagAlertingImportAlertmanagerAPI) srv, _, _ := createConvertPrometheusSrv(t, withAlertmanager(mockAM), withFeatureToggles(ft)) @@ -2060,7 +2060,7 @@ func TestRouteConvertPrometheusPostAlertmanagerConfig(t *testing.T) { mockAM.On("IsExternalAMSyncConfiguredForOrg", mock.Anything, int64(1)).Return(false, nil).Maybe() mockAM.On("SaveAndApplyExtraConfiguration", mock.Anything, int64(1), mock.Anything, mock.Anything, mock.MatchedBy(func(extraConfig v1.ExtraConfiguration) bool { return extraConfig.Identifier == defaultConfigIdentifier - }), true, false).Return(merge.MergeResult{}, nil) + }), true, false, false).Return(merge.MergeResult{}, nil) ft := featuremgmt.WithFeatures(featuremgmt.FlagAlertingMultiplePolicies, featuremgmt.FlagAlertingImportAlertmanagerAPI) srv, _, _ := createConvertPrometheusSrv(t, withAlertmanager(mockAM), withFeatureToggles(ft)) @@ -2090,7 +2090,7 @@ func TestRouteConvertPrometheusPostAlertmanagerConfig(t *testing.T) { mockAM.On("IsExternalAMSyncConfiguredForOrg", mock.Anything, int64(1)).Return(false, nil).Maybe() mockAM.On("SaveAndApplyExtraConfiguration", mock.Anything, int64(1), mock.Anything, mock.Anything, mock.MatchedBy(func(extraConfig v1.ExtraConfiguration) bool { return extraConfig.Identifier == defaultConfigIdentifier - }), false, true).Return(merge.MergeResult{}, nil) + }), false, true, false).Return(merge.MergeResult{}, nil) ft := featuremgmt.WithFeatures(featuremgmt.FlagAlertingMultiplePolicies, featuremgmt.FlagAlertingImportAlertmanagerAPI) srv, _, _ := createConvertPrometheusSrv(t, withAlertmanager(mockAM), withFeatureToggles(ft)) @@ -2137,7 +2137,7 @@ func TestRouteConvertPrometheusPostAlertmanagerConfig(t *testing.T) { mockAM.On("SaveAndApplyExtraConfiguration", mock.Anything, int6 ... [truncated]