From f4c597da882a69f597d9b749f9924c84e05c5e01 Mon Sep 17 00:00:00 2001 From: Nikolay Date: Fri, 29 Nov 2024 10:25:47 +0100 Subject: [PATCH] app/vmauth: add removeXFFHTTPHeaderValue flag Previously, there was no option to replace value of `X-Forwarded-For` HTTP Header. It was only possible to completely remove it. It's not good solution, since backend may require this information. But using direct value of this header is insecure. And requires complex knowledge of infrastruce at backend side (see spoofing X-Forwarded-For articles). This commit adds new flag, that replaces content of `X-Forwarded-For` HTTP Header value with current `RemoteAddress` of client that send request. It should be used if `vmauth` is directly attached to the internet. Related issue: https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6883 --------- Signed-off-by: f41gh7 --- app/vmauth/main.go | 6 ++++-- docs/changelog/CHANGELOG.md | 1 + docs/vmauth.md | 3 +++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/app/vmauth/main.go b/app/vmauth/main.go index 16ea0db1e..26814b721 100644 --- a/app/vmauth/main.go +++ b/app/vmauth/main.go @@ -61,7 +61,9 @@ var ( "See https://docs.victoriametrics.com/vmauth/#backend-tls-setup") backendTLSServerName = flag.String("backend.TLSServerName", "", "Optional TLS ServerName, which must be sent to HTTPS backend. "+ "See https://docs.victoriametrics.com/vmauth/#backend-tls-setup") - dryRun = flag.Bool("dryRun", false, "Whether to check only config files without running vmauth. The auth configuration file is validated. The -auth.config flag must be specified.") + dryRun = flag.Bool("dryRun", false, "Whether to check only config files without running vmauth. The auth configuration file is validated. The -auth.config flag must be specified.") + removeXFFHTTPHeaderValue = flag.Bool(`removeXFFHTTPHeaderValue`, false, "Whether to remove the X-Forwarded-For HTTP header value from client requests before forwarding them to the backend. "+ + "Recommended when vmauth is exposed to the internet.") ) func main() { @@ -386,7 +388,7 @@ func sanitizeRequestHeaders(r *http.Request) *http.Request { // X-Forwarded-For information as a comma+space // separated list and fold multiple headers into one. prior := req.Header["X-Forwarded-For"] - if len(prior) > 0 { + if len(prior) > 0 && !*removeXFFHTTPHeaderValue { clientIP = strings.Join(prior, ", ") + ", " + clientIP } req.Header.Set("X-Forwarded-For", clientIP) diff --git a/docs/changelog/CHANGELOG.md b/docs/changelog/CHANGELOG.md index 4c06cbede..4e7346b11 100644 --- a/docs/changelog/CHANGELOG.md +++ b/docs/changelog/CHANGELOG.md @@ -27,6 +27,7 @@ See also [LTS releases](https://docs.victoriametrics.com/lts-releases/). * FEATURE: [vmauth](https://docs.victoriametrics.com/vmauth/): add `dump_request_on_errors` bool setting to [auth config](https://docs.victoriametrics.com/vmauth/#auth-config) for debugging HTTP requests that missed routing rules. This should improve debugability of vmauth settings. * FEATURE: [vmauth](https://docs.victoriametrics.com/vmauth/): add `real_ip_header` setting to [ip_filters](https://docs.victoriametrics.com/vmauth/#ip-filters) and corresponding global flag `httpRealIPHeader`. It allows `vmauth` obtain client IP from HTTP headers for filtering. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6883) for details. * FEATURE: [vmauth](https://docs.victoriametrics.com/vmauth/): add `dryRun` flag to validate configuration. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/7505) for details. +* FEATURE: [vmauth](https://docs.victoriametrics.com/vmauth/): add `removeXFFHTTPHeaderValue` flag to remove content of `X-Forwarded-For` HTTP Header before proxy it to the backend. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6883) for details. * BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent): Properly return `200 OK` HTTP status code when importing data via [Pushgateway protocol](https://docs.victoriametrics.com/#how-to-import-data-in-prometheus-exposition-format) using [multitenant URL format](https://docs.victoriametrics.com/cluster-victoriametrics/#url-format). See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3636) and [this pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/7571). * BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent): Properly set `TCP` connection timeout for `Kubernetes API server` connection for metric scrapping with `kubernetes_sd_configs`. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/7127). diff --git a/docs/vmauth.md b/docs/vmauth.md index 9dce0b6eb..817914787 100644 --- a/docs/vmauth.md +++ b/docs/vmauth.md @@ -837,6 +837,7 @@ By default, the client's TCP address is utilized for IP filtering. In scenarios * Dropping `X-Forwarded-For` headers at the internet-facing reverse proxy (e.g., before traffic reaches `vmauth`). * Do not use `-httpRealIPHeader` at internet-facing `vmauth`. +* Add `removeXFFHTTPHeaderValue` for the internet-facing `vmauth`. It instructs `vmauth` to replace value of `X-Forwarded-For` HTTP header with `remoteAddr` of the client. See additional recommendations at [link](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For#security_and_privacy_concerns) @@ -1372,6 +1373,8 @@ See the docs at https://docs.victoriametrics.com/vmauth/ . -reloadAuthKey value Auth key for /-/reload http endpoint. It must be passed via authKey query arg. It overrides -httpAuth.* Flag value can be read from the given file when using -reloadAuthKey=file:///abs/path/to/file or -reloadAuthKey=file://./relative/path/to/file . Flag value can be read from the given http/https url when using -reloadAuthKey=http://host/path or -reloadAuthKey=https://host/path + -removeXFFHTTPHeaderValue + Whether to remove the X-Forwarded-For HTTP header value from client requests before forwarding them to the backend. Recommended when vmauth is exposed to the internet. -responseTimeout duration The timeout for receiving a response from backend (default 5m0s) -retryStatusCodes array