Repository navigation
fix(iac/aws): grant kms:TagResource for KMS CreateKey tag-on-create - #1671
Conversation
CreateKey's Tags parameter requires kms:TagResource per the AWS API reference, but the only existing grant (KMSMutateTaggedOnly) is gated on aws:ResourceTag/Project, which can never be satisfied at create time since the key doesn't exist yet. Every AWS deploy that creates the OIDC signing KMS key was failing with AccessDeniedException. Add a separate, aws:RequestTag-gated statement, restricted via a Null condition on aws:ResourceTag/Project to resources with no Project tag yet, so it only ever performs the initial tag-on-create and can't be used to self-tag and hijack an existing, unrelated key through KMSMutateTaggedOnly. Closes #1670
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 17 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Deferred completeness checks (non-blocking, per team-lead direction to ship the minimal fix first)1. Sweep of other 2. Sibling clouds (GCP/Azure): checked 3. |
Summary
Every AWS deploy that creates a new KMS CMK is currently failing:
CreateKey'sTagsparameter requireskms:TagResource(AWS KMS API reference, "Required permissions"), but the only existing grant (KMSMutateTaggedOnlyinpolicy_compute.tf) is gated onaws:ResourceTag/Project, a condition key evaluated against tags already on an existing resource. AtCreateKeytime the key doesn't exist yet, so this condition can never be satisfied and the call is denied.Verified against current AWS docs:
CreateKeyAPI reference: "Required permissions: kms:CreateKey ... To use the Tags parameter, kms:TagResource."aws:RequestTaggates what tags can be passed in a request (the correct key for tag-on-create);aws:ResourceTaggates access based on tags already on the resource.Fix
Adds
KMSTagOnCreatetoterraform/environments/aws/ci-cd-permissions/policy_compute_b.tf(notpolicy_compute.tf— see "Whypolicy_compute_b.tf" below):{ Sid = "KMSTagOnCreate" Effect = "Allow" Action = ["kms:TagResource"] Resource = "*" Condition = { StringEqualsIgnoreCase = { "aws:RequestTag/Project" = "CUDly" } Null = { "aws:ResourceTag/Project" = "true" } } },The
Nullcondition is a deliberate hardening beyond the minimal fix:aws:RequestTagalone says nothing about a resource's current tags, so without it a leaked deploy token could callkms:TagResourceon any existing KMS key in the account (not just ones it creates), self-attachProject=CUDly, then useKMSMutateTaggedOnlyto disable/delete/repolicy that now-mistagged key — i.e. hijack a key it never created.Null: {"aws:ResourceTag/Project": "true"}restricts the statement to resources that currently have no Project tag at all (true for a brand-new key mid-CreateKey, false for anything already tagged, ours or not), so it can only ever perform the initial tag-on-create.Why
policy_compute_b.tfpolicy_compute.tf's compiled JSON is ~5923 of the AWS 6144-char managed-policy limit (per its own header comment, it's already the reasonpolicy_compute_b.tfexists). Adding the new statement there would land at ~6083 chars — technically under the limit but with only ~60 chars of margin, which is too fragile.policy_compute_b.tfalready houses the other KMS statements that were split out of the main policy for size reasons (KMSAliasMutate,KMSReadTaggedOnly), so it's the natural, low-risk home for this one too.Completeness checks
aws_kms_keythe deploy role creates carries a Project tag: the only KMS key actually created by this deploy role's live apply graph isaws_kms_key.signing(modules/compute/aws/lambda/signing-key.tf), tagged vialocal.common_tags.Project = var.project_name(same lowercase-cudlypath documented onKMSMutateTaggedOnly, matched case-insensitively).modules/monitoring/awsalso defines anaws_kms_key.sns, but that module isn't wired into anyterraform/environments/awsroot today, so it isn't exercised by this deploy role.kms:TagResourceis the only missing permission on this path: walked the AWS provider's create/read calls foraws_kms_key/aws_kms_alias(CreateKey, TagResource, DescribeKey, ListResourceTags, GetKeyPolicy, GetKeyRotationStatus, CreateAlias) against the granted statements. Tag-gated reads (GetKeyPolicy, the key-side of alias creation) are safe becauseCreateKey'sTagsparameter tags the key atomically during creation, so the tag is already present by the time any subsequent read happens.ci-cd-permissionsuses the broad predefinedroles/cloudkms.admin(no ABAC tag-condition, so this bug shape can't occur); Azure'sci-cd-permissions/role.tfhas zero tag/condition constructs. No equivalent gap found in either; no follow-up filed.test-iam-role.sh: this is a full live-AWS destroy+apply integration script (not run in CI), with a genericAccessDeniedgrep that already surfaces any missing action generically. No existing precedent in the script adds bespoke per-permission checks, so none was added here to avoid forcing a fit that doesn't match its structure.Manual apply required
This fix does not take effect until a privileged human re-applies
terraform/environments/aws/ci-cd-permissions/. The CI deploy role (cudly-terraform-deploy) cannot grant itself new IAM permissions — that would be self-privilege-escalation, and this bootstrap root is intentionally out of the CI deploy role's own write scope.Run this with your own (human, non-CI) AWS credentials that have permission to modify the
cudly-deploy-compute-bmanaged policy.Closes #1670