Skip to content

fix: require key object #373

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 9, 2021
Merged

fix: require key object #373

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,7 @@ $jwks = ['keys' => []];

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

Changelog
Expand Down
10 changes: 9 additions & 1 deletion src/JWK.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,15 @@ public static function parseKeySet(array $jwks)
foreach ($jwks['keys'] as $k => $v) {
$kid = isset($v['kid']) ? $v['kid'] : $k;
if ($key = self::parseKey($v)) {
$keys[$kid] = $key;
if (isset($v['alg'])) {
$keys[$kid] = new Key($key, $v['alg']);
} else {
// The "alg" parameter is optional in a KTY, but is required
// for parsing in this library. Add it manually to your JWK
// array if it doesn't already exist.
// @see https://datatracker.ietf.org/doc/html/rfc7517#section-4.4
throw new InvalidArgumentException('JWK key is missing "alg"');
}
}
}

Expand Down
64 changes: 22 additions & 42 deletions src/JWT.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,11 @@ class JWT
* Decodes a JWT string into a PHP object.
*
* @param string $jwt The JWT
* @param Key|array<Key>|mixed $keyOrKeyArray The Key or array of Key objects.
* @param Key|array<Key> $keyOrKeyArray The Key or array of Key objects.
* If the algorithm used is asymmetric, this is the public key
* Each Key object contains an algorithm and matching key.
* Supported algorithms are 'ES384','ES256', 'HS256', 'HS384',
* 'HS512', 'RS256', 'RS384', and 'RS512'
* @param array $allowed_algs [DEPRECATED] List of supported verification algorithms. Only
* should be used for backwards compatibility.
*
* @return object The JWT's payload as a PHP object
*
Expand All @@ -80,8 +78,9 @@ class JWT
* @uses jsonDecode
* @uses urlsafeB64Decode
*/
public static function decode($jwt, $keyOrKeyArray, array $allowed_algs = array())
public static function decode($jwt, $keyOrKeyArray)
{
// Validate JWT
$timestamp = \is_null(static::$timestamp) ? \time() : static::$timestamp;

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

list($keyMaterial, $algorithm) = self::getKeyMaterialAndAlgorithm(
$keyOrKeyArray,
empty($header->kid) ? null : $header->kid
);
$key = self::getKey($keyOrKeyArray, empty($header->kid) ? null : $header->kid);

if (empty($algorithm)) {
// Use deprecated "allowed_algs" to determine if the algorithm is supported.
// This opens up the possibility of an attack in some implementations.
// @see https://github.com/firebase/php-jwt/issues/351
if (!\in_array($header->alg, $allowed_algs)) {
throw new UnexpectedValueException('Algorithm not allowed');
}
} else {
// Check the algorithm
if (!self::constantTimeEquals($algorithm, $header->alg)) {
// See issue #351
throw new UnexpectedValueException('Incorrect key for this algorithm');
}
// Check the algorithm
if (!self::constantTimeEquals($key->getAlgorithm(), $header->alg)) {
// See issue #351
throw new UnexpectedValueException('Incorrect key for this algorithm');
}
if ($header->alg === 'ES256' || $header->alg === 'ES384') {
// OpenSSL expects an ASN.1 DER sequence for ES256/ES384 signatures
$sig = self::signatureToDER($sig);
}

if (!static::verify("$headb64.$bodyb64", $sig, $keyMaterial, $header->alg)) {
if (!static::verify("$headb64.$bodyb64", $sig, $key->getKeyMaterial(), $header->alg)) {
throw new SignatureInvalidException('Signature verification failed');
}

Expand Down Expand Up @@ -393,40 +379,34 @@ public static function urlsafeB64Encode($input)
*
* @return array containing the keyMaterial and algorithm
*/
private static function getKeyMaterialAndAlgorithm($keyOrKeyArray, $kid = null)
private static function getKey($keyOrKeyArray, $kid = null)
{
if (
is_string($keyOrKeyArray)
|| is_resource($keyOrKeyArray)
|| $keyOrKeyArray instanceof OpenSSLAsymmetricKey
) {
return array($keyOrKeyArray, null);
}

if ($keyOrKeyArray instanceof Key) {
return array($keyOrKeyArray->getKeyMaterial(), $keyOrKeyArray->getAlgorithm());
return $keyOrKeyArray;
}

if (is_array($keyOrKeyArray) || $keyOrKeyArray instanceof ArrayAccess) {
foreach ($keyOrKeyArray as $keyId => $key) {
if (!$key instanceof Key) {
throw new UnexpectedValueException(
'$keyOrKeyArray must be an instance of Firebase\JWT\Key key or an '
. 'array of Firebase\JWT\Key keys'
);
}
}
if (!isset($kid)) {
throw new UnexpectedValueException('"kid" empty, unable to lookup correct key');
}
if (!isset($keyOrKeyArray[$kid])) {
throw new UnexpectedValueException('"kid" invalid, unable to lookup correct key');
}

$key = $keyOrKeyArray[$kid];

if ($key instanceof Key) {
return array($key->getKeyMaterial(), $key->getAlgorithm());
}

return array($key, null);
return $keyOrKeyArray[$kid];
}

throw new UnexpectedValueException(
'$keyOrKeyArray must be a string|resource key, an array of string|resource keys, '
. 'an instance of Firebase\JWT\Key key or an array of Firebase\JWT\Key keys'
'$keyOrKeyArray must be an instance of Firebase\JWT\Key key or an '
. 'array of Firebase\JWT\Key keys'
);
}

Expand Down
26 changes: 21 additions & 5 deletions tests/JWKTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,26 +38,42 @@ public function testParsePrivateKey()
'UnexpectedValueException',
'RSA private keys are not supported'
);

$jwkSet = json_decode(
file_get_contents(__DIR__ . '/rsa-jwkset.json'),
true
);
$jwkSet['keys'][0]['d'] = 'privatekeyvalue';

JWK::parseKeySet($jwkSet);
}


public function testParsePrivateKeyWithoutAlg()
{
$this->setExpectedException(
'InvalidArgumentException',
'JWK key is missing "alg"'
);

$jwkSet = json_decode(
file_get_contents(__DIR__ . '/rsa-jwkset.json'),
true
);
unset($jwkSet['keys'][0]['alg']);

JWK::parseKeySet($jwkSet);
}

public function testParseKeyWithEmptyDValue()
{
$jwkSet = json_decode(
file_get_contents(__DIR__ . '/rsa-jwkset.json'),
true
);

// empty or null values are ok
$jwkSet['keys'][0]['d'] = null;

$keys = JWK::parseKeySet($jwkSet);
$this->assertTrue(is_array($keys));
}
Expand Down
Loading