From e4d6b750f6967386425738d1fd754047c2ec617e Mon Sep 17 00:00:00 2001 From: Nikolay Date: Mon, 20 Jun 2022 16:09:32 +0200 Subject: [PATCH] lib/httpserver: adds flagsAuthKey command-line flag (#2758) * lib/httpserver: adds flagsAuthKey command-line flag It protects /flags endpoint with authKey. https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2753O * Apply suggestions from code review Co-authored-by: Aliaksandr Valialkin --- app/vmauth/README.md | 6 +++++- docs/CHANGELOG.md | 2 ++ docs/vmauth.md | 6 +++++- lib/httpserver/httpserver.go | 6 ++++++ 4 files changed, 18 insertions(+), 2 deletions(-) diff --git a/app/vmauth/README.md b/app/vmauth/README.md index 6cf88ce96..ef9298c51 100644 --- a/app/vmauth/README.md +++ b/app/vmauth/README.md @@ -140,7 +140,11 @@ Do not transfer Basic Auth headers in plaintext over untrusted networks. Enable Alternatively, [https termination proxy](https://en.wikipedia.org/wiki/TLS_termination_proxy) may be put in front of `vmauth`. -It is recommended protecting `/-/reload` endpoint with `-reloadAuthKey` command-line flag, so external users couldn't trigger config reload. +It is recommended protecting following endpoints with authKeys: +* `/-/reload` with `-reloadAuthKey` command-line flag, so external users couldn't trigger config reload. +* `/flags` with `-flagsAuthkey` command-line flag, so unauthorized users couldn't get application command-line flags. +* `/metrics` with `metricsAuthkey` command-line flag, so unauthorized users couldn't get access to [vmauth metrics](#monitoring). +* `/debug/pprof` with `pprofAuthKey` command-line flag, so unauthorized users couldn't get access to [profiling information](#profiling). ## Monitoring diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 0696396ab..eb672322c 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -17,6 +17,8 @@ The following tip changes can be tested by building VictoriaMetrics components f **Update notes:** this release introduces backwards-incompatible changes to communication protocol between `vmselect` and `vmstorage` nodes in cluster version of VictoriaMetrics because of added [query tracing](https://docs.victoriametrics.com/Single-server-VictoriaMetrics.html#query-tracing), so `vmselect` and `vmstorage` nodes may log communication errors during the upgrade. These errors should stop after all the `vmselect` and `vmstorage` nodes are updated to new release. It is safe to downgrade to previous releases. +* SECURITY: Adds `flagsAuthKey` command-line flag, which protects `/flags` endpoint and prevents application configuration disclosure. See [This issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2753). + * FEATURE: support query tracing, which allows determining bottlenecks during query processing. See [these docs](https://docs.victoriametrics.com/Single-server-VictoriaMetrics.html#query-tracing) and [this feature request](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1403). * FEATURE: [vmui](https://docs.victoriametrics.com/#vmui): add `cardinality` tab, which can help identifying the source of [high cardinality](https://docs.victoriametrics.com/FAQ.html#what-is-high-cardinality) and [high churn rate](https://docs.victoriametrics.com/FAQ.html#what-is-high-churn-rate) issues. See [this](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2233) and [this](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2730) feature requests and [these docs](https://docs.victoriametrics.com/#cardinality-explorer). * FEATURE: [vmui](https://docs.victoriametrics.com/#vmui): small UX enhancements according to [this feature request](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2638). diff --git a/docs/vmauth.md b/docs/vmauth.md index 3d965ab9c..cde085e93 100644 --- a/docs/vmauth.md +++ b/docs/vmauth.md @@ -144,7 +144,11 @@ Do not transfer Basic Auth headers in plaintext over untrusted networks. Enable Alternatively, [https termination proxy](https://en.wikipedia.org/wiki/TLS_termination_proxy) may be put in front of `vmauth`. -It is recommended protecting `/-/reload` endpoint with `-reloadAuthKey` command-line flag, so external users couldn't trigger config reload. +It is recommended protecting following endpoints with authKeys: +* `/-/reload` with `-reloadAuthKey` command-line flag, so external users couldn't trigger config reload. +* `/flags` with `flagsAuthkey` command-line flag, so external users couldn't get application configuriton. +* `/metrics` with `metricsAuthkey` command-line flag, so external users couldn't get application metrics. +* `/debug/pprof` with `pprofAuthKey` command-line flag, so external users couldn't get access to application profiling information. ## Monitoring diff --git a/lib/httpserver/httpserver.go b/lib/httpserver/httpserver.go index 9ef82aaa5..381ea1547 100644 --- a/lib/httpserver/httpserver.go +++ b/lib/httpserver/httpserver.go @@ -48,6 +48,8 @@ var ( shutdownDelay = flag.Duration("http.shutdownDelay", 0, `Optional delay before http server shutdown. During this delay, the server returns non-OK responses from /health page, so load balancers can route new requests to other servers`) idleConnTimeout = flag.Duration("http.idleConnTimeout", time.Minute, "Timeout for incoming idle http connections") connTimeout = flag.Duration("http.connTimeout", 2*time.Minute, `Incoming http connections are closed after the configured timeout. This may help to spread the incoming load among a cluster of services behind a load balancer. Please note that the real timeout may be bigger by up to 10% as a protection against the thundering herd problem`) + + flagsAuthKey = flag.String("flagsAuthKey", "", "Auth key for /flags endpoint. It must be passed via authKey query arg. It overrides httpAuth.* settings") ) var ( @@ -284,6 +286,10 @@ 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) + return + } w.Header().Set("Content-Type", "text/plain; charset=utf-8") flagutil.WriteFlags(w) return