Add PROXY protocol support for rate limiting behind load balancers#6819
Add PROXY protocol support for rate limiting behind load balancers#6819raajheshkannaa wants to merge 2 commits intospiffe:mainfrom
Conversation
fde7861 to
32bcd17
Compare
pkg/server/endpoints/endpoints.go
Outdated
| defer l.Close() | ||
|
|
||
| if e.ListenProxyProtocol { | ||
| l = &proxyproto.Listener{Listener: l} |
There was a problem hiding this comment.
I'm thinking that the boolean opt-in makes sense for the cooperative case where the operator controls the network path between the load balancer and SPIRE Server. One thing worth considering though is the adversarial scenario: without a ConnPolicy on the listener, I believe that the library defaults to accepting PROXY protocol headers from any connection. If a client can reach the SPIRE Server port directly (bypassing the load balancer), they could forge an arbitrary source IP in the header and rotate through fake IPs to evade rate limiting.
One way to address this without adding a second config option would be to replace listen_proxy_protocol with something like proxy_protocol_trusted_cidrs (that's a list of CIDRs). When the list is non-empty, PROXY protocol is enabled and restricted to those sources via the library's ConnPolicy; when empty or unset, it's disabled. That way there's still a single knob, and it becomes impossible to enable the feature without specifying who's trusted.
What do you think? Is there a scenario in your use case where you'd need PROXY protocol enabled but wouldn't be able to enumerate the trusted source IPs?
There was a problem hiding this comment.
Good call. Replaced listen_proxy_protocol with proxy_protocol_trusted_cidrs (list of CIDRs). When non-empty, PROXY protocol is enabled and a ConnPolicy built via StrictWhiteListPolicy rejects connections from any source not in the list. When empty/unset, PROXY protocol is disabled. Single knob, forces enumeration of trusted sources.
Also added a test that verifies untrusted sources get rejected.
For our use case we always know the LB IPs, so enumerating trusted CIDRs is not a problem.
When SPIRE server runs behind a load balancer that does not preserve client IPs, all requests appear to originate from the load balancer, causing per-IP rate limiting to incorrectly throttle all agents as a single client. This adds opt-in PROXY protocol (RFC 5765) support via a new listen_proxy_protocol configuration option. When enabled, the TCP listener is wrapped with a PROXY protocol listener (using github.com/pires/go-proxyproto) so that conn.RemoteAddr() returns the real client IP encoded in the PROXY protocol header. The existing rate limiting middleware requires no changes since it already reads the client IP from the gRPC peer address. The option defaults to false and must be explicitly enabled, since the load balancer and SPIRE server must agree on PROXY protocol usage. Fixes spiffe#6678
…idrs Accept maintainer feedback to use a CIDR allowlist instead of a boolean. When non-empty, PROXY protocol is enabled and a ConnPolicy restricts header acceptance to connections from trusted CIDRs only. This prevents source IP spoofing by clients that bypass the load balancer. Signed-off-by: Raajhesh Kannaa Chidambaram <495042+raajheshkannaa@users.noreply.github.com>
32bcd17 to
ea5674e
Compare
Summary
When SPIRE server sits behind a load balancer that doesn't preserve client IPs, per-IP rate limiting treats all agents as one client (the LB IP). This adds opt-in PROXY protocol support so the server can extract real client IPs from the PROXY protocol header.
listen_proxy_protocolconfig option (defaults tofalse)github.com/pires/go-proxyprotowhen enabled, soconn.RemoteAddr()returns the real client IPConfiguration
This must only be enabled when the load balancer is configured to send PROXY protocol headers.
Fixes #6678
Test plan
listen_proxy_protocolin both file config and server config