Skip to content

Commit f9afd3e

Browse files
committed
Google says token_uri is always https://oauth2.googleapis.com/token
1 parent 4748b96 commit f9afd3e

3 files changed

Lines changed: 52 additions & 6 deletions

File tree

src/adapter/src/coord/sequencer/inner.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ use mz_repr::{
4343
CatalogItemId, Datum, Diff, GlobalId, RelationVersion, RelationVersionSelector, Row, RowArena,
4444
RowIterator, Timestamp,
4545
};
46+
use mz_secrets::SecretsReader;
4647
use mz_sql::ast::{
4748
AlterSourceAddSubsourceOption, CreateSinkOption, CreateSinkOptionName, CreateSourceOptionName,
4849
CreateSubsourceOption, CreateSubsourceOptionName, SqlServerConfigOption,
@@ -63,6 +64,7 @@ use mz_sql::plan::{
6364
StatementContext,
6465
};
6566
use mz_sql::pure::{PurifiedSourceExport, generate_subsource_statements};
67+
use mz_storage_types::connections::gcp::GcpServiceAccountKeyTokenUri;
6668
use mz_storage_types::sinks::StorageSinkDesc;
6769
// Import `plan` module, but only import select elements to avoid merge conflicts on use statements.
6870
use mz_sql::plan::{
@@ -662,6 +664,19 @@ impl Coordinator {
662664
}
663665
self.caching_secrets_reader.invalidate(connection_id);
664666
}
667+
ConnectionDetails::Gcp(gcp) => {
668+
// A service account key defines its own OAuth2 token URI.
669+
// We only want to send requests to the actual Google OAuth2 token API,
670+
// so we inspect the key as early as we can.
671+
if let Err(err) = self
672+
.caching_secrets_reader
673+
.read_string(gcp.credentials_json)
674+
.await
675+
.and_then(|json| GcpServiceAccountKeyTokenUri::validate_json(&json))
676+
{
677+
return ctx.retire(Err(err.into()));
678+
}
679+
}
665680
_ => (),
666681
};
667682

src/storage-types/src/connections/gcp.rs

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ use crate::controller::AlterError;
2525
/// endpoint without requiring any specific IAM grants on the service account.
2626
const VALIDATION_SCOPE: &str = "https://www.googleapis.com/auth/cloud-platform";
2727

28+
/// Modern GCP Service Account keys always use the same token URI.
29+
const OAUTH_TOKEN_URI: &str = "https://oauth2.googleapis.com/token";
30+
2831
/// GCP connection configuration.
2932
#[derive(Arbitrary, Clone, Debug, Eq, PartialEq, Serialize, Deserialize, Hash)]
3033
pub struct GcpConnection {
@@ -40,6 +43,28 @@ impl AlterCompatible for GcpConnection {
4043
}
4144
}
4245

