From 6003c601838b98261f10be83b264f295a2db24df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Gonz=C3=A1lez=20Di=20Antonio?= Date: Tue, 26 May 2026 10:25:19 +0200 Subject: [PATCH] chore(template): drop dead IAM grants and standardize on AWS::Partition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Housekeeping pass on template.yaml after the #533 / fix-permissions work surfaced several never-reached or never-matched grants. No runtime behavior change. Lambda role (CustomLambdaPolicy): * Drop S3ListBucketPolicy. internal/repository/s3.go only issues GetObject and PutObject on the single state object; ListBucket / ListObjectsV2 / HeadObject are never called, so the statement and its s3:prefix condition were unreachable. * Drop KMSGetDataPolicy. kms:GenerateDataKeyPair is an asymmetric-key API; the state CMK is the default symmetric key, so the action could never be invoked against it. The needed symmetric action (kms:GenerateDataKey) is already in the renamed KMSStateObjectPolicy. * Drop secretsmanager:GetResourcePolicy. pkg/aws/secretsmanager.go only calls GetSecretValue. * Rename SSMGetParameterPolicy -> SecretsManagerGetSecretValuePolicy (the old name was a copy-paste from a different service). * Rename KMSDecryptPolicy -> KMSStateObjectPolicy to reflect that it now covers all SSE-KMS operations on the state object (Decrypt, Encrypt, GenerateDataKey). KMS key policy: * Drop AllowAWSLambdaToRetrieveKMSKey. Its principal was the lambda service (Service: lambda.amazonaws.com), but at runtime KMS sees the function's assumed-role ARN — not the Lambda service — when S3 forwards the kms:* calls on the role's behalf. The statement granted nothing at runtime; the real grant is AllowIAMThisAccount combined with the role's identity-based policy. Removing the dead statement makes the grant model unambiguous. Partition portability: * Replace four hardcoded "arn:aws:..." ARNs with "arn:${AWS::Partition}:...": - KMSKey AllowIAMThisAccount principal - Bucket BucketEncryption.KMSMasterKeyID - BucketPolicy AllowAWSLambdaFunction principal - (The rest of the template already used ${AWS::Partition}.) docs/Whats-New.md updated with a new "Template housekeeping" entry. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/Whats-New.md | 22 ++++++++++++++++++++++ template.yaml | 47 +++++------------------------------------------ 2 files changed, 27 insertions(+), 42 deletions(-) diff --git a/docs/Whats-New.md b/docs/Whats-New.md index c383376..2010e13 100644 --- a/docs/Whats-New.md +++ b/docs/Whats-New.md @@ -4,6 +4,28 @@ This document tracks notable changes, new features, and bug fixes across release ## Unreleased +### Template housekeeping: remove dead/misleading IAM grants and standardize on `${AWS::Partition}` + +Cleans up the Lambda execution role, the KMS key policy, and a few hardcoded partition strings in `template.yaml`. No runtime behavior change — every removal is a permission or grant that was never reached or never matched at runtime. + +**Lambda role (`CustomLambdaPolicy`):** + +* **Removed `s3:ListBucket` statement** (`S3ListBucketPolicy`). The runtime code only calls `s3:GetObject` and `s3:PutObject` on the single state object ([`internal/repository/s3.go`](../internal/repository/s3.go)) — `ListBucket` / `ListObjectsV2` / `HeadObject` are never issued, so the statement and its `s3:prefix` condition were unreachable. +* **Removed `kms:GenerateDataKeyPair` statement** (`KMSGetDataPolicy`). `GenerateDataKeyPair` is an asymmetric-key API; the state-bucket CMK is the default symmetric KMS key, so this action could never be called against it. The remaining symmetric action `kms:GenerateDataKey` (needed by S3 for SSE-KMS with bucket keys) is already granted by the renamed `KMSStateObjectPolicy` statement. +* **Removed `secretsmanager:GetResourcePolicy` action.** The code only calls `GetSecretValue` ([`pkg/aws/secretsmanager.go`](../pkg/aws/secretsmanager.go)). `GetResourcePolicy` reads the secret's resource-policy JSON — nothing in this Lambda needs it. Renamed the surviving statement to `SecretsManagerGetSecretValuePolicy` (the old name `SSMGetParameterPolicy` was a copy-paste from a different service). + +**KMS key policy:** + +* **Removed `AllowAWSLambdaToRetrieveKMSKey` statement.** Its principal was `Service: lambda.amazonaws.com`, but at runtime KMS sees the Lambda's **assumed-role** ARN (not the Lambda service) when S3 forwards the `kms:Decrypt` / `kms:GenerateDataKey` calls. The statement therefore granted nothing at runtime — the real grant comes from `AllowIAMThisAccount` (the standard "delegate to IAM" pattern), combined with the role's identity-based policy. Removing the dead statement makes the grant model unambiguous. + +**Partition portability:** + +* Replaced four hardcoded `arn:aws:…` ARNs with `arn:${AWS::Partition}:…`: + * `KMSKey` key-policy principal (`AllowIAMThisAccount`) + * `Bucket` `BucketEncryption.KMSMasterKeyID` + * `BucketPolicy` `AllowAWSLambdaFunction` principal +* The rest of the template already used `${AWS::Partition}`; these were the last holdouts. The template is now deployable in non-commercial AWS partitions (GovCloud, China) without manual edits. + ### CI fix: cosign now signs the published container manifest by tag (closes the v0.45.0 signing failure) Fixes the `Cosign sign published container manifest (keyless / Sigstore)` step of the release workflow, which failed with **`MANIFEST_UNKNOWN: manifest unknown`** on every release attempt after the multi-arch build was restored (see ["CI fix: restore multi-arch container builds"](#ci-fix-restore-multi-arch-container-builds-in-the-release-workflow)). diff --git a/template.yaml b/template.yaml index 8d7f9ee..7ea4e17 100644 --- a/template.yaml +++ b/template.yaml @@ -332,10 +332,9 @@ Resources: PolicyDocument: Version: "2012-10-17" Statement: - - Sid: SSMGetParameterPolicy + - Sid: SecretsManagerGetSecretValuePolicy Effect: Allow Action: - - secretsmanager:GetResourcePolicy - secretsmanager:GetSecretValue Resource: - !Ref AWSGWSServiceAccountFileSecret @@ -352,27 +351,7 @@ Resources: - s3:PutObjectAcl Resource: - !Sub "arn:${AWS::Partition}:s3:::${BucketNamePrefix}-${AWS::AccountId}-${AWS::Region}/${BucketKey}" - - Sid: S3ListBucketPolicy - Effect: Allow - Action: - - s3:ListBucket - Resource: - - !Sub "arn:${AWS::Partition}:s3:::${BucketNamePrefix}-${AWS::AccountId}-${AWS::Region}" - Condition: - StringLike: - s3:prefix: - - !Ref BucketKey - - Sid: KMSGetDataPolicy - Effect: Allow - Action: - - kms:GenerateDataKeyPair - Resource: - - !GetAtt KMSKey.Arn - Condition: - StringEquals: - kms:ViaService: !Sub "s3.${AWS::Region}.amazonaws.com" - kms:EncryptionContext:aws:s3:arn: !Sub "arn:${AWS::Partition}:s3:::${BucketNamePrefix}-${AWS::AccountId}-${AWS::Region}" - - Sid: KMSDecryptPolicy + - Sid: KMSStateObjectPolicy Effect: Allow Action: - kms:Decrypt @@ -420,25 +399,9 @@ Resources: - Sid: AllowIAMThisAccount Effect: Allow Principal: - AWS: !Sub "arn:aws:iam::${AWS::AccountId}:root" + AWS: !Sub "arn:${AWS::Partition}:iam::${AWS::AccountId}:root" Action: "kms:*" Resource: "*" - - Sid: AllowAWSLambdaToRetrieveKMSKey - Effect: Allow - Principal: - Service: "lambda.amazonaws.com" - #AWS: !GetAtt LambdaFunctionRole.Arn # Fails because circular reference - #AWS: !Sub "arn:aws:iam::${AWS::AccountId}:role/serverless-idp-scim-sync-${AWS::AccountId}-${AWS::Region}" # Fails in runtime because the roles is not created yet - Action: - - kms:Encrypt - - kms:Decrypt - - kms:ReEncrypt* - - kms:GenerateDataKey* - - kms:DescribeKey - Resource: "*" - Condition: - StringEquals: - kms:CallerAccount: !Ref "AWS::AccountId" KMSKeyAlias: Type: AWS::KMS::Alias @@ -461,7 +424,7 @@ Resources: BucketEncryption: ServerSideEncryptionConfiguration: - ServerSideEncryptionByDefault: - KMSMasterKeyID: !Sub "arn:aws:kms:${AWS::Region}:${AWS::AccountId}:${KMSKeyAlias}" + KMSMasterKeyID: !Sub "arn:${AWS::Partition}:kms:${AWS::Region}:${AWS::AccountId}:${KMSKeyAlias}" SSEAlgorithm: "aws:kms" BucketKeyEnabled: true # https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucket-key.html @@ -475,7 +438,7 @@ Resources: - Sid: AllowAWSLambdaFunction Principal: AWS: - - !Sub "arn:aws:iam::${AWS::AccountId}:role/serverless-idp-scim-sync-${AWS::AccountId}-${AWS::Region}${RoleNameSuffix}" + - !Sub "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/serverless-idp-scim-sync-${AWS::AccountId}-${AWS::Region}${RoleNameSuffix}" Effect: Allow Action: - s3:GetObject