Skip to content

SQL-204: only trust forwarded client IPs behind a proxy - #38442

Draft
jubrad wants to merge 1 commit into
MaterializeInc:mainfrom
jubrad:justin/sql-204-network-policies-can-be-circumvented-if-you-have-direct
Draft

SQL-204: only trust forwarded client IPs behind a proxy#38442
jubrad wants to merge 1 commit into
MaterializeInc:mainfrom
jubrad:justin/sql-204-network-policies-can-be-circumvented-if-you-have-direct

Conversation

@jubrad

@jubrad jubrad commented Aug 25, 2026

Copy link
Copy Markdown
Member

https://linear.app/materializeinc/issue/SQL-204/network-policies-can-be-circumvented-if-you-have-direct-environmentd

Issue

A client reaching environmentd directly can set the mz_forwarded_for pgwire startup parameter, or prepend a PROXY protocol v2 header on HTTP, and have that address recorded as the session's client IP. Network policies are evaluated against that address, so any client can claim to connect from an allowed one. environmentd trusted the value on every listener, with no check that the connection came from balancerd.

Solution

Add a per-listener behind_trusted_proxy flag to mz_server_core::listeners::{BaseListenerConfig, HttpListenerConfig}, threaded into mz_pgwire::Config and HttpConfig. mz_forwarded_for, mz_connection_uuid, and the PROXY protocol header are honored only where it is set; elsewhere the direct peer address is used. The startup parameters are removed from the params map either way, so they never reach session var initialization.

The flag is #[serde(default)] false, so both listener config schema versions parse unchanged and every hand-written config is safe by default. orchestratord sets it on the external SQL and HTTP listeners, which balancerd fronts, and only when it also creates balancerd. internal and metrics listeners never get it.

Tests

  • test_forwarded_client_ip_requires_trusted_proxy (src/environmentd/tests/pgwire.rs): a startup packet carrying mz_forwarded_for yields the real peer IP on an untrusted listener and the forwarded IP on a trusted one.
  • test_proxy_header_requires_trusted_proxy (src/environmentd/tests/server.rs): same for a PROXY v2 header over HTTP.
  • src/server-core/src/listeners.rs: a config that omits the field parses as false.
  • test/balancerd: no_auth_https.json and frontegg_https.json mark their external listeners as proxy-fronted. The two PROXY-header workflows moved from environmentd's internal HTTP port to the external one, the only port balancerd actually fronts, which means they now send the header ahead of the TLS handshake as balancerd does.

For the reviewer

This does not close the hole on the balancerd-fronted external listener itself, which still trusts anything that reaches it directly. In cloud that path is blocked at the network layer. Tightening it further (a peer-address allowlist, or a shared balancerd <-> environmentd secret) is a separate design; balancerd connects with SslVerifyMode::NONE, so there is no client identity to verify today.

A self-managed deployment that fronts environmentd with its own proxy instead of balancerd will now see that proxy's address as the client IP. create_balancers is the only signal orchestratord has for this.

test/balancerd is nightly-only, so its changes are not covered by this PR's checks.

🤖 Generated with Claude Code

A client reaching environmentd directly could set the `mz_forwarded_for`
pgwire startup parameter, or prepend a PROXY protocol v2 header on HTTP,
and have that address recorded as the session's client IP. Network
policies are evaluated against that address, so any client could claim to
connect from an allowed one.

Add a per-listener `behind_trusted_proxy` flag, defaulting to false, and
honor `mz_forwarded_for`, `mz_connection_uuid`, and the PROXY protocol
header only where it is set. orchestratord sets it on the external
listeners, which balancerd fronts, and only when it also creates
balancerd.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant