-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[PROPOSAL][RFC] Enhance trusted_proxies to accept DNS names #7765
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
Comments
We've just adding support for IP ranges, which should be enough and don't have the performance impact that DNS resolving has. |
We could handle resolution only once at the container compilation time. Then one can add something like |
@lazyhammer I'd like to avoid adding more complexity if IP ranges work. |
@fabpot One of the issues I see with IP ranges is that in the case of EC2, private IP address are given according to RFC 1918 which is 10.0.0.0/8 (CIDR) that's over 16m possible addresses. For the ultra security conscious, this isn't the most secure option. I suppose the saving grace with EC2 is that I can set up a security group so that the instance itself is protected from every other EC2 instance on their network. |
And you can do the resolution yourself before configuring the trusted proxies: $ip = gethostbyname('example.com'); |
@fabpot I have given this a bit of thought and have come up with a workaround. I'm mostly happy with it but I have a question that I can't for the life of me answer for myself. We use GeoIP functionality quite heavily in our app (2.1.10-DEV). We are passing our GeoIP library the client's IP address using the Request::getClientIp() method. The way we are setting our trusted_proxies is in our front controller using:
The catch is, we're also using APC (ie: HttpCache). The HttpCache class rewrites the $_SERVER['REMOTE_ADDR'] value to 127.0.0.1 after it has added the original to the X-Forward-For header chain. This is all fine I understand the logic here. However, the very first line in Request::getClientIp() is:
My issue is that unless you add 127.0.0.1 to your trusted_proxies array, getClientIp will always return 127.0.0.1. Basically when the array_diff happens at the end of the method:
127.0.0.1 will always be the last element in the array and the return:
will always return 127.0.0.1 regardless of whether the $clientIps condition fails or not. The only way around this behaviour is to add 127.0.0.1 to the list of trusted proxies. Is this the intended logic? Obviously without HttpCache all is fine. Am I missing something? Or are we to treat HttpCache as a trusted proxy? |
To answer your question, yes, HttpCache acts as any other proxy, so it should be trusted explicitely. Closing this ticket as there are ways to use DNS names instead of IP as explained in the comments. Adding support for DNS names would add complexity and would have a performance impact (doing the resolution once when the container is compiled would defeat the purpose of the change). |
We are currently in the process of deploying a big Symfony 2.1 application and we've hit a minor snag. The application runs in a cluster of application servers behind 2 or more HAProxy load balancers.
All the servers are Amazon EC2 instances (including the HAProxy load balancers) whose internal IP addresses can change regularly. As a result, managing the "trusted_proxies" array is becoming a bit of a nightmare. At any one time we may have 10 app servers that would need their trusted_proxies config array updated with the new IP addresses of any newly provisioned HAProxy instances.
So, in the short term we've had to use the deprecated "trust_proxy_headers" parameter.
I would like to propose that the trusted_proxies parameter be able to handle DNS names. This way we could dynamically update our DNS zone file each time a new load balancer was brought in to the mix.
I'm interested in getting peoples thoughts on the various implications of adding this feature? How would it look from a config perspective? Obviously security and performance need to be taken in to consideration. I've pasted some ideas below:
Option 1:
Option 2:
The idea here being that by flagging the proxy entries as domains, the HttpKernel does not need to ascertain whether it's dealing with domain names or IP addresses (although it is a pretty simple regex)
Option 3:
This is a mixed approach where the HttpKernel would be responsible for determining whether an entry needed an address lookup (gethostbyname) or not. In my opinion this is the most flexible option as it allows for a mix of IPs and DNS names
Obviously gethostbyname opterations are expensive but I can't think of any other way around it.
I'm keen to hear peoples thoughts on the above.
The text was updated successfully, but these errors were encountered: