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

SSLProxyHeaders trusted without checking source #11

Open
jdub opened this issue Feb 8, 2015 · 1 comment
Open

SSLProxyHeaders trusted without checking source #11

jdub opened this issue Feb 8, 2015 · 1 comment

Comments

@jdub
Copy link

jdub commented Feb 8, 2015

I see there's no parameter to define the IP or CIDR of your load balancers / proxies / SSL offloaders, and thus the code can't (and doesn't) check if the connection was made from a trusted source before trusting the SSLProxyHeaders.

Thus, if you happen to deploy in production without a load balancer for some reason (or your proxy doesn't scrub XFP headers), anyone could spoof HTTPS request validity just by sending the appropriate header.

(Note: Knowing the connection was from a trusted source is also important for interpreting X-Forwarded-For headers, which don't seem to be dealt with in this handler or elsewhere in martini-contrib as far as I can see. Unless it makes sense to implement XFF support in the secure handler, the list of IPs / CIDRs to trust should be shared somehow.)

@unrolled
Copy link
Contributor

unrolled commented Feb 9, 2015

The practical thinking behind SSLProxyHeaders is that you would only add headers that you know will be provided by your load balancer or proxy. If you are not using either of those, the SSLProxyHeaders should be left empty. This is a bit naive, so I can see why a defined list of trusted IPs would be useful.

Are you currently implementing something like this? A pull request would be gladly accepted!

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

No branches or pull requests

2 participants