diff options
author | Steve Azzopardi <sazzopardi@gitlab.com> | 2018-11-23 14:42:45 +0000 |
---|---|---|
committer | Steve Azzopardi <sazzopardi@gitlab.com> | 2018-11-23 14:42:45 +0000 |
commit | b7e66318916b37bd5dba7e34f35afdd4d9e7b14e (patch) | |
tree | 19b4b08cd7a139425635ee50d76414566ae57e08 | |
parent | bb2d62f8852cd94a80dc11306fe73670849e8120 (diff) | |
parent | d61dc4982dacf79193bd8459df5ad0b130523981 (diff) | |
download | gitlab-ce-b7e66318916b37bd5dba7e34f35afdd4d9e7b14e.tar.gz |
Merge branch 'security-2736-prometheus-ssrf-11-4' into 'security-11-4'
[11.4] Do not follow redirects in prometheus service
See merge request gitlab/gitlabhq!2624
4 files changed, 25 insertions, 3 deletions
diff --git a/app/models/project_services/prometheus_service.rb b/app/models/project_services/prometheus_service.rb index 509e5b6089b..620efd3768c 100644 --- a/app/models/project_services/prometheus_service.rb +++ b/app/models/project_services/prometheus_service.rb @@ -72,7 +72,7 @@ class PrometheusService < MonitoringService end def prometheus_client - RestClient::Resource.new(api_url) if api_url && manual_configuration? && active? + RestClient::Resource.new(api_url, max_redirects: 0) if api_url && manual_configuration? && active? end def prometheus_installed? diff --git a/changelogs/unreleased/security-2736-prometheus-ssrf.yml b/changelogs/unreleased/security-2736-prometheus-ssrf.yml new file mode 100644 index 00000000000..9d0dda8a75f --- /dev/null +++ b/changelogs/unreleased/security-2736-prometheus-ssrf.yml @@ -0,0 +1,5 @@ +--- +title: Do not follow redirects in Prometheus service when making http requests to the configured api url +merge_request: +author: +type: security diff --git a/spec/models/project_services/prometheus_service_spec.rb b/spec/models/project_services/prometheus_service_spec.rb index 7afb1b4a8e3..ac92da6e1b1 100644 --- a/spec/models/project_services/prometheus_service_spec.rb +++ b/spec/models/project_services/prometheus_service_spec.rb @@ -11,6 +11,23 @@ describe PrometheusService, :use_clean_rails_memory_store_caching do it { is_expected.to belong_to :project } end + context 'redirects' do + it 'does not follow redirects' do + redirect_to = 'https://redirected.example.com' + redirect_req_stub = stub_prometheus_request(prometheus_query_url('1'), status: 302, headers: { location: redirect_to }) + redirected_req_stub = stub_prometheus_request(redirect_to, body: { 'status': 'success' }) + + result = service.test + + # result = { success: false, result: error } + expect(result[:success]).to be_falsy + expect(result[:result]).to be_instance_of(Gitlab::PrometheusClient::Error) + + expect(redirect_req_stub).to have_been_requested + expect(redirected_req_stub).not_to have_been_requested + end + end + describe 'Validations' do context 'when manual_configuration is enabled' do before do diff --git a/spec/support/helpers/prometheus_helpers.rb b/spec/support/helpers/prometheus_helpers.rb index 4212be2cc88..ce1f9fce10d 100644 --- a/spec/support/helpers/prometheus_helpers.rb +++ b/spec/support/helpers/prometheus_helpers.rb @@ -49,11 +49,11 @@ module PrometheusHelpers "https://prometheus.example.com/api/v1/series?#{query}" end - def stub_prometheus_request(url, body: {}, status: 200) + def stub_prometheus_request(url, body: {}, status: 200, headers: {}) WebMock.stub_request(:get, url) .to_return({ status: status, - headers: { 'Content-Type' => 'application/json' }, + headers: { 'Content-Type' => 'application/json' }.merge(headers), body: body.to_json }) end |