Feature #4247
PATCH: enable PROXY protocol and X-Forwarded-For
100%
Description
Dear developers,
For a research demo we have implemented the PROXY protocol support in HTTP connections. It’s based on the X-Forwarded-For patch from Steffen Vogel. And as someone has requested this functionality, we want to share our patch. It’s fully tested and functional.
Title: [TVHE-devel] enable PROXY protocol and X-Forwarded-For
Description: Adds support for the PROXY protocol in incoming TCP connections. Also adds support for the ‘X-Forwarded-For’ header in HTTP connections.
Comments: This new expert functionality is completely optional. By default is disabled. If you enable it, then it’s fully transparent. However, when enabled you can connect to any TCP listening port and use the PROXY protocol to first send the original IP address of the remote client. Also you can use the HTTP protocol header ‘X-Forwarded-For’ for the same functionality. If both are used in the same HTTP connection (PROXY & 'X-Forwarded-For') then the HTTP header has preference.
User case: When you are running the TVHE in a container with a virtual IP address and use a proxy server to route incoming connections. This functionality shows the correct IP address of the client if the proxy server forwards it.
Warning: It’s recommended to only enable this option when the TVHE server is behind a firewall forwarding the client address. If not, a malicious client might spoof the source address.
Related work:
- PROXY protocol description:
http://www.haproxy.org/download/1.8/doc/proxy-protocol.txt
- 'X-Forwarded-For' header description:
http://tools.ietf.org/html/rfc7239
- Original X-Forwarded-For patch from Steffen Vogel:
http://github.com/tvheadend/tvheadend/pull/640
You can merge it with the main branch if you want. As by default it’s disabled, it doesn’t creates a security hole.
Regards!
D.
Files
Subtasks
History
Updated by Mono Polimorph over 7 years ago
Updated by Great!
Someone has listening my request at #4078.
I hope Jaroslav merges this patch soon.
Thank you guys! 
Updated by DSA APF over 7 years ago
Updated by Hi,
Any problem with this patch?
We hope it will be accepted before publishing some other patches related to the SAT>IP protocol.
Regards,
D.
Updated by Jaroslav Kysela over 7 years ago
Updated by Please, create a PR (pull request) on github. I have some comments for the code. Also, you did not resolved the security hole. Only list of allowed (trusted) proxies should be accepted for 'PROXY' requests. So you should check, if the 'PROXY' request comes from an allowed IP list.
Updated by DSA APF over 7 years ago
Hi,
Please, create a PR (pull request) on github. I have some comments for the code.
We will do!
Also, you did not resolved the security hole.
Please, review our patch... the proxy mode is DISABLED by default. So, no security hole at all!
The original PROXY protocol RFC advises against using it in mixed environments with proxy/non-proxy connections.
Regards,
D.
Updated by Jaroslav Kysela over 7 years ago
DSA APF wrote:
Hi,
Please, create a PR (pull request) on github. I have some comments for the code.
We will do!
Thanks.
Also, you did not resolved the security hole.
Please, review our patch... the proxy mode is DISABLED by default. So, no security hole at all!
The original PROXY protocol RFC advises against using it in mixed environments with proxy/non-proxy connections.
I saw the general on/off switch in your patch, but it's not sufficient. Even if TVH is on the local network, the local clients should not be allowed to claim IP addresses of someone else. I would propose to have a list of known proxies and eventually implement '*' string as a general on without any filtering for special purposes.
Updated by DSA APF over 7 years ago
Hi,
Jaroslav Kysela wrote:
Also, you did not resolved the security hole.
Please, review our patch... the proxy mode is DISABLED by default. So, no security hole at all!
The original PROXY protocol RFC advises against using it in mixed environments with proxy/non-proxy connections.I saw the general on/off switch in your patch, but it's not sufficient. Even if TVH is on the local network, the local clients should not be allowed to claim IP addresses of someone else.
As I commented, the safe way for using the proxy protocol is: use it fully, or not use it at all.
So, if you enable it then is enforced to be behind a proxy for all incomming connections. Then the proxy will pass the correct address of the client, and it can be local or remote.
Jaroslav Kysela wrote:
I would propose to have a list of known proxies and eventually implement '*' string as a general on without any filtering for special purposes.
I agree with this. However, for us isn't trivial to implement. Can you do it (after our pull request), please?
Updated by Jaroslav Kysela over 7 years ago
OK, we can start with a simpler version. Please, create PR to get things rolling..
Updated by DSA APF over 7 years ago
Jaroslav Kysela wrote:
OK, we can start with a simpler version. Please, create PR to get things rolling..
Updated by DSA APF over 7 years ago
DSA APF wrote:
Jaroslav Kysela wrote:
OK, we can start with a simpler version. Please, create PR to get things rolling..
Hi Jaroslav,
No interest to commit our patch?
Updated by Mark Clarkstone over 7 years ago
Updated by DSA APF wrote:
DSA APF wrote:
Jaroslav Kysela wrote:
OK, we can start with a simpler version. Please, create PR to get things rolling..
Hi Jaroslav,
No interest to commit our patch?
I'm sure he'll get around to it eventually. Like many he's very busy right now :p
Updated by DSA APF over 7 years ago
Mark Clarkstone wrote:
DSA APF wrote:
DSA APF wrote:
Jaroslav Kysela wrote:
OK, we can start with a simpler version. Please, create PR to get things rolling..
Hi Jaroslav,
No interest to commit our patch?
I'm sure he'll get around to it eventually. Like many he's very busy right now :p
I hope the PR is accepted soon... 
Updated by DSA APF over 7 years ago
DSA APF wrote:
I hope the PR is accepted soon...
Done!
Please, close this ticket!