From 6ba6c84809b9458ab0c8808570dfcfe6e166e0d2 Mon Sep 17 00:00:00 2001 From: Clemens Beck Date: Tue, 8 Jul 2025 13:43:33 +0200 Subject: [PATCH 1/4] Handle reverse proxies with mapped IP addresses * Trust all private IP ranges by default. * Also trust proxies if they show with their IPv6 mapped IP address (`::ffff:1.1.1.1` instead of `1.1.1.1`). Relates https://gitlab.com/gitlab-com/gl-infra/delivery/-/issues/21318 Relates https://gitlab.com/gitlab-org/charts/gitlab/-/issues/2778 Relates https://gitlab.com/gitlab-org/charts/gitlab/-/issues/6084 --- config/initializers/trusted_proxies.rb | 19 ++++++--- spec/initializers/trusted_proxies_spec.rb | 42 +++++++++++++------ .../middleware/basic_health_check_spec.rb | 5 ++- 3 files changed, 46 insertions(+), 20 deletions(-) diff --git a/config/initializers/trusted_proxies.rb b/config/initializers/trusted_proxies.rb index f03561b561752d..7c69040f9c8f10 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.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.compact.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/spec/initializers/trusted_proxies_spec.rb b/spec/initializers/trusted_proxies_spec.rb index 63c96ce17d18a6..d499f68ed32ec0 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/middleware/basic_health_check_spec.rb b/spec/lib/gitlab/middleware/basic_health_check_spec.rb index 40e25e5e272462..451370498c04c1 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) -- GitLab From 56599b01900d2f948b1f3b08e9f3e5e9b3e85d60 Mon Sep 17 00:00:00 2001 From: Clemens Beck Date: Mon, 14 Jul 2025 11:38:13 +0200 Subject: [PATCH 2/4] Group IP restrictions: Handle mapped addresses --- ee/app/models/ip_restriction.rb | 11 ++++++++--- ee/spec/models/ip_restriction_spec.rb | 6 ++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/ee/app/models/ip_restriction.rb b/ee/app/models/ip_restriction.rb index ea3a3d124cfc5b..2f0a701f14e078 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 0b37c1624c00e1..168d7420144d82 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' } -- GitLab From 85ab7b4f9ca28901a368e6539e41a65f85200015 Mon Sep 17 00:00:00 2001 From: Clemens Beck Date: Mon, 14 Jul 2025 13:13:06 +0200 Subject: [PATCH 3/4] Internal IP Rate limiter: Handle mapped IPs --- config/initializers/trusted_proxies.rb | 4 ++-- lib/gitlab/auth/ip_rate_limiter.rb | 8 ++++++-- spec/lib/gitlab/auth/ip_rate_limiter_spec.rb | 12 ++++++++++++ 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/config/initializers/trusted_proxies.rb b/config/initializers/trusted_proxies.rb index 7c69040f9c8f10..12015c539dd95e 100644 --- a/config/initializers/trusted_proxies.rb +++ b/config/initializers/trusted_proxies.rb @@ -28,9 +28,9 @@ def trusted_proxy?(ip) trusted_proxies += [IPAddr.new('127.0.0.1'), IPAddr.new('::1')] # Trust all proxies in their IPv6 mapped format. -trusted_proxies += trusted_proxies.select(&:ipv4?).map(&:ipv4_mapped) +trusted_proxies += trusted_proxies.compact.select(&:ipv4?).map(&:ipv4_mapped) -Rails.application.config.action_dispatch.trusted_proxies = trusted_proxies.compact.uniq +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/lib/gitlab/auth/ip_rate_limiter.rb b/lib/gitlab/auth/ip_rate_limiter.rb index 0d50420b9f54f9..a941b87ff30928 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/spec/lib/gitlab/auth/ip_rate_limiter_spec.rb b/spec/lib/gitlab/auth/ip_rate_limiter_spec.rb index c5703cf5c243ed..604830f9d007c5 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 -- GitLab From d58af42b79c83e89ea7a6cd4dfd9a3c5ce95dd75 Mon Sep 17 00:00:00 2001 From: Clemens Beck Date: Tue, 15 Jul 2025 08:40:09 +0200 Subject: [PATCH 4/4] QA orchestrator: Enable gitlab sshd Use GitLab sshd with proxy protocol to ensure SSH process sees same IP as rails backend. --- .../gitlab/orchestrator/lib/deployment/default_values.rb | 8 +++++++- .../gitlab/orchestrator/deployment/default_values_spec.rb | 8 +++++++- 2 files changed, 14 insertions(+), 2 deletions(-) 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 0c788432410bd8..1265cb8d3753a5 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 577889a41cc536..3f65606b70f9c1 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 }, -- GitLab