From 6a944ca97209360b686f8b0c4e1351b8f0043211 Mon Sep 17 00:00:00 2001 From: Thomas Counsell Date: Thu, 1 Dec 2016 12:04:36 +0000 Subject: [PATCH 1/3] Need to reset $request->getTrustedProxies() between each test #54 --- tests/TrustedProxyTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/TrustedProxyTest.php b/tests/TrustedProxyTest.php index 4eb25e0..8ed6e75 100644 --- a/tests/TrustedProxyTest.php +++ b/tests/TrustedProxyTest.php @@ -122,6 +122,8 @@ protected function createProxiedRequest($serverOverRides = []) // Create a fake request made over "http", one that we'd get over a proxy // which is likely something like this: $request = Request::create('http://localhost:8888/tag/proxy', 'GET', [], [], [], $serverOverRides, null); + // Need to make sure these haven't already been set + $request->setTrustedProxies([]); return $request; } From 764b0b0ced6bc1807b9d5d1ab637ea79ee2af2ad Mon Sep 17 00:00:00 2001 From: Thomas Counsell Date: Thu, 1 Dec 2016 10:11:11 +0000 Subject: [PATCH 2/3] Test for multiple IPs in X_FORWARD_FOR header #49 #46 --- tests/TrustedProxyTest.php | 48 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/TrustedProxyTest.php b/tests/TrustedProxyTest.php index 8ed6e75..5ae66a7 100644 --- a/tests/TrustedProxyTest.php +++ b/tests/TrustedProxyTest.php @@ -55,6 +55,53 @@ public function test_trusted_proxy_sets_trusted_proxies() }); } + /** + * Test X-Forwarded-For header with multiple IP addresses + */ + public function test_get_client_ips() + { + $trustedProxy = $this->createTrustedProxy([], ['192.168.10.10']); + + $forwardedFor = [ + '192.0.2.2', + '192.0.2.2, 192.0.2.199', + '192.0.2.2, 192.0.2.199, 99.99.99.99', + '192.0.2.2,192.0.2.199', + ]; + + foreach($forwardedFor as $forwardedForHeader) { + $request = $this->createProxiedRequest(['HTTP_X_FORWARDED_FOR' => $forwardedForHeader]); + + $trustedProxy->handle($request, function ($request) use ($forwardedForHeader) { + $ips = $request->getClientIps(); + $this->assertEquals('192.0.2.2', end($ips), 'Assert sets the '.$forwardedForHeader); + }); + } + } + + /** + * Test X-Forwarded-For header with multiple IP addresses, with some of those being trusted + */ + public function test_get_client_ip_with_muliple_ip_addresses_some_of_which_are_trusted() + { + $trustedProxy = $this->createTrustedProxy([], ['192.168.10.10', '192.0.2.199']); + + $forwardedFor = [ + '192.0.2.2', + '192.0.2.2, 192.0.2.199', + '99.99.99.99, 192.0.2.2, 192.0.2.199', + '192.0.2.2,192.0.2.199', + ]; + + foreach($forwardedFor as $forwardedForHeader) { + $request = $this->createProxiedRequest(['HTTP_X_FORWARDED_FOR' => $forwardedForHeader]); + + $trustedProxy->handle($request, function ($request) use ($forwardedForHeader) { + $this->assertEquals('192.0.2.2', $request->getClientIp(), 'Assert sets the '.$forwardedForHeader); + }); + } + } + /** * Test renaming the X-Forwarded-For header. */ @@ -71,6 +118,7 @@ public function test_can_rename_forwarded_for_header() }); } + /** * Test renaming *all* the headers. */ From 6018dfb8168c6a4696edc4997f7c20e1b88ac8cd Mon Sep 17 00:00:00 2001 From: Thomas Counsell Date: Thu, 1 Dec 2016 14:38:02 +0000 Subject: [PATCH 3/3] Add '**' as an option for proxies, to trust any IP address (#46, #49) However, if you are in the situation where, say, you have a Content Distribution Network (like Amazon CloudFront) that passes to load balancer (like Amazon ELB) then you may end up with a chain of unknown proxies forwarding from one to another. In that case, '*' above would only match the final proxy (the load balancer in this case) which means that calling `$request->getClientIp()` would return the IP address of the next proxy in line (in this case one of the Content Distribution Network ips) rather than the original client IP.To always get the original client IP, you need to trust all the proxies in the route to your request. You can do this by setting proxies to ['0.0.0.0/0', '2000:0:0:0:0:0:0:0/3'] which means all IP addresses will be trusted. This patch implements ** as syntactic sugar for ['0.0.0.0/0', '2000:0:0:0:0:0:0:0/3'] --- config/trustedproxy.php | 24 +++++++++++-- readme.md | 51 ++++++++++++++++++++++---- src/TrustProxies.php | 74 +++++++++++++++++++++++--------------- tests/TrustedProxyTest.php | 64 +++++++++++++++++++++++++++++++++ 4 files changed, 175 insertions(+), 38 deletions(-) diff --git a/config/trustedproxy.php b/config/trustedproxy.php index fbf89bf..64006cd 100644 --- a/config/trustedproxy.php +++ b/config/trustedproxy.php @@ -9,19 +9,37 @@ * supported, along with CIDR notation. * * The "*" character is syntactic sugar - * within TrustedProxy to trust any proxy; + * within TrustedProxy to trust any proxy + * that connects directly to your server, * a requirement when you cannot know the address * of your proxy (e.g. if using Rackspace balancers). + * + * The "**" character is syntactic sugar within + * TrustedProxy to trust not just any proxy that + * connects directly to your server, but also + * proxies that connect to those proxies, and all + * the way back until you reach the original source + * IP. It will mean that $request->getClientIp() + * always gets the originating client IP, no matter + * how many proxies that client's request has + * subsequently passed through. */ 'proxies' => [ '192.168.1.10', ], /* - * Or, to trust all proxies, uncomment this: + * Or, to trust all proxies that connect + * directly to your server, uncomment this: */ # 'proxies' => '*', + /* + * Or, to trust ALL proxies, including those that + * are in a chain of fowarding, uncomment this: + */ + # 'proxies' => '**', + /* * Default Header Names * @@ -40,4 +58,4 @@ \Illuminate\Http\Request::HEADER_CLIENT_PROTO => 'X_FORWARDED_PROTO', \Illuminate\Http\Request::HEADER_CLIENT_PORT => 'X_FORWARDED_PORT', ] -]; \ No newline at end of file +]; diff --git a/readme.md b/readme.md index 2412e0d..fcd7033 100644 --- a/readme.md +++ b/readme.md @@ -181,19 +181,37 @@ return [ * supported, along with CIDR notation. * * The "*" character is syntactic sugar - * within TrustedProxy to trust any proxy; + * within TrustedProxy to trust any proxy + * that connects directly to your server, * a requirement when you cannot know the address * of your proxy (e.g. if using Rackspace balancers). + * + * The "**" character is syntactic sugar within + * TrustedProxy to trust not just any proxy that + * connects directly to your server, but also + * proxies that connect to those proxies, and all + * the way back until you reach the original source + * IP. It will mean that $request->getClientIp() + * always gets the originating client IP, no matter + * how many proxies that client's request has + * subsequently passed through. */ 'proxies' => [ '192.168.1.10', ], /* - * Or, to trust all proxies, uncomment this: + * Or, to trust all proxies that connect + * directly to your server, uncomment this: */ # 'proxies' => '*', + /* + * Or, to trust ALL proxies, including those that + * are in a chain of fowarding, uncomment this: + */ + # 'proxies' => '**', + /* * Default Header Names * @@ -207,10 +225,10 @@ return [ * \Symfony\Component\HttpFoundation\Request::$trustedHeaders */ 'headers' => [ - Illuminate\Http\Request::HEADER_CLIENT_IP => 'X_FORWARDED_FOR', - Illuminate\Http\Request::HEADER_CLIENT_HOST => 'X_FORWARDED_HOST', - Illuminate\Http\Request::HEADER_CLIENT_PROTO => 'X_FORWARDED_PROTO', - Illuminate\Http\Request::HEADER_CLIENT_PORT => 'X_FORWARDED_PORT', + \Illuminate\Http\Request::HEADER_CLIENT_IP => 'X_FORWARDED_FOR', + \Illuminate\Http\Request::HEADER_CLIENT_HOST => 'X_FORWARDED_HOST', + \Illuminate\Http\Request::HEADER_CLIENT_PROTO => 'X_FORWARDED_PROTO', + \Illuminate\Http\Request::HEADER_CLIENT_PORT => 'X_FORWARDED_PORT', ] ]; ``` @@ -233,6 +251,27 @@ return [ Using `*` will tell Laravel to trust all IP addresses as a proxy. +However, if you are in the situation where, say, you have a Content Distribution Network (like Amazon CloudFront) that passes to load balancer (like Amazon ELB) +then you may end up with a chain of unknown proxies forwarding from one to another. In that case, '*' above would only match +the final proxy (the load balancer in this case) which means that calling `$request->getClientIp()` would return the IP address +of the next proxy in line (in this case one of the Content Distribution Network ips) rather than the original client IP. +To always get the original client IP, you need to trust all the proxies in the route to your request. You can do this by: + +**In that case, you can set the 'proxies' variable to '**':** + +```php + '**', + +]; +``` + +Which will trust every single IP address. + + #### Changing X-Forwarded-* Header Names By default, the underlying Symfony `Request` class expects the following header names to be sent from a proxy: diff --git a/src/TrustProxies.php b/src/TrustProxies.php index 5705e4f..1c8f546 100644 --- a/src/TrustProxies.php +++ b/src/TrustProxies.php @@ -36,52 +36,68 @@ public function __construct(Repository $config) */ public function handle($request, Closure $next) { - // Set trusted header names - foreach ($this->getTrustedHeaders() as $headerKey => $headerName) { - $request->setTrustedHeaderName($headerKey, $headerName); - } - - $request->setTrustedProxies($this->getTrustedProxies($request->getClientIps())); - + $this->setTrustedProxyHeaderNames($request); + $this->setTrustedProxyIpAddresses($request); return $next($request); } /** - * Return an array of trusted proxy IP addresses. + * Sets the trusted proxies on the request to the value of trustedproxy.proxies * - * @param array $clientIpAddresses Array of client IP addresses retrieved - * *prior* to setting trusted proxy - * - * @return array + * @param \Illuminate\Http\Request $request */ - protected function getTrustedProxies(array $clientIpAddresses = []) + protected function setTrustedProxyIpAddresses($request) { - $trustedProxies = $this->config->get('trustedproxy.proxies'); + $trustedIps = $this->config->get('trustedproxy.proxies'); + + // We only trust specific IP addresses + if(is_array($trustedIps)) { + return $this->setTrustedProxyIpAddressesToSpecificIps($request, $trustedIps); + } - // To trust all proxies, we set trusted proxies to all IP addresses. - if ($trustedProxies === '*') { - return $clientIpAddresses; + // We trust any IP address that calls us, but not proxies further + // up the forwarding chain. + if ($trustedIps === '*') { + return $this->setTrustedProxyIpAddressesToTheCallingIp($request); } + + // We trust all proxies. Those that call us, and those that are + // further up the calling chain (e.g., where the X-FORWARDED-FOR + // header has multiple IP addresses listed); + if ($trustedIps === '**') { + return $this->setTrustedProxyIpAddressesToAllIps($request); + } + } - return (array) $trustedProxies; + private function setTrustedProxyIpAddressesToSpecificIps($request, $trustedIps) + { + $request->setTrustedProxies((array) $trustedIps); } + private function setTrustedProxyIpAddressesToTheCallingIp($request) { + $request->setTrustedProxies($request->getClientIps()); + } + + private function setTrustedProxyIpAddressesToAllIps($request) + { + // 0.0.0.0/0 is the CIDR for all ipv4 addresses + // 2000:0:0:0:0:0:0:0/3 is the CIDR for all ipv6 addresses currently + // allocated http://www.iana.org/assignments/ipv6-unicast-address-assignments/ipv6-unicast-address-assignments.xhtml + $request->setTrustedProxies(['0.0.0.0/0', '2000:0:0:0:0:0:0:0/3']); + } + /** - * Get trusted header names. + * Set the trusted header names based on teh content of trustedproxy.headers * - * @return array + * @param \Illuminate\Http\Request $request */ - protected function getTrustedHeaders() + protected function setTrustedProxyHeaderNames($request) { $trustedHeaderNames = $this->config->get('trustedproxy.headers'); + if(!is_array($trustedHeaderNames)) { return; } // Leave the defaults - /* - * In case the user does not pass an array of header names we - * will default to an empty array. This will force defaults from - * class \Symfony\Component\HttpFoundation\Request::$trustedHeaders - */ - $trustedHeaderNames = is_array($trustedHeaderNames) ? $trustedHeaderNames : []; - - return $trustedHeaderNames; + foreach ($trustedHeaderNames as $headerKey => $headerName) { + $request->setTrustedHeaderName($headerKey, $headerName); + } } } diff --git a/tests/TrustedProxyTest.php b/tests/TrustedProxyTest.php index 5ae66a7..61bb913 100644 --- a/tests/TrustedProxyTest.php +++ b/tests/TrustedProxyTest.php @@ -41,6 +41,22 @@ public function test_does_trust_trusted_proxy() $this->assertEquals(443, $req->getPort(), 'Assert trusted proxy x-forwarded-port header used'); } + /** + * Test the next most typical usage of TrustedProxies: + * Trusted X-Forwarded-For header, wilcard for TrustedProxies + */ + public function test_trusted_proxy_sets_trusted_proxies_with_wildcard() + { + $trustedProxy = $this->createTrustedProxy([], '*'); + $request = $this->createProxiedRequest(); + + $trustedProxy->handle($request, function ($request) { + $this->assertEquals('173.174.200.38', $request->getClientIp(), 'Assert trusted proxy x-forwarded-for header used with wildcard proxy setting'); + }); + } + + + /** * Test the most typical usage of TrustProxies: * Trusted X-Forwarded-For header @@ -102,6 +118,54 @@ public function test_get_client_ip_with_muliple_ip_addresses_some_of_which_are_t } } + /** + * Test X-Forwarded-For header with multiple IP addresses, with * wildcard trusting of all proxies + */ + public function test_get_client_ip_with_muliple_ip_addresses_all_proxies_are_trusted() + { + $trustedProxy = $this->createTrustedProxy([], '*'); + + $forwardedFor = [ + '192.0.2.2', + '192.0.2.199, 192.0.2.2', + '192.0.2.199,192.0.2.2', + '99.99.99.99,192.0.2.199,192.0.2.2', + ]; + + foreach($forwardedFor as $forwardedForHeader) { + $request = $this->createProxiedRequest(['HTTP_X_FORWARDED_FOR' => $forwardedForHeader]); + + $trustedProxy->handle($request, function ($request) use ($forwardedForHeader) { + $this->assertEquals('192.0.2.2', $request->getClientIp(), 'Assert sets the '.$forwardedForHeader); + }); + } + } + + /** + * Test X-Forwarded-For header with multiple IP addresses, with ** wildcard trusting of all proxies in the chain + */ + public function test_get_client_ip_with_muliple_ip_addresses_all_proxies_and_all_forwarding_proxies_are_trusted() + { + $trustedProxy = $this->createTrustedProxy([], '**'); + + $forwardedFor = [ + '192.0.2.2', + '192.0.2.2, 192.0.2.199', + '192.0.2.2, 99.99.99.99, 192.0.2.199', + '192.0.2.2, 2001:0db8:0a0b:12f0:0000:0000:0000:0001, 192.0.2.199', + '192.0.2.2, 2c01:0db8:0a0b:12f0:0000:0000:0000:0001, 192.0.2.199', + '192.0.2.2,192.0.2.199', + ]; + + foreach($forwardedFor as $forwardedForHeader) { + $request = $this->createProxiedRequest(['HTTP_X_FORWARDED_FOR' => $forwardedForHeader]); + + $trustedProxy->handle($request, function ($request) use ($forwardedForHeader) { + $this->assertEquals('192.0.2.2', $request->getClientIp(), 'Assert sets the '.$forwardedForHeader); + }); + } + } + /** * Test renaming the X-Forwarded-For header. */