sec(iac/aws): reject the all-zeros OIDC thumbprint in the aws-target bundles

iac/federation/aws-target/cloudformation/template.yaml

@@ -53,12 +53,46 @@ Parameters:
   OIDCThumbprint:
     Type: String
     Description: >
-      SHA-1 thumbprint of the OIDC provider root CA certificate.
-      For GCP (accounts.google.com) use: 08745487e891c19e3078c1f2a07e452950ef36f6
-      For Azure AD use the thumbprint of the DigiCert Global Root CA.
-      Leave as zeros only for well-known providers where AWS auto-validates the chain.
-    Default: "0000000000000000000000000000000000000000"
-    AllowedPattern: "^[0-9a-fA-F]{40}$"
+      OPTIONAL. SHA-1 thumbprint (40 hex characters) of the top intermediate CA
+      that signed the TLS certificate of the issuer's JWKS endpoint.
+      LEAVE THIS EMPTY unless that certificate does not chain to a publicly
+      trusted CA. Empty omits the ThumbprintList property entirely, and IAM then
+      retrieves the correct thumbprint itself, which is what the IAM console
+      does by default.
+      AWS verifies the JWKS endpoint's TLS certificate against its own library of
+      trusted root CAs, and falls back to this thumbprint only when that
+      certificate does not chain to one of them, when AWS cannot retrieve the
+      certificate, or when the endpoint requires TLS 1.3. Both documented
+      issuers (login.microsoftonline.com and accounts.google.com) present
+      publicly trusted certificates, so for them the value is read only if one
+      of those fallback conditions applies, whatever it is set to.
+      The all-zeros placeholder that used to be the default is now rejected. It
+      does not disable chain verification. It is simply not the SHA-1 of any
+      certificate, so on the fallback path it matches nothing and every
+      AssumeRoleWithWebIdentity call fails. It nulls out the only check that
+      path has while reading like a deliberate configuration.
+      Note: when an issuer's discovery endpoint and jwks_uri are on different
+      hosts, AWS requires the thumbprints of BOTH. This parameter carries a
+      single value, so such an issuer must be onboarded with the AWS CLI or the
+      IAM console instead of this template.
+    Default: ""
+    MaxLength: 40
+    # Empty, or up to 40 hex characters of which at least one is non-zero.
+    # The middle character class is what rejects the all-zeros placeholder: do
+    # not "simplify" this back to ^$|^[0-9a-fA-F]{40}$, which readmits it.
+    # MaxLength caps the upper end (the pattern alone would admit up to 79
+    # characters). A well-formed-but-short value (1-39 hex) passes here and is
+    # rejected by the IAM API, which fixes the length at 40: fail-closed, with
+    # the stack rolling back rather than deploying a broken provider.
+    AllowedPattern: "^$|^[0-9a-fA-F]{0,39}[1-9a-fA-F][0-9a-fA-F]{0,39}$"
+    ConstraintDescription: >
+      OIDCThumbprint must be empty (recommended, since IAM then retrieves the
+      thumbprint itself), or a 40-character hex SHA-1 thumbprint containing at
+      least one non-zero digit.
+      The all-zeros placeholder is rejected because it is not the fingerprint of
+      any certificate: whenever AWS actually falls back to thumbprint
+      verification it matches nothing, so role assumption fails outright. Supply
+      the issuer's real thumbprint, or leave this empty.
 
   OIDCSubjectClaim:
     Type: String
@@ -99,6 +133,26 @@ Parameters:
 
 Conditions:
   HasAudience: !Not [!Equals [!Ref OIDCAudience, ""]]
