diff --git a/app/vmalert/README.md b/app/vmalert/README.md index 342ae7a78..bd667459b 100644 --- a/app/vmalert/README.md +++ b/app/vmalert/README.md @@ -813,7 +813,8 @@ and check the `Last updates` section: Rows in the section represent ordered rule evaluations and their results. The column `curl` contains an example of HTTP request sent by vmalert to the `-datasource.url` during evaluation. If specific state shows that there were no samples returned and curl command returns data - then it is very likely there was no data in datasource on the -moment when rule was evaluated. +moment when rule was evaluated. Sensitive info is stripped from the `curl` examples - see [security](#security) section +for more details. ### Debug mode @@ -829,6 +830,8 @@ Just set `debug: true` in rule's configuration and vmalert will start printing a 2022-09-15T13:36:56.153Z DEBUG rule "TestGroup":"Conns" (2601299393013563564) at 2022-09-15T15:36:56+02:00: alert 10705778000901301787 {alertgroup="TestGroup",alertname="Conns",cluster="east-1",instance="localhost:8429",replica="a"} PENDING => FIRING: 1m0s since becoming active at 2022-09-15 15:35:56.126006 +0200 CEST m=+39.384575417 ``` +Sensitive info is stripped from the `curl` examples - see [security](#security) section for more details. + ### Never-firing alerts vmalert can detect if alert's expression doesn't match any time series in runtime @@ -873,6 +876,20 @@ The same issue can be caused by collision of configured `labels` on [Group](#gro To fix it one should avoid collisions by carefully picking label overrides in configuration. +## Security + +See general recommendations regarding security [here](https://docs.victoriametrics.com/Single-server-VictoriaMetrics.html#security). + +vmalert [web UI](#web) exposes configuration details such as list of [Groups](#groups), active alerts, +[alerts state](#alerts-state), [notifiers](#notifier-configuration-file). Notifier addresses (sanitized) are attached +as labels to metrics `vmalert_alerts_sent_.*` on `http:///metrics` page. Consider limiting user's access +to the web UI or `/metrics` page if this information is sensitive. + +[Alerts state](#alerts-state) page or [debug mode](#debug-mode) could emit additional information about configured +datasource URL, GET params and headers. Sensitive information such as passwords or auth tokens is stripped by default. +To disable stripping of such info pass `-datasource.showURL` cmd-line flag to vmalert. + + ## Profiling `vmalert` provides handlers for collecting the following [Go profiles](https://blog.golang.org/profiling-go-programs): @@ -955,7 +972,8 @@ The shortlist of configuration flags is the following: -datasource.roundDigits int Adds "round_digits" GET param to datasource requests. In VM "round_digits" limits the number of digits after the decimal point in response values. -datasource.showURL - Whether to show -datasource.url in the exported metrics. It is hidden by default, since it can contain sensitive info such as auth key + Whether to avoid stripping sensitive information such as auth headers or passwords from URLs in log messages or UI and exported metrics. + It is hidden by default, since it can contain sensitive info such as auth key. -datasource.tlsCAFile string Optional path to TLS CA file to use for verifying connections to -datasource.url. By default, system CA is used -datasource.tlsCertFile string @@ -975,7 +993,7 @@ The shortlist of configuration flags is the following: -disableAlertgroupLabel Whether to disable adding group's Name as label to generated alerts and time series. -dryRun - Whether to check only config files without running vmalert. The rules file are validated. The -rule flag must be specified. + Whether to check only config files without running vmalert. The rules file are validated. The -rule flag must be specified. -enableTCP6 Whether to enable IPv6 for listening and dialing. By default, only IPv4 TCP and UDP are used -envflag.enable @@ -1098,6 +1116,9 @@ The shortlist of configuration flags is the following: -notifier.url array Prometheus Alertmanager URL, e.g. http://127.0.0.1:9093. List all Alertmanager URLs if it runs in the cluster mode to ensure high availability. Supports an array of values separated by comma or specified via multiple flags. + -notifier.showURL bool + Whether to avoid stripping sensitive information such as passwords from URLs in log messages or UI for -notifier.url. + It is hidden by default, since it can contain sensitive info such as auth key. -notifier.blackhole bool Whether to blackhole alerting notifications. Enable this flag if you want vmalert to evaluate alerting rules without sending any notifications to external receivers (eg. alertmanager). `-notifier.url`, `-notifier.config` and `-notifier.blackhole` are mutually exclusive. -pprofAuthKey string diff --git a/app/vmalert/datasource/init.go b/app/vmalert/datasource/init.go index d4cd3d1c1..d1d4a807d 100644 --- a/app/vmalert/datasource/init.go +++ b/app/vmalert/datasource/init.go @@ -16,7 +16,7 @@ var ( addr = flag.String("datasource.url", "", "Datasource compatible with Prometheus HTTP API. It can be single node VictoriaMetrics or vmselect URL. Required parameter. "+ "E.g. http://127.0.0.1:8428 . See also -remoteRead.disablePathAppend and -datasource.showURL") appendTypePrefix = flag.Bool("datasource.appendTypePrefix", false, "Whether to add type prefix to -datasource.url based on the query type. Set to true if sending different query types to the vmselect URL.") - showDatasourceURL = flag.Bool("datasource.showURL", false, "Whether to show -datasource.url in the exported metrics. "+ + showDatasourceURL = flag.Bool("datasource.showURL", false, "Whether to avoid stripping sensitive information such as auth headers or passwords from URLs in log messages or UI and exported metrics. "+ "It is hidden by default, since it can contain sensitive info such as auth key") headers = flag.String("datasource.headers", "", "Optional HTTP extraHeaders to send with each request to the corresponding -datasource.url. "+ @@ -62,6 +62,11 @@ func InitSecretFlags() { } } +// ShowDatasourceURL whether to show -datasource.url with sensitive information +func ShowDatasourceURL() bool { + return *showDatasourceURL +} + // Param represents an HTTP GET param type Param struct { Key, Value string diff --git a/app/vmalert/datasource/vm.go b/app/vmalert/datasource/vm.go index 8a0f81c6a..ff1e1caa0 100644 --- a/app/vmalert/datasource/vm.go +++ b/app/vmalert/datasource/vm.go @@ -193,8 +193,12 @@ func (s *VMStorage) QueryRange(ctx context.Context, query string, start, end tim } func (s *VMStorage) do(ctx context.Context, req *http.Request) (*http.Response, error) { + ru := req.URL.Redacted() + if *showDatasourceURL { + ru = req.URL.String() + } if s.debug { - logger.Infof("DEBUG datasource request: executing %s request with params %q", req.Method, req.URL.RawQuery) + logger.Infof("DEBUG datasource request: executing %s request with params %q", req.Method, ru) } resp, err := s.c.Do(req.WithContext(ctx)) if errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) { @@ -203,12 +207,12 @@ func (s *VMStorage) do(ctx context.Context, req *http.Request) (*http.Response, resp, err = s.c.Do(req.WithContext(ctx)) } if err != nil { - return nil, fmt.Errorf("error getting response from %s: %w", req.URL.Redacted(), err) + return nil, fmt.Errorf("error getting response from %s: %w", ru, err) } if resp.StatusCode != http.StatusOK { body, _ := io.ReadAll(resp.Body) _ = resp.Body.Close() - return nil, fmt.Errorf("unexpected response code %d for %s. Response body %s", resp.StatusCode, req.URL.Redacted(), body) + return nil, fmt.Errorf("unexpected response code %d for %s. Response body %s", resp.StatusCode, ru, body) } return resp, nil } diff --git a/app/vmalert/main.go b/app/vmalert/main.go index 4e81ce838..8d7dcdc6d 100644 --- a/app/vmalert/main.go +++ b/app/vmalert/main.go @@ -101,6 +101,7 @@ func main() { remoteread.InitSecretFlags() remotewrite.InitSecretFlags() datasource.InitSecretFlags() + notifier.InitSecretFlags() buildinfo.Init() logger.Init() pushmetrics.Init() diff --git a/app/vmalert/notifier/alertmanager.go b/app/vmalert/notifier/alertmanager.go index 527ad17f8..8d1abbb83 100644 --- a/app/vmalert/notifier/alertmanager.go +++ b/app/vmalert/notifier/alertmanager.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "net/http" + "net/url" "strings" "time" @@ -17,7 +18,7 @@ import ( // AlertManager represents integration provider with Prometheus alert manager // https://github.com/prometheus/alertmanager type AlertManager struct { - addr string + addr *url.URL argFunc AlertURLGenerator client *http.Client timeout time.Duration @@ -48,7 +49,12 @@ func (am *AlertManager) Close() { } // Addr returns address where alerts are sent. -func (am AlertManager) Addr() string { return am.addr } +func (am AlertManager) Addr() string { + if *showNotifierURL { + return am.addr.String() + } + return am.addr.Redacted() +} // Send an alert or resolve message func (am *AlertManager) Send(ctx context.Context, alerts []Alert, headers map[string]string) error { @@ -64,7 +70,7 @@ func (am *AlertManager) send(ctx context.Context, alerts []Alert, headers map[st b := &bytes.Buffer{} writeamRequest(b, alerts, am.argFunc, am.relabelConfigs) - req, err := http.NewRequest(http.MethodPost, am.addr, b) + req, err := http.NewRequest(http.MethodPost, am.addr.String(), b) if err != nil { return err } @@ -91,12 +97,16 @@ func (am *AlertManager) send(ctx context.Context, alerts []Alert, headers map[st defer func() { _ = resp.Body.Close() }() + amURL := am.addr.Redacted() + if *showNotifierURL { + amURL = am.addr.String() + } if resp.StatusCode != http.StatusOK { body, err := io.ReadAll(resp.Body) if err != nil { - return fmt.Errorf("failed to read response from %q: %w", am.addr, err) + return fmt.Errorf("failed to read response from %q: %w", amURL, err) } - return fmt.Errorf("invalid SC %d from %q; response body: %s", resp.StatusCode, am.addr, string(body)) + return fmt.Errorf("invalid SC %d from %q; response body: %s", resp.StatusCode, amURL, string(body)) } return nil } @@ -136,8 +146,15 @@ func NewAlertManager(alertManagerURL string, fn AlertURLGenerator, authCfg proma return nil, fmt.Errorf("failed to configure auth: %w", err) } + amURL, err := url.Parse(alertManagerURL) + if err != nil { + return nil, fmt.Errorf("provided incorrect notifier url: %w", err) + } + if !*showNotifierURL { + alertManagerURL = amURL.Redacted() + } return &AlertManager{ - addr: alertManagerURL, + addr: amURL, argFunc: fn, authCfg: aCfg, relabelConfigs: relabelCfg, diff --git a/app/vmalert/notifier/init.go b/app/vmalert/notifier/init.go index 35cc6d9c1..dc4a6e4e7 100644 --- a/app/vmalert/notifier/init.go +++ b/app/vmalert/notifier/init.go @@ -19,6 +19,8 @@ var ( addrs = flagutil.NewArrayString("notifier.url", "Prometheus Alertmanager URL, e.g. http://127.0.0.1:9093. "+ "List all Alertmanager URLs if it runs in the cluster mode to ensure high availability.") + showNotifierURL = flag.Bool("notifier.showURL", false, "Whether to avoid stripping sensitive information such as passwords from URL in log messages or UI for -notifier.url. "+ + "It is hidden by default, since it can contain sensitive info such as auth key") blackHole = flag.Bool("notifier.blackhole", false, "Whether to blackhole alerting notifications. "+ "Enable this flag if you want vmalert to evaluate alerting rules without sending any notifications to external receivers (eg. alertmanager). "+ "`-notifier.url`, `-notifier.config` and `-notifier.blackhole` are mutually exclusive.") @@ -129,6 +131,13 @@ func Init(gen AlertURLGenerator, extLabels map[string]string, extURL string) (fu return cw.notifiers, nil } +// InitSecretFlags must be called after flag.Parse and before any logging +func InitSecretFlags() { + if !*showNotifierURL { + flagutil.RegisterSecretFlag("notifier.url") + } +} + func notifiersFromFlags(gen AlertURLGenerator) ([]Notifier, error) { var notifiers []Notifier for i, addr := range *addrs { diff --git a/app/vmalert/utils.go b/app/vmalert/utils.go index bf5aeca75..d5c729825 100644 --- a/app/vmalert/utils.go +++ b/app/vmalert/utils.go @@ -7,6 +7,7 @@ import ( "strings" "time" + "github.com/VictoriaMetrics/VictoriaMetrics/app/vmalert/datasource" "github.com/VictoriaMetrics/VictoriaMetrics/lib/prompbmarshal" ) @@ -80,6 +81,9 @@ func requestToCurl(req *http.Request) string { schema := req.URL.Scheme requestURL := req.URL.String() + if !datasource.ShowDatasourceURL() { + requestURL = req.URL.Redacted() + } if schema == "" { schema = "http" if req.TLS != nil { @@ -103,9 +107,25 @@ func requestToCurl(req *http.Request) string { for _, k := range keys { cw.add("-H") + if !datasource.ShowDatasourceURL() && isSecreteHeader(k) { + cw.addWithEsc(fmt.Sprintf("%s: ", k)) + continue + } cw.addWithEsc(fmt.Sprintf("%s: %s", k, strings.Join(req.Header[k], " "))) } cw.addWithEsc(requestURL) return cw.string() } + +var secretWords = []string{"auth", "pass", "key", "secret", "token"} + +func isSecreteHeader(str string) bool { + s := strings.ToLower(str) + for _, secret := range secretWords { + if strings.Contains(s, secret) { + return true + } + } + return false +} diff --git a/app/vmalert/utils_test.go b/app/vmalert/utils_test.go index 048d08cb6..5bee17ca6 100644 --- a/app/vmalert/utils_test.go +++ b/app/vmalert/utils_test.go @@ -7,41 +7,69 @@ import ( func TestRequestToCurl(t *testing.T) { f := func(req *http.Request, exp string) { + t.Helper() got := requestToCurl(req) if got != exp { t.Fatalf("expected to have %q; got %q instead", exp, got) } } - req, _ := http.NewRequest(http.MethodPost, "foo.com", nil) + newReq := func(url string, queryParams ...string) *http.Request { + t.Helper() + r, err := http.NewRequest(http.MethodPost, url, nil) + if err != nil { + t.Fatal(err) + } + params := r.URL.Query() + for i := 0; i < len(queryParams); i += 2 { + params.Add(queryParams[i], queryParams[i+1]) + } + r.URL.RawQuery = params.Encode() + + return r + } + + req := newReq("foo.com") f(req, "curl -X POST 'http://foo.com'") - req, _ = http.NewRequest(http.MethodGet, "https://foo.com", nil) - f(req, "curl -k -X GET 'https://foo.com'") - - req, _ = http.NewRequest(http.MethodPost, "foo.com", nil) + req = newReq("foo.com") req.Header.Set("foo", "bar") req.Header.Set("baz", "qux") f(req, "curl -X POST -H 'Baz: qux' -H 'Foo: bar' 'http://foo.com'") - req, _ = http.NewRequest(http.MethodPost, "foo.com", nil) - params := req.URL.Query() - params.Add("query", "up") - params.Add("step", "10") - req.URL.RawQuery = params.Encode() + req = newReq("foo.com", "query", "up", "step", "10") f(req, "curl -X POST 'http://foo.com?query=up&step=10'") - req, _ = http.NewRequest(http.MethodPost, "http://foo.com", nil) - params = req.URL.Query() - params.Add("query", "up") - params.Add("step", "10") - req.URL.RawQuery = params.Encode() + req = newReq("http://foo.com", "query", "up", "step", "10") f(req, "curl -X POST 'http://foo.com?query=up&step=10'") - req, _ = http.NewRequest(http.MethodPost, "https://foo.com", nil) - params = req.URL.Query() - params.Add("query", "up") - params.Add("step", "10") - req.URL.RawQuery = params.Encode() + req = newReq("https://foo.com", "query", "up", "step", "10") f(req, "curl -k -X POST 'https://foo.com?query=up&step=10'") + + req = newReq("https://user:pass@foo.com", "query", "up", "step", "10") + f(req, "curl -k -X POST 'https://user:xxxxx@foo.com?query=up&step=10'") + + req = newReq("https://user:pass@foo.com") + req.Header.Set("Authorisation", "Bearer 123456") + f(req, "curl -k -X POST -H 'Authorisation: ' 'https://user:xxxxx@foo.com'") + + req = newReq("https://user:pass@foo.com") + req.Header.Set("Authorisation", "Basic 123456") + f(req, "curl -k -X POST -H 'Authorisation: ' 'https://user:xxxxx@foo.com'") + + req = newReq("https://foo.com") + req.Header.Set("My-Password", "mypassword") + f(req, "curl -k -X POST -H 'My-Password: ' 'https://foo.com'") + + req = newReq("https://foo.com") + req.Header.Set("key-for", "my-new-key") + f(req, "curl -k -X POST -H 'Key-For: ' 'https://foo.com'") + + req = newReq("https://foo.com") + req.Header.Set("My-Secret-Org", "secret-organisation") + f(req, "curl -k -X POST -H 'My-Secret-Org: ' 'https://foo.com'") + + req = newReq("https://foo.com") + req.Header.Set("Token", "secret-token") + f(req, "curl -k -X POST -H 'Token: ' 'https://foo.com'") } diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index f43dd1e09..90cc54e5e 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -13,6 +13,7 @@ The following `tip` changes can be tested by building VictoriaMetrics components * SECURITY: upgrade Go builder from Go1.21.1 to Go1.21.3. See [the list of issues addressed in Go1.21.2](https://github.com/golang/go/issues?q=milestone%3AGo1.21.2+label%3ACherryPickApproved) and [the list of issues addressed in Go1.21.3](https://github.com/golang/go/issues?q=milestone%3AGo1.21.3+label%3ACherryPickApproved). +* BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): strip sensitive information such as auth headers or passwords from datasource, remote-read, remote-write or notifier URLs in log messages or UI. This behavior is by default and is controlled via `-datasource.showURL`, `-remoteRead.showURL`, `remoteWrite.showURL` or `-notifier.showURL` cmd-line flags. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5044). * BUGFIX: [vmbackup](https://docs.victoriametrics.com/vmbackup.html): properly copy `appliedRetention.txt` files inside `<-storageDataPath>/{data}` folders during [incremental backups](https://docs.victoriametrics.com/vmbackup.html#incremental-backups). Previously the new `appliedRetention.txt` could be skipped during incremental backups, which could lead to increased load on storage after restoring from backup. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5005). * BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): suppress `context canceled` error messages in logs when `vmagent` is reloading service discovery config. This error could appear starting from [v1.93.5](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.93.5). See [this PR](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/5048). * BUGFIX: [MetricsQL](https://docs.victoriametrics.com/MetricsQL.html): allow passing [median_over_time](https://docs.victoriametrics.com/MetricsQL.html#median_over_time) to [aggr_over_time](https://docs.victoriametrics.com/MetricsQL.html#aggr_over_time). See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5034). diff --git a/docs/vmalert.md b/docs/vmalert.md index ff47ec260..f614f89a0 100644 --- a/docs/vmalert.md +++ b/docs/vmalert.md @@ -824,7 +824,8 @@ and check the `Last updates` section: Rows in the section represent ordered rule evaluations and their results. The column `curl` contains an example of HTTP request sent by vmalert to the `-datasource.url` during evaluation. If specific state shows that there were no samples returned and curl command returns data - then it is very likely there was no data in datasource on the -moment when rule was evaluated. +moment when rule was evaluated. Sensitive info is stripped from the `curl` examples - see [security](#security) section +for more details. ### Debug mode @@ -840,6 +841,8 @@ Just set `debug: true` in rule's configuration and vmalert will start printing a 2022-09-15T13:36:56.153Z DEBUG rule "TestGroup":"Conns" (2601299393013563564) at 2022-09-15T15:36:56+02:00: alert 10705778000901301787 {alertgroup="TestGroup",alertname="Conns",cluster="east-1",instance="localhost:8429",replica="a"} PENDING => FIRING: 1m0s since becoming active at 2022-09-15 15:35:56.126006 +0200 CEST m=+39.384575417 ``` +Sensitive info is stripped from the `curl` examples - see [security](#security) section for more details. + ### Never-firing alerts vmalert can detect if alert's expression doesn't match any time series in runtime @@ -884,6 +887,20 @@ The same issue can be caused by collision of configured `labels` on [Group](#gro To fix it one should avoid collisions by carefully picking label overrides in configuration. +## Security + +See general recommendations regarding security [here](https://docs.victoriametrics.com/Single-server-VictoriaMetrics.html#security). + +vmalert [web UI](#web) exposes configuration details such as list of [Groups](#groups), active alerts, +[alerts state](#alerts-state), [notifiers](#notifier-configuration-file). Notifier addresses (sanitized) are attached +as labels to metrics `vmalert_alerts_sent_.*` on `http:///metrics` page. Consider limiting user's access +to the web UI or `/metrics` page if this information is sensitive. + +[Alerts state](#alerts-state) page or [debug mode](#debug-mode) could emit additional information about configured +datasource URL, GET params and headers. Sensitive information such as passwords or auth tokens is stripped by default. +To disable stripping of such info pass `-datasource.showURL` cmd-line flag to vmalert. + + ## Profiling `vmalert` provides handlers for collecting the following [Go profiles](https://blog.golang.org/profiling-go-programs): @@ -966,7 +983,8 @@ The shortlist of configuration flags is the following: -datasource.roundDigits int Adds "round_digits" GET param to datasource requests. In VM "round_digits" limits the number of digits after the decimal point in response values. -datasource.showURL - Whether to show -datasource.url in the exported metrics. It is hidden by default, since it can contain sensitive info such as auth key + Whether to avoid stripping sensitive information such as auth headers or passwords from URLs in log messages or UI and exported metrics. + It is hidden by default, since it can contain sensitive info such as auth key. -datasource.tlsCAFile string Optional path to TLS CA file to use for verifying connections to -datasource.url. By default, system CA is used -datasource.tlsCertFile string @@ -986,7 +1004,7 @@ The shortlist of configuration flags is the following: -disableAlertgroupLabel Whether to disable adding group's Name as label to generated alerts and time series. -dryRun - Whether to check only config files without running vmalert. The rules file are validated. The -rule flag must be specified. + Whether to check only config files without running vmalert. The rules file are validated. The -rule flag must be specified. -enableTCP6 Whether to enable IPv6 for listening and dialing. By default, only IPv4 TCP and UDP are used -envflag.enable @@ -1109,6 +1127,9 @@ The shortlist of configuration flags is the following: -notifier.url array Prometheus Alertmanager URL, e.g. http://127.0.0.1:9093. List all Alertmanager URLs if it runs in the cluster mode to ensure high availability. Supports an array of values separated by comma or specified via multiple flags. + -notifier.showURL bool + Whether to avoid stripping sensitive information such as passwords from URLs in log messages or UI for -notifier.url. + It is hidden by default, since it can contain sensitive info such as auth key. -notifier.blackhole bool Whether to blackhole alerting notifications. Enable this flag if you want vmalert to evaluate alerting rules without sending any notifications to external receivers (eg. alertmanager). `-notifier.url`, `-notifier.config` and `-notifier.blackhole` are mutually exclusive. -pprofAuthKey string