From 1a9d59c222968715e1ddfc30e5cbd266580129c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lu=C3=ADs=20Cobucci?= Date: Wed, 17 Aug 2022 21:27:48 +0200 Subject: [PATCH 1/5] Rename method to better reflect its intent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Luís Cobucci --- src/Signer/Ecdsa.php | 6 +++--- src/Signer/Ecdsa/Sha256.php | 2 +- src/Signer/Ecdsa/Sha384.php | 2 +- src/Signer/Ecdsa/Sha512.php | 2 +- src/Signer/Ecdsa/UnsafeSha256.php | 2 +- src/Signer/Ecdsa/UnsafeSha384.php | 2 +- src/Signer/Ecdsa/UnsafeSha512.php | 2 +- src/Signer/UnsafeEcdsa.php | 6 +++--- test/unit/Signer/Ecdsa/Sha256Test.php | 4 ++-- test/unit/Signer/Ecdsa/Sha384Test.php | 4 ++-- test/unit/Signer/Ecdsa/Sha512Test.php | 4 ++-- test/unit/Signer/Ecdsa/UnsafeSha256Test.php | 4 ++-- test/unit/Signer/Ecdsa/UnsafeSha384Test.php | 4 ++-- test/unit/Signer/Ecdsa/UnsafeSha512Test.php | 4 ++-- test/unit/Signer/EcdsaTest.php | 6 +++--- test/unit/Signer/UnsafeEcdsaTest.php | 6 +++--- 16 files changed, 30 insertions(+), 30 deletions(-) diff --git a/src/Signer/Ecdsa.php b/src/Signer/Ecdsa.php index efaec676..888868f1 100644 --- a/src/Signer/Ecdsa.php +++ b/src/Signer/Ecdsa.php @@ -26,14 +26,14 @@ final public function sign(string $payload, Key $key): string { return $this->converter->fromAsn1( $this->createSignature($key->contents(), $key->passphrase(), $payload), - $this->keyLength() + $this->pointLength() ); } final public function verify(string $expected, string $payload, Key $key): bool { return $this->verifySignature( - $this->converter->toAsn1($expected, $this->keyLength()), + $this->converter->toAsn1($expected, $this->pointLength()), $payload, $key->contents() ); @@ -54,5 +54,5 @@ final public function minimumBitsLengthForKey(): int * * @internal */ - abstract public function keyLength(): int; + abstract public function pointLength(): int; } diff --git a/src/Signer/Ecdsa/Sha256.php b/src/Signer/Ecdsa/Sha256.php index 36f6ec12..cdb16b59 100644 --- a/src/Signer/Ecdsa/Sha256.php +++ b/src/Signer/Ecdsa/Sha256.php @@ -19,7 +19,7 @@ public function algorithm(): int return OPENSSL_ALGO_SHA256; } - public function keyLength(): int + public function pointLength(): int { return 64; } diff --git a/src/Signer/Ecdsa/Sha384.php b/src/Signer/Ecdsa/Sha384.php index d3c5dbce..f3ed8d8c 100644 --- a/src/Signer/Ecdsa/Sha384.php +++ b/src/Signer/Ecdsa/Sha384.php @@ -19,7 +19,7 @@ public function algorithm(): int return OPENSSL_ALGO_SHA384; } - public function keyLength(): int + public function pointLength(): int { return 96; } diff --git a/src/Signer/Ecdsa/Sha512.php b/src/Signer/Ecdsa/Sha512.php index 1495d13b..0b0e9451 100644 --- a/src/Signer/Ecdsa/Sha512.php +++ b/src/Signer/Ecdsa/Sha512.php @@ -19,7 +19,7 @@ public function algorithm(): int return OPENSSL_ALGO_SHA512; } - public function keyLength(): int + public function pointLength(): int { return 132; } diff --git a/src/Signer/Ecdsa/UnsafeSha256.php b/src/Signer/Ecdsa/UnsafeSha256.php index 0fadd845..6f2699d0 100644 --- a/src/Signer/Ecdsa/UnsafeSha256.php +++ b/src/Signer/Ecdsa/UnsafeSha256.php @@ -20,7 +20,7 @@ public function algorithm(): int return OPENSSL_ALGO_SHA256; } - public function keyLength(): int + public function pointLength(): int { return 64; } diff --git a/src/Signer/Ecdsa/UnsafeSha384.php b/src/Signer/Ecdsa/UnsafeSha384.php index 1d6fd99d..96cc99d4 100644 --- a/src/Signer/Ecdsa/UnsafeSha384.php +++ b/src/Signer/Ecdsa/UnsafeSha384.php @@ -20,7 +20,7 @@ public function algorithm(): int return OPENSSL_ALGO_SHA384; } - public function keyLength(): int + public function pointLength(): int { return 96; } diff --git a/src/Signer/Ecdsa/UnsafeSha512.php b/src/Signer/Ecdsa/UnsafeSha512.php index b6920456..c4f6bc76 100644 --- a/src/Signer/Ecdsa/UnsafeSha512.php +++ b/src/Signer/Ecdsa/UnsafeSha512.php @@ -20,7 +20,7 @@ public function algorithm(): int return OPENSSL_ALGO_SHA512; } - public function keyLength(): int + public function pointLength(): int { return 132; } diff --git a/src/Signer/UnsafeEcdsa.php b/src/Signer/UnsafeEcdsa.php index 6e24e7a4..c437a3e9 100644 --- a/src/Signer/UnsafeEcdsa.php +++ b/src/Signer/UnsafeEcdsa.php @@ -27,14 +27,14 @@ final public function sign(string $payload, Key $key): string { return $this->converter->fromAsn1( $this->createSignature($key->contents(), $key->passphrase(), $payload), - $this->keyLength() + $this->pointLength() ); } final public function verify(string $expected, string $payload, Key $key): bool { return $this->verifySignature( - $this->converter->toAsn1($expected, $this->keyLength()), + $this->converter->toAsn1($expected, $this->pointLength()), $payload, $key->contents() ); @@ -55,5 +55,5 @@ final public function minimumBitsLengthForKey(): int * * @internal */ - abstract public function keyLength(): int; + abstract public function pointLength(): int; } diff --git a/test/unit/Signer/Ecdsa/Sha256Test.php b/test/unit/Signer/Ecdsa/Sha256Test.php index 9f01e1d3..49bded01 100644 --- a/test/unit/Signer/Ecdsa/Sha256Test.php +++ b/test/unit/Signer/Ecdsa/Sha256Test.php @@ -52,13 +52,13 @@ public function algorithmMustBeCorrect(): void /** * @test * - * @covers ::keyLength + * @covers ::pointLength * * @uses \Lcobucci\JWT\Signer\Ecdsa */ public function keyLengthMustBeCorrect(): void { - self::assertSame(64, $this->getSigner()->keyLength()); + self::assertSame(64, $this->getSigner()->pointLength()); } /** diff --git a/test/unit/Signer/Ecdsa/Sha384Test.php b/test/unit/Signer/Ecdsa/Sha384Test.php index 26ef3e2a..e3c9758f 100644 --- a/test/unit/Signer/Ecdsa/Sha384Test.php +++ b/test/unit/Signer/Ecdsa/Sha384Test.php @@ -52,13 +52,13 @@ public function algorithmMustBeCorrect(): void /** * @test * - * @covers ::keyLength + * @covers ::pointLength * * @uses \Lcobucci\JWT\Signer\Ecdsa */ public function keyLengthMustBeCorrect(): void { - self::assertSame(96, $this->getSigner()->keyLength()); + self::assertSame(96, $this->getSigner()->pointLength()); } /** diff --git a/test/unit/Signer/Ecdsa/Sha512Test.php b/test/unit/Signer/Ecdsa/Sha512Test.php index 967d433f..35281d8a 100644 --- a/test/unit/Signer/Ecdsa/Sha512Test.php +++ b/test/unit/Signer/Ecdsa/Sha512Test.php @@ -52,13 +52,13 @@ public function algorithmMustBeCorrect(): void /** * @test * - * @covers ::keyLength + * @covers ::pointLength * * @uses \Lcobucci\JWT\Signer\Ecdsa */ public function keyLengthMustBeCorrect(): void { - self::assertSame(132, $this->getSigner()->keyLength()); + self::assertSame(132, $this->getSigner()->pointLength()); } /** diff --git a/test/unit/Signer/Ecdsa/UnsafeSha256Test.php b/test/unit/Signer/Ecdsa/UnsafeSha256Test.php index fc65ff0b..8d216520 100644 --- a/test/unit/Signer/Ecdsa/UnsafeSha256Test.php +++ b/test/unit/Signer/Ecdsa/UnsafeSha256Test.php @@ -52,13 +52,13 @@ public function algorithmMustBeCorrect(): void /** * @test * - * @covers ::keyLength + * @covers ::pointLength * * @uses \Lcobucci\JWT\Signer\UnsafeEcdsa */ public function keyLengthMustBeCorrect(): void { - self::assertSame(64, $this->getSigner()->keyLength()); + self::assertSame(64, $this->getSigner()->pointLength()); } /** diff --git a/test/unit/Signer/Ecdsa/UnsafeSha384Test.php b/test/unit/Signer/Ecdsa/UnsafeSha384Test.php index 8dd86a26..01d33cbe 100644 --- a/test/unit/Signer/Ecdsa/UnsafeSha384Test.php +++ b/test/unit/Signer/Ecdsa/UnsafeSha384Test.php @@ -52,13 +52,13 @@ public function algorithmMustBeCorrect(): void /** * @test * - * @covers ::keyLength + * @covers ::pointLength * * @uses \Lcobucci\JWT\Signer\UnsafeEcdsa */ public function keyLengthMustBeCorrect(): void { - self::assertSame(96, $this->getSigner()->keyLength()); + self::assertSame(96, $this->getSigner()->pointLength()); } /** diff --git a/test/unit/Signer/Ecdsa/UnsafeSha512Test.php b/test/unit/Signer/Ecdsa/UnsafeSha512Test.php index 947a6de1..cbe139b4 100644 --- a/test/unit/Signer/Ecdsa/UnsafeSha512Test.php +++ b/test/unit/Signer/Ecdsa/UnsafeSha512Test.php @@ -52,13 +52,13 @@ public function algorithmMustBeCorrect(): void /** * @test * - * @covers ::keyLength + * @covers ::pointLength * * @uses \Lcobucci\JWT\Signer\UnsafeEcdsa */ public function keyLengthMustBeCorrect(): void { - self::assertSame(132, $this->getSigner()->keyLength()); + self::assertSame(132, $this->getSigner()->pointLength()); } /** diff --git a/test/unit/Signer/EcdsaTest.php b/test/unit/Signer/EcdsaTest.php index e65942a2..801fca00 100644 --- a/test/unit/Signer/EcdsaTest.php +++ b/test/unit/Signer/EcdsaTest.php @@ -40,7 +40,7 @@ private function getSigner(): Ecdsa $signer->method('algorithmId') ->willReturn('ES256'); - $signer->method('keyLength') + $signer->method('pointLength') ->willReturn(64); return $signer; @@ -72,7 +72,7 @@ public function signShouldReturnTheAHashBasedOnTheOpenSslSignature(): void 1, openssl_verify( $payload, - $this->pointsManipulator->toAsn1($signature, $signer->keyLength()), + $this->pointsManipulator->toAsn1($signature, $signer->pointLength()), $publicKey, OPENSSL_ALGO_SHA256 ) @@ -128,7 +128,7 @@ public function verifyShouldDelegateToEcdsaSignerUsingPublicKey(): void self::assertTrue( $signer->verify( - $this->pointsManipulator->fromAsn1($signature, $signer->keyLength()), + $this->pointsManipulator->fromAsn1($signature, $signer->pointLength()), $payload, self::$ecdsaKeys['public1'] ) diff --git a/test/unit/Signer/UnsafeEcdsaTest.php b/test/unit/Signer/UnsafeEcdsaTest.php index eabaefb3..2708d9a6 100644 --- a/test/unit/Signer/UnsafeEcdsaTest.php +++ b/test/unit/Signer/UnsafeEcdsaTest.php @@ -40,7 +40,7 @@ private function getSigner(): UnsafeEcdsa $signer->method('algorithmId') ->willReturn('ES256'); - $signer->method('keyLength') + $signer->method('pointLength') ->willReturn(64); return $signer; @@ -72,7 +72,7 @@ public function signShouldReturnTheAHashBasedOnTheOpenSslSignature(): void 1, openssl_verify( $payload, - $this->pointsManipulator->toAsn1($signature, $signer->keyLength()), + $this->pointsManipulator->toAsn1($signature, $signer->pointLength()), $publicKey, OPENSSL_ALGO_SHA256 ) @@ -130,7 +130,7 @@ public function verifyShouldDelegateToEcdsaSignerUsingPublicKey(): void self::assertTrue( $signer->verify( - $this->pointsManipulator->fromAsn1($signature, $signer->keyLength()), + $this->pointsManipulator->fromAsn1($signature, $signer->pointLength()), $payload, self::$ecdsaKeys['public1'] ) From e66e85ae4cf22039e39d4f7c39eddb433fd86ca5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lu=C3=ADs=20Cobucci?= Date: Wed, 17 Aug 2022 23:18:06 +0200 Subject: [PATCH 2/5] Ensure full OpenSSL error message is provided MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lines in OpenSSL error messages are split and we need to call `openssl_error_string()` multiple times to get all the occurred errors. More info: https://www.php.net/manual/en/function.openssl-error-string.php Signed-off-by: Luís Cobucci --- src/Signer/CannotSignPayload.php | 2 +- src/Signer/InvalidKeyProvided.php | 2 +- src/Signer/OpenSSL.php | 24 ++++++++++++------- test/unit/Signer/EcdsaTest.php | 9 +++++++ test/unit/Signer/RsaTest.php | 36 ++++++++++++++++++++++++++-- test/unit/Signer/UnsafeEcdsaTest.php | 9 +++++++ test/unit/Signer/UnsafeRsaTest.php | 16 ++++++++++--- 7 files changed, 82 insertions(+), 16 deletions(-) diff --git a/src/Signer/CannotSignPayload.php b/src/Signer/CannotSignPayload.php index 418a8dce..35cc4d6d 100644 --- a/src/Signer/CannotSignPayload.php +++ b/src/Signer/CannotSignPayload.php @@ -10,6 +10,6 @@ final class CannotSignPayload extends InvalidArgumentException implements Except { public static function errorHappened(string $error): self { - return new self('There was an error while creating the signature: ' . $error); + return new self('There was an error while creating the signature:' . $error); } } diff --git a/src/Signer/InvalidKeyProvided.php b/src/Signer/InvalidKeyProvided.php index a7e904f4..5dcc01c8 100644 --- a/src/Signer/InvalidKeyProvided.php +++ b/src/Signer/InvalidKeyProvided.php @@ -10,7 +10,7 @@ final class InvalidKeyProvided extends InvalidArgumentException implements Excep { public static function cannotBeParsed(string $details): self { - return new self('It was not possible to parse your key, reason: ' . $details); + return new self('It was not possible to parse your key, reason:' . $details); } public static function incompatibleKey(): self diff --git a/src/Signer/OpenSSL.php b/src/Signer/OpenSSL.php index 78cc7190..795ee7c0 100644 --- a/src/Signer/OpenSSL.php +++ b/src/Signer/OpenSSL.php @@ -11,7 +11,6 @@ use function is_array; use function is_bool; use function is_int; -use function is_string; use function openssl_error_string; use function openssl_free_key; use function openssl_pkey_get_details; @@ -20,6 +19,8 @@ use function openssl_sign; use function openssl_verify; +use const PHP_EOL; + abstract class OpenSSL implements Signer { /** @@ -37,10 +38,7 @@ final protected function createSignature( $signature = ''; if (! openssl_sign($payload, $signature, $key, $this->algorithm())) { - $error = openssl_error_string(); - assert(is_string($error)); - - throw CannotSignPayload::errorHappened($error); + throw CannotSignPayload::errorHappened($this->fullOpenSSLErrorString()); } return $signature; @@ -101,10 +99,7 @@ private function getPublicKey(string $pem) private function validateKey($key): void { if (is_bool($key)) { - $error = openssl_error_string(); - assert(is_string($error)); - - throw InvalidKeyProvided::cannotBeParsed($error); + throw InvalidKeyProvided::cannotBeParsed($this->fullOpenSSLErrorString()); } $details = openssl_pkey_get_details($key); @@ -121,6 +116,17 @@ private function validateKey($key): void } } + private function fullOpenSSLErrorString(): string + { + $error = ''; + + while ($msg = openssl_error_string()) { + $error .= PHP_EOL . '* ' . $msg; + } + + return $error; + } + /** @param resource|OpenSSLAsymmetricKey $key */ private function freeKey($key): void { diff --git a/test/unit/Signer/EcdsaTest.php b/test/unit/Signer/EcdsaTest.php index 801fca00..5725352a 100644 --- a/test/unit/Signer/EcdsaTest.php +++ b/test/unit/Signer/EcdsaTest.php @@ -10,6 +10,7 @@ use function assert; use function is_resource; +use function openssl_error_string; use function openssl_pkey_get_private; use function openssl_pkey_get_public; use function openssl_sign; @@ -24,6 +25,14 @@ final class EcdsaTest extends TestCase private MultibyteStringConverter $pointsManipulator; + /** @after */ + public function clearOpenSSLErrors(): void + { + // phpcs:ignore Generic.CodeAnalysis.EmptyStatement.DetectedWhile + while (openssl_error_string()) { + } + } + /** @before */ public function createDependencies(): void { diff --git a/test/unit/Signer/RsaTest.php b/test/unit/Signer/RsaTest.php index 20ba8990..a11c29d3 100644 --- a/test/unit/Signer/RsaTest.php +++ b/test/unit/Signer/RsaTest.php @@ -10,18 +10,28 @@ use function assert; use function is_resource; +use function openssl_error_string; use function openssl_pkey_get_private; use function openssl_pkey_get_public; use function openssl_sign; use function openssl_verify; use const OPENSSL_ALGO_SHA256; +use const PHP_EOL; /** @coversDefaultClass \Lcobucci\JWT\Signer\Rsa */ final class RsaTest extends TestCase { use Keys; + /** @after */ + public function clearOpenSSLErrors(): void + { + // phpcs:ignore Generic.CodeAnalysis.EmptyStatement.DetectedWhile + while (openssl_error_string()) { + } + } + /** * @test * @@ -59,7 +69,29 @@ public function signShouldRaiseAnExceptionWhenKeyIsNotParseable(): void $signer = $this->getSigner(); $this->expectException(InvalidKeyProvided::class); - $this->expectExceptionMessage('It was not possible to parse your key, reason: error:'); + $this->expectExceptionMessage('It was not possible to parse your key, reason:' . PHP_EOL . '* error:'); + + $signer->sign('testing', InMemory::plainText('blablabla')); + } + + /** + * @test + * + * @covers ::sign + * @covers \Lcobucci\JWT\Signer\OpenSSL + * @covers \Lcobucci\JWT\Signer\InvalidKeyProvided + * + * @uses \Lcobucci\JWT\Signer\Key\InMemory + */ + public function allOpenSSLErrorsShouldBeOnTheErrorMessage(): void + { + $signer = $this->getSigner(); + + // Injects a random OpenSSL error message + openssl_pkey_get_private('blahblah'); + + $this->expectException(InvalidKeyProvided::class); + $this->expectExceptionMessageMatches('/^.* reason:(' . PHP_EOL . '\* error:.*){2,}/'); $signer->sign('testing', InMemory::plainText('blablabla')); } @@ -143,7 +175,7 @@ public function verifyShouldRaiseAnExceptionWhenKeyIsNotParseable(): void $signer = $this->getSigner(); $this->expectException(InvalidKeyProvided::class); - $this->expectExceptionMessage('It was not possible to parse your key'); + $this->expectExceptionMessage('It was not possible to parse your key, reason:' . PHP_EOL . '* error:'); $signer->verify('testing', 'testing', InMemory::plainText('blablabla')); } diff --git a/test/unit/Signer/UnsafeEcdsaTest.php b/test/unit/Signer/UnsafeEcdsaTest.php index 2708d9a6..1fe8687b 100644 --- a/test/unit/Signer/UnsafeEcdsaTest.php +++ b/test/unit/Signer/UnsafeEcdsaTest.php @@ -10,6 +10,7 @@ use function assert; use function is_resource; +use function openssl_error_string; use function openssl_pkey_get_private; use function openssl_pkey_get_public; use function openssl_sign; @@ -24,6 +25,14 @@ final class UnsafeEcdsaTest extends TestCase private MultibyteStringConverter $pointsManipulator; + /** @after */ + public function clearOpenSSLErrors(): void + { + // phpcs:ignore Generic.CodeAnalysis.EmptyStatement.DetectedWhile + while (openssl_error_string()) { + } + } + /** @before */ public function createDependencies(): void { diff --git a/test/unit/Signer/UnsafeRsaTest.php b/test/unit/Signer/UnsafeRsaTest.php index a99e9b06..75ffb498 100644 --- a/test/unit/Signer/UnsafeRsaTest.php +++ b/test/unit/Signer/UnsafeRsaTest.php @@ -10,18 +10,28 @@ use function assert; use function is_resource; +use function openssl_error_string; use function openssl_pkey_get_private; use function openssl_pkey_get_public; use function openssl_sign; use function openssl_verify; use const OPENSSL_ALGO_SHA256; +use const PHP_EOL; /** @coversDefaultClass \Lcobucci\JWT\Signer\UnsafeRsa */ final class UnsafeRsaTest extends TestCase { use Keys; + /** @after */ + public function clearOpenSSLErrors(): void + { + // phpcs:ignore Generic.CodeAnalysis.EmptyStatement.DetectedWhile + while (openssl_error_string()) { + } + } + /** * @test * @@ -69,7 +79,7 @@ public function signShouldRaiseAnExceptionWhenKeyIsInvalid(): void $signer = $this->getSigner(); $this->expectException(CannotSignPayload::class); - $this->expectExceptionMessage('There was an error while creating the signature: error:'); + $this->expectExceptionMessage('There was an error while creating the signature:' . PHP_EOL . '* error:'); $signer->sign('testing', InMemory::plainText($key)); } @@ -88,7 +98,7 @@ public function signShouldRaiseAnExceptionWhenKeyIsNotParseable(): void $signer = $this->getSigner(); $this->expectException(InvalidKeyProvided::class); - $this->expectExceptionMessage('It was not possible to parse your key, reason: error:'); + $this->expectExceptionMessage('It was not possible to parse your key, reason:' . PHP_EOL . '* error:'); $signer->sign('testing', InMemory::plainText('blablabla')); } @@ -173,7 +183,7 @@ public function verifyShouldRaiseAnExceptionWhenKeyIsNotParseable(): void $signer = $this->getSigner(); $this->expectException(InvalidKeyProvided::class); - $this->expectExceptionMessage('It was not possible to parse your key'); + $this->expectExceptionMessage('It was not possible to parse your key, reason:' . PHP_EOL . '* error:'); $signer->verify('testing', 'testing', InMemory::plainText('blablabla')); } From c7ada979caff9d4075441ee2641fe83d30d6b031 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lu=C3=ADs=20Cobucci?= Date: Wed, 17 Aug 2022 23:36:15 +0200 Subject: [PATCH 3/5] Use correct ECDSA key size for each algorithm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Luís Cobucci --- test/performance/Ecdsa/EcdsaBench.php | 22 ------------------- test/performance/Ecdsa/Sha256Bench.php | 16 +++++++++++++- test/performance/Ecdsa/Sha384Bench.php | 16 +++++++++++++- test/performance/Ecdsa/Sha512Bench.php | 16 +++++++++++++- .../Ecdsa/{private.key => private-256.key} | 0 test/performance/Ecdsa/private-384.key | 6 +++++ test/performance/Ecdsa/private-521.key | 7 ++++++ .../Ecdsa/{public.key => public-256.key} | 0 test/performance/Ecdsa/public-384.key | 5 +++++ test/performance/Ecdsa/public-521.key | 6 +++++ 10 files changed, 69 insertions(+), 25 deletions(-) delete mode 100644 test/performance/Ecdsa/EcdsaBench.php rename test/performance/Ecdsa/{private.key => private-256.key} (100%) create mode 100644 test/performance/Ecdsa/private-384.key create mode 100644 test/performance/Ecdsa/private-521.key rename test/performance/Ecdsa/{public.key => public-256.key} (100%) create mode 100644 test/performance/Ecdsa/public-384.key create mode 100644 test/performance/Ecdsa/public-521.key diff --git a/test/performance/Ecdsa/EcdsaBench.php b/test/performance/Ecdsa/EcdsaBench.php deleted file mode 100644 index 6583e935..00000000 --- a/test/performance/Ecdsa/EcdsaBench.php +++ /dev/null @@ -1,22 +0,0 @@ - Date: Wed, 17 Aug 2022 23:22:48 +0200 Subject: [PATCH 4/5] Re-organise OpenSSL key incompatibility verification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The expected behaviour for key length verification between RSA and ECDSA algorithms are actually different. This shifts the responsibility to the base respective base implementations, simplifying the code a bit. Signed-off-by: Luís Cobucci --- src/Signer/Ecdsa.php | 21 +++++++---- src/Signer/Ecdsa/Sha256.php | 5 +++ src/Signer/Ecdsa/Sha384.php | 5 +++ src/Signer/Ecdsa/Sha512.php | 5 +++ src/Signer/InvalidKeyProvided.php | 15 ++++++-- src/Signer/OpenSSL.php | 35 +++++++++--------- src/Signer/Rsa.php | 18 ++++++---- src/Signer/UnsafeEcdsa.php | 15 ++++---- src/Signer/UnsafeRsa.php | 15 ++++---- test/functional/ES512TokenTest.php | 4 +-- test/functional/EcdsaTokenTest.php | 4 +-- test/functional/RsaTokenTest.php | 4 +-- test/unit/Signer/Ecdsa/Sha256Test.php | 6 ++-- test/unit/Signer/Ecdsa/Sha384Test.php | 6 ++-- test/unit/Signer/Ecdsa/Sha512Test.php | 6 ++-- test/unit/Signer/Ecdsa/UnsafeSha256Test.php | 12 ------- test/unit/Signer/Ecdsa/UnsafeSha384Test.php | 12 ------- test/unit/Signer/Ecdsa/UnsafeSha512Test.php | 12 ------- test/unit/Signer/EcdsaTest.php | 39 +++++++++++++++------ test/unit/Signer/Rsa/Sha256Test.php | 12 ------- test/unit/Signer/Rsa/Sha384Test.php | 12 ------- test/unit/Signer/Rsa/Sha512Test.php | 12 ------- test/unit/Signer/Rsa/UnsafeSha256Test.php | 12 ------- test/unit/Signer/Rsa/UnsafeSha384Test.php | 12 ------- test/unit/Signer/Rsa/UnsafeSha512Test.php | 12 ------- test/unit/Signer/RsaTest.php | 13 +++---- test/unit/Signer/UnsafeEcdsaTest.php | 21 ++++------- test/unit/Signer/UnsafeRsaTest.php | 21 +++++------ 28 files changed, 154 insertions(+), 212 deletions(-) diff --git a/src/Signer/Ecdsa.php b/src/Signer/Ecdsa.php index 888868f1..472cbbad 100644 --- a/src/Signer/Ecdsa.php +++ b/src/Signer/Ecdsa.php @@ -39,16 +39,25 @@ final public function verify(string $expected, string $payload, Key $key): bool ); } - final public function keyType(): int + /** {@inheritdoc} */ + final protected function guardAgainstIncompatibleKey(int $type, int $lengthInBits): void { - return OPENSSL_KEYTYPE_EC; - } + if ($type !== OPENSSL_KEYTYPE_EC) { + throw InvalidKeyProvided::incompatibleKeyType( + self::KEY_TYPE_MAP[OPENSSL_KEYTYPE_EC], + self::KEY_TYPE_MAP[$type], + ); + } - final public function minimumBitsLengthForKey(): int - { - return 224; + $expectedKeyLength = $this->expectedKeyLength(); + + if ($lengthInBits !== $expectedKeyLength) { + throw InvalidKeyProvided::incompatibleKeyLength($expectedKeyLength, $lengthInBits); + } } + abstract public function expectedKeyLength(): int; + /** * Returns the length of each point in the signature, so that we can calculate and verify R and S points properly * diff --git a/src/Signer/Ecdsa/Sha256.php b/src/Signer/Ecdsa/Sha256.php index cdb16b59..ff00f4d4 100644 --- a/src/Signer/Ecdsa/Sha256.php +++ b/src/Signer/Ecdsa/Sha256.php @@ -23,4 +23,9 @@ public function pointLength(): int { return 64; } + + public function expectedKeyLength(): int + { + return 256; + } } diff --git a/src/Signer/Ecdsa/Sha384.php b/src/Signer/Ecdsa/Sha384.php index f3ed8d8c..fa6dd657 100644 --- a/src/Signer/Ecdsa/Sha384.php +++ b/src/Signer/Ecdsa/Sha384.php @@ -23,4 +23,9 @@ public function pointLength(): int { return 96; } + + public function expectedKeyLength(): int + { + return 384; + } } diff --git a/src/Signer/Ecdsa/Sha512.php b/src/Signer/Ecdsa/Sha512.php index 0b0e9451..35857869 100644 --- a/src/Signer/Ecdsa/Sha512.php +++ b/src/Signer/Ecdsa/Sha512.php @@ -23,4 +23,9 @@ public function pointLength(): int { return 132; } + + public function expectedKeyLength(): int + { + return 521; + } } diff --git a/src/Signer/InvalidKeyProvided.php b/src/Signer/InvalidKeyProvided.php index 5dcc01c8..3491d022 100644 --- a/src/Signer/InvalidKeyProvided.php +++ b/src/Signer/InvalidKeyProvided.php @@ -13,9 +13,20 @@ public static function cannotBeParsed(string $details): self return new self('It was not possible to parse your key, reason:' . $details); } - public static function incompatibleKey(): self + public static function incompatibleKeyType(string $expectedType, string $actualType): self { - return new self('This key is not compatible with this signer'); + return new self( + 'The type of the provided key is not "' . $expectedType + . '", "' . $actualType . '" provided' + ); + } + + public static function incompatibleKeyLength(int $expectedLength, int $actualLength): self + { + return new self( + 'The length of the provided key is different than ' . $expectedLength . ' bits, ' + . $actualLength . ' bits provided' + ); } public static function cannotBeEmpty(): self diff --git a/src/Signer/OpenSSL.php b/src/Signer/OpenSSL.php index 795ee7c0..0af618f7 100644 --- a/src/Signer/OpenSSL.php +++ b/src/Signer/OpenSSL.php @@ -19,10 +19,21 @@ use function openssl_sign; use function openssl_verify; +use const OPENSSL_KEYTYPE_DH; +use const OPENSSL_KEYTYPE_DSA; +use const OPENSSL_KEYTYPE_EC; +use const OPENSSL_KEYTYPE_RSA; use const PHP_EOL; abstract class OpenSSL implements Signer { + protected const KEY_TYPE_MAP = [ + OPENSSL_KEYTYPE_RSA => 'RSA', + OPENSSL_KEYTYPE_DSA => 'DSA', + OPENSSL_KEYTYPE_DH => 'DH', + OPENSSL_KEYTYPE_EC => 'EC', + ]; + /** * @throws CannotSignPayload * @throws InvalidKeyProvided @@ -47,9 +58,6 @@ final protected function createSignature( } } - /** @return positive-int */ - abstract public function minimumBitsLengthForKey(): int; - /** * @return resource|OpenSSLAsymmetricKey * @@ -105,15 +113,12 @@ private function validateKey($key): void $details = openssl_pkey_get_details($key); assert(is_array($details)); - if (! array_key_exists('key', $details) || $details['type'] !== $this->keyType()) { - throw InvalidKeyProvided::incompatibleKey(); - } - assert(array_key_exists('bits', $details)); assert(is_int($details['bits'])); - if ($details['bits'] < $this->minimumBitsLengthForKey()) { - throw InvalidKeyProvided::tooShort($this->minimumBitsLengthForKey(), $details['bits']); - } + assert(array_key_exists('type', $details)); + assert(is_int($details['type'])); + + $this->guardAgainstIncompatibleKey($details['type'], $details['bits']); } private function fullOpenSSLErrorString(): string @@ -127,6 +132,9 @@ private function fullOpenSSLErrorString(): string return $error; } + /** @throws InvalidKeyProvided */ + abstract protected function guardAgainstIncompatibleKey(int $type, int $lengthInBits): void; + /** @param resource|OpenSSLAsymmetricKey $key */ private function freeKey($key): void { @@ -137,13 +145,6 @@ private function freeKey($key): void openssl_free_key($key); // Deprecated and no longer necessary as of PHP >= 8.0 } - /** - * Returns the type of key to be used to create/verify the signature (using OpenSSL constants) - * - * @internal - */ - abstract public function keyType(): int; - /** * Returns which algorithm to be used to create/verify the signature (using OpenSSL constants) * diff --git a/src/Signer/Rsa.php b/src/Signer/Rsa.php index 3af57bd8..ba7d72d5 100644 --- a/src/Signer/Rsa.php +++ b/src/Signer/Rsa.php @@ -7,6 +7,8 @@ abstract class Rsa extends OpenSSL { + private const MINIMUM_KEY_LENGTH = 2048; + final public function sign(string $payload, Key $key): string { return $this->createSignature($key->contents(), $key->passphrase(), $payload); @@ -17,13 +19,17 @@ final public function verify(string $expected, string $payload, Key $key): bool return $this->verifySignature($expected, $payload, $key->contents()); } - final public function keyType(): int + final protected function guardAgainstIncompatibleKey(int $type, int $lengthInBits): void { - return OPENSSL_KEYTYPE_RSA; - } + if ($type !== OPENSSL_KEYTYPE_RSA) { + throw InvalidKeyProvided::incompatibleKeyType( + self::KEY_TYPE_MAP[OPENSSL_KEYTYPE_RSA], + self::KEY_TYPE_MAP[$type], + ); + } - final public function minimumBitsLengthForKey(): int - { - return 2048; + if ($lengthInBits < self::MINIMUM_KEY_LENGTH) { + throw InvalidKeyProvided::tooShort(self::MINIMUM_KEY_LENGTH, $lengthInBits); + } } } diff --git a/src/Signer/UnsafeEcdsa.php b/src/Signer/UnsafeEcdsa.php index c437a3e9..be48c923 100644 --- a/src/Signer/UnsafeEcdsa.php +++ b/src/Signer/UnsafeEcdsa.php @@ -40,14 +40,15 @@ final public function verify(string $expected, string $payload, Key $key): bool ); } - final public function keyType(): int + // phpcs:ignore SlevomatCodingStandard.Functions.UnusedParameter.UnusedParameter + final protected function guardAgainstIncompatibleKey(int $type, int $lengthInBits): void { - return OPENSSL_KEYTYPE_EC; - } - - final public function minimumBitsLengthForKey(): int - { - return 1; + if ($type !== OPENSSL_KEYTYPE_EC) { + throw InvalidKeyProvided::incompatibleKeyType( + self::KEY_TYPE_MAP[OPENSSL_KEYTYPE_EC], + self::KEY_TYPE_MAP[$type], + ); + } } /** diff --git a/src/Signer/UnsafeRsa.php b/src/Signer/UnsafeRsa.php index 0075381f..182ef806 100644 --- a/src/Signer/UnsafeRsa.php +++ b/src/Signer/UnsafeRsa.php @@ -18,13 +18,14 @@ final public function verify(string $expected, string $payload, Key $key): bool return $this->verifySignature($expected, $payload, $key->contents()); } - final public function keyType(): int + // phpcs:ignore SlevomatCodingStandard.Functions.UnusedParameter.UnusedParameter + final protected function guardAgainstIncompatibleKey(int $type, int $lengthInBits): void { - return OPENSSL_KEYTYPE_RSA; - } - - final public function minimumBitsLengthForKey(): int - { - return 1; + if ($type !== OPENSSL_KEYTYPE_RSA) { + throw InvalidKeyProvided::incompatibleKeyType( + self::KEY_TYPE_MAP[OPENSSL_KEYTYPE_RSA], + self::KEY_TYPE_MAP[$type], + ); + } } } diff --git a/test/functional/ES512TokenTest.php b/test/functional/ES512TokenTest.php index 2b6a7004..6105de72 100644 --- a/test/functional/ES512TokenTest.php +++ b/test/functional/ES512TokenTest.php @@ -77,7 +77,7 @@ public function builderShouldRaiseExceptionWhenKeyIsNotEcdsaCompatible(): void $builder = $this->config->builder(); $this->expectException(InvalidKeyProvided::class); - $this->expectExceptionMessage('This key is not compatible with this signer'); + $this->expectExceptionMessage('The type of the provided key is not "EC", "RSA" provided'); $builder->identifiedBy('1') ->permittedFor('http://client.abc.com') @@ -168,7 +168,7 @@ public function signatureAssertionShouldRaiseExceptionWhenAlgorithmIsDifferent(T public function signatureAssertionShouldRaiseExceptionWhenKeyIsNotEcdsaCompatible(Token $token): void { $this->expectException(InvalidKeyProvided::class); - $this->expectExceptionMessage('This key is not compatible with this signer'); + $this->expectExceptionMessage('The type of the provided key is not "EC", "RSA" provided'); $this->config->validator()->assert( $token, diff --git a/test/functional/EcdsaTokenTest.php b/test/functional/EcdsaTokenTest.php index d3fa0695..4f553392 100644 --- a/test/functional/EcdsaTokenTest.php +++ b/test/functional/EcdsaTokenTest.php @@ -81,7 +81,7 @@ public function builderShouldRaiseExceptionWhenKeyIsNotEcdsaCompatible(): void $builder = $this->config->builder(); $this->expectException(InvalidKeyProvided::class); - $this->expectExceptionMessage('This key is not compatible with this signer'); + $this->expectExceptionMessage('The type of the provided key is not "EC", "RSA" provided'); $builder->identifiedBy('1') ->permittedFor('http://client.abc.com') @@ -172,7 +172,7 @@ public function signatureAssertionShouldRaiseExceptionWhenAlgorithmIsDifferent(T public function signatureAssertionShouldRaiseExceptionWhenKeyIsNotEcdsaCompatible(Token $token): void { $this->expectException(InvalidKeyProvided::class); - $this->expectExceptionMessage('This key is not compatible with this signer'); + $this->expectExceptionMessage('The type of the provided key is not "EC", "RSA" provided'); $this->config->validator()->assert( $token, diff --git a/test/functional/RsaTokenTest.php b/test/functional/RsaTokenTest.php index ec0c709f..969e9748 100644 --- a/test/functional/RsaTokenTest.php +++ b/test/functional/RsaTokenTest.php @@ -76,7 +76,7 @@ public function builderShouldRaiseExceptionWhenKeyIsNotRsaCompatible(): void $builder = $this->config->builder(); $this->expectException(InvalidKeyProvided::class); - $this->expectExceptionMessage('This key is not compatible with this signer'); + $this->expectExceptionMessage('The type of the provided key is not "RSA", "EC" provided'); $builder->identifiedBy('1') ->permittedFor('http://client.abc.com') @@ -156,7 +156,7 @@ public function signatureAssertionShouldRaiseExceptionWhenAlgorithmIsDifferent(T public function signatureAssertionShouldRaiseExceptionWhenKeyIsNotRsaCompatible(Token $token): void { $this->expectException(InvalidKeyProvided::class); - $this->expectExceptionMessage('This key is not compatible with this signer'); + $this->expectExceptionMessage('The type of the provided key is not "RSA", "EC" provided'); $this->config->validator()->assert( $token, diff --git a/test/unit/Signer/Ecdsa/Sha256Test.php b/test/unit/Signer/Ecdsa/Sha256Test.php index 49bded01..d8e33425 100644 --- a/test/unit/Signer/Ecdsa/Sha256Test.php +++ b/test/unit/Signer/Ecdsa/Sha256Test.php @@ -64,13 +64,13 @@ public function keyLengthMustBeCorrect(): void /** * @test * - * @covers ::minimumBitsLengthForKey + * @covers ::expectedKeyLength * * @uses \Lcobucci\JWT\Signer\Ecdsa::__construct */ - public function minimumBitsLengthForKeyMustBeCorrect(): void + public function expectedKeyLengthMustBeCorrect(): void { - self::assertSame(224, $this->getSigner()->minimumBitsLengthForKey()); + self::assertSame(256, $this->getSigner()->expectedKeyLength()); } private function getSigner(): Sha256 diff --git a/test/unit/Signer/Ecdsa/Sha384Test.php b/test/unit/Signer/Ecdsa/Sha384Test.php index e3c9758f..f2e355f6 100644 --- a/test/unit/Signer/Ecdsa/Sha384Test.php +++ b/test/unit/Signer/Ecdsa/Sha384Test.php @@ -64,13 +64,13 @@ public function keyLengthMustBeCorrect(): void /** * @test * - * @covers ::minimumBitsLengthForKey + * @covers ::expectedKeyLength * * @uses \Lcobucci\JWT\Signer\Ecdsa::__construct */ - public function minimumBitsLengthForKeyMustBeCorrect(): void + public function expectedKeyLengthMustBeCorrect(): void { - self::assertSame(224, $this->getSigner()->minimumBitsLengthForKey()); + self::assertSame(384, $this->getSigner()->expectedKeyLength()); } private function getSigner(): Sha384 diff --git a/test/unit/Signer/Ecdsa/Sha512Test.php b/test/unit/Signer/Ecdsa/Sha512Test.php index 35281d8a..484bc5fe 100644 --- a/test/unit/Signer/Ecdsa/Sha512Test.php +++ b/test/unit/Signer/Ecdsa/Sha512Test.php @@ -64,13 +64,13 @@ public function keyLengthMustBeCorrect(): void /** * @test * - * @covers ::minimumBitsLengthForKey + * @covers ::expectedKeyLength * * @uses \Lcobucci\JWT\Signer\Ecdsa::__construct */ - public function minimumBitsLengthForKeyMustBeCorrect(): void + public function expectedKeyLengthMustBeCorrect(): void { - self::assertSame(224, $this->getSigner()->minimumBitsLengthForKey()); + self::assertSame(521, $this->getSigner()->expectedKeyLength()); } private function getSigner(): Sha512 diff --git a/test/unit/Signer/Ecdsa/UnsafeSha256Test.php b/test/unit/Signer/Ecdsa/UnsafeSha256Test.php index 8d216520..6e1725e8 100644 --- a/test/unit/Signer/Ecdsa/UnsafeSha256Test.php +++ b/test/unit/Signer/Ecdsa/UnsafeSha256Test.php @@ -61,18 +61,6 @@ public function keyLengthMustBeCorrect(): void self::assertSame(64, $this->getSigner()->pointLength()); } - /** - * @test - * - * @covers ::minimumBitsLengthForKey - * - * @uses \Lcobucci\JWT\Signer\UnsafeEcdsa::__construct - */ - public function minimumBitsLengthForKeyMustBeCorrect(): void - { - self::assertSame(1, $this->getSigner()->minimumBitsLengthForKey()); - } - private function getSigner(): UnsafeSha256 { return new UnsafeSha256($this->createMock(SignatureConverter::class)); diff --git a/test/unit/Signer/Ecdsa/UnsafeSha384Test.php b/test/unit/Signer/Ecdsa/UnsafeSha384Test.php index 01d33cbe..d6875b00 100644 --- a/test/unit/Signer/Ecdsa/UnsafeSha384Test.php +++ b/test/unit/Signer/Ecdsa/UnsafeSha384Test.php @@ -61,18 +61,6 @@ public function keyLengthMustBeCorrect(): void self::assertSame(96, $this->getSigner()->pointLength()); } - /** - * @test - * - * @covers ::minimumBitsLengthForKey - * - * @uses \Lcobucci\JWT\Signer\UnsafeEcdsa::__construct - */ - public function minimumBitsLengthForKeyMustBeCorrect(): void - { - self::assertSame(1, $this->getSigner()->minimumBitsLengthForKey()); - } - private function getSigner(): UnsafeSha384 { return new UnsafeSha384($this->createMock(SignatureConverter::class)); diff --git a/test/unit/Signer/Ecdsa/UnsafeSha512Test.php b/test/unit/Signer/Ecdsa/UnsafeSha512Test.php index cbe139b4..da3a0b74 100644 --- a/test/unit/Signer/Ecdsa/UnsafeSha512Test.php +++ b/test/unit/Signer/Ecdsa/UnsafeSha512Test.php @@ -61,18 +61,6 @@ public function keyLengthMustBeCorrect(): void self::assertSame(132, $this->getSigner()->pointLength()); } - /** - * @test - * - * @covers ::minimumBitsLengthForKey - * - * @uses \Lcobucci\JWT\Signer\UnsafeEcdsa::__construct - */ - public function minimumBitsLengthForKeyMustBeCorrect(): void - { - self::assertSame(1, $this->getSigner()->minimumBitsLengthForKey()); - } - private function getSigner(): UnsafeSha512 { return new UnsafeSha512($this->createMock(SignatureConverter::class)); diff --git a/test/unit/Signer/EcdsaTest.php b/test/unit/Signer/EcdsaTest.php index 5725352a..911fdc13 100644 --- a/test/unit/Signer/EcdsaTest.php +++ b/test/unit/Signer/EcdsaTest.php @@ -52,6 +52,9 @@ private function getSigner(): Ecdsa $signer->method('pointLength') ->willReturn(64); + $signer->method('expectedKeyLength') + ->willReturn(256); + return $signer; } @@ -59,8 +62,7 @@ private function getSigner(): Ecdsa * @test * * @covers ::sign - * @covers ::keyType - * @covers \Lcobucci\JWT\Signer\Ecdsa::minimumBitsLengthForKey + * @covers ::guardAgainstIncompatibleKey * @covers \Lcobucci\JWT\Signer\Ecdsa\MultibyteStringConverter * @covers \Lcobucci\JWT\Signer\OpenSSL * @@ -91,37 +93,54 @@ public function signShouldReturnTheAHashBasedOnTheOpenSslSignature(): void /** * @test * - * @requires extension openssl < 3.0 + * @covers ::sign + * @covers ::guardAgainstIncompatibleKey + * @covers \Lcobucci\JWT\Signer\OpenSSL + * @covers \Lcobucci\JWT\Signer\InvalidKeyProvided + * + * @uses \Lcobucci\JWT\Signer\Ecdsa::__construct + * @uses \Lcobucci\JWT\Signer\Key\InMemory + */ + public function signShouldRaiseAnExceptionWhenKeyLengthIsNotTheExpectedOne(): void + { + $signer = $this->getSigner(); + + $this->expectException(InvalidKeyProvided::class); + $this->expectExceptionMessage('The length of the provided key is different than 256 bits, 521 bits provided'); + + $signer->sign('testing', self::$ecdsaKeys['private_ec512']); + } + + /** + * @test * * @covers ::sign - * @covers ::keyType - * @covers \Lcobucci\JWT\Signer\Ecdsa::minimumBitsLengthForKey + * @covers ::guardAgainstIncompatibleKey * @covers \Lcobucci\JWT\Signer\OpenSSL * @covers \Lcobucci\JWT\Signer\InvalidKeyProvided * * @uses \Lcobucci\JWT\Signer\Ecdsa::__construct * @uses \Lcobucci\JWT\Signer\Key\InMemory */ - public function signShouldRaiseAnExceptionWhenKeyLengthIsBelowMinimum(): void + public function signShouldRaiseAnExceptionWhenKeyTypeIsNotEC(): void { $signer = $this->getSigner(); $this->expectException(InvalidKeyProvided::class); - $this->expectExceptionMessage('Key provided is shorter than 224 bits, only 161 bits provided'); + $this->expectExceptionMessage('The type of the provided key is not "EC", "RSA" provided'); - $signer->sign('testing', self::$ecdsaKeys['private_short']); + $signer->sign('testing', self::$rsaKeys['private']); } /** * @test * * @covers ::verify - * @covers ::keyType + * @covers ::guardAgainstIncompatibleKey * @covers \Lcobucci\JWT\Signer\Ecdsa\MultibyteStringConverter * @covers \Lcobucci\JWT\Signer\OpenSSL * * @uses \Lcobucci\JWT\Signer\Ecdsa::__construct - * @uses \Lcobucci\JWT\Signer\Ecdsa::minimumBitsLengthForKey * @uses \Lcobucci\JWT\Signer\Key\InMemory */ public function verifyShouldDelegateToEcdsaSignerUsingPublicKey(): void diff --git a/test/unit/Signer/Rsa/Sha256Test.php b/test/unit/Signer/Rsa/Sha256Test.php index b84774f8..78911ad4 100644 --- a/test/unit/Signer/Rsa/Sha256Test.php +++ b/test/unit/Signer/Rsa/Sha256Test.php @@ -33,16 +33,4 @@ public function algorithmMustBeCorrect(): void self::assertEquals(OPENSSL_ALGO_SHA256, $signer->algorithm()); } - - /** - * @test - * - * @covers ::minimumBitsLengthForKey - */ - public function minimumBitsLengthForKeyMustBeCorrect(): void - { - $signer = new Sha256(); - - self::assertSame(2048, $signer->minimumBitsLengthForKey()); - } } diff --git a/test/unit/Signer/Rsa/Sha384Test.php b/test/unit/Signer/Rsa/Sha384Test.php index ba4a292a..41ea82be 100644 --- a/test/unit/Signer/Rsa/Sha384Test.php +++ b/test/unit/Signer/Rsa/Sha384Test.php @@ -33,16 +33,4 @@ public function algorithmMustBeCorrect(): void self::assertEquals(OPENSSL_ALGO_SHA384, $signer->algorithm()); } - - /** - * @test - * - * @covers ::minimumBitsLengthForKey - */ - public function minimumBitsLengthForKeyMustBeCorrect(): void - { - $signer = new Sha384(); - - self::assertSame(2048, $signer->minimumBitsLengthForKey()); - } } diff --git a/test/unit/Signer/Rsa/Sha512Test.php b/test/unit/Signer/Rsa/Sha512Test.php index d26530a5..76594576 100644 --- a/test/unit/Signer/Rsa/Sha512Test.php +++ b/test/unit/Signer/Rsa/Sha512Test.php @@ -33,16 +33,4 @@ public function algorithmMustBeCorrect(): void self::assertEquals(OPENSSL_ALGO_SHA512, $signer->algorithm()); } - - /** - * @test - * - * @covers ::minimumBitsLengthForKey - */ - public function minimumBitsLengthForKeyMustBeCorrect(): void - { - $signer = new Sha512(); - - self::assertSame(2048, $signer->minimumBitsLengthForKey()); - } } diff --git a/test/unit/Signer/Rsa/UnsafeSha256Test.php b/test/unit/Signer/Rsa/UnsafeSha256Test.php index 71320292..fcc26b53 100644 --- a/test/unit/Signer/Rsa/UnsafeSha256Test.php +++ b/test/unit/Signer/Rsa/UnsafeSha256Test.php @@ -33,16 +33,4 @@ public function algorithmMustBeCorrect(): void self::assertEquals(OPENSSL_ALGO_SHA256, $signer->algorithm()); } - - /** - * @test - * - * @covers ::minimumBitsLengthForKey - */ - public function minimumBitsLengthForKeyMustBeCorrect(): void - { - $signer = new UnsafeSha256(); - - self::assertSame(1, $signer->minimumBitsLengthForKey()); - } } diff --git a/test/unit/Signer/Rsa/UnsafeSha384Test.php b/test/unit/Signer/Rsa/UnsafeSha384Test.php index 6e3f0e55..4dd5f21f 100644 --- a/test/unit/Signer/Rsa/UnsafeSha384Test.php +++ b/test/unit/Signer/Rsa/UnsafeSha384Test.php @@ -33,16 +33,4 @@ public function algorithmMustBeCorrect(): void self::assertEquals(OPENSSL_ALGO_SHA384, $signer->algorithm()); } - - /** - * @test - * - * @covers ::minimumBitsLengthForKey - */ - public function minimumBitsLengthForKeyMustBeCorrect(): void - { - $signer = new UnsafeSha384(); - - self::assertSame(1, $signer->minimumBitsLengthForKey()); - } } diff --git a/test/unit/Signer/Rsa/UnsafeSha512Test.php b/test/unit/Signer/Rsa/UnsafeSha512Test.php index f80ca802..98b7a640 100644 --- a/test/unit/Signer/Rsa/UnsafeSha512Test.php +++ b/test/unit/Signer/Rsa/UnsafeSha512Test.php @@ -33,16 +33,4 @@ public function algorithmMustBeCorrect(): void self::assertEquals(OPENSSL_ALGO_SHA512, $signer->algorithm()); } - - /** - * @test - * - * @covers ::minimumBitsLengthForKey - */ - public function minimumBitsLengthForKeyMustBeCorrect(): void - { - $signer = new UnsafeSha512(); - - self::assertSame(1, $signer->minimumBitsLengthForKey()); - } } diff --git a/test/unit/Signer/RsaTest.php b/test/unit/Signer/RsaTest.php index a11c29d3..f440e91b 100644 --- a/test/unit/Signer/RsaTest.php +++ b/test/unit/Signer/RsaTest.php @@ -36,10 +36,9 @@ public function clearOpenSSLErrors(): void * @test * * @covers ::sign - * @covers ::keyType + * @covers ::guardAgainstIncompatibleKey * @covers \Lcobucci\JWT\Signer\OpenSSL * - * @uses \Lcobucci\JWT\Signer\Rsa::minimumBitsLengthForKey * @uses \Lcobucci\JWT\Signer\Key\InMemory */ public function signShouldReturnAValidOpensslSignature(): void @@ -100,7 +99,7 @@ public function allOpenSSLErrorsShouldBeOnTheErrorMessage(): void * @test * * @covers ::sign - * @covers ::keyType + * @covers ::guardAgainstIncompatibleKey * @covers \Lcobucci\JWT\Signer\OpenSSL * @covers \Lcobucci\JWT\Signer\InvalidKeyProvided * @@ -111,7 +110,7 @@ public function signShouldRaiseAnExceptionWhenKeyTypeIsNotRsa(): void $signer = $this->getSigner(); $this->expectException(InvalidKeyProvided::class); - $this->expectExceptionMessage('This key is not compatible with this signer'); + $this->expectExceptionMessage('The type of the provided key is not "RSA", "EC" provided'); $signer->sign('testing', self::$ecdsaKeys['private']); } @@ -120,8 +119,7 @@ public function signShouldRaiseAnExceptionWhenKeyTypeIsNotRsa(): void * @test * * @covers ::sign - * @covers ::keyType - * @covers \Lcobucci\JWT\Signer\Rsa::minimumBitsLengthForKey + * @covers ::guardAgainstIncompatibleKey * @covers \Lcobucci\JWT\Signer\OpenSSL * @covers \Lcobucci\JWT\Signer\InvalidKeyProvided * @@ -141,10 +139,9 @@ public function signShouldRaiseAnExceptionWhenKeyLengthIsBelowMinimum(): void * @test * * @covers ::verify - * @covers ::keyType + * @covers ::guardAgainstIncompatibleKey * @covers \Lcobucci\JWT\Signer\OpenSSL * - * @uses \Lcobucci\JWT\Signer\Rsa::minimumBitsLengthForKey * @uses \Lcobucci\JWT\Signer\Key\InMemory */ public function verifyShouldReturnTrueWhenSignatureIsValid(): void diff --git a/test/unit/Signer/UnsafeEcdsaTest.php b/test/unit/Signer/UnsafeEcdsaTest.php index 1fe8687b..a87b991b 100644 --- a/test/unit/Signer/UnsafeEcdsaTest.php +++ b/test/unit/Signer/UnsafeEcdsaTest.php @@ -59,8 +59,7 @@ private function getSigner(): UnsafeEcdsa * @test * * @covers ::sign - * @covers ::keyType - * @covers \Lcobucci\JWT\Signer\UnsafeEcdsa::minimumBitsLengthForKey + * @covers ::guardAgainstIncompatibleKey * @covers \Lcobucci\JWT\Signer\Ecdsa\MultibyteStringConverter * @covers \Lcobucci\JWT\Signer\OpenSSL * @@ -91,39 +90,33 @@ public function signShouldReturnTheAHashBasedOnTheOpenSslSignature(): void /** * @test * - * @requires extension openssl < 3.0 - * * @covers ::sign - * @covers ::keyType - * @covers \Lcobucci\JWT\Signer\UnsafeEcdsa::minimumBitsLengthForKey + * @covers ::guardAgainstIncompatibleKey * @covers \Lcobucci\JWT\Signer\OpenSSL * @covers \Lcobucci\JWT\Signer\InvalidKeyProvided * - * @uses \Lcobucci\JWT\Signer\Ecdsa\MultibyteStringConverter * @uses \Lcobucci\JWT\Signer\UnsafeEcdsa::__construct - * @uses \Lcobucci\JWT\Signer\UnsafeEcdsa::verify * @uses \Lcobucci\JWT\Signer\Key\InMemory */ - public function signShouldAcceptAKeyLengthBelowMinimum(): void + public function signShouldRaiseAnExceptionWhenKeyTypeIsNotEC(): void { $signer = $this->getSigner(); - $payload = 'testing'; - $signature = $signer->sign($payload, self::$ecdsaKeys['private_short']); + $this->expectException(InvalidKeyProvided::class); + $this->expectExceptionMessage('The type of the provided key is not "EC", "RSA" provided'); - self::assertTrue($signer->verify($signature, $payload, self::$ecdsaKeys['public_short'])); + $signer->sign('testing', self::$rsaKeys['private']); } /** * @test * * @covers ::verify - * @covers ::keyType + * @covers ::guardAgainstIncompatibleKey * @covers \Lcobucci\JWT\Signer\Ecdsa\MultibyteStringConverter * @covers \Lcobucci\JWT\Signer\OpenSSL * * @uses \Lcobucci\JWT\Signer\UnsafeEcdsa::__construct - * @uses \Lcobucci\JWT\Signer\UnsafeEcdsa::minimumBitsLengthForKey * @uses \Lcobucci\JWT\Signer\Key\InMemory */ public function verifyShouldDelegateToEcdsaSignerUsingPublicKey(): void diff --git a/test/unit/Signer/UnsafeRsaTest.php b/test/unit/Signer/UnsafeRsaTest.php index 75ffb498..ef4203c0 100644 --- a/test/unit/Signer/UnsafeRsaTest.php +++ b/test/unit/Signer/UnsafeRsaTest.php @@ -36,10 +36,9 @@ public function clearOpenSSLErrors(): void * @test * * @covers ::sign - * @covers ::keyType * @covers \Lcobucci\JWT\Signer\OpenSSL * - * @uses \Lcobucci\JWT\Signer\UnsafeRsa::minimumBitsLengthForKey + * @uses \Lcobucci\JWT\Signer\UnsafeRsa::guardAgainstIncompatibleKey * @uses \Lcobucci\JWT\Signer\Key\InMemory */ public function signShouldReturnAValidOpensslSignature(): void @@ -59,11 +58,10 @@ public function signShouldReturnAValidOpensslSignature(): void * @test * * @covers ::sign - * @covers ::keyType + * @covers ::guardAgainstIncompatibleKey * @covers \Lcobucci\JWT\Signer\OpenSSL * @covers \Lcobucci\JWT\Signer\CannotSignPayload * - * @uses \Lcobucci\JWT\Signer\UnsafeRsa::minimumBitsLengthForKey * @uses \Lcobucci\JWT\Signer\Key\InMemory */ public function signShouldRaiseAnExceptionWhenKeyIsInvalid(): void @@ -107,7 +105,7 @@ public function signShouldRaiseAnExceptionWhenKeyIsNotParseable(): void * @test * * @covers ::sign - * @covers ::keyType + * @covers ::guardAgainstIncompatibleKey * @covers \Lcobucci\JWT\Signer\OpenSSL * @covers \Lcobucci\JWT\Signer\InvalidKeyProvided * @@ -118,7 +116,7 @@ public function signShouldRaiseAnExceptionWhenKeyTypeIsNotRsa(): void $signer = $this->getSigner(); $this->expectException(InvalidKeyProvided::class); - $this->expectExceptionMessage('This key is not compatible with this signer'); + $this->expectExceptionMessage('The type of the provided key is not "RSA", "EC" provided'); $signer->sign('testing', self::$ecdsaKeys['private']); } @@ -127,13 +125,12 @@ public function signShouldRaiseAnExceptionWhenKeyTypeIsNotRsa(): void * @test * * @covers ::sign - * @covers ::keyType - * @covers \Lcobucci\JWT\Signer\UnsafeRsa::minimumBitsLengthForKey * @covers \Lcobucci\JWT\Signer\OpenSSL * @covers \Lcobucci\JWT\Signer\InvalidKeyProvided * * @uses \Lcobucci\JWT\Signer\Key\InMemory * @uses \Lcobucci\JWT\Signer\UnsafeRsa::verify + * @uses \Lcobucci\JWT\Signer\UnsafeRsa::guardAgainstIncompatibleKey */ public function signShouldAcceptAKeyLengthBelowMinimum(): void { @@ -149,11 +146,10 @@ public function signShouldAcceptAKeyLengthBelowMinimum(): void * @test * * @covers ::verify - * @covers ::keyType * @covers \Lcobucci\JWT\Signer\OpenSSL * - * @uses \Lcobucci\JWT\Signer\UnsafeRsa::minimumBitsLengthForKey * @uses \Lcobucci\JWT\Signer\Key\InMemory + * @uses \Lcobucci\JWT\Signer\UnsafeRsa::guardAgainstIncompatibleKey */ public function verifyShouldReturnTrueWhenSignatureIsValid(): void { @@ -192,6 +188,7 @@ public function verifyShouldRaiseAnExceptionWhenKeyIsNotParseable(): void * @test * * @covers ::verify + * @covers ::guardAgainstIncompatibleKey * @covers \Lcobucci\JWT\Signer\OpenSSL * @covers \Lcobucci\JWT\Signer\InvalidKeyProvided * @@ -202,9 +199,9 @@ public function verifyShouldRaiseAnExceptionWhenKeyTypeIsNotRsa(): void $signer = $this->getSigner(); $this->expectException(InvalidKeyProvided::class); - $this->expectExceptionMessage('It was not possible to parse your key'); + $this->expectExceptionMessage('The type of the provided key is not "RSA", "EC" provided'); - $signer->verify('testing', 'testing', self::$ecdsaKeys['private']); + $signer->verify('testing', 'testing', self::$ecdsaKeys['public1']); } private function getSigner(): UnsafeRsa From 367b7f4d66f1dcac4490d5c2a0949cf656e65b62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lu=C3=ADs=20Cobucci?= Date: Wed, 17 Aug 2022 23:39:26 +0200 Subject: [PATCH 5/5] Remove unused keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Luís Cobucci --- test/_keys/Keys.php | 2 -- test/_keys/ecdsa/private_ec160.key | 4 ---- test/_keys/ecdsa/public_ec160.key | 4 ---- 3 files changed, 10 deletions(-) delete mode 100644 test/_keys/ecdsa/private_ec160.key delete mode 100644 test/_keys/ecdsa/public_ec160.key diff --git a/test/_keys/Keys.php b/test/_keys/Keys.php index 15b1bf8a..cca8c19c 100644 --- a/test/_keys/Keys.php +++ b/test/_keys/Keys.php @@ -41,8 +41,6 @@ public static function createEcdsaKeys(): void 'private_ec512' => Key\InMemory::file(__DIR__ . '/ecdsa/private_ec512.key'), 'public_ec512' => Key\InMemory::file(__DIR__ . '/ecdsa/public_ec512.key'), 'public2_ec512' => Key\InMemory::file(__DIR__ . '/ecdsa/public2_ec512.key'), - 'private_short' => Key\InMemory::file(__DIR__ . '/ecdsa/private_ec160.key'), - 'public_short' => Key\InMemory::file(__DIR__ . '/ecdsa/public_ec160.key'), ]; } diff --git a/test/_keys/ecdsa/private_ec160.key b/test/_keys/ecdsa/private_ec160.key deleted file mode 100644 index b60b08ed..00000000 --- a/test/_keys/ecdsa/private_ec160.key +++ /dev/null @@ -1,4 +0,0 @@ ------BEGIN EC PRIVATE KEY----- -MFECAQEEFQBpEH+FRU6SRYbQP0Z6ewKcRaTB7qAHBgUrgQQAHqEsAyoABGBELmbR -K6CARY03gdIWUIQMoWRVKSrdbFjIb/ewb9m7aD1hWTKqUE0= ------END EC PRIVATE KEY----- diff --git a/test/_keys/ecdsa/public_ec160.key b/test/_keys/ecdsa/public_ec160.key deleted file mode 100644 index 34184052..00000000 --- a/test/_keys/ecdsa/public_ec160.key +++ /dev/null @@ -1,4 +0,0 @@ ------BEGIN PUBLIC KEY----- -MD4wEAYHKoZIzj0CAQYFK4EEAB4DKgAEYEQuZtEroIBFjTeB0hZQhAyhZFUpKt1s -WMhv97Bv2btoPWFZMqpQTQ== ------END PUBLIC KEY-----