perf(core): sanitize the X-Forwarded-* headers in the NGINX config

[AlinsRan]

Description

Fixes #13753.

handle_x_forwarded_headers runs on every request to overwrite X-Forwarded-Proto/Host/Port and clear Forwarded, and set_upstream_x_forwarded_headers then copies the result into $var_x_forwarded_* for proxy_set_header. Both do work the configuration can do in C, and both run on the path that matters most: with no apisix.trusted_addresses set — the default — no peer is trusted, so every request takes the same branch.

Config side. more_set_input_headers neutralizes r->headers_in in the rewrite phase, and two maps derive the observed host and port from the Host header:

map $http_host $var_x_forwarded_port {
    default         $server_port;
    "~:(?<p>\d+)$"  $p;
}
map $http_host $var_x_forwarded_host {
    default $http_host;
    ""      $host;
}

set $original_x_forwarded_proto  $http_x_forwarded_proto;
set $original_x_forwarded_host   $http_x_forwarded_host;
set $original_x_forwarded_port   $http_x_forwarded_port;
set $original_forwarded $http_forwarded;
more_set_input_headers "X-Forwarded-Proto: $scheme";
more_set_input_headers "X-Forwarded-Host: $var_x_forwarded_host";
more_set_input_headers "X-Forwarded-Port: $var_x_forwarded_port";
more_set_input_headers "Forwarded: ";

The upstream-facing proxy_set_header X-Forwarded-Proto/Host/Port are removed, along with $var_x_forwarded_*. r->headers_in already holds the values the request should carry and proxy_pass forwards it as it stands, so there is nothing left to copy — and nothing that can overwrite a plugin's rewrite of those headers, which is what the Lua copier existed to preserve. proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for stays: only that variable appends the connection address.

Lua side. What is left needs a trust decision, so it stays in Lua behind a check that is a constant for the worker's lifetime:

local function handle_trusted_x_forwarded_headers(api_ctx)
    if not trusted_addresses_util.is_configured() then
        return
    end
    ...
end

With no trusted_addresses configured this returns on its first line. When a boundary does exist, a trusted peer's values are restored from the $original_* copies and an untrusted peer additionally loses the inbound X-Forwarded-For chain.

Two details worth calling out for review:

• The set $original_* copies are rendered unconditionally rather than behind a template guard on trusted_addresses. The guard is tempting — the copies are only ever read when a boundary exists — but it makes correctness depend on the CLI seeing the same configuration the worker will, which is not guaranteed for a deployment whose configuration can arrive after render time. Getting it wrong is silent and inverts the trust semantics, so the five set directives are always emitted.
• Evaluating $http_x_forwarded_* in the rewrite phase caches the pre-neutralization value for the rest of the request. That is harmless here only because nothing downstream derives from those variables any more. It is the reason the upstream-facing headers must come from r->headers_in rather than from a variable, and the comment in the template says so.

Behaviour

Unchanged. Verified by running each new test case against the previous implementation and taking the expectation from what it produced.

One assertion moved: t/core/trusted-addresses.t TEST 1 no longer expects trusted_addresses_matcher is not initialized in the error log, because with no boundary configured the new code returns before consulting the matcher. The assertion is kept, inverted, in a --- no_error_log block.

ctx.var.original_x_forwarded_* is removed. It had no consumer in-tree, but it was an externally visible ctx variable — a custom plugin reading it will now see nil. var_x_forwarded_proto/port/host are likewise dropped from the writable-variable list in core/ctx.lua; a plugin that wants to change what the upstream receives should use core.request.set_header, which now works for these headers where before it was overwritten.

Fixes #13753 — plugins could not remove or override X-Forwarded-Host

location / carried proxy_set_header X-Forwarded-Host $var_x_forwarded_host;, and $var_x_forwarded_host was populated by set_upstream_x_forwarded_headers, which only assigned it when ctx.var.http_x_forwarded_host was non-nil. proxy-rewrite's headers.remove sets the header to nil, so the assignment was skipped, the variable kept its set $var_x_forwarded_host $host; default, and the original value went upstream anyway. The plugin did remove the header from r->headers_in; proxy_set_header put a value back afterwards.

Removing those three proxy_set_header directives is what this change needed for its own reasons, and it fixes that as a consequence. Measured, upstream echoing what it received:

	before	after
headers.remove: ["X-Forwarded-Host"]	x-forwarded-host: localhost	header not sent
headers.set: {"X-Forwarded-Host": "my-upstream.example.com"}	x-forwarded-host: localhost	x-forwarded-host: my-upstream.example.com