46+
/// `gcp_auth` parses the Service Account Key JSON into a private type,
47+
/// so we have to write our own deserializer to check whether it's safe.
48+
#[derive(Deserialize)]
49+
pub struct GcpServiceAccountKeyTokenUri {
50+
token_uri: String,
51+
}
52+
53+
impl GcpServiceAccountKeyTokenUri {
54+
pub fn validate_json(json: &str) -> Result<(), anyhow::Error> {
55+
let k: GcpServiceAccountKeyTokenUri =
56+
serde_json::from_str(json).map_err(anyhow::Error::from)?;
57+
58+
if k.token_uri != OAUTH_TOKEN_URI {
59+
return Err(anyhow::Error::msg(format!(
60+
"token_uri must be {OAUTH_TOKEN_URI}."
61+
)));
62+
}
63+
64+
Ok(())
65+
}
66+
}
67+
4368
impl GcpConnection {
4469
/// Validates this connection by reading the service-account key out of the
4570
/// secrets store, parsing it, and exchanging it for an OAuth2 access token
@@ -55,8 +80,10 @@ impl GcpConnection {
5580
.read_string(self.credentials_json)
5681
.await
5782
.map_err(GcpConnectionValidationError::SecretRead)?;
58-
let service_account = CustomServiceAccount::from_json(&json)
83+
GcpServiceAccountKeyTokenUri::validate_json(&json)
5984
.map_err(GcpConnectionValidationError::ParseKey)?;
85+
let service_account = CustomServiceAccount::from_json(&json)
86+
.map_err(|e| GcpConnectionValidationError::ParseKey(anyhow::Error::from(e)))?;
6087
service_account
6188
.token(&[VALIDATION_SCOPE])
6289
.await
@@ -75,7 +102,7 @@ pub enum GcpConnectionValidationError {
75102
#[error("failed to read service-account key from secret store: {}", .0.display_with_causes())]
76103
SecretRead(#[source] anyhow::Error),
77104
#[error("failed to parse service-account key JSON: {}", .0.display_with_causes())]
78-
ParseKey(#[source] gcp_auth::Error),
105+
ParseKey(#[source] anyhow::Error),
79106
#[error("failed to obtain access token from Google: {}", .0.display_with_causes())]
80107
FetchToken(#[source] gcp_auth::Error),
81108
}

test/testdrive/connection-create-external.td

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -231,18 +231,18 @@ $ set gcp-ssrf-key="-----BEGIN PRIVATE KEY-----\\nMIIEvAIBADANBgkqhkiG9w0BAQEFAA
231231
# IP, and testdrive stops at the first failure.
232232
> CREATE SECRET gcp_ssrf_private AS '{"type":"service_account","project_id":"x","client_email":"a@b.com","private_key":"${gcp-ssrf-key}","token_uri":"http://127.0.0.1:1/token"}'
233233
! CREATE CONNECTION gcp_ssrf_private_conn TO GCP (SERVICE ACCOUNT KEY = SECRET gcp_ssrf_private) WITH (VALIDATE = true);
234-
contains:disallowed token_uri
234+
contains:token_uri must be https://oauth2.googleapis.com/token
235235

236236
# Correct host, but plaintext HTTP -- the scheme must be HTTPS.
237237
> CREATE SECRET gcp_ssrf_scheme AS '{"type":"service_account","project_id":"x","client_email":"a@b.com","private_key":"${gcp-ssrf-key}","token_uri":"http://oauth2.googleapis.com/token"}'
238238
! CREATE CONNECTION gcp_ssrf_scheme_conn TO GCP (SERVICE ACCOUNT KEY = SECRET gcp_ssrf_scheme) WITH (VALIDATE = true);
239-
contains:disallowed token_uri
239+
contains:token_uri must be https://oauth2.googleapis.com/token
240240

241241
# Public host that merely ends in the allowlisted suffix -- only a host allowlist,
242242
# not a private-IP check, rejects this.
243243
> CREATE SECRET gcp_ssrf_host AS '{"type":"service_account","project_id":"x","client_email":"a@b.com","private_key":"${gcp-ssrf-key}","token_uri":"https://oauth2.googleapis.com.evil.com/token"}'
244244
! CREATE CONNECTION gcp_ssrf_host_conn TO GCP (SERVICE ACCOUNT KEY = SECRET gcp_ssrf_host) WITH (VALIDATE = true);
245-
contains:disallowed token_uri
245+
contains:token_uri must be https://oauth2.googleapis.com/token
246246

247247
$ postgres-execute connection=postgres://mz_system:materialize@${testdrive.materialize-internal-sql-addr}
248248
ALTER SYSTEM SET storage_enforce_external_addresses = false
@@ -258,4 +258,8 @@ contains:Connection refused
258258
# it does not consult storage_enforce_external_addresses. With enforcement now
259259
# off, the malicious token_uri is still rejected.
260260
! CREATE CONNECTION gcp_ssrf_private_conn TO GCP (SERVICE ACCOUNT KEY = SECRET gcp_ssrf_private) WITH (VALIDATE = true);
261-
contains:disallowed token_uri
261+
contains:token_uri must be https://oauth2.googleapis.com/token
262+
263+
# In fact, the GCP allowlist should even apply when VALIDATE = false.
264+
! CREATE CONNECTION gcp_ssrf_private_conn TO GCP (SERVICE ACCOUNT KEY = SECRET gcp_ssrf_private) WITH (VALIDATE = false);
265+
contains:token_uri must be https://oauth2.googleapis.com/token

0 commit comments

Comments
 (0)