Fixes incorrect RSA Keys PEM encoding (#420)

Spomky · web-flow · commit 8fe9bb9c4229 · 2022-12-27T20:47:08.000+01:00
diff --git a/src/Component/Core/Util/RSAKey.php b/src/Component/Core/Util/RSAKey.php
@@ -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;
@@ -19,7 +27,7 @@
  */
 final class RSAKey
 {
-    private null|RSAPrivateKey|RSAPublicKey $sequence = null;
+    private null|Sequence $sequence = null;
 
     private readonly array $values;
 
@@ -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();
     }
 
diff --git a/tests/Component/KeyManagement/CertificateTest.php b/tests/Component/KeyManagement/CertificateTest.php
@@ -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;
@@ -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',
@@ -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());
     }
 
     /**
diff --git a/tests/Component/KeyManagement/JWKFactoryTest.php b/tests/Component/KeyManagement/JWKFactoryTest.php
@@ -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;
@@ -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)
-        );
+        ];
     }
 
     /**
diff --git a/tests/Component/KeyManagement/JWKTest.php b/tests/Component/KeyManagement/JWKTest.php
@@ -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;
@@ -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
+        );
+    }
 }
diff --git a/tests/Component/KeyManagement/Keys/ECKeysTest.php b/tests/Component/KeyManagement/Keys/ECKeysTest.php
@@ -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;
@@ -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);
     }
 
     /**
diff --git a/tests/Component/KeyManagement/Keys/RSA/private.key b/tests/Component/KeyManagement/Keys/RSA/private.key
@@ -1,4 +1,4 @@
------BEGIN PRIVATE KEY-----
+-----BEGIN RSA PRIVATE KEY-----
 MIICdwIBADANBgkqhkiG9w0BAQEFAASCAmEwggJdAgEAAoGBAN91kQxBuaze3WjI
 CNjeR/HD8E3kDzp89+Lhtn3tMish4yQxhNl6BEkabuS3pUj3WDP6+AFjBVqA1j3f
 u8Wqu7hRJDPHOs2kCII+LhIqvqQTLx/nvNOUhW2DimKn0HuHnlwJODq0MHFJEq5R
@@ -13,4 +13,4 @@ RVdVjA/gFqJp1Q+VWdS1tvYRIqmadkECQCVdqQuwgedEHmcewtNod42crjmwvWBx
 BKMTl6/WT4zwVb41eUujVWo0LHRLuCoK//GDqmloIh6L3MU8MqnIGb0CQFWcpD4/
 roCkMblk0hPoQPpyapJexc438x7XuEGFEhyxxauqC5R4YFKCf+KBS2gZgr4GSwBU
 Qww+qZ3eRYM7faM=
------END PRIVATE KEY-----
+-----END RSA PRIVATE KEY-----
diff --git a/tests/Component/KeyManagement/Keys/RSA/public.key b/tests/Component/KeyManagement/Keys/RSA/public.key
@@ -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-----
diff --git a/tests/Component/KeyManagement/Keys/RSAKeysTest.php b/tests/Component/KeyManagement/Keys/RSAKeysTest.php
@@ -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;
@@ -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',
@@ -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());
     }
 
     /**
