Skip to content

Commit

Permalink
Merge pull request #38150 from kbrock/all_trusted_ips
Browse files Browse the repository at this point in the history
When all IPs are trusted, use the furthest away
  • Loading branch information
rafaelfranca committed Jan 3, 2020
2 parents d8b86e4 + b17aaae commit 7b29bc2
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 8 deletions.
7 changes: 7 additions & 0 deletions actionpack/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
* `ActionDispatch::Request.remote_ip` has ip address even when all sites are trusted.

Before, if all `X-Forwarded-For` sites were trusted, the `remote_ip` would default to `127.0.0.1`.
Now, the furthest proxy site is used. e.g.: It now gives an ip address when using curl from the load balancer.

*Keenan Brock*

* Fix possible information leak / session hijacking vulnerability.

The `ActionDispatch::Session::MemcacheStore` is still vulnerable given it requires the
Expand Down
7 changes: 4 additions & 3 deletions actionpack/lib/action_dispatch/middleware/remote_ip.rb
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,11 @@ def calculate_ip
# - X-Forwarded-For will be a list of IPs, one per proxy, or blank
# - Client-Ip is propagated from the outermost proxy, or is blank
# - REMOTE_ADDR will be the IP that made the request to Rack
ips = [forwarded_ips, client_ips, remote_addr].flatten.compact
ips = [forwarded_ips, client_ips].flatten.compact

# If every single IP option is in the trusted list, just return REMOTE_ADDR
filter_proxies(ips).first || remote_addr
# If every single IP option is in the trusted list, return the IP
# that's furthest away
filter_proxies(ips + [remote_addr]).first || ips.last || remote_addr
end

# Memoizes the value returned by #calculate_ip and returns it for
Expand Down
17 changes: 12 additions & 5 deletions actionpack/test/dispatch/request_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ class RequestIP < BaseRequestTest
"HTTP_X_FORWARDED_FOR" => "3.4.5.6"
assert_equal "3.4.5.6", request.remote_ip

request = stub_request "REMOTE_ADDR" => "127.0.0.1",
"HTTP_X_FORWARDED_FOR" => "172.31.4.4, 10.0.0.1"
assert_equal "172.31.4.4", request.remote_ip

request = stub_request "HTTP_X_FORWARDED_FOR" => "3.4.5.6,unknown"
assert_equal "3.4.5.6", request.remote_ip

Expand All @@ -89,14 +93,17 @@ class RequestIP < BaseRequestTest
request = stub_request "HTTP_X_FORWARDED_FOR" => "3.4.5.6,10.0.0.1"
assert_equal "3.4.5.6", request.remote_ip

request = stub_request "HTTP_X_FORWARDED_FOR" => "172.31.4.4, 10.0.0.1"
assert_equal "172.31.4.4", request.remote_ip

request = stub_request "HTTP_X_FORWARDED_FOR" => "3.4.5.6, 10.0.0.1, 10.0.0.1"
assert_equal "3.4.5.6", request.remote_ip

request = stub_request "HTTP_X_FORWARDED_FOR" => "3.4.5.6,127.0.0.1"
assert_equal "3.4.5.6", request.remote_ip

request = stub_request "HTTP_X_FORWARDED_FOR" => "unknown,192.168.0.1"
assert_nil request.remote_ip
assert_equal "192.168.0.1", request.remote_ip

request = stub_request "HTTP_X_FORWARDED_FOR" => "9.9.9.9, 3.4.5.6, 172.31.4.4, 10.0.0.1"
assert_equal "3.4.5.6", request.remote_ip
Expand Down Expand Up @@ -161,7 +168,7 @@ class RequestIP < BaseRequestTest
assert_equal "fe80:0000:0000:0000:0202:b3ff:fe1e:8329", request.remote_ip

request = stub_request "HTTP_X_FORWARDED_FOR" => "unknown,::1"
assert_nil request.remote_ip
assert_equal "::1", request.remote_ip

request = stub_request "HTTP_X_FORWARDED_FOR" => "2001:0db8:85a3:0000:0000:8a2e:0370:7334, fe80:0000:0000:0000:0202:b3ff:fe1e:8329, ::1, fc00::, fc01::, fdff"
assert_equal "fe80:0000:0000:0000:0202:b3ff:fe1e:8329", request.remote_ip
Expand Down Expand Up @@ -207,7 +214,7 @@ class RequestIP < BaseRequestTest
assert_equal "3.4.5.6", request.remote_ip

request = stub_request "HTTP_X_FORWARDED_FOR" => "67.205.106.73,unknown"
assert_nil request.remote_ip
assert_equal "67.205.106.73", request.remote_ip # change

request = stub_request "HTTP_X_FORWARDED_FOR" => "9.9.9.9, 3.4.5.6, 10.0.0.1, 67.205.106.73"
assert_equal "3.4.5.6", request.remote_ip
Expand All @@ -226,10 +233,10 @@ class RequestIP < BaseRequestTest

request = stub_request "REMOTE_ADDR" => "fe80:0000:0000:0000:0202:b3ff:fe1e:8329,::1",
"HTTP_X_FORWARDED_FOR" => "fe80:0000:0000:0000:0202:b3ff:fe1e:8329"
assert_equal "::1", request.remote_ip
assert_equal "fe80:0000:0000:0000:0202:b3ff:fe1e:8329", request.remote_ip

request = stub_request "HTTP_X_FORWARDED_FOR" => "unknown,fe80:0000:0000:0000:0202:b3ff:fe1e:8329"
assert_nil request.remote_ip
assert_equal "fe80:0000:0000:0000:0202:b3ff:fe1e:8329", request.remote_ip

request = stub_request "HTTP_X_FORWARDED_FOR" => "fe80:0000:0000:0000:0202:b3ff:fe1e:8329,2001:0db8:85a3:0000:0000:8a2e:0370:7334"
assert_equal "2001:0db8:85a3:0000:0000:8a2e:0370:7334", request.remote_ip
Expand Down

0 comments on commit 7b29bc2

Please sign in to comment.