rawilk / laravel-app-key-rotator

Rotate app keys around while re-encrypting data.
https://randallwilk.dev/docs/laravel-app-key-rotator
MIT License
22 stars 1 forks source link

unserialize(): Error at offset 0 of 27 bytes #1

Open singhpreet89 opened 2 years ago

singhpreet89 commented 2 years ago

Laravel App Key Rotator Version

v^2.0

Laravel Version

v9.22.1

PHP Version

v8.1.4

Bug description

I am getting the following error while running: php artisan app-key-rotator:rotate Code_-_Insiders_YtHE97DwgF

Steps to reproduce

Relevant log output

` [2022-08-02 17:17:35] laravel.EMERGENCY: Unable to create configured logger. Using emergency logger. {"exception":"[object] (InvalidArgumentException(code: 0): Log [deprecations] is not defined. at E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Log\LogManager.php:207) [stacktrace]

0 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Log\LogManager.php(132): Illuminate\Log\LogManager->resolve('deprecations', NULL)

1 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Log\LogManager.php(119): Illuminate\Log\LogManager->get('deprecations')

2 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Log\LogManager.php(108): Illuminate\Log\LogManager->driver('deprecations')

3 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Foundation\Bootstrap\HandleExceptions.php(122): Illuminate\Log\LogManager->channel('deprecations')

4 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Foundation\Bootstrap\HandleExceptions.php(71): Illuminate\Foundation\Bootstrap\HandleExceptions->handleDeprecationError('auto_detect_lin...', 'E:\\app_key_rota...', 138, 8192)

5 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Foundation\Bootstrap\HandleExceptions.php(257): Illuminate\Foundation\Bootstrap\HandleExceptions->handleError(8192, 'auto_detect_lin...', 'E:\\app_key_rota...', 138)

6 [internal function]: Illuminate\Foundation\Bootstrap\HandleExceptions->Illuminate\Foundation\Bootstrap\{closure}(8192, 'auto_detect_lin...', 'E:\\app_key_rota...', 138)

7 E:\app_key_rotation\vendor\jackiedo\dotenv-editor\src\Jackiedo\DotenvEditor\DotenvReader.php(138): ini_set('auto_detect_lin...', '1')

8 E:\app_key_rotation\vendor\jackiedo\dotenv-editor\src\Jackiedo\DotenvEditor\DotenvReader.php(110): Jackiedo\DotenvEditor\DotenvReader->readLinesFromFile()

9 E:\app_key_rotation\vendor\jackiedo\dotenv-editor\src\Jackiedo\DotenvEditor\DotenvEditor.php(224): Jackiedo\DotenvEditor\DotenvReader->keys()

10 E:\app_key_rotation\vendor\jackiedo\dotenv-editor\src\Jackiedo\DotenvEditor\DotenvEditor.php(244): Jackiedo\DotenvEditor\DotenvEditor->getKeys()

11 E:\app_key_rotation\vendor\jackiedo\dotenv-editor\src\Jackiedo\DotenvEditor\DotenvEditor.php(348): Jackiedo\DotenvEditor\DotenvEditor->keyExists('OLD_APP_KEY')

12 E:\app_key_rotation\vendor\jackiedo\dotenv-editor\src\Jackiedo\DotenvEditor\DotenvEditor.php(376): Jackiedo\DotenvEditor\DotenvEditor->setKeys(Array)

13 E:\app_key_rotation\vendor\rawilk\laravel-app-key-rotator\src\Commands\RotateAppKeyCommand.php(52): Jackiedo\DotenvEditor\DotenvEditor->setKey('OLD_APP_KEY', 'base64:D4y2Cp0E...', 'Rotated at: 202...')

14 E:\app_key_rotation\vendor\rawilk\laravel-app-key-rotator\src\Commands\RotateAppKeyCommand.php(41): Rawilk\AppKeyRotator\Commands\RotateAppKeyCommand->setKeyInEnvironmentFile('base64:D4y2Cp0E...', 'OLD_APP_KEY', 'Rotated at: 202...')

15 E:\app_key_rotation\vendor\rawilk\laravel-app-key-rotator\src\Commands\RotateAppKeyCommand.php(68): Rawilk\AppKeyRotator\Commands\RotateAppKeyCommand->getCurrentKey()

16 E:\app_key_rotation\vendor\rawilk\laravel-app-key-rotator\src\Commands\RotateAppKeyCommand.php(26): Rawilk\AppKeyRotator\Commands\RotateAppKeyCommand->setKeys()

17 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Container\BoundMethod.php(36): Rawilk\AppKeyRotator\Commands\RotateAppKeyCommand->handle()

18 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Container\Util.php(41): Illuminate\Container\BoundMethod::Illuminate\Container\{closure}()

19 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Container\BoundMethod.php(93): Illuminate\Container\Util::unwrapIfClosure(Object(Closure))

20 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Container\BoundMethod.php(37): Illuminate\Container\BoundMethod::callBoundMethod(Object(Illuminate\Foundation\Application), Array, Object(Closure))

21 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Container\Container.php(651): Illuminate\Container\BoundMethod::call(Object(Illuminate\Foundation\Application), Array, Array, NULL)

22 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Console\Command.php(139): Illuminate\Container\Container->call(Array)

23 E:\app_key_rotation\vendor\symfony\console\Command\Command.php(308): Illuminate\Console\Command->execute(Object(Symfony\Component\Console\Input\ArgvInput), Object(Illuminate\Console\OutputStyle))

24 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Console\Command.php(124): Symfony\Component\Console\Command\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Illuminate\Console\OutputStyle))

25 E:\app_key_rotation\vendor\symfony\console\Application.php(998): Illuminate\Console\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))

26 E:\app_key_rotation\vendor\symfony\console\Application.php(299): Symfony\Component\Console\Application->doRunCommand(Object(Rawilk\AppKeyRotator\Commands\RotateAppKeyCommand), Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))

27 E:\app_key_rotation\vendor\symfony\console\Application.php(171): Symfony\Component\Console\Application->doRun(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))

28 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Console\Application.php(102): Symfony\Component\Console\Application->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))

29 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Foundation\Console\Kernel.php(129): Illuminate\Console\Application->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))

30 E:\app_key_rotation\artisan(37): Illuminate\Foundation\Console\Kernel->handle(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))

31 {main}

"} [2022-08-02 17:17:35] laravel.WARNING: auto_detect_line_endings is deprecated in E:\app_key_rotation\vendor\jackiedo\dotenv-editor\src\Jackiedo\DotenvEditor\DotenvReader.php on line 138
[2022-08-02 17:17:35] laravel.EMERGENCY: Unable to create configured logger. Using emergency logger. {"exception":"[object] (InvalidArgumentException(code: 0): Log [deprecations] is not defined. at E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Log\LogManager.php:207) [stacktrace]

0 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Log\LogManager.php(132): Illuminate\Log\LogManager->resolve('deprecations', NULL)

1 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Log\LogManager.php(119): Illuminate\Log\LogManager->get('deprecations')

2 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Log\LogManager.php(108): Illuminate\Log\LogManager->driver('deprecations')

3 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Foundation\Bootstrap\HandleExceptions.php(122): Illuminate\Log\LogManager->channel('deprecations')

4 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Foundation\Bootstrap\HandleExceptions.php(71): Illuminate\Foundation\Bootstrap\HandleExceptions->handleDeprecationError('auto_detect_lin...', 'E:\\app_key_rota...', 138, 8192)

5 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Foundation\Bootstrap\HandleExceptions.php(257): Illuminate\Foundation\Bootstrap\HandleExceptions->handleError(8192, 'auto_detect_lin...', 'E:\\app_key_rota...', 138)

6 [internal function]: Illuminate\Foundation\Bootstrap\HandleExceptions->Illuminate\Foundation\Bootstrap\{closure}(8192, 'auto_detect_lin...', 'E:\\app_key_rota...', 138)

7 E:\app_key_rotation\vendor\jackiedo\dotenv-editor\src\Jackiedo\DotenvEditor\DotenvReader.php(138): ini_set('auto_detect_lin...', '1')

8 E:\app_key_rotation\vendor\jackiedo\dotenv-editor\src\Jackiedo\DotenvEditor\DotenvReader.php(110): Jackiedo\DotenvEditor\DotenvReader->readLinesFromFile()

9 E:\app_key_rotation\vendor\jackiedo\dotenv-editor\src\Jackiedo\DotenvEditor\DotenvEditor.php(224): Jackiedo\DotenvEditor\DotenvReader->keys()

10 E:\app_key_rotation\vendor\jackiedo\dotenv-editor\src\Jackiedo\DotenvEditor\DotenvEditor.php(351): Jackiedo\DotenvEditor\DotenvEditor->getKeys(Array)

11 E:\app_key_rotation\vendor\jackiedo\dotenv-editor\src\Jackiedo\DotenvEditor\DotenvEditor.php(376): Jackiedo\DotenvEditor\DotenvEditor->setKeys(Array)

12 E:\app_key_rotation\vendor\rawilk\laravel-app-key-rotator\src\Commands\RotateAppKeyCommand.php(52): Jackiedo\DotenvEditor\DotenvEditor->setKey('OLD_APP_KEY', 'base64:D4y2Cp0E...', 'Rotated at: 202...')

13 E:\app_key_rotation\vendor\rawilk\laravel-app-key-rotator\src\Commands\RotateAppKeyCommand.php(41): Rawilk\AppKeyRotator\Commands\RotateAppKeyCommand->setKeyInEnvironmentFile('base64:D4y2Cp0E...', 'OLD_APP_KEY', 'Rotated at: 202...')

14 E:\app_key_rotation\vendor\rawilk\laravel-app-key-rotator\src\Commands\RotateAppKeyCommand.php(68): Rawilk\AppKeyRotator\Commands\RotateAppKeyCommand->getCurrentKey()

15 E:\app_key_rotation\vendor\rawilk\laravel-app-key-rotator\src\Commands\RotateAppKeyCommand.php(26): Rawilk\AppKeyRotator\Commands\RotateAppKeyCommand->setKeys()

16 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Container\BoundMethod.php(36): Rawilk\AppKeyRotator\Commands\RotateAppKeyCommand->handle()

17 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Container\Util.php(41): Illuminate\Container\BoundMethod::Illuminate\Container\{closure}()

18 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Container\BoundMethod.php(93): Illuminate\Container\Util::unwrapIfClosure(Object(Closure))

19 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Container\BoundMethod.php(37): Illuminate\Container\BoundMethod::callBoundMethod(Object(Illuminate\Foundation\Application), Array, Object(Closure))

20 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Container\Container.php(651): Illuminate\Container\BoundMethod::call(Object(Illuminate\Foundation\Application), Array, Array, NULL)

21 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Console\Command.php(139): Illuminate\Container\Container->call(Array)

22 E:\app_key_rotation\vendor\symfony\console\Command\Command.php(308): Illuminate\Console\Command->execute(Object(Symfony\Component\Console\Input\ArgvInput), Object(Illuminate\Console\OutputStyle))

23 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Console\Command.php(124): Symfony\Component\Console\Command\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Illuminate\Console\OutputStyle))

24 E:\app_key_rotation\vendor\symfony\console\Application.php(998): Illuminate\Console\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))

25 E:\app_key_rotation\vendor\symfony\console\Application.php(299): Symfony\Component\Console\Application->doRunCommand(Object(Rawilk\AppKeyRotator\Commands\RotateAppKeyCommand), Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))

26 E:\app_key_rotation\vendor\symfony\console\Application.php(171): Symfony\Component\Console\Application->doRun(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))

27 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Console\Application.php(102): Symfony\Component\Console\Application->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))

28 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Foundation\Console\Kernel.php(129): Illuminate\Console\Application->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))

29 E:\app_key_rotation\artisan(37): Illuminate\Foundation\Console\Kernel->handle(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))

30 {main}

"} [2022-08-02 17:17:35] laravel.WARNING: auto_detect_line_endings is deprecated in E:\app_key_rotation\vendor\jackiedo\dotenv-editor\src\Jackiedo\DotenvEditor\DotenvReader.php on line 138
[2022-08-02 17:17:35] local.ERROR: The MAC is invalid. {"exception":"[object] (Illuminate\Contracts\Encryption\DecryptException(code: 0): The MAC is invalid. at E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Encryption\Encrypter.php:218) [stacktrace]

0 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Encryption\Encrypter.php(151): Illuminate\Encryption\Encrypter->getJsonPayload(Array)

1 E:\app_key_rotation\vendor\rawilk\laravel-app-key-rotator\src\AppKeyRotator.php(27): Illuminate\Encryption\Encrypter->decrypt('eyJpdiI6Ii9LWGd...')

2 E:\app_key_rotation\vendor\rawilk\laravel-app-key-rotator\src\Actions\ReEncryptModels.php(50): Rawilk\AppKeyRotator\AppKeyRotator->reEncrypt('eyJpdiI6Ii9LWGd...')

3 E:\app_key_rotation\vendor\rawilk\laravel-app-key-rotator\src\Actions\ReEncryptModels.php(39): Rawilk\AppKeyRotator\Actions\ReEncryptModels->reEncryptModelInstance(Object(App\Models\User), Array)

4 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Collections\Traits\EnumeratesValues.php(264): Rawilk\AppKeyRotator\Actions\ReEncryptModels->Rawilk\AppKeyRotator\Actions\{closure}(Object(App\Models\User), 0)

5 E:\app_key_rotation\vendor\rawilk\laravel-app-key-rotator\src\Actions\ReEncryptModels.php(39): Illuminate\Support\Collection->each(Object(Closure))

6 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Database\Concerns\BuildsQueries.php(53): Rawilk\AppKeyRotator\Actions\ReEncryptModels->Rawilk\AppKeyRotator\Actions\{closure}(Object(Illuminate\Database\Eloquent\Collection), 1)

7 E:\app_key_rotation\vendor\rawilk\laravel-app-key-rotator\src\Actions\ReEncryptModels.php(40): Illuminate\Database\Eloquent\Builder->chunk(500, Object(Closure))

8 E:\app_key_rotation\vendor\rawilk\laravel-app-key-rotator\src\Actions\ReEncryptModels.php(28): Rawilk\AppKeyRotator\Actions\ReEncryptModels->reEncryptModel('App\\Models\\User', Array)

9 E:\app_key_rotation\vendor\rawilk\laravel-app-key-rotator\src\Commands\RotateAppKeyCommand.php(82): Rawilk\AppKeyRotator\Actions\ReEncryptModels->handle()

10 E:\app_key_rotation\vendor\rawilk\laravel-app-key-rotator\src\Commands\RotateAppKeyCommand.php(27): Rawilk\AppKeyRotator\Commands\RotateAppKeyCommand->executeActions()

11 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Container\BoundMethod.php(36): Rawilk\AppKeyRotator\Commands\RotateAppKeyCommand->handle()

12 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Container\Util.php(41): Illuminate\Container\BoundMethod::Illuminate\Container\{closure}()

13 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Container\BoundMethod.php(93): Illuminate\Container\Util::unwrapIfClosure(Object(Closure))

14 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Container\BoundMethod.php(37): Illuminate\Container\BoundMethod::callBoundMethod(Object(Illuminate\Foundation\Application), Array, Object(Closure))

15 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Container\Container.php(651): Illuminate\Container\BoundMethod::call(Object(Illuminate\Foundation\Application), Array, Array, NULL)

16 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Console\Command.php(139): Illuminate\Container\Container->call(Array)

17 E:\app_key_rotation\vendor\symfony\console\Command\Command.php(308): Illuminate\Console\Command->execute(Object(Symfony\Component\Console\Input\ArgvInput), Object(Illuminate\Console\OutputStyle))

18 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Console\Command.php(124): Symfony\Component\Console\Command\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Illuminate\Console\OutputStyle))

19 E:\app_key_rotation\vendor\symfony\console\Application.php(998): Illuminate\Console\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))

20 E:\app_key_rotation\vendor\symfony\console\Application.php(299): Symfony\Component\Console\Application->doRunCommand(Object(Rawilk\AppKeyRotator\Commands\RotateAppKeyCommand), Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))

21 E:\app_key_rotation\vendor\symfony\console\Application.php(171): Symfony\Component\Console\Application->doRun(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))

22 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Console\Application.php(102): Symfony\Component\Console\Application->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))

23 E:\app_key_rotation\vendor\laravel\framework\src\Illuminate\Foundation\Console\Kernel.php(129): Illuminate\Console\Application->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))

24 E:\app_key_rotation\artisan(37): Illuminate\Foundation\Console\Kernel->handle(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))

25 {main}

"}

`

