Merge pull request #166 from patchlevel/improve-cryptography

improve cryptography implementation

Author: DavidBadura

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

src/Extension/Cryptography/BaseCryptographer.php

@@ -5,31 +5,37 @@
 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
 {
+    private const VERSION_KEY = 'v';
+    private const METHOD_KEY = 'a';
+    private const KEY_ID_KEY = 'k';
+    private const NONCE_KEY = 'n';
+    private const DATA_KEY = 'd';
+    private const TAG_KEY = 't';
+
     public function __construct(
         private readonly Cipher $cipher,
         private readonly CipherKeyStore $cipherKeyStore,
@@ -38,50 +44,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 = [
+            self::VERSION_KEY => 1,
+            self::METHOD_KEY => $parameter->method,
+            self::KEY_ID_KEY => $cipherKey->id,
+            self::DATA_KEY => $parameter->data,
         ];
+
+        if ($parameter->nonce !== null) {
+            $result[self::NONCE_KEY] = $parameter->nonce;
+        }
+
+        if ($parameter->tag !== null) {
+            $result[self::TAG_KEY] = $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[self::KEY_ID_KEY] ?? 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[self::DATA_KEY],
+                $encryptedData[self::METHOD_KEY],
+                $encryptedData[self::NONCE_KEY] ?? null,
+                $encryptedData[self::TAG_KEY] ?? 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[self::VERSION_KEY], $value[self::METHOD_KEY], $value[self::KEY_ID_KEY], $value[self::DATA_KEY])
+            && $value[self::VERSION_KEY] === 1;
     }
 
     /** @param non-empty-string $method */

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;
 }

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,
     ) {
     }
 }

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;
 }

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));
     }
 }

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);
     }
 }

src/Extension/Cryptography/Cipher/EncryptedData.php

@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Patchlevel\Hydrator\Extension\Cryptography\Cipher;
+
+/** @experimental */
+final readonly class EncryptedData
+{
+    /**
+     * @param non-empty-string      $data
+     * @param non-empty-string      $method
+     * @param non-empty-string|null $nonce
+     * @param non-empty-string|null $tag
+     */
+    public function __construct(
+        public string $data,
+        public string $method,
+        public string|null $nonce,
+        public string|null $tag = null,
+    ) {
+    }
+}

src/Extension/Cryptography/Cipher/EncryptionFailed.php

@@ -6,12 +6,25 @@
 
 use Patchlevel\Hydrator\HydratorException;
 use RuntimeException;
+use Throwable;
+
+use function sprintf;
 
 /** @experimental */
 final class EncryptionFailed 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('Encryption failed for method "%s".', $method), $previous);
+    }
+
+    public static function invalidIvLength(string $method): self
     {
-        parent::__construct('Encryption failed.');
+        return new self(sprintf('Invalid IV length for method "%s".', $method));
     }
 }