Per-gateway PROXY protocol configuration by k-wall · Pull Request #147 · kroxylicious/design

k-wall · 2026-10-22T11:01:29Z

Relates to: kroxylicious/kroxylicious#5118

Summary

Proposes moving PROXY protocol configuration from the existing global setting to a per-gateway level, so different gateways on the same virtual cluster can have different proxy protocol requirements.
Adds proxyProtocol: { mode: required|disabled } to VirtualClusterGateway in the plain config and under spec.loadBalancer in the Kubernetes KafkaProxyIngress CRD.
Removes the insecure allowed (auto-detect) mode; deprecates and ignores the existing global proxyProtocol field.
Introduces two new cause label values on the kroxylicious_client_to_proxy_disconnects metric: proxy_protocol_header_missing and proxy_protocol_header_unexpected.

🤖 Generated with Claude Code

Proposal to allow PROXY protocol acceptance to be configured per gateway rather than via the current global setting. Covers: - Plain config: proxyProtocol field on VirtualClusterGateway - Kubernetes API: proxyProtocol under KafkaProxyIngress spec.loadBalancer - Removal of the insecure 'allowed' (auto-detect) mode - Migration/deprecation of the global proxyProtocol field - New disconnect cause metric labels for protocol mismatches - Out of scope: source address filtering, upstream propagation Relates-to: kroxylicious/kroxylicious#5118 Assisted-by: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Keith Wall <kwall@apache.org>

k-wall · 2026-10-22T11:08:10Z

mapan1984 I'd really like to hear your thoughts in this proposal. How well would it fit your use-case?

Signed-off-by: Keith Wall <kwall@apache.org>

MarkNSweep · 2026-10-22T16:01:56Z

+
+**Mode semantics:**
+
+| Mode | Behaviour |


The table says disabled means "No PROXY protocol handling", but the same row says connections that carry a header are rejected. Those conflict. "Disabled" suggests the feature is off and a header would be ignored or treated as data. The actual behaviour is that the gateway actively rejects it, which is what proxy_protocol_header_unexpected describes.

Since allowed is rejected, there is only one real choice per gateway: PROXY headers are required, or they are not. The consequence follows in each case (a missing header is rejected, or an unexpected header is rejected). That's one boolean, not a two-value mode.

Suggestion: keep an object, so the version constraint mentioned under "Restricting accepted PROXY protocol versions" can be added later without a breaking change, and put a boolean in it:

proxyProtocol: required: true

An absent proxyProtocol block means off.

An omitted required value is a validation error, so there is no silent default.

I'd avoid enabled, which has the same flaw as disabled.

A bare proxyProtocol: true is simpler, but can't be extended later without accepting both a boolean and an object.

proxyProtocol: {} as "on" is risky: proxyProtocol: with no value is YAML null and may be read as absent, and empty maps can be dropped by templating tools. A security setting shouldn't depend on that.

I see what you're saying @MarkNSweep, but I think

proxyProtocol: required: false

is not very clear: A reader could interpret that to mean, "it's not required, so it's optional".

But I'm also a bit unclear about what's meant with

Connections that carry a PROXY header are rejected to prevent injection.

How would to do that @k-wall, but trying to parse the proxy header and rejecting if you succeed, or by just not trying to parse a proxy header at all, and hoping the next handler throws an exception because it doesn't see a TLS ClientHello, or a Kafka request? The latter is easier, but I'm not sure it would be easy to know from the exception that the problem was due to the presence of a PROXY header, rather than any other possible cause. But you need to know for sure if you want to increment proxy_protocol_header_unexpected correctly.

We could resolve the issue by redefining "disabled", saying it means all Proxy Protocol decoding is disabled and acknowledge users would experience cryptic explosive failures if they connect with Proxy Protocol to a disabled gateway. The same as our TLS experience if you connect to a non-TLS gateway.

You could potentially separate the concepts and say that "misconfiguration detection" is a separate feature?