rawilk commented 1 year ago

I can't replicate this issue myself, even on a fresh Laravel install. The package has also been updated now and is at v3.1.0, so I would recommend updating and see if the issue persists.

noahad commented 1 year ago

I'm running into this same issue (or a related one) with laravel-app-key-rotator 3.1.0 (on Laravel 9.39.0, php 8.1.11, storage is MariaDB) and I think I can shed some light.

The issue is that Illuminate\Encryption\Encrypter::decrypt() by default tries to do PHP deserialization after it decrypts, unless its second param is false. I'm using the encrypted cast on my Model attrs, and Laravel must be doing some logic and deciding not to serialize() these fields on the way in.

My raw data in the DB decrypts just fine with Crypt::decryptString($cipher_text), or decrypt($cipher_text, false). So the solution is duplicating Laravel's logic, in Rawilk\AppKeyRotator\AppKeyRotator.

Does that seem right?

singhpreet89 commented 1 year ago

@rawilk please confirm.

rkeshwani commented 1 year ago

I'm running into this same issue (or a related one) with laravel-app-key-rotator 3.1.0 (on Laravel 9.39.0, php 8.1.11, storage is MariaDB) and I think I can shed some light.

The issue is that Illuminate\Encryption\Encrypter::decrypt() by default tries to do PHP deserialization after it decrypts, unless its second param is false. I'm using the encrypted cast on my Model attrs, and Laravel must be doing some logic and deciding not to serialize() these fields on the way in.

