From 0d9e8cba3a19c353dcd3c52b062f59cdf04994f5 Mon Sep 17 00:00:00 2001 From: Tim Goudriaan Date: Thu, 13 Feb 2025 00:20:00 +0100 Subject: [PATCH 1/2] Add encryption functionality Encryption keys can be configured directly in the configuration or through files. Keys can be rotated manually. All parameters are passed to services to facilitate caching in the service container, regardless of the given configuration. Improvements include: - Adds configuration options for encryption keys - Adds compiler pass to pass configuration to encryption services and remove sensitive parameters from the container - Adds console command to generate encryption keys - Adds custom Doctrine type for encrypted values - Convert sensitive Credentials entity fields to encrypted fields with migration that automatically encrypts the data - Generate encryption keys during initialization in the standalone image --- .github/workflows/tests.yaml | 6 + .gitignore | 1 + Dockerfile | 1 + composer.json | 1 + composer.lock | 5 +- config/packages/doctrine.yaml | 3 + config/packages/doctrine_migrations.yaml | 2 + config/services.yaml | 1 + docker/config.yaml | 7 + docker/env.php | 4 + .../init/40-encryption-generate-keys.sh | 5 + migrations/Version20250311205816.php | 79 +++++++++ phpstan.dist.neon | 15 ++ src/Command/EncryptionGenerateKeysCommand.php | 91 +++++++++++ .../Compiler/EncryptionPass.php | 51 ++++++ .../DirigentConfiguration.php | 39 +++++ src/DependencyInjection/DirigentExtension.php | 16 ++ src/Doctrine/Entity/Credentials.php | 7 +- src/Doctrine/MigrationFactory.php | 28 ++++ src/Doctrine/Type/EncryptedTextType.php | 43 +++++ src/Encryption/Encryption.php | 115 +++++++++++++ src/Encryption/EncryptionException.php | 7 + src/EventListener/EncryptionListener.php | 28 ++++ src/Kernel.php | 4 + tests/Docker/Standalone/InitTest.php | 10 ++ .../EncryptionGenerateKeysCommandTest.php | 145 +++++++++++++++++ tests/UnitTests/Command/keys/.gitignore | 1 + tests/UnitTests/Encryption/EncryptionTest.php | 154 ++++++++++++++++++ tests/UnitTests/Encryption/keys/.gitignore | 1 + 29 files changed, 865 insertions(+), 5 deletions(-) create mode 100644 docker/scripts/init/40-encryption-generate-keys.sh create mode 100644 migrations/Version20250311205816.php create mode 100644 src/Command/EncryptionGenerateKeysCommand.php create mode 100644 src/DependencyInjection/Compiler/EncryptionPass.php create mode 100644 src/Doctrine/MigrationFactory.php create mode 100644 src/Doctrine/Type/EncryptedTextType.php create mode 100644 src/Encryption/Encryption.php create mode 100644 src/Encryption/EncryptionException.php create mode 100644 src/EventListener/EncryptionListener.php create mode 100644 tests/UnitTests/Command/EncryptionGenerateKeysCommandTest.php create mode 100644 tests/UnitTests/Command/keys/.gitignore create mode 100644 tests/UnitTests/Encryption/EncryptionTest.php create mode 100644 tests/UnitTests/Encryption/keys/.gitignore diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 8e9f218f..e3ae11e0 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -49,6 +49,9 @@ jobs: - name: Install Composer dependencies run: composer install --ansi --no-interaction --no-progress + - name: Generate encryption keys + run: bin/console encryption:generate-keys + - name: Validate mapping run: bin/console doctrine:schema:validate --skip-sync -vvv --ansi --no-interaction @@ -113,6 +116,9 @@ jobs: - name: Build assets run: npm run build + - name: Generate encryption keys + run: bin/console encryption:generate-keys + - name: Create database schema run: bin/console doctrine:schema:create --env=test diff --git a/.gitignore b/.gitignore index e7403932..dc2ac74a 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,7 @@ /config/dirigent.php /config/dirigent.yaml /config/dirigent.yml +/config/encryption/ /config/packages/dirigent.yaml /storage/ diff --git a/Dockerfile b/Dockerfile index 8ba6bba2..3fdb94d4 100644 --- a/Dockerfile +++ b/Dockerfile @@ -61,6 +61,7 @@ RUN set -e; \ php83-phar \ php83-session \ php83-simplexml \ + php83-sodium \ php83-tokenizer \ php83-xml \ postgresql \ diff --git a/composer.json b/composer.json index c5dabc48..905047b8 100644 --- a/composer.json +++ b/composer.json @@ -11,6 +11,7 @@ "ext-ctype": "*", "ext-curl": "*", "ext-iconv": "*", + "ext-sodium": "*", "cebe/markdown": "^1.2", "composer/composer": "^2.7", "doctrine/doctrine-bundle": "^2.11", diff --git a/composer.lock b/composer.lock index 5854929e..6eae5871 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "d4736e74afacd0c77fb686ea33c920bf", + "content-hash": "49279781d9bb199b229164c2d4952d3a", "packages": [ { "name": "cebe/markdown", @@ -14293,7 +14293,8 @@ "php": ">=8.3", "ext-ctype": "*", "ext-curl": "*", - "ext-iconv": "*" + "ext-iconv": "*", + "ext-sodium": "*" }, "platform-dev": {}, "platform-overrides": { diff --git a/config/packages/doctrine.yaml b/config/packages/doctrine.yaml index 64c0fc8a..e556f776 100644 --- a/config/packages/doctrine.yaml +++ b/config/packages/doctrine.yaml @@ -8,6 +8,9 @@ doctrine: profiling_collect_backtrace: '%kernel.debug%' use_savepoints: true + + types: + encrypted_text: CodedMonkey\Dirigent\Doctrine\Type\EncryptedTextType orm: auto_generate_proxy_classes: true enable_lazy_ghost_objects: true diff --git a/config/packages/doctrine_migrations.yaml b/config/packages/doctrine_migrations.yaml index 29231d94..5804c28e 100644 --- a/config/packages/doctrine_migrations.yaml +++ b/config/packages/doctrine_migrations.yaml @@ -4,3 +4,5 @@ doctrine_migrations: # as migrations classes should NOT be autoloaded 'DoctrineMigrations': '%kernel.project_dir%/migrations' enable_profiler: false + services: + 'Doctrine\Migrations\Version\MigrationFactory': CodedMonkey\Dirigent\Doctrine\MigrationFactory diff --git a/config/services.yaml b/config/services.yaml index d934f90e..d1005dea 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -31,4 +31,5 @@ services: public: true arguments: - + 'encryption:generate-keys': '@CodedMonkey\Dirigent\Command\EncryptionGenerateKeysCommand' 'packages:update': '@CodedMonkey\Dirigent\Command\PackagesUpdateCommand' diff --git a/docker/config.yaml b/docker/config.yaml index 6b1b16a4..ee958293 100644 --- a/docker/config.yaml +++ b/docker/config.yaml @@ -2,6 +2,13 @@ parameters: kernel_secret: '%env(default:kernel_secret_file:KERNEL_SECRET)%' kernel_secret_file: '%env(default::file:KERNEL_SECRET_FILE)%' +dirigent: + encryption: + private_key: '%env(DECRYPTION_KEY)%' + private_key_path: '%env(DECRYPTION_KEY_FILE)%' + public_key: '%env(ENCRYPTION_KEY)%' + public_key_path: '%env(ENCRYPTION_KEY_FILE)%' + framework: secret: '%kernel_secret%' diff --git a/docker/env.php b/docker/env.php index bca488ee..ebeaf337 100644 --- a/docker/env.php +++ b/docker/env.php @@ -5,6 +5,10 @@ 'DIRIGENT_IMAGE' => '1', 'SYMFONY_DOTENV_PATH' => './.env.dirigent', + 'DECRYPTION_KEY' => '', + 'DECRYPTION_KEY_FILE' => '/srv/config/secrets/decryption_key', + 'ENCRYPTION_KEY' => '', + 'ENCRYPTION_KEY_FILE' => '/srv/config/secrets/encryption_key', 'GITHUB_TOKEN' => '', 'KERNEL_SECRET_FILE' => '/srv/config/secrets/kernel_secret', 'MAILER_DSN' => 'null://null', diff --git a/docker/scripts/init/40-encryption-generate-keys.sh b/docker/scripts/init/40-encryption-generate-keys.sh new file mode 100644 index 00000000..fa5f4445 --- /dev/null +++ b/docker/scripts/init/40-encryption-generate-keys.sh @@ -0,0 +1,5 @@ +#!/usr/bin/env sh + +set -e + +bin/console encryption:generate-keys --no-ansi --no-interaction diff --git a/migrations/Version20250311205816.php b/migrations/Version20250311205816.php new file mode 100644 index 00000000..0d1cb438 --- /dev/null +++ b/migrations/Version20250311205816.php @@ -0,0 +1,79 @@ +addSql('ALTER TABLE credentials ALTER username TYPE TEXT'); + $this->addSql('ALTER TABLE credentials ALTER password TYPE TEXT'); + $this->addSql('ALTER TABLE credentials ALTER token TYPE TEXT'); + + $credentialsCollection = $this->connection->fetchAllAssociative('SELECT id, username, password, token FROM credentials'); + + foreach ($credentialsCollection as $credentials) { + if (null !== $credentials['username']) { + $sealedUsername = $this->encryptionUtility->seal($credentials['username']); + $this->addSql('UPDATE credentials SET username = ? WHERE id = ?', [$sealedUsername, $credentials['id']]); + } + + if (null !== $credentials['password']) { + $sealedPassword = $this->encryptionUtility->seal($credentials['password']); + $this->addSql('UPDATE credentials SET password = ? WHERE id = ?', [$sealedPassword, $credentials['id']]); + } + + if (null !== $credentials['token']) { + $sealedToken = $this->encryptionUtility->seal($credentials['token']); + $this->addSql('UPDATE credentials SET token = ? WHERE id = ?', [$sealedToken, $credentials['id']]); + } + } + } + + public function down(Schema $schema): void + { + $credentialsCollection = $this->connection->fetchAllAssociative('SELECT id, username, password, token FROM credentials'); + + foreach ($credentialsCollection as $credentials) { + if (null !== $credentials['username']) { + $username = $this->encryptionUtility->reveal($credentials['username']); + $this->addSql('UPDATE credentials SET username = ? WHERE id = ?', [$username, $credentials['id']]); + } + + if (null !== $credentials['password']) { + $password = $this->encryptionUtility->reveal($credentials['password']); + $this->addSql('UPDATE credentials SET password = ? WHERE id = ?', [$password, $credentials['id']]); + } + + if (null !== $credentials['token']) { + $token = $this->encryptionUtility->reveal($credentials['token']); + $this->addSql('UPDATE credentials SET token = ? WHERE id = ?', [$token, $credentials['id']]); + } + } + + $this->addSql('ALTER TABLE credentials ALTER username TYPE VARCHAR(255)'); + $this->addSql('ALTER TABLE credentials ALTER password TYPE VARCHAR(255)'); + $this->addSql('ALTER TABLE credentials ALTER token TYPE VARCHAR(255)'); + } +} diff --git a/phpstan.dist.neon b/phpstan.dist.neon index 6cb1eb7c..848d868b 100644 --- a/phpstan.dist.neon +++ b/phpstan.dist.neon @@ -16,3 +16,18 @@ parameters: identifier: method.notFound count: 1 path: src/DependencyInjection/DirigentConfiguration.php + - + message: '#^Left side of \|\| is always false\.$#' + identifier: booleanOr.leftAlwaysFalse + count: 1 + path: src/Encryption/Encryption.php + - + message: '#^Right side of \|\| is always false\.$#' + identifier: booleanOr.rightAlwaysFalse + count: 1 + path: src/Encryption/Encryption.php + - + message: '#^Property CodedMonkey\\Dirigent\\EventListener\\EncryptionListener\:\:\$connection is never read, only written\.$#' + identifier: property.onlyWritten + count: 1 + path: src/EventListener/EncryptionListener.php diff --git a/src/Command/EncryptionGenerateKeysCommand.php b/src/Command/EncryptionGenerateKeysCommand.php new file mode 100644 index 00000000..37d46ebb --- /dev/null +++ b/src/Command/EncryptionGenerateKeysCommand.php @@ -0,0 +1,91 @@ +privateKey || $this->publicKey) { + $io->info('Encryption key files are disabled.'); + + return Command::SUCCESS; + } + + if (!$this->privateKeyPath || !$this->publicKeyPath) { + $io->warning('Please provide a path for both a public and a private encryption key.'); + + return Command::FAILURE; + } + + $filesystem = new Filesystem(); + + $decryptionKeyExists = $filesystem->exists($this->privateKeyPath); + $encryptionKeyExists = $filesystem->exists($this->publicKeyPath); + + if (!$decryptionKeyExists && $encryptionKeyExists) { + $io->error('Unable to generate (private) decryption key because a (public) encryption key exists.'); + + return Command::FAILURE; + } elseif ($decryptionKeyExists && $encryptionKeyExists) { + $io->info('Encryption keys already exist.'); + } elseif ($decryptionKeyExists && !$encryptionKeyExists) { + $decryptionKey = sodium_hex2bin($filesystem->readFile($this->privateKeyPath)); + $encryptionKey = sodium_crypto_box_publickey($decryptionKey); + + $filesystem->dumpFile($this->publicKeyPath, sodium_bin2hex($encryptionKey)); + + $io->success('Generated a new (public) encryption key.'); + } else { + $decryptionKey = sodium_crypto_box_keypair(); + $encryptionKey = sodium_crypto_box_publickey($decryptionKey); + + $filesystem->dumpFile($this->privateKeyPath, sodium_bin2hex($decryptionKey)); + $filesystem->dumpFile($this->publicKeyPath, sodium_bin2hex($encryptionKey)); + + $io->success('Generated encryption keys.'); + } + + return $this->validateKeys($io); + } + + private function validateKeys(StyleInterface $output): int + { + $encryption = Encryption::create(null, $this->privateKeyPath, null, $this->publicKeyPath, [], []); + + try { + $encryption->validate(); + + return Command::SUCCESS; + } catch (EncryptionException $exception) { + $output->error($exception->getMessage()); + + return Command::FAILURE; + } + } +} diff --git a/src/DependencyInjection/Compiler/EncryptionPass.php b/src/DependencyInjection/Compiler/EncryptionPass.php new file mode 100644 index 00000000..85453c2b --- /dev/null +++ b/src/DependencyInjection/Compiler/EncryptionPass.php @@ -0,0 +1,51 @@ +getParameterBag(); + + $privateKey = $parameterBag->get('dirigent.encryption.private_key'); + $publicKey = $parameterBag->get('dirigent.encryption.public_key'); + $rotatedKeys = $parameterBag->get('dirigent.encryption.rotated_keys'); + + $privateKeyPath = $parameterBag->get('dirigent.encryption.private_key_path'); + $publicKeyPath = $parameterBag->get('dirigent.encryption.public_key_path'); + $rotatedKeyPaths = $parameterBag->get('dirigent.encryption.rotated_key_paths'); + + $container->getDefinition(Encryption::class) + ->setFactory([Encryption::class, 'create']) + ->setArguments([ + $privateKey, + $privateKeyPath, + $publicKey, + $publicKeyPath, + $rotatedKeys, + $rotatedKeyPaths, + ]); + + $container->getDefinition(EncryptionGenerateKeysCommand::class) + ->setArguments([ + $privateKey, + $privateKeyPath, + $publicKey, + $publicKeyPath, + ]); + + $parameterBag->remove('dirigent.encryption.private_key'); + $parameterBag->remove('dirigent.encryption.public_key'); + $parameterBag->remove('dirigent.encryption.rotated_keys'); + + $parameterBag->remove('dirigent.encryption.private_key_path'); + $parameterBag->remove('dirigent.encryption.public_key_path'); + $parameterBag->remove('dirigent.encryption.rotated_key_paths'); + } +} diff --git a/src/DependencyInjection/DirigentConfiguration.php b/src/DependencyInjection/DirigentConfiguration.php index 96a741c9..dac63d12 100644 --- a/src/DependencyInjection/DirigentConfiguration.php +++ b/src/DependencyInjection/DirigentConfiguration.php @@ -4,6 +4,7 @@ use Symfony\Component\Config\Definition\Builder\TreeBuilder; use Symfony\Component\Config\Definition\ConfigurationInterface; +use function Symfony\Component\String\u; class DirigentConfiguration implements ConfigurationInterface { @@ -22,6 +23,44 @@ public function getConfigTreeBuilder(): TreeBuilder ->booleanNode('registration')->defaultFalse()->end() ->end() ->end() + ->arrayNode('encryption') + ->info('Dirigent uses a X25519 keypair to encrypt sensitive info stored in the database') + ->addDefaultsIfNotSet() + ->children() + ->scalarNode('private_key') + ->defaultNull() + ->info('The (private) decryption key, if empty, a file will be used to store the key instead') + ->end() + ->scalarNode('private_key_path') + ->defaultValue('%kernel.project_dir%/config/encryption/private.key') + ->info('Path to the (private) decryption key if private_key is empty') + ->end() + ->scalarNode('public_key') + ->defaultNull() + ->info('The (public) encryption key, if empty, a file will be used to store the key instead') + ->end() + ->scalarNode('public_key_path') + ->defaultValue('%kernel.project_dir%/config/encryption/public.key') + ->info('Path to the (public) encryption key if public_key is empty') + ->end() + ->arrayNode('rotated_keys') + ->info('Previously used (private) decryption keys') + ->beforeNormalization() + ->ifString() + ->then(fn (string $keys): array => u($$keys)->split(',')) + ->end() + ->prototype('scalar')->end() + ->end() + ->arrayNode('rotated_key_paths') + ->info('Paths to previously used (private) decryption keys') + ->beforeNormalization() + ->ifString() + ->then(fn (string $paths): array => u($$paths)->split(',')) + ->end() + ->prototype('scalar')->end() + ->end() + ->end() + ->end() ->arrayNode('storage') ->addDefaultsIfNotSet() ->children() diff --git a/src/DependencyInjection/DirigentExtension.php b/src/DependencyInjection/DirigentExtension.php index 59f656f7..824f814c 100644 --- a/src/DependencyInjection/DirigentExtension.php +++ b/src/DependencyInjection/DirigentExtension.php @@ -19,6 +19,8 @@ protected function loadInternal(array $mergedConfig, ContainerBuilder $container $container->setParameter('dirigent.title', $mergedConfig['title']); $container->setParameter('dirigent.slug', $slug); + $this->registerEncryptionConfiguration($mergedConfig['encryption'], $container); + $container->setParameter('dirigent.security.public_access', $mergedConfig['security']['public']); $container->setParameter('dirigent.security.registration_enabled', $mergedConfig['security']['registration']); @@ -42,4 +44,18 @@ public function getConfiguration(array $config, ContainerBuilder $container): Co { return new DirigentConfiguration(); } + + /** + * @param array{private_key: ?string, private_key_path: ?string, public_key: ?string, public_key_path: ?string, rotated_keys: array, rotated_key_paths: array} $config + */ + private function registerEncryptionConfiguration(array $config, ContainerBuilder $container): void + { + $container->setParameter('dirigent.encryption.private_key', $config['private_key']); + $container->setParameter('dirigent.encryption.public_key', $config['public_key']); + $container->setParameter('dirigent.encryption.rotated_keys', $config['rotated_keys']); + + $container->setParameter('dirigent.encryption.private_key_path', $config['private_key_path']); + $container->setParameter('dirigent.encryption.public_key_path', $config['public_key_path']); + $container->setParameter('dirigent.encryption.rotated_key_paths', $config['rotated_key_paths']); + } } diff --git a/src/Doctrine/Entity/Credentials.php b/src/Doctrine/Entity/Credentials.php index 8e82b704..c428ed54 100644 --- a/src/Doctrine/Entity/Credentials.php +++ b/src/Doctrine/Entity/Credentials.php @@ -3,6 +3,7 @@ namespace CodedMonkey\Dirigent\Doctrine\Entity; use CodedMonkey\Dirigent\Doctrine\Repository\CredentialsRepository; +use CodedMonkey\Dirigent\Doctrine\Type\EncryptedTextType; use Doctrine\DBAL\Types\Types; use Doctrine\ORM\Mapping as ORM; @@ -23,13 +24,13 @@ class Credentials #[ORM\Column(type: Types::STRING, enumType: CredentialsType::class)] private CredentialsType|string $type = CredentialsType::HttpBasic; - #[ORM\Column(nullable: true)] + #[ORM\Column(type: EncryptedTextType::TYPE, nullable: true)] private ?string $username = null; - #[ORM\Column(nullable: true)] + #[ORM\Column(type: EncryptedTextType::TYPE, nullable: true)] private ?string $password = null; - #[ORM\Column(nullable: true)] + #[ORM\Column(type: EncryptedTextType::TYPE, nullable: true)] private ?string $token = null; public function getId(): ?int diff --git a/src/Doctrine/MigrationFactory.php b/src/Doctrine/MigrationFactory.php new file mode 100644 index 00000000..d6bf384d --- /dev/null +++ b/src/Doctrine/MigrationFactory.php @@ -0,0 +1,28 @@ +connection, $this->logger, $this->encryptionUtility); + } + + return new $migrationClassName($this->connection, $this->logger); + } +} diff --git a/src/Doctrine/Type/EncryptedTextType.php b/src/Doctrine/Type/EncryptedTextType.php new file mode 100644 index 00000000..0059f3d8 --- /dev/null +++ b/src/Doctrine/Type/EncryptedTextType.php @@ -0,0 +1,43 @@ +encryption = $encryption; + } + + public function getSQLDeclaration(array $column, AbstractPlatform $platform): string + { + // Use LONGTEXT column + return $platform->getClobTypeDeclarationSQL($column); + } + + public function convertToPHPValue($value, AbstractPlatform $platform): mixed + { + if (null === $value) { + return null; + } + + return $this->encryption->reveal($value); + } + + public function convertToDatabaseValue($value, AbstractPlatform $platform): mixed + { + if (null === $value) { + return null; + } + + return $this->encryption->seal($value); + } +} diff --git a/src/Encryption/Encryption.php b/src/Encryption/Encryption.php new file mode 100644 index 00000000..1a9d04ac --- /dev/null +++ b/src/Encryption/Encryption.php @@ -0,0 +1,115 @@ +exists($privateKeyPath)) { + throw new \RuntimeException("Private decryption key file \"$privateKeyPath\" does not exist."); + } elseif (!$filesystem->exists($publicKeyPath)) { + throw new \RuntimeException("Public encryption key file \"$publicKeyPath\" does not exist."); + } + + foreach ($rotatedKeyPaths as $rotatedKeyPath) { + if (!$filesystem->exists($rotatedKeyPath)) { + throw new \RuntimeException("Rotated key file \"$rotatedKeyPath\" does not exist."); + } + } + + $privateKey = $filesystem->readFile($privateKeyPath); + $publicKey = $filesystem->readFile($publicKeyPath); + $rotatedKeys = array_map( + fn (string $rotatedKeyPath): string => $filesystem->readFile($rotatedKeyPath), + $rotatedKeyPaths + ); + } + + $binaryPrivateKey = sodium_hex2bin($privateKey); + $binaryPublicKey = sodium_hex2bin($publicKey); + $binaryRotatedKeys = array_map( + fn (string $rotatedKey): string => sodium_hex2bin($rotatedKey), + $rotatedKeys, + ); + + return new self($binaryPrivateKey, $binaryPublicKey, $binaryRotatedKeys); + } + + public function seal(#[\SensitiveParameter] string $data): string + { + $binary = sodium_crypto_box_seal($data, $this->publicKey); + + return sodium_bin2hex($binary); + } + + public function reveal(#[\SensitiveParameter] string $data): string + { + $binary = sodium_hex2bin($data); + $value = sodium_crypto_box_seal_open($binary, $this->privateKey); + + if (false !== $value) { + return $value; + } + + foreach ($this->rotatedKeys as $rotatedKey) { + $value = sodium_crypto_box_seal_open($binary, $rotatedKey); + + if (false !== $value) { + return $value; + } + } + + throw new EncryptionException('Unable to decrypt data.'); + } + + public function validate(): void + { + $value = 'thank you for the music'; + $sealedValue = $this->seal($value); + + $binary = sodium_hex2bin($sealedValue); + $revealedValue = sodium_crypto_box_seal_open($binary, $this->privateKey); + + if (false === $revealedValue) { + throw new EncryptionException('The encryption key is not valid.'); + } + } +} diff --git a/src/Encryption/EncryptionException.php b/src/Encryption/EncryptionException.php new file mode 100644 index 00000000..54463d34 --- /dev/null +++ b/src/Encryption/EncryptionException.php @@ -0,0 +1,7 @@ +setEncryptionUtility($this->encryption); + } +} diff --git a/src/Kernel.php b/src/Kernel.php index 898d5420..ff51bba5 100644 --- a/src/Kernel.php +++ b/src/Kernel.php @@ -2,6 +2,7 @@ namespace CodedMonkey\Dirigent; +use CodedMonkey\Dirigent\DependencyInjection\Compiler\EncryptionPass; use CodedMonkey\Dirigent\DependencyInjection\DirigentExtension; use Symfony\Bundle\FrameworkBundle\Kernel\MicroKernelTrait; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -30,6 +31,9 @@ protected function configureContainer(ContainerConfigurator $container): void protected function build(ContainerBuilder $container): void { $container->registerExtension(new DirigentExtension()); + + // The encryption pass has to be the first pass to run as it removes sensitive data from the container + $container->addCompilerPass(new EncryptionPass(), priority: 2048); } public function boot(): void diff --git a/tests/Docker/Standalone/InitTest.php b/tests/Docker/Standalone/InitTest.php index 461265d1..0aed9bac 100644 --- a/tests/Docker/Standalone/InitTest.php +++ b/tests/Docker/Standalone/InitTest.php @@ -33,6 +33,16 @@ public function testKernelSecretGenerated(): void '/srv/config/secrets/kernel_secret', 'A kernel_secret file must be generated.', ); + + $this->assertContainerFileExists( + '/srv/config/secrets/decryption_key', + 'A decryption_key file must be generated.', + ); + + $this->assertContainerFileExists( + '/srv/config/secrets/encryption_key', + 'A encryption_key file must be generated.', + ); } public function testKernelSecretNotRegeneratedOnRestart(): void diff --git a/tests/UnitTests/Command/EncryptionGenerateKeysCommandTest.php b/tests/UnitTests/Command/EncryptionGenerateKeysCommandTest.php new file mode 100644 index 00000000..89b513d1 --- /dev/null +++ b/tests/UnitTests/Command/EncryptionGenerateKeysCommandTest.php @@ -0,0 +1,145 @@ +filesystem = new Filesystem(); + + $this->clearKeys(); + } + + protected function tearDown(): void + { + $this->clearKeys(); + } + + private function clearKeys(): void + { + $files = Finder::create() + ->files() + ->in(__DIR__ . '/keys') + ->name('*.key'); + + foreach ($files as $file) { + $this->filesystem->remove($file->getRealPath()); + } + } + + public function testGenerateNewKeys(): void + { + $command = new EncryptionGenerateKeysCommand(null, $this->privateKeyPath, null, $this->publicKeyPath); + $output = new BufferedOutput(); + + $exitCode = $command->run(new ArrayInput([]), $output); + + $this->assertSame(Command::SUCCESS, $exitCode); + $this->assertStringContainsString('Generated encryption keys.', $output->fetch()); + } + + public function testEncryptionKeysAlreadyExist(): void + { + $command = new EncryptionGenerateKeysCommand(null, $this->privateKeyPath, null, $this->publicKeyPath); + $output = new BufferedOutput(); + + // Generate keys first + $command->run(new ArrayInput([]), new NullOutput()); + + $exitCode = $command->run(new ArrayInput([]), $output); + + $this->assertSame(Command::SUCCESS, $exitCode); + $this->assertStringContainsString('Encryption keys already exist.', $output->fetch()); + } + + public function testGenerateNewPublicKey(): void + { + $command = new EncryptionGenerateKeysCommand(null, $this->privateKeyPath, null, $this->publicKeyPath); + $output = new BufferedOutput(); + + // Generate keys first + $command->run(new ArrayInput([]), new NullOutput()); + + $this->filesystem->remove($this->publicKeyPath); + + $exitCode = $command->run(new ArrayInput([]), $output); + + $this->assertSame(Command::SUCCESS, $exitCode); + $this->assertStringContainsString('Generated a new (public) encryption key.', $output->fetch()); + } + + public function testMissingPrivateKey(): void + { + $command = new EncryptionGenerateKeysCommand(null, $this->privateKeyPath, null, $this->publicKeyPath); + $output = new BufferedOutput(); + + // Generate keys first + $command->run(new ArrayInput([]), new NullOutput()); + + $this->filesystem->remove($this->privateKeyPath); + + $exitCode = $command->run(new ArrayInput([]), $output); + + $this->assertSame(Command::FAILURE, $exitCode); + $this->assertStringContainsString('Unable to generate (private) decryption key', $output->fetch()); + } + + public function testInvalidKeys(): void + { + $command = new EncryptionGenerateKeysCommand(null, $this->privateKeyPath, null, $this->publicKeyPath); + $output = new BufferedOutput(); + + // Generate keys first + $command->run(new ArrayInput([]), new NullOutput()); + + $this->filesystem->rename($this->privateKeyPath, $this->privateKeyPath . '_tmp'); + $this->filesystem->remove($this->publicKeyPath); + + // Generate new keys + $command->run(new ArrayInput([]), new NullOutput()); + + $this->filesystem->rename($this->privateKeyPath . '_tmp', $this->privateKeyPath, true); + + $exitCode = $command->run(new ArrayInput([]), $output); + + $this->assertSame(Command::FAILURE, $exitCode); + $this->assertStringContainsString('The encryption key is not valid.', $output->fetch()); + } + + public function testEncryptionKeysDisabled(): void + { + $command = new EncryptionGenerateKeysCommand('123', null, '123', null); + $output = new BufferedOutput(); + + $exitCode = $command->run(new ArrayInput([]), $output); + + $this->assertSame(Command::SUCCESS, $exitCode); + $this->assertStringContainsString('Encryption key files are disabled.', $output->fetch()); + } + + public function testEncryptionKeysMissing(): void + { + $command = new EncryptionGenerateKeysCommand(null, null, null, null); + $output = new BufferedOutput(); + + $exitCode = $command->run(new ArrayInput([]), $output); + + $this->assertSame(Command::FAILURE, $exitCode); + $this->assertStringContainsString('Please provide a path for both a public and a private encryption key.', $output->fetch()); + } +} diff --git a/tests/UnitTests/Command/keys/.gitignore b/tests/UnitTests/Command/keys/.gitignore new file mode 100644 index 00000000..c996e507 --- /dev/null +++ b/tests/UnitTests/Command/keys/.gitignore @@ -0,0 +1 @@ +*.key diff --git a/tests/UnitTests/Encryption/EncryptionTest.php b/tests/UnitTests/Encryption/EncryptionTest.php new file mode 100644 index 00000000..7c441bcd --- /dev/null +++ b/tests/UnitTests/Encryption/EncryptionTest.php @@ -0,0 +1,154 @@ +files() + ->in(__DIR__ . '/keys') + ->name('*.key'); + + foreach ($files as $file) { + (new Filesystem())->remove($file->getRealPath()); + } + } + + public function testSeal(): void + { + $privateKey = sodium_crypto_box_keypair(); + $publicKey = sodium_crypto_box_publickey($privateKey); + + $encryption = new Encryption($privateKey, $publicKey, []); + + $sealedData = $encryption->seal(self::DATA); + + $this->assertSame(self::DATA, sodium_crypto_box_seal_open(sodium_hex2bin($sealedData), $privateKey)); + } + + public function testReveal(): void + { + $privateKey = sodium_crypto_box_keypair(); + $publicKey = sodium_crypto_box_publickey($privateKey); + + $encryption = new Encryption($privateKey, $publicKey, []); + + $sealedData = sodium_bin2hex(sodium_crypto_box_seal(self::DATA, $publicKey)); + + $this->assertSame(self::DATA, $encryption->reveal($sealedData)); + } + + public function testRevealWithRotatedKey(): void + { + $rotatedPrivateKey = sodium_crypto_box_keypair(); + $rotatedPublicKey = sodium_crypto_box_publickey($rotatedPrivateKey); + + $privateKey = sodium_crypto_box_keypair(); + $publicKey = sodium_crypto_box_publickey($privateKey); + + $encryption = new Encryption($privateKey, $publicKey, [$rotatedPrivateKey]); + + $sealedData = sodium_bin2hex(sodium_crypto_box_seal(self::DATA, $rotatedPublicKey)); + + $this->assertSame(self::DATA, $encryption->reveal($sealedData)); + } + + public function testRevealFailsWithInvalidPublicKey(): void + { + $privateKey = sodium_crypto_box_keypair(); + $publicKey = sodium_crypto_box_publickey(sodium_crypto_box_keypair()); + + $encryption = new Encryption($privateKey, $publicKey, []); + + $sealedData = $encryption->seal(self::DATA); + + $this->expectException(EncryptionException::class); + $this->expectExceptionMessage('Unable to decrypt data'); + + $encryption->reveal($sealedData); + } + + public function testCreateFromHex(): void + { + $privateKey = sodium_crypto_box_keypair(); + $publicKey = sodium_crypto_box_publickey($privateKey); + + $encryption = Encryption::create( + sodium_bin2hex($privateKey), + null, + sodium_bin2hex($publicKey), + null, + [], + [], + ); + + $this->validateEncryption($encryption); + } + + public function testCreateFromFiles(): void + { + $filesystem = new Filesystem(); + + $privateKey = sodium_crypto_box_keypair(); + $publicKey = sodium_crypto_box_publickey($privateKey); + + $privateKeyPath = __DIR__ . '/keys/private.key'; + $publicKeyPath = __DIR__ . '/keys/public.key'; + + $filesystem->dumpFile($privateKeyPath, sodium_bin2hex($privateKey)); + $filesystem->dumpFile($publicKeyPath, sodium_bin2hex($publicKey)); + + $encryption = Encryption::create( + null, + $privateKeyPath, + null, + $publicKeyPath, + [], + [], + ); + + $this->validateEncryption($encryption); + } + + public function testValidate(): void + { + $this->expectNotToPerformAssertions(); + + $privateKey = sodium_crypto_box_keypair(); + $publicKey = sodium_crypto_box_publickey($privateKey); + + $encryption = new Encryption($privateKey, $publicKey, []); + + $encryption->validate(); + } + + public function testValidateFailsWithInvalidPublicKey(): void + { + $privateKey = sodium_crypto_box_keypair(); + $publicKey = sodium_crypto_box_publickey(sodium_crypto_box_keypair()); + + $this->expectException(EncryptionException::class); + $this->expectExceptionMessage('The encryption key is not valid.'); + + $encryption = new Encryption($privateKey, $publicKey, []); + + $encryption->validate(); + } + + private function validateEncryption(Encryption $encryption): void + { + $data = $encryption->reveal($encryption->seal(self::DATA)); + + $this->assertSame(self::DATA, $data); + } +} diff --git a/tests/UnitTests/Encryption/keys/.gitignore b/tests/UnitTests/Encryption/keys/.gitignore new file mode 100644 index 00000000..c996e507 --- /dev/null +++ b/tests/UnitTests/Encryption/keys/.gitignore @@ -0,0 +1 @@ +*.key From bfc5b02b119acf32ec79018db017f6415871fb1d Mon Sep 17 00:00:00 2001 From: Tim Goudriaan Date: Wed, 12 Mar 2025 00:17:41 +0100 Subject: [PATCH 2/2] Add security section to documentation --- docs/security.md | 71 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 69 insertions(+), 2 deletions(-) diff --git a/docs/security.md b/docs/security.md index d99b4319..2d4fa23a 100644 --- a/docs/security.md +++ b/docs/security.md @@ -4,10 +4,77 @@ sidebar_position: 90 # Security +## Kernel secret + +To learn more about how and why the kernel secret is used, check out the [Symfony documentation](https://symfony.com/doc/7.2/reference/configuration/framework.html#secret). + +:::note + +When using the standalone image, the kernel secret is generated automatically. See the [Image secrets](#image-secrets) +section to learn more. + +::: + +To configure the kernel secret through a custom environment variable, use the following configuration: + +```yaml +framework: + secret: '%env(KERNEL_SECRET)%' +``` + +## Encryption + +In some cases, Dirigent needs to store sensitive information in the database, like GitHub access tokens or SSH keys +that are used for authenticating to private repositories. As a safety precaution, this data is encrypted during +runtime through an encryption key before being stored securely in the database. The encryption key has to be created +before running the application. + +### Generate encryption key pair + +To generate an encryption key pair, run the following command: + +```shell +bin/dirigent encryption:generate-keys +``` + :::note -This page is a stub. +When using the standalone image, this is done automatically when starting the container. See the [Image secrets](#image-secrets) +section to learn more. ::: -## Secrets +This generates both a (private) decryption key and a (public) encryption key, both need to exist for Dirigent to +function. The location of the keys can be changed in the configuration. For example, to use environment variables +to configure the encryption keys, use the following configuration: + +```yaml +dirigent: + encryption: + private_key: '%env(DECRYPTION_KEY)%' + private_key_path: '%env(DECRYPTION_KEY_FILE)%' + public_key: '%env(ENCRYPTION_KEY)%' + public_key_path: '%env(ENCRYPTION_KEY_FILE)%' +``` + +### Rotate encryption keys + +```yaml +dirigent: + encryption: + rotated_keys: + - '%env(OLD_DECRYPTION_KEY)%' + rotated_key_paths: + - '%env(OLD_DECRYPTION_KEY_FILE)%' +``` + +## Image secrets + +When using the standalone image, secrets are stored in the `/srv/config/secrets` directory by default. + +- `decryption_key` + Unless configured through `DECRYPTION_KEY` or `DECRYPTION_KEY_FILE` environment variables. +- `encryption_key` + Unless configured through `ENCRYPTION_KEY` or `ENCRYPTION_KEY_FILE` environment variables. +- `kernel_secret` + Unless configured through `KERNEL_SECRET` or `KERNEL_SECRET_FILE` environment variables.