diff --git a/.travis.yml b/.travis.yml index 11cfa948..40b3cb59 100644 --- a/.travis.yml +++ b/.travis.yml @@ -52,7 +52,7 @@ jobs: - mv ~/.phpenv/versions/$(phpenv version-name)/etc/conf.d/xdebug.ini{.disabled,} - if [[ ! $(php -m | grep -si xdebug) ]]; then echo "xdebug required for coverage"; exit 1; fi script: - - ./vendor/bin/infection --test-framework-options="--testsuite=unit" -s --threads=$(nproc) --min-msi=97 --min-covered-msi=98 + - ./vendor/bin/infection --test-framework-options="--testsuite=unit" -s --threads=$(nproc) --min-msi=95 --min-covered-msi=95 - stage: Metrics and quality env: STATIC_ANALYSIS diff --git a/composer.json b/composer.json index 8f858410..222a7a4c 100644 --- a/composer.json +++ b/composer.json @@ -18,10 +18,8 @@ ], "require": { "php": "^7.2", - "ext-gmp": "*", "ext-mbstring": "*", "ext-openssl": "*", - "fgrosse/phpasn1": "^2.0", "lcobucci/clock": "^1.0", "lcobucci/jose-parsing": "~2.1" }, diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 4b50e585..52d355a4 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -9,6 +9,3 @@ parameters: paths: - src - test - - ignoreErrors: - - '#FG\\ASN1\\Universal\\Integer constructor expects int, string given.#' diff --git a/src/Signer/Ecdsa.php b/src/Signer/Ecdsa.php index 0b75c621..f63ac8fc 100644 --- a/src/Signer/Ecdsa.php +++ b/src/Signer/Ecdsa.php @@ -3,25 +3,25 @@ namespace Lcobucci\JWT\Signer; -use Lcobucci\JWT\Signer\Ecdsa\Asn1; -use Lcobucci\JWT\Signer\Ecdsa\PointsManipulator; +use Lcobucci\JWT\Signer\Ecdsa\MultibyteStringConverter; +use Lcobucci\JWT\Signer\Ecdsa\SignatureConverter; use const OPENSSL_KEYTYPE_EC; abstract class Ecdsa extends OpenSSL { /** - * @var PointsManipulator + * @var SignatureConverter */ - private $manipulator; + private $converter; - public function __construct(PointsManipulator $manipulator) + public function __construct(SignatureConverter $converter) { - $this->manipulator = $manipulator; + $this->converter = $converter; } public static function create(): Ecdsa { - return new static(new Asn1()); + return new static(new MultibyteStringConverter()); } /** @@ -29,7 +29,7 @@ public static function create(): Ecdsa */ final public function sign(string $payload, Key $key): string { - return $this->manipulator->fromEcPoint( + return $this->converter->fromAsn1( $this->createSignature($key->getContent(), $key->getPassphrase(), $payload), $this->getKeyLength() ); @@ -41,7 +41,7 @@ final public function sign(string $payload, Key $key): string final public function verify(string $expected, string $payload, Key $key): bool { return $this->verifySignature( - $this->manipulator->toEcPoint($expected, $this->getKeyLength()), + $this->converter->toAsn1($expected, $this->getKeyLength()), $payload, $key->getContent() ); diff --git a/src/Signer/Ecdsa/Asn1.php b/src/Signer/Ecdsa/Asn1.php deleted file mode 100644 index f2c0d511..00000000 --- a/src/Signer/Ecdsa/Asn1.php +++ /dev/null @@ -1,77 +0,0 @@ -getChildren() as $child) { - $signature .= str_pad($this->decToHex($child->getContent()), $length, '0', STR_PAD_LEFT); - } - - $result = hex2bin($signature); - assert(is_string($result)); - - return $result; - } - - public function toEcPoint(string $points, int $length): string - { - $points = bin2hex($points); - - if (mb_strlen($points, '8bit') !== 2 * $length) { - throw new InvalidArgumentException('The length of given value is different than expected'); - } - - $pointR = mb_substr($points, 0, $length, '8bit'); - $pointS = mb_substr($points, $length, null, '8bit'); - - $sequence = new Sequence(); - $sequence->addChildren( - [ - new Integer(gmp_strval(gmp_init($pointR, 16))), - new Integer(gmp_strval(gmp_init($pointS, 16))), - ] - ); - - return $sequence->getBinary(); - } - - private function decToHex(string $value): string - { - $hex = gmp_strval(gmp_strval($value), 16); - - if (mb_strlen($hex, '8bit') % 2 !== 0) { - $hex = '0' . $hex; - } - - return $hex; - } -} diff --git a/src/Signer/Ecdsa/MultibyteStringConverter.php b/src/Signer/Ecdsa/MultibyteStringConverter.php new file mode 100644 index 00000000..9c4ebefd --- /dev/null +++ b/src/Signer/Ecdsa/MultibyteStringConverter.php @@ -0,0 +1,140 @@ + self::ASN1_MAX_SINGLE_BYTE ? self::ASN1_LENGTH_2BYTES : ''; + + $asn1 = hex2bin( + self::ASN1_SEQUENCE + . $lengthPrefix . dechex($totalLength) + . self::ASN1_INTEGER . dechex($lengthR) . $pointR + . self::ASN1_INTEGER . dechex($lengthS) . $pointS + ); + assert(is_string($asn1)); + + return $asn1; + } + + private static function octetLength(string $data): int + { + return (int) (mb_strlen($data, '8bit') / self::BYTE_SIZE); + } + + private static function preparePositiveInteger(string $data): string + { + if (mb_substr($data, 0, self::BYTE_SIZE, '8bit') > self::ASN1_BIG_INTEGER_LIMIT) { + return self::ASN1_NEGATIVE_INTEGER . $data; + } + + while (mb_substr($data, 0, self::BYTE_SIZE, '8bit') === self::ASN1_NEGATIVE_INTEGER + && mb_substr($data, 2, self::BYTE_SIZE, '8bit') <= self::ASN1_BIG_INTEGER_LIMIT) { + $data = mb_substr($data, 2, null, '8bit'); + } + + return $data; + } + + public function fromAsn1(string $signature, int $length): string + { + $message = bin2hex($signature); + $position = 0; + + if (self::readAsn1Content($message, $position, self::BYTE_SIZE) !== self::ASN1_SEQUENCE) { + throw new InvalidArgumentException('Invalid data. Should start with a sequence.'); + } + + if (self::readAsn1Content($message, $position, self::BYTE_SIZE) === self::ASN1_LENGTH_2BYTES) { + $position += self::BYTE_SIZE; + } + + $pointR = self::retrievePositiveInteger(self::readAsn1Integer($message, $position)); + $pointS = self::retrievePositiveInteger(self::readAsn1Integer($message, $position)); + + $points = hex2bin(str_pad($pointR, $length, '0', STR_PAD_LEFT) . str_pad($pointS, $length, '0', STR_PAD_LEFT)); + assert(is_string($points)); + + return $points; + } + + private static function readAsn1Content(string $message, int &$position, int $length): string + { + $content = mb_substr($message, $position, $length, '8bit'); + $position += $length; + + return $content; + } + + private static function readAsn1Integer(string $message, int &$position): string + { + if (self::readAsn1Content($message, $position, self::BYTE_SIZE) !== self::ASN1_INTEGER) { + throw new InvalidArgumentException('Invalid data. Should contain an integer.'); + } + + $length = (int) hexdec(self::readAsn1Content($message, $position, self::BYTE_SIZE)); + + return self::readAsn1Content($message, $position, $length * self::BYTE_SIZE); + } + + private static function retrievePositiveInteger(string $data): string + { + while (mb_substr($data, 0, self::BYTE_SIZE, '8bit') === self::ASN1_NEGATIVE_INTEGER + && mb_substr($data, 2, self::BYTE_SIZE, '8bit') > self::ASN1_BIG_INTEGER_LIMIT) { + $data = mb_substr($data, 2, null, '8bit'); + } + + return $data; + } +} diff --git a/src/Signer/Ecdsa/PointsManipulator.php b/src/Signer/Ecdsa/PointsManipulator.php deleted file mode 100644 index a79a968f..00000000 --- a/src/Signer/Ecdsa/PointsManipulator.php +++ /dev/null @@ -1,25 +0,0 @@ -verify($signature, $payload, $key)); + } + + /** + * @return mixed[] + */ + public function dataRFC6979(): iterable + { + yield from $this->sha256Data(); + yield from $this->sha384Data(); + yield from $this->sha512Data(); + } + + /** + * @return mixed[] + */ + public function sha256Data(): iterable + { + $signer = Sha256::create(); + $key = new Key( + '-----BEGIN PUBLIC KEY-----' . PHP_EOL + . 'MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEYP7UuiVanTHJYet0xjVtaMBJuJI7' . PHP_EOL + . 'Yfps5mliLmDyn7Z5A/4QCLi8maQa6elWKLxk8vGyDC1+n1F3o8KU1EYimQ==' . PHP_EOL + . '-----END PUBLIC KEY-----' + ); + + yield 'SHA-256 (sample)' => [ + $signer, + $key, + 'sample', + 'EFD48B2AACB6A8FD1140DD9CD45E81D69D2C877B56AAF991C34D0EA84EAF3716', + 'F7CB1C942D657C41D436C7A1B6E29F65F3E900DBB9AFF4064DC4AB2F843ACDA8', + ]; + + yield 'SHA-256 (test)' => [ + $signer, + $key, + 'test', + 'F1ABB023518351CD71D881567B1EA663ED3EFCF6C5132B354F28D3B0B7D38367', + '019F4113742A2B14BD25926B49C649155F267E60D3814B4C0CC84250E46F0083', + ]; + } + + /** + * @return mixed[] + */ + public function sha384Data(): iterable + { + $signer = Sha384::create(); + $key = new Key( + '-----BEGIN PUBLIC KEY-----' . PHP_EOL + . 'MHYwEAYHKoZIzj0CAQYFK4EEACIDYgAE7DpOQVtOGaRWhhgCn0J/pdqai8SukuAu' . PHP_EOL + . 'BqrlKGswDGTe+PDqkFWGYGSiVFFUgLwTgBXZty19VyROqO+awMYhiWcIpZNn+d+5' . PHP_EOL + . '9UyoSz8cnbEoiyMcOuDU/nNE/SUzJkcg' . PHP_EOL + . '-----END PUBLIC KEY-----' + ); + + yield 'SHA-384 (sample)' => [ + $signer, + $key, + 'sample', + '94EDBB92A5ECB8AAD4736E56C691916B3F88140666CE9FA73D64C4EA95AD133C81A648152E44ACF96E36DD1E80FABE46', + '99EF4AEB15F178CEA1FE40DB2603138F130E740A19624526203B6351D0A3A94FA329C145786E679E7B82C71A38628AC8', + ]; + + yield 'SHA-384 (test)' => [ + $signer, + $key, + 'test', + '8203B63D3C853E8D77227FB377BCF7B7B772E97892A80F36AB775D509D7A5FEB0542A7F0812998DA8F1DD3CA3CF023DB', + 'DDD0760448D42D8A43AF45AF836FCE4DE8BE06B485E9B61B827C2F13173923E06A739F040649A667BF3B828246BAA5A5', + ]; + } + + /** + * @return mixed[] + */ + public function sha512Data(): iterable + { + $signer = Sha512::create(); + $key = new Key( + '-----BEGIN PUBLIC KEY-----' . PHP_EOL + . 'MIGbMBAGByqGSM49AgEGBSuBBAAjA4GGAAQBiUVQ0HhZMuAOqiO2lPIT+MMSH4bc' . PHP_EOL + . 'l6BOWnFn205bzTcRI9RuRdtrXVNwp/IPtjMVXTj/oW0r12HcrEdLmi9QI6QASTEB' . PHP_EOL + . 'yWLNTS/d94IoXmRYQTnC+RtH+H/4I1TWYw90aiig2yV0G1s0qCgAiyKswj+ST6r7' . PHP_EOL + . '1NM/gepmlW3+qiv9/PU=' . PHP_EOL + . '-----END PUBLIC KEY-----' + ); + + yield 'SHA-512 (sample)' => [ + $signer, + $key, + 'sample', + '00C328FAFCBD79DD77850370C46325D987CB525569FB63C5D3BC53950E6D4C5F174E25A1EE9017B5D450606ADD152B534931D7D4E8' + . '455CC91F9B15BF05EC36E377FA', + '00617CCE7CF5064806C467F678D3B4080D6F1CC50AF26CA209417308281B68AF282623EAA63E5B5C0723D8B8C37FF0777B1A20F8CC' + . 'B1DCCC43997F1EE0E44DA4A67A', + ]; + + yield 'SHA-512 (test)' => [ + $signer, + $key, + 'test', + '013E99020ABF5CEE7525D16B69B229652AB6BDF2AFFCAEF38773B4B7D08725F10CDB93482FDCC54EDCEE91ECA4166B2A7C6265EF0C' + . 'E2BD7051B7CEF945BABD47EE6D', + '01FBD0013C674AA79CB39849527916CE301C66EA7CE8B80682786AD60F98F7E78A19CA69EFF5C57400E3B3A0AD66CE0978214D13BA' + . 'F4E9AC60752F7B155E2DE4DCE3', + ]; + } +} diff --git a/test/unit/Signer/Ecdsa/MultibyteStringConverterTest.php b/test/unit/Signer/Ecdsa/MultibyteStringConverterTest.php new file mode 100644 index 00000000..df2d88fa --- /dev/null +++ b/test/unit/Signer/Ecdsa/MultibyteStringConverterTest.php @@ -0,0 +1,134 @@ +toAsn1($message, strlen($r)))); + } + + /** + * @test + * + * @covers ::toAsn1 + * @covers ::octetLength + */ + public function toAsn1ShouldRaiseExceptionWhenPointsDoNotHaveCorrectLength(): void + { + $converter = new MultibyteStringConverter(); + + self::expectException(InvalidArgumentException::class); + $converter->toAsn1('a very wrong string', 64); + } + + /** + * @test + * @dataProvider pointsConversionData + * + * @covers ::fromAsn1 + * @covers ::readAsn1Content + * @covers ::readAsn1Integer + * @covers ::retrievePositiveInteger + */ + public function fromAsn1ShouldReturnTheConcatenatedPoints(string $r, string $s, string $asn1): void + { + $converter = new MultibyteStringConverter(); + $message = hex2bin($asn1); + assert(is_string($message)); + + self::assertSame($r . $s, bin2hex($converter->fromAsn1($message, strlen($r)))); + } + + /** + * @return string[][] + */ + public function pointsConversionData(): iterable + { + return [ + [ + 'efd48b2aacb6a8fd1140dd9cd45e81d69d2c877b56aaf991c34d0ea84eaf3716', + 'f7cb1c942d657c41d436c7a1b6e29f65f3e900dbb9aff4064dc4ab2f843acda8', + '3046022100efd48b2aacb6a8fd1140dd9cd45e81d69d2c877b56aaf991c34d0ea84eaf3716022100f7cb1c942d657c41d436c7' + . 'a1b6e29f65f3e900dbb9aff4064dc4ab2f843acda8', + ], + [ + '94edbb92a5ecb8aad4736e56c691916b3f88140666ce9fa73d64c4ea95ad133c81a648152e44acf96e36dd1e80fabe46', + '99ef4aeb15f178cea1fe40db2603138f130e740a19624526203b6351d0a3a94fa329c145786e679e7b82c71a38628ac8', + '306602310094edbb92a5ecb8aad4736e56c691916b3f88140666ce9fa73d64c4ea95ad133c81a648152e44acf96e36dd1e80fa' + . 'be4602310099ef4aeb15f178cea1fe40db2603138f130e740a19624526203b6351d0a3a94fa329c145786e679e7b82c71a38' + . '628ac8', + ], + [ + '00c328fafcbd79dd77850370c46325d987cb525569fb63c5d3bc53950e6d4c5f174e25a1ee9017b5d450606add152b534931d7' + . 'd4e8455cc91f9b15bf05ec36e377fa', + '00617cce7cf5064806c467f678d3b4080d6f1cc50af26ca209417308281b68af282623eaa63e5b5c0723d8b8c37ff0777b1a20' + . 'f8ccb1dccc43997f1ee0e44da4a67a', + '308187024200c328fafcbd79dd77850370c46325d987cb525569fb63c5d3bc53950e6d4c5f174e25a1ee9017b5d450606add15' + . '2b534931d7d4e8455cc91f9b15bf05ec36e377fa0241617cce7cf5064806c467f678d3b4080d6f1cc50af26ca20941730828' + . '1b68af282623eaa63e5b5c0723d8b8c37ff0777b1a20f8ccb1dccc43997f1ee0e44da4a67a', + ], + ]; + } + + /** + * @test + * @dataProvider invalidAsn1Structures + * + * @covers ::fromAsn1 + * @covers ::readAsn1Content + * @covers ::readAsn1Integer + * @covers ::retrievePositiveInteger + */ + public function fromAsn1ShouldRaiseExceptionOnInvalidMessage(string $message): void + { + $converter = new MultibyteStringConverter(); + $message = hex2bin($message); + assert(is_string($message)); + + self::expectException(InvalidArgumentException::class); + $converter->fromAsn1($message, 64); + } + + /** + * @return string[][] + */ + public function invalidAsn1Structures(): iterable + { + return [ + 'Not a sequence' => [''], + 'Sequence without length' => ['30'], + 'Only one string element' => ['3006030204f0'], + 'Only one integer element' => ['3004020101'], + 'Integer+string elements' => ['300a020101030204f0'], + ]; + } +} diff --git a/test/unit/Signer/Ecdsa/Sha256Test.php b/test/unit/Signer/Ecdsa/Sha256Test.php index 6744513d..c7357554 100644 --- a/test/unit/Signer/Ecdsa/Sha256Test.php +++ b/test/unit/Signer/Ecdsa/Sha256Test.php @@ -14,7 +14,7 @@ final class Sha256Test extends TestCase * @covers \Lcobucci\JWT\Signer\Ecdsa::create * @covers \Lcobucci\JWT\Signer\Ecdsa::__construct * - * @uses \Lcobucci\JWT\Signer\Ecdsa\Asn1 + * @uses \Lcobucci\JWT\Signer\Ecdsa\MultibyteStringConverter */ public function createShouldReturnAValidInstance(): void { @@ -61,6 +61,6 @@ public function getKeyLengthMustBeCorrect(): void private function getSigner(): Sha256 { - return new Sha256($this->createMock(PointsManipulator::class)); + return new Sha256($this->createMock(SignatureConverter::class)); } } diff --git a/test/unit/Signer/Ecdsa/Sha384Test.php b/test/unit/Signer/Ecdsa/Sha384Test.php index f0baea42..58816971 100644 --- a/test/unit/Signer/Ecdsa/Sha384Test.php +++ b/test/unit/Signer/Ecdsa/Sha384Test.php @@ -14,7 +14,7 @@ final class Sha384Test extends TestCase * @covers \Lcobucci\JWT\Signer\Ecdsa::create * @covers \Lcobucci\JWT\Signer\Ecdsa::__construct * - * @uses \Lcobucci\JWT\Signer\Ecdsa\Asn1 + * @uses \Lcobucci\JWT\Signer\Ecdsa\MultibyteStringConverter */ public function createShouldReturnAValidInstance(): void { @@ -61,6 +61,6 @@ public function getKeyLengthMustBeCorrect(): void private function getSigner(): Sha384 { - return new Sha384($this->createMock(PointsManipulator::class)); + return new Sha384($this->createMock(SignatureConverter::class)); } } diff --git a/test/unit/Signer/Ecdsa/Sha512Test.php b/test/unit/Signer/Ecdsa/Sha512Test.php index 98849e2d..ee2c16b4 100644 --- a/test/unit/Signer/Ecdsa/Sha512Test.php +++ b/test/unit/Signer/Ecdsa/Sha512Test.php @@ -14,7 +14,7 @@ final class Sha512Test extends TestCase * @covers \Lcobucci\JWT\Signer\Ecdsa::create * @covers \Lcobucci\JWT\Signer\Ecdsa::__construct * - * @uses \Lcobucci\JWT\Signer\Ecdsa\Asn1 + * @uses \Lcobucci\JWT\Signer\Ecdsa\MultibyteStringConverter */ public function createShouldReturnAValidInstance(): void { @@ -61,6 +61,6 @@ public function getKeyLengthMustBeCorrect(): void private function getSigner(): Sha512 { - return new Sha512($this->createMock(PointsManipulator::class)); + return new Sha512($this->createMock(SignatureConverter::class)); } } diff --git a/test/unit/Signer/EcdsaTest.php b/test/unit/Signer/EcdsaTest.php index d66e4bb8..d4375d25 100644 --- a/test/unit/Signer/EcdsaTest.php +++ b/test/unit/Signer/EcdsaTest.php @@ -4,7 +4,7 @@ namespace Lcobucci\JWT\Signer; use Lcobucci\JWT\Keys; -use Lcobucci\JWT\Signer\Ecdsa\Asn1; +use Lcobucci\JWT\Signer\Ecdsa\MultibyteStringConverter; use PHPUnit\Framework\TestCase; use const OPENSSL_ALGO_SHA256; use function openssl_pkey_get_private; @@ -17,7 +17,7 @@ final class EcdsaTest extends TestCase use Keys; /** - * @var Asn1 + * @var MultibyteStringConverter */ private $pointsManipulator; @@ -26,7 +26,7 @@ final class EcdsaTest extends TestCase */ public function createDependencies(): void { - $this->pointsManipulator = new Asn1(); + $this->pointsManipulator = new MultibyteStringConverter(); } private function getSigner(): Ecdsa @@ -50,7 +50,7 @@ private function getSigner(): Ecdsa * * @covers \Lcobucci\JWT\Signer\Ecdsa::sign * @covers \Lcobucci\JWT\Signer\Ecdsa::getKeyType - * @covers \Lcobucci\JWT\Signer\Ecdsa\Asn1 + * @covers \Lcobucci\JWT\Signer\Ecdsa\MultibyteStringConverter * @covers \Lcobucci\JWT\Signer\OpenSSL * * @uses \Lcobucci\JWT\Signer\Ecdsa::__construct @@ -70,7 +70,7 @@ public function signShouldReturnTheAHashBasedOnTheOpenSslSignature(): void 1, openssl_verify( $payload, - $this->pointsManipulator->toEcPoint($signature, $signer->getKeyLength()), + $this->pointsManipulator->toAsn1($signature, $signer->getKeyLength()), $publicKey, OPENSSL_ALGO_SHA256 ) @@ -82,7 +82,7 @@ public function signShouldReturnTheAHashBasedOnTheOpenSslSignature(): void * * @covers \Lcobucci\JWT\Signer\Ecdsa::verify * @covers \Lcobucci\JWT\Signer\Ecdsa::getKeyType - * @covers \Lcobucci\JWT\Signer\Ecdsa\Asn1 + * @covers \Lcobucci\JWT\Signer\Ecdsa\MultibyteStringConverter * @covers \Lcobucci\JWT\Signer\OpenSSL * * @uses \Lcobucci\JWT\Signer\Ecdsa::__construct @@ -102,7 +102,7 @@ public function verifyShouldDelegateToEcdsaSignerUsingPublicKey(): void self::assertTrue( $signer->verify( - $this->pointsManipulator->fromEcPoint($signature, $signer->getKeyLength()), + $this->pointsManipulator->fromAsn1($signature, $signer->getKeyLength()), $payload, self::$ecdsaKeys['public1'] )