From 257897b701ce308e0a9fd934476b0ee044be2c65 Mon Sep 17 00:00:00 2001 From: Friedrich Gonzalez Date: Thu, 15 Dec 2022 20:23:33 +0100 Subject: [PATCH 1/4] Validate OpsGenie alertmanager configuration Signed-off-by: Friedrich Gonzalez --- pkg/alertmanager/api.go | 15 +++++++++++++++ pkg/alertmanager/api_test.go | 14 ++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/pkg/alertmanager/api.go b/pkg/alertmanager/api.go index 3ed63a6e4d..28e8bb4dd1 100644 --- a/pkg/alertmanager/api.go +++ b/pkg/alertmanager/api.go @@ -45,6 +45,7 @@ var ( errTLSFileNotAllowed = errors.New("setting TLS ca_file, cert_file and key_file is not allowed") errSlackAPIURLFileNotAllowed = errors.New("setting Slack api_url_file and global slack_api_url_file is not allowed") errVictorOpsAPIKeyFileNotAllowed = errors.New("setting VictorOps api_key_file is not allowed") + errOpsGenieAPIKeyFileNotAllowed = errors.New("setting OpsGenie api_key_file is not allowed") ) // UserConfig is used to communicate a users alertmanager configs @@ -336,6 +337,11 @@ func validateAlertmanagerConfig(cfg interface{}) error { return err } + case reflect.TypeOf(config.OpsGenieConfig{}): + if err := validateOpsGenieConfig(v.Interface().(config.OpsGenieConfig)); err != nil { + return err + } + case reflect.TypeOf(commoncfg.TLSConfig{}): if err := validateReceiverTLSConfig(v.Interface().(commoncfg.TLSConfig)); err != nil { return err @@ -432,6 +438,15 @@ func validateGlobalConfig(cfg config.GlobalConfig) error { return nil } +// validateOpsGenieConfig validates the OpsGenie config and returns an error if it contains +// settings now allowed by Cortex. +func validateOpsGenieConfig(cfg config.OpsGenieConfig) error { + if cfg.APIKeyFile != "" { + return errOpsGenieAPIKeyFileNotAllowed + } + return nil +} + // validateSlackConfig validates the Slack config and returns an error if it contains // settings now allowed by Cortex. func validateSlackConfig(cfg config.SlackConfig) error { diff --git a/pkg/alertmanager/api_test.go b/pkg/alertmanager/api_test.go index 31367918fd..e25a2ead78 100644 --- a/pkg/alertmanager/api_test.go +++ b/pkg/alertmanager/api_test.go @@ -402,6 +402,20 @@ alertmanager_config: | `, err: errors.Wrap(errSlackAPIURLFileNotAllowed, "error validating Alertmanager config"), }, + { + name: "Should return error if OpsGenie api_key_file is set", + cfg: ` +alertmanager_config: | + receivers: + - name: default-receiver + opsgenie_configs: + - api_key_file: /secrets + + route: + receiver: 'default-receiver' +`, + err: errors.Wrap(errOpsGenieAPIKeyFileNotAllowed, "error validating Alertmanager config"), + }, { name: "Should return error if VictorOps api_key_file is set", cfg: ` From 439864c192e723436e200af018fb5b2edba380bb Mon Sep 17 00:00:00 2001 From: Friedrich Gonzalez Date: Thu, 15 Dec 2022 20:35:11 +0100 Subject: [PATCH 2/4] Update changelog Signed-off-by: Friedrich Gonzalez --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e413dc7ac0..9f459b91da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## master / unreleased +* [CHANGE] Alertmanager: Local file disclosure vulnerability in OpsGenie configuration has been fixed. #11 * [ENHANCEMENT] Update Go version to 1.19.3. #4988 * [ENHANCEMENT] Querier: limit series query to only ingesters if `start` param is not specified. #4976 * [ENHANCEMENT] Query-frontend/scheduler: add a new limit `frontend.max-outstanding-requests-per-tenant` for configuring queue size per tenant. Started deprecating two flags `-query-scheduler.max-outstanding-requests-per-tenant` and `-querier.max-outstanding-requests-per-tenant`, and change their value default to 0. Now if both the old flag and new flag are specified, the old flag's queue size will be picked. #5005 From 55fbcbd55415ca2e3e185658582debfb936f8b4c Mon Sep 17 00:00:00 2001 From: Friedrich Gonzalez Date: Fri, 16 Dec 2022 13:23:21 +0100 Subject: [PATCH 3/4] Validate global config too Signed-off-by: Friedrich Gonzalez --- pkg/alertmanager/api.go | 3 +++ pkg/alertmanager/api_test.go | 17 +++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/pkg/alertmanager/api.go b/pkg/alertmanager/api.go index 28e8bb4dd1..2389e8f2aa 100644 --- a/pkg/alertmanager/api.go +++ b/pkg/alertmanager/api.go @@ -432,6 +432,9 @@ func validateReceiverTLSConfig(cfg commoncfg.TLSConfig) error { // validateGlobalConfig validates the Global config and returns an error if it contains // settings now allowed by Cortex. func validateGlobalConfig(cfg config.GlobalConfig) error { + if cfg.OpsGenieAPIKeyFile != "" { + return errOpsGenieAPIKeyFileNotAllowed + } if cfg.SlackAPIURLFile != "" { return errSlackAPIURLFileNotAllowed } diff --git a/pkg/alertmanager/api_test.go b/pkg/alertmanager/api_test.go index e25a2ead78..981f81b310 100644 --- a/pkg/alertmanager/api_test.go +++ b/pkg/alertmanager/api_test.go @@ -371,6 +371,23 @@ alertmanager_config: | `, err: errors.Wrap(errOAuth2SecretFileNotAllowed, "error validating Alertmanager config"), }, + { + name: "Should return error if global opsgenie_api_key_file is set", + cfg: ` +alertmanager_config: | + global: + opsgenie_api_key_file: /secrets + + receivers: + - name: default-receiver + webhook_configs: + - url: http://localhost + + route: + receiver: 'default-receiver' +`, + err: errors.Wrap(errOpsGenieAPIKeyFileNotAllowed, "error validating Alertmanager config"), + }, { name: "Should return error if global slack_api_url_file is set", cfg: ` From 8f053825fc4e783c1829b381951a51c6c219dd0d Mon Sep 17 00:00:00 2001 From: Friedrich Gonzalez Date: Fri, 16 Dec 2022 13:39:29 +0100 Subject: [PATCH 4/4] fix pr number in changelog Signed-off-by: Friedrich Gonzalez --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f459b91da..4e50a507f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ # Changelog ## master / unreleased -* [CHANGE] Alertmanager: Local file disclosure vulnerability in OpsGenie configuration has been fixed. #11 +* [CHANGE] Alertmanager: Local file disclosure vulnerability in OpsGenie configuration has been fixed. #5045 * [ENHANCEMENT] Update Go version to 1.19.3. #4988 * [ENHANCEMENT] Querier: limit series query to only ingesters if `start` param is not specified. #4976 * [ENHANCEMENT] Query-frontend/scheduler: add a new limit `frontend.max-outstanding-requests-per-tenant` for configuring queue size per tenant. Started deprecating two flags `-query-scheduler.max-outstanding-requests-per-tenant` and `-querier.max-outstanding-requests-per-tenant`, and change their value default to 0. Now if both the old flag and new flag are specified, the old flag's queue size will be picked. #5005