Merge branch 'security-2736-prometheus-ssrf' into 'master'
[master] Do not follow redirects in prometheus service See merge request gitlab/gitlabhq!2617
This commit is contained in:
parent
4bc6f2e3ac
commit
94ab2d5fc8
4 changed files with 25 additions and 3 deletions
|
@ -71,7 +71,7 @@ class PrometheusService < MonitoringService
|
||||||
end
|
end
|
||||||
|
|
||||||
def prometheus_client
|
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
|
end
|
||||||
|
|
||||||
def prometheus_available?
|
def prometheus_available?
|
||||||
|
|
5
changelogs/unreleased/security-2736-prometheus-ssrf.yml
Normal file
5
changelogs/unreleased/security-2736-prometheus-ssrf.yml
Normal file
|
@ -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
|
|
@ -13,6 +13,23 @@ describe PrometheusService, :use_clean_rails_memory_store_caching do
|
||||||
it { is_expected.to belong_to :project }
|
it { is_expected.to belong_to :project }
|
||||||
end
|
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
|
describe 'Validations' do
|
||||||
context 'when manual_configuration is enabled' do
|
context 'when manual_configuration is enabled' do
|
||||||
before do
|
before do
|
||||||
|
|
|
@ -49,11 +49,11 @@ module PrometheusHelpers
|
||||||
"https://prometheus.example.com/api/v1/series?#{query}"
|
"https://prometheus.example.com/api/v1/series?#{query}"
|
||||||
end
|
end
|
||||||
|
|
||||||
def stub_prometheus_request(url, body: {}, status: 200)
|
def stub_prometheus_request(url, body: {}, status: 200, headers: {})
|
||||||
WebMock.stub_request(:get, url)
|
WebMock.stub_request(:get, url)
|
||||||
.to_return({
|
.to_return({
|
||||||
status: status,
|
status: status,
|
||||||
headers: { 'Content-Type' => 'application/json' },
|
headers: { 'Content-Type' => 'application/json' }.merge(headers),
|
||||||
body: body.to_json
|
body: body.to_json
|
||||||
})
|
})
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in a new issue