Skip to content

Commit c6964eb

Browse files
bshafferbshaffer
committed
feat: require "Key" object when decoding JWTs
1 parent 65920fd commit c6964eb

21 files changed

+140
-151
lines changed

‎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 y our 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 & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55
use ArrayAccess;
66
use DomainException;
77
use Exception;
8-
use Firebase\JWT\Keys\JWTKey;
9-
use Firebase\JWT\Keys\Keyring;
108
use InvalidArgumentException;
119
use UnexpectedValueException;
1210
use DateTime;
@@ -81,8 +79,9 @@ class JWT
8179
* @uses jsonDecode
8280
* @uses urlsafeB64Decode
8381
*/
84-
public static function decode($jwt, $keyOrKeyArray, array $allowed_algs = array())
82+
public static function decode($jwt, $keyOrKeyArray)
8583
{
84+
// Validate JWT
8685
$timestamp = \is_null(static::$timestamp) ? \time() : static::$timestamp;
8786

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

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

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

@@ -391,36 +377,34 @@ public static function urlsafeB64Encode($input)
391377
*
392378
* @return an array containing the keyMaterial and algorithm
393379
*/
394-
private static function getKeyMaterialAndAlgorithm($keyOrKeyArray, $kid = null)
380+
private static function getKey($keyOrKeyArray, $kid = null)
395381
{
396-
if (is_string($keyOrKeyArray)) {
397-
return array($keyOrKeyArray, null);
398-
}
399-
400382
if ($keyOrKeyArray instanceof Key) {
401-
return array($keyOrKeyArray->getKeyMaterial(), $keyOrKeyArray->getAlgorithm());
383+
return $keyOrKeyArray;
402384
}
403385

404386
if (is_array($keyOrKeyArray) || $keyOrKeyArray instanceof ArrayAccess) {
387+
foreach ($keyOrKeyArray as $keyId => $key) {
388+
if (!$key instanceof Key) {
389+
throw new UnexpectedValueException(
390+
'$keyOrKeyArray must be an instance of Firebase\JWT\Key key or an '
391+
. 'array of Firebase\JWT\Key keys'
392+
);
393+
}
394+
}
405395
if (!isset($kid)) {
406396
throw new UnexpectedValueException('"kid" empty, unable to lookup correct key');
407397
}
408398
if (!isset($keyOrKeyArray[$kid])) {
409399
throw new UnexpectedValueException('"kid" invalid, unable to lookup correct key');
410400
}
411401

412-
$key = $keyOrKeyArray[$kid];
413-
414-
if ($key instanceof Key) {
415-
return array($key->getKeyMaterial(), $key->getAlgorithm());
416-
}
417-
418-
return array($key, null);
402+
return $keyOrKeyArray[$kid];
419403
}
420404

421405
throw new UnexpectedValueException(
422-
'$keyOrKeyArray must be a string key, an array of string keys, '
423-
. 'an instance of Firebase\JWT\Key key or an array of Firebase\JWT\Key keys'
406+
'$keyOrKeyArray must be an instance of Firebase\JWT\Key key or an '
407+
. 'array of Firebase\JWT\Key keys'
424408
);
425409
}
426410

@@ -475,7 +459,7 @@ private static function handleJsonError($errno)
475459
*
476460
* @return int
477461
*/
478-
public static function safeStrlen($str)
462+
private static function safeStrlen($str)
479463
{
480464
if (\function_exists('mb_strlen')) {
481465
return \mb_strlen($str, '8bit');

‎src/Key.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public function getAlgorithm()
4949
}
5050

5151
/**
52-
* @return string|resource
52+
* @return string|resource|OpenSSLAsymmetricKey
5353
*/
5454
public function getKeyMaterial()
5555
{

‎tests/JWKTest.php

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -38,34 +38,50 @@ public function testParsePrivateKey()
3838
'UnexpectedValueException',
3939
'RSA private keys are not supported'
4040
);
41-
41+
4242
$jwkSet = json_decode(
43-
file_get_contents(__DIR__ . '/rsa-jwkset.json'),
43+
file_get_contents(__DIR__ . '/data/rsa-jwkset.json'),
4444
true
4545
);
4646
$jwkSet['keys'][0]['d'] = 'privatekeyvalue';
47-
47+
48+
JWK::parseKeySet($jwkSet);
49+
}
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__ . '/data/rsa-jwkset.json'),
60+
true
61+
);
62+
unset($jwkSet['keys'][0]['alg']);
63+
4864
JWK::parseKeySet($jwkSet);
4965
}
50-
66+
5167
public function testParseKeyWithEmptyDValue()
5268
{
5369
$jwkSet = json_decode(
54-
file_get_contents(__DIR__ . '/rsa-jwkset.json'),
70+
file_get_contents(__DIR__ . '/data/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
}
6480

6581
public function testParseJwkKeySet()
6682
{
6783
$jwkSet = json_decode(
68-
file_get_contents(__DIR__ . '/rsa-jwkset.json'),
84+
file_get_contents(__DIR__ . '/data/rsa-jwkset.json'),
6985
true
7086
);
7187
$keys = JWK::parseKeySet($jwkSet);
@@ -93,7 +109,7 @@ public function testParseJwkKeySet_empty()
93109
*/
94110
public function testDecodeByJwkKeySetTokenExpired()
95111
{
96-
$privKey1 = file_get_contents(__DIR__ . '/rsa1-private.pem');
112+
$privKey1 = file_get_contents(__DIR__ . '/data/rsa1-private.pem');
97113
$payload = array('exp' => strtotime('-1 hour'));
98114
$msg = JWT::encode($payload, $privKey1, 'RS256', 'jwk1');
99115

@@ -107,7 +123,7 @@ public function testDecodeByJwkKeySetTokenExpired()
107123
*/
108124
public function testDecodeByJwkKeySet()
109125
{
110-
$privKey1 = file_get_contents(__DIR__ . '/rsa1-private.pem');
126+
$privKey1 = file_get_contents(__DIR__ . '/data/rsa1-private.pem');
111127
$payload = array('sub' => 'foo', 'exp' => strtotime('+10 seconds'));
112128
$msg = JWT::encode($payload, $privKey1, 'RS256', 'jwk1');
113129

@@ -121,7 +137,7 @@ public function testDecodeByJwkKeySet()
121137
*/
122138
public function testDecodeByMultiJwkKeySet()
123139
{
124-
$privKey2 = file_get_contents(__DIR__ . '/rsa2-private.pem');
140+
$privKey2 = file_get_contents(__DIR__ . '/data/rsa2-private.pem');
125141
$payload = array('sub' => 'bar', 'exp' => strtotime('+10 seconds'));
126142
$msg = JWT::encode($payload, $privKey2, 'RS256', 'jwk2');
127143

0 commit comments

Comments
 (0)