diff --git a/config/initializers/trusted_proxies.rb b/config/initializers/trusted_proxies.rb index f03561b561752dbf0f6ca0ff9358b0d502f37834..12015c539dd95e2413c5b278fc03d94620dc7fcd 100644 --- a/config/initializers/trusted_proxies.rb +++ b/config/initializers/trusted_proxies.rb @@ -8,20 +8,29 @@ module Rack class Request def trusted_proxy?(ip) - Rails.application.config.action_dispatch.trusted_proxies.any? { |proxy| proxy === ip } + Rails.application.config.action_dispatch.trusted_proxies.any? { |proxy| proxy.include?(ip) } rescue IPAddr::InvalidAddressError false end end end -gitlab_trusted_proxies = Array(Gitlab.config.gitlab.trusted_proxies).map do |proxy| +# Trust custom proxies from config. +trusted_proxies = Array(Gitlab.config.gitlab.trusted_proxies).map do |proxy| IPAddr.new(proxy) rescue IPAddr::InvalidAddressError -end.compact +end + +# Default to private IPs if no proxies configured. +trusted_proxies = ActionDispatch::RemoteIp::TRUSTED_PROXIES if trusted_proxies.empty? + +# Always trust localhost. +trusted_proxies += [IPAddr.new('127.0.0.1'), IPAddr.new('::1')] + +# Trust all proxies in their IPv6 mapped format. +trusted_proxies += trusted_proxies.compact.select(&:ipv4?).map(&:ipv4_mapped) -Rails.application.config.action_dispatch.trusted_proxies = ( - ['127.0.0.1', '::1'] + gitlab_trusted_proxies) +Rails.application.config.action_dispatch.trusted_proxies = trusted_proxies.uniq # A monkey patch to make trusted proxies work with Rails 5.0. # Inspired by https://github.com/rails/rails/issues/5223#issuecomment-263778719 diff --git a/ee/app/models/ip_restriction.rb b/ee/app/models/ip_restriction.rb index ea3a3d124cfc5b859f75e0b6ed193b25d0352fdb..2f0a701f14e078e29356054c724a5e7396c751ec 100644 --- a/ee/app/models/ip_restriction.rb +++ b/ee/app/models/ip_restriction.rb @@ -12,9 +12,14 @@ class IpRestriction < ApplicationRecord validate :allow_root_group_only def allows_address?(address) - IPAddr.new(range).include?(address) - rescue *INVALID_SUBNET_ERRORS - false + begin + ranges = [IPAddr.new(range)] + rescue *INVALID_SUBNET_ERRORS + return false + end + ranges += ranges.select(&:ipv4?).map(&:ipv4_mapped) + + ranges.any? { |r| r.include?(address) } end private diff --git a/ee/spec/models/ip_restriction_spec.rb b/ee/spec/models/ip_restriction_spec.rb index 0b37c1624c00e189df11b36f191f33d72db2bdf7..168d7420144d82c65bd68df494c14bcceb2b9e21 100644 --- a/ee/spec/models/ip_restriction_spec.rb +++ b/ee/spec/models/ip_restriction_spec.rb @@ -64,6 +64,12 @@ it { is_expected.to be_truthy } end + context 'mapped address is within the range' do + let(:address) { '::ffff:192.168.0.1' } + + it { is_expected.to be_truthy } + end + context 'address is outside the range' do let(:range) { '10.0.0.0/8' } diff --git a/lib/gitlab/auth/ip_rate_limiter.rb b/lib/gitlab/auth/ip_rate_limiter.rb index 0d50420b9f54f94bddecac9edaa800d641e94bee..a941b87ff30928ae86d5614d582137e478ee00c9 100644 --- a/lib/gitlab/auth/ip_rate_limiter.rb +++ b/lib/gitlab/auth/ip_rate_limiter.rb @@ -53,10 +53,14 @@ def config def trusted_ips strong_memoize(:trusted_ips) do - config.ip_whitelist.map do |proxy| + trusted = config.ip_whitelist.map do |proxy| IPAddr.new(proxy) rescue IPAddr::InvalidAddressError - end.compact + end + + trusted += trusted.compact.select(&:ipv4?).map(&:ipv4_mapped) + + trusted.uniq end end end diff --git a/qa/gems/gitlab-orchestrator/lib/gitlab/orchestrator/lib/deployment/default_values.rb b/qa/gems/gitlab-orchestrator/lib/gitlab/orchestrator/lib/deployment/default_values.rb index 0c788432410bd8774a92bd3441a895b0a12b8497..1265cb8d3753a54c07d7bc5fd29db19c311444a6 100644 --- a/qa/gems/gitlab-orchestrator/lib/gitlab/orchestrator/lib/deployment/default_values.rb +++ b/qa/gems/gitlab-orchestrator/lib/gitlab/orchestrator/lib/deployment/default_values.rb @@ -43,7 +43,13 @@ def common_values(domain) ] } }, - gitlab: { "gitlab-exporter": { enabled: false } }, + gitlab: { + "gitlab-exporter": { enabled: false }, + "gitlab-shell": { + sshDaemon: 'gitlab-sshd', + config: { proxyProtocol: true } + } + }, redis: { metrics: { enabled: false } }, prometheus: { install: false }, certmanager: { install: false }, diff --git a/qa/gems/gitlab-orchestrator/spec/unit/gitlab/orchestrator/deployment/default_values_spec.rb b/qa/gems/gitlab-orchestrator/spec/unit/gitlab/orchestrator/deployment/default_values_spec.rb index 577889a41cc53654a4120739eeabc324359c9ca8..3f65606b70f9c14fcd8bd1201d5fd3b3d5552692 100644 --- a/qa/gems/gitlab-orchestrator/spec/unit/gitlab/orchestrator/deployment/default_values_spec.rb +++ b/qa/gems/gitlab-orchestrator/spec/unit/gitlab/orchestrator/deployment/default_values_spec.rb @@ -72,7 +72,13 @@ ] } }, - gitlab: { "gitlab-exporter": { enabled: false } }, + gitlab: { + "gitlab-exporter": { enabled: false }, + "gitlab-shell": { + sshDaemon: 'gitlab-sshd', + config: { proxyProtocol: true } + } + }, redis: { metrics: { enabled: false } }, prometheus: { install: false }, certmanager: { install: false }, diff --git a/spec/initializers/trusted_proxies_spec.rb b/spec/initializers/trusted_proxies_spec.rb index 63c96ce17d18a64b3e60342afdfeb347d2f1517f..d499f68ed32ec0030722f0d81234cc26d92e7a5a 100644 --- a/spec/initializers/trusted_proxies_spec.rb +++ b/spec/initializers/trusted_proxies_spec.rb @@ -15,21 +15,15 @@ end 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') + request = stub_request('HTTP_X_FORWARDED_FOR' => '1.1.1.1, 11.1.5.89, 127.0.0.1') + expect(request.remote_ip).to eq('11.1.5.89') + expect(request.ip).to eq('11.1.5.89') end it 'filters out bad values' do - request = stub_request('HTTP_X_FORWARDED_FOR' => '(null), 10.1.5.89') - expect(request.remote_ip).to eq('10.1.5.89') - expect(request.ip).to eq('10.1.5.89') - end - end - - context 'with private IP ranges added' do - before do - set_trusted_proxies(["10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16"]) + request = stub_request('HTTP_X_FORWARDED_FOR' => '(null), 11.1.5.89') + expect(request.remote_ip).to eq('11.1.5.89') + expect(request.ip).to eq('11.1.5.89') end it 'filters out private and local IPs' do @@ -37,6 +31,14 @@ expect(request.remote_ip).to eq('1.1.1.1') expect(request.ip).to eq('1.1.1.1') end + + it 'filters out private and local IPv6 addresses' do + request = stub_request( + 'HTTP_X_FORWARDED_FOR' => '::ffff:1.2.3.6, ::ffff:1.1.1.1, ::ffff:10.1.5.89, ::ffff:127.0.0.1' + ) + expect(request.remote_ip).to eq('::ffff:1.1.1.1') + expect(request.ip).to eq('::ffff:1.1.1.1') + end end context 'with proxy IP added' do @@ -50,11 +52,25 @@ expect(request.ip).to eq('1.1.1.1') end - it 'handles invalid ip addresses' do + it 'handles mapped IPv6 addresses' do + request = stub_request( + 'HTTP_X_FORWARDED_FOR' => '::ffff:1.2.3.6, ::ffff:1.1.1.1, ::ffff:60.98.25.47, ::ffff::127.0.0.1' + ) + expect(request.remote_ip).to eq('::ffff:1.1.1.1') + expect(request.ip).to eq('::ffff:1.1.1.1') + end + + it 'handles invalid IP addresses' do request = stub_request('HTTP_X_FORWARDED_FOR' => '(null), 1.1.1.1:12345, 1.1.1.1') expect(request.remote_ip).to eq('1.1.1.1') expect(request.ip).to eq('1.1.1.1') end + + it 'does not trust private IPs' do + request = stub_request('HTTP_X_FORWARDED_FOR' => '1.2.3.6, 10.1.1.1, 60.98.25.47, 127.0.0.1') + expect(request.remote_ip).to eq('10.1.1.1') + expect(request.ip).to eq('10.1.1.1') + end end def stub_request(headers = {}) diff --git a/spec/lib/gitlab/auth/ip_rate_limiter_spec.rb b/spec/lib/gitlab/auth/ip_rate_limiter_spec.rb index c5703cf5c243ed70313653a3a9ad5d4d7f1df6da..604830f9d007c51d4899a792a23dca597b0a8c5f 100644 --- a/spec/lib/gitlab/auth/ip_rate_limiter_spec.rb +++ b/spec/lib/gitlab/auth/ip_rate_limiter_spec.rb @@ -107,10 +107,22 @@ it { is_expected.to be_truthy } end + context 'when mapped ip is in the trusted list' do + let(:ip) { '::ffff:127.0.0.1' } + + it { is_expected.to be_truthy } + end + context 'when ip is not in the trusted list' do let(:ip) { '10.0.0.1' } it { is_expected.to be_falsey } end + + context 'when mapped ip is not in the trusted list' do + let(:ip) { '::ffff:10.0.0.1' } + + it { is_expected.to be_falsey } + end end end diff --git a/spec/lib/gitlab/middleware/basic_health_check_spec.rb b/spec/lib/gitlab/middleware/basic_health_check_spec.rb index 40e25e5e272462eddef0df1f7f7459111ce0bec1..451370498c04c18f18f87a09e74463adf4f7ab77 100644 --- a/spec/lib/gitlab/middleware/basic_health_check_spec.rb +++ b/spec/lib/gitlab/middleware/basic_health_check_spec.rb @@ -60,11 +60,12 @@ context 'IPv6 compatibility' do before do - env['REMOTE_ADDR'] = '::ffff:127.0.0.1' + env['HTTP_X_FORWARDED_FOR'] = "::ffff:8.8.8.8" + env['REMOTE_ADDR'] = '::ffff:8.8.8.8' end it 'returns 200 response when entry is a mapped IPv4' do - allow(Settings.monitoring).to receive(:ip_whitelist).and_return(['127.0.0.1']) + allow(Settings.monitoring).to receive(:ip_whitelist).and_return(['8.8.8.8']) expect(app).not_to receive(:call) response = middleware.call(env)