The same mechanism was breaking t/plugin/proxy-rewrite2.t TESTs 4/5/7 and proxy-rewrite3.t 13/34 during development, which is how it was found.

Note that approaches 5 and 6 in that issue — ngx.var.var_x_forwarded_host = '' from a before_proxy plugin — do work today and stop working here; see behaviour change 5 below.

Behaviour changes

Five. The first three are confined to a configured trust boundary or to config-level variable reads; the fourth is visible to anyone running a logger plugin. The default request path — no apisix.trusted_addresses — is bit-identical to before.

1. A trusted peer that sent no X-Forwarded-Host / X-Forwarded-Port. The previous implementation skipped the rewrite entirely for a trusted peer, so those headers stayed absent and the upstream fell through to the NGINX defaults $host / $server_port. The config now injects the observed values first and there is nothing to restore. With Host: Example.COM:8443:

	before	now
upstream X-Forwarded-Host	example.com	Example.COM:8443
upstream X-Forwarded-Port	1984	8443
plugin view	absent	Example.COM:8443 / 8443

Kept deliberately: it makes a trusted peer agree with an untrusted one, which has always produced the Host with its port and case. The old asymmetry came from the code path being skipped, not from a decision. TEST 13 asserts it with a Host that carries a port and mixed case. An empty header value is indistinguishable from an absent one to set, so it lands in the same place — TEST 16.

2. Config-level readers of $http_x_forwarded_proto/host/port and $http_forwarded. Naming them in an access log format, an if, or a map reads the value cached when the override was applied — what the client sent, not the override. Those four are prefix variables, so Lua reads (core.request.header, ctx.var.http_x_forwarded_*) are re-evaluated and always see the overridden values. $http_x_forwarded_for is deliberately not named in the configuration and is unaffected in either direction. $scheme, $var_x_forwarded_host and $var_x_forwarded_port are the overridden values at config level.

3. ctx.var.original_x_forwarded_{proto,host,port,for} now come from the configuration. They were written from Lua on every request on the default path, which is the work this change removes. Five set $original_* directives hold the same values at config level, so ctx.var.original_x_forwarded_proto / _host / _port / _for resolve to them and a plugin reading those names is unaffected. $original_forwarded is new. The values are in fact more available than before: the Lua fields were only written for an untrusted peer, the config writes them for every request. They are also readable from an access log format. This matters most for X-Forwarded-For, which is cleared rather than overwritten when a boundary is configured and the peer is outside it — TEST 17 pins that a plugin still reads the original chain while the upstream does not.

4. Logger plugins now record X-Forwarded-Proto/Host/Port. The neutralization happens before any Lua runs, so those headers are on r->headers_in and ngx.req.get_headers() returns them. log-util.get_full_log reads that map, so every logger plugin — loggly, http-logger, kafka-logger, splunk-hec-logging and the rest — now emits three request headers it did not before. Anyone parsing those logs with a fixed schema should expect the extra fields.

This is the intended shape rather than a side effect. The gateway does put those headers on the request; a log that omits them describes a request that was never made. It is the model Envoy uses — sanitize once on the way in, and let filters, access logs and the upstream all read one value — and it is what #12551 asked for when a plugin was found making a security decision from a forged X-Forwarded-Proto. The alternative, sanitizing only the upstream copy, would leave every one of the ~100 plugins reading the client's raw value unless each is individually taught to ask for the trusted one.

If the client's raw value is wanted for forensics, it is available without giving up the sanitization: $original_x_forwarded_proto, $original_x_forwarded_host, $original_x_forwarded_port, $original_x_forwarded_for and $original_forwarded are rendered unconditionally and can be named in an access log format.

5. $var_x_forwarded_host and $var_x_forwarded_port are no longer writable, and $var_x_forwarded_proto no longer exists. They were set variables feeding proxy_set_header; they are now map outputs feeding more_set_input_headers, so assigning them from Lua is a silent no-op and naming $var_x_forwarded_proto in an access_log_format or a config snippet fails at startup with unknown "var_x_forwarded_proto" variable.

This closes off a workaround that #13753 lists — ngx.var.var_x_forwarded_host = '' in a before_proxy plugin, which does clear the header on master — while fixing the two approaches that issue actually asks for. Measured, proxy-rewrite against a route:

	master	this branch
headers.remove: ["X-Forwarded-Host"]	header still sent	header removed
headers.set: {"X-Forwarded-Host": "..."}	override ignored	override reaches the upstream
ngx.var.var_x_forwarded_host = '' in before_proxy	header removed	no effect

The right way to change what the upstream receives is now core.request.set_header, which works because nothing overwrites r->headers_in afterwards.

