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.

Sign up for GitHub

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

Jump to bottom

Limit KMS keys policies #512

Closed
dlpzx opened this issue Jun 12, 2023 · 2 comments · Fixed by #524
Closed

Limit KMS keys policies #512

dlpzx opened this issue Jun 12, 2023 · 2 comments · Fixed by #524
Assignees
@noah-paige
Labels
priority: high type: enhancement Feature enhacement

Comments

@dlpzx
Copy link
Contributor

dlpzx commented Jun 12, 2023

Is your idea related to a problem? Please describe.
As an additional layer of security besides restrictive IAM policies, I would like more specific KMS policies that restrict access to the KMS keys to the minimum.

Describe the solution you'd like
Define a KMS policy in CDK that restricts access to the different KMS keys to the IAM roles that use them in each case. Here is a list of keys that need an additional policy:
-TBD
-TBD

P.S. Don't attach files. Please, prefer add code snippets directly in the message body.
@dlpzx dlpzx added type: enhancement Feature enhacement priority: high status: not-picked-yet At the moment we have not picked this item. Anyone can pick it up labels Jun 12, 2023
@dlpzx dlpzx changed the title [WIP] - Limit KMS keys policies Limit KMS keys policies Jun 14, 2023
@noah-paige noah-paige mentioned this issue Jun 16, 2023
Merged
@noah-paige noah-paige linked a pull request Jun 22, 2023 that will close this issue
feat: Restrict kms keys #524
Merged
noah-paige added a commit that referenced this issue Jul 6, 2023
@noah-paige @dlpzx
feat: Restrict kms keys (#524)
150fdc4 
### Feature or Bugfix
<!-- please choose -->
- Feature

### Detail
- Restricting KMS Key Policies

This PR restricts the KMS Keys created in Data.all Environments (i.e.
linked AWS Accounts), including
- Environment KMS Keys (Used for subscriptions and Custom Resources)
- Dataset KMS Keys (To Encrypt Data in S3)
- Notebook KMS Keys (To Encrypt SageMaker Notebook Instance Storage
Volumes)
- SageMaker Studio KMS Keys (To Encrypt SageMaker Studio Storage
Volumes)

Additional as part of this PR:
- Consolidate Environment Keys 
- Reduce 4 KMS Keys to 1 Key to Encrypt all SQS DLQs used for Lambda
Custom Resource Failed Invocations
- Reduce 3 KMS Keys to 1 Key to Encrypt SNS and SQS Resources related to
Environment Subscriptions

- Remove Unused Custom Resources
- Remove Glue DB No Delete Custom Resource (this is no longer used and
is replaced by Glue DB Custom Resource which additionally handles
deletes of Glue DBs)
- Remove Data Location CustomResource (this is no longer used and is
replaced by CfnResource used to register S3 location in LakeFormation)

- Remove KMS Key Permissions in Environment Roles where applicable
- Since we are restricting KMS Key Permissions we do not need to
explicitly give KMS Permissions to IAM Roles for Env Group Roles where
the dataset is created by data.all (if the dataset is imported we still
add KMS Permissions to the IAM Role)


### Relates
- [#512 ]

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.

---------

Co-authored-by: dlpzx <dlpzx@amazon.com>
@noah-paige
Copy link
Contributor

Leaving open until v1m6m0 release and merge to main

@noah-paige noah-paige reopened this Jul 6, 2023
@noah-paige noah-paige added status: in-review This issue has been implemented and is currently in review and waiting for next release and removed status: not-picked-yet At the moment we have not picked this item. Anyone can pick it up labels Jul 7, 2023
@noah-paige
Copy link
Contributor

Implemented in v1.6

@noah-paige noah-paige removed the status: in-review This issue has been implemented and is currently in review and waiting for next release label Jul 19, 2023
@anmolsgandhi anmolsgandhi mentioned this issue Sep 6, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@noah-paige noah-paige

Labels
priority: high type: enhancement Feature enhacement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: Restrict kms keys data-dot-all/dataall
2 participants
@noah-paige @dlpzx