From 02187d044779d0a7531a8580faf047f36e8f46d2 Mon Sep 17 00:00:00 2001 From: Florent Morselli Date: Thu, 6 Aug 2026 19:01:50 +0200 Subject: [PATCH] Deprecate the hardcoded RSA1_5 CEK size table (#654) The JWEDecrypter now passes the size of the key expected by the content encryption algorithm as an additional argument of KeyEncryption::decryptKey() and KeyWrapping::unwrapKey(). That argument is not declared in the interfaces yet, as it would be a BC break for every implementation: it will be added and required in 5.0.0 (#653). Implementations that do not expect it simply ignore it, the others read it with func_num_args()/func_get_arg(3). RSA1_5 uses that size for the implicit rejection (GHSA-5739-39v2-5754) and only falls back to its hardcoded CEK_LENGTHS table when the caller does not provide it. That fallback now triggers a deprecation notice. --- composer.json | 1 + .../Algorithm/KeyEncryption/KeyEncryption.php | 5 + .../Algorithm/KeyEncryption/KeyWrapping.php | 5 + .../Algorithm/KeyEncryption/RSA15.php | 31 ++- src/Library/Encryption/JWEDecrypter.php | 11 +- src/Library/composer.json | 3 +- .../LegacyKeyEncryptionAlgorithm.php | 63 ++++++ .../Encryption/RSA15ExpectedCekSizeTest.php | 190 ++++++++++++++++++ 8 files changed, 304 insertions(+), 5 deletions(-) create mode 100644 tests/Component/Encryption/LegacyKeyEncryptionAlgorithm.php create mode 100644 tests/Component/Encryption/RSA15ExpectedCekSizeTest.php diff --git a/composer.json b/composer.json index 8214d63f..adfc43de 100644 --- a/composer.json +++ b/composer.json @@ -55,6 +55,7 @@ "symfony/config": "^7.0|^8.0", "symfony/console": "^7.0|^8.0", "symfony/dependency-injection": "^7.0|^8.0", + "symfony/deprecation-contracts": "^2.5|^3.0", "symfony/event-dispatcher": "^7.0|^8.0", "symfony/http-client-contracts": "^3.4", "symfony/http-kernel": "^7.0|^8.0" diff --git a/src/Library/Encryption/Algorithm/KeyEncryption/KeyEncryption.php b/src/Library/Encryption/Algorithm/KeyEncryption/KeyEncryption.php index f27f7fca..51c73c91 100644 --- a/src/Library/Encryption/Algorithm/KeyEncryption/KeyEncryption.php +++ b/src/Library/Encryption/Algorithm/KeyEncryption/KeyEncryption.php @@ -25,6 +25,11 @@ public function encryptKey(JWK $key, string $cek, array $completeHeader, array & * @param JWK $key The key used to wrap the CEK * @param string $encrypted_cek The CEK to decrypt * @param array $header The complete header of the JWT + * + * BC NOTE: since 4.2, the JWEDecrypter calls this method with an additional argument + * "int $encryptionKeyLength": the size (in bits) of the key expected by the content encryption algorithm, + * as returned by ContentEncryptionAlgorithm::getCEKSize(). As it is not declared yet, implementations that + * need it can read it with func_num_args()/func_get_arg(3). It will be declared and required in 5.0. */ public function decryptKey(JWK $key, string $encrypted_cek, array $header): string; } diff --git a/src/Library/Encryption/Algorithm/KeyEncryption/KeyWrapping.php b/src/Library/Encryption/Algorithm/KeyEncryption/KeyWrapping.php index e5b37825..65a3419a 100644 --- a/src/Library/Encryption/Algorithm/KeyEncryption/KeyWrapping.php +++ b/src/Library/Encryption/Algorithm/KeyEncryption/KeyWrapping.php @@ -25,6 +25,11 @@ public function wrapKey(JWK $key, string $cek, array $completeHeader, array &$ad * @param JWK $key The key used to wrap the CEK * @param string $encrypted_cek The CEK to decrypt * @param array $completeHeader The complete header of the JWT + * + * BC NOTE: since 4.2, the JWEDecrypter calls this method with an additional argument + * "int $encryptionKeyLength": the size (in bits) of the key expected by the content encryption algorithm, + * as returned by ContentEncryptionAlgorithm::getCEKSize(). As it is not declared yet, implementations that + * need it can read it with func_num_args()/func_get_arg(3). It will be declared and required in 5.0. */ public function unwrapKey(JWK $key, string $encrypted_cek, array $completeHeader): string; } diff --git a/src/Library/Encryption/Algorithm/KeyEncryption/RSA15.php b/src/Library/Encryption/Algorithm/KeyEncryption/RSA15.php index 5f65b2b7..56489ef7 100644 --- a/src/Library/Encryption/Algorithm/KeyEncryption/RSA15.php +++ b/src/Library/Encryption/Algorithm/KeyEncryption/RSA15.php @@ -9,11 +9,18 @@ use Jose\Component\Core\Util\RSAKey; use Jose\Component\Encryption\Algorithm\KeyEncryption\Util\RSACrypt; use Override; +use function func_get_arg; +use function func_num_args; +use function is_int; use function is_string; +use function trigger_deprecation; final readonly class RSA15 extends RSA { /** + * BC NOTE: deprecated since 4.2 and will be removed in 5.0. The expected CEK size is now provided by the + * caller as the fourth argument of the "decryptKey" method. + * * @var array */ private const CEK_LENGTHS = [ @@ -32,11 +39,16 @@ public function name(): string } /** + * The size (in bits) of the key expected by the content encryption algorithm may be passed as a fourth + * argument. That argument is not declared yet for BC reasons; it will be in 5.0 (see the KeyEncryption + * interface). + * * @param array $header */ #[Override] public function decryptKey(JWK $key, string $encrypted_cek, array $header): string { + $encryptionKeyLength = func_num_args() > 3 ? func_get_arg(3) : null; $this->checkKey($key); if (! $key->has('d')) { throw new InvalidArgumentException('The key is not a private key'); @@ -48,7 +60,7 @@ public function decryptKey(JWK $key, string $encrypted_cek, array $header): stri $encrypted_cek, RSACrypt::ENCRYPTION_PKCS1, null, - $this->getExpectedCekLength($header) + $this->getExpectedCekLength($header, $encryptionKeyLength) ); } @@ -65,10 +77,25 @@ protected function getHashAlgorithm(): ?string } /** + * Returns the expected CEK length in bytes. + * * @param array $header + * @param mixed $encryptionKeyLength Size (in bits) of the key expected by the content encryption + * algorithm, or null when the caller did not provide it */ - private function getExpectedCekLength(array $header): ?int + private function getExpectedCekLength(array $header, mixed $encryptionKeyLength): ?int { + if (is_int($encryptionKeyLength)) { + return intdiv($encryptionKeyLength, 8); + } + + trigger_deprecation( + 'web-token/jwt-framework', + '4.2.0', + 'Calling "%s::decryptKey()" without the size of the key expected by the content encryption algorithm as fourth argument is deprecated. That size is currently deduced from a hardcoded table that will be removed in 5.0.0: pass the value returned by "getCEKSize()" of the content encryption algorithm in use instead.', + self::class + ); + $enc = $header['enc'] ?? null; if (! is_string($enc)) { return null; diff --git a/src/Library/Encryption/JWEDecrypter.php b/src/Library/Encryption/JWEDecrypter.php index bf59845f..16ba6d34 100644 --- a/src/Library/Encryption/JWEDecrypter.php +++ b/src/Library/Encryption/JWEDecrypter.php @@ -226,18 +226,25 @@ private function decryptCEK( $completeHeader ); } + // The size of the key expected by the content encryption algorithm is passed as an additional + // argument. It is not part of the interfaces yet (it will be in 5.0.0): implementations that do not + // expect it simply ignore it, the others read it with func_num_args()/func_get_arg(3). if ($key_encryption_algorithm instanceof KeyEncryption) { + // @phpstan-ignore arguments.count (the fourth argument will be part of the interface in 5.0.0) return $key_encryption_algorithm->decryptKey( $recipientKey, $recipient->getEncryptedKey() ?? '', - $completeHeader + $completeHeader, + $content_encryption_algorithm->getCEKSize() ); } if ($key_encryption_algorithm instanceof KeyWrapping) { + // @phpstan-ignore arguments.count (the fourth argument will be part of the interface in 5.0.0) return $key_encryption_algorithm->unwrapKey( $recipientKey, $recipient->getEncryptedKey() ?? '', - $completeHeader + $completeHeader, + $content_encryption_algorithm->getCEKSize() ); } diff --git a/src/Library/composer.json b/src/Library/composer.json index 3aea385f..31a78dbc 100644 --- a/src/Library/composer.json +++ b/src/Library/composer.json @@ -41,7 +41,8 @@ "php": ">=8.2", "brick/math": "^0.12|^0.13|^0.14|^0.15|^0.16|^0.17|^0.18|^0.19", "psr/clock": "^1.0", - "spomky-labs/pki-framework": "^1.2.1" + "spomky-labs/pki-framework": "^1.2.1", + "symfony/deprecation-contracts": "^2.5|^3.0" }, "conflict": { "spomky-labs/jose": "*" diff --git a/tests/Component/Encryption/LegacyKeyEncryptionAlgorithm.php b/tests/Component/Encryption/LegacyKeyEncryptionAlgorithm.php new file mode 100644 index 00000000..ffac4160 --- /dev/null +++ b/tests/Component/Encryption/LegacyKeyEncryptionAlgorithm.php @@ -0,0 +1,63 @@ + $completeHeader + * @param array $additionalHeader + */ + #[Override] + public function encryptKey(JWK $key, string $cek, array $completeHeader, array &$additionalHeader): string + { + return $cek; + } + + /** + * @param array $header + */ + #[Override] + public function decryptKey(JWK $key, string $encrypted_cek, array $header): string + { + $this->receivedArgumentCount = func_num_args(); + + return $encrypted_cek; + } +} diff --git a/tests/Component/Encryption/RSA15ExpectedCekSizeTest.php b/tests/Component/Encryption/RSA15ExpectedCekSizeTest.php new file mode 100644 index 00000000..6ad36cb0 --- /dev/null +++ b/tests/Component/Encryption/RSA15ExpectedCekSizeTest.php @@ -0,0 +1,190 @@ +createKey(); + $algorithm = new RSA15(); + $cek = random_bytes(16); // A128GCM CEK + $header = [ + 'alg' => 'RSA1_5', + 'enc' => 'A128GCM', + ]; + $additionalHeader = []; + $encrypted = $algorithm->encryptKey($jwk, $cek, $header, $additionalHeader); + + $decrypted = null; + $deprecations = $this->collectDeprecations(static function () use ( + $algorithm, + $jwk, + $encrypted, + $header, + &$decrypted + ): void { + $decrypted = $algorithm->decryptKey($jwk, $encrypted, $header); + }); + + static::assertSame($cek, $decrypted); + static::assertCount(1, $deprecations); + static::assertStringContainsString( + 'Calling "Jose\Component\Encryption\Algorithm\KeyEncryption\RSA15::decryptKey()" without the size of the key expected by the content encryption algorithm as fourth argument is deprecated.', + $deprecations[0] + ); + } + + #[Test] + public function noDeprecationIsTriggeredWhenTheExpectedCekSizeIsProvided(): void + { + $jwk = $this->createKey(); + $algorithm = new RSA15(); + $cek = random_bytes(16); // A128GCM CEK + $header = [ + 'alg' => 'RSA1_5', + 'enc' => 'A128GCM', + ]; + $additionalHeader = []; + $encrypted = $algorithm->encryptKey($jwk, $cek, $header, $additionalHeader); + + $decrypted = null; + $deprecations = $this->collectDeprecations(static function () use ( + $algorithm, + $jwk, + $encrypted, + $header, + &$decrypted + ): void { + $decrypted = $algorithm->decryptKey($jwk, $encrypted, $header, 128); + }); + + static::assertSame($cek, $decrypted); + static::assertSame([], $deprecations); + } + + #[Test] + public function theProvidedCekSizeIsUsedForTheImplicitRejection(): void + { + $jwk = $this->createKey(); + $algorithm = new RSA15(); + $key = RSAKey::createFromJWK($jwk); + $garbage = "\x00" . random_bytes($key->getModulusLength() - 1); + // The content encryption algorithm is unknown to the hardcoded table. + $header = [ + 'alg' => 'RSA1_5', + 'enc' => 'FOO-256', + ]; + + $result = $algorithm->decryptKey($jwk, $garbage, $header, 256); + + static::assertSame(32, mb_strlen($result, '8bit')); + } + + #[Test] + public function theJweDecrypterProvidesTheExpectedCekSize(): void + { + $jwk = $this->createKey(); + $algorithmManager = new AlgorithmManager([new RSA15(), new A128GCM()]); + $token = $this->createToken($algorithmManager, $jwk, 'RSA1_5'); + + $jwe = (new CompactSerializer())->unserialize($token); + $decrypter = new JWEDecrypter($algorithmManager); + + $deprecations = $this->collectDeprecations(static function () use ($decrypter, $jwe, $jwk): void { + $jweToDecrypt = $jwe; + static::assertTrue($decrypter->decryptUsingKey($jweToDecrypt, $jwk, 0)); + static::assertSame('Live long and prosper.', $jweToDecrypt->getPayload()); + }); + + static::assertSame([], $deprecations); + } + + #[Test] + public function algorithmsThatDoNotExpectTheCekSizeStillWork(): void + { + $jwk = JWKFactory::createOctKey(256, [ + 'use' => 'enc', + ]); + $algorithm = new LegacyKeyEncryptionAlgorithm(); + $algorithmManager = new AlgorithmManager([$algorithm, new A128GCM()]); + $token = $this->createToken($algorithmManager, $jwk, $algorithm->name()); + + $jwe = (new CompactSerializer())->unserialize($token); + $decrypter = new JWEDecrypter($algorithmManager); + + static::assertTrue($decrypter->decryptUsingKey($jwe, $jwk, 0)); + static::assertSame('Live long and prosper.', $jwe->getPayload()); + static::assertSame(4, $algorithm->receivedArgumentCount); + } + + private function createKey(): JWK + { + return JWKFactory::createRSAKey(2048, [ + 'alg' => 'RSA1_5', + 'use' => 'enc', + ]); + } + + private function createToken(AlgorithmManager $algorithmManager, JWK $jwk, string $algorithm): string + { + $jwe = (new JWEBuilder($algorithmManager)) + ->create() + ->withPayload('Live long and prosper.') + ->withSharedProtectedHeader([ + 'alg' => $algorithm, + 'enc' => 'A128GCM', + ]) + ->addRecipient($jwk) + ->build(); + + return (new CompactSerializer())->serialize($jwe, 0); + } + + /** + * @param callable(): void $callback + * + * @return list + */ + private function collectDeprecations(callable $callback): array + { + $deprecations = []; + set_error_handler(static function (int $errno, string $errstr) use (&$deprecations): bool { + $deprecations[] = $errstr; + + return true; + }, E_USER_DEPRECATED); + + try { + $callback(); + } finally { + restore_error_handler(); + } + + return $deprecations; + } +}