sec(iac/gcp): pin WIF trust to the exact role ARN and narrow the SA grant

iac/federation/gcp-target/terraform/main.tf

@@ -22,6 +22,16 @@ locals {
   project                = var.project != "" ? var.project : data.google_project.current.project_id
   create_service_account = var.service_account_email == ""
   service_account_email  = local.create_service_account ? google_service_account.cudly[0].email : var.service_account_email
+
+  # The single AWS identity this deployment trusts. Both the provider's
+  # attribute_condition and the impersonation grant compare against this exact
+  # string, so they cannot drift apart.
+  #
+  # Standard AWS partition only: a GovCloud or China-partition caller presents
+  # arn:aws-us-gov:... / arn:aws-cn:..., which simply will not equal this value,
+  # so the token exchange is refused rather than admitted on a partition
+  # mismatch.
+  aws_role_arn = "arn:aws:sts::${var.aws_account_id}:assumed-role/${var.aws_role_name}"
 }
 
 resource "google_project_service" "iam" {
@@ -97,16 +107,46 @@ resource "google_iam_workload_identity_pool_provider" "cudly" {
   workload_identity_pool_id          = google_iam_workload_identity_pool.cudly.workload_identity_pool_id
   workload_identity_pool_provider_id = var.provider_id
 
+  # attribute.aws_role normalises the session ARN
+  # (arn:aws:sts::<acct>:assumed-role/<role>/<session>) down to the role ARN, so
+  # the condition below and the IAM grant on the service account can both match
+  # it with == instead of testing for a substring.
+  #
+  # A substring test on the raw assertion.arn is bypassable. AWS IAM user paths
+  # accept any printable ASCII (documented grammar: (/)|(/[!-~]+/)), so an
+  # attacker holding only iam:CreateUser in the trusted account can create a
+  # user at path /assumed-role/<pinned role>/ whose ARN,
+  # arn:aws:iam::<acct>:user/assumed-role/<pinned role>/<name>, contains the
+  # substring the old condition looked for. Normalising first maps that ARN to
+  # arn:aws:iam::<acct>:user/assumed-role/<pinned role>, which is not equal to
+  # the arn:aws:sts::<acct>:assumed-role/<pinned role> pinned below, so it is
+  # refused. STS drops the path from assumed-role ARNs, so the same trick does
+  # not work through an IAM role.
+  #
+  # This expression, the attribute_condition below, the impersonation grant's
+  # member, and the set of keys mapped here are byte-identical to their
+  # counterparts in arm/CUDly-CrossSubscription/setup-gcp-wif.sh, so the two
+  # customer-facing onboarding paths pin the same identity and configure the
+  # same provider. The two paths do converge on one provider rather than staying
+  # independent: both default to pool 'cudly-pool' and provider
+  # 'cudly-provider', so a customer who applies this module and then runs the
+  # script lands on the provider this resource created.
+  #
+  # Do not map an attribute here without mapping it in the script too. The
+  # script refuses to reuse a provider whose configuration differs from the one
+  # it would have written, and the only remedy it offers is deleting the
+  # provider, which detaches every live federated session. An extra key on one
+  # side alone is enough to trigger that, even when nothing reads its value.
+  # TestGCPTargetMappingMatchesSetupScript guards the parity.
   attribute_mapping = var.provider_type == "aws" ? {
     "google.subject"     = "assertion.arn"
-    "attribute.aws_role" = "assertion.arn"
-    "attribute.account"  = "assertion.account"
+    "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"
   } : var.oidc_attribute_mapping
 
   # Both branches are non-null: aws_role_name and oidc_subject are validated
   # as non-empty by lifecycle preconditions below, so neither falls back to null.
   attribute_condition = var.provider_type == "aws" ? (
-    "attribute.aws_role.contains('assumed-role/${var.aws_role_name}/')"
+    "attribute.aws_role == '${local.aws_role_arn}'"
     ) : (
     "google.subject == '${var.oidc_subject}'"
   )
@@ -148,17 +188,49 @@ resource "google_iam_workload_identity_pool_provider" "cudly" {
 }
 
 # Use _member (not _binding) to add one member without replacing existing bindings.
+#
+# Both branches name a single identity, so a future mapping or condition bug
+# cannot widen impersonation beyond the pinned principal:
+#   OIDC: principal://.../subject/<exact sub claim>.
+#   AWS:  principalSet://.../attribute.aws_role/<exact role ARN>. This is the
+#         exact-value form of a principalSet, not a prefix or wildcard: it
+#         admits only identities whose attribute.aws_role equals the value.
+#         Session ARNs carry a per-session suffix, which is why the grant names
+#         the normalised role attribute rather than google.subject; it is not a
+#         reason to fall back to the pool-wide wildcard principalSet this
+#         replaced, which granted impersonation to everything in the pool.
+#
+# Principal identifiers are pool-scoped, not provider-scoped: any provider added
+# to this pool that can mint the same attribute value satisfies this grant. Keep
+# var.pool_id dedicated to CUDly unless you intend to share it.
 resource "google_service_account_iam_member" "cudly_wif" {
   service_account_id = "projects/${local.project}/serviceAccounts/${local.service_account_email}"
   role               = "roles/iam.workloadIdentityUser"
-  # For OIDC: scope binding to the specific subject (oidc_subject is required; see
-  # lifecycle precondition above), so only that subject can impersonate this SA.
-  # For AWS: wildcard principalSet is intentional; session ARNs include variable
-  # session names so exact-match principalSet cannot work. Trust is scoped by the
-  # mandatory attribute_condition on the provider (aws_role_name is required).
   member = var.provider_type == "oidc" ? (
     "principal://iam.googleapis.com/${google_iam_workload_identity_pool.cudly.name}/subject/${var.oidc_subject}"
     ) : (
-    "principalSet://iam.googleapis.com/${google_iam_workload_identity_pool.cudly.name}/*"
+    "principalSet://iam.googleapis.com/${google_iam_workload_identity_pool.cudly.name}/attribute.aws_role/${local.aws_role_arn}"
   )
+
+  # Upgrade ordering for deployments applied before the pool-wide grant was
+  # narrowed. `member` is ForceNew, so changing it replaces this resource, and
+  # the wrong order breaks federation for live customers:
+  #
+  #   depends_on:            attribute_mapping and attribute_condition are
+  #                          updated in place by the provider (neither is
+  #                          ForceNew; both go into the PATCH updateMask), so the
+  #                          provider is reconfigured first. Until that lands,
+  #                          attribute.aws_role still holds the raw session ARN
+  #                          and the new member below would match nothing.
+  #   create_before_destroy: adds the narrow member while the old pool-wide one
+  #                          is still present, so there is no window in which the
+  #                          service account has neither.
+  #
+  # Net apply order: reconfigure provider -> add narrow member -> remove the old
+  # pool-wide member. Every step keeps at least one matching grant in place.
+  depends_on = [google_iam_workload_identity_pool_provider.cudly]
+
+  lifecycle {
+    create_before_destroy = true
+  }
 }

iac/federation/gcp-target/terraform/variables.tf

@@ -54,16 +54,66 @@ variable "oidc_attribute_mapping" {
   }
 }
 
+# Both values below are interpolated into a single-quoted string literal inside
+# the provider's CEL attribute_condition and into the IAM principal identifier of
+# the impersonation grant. The character checks reject the forms that fail OPEN
+# in those two destinations rather than merely looking wrong:
+#
+#   ' " \ `  terminate the CEL string literal, so the rest of the value rewrites
+#            the condition; aws_role_name = "x') || true || ('" renders an
+#            always-true condition that admits every identity;
+#   *        widens an IAM principal identifier to match every identity;
+#   $        catches a pasted ${...} placeholder, which is not expanded here and
+#            yields a binding matching nothing (or, where expanded, everything).
+#
+# The format checks that follow already exclude those characters. They are kept
+# as separate validations because Terraform reports every failing validation, and
+# "this would rewrite the attribute condition" is far more actionable than a bare
+# charset regex when someone pastes a crafted value.
+
 variable "aws_role_name" {
   description = "AWS IAM role name to restrict trust to (e.g. 'CUDly-Execution'). Required when provider_type is 'aws'. Without this the attribute_condition is null and any IAM role in the account can federate."
   type        = string
   default     = ""
+
+  validation {
+    condition     = !can(regex("['\"\\\\`*$]", var.aws_role_name))
+    error_message = "aws_role_name must not contain quotes, backslashes, backticks, '*' or '$'. It is interpolated into the provider's CEL attribute condition and into the IAM principal identifier of the impersonation grant, where those characters can escape the string literal or widen the grant to every identity."
+  }
+
+  # Matches AWS's own role-name charset [\w+=,.@-]{1,64}. The comma is safe here:
+  # the role name reaches attribute_condition (a plain string), never the
+  # comma-delimited attribute_mapping keys.
+  validation {
+    condition     = var.aws_role_name == "" || can(regex("^[A-Za-z0-9_+=,.@-]{1,64}$", var.aws_role_name))
+    error_message = "aws_role_name must be a bare IAM role name of 1-64 characters from [A-Za-z0-9_+=,.@-] (e.g. \"CUDly-Execution\"), not an ARN or a path."
+  }
 }
 
 variable "oidc_subject" {
   description = "OIDC subject claim to restrict trust to. Required when provider_type is 'oidc'. Without this the attribute_condition is null and any subject from the issuer can federate."
   type        = string
   default     = ""
+
+  validation {
+    condition     = !can(regex("['\"\\\\`*$]", var.oidc_subject))
+    error_message = "oidc_subject must not contain quotes, backslashes, backticks, '*' or '$'. It is interpolated into the provider's CEL attribute condition and into the IAM principal identifier of the impersonation grant, where those characters can escape the string literal or widen the grant to every identity."
+  }
+
+  # '|' 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.
+  validation {
+    condition     = var.oidc_subject == "" || can(regex("^[A-Za-z0-9][-A-Za-z0-9._:/@=+~|]*$", var.oidc_subject))
+    error_message = "oidc_subject must start with an alphanumeric character and contain only [-A-Za-z0-9._:/@=+~|] (e.g. \"repo:my-org/my-repo:ref:refs/heads/main\")."
+  }
+
+  # oidc_subject becomes google.subject, which GCP caps at 127 characters. A
+  # longer value is rejected at token-exchange time, long after apply succeeds.
+  validation {
+    condition     = length(var.oidc_subject) <= 127
+    error_message = "oidc_subject must be at most 127 characters: it becomes google.subject, which GCP rejects above that length at token-exchange time."
+  }
 }
 
 # ------------------------------------------------------------------------