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

Encrypted Secrets in secret manager should be rotated atleast annually and encrypted with a KMS key #443

Closed
manjulaK opened this issue May 3, 2023 · 2 comments · Fixed by #455
Closed

Encrypted Secrets in secret manager should be rotated atleast annually and encrypted with a KMS key #443

manjulaK opened this issue May 3, 2023 · 2 comments · Fixed by #455
Assignees
@dlpzx
Labels
priority: high type: enhancement Feature enhacement

Comments

@manjulaK
Copy link
Contributor

manjulaK commented May 3, 2023

Describe the bug

Encrypted Secrets in secret manager should be rotated atleast annually and encrypted with a KMS key. If the rotation is not an option then consider storing the secret under systems manager parameter store.

How to Reproduce

*P.S. Please do not attach files as it's considered a security risk. Add code snippets directly in the message body as much as possible.*

Expected behavior

Encrypted Secrets in secret manager should be rotated atleast annually and encrypted with a KMS key. If the rotation is not an option then consider storing the secret under systems manager parameter store.

Your project

No response

Screenshots

No response

OS

All

Python version

3.1

AWS data.all version

v1.3,v1.4,v1.5

Additional context

As per security best practice for secrets management the above need to be met.
@dlpzx dlpzx added type: enhancement Feature enhacement priority: high status: in-progress This issue has been picked and is being implemented labels May 10, 2023
@dlpzx
Copy link
Contributor

dlpzx commented May 11, 2023

Hi @manjulaK,

I have been looking at this issue, I think you are referring to dataall-pivot-role-name-<envname> and dataall-externalId-<envname>. They are both encrypted with a different KMS key see deploy/stacks/secrets_stack.py

They are constant values that are referenced in the code and in the environments stack, that's why we do not rotate them.

For the case of dataall-pivot-role-name-<envname>, it is more a parameter than a secret, so I will take your suggestion and replace it by an SSM parameter

For dataall-externalId-<envname> I am considering different options as it takes advantage of the aws secrets manager SecretStringGenerator to get created.

@dlpzx dlpzx mentioned this issue May 15, 2023
Merged
@dlpzx dlpzx linked a pull request May 15, 2023 that will close this issue
feat: Move parameters from secrets manager to ssm #455
Merged
@dlpzx dlpzx added status: in-review This issue has been implemented and is currently in review and waiting for next release and removed status: in-progress This issue has been picked and is being implemented labels May 17, 2023
@dlpzx dlpzx self-assigned this May 17, 2023
dlpzx added a commit that referenced this issue May 17, 2023
@dlpzx
Move parameters from secrets manager to ssm (#455)
99c28cd 
### Feature or Bugfix
- Refactoring

### Detail
Replaced externalId and PivotRoleName secrets in SecretsManager by SSM
parameters:
- modified parameter_stack and secrets_stack to create the parameters
and remove the secrets.
- the new externalId parameter uses a pre-existing secret if it exists,
otherwise it generates a random sequence of numbers and letters
- modified the sts handler to retrieve the pivotRoleName and externalId
from SSM

### Relates
- #443 

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
@noah-paige
Copy link
Contributor

Closing issue - 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dlpzx dlpzx

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

Successfully merging a pull request may close this issue.

feat: Move parameters from secrets manager to ssm data-dot-all/dataall
3 participants
@manjulaK @noah-paige @dlpzx