Skip to content

Commit edda0f9

Browse files
bshafferbshaffer
authored
feat!: require Key object, use JSON_UNESCAPED_SLASHES, remove constants (#376)
1 parent fbe6394 commit edda0f9

19 files changed

+161
-190
lines changed

‎.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,4 @@ phpunit.phar
33
phpunit.phar.asc
44
composer.phar
55
composer.lock
6+
.phpunit.result.cache

‎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: 41 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,12 @@
2525
*/
2626
class JWT
2727
{
28-
const ASN1_INTEGER = 0x02;
29-
const ASN1_SEQUENCE = 0x10;
30-
const ASN1_BIT_STRING = 0x03;
28+
// const ASN1_INTEGER = 0x02;
29+
// const ASN1_SEQUENCE = 0x10;
30+
// const ASN1_BIT_STRING = 0x03;
31+
private static $asn1Integer = 0x02;
32+
private static $asn1Sequence = 0x10;
33+
private static $asn1BitString = 0x03;
3134

3235
/**
3336
* When checking nbf, iat or expiration times,
@@ -60,13 +63,11 @@ class JWT
6063
* Decodes a JWT string into a PHP object.
6164
*
6265
* @param string $jwt The JWT
63-
* @param Key|array<Key>|mixed $keyOrKeyArray The Key or array of Key objects.
66+
* @param Key|array<Key> $keyOrKeyArray The Key or array of Key objects.
6467
* If the algorithm used is asymmetric, this is the public key
6568
* Each Key object contains an algorithm and matching key.
6669
* Supported algorithms are 'ES384','ES256', 'HS256', 'HS384',
6770
* 'HS512', 'RS256', 'RS384', and 'RS512'
68-
* @param array $allowed_algs [DEPRECATED] List of supported verification algorithms. Only
69-
* should be used for backwards compatibility.
7071
*
7172
* @return object The JWT's payload as a PHP object
7273
*
@@ -81,8 +82,9 @@ class JWT
8182
* @uses jsonDecode
8283
* @uses urlsafeB64Decode
8384
*/
84-
public static function decode($jwt, $keyOrKeyArray, array $allowed_algs = array())
85+
public static function decode($jwt, $keyOrKeyArray)
8586
{
87+
// Validate JWT
8688
$timestamp = \is_null(static::$timestamp) ? \time() : static::$timestamp;
8789

8890
if (empty($keyOrKeyArray)) {
@@ -109,31 +111,18 @@ public static function decode($jwt, $keyOrKeyArray, array $allowed_algs = array(
109111
throw new UnexpectedValueException('Algorithm not supported');
110112
}
111113

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

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-
}
116+
// Check the algorithm
117+
if (!self::constantTimeEquals($key->getAlgorithm(), $header->alg)) {
118+
// See issue #351
119+
throw new UnexpectedValueException('Incorrect key for this algorithm');
130120
}
131121
if ($header->alg === 'ES256' || $header->alg === 'ES384') {
132122
// OpenSSL expects an ASN.1 DER sequence for ES256/ES384 signatures
133123
$sig = self::signatureToDER($sig);
134124
}
135-
136-
if (!static::verify("$headb64.$bodyb64", $sig, $keyMaterial, $header->alg)) {
125+
if (!static::verify("$headb64.$bodyb64", $sig, $key->getKeyMaterial(), $header->alg)) {
137126
throw new SignatureInvalidException('Signature verification failed');
138127
}
139128

@@ -179,7 +168,7 @@ public static function decode($jwt, $keyOrKeyArray, array $allowed_algs = array(
179168
* @uses jsonEncode
180169
* @uses urlsafeB64Encode
181170
*/
182-
public static function encode($payload, $key, $alg = 'HS256', $keyId = null, $head = null)
171+
public static function encode($payload, $key, $alg, $keyId = null, $head = null)
183172
{
184173
$header = array('typ' => 'JWT', 'alg' => $alg);
185174
if ($keyId !== null) {
@@ -212,7 +201,7 @@ public static function encode($payload, $key, $alg = 'HS256', $keyId = null, $he
212201
*
213202
* @throws DomainException Unsupported algorithm or bad key was specified
214203
*/
215-
public static function sign($msg, $key, $alg = 'HS256')
204+
public static function sign($msg, $key, $alg)
216205
{
217206
if (empty(static::$supported_algs[$alg])) {
218207
throw new DomainException('Algorithm not supported');
@@ -345,7 +334,12 @@ public static function jsonDecode($input)
345334
*/
346335
public static function jsonEncode($input)
347336
{
348-
$json = \json_encode($input);
337+
if (PHP_VERSION_ID >= 50400) {
338+
$json = \json_encode($input, \JSON_UNESCAPED_SLASHES);
339+
} else {
340+
// PHP 5.3 only
341+
$json = \json_encode($input);
342+
}
349343
if ($errno = \json_last_error()) {
350344
static::handleJsonError($errno);
351345
} elseif ($json === 'null' && $input !== null) {
@@ -394,40 +388,34 @@ public static function urlsafeB64Encode($input)
394388
*
395389
* @return array containing the keyMaterial and algorithm
396390
*/
397-
private static function getKeyMaterialAndAlgorithm($keyOrKeyArray, $kid = null)
391+
private static function getKey($keyOrKeyArray, $kid = null)
398392
{
399-
if (
400-
is_string($keyOrKeyArray)
401-
|| is_resource($keyOrKeyArray)
402-
|| $keyOrKeyArray instanceof OpenSSLAsymmetricKey
403-
) {
404-
return array($keyOrKeyArray, null);
405-
}
406-
407393
if ($keyOrKeyArray instanceof Key) {
408-
return array($keyOrKeyArray->getKeyMaterial(), $keyOrKeyArray->getAlgorithm());
394+
return $keyOrKeyArray;
409395
}
410396

411397
if (is_array($keyOrKeyArray) || $keyOrKeyArray instanceof ArrayAccess) {
398+
foreach ($keyOrKeyArray as $keyId => $key) {
399+
if (!$key instanceof Key) {
400+
throw new UnexpectedValueException(
401+
'$keyOrKeyArray must be an instance of Firebase\JWT\Key key or an '
402+
. 'array of Firebase\JWT\Key keys'
403+
);
404+
}
405+
}
412406
if (!isset($kid)) {
413407
throw new UnexpectedValueException('"kid" empty, unable to lookup correct key');
414408
}
415409
if (!isset($keyOrKeyArray[$kid])) {
416410
throw new UnexpectedValueException('"kid" invalid, unable to lookup correct key');
417411
}
418412

419-
$key = $keyOrKeyArray[$kid];
420-
421-
if ($key instanceof Key) {
422-
return array($key->getKeyMaterial(), $key->getAlgorithm());
423-
}
424-
425-
return array($key, null);
413+
return $keyOrKeyArray[$kid];
426414
}
427415

428416
throw new UnexpectedValueException(
429-
'$keyOrKeyArray must be a string|resource key, an array of string|resource keys, '
430-
. 'an instance of Firebase\JWT\Key key or an array of Firebase\JWT\Key keys'
417+
'$keyOrKeyArray must be an instance of Firebase\JWT\Key key or an '
418+
. 'array of Firebase\JWT\Key keys'
431419
);
432420
}
433421

@@ -515,9 +503,9 @@ private static function signatureToDER($sig)
515503
}
516504

517505
return self::encodeDER(
518-
self::ASN1_SEQUENCE,
519-
self::encodeDER(self::ASN1_INTEGER, $r) .
520-
self::encodeDER(self::ASN1_INTEGER, $s)
506+
self::$asn1Sequence,
507+
self::encodeDER(self::$asn1Integer, $r) .
508+
self::encodeDER(self::$asn1Integer, $s)
521509
);
522510
}
523511

@@ -531,7 +519,7 @@ private static function signatureToDER($sig)
531519
private static function encodeDER($type, $value)
532520
{
533521
$tag_header = 0;
534-
if ($type === self::ASN1_SEQUENCE) {
522+
if ($type === self::$asn1Sequence) {
535523
$tag_header |= 0x20;
536524
}
537525

@@ -596,7 +584,7 @@ private static function readDER($der, $offset = 0)
596584
}
597585

598586
// Value
599-
if ($type == self::ASN1_BIT_STRING) {
587+
if ($type == self::$asn1BitString) {
600588
$pos++; // Skip the first contents octet (padding indicator)
601589
$data = \substr($der, $pos, $len - 1);
602590
$pos += $len - 1;

‎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)