Skip to content

Add ES384 support #324

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 4 commits into from
May 19, 2021
Merged

Add ES384 support #324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1c0dbeb
ES384 support
sdrsdr Mar 29, 2021
6749a4a
Update php doc comments with the newly supported algorithm
sdrsdr Mar 29, 2021
ab093a3
Merge branch 'master' into master
bshaffer May 17, 2021
e8fa9e5
Merge branch 'master' into master
bshaffer May 19, 2021
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
17 changes: 12 additions & 5 deletions src/JWT.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class JWT
public static $timestamp = null;

public static $supported_algs = array(
'ES384' => array('openssl', 'SHA384'),
'ES256' => array('openssl', 'SHA256'),
'HS256' => array('hash_hmac', 'SHA256'),
'HS384' => array('hash_hmac', 'SHA384'),
Expand All @@ -58,7 +59,8 @@ class JWT
* @param string|array|resource $key The key, or map of keys.
* If the algorithm used is asymmetric, this is the public key
* @param array $allowed_algs List of supported verification algorithms
* Supported algorithms are 'ES256', 'HS256', 'HS384', 'HS512', 'RS256', 'RS384', and 'RS512'
* Supported algorithms are 'ES384','ES256', 'HS256', 'HS384',
* 'HS512', 'RS256', 'RS384', and 'RS512'
*
* @return object The JWT's payload as a PHP object
*
Expand Down Expand Up @@ -102,8 +104,8 @@ public static function decode($jwt, $key, array $allowed_algs = array())
if (!\in_array($header->alg, $allowed_algs)) {
throw new UnexpectedValueException('Algorithm not allowed');
}
if ($header->alg === 'ES256') {
// OpenSSL expects an ASN.1 DER sequence for ES256 signatures
if ($header->alg === 'ES256' || $header->alg === 'ES384') {
// OpenSSL expects an ASN.1 DER sequence for ES256/ES384 signatures
$sig = self::signatureToDER($sig);
}

Expand Down Expand Up @@ -155,7 +157,8 @@ public static function decode($jwt, $key, array $allowed_algs = array())
* @param string $key The secret key.
* If the algorithm used is asymmetric, this is the private key
* @param string $alg The signing algorithm.
* Supported algorithms are 'ES256', 'HS256', 'HS384', 'HS512', 'RS256', 'RS384', and 'RS512'
* Supported algorithms are 'ES384','ES256', 'HS256', 'HS384',
* 'HS512', 'RS256', 'RS384', and 'RS512'
* @param mixed $keyId
* @param array $head An array with header elements to attach
*
Expand Down Expand Up @@ -190,7 +193,8 @@ public static function encode($payload, $key, $alg = 'HS256', $keyId = null, $he
* @param string $msg The message to sign
* @param string|resource $key The secret key
* @param string $alg The signing algorithm.
* Supported algorithms are 'ES256', 'HS256', 'HS384', 'HS512', 'RS256', 'RS384', and 'RS512'
* Supported algorithms are 'ES384','ES256', 'HS256', 'HS384',
* 'HS512', 'RS256', 'RS384', and 'RS512'
*
* @return string An encrypted message
*
Expand All @@ -214,6 +218,9 @@ public static function sign($msg, $key, $alg = 'HS256')
if ($alg === 'ES256') {
$signature = self::signatureFromDER($signature, 256);
}
if ($alg === 'ES384') {
$signature = self::signatureFromDER($signature, 384);
}
return $signature;
}
}
Expand Down
16 changes: 16 additions & 0 deletions tests/JWTTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,4 +300,20 @@ public function testEncodeAndDecodeEcdsaToken()

$this->assertEquals('bar', $decoded->foo);
}

/**
* @runInSeparateProcess
*/
public function testEncodeAndDecodeEcdsa384Token()
{
$privateKey = file_get_contents(__DIR__ . '/ecdsa384-private.pem');
$payload = array('foo' => 'bar');
$encoded = JWT::encode($payload, $privateKey, 'ES384');

// Verify decoding succeeds
$publicKey = file_get_contents(__DIR__ . '/ecdsa384-public.pem');
$decoded = JWT::decode($encoded, $publicKey, array('ES384'));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I switch both occurences of ES384 above to ES256, and likewise if I switch ES256 to ES384 in the test function testEncodeAndDecodeEcdsaToken, the tests still pass. Can we use test-cases which are not interchangeable?

Also, since these two tests are so similar, it would be better to use a data provider:

    /**
     * @runInSeparateProcess
     * @dataProvider provideEncodeAndDecodeToken
     */
    public function testEncodeAndDecodeToken($privateKeyFile, $publicKeyFile, $alg)
    {
        $privateKey = file_get_contents($privateKey);
        $payload = array('foo' => 'bar');
        $encoded = JWT::encode($payload, $privateKeyFile, $alg);

        // Verify decoding succeeds
        $publicKey = file_get_contents($publicKeyFile);
        $decoded = JWT::decode($encoded, $publicKey, array($alg));

        $this->assertEquals('bar', $decoded->foo);
    }

    public function provideEncodeAndDecodeToken()
    {
        return [
            [__DIR__ . '/ecdsa-private.pem', __DIR__ . '/ecdsa-public.pem', 'ES256'],
            [__DIR__ . '/ecdsa384-private.pem', __DIR__ . '/ecdsa384-public.pem', 'ES384'],
        ];
    }

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES384 and ES256 are meant to be interchangeable the test should and will fail if you try to have only one place changed. I'm not really familiar with PHP code testing. PHP is not in my daily workset, I've long ago removed the setup on my pc needed for testing php code. Can you please merge it as is and then fix it to you liking?


$this->assertEquals('bar', $decoded->foo);
}
}
6 changes: 6 additions & 0 deletions tests/ecdsa384-private.pem
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
-----BEGIN EC PRIVATE KEY-----
MIGkAgEBBDBQJuwafREZ1494Fm2MTVXuZbWXVAOwIAxGhyLdc3CChzi0FVXZq8e6
65oR0Qq9Jv2gBwYFK4EEACKhZANiAAQWFddzIqZaROR1VtZhhTd20mqknQmYsZ+0
R03NQQUQpJTkyWcuv8WNyd6zO9cCoQEzi94kX907/OEWTjhuH8QtdunT+ef1BpWJ
W1Cm5O+m7b155/Ho99QypfQr74hLg1A=
-----END EC PRIVATE KEY-----
5 changes: 5 additions & 0 deletions tests/ecdsa384-public.pem
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
-----BEGIN PUBLIC KEY-----
MHYwEAYHKoZIzj0CAQYFK4EEACIDYgAEFhXXcyKmWkTkdVbWYYU3dtJqpJ0JmLGf
tEdNzUEFEKSU5MlnLr/FjcneszvXAqEBM4veJF/dO/zhFk44bh/ELXbp0/nn9QaV
iVtQpuTvpu29eefx6PfUMqX0K++IS4NQ
-----END PUBLIC KEY-----