Skip to content

Commit 7480196

Browse files
bshafferbshaffer
authored
fix: require key object (#373)
1 parent 83b6090 commit 7480196

File tree

6 files changed

+98
-126
lines changed

6 files changed

+98
-126
lines changed

‎README.md‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,8 +200,7 @@ $jwks = ['keys' => []];
200200

201201
// JWK::parseKeySet($jwks) returns an associative array of **kid** to private
202202
// key. Pass this as the second parameter to JWT::decode.
203-
// NOTE: The deprecated $supportedAlgorithm must be supplied when parsing from JWK.
204-
JWT::decode($payload, JWK::parseKeySet($jwks), $supportedAlgorithm);
203+
JWT::decode($payload, JWK::parseKeySet($jwks));
205204
```
206205

207206
Changelog

‎src/JWK.php‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,15 @@ public static function parseKeySet(array $jwks)
4747
foreach ($jwks['keys'] as $k => $v) {
4848
$kid = isset($v['kid']) ? $v['kid'] : $k;
4949
if ($key = self::parseKey($v)) {
50-
$keys[$kid] = $key;
50+
if (isset($v['alg'])) {
51+
$keys[$kid] = new Key($key, $v['alg']);
52+
} else {
53+
// The "alg" parameter is optional in a KTY, but is required
54+
// for parsing in this library. Add it manually to your JWK
55+
// array if it doesn't already exist.
56+
// @see https://datatracker.ietf.org/doc/html/rfc7517#section-4.4
57+
throw new InvalidArgumentException('JWK key is missing "alg"');
58+
}
5159
}
5260
}
5361

‎src/JWT.php‎

Lines changed: 22 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -60,13 +60,11 @@ class JWT
6060
* Decodes a JWT string into a PHP object.
6161
*
6262
* @param string $jwt The JWT
63-
* @param Key|array<Key>|mixed $keyOrKeyArray The Key or array of Key objects.
63+
* @param Key|array<Key> $keyOrKeyArray The Key or array of Key objects.
6464
* If the algorithm used is asymmetric, this is the public key
6565
* Each Key object contains an algorithm and matching key.
6666
* Supported algorithms are 'ES384','ES256', 'HS256', 'HS384',
6767
* 'HS512', 'RS256', 'RS384', and 'RS512'
68-
* @param array $allowed_algs [DEPRECATED] List of supported verification algorithms. Only
69-
* should be used for backwards compatibility.
7068
*
7169
* @return object The JWT's payload as a PHP object
7270
*
@@ -80,8 +78,9 @@ class JWT
8078
* @uses jsonDecode
8179
* @uses urlsafeB64Decode
8280
*/
83-
public static function decode($jwt, $keyOrKeyArray, array $allowed_algs = array())
81+
public static function decode($jwt, $keyOrKeyArray)
8482
{
83+
// Validate JWT
8584
$timestamp = \is_null(static::$timestamp) ? \time() : static::$timestamp;
8685

8786
if (empty($keyOrKeyArray)) {
@@ -108,31 +107,18 @@ public static function decode($jwt, $keyOrKeyArray, array $allowed_algs = array(
108107
throw new UnexpectedValueException('Algorithm not supported');
109108
}
110109

111-
list($keyMaterial, $algorithm) = self::getKeyMaterialAndAlgorithm(
112-
$keyOrKeyArray,
113-
empty($header->kid) ? null : $header->kid
114-
);
110+
$key = self::getKey($keyOrKeyArray, empty($header->kid) ? null : $header->kid);
115111

116-
if (empty($algorithm)) {
117-
// Use deprecated "allowed_algs" to determine if the algorithm is supported.
118-
// This opens up the possibility of an attack in some implementations.
119-
// @see https://github.com/firebase/php-jwt/issues/351
120-
if (!\in_array($header->alg, $allowed_algs)) {
121-
throw new UnexpectedValueException('Algorithm not allowed');
122-
}
123-
} else {
124-
// Check the algorithm
125-
if (!self::constantTimeEquals($algorithm, $header->alg)) {
126-
// See issue #351
127-
throw new UnexpectedValueException('Incorrect key for this algorithm');
128-
}
112+
// Check the algorithm
113+
if (!self::constantTimeEquals($key->getAlgorithm(), $header->alg)) {
114+
// See issue #351
115+
throw new UnexpectedValueException('Incorrect key for this algorithm');
129116
}
130117
if ($header->alg === 'ES256' || $header->alg === 'ES384') {
131118
// OpenSSL expects an ASN.1 DER sequence for ES256/ES384 signatures
132119
$sig = self::signatureToDER($sig);
133120
}
134-
135-
if (!static::verify("$headb64.$bodyb64", $sig, $keyMaterial, $header->alg)) {
121+
if (!static::verify("$headb64.$bodyb64", $sig, $key->getKeyMaterial(), $header->alg)) {
136122
throw new SignatureInvalidException('Signature verification failed');
137123
}
138124

@@ -393,40 +379,34 @@ public static function urlsafeB64Encode($input)
393379
*
394380
* @return array containing the keyMaterial and algorithm
395381
*/
396-
private static function getKeyMaterialAndAlgorithm($keyOrKeyArray, $kid = null)
382+
private static function getKey($keyOrKeyArray, $kid = null)
397383
{
398-
if (
399-
is_string($keyOrKeyArray)
400-
|| is_resource($keyOrKeyArray)
401-
|| $keyOrKeyArray instanceof OpenSSLAsymmetricKey
402-
) {
403-
return array($keyOrKeyArray, null);
404-
}
405-
406384
if ($keyOrKeyArray instanceof Key) {
407-
return array($keyOrKeyArray->getKeyMaterial(), $keyOrKeyArray->getAlgorithm());
385+
return $keyOrKeyArray;
408386
}
409387

410388
if (is_array($keyOrKeyArray) || $keyOrKeyArray instanceof ArrayAccess) {
389+
foreach ($keyOrKeyArray as $keyId => $key) {
390+
if (!$key instanceof Key) {
391+
throw new UnexpectedValueException(
392+
'$keyOrKeyArray must be an instance of Firebase\JWT\Key key or an '
393+
. 'array of Firebase\JWT\Key keys'
394+
);
395+
}
396+
}
411397
if (!isset($kid)) {
412398
throw new UnexpectedValueException('"kid" empty, unable to lookup correct key');
413399
}
414400
if (!isset($keyOrKeyArray[$kid])) {
415401
throw new UnexpectedValueException('"kid" invalid, unable to lookup correct key');
416402
}
417403

418-
$key = $keyOrKeyArray[$kid];
419-
420-
if ($key instanceof Key) {
421-
return array($key->getKeyMaterial(), $key->getAlgorithm());
422-
}
423-
424-
return array($key, null);
404+
return $keyOrKeyArray[$kid];
425405
}
426406

427407
throw new UnexpectedValueException(
428-
'$keyOrKeyArray must be a string|resource key, an array of string|resource keys, '
429-
. 'an instance of Firebase\JWT\Key key or an array of Firebase\JWT\Key keys'
408+
'$keyOrKeyArray must be an instance of Firebase\JWT\Key key or an '
409+
. 'array of Firebase\JWT\Key keys'
430410
);
431411
}
432412

‎tests/JWKTest.php‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,26 +38,42 @@ public function testParsePrivateKey()
3838
'UnexpectedValueException',
3939
'RSA private keys are not supported'
4040
);
41-
41+
4242
$jwkSet = json_decode(
4343
file_get_contents(__DIR__ . '/rsa-jwkset.json'),
4444
true
4545
);
4646
$jwkSet['keys'][0]['d'] = 'privatekeyvalue';
47-
47+
4848
JWK::parseKeySet($jwkSet);
4949
}
50-
50+
51+
public function testParsePrivateKeyWithoutAlg()
52+
{
53+
$this->setExpectedException(
54+
'InvalidArgumentException',
55+
'JWK key is missing "alg"'
56+
);
57+
58+
$jwkSet = json_decode(
59+
file_get_contents(__DIR__ . '/rsa-jwkset.json'),
60+
true
61+
);
62+
unset($jwkSet['keys'][0]['alg']);
63+
64+
JWK::parseKeySet($jwkSet);
65+
}
66+
5167
public function testParseKeyWithEmptyDValue()
5268
{
5369
$jwkSet = json_decode(
5470
file_get_contents(__DIR__ . '/rsa-jwkset.json'),
5571
true
5672
);
57-
73+
5874
// empty or null values are ok
5975
$jwkSet['keys'][0]['d'] = null;
60-
76+
6177
$keys = JWK::parseKeySet($jwkSet);
6278
$this->assertTrue(is_array($keys));
6379
}

0 commit comments

Comments
 (0)