From 8b3ebdf51150520a28d3deb80d737b73caa4c057 Mon Sep 17 00:00:00 2001 From: Bruno Fache Date: Mon, 22 Jul 2024 12:29:38 +0200 Subject: [PATCH 1/5] feat: Add ability to specify the encryption key to use in 'generate' command --- Console/GenerateEncryptionKey.php | 15 ++++++++++++++- README.md | 5 +++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/Console/GenerateEncryptionKey.php b/Console/GenerateEncryptionKey.php index 756c947..f4490ac 100644 --- a/Console/GenerateEncryptionKey.php +++ b/Console/GenerateEncryptionKey.php @@ -18,6 +18,8 @@ class GenerateEncryptionKey extends Command { public const INPUT_KEY_FORCE = 'force'; + public const INPUT_KEY_KEY = 'key'; + public const INPUT_KEY_KEY_SHORTCUT = 'k'; /** * @param ChangeEncryptionKeyService $changeEncryptionKey @@ -51,6 +53,12 @@ protected function configure() null, InputOption::VALUE_NONE, 'Whether to force this action to take effect' + ), + new InputOption( + self::INPUT_KEY_KEY, + self::INPUT_KEY_KEY_SHORTCUT, + InputOption::VALUE_OPTIONAL, + 'The crypt key to use' ) ]; @@ -68,10 +76,15 @@ protected function configure() */ protected function execute(InputInterface $input, OutputInterface $output): int { + $newKey = null; if (!$input->getOption(self::INPUT_KEY_FORCE)) { $output->writeln('Run with --force to generate a new key. This will decrypt and reencrypt values in core_config_data and saved credit card info'); return Cli::RETURN_FAILURE; } + if ($input->hasOption(self::INPUT_KEY_KEY)) { + $newKey = $input->getOption(self::INPUT_KEY_KEY); + $output->writeln('The new crypt key to use for re-encryption (32 chars). If not set, the new key will be generated'); + } try { $countOfKeys = count(explode(PHP_EOL, $this->encryptor->exportKeys())); @@ -86,7 +99,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $this->emulation->startEnvironmentEmulation(0, 'adminhtml'); $output->writeln('Generating a new encryption key using the magento core class'); $this->changeEncryptionKey->setOutput($output); - $this->changeEncryptionKey->changeEncryptionKey(); + $this->changeEncryptionKey->changeEncryptionKey($newKey); $this->emulation->stopEnvironmentEmulation(); $output->writeln('Cleaning cache'); diff --git a/README.md b/README.md index 9e419cf..6bae244 100644 --- a/README.md +++ b/README.md @@ -54,8 +54,9 @@ adobe_user_profile 2. **Review functions** using `->hash(` from the encryptor class. Changing the keys will result in a different hash. 3. If you have **custom logic** to handle that, it will be something you need to work that out manually. 3. **Generate a new key** `php bin/magento gene:encryption-key-manager:generate` - 1. `Magento\Catalog\Model\View\Asset\Image` will continue to use the key at the `0` index - 1. `Magento\JwtUserToken\Model\SecretBasedJwksFactory` will only use the most recently generated key at the highest index + 1. You can specify the new crypt key to use with 'php bin/magento gene:encryption-key-manager:generate --key=MY_32_CHAR_CRYPT_KEY' + 2. `Magento\Catalog\Model\View\Asset\Image` will continue to use the key at the `0` index + 3. `Magento\JwtUserToken\Model\SecretBasedJwksFactory` will only use the most recently generated key at the highest index 4. **Fix missing config values** `php bin/magento gene:encryption-key-manager:reencrypt-unhandled-core-config-data` 1. Re-run to verify `php bin/magento gene:encryption-key-manager:reencrypt-unhandled-core-config-data` 4. **Fix 2FA data** `php bin/magento gene:encryption-key-manager:reencrypt-tfa-data` From a38877b4a81bc29c7c7be84f20069b9fcaac8976 Mon Sep 17 00:00:00 2001 From: Bruno Fache Date: Mon, 22 Jul 2024 12:37:12 +0200 Subject: [PATCH 2/5] fix(key-change):Fix wording in generate commands output --- Console/GenerateEncryptionKey.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Console/GenerateEncryptionKey.php b/Console/GenerateEncryptionKey.php index f4490ac..ddcac12 100644 --- a/Console/GenerateEncryptionKey.php +++ b/Console/GenerateEncryptionKey.php @@ -58,7 +58,7 @@ protected function configure() self::INPUT_KEY_KEY, self::INPUT_KEY_KEY_SHORTCUT, InputOption::VALUE_OPTIONAL, - 'The crypt key to use' + 'The new crypt key to use for re-encryption (32 chars). If not set, the new key will be generated' ) ]; @@ -83,7 +83,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int } if ($input->hasOption(self::INPUT_KEY_KEY)) { $newKey = $input->getOption(self::INPUT_KEY_KEY); - $output->writeln('The new crypt key to use for re-encryption (32 chars). If not set, the new key will be generated'); + $output->writeln('The provided crypt key will be used for reencryption.'); + }else{ + $output->writeln('A new key will be generated for reencryption, use "--key" to specify a custom key.'); } try { From be0d8a08c4076c18327e75e25f93586e5bb72510 Mon Sep 17 00:00:00 2001 From: Bruno Fache Date: Mon, 22 Jul 2024 14:12:53 +0200 Subject: [PATCH 3/5] doc: Update doc about --key option for ene:encryption-key-manager:generate --- README.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 6bae244..5b01750 100644 --- a/README.md +++ b/README.md @@ -32,10 +32,12 @@ This is a rough list of steps that should be followed to prevent attacks with Co This should be every merchant's **priority!** Install this module and generate a new key with: -`php bin/magento gene:encryption-key-manager:generate` +`php bin/magento gene:encryption-key-manager:generate [--key=MY_32_CHAR_CRYPT_KEY]` This will force the JWT factory to use the newly generated key. Other areas of the application may continue to use the old keys. This step is the absolute priority and will help prevent attacks with CosmicSting. +> Use the `--key` option to manually define the new key to use during re-encryption. If no custom key is provided, a new key will be generated. + ## Fully rotate your old keys 1. **Review your database** for any tables with encrypted values. Make sure your dump is `--human-readable` (magerun) or `--extended-insert=FALSE` (mysqldump) so that all values are on the same line as the `INSERT INTO` @@ -54,7 +56,7 @@ adobe_user_profile 2. **Review functions** using `->hash(` from the encryptor class. Changing the keys will result in a different hash. 3. If you have **custom logic** to handle that, it will be something you need to work that out manually. 3. **Generate a new key** `php bin/magento gene:encryption-key-manager:generate` - 1. You can specify the new crypt key to use with 'php bin/magento gene:encryption-key-manager:generate --key=MY_32_CHAR_CRYPT_KEY' + 1. You can specify the new crypt key to use with `php bin/magento gene:encryption-key-manager:generate --key=MY_32_CHAR_CRYPT_KEY` 2. `Magento\Catalog\Model\View\Asset\Image` will continue to use the key at the `0` index 3. `Magento\JwtUserToken\Model\SecretBasedJwksFactory` will only use the most recently generated key at the highest index 4. **Fix missing config values** `php bin/magento gene:encryption-key-manager:reencrypt-unhandled-core-config-data` From a22f065beae6ea614a5cfaf303f7d4e50912c13c Mon Sep 17 00:00:00 2001 From: Bruno Fache Date: Mon, 22 Jul 2024 14:15:14 +0200 Subject: [PATCH 4/5] refactor: Check the '--key' param first to display the key notice even if the '--force' param is not set --- Console/GenerateEncryptionKey.php | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/Console/GenerateEncryptionKey.php b/Console/GenerateEncryptionKey.php index ddcac12..0be9f32 100644 --- a/Console/GenerateEncryptionKey.php +++ b/Console/GenerateEncryptionKey.php @@ -77,16 +77,17 @@ protected function configure() protected function execute(InputInterface $input, OutputInterface $output): int { $newKey = null; + if ($input->hasOption(self::INPUT_KEY_KEY)) { + $newKey = $input->getOption(self::INPUT_KEY_KEY); + $output->writeln('The provided crypt key will be used for re-encryption.'); + } else { + $output->writeln('A new key will be generated for re-encryption, use "--key" to specify a custom key.'); + } + if (!$input->getOption(self::INPUT_KEY_FORCE)) { $output->writeln('Run with --force to generate a new key. This will decrypt and reencrypt values in core_config_data and saved credit card info'); return Cli::RETURN_FAILURE; } - if ($input->hasOption(self::INPUT_KEY_KEY)) { - $newKey = $input->getOption(self::INPUT_KEY_KEY); - $output->writeln('The provided crypt key will be used for reencryption.'); - }else{ - $output->writeln('A new key will be generated for reencryption, use "--key" to specify a custom key.'); - } try { $countOfKeys = count(explode(PHP_EOL, $this->encryptor->exportKeys())); From 9f412a40843b0ff540d1916c46647b2eca5c9465 Mon Sep 17 00:00:00 2001 From: Bruno Fache Date: Mon, 22 Jul 2024 15:42:44 +0200 Subject: [PATCH 5/5] fix: replace hasOption by getOption to fix wrong displayed message --- Console/GenerateEncryptionKey.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Console/GenerateEncryptionKey.php b/Console/GenerateEncryptionKey.php index 0be9f32..644f2e2 100644 --- a/Console/GenerateEncryptionKey.php +++ b/Console/GenerateEncryptionKey.php @@ -77,7 +77,7 @@ protected function configure() protected function execute(InputInterface $input, OutputInterface $output): int { $newKey = null; - if ($input->hasOption(self::INPUT_KEY_KEY)) { + if ($input->getOption(self::INPUT_KEY_KEY)) { $newKey = $input->getOption(self::INPUT_KEY_KEY); $output->writeln('The provided crypt key will be used for re-encryption.'); } else {