Though perhaps worth an experiment in how that manifests, TLS client hello happens to map to a giant negative framesize, deterministically, so you get a really early failure. Maybe Proxy Protocol has different features, like if the first int happens to decode to a valid but enormous frame size maybe it will stall the connection waiting for data?

I think @robobario's analogy with TLS is hinting at the problem. Each different hander is pulling objects out of the pipe and expects to find only certain things. The SslHandler expects TLS protocol. The PROXY protocol handler expects PROXY protocol. The KafkaRequestDecode expects a Kafka request. When it pulls out something that's unexpected it shouldn't need to figure out what it got just to update a metric. So a metric like proxy_protocol_header_unexpected seems flawed.

The metric instead should be counting the number of times a handler found something unexpected, and should discriminate by the handler's type (essentially the "protocol" at that layer of the network model). So if:

unexpected_client_data{handler=PROXY,gateway=my-gw} then the user could infer "someone sent some something that was not a PROXY header."

unexpected_client_data{handler=TLS,gateway=my-gw} then the user could infer "someone sent some something that was not a TLS frame."

Structuring the metric in this way saves us from having to figure out we pulled out of the pipe. But it also smears the responsibility for updating this metric across each of the layered protocol handlers, so it's not as self-contained as what Keith is suggesting right now.

I've made some edits to try to address the comments.

I've changed proxyProtocol is use values required or forbidden. I hope that addresses @MarkNSweep's concern about disabled's ambiguity. I do prefer an enumerated value over true and false.

I've added a bit more detail to say how/when the enforcement will be carried out. The enforcement point is different for plain vs TLS.

I think using the existing kroxylicious_client_to_proxy_disconnects to record proxy protocol transgressions is good enough for this proposal.

I think detecting TLS/Plain mistakes and unexpected protocols (HTTP) would be good, and ticking a metric, would be desirable, but that's out of scope. I'll raise an issue for that.

MarkNSweep · 2026-10-22T16:16:09Z

+| Mode | Behaviour |
+|------|-----------|
+| `required` | Every connection must begin with a valid PROXY v1 or v2 header. Connections that do not carry the header are rejected. |
+| `disabled` | No PROXY protocol handling. Connections that carry a PROXY header are rejected to prevent injection. This is the default when `proxyProtocol` is omitted. |


Load balancers can be configured to use the proxy protocol for health checks. I believe that the proxy header contains a special value (based on the version of the proxy protocol in use) that would need bespoke handling as it's not a normal request.

could you elaborate further about the problem you see here?

