Merge branch 'rack-request-trusted-proxies' into 'master'
Make Rack::Request use our trusted proxies when filtering IP addresses ## What does this MR do? This allows us to control the trusted proxies while deployed in a private network. ## Are there points in the code the reviewer needs to double check? If we want to limit what is impacted, we can do this specifically for the rack_attack request object. ## Why was this MR needed? Normally Rack::Request will trust all private IPs as trusted proxies, which can cause problems if your users are connection on you network via private IP ranges. Normally in a rails app this is handled by action_dispatch request, but rack_attack is specifically using the Rack::Request object instead. ## What are the relevant issue numbers? Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/17550 ## Does this MR meet the acceptance criteria? - [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added - [ ] ~~[Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)~~ - [ ] ~~API support added~~ - Tests - [x] Added for this feature/bug - [x] All builds are passing - [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides) - [ ] Branch has no merge conflicts with `master` (if you do - rebase it please) - [ ] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits) \cc @stanhu See merge request !4958
This commit is contained in:
commit
8a245b80a5
|
@ -8,6 +8,7 @@ v 8.10.0 (unreleased)
|
|||
- Wrap code blocks on Activies and Todos page. !4783 (winniehell)
|
||||
- Align flash messages with left side of page content !4959 (winniehell)
|
||||
- Display last commit of deleted branch in push events !4699 (winniehell)
|
||||
- Apply the trusted_proxies config to the rack request object for use with rack_attack
|
||||
- Add Sidekiq queue duration to transaction metrics.
|
||||
- Let Workhorse serve format-patch diffs
|
||||
- Make images fit to the size of the viewport !4810
|
||||
|
|
|
@ -1,3 +1,16 @@
|
|||
# Override Rack::Request to make use of the same list of trusted_proxies
|
||||
# as the ActionDispatch::Request object. This is necessary for libraries
|
||||
# like rack_attack where they don't use ActionDispatch, and we want them
|
||||
# to block/throttle requests on private networks.
|
||||
# Rack Attack specific issue: https://github.com/kickstarter/rack-attack/issues/145
|
||||
module Rack
|
||||
class Request
|
||||
def trusted_proxy?(ip)
|
||||
Rails.application.config.action_dispatch.trusted_proxies.any? { |proxy| proxy === ip }
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
Rails.application.config.action_dispatch.trusted_proxies = (
|
||||
[ '127.0.0.1', '::1' ] + Array(Gitlab.config.gitlab.trusted_proxies)
|
||||
).map { |proxy| IPAddr.new(proxy) }
|
||||
|
|
|
@ -6,14 +6,16 @@ describe 'trusted_proxies', lib: true do
|
|||
set_trusted_proxies([])
|
||||
end
|
||||
|
||||
it 'preserves private IPs as remote_ip' do
|
||||
it 'preserves private IPs' do
|
||||
request = stub_request('HTTP_X_FORWARDED_FOR' => '10.1.5.89')
|
||||
expect(request.remote_ip).to eq('10.1.5.89')
|
||||
expect(request.ip).to eq('10.1.5.89')
|
||||
end
|
||||
|
||||
it 'filters out localhost from remote_ip' do
|
||||
it 'filters out localhost' do
|
||||
request = stub_request('HTTP_X_FORWARDED_FOR' => '1.1.1.1, 10.1.5.89, 127.0.0.1')
|
||||
expect(request.remote_ip).to eq('10.1.5.89')
|
||||
expect(request.ip).to eq('10.1.5.89')
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -22,9 +24,10 @@ describe 'trusted_proxies', lib: true do
|
|||
set_trusted_proxies([ "10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16" ])
|
||||
end
|
||||
|
||||
it 'filters out private and local IPs from remote_ip' do
|
||||
it 'filters out private and local IPs' do
|
||||
request = stub_request('HTTP_X_FORWARDED_FOR' => '1.2.3.6, 1.1.1.1, 10.1.5.89, 127.0.0.1')
|
||||
expect(request.remote_ip).to eq('1.1.1.1')
|
||||
expect(request.ip).to eq('1.1.1.1')
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -33,9 +36,10 @@ describe 'trusted_proxies', lib: true do
|
|||
set_trusted_proxies([ "60.98.25.47" ])
|
||||
end
|
||||
|
||||
it 'filters out proxy IP from remote_ip' do
|
||||
it 'filters out proxy IP' do
|
||||
request = stub_request('HTTP_X_FORWARDED_FOR' => '1.2.3.6, 1.1.1.1, 60.98.25.47, 127.0.0.1')
|
||||
expect(request.remote_ip).to eq('1.1.1.1')
|
||||
expect(request.ip).to eq('1.1.1.1')
|
||||
end
|
||||
end
|
||||
|
||||
|
|
Loading…
Reference in New Issue