Skip to content

Commit c1e7da3

Browse files
SephsterSephster
authored
Merge pull request #1215 from eugene-borovov/phpseclib-cryptokey
Validate key with openssl
2 parents 93ace54 + de16703 commit c1e7da3

File tree

2 files changed

+137
-39
lines changed

2 files changed

+137
-39
lines changed

‎src/CryptKey.php‎

Lines changed: 55 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,12 @@
1616

1717
class CryptKey
1818
{
19+
/** @deprecated left for backward compatibility check */
1920
const RSA_KEY_PATTERN =
2021
'/^(-----BEGIN (RSA )?(PUBLIC|PRIVATE) KEY-----)\R.*(-----END (RSA )?(PUBLIC|PRIVATE) KEY-----)\R?$/s';
2122

23+
private const FILE_PREFIX = 'file://';
24+
2225
/**
2326
* @var string
2427
*/
@@ -36,36 +39,43 @@ class CryptKey
3639
*/
3740
public function __construct($keyPath, $passPhrase = null, $keyPermissionsCheck = true)
3841
{
39-
if ($rsaMatch = \preg_match(static::RSA_KEY_PATTERN, $keyPath)) {
40-
$keyPath = $this->saveKeyToFile($keyPath);
41-
} elseif ($rsaMatch === false) {
42-
throw new \RuntimeException(
43-
\sprintf('PCRE error [%d] encountered during key match attempt', \preg_last_error())
44-
);
45-
}
42+
$this->passPhrase = $passPhrase;
4643

47-
if (\strpos($keyPath, 'file://') !== 0) {
48-
$keyPath = 'file://' . $keyPath;
49-
}
44+
if (\is_file($keyPath)) {
45+
if (\strpos($keyPath, self::FILE_PREFIX) !== 0) {
46+
$keyPath = self::FILE_PREFIX . $keyPath;
47+
}
5048

51-
if (!\file_exists($keyPath) || !\is_readable($keyPath)) {
52-
throw new LogicException(\sprintf('Key path "%s" does not exist or is not readable', $keyPath));
49+
if (!\is_readable($keyPath)) {
50+
throw new LogicException(\sprintf('Key path "%s" does not exist or is not readable', $keyPath));
51+
}
52+
$isFileKey = true;
53+
$contents = \file_get_contents($keyPath);
54+
$this->keyPath = $keyPath;
55+
} else {
56+
$isFileKey = false;
57+
$contents = $keyPath;
58+
$this->keyPath = $this->saveKeyToFile($keyPath);
5359
}
5460

5561
if ($keyPermissionsCheck === true) {
5662
// Verify the permissions of the key
57-
$keyPathPerms = \decoct(\fileperms($keyPath) & 0777);
63+
$keyPathPerms = \decoct(\fileperms($this->keyPath) & 0777);
5864
if (\in_array($keyPathPerms, ['400', '440', '600', '640', '660'], true) === false) {
59-
\trigger_error(\sprintf(
60-
'Key file "%s" permissions are not correct, recommend changing to 600 or 660 instead of %s',
61-
$keyPath,
62-
$keyPathPerms
63-
), E_USER_NOTICE);
65+
\trigger_error(
66+
\sprintf(
67+
'Key file "%s" permissions are not correct, recommend changing to 600 or 660 instead of %s',
68+
$this->keyPath,
69+
$keyPathPerms
70+
),
71+
E_USER_NOTICE
72+
);
6473
}
6574
}
6675

67-
$this->keyPath = $keyPath;
68-
$this->passPhrase = $passPhrase;
76+
if (!$this->isValidKey($contents, $this->passPhrase ?? '')) {
77+
throw new LogicException('Unable to read key' . ($isFileKey ? " from file $keyPath" : ''));
78+
}
6979
}
7080

7181
/**
@@ -81,7 +91,7 @@ private function saveKeyToFile($key)
8191
$keyPath = $tmpDir . '/' . \sha1($key) . '.key';
8292

8393
if (\file_exists($keyPath)) {
84-
return 'file://' . $keyPath;
94+
return self::FILE_PREFIX . $keyPath;
8595
}
8696

8797
if (\file_put_contents($keyPath, $key) === false) {
@@ -96,7 +106,30 @@ private function saveKeyToFile($key)
96106
// @codeCoverageIgnoreEnd
97107
}
98108

99-
return 'file://' . $keyPath;
109+
return self::FILE_PREFIX . $keyPath;
110+
}
111+
112+
/**
113+
* Validate key contents.
114+
*
115+
* @param string $contents
116+
* @param string $passPhrase
117+
*
118+
* @return bool
119+
*/
120+
private function isValidKey($contents, $passPhrase)
121+
{
122+
$pkey = \openssl_pkey_get_private($contents, $passPhrase) ?: \openssl_pkey_get_public($contents);
123+
if ($pkey === false) {
124+
return false;
125+
}
126+
$details = \openssl_pkey_get_details($pkey);
127+
128+
return $details !== false && \in_array(
129+
$details['type'] ?? -1,
130+
[OPENSSL_KEYTYPE_RSA, OPENSSL_KEYTYPE_EC],
131+
true
132+
);
100133
}
101134

102135
/**

‎tests/Utils/CryptKeyTest.php‎

Lines changed: 82 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,7 @@ public function testKeyFileCreation()
3333

3434
$key = new CryptKey($keyContent);
3535

36-
$this->assertEquals(
37-
'file://' . \sys_get_temp_dir() . '/' . \sha1($keyContent) . '.key',
38-
$key->getKeyPath()
39-
);
36+
$this->assertEquals(self::generateKeyPath($keyContent), $key->getKeyPath());
4037

4138
$keyContent = \file_get_contents(__DIR__ . '/../Stubs/private.key.crlf');
4239

@@ -46,24 +43,92 @@ public function testKeyFileCreation()
4643

4744
$key = new CryptKey($keyContent);
4845

49-
$this->assertEquals(
50-
'file://' . \sys_get_temp_dir() . '/' . \sha1($keyContent) . '.key',
51-
$key->getKeyPath()
52-
);
46+
$this->assertEquals(self::generateKeyPath($keyContent), $key->getKeyPath());
47+
}
48+
49+
public function testUnsupportedKeyType()
50+
{
51+
$this->expectException(\LogicException::class);
52+
$this->expectExceptionMessage('Unable to read key');
53+
54+
try {
55+
// Create the keypair
56+
$res = \openssl_pkey_new([
57+
'digest_alg' => 'sha512',
58+
'private_key_bits' => 2048,
59+
'private_key_type' => OPENSSL_KEYTYPE_DSA,
60+
]);
61+
// Get private key
62+
\openssl_pkey_export($res, $keyContent, 'mystrongpassword');
63+
$path = self::generateKeyPath($keyContent);
64+
65+
new CryptKey($keyContent, 'mystrongpassword');
66+
} finally {
67+
if (isset($path)) {
68+
@\unlink($path);
69+
}
70+
}
71+
}
72+
73+
public function testECKeyType()
74+
{
75+
try {
76+
// Create the keypair
77+
$res = \openssl_pkey_new([
78+
'digest_alg' => 'sha512',
79+
'curve_name' => 'prime256v1',
80+
'private_key_type' => OPENSSL_KEYTYPE_EC,
81+
]);
82+
// Get private key
83+
\openssl_pkey_export($res, $keyContent, 'mystrongpassword');
84+
85+
$key = new CryptKey($keyContent, 'mystrongpassword');
86+
$path = self::generateKeyPath($keyContent);
87+
88+
$this->assertEquals($path, $key->getKeyPath());
89+
$this->assertEquals('mystrongpassword', $key->getPassPhrase());
90+
} catch (\Throwable $e) {
91+
$this->fail('The EC key was not created');
92+
} finally {
93+
if (isset($path)) {
94+
@\unlink($path);
95+
}
96+
}
97+
}
98+
99+
public function testRSAKeyType()
100+
{
101+
try {
102+
// Create the keypair
103+
$res = \openssl_pkey_new([
104+
'digest_alg' => 'sha512',
105+
'private_key_bits' => 2048,
106+
'private_key_type' => OPENSSL_KEYTYPE_RSA,
107+
]);
108+
// Get private key
109+
\openssl_pkey_export($res, $keyContent, 'mystrongpassword');
110+
111+
$key = new CryptKey($keyContent, 'mystrongpassword');
112+
$path = self::generateKeyPath($keyContent);
113+
114+
$this->assertEquals($path, $key->getKeyPath());
115+
$this->assertEquals('mystrongpassword', $key->getPassPhrase());
116+
} catch (\Throwable $e) {
117+
$this->fail('The RSA key was not created');
118+
} finally {
119+
if (isset($path)) {
120+
@\unlink($path);
121+
}
122+
}
53123
}
54124

55125
/**
56-
* Test whether we get a RuntimeException if a PCRE error is encountered.
126+
* @param string $keyContent
57127
*
58-
* @link https://www.php.net/manual/en/function.preg-last-error.php
128+
* @return string
59129
*/
60-
public function testPcreErrorExceptions()
130+
private static function generateKeyPath($keyContent)
61131
{
62-
$this->expectException(\RuntimeException::class);
63-
$this->expectExceptionMessageMatches('/^PCRE error/');
64-
65-
new class('foobar foobar foobar') extends CryptKey {
66-
const RSA_KEY_PATTERN = '/(?:\D+|<\d+>)*[!?]/';
67-
};
132+
return 'file://' . \sys_get_temp_dir() . '/' . \sha1($keyContent) . '.key';
68133
}
69134
}

0 commit comments

Comments
 (0)