Skip to content

Commit

Permalink
Merge pull request from GHSA-7322-jrq4-x5hf
Browse files Browse the repository at this point in the history
Ensure key contents is used for all hashing algorithms
  • Loading branch information
lcobucci authored Sep 28, 2021
2 parents 80784ee + 8175de5 commit 5556426
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 25 deletions.
5 changes: 1 addition & 4 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,8 @@ Or provide a file path:

```php
use Lcobucci\JWT\Signer\Key\InMemory;
use Lcobucci\JWT\Signer\Key\LocalFileReference;

$key = InMemory::file(__DIR__ . '/path-to-my-key-stored-in-a-file.pem'); // this reads the file and keeps its contents in memory
$key = LocalFileReference::file(__DIR__ . '/path-to-my-key-stored-in-a-file.pem'); // this just keeps a reference to file
```

#### For symmetric algorithms
Expand Down Expand Up @@ -78,13 +76,12 @@ This means that it's fine to distribute your **public key**. However, the **priv
```php
use Lcobucci\JWT\Configuration;
use Lcobucci\JWT\Signer;
use Lcobucci\JWT\Signer\Key\LocalFileReference;
use Lcobucci\JWT\Signer\Key\InMemory;

$configuration = Configuration::forAsymmetricSigner(
// You may use RSA or ECDSA and all their variations (256, 384, and 512)
new Signer\Rsa\Sha256(),
LocalFileReference::file(__DIR__ . '/my-private-key.pem'),
InMemory::file(__DIR__ . '/my-private-key.pem'),
InMemory::base64Encoded('mBC5v1sOKVvbdEitdSBenu59nfNfhwkedkJVNabosTw=')
// You may also override the JOSE encoder/decoder if needed by providing extra arguments here
);
Expand Down
3 changes: 1 addition & 2 deletions docs/upgrading.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,8 @@ You can find more information on how to use the configuration object, [here](con
### Use new `Key` objects

`Lcobucci\JWT\Signer\Key` has been converted to an interface in `v4.0`.
We provide two new implementations: `Lcobucci\JWT\Signer\Key\InMemory` and `Lcobucci\JWT\Signer\Key\LocalFileReference`.

`Lcobucci\JWT\Signer\Key\InMemory` is a drop-in replacement of the behaviour for `Lcobucci\JWT\Signer\Key` in `v3.x`.
We provide `Lcobucci\JWT\Signer\Key\InMemory`, a drop-in replacement of the behaviour for `Lcobucci\JWT\Signer\Key` in `v3.x`.
You will need to pick the appropriated named constructor to migrate your code:

```diff
Expand Down
1 change: 1 addition & 0 deletions phpstan.neon.dist
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ parameters:

ignoreErrors:
- '#.*OpenSSLAsymmetricKey.*#'
- '#Call to method .* of deprecated class Lcobucci\\JWT\\Signer\\Key\\LocalFileReference#'
8 changes: 7 additions & 1 deletion src/Signer/Key/LocalFileReference.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@
use function strpos;
use function substr;

