Skip to content

Commit

Permalink
Fix opsgenie validation (#5045)
Browse files Browse the repository at this point in the history
* Validate OpsGenie alertmanager configuration

Signed-off-by: Friedrich Gonzalez <friedrichg@gmail.com>

* Update changelog

Signed-off-by: Friedrich Gonzalez <friedrichg@gmail.com>

* Validate global config too
Signed-off-by: Friedrich Gonzalez <friedrichg@gmail.com>

* fix pr number in changelog

Signed-off-by: Friedrich Gonzalez <friedrichg@gmail.com>

Signed-off-by: Friedrich Gonzalez <friedrichg@gmail.com>
  • Loading branch information
friedrichg authored Dec 16, 2022
1 parent e6a3da9 commit 03e023d
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Changelog

## master / unreleased
* [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
Expand Down
18 changes: 18 additions & 0 deletions pkg/alertmanager/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -426,12 +432,24 @@ 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
}
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 {
Expand Down
31 changes: 31 additions & 0 deletions pkg/alertmanager/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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: `
Expand Down Expand Up @@ -402,6 +419,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: `
Expand Down

0 comments on commit 03e023d

Please sign in to comment.