From e5c60aa477c4dbfb4dd2b7910cdb0339b693f143 Mon Sep 17 00:00:00 2001 From: David Badura Date: Thu, 5 Mar 2026 16:26:31 +0100 Subject: [PATCH] improve cryptography implementation --- phpstan-baseline.neon | 34 +++++- .../Cryptography/BaseCryptographer.php | 72 +++++++---- src/Extension/Cryptography/Cipher/Cipher.php | 10 +- .../Cryptography/Cipher/CipherKey.php | 11 +- .../Cryptography/Cipher/CipherKeyFactory.php | 8 +- .../Cipher/CreateCipherKeyFailed.php | 17 ++- .../Cryptography/Cipher/DecryptionFailed.php | 27 ++++- .../Cryptography/Cipher/EncryptedData.php | 23 ++++ .../Cryptography/Cipher/EncryptionFailed.php | 17 ++- .../Cryptography/Cipher/OpensslCipher.php | 67 +++++++---- .../Cipher/OpensslCipherKeyFactory.php | 21 ++-- .../Cryptography/Store/CipherKeyNotExists.php | 14 ++- .../Cryptography/Store/CipherKeyStore.php | 7 +- .../Store/InMemoryCipherKeyStore.php | 70 ++++++++++- .../Cryptography/BaseCryptographerTest.php | 113 ++++++++---------- .../Cipher/CreateCipherKeyFailedTest.php | 5 +- .../Cipher/DecryptionFailedTest.php | 4 +- .../Cipher/EncryptionFailedTest.php | 4 +- .../Cipher/OpensslCipherKeyFactoryTest.php | 10 +- .../Cryptography/Cipher/OpensslCipherTest.php | 51 ++++---- .../CryptographyMiddlewareTest.php | 6 +- .../Store/CipherKeyNotExistsTest.php | 4 +- .../Store/InMemoryCipherKeyStoreTest.php | 7 ++ 23 files changed, 409 insertions(+), 193 deletions(-) create mode 100644 src/Extension/Cryptography/Cipher/EncryptedData.php diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 7125343c..73ed976a 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -19,25 +19,49 @@ parameters: path: src/Cryptography/PersonalDataPayloadCryptographer.php - - message: '#^Parameter \#3 \$iv of class Patchlevel\\Hydrator\\Extension\\Cryptography\\Cipher\\CipherKey constructor expects non\-empty\-string, string given\.$#' + message: '#^Offset ''k'' on array\{v\: 1, a\: non\-empty\-string, k\: non\-empty\-string, n\?\: non\-empty\-string, d\: non\-empty\-string, t\?\: non\-empty\-string\} on left side of \?\? always exists and is not nullable\.$#' + identifier: nullCoalesce.offset + count: 1 + path: src/Extension/Cryptography/BaseCryptographer.php + + - + message: '#^Parameter \#1 \$subjectId of callable Patchlevel\\Hydrator\\Extension\\Cryptography\\Cipher\\CipherKeyFactory expects non\-empty\-string, string given\.$#' identifier: argument.type count: 1 path: src/Extension/Cryptography/BaseCryptographer.php - - message: '#^Method Patchlevel\\Hydrator\\Extension\\Cryptography\\Cipher\\OpensslCipher\:\:encrypt\(\) should return non\-empty\-string but returns string\.$#' - identifier: return.type + message: '#^Strict comparison using \=\=\= between non\-empty\-string and null will always evaluate to false\.$#' + identifier: identical.alwaysFalse + count: 1 + path: src/Extension/Cryptography/BaseCryptographer.php + + - + message: '#^Parameter \#1 \$data of class Patchlevel\\Hydrator\\Extension\\Cryptography\\Cipher\\EncryptedData constructor expects non\-empty\-string, string given\.$#' + identifier: argument.type + count: 1 + path: src/Extension/Cryptography/Cipher/OpensslCipher.php + + - + message: '#^Parameter \#1 \$data of function openssl_decrypt expects string, string\|false given\.$#' + identifier: argument.type + count: 1 + path: src/Extension/Cryptography/Cipher/OpensslCipher.php + + - + message: '#^Parameter \#3 \$nonce of class Patchlevel\\Hydrator\\Extension\\Cryptography\\Cipher\\EncryptedData constructor expects non\-empty\-string\|null, string\|null given\.$#' + identifier: argument.type count: 1 path: src/Extension/Cryptography/Cipher/OpensslCipher.php - - message: '#^Parameter \#1 \$key of class Patchlevel\\Hydrator\\Extension\\Cryptography\\Cipher\\CipherKey constructor expects non\-empty\-string, string given\.$#' + message: '#^Parameter \#1 \$id of class Patchlevel\\Hydrator\\Extension\\Cryptography\\Cipher\\CipherKey constructor expects non\-empty\-string, string given\.$#' identifier: argument.type count: 1 path: src/Extension/Cryptography/Cipher/OpensslCipherKeyFactory.php - - message: '#^Parameter \#3 \$iv of class Patchlevel\\Hydrator\\Extension\\Cryptography\\Cipher\\CipherKey constructor expects non\-empty\-string, string given\.$#' + message: '#^Parameter \#3 \$key of class Patchlevel\\Hydrator\\Extension\\Cryptography\\Cipher\\CipherKey constructor expects non\-empty\-string, string given\.$#' identifier: argument.type count: 1 path: src/Extension/Cryptography/Cipher/OpensslCipherKeyFactory.php diff --git a/src/Extension/Cryptography/BaseCryptographer.php b/src/Extension/Cryptography/BaseCryptographer.php index 8769c6b2..07671ab6 100644 --- a/src/Extension/Cryptography/BaseCryptographer.php +++ b/src/Extension/Cryptography/BaseCryptographer.php @@ -5,27 +5,26 @@ namespace Patchlevel\Hydrator\Extension\Cryptography; use Patchlevel\Hydrator\Extension\Cryptography\Cipher\Cipher; -use Patchlevel\Hydrator\Extension\Cryptography\Cipher\CipherKey; use Patchlevel\Hydrator\Extension\Cryptography\Cipher\CipherKeyFactory; use Patchlevel\Hydrator\Extension\Cryptography\Cipher\DecryptionFailed; +use Patchlevel\Hydrator\Extension\Cryptography\Cipher\EncryptedData; use Patchlevel\Hydrator\Extension\Cryptography\Cipher\EncryptionFailed; use Patchlevel\Hydrator\Extension\Cryptography\Cipher\OpensslCipher; use Patchlevel\Hydrator\Extension\Cryptography\Cipher\OpensslCipherKeyFactory; use Patchlevel\Hydrator\Extension\Cryptography\Store\CipherKeyNotExists; use Patchlevel\Hydrator\Extension\Cryptography\Store\CipherKeyStore; -use function array_key_exists; -use function base64_decode; -use function base64_encode; use function is_array; /** * @experimental - * @phpstan-type EncryptedDataV1 array{ - * __enc: 'v1', - * data: non-empty-string, - * method?: non-empty-string, - * iv?: non-empty-string, + * @phpstan-type EncryptedDataArray array{ + * v: 1, + * a: non-empty-string, + * k: non-empty-string, + * n?: non-empty-string, // base64 + * d: non-empty-string, // base64 ciphertext + * t?: non-empty-string, // base64 (for AEAD) * } */ final class BaseCryptographer implements Cryptographer @@ -38,50 +37,71 @@ public function __construct( } /** - * @return EncryptedDataV1 + * @return EncryptedDataArray * * @throws EncryptionFailed */ public function encrypt(string $subjectId, mixed $value): array { try { - $cipherKey = $this->cipherKeyStore->get($subjectId); + $cipherKey = $this->cipherKeyStore->currentKeyFor($subjectId); } catch (CipherKeyNotExists) { - $cipherKey = ($this->cipherKeyFactory)(); - $this->cipherKeyStore->store($subjectId, $cipherKey); + $cipherKey = ($this->cipherKeyFactory)($subjectId); + $this->cipherKeyStore->store($cipherKey->id, $cipherKey); } - return [ - '__enc' => 'v1', - 'data' => $this->cipher->encrypt($cipherKey, $value), - 'method' => $cipherKey->method, - 'iv' => base64_encode($cipherKey->iv), + $parameter = $this->cipher->encrypt($cipherKey, $value); + + $result = [ + 'v' => 1, + 'a' => $parameter->method, + 'k' => $cipherKey->id, + 'd' => $parameter->data, ]; + + if ($parameter->nonce !== null) { + $result['n'] = $parameter->nonce; + } + + if ($parameter->tag !== null) { + $result['t'] = $parameter->tag; + } + + return $result; } /** - * @param EncryptedDataV1 $encryptedData + * @param EncryptedDataArray $encryptedData * * @throws CipherKeyNotExists * @throws DecryptionFailed */ public function decrypt(string $subjectId, mixed $encryptedData): mixed { - $cipherKey = $this->cipherKeyStore->get($subjectId); + $keyId = $encryptedData['k'] ?? null; + + if ($keyId === null) { + throw DecryptionFailed::missingKeyId(); + } + + $cipherKey = $this->cipherKeyStore->get($keyId); return $this->cipher->decrypt( - new CipherKey( - $cipherKey->key, - $encryptedData['method'] ?? $cipherKey->method, - isset($encryptedData['iv']) ? base64_decode($encryptedData['iv']) : $cipherKey->iv, + $cipherKey, + new EncryptedData( + $encryptedData['d'], + $encryptedData['a'], + $encryptedData['n'] ?? null, + $encryptedData['t'] ?? null, ), - $encryptedData['data'], ); } public function supports(mixed $value): bool { - return is_array($value) && array_key_exists('__enc', $value) && $value['__enc'] === 'v1'; + return is_array($value) + && isset($value['v'], $value['a'], $value['k'], $value['d']) + && $value['v'] === 1; } /** @param non-empty-string $method */ diff --git a/src/Extension/Cryptography/Cipher/Cipher.php b/src/Extension/Cryptography/Cipher/Cipher.php index 03f38ec8..184829e9 100644 --- a/src/Extension/Cryptography/Cipher/Cipher.php +++ b/src/Extension/Cryptography/Cipher/Cipher.php @@ -7,13 +7,9 @@ /** @experimental */ interface Cipher { - /** - * @return non-empty-string - * - * @throws EncryptionFailed - */ - public function encrypt(CipherKey $key, mixed $data): string; + /** @throws EncryptionFailed */ + public function encrypt(CipherKey $key, mixed $data): EncryptedData; /** @throws DecryptionFailed */ - public function decrypt(CipherKey $key, string $data): mixed; + public function decrypt(CipherKey $key, EncryptedData $parameter): mixed; } diff --git a/src/Extension/Cryptography/Cipher/CipherKey.php b/src/Extension/Cryptography/Cipher/CipherKey.php index ced9c9aa..54bf4263 100644 --- a/src/Extension/Cryptography/Cipher/CipherKey.php +++ b/src/Extension/Cryptography/Cipher/CipherKey.php @@ -4,18 +4,25 @@ namespace Patchlevel\Hydrator\Extension\Cryptography\Cipher; +use DateTimeImmutable; +use SensitiveParameter; + /** @experimental */ final class CipherKey { /** + * @param non-empty-string $id + * @param non-empty-string $subjectId * @param non-empty-string $key * @param non-empty-string $method - * @param non-empty-string $iv */ public function __construct( + public readonly string $id, + public readonly string $subjectId, + #[SensitiveParameter] public readonly string $key, public readonly string $method, - public readonly string $iv, + public readonly DateTimeImmutable $createdAt, ) { } } diff --git a/src/Extension/Cryptography/Cipher/CipherKeyFactory.php b/src/Extension/Cryptography/Cipher/CipherKeyFactory.php index 85050f1f..9a82ba98 100644 --- a/src/Extension/Cryptography/Cipher/CipherKeyFactory.php +++ b/src/Extension/Cryptography/Cipher/CipherKeyFactory.php @@ -7,6 +7,10 @@ /** @experimental */ interface CipherKeyFactory { - /** @throws CreateCipherKeyFailed */ - public function __invoke(): CipherKey; + /** + * @param non-empty-string $subjectId + * + * @throws CreateCipherKeyFailed + */ + public function __invoke(string $subjectId): CipherKey; } diff --git a/src/Extension/Cryptography/Cipher/CreateCipherKeyFailed.php b/src/Extension/Cryptography/Cipher/CreateCipherKeyFailed.php index 60f2fcf2..50f0b759 100644 --- a/src/Extension/Cryptography/Cipher/CreateCipherKeyFailed.php +++ b/src/Extension/Cryptography/Cipher/CreateCipherKeyFailed.php @@ -6,12 +6,25 @@ use Patchlevel\Hydrator\HydratorException; use RuntimeException; +use Throwable; + +use function sprintf; /** @experimental */ final class CreateCipherKeyFailed extends RuntimeException implements HydratorException { - public function __construct() + private function __construct(string $message, Throwable|null $previous = null) + { + parent::__construct($message, 0, $previous); + } + + public static function forMethod(string $method, string $reason): self + { + return new self(sprintf('Failed to create cipher key for method "%s": %s', $method, $reason)); + } + + public static function invalidKeyLength(string $method): self { - parent::__construct('Create cipher key failed.'); + return new self(sprintf('Invalid key length for method "%s".', $method)); } } diff --git a/src/Extension/Cryptography/Cipher/DecryptionFailed.php b/src/Extension/Cryptography/Cipher/DecryptionFailed.php index 8b7059e8..a2f99765 100644 --- a/src/Extension/Cryptography/Cipher/DecryptionFailed.php +++ b/src/Extension/Cryptography/Cipher/DecryptionFailed.php @@ -6,12 +6,35 @@ use Patchlevel\Hydrator\HydratorException; use RuntimeException; +use Throwable; + +use function sprintf; /** @experimental */ final class DecryptionFailed extends RuntimeException implements HydratorException { - public function __construct() + private function __construct(string $message, Throwable|null $previous = null) + { + parent::__construct($message, 0, $previous); + } + + public static function forMethod(string $method, Throwable|null $previous = null): self + { + return new self(sprintf('Decryption failed for method "%s".', $method), $previous); + } + + public static function invalidBase64(string $field): self + { + return new self(sprintf('Invalid base64 encoding in field "%s".', $field)); + } + + public static function missingKeyId(): self + { + return new self('Missing key ID in encrypted data.'); + } + + public static function invalidJson(Throwable|null $previous = null): self { - parent::__construct('Decryption failed.'); + return new self('Failed to decode JSON data.', $previous); } } diff --git a/src/Extension/Cryptography/Cipher/EncryptedData.php b/src/Extension/Cryptography/Cipher/EncryptedData.php new file mode 100644 index 00000000..ec86be2e --- /dev/null +++ b/src/Extension/Cryptography/Cipher/EncryptedData.php @@ -0,0 +1,23 @@ +method); + + if ($ivLength === false) { + throw EncryptionFailed::invalidIvLength($key->method); + } + + $nonce = $ivLength > 0 ? openssl_random_pseudo_bytes($ivLength) : null; + $tag = null; + $encryptedData = @openssl_encrypt( - $this->dataEncode($data), + json_encode($data, JSON_THROW_ON_ERROR), $key->method, $key->key, 0, - $key->iv, + $nonce ?? '', + $tag, ); if ($encryptedData === false) { - throw new EncryptionFailed(); + throw EncryptionFailed::forMethod($key->method); } - return base64_encode($encryptedData); + return new EncryptedData( + base64_encode($encryptedData), + $key->method, + $nonce !== null ? base64_encode($nonce) : null, + $tag !== null ? base64_encode($tag) : null, + ); } - public function decrypt(CipherKey $key, string $data): mixed + public function decrypt(CipherKey $key, EncryptedData $parameter): mixed { + $tag = $parameter->tag !== null ? base64_decode($parameter->tag, true) : null; + + if ($parameter->tag !== null && $tag === false) { + throw DecryptionFailed::invalidBase64('tag'); + } + + $nonce = $parameter->nonce !== null ? base64_decode($parameter->nonce, true) : null; + + if ($parameter->nonce !== null && $nonce === false) { + throw DecryptionFailed::invalidBase64('nonce'); + } + $data = @openssl_decrypt( - base64_decode($data), - $key->method, + base64_decode($parameter->data, true), + $parameter->method, $key->key, 0, - $key->iv, + $nonce ?: '', + $tag ?: '', ); if ($data === false) { - throw new DecryptionFailed(); + throw DecryptionFailed::forMethod($parameter->method); } try { - return $this->dataDecode($data); - } catch (JsonException) { - throw new DecryptionFailed(); + return json_decode($data, true, 512, JSON_THROW_ON_ERROR); + } catch (JsonException $e) { + throw DecryptionFailed::invalidJson($e); } } - - private function dataEncode(mixed $data): string - { - return json_encode($data, JSON_THROW_ON_ERROR); - } - - private function dataDecode(string $data): mixed - { - return json_decode($data, true, 512, JSON_THROW_ON_ERROR); - } } diff --git a/src/Extension/Cryptography/Cipher/OpensslCipherKeyFactory.php b/src/Extension/Cryptography/Cipher/OpensslCipherKeyFactory.php index 67f1a5b0..1a7442d6 100644 --- a/src/Extension/Cryptography/Cipher/OpensslCipherKeyFactory.php +++ b/src/Extension/Cryptography/Cipher/OpensslCipherKeyFactory.php @@ -4,9 +4,11 @@ namespace Patchlevel\Hydrator\Extension\Cryptography\Cipher; +use DateTimeImmutable; + +use function bin2hex; use function function_exists; use function in_array; -use function openssl_cipher_iv_length; use function openssl_cipher_key_length; use function openssl_get_cipher_methods; use function openssl_random_pseudo_bytes; @@ -14,12 +16,10 @@ /** @experimental */ final class OpensslCipherKeyFactory implements CipherKeyFactory { - public const DEFAULT_METHOD = 'aes128'; + public const DEFAULT_METHOD = 'aes-128-gcm'; private readonly int $keyLength; - private readonly int $ivLength; - /** @param non-empty-string $method */ public function __construct( private readonly string $method = self::DEFAULT_METHOD, @@ -34,22 +34,21 @@ public function __construct( $keyLength = @openssl_cipher_key_length($this->method); } - $ivLength = @openssl_cipher_iv_length($this->method); - - if ($keyLength === false || $ivLength === false) { + if ($keyLength === false) { throw new MethodNotSupported($this->method); } $this->keyLength = $keyLength; - $this->ivLength = $ivLength; } - public function __invoke(): CipherKey + public function __invoke(string $subjectId): CipherKey { return new CipherKey( - openssl_random_pseudo_bytes($this->keyLength), + bin2hex(openssl_random_pseudo_bytes(16)), + $subjectId, + bin2hex(openssl_random_pseudo_bytes($this->keyLength)), $this->method, - openssl_random_pseudo_bytes($this->ivLength), + new DateTimeImmutable(), ); } diff --git a/src/Extension/Cryptography/Store/CipherKeyNotExists.php b/src/Extension/Cryptography/Store/CipherKeyNotExists.php index 8679caf9..71310fbf 100644 --- a/src/Extension/Cryptography/Store/CipherKeyNotExists.php +++ b/src/Extension/Cryptography/Store/CipherKeyNotExists.php @@ -12,8 +12,18 @@ /** @experimental */ final class CipherKeyNotExists extends RuntimeException implements HydratorException { - public function __construct(string $id) + private function __construct(string $message) { - parent::__construct(sprintf('Cipher key with subject id "%s" not found.', $id)); + parent::__construct($message); + } + + public static function forKeyId(string $id): self + { + return new self(sprintf('Cipher key with id "%s" does not exist.', $id)); + } + + public static function forSubjectId(string $subjectId): self + { + return new self(sprintf('Cipher key for subject id "%s" does not exist.', $subjectId)); } } diff --git a/src/Extension/Cryptography/Store/CipherKeyStore.php b/src/Extension/Cryptography/Store/CipherKeyStore.php index fe589f28..82541c99 100644 --- a/src/Extension/Cryptography/Store/CipherKeyStore.php +++ b/src/Extension/Cryptography/Store/CipherKeyStore.php @@ -10,9 +10,14 @@ interface CipherKeyStore { /** @throws CipherKeyNotExists */ - public function get(string $id): CipherKey; + public function currentKeyFor(string $subjectId): CipherKey; + + /** @throws CipherKeyNotExists */ + public function get(string $keyId): CipherKey; public function store(string $id, CipherKey $key): void; public function remove(string $id): void; + + public function removeWithSubjectId(string $subjectId): void; } diff --git a/src/Extension/Cryptography/Store/InMemoryCipherKeyStore.php b/src/Extension/Cryptography/Store/InMemoryCipherKeyStore.php index dd328233..20a0322a 100644 --- a/src/Extension/Cryptography/Store/InMemoryCipherKeyStore.php +++ b/src/Extension/Cryptography/Store/InMemoryCipherKeyStore.php @@ -6,29 +6,87 @@ use Patchlevel\Hydrator\Extension\Cryptography\Cipher\CipherKey; +use function array_key_last; + /** @experimental */ final class InMemoryCipherKeyStore implements CipherKeyStore { /** @var array */ - private array $keys = []; + private array $indexById = []; + + /** @var array> */ + private array $indexBySubjectId = []; + + public function currentKeyFor(string $subjectId): CipherKey + { + if (!isset($this->indexBySubjectId[$subjectId])) { + throw CipherKeyNotExists::forSubjectId($subjectId); + } + + $lastKey = array_key_last($this->indexBySubjectId[$subjectId]); - public function get(string $id): CipherKey + if ($lastKey === null) { + throw CipherKeyNotExists::forSubjectId($subjectId); + } + + return $this->indexBySubjectId[$subjectId][$lastKey]; + } + + public function get(string $keyId): CipherKey { - return $this->keys[$id] ?? throw new CipherKeyNotExists($id); + return $this->indexById[$keyId] ?? throw CipherKeyNotExists::forKeyId($keyId); } public function store(string $id, CipherKey $key): void { - $this->keys[$id] = $key; + $this->indexById[$id] = $key; + + if (!isset($this->indexBySubjectId[$key->subjectId])) { + $this->indexBySubjectId[$key->subjectId] = []; + } + + $this->indexBySubjectId[$key->subjectId][] = $key; } public function remove(string $id): void { - unset($this->keys[$id]); + unset($this->indexById[$id]); + + foreach ($this->indexBySubjectId as $subjectId => $keys) { + $filtered = []; + + foreach ($keys as $key) { + if ($key->id === $id) { + continue; + } + + $filtered[] = $key; + } + + if ($filtered === []) { + unset($this->indexBySubjectId[$subjectId]); + } else { + $this->indexBySubjectId[$subjectId] = $filtered; + } + } } public function clear(): void { - $this->keys = []; + $this->indexById = []; + $this->indexBySubjectId = []; + } + + public function removeWithSubjectId(string $subjectId): void + { + if (!isset($this->indexBySubjectId[$subjectId])) { + return; + } + + foreach ($this->indexBySubjectId[$subjectId] as $key) { + unset($this->indexById[$key->id]); + } + + unset($this->indexBySubjectId[$subjectId]); } } diff --git a/tests/Unit/Extension/Cryptography/BaseCryptographerTest.php b/tests/Unit/Extension/Cryptography/BaseCryptographerTest.php index 9d714b4b..296026aa 100644 --- a/tests/Unit/Extension/Cryptography/BaseCryptographerTest.php +++ b/tests/Unit/Extension/Cryptography/BaseCryptographerTest.php @@ -4,10 +4,12 @@ namespace Patchlevel\Hydrator\Tests\Unit\Extension\Cryptography; +use DateTimeImmutable; use Patchlevel\Hydrator\Extension\Cryptography\BaseCryptographer; use Patchlevel\Hydrator\Extension\Cryptography\Cipher\Cipher; use Patchlevel\Hydrator\Extension\Cryptography\Cipher\CipherKey; use Patchlevel\Hydrator\Extension\Cryptography\Cipher\CipherKeyFactory; +use Patchlevel\Hydrator\Extension\Cryptography\Cipher\EncryptedData; use Patchlevel\Hydrator\Extension\Cryptography\Store\CipherKeyNotExists; use Patchlevel\Hydrator\Extension\Cryptography\Store\CipherKeyStore; use PHPUnit\Framework\Attributes\CoversClass; @@ -19,22 +21,22 @@ final class BaseCryptographerTest extends TestCase { public function testEncrypt(): void { - $cipherKey = new CipherKey('foo', 'methodA', 'random'); + $cipherKey = new CipherKey('key-123', 'subject-foo', 'secret-key', 'aes-256-gcm', new DateTimeImmutable()); + $encryptionParameter = new EncryptedData('encrypted-data', 'aes-256-gcm', 'random-nonce', 'auth-tag'); $cipherKeyStore = $this->createMock(CipherKeyStore::class); - $cipherKeyStore->method('get')->with('foo')->willReturn($cipherKey); + $cipherKeyStore->method('currentKeyFor')->with('subject-foo')->willReturn($cipherKey); $cipherKeyStore ->expects($this->never()) - ->method('store') - ->with('foo', $this->isInstanceOf(CipherKey::class)); + ->method('store'); $cipherKeyFactory = $this->createMock(CipherKeyFactory::class); $cipherKeyFactory->expects($this->never())->method('__invoke'); $cipher = $this->createMock(Cipher::class); $cipher->expects($this->once())->method('encrypt')->with($cipherKey, 'info@patchlevel.de') - ->willReturn('encrypted'); + ->willReturn($encryptionParameter); $cryptographer = new BaseCryptographer( $cipher, @@ -43,33 +45,36 @@ public function testEncrypt(): void ); $expected = [ - '__enc' => 'v1', - 'data' => 'encrypted', - 'method' => 'methodA', - 'iv' => 'cmFuZG9t', + 'v' => 1, + 'a' => 'aes-256-gcm', + 'k' => 'key-123', + 'd' => 'encrypted-data', + 'n' => 'random-nonce', + 't' => 'auth-tag', ]; - self::assertEquals($expected, $cryptographer->encrypt('foo', 'info@patchlevel.de')); + self::assertEquals($expected, $cryptographer->encrypt('subject-foo', 'info@patchlevel.de')); } public function testEncryptWithoutKey(): void { - $cipherKey = new CipherKey('foo', 'methodA', 'random'); + $cipherKey = new CipherKey('key-456', 'subject-bar', 'secret-key', 'aes-256-gcm', new DateTimeImmutable()); + $encryptionParameter = new EncryptedData('encrypted-data', 'aes-256-gcm', 'random-nonce', 'auth-tag'); $cipherKeyStore = $this->createMock(CipherKeyStore::class); - $cipherKeyStore->method('get')->with('foo')->willThrowException(new CipherKeyNotExists('foo')); + $cipherKeyStore->method('currentKeyFor')->with('subject-bar')->willThrowException(CipherKeyNotExists::forSubjectId('subject-bar')); $cipherKeyFactory = $this->createMock(CipherKeyFactory::class); - $cipherKeyFactory->expects($this->once())->method('__invoke')->willReturn($cipherKey); + $cipherKeyFactory->expects($this->once())->method('__invoke')->with('subject-bar')->willReturn($cipherKey); $cipherKeyStore ->expects($this->once()) ->method('store') - ->with('foo', $cipherKey); + ->with('key-456', $cipherKey); $cipher = $this->createMock(Cipher::class); $cipher->expects($this->once())->method('encrypt')->with($cipherKey, 'info@patchlevel.de') - ->willReturn('encrypted'); + ->willReturn($encryptionParameter); $cryptographer = new BaseCryptographer( $cipher, @@ -78,62 +83,37 @@ public function testEncryptWithoutKey(): void ); $expected = [ - '__enc' => 'v1', - 'data' => 'encrypted', - 'method' => 'methodA', - 'iv' => 'cmFuZG9t', + 'v' => 1, + 'a' => 'aes-256-gcm', + 'k' => 'key-456', + 'd' => 'encrypted-data', + 'n' => 'random-nonce', + 't' => 'auth-tag', ]; - self::assertEquals($expected, $cryptographer->encrypt('foo', 'info@patchlevel.de')); + self::assertEquals($expected, $cryptographer->encrypt('subject-bar', 'info@patchlevel.de')); } public function testDecrypt(): void { - $cipherKey = new CipherKey('foo', 'methodA', 'random'); - - $cipherKeyStore = $this->createMock(CipherKeyStore::class); - $cipherKeyStore->method('get')->with('foo')->willReturn($cipherKey); - - $cipherKeyFactory = $this->createMock(CipherKeyFactory::class); - $cipherKeyFactory->expects($this->never())->method('__invoke'); - - $cipher = $this->createMock(Cipher::class); - $cipher->expects($this->once())->method('decrypt')->with($cipherKey, 'encrypted') - ->willReturn('info@patchlevel.de'); - - $cryptographer = new BaseCryptographer( - $cipher, - $cipherKeyStore, - $cipherKeyFactory, - ); - - self::assertEquals( - 'info@patchlevel.de', - $cryptographer->decrypt( - 'foo', - [ - '__enc' => 'v1', - 'data' => 'encrypted', - 'method' => 'methodA', - 'iv' => 'cmFuZG9t', - ], - ), - ); - } - - public function testDecryptWithFallback(): void - { - $cipherKey = new CipherKey('foo', 'methodA', 'random'); + $cipherKey = new CipherKey('key-789', 'subject-baz', 'secret-key', 'aes-256-gcm', new DateTimeImmutable()); $cipherKeyStore = $this->createMock(CipherKeyStore::class); - $cipherKeyStore->method('get')->with('foo')->willReturn($cipherKey); + $cipherKeyStore->method('get')->with('key-789')->willReturn($cipherKey); $cipherKeyFactory = $this->createMock(CipherKeyFactory::class); $cipherKeyFactory->expects($this->never())->method('__invoke'); $cipher = $this->createMock(Cipher::class); - $cipher->expects($this->once())->method('decrypt')->with($cipherKey, 'encrypted') - ->willReturn('info@patchlevel.de'); + $cipher->expects($this->once())->method('decrypt')->with( + $cipherKey, + $this->callback(static function (EncryptedData $param) { + return $param->data === 'encrypted-data' + && $param->method === 'aes-256-gcm' + && $param->nonce === 'random-nonce' + && $param->tag === 'auth-tag'; + }), + )->willReturn('info@patchlevel.de'); $cryptographer = new BaseCryptographer( $cipher, @@ -144,10 +124,14 @@ public function testDecryptWithFallback(): void self::assertEquals( 'info@patchlevel.de', $cryptographer->decrypt( - 'foo', + 'subject-baz', [ - '__enc' => 'v1', - 'data' => 'encrypted', + 'v' => 1, + 'a' => 'aes-256-gcm', + 'k' => 'key-789', + 'd' => 'encrypted-data', + 'n' => 'random-nonce', + 't' => 'auth-tag', ], ), ); @@ -171,7 +155,10 @@ public static function dataProviderSupports(): iterable yield ['foo', false]; yield [[], false]; yield [null, false]; - yield [['__enc' => 'foo'], false]; - yield [['__enc' => 'v1'], true]; + yield [['v' => 'foo'], false]; + yield [['v' => 2], false]; + yield [['v' => 1], false]; // missing required fields + yield [['v' => 1, 'a' => 'aes-256-gcm'], false]; // missing k and d + yield [['v' => 1, 'a' => 'aes-256-gcm', 'k' => 'key-123', 'd' => 'data'], true]; } } diff --git a/tests/Unit/Extension/Cryptography/Cipher/CreateCipherKeyFailedTest.php b/tests/Unit/Extension/Cryptography/Cipher/CreateCipherKeyFailedTest.php index 86ee0a2e..13fd8bdd 100644 --- a/tests/Unit/Extension/Cryptography/Cipher/CreateCipherKeyFailedTest.php +++ b/tests/Unit/Extension/Cryptography/Cipher/CreateCipherKeyFailedTest.php @@ -13,8 +13,9 @@ final class CreateCipherKeyFailedTest extends TestCase { public function testCreation(): void { - $exception = new CreateCipherKeyFailed(); + $exception = CreateCipherKeyFailed::forMethod('aes-256-gcm', 'test reason'); - self::assertSame('Create cipher key failed.', $exception->getMessage()); + self::assertStringContainsString('aes-256-gcm', $exception->getMessage()); + self::assertStringContainsString('test reason', $exception->getMessage()); } } diff --git a/tests/Unit/Extension/Cryptography/Cipher/DecryptionFailedTest.php b/tests/Unit/Extension/Cryptography/Cipher/DecryptionFailedTest.php index a23bed2e..a3bb2b4c 100644 --- a/tests/Unit/Extension/Cryptography/Cipher/DecryptionFailedTest.php +++ b/tests/Unit/Extension/Cryptography/Cipher/DecryptionFailedTest.php @@ -13,8 +13,8 @@ final class DecryptionFailedTest extends TestCase { public function testCreation(): void { - $exception = new DecryptionFailed(); + $exception = DecryptionFailed::forMethod('aes-256-gcm'); - self::assertSame('Decryption failed.', $exception->getMessage()); + self::assertStringContainsString('aes-256-gcm', $exception->getMessage()); } } diff --git a/tests/Unit/Extension/Cryptography/Cipher/EncryptionFailedTest.php b/tests/Unit/Extension/Cryptography/Cipher/EncryptionFailedTest.php index be8d4851..a1e8d833 100644 --- a/tests/Unit/Extension/Cryptography/Cipher/EncryptionFailedTest.php +++ b/tests/Unit/Extension/Cryptography/Cipher/EncryptionFailedTest.php @@ -13,8 +13,8 @@ final class EncryptionFailedTest extends TestCase { public function testCreation(): void { - $exception = new EncryptionFailed(); + $exception = EncryptionFailed::forMethod('aes-256-gcm'); - self::assertSame('Encryption failed.', $exception->getMessage()); + self::assertStringContainsString('aes-256-gcm', $exception->getMessage()); } } diff --git a/tests/Unit/Extension/Cryptography/Cipher/OpensslCipherKeyFactoryTest.php b/tests/Unit/Extension/Cryptography/Cipher/OpensslCipherKeyFactoryTest.php index f0ea9467..79f87112 100644 --- a/tests/Unit/Extension/Cryptography/Cipher/OpensslCipherKeyFactoryTest.php +++ b/tests/Unit/Extension/Cryptography/Cipher/OpensslCipherKeyFactoryTest.php @@ -17,11 +17,11 @@ final class OpensslCipherKeyFactoryTest extends TestCase public function testCreateKey(): void { $cipherKeyFactory = new OpensslCipherKeyFactory(); - $cipherKey = $cipherKeyFactory(); + $cipherKey = $cipherKeyFactory('test-subject'); - $this->assertSame(16, strlen($cipherKey->key)); - $this->assertSame('aes128', $cipherKey->method); - $this->assertSame(16, strlen($cipherKey->iv)); + $this->assertSame(32, strlen($cipherKey->key)); + $this->assertSame('aes-128-gcm', $cipherKey->method); + $this->assertSame('test-subject', $cipherKey->subjectId); } public function testMethodNotSupported(): void @@ -29,6 +29,6 @@ public function testMethodNotSupported(): void $this->expectException(MethodNotSupported::class); $cipherKeyFactory = new OpensslCipherKeyFactory(method: 'foo'); - $cipherKeyFactory(); + $cipherKeyFactory('test-subject'); } } diff --git a/tests/Unit/Extension/Cryptography/Cipher/OpensslCipherTest.php b/tests/Unit/Extension/Cryptography/Cipher/OpensslCipherTest.php index f584fedb..4d05d15d 100644 --- a/tests/Unit/Extension/Cryptography/Cipher/OpensslCipherTest.php +++ b/tests/Unit/Extension/Cryptography/Cipher/OpensslCipherTest.php @@ -4,9 +4,11 @@ namespace Patchlevel\Hydrator\Tests\Unit\Extension\Cryptography\Cipher; +use DateTimeImmutable; use Generator; use Patchlevel\Hydrator\Extension\Cryptography\Cipher\CipherKey; use Patchlevel\Hydrator\Extension\Cryptography\Cipher\DecryptionFailed; +use Patchlevel\Hydrator\Extension\Cryptography\Cipher\EncryptedData; use Patchlevel\Hydrator\Extension\Cryptography\Cipher\EncryptionFailed; use Patchlevel\Hydrator\Extension\Cryptography\Cipher\OpensslCipher; use PHPUnit\Framework\Attributes\CoversClass; @@ -17,12 +19,20 @@ final class OpensslCipherTest extends TestCase { #[DataProvider('dataProvider')] - public function testEncrypt(mixed $value, string $encryptedString): void + public function testEncryptDecrypt(mixed $value): void { $cipher = new OpensslCipher(); - $return = $cipher->encrypt($this->createKey(), $value); + $key = $this->createKey(); - self::assertEquals($encryptedString, $return); + $encrypted = $cipher->encrypt($key, $value); + + self::assertEquals('aes-128-cbc', $encrypted->method); + self::assertNotNull($encrypted->nonce); + self::assertNotEmpty($encrypted->data); + + $decrypted = $cipher->decrypt($key, $encrypted); + + self::assertEquals($value, $decrypted); } public function testEncryptFailed(): void @@ -34,37 +44,32 @@ public function testEncryptFailed(): void 'key', 'bar', 'abcdefg123456789', + 'invalid-method', + new DateTimeImmutable(), ), ''); } - #[DataProvider('dataProvider')] - public function testDecrypt(mixed $value, string $encryptedString): void - { - $cipher = new OpensslCipher(); - $return = $cipher->decrypt($this->createKey(), $encryptedString); - - self::assertEquals($value, $return); - } - public function testDecryptFailed(): void { $this->expectException(DecryptionFailed::class); $cipher = new OpensslCipher(); - $cipher->decrypt($this->createKey('foo'), 'emNpWDlMWFBnRStpZk9YZktrUStRQT09'); + $encryptedData = new EncryptedData('invalid-data', 'aes-128-cbc', 'invalid-nonce', null); + $cipher->decrypt($this->createKey(), $encryptedData); } + /** @return Generator */ public static function dataProvider(): Generator { - yield 'empty' => ['', 'emNpWDlMWFBnRStpZk9YZktrUStRQT09']; - yield 'string' => ['foo bar baz', 'YUlYRnJZMEd1RkFycjNrQitETHhqQT09']; - yield 'integer' => [42, 'M1FHSnlnbWNlZFJiV2xwdzZIZUhDdz09']; - yield 'float' => [0.5, 'N2tOWGNia3lrdUJ1ancrMFA4OEY0Zz09']; - yield 'null' => [null, 'OUE1T081cXdpNmFMc1FIMGsrME5vdz09']; - yield 'true' => [true, 'NCtWMDE4WnV5NEtCamVVdkIxZjRrdz09']; - yield 'false' => [false, 'czh5NUYxWXhQOWhSbGVwWG5ETFdVQT09']; - yield 'array' => [['foo' => 'bar'], 'cHo2QlhxSnNFZG1kUEhRZ3pjcFJrUT09']; - yield 'long text' => ['Lorem ipsum dolor sit amet, consectetur adipiscing elit. Sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.', 'eDNCalYzSS9LbkZIcGdKNWVmUFQwTTI0YXhhSnNmdUxXeXhGUGFwMWZkTmx1ZnNwNzBUa29NcUFxUzRFV3V2WWNlUmt6YWhTSlRzVXpqd3RLZkpzUWFWYVRCR1pvbkt3TUE4UzZmaDVQcTYzMzJoWVBRRzllbHhhNjYrenNWbzFDZ2lnVm1PRFhvamozZEVmcXFYVTZGQ1dIWEgzcE1mU2w2SWlRQ2o2WFdNPQ==']; + yield 'empty' => ['']; + yield 'string' => ['foo bar baz']; + yield 'integer' => [42]; + yield 'float' => [0.5]; + yield 'null' => [null]; + yield 'true' => [true]; + yield 'false' => [false]; + yield 'array' => [['foo' => 'bar']]; + yield 'long text' => ['Lorem ipsum dolor sit amet, consectetur adipiscing elit. Sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.']; } /** @param non-empty-string $key */ @@ -74,6 +79,8 @@ private function createKey(string $key = 'key'): CipherKey $key, 'aes128', 'abcdefg123456789', + 'aes-128-cbc', + new DateTimeImmutable(), ); } } diff --git a/tests/Unit/Extension/Cryptography/CryptographyMiddlewareTest.php b/tests/Unit/Extension/Cryptography/CryptographyMiddlewareTest.php index 3bcd85bb..92648311 100644 --- a/tests/Unit/Extension/Cryptography/CryptographyMiddlewareTest.php +++ b/tests/Unit/Extension/Cryptography/CryptographyMiddlewareTest.php @@ -166,7 +166,7 @@ public function testDecryptWithCipherKeyNotExists(): void { $cryptographer = $this->createMock(Cryptographer::class); $cryptographer->method('supports')->willReturn(true); - $cryptographer->method('decrypt')->willThrowException(new CipherKeyNotExists('foo')); + $cryptographer->method('decrypt')->willThrowException(CipherKeyNotExists::forSubjectId('foo')); $middleware = new CryptographyMiddleware($cryptographer); @@ -186,7 +186,7 @@ public function testDecryptWithDecryptionFailed(): void { $cryptographer = $this->createMock(Cryptographer::class); $cryptographer->method('supports')->willReturn(true); - $cryptographer->method('decrypt')->willThrowException(new DecryptionFailed()); + $cryptographer->method('decrypt')->willThrowException(DecryptionFailed::forMethod('aes-256-gcm')); $middleware = new CryptographyMiddleware($cryptographer); @@ -206,7 +206,7 @@ public function testDecryptWithFallbackCallback(): void { $cryptographer = $this->createMock(Cryptographer::class); $cryptographer->method('supports')->willReturn(true); - $cryptographer->method('decrypt')->willThrowException(new DecryptionFailed()); + $cryptographer->method('decrypt')->willThrowException(DecryptionFailed::forMethod('aes-256-gcm')); $middleware = new CryptographyMiddleware($cryptographer); diff --git a/tests/Unit/Extension/Cryptography/Store/CipherKeyNotExistsTest.php b/tests/Unit/Extension/Cryptography/Store/CipherKeyNotExistsTest.php index 95c8d4f3..80f00e80 100644 --- a/tests/Unit/Extension/Cryptography/Store/CipherKeyNotExistsTest.php +++ b/tests/Unit/Extension/Cryptography/Store/CipherKeyNotExistsTest.php @@ -13,8 +13,8 @@ final class CipherKeyNotExistsTest extends TestCase { public function testCreation(): void { - $exception = new CipherKeyNotExists('foo'); + $exception = CipherKeyNotExists::forSubjectId('foo'); - self::assertSame('Cipher key with subject id "foo" not found.', $exception->getMessage()); + self::assertStringContainsString('foo', $exception->getMessage()); } } diff --git a/tests/Unit/Extension/Cryptography/Store/InMemoryCipherKeyStoreTest.php b/tests/Unit/Extension/Cryptography/Store/InMemoryCipherKeyStoreTest.php index a56ed9fa..99dcb441 100644 --- a/tests/Unit/Extension/Cryptography/Store/InMemoryCipherKeyStoreTest.php +++ b/tests/Unit/Extension/Cryptography/Store/InMemoryCipherKeyStoreTest.php @@ -4,6 +4,7 @@ namespace Patchlevel\Hydrator\Tests\Unit\Extension\Cryptography\Store; +use DateTimeImmutable; use Patchlevel\Hydrator\Extension\Cryptography\Cipher\CipherKey; use Patchlevel\Hydrator\Extension\Cryptography\Store\CipherKeyNotExists; use Patchlevel\Hydrator\Extension\Cryptography\Store\InMemoryCipherKeyStore; @@ -19,6 +20,8 @@ public function testStoreAndLoad(): void 'foo', 'bar', 'baz', + 'aes-256-gcm', + new DateTimeImmutable(), ); $store = new InMemoryCipherKeyStore(); @@ -41,6 +44,8 @@ public function testRemove(): void 'foo', 'bar', 'baz', + 'aes-256-gcm', + new DateTimeImmutable(), ); $store = new InMemoryCipherKeyStore(); @@ -61,6 +66,8 @@ public function testClear(): void 'foo', 'bar', 'baz', + 'aes-256-gcm', + new DateTimeImmutable(), ); $store = new InMemoryCipherKeyStore();