+  HasThumbprint: !Not [!Equals [!Ref OIDCThumbprint, ""]]
+
+# Rules are evaluated before CloudFormation creates or updates any resource; a
+# failed assertion aborts the stack operation. This duplicates the all-zeros
+# rejection already encoded in the OIDCThumbprint AllowedPattern on purpose:
+# the two are independent, so neither a regex "simplification" nor a submission
+# path that skips one of the layers can readmit the placeholder on its own.
+Rules:
+  RejectPlaceholderThumbprint:
+    Assertions:
+      - Assert: !Not
+          - !Equals
+            - !Ref OIDCThumbprint
+            - "0000000000000000000000000000000000000000"
+        AssertDescription: >
+          OIDCThumbprint must not be the all-zeros placeholder. It is not the
+          SHA-1 fingerprint of any certificate, so on the one path where AWS
+          still consults a thumbprint it matches nothing and every
+          AssumeRoleWithWebIdentity call fails. Leave OIDCThumbprint empty to
+          have IAM retrieve the real thumbprint, or supply the issuer's own.
 
 Resources:
   OIDCProvider:
@@ -107,8 +161,15 @@ Resources:
       Url: !Ref OIDCIssuerURL
       ClientIdList:
         - !If [HasAudience, !Ref OIDCAudience, sts.amazonaws.com]
-      ThumbprintList:
-        - !Ref OIDCThumbprint
+      # Omitted entirely when OIDCThumbprint is empty, so IAM retrieves and uses
+      # the issuer's real top intermediate CA thumbprint instead of whatever
+      # placeholder the operator was handed. ThumbprintList is optional on
+      # AWS::IAM::OIDCProvider and updates with no interruption, so switching
+      # between the two forms never replaces the provider or changes its ARN.
+      ThumbprintList: !If
+        - HasThumbprint
+        - [!Ref OIDCThumbprint]
+        - !Ref "AWS::NoValue"
 
   CUDlyPolicy:
     Type: AWS::IAM::ManagedPolicy

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

