Skip to content
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

Jump to bottom

Inmemory cryptokey #1196

Closed
wants to merge 6 commits into from
Closed

Inmemory cryptokey #1196

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
62172a7
Disable permission check for Windows
eugene-borovov Mar 7, 2021
df6a4c2
Allow store the key content
eugene-borovov Mar 7, 2021
ff2d705
Added signer key fabric method
eugene-borovov Mar 7, 2021
76afb31
Testing key usage in memory
eugene-borovov Mar 7, 2021
2c7bc56
Fix tests
eugene-borovov Mar 7, 2021
44b1d14
Fix Style
eugene-borovov Mar 7, 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
3 changes: 1 addition & 2 deletions src/AuthorizationValidators/BearerTokenValidator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
use Lcobucci\JWT\Configuration;
use Lcobucci\JWT\Encoding\CannotDecodeContent;
use Lcobucci\JWT\Signer\Key\InMemory;
use Lcobucci\JWT\Signer\Key\LocalFileReference;
use Lcobucci\JWT\Signer\Rsa\Sha256;
use Lcobucci\JWT\Token\InvalidTokenStructure;
use Lcobucci\JWT\Token\UnsupportedHeaderFound;
Expand Down Expand Up @@ -78,7 +77,7 @@ private function initJwtConfiguration()

$this->jwtConfiguration->setValidationConstraints(
new ValidAt(new SystemClock(new DateTimeZone(\date_default_timezone_get()))),
new SignedWith(new Sha256(), LocalFileReference::file($this->publicKey->getKeyPath()))
new SignedWith(new Sha256(), $this->publicKey->createSignerKey())
);
}

Expand Down
107 changes: 54 additions & 53 deletions src/CryptKey.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,17 @@

namespace League\OAuth2\Server;

use Lcobucci\JWT\Signer\Key\InMemory;
use Lcobucci\JWT\Signer\Key\LocalFileReference;
use LogicException;
use RuntimeException;