t/core/trusted-addresses.t TEST 1 is modified rather than only added to: its --- error_log expectation of trusted_addresses_matcher is not initialized is inverted into a --- no_error_log block, because with no boundary configured the new code returns before consulting the matcher. The assertion is kept, not dropped.

Tests

Eleven cases added. Every expectation was taken from what the previous implementation produced for the same request, rather than written from the specification; the ones marked guard fail against it and pass here.

t/core/trusted-addresses.t:

TEST 11	Host: example.com:8443, no trust boundary → X-Forwarded-Host: example.com:8443, X-Forwarded-Port: 8443
TEST 12	HTTP/1.0 request with no Host header → falls back to $host
TEST 13	trusted peer that sent no X-Forwarded-*, Host carrying a port and mixed case → pins behaviour change 1
TEST 14	trusted peer sending X-Forwarded-Proto: grpc, rewritten to https by proxy-rewrite → upstream receives https
TEST 15	untrusted peer with a boundary, every forwarding header forged → observed values only, no Forwarded, X-Forwarded-For reduced to the connection address
TEST 16	trusted peer sending an empty X-Forwarded-Proto → treated as not sent
TEST 17	guard — untrusted peer with a boundary: a plugin reads the original chain, ctx.var.http_x_forwarded_for is nil, the upstream does not see it
TEST 18	guard — a route matching on http_x_forwarded_for does not see the cleared chain (404, as before)
TEST 19	no trust boundary: the original chain is still preserved for plugins

t/plugin/proxy-rewrite2.t, for #13753:

TEST 9	guard — headers.remove: ["X-Forwarded-Host"] with no trusted_addresses, the reporter's configuration
TEST 10	guard — the same from a trusted client, which takes a different path through the restore logic
TEST 11	overriding X-Forwarded-Host to a value; passes before and after, so it pins that removing the proxy_set_header directives did not cost the case that already worked

The suites that exercise this code — t/core/trusted-addresses.t, t/core/request.t, t/plugin/proxy-rewrite2.t, proxy-rewrite3.t, real-ip.t, redirect.t, ip-restriction.t, proxy-mirror2.t, loggly.t, forward-auth.t — were also run against a worktree at the merge base and compared, rather than judged on their own.

Checklist

• I have explained the need for this PR and the problem it solves
• I have explained the changes or the new features added to this PR
• I have added tests corresponding to this change
• I have updated the documentation to reflect this change — not needed: the behaviour this documents is unchanged, and the existing note in docs/en/latest/plugins/real-ip.md still describes it accurately
• I have verified that this change is backward compatible (please explain if not)

[membphis]

[P2] Preserve access to the original X-Forwarded-For chain

The old untrusted-peer path stores api_ctx.var.original_x_forwarded_for before clearing X-Forwarded-For. This change removes that field and still clears the header before plugins run, while the new $apisix_orig_* variables preserve proto, host, port, and Forwarded but not XFF. Out-of-tree audit or security plugins therefore lose the raw chain with no migration path. Please retain the compatibility field or expose and document an equivalent original-XFF variable, with a regression covering plugin access after sanitization.

[AlinsRan]

Follow-up on the naming, which turns this into a cleaner answer to your point than my last reply gave.

The preserved values are now named after the fields they replace:

set $original_x_forwarded_proto $http_x_forwarded_proto;
set $original_x_forwarded_host  $http_x_forwarded_host;
set $original_x_forwarded_port  $http_x_forwarded_port;
set $original_x_forwarded_for   $http_x_forwarded_for;
set $original_forwarded         $http_forwarded;

Those are the names the values were already kept under as ctx.var.original_x_forwarded_*, so ctx.var resolves them to the NGINX variables and a plugin reading the old names keeps working unchanged. There is no rename to migrate and no compatibility field to retain separately — TEST 17 now reads ctx.var.original_x_forwarded_for and passes.

The values are also strictly more available than before: the Lua fields were only written for an untrusted peer, while the configuration writes them for every request, and they can now be named in an access log format as well.

(The vendor prefix went too — nothing else in ngx_tpl.lua carries one.)

[membphis]

[P2][non-blocking] Preserve original_x_forwarded_for on the default path

The current template initializes $original_x_forwarded_for to an empty value, and Lua populates it only when trusted_addresses is configured and the peer is untrusted. When trusted_addresses is not configured, handle_trusted_x_forwarded_headers returns immediately, so custom plugins or access logs reading ctx.var.original_x_forwarded_for no longer see the incoming X-Forwarded-For chain.

Please consider preserving the original XFF on this default path and adding a regression test for it. This does not block my approval; whether to address it in this PR is up to the author.

[membphis]

LGTM

[membphis]

LGTM