Skip to content

Commit 3be6da2

Browse files
lcobuccilcobucci
committed
Re-organise OpenSSL key incompatibility verification
The expected behaviour for key length verification between RSA and ECDSA algorithms are actually different. This shifts the responsibility to the base respective base implementations, simplifying the code a bit. Signed-off-by: Luís Cobucci <lcobucci@gmail.com>
1 parent d2feb20 commit 3be6da2

28 files changed

+154
-212
lines changed

‎src/Signer/Ecdsa.php‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,25 @@ final public function verify(string $expected, string $payload, Key $key): bool
3939
);
4040
}
4141

42-
final public function keyType(): int
42+
/** {@inheritdoc} */
43+
final protected function guardAgainstIncompatibleKey(int $type, int $lengthInBits): void
4344
{
44-
return OPENSSL_KEYTYPE_EC;
45-
}
45+
if ($type !== OPENSSL_KEYTYPE_EC) {
46+
throw InvalidKeyProvided::incompatibleKeyType(
47+
self::KEY_TYPE_MAP[OPENSSL_KEYTYPE_EC],
48+
self::KEY_TYPE_MAP[$type],
49+
);
50+
}
4651

47-
final public function minimumBitsLengthForKey(): int
48-
{
49-
return 224;
52+
$expectedKeyLength = $this->expectedKeyLength();
53+
54+
if ($lengthInBits !== $expectedKeyLength) {
55+
throw InvalidKeyProvided::incompatibleKeyLength($expectedKeyLength, $lengthInBits);
56+
}
5057
}
5158

59+
abstract public function expectedKeyLength(): int;
60+
5261
/**
5362
* Returns the length of each point in the signature, so that we can calculate and verify R and S points properly
5463
*

‎src/Signer/Ecdsa/Sha256.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,9 @@ public function pointLength(): int
2323
{
2424
return 64;
2525
}
26+
27+
public function expectedKeyLength(): int
28+
{
29+
return 256;
30+
}
2631
}

‎src/Signer/Ecdsa/Sha384.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,9 @@ public function pointLength(): int
2323
{
2424
return 96;
2525
}
26+
27+
public function expectedKeyLength(): int
28+
{
29+
return 384;
30+
}
2631
}

‎src/Signer/Ecdsa/Sha512.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,9 @@ public function pointLength(): int
2323
{
2424
return 132;
2525
}
26+
27+
public function expectedKeyLength(): int
28+
{
29+
return 521;
30+
}
2631
}

‎src/Signer/InvalidKeyProvided.php‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,20 @@ public static function cannotBeParsed(string $details): self
1313
return new self('It was not possible to parse your key, reason:' . $details);
1414
}
1515

16-
public static function incompatibleKey(): self
16+
public static function incompatibleKeyType(string $expectedType, string $actualType): self
1717
{
18-
return new self('This key is not compatible with this signer');
18+
return new self(
19+
'The type of the provided key is not "' . $expectedType
20+
. '", "' . $actualType . '" provided'
21+
);
22+
}
23+
24+
public static function incompatibleKeyLength(int $expectedLength, int $actualLength): self
25+
{
26+
return new self(
27+
'The length of the provided key is different than ' . $expectedLength . ' bits, '
28+
. $actualLength . ' bits provided'
29+
);
1930
}
2031

2132
public static function cannotBeEmpty(): self

‎src/Signer/OpenSSL.php‎

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,21 @@
1919
use function openssl_sign;
2020
use function openssl_verify;
2121

22+
use const OPENSSL_KEYTYPE_DH;
23+
use const OPENSSL_KEYTYPE_DSA;
24+
use const OPENSSL_KEYTYPE_EC;
25+
use const OPENSSL_KEYTYPE_RSA;
2226
use const PHP_EOL;
2327

2428
abstract class OpenSSL implements Signer
2529
{
30+
protected const KEY_TYPE_MAP = [
31+
OPENSSL_KEYTYPE_RSA => 'RSA',
32+
OPENSSL_KEYTYPE_DSA => 'DSA',
33+
OPENSSL_KEYTYPE_DH => 'DH',
34+
OPENSSL_KEYTYPE_EC => 'EC',
35+
];
36+
2637
/**
2738
* @throws CannotSignPayload
2839
* @throws InvalidKeyProvided
@@ -47,9 +58,6 @@ final protected function createSignature(
4758
}
4859
}
4960

50-
/** @return positive-int */
51-
abstract public function minimumBitsLengthForKey(): int;
52-
5361
/**
5462
* @return resource|OpenSSLAsymmetricKey
5563
*
@@ -105,15 +113,12 @@ private function validateKey($key): void
105113
$details = openssl_pkey_get_details($key);
106114
assert(is_array($details));
107115

108-
if (! array_key_exists('key', $details) || $details['type'] !== $this->keyType()) {
109-
throw InvalidKeyProvided::incompatibleKey();
110-
}
111-
112116
assert(array_key_exists('bits', $details));
113117
assert(is_int($details['bits']));
114-
if ($details['bits'] < $this->minimumBitsLengthForKey()) {
115-
throw InvalidKeyProvided::tooShort($this->minimumBitsLengthForKey(), $details['bits']);
116-
}
118+
assert(array_key_exists('type', $details));
119+
assert(is_int($details['type']));
120+
121+
$this->guardAgainstIncompatibleKey($details['type'], $details['bits']);
117122
}
118123

119124
private function fullOpenSSLErrorString(): string
@@ -127,6 +132,9 @@ private function fullOpenSSLErrorString(): string
127132
return $error;
128133
}
129134

135+
/** @throws InvalidKeyProvided */
136+
abstract protected function guardAgainstIncompatibleKey(int $type, int $lengthInBits): void;
137+
130138
/** @param resource|OpenSSLAsymmetricKey $key */
131139
private function freeKey($key): void
132140
{
@@ -137,13 +145,6 @@ private function freeKey($key): void
137145
openssl_free_key($key); // Deprecated and no longer necessary as of PHP >= 8.0
138146
}
139147

