Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ $passphrase = '[YOUR_PASSPHRASE]';
// Can be generated with "ssh-keygen -t rsa -m pem"
$privateKeyFile = '/path/to/key-with-passphrase.pem';

// Create a private key of type "resource"
/** @var OpenSSLAsymmetricKey $privateKey */
$privateKey = openssl_pkey_get_private(
file_get_contents($privateKeyFile),
$passphrase
Expand Down
45 changes: 21 additions & 24 deletions src/JWT.php
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ public static function decode(
* Converts and signs a PHP array into a JWT string.
*
* @param array<mixed> $payload PHP array
* @param string|resource|OpenSSLAsymmetricKey|OpenSSLCertificate $key The secret key.
* @param string|OpenSSLAsymmetricKey|OpenSSLCertificate $key The secret key.
* @param string $alg Supported algorithms are 'ES384','ES256', 'ES256K', 'HS256',
* 'HS384', 'HS512', 'RS256', 'RS384', and 'RS512'
* @param string $keyId
Expand Down Expand Up @@ -239,7 +239,7 @@ public static function encode(
* Sign a string with a given key and algorithm.
*
* @param string $msg The message to sign
* @param string|resource|OpenSSLAsymmetricKey|OpenSSLCertificate $key The secret key.
* @param string|OpenSSLAsymmetricKey|OpenSSLCertificate $key The secret key.
* @param string $alg Supported algorithms are 'EdDSA', 'ES384', 'ES256', 'ES256K', 'HS256',
* 'HS384', 'HS512', 'RS256', 'RS384', and 'RS512'
*
Expand All @@ -265,17 +265,13 @@ public static function sign(
return \hash_hmac($algorithm, $msg, $key, true);
case 'openssl':
$signature = '';
if (!\is_resource($key)) {
/** @var OpenSSLAsymmetricKey|OpenSSLCertificate|string $key */
$key = $key;
if (!$key = openssl_pkey_get_private($key)) {
throw new DomainException('OpenSSL unable to validate key');
}
if (str_starts_with($alg, 'RS')) {
self::validateRsaKeyLength($key);
}
if (!$key = openssl_pkey_get_private($key)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is nit and something that I don't think it needs to be changed, but I find this logic a bit convoluted.

It might be good to separate these cases into their own methods like :

private function handleOpenSsl()
private function handleSodyumCrypto()

But this was already this way. Just think it would be a good cleanup for later.

throw new DomainException('OpenSSL unable to validate key');
}
$success = \openssl_sign($msg, $signature, $key, $algorithm); // @phpstan-ignore-line
if (str_starts_with($alg, 'RS')) {
self::validateRsaKeyLength($key);
}
$success = \openssl_sign($msg, $signature, $key, $algorithm);
if (!$success) {
throw new DomainException('OpenSSL unable to sign data');
}
Expand Down Expand Up @@ -314,7 +310,7 @@ public static function sign(
*
* @param string $msg The original message (header and body)
* @param string $signature The original signature
* @param string|resource|OpenSSLAsymmetricKey|OpenSSLCertificate $keyMaterial For Ed*, ES*, HS*, a string key works. for RS*, must be an instance of OpenSSLAsymmetricKey
* @param string|OpenSSLAsymmetricKey|OpenSSLCertificate $keyMaterial For Ed*, ES*, HS*, a string key works. for RS*, must be an instance of OpenSSLAsymmetricKey
* @param string $alg The algorithm
*
* @return bool
Expand All @@ -334,14 +330,13 @@ private static function verify(
list($function, $algorithm) = static::$supported_algs[$alg];
switch ($function) {
case 'openssl':
if (!\is_resource($keyMaterial) && str_starts_with($algorithm, 'RS')) {
/** @var OpenSSLAsymmetricKey|OpenSSLCertificate|string $keyMaterial */
$keyMaterial = $keyMaterial;
if ($key = openssl_pkey_get_private($keyMaterial)) {
self::validateRsaKeyLength($key);
if (str_starts_with($algorithm, 'RS')) {
if (!$key = openssl_pkey_get_private($keyMaterial)) {
throw new DomainException('OpenSSL unable to validate key');
}
self::validateRsaKeyLength($key);
}
$success = \openssl_verify($msg, $signature, $keyMaterial, $algorithm); // @phpstan-ignore-line
$success = \openssl_verify($msg, $signature, $keyMaterial, $algorithm);
if ($success === 1) {
return true;
}
Expand Down Expand Up @@ -699,6 +694,7 @@ private static function readDER(string $der, int $offset = 0): array
*
* @param string $key HMAC key material
* @param string $algorithm The algorithm
*
* @throws DomainException Provided key is too short
*/
private static function validateHmacKeyLength(string $key, string $algorithm): void
Expand All @@ -716,12 +712,13 @@ private static function validateHmacKeyLength(string $key, string $algorithm): v
* @param OpenSSLAsymmetricKey $key RSA key material
* @throws DomainException Provided key is too short
*/
private static function validateRsaKeyLength(OpenSSLAsymmetricKey $key): void
private static function validateRsaKeyLength(#[\SensitiveParameter] OpenSSLAsymmetricKey $key): void
{
if ($keyDetails = \openssl_pkey_get_details($key)) {
if ($keyDetails['bits'] < self::RSA_KEY_MIN_LENGTH) {
throw new DomainException('Provided key is too short');
}
if (!$keyDetails = openssl_pkey_get_details($key)) {
throw new DomainException('Unable to validate key');
}
if ($keyDetails['bits'] < self::RSA_KEY_MIN_LENGTH) {
throw new DomainException('Provided key is too short');
}
}
}
7 changes: 3 additions & 4 deletions src/Key.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
class Key
{
/**
* @param string|resource|OpenSSLAsymmetricKey|OpenSSLCertificate $keyMaterial
* @param string|OpenSSLAsymmetricKey|OpenSSLCertificate $keyMaterial
* @param string $algorithm
*/
public function __construct(
Expand All @@ -21,9 +21,8 @@ public function __construct(
!\is_string($keyMaterial)
&& !$keyMaterial instanceof OpenSSLAsymmetricKey
&& !$keyMaterial instanceof OpenSSLCertificate
&& !\is_resource($keyMaterial)
) {
throw new TypeError('Key material must be a string, resource, or OpenSSLAsymmetricKey');
throw new TypeError('Key material must be a string, OpenSSLCertificate, or OpenSSLAsymmetricKey');
}

if (empty($keyMaterial)) {
Expand All @@ -46,7 +45,7 @@ public function getAlgorithm(): string
}

/**
* @return string|resource|OpenSSLAsymmetricKey|OpenSSLCertificate
* @return string|OpenSSLAsymmetricKey|OpenSSLCertificate
*/
public function getKeyMaterial()
{
Expand Down
7 changes: 3 additions & 4 deletions tests/JWTTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ public function testUrlSafeCharacters()

public function testMalformedUtf8StringsFail()
{

$this->expectException(DomainException::class);
JWT::encode(['message' => pack('c', 128)], $this->hmacKey->getKeyMaterial(), 'HS256');
}
Expand Down Expand Up @@ -531,17 +530,17 @@ public function provideEncodeDecode()
];
}

public function testEncodeDecodeWithResource()
public function testEncodeDecodeWithOpenSSLAsymmetricKey()
{
$pem = file_get_contents(__DIR__ . '/data/rsa1-public.pub');
$resource = openssl_pkey_get_public($pem);
$keyMaterial = openssl_pkey_get_public($pem);
$privateKey = file_get_contents(__DIR__ . '/data/rsa1-private.pem');

$payload = ['foo' => 'bar'];
$encoded = JWT::encode($payload, $privateKey, 'RS512');

// Verify decoding succeeds
$decoded = JWT::decode($encoded, new Key($resource, 'RS512'));
$decoded = JWT::decode($encoded, new Key($keyMaterial, 'RS512'));

$this->assertSame('bar', $decoded->foo);
}
Expand Down