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

Add support for proxy headers #120

Merged
merged 1 commit into from
Jul 23, 2019
Merged

Add support for proxy headers #120

merged 1 commit into from
Jul 23, 2019

Conversation

dunglas
Copy link
Owner

@dunglas dunglas commented Jul 23, 2019

This PR introduces a new USE_FORWARDED_HEADERS. When set to 1, the hub will read the X-Forwarded-* or the Forwarded headers provided by intermediate proxy servers.

Closes #114 .

@dunglas dunglas merged commit d48bf28 into master Jul 23, 2019
@dunglas dunglas deleted the use-forwarded-headers branch July 23, 2019 04:21
@stof
Copy link

stof commented Jul 24, 2019

Many proxies are using either the X-Forwarded-* headers or the Forwarded header, but not both in parallel. So the config should allow trusting only one of them.
Same is true for X-Real-IP btw.

@stof
Copy link

stof commented Jul 24, 2019

Btw, this is exactly the reason why symfony/http-foundation requires to set which headers get trusted.

@dunglas
Copy link
Owner Author

dunglas commented Jul 24, 2019

It’s how the Gorilla handler works. You don’t have to set both in parallel, only one between X-Forwarded-For, Forwarded and X-Real-IP is enough.

However, headers must be set for both the IP, the host and the protocol.

@stof
Copy link

stof commented Jul 24, 2019

the thing is, if you set one of them in your proxy and you don't clear the other one (because your proxy just does not support it, which might be out of your control), an attacker could use the other header to inject a fake value (which unsupported proxy header allows such attack depends on the header priority in the Gorilla handler)

@dunglas
Copy link
Owner Author

dunglas commented Jul 25, 2019

Indeed. It’s why we’re documenting that both headers have to be set. Having a more flexible configuration could be nice, but it must be added to Gorilla first. Currently Gorilla doesn’t support this.

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.

mercure.error.log : Client IP wrong
2 participants