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

Look at X-Forwarded-Proto for the HTTPS environment variable #11

Merged
merged 1 commit into from
Nov 21, 2013
Merged

Look at X-Forwarded-Proto for the HTTPS environment variable #11

merged 1 commit into from
Nov 21, 2013

Conversation

mpdude
Copy link
Contributor

@mpdude mpdude commented Jul 19, 2013

Amazons ELB don't use X-HTTPS but instead set an "X-Forwarded-Proto: https" header when forwarding https connections.

It would be great if that header (and its value) could be considered alternatively to X-HTTPS.

@mpdude
Copy link
Contributor Author

mpdude commented Jul 19, 2013

Seems this also gets the SCRIPT_URI variable right that mod_rewrite adds. Without this, you might end up with "http://...:443/..." URLs - that is, the X-Forwarded-Port is honored but added explicitely because it differs from the default for the assumed scheme.

gnif pushed a commit that referenced this pull request Nov 21, 2013
Look at X-Forwarded-Proto for the HTTPS environment variable
@gnif gnif merged commit 0e024b1 into gnif:master Nov 21, 2013
@mpdude mpdude deleted the issue-11 branch November 21, 2013 20:39
@gberche-orange
Copy link

I understand that with this patch, both X-HTTPS and X-Forwarded-Proto headers are used to detect HTTPS protocol by the upstream reverse proxy.

For load balancers that only define X-Forwarded-Proto and not X-HTTPS, this opens a security breach in that a malicous client might generate an HTTP request with a additional "X-HTTPS" header to fool the origin server into thinking it was contacted over HTTPS.

Would it make sense to add a new RPAF_Proto_Header configuration directive that specifies the expected HTTP header used to transport the protocol of the request as seen by the reverse proxy ? Default value might be X-HTTPS if backward compatibility needs to be present.

A sample conf could then look as:

LoadModule        rpaf_module modules/mod_rpaf.so
RPAF_Enable       On
RPAF_ProxyIPs     127.0.0.1 10.0.0.10 10.0.0.20
RPAF_SetHostName  On
RPAF_SetHTTPS     On
RPAF_Proto_Header  X-FORWARDED-PROTO
RPAF_SetPort      On

@gnif
Copy link
Owner

gnif commented Jan 2, 2014

Good point, I will see about patching this in, it is indeed a possible security issue.

@mpdude
Copy link
Contributor Author

mpdude commented Oct 6, 2015

@gberche-orange I think your above point is still valid, isn't it? You think you could craft a PR for that?

@gberche-orange
Copy link

@mpdude

Sorry, I was suggesting use of mod_rpaf into the php-buildpack, but they choosed to implement the x-forwarded-* differently not using mod_rpaf. I won't be able to contribute a PR on mod_rpaf.

@mpdude
Copy link
Contributor Author

mpdude commented Oct 6, 2015

Ok, thanks for your reply!

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.

3 participants