feature #34295 [DI][FrameworkBundle] add EnvVarLoaderInterface - remove SecretEnvVarProcessor (nicolas-grekas)

This PR was merged into the 4.4 branch. Discussion ---------- [DI][FrameworkBundle] add EnvVarLoaderInterface - remove SecretEnvVarProcessor | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - This PR allows encrypting any env vars - not only those using the `%env(secret:<...>)%` processor (and the processor is removed actually). It does so by introducing a new `EnvVarLoaderInterface` (and a corresponding `container.env_var_loader` tag), which are objects that should return a list of key/value pairs that will be accessible via the regular `%env(FOO)%` syntax. The PR fixes a few issues found meanwhile. One is especially important: files in the vault should end with `.php` to protect against inadvertant exposures of the document root. Commits ------- ba2148fff3 [DI][FrameworkBundle] add EnvVarLoaderInterface - remove SecretEnvVarProcessor
2019-11-08 16:58:24 +01:00 · 2019-11-08 16:58:24 +01:00 · 57e9b81657
commit 57e9b81657
parent 585c0df909 ba2148fff3
11 changed files with 137 additions and 97 deletions
--- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
+++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
@ -49,6 +49,7 @@ use Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\DependencyInjection\Definition;
+use Symfony\Component\DependencyInjection\EnvVarLoaderInterface;
 use Symfony\Component\DependencyInjection\EnvVarProcessorInterface;
 use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
 use Symfony\Component\DependencyInjection\Exception\LogicException;
@ -376,6 +377,8 @@ class FrameworkExtension extends Extension
            ->addTag('console.command');
        $container->registerForAutoconfiguration(ResourceCheckerInterface::class)
            ->addTag('config_cache.resource_checker');
+        $container->registerForAutoconfiguration(EnvVarLoaderInterface::class)
+            ->addTag('container.env_var_loader');
        $container->registerForAutoconfiguration(EnvVarProcessorInterface::class)
            ->addTag('container.env_var_processor');
        $container->registerForAutoconfiguration(ServiceLocator::class)
--- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/secrets.xml
+++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/secrets.xml
@ -6,6 +6,7 @@

    <services>
        <service id="secrets.vault" class="Symfony\Bundle\FrameworkBundle\Secrets\SodiumVault">
+            <tag name="container.env_var_loader" />
            <argument>%kernel.project_dir%/config/secrets/%kernel.environment%</argument>
            <argument type="service" id="secrets.decryption_key" on-invalid="ignore" />
        </service>
@ -31,11 +32,5 @@
        <service id="secrets.local_vault" class="Symfony\Bundle\FrameworkBundle\Secrets\DotenvVault">
            <argument>%kernel.project_dir%/.env.local</argument>
        </service>
-
-        <service id="secrets.env_var_processor" class="Symfony\Bundle\FrameworkBundle\Secrets\SecretEnvVarProcessor">
-            <argument type="service" id="secrets.vault" />
-            <argument type="service" id="secrets.local_vault" on-invalid="ignore" />
-            <tag name="container.env_var_processor" />
-        </service>
    </services>
 </container>
--- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/services.xml
+++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/services.xml
@ -121,5 +121,11 @@
            <argument type="service" id="request_stack" />
            <tag name="kernel.event_subscriber" />
        </service>
+
+        <service id="container.env_var_processor" class="Symfony\Component\DependencyInjection\EnvVarProcessor">
+            <tag name="container.env_var_processor" />
+            <argument type="service" id="service_container" />
+            <argument type="tagged_iterator" tag="container.env_var_loader" />
+        </service>
    </services>
 </container>
