diff --git a/app/vmagent/main.go b/app/vmagent/main.go index ce2ab12d1a..19c54e95c7 100644 --- a/app/vmagent/main.go +++ b/app/vmagent/main.go @@ -347,12 +347,7 @@ func requestHandler(w http.ResponseWriter, r *http.Request) bool { } return true case "/prometheus/config", "/config": - if *configAuthKey != "" && r.FormValue("authKey") != *configAuthKey { - err := &httpserver.ErrorWithStatusCode{ - Err: fmt.Errorf("The provided authKey doesn't match -configAuthKey"), - StatusCode: http.StatusUnauthorized, - } - httpserver.Errorf(w, r, "%s", err) + if !httpserver.CheckAuthFlag(w, r, configAuthKey, "configAuthKey") { return true } promscrapeConfigRequests.Inc() @@ -361,12 +356,7 @@ func requestHandler(w http.ResponseWriter, r *http.Request) bool { return true case "/prometheus/api/v1/status/config", "/api/v1/status/config": // See https://prometheus.io/docs/prometheus/latest/querying/api/#config - if *configAuthKey != "" && r.FormValue("authKey") != *configAuthKey { - err := &httpserver.ErrorWithStatusCode{ - Err: fmt.Errorf("The provided authKey doesn't match -configAuthKey"), - StatusCode: http.StatusUnauthorized, - } - httpserver.Errorf(w, r, "%s", err) + if !httpserver.CheckAuthFlag(w, r, configAuthKey, "configAuthKey") { return true } promscrapeStatusConfigRequests.Inc() diff --git a/app/vmauth/main.go b/app/vmauth/main.go index c05ff441d0..2241a5626c 100644 --- a/app/vmauth/main.go +++ b/app/vmauth/main.go @@ -60,9 +60,7 @@ func main() { func requestHandler(w http.ResponseWriter, r *http.Request) bool { switch r.URL.Path { case "/-/reload": - authKey := r.FormValue("authKey") - if authKey != *reloadAuthKey { - httpserver.Errorf(w, r, "invalid authKey %q. It must match the value from -reloadAuthKey command line flag", authKey) + if !httpserver.CheckAuthFlag(w, r, reloadAuthKey, "reloadAuthKey") { return true } configReloadRequests.Inc() diff --git a/app/vmselect/main.go b/app/vmselect/main.go index e0bba8317a..b1ed4a5291 100644 --- a/app/vmselect/main.go +++ b/app/vmselect/main.go @@ -237,8 +237,7 @@ func requestHandler(w http.ResponseWriter, r *http.Request) bool { path := strings.Replace(r.URL.Path, "//", "/", -1) if path == "/internal/resetRollupResultCache" { - if len(*resetCacheAuthKey) > 0 && r.FormValue("authKey") != *resetCacheAuthKey { - sendPrometheusError(w, r, fmt.Errorf("invalid authKey=%q for %q", r.FormValue("authKey"), path)) + if !httpserver.CheckAuthFlag(w, r, resetCacheAuthKey, "resetCacheAuthKey") { return true } promql.ResetRollupResultCache() diff --git a/app/vmstorage/main.go b/app/vmstorage/main.go index 41cda2d07d..31511dd80b 100644 --- a/app/vmstorage/main.go +++ b/app/vmstorage/main.go @@ -170,9 +170,7 @@ func newRequestHandler(strg *storage.Storage) httpserver.RequestHandler { func requestHandler(w http.ResponseWriter, r *http.Request, strg *storage.Storage) bool { path := r.URL.Path if path == "/internal/force_merge" { - authKey := r.FormValue("authKey") - if authKey != *forceMergeAuthKey { - httpserver.Errorf(w, r, "invalid authKey %q. It must match the value from -forceMergeAuthKey command line flag", authKey) + if !httpserver.CheckAuthFlag(w, r, forceMergeAuthKey, "forceMergeAuthKey") { return true } // Run force merge in background @@ -191,9 +189,7 @@ func requestHandler(w http.ResponseWriter, r *http.Request, strg *storage.Storag return true } if path == "/internal/force_flush" { - authKey := r.FormValue("authKey") - if authKey != *forceFlushAuthKey { - httpserver.Errorf(w, r, "invalid authKey %q. It must match the value from -forceFlushAuthKey command line flag", authKey) + if !httpserver.CheckAuthFlag(w, r, forceFlushAuthKey, "forceFlushAuthKey") { return true } logger.Infof("flushing storage to make pending data available for reading") @@ -203,9 +199,7 @@ func requestHandler(w http.ResponseWriter, r *http.Request, strg *storage.Storag if !strings.HasPrefix(path, "/snapshot") { return false } - authKey := r.FormValue("authKey") - if authKey != *snapshotAuthKey { - httpserver.Errorf(w, r, "invalid authKey %q. It must match the value from -snapshotAuthKey command line flag", authKey) + if !httpserver.CheckAuthFlag(w, r, snapshotAuthKey, "snapshotAuthKey") { return true } path = path[len("/snapshot"):] diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 74bd50eb1e..a343393191 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -30,6 +30,7 @@ The following tip changes can be tested by building VictoriaMetrics components f * FEATURE: [csvimport](https://docs.victoriametrics.com/#how-to-import-csv-data): support empty values for imported metrics. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3540). * FEATURE: [vmalert](httpпоs://docs.victoriametrics.com/vmalert.html): allow configuring the default number of stored rule's update states in memory via global `-rule.updateEntriesLimit` command-line flag or per-rule via rule's `update_entries_limit` configuration param. See [these docs](https://docs.victoriametrics.com/vmalert.html#rules) and [this pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/3556). * FEATURE: improve the logic benhind `-maxConcurrentInserts` command-line flag. Previously this flag was limiting the number of concurrent connections from clients, which write data to VictoriaMetrics or [vmagent](https://docs.victoriametrics.com/vmagent.html). Some of these connections could be idle for some time. These connections do not need significant amounts of CPU and memory, so there is no sense in limiting their count. The updated logic behind `-maxConcurrentInserts` limits the number of **active** insert requests, not counting idle connections. +* FEATURE: protect all the http endpoints with `-httpAuth.*` command-line flag. Previously endpoints protected by `-*AuthKey` command-line flags weren't protected by `-httpAuth.*`. This could complicate the proper security setup. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3060). * FEATURE: [VictoriaMetrics cluster](https://docs.victoriametrics.com/Cluster-VictoriaMetrics.html): add `-maxConcurrentInserts` and `-insert.maxQueueDuration` command-line flags to `vmstorage`, so they could be tuned if needed in the same way as at `vminsert` nodes. * FEATURE: [VictoriaMetrics cluster](https://docs.victoriametrics.com/Cluster-VictoriaMetrics.html): limit the number of concurrently executed requests at `vmstorage` proportionally to the number of available CPU cores, since every request can saturate a single CPU core at `vmstorage`. Previously a single `vmstorage` could accept and start processing arbitrary number of concurrent requests received from big number of `vmselect` nodes. This could result in increased RAM, CPU and disk IO usage or event to out of memory crash at `vmstorage` side under high load. The limit can be fine-tuned if needed via `-search.maxConcurrentRequests` command-line flag at `vmstorage` according to [these docs](https://docs.victoriametrics.com/Cluster-VictoriaMetrics.html#resource-usage-limits). `vmstorage` now [exposes](https://docs.victoriametrics.com/Cluster-VictoriaMetrics.html#monitoring) the following additional metrics at `http://vmstorage:8482/metrics` page: - `vm_vmselect_concurrent_requests_capacity` - the maximum number of requests allowed to execute concurrently @@ -46,6 +47,7 @@ The following tip changes can be tested by building VictoriaMetrics components f * BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): fix possible resource leak after hot reload of the updated [consul_sd_configs](https://docs.victoriametrics.com/sd_configs.html#consul_sd_configs). See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3468). * BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): fix a panic in [gce_sd_configs](https://docs.victoriametrics.com/sd_configs.html#gce_sd_configs) when the discovered instance has zero labels. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3624). The issue has been introduced in [v1.85.0](https://docs.victoriametrics.com/CHANGELOG.html#v1850). * BUGFIX: properly return label names starting from uppercase such as `CamelCaseLabel` from [/api/v1/labels](https://docs.victoriametrics.com/url-examples.html#apiv1labels). See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3566). +* BUGFIX: fix `opentsdb` HTTP endpoint not respecting `-httpAuth.*` flags. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3060) * BUGFIX: consistently select the sample with the biggest value out of samples with identical timestamps during querying when the [deduplication](https://docs.victoriametrics.com/#deduplication) is enabled according to [this feature request](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3333). Previously random samples could be selected during querying. diff --git a/lib/httpserver/httpserver.go b/lib/httpserver/httpserver.go index 32dd2d4573..a307e2db83 100644 --- a/lib/httpserver/httpserver.go +++ b/lib/httpserver/httpserver.go @@ -292,8 +292,7 @@ func handlerWrapper(s *server, w http.ResponseWriter, r *http.Request, rh Reques return case "/metrics": metricsRequests.Inc() - if len(*metricsAuthKey) > 0 && r.FormValue("authKey") != *metricsAuthKey { - http.Error(w, "The provided authKey doesn't match -metricsAuthKey", http.StatusUnauthorized) + if !CheckAuthFlag(w, r, metricsAuthKey, "metricsAuthKey") { return } startTime := time.Now() @@ -302,8 +301,7 @@ func handlerWrapper(s *server, w http.ResponseWriter, r *http.Request, rh Reques metricsHandlerDuration.UpdateDuration(startTime) return case "/flags": - if len(*flagsAuthKey) > 0 && r.FormValue("authKey") != *flagsAuthKey { - http.Error(w, "The provided authKey doesn't match -flagsAuthKey", http.StatusUnauthorized) + if !CheckAuthFlag(w, r, flagsAuthKey, "flagsAuthKey") { return } w.Header().Set("Content-Type", "text/plain; charset=utf-8") @@ -322,8 +320,7 @@ func handlerWrapper(s *server, w http.ResponseWriter, r *http.Request, rh Reques default: if strings.HasPrefix(r.URL.Path, "/debug/pprof/") { pprofRequests.Inc() - if len(*pprofAuthKey) > 0 && r.FormValue("authKey") != *pprofAuthKey { - http.Error(w, "The provided authKey doesn't match -pprofAuthKey", http.StatusUnauthorized) + if !CheckAuthFlag(w, r, pprofAuthKey, "pprofAuthKey") { return } DisableResponseCompression(w) @@ -331,7 +328,7 @@ func handlerWrapper(s *server, w http.ResponseWriter, r *http.Request, rh Reques return } - if !checkBasicAuth(w, r) { + if !CheckBasicAuth(w, r) { return } if rh(w, r) { @@ -344,7 +341,25 @@ func handlerWrapper(s *server, w http.ResponseWriter, r *http.Request, rh Reques } } -func checkBasicAuth(w http.ResponseWriter, r *http.Request) bool { +// CheckAuthFlag checks whether the given authKey is set and valid +// +// Falls back to checkBasicAuth if authKey is not set +func CheckAuthFlag(w http.ResponseWriter, r *http.Request, flagValue *string, flagName string) bool { + if len(*flagValue) == 0 { + return CheckBasicAuth(w, r) + } + + if r.FormValue("authKey") != *flagValue { + http.Error(w, fmt.Sprintf("The provided authKey doesn't match -%s", flagName), http.StatusUnauthorized) + return false + } + + return true +} + +// CheckBasicAuth validates credentials provided in request if httpAuth.* flags are set +// returns true if credentials are valid or httpAuth.* flags are not set +func CheckBasicAuth(w http.ResponseWriter, r *http.Request) bool { if len(*httpAuthUsername) == 0 { // HTTP Basic Auth is disabled. return true diff --git a/lib/ingestserver/opentsdbhttp/server.go b/lib/ingestserver/opentsdbhttp/server.go index 99e2e924d9..34d71ff63c 100644 --- a/lib/ingestserver/opentsdbhttp/server.go +++ b/lib/ingestserver/opentsdbhttp/server.go @@ -86,6 +86,9 @@ func (s *Server) MustStop() { func newRequestHandler(insertHandler func(r *http.Request) error) http.Handler { rh := func(w http.ResponseWriter, r *http.Request) { + if !httpserver.CheckBasicAuth(w, r) { + return + } writeRequests.Inc() if err := insertHandler(r); err != nil { writeErrors.Inc()