Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces validation for external account credentials in GCPUtils to prevent potential token theft and exfiltration attacks by ensuring that token and impersonation URLs match expected Google endpoints. It also adds comprehensive unit tests in GCPUtilsTest to verify these security constraints. The review feedback highlights a potential NullPointerException when parsing empty or null JSON configurations and suggests simplifying the test code by replacing a custom stream-reading helper with Guava's ByteStreams.toByteArray.
| try (InputStream is = exchange.getRequestBody()) { | ||
| attackerBody.set(new String(readAll(is), StandardCharsets.UTF_8)); | ||
| } |
There was a problem hiding this comment.
Instead of using a custom readAll helper method, you can use Guava's ByteStreams.toByteArray(InputStream) which is already available on the classpath and used in the main source code. This simplifies the test code and avoids custom stream copying logic.
| try (InputStream is = exchange.getRequestBody()) { | |
| attackerBody.set(new String(readAll(is), StandardCharsets.UTF_8)); | |
| } | |
| try (InputStream is = exchange.getRequestBody()) { | |
| attackerBody.set(new String(com.google.common.io.ByteStreams.toByteArray(is), StandardCharsets.UTF_8)); | |
| } |
A credential configuration of type external_account tells the auth library where to read a subject token from (credential_source) and where to send it: the subject token is POSTed to token_url and the resulting access token to service_account_impersonation_url. When such a configuration is pasted into the service account JSON field of a plugin, a user with only validation access could point credential_source at the Compute Engine metadata server and token_url at an endpoint they control, causing the service agent access token of the environment to be sent to them. Following https://cloud.google.com/docs/authentication/client-libraries#validate_other_credential_configurations GCPUtils.loadServiceAccountCredentials now requires, for external_account configurations, that - token_url is exactly https://sts.googleapis.com/v1/token - service_account_impersonation_url, if present, is exactly https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/<email>:generateAccessToken With both endpoints pinned to Google, whatever the configuration reads can only be sent to Google. The configuration is parsed with the same parser that GoogleCredentials.fromStream uses, so validation and the library see exactly the same document and unparseable content fails with the library's own error. The external_account credential generated by CDAP for GKE Workload Identity and gcloud-generated configurations use these exact endpoints and continue to load; service_account JSON and all other content are handled by the library exactly as before. Fixes b/501543027, b/501546932
92b621c to
13fad0e
Compare
| } | ||
|
|
||
| /** | ||
| * Reproduces b/501543027: the credential configuration points the subject token source at the metadata server |
There was a problem hiding this comment.
Please remove internal details
|
Superseded by #1628 (same change, review comments addressed there). |
Issue
The GCP plugins accept a service account credential as inline JSON or a file path and load it with GoogleCredentials.fromStream, which accepts any credential type the auth library supports, including external_account (workload identity federation).
An external_account configuration tells the auth library where to obtain a subject token (credential_source) and where to send it: the subject token is sent to token_url and the resulting access token to service_account_impersonation_url. When such a configuration comes from user input, these endpoints must be validated; otherwise the configuration could direct tokens to an arbitrary endpoint. See Google's guidance on validating credential configurations from external sources: https://cloud.google.com/docs/authentication/client-libraries#validate_other_credential_configurations
Fix
This change adds that validation in GCPUtils.loadServiceAccountCredentials, the single method all plugin credential paths (validation, connection test, deploy and runtime) go through. For configurations of type external_account, both endpoints must be Google-owned:
• token_url must match ^https://sts(\.[a-z0-9-]+\.rep)?(\.mtls)?\.googleapis\.com/v1/token$
— i.e. https://sts.googleapis.com/v1/token, or the locational (sts..rep.googleapis.com) and mTLS (sts.mtls.googleapis.com) variants generated by
gcloud iam workload-identity-pools create-cred-config --sts-location / --enable-mtls.
• service_account_impersonation_url, if present, must match
^https://iamcredentials(\.mtls)?\.googleapis\.com/v1/projects/-/serviceAccounts/:generateAccessToken$
With both endpoints pinned to googleapis.com, whatever the configuration reads can only be sent to Google. Non-conforming configurations are rejected with a validation error naming the field. Configurations of any other type are passed to the library unchanged.
The JSON is parsed with the same parser GoogleCredentials.fromStream uses, so the validation sees exactly the document the library will act on (including lenient handling of trailing content).
Tests
GCPUtilsTest covers:
• service_account JSON and external_account configurations with the global, locational and mTLS endpoints load successfully.
• The reported token-theft payload (credential_source.url → metadata server, token_url → attacker) is rejected before any endpoint is contacted, verified with local stand-in HTTP servers.
• Exfiltration via service_account_impersonation_url, host-suffix/userinfo/scheme/case/query/trailing-slash tricks, and trailing content after the JSON object are all rejected.
• Non-external_account content (including unparseable or null input) yields the same exception as the library alone.