class CryptKey
{
const RSA_KEY_PATTERN =
'/^(-----BEGIN (RSA )?(PUBLIC|PRIVATE) KEY-----)\R.*(-----END (RSA )?(PUBLIC|PRIVATE) KEY-----)\R?$/s';

const FILE_PREFIX = 'file://';

/**
* @var string
*/
Expand All @@ -36,67 +39,49 @@ class CryptKey
*/
public function __construct($keyPath, $passPhrase = null, $keyPermissionsCheck = true)
{
if ($rsaMatch = \preg_match(static::RSA_KEY_PATTERN, $keyPath)) {
$keyPath = $this->saveKeyToFile($keyPath);
} elseif ($rsaMatch === false) {
throw new \RuntimeException(
\sprintf('PCRE error [%d] encountered during key match attempt', \preg_last_error())
);
}
$this->keyPath = $keyPath;
$this->passPhrase = $passPhrase;

if (\strpos($keyPath, 'file://') !== 0) {
$keyPath = 'file://' . $keyPath;
if (\is_file($this->keyPath) && !$this->isFilePath()) {
$this->keyPath = self::FILE_PREFIX . $this->keyPath;
}

if (!\file_exists($keyPath) || !\is_readable($keyPath)) {
throw new LogicException(\sprintf('Key path "%s" does not exist or is not readable', $keyPath));
}
if ($this->isFilePath()) {
if (!\file_exists($keyPath) || !\is_readable($keyPath)) {
throw new LogicException(\sprintf('Key path "%s" does not exist or is not readable', $keyPath));
}

if ($keyPermissionsCheck === true) {
// Verify the permissions of the key
$keyPathPerms = \decoct(\fileperms($keyPath) & 0777);
if (\in_array($keyPathPerms, ['400', '440', '600', '640', '660'], true) === false) {
\trigger_error(\sprintf(
'Key file "%s" permissions are not correct, recommend changing to 600 or 660 instead of %s',
$keyPath,
$keyPathPerms
), E_USER_NOTICE);
if ($keyPermissionsCheck === true && PHP_OS_FAMILY !== 'Windows') {
// Verify the permissions of the key
$keyPathPerms = \decoct(\fileperms($keyPath) & 0777);
if (\in_array($keyPathPerms, ['400', '440', '600', '640', '660'], true) === false) {
\trigger_error(
\sprintf(
'Key file "%s" permissions are not correct, recommend changing to 600 or 660 instead of %s',
$keyPath,
$keyPathPerms
),
E_USER_NOTICE
);
}
}
} else {
$rsaMatch = \preg_match(static::RSA_KEY_PATTERN, $this->keyPath);
if ($rsaMatch === 0) {
throw new LogicException('This is not a RSA key');
}
}

$this->keyPath = $keyPath;
$this->passPhrase = $passPhrase;
if ($rsaMatch === false) {
throw new \RuntimeException(
\sprintf('PCRE error [%d] encountered during key match attempt', \preg_last_error())
);
}
}
}

/**
* @param string $key
*
* @throws RuntimeException
*
* @return string
*/
private function saveKeyToFile($key)
public function isFilePath(): bool
{
$tmpDir = \sys_get_temp_dir();
$keyPath = $tmpDir . '/' . \sha1($key) . '.key';

if (\file_exists($keyPath)) {
return 'file://' . $keyPath;
}

if (\file_put_contents($keyPath, $key) === false) {
// @codeCoverageIgnoreStart
throw new RuntimeException(\sprintf('Unable to write key file to temporary directory "%s"', $tmpDir));
// @codeCoverageIgnoreEnd
}

if (\chmod($keyPath, 0600) === false) {
// @codeCoverageIgnoreStart
throw new RuntimeException(\sprintf('The key file "%s" file mode could not be changed with chmod to 600', $keyPath));
// @codeCoverageIgnoreEnd
}

return 'file://' . $keyPath;
return \strpos($this->keyPath, self::FILE_PREFIX) === 0;
}

/**
Expand All @@ -118,4 +103,20 @@ public function getPassPhrase()
{
return $this->passPhrase;
}

/**
* Create signer key
*
* @internal Remove when the JWT configuration is moved to the dependency injection container
*
* @return \Lcobucci\JWT\Signer\Key
*/
public function createSignerKey(): \Lcobucci\JWT\Signer\Key
{
if ($this->isFilePath()) {
return LocalFileReference::file($this->keyPath, $this->passPhrase ?? '');
}

return InMemory::plainText($this->keyPath, $this->passPhrase ?? '');
}
}
3 changes: 1 addition & 2 deletions src/Entities/Traits/AccessTokenTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
use DateTimeImmutable;
use Lcobucci\JWT\Configuration;
use Lcobucci\JWT\Signer\Key\InMemory;
use Lcobucci\JWT\Signer\Key\LocalFileReference;
use Lcobucci\JWT\Signer\Rsa\Sha256;
use Lcobucci\JWT\Token;
use League\OAuth2\Server\CryptKey;
Expand Down Expand Up @@ -46,7 +45,7 @@ public function initJwtConfiguration()
{
$this->jwtConfiguration = Configuration::forAsymmetricSigner(
new Sha256(),
LocalFileReference::file($this->privateKey->getKeyPath(), $this->privateKey->getPassPhrase() ?? ''),
$this->privateKey->createSignerKey(),
InMemory::plainText('')
);
}
Expand Down
92 changes: 64 additions & 28 deletions tests/AuthorizationServerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,16 @@ public function setUp(): void
\chmod(__DIR__ . '/Stubs/private.key.crlf', 0600);
}

public function testRespondToRequestInvalidGrantType()
/**
* @dataProvider signerKeys
*/
public function testRespondToRequestInvalidGrantType($privateKey, $encryptionKey)
{
$server = new AuthorizationServer(
$this->getMockBuilder(ClientRepositoryInterface::class)->getMock(),
$this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(),
$this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(),
'file://' . __DIR__ . '/Stubs/private.key',
$privateKey,
\base64_encode(\random_bytes(36)),
new StubResponseType()
);
Expand All @@ -60,7 +63,10 @@ public function testRespondToRequestInvalidGrantType()
}
}

public function testRespondToRequest()
/**
* @dataProvider signerKeys
*/
public function testRespondToRequest($privateKey, $encryptionKey)
{
$client = new ClientEntity();

Expand All @@ -82,7 +88,7 @@ public function testRespondToRequest()
$clientRepository,
$accessTokenRepositoryMock,
$scopeRepositoryMock,
'file://' . __DIR__ . '/Stubs/private.key',
$privateKey,
\base64_encode(\random_bytes(36)),
new StubResponseType()
);
Expand All @@ -97,16 +103,19 @@ public function testRespondToRequest()
$this->assertEquals(200, $response->getStatusCode());
}

public function testGetResponseType()
/**
* @dataProvider signerKeys
*/
public function testGetResponseType($privateKey, $encryptionKey)
{
$clientRepository = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();

$server = new AuthorizationServer(
$clientRepository,
$this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(),
$this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(),
'file://' . __DIR__ . '/Stubs/private.key',
'file://' . __DIR__ . '/Stubs/public.key'
$privateKey,
$encryptionKey
);

$abstractGrantReflection = new \ReflectionClass($server);
Expand All @@ -116,18 +125,19 @@ public function testGetResponseType()
$this->assertInstanceOf(BearerTokenResponse::class, $method->invoke($server));
}

