Skip to content

Commit

Permalink
Ensure key contents is used for all hashing algorithms
Browse files Browse the repository at this point in the history
On v3.4.0 we introduced the
`Lcobucci\JWT\Signer\Key\LocalFileReference`, which was designed to be
used with OpenSSL-based algorithms and avoid having to load the file
contents into user-land to sign/verify tokens.

However, other algorithms don't understand the scheme `file://` and will
incorrectly use the file path as the signing key.

This modifies and deprecates `LocalFileReference` to avoid that
misleading behaviour.

Co-authored-by: Anton Smirnov <sandfox@sandfox.me>
Co-authored-by: Marco Pivetta <ocramius@gmail.com>
  • Loading branch information
3 people committed Sep 27, 2021
1 parent 511629a commit c45bb8b
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 14 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
10 changes: 2 additions & 8 deletions src/Signer/Key/LocalFileReference.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use function strpos;
use function substr;

/** @deprecated Use \Lcobucci\JWT\Signer\Key\InMemory::file() instead */
final class LocalFileReference extends Key
{
const PATH_PREFIX = 'file://';
Expand All @@ -26,13 +27,6 @@ public static function file($path, $passphrase = '')
$path = substr($path, 7);
}

if (! file_exists($path)) {
throw FileCouldNotBeRead::onPath($path);
}

$key = new self('', $passphrase);
$key->content = self::PATH_PREFIX . $path;

return $key;
return new self(self::PATH_PREFIX . $path, $passphrase);
}
}
41 changes: 41 additions & 0 deletions test/functional/HmacTokenTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,16 @@
use Lcobucci\JWT\Builder;
use Lcobucci\JWT\CheckForDeprecations;
use Lcobucci\JWT\Parser;
use Lcobucci\JWT\Signer\Key\InMemory;
use Lcobucci\JWT\Signer\Key\LocalFileReference;
use Lcobucci\JWT\Token;
use Lcobucci\JWT\Signature;
use Lcobucci\JWT\Signer\Hmac\Sha256;
use Lcobucci\JWT\Signer\Hmac\Sha512;
use ReflectionProperty;
use function file_put_contents;
use function sys_get_temp_dir;
use function tempnam;

/**
* @author Luís Otávio Cobucci Oblonczyk <lcobucci@gmail.com>
Expand Down Expand Up @@ -197,4 +203,39 @@ public function everythingShouldWorkWhenUsingATokenGeneratedByOtherLibs()
$this->assertEquals('world', $token->getClaim('hello'));
$this->assertTrue($token->verify($this->signer, 'testing'));
}

/**
* @test
*
* @coversNothing
*
* @see \Lcobucci\JWT\Signer\Key::setContent()
*/
public function signatureValidationWithLocalFileKeyReferenceWillOperateWithKeyContents()
{
$key = tempnam(sys_get_temp_dir(), 'key');
file_put_contents($key, 'just a dummy key');

$validKey = LocalFileReference::file($key);
$invalidKey = InMemory::plainText($key);
$signer = new Sha256();

// 3.4.x implicitly extracts key contents, when `file://` is detected
$reflectionContents = new ReflectionProperty(InMemory::class, 'content');
$reflectionContents->setAccessible(true);
$reflectionContents->setValue($invalidKey, 'file://' . $key);

$token = (new Builder())
->withClaim('foo', 'bar')
->getToken($signer, $validKey);

self::assertFalse(
$token->verify($signer, $invalidKey),
'Token cannot be validated against the **path** of the key'
);
self::assertTrue(
$token->verify($signer, $validKey),
'Token can be validated against the **contents** of the key'
);
}
}

0 comments on commit c45bb8b

Please sign in to comment.