diff --git a/app/vmagent/main.go b/app/vmagent/main.go index ce2ab12d1..19c54e95c 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 c05ff441d..2241a5626 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/vminsert/main.go b/app/vminsert/main.go index 118726236..c75e4af4b 100644 --- a/app/vminsert/main.go +++ b/app/vminsert/main.go @@ -246,12 +246,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() @@ -260,12 +255,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/vmselect/main.go b/app/vmselect/main.go index f17f2e438..40a789b40 100644 --- a/app/vmselect/main.go +++ b/app/vmselect/main.go @@ -145,8 +145,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() @@ -412,12 +411,10 @@ func RequestHandler(w http.ResponseWriter, r *http.Request) bool { } return true case "/tags/delSeries": - graphiteTagsDelSeriesRequests.Inc() - authKey := r.FormValue("authKey") - if authKey != *deleteAuthKey { - httpserver.Errorf(w, r, "invalid authKey %q. It must match the value from -deleteAuthKey command line flag", authKey) + if !httpserver.CheckAuthFlag(w, r, deleteAuthKey, "deleteAuthKey") { return true } + graphiteTagsDelSeriesRequests.Inc() if err := graphite.TagsDelSeriesHandler(startTime, w, r); err != nil { graphiteTagsDelSeriesErrors.Inc() httpserver.Errorf(w, r, "%s", err) @@ -474,12 +471,10 @@ func RequestHandler(w http.ResponseWriter, r *http.Request) bool { fmt.Fprintf(w, "%s", `{"status":"success","data":[]}`) return true case "/api/v1/admin/tsdb/delete_series": - deleteRequests.Inc() - authKey := r.FormValue("authKey") - if authKey != *deleteAuthKey { - httpserver.Errorf(w, r, "invalid authKey %q. It must match the value from -deleteAuthKey command line flag", authKey) + if !httpserver.CheckAuthFlag(w, r, deleteAuthKey, "deleteAuthKey") { return true } + deleteRequests.Inc() if err := prometheus.DeleteHandler(startTime, r); err != nil { deleteErrors.Inc() httpserver.Errorf(w, r, "%s", err) diff --git a/app/vmstorage/main.go b/app/vmstorage/main.go index d9efd46d9..d2db0305f 100644 --- a/app/vmstorage/main.go +++ b/app/vmstorage/main.go @@ -255,9 +255,7 @@ func Stop() { func RequestHandler(w http.ResponseWriter, r *http.Request) 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 @@ -275,9 +273,7 @@ func RequestHandler(w http.ResponseWriter, r *http.Request) bool { 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") @@ -293,9 +289,7 @@ func RequestHandler(w http.ResponseWriter, r *http.Request) bool { 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 268d57ed3..a34339319 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -30,13 +30,13 @@ 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 - `vm_vmselect_concurrent_requests_current` - the current number of concurrently executed requests - `vm_vmselect_concurrent_requests_limit_reached_total` - the total number of requests, which were put in the wait queue when `-search.maxConcurrentRequests` concurrent requests are being executed - `vm_vmselect_concurrent_requests_limit_timeout_total` - the total number of canceled requests because they were sitting in the wait queue for more than `-search.maxQueueDuration` -* FEATURE [vmctl](https://docs.victoriametrics.com/vmctl.html): add `-remote-read-insecure-skip-verify` command-line flag for remote read protocol. It can be used for skipping TLS certificate verification when connecting to the remote read address. * BUGFIX: [vmui](https://docs.victoriametrics.com/#vmui): properly update the `step` value in url after the `step` input field has been manually changed. This allows preserving the proper `step` when copy-n-pasting the url to another instance of web browser. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3513). * BUGFIX: [vmui](https://docs.victoriametrics.com/#vmui): properly update tooltip when quickly hovering multiple lines on the graph. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3530). @@ -47,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 32dd2d457..a307e2db8 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 99e2e924d..34d71ff63 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()