Skip to content

Commit

Permalink
Fix for CVE-2021-31232
Browse files Browse the repository at this point in the history
* Improve alertmanager template checks.
* Added HTTP and TLS validation
* Use reflection to scan alertmanager config for validation
* Fix validation of template files.

Signed-off-by: Marco Pracucci <[email protected]>
Signed-off-by: Peter Štibraný <[email protected]>
  • Loading branch information
pstibrany authored Apr 17, 2021
1 parent 3a3015e commit ef75b8b
Show file tree
Hide file tree
Showing 5 changed files with 563 additions and 47 deletions.
9 changes: 6 additions & 3 deletions pkg/alertmanager/alertmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,10 +209,13 @@ func clusterWait(p *cluster.Peer, timeout time.Duration) func() time.Duration {
// ApplyConfig applies a new configuration to an Alertmanager.
func (am *Alertmanager) ApplyConfig(userID string, conf *config.Config, rawCfg string) error {
templateFiles := make([]string, len(conf.Templates))
if len(conf.Templates) > 0 {
for i, t := range conf.Templates {
templateFiles[i] = filepath.Join(am.cfg.DataDir, "templates", userID, t)
for i, t := range conf.Templates {
templateFilepath, err := safeTemplateFilepath(filepath.Join(am.cfg.DataDir, "templates", userID), t)
if err != nil {
return err
}

templateFiles[i] = templateFilepath
}

tmpl, err := template.FromGlobs(templateFiles...)
Expand Down
138 changes: 132 additions & 6 deletions pkg/alertmanager/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,18 @@ import (
"net/http"
"os"
"path/filepath"
"reflect"

"github.com/cortexproject/cortex/pkg/alertmanager/alerts"
"github.com/cortexproject/cortex/pkg/tenant"
"github.com/cortexproject/cortex/pkg/util"

"github.com/go-kit/kit/log"
"github.com/go-kit/kit/log/level"
"github.com/pkg/errors"
"github.com/prometheus/alertmanager/config"
"github.com/prometheus/alertmanager/template"
commoncfg "github.com/prometheus/common/config"
"gopkg.in/yaml.v2"
)

Expand All @@ -27,6 +30,11 @@ const (
errNoOrgID = "unable to determine the OrgID"
)

var (
errPasswordFileNotAllowed = errors.New("setting password_file, bearer_token_file and credentials_file is not allowed")
errTLSFileNotAllowed = errors.New("setting TLS ca_file, cert_file and key_file is not allowed")
)

// UserConfig is used to communicate a users alertmanager configs
type UserConfig struct {
TemplateFiles map[string]string `yaml:"template_files"`
Expand Down Expand Up @@ -146,28 +154,52 @@ func validateUserConfig(logger log.Logger, cfg alerts.AlertConfigDesc) error {
return err
}

// Validate the config recursively scanning it.
if err := validateAlertmanagerConfig(amCfg); err != nil {
return err
}

// Validate templates referenced in the alertmanager config.
for _, name := range amCfg.Templates {
if err := validateTemplateFilename(name); err != nil {
return err
}
}

// Validate template files.
for _, tmpl := range cfg.Templates {
if err := validateTemplateFilename(tmpl.Filename); err != nil {
return err
}
}

// Create templates on disk in a temporary directory.
// Note: This means the validation will succeed if we can write to tmp but
// not to configured data dir, and on the flipside, it'll fail if we can't write
// to tmpDir. Ignoring both cases for now as they're ultra rare but will revisit if
// we see this in the wild.
tmpDir, err := ioutil.TempDir("", "validate-config")
userTmpDir, err := ioutil.TempDir("", "validate-config-"+cfg.User)
if err != nil {
return err
}
defer os.RemoveAll(tmpDir)
defer os.RemoveAll(userTmpDir)

for _, tmpl := range cfg.Templates {
_, err := createTemplateFile(tmpDir, cfg.User, tmpl.Filename, tmpl.Body)
templateFilepath, err := safeTemplateFilepath(userTmpDir, tmpl.Filename)
if err != nil {
level.Error(logger).Log("msg", "unable to create template file", "err", err, "user", cfg.User)
return fmt.Errorf("unable to create template file '%s'", tmpl.Filename)
level.Error(logger).Log("msg", "unable to create template file path", "err", err, "user", cfg.User)
return err
}

if _, err = storeTemplateFile(templateFilepath, tmpl.Body); err != nil {
level.Error(logger).Log("msg", "unable to store template file", "err", err, "user", cfg.User)
return fmt.Errorf("unable to store template file '%s'", tmpl.Filename)
}
}

templateFiles := make([]string, len(amCfg.Templates))
for i, t := range amCfg.Templates {
templateFiles[i] = filepath.Join(tmpDir, "templates", cfg.User, t)
templateFiles[i] = filepath.Join(userTmpDir, t)
}

_, err = template.FromGlobs(templateFiles...)
Expand All @@ -182,3 +214,97 @@ func validateUserConfig(logger log.Logger, cfg alerts.AlertConfigDesc) error {

return nil
}

// validateAlertmanagerConfig recursively scans the input config looking for data types for which
// we have a specific validation and, whenever encountered, it runs their validation. Returns the
// first error or nil if validation succeeds.
func validateAlertmanagerConfig(cfg interface{}) error {
v := reflect.ValueOf(cfg)
t := v.Type()

// Skip invalid, the zero value or a nil pointer (checked by zero value).
if !v.IsValid() || v.IsZero() {
return nil
}

// If the input config is a pointer then we need to get its value.
// At this point the pointer value can't be nil.
if v.Kind() == reflect.Ptr {
v = v.Elem()
t = v.Type()
}

// Check if the input config is a data type for which we have a specific validation.
// At this point the value can't be a pointer anymore.
switch t {
case reflect.TypeOf(commoncfg.HTTPClientConfig{}):
return validateReceiverHTTPConfig(v.Interface().(commoncfg.HTTPClientConfig))

case reflect.TypeOf(commoncfg.TLSConfig{}):
return validateReceiverTLSConfig(v.Interface().(commoncfg.TLSConfig))
}

// If the input config is a struct, recursively iterate on all fields.
if t.Kind() == reflect.Struct {
for i := 0; i < t.NumField(); i++ {
field := t.Field(i)
fieldValue := v.FieldByIndex(field.Index)

// Skip any field value which can't be converted to interface (eg. primitive types).
if fieldValue.CanInterface() {
if err := validateAlertmanagerConfig(fieldValue.Interface()); err != nil {
return err
}
}
}
}

if t.Kind() == reflect.Slice || t.Kind() == reflect.Array {
for i := 0; i < v.Len(); i++ {
fieldValue := v.Index(i)

// Skip any field value which can't be converted to interface (eg. primitive types).
if fieldValue.CanInterface() {
if err := validateAlertmanagerConfig(fieldValue.Interface()); err != nil {
return err
}
}
}
}

if t.Kind() == reflect.Map {
for _, key := range v.MapKeys() {
fieldValue := v.MapIndex(key)

// Skip any field value which can't be converted to interface (eg. primitive types).
if fieldValue.CanInterface() {
if err := validateAlertmanagerConfig(fieldValue.Interface()); err != nil {
return err
}
}
}
}

return nil
}

// validateReceiverHTTPConfig validates the HTTP config and returns an error if it contains
// settings not allowed by Cortex.
func validateReceiverHTTPConfig(cfg commoncfg.HTTPClientConfig) error {
if cfg.BasicAuth != nil && cfg.BasicAuth.PasswordFile != "" {
return errPasswordFileNotAllowed
}
if cfg.BearerTokenFile != "" {
return errPasswordFileNotAllowed
}
return validateReceiverTLSConfig(cfg.TLSConfig)
}

// validateReceiverTLSConfig validates the TLS config and returns an error if it contains
// settings not allowed by Cortex.
func validateReceiverTLSConfig(cfg commoncfg.TLSConfig) error {
if cfg.CAFile != "" || cfg.CertFile != "" || cfg.KeyFile != "" {
return errTLSFileNotAllowed
}
return nil
}
Loading

0 comments on commit ef75b8b

Please sign in to comment.