Skip to content

AWS KMS.1 compliant policy with dedicated kms key list#1020

Merged
ge0Aja merged 2 commits intoDataDog:masterfrom
n0pants:kms1
Nov 14, 2025
Merged

AWS KMS.1 compliant policy with dedicated kms key list#1020
ge0Aja merged 2 commits intoDataDog:masterfrom
n0pants:kms1

Conversation

@n0pants
Copy link
Contributor

@n0pants n0pants commented Nov 5, 2025 •
edited
Loading

What does this PR do?

Adding an input parameter KmsKeyList to the CFN template.yaml to define a comma seperated list of KMS Key ARNs.
That list will be used to tailor kms:decrypt resource access in the IAM policy of the datadog serverless forwarder.

Motivation

AWS security Hub finding KMS.1 as well as CKV_AWS_356 in the default template.
Having wildcard permissions on all AWS KMS keys is a security issue.
Allowing access to only dedicated KMS keys keeps the setup secure.
Keys using default encryption aws/secretsmanager are accessible by default.

Testing Guidelines

Additional Notes

While the original CFN template contains a wildcard permission for "customer managed" keys, integrating this version will break access to those CMKs, if they are not listed in the parameter.

Types of changes

  • Bug fix
  • New feature
  • Breaking change
  • Misc (docs, refactoring, dependency upgrade, etc.)

Check all that apply

  • This PR's description is comprehensive
  • This PR contains breaking changes that are documented in the description
  • This PR introduces new APIs or parameters that are documented and unlikely to change in the foreseeable future
  • This PR impacts documentation, and it has been updated (or a ticket has been logged)
  • This PR's changes are covered by the automated tests
  • This PR collects user input/sensitive content into Datadog
  • This PR passes the integration tests (ask a Datadog member to run the tests)
  • This PR passes the unit tests
  • This PR passes the installation tests (ask a Datadog member to run the tests)

@n0pants
AWS KMS.1 compliant policy with dedicated kms key list
88de6c8 
Wildcard permissions on all kms keys is a security issue.
Allowing access to only dedicated KMS keys keeps the setup secure.
Keys using default encryption aws/secretsmanager are accessible by default.
@n0pants n0pants requested a review from a team as a code owner November 5, 2025 14:24
@ge0Aja ge0Aja self-assigned this Nov 5, 2025
@ge0Aja
Copy link
Contributor

ge0Aja commented Nov 5, 2025

👋 @n0pants
There's already a PermissionsBoundaryArn template variable to set the permission boundary for the forwarder.
The forwarder still needs kms:Decrypt action for SSE-KMS keys.

@n0pants
Copy link
Contributor Author

n0pants commented Nov 5, 2025

Hi @ge0Aja 👋
thanks for having a look and your feedback.
Permission boundaries are useful to handle generic actions or resources. Obviously secrets are pretty unique per AWS account. Running hundreds of AWS accounts makes it kind of impossible to set a generic boundary.

Rather than opting out of wildcard secret resource permissions, it should be an explicit opt in to keep it secure.

I do completely agree that you need kms:Decrypt for SSE-KMS access, like for all encryption using CMKs. But that´s exactly what the change does. Providing an allow list to access to CMKs and prevent access to all other keys.

@ge0Aja
Copy link
Contributor

ge0Aja commented Nov 6, 2025

Hi @ge0Aja 👋 thanks for having a look and your feedback. Permission boundaries are useful to handle generic actions or resources. Obviously secrets are pretty unique per AWS account. Running hundreds of AWS accounts makes it kind of impossible to set a generic boundary.

Rather than opting out of wildcard secret resource permissions, it should be an explicit opt in to keep it secure.

I do completely agree that you need kms:Decrypt for SSE-KMS access, like for all encryption using CMKs. But that´s exactly what the change does. Providing an allow list to access to CMKs and prevent access to all other keys.

Thanks for the explanation, I'd suggest to pass the list of keys as an optional parameter then. I believe we should still keep the option to use a wildcard if no keys were specified.

@n0pants
refactor kms:Decrypt policy
af83439 
Input parameter KmsKeyList set default empty, gives permission on all resources.
Override allow explicit list via input parameter
@n0pants
Copy link
Contributor Author

n0pants commented Nov 14, 2025

Thanks & agree. As it would also remove the breaking character of the change.
I´ve turned the default of the input to empty which means kms:Decrypt on all resources (equals current state)
or explicit access in case the input parameter is set.

@ge0Aja ge0Aja merged commit 987f806 into DataDog:master Nov 14, 2025
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ge0Aja ge0Aja ge0Aja approved these changes

Assignees

@ge0Aja ge0Aja

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@n0pants @ge0Aja