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

[AlinsRan]

Description

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 $apisix_orig_xf_proto  $http_x_forwarded_proto;
set $apisix_orig_xf_host   $http_x_forwarded_host;
set $apisix_orig_xf_port   $http_x_forwarded_port;
set $apisix_orig_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 $apisix_orig_* copies and an untrusted peer additionally loses the inbound X-Forwarded-For chain.

Two details worth calling out for review:

• The set $apisix_orig_* 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 four 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.

Behaviour changes

Four. 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. $http_x_forwarded_proto/host/port and $http_forwarded at config level. Reading them in the rewrite phase indexes them, so they keep the client's raw value for the rest of the request. Nothing downstream derives from them — the upstream headers come from r->headers_in, and Lua's ctx.var.http_x_forwarded_* re-reads it through the prefix handler — but an access log format that names them now logs what the client sent rather than the sanitized value. $http_x_forwarded_for, by far the most commonly logged, is not in the set list and is unaffected. Documented next to trusted_addresses, naming $scheme / $var_x_forwarded_host / $var_x_forwarded_port as the sanitized substitutes.

3. ctx.var.original_x_forwarded_{proto,host,port,for} replaced by ctx.var.apisix_orig_xf_*. The old fields were written from Lua on every request on the default path, which is the work this change removes. The five set $apisix_orig_* directives hold the same values at config level, readable from a log format or as ctx.var.apisix_orig_xf_proto / _host / _port / _for and ctx.var.apisix_orig_forwarded. An out-of-tree plugin reading the old names will see nil and needs the rename; the mapping is documented next to trusted_addresses. X-Forwarded-For matters most here, since it 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. var_x_forwarded_proto/port/host are likewise gone 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.

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: $apisix_orig_xf_proto, $apisix_orig_xf_host, $apisix_orig_xf_port and $apisix_orig_forwarded are rendered unconditionally and can be named in an access log format.

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

t/core/trusted-addresses.t gains seven cases:

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-* → upstream still receives the observed values
TEST 14	trusted peer sending X-Forwarded-Proto: grpc, route rewrites it to https via proxy-rewrite → upstream receives https
TEST 15	untrusted peer with a boundary configured, forged headers → 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, gets the observed one
TEST 17	untrusted peer with a boundary: a plugin reads the original X-Forwarded-For chain after it is cleared, and the upstream does not

Ran against a pristine master worktree for comparison:

suite	master	this branch
t/core/trusted-addresses.t	ok	ok
t/core/request.t	ok	ok
t/plugin/real-ip.t	ok	ok
t/plugin/redirect.t	ok	ok
t/plugin/proxy-rewrite2.t	ok	ok
t/plugin/ip-restriction.t	ok	ok
t/plugin/proxy-mirror2.t	ok	ok
t/plugin/forward-auth.t	Failed 43-44	Failed 43-44
t/plugin/proxy-rewrite3.t	Failed 112	Failed 112

The two remaining failures are pre-existing on master in this environment and unrelated.

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]

Good catch, and the asymmetry was an oversight rather than a decision — thank you.

X-Forwarded-For is the one header this change clears rather than overwrites, so it is the one where the original is actually lost, and it was the one whose set I removed as unused. Restored in 612a826, together with documentation for all five next to trusted_addresses:

set $apisix_orig_xf_proto  $http_x_forwarded_proto;
set $apisix_orig_xf_host   $http_x_forwarded_host;
set $apisix_orig_xf_port   $http_x_forwarded_port;
set $apisix_orig_forwarded $http_forwarded;
set $apisix_orig_xf_for    $http_x_forwarded_for;

Readable from a log format, or from Lua as ctx.var.apisix_orig_xf_for.

On retaining ctx.var.original_x_forwarded_for under its old name: I went with the equivalent variable instead. The old field was written from Lua on every request on the default path — no trusted_addresses configured, nothing trusted — which is the work this change exists to remove. The config-level variable costs one rewrite-phase assignment in C and is symmetric with the other four. Migration for an out-of-tree plugin is ctx.var.original_x_forwarded_for → ctx.var.apisix_orig_xf_for, and the conf/config.yaml.example note names it as the replacement. Happy to restore the Lua field as well if you would rather not have any rename at all.

One thing worth recording, since it looks like it should be a problem and is not: reading $http_x_forwarded_for in the rewrite phase indexes the variable, so it keeps the client's raw value for the rest of the request. That does not weaken the sanitization — $proxy_add_x_forwarded_for builds its value from r->headers_in.x_forwarded_for directly rather than from the variable, so an untrusted peer's chain still does not reach the upstream.

TEST 17 covers the regression you asked for, pinning both halves at once: with trusted_addresses: 10.0.0.0/8 and a request carrying X-Forwarded-For: 9.9.9.9, 8.8.8.8 from 127.0.0.1, a plugin in the access phase — after sanitization — reads 9.9.9.9, 8.8.8.8 from ctx.var.apisix_orig_xf_for, while the upstream receives x-forwarded-for: 127.0.0.1.