From 58b57b45b3432d9b8f991d85301e09fc69c787d0 Mon Sep 17 00:00:00 2001 From: Filippo Tessarotto Date: Wed, 12 Oct 2022 14:35:32 +0200 Subject: [PATCH] Expand `non-empty-string` tightening --- src/Signer.php | 3 +++ src/Signer/Ecdsa.php | 4 ---- src/Signer/Key.php | 1 + src/Signer/Key/InMemory.php | 2 ++ src/Token/InvalidTokenStructure.php | 5 +++++ src/Token/Parser.php | 5 +++-- src/Token/Signature.php | 7 +++++++ test/functional/RFC6978VectorTest.php | 5 ++--- test/performance/SignerBench.php | 1 + test/unit/Signer/Blake2bTest.php | 1 + test/unit/Signer/EddsaTest.php | 1 + test/unit/Signer/HmacTest.php | 6 ++++++ test/unit/Token/ParserTest.php | 18 ++++++++++++++++++ 13 files changed, 50 insertions(+), 9 deletions(-) diff --git a/src/Signer.php b/src/Signer.php index 7e787d2ee..a8efe2963 100644 --- a/src/Signer.php +++ b/src/Signer.php @@ -22,6 +22,8 @@ public function algorithmId(): string; * * @param non-empty-string $payload * + * @return non-empty-string + * * @throws CannotSignPayload When payload signing fails. * @throws InvalidKeyProvided When issue key is invalid/incompatible. * @throws ConversionFailed When signature could not be converted. @@ -31,6 +33,7 @@ public function sign(string $payload, Key $key): string; /** * Returns if the expected hash matches with the data and key * + * @param non-empty-string $expected * @param non-empty-string $payload * * @throws InvalidKeyProvided When issue key is invalid/incompatible. diff --git a/src/Signer/Ecdsa.php b/src/Signer/Ecdsa.php index 793cb83db..6af40f092 100644 --- a/src/Signer/Ecdsa.php +++ b/src/Signer/Ecdsa.php @@ -6,8 +6,6 @@ use Lcobucci\JWT\Signer\Ecdsa\MultibyteStringConverter; use Lcobucci\JWT\Signer\Ecdsa\SignatureConverter; -use function assert; - use const OPENSSL_KEYTYPE_EC; abstract class Ecdsa extends OpenSSL @@ -32,8 +30,6 @@ final public function sign(string $payload, Key $key): string final public function verify(string $expected, string $payload, Key $key): bool { - assert($expected !== ''); - return $this->verifySignature( $this->converter->toAsn1($expected, $this->pointLength()), $payload, diff --git a/src/Signer/Key.php b/src/Signer/Key.php index ea6b725ab..c19a0ec82 100644 --- a/src/Signer/Key.php +++ b/src/Signer/Key.php @@ -5,6 +5,7 @@ interface Key { + /** @return non-empty-string */ public function contents(): string; public function passphrase(): string; diff --git a/src/Signer/Key/InMemory.php b/src/Signer/Key/InMemory.php index 5b2cce2d6..c84a7678c 100644 --- a/src/Signer/Key/InMemory.php +++ b/src/Signer/Key/InMemory.php @@ -14,6 +14,7 @@ final class InMemory implements Key { + /** @param non-empty-string $contents */ private function __construct(public readonly string $contents, public readonly string $passphrase) { } @@ -53,6 +54,7 @@ public static function file(string $path, string $passphrase = ''): self assert(is_string($contents)); self::guardAgainstEmptyKey($contents); + assert($contents !== ''); return new self($contents, $passphrase); } diff --git a/src/Token/InvalidTokenStructure.php b/src/Token/InvalidTokenStructure.php index 7abb6c3a2..c62c6e016 100644 --- a/src/Token/InvalidTokenStructure.php +++ b/src/Token/InvalidTokenStructure.php @@ -23,6 +23,11 @@ public static function missingClaimsPart(): self return new self('The JWT string is missing the Claim part'); } + public static function missingSignaturePart(): self + { + return new self('The JWT string is missing the Signature part'); + } + public static function arrayExpected(string $part): self { return new self($part . ' must be an array'); diff --git a/src/Token/Parser.php b/src/Token/Parser.php index 1565a8336..b2bab5a35 100644 --- a/src/Token/Parser.php +++ b/src/Token/Parser.php @@ -9,7 +9,6 @@ use Lcobucci\JWT\Token as TokenInterface; use function array_key_exists; -use function assert; use function count; use function explode; use function is_array; @@ -36,7 +35,9 @@ public function parse(string $jwt): TokenInterface throw InvalidTokenStructure::missingClaimsPart(); } - assert($encodedSignature !== ''); + if ($encodedSignature === '') { + throw InvalidTokenStructure::missingSignaturePart(); + } $header = $this->parseHeader($encodedHeaders); diff --git a/src/Token/Signature.php b/src/Token/Signature.php index ec491d292..5deca086d 100644 --- a/src/Token/Signature.php +++ b/src/Token/Signature.php @@ -5,10 +5,15 @@ final class Signature { + /** + * @param non-empty-string $hash + * @param non-empty-string $encoded + */ public function __construct(private readonly string $hash, private readonly string $encoded) { } + /** @return non-empty-string */ public function hash(): string { return $this->hash; @@ -16,6 +21,8 @@ public function hash(): string /** * Returns the encoded version of the signature + * + * @return non-empty-string */ public function toString(): string { diff --git a/test/functional/RFC6978VectorTest.php b/test/functional/RFC6978VectorTest.php index 0bc218338..1ffde1a92 100644 --- a/test/functional/RFC6978VectorTest.php +++ b/test/functional/RFC6978VectorTest.php @@ -11,9 +11,7 @@ use Lcobucci\JWT\Signer\Key\InMemory; use PHPUnit\Framework\TestCase; -use function assert; use function hex2bin; -use function is_string; use const PHP_EOL; @@ -45,7 +43,8 @@ public function theVectorsFromRFC6978CanBeVerified( string $expectedS, ): void { $signature = hex2bin($expectedR . $expectedS); - assert(is_string($signature)); + self::assertIsString($signature); + self::assertNotSame('', $signature); static::assertTrue($signer->verify($signature, $payload, $key)); } diff --git a/test/performance/SignerBench.php b/test/performance/SignerBench.php index de71fdee9..6b261f17e 100644 --- a/test/performance/SignerBench.php +++ b/test/performance/SignerBench.php @@ -22,6 +22,7 @@ abstract class SignerBench private Signer $signer; private Key $signingKey; private Key $verificationKey; + /** @var non-empty-string */ private string $signature; final public function init(): void diff --git a/test/unit/Signer/Blake2bTest.php b/test/unit/Signer/Blake2bTest.php index 1607882f6..bf7e766f9 100644 --- a/test/unit/Signer/Blake2bTest.php +++ b/test/unit/Signer/Blake2bTest.php @@ -26,6 +26,7 @@ final class Blake2bTest extends TestCase private InMemory $keyOne; private InMemory $keyTwo; + /** @var non-empty-string */ private string $expectedHashWithKeyOne; /** @before */ diff --git a/test/unit/Signer/EddsaTest.php b/test/unit/Signer/EddsaTest.php index 1ec9ad173..77c04eaf3 100644 --- a/test/unit/Signer/EddsaTest.php +++ b/test/unit/Signer/EddsaTest.php @@ -74,6 +74,7 @@ public function verifyShouldReturnTrueWhenSignatureIsValid(): void { $payload = 'testing'; $signature = sodium_crypto_sign_detached($payload, self::$eddsaKeys['private']->contents()); + self::assertNotSame('', $signature); $signer = $this->getSigner(); diff --git a/test/unit/Signer/HmacTest.php b/test/unit/Signer/HmacTest.php index ec0156bfc..7e65a1245 100644 --- a/test/unit/Signer/HmacTest.php +++ b/test/unit/Signer/HmacTest.php @@ -39,6 +39,8 @@ public function initializeDependencies(): void * @covers ::sign * * @uses \Lcobucci\JWT\Signer\Key\InMemory + * + * @return non-empty-string */ public function signMustReturnAHashAccordingWithTheAlgorithm(): string { @@ -57,6 +59,8 @@ public function signMustReturnAHashAccordingWithTheAlgorithm(): string * * @uses \Lcobucci\JWT\Signer\Hmac::sign * @uses \Lcobucci\JWT\Signer\Key\InMemory + * + * @param non-empty-string $expected */ public function verifyShouldReturnTrueWhenExpectedHashWasCreatedWithSameInformation(string $expected): void { @@ -71,6 +75,8 @@ public function verifyShouldReturnTrueWhenExpectedHashWasCreatedWithSameInformat * * @uses \Lcobucci\JWT\Signer\Hmac::sign * @uses \Lcobucci\JWT\Signer\Key\InMemory + * + * @param non-empty-string $expected */ public function verifyShouldReturnFalseWhenExpectedHashWasNotCreatedWithSameInformation(string $expected): void { diff --git a/test/unit/Token/ParserTest.php b/test/unit/Token/ParserTest.php index 5b9c303c7..bfc195a17 100644 --- a/test/unit/Token/ParserTest.php +++ b/test/unit/Token/ParserTest.php @@ -80,6 +80,24 @@ public function parseMustRaiseExceptionWhenTokenDoesNotHaveClaims(): void $parser->parse('a..c'); } + /** + * @test + * + * @covers ::__construct + * @covers ::parse + * @covers ::splitJwt + * @covers \Lcobucci\JWT\Token\InvalidTokenStructure + */ + public function parseMustRaiseExceptionWhenTokenDoesNotHaveSignature(): void + { + $parser = $this->createParser(); + + $this->expectException(InvalidTokenStructure::class); + $this->expectExceptionMessage('The JWT string is missing the Signature part'); + + $parser->parse('a.b.'); + } + /** * @test *