Skip to content

Commit

Permalink
feat!: require Key object, use JSON_UNESCAPED_SLASHES, remove constan…
Browse files Browse the repository at this point in the history 
…ts (#376)
  • Loading branch information
@bshaffer
bshaffer authored Dec 30, 2021
1 parent fbe6394 commit edda0f9
Show file tree
Hide file tree
Showing 19 changed files with 161 additions and 190 deletions.
1 change: 1 addition & 0 deletions .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ phpunit.phar
phpunit.phar.asc
composer.phar
composer.lock
.phpunit.result.cache
3 changes: 1 addition & 2 deletions README.md
View file
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
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
94 changes: 41 additions & 53 deletions src/JWT.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,12 @@
*/
class JWT
{
const ASN1_INTEGER = 0x02;
const ASN1_SEQUENCE = 0x10;
const ASN1_BIT_STRING = 0x03;
// const ASN1_INTEGER = 0x02;
// const ASN1_SEQUENCE = 0x10;
// const ASN1_BIT_STRING = 0x03;
private static $asn1Integer = 0x02;
private static $asn1Sequence = 0x10;
private static $asn1BitString = 0x03;

/**
* When checking nbf, iat or expiration times,
Expand Down Expand Up @@ -60,13 +63,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 @@ -81,8 +82,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 @@ -109,31 +111,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 @@ -179,7 +168,7 @@ public static function decode($jwt, $keyOrKeyArray, array $allowed_algs = array(
* @uses jsonEncode
* @uses urlsafeB64Encode
*/
public static function encode($payload, $key, $alg = 'HS256', $keyId = null, $head = null)
public static function encode($payload, $key, $alg, $keyId = null, $head = null)
{
$header = array('typ' => 'JWT', 'alg' => $alg);
if ($keyId !== null) {
Expand Down Expand Up @@ -212,7 +201,7 @@ public static function encode($payload, $key, $alg = 'HS256', $keyId = null, $he
*
* @throws DomainException Unsupported algorithm or bad key was specified
*/
public static function sign($msg, $key, $alg = 'HS256')
public static function sign($msg, $key, $alg)
{
if (empty(static::$supported_algs[$alg])) {
throw new DomainException('Algorithm not supported');
Expand Down Expand Up @@ -345,7 +334,12 @@ public static function jsonDecode($input)
*/
public static function jsonEncode($input)
{
$json = \json_encode($input);
if (PHP_VERSION_ID >= 50400) {
$json = \json_encode($input, \JSON_UNESCAPED_SLASHES);
} else {
// PHP 5.3 only
$json = \json_encode($input);
}
if ($errno = \json_last_error()) {
static::handleJsonError($errno);
} elseif ($json === 'null' && $input !== null) {
Expand Down Expand Up @@ -394,40 +388,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 Expand Up @@ -515,9 +503,9 @@ private static function signatureToDER($sig)
}

return self::encodeDER(
self::ASN1_SEQUENCE,
self::encodeDER(self::ASN1_INTEGER, $r) .
self::encodeDER(self::ASN1_INTEGER, $s)
self::$asn1Sequence,
self::encodeDER(self::$asn1Integer, $r) .
self::encodeDER(self::$asn1Integer, $s)
);
}

Expand All @@ -531,7 +519,7 @@ private static function signatureToDER($sig)
private static function encodeDER($type, $value)
{
$tag_header = 0;
if ($type === self::ASN1_SEQUENCE) {
if ($type === self::$asn1Sequence) {
$tag_header |= 0x20;
}

Expand Down Expand Up @@ -596,7 +584,7 @@ private static function readDER($der, $offset = 0)
}

// Value
if ($type == self::ASN1_BIT_STRING) {
if ($type == self::$asn1BitString) {
$pos++; // Skip the first contents octet (padding indicator)
$data = \substr($der, $pos, $len - 1);
$pos += $len - 1;
Expand Down
38 changes: 27 additions & 11 deletions tests/JWKTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,34 +38,50 @@ public function testParsePrivateKey()
'UnexpectedValueException',
'RSA private keys are not supported'
);

$jwkSet = json_decode(
file_get_contents(__DIR__ . '/rsa-jwkset.json'),
file_get_contents(__DIR__ . '/data/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__ . '/data/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'),
file_get_contents(__DIR__ . '/data/rsa-jwkset.json'),
true
);

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

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

public function testParseJwkKeySet()
{
$jwkSet = json_decode(
file_get_contents(__DIR__ . '/rsa-jwkset.json'),
file_get_contents(__DIR__ . '/data/rsa-jwkset.json'),
true
);
$keys = JWK::parseKeySet($jwkSet);
Expand Down Expand Up @@ -93,7 +109,7 @@ public function testParseJwkKeySet_empty()
*/
public function testDecodeByJwkKeySetTokenExpired()
{
$privKey1 = file_get_contents(__DIR__ . '/rsa1-private.pem');
$privKey1 = file_get_contents(__DIR__ . '/data/rsa1-private.pem');
$payload = array('exp' => strtotime('-1 hour'));
$msg = JWT::encode($payload, $privKey1, 'RS256', 'jwk1');

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

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

Expand Down
Loading

0 comments on commit edda0f9

Please sign in to comment.