@@ -23,9 +23,20 @@ locals {
 }
 
 resource "aws_iam_openid_connect_provider" "cudly" {
-  url             = local.oidc_issuer_url_normalized
-  client_id_list  = [local.audience]
-  thumbprint_list = var.thumbprint_list
+  url            = local.oidc_issuer_url_normalized
+  client_id_list = [local.audience]
+
+  # null, not [], when no thumbprint is configured: thumbprint_list is optional
+  # on this resource, and leaving it unset is what makes IAM retrieve and use
+  # the issuer's real top intermediate CA thumbprint. An empty list would
+  # instead send a provider that has no thumbprints at all.
+  #
+  # Note for existing deployments: the argument is Optional+Computed, so
+  # clearing it does not clear the value already stored on an existing
+  # provider -- Terraform keeps reading back what is there. A provider created
+  # with the old all-zeros default must be corrected out of band with
+  # `aws iam update-open-id-connect-provider-thumbprint`, or replaced.
+  thumbprint_list = length(var.thumbprint_list) > 0 ? var.thumbprint_list : null
 }
 
 resource "aws_iam_policy" "cudly" {

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

@@ -86,20 +86,26 @@ variable "role_name" {
 
 variable "thumbprint_list" {
   description = <<-EOT
-    TLS root CA thumbprints for the OIDC provider (40-character hex SHA-1).
-    AWS auto-validates well-known providers (Azure AD, Google); for those the
-    all-zeros placeholder is intentional and accepted.
-    For any other issuer you MUST supply the real root CA SHA-1 thumbprint.
-    Supplying the all-zeros placeholder for a custom issuer is rejected by this
-    module to prevent operators from silently bypassing CA-chain validation.
+    OPTIONAL. TLS thumbprints (40-character hex SHA-1) of the top intermediate
+    CA that signed the certificate of the issuer's JWKS endpoint.
+
+    LEAVE THIS EMPTY unless that certificate does not chain to a publicly
+    trusted CA. An empty list omits the argument entirely, and IAM then
+    retrieves the correct thumbprint itself.
+
+    AWS verifies the JWKS endpoint's TLS certificate against its own library of
+    trusted root CAs, and falls back to these thumbprints only when that
+    certificate does not chain to one of them, when AWS cannot retrieve the
+    certificate, or when the endpoint requires TLS 1.3. Both documented
+    issuers (login.microsoftonline.com and accounts.google.com) present
+    publicly trusted certificates, so for them the value is read only if one of
+    those fallback conditions applies, whatever it is set to.
+
+    When the issuer's discovery endpoint and jwks_uri are on different hosts,
+    AWS requires the thumbprints of BOTH; supply both entries in that case.
   EOT
   type        = list(string)
-  default     = ["0000000000000000000000000000000000000000"]
-
-  validation {
-    condition     = length(var.thumbprint_list) > 0
-    error_message = "thumbprint_list must contain at least one thumbprint."
-  }
+  default     = []
 
   validation {
     condition = alltrue([
@@ -108,18 +114,22 @@ variable "thumbprint_list" {
     error_message = "Each thumbprint in thumbprint_list must be a 40-character SHA-1 hex string."
   }
 
-  # Guard against copy-paste of the all-zeros placeholder for custom OIDC
-  # issuers. AWS natively validates Azure AD and Google endpoints, so
-  # all-zeros is safe for those. Any other issuer URL requires a real CA
-  # thumbprint; the all-zeros value bypasses the CA-chain check entirely.
+  # The all-zeros placeholder used to be this variable's default, guarded by an
+  # issuer allowlist justified as "the all-zeros value bypasses the CA-chain
+  # check entirely". That justification was wrong in both directions, so the
+  # guard is now unconditional and the default is empty.
+  #
+  # All-zeros bypasses nothing: it is simply not the SHA-1 of any certificate.
+  # On the primary path AWS does not read it at all, and on the fallback path it
+  # matches nothing, so role assumption fails outright. Restricting it to Azure AD and
+  # Google was equally beside the point -- the thumbprint is unread for every
+  # publicly-trusted issuer, not only those two, and setting it for a
+  # private-CA issuer breaks that issuer no matter which one it is.
   validation {
-    condition = !(
-      length(var.thumbprint_list) == 1 &&
-      var.thumbprint_list[0] == "0000000000000000000000000000000000000000" &&
-      !startswith(var.oidc_issuer_url, "https://login.microsoftonline.com/") &&
-      !startswith(var.oidc_issuer_url, "https://accounts.google.com")
-    )
-    error_message = "thumbprint_list is the all-zeros placeholder, which is only safe for Azure AD (login.microsoftonline.com) and Google (accounts.google.com) issuers that AWS validates natively. For any other OIDC issuer you must supply the real root CA SHA-1 thumbprint."
+    condition = alltrue([
+      for t in var.thumbprint_list : t != "0000000000000000000000000000000000000000"
+    ])
+    error_message = "thumbprint_list must not contain the all-zeros placeholder. It is not the fingerprint of any certificate, so whenever AWS actually falls back to thumbprint verification it matches nothing and role assumption fails. Leave thumbprint_list empty to have IAM retrieve the issuer's real thumbprint, or supply that thumbprint."
   }
 }
 

internal/api/handler_federation_test.go

@@ -8,13 +8,16 @@ import (
 	"encoding/json"
 	"io/fs"
 	"os"
+	"regexp"
+	"strconv"
 	"strings"
 	"testing"
 
 	"github.com/aws/aws-lambda-go/events"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
+	"gopkg.in/yaml.v3"
 )
 
 // federationHandler returns a Handler wired for federation IaC tests.
@@ -1499,3 +1502,154 @@ func TestValidateFederationTargetSource(t *testing.T) {
 		})
 	}
 }
+
+// ---------------------------------------------------------------------------
+// aws-target OIDC provider thumbprint guard (issue #1615)
+// ---------------------------------------------------------------------------
+
+const awsTargetCFNTemplate = "../../iac/federation/aws-target/cloudformation/template.yaml"
+
+// allZeroThumbprint is the placeholder that used to be the OIDCThumbprint
+// default. It is not the SHA-1 fingerprint of any certificate, so on the one
+// path where AWS still consults a thumbprint (the JWKS endpoint's TLS
+// certificate does not chain to a CA in AWS's trusted-root library, AWS cannot
+// retrieve that certificate, or the endpoint requires TLS 1.3) it matches
+// nothing and every AssumeRoleWithWebIdentity call fails.
+const allZeroThumbprint = "0000000000000000000000000000000000000000"
+
+// cfnParamBlock returns the lines of a top-level Parameters entry, from
+// "  <name>:" up to the next sibling key at the same indent.
+func cfnParamBlock(t *testing.T, src, name string) string {
+	t.Helper()
+	start := strings.Index(src, "\n  "+name+":\n")
+	require.GreaterOrEqual(t, start, 0, "parameter %s not found in %s", name, awsTargetCFNTemplate)
+	rest := src[start+1:]
+	lines := strings.Split(rest, "\n")
+	var out []string
+	for i, line := range lines {
+		if i > 0 && len(line) > 2 && line[0] == ' ' && line[1] == ' ' && line[2] != ' ' {
+			break
+		}
+		out = append(out, line)
+	}
+	return strings.Join(out, "\n")
+}
+
+// cfnScalar pulls a scalar property out of a parameter block and decodes it
+// with the YAML decoder, so the value under test is exactly what CloudFormation
+// receives rather than a copy retyped into this test.
+func cfnScalar(t *testing.T, block, key string) string {
+	t.Helper()
+	for _, line := range strings.Split(block, "\n") {
+		trimmed := strings.TrimSpace(line)
+		if !strings.HasPrefix(trimmed, key+":") {
+			continue
+		}
+		var holder struct {
+			V string `yaml:"v"`
+		}
+		require.NoError(t, yaml.Unmarshal([]byte("v:"+strings.TrimPrefix(trimmed, key+":")), &holder),
+			"could not YAML-decode %s from %q", key, trimmed)
+		return holder.V
+	}
+	t.Fatalf("key %s not found in parameter block:\n%s", key, block)
+	return ""
+}
+
+// TestAWSTargetTemplate_RejectsPlaceholderThumbprint is the regression test for
+// issue #1615. The template used to default OIDCThumbprint to the all-zeros
+// placeholder and validate it only as 40 hex characters, which that placeholder
+// satisfies.
+//
+// The AllowedPattern and MaxLength are read back out of the committed template
+// and evaluated the way CloudFormation evaluates them (anchored full match plus
+// the length cap), so a future edit that relaxes either one fails here.
+func TestAWSTargetTemplate_RejectsPlaceholderThumbprint(t *testing.T) {
+	raw, err := os.ReadFile(awsTargetCFNTemplate)
+	require.NoError(t, err)
+	src := string(raw)
+
+	block := cfnParamBlock(t, src, "OIDCThumbprint")
+
+	// The placeholder must not be reachable by leaving the parameter alone.
+	require.Equal(t, "", cfnScalar(t, block, "Default"),
+		"OIDCThumbprint must default to empty so ThumbprintList is omitted and IAM "+
+			"retrieves the issuer's real thumbprint")
+
+	pattern := cfnScalar(t, block, "AllowedPattern")
+	maxLen, err := strconv.Atoi(cfnScalar(t, block, "MaxLength"))
+	require.NoError(t, err, "MaxLength must be an integer")
+
+	re, err := regexp.Compile(pattern)
+	require.NoError(t, err, "AllowedPattern must be a valid regular expression")
+
+	// CloudFormation applies AllowedPattern as a full match and MaxLength
+	// independently; a value is accepted only when it clears both.
+	accepted := func(v string) bool {
+		return len(v) <= maxLen && re.FindString(v) == v && re.MatchString(v)
+	}
+
+	mustAccept := []struct{ value, why string }{
+		{"", "empty: ThumbprintList is omitted and IAM retrieves the real thumbprint"},
+		{"08745487e891c19e3078c1f2a07e452950ef36f6", "a real root CA thumbprint"},
+		{"990F4193972F2BECF12DDEDA5237F9C952F20D9E", "uppercase hex"},
+		{"1" + strings.Repeat("0", 39), "non-zero digit in the first position only"},
+		{strings.Repeat("0", 39) + "1", "non-zero digit in the last position only"},
+		{strings.Repeat("0", 20) + "9" + strings.Repeat("0", 19), "non-zero digit in the middle only"},
+		{strings.Repeat("0", 39) + "F", "non-zero position holding an uppercase hex letter"},
+		{strings.Repeat("0", 39) + "a", "non-zero position holding a lowercase hex letter"},
+	}
+	for _, tc := range mustAccept {
+		assert.True(t, accepted(tc.value), "must accept %q (%s)", tc.value, tc.why)
+	}
+
+	mustReject := []struct{ value, why string }{
+		{allZeroThumbprint, "the #1615 placeholder itself"},
+		{strings.Repeat("0", 39), "all zeros, one character short"},
+		{strings.Repeat("0", 41), "all zeros, one character long"},
+		{"0", "a single zero"},
+		{strings.Repeat("a", 41), "41 hex characters: caught by MaxLength, not the pattern"},
+		{strings.Repeat("a", 40) + "\n", "trailing newline: MaxLength rejects it even if $ matched before it"},
+		{strings.Repeat("g", 40), "not hexadecimal"},
+		{"0x" + strings.Repeat("0", 38), "hex-literal prefix"},
+		{" " + strings.Repeat("a", 39), "leading whitespace"},
+		{strings.Repeat("a", 39) + " ", "trailing whitespace"},
+		{"*", "wildcard"},
+		{"${accounts.google.com:sub}", "an IAM policy variable"},
+	}
+	for _, tc := range mustReject {
+		assert.False(t, accepted(tc.value), "must reject %q (%s)", tc.value, tc.why)
+	}
+}
+
+// TestAWSTargetTemplate_ThumbprintWiring asserts the second, independent layer
+// of the #1615 guard and the resource wiring it protects. The Rules assertion
+// duplicates the AllowedPattern's rejection of the placeholder on purpose, so
+// neither a regex "simplification" nor a submission path that skips one layer
+// can readmit it alone.
+func TestAWSTargetTemplate_ThumbprintWiring(t *testing.T) {
+	raw, err := os.ReadFile(awsTargetCFNTemplate)
+	require.NoError(t, err)
+	src := string(raw)
+
+	assert.Contains(t, src, "Rules:",
+		"template must declare a Rules section; rules are evaluated before any resource is created")
+	assert.Equal(t, 1, strings.Count(src, allZeroThumbprint),
+		"the all-zeros placeholder must appear exactly once in the template: in the "+
+			"Rules assertion that rejects it, and nowhere else")
+	assert.Contains(t, src, `HasThumbprint: !Not [!Equals [!Ref OIDCThumbprint, ""]]`,
+		"HasThumbprint must be defined as a non-empty OIDCThumbprint")
+
+	// The empty branch must omit ThumbprintList entirely rather than send an
+	// empty or placeholder list, which is what makes IAM retrieve the real
+	// thumbprint for itself.
+	assert.Contains(t, src, `ThumbprintList: !If`,
+		"ThumbprintList must be conditional on HasThumbprint")
+	assert.Contains(t, src, `- !Ref "AWS::NoValue"`,
+		"the empty branch must resolve to AWS::NoValue so the property is omitted")
+
+	// The placeholder must appear only inside the Rules assertion that rejects
+	// it, never as a parameter default or a resource value.
+	assert.NotContains(t, src, `Default: "`+allZeroThumbprint+`"`,
+		"the all-zeros placeholder must not be reintroduced as a default")
+}