diff --git a/internal/iacfiles/templates/aws-wif-cli.sh.tmpl b/internal/iacfiles/templates/aws-wif-cli.sh.tmpl index ded3c0f0..0929ae15 100644 --- a/internal/iacfiles/templates/aws-wif-cli.sh.tmpl +++ b/internal/iacfiles/templates/aws-wif-cli.sh.tmpl @@ -6,18 +6,13 @@ # Usage: # bash {{.AccountSlug}}-aws-wif-cli.sh # -# Requirements: aws-cli v2, credentials for target account {{.AccountExternalID}}. +# Requirements: aws-cli v2, jq, credentials for target account {{.AccountExternalID}}. set -euo pipefail ROLE_NAME="${ROLE_NAME:-CUDly-{{.AccountSlug}}}" OIDC_ISSUER_URL="${OIDC_ISSUER_URL:-{{.OIDCIssuerURL}}}" -OIDC_AUDIENCE="${OIDC_AUDIENCE:-{{.OIDCAudience}}}" -# REQUIRED: restricts which OIDC subject can assume the role. Without a :sub -# condition the trust policy accepts every identity the issuer can mint — the -# same hole #1543/#1602 closed in the CloudFormation template — so this is -# validated below rather than left to build a subject-less policy silently. -# Matches the required TF variable oidc_subject_claim and the required -# CloudFormation OIDCSubjectClaim parameter. +OIDC_AUDIENCE="${OIDC_AUDIENCE-{{.OIDCAudience}}}" +OIDC_AUDIENCE="${OIDC_AUDIENCE:-sts.amazonaws.com}" OIDC_SUBJECT_CLAIM="${OIDC_SUBJECT_CLAIM:-{{.OIDCSubjectClaim}}}" if [[ -z "${OIDC_SUBJECT_CLAIM}" ]]; then echo "Error: OIDC_SUBJECT_CLAIM is required. Without it the trust policy has no" >&2 @@ -29,7 +24,7 @@ if [[ -z "${OIDC_SUBJECT_CLAIM}" ]]; then exit 1 fi case "${OIDC_SUBJECT_CLAIM}" in - *[[:space:]]*|*'$'*|*'*'*) + *[$' \t\n\r\v\f']*|*'$'*|*'*'*) echo "Error: OIDC_SUBJECT_CLAIM must not contain whitespace, '\$' or '*'." >&2 echo " IAM expands \${...} policy variables inside Condition values, so a" >&2 echo " value such as \${accounts.google.com:sub} would expand to the token's" >&2 @@ -39,45 +34,68 @@ case "${OIDC_SUBJECT_CLAIM}" in exit 1 ;; esac +case "${OIDC_AUDIENCE}" in + *[$' \t\n\r\v\f']*|*'$'*|*'*'*) + echo "Error: OIDC_AUDIENCE must not contain whitespace, '\$' or '*'." >&2 + exit 1 ;; +esac +# Keep this issuer check equivalent to aws-target/terraform/variables.tf. +if ! [[ "$OIDC_ISSUER_URL" =~ ^https://[^/]+(/[^/].*[^/]|/[^/]+)?$ ]] || [[ "$OIDC_ISSUER_URL" == */ ]]; then + echo "Error: OIDC_ISSUER_URL must start with https:// and must not end with a trailing slash." >&2 + exit 1 +fi +command -v jq >/dev/null || { echo "Error: jq is required." >&2; exit 1; } PROFILE_ARG="" if [[ -n "${AWS_PROFILE:-}" ]]; then PROFILE_ARG="--profile ${AWS_PROFILE}"; fi -OIDC_HOST=$(echo "${OIDC_ISSUER_URL}" | sed -E 's#^https://##; s#/$##') +OIDC_HOST="${OIDC_ISSUER_URL#https://}" echo "Creating/ensuring OIDC provider for ${OIDC_ISSUER_URL}..." # shellcheck disable=SC2086 -EXISTING_PROVIDER=$(aws iam list-open-id-connect-providers $PROFILE_ARG \ - --query "OpenIDConnectProviderList[?contains(Arn, '${OIDC_HOST}')].Arn | [0]" \ - --output text 2>/dev/null || echo "None") -if [[ "${EXISTING_PROVIDER}" == "None" || -z "${EXISTING_PROVIDER}" ]]; then +PROVIDERS=$(aws iam list-open-id-connect-providers $PROFILE_ARG --output json) +PROVIDER_ARN=$(jq -er --arg host "$OIDC_HOST" ' + if (.OpenIDConnectProviderList | type) != "array" then error("Invalid provider list") + else [.OpenIDConnectProviderList[].Arn | select(split(":oidc-provider/")[1] == $host)] + | if length > 1 then error("Multiple providers match the issuer") else .[0] // "" end + end' <<< "$PROVIDERS") +if [[ -z "$PROVIDER_ARN" ]]; then # shellcheck disable=SC2086 - aws iam create-open-id-connect-provider $PROFILE_ARG \ + CREATED_PROVIDER=$(aws iam create-open-id-connect-provider $PROFILE_ARG \ --url "${OIDC_ISSUER_URL}" \ --client-id-list "${OIDC_AUDIENCE}" \ - --thumbprint-list "0000000000000000000000000000000000000000" >/dev/null + --output json) + PROVIDER_ARN=$(jq -er '.OpenIDConnectProviderArn | select(type == "string" and length > 0)' <<< "$CREATED_PROVIDER") +fi +if ! [[ "$PROVIDER_ARN" =~ ^arn:[a-z0-9-]+:iam::[0-9]{12}:oidc-provider/ ]] || [[ "${PROVIDER_ARN#*:oidc-provider/}" != "$OIDC_HOST" ]]; then + echo "Error: returned provider ARN does not match OIDC_ISSUER_URL." >&2 + exit 1 fi # shellcheck disable=SC2086 -PROVIDER_ARN=$(aws iam list-open-id-connect-providers $PROFILE_ARG \ - --query "OpenIDConnectProviderList[?contains(Arn, '${OIDC_HOST}')].Arn | [0]" \ - --output text) +PROVIDER=$(aws iam get-open-id-connect-provider $PROFILE_ARG --open-id-connect-provider-arn "$PROVIDER_ARN" --output json) +if ! jq -e --arg host "$OIDC_HOST" --arg audience "$OIDC_AUDIENCE" ' + .Url == $host and + (.ClientIDList | type == "array" and all(.[]; type == "string") and index($audience) != null) and + (.ThumbprintList | type == "array" and all(.[]; type == "string" and + test("^[0-9a-fA-F]{40}$") and . != "0000000000000000000000000000000000000000")) + ' <<< "$PROVIDER" >/dev/null; then + echo "Error: provider URL, audience or thumbprints do not match. Inspect $PROVIDER_ARN before rerunning; shared providers are not modified." >&2 + exit 1 +fi -# Build trust policy. OIDC_SUBJECT_CLAIM is validated non-empty above, so the -# :sub condition below is always present — there is no branch that omits it. -TRUST_POLICY=$(cat <> %q +[[ "$1 $2" != "${FAIL_OPERATION:-}" ]] || exit 42 +case "$1 $2" in + 'iam list-open-id-connect-providers') + if [[ "$*" == *'--output text'* ]]; then + jq -r --arg host "$TEST_HOST" '[.OpenIDConnectProviderList[].Arn | select(contains($host))][0] // "None"' <<< "$LIST_RESPONSE" + else + printf '%%s\n' "$LIST_RESPONSE" + fi ;; + 'iam create-open-id-connect-provider') printf '%%s\n' "$CREATE_RESPONSE" ;; + 'iam get-open-id-connect-provider') printf '%%s\n' "$PROVIDER_RESPONSE" ;; + 'iam get-role') echo 'arn:aws:iam::123456789012:role/CUDly' ;; + 'sts get-caller-identity') echo 123456789012 ;; + 'iam create-role'|'iam put-role-policy') echo '{}' ;; + *) exit 43 ;; +esac +`, logPath) +} + +func runAWSWIF(t *testing.T, data testTemplateData, overrides map[string]string) (int, string, string, [][]string) { + t.Helper() + host := strings.TrimPrefix(data.OIDCIssuerURL, "https://") + arn := "arn:aws:iam::123456789012:oidc-provider/" + host + provider, err := json.Marshal(map[string]any{ + "Url": host, "ClientIDList": []string{data.OIDCAudience, "sts.amazonaws.com"}, + "ThumbprintList": []string{strings.Repeat("a", 40)}, + }) + require.NoError(t, err) + env := map[string]string{ + "TEST_HOST": host, "LIST_RESPONSE": `{"OpenIDConnectProviderList":[]}`, + "CREATE_RESPONSE": fmt.Sprintf(`{"OpenIDConnectProviderArn":%q}`, arn), + "PROVIDER_RESPONSE": string(provider), + } + for key, value := range overrides { + env[key] = value + } + code, stdout, stderr, lines := runRenderedScript(t, "aws-wif-cli.sh", + renderCLITemplate(t, "templates/aws-wif-cli.sh.tmpl", data), + func(logPath string) map[string]string { + return map[string]string{"aws": awsWIFStub(logPath), "curl": fmt.Sprintf("#!/usr/bin/env bash\njq -cn --args '[\"curl\"] + $ARGS.positional' -- \"$@\" >> %q\necho 409\n", logPath)} + }, env) + calls := make([][]string, 0, len(lines)) + for _, line := range lines { + var args []string + require.NoError(t, json.Unmarshal([]byte(line), &args)) + calls = append(calls, args) + } + return code, stdout, stderr, calls +} + +func awsWIFData() testTemplateData { + data := baseData() + data.OIDCIssuerURL = "https://accounts.google.com" + data.OIDCAudience = "sts.amazonaws.com" + data.OIDCSubjectClaim = "123456789012345678901" + return data +} + +func awsWIFOperation(calls [][]string, operation string) []string { + for _, call := range calls { + if len(call) > 1 && call[1] == operation { + return call + } + } + return nil +} + +func awsWIFArgument(t *testing.T, call []string, flag string) string { + t.Helper() + for i, arg := range call { + if arg == flag && i+1 < len(call) { + return call[i+1] + } + } + t.Fatalf("missing %s in %v", flag, call) + return "" +} + +func TestAWSWIFCLIProviderGuards(t *testing.T) { + listed := `{"OpenIDConnectProviderList":[{"Arn":"` + awsWIFProvider + `.evil.example"},{"Arn":"` + awsWIFProvider + `/other"},{"Arn":"` + awsWIFProvider + `"}]}` + cases := []struct { + name string + env map[string]string + create, reject bool + }{ + {"absent", nil, true, false}, + {"exact with lookalikes", map[string]string{"LIST_RESPONSE": listed}, false, false}, + {"only lookalike", map[string]string{"LIST_RESPONSE": `{"OpenIDConnectProviderList":[{"Arn":"` + awsWIFProvider + `.evil.example"}]}`}, true, false}, + {"list denied", map[string]string{"FAIL_OPERATION": "iam list-open-id-connect-providers"}, false, true}, + {"create denied", map[string]string{"FAIL_OPERATION": "iam create-open-id-connect-provider"}, true, true}, + {"get denied", map[string]string{"FAIL_OPERATION": "iam get-open-id-connect-provider"}, true, true}, + {"missing audience", map[string]string{"LIST_RESPONSE": listed, "PROVIDER_RESPONSE": `{"Url":"accounts.google.com","ClientIDList":["other"],"ThumbprintList":[]}`}, false, true}, + {"wrong URL", map[string]string{"PROVIDER_RESPONSE": `{"Url":"accounts.google.com.evil","ClientIDList":["sts.amazonaws.com"],"ThumbprintList":[]}`}, true, true}, + {"zero thumbprint", map[string]string{"PROVIDER_RESPONSE": `{"Url":"accounts.google.com","ClientIDList":["sts.amazonaws.com"],"ThumbprintList":["0000000000000000000000000000000000000000"]}`}, true, true}, + {"malformed list", map[string]string{"LIST_RESPONSE": `not-json`}, false, true}, + {"malformed provider", map[string]string{"PROVIDER_RESPONSE": `{}`}, true, true}, + {"wrong created ARN", map[string]string{"CREATE_RESPONSE": `{"OpenIDConnectProviderArn":"` + awsWIFProvider + `.evil"}`}, true, true}, + {"duplicate exact", map[string]string{"LIST_RESPONSE": `{"OpenIDConnectProviderList":[{"Arn":"` + awsWIFProvider + `"},{"Arn":"` + awsWIFProvider + `"}]}`}, false, true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + code, stdout, stderr, calls := runAWSWIF(t, awsWIFData(), tc.env) + require.Equal(t, tc.create, awsWIFOperation(calls, "create-open-id-connect-provider") != nil, "%v", calls) + if tc.reject { + require.NotZero(t, code, "stdout=%s stderr=%s", stdout, stderr) + require.Nil(t, awsWIFOperation(calls, "create-role")) + require.Nil(t, awsWIFOperation(calls, "put-role-policy")) + require.NotContains(t, stdout, "=== Done ===") + for _, call := range calls { + require.NotEqual(t, "curl", call[0], "registration after failed provider verification") + } + return + } + require.Zero(t, code, "%s", stderr) + if tc.create { + require.NotContains(t, awsWIFOperation(calls, "create-open-id-connect-provider"), "--thumbprint-list") + } + require.Equal(t, awsWIFProvider, awsWIFArgument(t, awsWIFOperation(calls, "get-open-id-connect-provider"), "--open-id-connect-provider-arn")) + var policy map[string]any + require.NoError(t, json.Unmarshal([]byte(awsWIFArgument(t, awsWIFOperation(calls, "create-role"), "--assume-role-policy-document")), &policy)) + want := `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"Federated":"` + awsWIFProvider + `"},"Action":"sts:AssumeRoleWithWebIdentity","Condition":{"StringEquals":{"accounts.google.com:aud":"sts.amazonaws.com","accounts.google.com:sub":"123456789012345678901"}}}]}` + actual, err := json.Marshal(policy) + require.NoError(t, err) + require.JSONEq(t, want, string(actual)) + }) + } +} + +func TestAWSWIFCLIInputGuards(t *testing.T) { + for _, key := range []string{"OIDC_AUDIENCE", "OIDC_SUBJECT_CLAIM"} { + for _, invalid := range []string{"a b", "a\tb", "a\nb", "a\rb", "a\vb", "a\fb", "a$b", "a*b"} { + t.Run(key+"/"+fmt.Sprintf("%q", invalid), func(t *testing.T) { + code, _, stderr, calls := runAWSWIF(t, awsWIFData(), map[string]string{key: invalid}) + require.NotZero(t, code) + require.Empty(t, calls) + require.Contains(t, stderr, key) + }) + } + } + for _, issuer := range []string{"http://accounts.google.com", "https://accounts.google.com/", "https://"} { + t.Run(issuer, func(t *testing.T) { + code, _, stderr, calls := runAWSWIF(t, awsWIFData(), map[string]string{"OIDC_ISSUER_URL": issuer}) + require.NotZero(t, code) + require.Empty(t, calls) + require.Contains(t, stderr, "OIDC_ISSUER_URL") + }) + } + t.Run("missing jq", func(t *testing.T) { + code, _, stderr, calls := runAWSWIF(t, awsWIFData(), map[string]string{"PATH": t.TempDir()}) + require.NotZero(t, code) + require.Empty(t, calls) + require.Contains(t, stderr, "jq is required") + }) +} + +func TestAWSWIFCLIAudienceAndLiteralClaims(t *testing.T) { + for _, tc := range []struct { + name, rendered, want string + override map[string]string + }{ + {"Azure default", "api://AzureADTokenExchange", "api://AzureADTokenExchange", nil}, + {"rendered empty", "", "sts.amazonaws.com", nil}, + {"explicit empty", "api://AzureADTokenExchange", "sts.amazonaws.com", map[string]string{"OIDC_AUDIENCE": ""}}, + {"literal override", "default", `a"b\c`, map[string]string{"OIDC_AUDIENCE": `a"b\c`}}, + } { + t.Run(tc.name, func(t *testing.T) { + data := awsWIFData() + data.OIDCAudience = tc.rendered + if tc.name == "Azure default" { + data.Source = "azure" + data.OIDCIssuerURL = "https://login.microsoftonline.com/11111111-1111-1111-1111-111111111111/v2.0" + } + host := strings.TrimPrefix(data.OIDCIssuerURL, "https://") + env := map[string]string{"OIDC_SUBJECT_CLAIM": `a"b\c`} + for k, v := range tc.override { + env[k] = v + } + body, err := json.Marshal(map[string]any{"Url": host, "ClientIDList": []string{tc.want}, "ThumbprintList": []string{strings.Repeat("a", 40)}}) + require.NoError(t, err) + env["PROVIDER_RESPONSE"] = string(body) + code, _, stderr, calls := runAWSWIF(t, data, env) + require.Zero(t, code, "%s", stderr) + require.Equal(t, tc.want, awsWIFArgument(t, awsWIFOperation(calls, "create-open-id-connect-provider"), "--client-id-list")) + var policy struct { + Statement []struct { + Condition struct{ StringEquals map[string]string } + } + } + require.NoError(t, json.Unmarshal([]byte(awsWIFArgument(t, awsWIFOperation(calls, "create-role"), "--assume-role-policy-document")), &policy)) + require.Len(t, policy.Statement, 1) + require.Equal(t, map[string]string{host + ":aud": tc.want, host + ":sub": `a"b\c`}, policy.Statement[0].Condition.StringEquals) + }) + } +} diff --git a/internal/iacfiles/templates_test.go b/internal/iacfiles/templates_test.go index 28d2e280..58c55158 100644 --- a/internal/iacfiles/templates_test.go +++ b/internal/iacfiles/templates_test.go @@ -102,7 +102,7 @@ func TestCLITemplatesAutoRegister(t *testing.T) { `TARGET_ACCOUNT_ID=$(aws sts get-caller-identity`, // #1640: the trust policy's :sub condition must be present // unconditionally — there is no longer a code path that omits it. - `"${OIDC_HOST}:sub": "${OIDC_SUBJECT_CLAIM}"`, + `($host + ":sub"): $subject`, }, mustNot: []string{ "/api/registrations", @@ -237,7 +237,7 @@ func TestAWSWIFCLI_SubjectClaimRequired(t *testing.T) { // Exactly one Condition block, and it always carries :sub — no branch // builds a StringEquals map with only :aud. - subCondition := `"${OIDC_HOST}:sub": "${OIDC_SUBJECT_CLAIM}"` + subCondition := `($host + ":sub"): $subject` if n := strings.Count(rendered, subCondition); n != 1 { t.Errorf("expected exactly one :sub condition in the rendered trust policy, found %d", n) } @@ -276,9 +276,12 @@ func awsStubScript(logPath string) string { // JSON, so newlines inside the arguments are folded to spaces first. "args=\"$*\"\n" + "printf '%s\\n' \"${args//$'\\n'/ }\" >> '" + logPath + "'\n" + - // "None" is what the script's provider-lookup branches expect when no - // OIDC provider exists yet, so the rest of the script proceeds. - "echo None\n" + "case \"$1 $2\" in\n" + + " 'iam list-open-id-connect-providers') echo '{\"OpenIDConnectProviderList\":[]}' ;;\n" + + " 'iam create-open-id-connect-provider') echo '{\"OpenIDConnectProviderArn\":\"" + awsWIFProvider + "\"}' ;;\n" + + " 'iam get-open-id-connect-provider') echo '{\"Url\":\"accounts.google.com\",\"ClientIDList\":[\"sts.amazonaws.com\"],\"ThumbprintList\":[]}' ;;\n" + + " *) echo None ;;\n" + + "esac\n" } // runRenderedWIFScript writes the rendered aws-wif-cli.sh to a temp file, puts a