From c45bb8b961a8e742d8f6b88ef5ff1bd5cca5d01c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lu=C3=ADs=20Cobucci?= Date: Mon, 27 Sep 2021 13:53:37 +0200 Subject: [PATCH] Ensure key contents is used for all hashing algorithms 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 Co-authored-by: Marco Pivetta --- docs/configuration.md | 5 +--- docs/upgrading.md | 3 +- src/Signer/Key/LocalFileReference.php | 10 ++----- test/functional/HmacTokenTest.php | 41 +++++++++++++++++++++++++++ 4 files changed, 45 insertions(+), 14 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 3fde6911..b2ace511 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -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 @@ -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 ); diff --git a/docs/upgrading.md b/docs/upgrading.md index 645d97cb..7401fdc6 100644 --- a/docs/upgrading.md +++ b/docs/upgrading.md @@ -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 diff --git a/src/Signer/Key/LocalFileReference.php b/src/Signer/Key/LocalFileReference.php index 6b52ec8d..dafce109 100644 --- a/src/Signer/Key/LocalFileReference.php +++ b/src/Signer/Key/LocalFileReference.php @@ -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://'; @@ -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); } } diff --git a/test/functional/HmacTokenTest.php b/test/functional/HmacTokenTest.php index 52a2e1ac..be344928 100644 --- a/test/functional/HmacTokenTest.php +++ b/test/functional/HmacTokenTest.php @@ -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 @@ -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' + ); + } }