Skip to content

sec(iac): scope kms:GetKeyPolicy away from Resource=* - #525

Merged
cristim merged 1 commit into
feat/multicloud-web-frontendfrom
sec/iac-kms-getkeypolicy-scope
Jun 8, 2026
Merged

sec(iac): scope kms:GetKeyPolicy away from Resource=*#525
cristim merged 1 commit into
feat/multicloud-web-frontendfrom
sec/iac-kms-getkeypolicy-scope

Conversation

@cristim

@cristim cristim commented May 19, 2026

Copy link
Copy Markdown
Member

Summary

  • policy_compute.tf: remove kms:GetKeyPolicy from KMSCreateAndRead (Resource="*") — this action reveals the full trust model of a CMK (all principals and conditions).
  • policy_compute_b.tf: add new KMSReadTaggedOnly statement for kms:GetKeyPolicy gated on aws:ResourceTag/Project=CUDly. A compromised deploy token can now only read key policies of CUDly-owned CMKs, not enumerate the key policies of every CMK in the account.

The tag condition works correctly because aws_kms_key sets the Project=CUDly tag at CreateKey time (via the Tags parameter), before GetKeyPolicy is ever called on the new key.

Split to policy_compute_b.tf because policy_compute.tf is at the AWS 6144-char managed-policy limit.

Test plan

  • terraform fmt -check -recursive passes (pre-commit verified)
  • terraform validate in ci-cd-permissions passes
  • Confirm Terraform plan and apply still succeed for KMS operations on CUDly keys

Closes #427.

Summary by CodeRabbit

  • Chores
    • Updated KMS permissions in compute infrastructure IAM policies by removing specific key-related API operations, refining access controls, and improving documentation for permission boundaries and access restrictions.
    • Added new conditional KMS read-only permissions with resource-level restrictions limiting access exclusively to resources matching specific attributes, providing enhanced security and fine-grained access control.

Review Change Stack

Move kms:GetKeyPolicy from KMSCreateAndRead (Resource="*", account-wide)
to a new KMSReadTaggedOnly statement in policy_compute_b.tf gated on
aws:ResourceTag/Project=CUDly. This prevents a compromised deploy token
from reading key policies of unrelated KMS CMKs in the account
(key-policy reconnaissance), while still allowing the deploy SA to
read policies of CUDly-owned keys (which are tagged at CreateKey time
by the Terraform aws_kms_key resource).

Split to policy_compute_b.tf because policy_compute.tf is at the
AWS 6144-char managed-policy limit.

Closes #427.
@cristim cristim added triaged Item has been triaged priority/p2 Backlog-worthy severity/medium Moderate harm urgency/this-sprint Within the current sprint impact/internal Team-internal only effort/xs Trivial / one-liner type/security Security finding labels May 19, 2026
@coderabbitai

coderabbitai Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@cristim, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 43 minutes and 48 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7a4b5662-ecdc-4877-9f2b-fe16c15c696e

📥 Commits

Reviewing files that changed from the base of the PR and between b1ea4b1 and 3b54fd5.

📒 Files selected for processing (2)
  • terraform/environments/aws/ci-cd-permissions/policy_compute.tf
  • terraform/environments/aws/ci-cd-permissions/policy_compute_b.tf
📝 Walkthrough

Walkthrough

This PR refines KMS IAM permissions to enforce least-privilege by removing unrestricted kms:GetKeyPolicy from the compute policy and replacing it with a tag-gated statement in a separate policy. The change prevents account-wide key-policy reconnaissance while maintaining required access to CUDly-tagged keys.

Changes

KMS Permission Least-Privilege Refinement

Layer / File(s) Summary
Remove unrestricted GetKeyPolicy from KMSCreateAndRead
terraform/environments/aws/ci-cd-permissions/policy_compute.tf
Comments are reworded to document that kms:GetKeyPolicy is intentionally excluded; the action is removed from the KMSCreateAndRead statement to eliminate account-wide key-policy read access on Resource = "*".
Add tag-gated KMSReadTaggedOnly statement
terraform/environments/aws/ci-cd-permissions/policy_compute_b.tf
A new KMSReadTaggedOnly statement is added to grant kms:GetKeyPolicy only for KMS keys tagged with aws:ResourceTag/Project=CUDly, splitting the permission into a separate policy to avoid size limits and enforce least-privilege.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • LeanerCloud/CUDly#103: The main PR continues PR #103's KMS IAM restructuring by removing kms:GetKeyPolicy from policy_compute.tf's KMSCreateAndRead and adding a new tag-gated KMSReadTaggedOnly statement in policy_compute_b.tf, directly extending the same KMS permission split/tightening.

Suggested labels

effort/s

Poem

🐰 A policy most permissive, now refined with care,
GetKeyPolicy split and scoped, with tagging condition fair,
CUDly-tagged keys alone get read, while others stand apart—
Least-privilege secured, a fix from the heart! 🔐

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'sec(iac): scope kms:GetKeyPolicy away from Resource=*' clearly and concisely describes the main security change: scoping down the kms:GetKeyPolicy permission from wildcard resources.
Linked Issues check ✅ Passed The PR successfully addresses issue #427 by removing kms:GetKeyPolicy from the KMSCreateAndRead statement with Resource='*' and adding a new KMSReadTaggedOnly statement that scopes kms:GetKeyPolicy to keys tagged with Project=CUDly.
Out of Scope Changes check ✅ Passed All changes in policy_compute.tf and policy_compute_b.tf are directly related to scoping kms:GetKeyPolicy permissions and addressing the security concern identified in issue #427; no out-of-scope modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sec/iac-kms-getkeypolicy-scope

Comment @coderabbitai help to get the list of available commands and usage tips.

@cristim

cristim commented May 19, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim

cristim commented May 19, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim

cristim commented May 20, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Full review triggered.

@cristim

cristim commented May 22, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim

cristim commented May 22, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

1 similar comment
@cristim

cristim commented May 22, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim

cristim commented May 27, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim

cristim commented May 27, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Full review triggered.

@cristim

cristim commented May 27, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Full review triggered.

@cristim

cristim commented May 28, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Full review triggered.

@cristim

cristim commented May 30, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented May 30, 2026

Copy link
Copy Markdown
Contributor

Rate Limit Exceeded

@cristim have exceeded the limit for the number of chat messages per hour. Please wait 54 minutes and 1 seconds before sending another message.

@cristim

cristim commented Jun 4, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim

cristim commented Jun 7, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@cristim
cristim merged commit 9fada6d into feat/multicloud-web-frontend Jun 8, 2026
4 checks passed
@cristim
cristim deleted the sec/iac-kms-getkeypolicy-scope branch July 27, 2026 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/xs Trivial / one-liner impact/internal Team-internal only priority/p2 Backlog-worthy severity/medium Moderate harm triaged Item has been triaged type/security Security finding urgency/this-sprint Within the current sprint

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant