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

Store secrets in the Secret Manager #587

Closed
FearlessHyena opened this issue Feb 25, 2021 · 4 comments · Fixed by #941
Closed

Store secrets in the Secret Manager #587

FearlessHyena opened this issue Feb 25, 2021 · 4 comments · Fixed by #941

Comments

@FearlessHyena
Copy link

Currently the secrets are encrypted via KMS and stored as environment variables in each Lambda
It would be much cleaner (and probably secure) if the secrets were instead stored in the Secret Manager and read in directly in the corresponding Lambdas

Another benefit of this approach is that the secrets can be directly updated in the Secret Manager and the Lambda would always have access to the latest secret without any changes
In the current model, if you want to, say rotate the KMS, you'll have to re-encrypt the secrets with the new key and update the environment variables

@npalm
Copy link
Member

npalm commented Mar 8, 2021

At the time we have implement we had a thought about both approaches. Cannot find any good design decsion log :( But we took into account would like to give the user of the module a choice and keep the costs low. Therefore we choose KMS and give the user to option to set his own KMS key instead of let the module create one.

I welcome alternatives. But I really like to give the user as much flexibilty as possible, with keep security in scope. Another rationale is to keep the costs of the runner to a bare minimum for those who would to like to use the runners with almost no load.

Feel free to suggest a more detailed changed, can you keep in mind what kind of migration steps are required? And what the mandatory costs aspects are involved in the change?

@Procrat
Copy link
Contributor

Procrat commented May 5, 2021

Because we didn't want to store these secrets in our Terraform state, we changed it on our fork to read them from the AWS Parameter Store. This is the relevant PR. In no way does it keep in mind the issues that Niek raised here, so I didn't bother creating an upstream PR, but perhaps it can help some people who have similar requirements.

@npalm
Copy link
Member

npalm commented May 5, 2021

Thanks @Procrat will check you branch.

@mcaulifn
Copy link
Contributor

The environment variables have a max size of 4kb. By adding instance types and having a longer environment name, the limit has been exceeded.

Standard Parameter Store usage is free, and that should include secure strings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants