summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorrpereira2 <rpereira@gitlab.com>2018-11-14 19:50:34 +0530
committerrpereira2 <rpereira@gitlab.com>2018-11-14 20:25:48 +0530
commit10a9060d214a9b24576e0eecaac5e3e73c8ba3cf (patch)
tree43c9b6c7cb32b2425221af8ee08697e8a4d99c1c
parent1c315f4c26ee0d682dd232c077a1bf38a7634b70 (diff)
downloadgitlab-ce-10a9060d214a9b24576e0eecaac5e3e73c8ba3cf.tar.gz
No redirects in prometheus service
Do not allow redirects in the prometheus service to prevent SSRFs.
-rw-r--r--app/models/project_services/prometheus_service.rb2
-rw-r--r--changelogs/unreleased/security-2736-prometheus-ssrf.yml5
-rw-r--r--spec/models/project_services/prometheus_service_spec.rb17
-rw-r--r--spec/support/helpers/prometheus_helpers.rb4
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