From 9eda652c122af42c7e613997cb8678b4e2f2c805 Mon Sep 17 00:00:00 2001 From: nathanpage Date: Tue, 8 Aug 2023 11:01:55 +1000 Subject: [PATCH 1/4] Implement support for AWS SecretsManager --- composer.json | 1 + src/Secrets.php | 113 ++++++++++++++++++++++++++++++++++-------- tests/SecretsTest.php | 99 ++++++++++++++++++++++++++++++++++++ 3 files changed, 193 insertions(+), 20 deletions(-) diff --git a/composer.json b/composer.json index fac18e5..8f5fdef 100644 --- a/composer.json +++ b/composer.json @@ -19,6 +19,7 @@ "async-aws/ssm": "^1.3" }, "require-dev": { + "async-aws/secrets-manager": "^1.0", "phpunit/phpunit": "^9.6.10", "mnapoli/hard-mode": "^0.3.0", "phpstan/phpstan": "^1.10.26" diff --git a/src/Secrets.php b/src/Secrets.php index 639e815..07f2f3e 100644 --- a/src/Secrets.php +++ b/src/Secrets.php @@ -2,6 +2,7 @@ namespace Bref\Secrets; +use AsyncAws\SecretsManager\SecretsManagerClient; use AsyncAws\Ssm\SsmClient; use Closure; use JsonException; @@ -13,9 +14,10 @@ class Secrets * Decrypt environment variables that are encrypted with AWS SSM. * * @param SsmClient|null $ssmClient To allow mocking in tests. + * @param SecretsManagerClient|null $secretsManagerClient To allow mocking in tests. * @throws JsonException */ - public static function loadSecretEnvironmentVariables(?SsmClient $ssmClient = null): void + public static function loadSecretEnvironmentVariables(?SsmClient $ssmClient = null, ?SecretsManagerClient $secretsManagerClient = null): void { /** @var array|string|false $envVars */ $envVars = getenv(local_only: true); // @phpstan-ignore-line PHPStan is wrong @@ -23,36 +25,63 @@ public static function loadSecretEnvironmentVariables(?SsmClient $ssmClient = nu return; } - // Only consider environment variables that start with "bref-ssm:" + // Only consider environment variables that start with "bref-ssm:" or "bref-secretsmanager:" $envVarsToDecrypt = array_filter($envVars, function (string $value): bool { - return str_starts_with($value, 'bref-ssm:'); + return str_starts_with($value, 'bref-ssm:') || str_starts_with($value, 'bref-secretsmanager:'); }); if (empty($envVarsToDecrypt)) { return; } - // Extract the SSM parameter names by removing the "bref-ssm:" prefix - $ssmNames = array_map(function (string $value): string { - return substr($value, strlen('bref-ssm:')); - }, $envVarsToDecrypt); + $ssmNames = []; + $secretsManagerNames = []; + + // Extract the SSM and SecretsManager parameter names by removing the prefixes + foreach ($envVarsToDecrypt as $key => $envVar) { + if (str_starts_with($envVar, 'bref-ssm:')) { + $ssmNames[$key] = substr($envVar, strlen('bref-ssm:')); + } + if (str_starts_with($envVar, 'bref-secretsmanager:')) { + $secretsManagerNames[$key] = substr($envVar, strlen('bref-secretsmanager:')); + } + } + + if (count($secretsManagerNames) > 0 && class_exists(SecretsManagerClient::class) === false) { + throw new RuntimeException('In order to load secrets from SecretsManager you must install "async-aws/secrets-manager" package'); + } $actuallyCalledSsm = false; - $parameters = self::readParametersFromCacheOr(function () use ($ssmClient, $ssmNames, &$actuallyCalledSsm) { - $actuallyCalledSsm = true; - return self::retrieveParametersFromSsm($ssmClient, array_values($ssmNames)); - }); + if (count($ssmNames) > 0) { + $ssmParameters = self::readParametersFromCacheOr('ssm', function () use ($ssmClient, $ssmNames, &$actuallyCalledSsm) { + $actuallyCalledSsm = true; + return self::retrieveParametersFromSsm($ssmClient, array_values($ssmNames)); + }); + + foreach ($ssmParameters as $parameterName => $parameterValue) { + $envVar = array_search($parameterName, $ssmNames, true); + $_SERVER[$envVar] = $_ENV[$envVar] = $parameterValue; + putenv("$envVar=$parameterValue"); + } + } + + $actuallyCalledSecretsManager = false; + if (count($secretsManagerNames) > 0) { + $secretsManagerParameters = self::readParametersFromCacheOr('secretsmanager', function () use ($secretsManagerClient, $secretsManagerNames, &$actuallyCalledSecretsManager) { + $actuallyCalledSecretsManager = true; + return self::retrieveParametersFromSecretsManager($secretsManagerClient, $secretsManagerNames); + }); - foreach ($parameters as $parameterName => $parameterValue) { - $envVar = array_search($parameterName, $ssmNames, true); - $_SERVER[$envVar] = $_ENV[$envVar] = $parameterValue; - putenv("$envVar=$parameterValue"); + foreach ($secretsManagerParameters as $parameterName => $parameterValue) { + $_SERVER[$parameterName] = $_ENV[$parameterName] = $parameterValue; + putenv("$parameterName=$parameterValue"); + } } // Only log once (when the cache was empty) else it might spam the logs in the function runtime // (where the process restarts on every invocation) - if ($actuallyCalledSsm) { + if ($actuallyCalledSsm || $actuallyCalledSecretsManager) { $stderr = fopen('php://stderr', 'ab'); - fwrite($stderr, '[Bref] Loaded these environment variables from SSM: ' . implode(', ', array_keys($envVarsToDecrypt)) . PHP_EOL); + fwrite($stderr, '[Bref] Loaded these environment variables from SSM/SecretsManager: ' . implode(', ', array_keys($envVarsToDecrypt)) . PHP_EOL); } } @@ -60,16 +89,17 @@ public static function loadSecretEnvironmentVariables(?SsmClient $ssmClient = nu * Cache the parameters in a temp file. * Why? Because on the function runtime, the PHP process might * restart on every invocation (or on error), so we don't want to - * call SSM every time. + * call SSM/SecretsManager every time. * + * @param string $paramType * @param Closure(): array $paramResolver * @return array Map of parameter name -> value * @throws JsonException */ - private static function readParametersFromCacheOr(Closure $paramResolver): array + private static function readParametersFromCacheOr(string $paramType, Closure $paramResolver): array { // Check in cache first - $cacheFile = sys_get_temp_dir() . '/bref-ssm-parameters.php'; + $cacheFile = sprintf('%s/bref-%s-parameters.php', sys_get_temp_dir(), $paramType); if (is_file($cacheFile)) { $parameters = json_decode(file_get_contents($cacheFile), true, 512, JSON_THROW_ON_ERROR); if (is_array($parameters)) { @@ -86,6 +116,49 @@ private static function readParametersFromCacheOr(Closure $paramResolver): array return $parameters; } + /** + * @param string[] $secretNames + * @return array Map of parameter name -> value + * @throws JsonException + */ + private static function retrieveParametersFromSecretsManager( + ?SecretsManagerClient $secretsManagerClient, + array $secretNames + ): array { + $secretsManager = $secretsManagerClient ?? new SecretsManagerClient([ + 'region' => $_ENV['AWS_REGION'] ?? $_ENV['AWS_DEFAULT_REGION'], + ]); + + /** @var array $parameters Map of parameter name -> value */ + $parameters = []; + + foreach ($secretNames as $secretId) { + try { + $result = $secretsManager->getSecretValue([ + 'SecretId' => $secretId, + ]); + $secretString = $result->getSecretString(); + } catch (RuntimeException $e) { + if ($e->getCode() === 400) { + // Extra descriptive error message for the most common error + throw new RuntimeException( + "Bref was not able to resolve secrets contained in environment variables from SecretsManager because of a permissions issue with the SecretsManager API. Did you add IAM permissions in serverless.yml to allow Lambda to access SecretsManager? (docs: https://bref.sh/docs/environment/variables.html#at-deployment-time).\nFull exception message: {$e->getMessage()}", + $e->getCode(), + $e, + ); + } + throw $e; + } + + $secretValues = json_decode($secretString, true, 512, JSON_THROW_ON_ERROR); + foreach ($secretValues as $name => $value) { + $parameters[$name] = $value; + } + } + + return $parameters; + } + /** * @param string[] $ssmNames * @return array Map of parameter name -> value diff --git a/tests/SecretsTest.php b/tests/SecretsTest.php index 6792ace..b1becd5 100644 --- a/tests/SecretsTest.php +++ b/tests/SecretsTest.php @@ -3,6 +3,8 @@ namespace Bref\Secrets\Test; use AsyncAws\Core\Test\ResultMockFactory; +use AsyncAws\SecretsManager\Result\GetSecretValueResponse; +use AsyncAws\SecretsManager\SecretsManagerClient; use AsyncAws\Ssm\Result\GetParametersResult; use AsyncAws\Ssm\SsmClient; use AsyncAws\Ssm\ValueObject\Parameter; @@ -16,6 +18,9 @@ public function setUp(): void if (file_exists(sys_get_temp_dir() . '/bref-ssm-parameters.php')) { unlink(sys_get_temp_dir() . '/bref-ssm-parameters.php'); } + if (file_exists(sys_get_temp_dir() . '/bref-secretsmanager-parameters.php')) { + unlink(sys_get_temp_dir() . '/bref-secretsmanager-parameters.php'); + } } public function test decrypts env variables(): void @@ -36,6 +41,79 @@ public function test decrypts env variables(): void $this->assertSame('helloworld', getenv('SOME_OTHER_VARIABLE')); } + public function test decrypts env variables from secretsmanager(): void + { + putenv('SOME_VARIABLE=bref-secretsmanager:/some/parameter'); + putenv('SOME_OTHER_VARIABLE=helloworld'); + + // Sanity checks + $this->assertSame('bref-secretsmanager:/some/parameter', getenv('SOME_VARIABLE')); + $this->assertSame('helloworld', getenv('SOME_OTHER_VARIABLE')); + + Secrets::loadSecretEnvironmentVariables(null, $this->mockSecretsManagerClient()); + + $this->assertSame('foobar_1', getenv('SOME_VARIABLE_1')); + $this->assertSame('foobar_1', $_SERVER['SOME_VARIABLE_1']); + $this->assertSame('foobar_1', $_ENV['SOME_VARIABLE_1']); + $this->assertSame('foobar_2', getenv('SOME_VARIABLE_2')); + $this->assertSame('foobar_2', $_SERVER['SOME_VARIABLE_2']); + $this->assertSame('foobar_2', $_ENV['SOME_VARIABLE_2']); + // Check that the other variable was not modified + $this->assertSame('helloworld', getenv('SOME_OTHER_VARIABLE')); + } + + public function test decrypts env variables from both ssm and secretsmanager(): void + { + putenv('SOME_VARIABLE=bref-ssm:/some/parameter'); + putenv('SOME_VARIABLE_1=bref-secretsmanager:/some/parameter'); + putenv('SOME_OTHER_VARIABLE=helloworld'); + + // Sanity checks + $this->assertSame('bref-ssm:/some/parameter', getenv('SOME_VARIABLE')); + $this->assertSame('bref-secretsmanager:/some/parameter', getenv('SOME_VARIABLE_1')); + $this->assertSame('helloworld', getenv('SOME_OTHER_VARIABLE')); + + Secrets::loadSecretEnvironmentVariables($this->mockSsmClient(), $this->mockSecretsManagerClient()); + + // Check value from ssm + $this->assertSame('foobar', getenv('SOME_VARIABLE')); + $this->assertSame('foobar', $_SERVER['SOME_VARIABLE']); + $this->assertSame('foobar', $_ENV['SOME_VARIABLE']); + // Check value from secretsmanager + $this->assertSame('foobar_1', getenv('SOME_VARIABLE_1')); + $this->assertSame('foobar_1', $_SERVER['SOME_VARIABLE_1']); + $this->assertSame('foobar_1', $_ENV['SOME_VARIABLE_1']); + $this->assertSame('foobar_2', getenv('SOME_VARIABLE_2')); + $this->assertSame('foobar_2', $_SERVER['SOME_VARIABLE_2']); + $this->assertSame('foobar_2', $_ENV['SOME_VARIABLE_2']); + // Check that the other variable was not modified + $this->assertSame('helloworld', getenv('SOME_OTHER_VARIABLE')); + } + + public function test env variables from secretsmanager overrides ssm(): void + { + putenv('SOME_VARIABLE=bref-ssm:/some/parameter'); + putenv('SOME_VARIABLE_1=bref-secretsmanager:/some/parameter'); + putenv('SOME_OTHER_VARIABLE=helloworld'); + + // Sanity checks + $this->assertSame('bref-ssm:/some/parameter', getenv('SOME_VARIABLE')); + $this->assertSame('bref-secretsmanager:/some/parameter', getenv('SOME_VARIABLE_1')); + $this->assertSame('helloworld', getenv('SOME_OTHER_VARIABLE')); + + Secrets::loadSecretEnvironmentVariables( + $this->mockSsmClient(), + $this->mockSecretsManagerClient('{"SOME_VARIABLE":"foobar_from_secretsmanager"}') + ); + + // Check value from ssm + $this->assertSame('foobar_from_secretsmanager', getenv('SOME_VARIABLE')); + $this->assertSame('foobar_from_secretsmanager', $_SERVER['SOME_VARIABLE']); + $this->assertSame('foobar_from_secretsmanager', $_ENV['SOME_VARIABLE']); + // Check that the other variable was not modified + $this->assertSame('helloworld', getenv('SOME_OTHER_VARIABLE')); + } + public function test caches parameters to call SSM only once(): void { putenv('SOME_VARIABLE=bref-ssm:/some/parameter'); @@ -64,6 +142,27 @@ public function test throws a clear error message on missing permissions Secrets::loadSecretEnvironmentVariables($ssmClient); } + private function mockSecretsManagerClient(?string $secretString = null): SecretsManagerClient + { + $secretsManagerClient = $this->getMockBuilder(SecretsManagerClient::class) + ->disableOriginalConstructor() + ->onlyMethods(['getSecretValue']) + ->getMock(); + + $result = ResultMockFactory::create(GetSecretValueResponse::class, [ + 'SecretString' => $secretString ?? '{"SOME_VARIABLE_1":"foobar_1","SOME_VARIABLE_2":"foobar_2"}', + ]); + + $secretsManagerClient->expects($this->once()) + ->method('getSecretValue') + ->with([ + 'SecretId' => '/some/parameter', + ]) + ->willReturn($result); + + return $secretsManagerClient; + } + private function mockSsmClient(): SsmClient { $ssmClient = $this->getMockBuilder(SsmClient::class) From 269868d0b7e05e75fbacea26650df77bcd6e10a6 Mon Sep 17 00:00:00 2001 From: nathanpage Date: Wed, 9 Aug 2023 00:39:15 +1000 Subject: [PATCH 2/4] Fix test and cs --- src/Secrets.php | 1 - tests/SecretsTest.php | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Secrets.php b/src/Secrets.php index 07f2f3e..019659e 100644 --- a/src/Secrets.php +++ b/src/Secrets.php @@ -91,7 +91,6 @@ public static function loadSecretEnvironmentVariables(?SsmClient $ssmClient = nu * restart on every invocation (or on error), so we don't want to * call SSM/SecretsManager every time. * - * @param string $paramType * @param Closure(): array $paramResolver * @return array Map of parameter name -> value * @throws JsonException diff --git a/tests/SecretsTest.php b/tests/SecretsTest.php index b1becd5..1537a0f 100644 --- a/tests/SecretsTest.php +++ b/tests/SecretsTest.php @@ -116,6 +116,8 @@ public function test env variables from secretsmanager overrides ssm(): vo public function test caches parameters to call SSM only once(): void { + // Unset this env variable to prevent calling SecretsManager client because of previous test + putenv('SOME_VARIABLE_1'); putenv('SOME_VARIABLE=bref-ssm:/some/parameter'); // Call twice, the mock will assert that SSM was only called once From 13e6914985f18ee990c4fc7ad22763b8ccee41cd Mon Sep 17 00:00:00 2001 From: nathanpage Date: Wed, 9 Aug 2023 07:22:45 +1000 Subject: [PATCH 3/4] Fix minimum version for symfony/polyfill-uuid --- composer.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 8f5fdef..c41c605 100644 --- a/composer.json +++ b/composer.json @@ -22,7 +22,8 @@ "async-aws/secrets-manager": "^1.0", "phpunit/phpunit": "^9.6.10", "mnapoli/hard-mode": "^0.3.0", - "phpstan/phpstan": "^1.10.26" + "phpstan/phpstan": "^1.10.26", + "symfony/polyfill-uuid": "^1.13.1" }, "config": { "allow-plugins": { From 6b8ceba9cf6f74e2447b566fb348ecad87bdf232 Mon Sep 17 00:00:00 2001 From: nathanpage Date: Mon, 2 Oct 2023 17:18:03 +1100 Subject: [PATCH 4/4] Implement json processor for bref-secretsmanager --- src/Secrets.php | 17 ++++++++++++++++- tests/SecretsTest.php | 26 +++++++++++++++++++++----- 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/Secrets.php b/src/Secrets.php index 019659e..c7c4524 100644 --- a/src/Secrets.php +++ b/src/Secrets.php @@ -131,7 +131,14 @@ private static function retrieveParametersFromSecretsManager( /** @var array $parameters Map of parameter name -> value */ $parameters = []; - foreach ($secretNames as $secretId) { + foreach ($secretNames as $originalEnvVar => $secretId) { + $isJson = false; + + if (str_starts_with($secretId, 'json:')) { + $isJson = true; + $secretId = substr($secretId, strlen('json:')); + } + try { $result = $secretsManager->getSecretValue([ 'SecretId' => $secretId, @@ -149,6 +156,14 @@ private static function retrieveParametersFromSecretsManager( throw $e; } + // If json processor not set, replace original env var with secretString value + if ($isJson === false) { + $parameters[$originalEnvVar] = $secretString; + + continue; + } + + // Otherwise JSON decode secretString and add parameters from decoded array $secretValues = json_decode($secretString, true, 512, JSON_THROW_ON_ERROR); foreach ($secretValues as $name => $value) { $parameters[$name] = $value; diff --git a/tests/SecretsTest.php b/tests/SecretsTest.php index 1537a0f..ab7ff5d 100644 --- a/tests/SecretsTest.php +++ b/tests/SecretsTest.php @@ -41,7 +41,7 @@ public function test decrypts env variables(): void $this->assertSame('helloworld', getenv('SOME_OTHER_VARIABLE')); } - public function test decrypts env variables from secretsmanager(): void + public function test decrypts env variables from secretsmanager not json(): void { putenv('SOME_VARIABLE=bref-secretsmanager:/some/parameter'); putenv('SOME_OTHER_VARIABLE=helloworld'); @@ -52,6 +52,22 @@ public function test decrypts env variables from secretsmanager(): void Secrets::loadSecretEnvironmentVariables(null, $this->mockSecretsManagerClient()); + $this->assertSame('{"SOME_VARIABLE_1":"foobar_1","SOME_VARIABLE_2":"foobar_2"}', getenv('SOME_VARIABLE')); + // Check that the other variable was not modified + $this->assertSame('helloworld', getenv('SOME_OTHER_VARIABLE')); + } + + public function test decrypts env variables from secretsmanager(): void + { + putenv('SOME_VARIABLE=bref-secretsmanager:json:/some/parameter'); + putenv('SOME_OTHER_VARIABLE=helloworld'); + + // Sanity checks + $this->assertSame('bref-secretsmanager:json:/some/parameter', getenv('SOME_VARIABLE')); + $this->assertSame('helloworld', getenv('SOME_OTHER_VARIABLE')); + + Secrets::loadSecretEnvironmentVariables(null, $this->mockSecretsManagerClient()); + $this->assertSame('foobar_1', getenv('SOME_VARIABLE_1')); $this->assertSame('foobar_1', $_SERVER['SOME_VARIABLE_1']); $this->assertSame('foobar_1', $_ENV['SOME_VARIABLE_1']); @@ -65,12 +81,12 @@ public function test decrypts env variables from secretsmanager(): void public function test decrypts env variables from both ssm and secretsmanager(): void { putenv('SOME_VARIABLE=bref-ssm:/some/parameter'); - putenv('SOME_VARIABLE_1=bref-secretsmanager:/some/parameter'); + putenv('SOME_VARIABLE_1=bref-secretsmanager:json:/some/parameter'); putenv('SOME_OTHER_VARIABLE=helloworld'); // Sanity checks $this->assertSame('bref-ssm:/some/parameter', getenv('SOME_VARIABLE')); - $this->assertSame('bref-secretsmanager:/some/parameter', getenv('SOME_VARIABLE_1')); + $this->assertSame('bref-secretsmanager:json:/some/parameter', getenv('SOME_VARIABLE_1')); $this->assertSame('helloworld', getenv('SOME_OTHER_VARIABLE')); Secrets::loadSecretEnvironmentVariables($this->mockSsmClient(), $this->mockSecretsManagerClient()); @@ -93,12 +109,12 @@ public function test decrypts env variables from both ssm and secretsman public function test env variables from secretsmanager overrides ssm(): void { putenv('SOME_VARIABLE=bref-ssm:/some/parameter'); - putenv('SOME_VARIABLE_1=bref-secretsmanager:/some/parameter'); + putenv('SOME_VARIABLE_1=bref-secretsmanager:json:/some/parameter'); putenv('SOME_OTHER_VARIABLE=helloworld'); // Sanity checks $this->assertSame('bref-ssm:/some/parameter', getenv('SOME_VARIABLE')); - $this->assertSame('bref-secretsmanager:/some/parameter', getenv('SOME_VARIABLE_1')); + $this->assertSame('bref-secretsmanager:json:/some/parameter', getenv('SOME_VARIABLE_1')); $this->assertSame('helloworld', getenv('SOME_OTHER_VARIABLE')); Secrets::loadSecretEnvironmentVariables(