My raw data in the DB decrypts just fine with Crypt::decryptString($cipher_text), or decrypt($cipher_text, false). So the solution is duplicating Laravel's logic, in Rawilk\AppKeyRotator\AppKeyRotator.

Does that seem right?

This is exactly the issue. If you read Laravel's documentation here: https://laravel.com/docs/9.x/eloquent-mutators#encrypted-casting It says that it uses their built-in encryption features found here https://laravel.com/docs/9.x/encryption. Assuming that it uses decryptString like their example, it basically runs decrypt($cipher_text, false). But Rawilk\AppKeyRotator\AppKeyRotator uses the Encrypter->decrypt($cipher_text) method so the $serialize parameter is true by default.

Solution is to change https://github.com/rawilk/laravel-app-key-rotator/blob/5d8b8bfe8d3520cd3c4dca6f892b98c87894c239/src/AppKeyRotator.php#L26 to return $this->newEncrypter->encrypt($this->oldEncrypter->decrypt($value,false),false); but this is me making an assumption about how the encryption casting works, their other encryption casts of collection, array, object, and array object may serialize the data in which case this solution may not work.

Perhaps a better way to solve this is have the reEncrypt function have a serializable parameter and change that based on if the attribute is an object, collection or string. See: https://github.com/rawilk/laravel-app-key-rotator/blob/5d8b8bfe8d3520cd3c4dca6f892b98c87894c239/src/Actions/ReEncryptModels.php#L32 Here is where the attributes are retrieved for the model. Maybe there is another method like getCasts() which I found on a trait HasAttributes which is where getAttributes() comes from that returns the type of the attribute and based on that you can set the serializable parameter to true if its an array, collection, or object, and false if its a string.