140-
/**
141-
* Returns the type of key to be used to create/verify the signature (using OpenSSL constants)
142-
*
143-
* @internal
144-
*/
145-
abstract public function keyType(): int;
146-
147148
/**
148149
* Returns which algorithm to be used to create/verify the signature (using OpenSSL constants)
149150
*

‎src/Signer/Rsa.php‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77

88
abstract class Rsa extends OpenSSL
99
{
10+
private const MINIMUM_KEY_LENGTH = 2048;
11+
1012
final public function sign(string $payload, Key $key): string
1113
{
1214
return $this->createSignature($key->contents(), $key->passphrase(), $payload);
@@ -17,13 +19,17 @@ final public function verify(string $expected, string $payload, Key $key): bool
1719
return $this->verifySignature($expected, $payload, $key->contents());
1820
}
1921

20-
final public function keyType(): int
22+
final protected function guardAgainstIncompatibleKey(int $type, int $lengthInBits): void
2123
{
22-
return OPENSSL_KEYTYPE_RSA;
23-
}
24+
if ($type !== OPENSSL_KEYTYPE_RSA) {
25+
throw InvalidKeyProvided::incompatibleKeyType(
26+
self::KEY_TYPE_MAP[OPENSSL_KEYTYPE_RSA],
27+
self::KEY_TYPE_MAP[$type],
28+
);
29+
}
2430

25-
final public function minimumBitsLengthForKey(): int
26-
{
27-
return 2048;
31+
if ($lengthInBits < self::MINIMUM_KEY_LENGTH) {
32+
throw InvalidKeyProvided::tooShort(self::MINIMUM_KEY_LENGTH, $lengthInBits);
33+
}
2834
}
2935
}

‎src/Signer/UnsafeEcdsa.php‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,15 @@ final public function verify(string $expected, string $payload, Key $key): bool
4040
);
4141
}
4242

43-
final public function keyType(): int
43+
// phpcs:ignore SlevomatCodingStandard.Functions.UnusedParameter.UnusedParameter
44+
final protected function guardAgainstIncompatibleKey(int $type, int $lengthInBits): void
4445
{
45-
return OPENSSL_KEYTYPE_EC;
46-
}
47-
48-
final public function minimumBitsLengthForKey(): int
49-
{
50-
return 1;
46+
if ($type !== OPENSSL_KEYTYPE_EC) {
47+
throw InvalidKeyProvided::incompatibleKeyType(
48+
self::KEY_TYPE_MAP[OPENSSL_KEYTYPE_EC],
49+
self::KEY_TYPE_MAP[$type],
50+
);
51+
}
5152
}
5253

5354
/**

‎src/Signer/UnsafeRsa.php‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,14 @@ final public function verify(string $expected, string $payload, Key $key): bool
1818
return $this->verifySignature($expected, $payload, $key->contents());
1919
}
2020

21-
final public function keyType(): int
21+
// phpcs:ignore SlevomatCodingStandard.Functions.UnusedParameter.UnusedParameter
22+
final protected function guardAgainstIncompatibleKey(int $type, int $lengthInBits): void
2223
{
23-
return OPENSSL_KEYTYPE_RSA;
24-
}
25-
26-
final public function minimumBitsLengthForKey(): int
27-
{
28-
return 1;
24+
if ($type !== OPENSSL_KEYTYPE_RSA) {
25+
throw InvalidKeyProvided::incompatibleKeyType(
26+
self::KEY_TYPE_MAP[OPENSSL_KEYTYPE_RSA],
27+
self::KEY_TYPE_MAP[$type],
28+
);
29+
}
2930
}
3031
}

‎test/functional/ES512TokenTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public function builderShouldRaiseExceptionWhenKeyIsNotEcdsaCompatible(): void
7777
$builder = $this->config->builder();
7878

7979
$this->expectException(InvalidKeyProvided::class);
80-
$this->expectExceptionMessage('This key is not compatible with this signer');
80+
$this->expectExceptionMessage('The type of the provided key is not "EC", "RSA" provided');
8181

8282
$builder->identifiedBy('1')
8383
->permittedFor('http://client.abc.com')
@@ -168,7 +168,7 @@ public function signatureAssertionShouldRaiseExceptionWhenAlgorithmIsDifferent(T
168168
public function signatureAssertionShouldRaiseExceptionWhenKeyIsNotEcdsaCompatible(Token $token): void
169169
{
170170
$this->expectException(InvalidKeyProvided::class);
171-
$this->expectExceptionMessage('This key is not compatible with this signer');
171+
$this->expectExceptionMessage('The type of the provided key is not "EC", "RSA" provided');
172172

173173
$this->config->validator()->assert(
174174
$token,

0 commit comments

Comments
 (0)
⚡