From 3ba59020466a934f38a0c62044d40e03e37eff7a Mon Sep 17 00:00:00 2001 From: terrafrost Date: Sat, 4 Jan 2020 16:26:55 -0600 Subject: [PATCH] RSA / X509: misc fixes (mostly related to PSS) --- phpseclib/Crypt/RSA.php | 10 +++++- phpseclib/Crypt/RSA/Formats/Keys/PSS.php | 2 +- phpseclib/File/X509.php | 21 ++++++++--- tests/Unit/Crypt/RSA/ModeTest.php | 28 +++++++++++++++ tests/Unit/File/X509/X509Test.php | 44 ++++++++++++++++++++++++ 5 files changed, 98 insertions(+), 7 deletions(-) diff --git a/phpseclib/Crypt/RSA.php b/phpseclib/Crypt/RSA.php index d73da4c4..431eb5fd 100644 --- a/phpseclib/Crypt/RSA.php +++ b/phpseclib/Crypt/RSA.php @@ -37,6 +37,14 @@ * ?> * * + * One thing to consider when using this: so phpseclib uses PSS mode by default. + * Technically, id-RSASSA-PSS has a different key format than rsaEncryption. So + * should phpseclib save to the id-RSASSA-PSS format by default or the + * rsaEncryption format? For stand-alone keys I figure rsaEncryption is better + * because SSH doesn't use PSS and idk how many SSH servers would be able to + * decode an id-RSASSA-PSS key. For X.509 certificates the id-RSASSA-PSS + * format is used by default (unless you change it up to use PKCS1 instead) + * * @category Crypt * @package RSA * @author Jim Wigginton @@ -660,7 +668,7 @@ abstract class RSA extends AsymmetricKey * * @access public */ - public function getHash() + public function getMGFHash() { return $this->mgfHash->getHash(); } diff --git a/phpseclib/Crypt/RSA/Formats/Keys/PSS.php b/phpseclib/Crypt/RSA/Formats/Keys/PSS.php index 69818f4d..4e2276a0 100644 --- a/phpseclib/Crypt/RSA/Formats/Keys/PSS.php +++ b/phpseclib/Crypt/RSA/Formats/Keys/PSS.php @@ -202,7 +202,7 @@ abstract class PSS extends Progenitor * @param array $options * @return string */ - private static function savePSSParams(array $options) + public static function savePSSParams(array $options) { /* The trailerField field is an integer. It provides diff --git a/phpseclib/File/X509.php b/phpseclib/File/X509.php index ae61cdf6..c52d0052 100644 --- a/phpseclib/File/X509.php +++ b/phpseclib/File/X509.php @@ -39,6 +39,7 @@ use phpseclib3\Exception\UnsupportedAlgorithmException; use phpseclib3\File\ASN1\Element; use phpseclib3\Math\BigInteger; use phpseclib3\File\ASN1\Maps; +use phpseclib3\Crypt\RSA\Formats\Keys\PSS; use DateTime; use DateTimeZone; @@ -2520,11 +2521,21 @@ class X509 $currentCert = isset($this->currentCert) ? $this->currentCert : null; $signatureSubject = isset($this->signatureSubject) ? $this->signatureSubject : null; $signatureAlgorithm = self::identifySignatureAlgorithm($issuer->privateKey); + if ($signatureAlgorithm != 'id-RSASSA-PSS') { + $signatureAlgorithm = ['algorithm' => $signatureAlgorithm]; + } else { + $r = PSS::load($issuer->privateKey->toString('PSS')); + $signatureAlgorithm = [ + 'algorithm' => 'id-RSASSA-PSS', + 'parameters' => PSS::savePSSParams($r) + ]; + } if (isset($subject->currentCert) && is_array($subject->currentCert) && isset($subject->currentCert['tbsCertificate'])) { $this->currentCert = $subject->currentCert; - $this->currentCert['tbsCertificate']['signature']['algorithm'] = $signatureAlgorithm; - $this->currentCert['signatureAlgorithm']['algorithm'] = $signatureAlgorithm; + $this->currentCert['tbsCertificate']['signature'] = $signatureAlgorithm; + $this->currentCert['signatureAlgorithm'] = $signatureAlgorithm; + if (!empty($this->startDate)) { $this->currentCert['tbsCertificate']['validity']['notBefore'] = $this->timeField($this->startDate); @@ -2574,7 +2585,7 @@ class X509 [ 'version' => 'v3', 'serialNumber' => $serialNumber, // $this->setSerialNumber() - 'signature' => ['algorithm' => $signatureAlgorithm], + 'signature' => $signatureAlgorithm, 'issuer' => false, // this is going to be overwritten later 'validity' => [ 'notBefore' => $this->timeField($startDate), // $this->setStartDate() @@ -2583,7 +2594,7 @@ class X509 'subject' => $subject->dn, 'subjectPublicKeyInfo' => $subjectPublicKey ], - 'signatureAlgorithm' => ['algorithm' => $signatureAlgorithm], + 'signatureAlgorithm' => $signatureAlgorithm, 'signature' => false // this is going to be overwritten later ]; @@ -3959,4 +3970,4 @@ class X509 return false; } -} +} \ No newline at end of file diff --git a/tests/Unit/Crypt/RSA/ModeTest.php b/tests/Unit/Crypt/RSA/ModeTest.php index df965dc9..29ea8b83 100644 --- a/tests/Unit/Crypt/RSA/ModeTest.php +++ b/tests/Unit/Crypt/RSA/ModeTest.php @@ -155,4 +155,32 @@ HERE; $payload = 'eyJraWQiOiJ0RkMyVUloRnBUTV9FYTNxY09kX01xUVQxY0JCbTlrRkxTRGZlSmhzUkc4IiwiYWxnIjoiUFMyNTYifQ.eyJhcHAiOiJhY2NvdW50cG9ydGFsIiwic3ViIjoiNTliOGM4YzA5NTVhNDA5MDg2MGRmYmM3ZGQwMjVjZWEiLCJjbGlkIjoiZTQ5ZTA2N2JiMTFjNDcyMmEzNGIyYjNiOGE2YTYzNTUiLCJhbSI6InBhc3N3b3JkIiwicCI6ImVOcDFrRUZQd3pBTWhmXC9QdEVOYU5kQkc2bUZDNHNpbENNNXU0aTNXMHFSS0hFVDU5V1JzcXpZRUp4XC84M3ZQbkIxcUg3Rm5CZVNabEtNME9saGVZVUVWTXlHOEVUOEZnWDI4dkdqWG4wWkcrV2hSK01rWVBicGZacHI2U3E0N0RFYjBLYkRFT21CSUZuOTZKN1ZDaWg1Q2p4dWNRZDJmdHJlMCt2cSthZFFObUluK0poWEl0UlBvQ0xya1wvZ05VV3N3T09vSVwva0Q5ZVk4c05jRHFPUzNkanFWb3RPU21oRUo5b0hZZmFqZmpSRzFGSWpGRFwvOExtT2pKbVF3d0tBMnQ0aXJBQ2NncHo0dzBuN3BtXC84YXV2T0dFM2twVFZ2d0IzdzlQZk1YZnJJUTBhejRsaEtIdVBUMU42XC9sb1FJPSIsImlhaSI6IjU5YjhjOGMwOTU1YTQwOTA4NjBkZmJjN2RkMDI1Y2VhIiwiY2xzdmMiOiJhY2NvdW50cG9ydGFsIiwibHB2IjoxNTQ3Njc1NDM4LCJ0IjoicyIsImljIjp0cnVlLCJleHAiOjE1NDc3MDQyMzgsImlhdCI6MTU0NzY3NTQzOCwianRpIjoiZTE0N2UzM2UzNzVhNDkyNWJjMzdjZTRjMDIwMmJjNDYifQ'; $this->assertTrue($rsa->verify($payload, $sig)); } + + public function testHash() + { + $pub = <<withHash('sha1') + ->withSaltLength(5) + ->withMGFHash('sha512'); + + $this->assertSame('sha1', $rsa->getHash()); + $this->assertSame(5, $rsa->getSaltLength()); + $this->assertSame('sha512', $rsa->getMGFHash()); + + $rsa = $rsa + ->withHash('sha512') + ->withSaltLength(6) + ->withMGFHash('sha1'); + + $this->assertSame('sha512', $rsa->getHash()); + $this->assertSame(6, $rsa->getSaltLength()); + $this->assertSame('sha1', $rsa->getMGFHash()); + } } diff --git a/tests/Unit/File/X509/X509Test.php b/tests/Unit/File/X509/X509Test.php index 5caa7e8f..f46c128b 100644 --- a/tests/Unit/File/X509/X509Test.php +++ b/tests/Unit/File/X509/X509Test.php @@ -939,8 +939,52 @@ U9VQQSQzY1oZMVX8i1m5WUTLPz2yLJIBQVdXqhMCQBGoiuSoSjafUhV7i1cEGpb88h5NBYZzWXGZ $r = $x509->loadX509($result); $this->assertSame('id-RSASSA-PSS', $r['tbsCertificate']['signature']['algorithm']); + $this->assertArrayHasKey('parameters', $r['tbsCertificate']['signature']); $this->assertSame('id-RSASSA-PSS', $r['tbsCertificate']['subjectPublicKeyInfo']['algorithm']['algorithm']); + $this->assertArrayHasKey('parameters', $r['tbsCertificate']['subjectPublicKeyInfo']['algorithm']); $this->assertSame('id-RSASSA-PSS', $r['signatureAlgorithm']['algorithm']); + $this->assertArrayHasKey('parameters', $r['signatureAlgorithm']); + } + + public function testPKCS1Save() + { + $private = '-----BEGIN RSA PRIVATE KEY----- +MIICXAIBAAKBgQCqGKukO1De7zhZj6+H0qtjTkVxwTCpvKe4eCZ0FPqri0cb2JZfXJ/DgYSF6vUp +wmJG8wVQZKjeGcjDOL5UlsuusFncCzWBQ7RKNUSesmQRMSGkVb1/3j+skZ6UtW+5u09lHNsj6tQ5 +1s1SPrCBkedbNf0Tp0GbMJDyR4e9T04ZZwIDAQABAoGAFijko56+qGyN8M0RVyaRAXz++xTqHBLh +3tx4VgMtrQ+WEgCjhoTwo23KMBAuJGSYnRmoBZM3lMfTKevIkAidPExvYCdm5dYq3XToLkkLv5L2 +pIIVOFMDG+KESnAFV7l2c+cnzRMW0+b6f8mR1CJzZuxVLL6Q02fvLi55/mbSYxECQQDeAw6fiIQX +GukBI4eMZZt4nscy2o12KyYner3VpoeE+Np2q+Z3pvAMd/aNzQ/W9WaI+NRfcxUJrmfPwIGm63il +AkEAxCL5HQb2bQr4ByorcMWm/hEP2MZzROV73yF41hPsRC9m66KrheO9HPTJuo3/9s5p+sqGxOlF +L0NDt4SkosjgGwJAFklyR1uZ/wPJjj611cdBcztlPdqoxssQGnh85BzCj/u3WqBpE2vjvyyvyI5k +X6zk7S0ljKtt2jny2+00VsBerQJBAJGC1Mg5Oydo5NwD6BiROrPxGo2bpTbu/fhrT8ebHkTz2epl +U9VQQSQzY1oZMVX8i1m5WUTLPz2yLJIBQVdXqhMCQBGoiuSoSjafUhV7i1cEGpb88h5NBYZzWXGZ +37sJ5QsW+sJyoNde3xH8vdXhzU7eT82D6X/scw9RZz+/6rCJ4p0= +-----END RSA PRIVATE KEY-----'; + $private = PublicKeyLoader::load($private) + ->withPadding(RSA::SIGNATURE_PKCS1) + ->withHash('sha256'); + $public = $private->getPublicKey(); + + $subject = new X509(); + $subject->setDNProp('id-at-organizationName', 'phpseclib demo cert'); + $subject->setPublicKey($public); + + $issuer = new X509(); + $issuer->setPrivateKey($private); + $issuer->setDN($subject->getDN()); + + $x509 = new X509(); + + $result = $x509->sign($issuer, $subject); + $result = $x509->saveX509($result); + + $this->assertInternalType('string', $result); + + $r = $x509->loadX509($result); + $this->assertSame('sha256WithRSAEncryption', $r['tbsCertificate']['signature']['algorithm']); + $this->assertSame('rsaEncryption', $r['tbsCertificate']['subjectPublicKeyInfo']['algorithm']['algorithm']); + $this->assertSame('sha256WithRSAEncryption', $r['signatureAlgorithm']['algorithm']); } public function testLongTagOnBadCert()