--- a/src/Symfony/Bundle/FrameworkBundle/Secrets/DotenvVault.php
+++ b/src/Symfony/Bundle/FrameworkBundle/Secrets/DotenvVault.php
@ -54,7 +54,7 @@ class DotenvVault extends AbstractVault
    {
        $this->lastMessage = null;
        $this->validateName($name);
-        $v = \is_string($_SERVER[$name] ?? null) ? $_SERVER[$name] : ($_ENV[$name] ?? null);
+        $v = \is_string($_SERVER[$name] ?? null) && 0 !== strpos($name, 'HTTP_') ? $_SERVER[$name] : ($_ENV[$name] ?? null);

        if (null === $v) {
            $this->lastMessage = sprintf('Secret "%s" not found in "%s".', $name, $this->getPrettyPath($this->dotenvFile));
--- a/src/Symfony/Bundle/FrameworkBundle/Secrets/SecretEnvVarProcessor.php
+++ b/src/Symfony/Bundle/FrameworkBundle/Secrets/SecretEnvVarProcessor.php
@ -1,59 +0,0 @@
-<?php
-
-/*
- * This file is part of the Symfony package.
- *
- * (c) Fabien Potencier <fabien@symfony.com>
- *
- * For the full copyright and license information, please view the LICENSE
- * file that was distributed with this source code.
- */
-
-namespace Symfony\Bundle\FrameworkBundle\Secrets;
-
-use Symfony\Component\DependencyInjection\EnvVarProcessorInterface;
-use Symfony\Component\DependencyInjection\Exception\EnvNotFoundException;
-
-/**
- * @author Tobias Schultze <http://tobion.de>
- * @author Nicolas Grekas <p@tchwork.com>
- *
- * @internal
- */
-class SecretEnvVarProcessor implements EnvVarProcessorInterface
-{
-    private $vault;
-    private $localVault;
-
-    public function __construct(AbstractVault $vault, AbstractVault $localVault = null)
-    {
-        $this->vault = $vault;
-        $this->localVault = $localVault;
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public static function getProvidedTypes()
-    {
-        return [
-            'secret' => 'string',
-        ];
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function getEnv($prefix, $name, \Closure $getEnv): string
-    {
-        if (null !== $this->localVault && null !== ($secret = $this->localVault->reveal($name)) && \array_key_exists($name, $this->vault->list())) {
-            return $secret;
-        }
-
-        if (null !== $secret = $this->vault->reveal($name)) {
-            return $secret;
-        }
-
-        throw new EnvNotFoundException($this->vault->getLastMessage() ?? sprintf('Secret "%s" not found or decryption key is missing.', $name));
-    }
-}
--- a/src/Symfony/Bundle/FrameworkBundle/Secrets/SodiumVault.php
+++ b/src/Symfony/Bundle/FrameworkBundle/Secrets/SodiumVault.php
@ -11,6 +11,8 @@

 namespace Symfony\Bundle\FrameworkBundle\Secrets;

+use Symfony\Component\DependencyInjection\EnvVarLoaderInterface;
+
 /**
 * @author Tobias Schultze <http://tobion.de>
 * @author Jérémy Derussé <jeremy@derusse.com>
@ -18,7 +20,7 @@ namespace Symfony\Bundle\FrameworkBundle\Secrets;
 *
 * @internal
 */
-class SodiumVault extends AbstractVault
+class SodiumVault extends AbstractVault implements EnvVarLoaderInterface
 {
    private $encryptionKey;
    private $decryptionKey;
@ -56,8 +58,8 @@ class SodiumVault extends AbstractVault
            // ignore failures to load keys
        }

-        if ('' !== $this->decryptionKey && !file_exists($this->pathPrefix.'sodium.encrypt.public')) {
-            $this->export('sodium.encrypt.public', $this->encryptionKey);
+        if ('' !== $this->decryptionKey && !file_exists($this->pathPrefix.'encrypt.public.php')) {
+            $this->export('encrypt.public', $this->encryptionKey);
        }

        if (!$override && null !== $this->encryptionKey) {
@ -69,10 +71,10 @@ class SodiumVault extends AbstractVault
        $this->decryptionKey = sodium_crypto_box_keypair();
        $this->encryptionKey = sodium_crypto_box_publickey($this->decryptionKey);

-        $this->export('sodium.encrypt.public', $this->encryptionKey);
-        $this->export('sodium.decrypt.private', $this->decryptionKey);
+        $this->export('encrypt.public', $this->encryptionKey);
+        $this->export('decrypt.private', $this->decryptionKey);

-        $this->lastMessage = sprintf('Sodium keys have been generated at "%s*.{public,private}".', $this->getPrettyPath($this->pathPrefix));
+        $this->lastMessage = sprintf('Sodium keys have been generated at "%s*.public/private.php".', $this->getPrettyPath($this->pathPrefix));

        return true;
    }
@ -82,12 +84,12 @@ class SodiumVault extends AbstractVault
        $this->lastMessage = null;
        $this->validateName($name);
        $this->loadKeys();
-        $this->export($name.'.'.substr_replace(md5($name), '.sodium', -26), sodium_crypto_box_seal($value, $this->encryptionKey ?? sodium_crypto_box_publickey($this->decryptionKey)));
+        $this->export($name.'.'.substr(md5($name), 0, 6), sodium_crypto_box_seal($value, $this->encryptionKey ?? sodium_crypto_box_publickey($this->decryptionKey)));

        $list = $this->list();
        $list[$name] = null;
        uksort($list, 'strnatcmp');
-        file_put_contents($this->pathPrefix.'sodium.list', sprintf("<?php\n\nreturn %s;\n", var_export($list, true), LOCK_EX));
+        file_put_contents($this->pathPrefix.'list.php', sprintf("<?php\n\nreturn %s;\n", var_export($list, true), LOCK_EX));

        $this->lastMessage = sprintf('Secret "%s" encrypted in "%s"; you can commit it.', $name, $this->getPrettyPath(\dirname($this->pathPrefix).\DIRECTORY_SEPARATOR));
    }
@ -97,7 +99,7 @@ class SodiumVault extends AbstractVault
        $this->lastMessage = null;
        $this->validateName($name);

-        if (!file_exists($file = $this->pathPrefix.$name.'.'.substr_replace(md5($name), '.sodium', -26))) {
+        if (!file_exists($file = $this->pathPrefix.$name.'.'.substr_replace(md5($name), '.php', -26))) {
            $this->lastMessage = sprintf('Secret "%s" not found in "%s".', $name, $this->getPrettyPath(\dirname($this->pathPrefix).\DIRECTORY_SEPARATOR));

            return null;
@ -131,7 +133,7 @@ class SodiumVault extends AbstractVault
        $this->lastMessage = null;
        $this->validateName($name);

-        if (!file_exists($file = $this->pathPrefix.$name.'.'.substr_replace(md5($name), '.sodium', -26))) {
+        if (!file_exists($file = $this->pathPrefix.$name.'.'.substr_replace(md5($name), '.php', -26))) {
            $this->lastMessage = sprintf('Secret "%s" not found in "%s".', $name, $this->getPrettyPath(\dirname($this->pathPrefix).\DIRECTORY_SEPARATOR));

            return false;
@ -139,7 +141,7 @@ class SodiumVault extends AbstractVault

        $list = $this->list();
        unset($list[$name]);
-        file_put_contents($this->pathPrefix.'sodium.list', sprintf("<?php\n\nreturn %s;\n", var_export($list, true), LOCK_EX));
+        file_put_contents($this->pathPrefix.'list.php', sprintf("<?php\n\nreturn %s;\n", var_export($list, true), LOCK_EX));

        $this->lastMessage = sprintf('Secret "%s" removed from "%s".', $name, $this->getPrettyPath(\dirname($this->pathPrefix).\DIRECTORY_SEPARATOR));

@ -150,7 +152,7 @@ class SodiumVault extends AbstractVault
    {
        $this->lastMessage = null;

-        if (!file_exists($file = $this->pathPrefix.'sodium.list')) {
+        if (!file_exists($file = $this->pathPrefix.'list.php')) {
            return [];
        }

@ -167,6 +169,11 @@ class SodiumVault extends AbstractVault
        return $secrets;
    }

+    public function loadEnvVars(): array
+    {
+        return $this->list(true);
+    }
+
    private function loadKeys(): void
    {
        if (!\function_exists('sodium_crypto_box_seal')) {
@ -177,12 +184,12 @@ class SodiumVault extends AbstractVault
            return;
        }

-        if (file_exists($this->pathPrefix.'sodium.decrypt.private')) {
-            $this->decryptionKey = (string) include $this->pathPrefix.'sodium.decrypt.private';
+        if (file_exists($this->pathPrefix.'decrypt.private.php')) {
+            $this->decryptionKey = (string) include $this->pathPrefix.'decrypt.private.php';
        }

-        if (file_exists($this->pathPrefix.'sodium.encrypt.public')) {
-            $this->encryptionKey = (string) include $this->pathPrefix.'sodium.encrypt.public';
+        if (file_exists($this->pathPrefix.'encrypt.public.php')) {
+            $this->encryptionKey = (string) include $this->pathPrefix.'encrypt.public.php';
        } elseif ('' !== $this->decryptionKey) {
            $this->encryptionKey = sodium_crypto_box_publickey($this->decryptionKey);
        } else {
@ -196,7 +203,7 @@ class SodiumVault extends AbstractVault
        $data = str_replace('%', '\x', rawurlencode($data));
        $data = sprintf("<?php // %s on %s\n\nreturn \"%s\";\n", $name, date('r'), $data);

-        if (false === file_put_contents($this->pathPrefix.$file, $data, LOCK_EX)) {
+        if (false === file_put_contents($this->pathPrefix.$file.'.php', $data, LOCK_EX)) {
            $e = error_get_last();
            throw new \ErrorException($e['message'] ?? 'Failed to write secrets data.', 0, $e['type'] ?? E_USER_WARNING);
        }
--- a/src/Symfony/Bundle/FrameworkBundle/Tests/Secrets/SodiumVaultTest.php
+++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Secrets/SodiumVaultTest.php
@ -26,19 +26,19 @@ class SodiumVaultTest extends TestCase
        $vault = new SodiumVault($this->secretsDir);

        $this->assertTrue($vault->generateKeys());
-        $this->assertFileExists($this->secretsDir.'/test.sodium.encrypt.public');
-        $this->assertFileExists($this->secretsDir.'/test.sodium.decrypt.private');
+        $this->assertFileExists($this->secretsDir.'/test.encrypt.public.php');
+        $this->assertFileExists($this->secretsDir.'/test.decrypt.private.php');

-        $encKey = file_get_contents($this->secretsDir.'/test.sodium.encrypt.public');
-        $decKey = file_get_contents($this->secretsDir.'/test.sodium.decrypt.private');
+        $encKey = file_get_contents($this->secretsDir.'/test.encrypt.public.php');
+        $decKey = file_get_contents($this->secretsDir.'/test.decrypt.private.php');

        $this->assertFalse($vault->generateKeys());
-        $this->assertStringEqualsFile($this->secretsDir.'/test.sodium.encrypt.public', $encKey);
-        $this->assertStringEqualsFile($this->secretsDir.'/test.sodium.decrypt.private', $decKey);
+        $this->assertStringEqualsFile($this->secretsDir.'/test.encrypt.public.php', $encKey);
+        $this->assertStringEqualsFile($this->secretsDir.'/test.decrypt.private.php', $decKey);

        $this->assertTrue($vault->generateKeys(true));
-        $this->assertStringNotEqualsFile($this->secretsDir.'/test.sodium.encrypt.public', $encKey);
-        $this->assertStringNotEqualsFile($this->secretsDir.'/test.sodium.decrypt.private', $decKey);
+        $this->assertStringNotEqualsFile($this->secretsDir.'/test.encrypt.public.php', $encKey);
+        $this->assertStringNotEqualsFile($this->secretsDir.'/test.decrypt.private.php', $decKey);
    }

    public function testEncryptAndDecrypt()
--- a/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeDeclarationsPass.php
+++ b/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeDeclarationsPass.php
@ -188,7 +188,7 @@ final class CheckTypeDeclarationsPass extends AbstractRecursivePass
        $checkFunction = sprintf('is_%s', $parameter->getType()->getName());

        if (!$parameter->getType()->isBuiltin() || !$checkFunction($value)) {
-            throw new InvalidParameterTypeException($this->currentId, \gettype($value), $parameter);
+            throw new InvalidParameterTypeException($this->currentId, \is_object($value) ? \get_class($value) : \gettype($value), $parameter);
        }
    }
 }
--- a/src/Symfony/Component/DependencyInjection/EnvVarLoaderInterface.php
+++ b/src/Symfony/Component/DependencyInjection/EnvVarLoaderInterface.php
@ -0,0 +1,25 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\DependencyInjection;
+
+/**
+ * EnvVarLoaderInterface objects return key/value pairs that are added to the list of available env vars.
+ *
+ * @author Nicolas Grekas <p@tchwork.com>
+ */
+interface EnvVarLoaderInterface
+{
+    /**
+     * @return string[] Key/value pairs that can be accessed using the regular "%env()%" syntax
+     */
+    public function loadEnvVars(): array;
+}
--- a/src/Symfony/Component/DependencyInjection/EnvVarProcessor.php
+++ b/src/Symfony/Component/DependencyInjection/EnvVarProcessor.php
@ -12,6 +12,7 @@
 namespace Symfony\Component\DependencyInjection;

 use Symfony\Component\DependencyInjection\Exception\EnvNotFoundException;
+use Symfony\Component\DependencyInjection\Exception\ParameterCircularReferenceException;
 use Symfony\Component\DependencyInjection\Exception\RuntimeException;

 /**
@ -20,10 +21,17 @@ use Symfony\Component\DependencyInjection\Exception\RuntimeException;
 class EnvVarProcessor implements EnvVarProcessorInterface
 {
    private $container;
+    private $loaders;
+    private $loadedVars = [];

-    public function __construct(ContainerInterface $container)
+    /**
+     * @param EnvVarLoaderInterface[] $loaders
+     */
+    public function __construct(ContainerInterface $container, \Traversable $loaders = null)
    {
        $this->container = $container;
+        $this->loaders = new \IteratorIterator($loaders ?? new \ArrayIterator());
+        $this->loaders = $this->loaders->getInnerIterator();
    }

    /**
@ -127,12 +135,31 @@ class EnvVarProcessor implements EnvVarProcessorInterface
        } elseif (isset($_SERVER[$name]) && 0 !== strpos($name, 'HTTP_')) {
            $env = $_SERVER[$name];
        } elseif (false === ($env = getenv($name)) || null === $env) { // null is a possible value because of thread safety issues
-            if (!$this->container->hasParameter("env($name)")) {
-                throw new EnvNotFoundException(sprintf('Environment variable not found: "%s".', $name));
+            foreach ($this->loadedVars as $vars) {
+                if (false !== $env = ($vars[$name] ?? false)) {
+                    break;
+                }
            }

-            if (null === $env = $this->container->getParameter("env($name)")) {
-                return null;
+            try {
+                while ((false === $env || null === $env) && $this->loaders->valid()) {
+                    $loader = $this->loaders->current();
+                    $this->loaders->next();
+                    $this->loadedVars[] = $vars = $loader->loadEnvVars();
+                    $env = $vars[$name] ?? false;
+                }
+            } catch (ParameterCircularReferenceException $e) {
+                // skip loaders that need an env var that is not defined
+            }
+
+            if (false === $env || null === $env) {
+                if (!$this->container->hasParameter("env($name)")) {
+                    throw new EnvNotFoundException(sprintf('Environment variable not found: "%s".', $name));
+                }
+
+                if (null === $env = $this->container->getParameter("env($name)")) {
+                    return null;
+                }
            }
        }

--- a/src/Symfony/Component/DependencyInjection/Tests/EnvVarProcessorTest.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/EnvVarProcessorTest.php
@ -5,6 +5,7 @@ namespace Symfony\Component\DependencyInjection\Tests;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\DependencyInjection\Container;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\EnvVarLoaderInterface;
 use Symfony\Component\DependencyInjection\EnvVarProcessor;

 class EnvVarProcessorTest extends TestCase
@ -517,4 +518,39 @@ CSV;
            [null, null],
        ];
    }
+
+    public function testEnvLoader()
+    {
+        $loaders = function () {
+            yield new class() implements EnvVarLoaderInterface {
+                public function loadEnvVars(): array
+                {
+                    return [
+                        'FOO_ENV_LOADER' => '123',
+                    ];
+                }
+            };
+
+            yield new class() implements EnvVarLoaderInterface {
+                public function loadEnvVars(): array
+                {
+                    return [
+                        'FOO_ENV_LOADER' => '234',
+                        'BAR_ENV_LOADER' => '456',
+                    ];
+                }
+            };
+        };
+
+        $processor = new EnvVarProcessor(new Container(), $loaders());
+
+        $result = $processor->getEnv('string', 'FOO_ENV_LOADER', function () {});
+        $this->assertSame('123', $result);
+
+        $result = $processor->getEnv('string', 'BAR_ENV_LOADER', function () {});
+        $this->assertSame('456', $result);
+
+        $result = $processor->getEnv('string', 'FOO_ENV_LOADER', function () {});
+        $this->assertSame('123', $result); // check twice
+    }
 }