Skip to content

Commit a45821e

Browse files
committed
Google says token_uri is always https://oauth2.googleapis.com/token
1 parent dd20f7b commit a45821e

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::{
@@ -656,6 +658,19 @@ impl Coordinator {
656658
}
657659
self.caching_secrets_reader.invalidate(connection_id);
658660
}
661+
ConnectionDetails::Gcp(gcp) => {
662+
// A service account key defines its own OAuth2 token URI.
663+
// We only want to send requests to the actual Google OAuth2 token API,
664+
// so we inspect the key as early as we can.
665+
if let Err(err) = self
666+
.caching_secrets_reader
667+
.read_string(gcp.credentials_json)
668+
.await
669+
.and_then(|json| GcpServiceAccountKeyTokenUri::validate_json(&json))
670+
{
671+
return ctx.retire(Err(err.into()));
672+
}
673+
}
659674
_ => (),
660675
};
661676

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
@@ -157,18 +157,18 @@ $ set gcp-ssrf-key="-----BEGIN PRIVATE KEY-----\\nMIIEvAIBADANBgkqhkiG9w0BAQEFAA
157157
# IP, and testdrive stops at the first failure.
158158
> 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"}'
159159
! CREATE CONNECTION gcp_ssrf_private_conn TO GCP (SERVICE ACCOUNT KEY = SECRET gcp_ssrf_private) WITH (VALIDATE = true);
160-
contains:disallowed token_uri
160+
contains:token_uri must be https://oauth2.googleapis.com/token
161161

162162
# Correct host, but plaintext HTTP -- the scheme must be HTTPS.
163163
> 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"}'
164164
! CREATE CONNECTION gcp_ssrf_scheme_conn TO GCP (SERVICE ACCOUNT KEY = SECRET gcp_ssrf_scheme) WITH (VALIDATE = true);
165-
contains:disallowed token_uri
165+
contains:token_uri must be https://oauth2.googleapis.com/token
166166

167167
# Public host that merely ends in the allowlisted suffix -- only a host allowlist,
168168
# not a private-IP check, rejects this.
169169
> 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"}'
170170
! CREATE CONNECTION gcp_ssrf_host_conn TO GCP (SERVICE ACCOUNT KEY = SECRET gcp_ssrf_host) WITH (VALIDATE = true);
171-
contains:disallowed token_uri
171+
contains:token_uri must be https://oauth2.googleapis.com/token
172172

173173
$ postgres-execute connection=postgres://mz_system:materialize@${testdrive.materialize-internal-sql-addr}
174174
ALTER SYSTEM SET storage_enforce_external_addresses = false
@@ -184,4 +184,8 @@ contains:Connection refused
184184
# it does not consult storage_enforce_external_addresses. With enforcement now
185185
# off, the malicious token_uri is still rejected.
186186
! CREATE CONNECTION gcp_ssrf_private_conn TO GCP (SERVICE ACCOUNT KEY = SECRET gcp_ssrf_private) WITH (VALIDATE = true);
187-
contains:disallowed token_uri
187+
contains:token_uri must be https://oauth2.googleapis.com/token
188+
189+
# In fact, the GCP allowlist should even apply when VALIDATE = false.
190+
! CREATE CONNECTION gcp_ssrf_private_conn TO GCP (SERVICE ACCOUNT KEY = SECRET gcp_ssrf_private) WITH (VALIDATE = false);
191+
contains:token_uri must be https://oauth2.googleapis.com/token

0 commit comments

Comments
 (0)