fix: read Bazel RBE auth tag from NGAT env instead of hardcoding#3590
fix: read Bazel RBE auth tag from NGAT env instead of hardcoding#3590anandppatil wants to merge 1 commit intoangular:mainfrom
Conversation
The Bazel configure-remote action hardcodes the AES-256-GCM authentication tag in constants.ts, while the SauceLabs and BrowserStack actions correctly read it from the NGAT environment variable. This inconsistency means the embedded GCP service account credential can be decrypted using only publicly available parameters. This change aligns the Bazel action with the existing SauceLabs and BrowserStack patterns by reading the auth tag from process.env.NGAT. Note: The checked-in configure-remote.js bundle will need to be rebuilt by maintainers. The exposed credential should be rotated and gcp_token.data re-encrypted.
There was a problem hiding this comment.
Code Review
This pull request replaces a hardcoded authentication tag with an environment variable in the cryptographic constants. The review identifies a critical security vulnerability, noting that the change fails to prevent offline decryption because the encryption key is derived from a public value and the initialization vector remains hardcoded. It is recommended to move the encryption key to a secret environment variable and address the nonce reuse issue to ensure proper confidentiality.
|
|
||
| export const alg = 'aes-256-gcm'; | ||
| export const at = 'QwbjZ/z+yDtD+XZjKj9Ynw=='; | ||
| export const at = process.env.NGAT!; |
There was a problem hiding this comment.
This change does not actually prevent offline decryption of the credential, which is the primary problem described in the PR. In AES-GCM, the authentication tag (at) is used for integrity and authenticity, but it does not provide confidentiality. Because the encryption key (k) is derived from a public value (GITHUB_REPOSITORY_OWNER) and the initialization vector (iv) is hardcoded, an attacker can still decrypt the gcp_token.data file without the authentication tag. In Node.js, the decipher.update() method returns the decrypted plaintext even if setAuthTag() is not called or if final() fails. To properly secure the credential, the encryption key (k) should be moved to a secret environment variable. Relying on the secrecy of the authentication tag is not an effective security measure for AES-GCM. Additionally, as noted in the PR description, reusing the same (Key, IV) pair across different integrations is a critical vulnerability (nonce reuse) that should be addressed by using unique, secret keys and IVs.
|
Thanks for the review. The observation about AES-GCM's behavior is technically correct — decipher.update() returns plaintext before final() validates the auth tag, so auth tag secrecy alone doesn't provide full confidentiality. This PR intentionally matches the existing pattern used by github-actions/saucelabs/constants.ts and github-actions/browserstack/constants.ts in this same repository, which both use process.env.NGAT! for the auth tag. If the current pattern is considered insufficient, that's a broader design issue affecting all three integrations — not specific to this patch. The goal of this patch is to remove the hardcoded auth tag, which currently makes the Bazel credential trivially recoverable (all four parameters public). Even if the auth tag alone doesn't provide full AES-GCM confidentiality, hardcoding it in source is strictly worse than reading it from a secret, and inconsistent with the rest of the codebase. |
fix: read Bazel RBE auth tag from
NGATenv instead of hardcodingProblem
The
github-actions/bazel/configure-remote/constants.tsfile hardcodes the AES-256-GCM authentication tag used to decryptgcp_token.data:This means all four parameters needed to decrypt the embedded GCP service account credential are publicly available:
constants.ts→aes-256-gcmGITHUB_REPOSITORY_OWNER("angular")constants.tsconstants.tsgcp_token.datain repoThe credential can be decrypted entirely offline — no secrets, authentication, or internal access required.
Existing Pattern
The SauceLabs and BrowserStack actions in this same repository already handle this correctly by reading the auth tag from a secret environment variable:
Fix
This PR applies the same pattern to the Bazel configure-remote action:
Additional Steps Required by Maintainers
configure-remote.jsneeds to be regenerated via the Bazel build after this TypeScript change.internal-200822should be rotated, since the currentgcp_token.datais decryptable with the previously hardcoded auth tag.gcp_token.data: After rotation, re-encrypt the new credential usingencrypt.ts(which will now produce a new auth tag that should be set as theNGATsecret).NGATis available: Verify that theNGATsecret is passed to workflow steps that invokeconfigure-remote(it is already available for SauceLabs/BrowserStack steps).Secondary Concern: AES-GCM Nonce Reuse
The Bazel and SauceLabs integrations share the same
(key, IV)pair (angular<<<...,000003213213123213). Since AES-GCM is a stream cipher mode, reusing the same key+nonce for different plaintexts enables crib-dragging attacks. Ideally each integration should use a unique IV. This is noted for awareness but not addressed in this PR to keep the change minimal and focused.Related