public function testGetResponseTypeExtended()
/**
* @dataProvider signerKeys
*/
public function testGetResponseTypeExtended($privateKey, $encryptionKey)
{
$clientRepository = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
$privateKey = 'file://' . __DIR__ . '/Stubs/private.key';
$encryptionKey = 'file://' . __DIR__ . '/Stubs/public.key';

$server = new AuthorizationServer(
$clientRepository,
$this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(),
$this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(),
'file://' . __DIR__ . '/Stubs/private.key',
'file://' . __DIR__ . '/Stubs/public.key'
$privateKey,
$encryptionKey
);

$abstractGrantReflection = new \ReflectionClass($server);
Expand All @@ -149,11 +159,11 @@ public function testGetResponseTypeExtended()
$this->assertSame($encryptionKey, $encryptionKeyProperty->getValue($responseType));
}

public function testMultipleRequestsGetDifferentResponseTypeInstances()
/**
* @dataProvider signerKeys
*/
public function testMultipleRequestsGetDifferentResponseTypeInstances($privateKey, $encryptionKey)
{
$privateKey = 'file://' . __DIR__ . '/Stubs/private.key';
$encryptionKey = 'file://' . __DIR__ . '/Stubs/public.key';

$responseTypePrototype = new class extends BearerTokenResponse {
/* @return null|CryptKey */
public function getPrivateKey()
Expand Down Expand Up @@ -201,16 +211,19 @@ public function getEncryptionKey()
$this->assertNotSame($responseTypeA, $responseTypeB);
}

public function testCompleteAuthorizationRequest()
/**
* @dataProvider signerKeys
*/
public function testCompleteAuthorizationRequest($privateKey, $encryptionKey)
{
$clientRepository = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();

$server = new AuthorizationServer(
$clientRepository,
$this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(),
$this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(),
'file://' . __DIR__ . '/Stubs/private.key',
'file://' . __DIR__ . '/Stubs/public.key'
$privateKey,
$encryptionKey
);

$authCodeRepository = $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock();
Expand All @@ -236,7 +249,10 @@ public function testCompleteAuthorizationRequest()
);
}

public function testValidateAuthorizationRequest()
/**
* @dataProvider signerKeys
*/
public function testValidateAuthorizationRequest($privateKey, $encryptionKey)
{
$client = new ClientEntity();
$client->setRedirectUri('http://foo/bar');
Expand All @@ -259,8 +275,8 @@ public function testValidateAuthorizationRequest()
$clientRepositoryMock,
$this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(),
$scopeRepositoryMock,
'file://' . __DIR__ . '/Stubs/private.key',
'file://' . __DIR__ . '/Stubs/public.key'
$privateKey,
$encryptionKey
);

$server->setDefaultScope(self::DEFAULT_SCOPE);
Expand All @@ -283,7 +299,10 @@ public function testValidateAuthorizationRequest()
$this->assertInstanceOf(AuthorizationRequest::class, $server->validateAuthorizationRequest($request));
}

public function testValidateAuthorizationRequestWithMissingRedirectUri()
/**
* @dataProvider signerKeys
*/
public function testValidateAuthorizationRequestWithMissingRedirectUri($privateKey, $encryptionKey)
{
$client = new ClientEntity();
$client->setConfidential();
Expand All @@ -302,8 +321,8 @@ public function testValidateAuthorizationRequestWithMissingRedirectUri()
$clientRepositoryMock,
$this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(),
$this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(),
'file://' . __DIR__ . '/Stubs/private.key',
'file://' . __DIR__ . '/Stubs/public.key'
$privateKey,
$encryptionKey
);
$server->enableGrantType($grant);

Expand All @@ -329,14 +348,17 @@ public function testValidateAuthorizationRequestWithMissingRedirectUri()
}
}

public function testValidateAuthorizationRequestUnregistered()
/**
* @dataProvider signerKeys
*/
public function testValidateAuthorizationRequestUnregistered($privateKey, $encryptionKey)
{
$server = new AuthorizationServer(
$this->getMockBuilder(ClientRepositoryInterface::class)->getMock(),
$this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(),
$this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(),
'file://' . __DIR__ . '/Stubs/private.key',
'file://' . __DIR__ . '/Stubs/public.key'
$privateKey,
$encryptionKey
);

$request = (new ServerRequest())->withQueryParams([
Expand All @@ -349,4 +371,18 @@ public function testValidateAuthorizationRequestUnregistered()

$server->validateAuthorizationRequest($request);
}

public function signerKeys(): array
{
return [
'file key' => [
'file://' . __DIR__ . '/Stubs/private.key',
'file://' . __DIR__ . '/Stubs/public.key',
],
'inmemory key' => [
\file_get_contents(__DIR__ . '/Stubs/private.key'),
\file_get_contents(__DIR__ . '/Stubs/public.key'),
],
];
}
}
Loading