From 992ddf26ea9f492f975c8755a60582c0d377b704 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 28 Jul 2026 21:16:39 +0200 Subject: [PATCH 1/3] fix(arm): pin GCP WIF attribute condition and narrow SA grant setup-gcp-wif.sh created Workload Identity Federation providers that admitted every identity the issuer would vouch for, then granted roles/iam.workloadIdentityUser to the whole pool. Customers run this against their own GCP projects, so both defects exposed their service account, which carries commitment-purchase authority. AWS mode called providers create-aws with only --account-id and no --attribute-condition, so any IAM principal in the referenced account could exchange credentials at sts.googleapis.com and impersonate the service account. OIDC mode printed a stderr warning when --subject-condition was empty and then created the provider anyway; the documented copy-paste example used token.actions.githubusercontent.com and omitted the flag, so any GitHub Actions workflow on the platform could federate in. The impersonation grant was bound to principalSet://.../workloadIdentityPools//*, so anything admitted to the pool could impersonate regardless. The Terraform sibling at iac/federation/gcp-target already enforces this via lifecycle preconditions; the shell path for the same onboarding job did not. Both modes now require an identity to pin and exit nonzero without one: --aws-role-name for aws, --oidc-subject for oidc. The provider is always created with an attribute condition, and the grant names a single principal (attribute.aws_role for AWS, principal://.../subject/ for OIDC). The AWS attribute mapping normalises the session ARN to the role ARN so both the condition and the grant match it exactly rather than by substring. --subject-condition is removed rather than made mandatory: it took a raw CEL expression, so a value such as "true" looked like a restriction while admitting every subject. Values interpolated into the condition or the principal identifier are now validated, rejecting the forms that fail open: '*' widens an IAM principal to match everything, a quote or backslash terminates the CEL string literal and lets the rest of the value append "|| true", and a '$' catches pasted ${...} placeholders that are never expanded here. Issuer URIs must be https, and account IDs, role names, pool and provider IDs are format-checked. Re-running over an earlier setup no longer reports success while the pool-wide grant survives: it is detected and the script exits nonzero with the removal command, or deletes it under --remove-legacy-pool-binding. A pre-existing provider carrying no attribute condition is likewise refused instead of reused. BREAKING CHANGE: --aws-role-name (aws) and --oidc-subject (oidc) are now required, and --subject-condition is rejected. Anyone who ran an earlier version still has the pool-wide grant and remains exposed until they re-run with --remove-legacy-pool-binding. --- arm/CUDly-CrossSubscription/setup-gcp-wif.sh | 212 +++++++++++++++---- 1 file changed, 166 insertions(+), 46 deletions(-) diff --git a/arm/CUDly-CrossSubscription/setup-gcp-wif.sh b/arm/CUDly-CrossSubscription/setup-gcp-wif.sh index 117d0e3c5..9c94c42db 100755 --- a/arm/CUDly-CrossSubscription/setup-gcp-wif.sh +++ b/arm/CUDly-CrossSubscription/setup-gcp-wif.sh @@ -4,6 +4,11 @@ # account key file. Outputs the external-account credential config JSON that # should be stored as gcp_workload_identity_config in CUDly. # +# SECURITY: the provider is always created with an --attribute-condition and the +# service account impersonation grant is always scoped to a single principal. +# Both require an explicit identity to pin (--aws-role-name / --oidc-subject); +# there is no permissive default, and the script exits nonzero if one is missing. +# # Prerequisites: # gcloud auth login (with roles/iam.workloadIdentityPoolAdmin + roles/iam.serviceAccountAdmin # on the target project, and roles/iam.serviceAccountTokenCreator on the SA) @@ -15,6 +20,7 @@ # --provider-id cudly-aws \ # --provider-type aws \ # --aws-account-id 123456789012 \ +# --aws-role-name CUDly-Execution \ # --sa-email cudly@my-gcp-project.iam.gserviceaccount.com # # Usage (OIDC provider): @@ -24,7 +30,15 @@ # --provider-id cudly-oidc \ # --provider-type oidc \ # --issuer-uri https://token.actions.githubusercontent.com \ +# --oidc-subject 'repo:my-org/my-repo:ref:refs/heads/main' \ # --sa-email cudly@my-gcp-project.iam.gserviceaccount.com +# +# Upgrading from an earlier run: versions of this script before the +# --aws-role-name / --oidc-subject requirement granted +# roles/iam.workloadIdentityUser to every identity in the pool +# (principalSet://.../workloadIdentityPools//*). Re-running this script +# detects that binding and refuses to report success while it exists; pass +# --remove-legacy-pool-binding to delete it once the narrow grant is in place. set -euo pipefail @@ -34,11 +48,12 @@ PROVIDER_ID="cudly-provider" PROVIDER_TYPE="" # "aws" or "oidc" SA_EMAIL="" AWS_ACCOUNT_ID="" +AWS_ROLE_NAME="" ISSUER_URI="" -# Optional: restrict which OIDC subject (sub claim) may impersonate the SA. -# Example: "assertion.sub=='repo:my-org/my-repo:ref:refs/heads/main'" (GitHub Actions) -# STRONGLY RECOMMENDED for public issuers to prevent privilege escalation. -SUBJECT_CONDITION="" +OIDC_SUBJECT="" +REMOVE_LEGACY_POOL_BINDING="false" + +die() { echo "Error: $*" >&2; exit 1; } # ── Argument parsing ─────────────────────────────────────────────────────────── while [[ $# -gt 0 ]]; do @@ -49,35 +64,89 @@ while [[ $# -gt 0 ]]; do --provider-type) PROVIDER_TYPE="$2"; shift 2 ;; --sa-email) SA_EMAIL="$2"; shift 2 ;; --aws-account-id) AWS_ACCOUNT_ID="$2"; shift 2 ;; + --aws-role-name) AWS_ROLE_NAME="$2"; shift 2 ;; --issuer-uri) ISSUER_URI="$2"; shift 2 ;; - --subject-condition) SUBJECT_CONDITION="$2"; shift 2 ;; - *) echo "Unknown argument: $1" >&2; exit 1 ;; + --oidc-subject) OIDC_SUBJECT="$2"; shift 2 ;; + --remove-legacy-pool-binding) REMOVE_LEGACY_POOL_BINDING="true"; shift ;; + # Removed on purpose: --subject-condition took a raw CEL expression, so a + # value such as "true" or "assertion.sub != ''" looked like a restriction + # while admitting every subject from the issuer. --oidc-subject takes the + # subject itself and this script builds the equality condition around it. + --subject-condition) + die "--subject-condition has been removed; pass --oidc-subject '' instead (this script builds the attribute condition, so a hand-written CEL expression can no longer silently admit every subject)" ;; + *) die "Unknown argument: $1" ;; esac done -# ── Validate required args ───────────────────────────────────────────────────── +# ── Value validation ─────────────────────────────────────────────────────────── +# Every value below is interpolated into either the provider's CEL +# --attribute-condition or the IAM principal identifier of the impersonation +# grant. Reject the forms that fail OPEN there rather than merely looking wrong: +# '*' would widen an IAM principal identifier to match every identity; +# '$' catches pasted ${...} placeholders, which IAM does not expand here +# (they yield a binding that matches nothing, or in a context that +# does expand them, one that matches everything); +# ' " \ ` would terminate the CEL string literal in --attribute-condition and +# let the rest of the value rewrite the condition (e.g. "|| true"). +validate_principal_value() { + local flag="$1" value="$2" pattern="$3" + [[ -n "$value" ]] || die "$flag must not be empty" + case "$value" in + *'*'*) die "$flag must not contain '*' (got: $value): a wildcard would match every identity and defeat the restriction this flag exists to apply" ;; + *'$'*) die "$flag must not contain '\$' (got: $value): it looks like an unexpanded \${...} placeholder, which is not substituted here" ;; + *\'*|*\"*|*\\*|*'`'*) die "$flag must not contain quotes or backslashes (got: $value): they would break out of the CEL string literal in the provider's attribute condition" ;; + esac + [[ "$value" =~ $pattern ]] || die "$flag has an unexpected format (got: $value)" +} + if [[ -z "$PROJECT" || -z "$SA_EMAIL" || -z "$PROVIDER_TYPE" ]]; then - echo "Error: --project, --sa-email, and --provider-type are required" >&2 - exit 1 -fi -if [[ "$PROVIDER_TYPE" == "aws" && -z "$AWS_ACCOUNT_ID" ]]; then - echo "Error: --aws-account-id is required for --provider-type aws" >&2 - exit 1 -fi -if [[ "$PROVIDER_TYPE" == "oidc" && -z "$ISSUER_URI" ]]; then - echo "Error: --issuer-uri is required for --provider-type oidc" >&2 - exit 1 + die "--project, --sa-email, and --provider-type are required" fi if [[ "$PROVIDER_TYPE" != "aws" && "$PROVIDER_TYPE" != "oidc" ]]; then - echo "Error: --provider-type must be 'aws' or 'oidc'" >&2 - exit 1 + die "--provider-type must be 'aws' or 'oidc'" +fi + +validate_principal_value "--project" "$PROJECT" '^[a-z][-a-z0-9]{4,28}[a-z0-9]$' +validate_principal_value "--pool-id" "$POOL_ID" '^[a-z0-9][-a-z0-9]{2,30}[a-z0-9]$' +validate_principal_value "--provider-id" "$PROVIDER_ID" '^[a-z0-9][-a-z0-9]{2,30}[a-z0-9]$' +validate_principal_value "--sa-email" "$SA_EMAIL" '^[a-zA-Z0-9][-a-zA-Z0-9._]*@[a-z0-9.-]+\.gserviceaccount\.com$' + +if [[ "$PROVIDER_TYPE" == "aws" ]]; then + [[ -n "$AWS_ACCOUNT_ID" ]] || die "--aws-account-id is required for --provider-type aws" + # Mandatory: without a role to pin, the provider has no attribute condition and + # every IAM principal in the AWS account can federate as the service account. + [[ -n "$AWS_ROLE_NAME" ]] || die "--aws-role-name is required for --provider-type aws. Without it the provider has no attribute condition and any IAM role in account ${AWS_ACCOUNT_ID} can federate as ${SA_EMAIL}." + validate_principal_value "--aws-account-id" "$AWS_ACCOUNT_ID" '^[0-9]{12}$' + validate_principal_value "--aws-role-name" "$AWS_ROLE_NAME" '^[A-Za-z0-9_+=.@-]{1,64}$' + # Standard AWS partition. A GovCloud or China-partition caller presents a + # different ARN prefix, so the condition below simply would not match: the + # token exchange is refused rather than admitted on a partition mismatch. + AWS_ROLE_ARN="arn:aws:sts::${AWS_ACCOUNT_ID}:assumed-role/${AWS_ROLE_NAME}" +else + [[ -n "$ISSUER_URI" ]] || die "--issuer-uri is required for --provider-type oidc" + # Mandatory: public issuers such as token.actions.githubusercontent.com will + # mint a token for any workload on the platform, so without a pinned subject + # any repository in the world can federate as the service account. + [[ -n "$OIDC_SUBJECT" ]] || die "--oidc-subject is required for --provider-type oidc. Without it the provider has no attribute condition and any subject issued by ${ISSUER_URI} can federate as ${SA_EMAIL}." + validate_principal_value "--issuer-uri" "$ISSUER_URI" '^https://[a-zA-Z0-9.-]+(:[0-9]+)?(/[-a-zA-Z0-9._~/]*)?$' + validate_principal_value "--oidc-subject" "$OIDC_SUBJECT" '^[A-Za-z0-9][-A-Za-z0-9._:/@=+~]*$' + # google.subject is capped at 127 characters by GCP; a longer value would be + # rejected at token-exchange time, long after this script reported success. + [[ ${#OIDC_SUBJECT} -le 127 ]] || die "--oidc-subject must be at most 127 characters (google.subject limit); got ${#OIDC_SUBJECT}" fi PROJECT_NUMBER=$(gcloud projects describe "$PROJECT" --format='value(projectNumber)') +[[ "$PROJECT_NUMBER" =~ ^[0-9]+$ ]] || die "could not resolve a numeric project number for '$PROJECT' (got: '$PROJECT_NUMBER')" + echo "Project : $PROJECT (number: $PROJECT_NUMBER)" echo "Pool : $POOL_ID" echo "Provider : $PROVIDER_ID ($PROVIDER_TYPE)" echo "Service Acct : $SA_EMAIL" +if [[ "$PROVIDER_TYPE" == "aws" ]]; then + echo "Trusted role : $AWS_ROLE_ARN" +else + echo "Trusted subj : $OIDC_SUBJECT (issuer: $ISSUER_URI)" +fi echo "" # ── Idempotent pool creation ─────────────────────────────────────────────────── @@ -89,6 +158,14 @@ if ! gcloud iam workload-identity-pools describe "$POOL_ID" \ --display-name="CUDly WIF pool" --quiet else echo "Reusing existing pool '${POOL_ID}'" + # GCP principal identifiers are pool-scoped, not provider-scoped: the grant + # below names an attribute value, and any provider in this pool that can mint + # that attribute satisfies it. A pool dedicated to this provider keeps the + # trust boundary equal to the attribute condition set below. + echo "Note: principal identifiers are pool-scoped, not provider-scoped. Another" >&2 + echo " provider in this pool that maps the same attribute value would also" >&2 + echo " satisfy the grant below. Use a pool dedicated to CUDly (--pool-id)" >&2 + echo " unless you intend to share it." >&2 fi # ── Idempotent provider creation ─────────────────────────────────────────────── @@ -97,49 +174,92 @@ if ! gcloud iam workload-identity-pools providers describe "$PROVIDER_ID" \ --workload-identity-pool="$POOL_ID" &>/dev/null; then echo "Creating ${PROVIDER_TYPE} provider '${PROVIDER_ID}'..." if [[ "$PROVIDER_TYPE" == "aws" ]]; then + # attribute.aws_role normalises the session ARN + # (arn:aws:sts:::assumed-role//) down to the role ARN, + # so both the condition below and the IAM grant can match it exactly instead + # of relying on a substring test. gcloud iam workload-identity-pools providers create-aws "$PROVIDER_ID" \ --project="$PROJECT" --location=global \ --workload-identity-pool="$POOL_ID" \ - --account-id="$AWS_ACCOUNT_ID" --quiet + --account-id="$AWS_ACCOUNT_ID" \ + --attribute-mapping="google.subject=assertion.arn,attribute.aws_role=assertion.arn.contains('assumed-role') ? assertion.arn.extract('{account_arn}assumed-role/') + 'assumed-role/' + assertion.arn.extract('assumed-role/{role_name}/') : assertion.arn" \ + --attribute-condition="attribute.aws_role == '${AWS_ROLE_ARN}'" \ + --quiet else # --attribute-mapping is required; without it no subject claims are mapped and - # all token exchanges will be rejected at runtime despite successful pool setup. - # --attribute-condition restricts which OIDC subjects can impersonate the SA. - # For public issuers (GitHub Actions, etc.) omitting --attribute-condition allows - # ANY token from the issuer to impersonate — pass --subject-condition to scope it. - if [[ -z "$SUBJECT_CONDITION" ]]; then - echo "WARNING: --subject-condition not set. Any OIDC token from '${ISSUER_URI}'" >&2 - echo " will be able to impersonate ${SA_EMAIL}." >&2 - echo " For public issuers pass e.g. --subject-condition \"assertion.sub==''\"" >&2 - fi - OIDC_ARGS=( - "$PROVIDER_ID" - --project="$PROJECT" --location=global - --workload-identity-pool="$POOL_ID" - --issuer-uri="$ISSUER_URI" - --attribute-mapping="google.subject=assertion.sub" + # all token exchanges are rejected at runtime despite a successful setup. + # --attribute-condition is built here from --oidc-subject so that only that + # exact subject is admitted to the pool. + gcloud iam workload-identity-pools providers create-oidc "$PROVIDER_ID" \ + --project="$PROJECT" --location=global \ + --workload-identity-pool="$POOL_ID" \ + --issuer-uri="$ISSUER_URI" \ + --attribute-mapping="google.subject=assertion.sub" \ + --attribute-condition="google.subject == '${OIDC_SUBJECT}'" \ --quiet - ) - if [[ -n "$SUBJECT_CONDITION" ]]; then - OIDC_ARGS+=(--attribute-condition="$SUBJECT_CONDITION") - fi - gcloud iam workload-identity-pools providers create-oidc "${OIDC_ARGS[@]}" fi else - echo "Reusing existing provider '${PROVIDER_ID}'" + # An existing provider keeps whatever attribute condition it was created with, + # including none at all. Do not report success over a provider this script + # cannot vouch for. + EXISTING_CONDITION=$(gcloud iam workload-identity-pools providers describe "$PROVIDER_ID" \ + --project="$PROJECT" --location=global \ + --workload-identity-pool="$POOL_ID" \ + --format='value(attributeCondition)') + if [[ -z "$EXISTING_CONDITION" ]]; then + die "provider '${PROVIDER_ID}' already exists with no attribute condition, so every identity it accepts can enter pool '${POOL_ID}'. Delete it and re-run: + gcloud iam workload-identity-pools providers delete ${PROVIDER_ID} --project=${PROJECT} --location=global --workload-identity-pool=${POOL_ID}" + fi + echo "Reusing existing provider '${PROVIDER_ID}' (attribute condition: ${EXISTING_CONDITION})" fi -# ── Grant service account impersonation to the pool ─────────────────────────── -POOL_PRINCIPAL="principalSet://iam.googleapis.com/projects/${PROJECT_NUMBER}/locations/global/workloadIdentityPools/${POOL_ID}/*" -echo "Granting roles/iam.workloadIdentityUser to pool members on ${SA_EMAIL}..." +# ── Grant service account impersonation to one principal ────────────────────── +POOL_RESOURCE="projects/${PROJECT_NUMBER}/locations/global/workloadIdentityPools/${POOL_ID}" +if [[ "$PROVIDER_TYPE" == "aws" ]]; then + # Session ARNs carry a per-session suffix, so the grant names the normalised + # role attribute rather than an exact subject. + MEMBER="principalSet://iam.googleapis.com/${POOL_RESOURCE}/attribute.aws_role/${AWS_ROLE_ARN}" +else + MEMBER="principal://iam.googleapis.com/${POOL_RESOURCE}/subject/${OIDC_SUBJECT}" +fi +echo "Granting roles/iam.workloadIdentityUser on ${SA_EMAIL} to:" +echo " ${MEMBER}" gcloud iam service-accounts add-iam-policy-binding "$SA_EMAIL" \ --role=roles/iam.workloadIdentityUser \ - --member="$POOL_PRINCIPAL" \ + --member="$MEMBER" \ --project="$PROJECT" \ --quiet +# ── Retire the pool-wide grant left by earlier versions of this script ──────── +LEGACY_MEMBER="principalSet://iam.googleapis.com/${POOL_RESOURCE}/*" +# Fetched into a variable first: a pipeline straight into grep would let a failed +# get-iam-policy read as "no legacy binding found" and report success over one. +SA_POLICY=$(gcloud iam service-accounts get-iam-policy "$SA_EMAIL" \ + --project="$PROJECT" --format=json) +if grep -qF -- "$LEGACY_MEMBER" <<<"$SA_POLICY"; then + if [[ "$REMOVE_LEGACY_POOL_BINDING" == "true" ]]; then + echo "Removing legacy pool-wide impersonation grant..." + gcloud iam service-accounts remove-iam-policy-binding "$SA_EMAIL" \ + --role=roles/iam.workloadIdentityUser \ + --member="$LEGACY_MEMBER" \ + --project="$PROJECT" \ + --quiet + else + die "${SA_EMAIL} still grants roles/iam.workloadIdentityUser to every identity in pool '${POOL_ID}': + ${LEGACY_MEMBER} +That grant was created by an earlier version of this script and makes the narrow +grant added above irrelevant: anything admitted to the pool can still impersonate +the service account. Re-run this command with --remove-legacy-pool-binding, or +remove it yourself with: + gcloud iam service-accounts remove-iam-policy-binding ${SA_EMAIL} \\ + --role=roles/iam.workloadIdentityUser \\ + --member='${LEGACY_MEMBER}' \\ + --project=${PROJECT}" + fi +fi + # ── Generate external account credential config (no secrets) ────────────────── -PROVIDER_RESOURCE="projects/${PROJECT_NUMBER}/locations/global/workloadIdentityPools/${POOL_ID}/providers/${PROVIDER_ID}" +PROVIDER_RESOURCE="${POOL_RESOURCE}/providers/${PROVIDER_ID}" echo "" echo "══════════════════════════════════════════════════════════════" echo " External account credential config (store in CUDly as" From 21af9ccdd9714024e4848bae260ebe944a64c611 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 28 Jul 2026 21:34:46 +0200 Subject: [PATCH 2/3] fix(arm): close fail-open gaps in GCP WIF setup re-run paths An adversarial review of the previous commit found four ways the hardening could still be bypassed on a re-run against an already provisioned project. The legacy pool-wide grant was only looked for under the --pool-id of the current run, and only under roles/iam.workloadIdentityUser. Both scopings were escapable. The script's own pool-reuse note recommends moving to a dedicated pool, and doing so rebuilt the search string for the new pool, so the original wildcard grant went unseen and the script exited 0. A wildcard under roles/iam.serviceAccountTokenCreator was matched by the policy grep but removal was hardcoded to workloadIdentityUser, so it survived a run that reported success; generateAccessToken confers the same impersonation power. The scan is now pool-agnostic and role-agnostic, removal uses the role each member is actually bound to, and the policy is re-read afterwards so a partial removal cannot pass. The existing-provider gate only asserted that the attribute condition was non-empty, which grandfathered exactly the values --subject-condition was removed to prevent: "true" and "assertion.sub != ''" are both non-empty and both admit every identity. The condition is now compared to the one this script would have written, and the provider type is checked so an OIDC provider is not reused under --provider-type aws. This also rejects a provider created by the Terraform sibling, whose contains() condition pairs with a different attribute mapping that the grant here would never match. The get-iam-policy read is kept out of the grep pipeline: piping it into `grep ... || true` would let a failed policy read produce empty output and pass as "no wildcard grants found". Also: surface narrow grants for identities other than this run's, since re-running with a corrected role or subject otherwise leaves the previous one able to impersonate; warn when the AWS role name is long enough that the session ARN risks exceeding GCP's 127-character google.subject limit; and accept two characters real identities use, "," in AWS role names (AWS permits it and it never reaches the comma-delimited attribute mapping) and "|" in OIDC subjects (Auth0 and Okta style), both inert because quotes remain rejected. --- arm/CUDly-CrossSubscription/setup-gcp-wif.sh | 144 ++++++++++++++----- 1 file changed, 111 insertions(+), 33 deletions(-) diff --git a/arm/CUDly-CrossSubscription/setup-gcp-wif.sh b/arm/CUDly-CrossSubscription/setup-gcp-wif.sh index 9c94c42db..1356dd40e 100755 --- a/arm/CUDly-CrossSubscription/setup-gcp-wif.sh +++ b/arm/CUDly-CrossSubscription/setup-gcp-wif.sh @@ -117,11 +117,23 @@ if [[ "$PROVIDER_TYPE" == "aws" ]]; then # every IAM principal in the AWS account can federate as the service account. [[ -n "$AWS_ROLE_NAME" ]] || die "--aws-role-name is required for --provider-type aws. Without it the provider has no attribute condition and any IAM role in account ${AWS_ACCOUNT_ID} can federate as ${SA_EMAIL}." validate_principal_value "--aws-account-id" "$AWS_ACCOUNT_ID" '^[0-9]{12}$' - validate_principal_value "--aws-role-name" "$AWS_ROLE_NAME" '^[A-Za-z0-9_+=.@-]{1,64}$' + # Matches AWS's own role-name charset [\w+=,.@-]{1,64}. The comma is safe here: + # the role name reaches --attribute-condition (a plain string flag), never the + # comma-delimited --attribute-mapping dict. + validate_principal_value "--aws-role-name" "$AWS_ROLE_NAME" '^[A-Za-z0-9_+=,.@-]{1,64}$' # Standard AWS partition. A GovCloud or China-partition caller presents a # different ARN prefix, so the condition below simply would not match: the # token exchange is refused rather than admitted on a partition mismatch. AWS_ROLE_ARN="arn:aws:sts::${AWS_ACCOUNT_ID}:assumed-role/${AWS_ROLE_NAME}" + EXPECTED_CONDITION="attribute.aws_role == '${AWS_ROLE_ARN}'" + # google.subject is the full session ARN here and GCP caps it at 127 chars. + # "arn:aws:sts::<12 digits>:assumed-role//" leaves + # 127 - 38 - len(role) - 1 for the session name, which AWS allows up to 64. + if [[ $((127 - 38 - ${#AWS_ROLE_NAME} - 1)) -lt 64 ]]; then + echo "Note: role name '${AWS_ROLE_NAME}' leaves only $((127 - 38 - ${#AWS_ROLE_NAME} - 1)) characters" >&2 + echo " for the session name before google.subject exceeds GCP's 127-character" >&2 + echo " limit. Longer session names will be rejected at token-exchange time." >&2 + fi else [[ -n "$ISSUER_URI" ]] || die "--issuer-uri is required for --provider-type oidc" # Mandatory: public issuers such as token.actions.githubusercontent.com will @@ -129,10 +141,14 @@ else # any repository in the world can federate as the service account. [[ -n "$OIDC_SUBJECT" ]] || die "--oidc-subject is required for --provider-type oidc. Without it the provider has no attribute condition and any subject issued by ${ISSUER_URI} can federate as ${SA_EMAIL}." validate_principal_value "--issuer-uri" "$ISSUER_URI" '^https://[a-zA-Z0-9.-]+(:[0-9]+)?(/[-a-zA-Z0-9._~/]*)?$' - validate_principal_value "--oidc-subject" "$OIDC_SUBJECT" '^[A-Za-z0-9][-A-Za-z0-9._:/@=+~]*$' + # '|' is permitted because Auth0/Okta-style subjects use it (google-oauth2|123). + # It is inert in both destinations: quotes are rejected above, so it cannot + # escape the CEL string literal, and it carries no meaning in a principal path. + validate_principal_value "--oidc-subject" "$OIDC_SUBJECT" '^[A-Za-z0-9][-A-Za-z0-9._:/@=+~|]*$' # google.subject is capped at 127 characters by GCP; a longer value would be # rejected at token-exchange time, long after this script reported success. [[ ${#OIDC_SUBJECT} -le 127 ]] || die "--oidc-subject must be at most 127 characters (google.subject limit); got ${#OIDC_SUBJECT}" + EXPECTED_CONDITION="google.subject == '${OIDC_SUBJECT}'" fi PROJECT_NUMBER=$(gcloud projects describe "$PROJECT" --format='value(projectNumber)') @@ -183,7 +199,7 @@ if ! gcloud iam workload-identity-pools providers describe "$PROVIDER_ID" \ --workload-identity-pool="$POOL_ID" \ --account-id="$AWS_ACCOUNT_ID" \ --attribute-mapping="google.subject=assertion.arn,attribute.aws_role=assertion.arn.contains('assumed-role') ? assertion.arn.extract('{account_arn}assumed-role/') + 'assumed-role/' + assertion.arn.extract('assumed-role/{role_name}/') : assertion.arn" \ - --attribute-condition="attribute.aws_role == '${AWS_ROLE_ARN}'" \ + --attribute-condition="$EXPECTED_CONDITION" \ --quiet else # --attribute-mapping is required; without it no subject claims are mapped and @@ -195,22 +211,44 @@ if ! gcloud iam workload-identity-pools providers describe "$PROVIDER_ID" \ --workload-identity-pool="$POOL_ID" \ --issuer-uri="$ISSUER_URI" \ --attribute-mapping="google.subject=assertion.sub" \ - --attribute-condition="google.subject == '${OIDC_SUBJECT}'" \ + --attribute-condition="$EXPECTED_CONDITION" \ --quiet fi else - # An existing provider keeps whatever attribute condition it was created with, - # including none at all. Do not report success over a provider this script - # cannot vouch for. + # An existing provider keeps whatever attribute condition it was created with. + # Checking only that the condition is non-empty would grandfather exactly the + # values --subject-condition was removed to prevent: "true", or + # "assertion.sub != ''", both non-empty and both admitting every identity. The + # condition is therefore compared to the one this script would have written. + DELETE_HINT=" gcloud iam workload-identity-pools providers delete ${PROVIDER_ID} --project=${PROJECT} --location=global --workload-identity-pool=${POOL_ID}" EXISTING_CONDITION=$(gcloud iam workload-identity-pools providers describe "$PROVIDER_ID" \ --project="$PROJECT" --location=global \ --workload-identity-pool="$POOL_ID" \ --format='value(attributeCondition)') - if [[ -z "$EXISTING_CONDITION" ]]; then - die "provider '${PROVIDER_ID}' already exists with no attribute condition, so every identity it accepts can enter pool '${POOL_ID}'. Delete it and re-run: - gcloud iam workload-identity-pools providers delete ${PROVIDER_ID} --project=${PROJECT} --location=global --workload-identity-pool=${POOL_ID}" + # A provider of the other type would emit a credential config that does not + # match it and mint an attribute the grant below never names. + EXISTING_TYPE=$(gcloud iam workload-identity-pools providers describe "$PROVIDER_ID" \ + --project="$PROJECT" --location=global \ + --workload-identity-pool="$POOL_ID" \ + --format='value(aws.accountId,oidc.issuerUri)') + if [[ "$PROVIDER_TYPE" == "aws" && "$EXISTING_TYPE" != "${AWS_ACCOUNT_ID}"* ]]; then + die "provider '${PROVIDER_ID}' already exists but is not an AWS provider for account ${AWS_ACCOUNT_ID} (describe reports: ${EXISTING_TYPE}). Delete it and re-run: +${DELETE_HINT}" + fi + if [[ "$PROVIDER_TYPE" == "oidc" && "$EXISTING_TYPE" != *"${ISSUER_URI}" ]]; then + die "provider '${PROVIDER_ID}' already exists but is not an OIDC provider for issuer ${ISSUER_URI} (describe reports: ${EXISTING_TYPE}). Delete it and re-run: +${DELETE_HINT}" fi - echo "Reusing existing provider '${PROVIDER_ID}' (attribute condition: ${EXISTING_CONDITION})" + if [[ "$EXISTING_CONDITION" != "$EXPECTED_CONDITION" ]]; then + die "provider '${PROVIDER_ID}' already exists with a different attribute condition, so this script cannot vouch for which identities enter pool '${POOL_ID}'. + found : ${EXISTING_CONDITION:-(none)} + expected: ${EXPECTED_CONDITION} +A condition that is merely non-empty is not a restriction: 'true' and +\"assertion.sub != ''\" both admit every identity the issuer will vouch for. +Delete the provider and re-run: +${DELETE_HINT}" + fi + echo "Reusing existing provider '${PROVIDER_ID}' (attribute condition matches: ${EXISTING_CONDITION})" fi # ── Grant service account impersonation to one principal ────────────────────── @@ -230,34 +268,74 @@ gcloud iam service-accounts add-iam-policy-binding "$SA_EMAIL" \ --project="$PROJECT" \ --quiet -# ── Retire the pool-wide grant left by earlier versions of this script ──────── -LEGACY_MEMBER="principalSet://iam.googleapis.com/${POOL_RESOURCE}/*" -# Fetched into a variable first: a pipeline straight into grep would let a failed -# get-iam-policy read as "no legacy binding found" and report success over one. -SA_POLICY=$(gcloud iam service-accounts get-iam-policy "$SA_EMAIL" \ - --project="$PROJECT" --format=json) -if grep -qF -- "$LEGACY_MEMBER" <<<"$SA_POLICY"; then +# ── Retire pool-wide grants left by earlier versions of this script ─────────── +# Scanned across every pool and every role, not just this run's --pool-id and +# roles/iam.workloadIdentityUser. Scoping the scan to the current pool would let +# a customer hide the original grant simply by re-running with a different +# --pool-id (which the pool-reuse note above actively recommends), and scoping it +# to one role would miss the same wildcard under roles/iam.serviceAccountTokenCreator, +# which confers the same impersonation power via generateAccessToken. +# +# Emitted one "rolemember" line per member so the role owning each wildcard +# is known; a bare policy grep cannot tell which role to remove it from. +# +# The fetch is deliberately kept out of the grep pipeline. Piping gcloud straight +# into `grep ... || true` would let a failed get-iam-policy produce empty output +# and read as "no wildcard grants found", reporting success over the very grant +# this check exists to catch. As a plain assignment it aborts under `set -e`. +sa_bindings() { + gcloud iam service-accounts get-iam-policy "$SA_EMAIL" \ + --project="$PROJECT" --flatten="bindings[].members" \ + --format="value(bindings.role,bindings.members)" +} +pool_wide_grants() { + grep -E $'\t''principalSet://iam\.googleapis\.com/projects/[0-9]+/locations/[^/]+/workloadIdentityPools/[^/]+/\*$' <<<"$1" || true +} + +SA_BINDINGS=$(sa_bindings) +LEGACY_GRANTS=$(pool_wide_grants "$SA_BINDINGS") +if [[ -n "$LEGACY_GRANTS" ]]; then if [[ "$REMOVE_LEGACY_POOL_BINDING" == "true" ]]; then - echo "Removing legacy pool-wide impersonation grant..." - gcloud iam service-accounts remove-iam-policy-binding "$SA_EMAIL" \ - --role=roles/iam.workloadIdentityUser \ - --member="$LEGACY_MEMBER" \ - --project="$PROJECT" \ - --quiet + while IFS=$'\t' read -r legacy_role legacy_member; do + [[ -n "$legacy_member" ]] || continue + echo "Removing pool-wide grant: ${legacy_role} -> ${legacy_member}" + gcloud iam service-accounts remove-iam-policy-binding "$SA_EMAIL" \ + --role="$legacy_role" \ + --member="$legacy_member" \ + --project="$PROJECT" \ + --quiet + done <<<"$LEGACY_GRANTS" + # Re-read the policy: removing one role's wildcard says nothing about the + # others, and reporting success here is the whole point of the check. + SA_BINDINGS=$(sa_bindings) + REMAINING=$(pool_wide_grants "$SA_BINDINGS") + [[ -z "$REMAINING" ]] || die "pool-wide grants survived removal on ${SA_EMAIL}: +${REMAINING}" else - die "${SA_EMAIL} still grants roles/iam.workloadIdentityUser to every identity in pool '${POOL_ID}': - ${LEGACY_MEMBER} -That grant was created by an earlier version of this script and makes the narrow -grant added above irrelevant: anything admitted to the pool can still impersonate -the service account. Re-run this command with --remove-legacy-pool-binding, or -remove it yourself with: + die "${SA_EMAIL} still grants impersonation to every identity in a workload identity pool: +${LEGACY_GRANTS} +Grants of this shape were created by earlier versions of this script and make the +narrow grant added above irrelevant: anything admitted to that pool can still +impersonate the service account. Re-run with --remove-legacy-pool-binding, or +remove each one yourself with: gcloud iam service-accounts remove-iam-policy-binding ${SA_EMAIL} \\ - --role=roles/iam.workloadIdentityUser \\ - --member='${LEGACY_MEMBER}' \\ - --project=${PROJECT}" + --role='' --member='' --project=${PROJECT}" fi fi +# Narrow grants for identities other than this run's are not removed (they may +# belong to a second legitimate CUDly account), but they are surfaced: a re-run +# with a corrected role or subject otherwise leaves the previous one impersonating. +OTHER_GRANTS=$(grep -F "iam.googleapis.com/${POOL_RESOURCE}/" <<<"$SA_BINDINGS" \ + | grep -vF -- "$MEMBER" || true) +if [[ -n "$OTHER_GRANTS" ]]; then + echo "Note: ${SA_EMAIL} is also impersonable by other identities in pool '${POOL_ID}':" >&2 + while IFS= read -r grant_line; do + echo " ${grant_line}" >&2 + done <<<"$OTHER_GRANTS" + echo " Remove any that are no longer expected." >&2 +fi + # ── Generate external account credential config (no secrets) ────────────────── PROVIDER_RESOURCE="${POOL_RESOURCE}/providers/${PROVIDER_ID}" echo "" From 8fde9daafc8198bf0795c9dceb9b1fb4c2d0772c Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 28 Jul 2026 21:51:55 +0200 Subject: [PATCH 3/3] fix(arm): correct sibling-grant match, ARN budget, pool-scope notice Three follow-ups from review, all in the advisory paths of the GCP WIF setup script. The sibling-grant notice excluded this run's member with `grep -vF`, which drops every line containing it rather than every line equal to it. A grant to CUDly-Execution-Admin was therefore suppressed while pinning CUDly-Execution, hiding a stale grant during exactly the "re-ran with a corrected role" case the notice exists for. The member is now compared as a whole field. The google.subject budget used 38 characters for "arn:aws:sts::<12 digits>:assumed-role/", which is 39. A 24-character role name skipped the warning while still producing a 128-character subject that GCP rejects at token-exchange time. The pool-scope notice printed only when reusing an existing pool. A pool this script creates fresh is equally exposed once a second provider is added to it later, and that operator never saw the warning. It now prints unconditionally. --- arm/CUDly-CrossSubscription/setup-gcp-wif.sh | 41 +++++++++++++------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/arm/CUDly-CrossSubscription/setup-gcp-wif.sh b/arm/CUDly-CrossSubscription/setup-gcp-wif.sh index 1356dd40e..5fd0c8eb0 100755 --- a/arm/CUDly-CrossSubscription/setup-gcp-wif.sh +++ b/arm/CUDly-CrossSubscription/setup-gcp-wif.sh @@ -127,10 +127,12 @@ if [[ "$PROVIDER_TYPE" == "aws" ]]; then AWS_ROLE_ARN="arn:aws:sts::${AWS_ACCOUNT_ID}:assumed-role/${AWS_ROLE_NAME}" EXPECTED_CONDITION="attribute.aws_role == '${AWS_ROLE_ARN}'" # google.subject is the full session ARN here and GCP caps it at 127 chars. - # "arn:aws:sts::<12 digits>:assumed-role//" leaves - # 127 - 38 - len(role) - 1 for the session name, which AWS allows up to 64. - if [[ $((127 - 38 - ${#AWS_ROLE_NAME} - 1)) -lt 64 ]]; then - echo "Note: role name '${AWS_ROLE_NAME}' leaves only $((127 - 38 - ${#AWS_ROLE_NAME} - 1)) characters" >&2 + # "arn:aws:sts::<12 digits>:assumed-role/" is 39 characters, and the session + # name is preceded by a "/", so the role name leaves 127 - 39 - len(role) - 1 + # for a session name that AWS allows to reach 64. + AWS_SESSION_BUDGET=$((127 - 39 - ${#AWS_ROLE_NAME} - 1)) + if [[ $AWS_SESSION_BUDGET -lt 64 ]]; then + echo "Note: role name '${AWS_ROLE_NAME}' leaves only ${AWS_SESSION_BUDGET} characters" >&2 echo " for the session name before google.subject exceeds GCP's 127-character" >&2 echo " limit. Longer session names will be rejected at token-exchange time." >&2 fi @@ -174,16 +176,22 @@ if ! gcloud iam workload-identity-pools describe "$POOL_ID" \ --display-name="CUDly WIF pool" --quiet else echo "Reusing existing pool '${POOL_ID}'" - # GCP principal identifiers are pool-scoped, not provider-scoped: the grant - # below names an attribute value, and any provider in this pool that can mint - # that attribute satisfies it. A pool dedicated to this provider keeps the - # trust boundary equal to the attribute condition set below. - echo "Note: principal identifiers are pool-scoped, not provider-scoped. Another" >&2 - echo " provider in this pool that maps the same attribute value would also" >&2 - echo " satisfy the grant below. Use a pool dedicated to CUDly (--pool-id)" >&2 - echo " unless you intend to share it." >&2 fi +# GCP principal identifiers are pool-scoped, not provider-scoped: the grant below +# names an attribute value, and any provider in this pool that can mint that +# attribute satisfies it. A pool dedicated to this provider keeps the trust +# boundary equal to the attribute condition set below. +# +# Printed unconditionally, not only when reusing a pool: a pool this run creates +# fresh is equally exposed the moment a second provider is added to it later, and +# that operator would otherwise never have seen the warning. No script can close +# this - GCP has no provider-scoped principal form - so a notice is the ceiling. +echo "Note: principal identifiers are pool-scoped, not provider-scoped. Another" >&2 +echo " provider in pool '${POOL_ID}' that maps the same attribute value would" >&2 +echo " also satisfy the grant below. Keep this pool dedicated to CUDly" >&2 +echo " (--pool-id) unless you intend to share it." >&2 + # ── Idempotent provider creation ─────────────────────────────────────────────── if ! gcloud iam workload-identity-pools providers describe "$PROVIDER_ID" \ --project="$PROJECT" --location=global \ @@ -326,8 +334,13 @@ fi # Narrow grants for identities other than this run's are not removed (they may # belong to a second legitimate CUDly account), but they are surfaced: a re-run # with a corrected role or subject otherwise leaves the previous one impersonating. -OTHER_GRANTS=$(grep -F "iam.googleapis.com/${POOL_RESOURCE}/" <<<"$SA_BINDINGS" \ - | grep -vF -- "$MEMBER" || true) +# The member is compared as a whole field, not with `grep -vF`. A substring +# exclusion drops every line CONTAINING this run's member, so a sibling grant +# whose member merely starts with it is suppressed: pinning CUDly-Execution would +# hide an existing grant to CUDly-Execution-Admin, in exactly the "re-ran with a +# corrected role" case this notice exists to surface. +OTHER_GRANTS=$(awk -F'\t' -v pool="iam.googleapis.com/${POOL_RESOURCE}/" -v m="$MEMBER" \ + 'index($2, pool) && $2 != m' <<<"$SA_BINDINGS" || true) if [[ -n "$OTHER_GRANTS" ]]; then echo "Note: ${SA_EMAIL} is also impersonable by other identities in pool '${POOL_ID}':" >&2 while IFS= read -r grant_line; do