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

http: add interpretation of "X-Forwarded-For" header #640

Closed
wants to merge 1 commit into from

Conversation

stv0g
Copy link

@stv0g stv0g commented May 3, 2015

This allows to use the builtin ACL when serving TVH behind a SSL reverse
proxy. Before the peer address of the HTTP socket was used.

This allows to use the builtin ACL when serving TVH behind a SSL reverse
proxy.
@InuSasha
Copy link
Contributor

InuSasha commented May 4, 2015

You should add check for allowed reverse proxies.
Now, any client can send the header, and avoiding the ACL.
This affects also a non-proxied environment.

@perexg
Copy link
Contributor

perexg commented May 4, 2015

I agree. It looks like a big security hole.

@stv0g
Copy link
Author

stv0g commented May 4, 2015

Sorry for this really embarrassing mistake :-/
I’m feeling really stupid right now.

I will add an option to enable the interpretation manually.
Do you prefer this to be done with a global setting or one attached to the ACL entries?

Update: Im planning to use config_get_str() for this. To have a clean implementation, we should move this check into the access_{get_verify}() functions.
This requires to pass the HTTP headers to the named functions.

What do you think about this idea @perexg, @InuSasha ?

Steffen

@perexg
Copy link
Contributor

perexg commented Jun 9, 2015

@stv0g : I think it belongs to the global config, so the X-Forwarded-For will be used only when the source IP address is in the proxy whitelist defined in the global config.

@nbittmann
Copy link

Just to bring this up again as I believe it's still an issue? Can we get support for the X Forward header at some point? I'm using a simple Apache mod_proxy setup and I'm very restricted in terms of acl because I only see my proxies address a feature like this would solve it.

@perexg
Copy link
Contributor

perexg commented Apr 4, 2017

Added to latest (configurable).

@perexg perexg closed this Apr 4, 2017
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

4 participants