Skip to content

Commit ffed276

Browse files
author
Chris Raynor
committed
Merge pull request firebase#35 from firebase/rd-verify-refactor
Refactor decode() code path to address security vulnerabilities
2 parents 8049f19 + ffb412e commit ffed276

File tree

3 files changed

+42
-61
lines changed

3 files changed

+42
-61
lines changed

‎Authentication/JWT.php

Lines changed: 30 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,7 @@
1515
*/
1616
class JWT
1717
{
18-
public static $only_method = 'HS256';
19-
20-
public static $methods = array(
18+
public static $supported_algs = array(
2119
'HS256' => array('hash_hmac', 'SHA256'),
2220
'HS512' => array('hash_hmac', 'SHA512'),
2321
'HS384' => array('hash_hmac', 'SHA384'),
@@ -27,9 +25,9 @@ class JWT
2725
/**
2826
* Decodes a JWT string into a PHP object.
2927
*
30-
* @param string $jwt The JWT
31-
* @param string|Array|null $key The secret key, or map of keys
32-
* @param bool $verify Don't skip verification process
28+
* @param string $jwt The JWT
29+
* @param string|Array|null $key The secret key, or map of keys
30+
* @param Array $allowed_algs List of supported verification algorithms
3331
*
3432
* @return object The JWT's payload as a PHP object
3533
*
@@ -43,7 +41,7 @@ class JWT
4341
* @uses jsonDecode
4442
* @uses urlsafeB64Decode
4543
*/
46-
public static function decode($jwt, $key = null, $verify = true)
44+
public static function decode($jwt, $key = null, $allowed_algs = array())
4745
{
4846
$tks = explode('.', $jwt);
4947
if (count($tks) != 3) {
@@ -57,10 +55,16 @@ public static function decode($jwt, $key = null, $verify = true)
5755
throw new UnexpectedValueException('Invalid claims encoding');
5856
}
5957
$sig = JWT::urlsafeB64Decode($cryptob64);
60-
if ($verify) {
58+
if (isset($key)) {
6159
if (empty($header->alg)) {
6260
throw new DomainException('Empty algorithm');
6361
}
62+
if (empty(self::$supported_algs[$header->alg])) {
63+
throw new DomainException('Algorithm not supported');
64+
}
65+
if (!is_array($allowed_algs) || !in_array($header->alg, $allowed_algs)) {
66+
throw new DomainException('Algorithm not allowed');
67+
}
6468
if (is_array($key)) {
6569
if (isset($header->kid)) {
6670
$key = $key[$header->kid];
@@ -105,16 +109,16 @@ public static function decode($jwt, $key = null, $verify = true)
105109
*
106110
* @param object|array $payload PHP object or array
107111
* @param string $key The secret key
108-
* @param string $algo The signing algorithm. Supported
112+
* @param string $alg The signing algorithm. Supported
109113
* algorithms are 'HS256', 'HS384' and 'HS512'
110114
*
111115
* @return string A signed JWT
112116
* @uses jsonEncode
113117
* @uses urlsafeB64Encode
114118
*/
115-
public static function encode($payload, $key, $algo = 'HS256', $keyId = null)
119+
public static function encode($payload, $key, $alg = 'HS256', $keyId = null)
116120
{
117-
$header = array('typ' => 'JWT', 'alg' => $algo);
121+
$header = array('typ' => 'JWT', 'alg' => $alg);
118122
if ($keyId !== null) {
119123
$header['kid'] = $keyId;
120124
}
@@ -123,7 +127,7 @@ public static function encode($payload, $key, $algo = 'HS256', $keyId = null)
123127
$segments[] = JWT::urlsafeB64Encode(JWT::jsonEncode($payload));
124128
$signing_input = implode('.', $segments);
125129

126-
$signature = JWT::sign($signing_input, $key, $algo);
130+
$signature = JWT::sign($signing_input, $key, $alg);
127131
$segments[] = JWT::urlsafeB64Encode($signature);
128132

129133
return implode('.', $segments);
@@ -134,24 +138,24 @@ public static function encode($payload, $key, $algo = 'HS256', $keyId = null)
134138
*
135139
* @param string $msg The message to sign
136140
* @param string|resource $key The secret key
137-
* @param string $method The signing algorithm. Supported algorithms
141+
* @param string $alg The signing algorithm. Supported algorithms
138142
* are 'HS256', 'HS384', 'HS512' and 'RS256'
139143
*
140144
* @return string An encrypted message
141145
* @throws DomainException Unsupported algorithm was specified
142146
*/
143-
public static function sign($msg, $key, $method = 'HS256')
147+
public static function sign($msg, $key, $alg = 'HS256')
144148
{
145-
if (empty(self::$methods[$method])) {
149+
if (empty(self::$supported_algs[$alg])) {
146150
throw new DomainException('Algorithm not supported');
147151
}
148-
list($function, $algo) = self::$methods[$method];
152+
list($function, $algorithm) = self::$supported_algs[$alg];
149153
switch($function) {
150154
case 'hash_hmac':
151-
return hash_hmac($algo, $msg, $key, true);
155+
return hash_hmac($algorithm, $msg, $key, true);
152156
case 'openssl':
153157
$signature = '';
154-
$success = openssl_sign($msg, $signature, $key, $algo);
158+
$success = openssl_sign($msg, $signature, $key, $algorithm);
155159
if (!$success) {
156160
throw new DomainException("OpenSSL unable to sign data");
157161
} else {
@@ -166,32 +170,28 @@ public static function sign($msg, $key, $method = 'HS256')
166170
* @param string $msg the original message
167171
* @param string $signature
168172
* @param string|resource $key for HS*, a string key works. for RS*, must be a resource of an openssl public key
169-
* @param string $method
173+
* @param string $alg
170174
* @return bool
171175
* @throws DomainException Invalid Algorithm or OpenSSL failure
172176
*/
173-
public static function verify($msg, $signature, $key, $method = 'HS256')
177+
private static function verify($msg, $signature, $key, $alg)
174178
{
175-
if (empty(self::$methods[$method])) {
179+
if (empty(self::$supported_algs[$alg])) {
176180
throw new DomainException('Algorithm not supported');
177181
}
178-
if (self::$only_method === null) {
179-
throw new DomainException('Algorithm not specified');
180-
} elseif ($method !== self::$only_method) {
181-
throw new DomainException('Incorrect algorithm error');
182-
}
183-
list($function, $algo) = self::$methods[$method];
182+
183+
list($function, $algorithm) = self::$supported_algs[$alg];
184184
switch($function) {
185185
case 'openssl':
186-
$success = openssl_verify($msg, $signature, $key, $algo);
186+
$success = openssl_verify($msg, $signature, $key, $algorithm);
187187
if (!$success) {
188188
throw new DomainException("OpenSSL unable to verify data: " . openssl_error_string());
189189
} else {
190190
return $signature;
191191
}
192192
case 'hash_hmac':
193193
default:
194-
$hash = hash_hmac($algo, $msg, $key, true);
194+
$hash = hash_hmac($algorithm, $msg, $key, true);
195195
if (function_exists('hash_equals')) {
196196
return hash_equals($signature, $hash);
197197
}
@@ -309,7 +309,7 @@ private static function handleJsonError($errno)
309309
: 'Unknown JSON error: ' . $errno
310310
);
311311
}
312-
312+
313313
/**
314314
* Get the number of bytes in cryptographic strings.
315315
*
@@ -323,22 +323,4 @@ private static function safeStrlen($str)
323323
}
324324
return strlen($str);
325325
}
326-
327-
/**
328-
* Set the only allowed method for this server.
329-
*
330-
* @ref https://auth0.com/blog/2015/03/31/critical-vulnerabilities-in-json-web-token-libraries/
331-
*
332-
* @param string $method array index in self::$methods
333-
*
334-
* @return boolean
335-
*/
336-
public static function setOnlyAllowedMethod($method)
337-
{
338-
if (!empty(self::$methods[$method])) {
339-
self::$only_method = $method;
340-
return true;
341-
}
342-
return false;
343-
}
344326
}

‎run-tests.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ if [ $? -eq 0 ]; then
2626
echo
2727
echo -e "\033[33mBegin Unit Testing\033[0m"
2828
# Run the testing suite
29+
php --version
2930
php phpunit.phar --configuration phpunit.xml.dist
3031
else
3132
echo

‎tests/JWTTest.php

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,22 +5,22 @@ class JWTTest extends PHPUnit_Framework_TestCase
55
public function testEncodeDecode()
66
{
77
$msg = JWT::encode('abc', 'my_key');
8-
$this->assertEquals(JWT::decode($msg, 'my_key'), 'abc');
8+
$this->assertEquals(JWT::decode($msg, 'my_key', array('HS256')), 'abc');
99
}
1010

1111
public function testDecodeFromPython()
1212
{
1313
$msg = 'eyJhbGciOiAiSFMyNTYiLCAidHlwIjogIkpXVCJ9.Iio6aHR0cDovL2FwcGxpY2F0aW9uL2NsaWNreT9ibGFoPTEuMjMmZi5vbz00NTYgQUMwMDAgMTIzIg.E_U8X2YpMT5K1cEiT_3-IvBYfrdIFIeVYeOqre_Z5Cg';
1414
$this->assertEquals(
15-
JWT::decode($msg, 'my_key'),
15+
JWT::decode($msg, 'my_key', array('HS256')),
1616
'*:http://application/clicky?blah=1.23&f.oo=456 AC000 123'
1717
);
1818
}
1919

2020
public function testUrlSafeCharacters()
2121
{
2222
$encoded = JWT::encode('f?', 'a');
23-
$this->assertEquals('f?', JWT::decode($encoded, 'a'));
23+
$this->assertEquals('f?', JWT::decode($encoded, 'a', array('HS256')));
2424
}
2525

2626
public function testMalformedUtf8StringsFail()
@@ -42,7 +42,7 @@ public function testExpiredToken()
4242
"message" => "abc",
4343
"exp" => time() - 20); // time in the past
4444
$encoded = JWT::encode($payload, 'my_key');
45-
JWT::decode($encoded, 'my_key');
45+
JWT::decode($encoded, 'my_key', array('HS256'));
4646
}
4747

4848
public function testBeforeValidTokenWithNbf()
@@ -52,7 +52,7 @@ public function testBeforeValidTokenWithNbf()
5252
"message" => "abc",
5353
"nbf" => time() + 20); // time in the future
5454
$encoded = JWT::encode($payload, 'my_key');
55-
JWT::decode($encoded, 'my_key');
55+
JWT::decode($encoded, 'my_key', array('HS256'));
5656
}
5757

5858
public function testBeforeValidTokenWithIat()
@@ -62,7 +62,7 @@ public function testBeforeValidTokenWithIat()
6262
"message" => "abc",
6363
"iat" => time() + 20); // time in the future
6464
$encoded = JWT::encode($payload, 'my_key');
65-
JWT::decode($encoded, 'my_key');
65+
JWT::decode($encoded, 'my_key', array('HS256'));
6666
}
6767

6868
public function testValidToken()
@@ -71,7 +71,7 @@ public function testValidToken()
7171
"message" => "abc",
7272
"exp" => time() + 20); // time in the future
7373
$encoded = JWT::encode($payload, 'my_key');
74-
$decoded = JWT::decode($encoded, 'my_key');
74+
$decoded = JWT::decode($encoded, 'my_key', array('HS256'));
7575
$this->assertEquals($decoded->message, 'abc');
7676
}
7777

@@ -83,7 +83,7 @@ public function testValidTokenWithNbf()
8383
"exp" => time() + 20, // time in the future
8484
"nbf" => time() - 20);
8585
$encoded = JWT::encode($payload, 'my_key');
86-
$decoded = JWT::decode($encoded, 'my_key');
86+
$decoded = JWT::decode($encoded, 'my_key', array('HS256'));
8787
$this->assertEquals($decoded->message, 'abc');
8888
}
8989

@@ -94,28 +94,26 @@ public function testInvalidToken()
9494
"exp" => time() + 20); // time in the future
9595
$encoded = JWT::encode($payload, 'my_key');
9696
$this->setExpectedException('SignatureInvalidException');
97-
$decoded = JWT::decode($encoded, 'my_key2');
97+
$decoded = JWT::decode($encoded, 'my_key2', array('HS256'));
9898
}
9999

100100
public function testRSEncodeDecode()
101101
{
102102
$privKey = openssl_pkey_new(array('digest_alg' => 'sha256',
103103
'private_key_bits' => 1024,
104104
'private_key_type' => OPENSSL_KEYTYPE_RSA));
105-
JWT::setOnlyAllowedMethod('RS256');
106105
$msg = JWT::encode('abc', $privKey, 'RS256');
107106
$pubKey = openssl_pkey_get_details($privKey);
108107
$pubKey = $pubKey['key'];
109-
$decoded = JWT::decode($msg, $pubKey, true);
108+
$decoded = JWT::decode($msg, $pubKey, array('RS256'));
110109
$this->assertEquals($decoded, 'abc');
111110
}
112111

113112
public function testKIDChooser()
114113
{
115114
$keys = array('1' => 'my_key', '2' => 'my_key2');
116-
JWT::setOnlyAllowedMethod('HS256');
117115
$msg = JWT::encode('abc', $keys['1'], 'HS256', '1');
118-
$decoded = JWT::decode($msg, $keys, true);
116+
$decoded = JWT::decode($msg, $keys, array('HS256'));
119117
$this->assertEquals($decoded, 'abc');
120118
}
121119
}

0 commit comments

Comments
 (0)