/** @deprecated please use {@see InMemory} instead */
final class LocalFileReference implements Key
{
private const PATH_PREFIX = 'file://';

private string $path;
private string $passphrase;
private string $contents;

private function __construct(string $path, string $passphrase)
{
Expand All @@ -38,7 +40,11 @@ public static function file(string $path, string $passphrase = ''): self

public function contents(): string
{
return self::PATH_PREFIX . $this->path;
if (! isset($this->contents)) {
$this->contents = InMemory::file($this->path)->contents();
}

return $this->contents;
}

public function passphrase(): string
Expand Down
40 changes: 40 additions & 0 deletions test/functional/HmacTokenTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,17 @@
use Lcobucci\JWT\Signer\Hmac\Sha256;
use Lcobucci\JWT\Signer\Hmac\Sha512;
use Lcobucci\JWT\Signer\Key\InMemory;
use Lcobucci\JWT\Signer\Key\LocalFileReference;
use Lcobucci\JWT\Token;
use Lcobucci\JWT\Validation\Constraint\SignedWith;
use Lcobucci\JWT\Validation\RequiredConstraintsViolated;
use PHPUnit\Framework\TestCase;

use function assert;
use function file_put_contents;
use function is_string;
use function sys_get_temp_dir;
use function tempnam;

/**
* @covers \Lcobucci\JWT\Configuration
Expand Down Expand Up @@ -131,4 +136,39 @@ public function everythingShouldWorkWhenUsingATokenGeneratedByOtherLibs(): void
self::assertTrue($this->config->validator()->validate($token, $constraint));
self::assertEquals('world', $token->claims()->get('hello'));
}

/** @test */
public function signatureValidationWithLocalFileKeyReferenceWillOperateWithKeyContents(): void
{
$key = tempnam(sys_get_temp_dir(), 'key');
assert(is_string($key));

file_put_contents($key, 'just a dummy key');

$validKey = LocalFileReference::file($key);
$invalidKey = InMemory::plainText('file://' . $key);
$signer = new Sha256();
$configuration = Configuration::forSymmetricSigner($signer, $validKey);
$validator = $configuration->validator();

$token = $configuration->builder()
->withClaim('foo', 'bar')
->getToken($configuration->signer(), $configuration->signingKey());

self::assertFalse(
$validator->validate(
$token,
new SignedWith($signer, $invalidKey)
),
'Token cannot be validated against the **path** of the key'
);

self::assertTrue(
$validator->validate(
$token,
new SignedWith($signer, $validKey)
),
'Token can be validated against the **contents** of the key'
);
}
}
2 changes: 2 additions & 0 deletions test/unit/Signer/EcdsaTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ private function getSigner(): Ecdsa
*
* @uses \Lcobucci\JWT\Signer\Ecdsa::__construct
* @uses \Lcobucci\JWT\Signer\Key\LocalFileReference
* @uses \Lcobucci\JWT\Signer\Key\InMemory
*/
public function signShouldReturnTheAHashBasedOnTheOpenSslSignature(): void
{
Expand Down Expand Up @@ -88,6 +89,7 @@ public function signShouldReturnTheAHashBasedOnTheOpenSslSignature(): void
*
* @uses \Lcobucci\JWT\Signer\Ecdsa::__construct
* @uses \Lcobucci\JWT\Signer\Key\LocalFileReference
* @uses \Lcobucci\JWT\Signer\Key\InMemory
*/
public function verifyShouldDelegateToEcdsaSignerUsingPublicKey(): void
{
Expand Down
26 changes: 8 additions & 18 deletions test/unit/Signer/Key/LocalFileReferenceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@
use org\bovigo\vfs\vfsStream;
use PHPUnit\Framework\TestCase;

/** @coversDefaultClass \Lcobucci\JWT\Signer\Key\LocalFileReference */
/**
* @coversDefaultClass \Lcobucci\JWT\Signer\Key\LocalFileReference
*
* @uses \Lcobucci\JWT\Signer\Key\InMemory
*/
final class LocalFileReferenceTest extends TestCase
{
/** @before */
Expand All @@ -33,20 +37,6 @@ public function thereShouldBeNoReferenceToAFileThatDoesNotExist(): void
LocalFileReference::file(vfsStream::url('root/test2.pem'));
}

/**
* @test
*
* @covers ::file
* @covers ::__construct
* @covers ::contents
*/
public function pathShouldBeNormalised(): void
{
$key = LocalFileReference::file('file://' . vfsStream::url('root/test.pem'));

self::assertSame('file://vfs://root/test.pem', $key->contents());
}

/**
* @test
*
Expand All @@ -55,11 +45,11 @@ public function pathShouldBeNormalised(): void
* @covers ::contents
* @covers ::passphrase
*/
public function contentsShouldReturnOnlyTheReferenceToTheFile(): void
public function pathShouldBeNormalised(): void
{
$key = LocalFileReference::file(vfsStream::url('root/test.pem'), 'test');
$key = LocalFileReference::file('file://' . vfsStream::url('root/test.pem'), 'test');

self::assertSame('file://vfs://root/test.pem', $key->contents());
self::assertSame('testing', $key->contents());
self::assertSame('test', $key->passphrase());
}
}
4 changes: 4 additions & 0 deletions test/unit/Signer/RsaTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ final class RsaTest extends TestCase
* @covers \Lcobucci\JWT\Signer\OpenSSL
*
* @uses \Lcobucci\JWT\Signer\Key\LocalFileReference
* @uses \Lcobucci\JWT\Signer\Key\InMemory
*/
public function signShouldReturnAValidOpensslSignature(): void
{
Expand Down Expand Up @@ -100,6 +101,7 @@ public function signShouldRaiseAnExceptionWhenKeyIsNotParseable(): void
* @covers \Lcobucci\JWT\Signer\InvalidKeyProvided
*
* @uses \Lcobucci\JWT\Signer\Key\LocalFileReference
* @uses \Lcobucci\JWT\Signer\Key\InMemory
*/
public function signShouldRaiseAnExceptionWhenKeyTypeIsNotRsa(): void
{
Expand All @@ -119,6 +121,7 @@ public function signShouldRaiseAnExceptionWhenKeyTypeIsNotRsa(): void
* @covers \Lcobucci\JWT\Signer\OpenSSL
*
* @uses \Lcobucci\JWT\Signer\Key\LocalFileReference
* @uses \Lcobucci\JWT\Signer\Key\InMemory
*/
public function verifyShouldReturnTrueWhenSignatureIsValid(): void
{
Expand Down Expand Up @@ -161,6 +164,7 @@ public function verifyShouldRaiseAnExceptionWhenKeyIsNotParseable(): void
* @covers \Lcobucci\JWT\Signer\InvalidKeyProvided
*
* @uses \Lcobucci\JWT\Signer\Key\LocalFileReference
* @uses \Lcobucci\JWT\Signer\Key\InMemory
*/
public function verifyShouldRaiseAnExceptionWhenKeyTypeIsNotRsa(): void
{
Expand Down

0 comments on commit 5556426

Please sign in to comment.