Skip to content

Commit 981506b

Browse files
bshafferbshaffer
committed
feat: require "Key" object when decoding JWTs
1 parent e08160a commit 981506b

24 files changed

+204
-417
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: 34 additions & 52 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;
@@ -61,9 +59,9 @@ class JWT
6159
* Decodes a JWT string into a PHP object.
6260
*
6361
* @param string $jwt The JWT
64-
* @param string|array|resource $key The key, or map of keys.
62+
* @param Key|array<Key> $key The Key or array of Key objects.
6563
* If the algorithm used is asymmetric, this is the public key
66-
* @param array $allowed_algs List of supported verification algorithms
64+
* Each Key object contains an algorithm and matching key.
6765
* Supported algorithms are 'ES384','ES256', 'HS256', 'HS384',
6866
* 'HS512', 'RS256', 'RS384', and 'RS512'
6967
*
@@ -79,13 +77,10 @@ class JWT
7977
* @uses jsonDecode
8078
* @uses urlsafeB64Decode
8179
*/
82-
public static function decode($jwt, $key, array $allowed_algs = array())
80+
public static function decode($jwt, $keyOrArrayOfKeys)
8381
{
82+
// Validate JWT
8483
$timestamp = \is_null(static::$timestamp) ? \time() : static::$timestamp;
85-
86-
if (empty($key)) {
87-
throw new InvalidArgumentException('Key may not be empty');
88-
}
8984
$tks = \explode('.', $jwt);
9085
if (\count($tks) != 3) {
9186
throw new UnexpectedValueException('Wrong number of segments');
@@ -106,35 +101,45 @@ public static function decode($jwt, $key, array $allowed_algs = array())
106101
if (empty(static::$supported_algs[$header->alg])) {
107102
throw new UnexpectedValueException('Algorithm not supported');
108103
}
109-
if (!\in_array($header->alg, $allowed_algs)) {
110-
throw new UnexpectedValueException('Algorithm not allowed');
111-
}
112-
if ($header->alg === 'ES256' || $header->alg === 'ES384') {
113-
// OpenSSL expects an ASN.1 DER sequence for ES256/ES384 signatures
114-
$sig = self::signatureToDER($sig);
104+
105+
// Validate the key
106+
if (empty($keyOrArrayOfKeys)) {
107+
throw new InvalidArgumentException('$keyOrArrayOfKeys may not be empty');
115108
}
116109

117-
/** @var Keyring|JWTKey $key */
118-
$key = self::getKeyType($key, $allowed_algs);
119-
if ($key instanceof Keyring) {
120-
if (isset($header->kid)) {
121-
if (!isset($key[$header->kid])) {
122-
throw new UnexpectedValueException('"kid" invalid, unable to lookup correct key');
110+
if ($keyOrArrayOfKeys instanceof Key) {
111+
$key = $keyOrArrayOfKeys;
112+
} elseif (is_array($keyOrArrayOfKeys) || $keyOrArrayOfKeys instanceof ArrayAccess) {
113+
foreach ($keyOrArrayOfKeys as $kid => $key) {
114+
if (!$key instanceof Key) {
115+
throw new InvalidArgumentException('Type error: array values in $keyOrArrayOfKeys must be instances of Key');
123116
}
124-
$key = $key[$header->kid];
125-
} else {
117+
}
118+
119+
if (!isset($header->kid)) {
126120
throw new UnexpectedValueException('"kid" empty, unable to lookup correct key');
127121
}
128-
}
129-
if (!($key instanceof JWTKey)) {
130-
throw new UnexpectedValueException('$key should be an instance of JWTKey');
122+
if (!isset($keyOrArrayOfKeys[$header->kid])) {
123+
throw new UnexpectedValueException('"kid" invalid, unable to lookup correct key');
124+
}
125+
$key = $keyOrArrayOfKeys[$header->kid];
126+
} else {
127+
throw new UnexpectedValueException(
128+
'$keyOrArrayOfKeys must be an instance of Firebase\JWT\Key or an array of Firebase\JWT\Key objects'
129+
);
131130
}
132131

133-
// Check the signature
134-
if (!$key->isValidForAlg($header->alg)) {
132+
// Check the algorithm
133+
if (!self::constantTimeEquals($key->getAlgorithm(), $header->alg)) {
135134
// See issue #351
136135
throw new UnexpectedValueException('Incorrect key for this algorithm');
137136
}
137+
138+
// Check the signature
139+
if ($header->alg === 'ES256' || $header->alg === 'ES384') {
140+
// OpenSSL expects an ASN.1 DER sequence for ES256/ES384 signatures
141+
$sig = self::signatureToDER($sig);
142+
}
138143
if (!static::verify("$headb64.$bodyb64", $sig, $key->getKeyMaterial(), $header->alg)) {
139144
throw new SignatureInvalidException('Signature verification failed');
140145
}
@@ -406,29 +411,6 @@ public static function constantTimeEquals($left, $right)
406411
return ($status === 0);
407412
}
408413

409-
/**
410-
* @param string|array|ArrayAccess $oldType
411-
* @param string[] $algs
412-
* @return KeyInterface
413-
*/
414-
public static function getKeyType($oldType, $algs)
415-
{
416-
if ($oldType instanceof KeyInterface) {
417-
return $oldType;
418-
}
419-
if (is_string($oldType)) {
420-
return new JWTKey($oldType, $algs);
421-
}
422-
if (is_array($oldType) || $oldType instanceof ArrayAccess) {
423-
$keyring = new Keyring(array());
424-
foreach ($oldType as $kid => $key) {
425-
$keyring[$kid] = new JWTKey($key, $algs);
426-
}
427-
return $keyring;
428-
}
429-
throw new InvalidArgumentException('Invalid type: Must be string or array');
430-
}
431-
432414
/**
433415
* Helper method to create a JSON error.
434416
*
@@ -459,7 +441,7 @@ private static function handleJsonError($errno)
459441
*
460442
* @return int
461443
*/
462-
public static function safeStrlen($str)
444+
private static function safeStrlen($str)
463445
{
464446
if (\function_exists('mb_strlen')) {
465447
return \mb_strlen($str, '8bit');

‎src/Key.php

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
<?php
2+
3+
namespace Firebase\JWT;
4+
5+
use InvalidArgumentException;
6+
7+
class Key
8+
{
9+
/** @var string $algorithm */
10+
private $algorithm;
11+
12+
/** @var string $keyMaterial */
13+
private $keyMaterial;
14+
15+
/**
16+
* @param string|resource $keyMaterial
17+
* @param string $algorithm
18+
*/
19+
public function __construct($keyMaterial, $algorithm)
20+
{
21+
if (!(is_string($keyMaterial) || is_resource($keyMaterial)) || empty($keyMaterial)) {
22+
throw new InvalidArgumentException('Type error: $keyMaterial must be a string');
23+
}
24+
if (!is_string($algorithm)|| empty($keyMaterial)) {
25+
throw new InvalidArgumentException('Type error: $algorithm must be a string');
26+
}
27+
$this->keyMaterial = $keyMaterial;
28+
$this->algorithm = $algorithm;
29+
}
30+
31+
public function __toString()
32+
{
33+
return $this->keyMaterial;
34+
}
35+
36+
/**
37+
* Return the algorithm valid for this key
38+
*
39+
* @return string
40+
*/
41+
public function getAlgorithm()
42+
{
43+
return $this->algorithm;
44+
}
45+
46+
/**
47+
* @return string
48+
*/
49+
public function getKeyMaterial()
50+
{
51+
return $this->keyMaterial;
52+
}
53+
}

‎src/Keys/JWTKey.php

Lines changed: 0 additions & 113 deletions
This file was deleted.

0 commit comments

Comments
 (0)