From reading around it sounds more typical that LoadBalancer healthchecks to the Proxy would be either a TCP probe, or an HTTP check to a different endpoint. We currently only support Proxy Protocol on the kafka protocol ports, not the proxy HTTP management endpoint (currently the operator doesn't expose this port, just uses it for internal health/liveness of the proxy pod).

Thanks @MarkNSweep. I hadn't noticed the LOCAL command of v2 before today. I'm no expert in proxy protocol (I've never actually deployed it in production), but I agree with @robobario that probes shouldn't be directed at the gateway. I've added a section to the proposal to say we'll log their arrival to help an administrator spot/rectify the issue.

MarkNSweep · 2026-10-22T16:22:36Z

+
+### `spec.clusterIP.proxyProtocol`
+
+On-cluster clients connect directly to the proxy without an intermediate load balancer. Their network address is already accurate and directly observable. Accepting PROXY headers from on-cluster clients would allow any workload to claim an arbitrary source address, undermining the security model that this feature is designed to enforce.


As far as I know, a LoadBalancer Service can also get a ClusterIP, so an on-cluster client may be able to reach the same pod port directly. If so, required would accept a forged header from it. I don't think this is anything that the proxy can address as it is effectively trusting the load-balancer (and for that load-balancer to be correctly configured). Just mentioning it in passing.

Yes, you are right. A LocalBalancer service gets a ClusterIP and a NodePort, all of which would allow a malicious client to forge a proxy protocol. I think this is a documentation issue, it needs to make it clear that the administrator needs to configure defence(s). This could be firewall rules (baremetal), the source filtering, or kube network policies.

robobario · 2026-10-23T00:46:08Z

+| Mode | Behaviour |
+|------|-----------|
+| `required` | Every connection must begin with a valid PROXY v1 or v2 header. Connections that do not carry the header are rejected. |
+| `disabled` | No PROXY protocol handling. Connections that carry a PROXY header are rejected to prevent injection. This is the default when `proxyProtocol` is omitted. |


could you elaborate further about the problem you see here?

From reading around it sounds more typical that LoadBalancer healthchecks to the Proxy would be either a TCP probe, or an HTTP check to a different endpoint. We currently only support Proxy Protocol on the kafka protocol ports, not the proxy HTTP management endpoint (currently the operator doesn't expose this port, just uses it for internal health/liveness of the proxy pod).

robobario · 2026-10-23T01:47:48Z

+
+**Mode semantics:**
+
+| Mode | Behaviour |


We could resolve the issue by redefining "disabled", saying it means all Proxy Protocol decoding is disabled and acknowledge users would experience cryptic explosive failures if they connect with Proxy Protocol to a disabled gateway. The same as our TLS experience if you connect to a non-TLS gateway.

You could potentially separate the concepts and say that "misconfiguration detection" is a separate feature?

Though perhaps worth an experiment in how that manifests, TLS client hello happens to map to a giant negative framesize, deterministically, so you get a really early failure. Maybe Proxy Protocol has different features, like if the first int happens to decode to a valid but enormous frame size maybe it will stall the connection waiting for data?

mapan1984 · 2026-10-23T03:58:30Z

mapan1984 I'd really like to hear your thoughts in this proposal. How well would it fit your use-case?mapan1984 我很想听听你对这个方案的看法。它是否符合你的使用场景？

Thanks @k-wall — this proposal, together with #148, is a strong complement on the security side, and it's exactly what #139 needs. #139 stresses in its documentation that once proxyProtocol is enabled(allowed/required), the deployment must ensure that only a trusted PROXY sender can reach Kroxylicious, otherwise the header (and therefore the client address a filter sees) is untrusted. #147 and #148 make it easier for users to build that trusted network environment

I don't think either proposal affects the FilterContext API that #139 fixed — the accessors are independent of how the PROXY config is shaped. So I'm happy to let #147 and #148 settle, and update the relevant implementation (and kroxylicious/kroxylicious#5196's ProxyProtocolIT / proxy-protocol docs) once they're finalized and implemented.

Co-authored-by: Robert Young <robertyoungnz@gmail.com> Signed-off-by: Keith Wall <kwall@redhat.com>

…bidden `disabled` implied passive behaviour; `forbidden` makes clear that PROXY headers are actively rejected on gateways where they are not expected. The metric cause label is renamed to match. Assisted-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Keith Wall <kwall@apache.org>

…proach Replace the port-identification-mode axis in the implementation note with the plain/TLS axis, which reflects the real constraint: in TLS connections (both portIdentifiesNode and sniHostIdentifiesNode) the PROXY header arrives before the ClientHello, so detection completes before the gateway is identified and enforcement can happen before TLS negotiation proceeds. Plain portIdentifiesNode connections enforce on the first data bytes. Unconditional use of HaProxyProtocolDetectionHandler is described as the mechanism that makes the metric reliable in all cases. Assisted-by: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Keith Wall <kwall@apache.org>

…command note Proposal 147 must be implemented before 139: once 139 exposes the client address to filters, the existing allowed mode becomes an injection vector. Removing allowed and enforcing per-gateway trust is a prerequisite for the address exposed by 139 to be trustworthy. Also restructures the implementation note into subsections and adds a subsection on PROXY protocol v2 LOCAL command handling. Assisted-by: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Keith Wall <kwall@apache.org>

…nfig Silent ignore of the existing global proxyProtocol field risks operators upgrading and finding clients unable to connect at runtime with no clear indication why. Failing to start with a clear error message is safer. The allowed mode is likewise rejected at startup. Assisted-by: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Keith Wall <kwall@apache.org>

k-wall · 2026-10-24T18:34:45Z

@tombentley @robobario @MarkNSweep thanks for the great feedback. proposal updated. PTAL.

robobario

The forbidden/required posture works for me, though there is some possible contradiction in the LOCAL section, would be good to clarify how it interacts with forbidden.

robobario · 2026-10-26T22:39:03Z

+
+When a `LOCAL` command connection arrives, the proxy logs this at WARN level. This is intended to help the administrator identify misconfigured load balancer health checks: if a `LOCAL` connection appears on a Kafka port, it indicates the load balancer is health checking that port directly rather than using the proxy's HTTP management endpoint, which is the recommended target for health checks.
+
+The connection is otherwise accepted in accordance with the specification. As the load balancer will send HTTP rather than a Kafka RPC, the proxy will treat the subsequent data as a failed TLS negotiation or an invalid Kafka RPC and close the connection, as it does today. Adding a metric for this type of connection error is out of scope for this proposal.


The "otherwise accepted" sounds a little in conflict with our forbidden rule. I would expect it to be rejected if the gateway is set to forbidden.

robobario · 2026-10-26T22:47:48Z

+
+Add an optional `proxyProtocol` block to `gateway` block.
+This will specify if the gateway requires the proxy protocol to be sent or not.
+When the `proxyProtocol` block is absent or empty, the gateway behaves as `forbidden`.


I wonder if we should support empty, maybe if they're supplying this block mode should be required?

k-wall requested a review from a team as a code owner October 22, 2026 11:01

k-wall force-pushed the per-gateway-proxy-protocol branch from d500751 to c1db6ba Compare October 22, 2026 11:04

k-wall force-pushed the per-gateway-proxy-protocol branch from c1db6ba to 462bae7 Compare October 22, 2026 11:05

k-wall added the network label Oct 22, 2026

docs: clarify LB annotation responsibility in proposal 147

b4ad3d8

Signed-off-by: Keith Wall <kwall@apache.org>

MarkNSweep reviewed Oct 22, 2026

View reviewed changes

robobario reviewed Oct 23, 2026

View reviewed changes

k-wall and others added 5 commits October 24, 2026 19:30

Update proposals/147-per-gateway-proxy-protocol.md

e972015

Co-authored-by: Robert Young <robertyoungnz@gmail.com> Signed-off-by: Keith Wall <kwall@redhat.com>

k-wall requested review from MarkNSweep, robobario and tombentley October 24, 2026 18:32

robobario reviewed Oct 26, 2026

View reviewed changes


		### `spec.clusterIP.proxyProtocol`

		On-cluster clients connect directly to the proxy without an intermediate load balancer. Their network address is already accurate and directly observable. Accepting PROXY headers from on-cluster clients would allow any workload to claim an arbitrary source address, undermining the security model that this feature is designed to enforce.


		When a `LOCAL` command connection arrives, the proxy logs this at WARN level. This is intended to help the administrator identify misconfigured load balancer health checks: if a `LOCAL` connection appears on a Kafka port, it indicates the load balancer is health checking that port directly rather than using the proxy's HTTP management endpoint, which is the recommended target for health checks.

		The connection is otherwise accepted in accordance with the specification. As the load balancer will send HTTP rather than a Kafka RPC, the proxy will treat the subsequent data as a failed TLS negotiation or an invalid Kafka RPC and close the connection, as it does today. Adding a metric for this type of connection error is out of scope for this proposal.

Conversation

k-wall commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

k-wall commented Oct 22, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tombentley Oct 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k-wall Oct 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mapan1984 commented Oct 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k-wall commented Oct 24, 2026

Uh oh!

robobario left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

k-wall commented Oct 22, 2026 •

edited

Loading

tombentley Oct 23, 2026 •

edited

Loading

k-wall Oct 24, 2026 •

edited

Loading

mapan1984 commented Oct 23, 2026 •

edited

Loading