Commit 94c62315 authored by Peter Leitzen's avatar Peter Leitzen

Prevent SSRF requests for Prometheus when secured by Google IAP

Strip the `token_credential_uri` key from user-provided JSON.
parent be054b4a
...@@ -183,7 +183,17 @@ class PrometheusService < MonitoringService ...@@ -183,7 +183,17 @@ class PrometheusService < MonitoringService
manual_configuration? && google_iap_audience_client_id.present? && google_iap_service_account_json.present? manual_configuration? && google_iap_audience_client_id.present? && google_iap_service_account_json.present?
end end
def clean_google_iap_service_account
return unless google_iap_service_account_json
google_iap_service_account_json
.then { |json| Gitlab::Json.parse(json) }
.except('token_credential_uri')
end
def iap_client def iap_client
@iap_client ||= Google::Auth::Credentials.new(Gitlab::Json.parse(google_iap_service_account_json), target_audience: google_iap_audience_client_id).client @iap_client ||= Google::Auth::Credentials
.new(clean_google_iap_service_account, target_audience: google_iap_audience_client_id)
.client
end end
end end
---
title: Prevent Server-side Request Forgery for Prometheus when secured by Google IAP
merge_request:
author:
type: security
...@@ -198,6 +198,8 @@ service account can be found at Google's documentation for ...@@ -198,6 +198,8 @@ service account can be found at Google's documentation for
Prometheus OAuth Client secured with Google IAP. Prometheus OAuth Client secured with Google IAP.
1. (Optional) In **Google IAP Service Account JSON**, provide the contents of the 1. (Optional) In **Google IAP Service Account JSON**, provide the contents of the
Service Account credentials file that is authorized to access the Prometheus resource. Service Account credentials file that is authorized to access the Prometheus resource.
The JSON key `token_credential_uri` is discarded to prevent
[Server-side Request Forgery (SSRF)](https://www.hackerone.com/blog-How-To-Server-Side-Request-Forgery-SSRF).
1. Click **Save changes**. 1. Click **Save changes**.
![Configure Prometheus Service](img/prometheus_manual_configuration_v13_2.png) ![Configure Prometheus Service](img/prometheus_manual_configuration_v13_2.png)
......
...@@ -2,11 +2,13 @@ ...@@ -2,11 +2,13 @@
require 'spec_helper' require 'spec_helper'
require 'googleauth'
RSpec.describe PrometheusService, :use_clean_rails_memory_store_caching, :snowplow do RSpec.describe PrometheusService, :use_clean_rails_memory_store_caching, :snowplow do
include PrometheusHelpers include PrometheusHelpers
include ReactiveCachingHelpers include ReactiveCachingHelpers
let(:project) { create(:prometheus_project) } let_it_be_with_reload(:project) { create(:prometheus_project) }
let(:service) { project.prometheus_service } let(:service) { project.prometheus_service }
describe "Associations" do describe "Associations" do
...@@ -256,19 +258,66 @@ RSpec.describe PrometheusService, :use_clean_rails_memory_store_caching, :snowpl ...@@ -256,19 +258,66 @@ RSpec.describe PrometheusService, :use_clean_rails_memory_store_caching, :snowpl
context 'behind IAP' do context 'behind IAP' do
let(:manual_configuration) { true } let(:manual_configuration) { true }
before do let(:google_iap_service_account) do
# dummy private key generated only for this test to pass openssl validation {
service.google_iap_service_account_json = '{"type":"service_account","private_key":"-----BEGIN RSA PRIVATE KEY-----\nMIIBOAIBAAJAU85LgUY5o6j6j/07GMLCNUcWJOBA1buZnNgKELayA6mSsHrIv31J\nY8kS+9WzGPQninea7DcM4hHA7smMgQD1BwIDAQABAkAqKxMy6PL3tn7dFL43p0ex\nJyOtSmlVIiAZG1t1LXhE/uoLpYi5DnbYqGgu0oih+7nzLY/dXpNpXUmiRMOUEKmB\nAiEAoTi2rBXbrLSi2C+H7M/nTOjMQQDuZ8Wr4uWpKcjYJTMCIQCFEskL565oFl/7\nRRQVH+cARrAsAAoJSbrOBAvYZ0PI3QIgIEFwis10vgEF86rOzxppdIG/G+JL0IdD\n9IluZuXAGPECIGUo7qSaLr75o2VEEgwtAFH5aptIPFjrL5LFCKwtdB4RAiAYZgFV\nHCMmaooAw/eELuMoMWNYmujZ7VaAnOewGDW0uw==\n-----END RSA PRIVATE KEY-----\n"}' type: "service_account",
service.google_iap_audience_client_id = "IAP_CLIENT_ID.apps.googleusercontent.com" # dummy private key generated only for this test to pass openssl validation
private_key: <<~KEY
-----BEGIN RSA PRIVATE KEY-----
MIIBOAIBAAJAU85LgUY5o6j6j/07GMLCNUcWJOBA1buZnNgKELayA6mSsHrIv31J
Y8kS+9WzGPQninea7DcM4hHA7smMgQD1BwIDAQABAkAqKxMy6PL3tn7dFL43p0ex
JyOtSmlVIiAZG1t1LXhE/uoLpYi5DnbYqGgu0oih+7nzLY/dXpNpXUmiRMOUEKmB
AiEAoTi2rBXbrLSi2C+H7M/nTOjMQQDuZ8Wr4uWpKcjYJTMCIQCFEskL565oFl/7
RRQVH+cARrAsAAoJSbrOBAvYZ0PI3QIgIEFwis10vgEF86rOzxppdIG/G+JL0IdD
9IluZuXAGPECIGUo7qSaLr75o2VEEgwtAFH5aptIPFjrL5LFCKwtdB4RAiAYZgFV
HCMmaooAw/eELuMoMWNYmujZ7VaAnOewGDW0uw==
-----END RSA PRIVATE KEY-----
KEY
}
end
def stub_iap_request
service.google_iap_service_account_json = Gitlab::Json.generate(google_iap_service_account)
service.google_iap_audience_client_id = 'IAP_CLIENT_ID.apps.googleusercontent.com'
stub_request(:post, "https://oauth2.googleapis.com/token").to_return(status: 200, body: '{"id_token": "FOO"}', headers: { 'Content-Type': 'application/json; charset=UTF-8' }) stub_request(:post, 'https://oauth2.googleapis.com/token')
.to_return(
status: 200,
body: '{"id_token": "FOO"}',
headers: { 'Content-Type': 'application/json; charset=UTF-8' }
)
end end
it 'includes the authorization header' do it 'includes the authorization header' do
stub_iap_request
expect(service.prometheus_client).not_to be_nil expect(service.prometheus_client).not_to be_nil
expect(service.prometheus_client.send(:options)).to have_key(:headers) expect(service.prometheus_client.send(:options)).to have_key(:headers)
expect(service.prometheus_client.send(:options)[:headers]).to eq(authorization: "Bearer FOO") expect(service.prometheus_client.send(:options)[:headers]).to eq(authorization: "Bearer FOO")
end end
context 'when passed with token_credential_uri', issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/284819' do
let(:malicious_host) { 'http://example.com' }
where(:param_name) do
[
:token_credential_uri,
:tokencredentialuri,
:Token_credential_uri,
:tokenCredentialUri
]
end
with_them do
it 'does not make any unexpected HTTP requests' do
google_iap_service_account[param_name] = malicious_host
stub_iap_request
stub_request(:any, malicious_host).to_raise('Making additional HTTP requests is forbidden!')
expect(service.prometheus_client).not_to be_nil
end
end
end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment