diff --git a/pkg/services/ngalert/api/api_alertmanager_test.go b/pkg/services/ngalert/api/api_alertmanager_test.go index c6e8deadb8a..5828dd98c8f 100644 --- a/pkg/services/ngalert/api/api_alertmanager_test.go +++ b/pkg/services/ngalert/api/api_alertmanager_test.go @@ -3,6 +3,7 @@ package api import ( "context" "crypto/md5" + "encoding/base64" "encoding/json" "math/rand" "net/http" @@ -16,6 +17,7 @@ import ( amv2 "github.com/prometheus/alertmanager/api/v2/models" "github.com/prometheus/alertmanager/pkg/labels" "github.com/prometheus/client_golang/prometheus" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/api/response" @@ -329,6 +331,118 @@ func TestAlertmanagerConfig(t *testing.T) { }) } +func TestGetAlertmanagerConfiguration_NewSecretField(t *testing.T) { + // This test has the following goals: + // Given: + // - A saved notifier config with an existing secret field stored unencrypted in Settings. + // Ensure: + // - The secret field is not returned in plaintext. + // - The secret field is returned as a bool in SecureFields. + // - The secret field is correctly saved and encrypted in SecureSettings when saving the notifier config without changes. + // - The secret field is removed from Settings when saving the notifier config. + + sut := createSut(t) + orgId := int64(1) + + // This config has the secret field "integrationKey" stored incorrectly and unencrypted in Settings. + configs := map[int64]*ngmodels.AlertConfiguration{ + 1: { + OrgID: orgId, + AlertmanagerConfiguration: `{ + "alertmanager_config": { + "route": { + "receiver": "configWithNewlySecretSetting" + }, + "receivers": [{ + "name": "configWithNewlySecretSetting", + "grafana_managed_receiver_configs": [{ + "uid": "configWithNewlySecretSetting-uid", + "name": "configWithNewlySecretSetting", + "type": "pagerduty", + "settings": {"integrationKey": "unencrypted secure secret"}, + "secureSettings": {} + }] + }] + } +} +`, + CreatedAt: time.Now().Unix(), + Default: false, + }, + } + + // Store the config as-is in the database. Bypasses normal save route so it doesn't get pre-emptively fixed. + mam := createMultiOrgAlertmanager(t, configs) + sut.mam = mam + + rc := createRequestCtxInOrg(orgId) + res := sut.RouteGetAlertingConfig(rc) + gettable := asGettableUserConfig(t, res) + + integration := gettable.GetGrafanaReceiverMap()["configWithNewlySecretSetting-uid"] + require.NotNil(t, integration) + + var settings map[string]string + err := json.Unmarshal(integration.Settings, &settings) + require.NoError(t, err) + + // The secret field "integrationKey" should not be returned in plaintext. + assert.NotEqual(t, "unencrypted secure secret", settings["integrationKey"]) + // Just in case let's look for the unencrypted value anywhere in the settings. + assert.NotContains(t, string(integration.Settings), "unencrypted") + + // The secret fields should be returned as a bool in SecureFields. + assert.True(t, integration.SecureFields["integrationKey"]) + + // Now we save the config without changes. This should encrypt the field "integrationKey" into SecureSettings and + // remove it from Settings. + + // Simulates FE-API interaction, "integrationKey" is not sent in Settings as the caller. + // Instead, it leaves it out of "SecureSettings" to indicate the API should keep the existing value. + var postWithoutChanges = `{ + "alertmanager_config": { + "route": { + "receiver": "configWithNewlySecretSetting" + }, + "receivers": [{ + "name": "configWithNewlySecretSetting", + "grafana_managed_receiver_configs": [{ + "uid": "configWithNewlySecretSetting-uid", + "name": "configWithNewlySecretSetting", + "type": "pagerduty", + "settings": {}, + "secureSettings": {} + }] + }] + } +} +` + postable := createAmConfigRequest(t, postWithoutChanges) + + res = sut.RoutePostAlertingConfig(rc, postable) + require.Equal(t, 202, res.Status()) + + // Check that the secret field "integrationKey" is now encrypted in SecureSettings. + savedConfig := &apimodels.PostableUserConfig{} + err = json.Unmarshal([]byte(configs[orgId].AlertmanagerConfiguration), savedConfig) + require.NoError(t, err) + + savedIntegration := savedConfig.GetGrafanaReceiverMap()["configWithNewlySecretSetting-uid"] + require.NotNil(t, savedIntegration) + + // No longer in Settings. + assert.Equal(t, "{}", string(savedIntegration.Settings)) + + // Encrypted in SecureSettings. + secureSecret := savedIntegration.SecureSettings["integrationKey"] + assert.NotEmpty(t, secureSecret) + encryptedSecret, err := base64.StdEncoding.DecodeString(secureSecret) + require.NoError(t, err) + + // No access to .Decrypt, but we can check that it's not the same as the unencrypted value. + assert.NotEqual(t, "unencrypted secure secret", string(encryptedSecret)) +} + func TestAlertmanagerAutogenConfig(t *testing.T) { createSutForAutogen := func(t *testing.T) (AlertmanagerSrv, map[int64]*ngmodels.AlertConfiguration) { sut := createSut(t) diff --git a/pkg/services/ngalert/api/api_provisioning_test.go b/pkg/services/ngalert/api/api_provisioning_test.go index df1d266747e..ab906c9696d 100644 --- a/pkg/services/ngalert/api/api_provisioning_test.go +++ b/pkg/services/ngalert/api/api_provisioning_test.go @@ -1429,7 +1429,7 @@ func TestProvisioningApiContactPointExport(t *testing.T) { }) t.Run("json body content is as expected", func(t *testing.T) { - expectedRedactedResponse := `{"apiVersion":1,"contactPoints":[{"orgId":1,"name":"grafana-default-email","receivers":[{"uid":"ad95bd8a-49ed-4adc-bf89-1b444fa1aa5b","type":"email","settings":{"addresses":"\u003cexample@email.com\u003e"},"disableResolveMessage":false}]},{"orgId":1,"name":"multiple integrations","receivers":[{"uid":"c2090fda-f824-4add-b545-5a4d5c2ef082","type":"prometheus-alertmanager","settings":{"basicAuthPassword":"[REDACTED]","basicAuthUser":"test","url":"http://localhost:9093"},"disableResolveMessage":true},{"uid":"c84539ec-f87e-4fc5-9a91-7a687d34bbd1","type":"discord","settings":{"avatar_url":"some avatar","url":"some url","use_discord_username":true},"disableResolveMessage":false}]},{"orgId":1,"name":"pagerduty test","receivers":[{"uid":"b9bf06f8-bde2-4438-9d4a-bba0522dcd4d","type":"pagerduty","settings":{"client":"some client","integrationKey":"[REDACTED]","severity":"criticalish"},"disableResolveMessage":false}]},{"orgId":1,"name":"slack test","receivers":[{"uid":"cbfd0976-8228-4126-b672-4419f30a9e50","type":"slack","settings":{"text":"title body test","title":"title test","url":"[REDACTED]"},"disableResolveMessage":true}]}]}` + expectedRedactedResponse := `{"apiVersion":1,"contactPoints":[{"orgId":1,"name":"grafana-default-email","receivers":[{"uid":"ad95bd8a-49ed-4adc-bf89-1b444fa1aa5b","type":"email","settings":{"addresses":"\u003cexample@email.com\u003e"},"disableResolveMessage":false}]},{"orgId":1,"name":"multiple integrations","receivers":[{"uid":"c2090fda-f824-4add-b545-5a4d5c2ef082","type":"prometheus-alertmanager","settings":{"basicAuthPassword":"[REDACTED]","basicAuthUser":"test","url":"http://localhost:9093"},"disableResolveMessage":true},{"uid":"c84539ec-f87e-4fc5-9a91-7a687d34bbd1","type":"discord","settings":{"avatar_url":"some avatar","url":"[REDACTED]","use_discord_username":true},"disableResolveMessage":false}]},{"orgId":1,"name":"pagerduty test","receivers":[{"uid":"b9bf06f8-bde2-4438-9d4a-bba0522dcd4d","type":"pagerduty","settings":{"client":"some client","integrationKey":"[REDACTED]","severity":"criticalish"},"disableResolveMessage":false}]},{"orgId":1,"name":"slack test","receivers":[{"uid":"cbfd0976-8228-4126-b672-4419f30a9e50","type":"slack","settings":{"text":"title body test","title":"title test","url":"[REDACTED]"},"disableResolveMessage":true}]}]}` t.Run("decrypt false", func(t *testing.T) { env := createTestEnv(t, testContactPointConfig) sut := createProvisioningSrvSutFromEnv(t, &env) @@ -1482,14 +1482,14 @@ func TestProvisioningApiContactPointExport(t *testing.T) { response := sut.RouteGetContactPointsExport(&rc) - expectedResponse := `{"apiVersion":1,"contactPoints":[{"orgId":1,"name":"multiple integrations","receivers":[{"uid":"c2090fda-f824-4add-b545-5a4d5c2ef082","type":"prometheus-alertmanager","settings":{"basicAuthPassword":"[REDACTED]","basicAuthUser":"test","url":"http://localhost:9093"},"disableResolveMessage":true},{"uid":"c84539ec-f87e-4fc5-9a91-7a687d34bbd1","type":"discord","settings":{"avatar_url":"some avatar","url":"some url","use_discord_username":true},"disableResolveMessage":false}]}]}` + expectedResponse := `{"apiVersion":1,"contactPoints":[{"orgId":1,"name":"multiple integrations","receivers":[{"uid":"c2090fda-f824-4add-b545-5a4d5c2ef082","type":"prometheus-alertmanager","settings":{"basicAuthPassword":"[REDACTED]","basicAuthUser":"test","url":"http://localhost:9093"},"disableResolveMessage":true},{"uid":"c84539ec-f87e-4fc5-9a91-7a687d34bbd1","type":"discord","settings":{"avatar_url":"some avatar","url":"[REDACTED]","use_discord_username":true},"disableResolveMessage":false}]}]}` require.Equal(t, 200, response.Status()) require.Equal(t, expectedResponse, string(response.Body())) }) }) t.Run("yaml body content is as expected", func(t *testing.T) { - expectedRedactedResponse := "apiVersion: 1\ncontactPoints:\n - orgId: 1\n name: grafana-default-email\n receivers:\n - uid: ad95bd8a-49ed-4adc-bf89-1b444fa1aa5b\n type: email\n settings:\n addresses: \n disableResolveMessage: false\n - orgId: 1\n name: multiple integrations\n receivers:\n - uid: c2090fda-f824-4add-b545-5a4d5c2ef082\n type: prometheus-alertmanager\n settings:\n basicAuthPassword: '[REDACTED]'\n basicAuthUser: test\n url: http://localhost:9093\n disableResolveMessage: true\n - uid: c84539ec-f87e-4fc5-9a91-7a687d34bbd1\n type: discord\n settings:\n avatar_url: some avatar\n url: some url\n use_discord_username: true\n disableResolveMessage: false\n - orgId: 1\n name: pagerduty test\n receivers:\n - uid: b9bf06f8-bde2-4438-9d4a-bba0522dcd4d\n type: pagerduty\n settings:\n client: some client\n integrationKey: '[REDACTED]'\n severity: criticalish\n disableResolveMessage: false\n - orgId: 1\n name: slack test\n receivers:\n - uid: cbfd0976-8228-4126-b672-4419f30a9e50\n type: slack\n settings:\n text: title body test\n title: title test\n url: '[REDACTED]'\n disableResolveMessage: true\n" + expectedRedactedResponse := "apiVersion: 1\ncontactPoints:\n - orgId: 1\n name: grafana-default-email\n receivers:\n - uid: ad95bd8a-49ed-4adc-bf89-1b444fa1aa5b\n type: email\n settings:\n addresses: \n disableResolveMessage: false\n - orgId: 1\n name: multiple integrations\n receivers:\n - uid: c2090fda-f824-4add-b545-5a4d5c2ef082\n type: prometheus-alertmanager\n settings:\n basicAuthPassword: '[REDACTED]'\n basicAuthUser: test\n url: http://localhost:9093\n disableResolveMessage: true\n - uid: c84539ec-f87e-4fc5-9a91-7a687d34bbd1\n type: discord\n settings:\n avatar_url: some avatar\n url: '[REDACTED]'\n use_discord_username: true\n disableResolveMessage: false\n - orgId: 1\n name: pagerduty test\n receivers:\n - uid: b9bf06f8-bde2-4438-9d4a-bba0522dcd4d\n type: pagerduty\n settings:\n client: some client\n integrationKey: '[REDACTED]'\n severity: criticalish\n disableResolveMessage: false\n - orgId: 1\n name: slack test\n receivers:\n - uid: cbfd0976-8228-4126-b672-4419f30a9e50\n type: slack\n settings:\n text: title body test\n title: title test\n url: '[REDACTED]'\n disableResolveMessage: true\n" t.Run("decrypt false", func(t *testing.T) { env := createTestEnv(t, testContactPointConfig) sut := createProvisioningSrvSutFromEnv(t, &env) @@ -1542,7 +1542,7 @@ func TestProvisioningApiContactPointExport(t *testing.T) { response := sut.RouteGetContactPointsExport(&rc) - expectedResponse := "apiVersion: 1\ncontactPoints:\n - orgId: 1\n name: multiple integrations\n receivers:\n - uid: c2090fda-f824-4add-b545-5a4d5c2ef082\n type: prometheus-alertmanager\n settings:\n basicAuthPassword: '[REDACTED]'\n basicAuthUser: test\n url: http://localhost:9093\n disableResolveMessage: true\n - uid: c84539ec-f87e-4fc5-9a91-7a687d34bbd1\n type: discord\n settings:\n avatar_url: some avatar\n url: some url\n use_discord_username: true\n disableResolveMessage: false\n" + expectedResponse := "apiVersion: 1\ncontactPoints:\n - orgId: 1\n name: multiple integrations\n receivers:\n - uid: c2090fda-f824-4add-b545-5a4d5c2ef082\n type: prometheus-alertmanager\n settings:\n basicAuthPassword: '[REDACTED]'\n basicAuthUser: test\n url: http://localhost:9093\n disableResolveMessage: true\n - uid: c84539ec-f87e-4fc5-9a91-7a687d34bbd1\n type: discord\n settings:\n avatar_url: some avatar\n url: '[REDACTED]'\n use_discord_username: true\n disableResolveMessage: false\n" require.Equal(t, 200, response.Status()) require.Equal(t, expectedResponse, string(response.Body())) }) @@ -2009,10 +2009,11 @@ var testContactPointConfig = ` "disableResolveMessage":false, "settings":{ "avatar_url":"some avatar", - "url":"some url", "use_discord_username":true }, - "secureSettings":{} + "secureSettings":{ + "url":"some url" + } } ] }, diff --git a/pkg/services/ngalert/notifier/alertmanager_config.go b/pkg/services/ngalert/notifier/alertmanager_config.go index 32ae92baea2..3e00d709ba2 100644 --- a/pkg/services/ngalert/notifier/alertmanager_config.go +++ b/pkg/services/ngalert/notifier/alertmanager_config.go @@ -12,6 +12,7 @@ import ( "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/store" + "github.com/grafana/grafana/pkg/services/secrets" "github.com/grafana/grafana/pkg/util" ) @@ -151,6 +152,16 @@ func (moa *MultiOrgAlertmanager) gettableUserConfigFromAMConfigString(ctx contex Config: cfg.AlertmanagerConfig.Config, }, } + + // First we encrypt the secure settings. + // This is done to ensure that any secure settings incorrectly stored in Settings are encrypted and moved to + // SecureSettings. This can happen if an integration definition is updated to make a field secure. + if err := EncryptReceiverConfigSettings(cfg.AlertmanagerConfig.Receivers, func(ctx context.Context, payload []byte) ([]byte, error) { + return moa.Crypto.Encrypt(ctx, payload, secrets.WithoutScope()) + }); err != nil { + return definitions.GettableUserConfig{}, fmt.Errorf("failed to encrypt receivers: %w", err) + } + for _, recv := range cfg.AlertmanagerConfig.Receivers { receivers := make([]*definitions.GettableGrafanaReceiver, 0, len(recv.PostableGrafanaReceivers.GrafanaManagedReceivers)) for _, pr := range recv.PostableGrafanaReceivers.GrafanaManagedReceivers { diff --git a/pkg/services/ngalert/notifier/crypto.go b/pkg/services/ngalert/notifier/crypto.go index f710181f45c..221e9094943 100644 --- a/pkg/services/ngalert/notifier/crypto.go +++ b/pkg/services/ngalert/notifier/crypto.go @@ -3,11 +3,13 @@ package notifier import ( "context" "encoding/base64" + "encoding/json" "errors" "fmt" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" + "github.com/grafana/grafana/pkg/services/ngalert/notifier/channels_config" "github.com/grafana/grafana/pkg/services/ngalert/store" "github.com/grafana/grafana/pkg/services/secrets" ) @@ -57,17 +59,90 @@ func (c *alertmanagerCrypto) ProcessSecureSettings(ctx context.Context, orgId in // EncryptReceiverConfigs encrypts all SecureSettings in the given receivers. func EncryptReceiverConfigs(c []*definitions.PostableApiReceiver, encrypt definitions.EncryptFn) error { + return encryptReceiverConfigs(c, encrypt, true) +} + +func EncryptReceiverConfigSettings(c []*definitions.PostableApiReceiver, encrypt definitions.EncryptFn) error { + return encryptReceiverConfigs(c, encrypt, false) +} + +// encryptReceiverConfigs encrypts all SecureSettings in the given receivers. +// encryptExisting determines whether to encrypt existing secure settings. +func encryptReceiverConfigs(c []*definitions.PostableApiReceiver, encrypt definitions.EncryptFn, encryptExisting bool) error { // encrypt secure settings for storing them in DB for _, r := range c { switch r.Type() { case definitions.GrafanaReceiverType: for _, gr := range r.PostableGrafanaReceivers.GrafanaManagedReceivers { - for k, v := range gr.SecureSettings { - encryptedData, err := encrypt(context.Background(), []byte(v)) + if encryptExisting { + for k, v := range gr.SecureSettings { + encryptedData, err := encrypt(context.Background(), []byte(v)) + if err != nil { + return fmt.Errorf("failed to encrypt secure settings: %w", err) + } + gr.SecureSettings[k] = base64.StdEncoding.EncodeToString(encryptedData) + } + } + + if len(gr.Settings) > 0 { + // We need to parse the settings to check for secret keys. If we find any, we encrypt them and + // store them in SecureSettings. This can happen from incorrect configuration or when an integration + // definition is updated to make a field secure. + settings := make(map[string]any) + if err := json.Unmarshal(gr.Settings, &settings); err != nil { + return fmt.Errorf("integration '%s' of receiver '%s' has settings that cannot be parsed as JSON: %w", gr.Type, gr.Name, err) + } + + secretKeys, err := channels_config.GetSecretKeysForContactPointType(gr.Type) if err != nil { - return fmt.Errorf("failed to encrypt secure settings: %w", err) + return fmt.Errorf("failed to get secret keys for contact point type %s: %w", gr.Type, err) + } + + secureSettings := gr.SecureSettings + if secureSettings == nil { + secureSettings = make(map[string]string) + } + + settingsChanged := false + secureSettingsChanged := false + for _, secretKey := range secretKeys { + settingsValue, ok := settings[secretKey] + if !ok { + continue + } + + // Secrets should not be stored in settings regardless. + delete(settings, secretKey) + settingsChanged = true + + // If the secret is already encrypted, we don't need to encrypt it again. + if _, ok := secureSettings[secretKey]; ok { + continue + } + + if strVal, isString := settingsValue.(string); isString { + encrypted, err := encrypt(context.Background(), []byte(strVal)) + if err != nil { + return fmt.Errorf("failed to encrypt secure settings: %w", err) + } + secureSettings[secretKey] = base64.StdEncoding.EncodeToString(encrypted) + secureSettingsChanged = true + } + } + + // Defensive checks to limit the risk of unintentional edge case changes in this legacy API. + if settingsChanged { + // If we removed any secret keys from settings, we need to save the updated settings. + jsonBytes, err := json.Marshal(settings) + if err != nil { + return err + } + gr.Settings = jsonBytes + } + if secureSettingsChanged { + // If we added any secure settings, we need to save the updated secure settings. + gr.SecureSettings = secureSettings } - gr.SecureSettings[k] = base64.StdEncoding.EncodeToString(encryptedData) } } default: @@ -94,6 +169,14 @@ func (c *alertmanagerCrypto) LoadSecureSettings(ctx context.Context, orgId int64 if err != nil { c.log.Warn("Last known alertmanager configuration was invalid. Overwriting...") } else { + // First we encrypt the secure settings in the existing configuration. + // This is done to ensure that any secure settings incorrectly stored in Settings are encrypted and moved to + // SecureSettings. This can happen if an integration definition is updated to make a field secure. + if err := EncryptReceiverConfigSettings(currentConfig.AlertmanagerConfig.Receivers, func(ctx context.Context, payload []byte) ([]byte, error) { + return c.Encrypt(ctx, payload, secrets.WithoutScope()) + }); err != nil { + return fmt.Errorf("failed to encrypt receivers: %w", err) + } currentReceiverMap = currentConfig.GetGrafanaReceiverMap() } } diff --git a/pkg/tests/api/alerting/api_notification_channel_test.go b/pkg/tests/api/alerting/api_notification_channel_test.go index 6e568039176..a2ded10008b 100644 --- a/pkg/tests/api/alerting/api_notification_channel_test.go +++ b/pkg/tests/api/alerting/api_notification_channel_test.go @@ -2168,10 +2168,8 @@ var expAlertmanagerConfigFromAPI = ` "name": "discord_test", "type": "discord", "disableResolveMessage": false, - "settings": { - "url": "http://CHANNEL_ADDR/discord_recv/discord_test" - }, - "secureFields": {} + "settings": {}, + "secureFields": {"url": true} } ] }, @@ -2214,10 +2212,8 @@ var expAlertmanagerConfigFromAPI = ` "name": "victorops_test", "type": "victorops", "disableResolveMessage": false, - "settings": { - "url": "http://CHANNEL_ADDR/victorops_recv/victorops_test" - }, - "secureFields": {} + "settings": {}, + "secureFields": {"url": true} } ] },