From 713393c8ad1c53042491007567343054a805f364 Mon Sep 17 00:00:00 2001 From: terrafrost Date: Sat, 6 Sep 2014 11:13:11 -0500 Subject: [PATCH 1/5] RSA: make XML keys use unsigned integers PKCS1 / PKCS8 keys need *signed* integers because of section 8.3.3 at http://www.itu.int/ITU-T/studygroups/com17/languages/X.690-0207.pdf#page=7 --- phpseclib/Crypt/RSA.php | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/phpseclib/Crypt/RSA.php b/phpseclib/Crypt/RSA.php index dd18bc26..95617510 100644 --- a/phpseclib/Crypt/RSA.php +++ b/phpseclib/Crypt/RSA.php @@ -742,17 +742,18 @@ class Crypt_RSA */ function _convertPrivateKey($n, $e, $d, $primes, $exponents, $coefficients) { + $unsigned = $this->privateKeyFormat == CRYPT_RSA_PRIVATE_FORMAT_XML; $num_primes = count($primes); $raw = array( 'version' => $num_primes == 2 ? chr(0) : chr(1), // two-prime vs. multi - 'modulus' => $n->toBytes(true), - 'publicExponent' => $e->toBytes(true), - 'privateExponent' => $d->toBytes(true), - 'prime1' => $primes[1]->toBytes(true), - 'prime2' => $primes[2]->toBytes(true), - 'exponent1' => $exponents[1]->toBytes(true), - 'exponent2' => $exponents[2]->toBytes(true), - 'coefficient' => $coefficients[2]->toBytes(true) + 'modulus' => $n->toBytes($unsigned), + 'publicExponent' => $e->toBytes($unsigned), + 'privateExponent' => $d->toBytes($unsigned), + 'prime1' => $primes[1]->toBytes($unsigned), + 'prime2' => $primes[2]->toBytes($unsigned), + 'exponent1' => $exponents[1]->toBytes($unsigned), + 'exponent2' => $exponents[2]->toBytes($unsigned), + 'coefficient' => $coefficients[2]->toBytes($unsigned) ); // if the format in question does not support multi-prime rsa and multi-prime rsa was used, @@ -941,8 +942,10 @@ class Crypt_RSA */ function _convertPublicKey($n, $e) { - $modulus = $n->toBytes(true); - $publicExponent = $e->toBytes(true); + $unsigned = $this->publicKeyFormat == CRYPT_RSA_PUBLIC_FORMAT_XML; + + $modulus = $n->toBytes($unsigned); + $publicExponent = $e->toBytes($unsigned); switch ($this->publicKeyFormat) { case CRYPT_RSA_PUBLIC_FORMAT_RAW: From 43290156299768d9a11e02e9f012774d113e1b8e Mon Sep 17 00:00:00 2001 From: terrafrost Date: Mon, 8 Sep 2014 22:26:14 -0500 Subject: [PATCH 2/5] RSA: add unit test --- tests/Unit/Crypt/RSA/LoadKeyTest.php | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/Unit/Crypt/RSA/LoadKeyTest.php b/tests/Unit/Crypt/RSA/LoadKeyTest.php index 9402ab42..ff5d332e 100644 --- a/tests/Unit/Crypt/RSA/LoadKeyTest.php +++ b/tests/Unit/Crypt/RSA/LoadKeyTest.php @@ -259,4 +259,24 @@ Ao8eayMp6FcvNucIpUndo1X8dKMv3Y26ZQIDAQAB $this->assertGreaterThanOrEqual(1, strlen("$rsa")); $this->assertFalse($rsa->getPublicKey()); } + + /** + * make phpseclib generated XML keys be unsigned. this may need to be reverted + * if it is later learned that XML keys are, in fact, supposed to be signed + * @group github468 + */ + public function testUnsignedXML() + { + $rsa = new Crypt_RSA(); + + $key = ' +v5OxcEgxPUfa701NpxnScCmlRkbwSGBiTWobHkIWZEB+AlRTHaVoZg/D8l6YzR7VdQidG6gF+nuUMjY75dBXgY/XcyVq0Hccf1jTfgARuNuq4GGG3hnCJVi2QsOgcf9R7TeXn+p1RKIhjQoWCiEQeEBTotNbJhcabNcPGSEJw+s= +AQAB +'; + + $rsa->loadKey($key); + $newkey = $rsa->getPublicKey(CRYPT_RSA_PUBLIC_FORMAT_XML); + + $this->assertSame($key, $newkey); + } } From c489852332e7fed5fbd4a286f80faa88c1abc0f4 Mon Sep 17 00:00:00 2001 From: terrafrost Date: Tue, 9 Sep 2014 00:28:38 -0500 Subject: [PATCH 3/5] RSA: update unit test --- phpseclib/Crypt/RSA.php | 24 ++++++++++++------------ tests/Unit/Crypt/RSA/LoadKeyTest.php | 26 ++++++++++++++++++++++++-- 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/phpseclib/Crypt/RSA.php b/phpseclib/Crypt/RSA.php index 95617510..a0fbc285 100644 --- a/phpseclib/Crypt/RSA.php +++ b/phpseclib/Crypt/RSA.php @@ -742,18 +742,18 @@ class Crypt_RSA */ function _convertPrivateKey($n, $e, $d, $primes, $exponents, $coefficients) { - $unsigned = $this->privateKeyFormat == CRYPT_RSA_PRIVATE_FORMAT_XML; + $signed = $this->privateKeyFormat != CRYPT_RSA_PRIVATE_FORMAT_XML; $num_primes = count($primes); $raw = array( 'version' => $num_primes == 2 ? chr(0) : chr(1), // two-prime vs. multi - 'modulus' => $n->toBytes($unsigned), - 'publicExponent' => $e->toBytes($unsigned), - 'privateExponent' => $d->toBytes($unsigned), - 'prime1' => $primes[1]->toBytes($unsigned), - 'prime2' => $primes[2]->toBytes($unsigned), - 'exponent1' => $exponents[1]->toBytes($unsigned), - 'exponent2' => $exponents[2]->toBytes($unsigned), - 'coefficient' => $coefficients[2]->toBytes($unsigned) + 'modulus' => $n->toBytes($signed), + 'publicExponent' => $e->toBytes($signed), + 'privateExponent' => $d->toBytes($signed), + 'prime1' => $primes[1]->toBytes($signed), + 'prime2' => $primes[2]->toBytes($signed), + 'exponent1' => $exponents[1]->toBytes($signed), + 'exponent2' => $exponents[2]->toBytes($signed), + 'coefficient' => $coefficients[2]->toBytes($signed) ); // if the format in question does not support multi-prime rsa and multi-prime rsa was used, @@ -942,10 +942,10 @@ class Crypt_RSA */ function _convertPublicKey($n, $e) { - $unsigned = $this->publicKeyFormat == CRYPT_RSA_PUBLIC_FORMAT_XML; + $signed = $this->publicKeyFormat != CRYPT_RSA_PUBLIC_FORMAT_XML; - $modulus = $n->toBytes($unsigned); - $publicExponent = $e->toBytes($unsigned); + $modulus = $n->toBytes($signed); + $publicExponent = $e->toBytes($signed); switch ($this->publicKeyFormat) { case CRYPT_RSA_PUBLIC_FORMAT_RAW: diff --git a/tests/Unit/Crypt/RSA/LoadKeyTest.php b/tests/Unit/Crypt/RSA/LoadKeyTest.php index ff5d332e..b1b078ad 100644 --- a/tests/Unit/Crypt/RSA/LoadKeyTest.php +++ b/tests/Unit/Crypt/RSA/LoadKeyTest.php @@ -270,13 +270,35 @@ Ao8eayMp6FcvNucIpUndo1X8dKMv3Y26ZQIDAQAB $rsa = new Crypt_RSA(); $key = ' -v5OxcEgxPUfa701NpxnScCmlRkbwSGBiTWobHkIWZEB+AlRTHaVoZg/D8l6YzR7VdQidG6gF+nuUMjY75dBXgY/XcyVq0Hccf1jTfgARuNuq4GGG3hnCJVi2QsOgcf9R7TeXn+p1RKIhjQoWCiEQeEBTotNbJhcabNcPGSEJw+s= -AQAB + v5OxcEgxPUfa701NpxnScCmlRkbwSGBiTWobHkIWZEB+AlRTHaVoZg/D8l6YzR7VdQidG6gF+nuUMjY75dBXgY/XcyVq0Hccf1jTfgARuNuq4GGG3hnCJVi2QsOgcf9R7TeXn+p1RKIhjQoWCiEQeEBTotNbJhcabNcPGSEJw+s= + AQAB '; $rsa->loadKey($key); + $rsa->setPublicKey(); $newkey = $rsa->getPublicKey(CRYPT_RSA_PUBLIC_FORMAT_XML); $this->assertSame($key, $newkey); } + + /** + * @group github468 + */ + public function testSignedPKCS1() + { + $rsa = new Crypt_RSA(); + + $key = '-----BEGIN PUBLIC KEY----- +MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQC/k7FwSDE9R9rvTU2nGdJwKaVG +RvBIYGJNahseQhZkQH4CVFMdpWhmD8PyXpjNHtV1CJ0bqAX6e5QyNjvl0FeBj9dz +JWrQdxx/WNN+ABG426rgYYbeGcIlWLZCw6Bx/1HtN5ef6nVEoiGNChYKIRB4QFOi +01smFxps1w8ZIQnD6wIDAQAB +-----END PUBLIC KEY-----'; + + $rsa->loadKey($key); + $rsa->setPublicKey(); + $newkey = $rsa->getPublicKey(); + + $this->assertSame($key, $newkey); + } } From e092733808873a214a9d028ec2343e6f4410c67a Mon Sep 17 00:00:00 2001 From: terrafrost Date: Tue, 9 Sep 2014 00:34:41 -0500 Subject: [PATCH 4/5] RSA: make unit tests perform string comparisons sans white space chars --- tests/Unit/Crypt/RSA/LoadKeyTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Unit/Crypt/RSA/LoadKeyTest.php b/tests/Unit/Crypt/RSA/LoadKeyTest.php index b1b078ad..cbbfaff8 100644 --- a/tests/Unit/Crypt/RSA/LoadKeyTest.php +++ b/tests/Unit/Crypt/RSA/LoadKeyTest.php @@ -278,7 +278,7 @@ Ao8eayMp6FcvNucIpUndo1X8dKMv3Y26ZQIDAQAB $rsa->setPublicKey(); $newkey = $rsa->getPublicKey(CRYPT_RSA_PUBLIC_FORMAT_XML); - $this->assertSame($key, $newkey); + $this->assertSame(preg_replace('#\s\', '', $key), preg_replace('#\s\', '', $newkey)); } /** @@ -299,6 +299,6 @@ JWrQdxx/WNN+ABG426rgYYbeGcIlWLZCw6Bx/1HtN5ef6nVEoiGNChYKIRB4QFOi $rsa->setPublicKey(); $newkey = $rsa->getPublicKey(); - $this->assertSame($key, $newkey); + $this->assertSame(preg_replace('#\s\', '', $key), preg_replace('#\s\', '', $newkey)); } } From f6bd3542b3d3a98d45e08f2bd853a1e429cb4fa5 Mon Sep 17 00:00:00 2001 From: terrafrost Date: Tue, 9 Sep 2014 00:43:28 -0500 Subject: [PATCH 5/5] RSA: syntax error in unit tests --- tests/Unit/Crypt/RSA/LoadKeyTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Unit/Crypt/RSA/LoadKeyTest.php b/tests/Unit/Crypt/RSA/LoadKeyTest.php index cbbfaff8..6506c478 100644 --- a/tests/Unit/Crypt/RSA/LoadKeyTest.php +++ b/tests/Unit/Crypt/RSA/LoadKeyTest.php @@ -278,7 +278,7 @@ Ao8eayMp6FcvNucIpUndo1X8dKMv3Y26ZQIDAQAB $rsa->setPublicKey(); $newkey = $rsa->getPublicKey(CRYPT_RSA_PUBLIC_FORMAT_XML); - $this->assertSame(preg_replace('#\s\', '', $key), preg_replace('#\s\', '', $newkey)); + $this->assertSame(preg_replace('#\s#', '', $key), preg_replace('#\s#', '', $newkey)); } /** @@ -299,6 +299,6 @@ JWrQdxx/WNN+ABG426rgYYbeGcIlWLZCw6Bx/1HtN5ef6nVEoiGNChYKIRB4QFOi $rsa->setPublicKey(); $newkey = $rsa->getPublicKey(); - $this->assertSame(preg_replace('#\s\', '', $key), preg_replace('#\s\', '', $newkey)); + $this->assertSame(preg_replace('#\s#', '', $key), preg_replace('#\s#', '', $newkey)); } }