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

Load environment variable values from SSM at runtime #1376

Merged
merged 9 commits into from
Jan 27, 2023
Merged

Conversation

mnapoli
Copy link
Member

@mnapoli mnapoli commented Jan 6, 2023

This PR proposes a new feature for Bref 2.0 that lets you set secrets in environment variables without deploying them as plain text.

For example:

provider:
    # ...
    environment:

        DB_HOST: ${ssm:/app-${sls:stage}/database/host}

        DB_PASSWORD: bref-ssm:/app-${sls:stage}/database/password

DB_HOST is the current solution documented in Bref. It works, but the SSM value is replaced at deployment time with the value stored in SSM. The secret value is readable by anyone who can access the Lambda function in the AWS console. That's OK for many scenarios, but not the most secure.

DB_PASSWORD uses the new proposed feature. Its value will be replaced on cold start (at runtime) with the value found at SSM in the given path. That way the secret is never stored anywhere in plain text.

The downside is that it will add a tiny bit of extra latency on cold start (1 call to SSM to get all the values), and it will do an SSM request (paid once you go over the free tier) for each cold start. But the upside is that the value is secure all the way.

This new feature has (almost) zero impact if you don't use it.

TODO:

  • Docs
  • Test in Lambda with encrypted and non-encrypted parameters
  • Check why there is a double log line on cold start with the function runtime
  • Check it's only resolved once with the FPM runtime
  • Solve that it's executed on each invocation with the function runtime
  • Add to the FPM runtime

Note: I re-discovered after implementing this that this feature exists for Symfony in Async-Aws: https://github.com/async-aws/aws/blob/master/src/Integration/Symfony/Bundle/src/Secrets/SsmVault.php (docs: https://async-aws.com/integration/symfony-bundle.html#using-ssm-to-store-secrets). I think both are very similar, though the feature proposed here might be a bit more verbose but also more explicit (you don't have to guess the name of the environment variables because you defined them all). I'm fine with both options being available for Symfony users 🤷. @jderusse @shouze let me know if you have any opinion there.

This feature is directly inspired by https://github.com/cachewerk/bref-laravel-bridge (I'm looking at making this feature available for all frameworks and apps, not just Laravel), and its syntax/approach is inspired by https://github.com/customink/crypteia

@deleugpn
Copy link
Member

deleugpn commented Jan 6, 2023

Small thing I think would be worth mentioning when documenting: does this work even if the SSM parameter is not encrypted?

Reasoning: Encrypted parameter have an additional complexity of AWS KMS configuration (more relevant when using IaC). Since Lambda decrypts everything at runtime, I have seen projects that decided to just go with plain text secret. Since it would be visibile on AWS Lambda anyway, why bother with the complexity of encrypting it?

Now, with this feature, there's a compelling reason to use Encryption on SSM, but there's also a transition. If I were to use a parameter that is not encrypted at all, will this fail?

  • If it fails, users must create a new encrypted SSM and deploy a new version with a new parameter.
  • If it works, users can deploy a new version using this while SSM is still in plain text and gradually go through encrypting their secret

Copy link

@marcguyer marcguyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! It'll make it much easier to use SSM with Bref. Before, it was possible, but clunky. Nice improvement!

I will implement in my current project in a few days.

@mnapoli
Copy link
Member Author

mnapoli commented Jan 6, 2023

@deleugpn I don't think there is any issue with non-encrypted parameters:

image

But I'll test this, I want this to work out of the box in both cases.

Also I think there is an upside even with non-encrypted parameters: users can now deploy even if they don't have permission to access the SSM parameter (Lambda will access it, not the deployment itself).

@aran112000
Copy link
Contributor

Love the structure proposed here, we've formally used SSM in the way you mentioned where the ENV values were shown in the management console and that wasn't okay for our usage.

Since then we moved to Secrets Manager with auto rotating creds, would be amazing if these was a variant of this which could be used for Secrets Manager too with a different prefix, perhaps: bref-secrets-manager/...

@mnapoli
Copy link
Member Author

mnapoli commented Jan 26, 2023

I did some testing to double check and it works well with encrypted and non-encrypted parameters.

I added some docs as well: https://github.com/brefphp/bref/blob/ssm-secrets/docs/environment/variables.md#retrieving-secrets

I spotted a few things I need to investigate, but especially one behavior I completely missed at first: the variables will be resolved on every execution of a function, because the PHP process restarts (by default) for each invocation. I think caching the values in a temp file might be a simple and efficient solution. That makes the feature a bit more complex though, I need to work more on this.

Once that's solved we should be able to merge for v2.

Since then we moved to Secrets Manager with auto rotating creds, would be amazing if these was a variant of this which could be used for Secrets Manager too with a different prefix, perhaps: bref-secrets-manager/...

@aran112000 that sounds like a great idea. I probably won't implement that for v2 because of the extra effort involved though, but the way the implementation is done it could be 100% extended to fetch from Secrets Manager as well.

src/Secrets/Secrets.php Outdated Show resolved Hide resolved
@mnapoli
Copy link
Member Author

mnapoli commented Jan 27, 2023

Merging in v2, will add that in the FPM runtime as well.

@mnapoli mnapoli merged commit a606631 into v2 Jan 27, 2023
@mnapoli mnapoli deleted the ssm-secrets branch January 27, 2023 15:43
mnapoli added a commit to brefphp/php-fpm-runtime that referenced this pull request Jan 27, 2023
bernier-db added a commit to bernier-db/bref that referenced this pull request Nov 20, 2023
Since the example only talks about the `String` parameter type, I did not felt comfortable in storing the secrets as plain text in SSM. 
Upon digging in github, I found this [comment on a PR](brefphp#1376 (comment)). Thought it would be helpful to specify that both SecureString and String are taken into account.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants