Skip to content

Bugs/incorrect rsa keys #420

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.

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

Merged
merged 1 commit into from
Dec 27, 2022
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
58 changes: 38 additions & 20 deletions src/Component/Core/Util/RSAKey.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@

namespace Jose\Component\Core\Util;

use SpomkyLabs\Pki\ASN1\Type\Constructed\Sequence;
use SpomkyLabs\Pki\ASN1\Type\Primitive\BitString;
use SpomkyLabs\Pki\ASN1\Type\Primitive\Integer;
use SpomkyLabs\Pki\ASN1\Type\Primitive\NullType;
use SpomkyLabs\Pki\ASN1\Type\Primitive\ObjectIdentifier;
use SpomkyLabs\Pki\ASN1\Type\Primitive\OctetString;
use SpomkyLabs\Pki\CryptoEncoding\PEM;
use SpomkyLabs\Pki\CryptoTypes\AlgorithmIdentifier\Asymmetric\RSAEncryptionAlgorithmIdentifier;
use function array_key_exists;
use function count;
use InvalidArgumentException;
Expand All @@ -19,7 +27,7 @@
*/
final class RSAKey
{
private null|RSAPrivateKey|RSAPublicKey $sequence = null;
private null|Sequence $sequence = null;

private readonly array $values;

Expand Down Expand Up @@ -131,27 +139,37 @@ public function toArray(): array
public function toPEM(): string
{
if (array_key_exists('d', $this->values)) {
$this->sequence = RSAPrivateKey::create(
$this->fromBase64ToInteger($this->values['n']),
$this->fromBase64ToInteger($this->values['e']),
$this->fromBase64ToInteger($this->values['d']),
isset($this->values['p']) ? $this->fromBase64ToInteger($this->values['p']) : '0',
isset($this->values['q']) ? $this->fromBase64ToInteger($this->values['q']) : '0',
isset($this->values['dp']) ? $this->fromBase64ToInteger($this->values['dp']) : '0',
isset($this->values['dq']) ? $this->fromBase64ToInteger($this->values['dq']) : '0',
isset($this->values['qi']) ? $this->fromBase64ToInteger($this->values['qi']) : '0',
$this->sequence = Sequence::create(
Integer::create(0),
RSAEncryptionAlgorithmIdentifier::create()->toASN1(),
OctetString::create(
RSAPrivateKey::create(
$this->fromBase64ToInteger($this->values['n']),
$this->fromBase64ToInteger($this->values['e']),
$this->fromBase64ToInteger($this->values['d']),
isset($this->values['p']) ? $this->fromBase64ToInteger($this->values['p']) : '0',
isset($this->values['q']) ? $this->fromBase64ToInteger($this->values['q']) : '0',
isset($this->values['dp']) ? $this->fromBase64ToInteger($this->values['dp']) : '0',
isset($this->values['dq']) ? $this->fromBase64ToInteger($this->values['dq']) : '0',
isset($this->values['qi']) ? $this->fromBase64ToInteger($this->values['qi']) : '0',
)->toDER()
)
);
} else {
$this->sequence = RSAPublicKey::create(
$this->fromBase64ToInteger($this->values['n']),
$this->fromBase64ToInteger($this->values['e'])
);
}
if ($this->sequence === null) {
throw new RuntimeException();
}

return $this->sequence->toPEM()
return PEM::create(PEM::TYPE_RSA_PRIVATE_KEY, $this->sequence->toDER())
->string();
}
$this->sequence = Sequence::create(
RSAEncryptionAlgorithmIdentifier::create()->toASN1(),
BitString::create(
RSAPublicKey::create(
$this->fromBase64ToInteger($this->values['n']),
$this->fromBase64ToInteger($this->values['e'])
)->toDER()
)
);

return PEM::create(PEM::TYPE_RSA_PUBLIC_KEY, $this->sequence->toDER())
->string();
}

Expand Down
26 changes: 22 additions & 4 deletions tests/Component/KeyManagement/CertificateTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use InvalidArgumentException;
use Jose\Component\Core\JWK;
use Jose\Component\Core\Util\RSAKey;
use Jose\Component\KeyManagement\JWKFactory;
use Jose\Component\KeyManagement\KeyConverter\KeyConverter;
use PHPUnit\Framework\TestCase;
Expand Down Expand Up @@ -41,9 +42,12 @@ public function fileNotValid(): void
/**
* @test
*/
public function certificateConversion(): void
public function rsaEncryptedPrivateKeyConversion(): void
{
// When
$details = KeyConverter::loadFromKeyFile(__DIR__ . '/Keys/RSA/private.encrypted.key', 'tests');

// Then
static::assertEqualsCanonicalizing($details, [
'kty' => 'RSA',
'n' => 'tpS1ZmfVKVP5KofIhMBP0tSWc4qlh6fm2lrZSkuKxUjEaWjzZSzs72gEIGxraWusMdoRuV54xsWRyf5KeZT0S-I5Prle3Idi3gICiO4NwvMk6JwSBcJWwmSLFEKyUSnB2CtfiGc0_5rQCpcEt_Dn5iM-BNn7fqpoLIbks8rXKUIj8-qMVqkTXsEKeKinE23t1ykMldsNaaOH-hvGti5Jt2DMnH1JjoXdDXfxvSP_0gjUYb0ektudYFXoA6wekmQyJeImvgx4Myz1I4iHtkY_Cp7J4Mn1ejZ6HNmyvoTE_4OuY1uCeYv4UyXFc1s1uUyYtj4z57qsHGsS4dQ3A2MJsw',
Expand All @@ -55,13 +59,27 @@ public function certificateConversion(): void
'dq' => 'Swz1-m_vmTFN_pu1bK7vF7S5nNVrL4A0OFiEsGliCmuJWzOKdL14DiYxctvnw3H6qT2dKZZfV2tbse5N9-JecdldUjfuqAoLIe7dD7dKi42YOlTC9QXmqvTh1ohnJu8pmRFXEZQGUm_BVhoIb2_WPkjav6YSkguCUHt4HRd2YwE',
'qi' => 'BocuCOEOq-oyLDALwzMXU8gOf3IL1Q1_BWwsdoANoh6i179psxgE4JXToWcpXZQQqub8ngwE6uR9fpd3m6N_PL4T55vbDDyjPKmrL2ttC2gOtx9KrpPh-Z7LQRo4BE48nHJJrystKHfFlaH2G7JxHNgMBYVADyttN09qEoav8Os',
]);
}

$details = KeyConverter::loadFromKeyFile(__DIR__ . '/Keys/RSA/public.key');
static::assertSame($details, [
/**
* @test
*/
public function rsaPublicKeyConversion(): void
{
// When
$filename = __DIR__ . '/Keys/RSA/public.key';
$content = trim(file_get_contents($filename));
$details = KeyConverter::loadFromKeyFile($filename);
$values = [
'kty' => 'RSA',
'n' => 'tpS1ZmfVKVP5KofIhMBP0tSWc4qlh6fm2lrZSkuKxUjEaWjzZSzs72gEIGxraWusMdoRuV54xsWRyf5KeZT0S-I5Prle3Idi3gICiO4NwvMk6JwSBcJWwmSLFEKyUSnB2CtfiGc0_5rQCpcEt_Dn5iM-BNn7fqpoLIbks8rXKUIj8-qMVqkTXsEKeKinE23t1ykMldsNaaOH-hvGti5Jt2DMnH1JjoXdDXfxvSP_0gjUYb0ektudYFXoA6wekmQyJeImvgx4Myz1I4iHtkY_Cp7J4Mn1ejZ6HNmyvoTE_4OuY1uCeYv4UyXFc1s1uUyYtj4z57qsHGsS4dQ3A2MJsw',
'e' => 'AQAB',
]);
];

// Then
static::assertSame($details, $values);
$rsaKey = RSAKey::createFromJWK(new JWK($values));
static::assertSame($content, $rsaKey->toPEM());
}

/**
Expand Down
48 changes: 23 additions & 25 deletions tests/Component/KeyManagement/JWKFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Jose\Tests\Component\KeyManagement;

use Jose\Component\Core\Util\ECKey;
use Jose\Component\KeyManagement\JWKFactory;
use const JSON_THROW_ON_ERROR;
use ParagonIE\ConstantTime\Base64UrlSafe;
Expand Down Expand Up @@ -219,42 +220,39 @@ public function createFromPrivateEC512KeyFileEncrypted(): void
}

/**
* @dataProvider publicKeysAndPem
* @test
*/
public function createFromPublicEC256KeyFile(): void
public function createFromPublicEC512KeyFile(string $filename, string $expectedJWK): void
{
$result = JWKFactory::createFromKeyFile(__DIR__ . '/Keys/EC/public.es256.key');
// Given
$content = file_get_contents($filename);

static::assertSame(
'{"kty":"EC","crv":"P-256","x":"vuYsP-QnrqAbM7Iyhzjt08hFSuzapyojCB_gFsBt65U","y":"oq-E2K-X0kPeqGuKnhlXkxc5fnxomRSC6KLby7Ij8AE"}',
json_encode($result, JSON_THROW_ON_ERROR)
);
}

/**
* @test
*/
public function createFromPublicEC384KeyFile(): void
{
$result = JWKFactory::createFromKeyFile(__DIR__ . '/Keys/EC/public.es384.key');
// When
$jwk = JWKFactory::createFromKeyFile($filename);

// Then
static::assertSame(
'{"kty":"EC","crv":"P-384","x":"6f-XZsg2Tvn0EoEapQ-ylMYNtsm8CPf0cb8HI2EkfY9Bqpt3QMzwlM7mVsFRmaMZ","y":"b8nOnRwmpmEnvA2U8ydS-dbnPv7bwYl-q1qNeh8Wpjor3VO-RTt4ce0Pn25oGGWU"}',
json_encode($result, JSON_THROW_ON_ERROR)
$expectedJWK,
json_encode($jwk, JSON_THROW_ON_ERROR)
);
static::assertSame($content, ECKey::convertPublicKeyToPEM($jwk));
}

/**
* @test
*/
public function createFromPublicEC512KeyFile(): void
public function publicKeysAndPem(): iterable
{
$result = JWKFactory::createFromKeyFile(__DIR__ . '/Keys/EC/public.es512.key');

static::assertSame(
yield [
__DIR__ . '/Keys/EC/public.es256.key',
'{"kty":"EC","crv":"P-256","x":"vuYsP-QnrqAbM7Iyhzjt08hFSuzapyojCB_gFsBt65U","y":"oq-E2K-X0kPeqGuKnhlXkxc5fnxomRSC6KLby7Ij8AE"}',
];
yield [
__DIR__ . '/Keys/EC/public.es384.key',
'{"kty":"EC","crv":"P-384","x":"6f-XZsg2Tvn0EoEapQ-ylMYNtsm8CPf0cb8HI2EkfY9Bqpt3QMzwlM7mVsFRmaMZ","y":"b8nOnRwmpmEnvA2U8ydS-dbnPv7bwYl-q1qNeh8Wpjor3VO-RTt4ce0Pn25oGGWU"}',
];
yield [
__DIR__ . '/Keys/EC/public.es512.key',
'{"kty":"EC","crv":"P-521","x":"AVpvo7TGpQk5P7ZLo0qkBpaT-fFDv6HQrWElBKMxcrJd_mRNapweATsVv83YON4lTIIRXzgGkmWeqbDr6RQO-1cS","y":"AIs-MoRmLaiPyG2xmPwQCHX2CGX_uCZiT3iOxTAJEZuUbeSA828K4WfAA4ODdGiB87YVShhPOkiQswV3LpbpPGhC"}',
json_encode($result, JSON_THROW_ON_ERROR)
);
];
}

/**
Expand Down
36 changes: 36 additions & 0 deletions tests/Component/KeyManagement/JWKTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use InvalidArgumentException;
use Jose\Component\Core\JWK;
use Jose\Component\Core\JWKSet;
use Jose\Component\Core\Util\RSAKey;
use Jose\Component\KeyManagement\JWKFactory;
use const JSON_THROW_ON_ERROR;
use ParagonIE\ConstantTime\Base64UrlSafe;
Expand Down Expand Up @@ -239,4 +240,39 @@ public function loadCertificateChain(): void
$key->all()
);
}

/**
* @test
*/
public function theRSAKeyIsCorrectlyConvertedIntoPEM(): void
{
// Given
$key = new JWK([
'kty' => 'RSA',
'n' => 'z62tHQzm4fDHipqlcrNhC1gUdn0N38pmlcQbVlLvtZf1aRm1OO43cB9YQyWr1MsTrYH4nyWZDMPIGY_BsIfYw1lp9fo2D1tpG2vtCaKRETVimu-N9DySQ9vYs6n8lG0vXy_spK7sGrOLFooijDSt0LYrYrZY9UI3OkyEAKUbZLJhxi7nT3CPtMCYDUMIIt1LgWdR6-ha5fQQrWF7YbyiMNmITg64DZ9yof4-OfouNE2dFXGl3Nr92HaugXbMZF_pILpcB61NT215aql1ifVXvEyGAsyPBnxIcjadfcgQ0UUtepN2BJRj_pq55jfQR2Nl0e11JeKEIPR3ypqvKeDI10Cl-qr9GpU0rFfw2vcp8IHTNrAeam4nTRDVCmXGwiMaLifAKbvfGwxaA2mHbO5i4669KiPf_lXAQz9FzAZZRwpdM1FTB9BlB5R-JgvtBabP5ZGhqlUOgkJM_4UfrpcIkS8Ub4Y60QvPkInCGBMHNdUqpJUkLoA5Mddl8hVW-cMjC2qCckgT1KgZxIsZTgOJXCARX1IObFJNoinxYJ5SNX9bCSRtgefuBKE7BSNukAkHyBPf---kEi9GbYXzlJr-yCMAIsA0UoiEx264hkAF9zF-N1yRhS_QmrhzU5hpj1IE8WRCqyIZV8f_IbSGXBue7MmgknLVRWHuGqehkTSfiNE',
'e' => 'AQAB',
]);

// When
$pem = RSAKey::createFromJWK($key)->toPEM();

// Then
static::assertSame(
'-----BEGIN RSA PUBLIC KEY-----
MIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIICCgKCAgEAz62tHQzm4fDHipqlcrNh
C1gUdn0N38pmlcQbVlLvtZf1aRm1OO43cB9YQyWr1MsTrYH4nyWZDMPIGY/BsIfY
w1lp9fo2D1tpG2vtCaKRETVimu+N9DySQ9vYs6n8lG0vXy/spK7sGrOLFooijDSt
0LYrYrZY9UI3OkyEAKUbZLJhxi7nT3CPtMCYDUMIIt1LgWdR6+ha5fQQrWF7Ybyi
MNmITg64DZ9yof4+OfouNE2dFXGl3Nr92HaugXbMZF/pILpcB61NT215aql1ifVX
vEyGAsyPBnxIcjadfcgQ0UUtepN2BJRj/pq55jfQR2Nl0e11JeKEIPR3ypqvKeDI
10Cl+qr9GpU0rFfw2vcp8IHTNrAeam4nTRDVCmXGwiMaLifAKbvfGwxaA2mHbO5i
4669KiPf/lXAQz9FzAZZRwpdM1FTB9BlB5R+JgvtBabP5ZGhqlUOgkJM/4UfrpcI
kS8Ub4Y60QvPkInCGBMHNdUqpJUkLoA5Mddl8hVW+cMjC2qCckgT1KgZxIsZTgOJ
XCARX1IObFJNoinxYJ5SNX9bCSRtgefuBKE7BSNukAkHyBPf+++kEi9GbYXzlJr+
yCMAIsA0UoiEx264hkAF9zF+N1yRhS/QmrhzU5hpj1IE8WRCqyIZV8f/IbSGXBue
7MmgknLVRWHuGqehkTSfiNECAwEAAQ==
-----END RSA PUBLIC KEY-----',
$pem
);
}
}
15 changes: 12 additions & 3 deletions tests/Component/KeyManagement/Keys/ECKeysTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Jose\Tests\Component\KeyManagement\Keys;

use Jose\Component\Core\Util\ECKey;
use const DIRECTORY_SEPARATOR;
use InvalidArgumentException;
use Jose\Component\Core\JWK;
Expand Down Expand Up @@ -71,17 +72,25 @@ public function loadPublicEC256Key(): void
*/
public function loadPrivateEC256Key(): void
{
// Given
$private_pem = file_get_contents(
'file://' . __DIR__ . DIRECTORY_SEPARATOR . 'EC' . DIRECTORY_SEPARATOR . 'private.es256.key'
);
$details = KeyConverter::loadFromKey($private_pem);
static::assertSame($details, [
$expectedValues = [
'kty' => 'EC',
'crv' => 'P-256',
'd' => 'q_VkzNnxTG39jHB0qkwA_SeVXud7yCHT7kb7kZv-0xQ',
'x' => 'vuYsP-QnrqAbM7Iyhzjt08hFSuzapyojCB_gFsBt65U',
'y' => 'oq-E2K-X0kPeqGuKnhlXkxc5fnxomRSC6KLby7Ij8AE',
]);
];

// Whent
$details = KeyConverter::loadFromKey($private_pem, 'test');

//Then
static::assertSame($details, $expectedValues);
$ecKey = ECKey::convertPrivateKeyToPEM(new JWK($expectedValues));
static::assertSame($private_pem, $ecKey);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions tests/Component/KeyManagement/Keys/RSA/private.key
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
-----BEGIN PRIVATE KEY-----
-----BEGIN RSA PRIVATE KEY-----
MIICdwIBADANBgkqhkiG9w0BAQEFAASCAmEwggJdAgEAAoGBAN91kQxBuaze3WjI
CNjeR/HD8E3kDzp89+Lhtn3tMish4yQxhNl6BEkabuS3pUj3WDP6+AFjBVqA1j3f
u8Wqu7hRJDPHOs2kCII+LhIqvqQTLx/nvNOUhW2DimKn0HuHnlwJODq0MHFJEq5R
Expand All @@ -13,4 +13,4 @@ RVdVjA/gFqJp1Q+VWdS1tvYRIqmadkECQCVdqQuwgedEHmcewtNod42crjmwvWBx
BKMTl6/WT4zwVb41eUujVWo0LHRLuCoK//GDqmloIh6L3MU8MqnIGb0CQFWcpD4/
roCkMblk0hPoQPpyapJexc438x7XuEGFEhyxxauqC5R4YFKCf+KBS2gZgr4GSwBU
Qww+qZ3eRYM7faM=
-----END PRIVATE KEY-----
-----END RSA PRIVATE KEY-----
4 changes: 2 additions & 2 deletions tests/Component/KeyManagement/Keys/RSA/public.key
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
-----BEGIN PUBLIC KEY-----
-----BEGIN RSA PUBLIC KEY-----
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAtpS1ZmfVKVP5KofIhMBP
0tSWc4qlh6fm2lrZSkuKxUjEaWjzZSzs72gEIGxraWusMdoRuV54xsWRyf5KeZT0
S+I5Prle3Idi3gICiO4NwvMk6JwSBcJWwmSLFEKyUSnB2CtfiGc0/5rQCpcEt/Dn
5iM+BNn7fqpoLIbks8rXKUIj8+qMVqkTXsEKeKinE23t1ykMldsNaaOH+hvGti5J
t2DMnH1JjoXdDXfxvSP/0gjUYb0ektudYFXoA6wekmQyJeImvgx4Myz1I4iHtkY/
Cp7J4Mn1ejZ6HNmyvoTE/4OuY1uCeYv4UyXFc1s1uUyYtj4z57qsHGsS4dQ3A2MJ
swIDAQAB
-----END PUBLIC KEY-----
-----END RSA PUBLIC KEY-----
19 changes: 14 additions & 5 deletions tests/Component/KeyManagement/Keys/RSAKeysTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

namespace Jose\Tests\Component\KeyManagement\Keys;

use SpomkyLabs\Pki\ASN1\Type\Constructed\Sequence;
use SpomkyLabs\Pki\ASN1\Type\Primitive\RelativeOID;
use SpomkyLabs\Pki\CryptoEncoding\PEM;
use const DIRECTORY_SEPARATOR;
use InvalidArgumentException;
use Jose\Component\Core\JWK;
Expand Down Expand Up @@ -134,9 +137,14 @@ public function loadPublicRSAKeyFromValues(): void
*/
public function loadPrivateRSAKey(): void
{
$file = 'file://' . __DIR__ . DIRECTORY_SEPARATOR . 'RSA' . DIRECTORY_SEPARATOR . 'private.key';
$rsa_key = RSAKey::createFromPEM($file);
// Given
$file = __DIR__ . '/RSA/private.key';
$content = trim(file_get_contents($file));

// When
$rsaKey = RSAKey::createFromPEM('file://'.$file);

// Then
static::assertEqualsCanonicalizing([
'kty' => 'RSA',
'n' => '33WRDEG5rN7daMgI2N5H8cPwTeQPOnz34uG2fe0yKyHjJDGE2XoESRpu5LelSPdYM_r4AWMFWoDWPd-7xaq7uFEkM8c6zaQIgj4uEiq-pBMvH-e805SFbYOKYqfQe4eeXAk4OrQwcUkSrlGskf6YUaw_3IwbPgzEDTgTZFVtQlE',
Expand All @@ -147,16 +155,17 @@ public function loadPrivateRSAKey(): void
'dp' => '5m79fpE1Jz0YE1ijT7ivOMAws-fnTCnR08eiB8-W36GBWplbHaXejrJFV1WMD-AWomnVD5VZ1LW29hEiqZp2QQ',
'dq' => 'JV2pC7CB50QeZx7C02h3jZyuObC9YHEEoxOXr9ZPjPBVvjV5S6NVajQsdEu4Kgr_8YOqaWgiHovcxTwyqcgZvQ',
'qi' => 'VZykPj-ugKQxuWTSE-hA-nJqkl7FzjfzHte4QYUSHLHFq6oLlHhgUoJ_4oFLaBmCvgZLAFRDDD6pnd5Fgzt9ow',
], $rsa_key->toArray());
static::assertFalse($rsa_key->isPublic());
], $rsaKey->toArray());
static::assertFalse($rsaKey->isPublic());

$public_key = RSAKey::toPublic($rsa_key);
$public_key = RSAKey::toPublic($rsaKey);
static::assertSame([
'kty' => 'RSA',
'n' => '33WRDEG5rN7daMgI2N5H8cPwTeQPOnz34uG2fe0yKyHjJDGE2XoESRpu5LelSPdYM_r4AWMFWoDWPd-7xaq7uFEkM8c6zaQIgj4uEiq-pBMvH-e805SFbYOKYqfQe4eeXAk4OrQwcUkSrlGskf6YUaw_3IwbPgzEDTgTZFVtQlE',
'e' => 'AQAB',
], $public_key->toArray());
static::assertTrue($public_key->isPublic());
static::assertSame($content, \Jose\Component\Core\Util\RSAKey::createFromJWK($rsaKey->toJwk())->toPEM());
}

/**
Expand Down