Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Work around gRPC-Gateway bug in X-Forwarded-For handling #2152

Merged
merged 1 commit into from
May 14, 2024

Conversation

haines
Copy link
Member

@haines haines commented May 14, 2024

This PR primarily aims to work around an issue with X-Forwarded-For handling in the gRPC-Gateway (grpc-ecosystem/grpc-gateway#4320).

Given a remote IP of 4.4.4.4 sending incoming headers of

X-Forwarded-For: 1.1.1.1, 2.2.2.2
X-Forwarded-For: 3.3.3.3

the resulting headers forwarded by the gRPC-gateway are

X-Forwarded-For: 1.1.1.1, 2.2.2.2
X-Forwarded-For: 3.3.3.3
X-Forwarded-For: 1.1.1.1, 2.2.2.2, 4.4.4.4

The workaround I have added means that the resulting headers are

X-Forwarded-For: 1.1.1.1, 2.2.2.2
X-Forwarded-For: 3.3.3.3
X-Forwarded-For: 4.4.4.4

We now join the values with ", " rather than "|" to preserve the de-facto standard syntax.

This PR also prevents spoofing the remote address in the audit logs by setting the x-cerbos-http-remote-addr header; currently we take that header into account even for requests not originating from the gRPC-Gateway where it should be set. Even in the case of requests from the gRPC-Gateway, our header is appended to the request headers, so we need to make sure to use the last value for the header, not the first.

@haines haines requested a review from charithe May 14, 2024 09:18
@haines haines force-pushed the xff branch 2 times, most recently from b80f0cf to 8d19bb4 Compare May 14, 2024 09:29
@haines haines merged commit c8edda5 into cerbos:main May 14, 2024
22 checks passed
@haines haines deleted the xff branch May 14, 2024 09:40
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.

None yet

2 participants