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

KMS CMK key is not being rotated #879

Closed
zsaltys opened this issue Nov 16, 2023 · 2 comments · Fixed by #923
Closed

KMS CMK key is not being rotated #879

zsaltys opened this issue Nov 16, 2023 · 2 comments · Fixed by #923

Comments

@zsaltys
Copy link
Contributor

zsaltys commented Nov 16, 2023

We are using a KMS CMK key which is not being rotated and is picked up by Checkov scanner:

CheckID		: CKV_AWS_7
CheckName	: Ensure rotation for customer created CMKs is enabled
File		: /cdk.out/dataall-cdk-template-cicd-stack.template.json:1755-1819
Resource	: AWS::KMS::Key.dataallcdktemplatecdkpipelinePipelineArtifactsBucketEncryptionKeyDCDBA3C9
Guideline	: CKV_AWS_7 

All KMS CMK keys should be rotated as a best security practice.

@noah-paige
Copy link
Contributor

I believe this is in reference to the CodePipeline created in dataall/deploy/stacks/pipeline.py file. We need to test if adding the enable_key_rotation=True parameter to the CodePipeline would cause any unforeseen issues but should be easy to implement and align with best practices

Should be good to pick this up shortly barring any capacity setbacks, thank @zsaltys

noah-paige pushed a commit that referenced this issue Dec 18, 2023
### Feature or Bugfix
Bugfix

### Detail
Enabled key rotation for KMS in code pipeline 

### Relates
#879

### Security
Please answer the questions below briefly where applicable, or write
`N/A`. Based on
[OWASP 10](https://owasp.org/Top10/en/).

- Does this PR introduce or modify any input fields or queries - this
includes
fetching data from storage outside the application (e.g. a database, an
S3 bucket)? No
  - Is the input sanitized? N/A
- What precautions are you taking before deserializing the data you
consume? N/A
  - Is injection prevented by parametrizing queries? N/A
  - Have you ensured no `eval` or similar functions are used? N/A
- Does this PR introduce any functionality or component that requires
authorization? N/A
- How have you ensured it respects the existing AuthN/AuthZ mechanisms?
N/A
  - Are you logging failed auth attempts? N/A
- Are you using or adding any cryptographic features? N/A
  - Do you use a standard proven implementations? Yes
- Are the used keys controlled by the customer? Where are they stored?
N/A
- Are you introducing any new policies/roles/users? N/A
  - Have you used the least-privilege principle? How? N/A


By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
@noah-paige noah-paige linked a pull request Dec 18, 2023 that will close this issue
@noah-paige noah-paige added this to the v2.3.0 milestone Dec 18, 2023
@dlpzx
Copy link
Contributor

dlpzx commented Jan 3, 2024

Completed in #923

@dlpzx dlpzx closed this as completed Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants