diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 9e8a3d8..5ebe887 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -14,11 +14,11 @@ jobs: strategy: fail-fast: false matrix: - php-versions: ['7.4', '8.0', '8.1', '8.2', '8.3'] + php-versions: ['8.0', '8.1', '8.2', '8.3', '8.4'] steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Setup PHP uses: shivammathur/setup-php@v2 @@ -33,7 +33,7 @@ jobs: echo "::set-output name=dir::$(composer config cache-files-dir)" - name: Composer cache - uses: actions/cache@v3 + uses: actions/cache@v4 with: path: ${{ steps.composer-cache.outputs.dir }} key: ${{ runner.os }}-composer @@ -56,12 +56,12 @@ jobs: run: composer run phpunit -- --teamcity - name: Set up Go - uses: actions/setup-go@v3 + uses: actions/setup-go@v5 with: go-version: ^1.16 - name: Go cache - uses: actions/cache@v3 + uses: actions/cache@v4 with: path: ~/go/pkg/mod key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} diff --git a/LICENSE b/LICENSE index 389773d..b7a12d2 100644 --- a/LICENSE +++ b/LICENSE @@ -1,4 +1,4 @@ -Copyright (C) 2021 Angry Bytes +Copyright (C) 2025 Angry Bytes Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in diff --git a/composer.json b/composer.json index fef2d8d..5339a6f 100644 --- a/composer.json +++ b/composer.json @@ -6,12 +6,12 @@ "authors": [ { "name": "Stéphan Kochen", - "email": "stephan@kochen.nl" + "email": "mail@stephank.nl" } ], "scripts": { "php-cs-fixer": "php-cs-fixer fix", - "phpstan": "phpstan analyse -l max -c phpstan.neon src/", + "phpstan": "phpstan analyse -l max src/", "phpunit": "phpunit" }, "autoload": { @@ -20,14 +20,13 @@ } }, "require": { - "fgrosse/phpasn1": "^2.3.1", "lcobucci/clock": "^2.0.0 || ^3.0.0", "lcobucci/jwt": "^4.1.0 || ^5.0.0", "guzzlehttp/guzzle": "^7.2.0" }, "require-dev": { "phpunit/phpunit": "^9.5 || ^10.4.2", - "phpstan/phpstan": "^1.2.0", + "phpstan/phpstan": "^2.1.5", "friendsofphp/php-cs-fixer": "^3.14" } } diff --git a/phpstan.neon b/phpstan.neon deleted file mode 100644 index 18ea5d1..0000000 --- a/phpstan.neon +++ /dev/null @@ -1,12 +0,0 @@ -parameters: - ignoreErrors: - - - message: "#^Parameter \\#1 \\$value of class FG\\\\ASN1\\\\Universal\\\\Integer constructor expects int, string given\\.$#" - count: 2 - path: src/Client.php - - - - message: "#^Cannot call method del\\(\\) on string\\|false\\.$#" - count: 1 - path: src/RedisStore.php - diff --git a/src/AbstractStore.php b/src/AbstractStore.php index 7d8bbf2..5b475d9 100644 --- a/src/AbstractStore.php +++ b/src/AbstractStore.php @@ -19,14 +19,14 @@ abstract class AbstractStore implements StoreInterface /** * Lifespan of a nonce. * - * @var float + * @var int */ public $nonceTtl = 15 * 60; /** * Minimum time to cache a HTTP response. * - * @var float + * @var int */ public $cacheMinTtl = 60 * 60; @@ -57,9 +57,9 @@ public function generateNonce(string $email): string * * @param string $url the URL to fetch * - * @return \stdClass an object with `ttl` and `data` properties + * @return object{data: \stdClass, ttl: int} */ - public function fetch(string $url): \stdClass + public function fetch(string $url): object { $res = $this->guzzle->get($url); diff --git a/src/Client.php b/src/Client.php index 1d08796..46eaf6f 100644 --- a/src/Client.php +++ b/src/Client.php @@ -68,7 +68,7 @@ public static function normalize(string $email): string assert(defined('MB_CASE_FOLD') && function_exists('idn_to_ascii')); $localEnd = strrpos($email, '@'); - if (false === $localEnd) { + if (false === $localEnd || $localEnd + 1 === strlen($email)) { return ''; } @@ -101,7 +101,7 @@ public static function normalize(string $email): string * * @return string URL to redirect the browser to */ - public function authenticate(string $email, string $state = null): string + public function authenticate(string $email, ?string $state = null): string { $authEndpoint = $this->fetchDiscovery()->authorization_endpoint ?? null; if (!is_string($authEndpoint)) { @@ -161,19 +161,23 @@ public function verify(string $token): string } // Find the matching public key, and verify the signature. - $publicKey = null; + $publicKey = ''; foreach ($keysDoc->keys as $key) { if ($key instanceof \stdClass - && isset($key->alg) && 'RS256' === $key->alg - && isset($key->kid) && $key->kid === $kid - && isset($key->n) && isset($key->e)) { - $publicKey = self::parseJwk($key); + && isset($key->alg) && 'RS256' === $key->alg + && isset($key->kid) && $key->kid === $kid + ) { + try { + $publicKey = JWK::toPem($key); + } catch (\Exception) { + } break; } } - if (null === $publicKey) { + if ('' === $publicKey) { throw new \Exception('Cannot find the public key used to sign the token'); } + $publicKey = JwtSigner\Key\InMemory::plainText($publicKey); // Validate the token claims. $clock = \Lcobucci\Clock\SystemClock::fromUTC(); @@ -228,28 +232,6 @@ private function fetchDiscovery(): \stdClass return $this->store->fetchCached('discovery', $discoveryUrl); } - /** - * Parse a JWK into a PEM public key. - */ - private static function parseJwk(\stdClass $jwk): JwtSigner\Key - { - $n = gmp_init(bin2hex(self::decodeBase64Url($jwk->n)), 16); - $e = gmp_init(bin2hex(self::decodeBase64Url($jwk->e)), 16); - - $seq = new \FG\ASN1\Universal\Sequence(); - $seq->addChild(new \FG\ASN1\Universal\Integer(gmp_strval($n))); - $seq->addChild(new \FG\ASN1\Universal\Integer(gmp_strval($e))); - $pkey = new \FG\X509\PublicKey(bin2hex($seq->getBinary())); - - $encoded = base64_encode($pkey->getBinary()); - - return JwtSigner\Key\InMemory::plainText( - "-----BEGIN PUBLIC KEY-----\n". - chunk_split($encoded, 64, "\n"). - "-----END PUBLIC KEY-----\n" - ); - } - /** * Get the origin for a URL. */ @@ -281,14 +263,4 @@ private static function getOrigin(string $url): string return $res; } - - private static function decodeBase64Url(string $input): string - { - $output = base64_decode(strtr($input, '-_', '+/'), true); - if (false === $output) { - throw new \Exception('Invalid base64'); - } - - return $output; - } } diff --git a/src/DER.php b/src/DER.php new file mode 100644 index 0000000..3608a09 --- /dev/null +++ b/src/DER.php @@ -0,0 +1,90 @@ +>= 7; + while ($num > 0) { + $result .= chr(($num & 0x7F) | 0x80); + $num >>= 7; + } + + return strrev($result); + } + + /** + * Encode a sequence of values. + */ + public static function encodeSequence(string ...$values): string + { + return self::encodeValue(self::ID_SEQUENCE, implode('', $values)); + } + + /** + * Encode an object identifier. + */ + public static function encodeOid(int ...$values): string + { + $bin = ''; + foreach ($values as $value) { + $bin .= self::encodeBase128($value); + } + + return self::encodeValue(self::ID_OBJECT_ID, $bin); + } + + /** + * Encode some data as a bit string. + */ + public static function encodeBitString(string $data): string + { + return self::encodeValue(self::ID_BIT_STRING, "\0".$data); + } +} diff --git a/src/JWK.php b/src/JWK.php new file mode 100644 index 0000000..0703143 --- /dev/null +++ b/src/JWK.php @@ -0,0 +1,150 @@ +kty) || !is_string($jwk->kty)) { + throw new \Exception('Missing or invalid kty'); + } + + switch ($jwk->kty) { + case 'RSA': + return self::rsaToPem($jwk); + case 'EC': + return self::ecToPem($jwk); + case 'OKP': + return self::okpToPem($jwk); + default: + throw new \Exception('Unsupported kty: '.substr($jwk->kty, 0, 10)); + } + } + + private static function rsaToPem(\stdClass $jwk): string + { + if (!isset($jwk->n) || !is_string($jwk->n) + || !isset($jwk->e) || !is_string($jwk->e)) { + throw new \Exception('Incomplete RSA public key'); + } + + // RSAPublicKey + $n = DER::encodeValue(DER::ID_INTEGER, self::decodeBase64Url($jwk->n)); + $e = DER::encodeValue(DER::ID_INTEGER, self::decodeBase64Url($jwk->e)); + $key = DER::encodeSequence($n, $e); + + // PublicKeyInfo + $oid = DER::encodeOid(42, 840, 113549, 1, 1, 1); // RSA + $alg = DER::encodeSequence($oid, DER::NULL); + $key = DER::encodeBitString($key); + $info = DER::encodeSequence($alg, $key); + + return self::derToPem($info); + } + + private static function ecToPem(\stdClass $jwk): string + { + if (!isset($jwk->crv) || !is_string($jwk->crv) + || !isset($jwk->x) || !is_string($jwk->x) + || !isset($jwk->y) || !is_string($jwk->y)) { + throw new \Exception('Incomplete EC public key'); + } + + $curveOid = null; + switch ($jwk->crv) { + case 'P-256': + $curveOid = [42, 840, 10045, 3, 1, 7]; + break; + case 'P-384': + $curveOid = [43, 132, 0, 34]; + break; + case 'P-521': + $curveOid = [43, 132, 0, 35]; + break; + case 'secp256k1': + $curveOid = [43, 132, 0, 10]; + break; + default: + throw new \Exception('Unsupported EC curve: '.substr($jwk->crv, 0, 10)); + } + + // ECPoint + $x = self::decodeBase64Url($jwk->x); + $y = self::decodeBase64Url($jwk->y); + $key = "\x04".$x.$y; + + // PublicKeyInfo + $oid = DER::encodeOid(42, 840, 10045, 2, 1); + $curveOid = DER::encodeOid(...$curveOid); + $alg = DER::encodeSequence($oid, $curveOid); + $key = DER::encodeBitString($key); + $info = DER::encodeSequence($alg, $key); + + return self::derToPem($info); + } + + private static function okpToPem(\stdClass $jwk): string + { + if (!isset($jwk->crv) || !is_string($jwk->crv) + || !isset($jwk->x) || !is_string($jwk->x)) { + throw new \Exception('Incomplete OKP public key'); + } + + $oid = null; + switch ($jwk->crv) { + case 'X25519': + $oid = [43, 101, 110]; + break; + case 'X448': + $oid = [43, 101, 111]; + break; + case 'Ed25519': + $oid = [43, 101, 112]; + break; + case 'X25519': + $oid = [43, 101, 113]; + break; + default: + throw new \Exception('Unsupported OKP curve: '.substr($jwk->crv, 0, 10)); + } + + $key = self::decodeBase64Url($jwk->x); + + // PublicKeyInfo + $oid = DER::encodeOid(...$oid); + $alg = DER::encodeSequence($oid); + $key = DER::encodeBitString($key); + $info = DER::encodeSequence($alg, $key); + + return self::derToPem($info); + } + + private static function derToPem(string $der): string + { + return + "-----BEGIN PUBLIC KEY-----\n". + chunk_split(base64_encode($der), 64, "\n"). + "-----END PUBLIC KEY-----\n"; + } + + /** + * @internal for tests only + */ + public static function decodeBase64Url(string $input): string + { + $output = base64_decode(strtr($input, '-_', '+/'), true); + if (false === $output) { + throw new \Exception('Invalid base64'); + } + + return $output; + } +} diff --git a/src/MemoryStore.php b/src/MemoryStore.php index cf4bfaf..992e645 100644 --- a/src/MemoryStore.php +++ b/src/MemoryStore.php @@ -10,9 +10,9 @@ */ class MemoryStore extends AbstractStore { - /** @var \stdClass[] */ + /** @var array */ private $cache; - /** @var \stdClass[] */ + /** @var array */ private $nonces; /** diff --git a/src/RedisStore.php b/src/RedisStore.php index 9f21833..3474a59 100644 --- a/src/RedisStore.php +++ b/src/RedisStore.php @@ -26,7 +26,7 @@ public function fetchCached(string $cacheId, string $url): \stdClass $key = 'cache:'.$cacheId; $data = $this->redis->get($key); - if ($data) { + if (is_string($data) && $data) { $data = json_decode($data); assert($data instanceof \stdClass); @@ -58,10 +58,10 @@ public function createNonce(string $email): string public function consumeNonce(string $nonce, string $email): void { $key = 'nonce:'.$nonce; - $res = $this->redis->multi() - ->get($key) - ->del($key) - ->exec(); + $this->redis->multi(); + $this->redis->get($key); + $this->redis->del($key); + $res = $this->redis->exec(); if ($res[0] !== $email) { throw new \Exception('Invalid or expired nonce'); } diff --git a/tests/ClientTest.php b/tests/ClientTest.php index 61ca561..9d33ae2 100644 --- a/tests/ClientTest.php +++ b/tests/ClientTest.php @@ -35,7 +35,7 @@ public function testNormalize() public function testAuthenticate() { - $store = new class() implements Client\StoreInterface { + $store = new class implements Client\StoreInterface { public bool $fetchCachedCalled = false; public bool $createNonceCalled = false; diff --git a/tests/JWKTest.php b/tests/JWKTest.php new file mode 100644 index 0000000..0ab9b1e --- /dev/null +++ b/tests/JWKTest.php @@ -0,0 +1,87 @@ + 'rsa', + 'ES256' => 'p256', + 'ES384' => 'p384', + 'ES512' => 'p521', + 'EdDSA' => 'ed25519', + ]; + + /** + * Tests JWK to PEM conversion by using the PEM result to verify a signature. + */ + public function testToPem(): void + { + $signers = [ + 'RS256' => new Signer\Rsa\Sha256(), + 'ES256' => new Signer\Ecdsa\Sha256(), + 'ES384' => new Signer\Ecdsa\Sha384(), + 'ES512' => new Signer\Ecdsa\Sha512(), + 'EdDSA' => new Signer\Eddsa(), + ]; + + foreach (self::FIXTURES as $alg => $basename) { + $jwk = file_get_contents(__DIR__."/fixtures/{$basename}.jwk"); + $this->assertNotFalse($jwk, "read {$basename} jwk fixture"); + + $sig = file_get_contents(__DIR__."/fixtures/{$basename}.sig"); + $this->assertNotFalse($sig, "read {$basename} jws fixture"); + + $jwk = json_decode($jwk, flags: JSON_THROW_ON_ERROR); + $pem = JWK::toPem($jwk); + + // NOTE: This check is crucial for testing Ed25519 because of the below hack. + // Without it, we wouldn't really be testing the DER encoding is correct. + $result = openssl_get_publickey($pem); + $this->assertNotFalse($result, "parse {$basename} converted pem"); + + [$header, $payload, $signature] = explode('.', $sig); + $signedPart = "{$header}.{$payload}"; + + $header = json_decode(JWK::decodeBase64Url($header), flags: JSON_THROW_ON_ERROR); + $this->assertEquals($header->alg, $alg, "check {$basename} fixture jws alg"); + + $this->assertEquals(JWK::decodeBase64Url($payload), 'hello', 'check fixture payload'); + + // HACK: The JWT lib expects a raw EdDSA key, not PEM. This is because it uses the + // libsodium bindings for EdDSA, rather than the OpenSSL bindings. It appears the + // PHP OpenSSL bindings can't verify Ed25519 at the moment. This simply extracts + // the public key from a known offset in the DER encoding. + if ('EdDSA' === $alg) { + $lines = explode("\n", trim($pem)); + array_pop($lines); + array_shift($lines); + $der = base64_decode(implode('', $lines)); + $key = substr($der, 12); + $key = Signer\Key\InMemory::plainText($key); + } else { + $key = Signer\Key\InMemory::plainText($pem); + } + + $signature = JWK::decodeBase64Url($signature); + $result = $signers[$alg]->verify($signature, $signedPart, $key); + $this->assertTrue($result, "check signature with converted {$basename} key"); + } + } +} diff --git a/tests/fixtures/ed25519.jwk b/tests/fixtures/ed25519.jwk new file mode 100644 index 0000000..26e92e2 --- /dev/null +++ b/tests/fixtures/ed25519.jwk @@ -0,0 +1,5 @@ +{ + "kty": "OKP", + "crv": "Ed25519", + "x": "7kJIranwstm9CFxhl3Rbqru2gwhwItpztlBtXaCzjnU" +} diff --git a/tests/fixtures/ed25519.sig b/tests/fixtures/ed25519.sig new file mode 100644 index 0000000..4de0654 --- /dev/null +++ b/tests/fixtures/ed25519.sig @@ -0,0 +1 @@ +eyJhbGciOiJFZERTQSJ9.aGVsbG8.z6VBFAus2vvt2MRzZu7e-2c_rq5TJY5Z8lTxDjb6XV3U3jX4OvSL0m5NQofybN_c4S_nX8WhQr5skmjhktZhBw \ No newline at end of file diff --git a/tests/fixtures/p256.jwk b/tests/fixtures/p256.jwk new file mode 100644 index 0000000..b07ce78 --- /dev/null +++ b/tests/fixtures/p256.jwk @@ -0,0 +1,6 @@ +{ + "kty": "EC", + "crv": "P-256", + "x": "H8zYYFuFrWQIiJiK5v94UnYbgLHDYViPBkDcEZS0nd0", + "y": "3vQ2k5gnXXgAeT51qNtfV_K9nFIVjokmREytSXHWVcE" +} diff --git a/tests/fixtures/p256.sig b/tests/fixtures/p256.sig new file mode 100644 index 0000000..d6d012c --- /dev/null +++ b/tests/fixtures/p256.sig @@ -0,0 +1 @@ +eyJhbGciOiJFUzI1NiJ9.aGVsbG8.jF_haaufDyjNxDMX2ycKyfbaCb3lIyuBB8nGcInutnXH5sKW5zHD1XwAwF0aEBfzFN5E3-8fB9AISzbR-gvBjQ \ No newline at end of file diff --git a/tests/fixtures/p384.jwk b/tests/fixtures/p384.jwk new file mode 100644 index 0000000..5c23d2e --- /dev/null +++ b/tests/fixtures/p384.jwk @@ -0,0 +1,6 @@ +{ + "kty": "EC", + "crv": "P-384", + "x": "pHC_2jFRNwF4mBEvzOBAEUMx9Rwi-F8CRf2TF7ok_gtUvG57HQtCMpaw6h6Pm1lb", + "y": "gaGd5vGHO9gqG2WE879qImbDF1T9g388Y97zihTW0yl-qk2vXd6mdTBUOHb-xniT" +} diff --git a/tests/fixtures/p384.sig b/tests/fixtures/p384.sig new file mode 100644 index 0000000..3ef1978 --- /dev/null +++ b/tests/fixtures/p384.sig @@ -0,0 +1 @@ +eyJhbGciOiJFUzM4NCJ9.aGVsbG8.GQswXIWkEkBKIC4IE_RhvZyIn4XgT_dwVOyThHApLbJ60QCVgZAv23AXpjqE6ZVJeEL8rO5b7P3zOBRLvUxlpCIHeb1OSeD5iQvhcoszJIi1aGQKpyabJaXeqy51XrcV \ No newline at end of file diff --git a/tests/fixtures/p521.jwk b/tests/fixtures/p521.jwk new file mode 100644 index 0000000..e34e6c7 --- /dev/null +++ b/tests/fixtures/p521.jwk @@ -0,0 +1,6 @@ +{ + "kty": "EC", + "crv": "P-521", + "x": "ANV-IHRO1TMNpgGEe2ZWRdYOwoW3UExMz8fWxDZTSaOabD6K2KiUhk5teWlridaxIYsp4LkKXljwrZoVmfS0ec4w", + "y": "AGxt7g6df9rshm8hijAMVHnmwsN3ZLgVF82lfdoQNkyjxmIdFxjcH6aobX6wmL33RxwdEhcZyydXjmbDClBIkLiu" +} diff --git a/tests/fixtures/p521.sig b/tests/fixtures/p521.sig new file mode 100644 index 0000000..4d1ead3 --- /dev/null +++ b/tests/fixtures/p521.sig @@ -0,0 +1 @@ +eyJhbGciOiJFUzUxMiJ9.aGVsbG8.AWYqMWzozFm0cuwY5OYxCIJGTlqwmGXyVgFcWFGrG1CqQhCvpOFITKggQh54T5tf1hd37CX7IKBiyZj5h_1fZJQGAOrf9EF5mpK2OTJdtl87OY2wEOmZsTXSVRrjWJC4kstxe00Rol6m8RDz09Sm1NOjv8LY5ukkiUGbNLCRtlVI6sfZ \ No newline at end of file diff --git a/tests/fixtures/rsa.jwk b/tests/fixtures/rsa.jwk new file mode 100644 index 0000000..1167c83 --- /dev/null +++ b/tests/fixtures/rsa.jwk @@ -0,0 +1,5 @@ +{ + "kty": "RSA", + "n": "yVs2FiQuTWgL-PQgWshRvkw606ki4kGzxSxXjbCzsKV-ZtcadDjIBapxmEzaNtz_kKon9NW1MAzCZJwGbTKYMiaDx9lavbGsGk6Wl91jp0GyeAIMJ3N-d8UlNwE-1RbXxpJWcJlVMzCK-NKCvvsBJ9UBSoO8JsVBRK5IOiCwRqgzG3L6S3KHFMi2GmdUs2PR7SOx7cqJAo25N0EJ4YUvqeKenPXDcg9fGTL4MiR3l7Daj83qUve85PpEEp9otwOw9reoKOJCghh7GypYeoUhxJRzaftOATVO7XtJ-Mf75ZEr6K_Ie4-c5b3Dtlny6y0XRIVifQARQ7iRnS6Wxm92CQ", + "e": "AQAB" +} diff --git a/tests/fixtures/rsa.sig b/tests/fixtures/rsa.sig new file mode 100644 index 0000000..30071a8 --- /dev/null +++ b/tests/fixtures/rsa.sig @@ -0,0 +1 @@ +eyJhbGciOiJSUzI1NiJ9.aGVsbG8.N__5AlqGUqTTekH1_nbwSIyjYYhn0T3HQsNa-X6_YLCyGhDhsXj7DdCpq5_-WjbMxBGHPpplM73sV0WmWIbWFwisQmA3OlYeVGT8UJkX47C1gy0dTNSUc32joMSU7AximM14Rudf--OWECj2uUtOWIlTxfZeHdRR-QAG6suPiSHJ5DmdlHzeU2qGiANii2OJb2bNPvIIxIMGvmX4dtj4S-B3Yj2muJMKVgLV1rvria0yE_irJIJoV9KnQcHwkhnp1aAI8VUEglymAlN4Tstn2PCEIJvLIQ3jUcvVdtKxAq3YVvJw7XaWJ0Fjnk3KBRGY3Vh7vcet9JHhD0sBqpsRCg \ No newline at end of file