From d7abfaadbc4bc5234d313bd3514d0a17150171f8 Mon Sep 17 00:00:00 2001 From: terrafrost Date: Sun, 23 Jun 2019 21:51:38 -0500 Subject: [PATCH] X509: rm $signatureAlgorithm parameter from signature methods --- phpseclib/Crypt/Common/AsymmetricKey.php | 10 ++ phpseclib/Crypt/DSA.php | 10 ++ phpseclib/Crypt/ECDSA.php | 20 ++++ phpseclib/Crypt/RSA.php | 53 +++++++++- phpseclib/Crypt/RSA/Keys/PSS.php | 2 + phpseclib/Crypt/RSA/PublicKey.php | 3 +- phpseclib/File/X509.php | 128 +++++++++++------------ tests/Unit/File/X509/CSRTest.php | 6 +- tests/Unit/File/X509/SPKACTest.php | 4 +- tests/Unit/File/X509/X509Test.php | 29 +++-- 10 files changed, 181 insertions(+), 84 deletions(-) diff --git a/phpseclib/Crypt/Common/AsymmetricKey.php b/phpseclib/Crypt/Common/AsymmetricKey.php index df6187cb..d19d8c06 100644 --- a/phpseclib/Crypt/Common/AsymmetricKey.php +++ b/phpseclib/Crypt/Common/AsymmetricKey.php @@ -356,6 +356,16 @@ abstract class AsymmetricKey return $new; } + /** + * Returns the hash algorithm currently being used + * + * @access public + */ + public function getHash() + { + return $this->hash->getHash(); + } + /** * Compute the pseudorandom k for signature generation, * using the process specified for deterministic DSA. diff --git a/phpseclib/Crypt/DSA.php b/phpseclib/Crypt/DSA.php index 8cec196e..6139122c 100644 --- a/phpseclib/Crypt/DSA.php +++ b/phpseclib/Crypt/DSA.php @@ -338,4 +338,14 @@ abstract class DSA extends AsymmetricKey $new->format = self::validatePlugin('Signature', $format); return $new; } + + /** + * Returns the signature format currently being used + * + * @access public + */ + public function getSignatureFormat() + { + return $this->shortFormat; + } } \ No newline at end of file diff --git a/phpseclib/Crypt/ECDSA.php b/phpseclib/Crypt/ECDSA.php index c2ce3fb3..c6f8ec9d 100644 --- a/phpseclib/Crypt/ECDSA.php +++ b/phpseclib/Crypt/ECDSA.php @@ -352,6 +352,16 @@ abstract class ECDSA extends AsymmetricKey return $new; } + /** + * Returns the signature format currently being used + * + * @access public + */ + public function getSignatureFormat() + { + return $this->shortFormat; + } + /** * Sets the context * @@ -383,6 +393,16 @@ abstract class ECDSA extends AsymmetricKey return $new; } + /** + * Returns the signature format currently being used + * + * @access public + */ + public function getContext() + { + return $this->context; + } + /** * Determines which hashing function should be used * diff --git a/phpseclib/Crypt/RSA.php b/phpseclib/Crypt/RSA.php index 78d25406..441663ce 100644 --- a/phpseclib/Crypt/RSA.php +++ b/phpseclib/Crypt/RSA.php @@ -51,8 +51,8 @@ use phpseclib\Crypt\Common\AsymmetricKey; use phpseclib\Crypt\RSA\PrivateKey; use phpseclib\Crypt\RSA\PublicKey; use phpseclib\Math\BigInteger; -use phpseclib\Exceptions\UnsupportedAlgorithmException; -use phpseclib\Exceptions\InconsistentSetupException; +use phpseclib\Exception\UnsupportedAlgorithmException; +use phpseclib\Exception\InconsistentSetupException; use phpseclib\Crypt\RSA\Keys\PSS; /** @@ -415,7 +415,14 @@ abstract class RSA extends AsymmetricKey } if ($components['format'] == PSS::class) { - $key = $key->withPadding(self::SIGNATURE_PSS); + // in the X509 world RSA keys are assumed to use PKCS1 padding by default. only if the key is + // explicitly a PSS key is the use of PSS assumed. phpseclib does not work like this. phpseclib + // uses PSS padding by default. it assumes the more secure method by default and altho it provides + // for the less secure PKCS1 method you have to go out of your way to use it. this is consistent + // with the latest trends in crypto. libsodium (NaCl) is actually a little more extreme in that + // not only does it defaults to the most secure methods - it doesn't even let you choose less + // secure methods + //$key = $key->withPadding(self::SIGNATURE_PSS); if (isset($components['hash'])) { $key = $key->withHash($components['hash']); } @@ -648,6 +655,16 @@ abstract class RSA extends AsymmetricKey return $new; } + /** + * Returns the MGF hash algorithm currently being used + * + * @access public + */ + public function getHash() + { + return $this->mgfHash->getHash(); + } + /** * Determines the salt length * @@ -668,6 +685,16 @@ abstract class RSA extends AsymmetricKey return $new; } + /** + * Returns the salt length currently being used + * + * @access public + */ + public function getSaltLength() + { + return $this->sLen; + } + /** * Determines the label * @@ -690,6 +717,16 @@ abstract class RSA extends AsymmetricKey return $new; } + /** + * Returns the label currently being used + * + * @access public + */ + public function getLabel() + { + return $this->label; + } + /** * Determines the padding modes * @@ -743,6 +780,16 @@ abstract class RSA extends AsymmetricKey return $new; } + /** + * Returns the padding currently being used + * + * @access public + */ + public function getPadding() + { + return $this->signaturePadding | $this->encryptionPadding; + } + /** * Returns the current engine being used * diff --git a/phpseclib/Crypt/RSA/Keys/PSS.php b/phpseclib/Crypt/RSA/Keys/PSS.php index 8795a67d..dbc82100 100644 --- a/phpseclib/Crypt/RSA/Keys/PSS.php +++ b/phpseclib/Crypt/RSA/Keys/PSS.php @@ -187,6 +187,8 @@ abstract class PSS extends Progenitor */ public static function savePublicKey(BigInteger $n, BigInteger $e, array $options = []) { + self::initialize_static_variables(); + $key = PKCS1::savePublicKey($n, $e); $key = ASN1::extractBER($key); $params = self::savePSSParams($options); diff --git a/phpseclib/Crypt/RSA/PublicKey.php b/phpseclib/Crypt/RSA/PublicKey.php index 14f6c9f1..e019f521 100644 --- a/phpseclib/Crypt/RSA/PublicKey.php +++ b/phpseclib/Crypt/RSA/PublicKey.php @@ -18,7 +18,8 @@ use phpseclib\Math\BigInteger; use phpseclib\File\ASN1; use phpseclib\Common\Functions\Strings; use phpseclib\Crypt\Hash; -use phpseclib\Exceptions\NoKeyLoadedException; +use phpseclib\Exception\NoKeyLoadedException; +use phpseclib\Exception\UnsupportedFormatException; use phpseclib\Crypt\Random; use phpseclib\Crypt\Common; use phpseclib\File\ASN1\Maps\DigestInfo; diff --git a/phpseclib/File/X509.php b/phpseclib/File/X509.php index 29e2a64a..87e2df57 100644 --- a/phpseclib/File/X509.php +++ b/phpseclib/File/X509.php @@ -2485,7 +2485,7 @@ class X509 * @access public * @return mixed */ - public function sign($issuer, $subject, $signatureAlgorithm = 'sha256WithRSAEncryption') + public function sign($issuer, $subject) { if (!is_object($issuer->privateKey) || empty($issuer->dn)) { return false; @@ -2497,6 +2497,7 @@ class X509 $currentCert = isset($this->currentCert) ? $this->currentCert : null; $signatureSubject = isset($this->signatureSubject) ? $this->signatureSubject : null; + $signatureAlgorithm = self::identifySignatureAlgorithm($issuer->privateKey); if (isset($subject->currentCert) && is_array($subject->currentCert) && isset($subject->currentCert['tbsCertificate'])) { $this->currentCert = $subject->currentCert; @@ -2651,7 +2652,8 @@ class X509 $tbsCertificate = $this->currentCert['tbsCertificate']; $this->loadX509($this->saveX509($this->currentCert)); - $result = $this->signHelper($issuer->privateKey, $signatureAlgorithm); + $result = $this->currentCert; + $this->currentCert['signature'] = $result['signature'] = "\0" . $issuer->privateKey->sign($this->signatureSubject); $result['tbsCertificate'] = $tbsCertificate; $this->currentCert = $currentCert; @@ -2667,7 +2669,7 @@ class X509 * @param string $signatureAlgorithm * @return mixed */ - public function signCSR($signatureAlgorithm = 'sha1WithRSAEncryption') + public function signCSR() { if (!is_object($this->privateKey) || empty($this->dn)) { return false; @@ -2680,6 +2682,7 @@ class X509 $currentCert = isset($this->currentCert) ? $this->currentCert : null; $signatureSubject = isset($this->signatureSubject) ? $this->signatureSubject : null; + $signatureAlgorithm = self::identifySignatureAlgorithm($this->privateKey); if (isset($this->currentCert) && is_array($this->currentCert) && isset($this->currentCert['certificationRequestInfo'])) { $this->currentCert['signatureAlgorithm']['algorithm'] = $signatureAlgorithm; @@ -2705,7 +2708,8 @@ class X509 $certificationRequestInfo = $this->currentCert['certificationRequestInfo']; $this->loadCSR($this->saveCSR($this->currentCert)); - $result = $this->signHelper($this->privateKey, $signatureAlgorithm); + $result = $this->currentCert; + $this->currentCert['signature'] = $result['signature'] = "\0" . $this->privateKey->sign($this->signatureSubject); $result['certificationRequestInfo'] = $certificationRequestInfo; $this->currentCert = $currentCert; @@ -2721,7 +2725,7 @@ class X509 * @param string $signatureAlgorithm * @return mixed */ - public function signSPKAC($signatureAlgorithm = 'sha1WithRSAEncryption') + public function signSPKAC() { if (!is_object($this->privateKey)) { return false; @@ -2734,6 +2738,7 @@ class X509 $currentCert = isset($this->currentCert) ? $this->currentCert : null; $signatureSubject = isset($this->signatureSubject) ? $this->signatureSubject : null; + $signatureAlgorithm = self::identifySignatureAlgorithm($this->privateKey); // re-signing a SPKAC seems silly but since everything else supports re-signing why not? if (isset($this->currentCert) && is_array($this->currentCert) && isset($this->currentCert['publicKeyAndChallenge'])) { @@ -2765,7 +2770,8 @@ class X509 $publicKeyAndChallenge = $this->currentCert['publicKeyAndChallenge']; $this->loadSPKAC($this->saveSPKAC($this->currentCert)); - $result = $this->signHelper($this->privateKey, $signatureAlgorithm); + $result = $this->currentCert; + $this->currentCert['signature'] = $result['signature'] = "\0" . $this->privateKey->sign($this->signatureSubject); $result['publicKeyAndChallenge'] = $publicKeyAndChallenge; $this->currentCert = $currentCert; @@ -2785,7 +2791,7 @@ class X509 * @access public * @return mixed */ - public function signCRL($issuer, $crl, $signatureAlgorithm = 'sha1WithRSAEncryption') + public function signCRL($issuer, $crl) { if (!is_object($issuer->privateKey) || empty($issuer->dn)) { return false; @@ -2793,6 +2799,7 @@ class X509 $currentCert = isset($this->currentCert) ? $this->currentCert : null; $signatureSubject = isset($this->signatureSubject) ? $this->signatureSubject : null; + $signatureAlgorithm = self::identifySignatureAlgorithm($issuer->privateKey); $thisUpdate = new DateTime('now', new DateTimeZone(@date_default_timezone_get())); $thisUpdate = !empty($this->startDate) ? $this->startDate : $thisUpdate->format('D, d M Y H:i:s O'); @@ -2898,7 +2905,8 @@ class X509 $tbsCertList = $this->currentCert['tbsCertList']; $this->loadCRL($this->saveCRL($this->currentCert)); - $result = $this->signHelper($issuer->privateKey, $signatureAlgorithm); + $result = $this->currentCert; + $this->currentCert['signature'] = $result['signature'] = "\0" . $issuer->privateKey->sign($this->signatureSubject); $result['tbsCertList'] = $tbsCertList; $this->currentCert = $currentCert; @@ -2908,82 +2916,60 @@ class X509 } /** - * X.509 certificate signing helper function. + * Identify signature algorithm from key settings * * @param object $key - * @param string $signatureAlgorithm - * @access public + * @access private * @throws \phpseclib\Exception\UnsupportedAlgorithmException if the algorithm is unsupported - * @return mixed + * @return string */ - private function signHelper(PrivateKey $key, $signatureAlgorithm) + private static function identifySignatureAlgorithm(PrivateKey $key) { if ($key instanceof RSA) { - switch ($signatureAlgorithm) { - case 'id-RSASSA-PSS': - $key = $key->withPadding(RSA::SIGNATURE_PSS); - break; - case 'md2WithRSAEncryption': - case 'md5WithRSAEncryption': - case 'sha1WithRSAEncryption': - case 'sha224WithRSAEncryption': - case 'sha256WithRSAEncryption': - case 'sha384WithRSAEncryption': - case 'sha512WithRSAEncryption': - $key = $key - ->withHash(preg_replace('#WithRSAEncryption$#', '', $signatureAlgorithm)) - ->withPadding(RSA::SIGNATURE_PKCS1); - break; - default: - throw new UnsupportedAlgorithmException('Signature algorithm unsupported'); + if ($key->getPadding() | RSA::SIGNATURE_PSS) { + return 'id-RSASSA-PSS'; } - $this->currentCert['signature'] = "\0" . $key->sign($this->signatureSubject); - return $this->currentCert; + switch ($key->getHash()) { + case 'md2': + case 'md5': + case 'sha1': + case 'sha224': + case 'sha256': + case 'sha384': + case 'sha512': + return $key->getHash() . 'WithRSAEncryption'; + } + throw new UnsupportedAlgorithmException('The only supported hash algorithms for RSA are: md2, md5, sha1, sha224, sha256, sha384, sha512'); } if ($key instanceof DSA) { - switch ($signatureAlgorithm) { - case 'id-dsa-with-sha1': - case 'id-dsa-with-sha224': - case 'id-dsa-with-sha256': - $key = $key - ->withHash(preg_replace('#^id-dsa-with-#', '', strtolower($signatureAlgorithm))); - $this->currentCert['signature'] = "\0" . $key->sign($this->signatureSubject); - return $this->currentCert; - default: - throw new UnsupportedAlgorithmException('Signature algorithm unsupported'); + switch ($key->getHash()) { + case 'sha1': + case 'sha224': + case 'sha256': + return 'id-dsa-with-' . $key->getHash(); } + throw new UnsupportedAlgorithmException('The only supported hash algorithms for DSA are: sha1, sha224, sha256'); } if ($key instanceof ECDSA) { - switch ($signatureAlgorithm) { - case 'id-Ed25519': - if ($key->getCurve() !== 'Ed25519') { - throw new UnsupportedAlgorithmException('Loaded ECDSA does not use the Ed25519 key and yet that is the signature algorithm that has been chosen'); - } - $this->currentCert['signature'] = "\0" . $key->sign($this->signatureSubject); - return $this->currentCert; - case 'id-Ed448': - if ($key->getCurve() !== 'Ed448') { - throw new UnsupportedAlgorithmException('Loaded ECDSA does not use the Ed448 key and yet that is the signature algorithm that has been chosen'); - } - $this->currentCert['signature'] = "\0" . $key->sign($this->signatureSubject); - return $this->currentCert; - case 'ecdsa-with-SHA1': - case 'ecdsa-with-SHA224': - case 'ecdsa-with-SHA256': - case 'ecdsa-with-SHA384': - case 'ecdsa-with-SHA512': - $key = $key - ->withHash(preg_replace('#^ecdsa-with-#', '', strtolower($signatureAlgorithm))); - $this->currentCert['signature'] = "\0" . $key->sign($this->signatureSubject); - return $this->currentCert; - default: - throw new UnsupportedAlgorithmException('Signature algorithm unsupported'); + switch ($key->getCurve()) { + case 'Ed25519': + case 'Ed448': + return 'id-' . $key->getCurve(); } + switch ($key->getHash()) { + case 'sha1': + case 'sha224': + case 'sha256': + case 'sha384': + case 'sha512': + return 'ecdsa-with-' . strtoupper($key->getHash()); + } + throw new UnsupportedAlgorithmException('The only supported hash algorithms for ECDSA are: sha1, sha224, sha256, sha384, sha512'); } - throw new UnsupportedAlgorithmException('Unsupported public key algorithm'); + throw new UnsupportedAlgorithmException('The only supported public key classes are: RSA, DSA, ECDSA'); } /** @@ -3666,12 +3652,16 @@ class X509 */ private function formatSubjectPublicKey() { - $publicKey = base64_decode(preg_replace('#-.+-|[\r\n]#', '', $this->publicKey)); + $format = $this->publicKey instanceof RSA && ($this->publicKey->getPadding() & RSA::SIGNATURE_PSS) ? + 'PSS' : + 'PKCS8'; + + $publicKey = base64_decode(preg_replace('#-.+-|[\r\n]#', '', $this->publicKey->toString($format))); $decoded = ASN1::decodeBER($publicKey); $mapped = ASN1::asn1map($decoded[0], Maps\SubjectPublicKeyInfo::MAP); - $mapped['subjectPublicKey'] = (string) $this->publicKey; + $mapped['subjectPublicKey'] = $this->publicKey->toString($format); return $mapped; } diff --git a/tests/Unit/File/X509/CSRTest.php b/tests/Unit/File/X509/CSRTest.php index a57ee17b..593a40a4 100644 --- a/tests/Unit/File/X509/CSRTest.php +++ b/tests/Unit/File/X509/CSRTest.php @@ -113,9 +113,11 @@ L0NDt4SkosjgGwJAFklyR1uZ/wPJjj611cdBcztlPdqoxssQGnh85BzCj/u3WqBpE2vjvyyvyI5k X6zk7S0ljKtt2jny2+00VsBerQJBAJGC1Mg5Oydo5NwD6BiROrPxGo2bpTbu/fhrT8ebHkTz2epl U9VQQSQzY1oZMVX8i1m5WUTLPz2yLJIBQVdXqhMCQBGoiuSoSjafUhV7i1cEGpb88h5NBYZzWXGZ 37sJ5QsW+sJyoNde3xH8vdXhzU7eT82D6X/scw9RZz+/6rCJ4p0= ------END RSA PRIVATE KEY-----'); +-----END RSA PRIVATE KEY-----') + ->withPadding(RSA::SIGNATURE_PKCS1) + ->withHash('sha1'); $x509->setPrivateKey($rsa); $x509->setDN(['cn' => 'website.com']); - $x509->saveCSR($x509->signCSR('sha256WithRSAEncryption'), X509::FORMAT_DER); + $x509->saveCSR($x509->signCSR(), X509::FORMAT_DER); } } diff --git a/tests/Unit/File/X509/SPKACTest.php b/tests/Unit/File/X509/SPKACTest.php index ebd0fe16..8e270cf9 100644 --- a/tests/Unit/File/X509/SPKACTest.php +++ b/tests/Unit/File/X509/SPKACTest.php @@ -46,7 +46,9 @@ class Unit_File_X509_SPKACTest extends PhpseclibTestCase public function testSaveSPKAC() { - $privatekey = RSA::createKey(); + $privatekey = RSA::createKey(512) + ->withPadding(RSA::SIGNATURE_PKCS1) + ->withHash('sha1'); $x509 = new X509(); $x509->setPrivateKey($privatekey); diff --git a/tests/Unit/File/X509/X509Test.php b/tests/Unit/File/X509/X509Test.php index 8a384400..cb18c499 100644 --- a/tests/Unit/File/X509/X509Test.php +++ b/tests/Unit/File/X509/X509Test.php @@ -167,6 +167,9 @@ TQDJcOBY0qgBTEFqbazr7PScJR/0X8m0eLYS/XqkPi3kYaHLpr3RcsVbmwg9hVtx aBtsWpliLSex/HHhtRW9AkBGcq67zKmEpJ9kXcYLEjJii3flFS+Ct/rNm+Hhm1l7 4vca9v/F2hGVJuHIMJ8mguwYlNYzh2NqoIDJTtgOkBmt -----END RSA PRIVATE KEY-----'); + $privKey = $privKey + ->withPadding(RSA::SIGNATURE_PKCS1) + ->withHash('sha1'); $pubKey = $privKey->getPublicKey(); @@ -356,7 +359,9 @@ Mj93S // fixed by #1104 public function testMultipleDomainNames() { - $privatekey = RSA::createKey(512); + $privatekey = RSA::createKey(512) + ->withPadding(RSA::SIGNATURE_PKCS1) + ->withHash('sha1'); $publickey = $privatekey->getPublicKey(); $subject = new X509(); @@ -552,7 +557,9 @@ keSg3sfr4VWT545guJlTe+6vvelxbPFIXCXnyVLoePBYZtEe8FQhIBxd3EQHsxuJ iSoMCxKCa8r5P1DrxKaJAkBBP87OdahRq0CBQjTFg0wmPs66PoTXA4hZvSxV77CO tMPj6Pas7Muejogm6JkmxXC/uT6Tzfknd0B3XSmtDzGL -----END RSA PRIVATE KEY-----'; - $cakey = PublicKeyLoader::load($pemcakey); + $cakey = PublicKeyLoader::load($pemcakey) + ->withPadding(RSA::SIGNATURE_PKCS1) + ->withHash('sha1'); $pemca = '-----BEGIN CERTIFICATE----- MIICADCCAWmgAwIBAgIUJXQulcz5xkTam8UGC/yn6iVaiWwwDQYJKoZIhvcNAQEF BQAwHDEaMBgGA1UECgwRcGhwc2VjbGliIGRlbW8gQ0EwHhcNMTgwMTIxMTc0NzM0 @@ -855,13 +862,15 @@ uhPlgkgknwIgdDqqKIAF60ouiynsbU53ERS0TwpjeFiYGA48SwYW3Nk= $x509 = new X509(); - $result = $x509->sign($issuer, $subject, 'id-dsa-with-sha256'); + $result = $x509->sign($issuer, $subject); $result = $x509->saveX509($result); $this->assertInternalType('string', $result); $r = $x509->loadX509($result); - $this->assertArrayHasKey('tbsCertificate', $r); + $this->assertSame('id-dsa-with-sha256', $r['tbsCertificate']['signature']['algorithm']); + $this->assertSame('id-dsa', $r['tbsCertificate']['subjectPublicKeyInfo']['algorithm']['algorithm']); + $this->assertSame('id-dsa-with-sha256', $r['signatureAlgorithm']['algorithm']); } public function testECDSASave() @@ -884,13 +893,15 @@ wkwhE/JaQAEHq2PHnEmvwyBiJcHSdLXkcLzYlg19Ho0BPqVKdulx8GAk $x509 = new X509(); - $result = $x509->sign($issuer, $subject, 'ecdsa-with-SHA256'); + $result = $x509->sign($issuer, $subject); $result = $x509->saveX509($result); $this->assertInternalType('string', $result); $r = $x509->loadX509($result); - $this->assertArrayHasKey('tbsCertificate', $r); + $this->assertSame('ecdsa-with-SHA256', $r['tbsCertificate']['signature']['algorithm']); + $this->assertSame('id-ecPublicKey', $r['tbsCertificate']['subjectPublicKeyInfo']['algorithm']['algorithm']); + $this->assertSame('ecdsa-with-SHA256', $r['signatureAlgorithm']['algorithm']); } public function testPSSSave() @@ -921,13 +932,15 @@ U9VQQSQzY1oZMVX8i1m5WUTLPz2yLJIBQVdXqhMCQBGoiuSoSjafUhV7i1cEGpb88h5NBYZzWXGZ $x509 = new X509(); - $result = $x509->sign($issuer, $subject, 'id-RSASSA-PSS'); + $result = $x509->sign($issuer, $subject); $result = $x509->saveX509($result); $this->assertInternalType('string', $result); $r = $x509->loadX509($result); - $this->assertArrayHasKey('tbsCertificate', $r); + $this->assertSame('id-RSASSA-PSS', $r['tbsCertificate']['signature']['algorithm']); + $this->assertSame('id-RSASSA-PSS', $r['tbsCertificate']['subjectPublicKeyInfo']['algorithm']['algorithm']); + $this->assertSame('id-RSASSA-PSS', $r['signatureAlgorithm']['algorithm']); } public function testLongTagOnBadCert()