rkeshwani commented 1 year ago

I adjusted ReEncryptsModels.php locally and it appeared to work on strings. I added a serializeCasts array to detect if it's serialized or not. Then pass through if its serializable or not in the reEncrypt method. ReEncryptsModels.php

class ReEncryptModels implements RotatorAction
{
    protected $serializeCasts =  ['encrypted:array','encrypted:collection','encrypted:object','AsEncryptedArrayObject','AsEncryptedCollection'];
    public function handle(AppKeyRotator $appKeyRotator, array $config): void
    {
        $models = $this->getModels($config['models'] ?? []);

        $models->each(fn (string $model) => $this->reEncryptModel($model, $appKeyRotator));
    }

    protected function reEncryptModel(string $modelClass, AppKeyRotator $appKeyRotator): void
    {
        $encryptedProperties = $modelClass::make()->encryptedProperties();

        $modelClass::query()
            ->select(['id', ...$encryptedProperties])
            ->cursor()
            ->each(function (Model $model) use ($appKeyRotator, $encryptedProperties) {
                // We get the attributes here to prevent any accessors or mutators from trying to
                // encrypt/decrypt values with the wrong encryption keys.
                $attributes = $model->getAttributes();
                $casts = $model->getCasts();
                foreach ($encryptedProperties as $field) {
                    $isSerializable = in_array($casts[$field],$this->serializeCasts);
                    $attributes[$field] = $appKeyRotator->reEncrypt($attributes[$field],$isSerializable);
                }

                $model->setRawAttributes($attributes);

                $model->timestamps = false;

                $model->saveQuietly();
            });
    }

Then in the AppKeyRotator.php method, I modified the reEncrypt function and added a serializable parameter

