Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(rds): Encrypt DatabaseCluster (aurora) or DatabaseInstance (rds) storage with AWS-managed key by default (behind feature flag) #32398

Open
2 tasks done
blimmer opened this issue Dec 6, 2024 · 4 comments · May be fixed by #32695
Labels
@aws-cdk/aws-rds Related to Amazon Relational Database breaking-change This issue requires a breaking change to remediate. effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2

Comments

@blimmer
Copy link
Contributor

blimmer commented Dec 6, 2024

Describe the feature

If the user doesn't customize the DatabaseCluster or DatabaseInstance storageEncrypted property and does not pass a custom storageEncryptionKey, we should default the value to true. This will cause the AWS-managed aws/rds KMS key to be used.

Notes:

  • Since this is a breaking change for existing users, we should guard the feature behind a feature flag.
  • We should also add an acknowledgable warning annotation to warn people that don't specify storageEncrypted or storageEncryptionKey (as they would be affected by this change in v3).

Use Case

Today, if you don't remember to pass storageEncrypted: true to your DatabaseInstance or DatabaseCluster, your database's storage will be unencrypted.

In almost all cases, your database storage should be encrypted. Here are reasons I think we should use AWS-managed keys by default:

  • AWS Well-Architected security pillar says all data should be encrypted at rest.
  • There is no additional fee to use the AWS-managed KMS key
  • When creating RDS instances and clusters via the AWS Console UI, encryption with AWS-managed keys is enabled by default.
  • Adding encryption to a previously unencrypted database is an involved process that requires recreating your database from a snapshot. Applying encryption to an unencrypted database is time-consuming and a bad developer and user experience.
  • Other L2 constructs apply encryption by default, including Neptune and DocDb
  • If you don't encrypt your storage, you'll see warnings in the RDS UI telling you that your storage is unencrypted.
  • Users can easily retain the existing behavior by simply passing storageEncrypted: false.

Proposed Solution

This applies to DatabaseCluster and DatabaseInstance.

If:

  • storageEncrypted is not specified in the props; and,
  • storageEncryptionKey is not specified in the props

Then:

  • If the feature flag is set to true, default storageEncrypted to true
  • If the feature flag is set to false, leave storageEncrypted undefined (existing logic).
    • Add a warning annotation that the user should explicitly set storageEncrypted to false to retain compatibility with the new behavior.

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.171.1

Environment details (OS name and version, etc.)

MacOS

@blimmer blimmer added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Dec 6, 2024
@github-actions github-actions bot added the @aws-cdk/aws-rds Related to Amazon Relational Database label Dec 6, 2024
@blimmer
Copy link
Contributor Author

blimmer commented Dec 6, 2024

If maintainers are OK with this change, I can create the PR.

@khushail khushail added investigating This issue is being investigated and/or work is in progress to resolve the issue. and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Dec 6, 2024
@khushail khushail self-assigned this Dec 6, 2024
@khushail khushail added investigating This issue is being investigated and/or work is in progress to resolve the issue. p2 and removed needs-triage This issue or PR still needs to be triaged. labels Dec 6, 2024
@khushail
Copy link
Contributor

khushail commented Dec 6, 2024

Hi @blimmer , thanks for raising this issue and volunteering for contribution. I can see that in the RDS, the default value is set to true for the stroageEncryption given that key is provided, which is contrary to DocDB clusters property. All other reasons stated also make sense.

https://github.com/aws/aws-cdk/blob/69163acdaa40fd2d280306320a4f53b4ffc4cddd/packages/aws-cdk-lib/aws-rds/lib/instance.ts#L1147C1-L1152C39

Although your approach and resolution seems right on, being a breaking change, I would like to reach out to Team for their review on the suggested approach and share their insights on the same.

@khushail khushail added breaking-change This issue requires a breaking change to remediate. p3 and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. p3 labels Dec 6, 2024
@khushail khushail removed their assignment Dec 6, 2024
@khushail khushail added the effort/medium Medium work item – several days of effort label Dec 6, 2024
@blimmer
Copy link
Contributor Author

blimmer commented Dec 6, 2024

the default value is set to true for the stroageEncryption given that key is provided

Correct - if you pass a custom KMS key, it makes sense to set storageEncrypted to true (since CloudFormation states that you must do this).

But, yep, I think we want to ensure best practices of encrypting with the AWS-managed key is the default moving forward.

I would like to reach out to Team for their review on the suggested approach and share their insights on the same.

Thanks - is there someone we should tag? Or are you doing that on your end?

@khushail
Copy link
Contributor

khushail commented Dec 6, 2024

@blimmer , I have added this to Abstraction team's board where SDE onCall will look into the matter and would share his insights.Hope that helps!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-rds Related to Amazon Relational Database breaking-change This issue requires a breaking change to remediate. effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2
Projects
None yet
2 participants