    public function reEncrypt($value, $serializable): string
    {
        return $this->newEncrypter->encrypt($this->oldEncrypter->decrypt($value,$serializable),$serializable);
    }

Then I ran php artisan app-key-rotator:rotate which ran successfully without the same error that @singhpreet89 mentioned. I have yet to test this with serialized objects as I don't have those in use currently. This isn't the best solution but I can't currently think of an elegant way to detect if something is serialized or not.

I also found a isEncryptedCastable method. So should probably have a $model->isEncryptedCastable in there somewhere so that you don't accidentally encrypt something that isn't encrypted.

noahad commented 1 year ago

I can't currently think of an elegant way to detect if something is serialized or not.

This is very similar to the solution I'm working with. As near as I can tell, Illuminate\HasAttributes never serializes or deserializes. It decrypts all of the "native encrypted casts" with the same method, and then typecasts them to the target type.

So you're right that it's tricky if @rawilk wants to support both serialized and unserialized fields. The easiest thing would be just setting a global boolean in the config, but some projects may use a mix.

noahad commented 1 year ago

Come to think of it, if the user of this package is using 'custom' encrypted attributes/mutators/accessors, like the included tests do (and until relatively recently I believe was the only way to accomplish Laravel DB encryption), I don't think there's any reasonable way to detect whether or not those attributes are serialized or not.

There's probably no bulletproof way to even detect that they're encrypted — that's why this package requires that the fields and models be enumerated manually.

So the most practical approach might be this: we know that any field we're reEncrypting should NOT be serialized if it's using one of the built-in encrypted casts, and we can detect those as you did with $model->getCasts(). Maybe assuming any other field SHOULD be serialized is a decent approach?

Just thinking out loud.

rkeshwani commented 1 year ago

Come to think of it, if the user of this package is using 'custom' encrypted attributes/mutators/accessors, like the included tests do (and until relatively recently I believe was the only way to accomplish Laravel DB encryption), I don't think there's any reasonable way to detect whether or not those attributes are serialized or not.

There's probably no bulletproof way to even detect that they're encrypted — that's why this package requires that the fields and models be enumerated manually.

So the most practical approach might be this: we know that any field we're reEncrypting should NOT be serialized if it's using one of the built-in encrypted casts, and we can detect those as you did with $model->getCasts(). Maybe assuming any other field SHOULD be serialized is a decent approach?

Just thinking out loud. Some good points there.

so if $model->isEncryptedCastable($field) then$serializable = false else $serializable = true? That's probably a good solution... what about combining that with what you said in the previous comment. $model->isEncryptedCastable($field) then $serializable = false else $serializable = $configurationVariable? This way user can control if it's serializable if it's not a built-in casted type.

noahad commented 1 year ago

what about combining that with what you said in the previous comment. $model->isEncryptedCastable($field) then $serializable = false else $serializable = $configurationVariable? This way user can control if it's serializable if it's not a built-in casted type.

Sounds great to me! Now who wants to make a PR? 😉

rkeshwani commented 1 year ago

I don't have the repo cloned nor the environment available to test it using the built-in tests. I'd also like @rawilk to weigh-in on implementation approaches.

rawilk commented 1 year ago

@noahad, @rkeshwani Thanks for the insights on this. I'm definitely open to supporting this basically how you suggested, however I'd also like to offer a way to customize how the action determines if a field is serializable (possibly by defining a closure or something in a service provider) without having to extend the class.

I'd be open to a PR or I can come up with something too, just not sure when I'll get a chance to work on it next.

rkeshwani commented 1 year ago

So I tried integrating what I had working locally into this and utilizing the code spaces feature to make an attempt at a PR. I couldn't get any of the encryption tests to pass because as @noahad said, they assume custom mutators/accessors/attributes in the tests. the HasAttributes trait's getCast method only returns fields that are utilizing the built-in casting mechanism. In fact, those models don't even appear to have the isEncryptedCastable method. I attempted to utilize method_exists($model,'isEncryptedCastable'); but this doesn't seem to work as it returns true but then when $model resolves and isEncryptedCastable runs, it throws an error indicating that it doesn't exist when running the tests. Does anybody know how to detect if the model has custom attributes? I'm not very well versed in php's reflection capabilities. Call to undefined method Rawilk\AppKeyRotator\Tests\Models\UserWithMutators::isEncryptedCastable() I'm trying to get this functioning for both cases of using built-in casting encryption mechanisms and for custom implementations.

                foreach ($encryptedProperties as $field) {
                    $encryptedCastableMethodExists = method_exists($model,'isEncryptedCastable');
                    if($encryptedCastableMethodExists) {
                        $serializable = !$model->isEncryptedCastable($field);
                    }
                    else {
                        $serializable = true;
                    }
                    // $serializable = $encryptedCastableMethodExists?(!$model->isEncryptedCastable($field)):true;
                    $attributes[$field] = $appKeyRotator->reEncrypt($attributes[$field],$serializable);
                }

Edit: Solution I tried doesn't work because isEncryptedCastable is protected.

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 21 days with no activity. Remove stale label or comment or this will be closed in 7 days.