diff --git a/src/Alg/Signature/AbstractSigner.php b/src/Alg/Signature/AbstractSigner.php index 45edb19a..b7f1af63 100644 --- a/src/Alg/Signature/AbstractSigner.php +++ b/src/Alg/Signature/AbstractSigner.php @@ -52,6 +52,7 @@ public function __construct( $backend = new (static::DEFAULT_BACKEND)(); $this->setBackend($backend); $this->backend->setDigestAlg($digest); + $this->backend->setSignaturePadding($algId); } diff --git a/src/Alg/Signature/HMAC.php b/src/Alg/Signature/HMAC.php index e00a13cd..35a6b0e3 100644 --- a/src/Alg/Signature/HMAC.php +++ b/src/Alg/Signature/HMAC.php @@ -27,7 +27,7 @@ final class HMAC extends AbstractSigner implements SignatureAlgorithmInterface public function __construct( #[\SensitiveParameter] SymmetricKey $key, - string $algId = C::SIG_HMAC_SHA256, + string $algId, ) { parent::__construct($key, $algId, C::$HMAC_DIGESTS[$algId]); } diff --git a/src/Alg/Signature/RSA.php b/src/Alg/Signature/RSA.php index d5592eb2..7c9cc0f8 100644 --- a/src/Alg/Signature/RSA.php +++ b/src/Alg/Signature/RSA.php @@ -23,7 +23,7 @@ final class RSA extends AbstractSigner implements SignatureAlgorithmInterface public function __construct( #[\SensitiveParameter] AsymmetricKey $key, - string $algId = C::SIG_RSA_SHA256, + string $algId, ) { parent::__construct($key, $algId, C::$RSA_DIGESTS[$algId]); } diff --git a/src/Alg/Signature/SignatureAlgorithmFactory.php b/src/Alg/Signature/SignatureAlgorithmFactory.php index 913ed736..f5f13466 100644 --- a/src/Alg/Signature/SignatureAlgorithmFactory.php +++ b/src/Alg/Signature/SignatureAlgorithmFactory.php @@ -23,12 +23,13 @@ final class SignatureAlgorithmFactory /** * An array of blacklisted algorithms. * - * Defaults to RSA-SHA1 & HMAC-SHA1 due to the weakness of SHA1. + * Defaults to RSA-SHA1, RSAPSS-SHA1 & HMAC-SHA1 due to the weakness of SHA1. * * @var string[] */ public const array DEFAULT_BLACKLIST = [ C::SIG_RSA_SHA1, + C::SIG_RSA_PSS_SHA1, C::SIG_HMAC_SHA1, ]; diff --git a/src/Backend/HMAC.php b/src/Backend/HMAC.php index 8f18b5f6..10fd22dc 100644 --- a/src/Backend/HMAC.php +++ b/src/Backend/HMAC.php @@ -31,6 +31,17 @@ public function __construct() } + /** + * Set the padding method to be used by this backend. + * + * @param string $algId The identifier of the signature algorithm. + */ + public function setSignaturePadding(string $algId): void + { + // No-op for HMAC + } + + /** * Set the digest algorithm to be used by this backend. * diff --git a/src/Backend/OpenSSL.php b/src/Backend/OpenSSL.php index ab3f7ed7..d9abbdd3 100644 --- a/src/Backend/OpenSSL.php +++ b/src/Backend/OpenSSL.php @@ -38,8 +38,11 @@ final class OpenSSL implements EncryptionBackend, SignatureBackend // digital signature options protected string $digest; + // signature padding method + protected int $sig_padding = OPENSSL_PKCS1_PADDING; + // asymmetric encryption options - protected int $padding = OPENSSL_PKCS1_OAEP_PADDING; + protected int $enc_padding = OPENSSL_PKCS1_OAEP_PADDING; // symmetric encryption options protected string $cipher; @@ -83,7 +86,7 @@ public function encrypt( } $ciphertext = ''; - if (!$fn($plaintext, $ciphertext, $key->getMaterial(), $this->padding)) { + if (!$fn($plaintext, $ciphertext, $key->getMaterial(), $this->enc_padding)) { throw new OpenSSLException('Cannot encrypt data'); } return $ciphertext; @@ -139,7 +142,7 @@ public function decrypt( } $plaintext = ''; - if (!$fn($ciphertext, $plaintext, $key->getMaterial(), $this->padding)) { + if (!$fn($ciphertext, $plaintext, $key->getMaterial(), $this->enc_padding)) { throw new OpenSSLException('Cannot decrypt data'); } return $plaintext; @@ -192,7 +195,7 @@ public function sign( KeyInterface $key, string $plaintext, ): string { - if (!openssl_sign($plaintext, $signature, $key->getMaterial(), $this->digest)) { + if (!openssl_sign($plaintext, $signature, $key->getMaterial(), $this->digest, $this->sig_padding)) { throw new OpenSSLException('Cannot sign data'); } return $signature; @@ -214,7 +217,7 @@ public function verify( string $plaintext, string $signature, ): bool { - return openssl_verify($plaintext, $signature, $key->getMaterial(), $this->digest) === 1; + return openssl_verify($plaintext, $signature, $key->getMaterial(), $this->digest, $this->sig_padding) === 1; } @@ -236,11 +239,11 @@ public function setCipher(string $cipher): void $this->cipher = $cipher; switch ($cipher) { case C::KEY_TRANSPORT_RSA_1_5: - $this->padding = OPENSSL_PKCS1_PADDING; + $this->enc_padding = OPENSSL_PKCS1_PADDING; break; case C::KEY_TRANSPORT_OAEP: case C::KEY_TRANSPORT_OAEP_MGF1P: - $this->padding = OPENSSL_PKCS1_OAEP_PADDING; + $this->enc_padding = OPENSSL_PKCS1_OAEP_PADDING; break; case C::BLOCK_ENC_AES128_GCM: case C::BLOCK_ENC_AES192_GCM: @@ -255,6 +258,27 @@ public function setCipher(string $cipher): void } + /** + * Set the padding method to be used by this backend. + * + * @param string $algId The identifier of the signature algorithm. + */ + public function setSignaturePadding(string $algId): void + { + $padding = match ($algId) { + C::SIG_RSA_PSS_SHA1 => OPENSSL_PKCS1_PSS_PADDING, + C::SIG_RSA_PSS_SHA224 => OPENSSL_PKCS1_PSS_PADDING, + C::SIG_RSA_PSS_SHA256 => OPENSSL_PKCS1_PSS_PADDING, + C::SIG_RSA_PSS_SHA384 => OPENSSL_PKCS1_PSS_PADDING, + C::SIG_RSA_PSS_SHA512 => OPENSSL_PKCS1_PSS_PADDING, + + default => OPENSSL_PKCS1_PADDING, + }; + + $this->sig_padding = $padding; + } + + /** * Set the digest algorithm to be used by this backend. * diff --git a/src/Backend/SignatureBackend.php b/src/Backend/SignatureBackend.php index d779f854..9e555ec5 100644 --- a/src/Backend/SignatureBackend.php +++ b/src/Backend/SignatureBackend.php @@ -23,6 +23,14 @@ interface SignatureBackend public function setDigestAlg(string $digest): void; + /** + * Set the padding method to be used by this backend. + * + * @param string $algId The identifier of the signature algorithm. + */ + public function setSignaturePadding(string $algId): void; + + /** * Sign a given plaintext with this cipher and a given key. * diff --git a/src/Constants.php b/src/Constants.php index ca49be22..3d68dae1 100644 --- a/src/Constants.php +++ b/src/Constants.php @@ -102,7 +102,7 @@ class Constants extends \SimpleSAML\XML\Constants public const string C14N11_INCLUSIVE_WITHOUT_COMMENTS = 'http://www.w3.org/2006/12/xml-c14n11#WithComments'; /** - * Signature algorithms + * RSA Signature algorithms */ public const string SIG_RSA_SHA1 = 'http://www.w3.org/2000/09/xmldsig#rsa-sha1'; @@ -116,6 +116,9 @@ class Constants extends \SimpleSAML\XML\Constants public const string SIG_RSA_RIPEMD160 = 'http://www.w3.org/2001/04/xmldsig-more#rsa-ripemd160'; + /** + * HMAC Signature algorithms + */ public const string SIG_HMAC_SHA1 = 'http://www.w3.org/2000/09/xmldsig#hmac-sha1'; public const string SIG_HMAC_SHA224 = 'http://www.w3.org/2001/04/xmldsig-more#hmac-sha224'; @@ -128,6 +131,19 @@ class Constants extends \SimpleSAML\XML\Constants public const string SIG_HMAC_RIPEMD160 = 'http://www.w3.org/2001/04/xmldsig-more#hmac-ripemd160'; + /** + * RSA-PSS Signature algorithms + */ + public const string SIG_RSA_PSS_SHA1 = 'http://www.w3.org/2007/05/xmldsig-more#sha1-rsa-MGF1'; + + public const string SIG_RSA_PSS_SHA224 = 'http://www.w3.org/2007/05/xmldsig-more#sha224-rsa-MGF1'; + + public const string SIG_RSA_PSS_SHA256 = 'http://www.w3.org/2007/05/xmldsig-more#sha256-rsa-MGF1'; + + public const string SIG_RSA_PSS_SHA384 = 'http://www.w3.org/2007/05/xmldsig-more#sha384-rsa-MGF1'; + + public const string SIG_RSA_PSS_SHA512 = 'http://www.w3.org/2007/05/xmldsig-more#sha512-rsa-MGF1'; + /** * Encoding algorithms */ @@ -232,6 +248,11 @@ class Constants extends \SimpleSAML\XML\Constants self::SIG_RSA_SHA384 => self::DIGEST_SHA384, self::SIG_RSA_SHA512 => self::DIGEST_SHA512, self::SIG_RSA_RIPEMD160 => self::DIGEST_RIPEMD160, + self::SIG_RSA_PSS_SHA1 => self::DIGEST_SHA1, + self::SIG_RSA_PSS_SHA224 => self::DIGEST_SHA224, + self::SIG_RSA_PSS_SHA256 => self::DIGEST_SHA256, + self::SIG_RSA_PSS_SHA384 => self::DIGEST_SHA384, + self::SIG_RSA_PSS_SHA512 => self::DIGEST_SHA512, ]; /** @var array */ diff --git a/src/XML/SignedElementTrait.php b/src/XML/SignedElementTrait.php index 5cafec5b..c70581c0 100644 --- a/src/XML/SignedElementTrait.php +++ b/src/XML/SignedElementTrait.php @@ -281,6 +281,7 @@ public function verify(?SignatureAlgorithmInterface $verifier = null): SignedEle $verifier->getAlgorithmId(), $algId, 'Algorithm provided in key does not match algorithm used in signature.', + SignatureVerificationFailedException::class, ); return $this->verifyInternal($verifier); @@ -303,7 +304,6 @@ public function verify(?SignatureAlgorithmInterface $verifier = null): SignedEle "-----BEGIN CERTIFICATE-----\n%s\n-----END CERTIFICATE-----", strval($data->getContent()), ); - $cert = new Key\X509Certificate(PEM::fromString($cert)); $verifier = $factory->getAlgorithm($algId->getValue(), $cert->getPublicKey()); diff --git a/tests/XML/SignableElementTest.php b/tests/XML/SignableElementTest.php index 7472c752..06690071 100644 --- a/tests/XML/SignableElementTest.php +++ b/tests/XML/SignableElementTest.php @@ -261,4 +261,57 @@ public function testSigningWithDifferentRoot(): void ); $customSignable->toXML($doc->documentElement); } + + + /** + * This test ensures we can sign using RSA-PSS algorithms and verify the signed element again. + */ + public function testSigningAndVerifyingRsaPssSha256(): void + { + $customSignable = CustomSignable::fromXML( + self::$xmlRepresentation->documentElement, + ); + + $factory = new SignatureAlgorithmFactory(); + + $signer = $factory->getAlgorithm( + C::SIG_RSA_PSS_SHA256, + self::$key, + ); + + $keyInfo = new KeyInfo([ + new X509Data([ + new X509Certificate( + Base64BinaryValue::fromString(self::$certificate), + ), + ]), + ]); + + $customSignable->sign( + $signer, + C::C14N_EXCLUSIVE_WITHOUT_COMMENTS, + $keyInfo, + ); + + $verified = CustomSignable::fromXML($customSignable->toXML())->verify(); + + $signature = $customSignable->getSignature(); + $this->assertEquals( + C::SIG_RSA_PSS_SHA256, + $signature + ->getSignedInfo() + ->getSignatureMethod() + ->getAlgorithm() + ->getValue(), + ); + + $this->assertInstanceOf(CustomSignable::class, $verified); + $this->assertFalse($verified->isSigned()); + + $this->assertEquals( + '' + . 'Some', + strval($verified), + ); + } } diff --git a/tests/XML/SignedElementTest.php b/tests/XML/SignedElementTest.php index 5bfd2ec8..d2fc35c0 100644 --- a/tests/XML/SignedElementTest.php +++ b/tests/XML/SignedElementTest.php @@ -7,6 +7,7 @@ use DOMElement; use PHPUnit\Framework\TestCase; use SimpleSAML\XML\DOMDocumentFactory; +use SimpleSAML\XMLSchema\Type\Base64BinaryValue; use SimpleSAML\XMLSecurity\Alg\Signature\SignatureAlgorithmFactory; use SimpleSAML\XMLSecurity\Constants as C; use SimpleSAML\XMLSecurity\CryptoEncoding\PEM; @@ -16,10 +17,16 @@ use SimpleSAML\XMLSecurity\Key\X509Certificate; use SimpleSAML\XMLSecurity\Test\XML\CustomSignable; use SimpleSAML\XMLSecurity\TestUtils\PEMCertificatesMock; +use SimpleSAML\XMLSecurity\XML\ds\KeyInfo; use SimpleSAML\XMLSecurity\XML\ds\Signature; +use SimpleSAML\XMLSecurity\XML\ds\X509Certificate as X509; +use SimpleSAML\XMLSecurity\XML\ds\X509Data; +use function base64_encode; +use function chunk_split; use function dirname; use function strval; +use function trim; /** * Class \SimpleSAML\XMLSecurity\Test\XML\SignedElementTest @@ -256,4 +263,123 @@ public function testSuccessfulVerifyingDocumentWithComments(): void ); $this->assertEquals($certificate->getPublicKey(), $verified->getVerifyingKey()); } + + + /** + * Verify an RSA-PSS signature + */ + public function testSuccessfulVerifyingRsaPssSignature(): void + { + // sign using RSAPSS_SHA256 + $customSignable = $this->sign(C::SIG_RSA_PSS_SHA256); + // No-op to _actually_ sign the element and set the signature-property. + $customSignable = CustomSignable::fromXML($customSignable->toXML()); + + $signature = $customSignable->getSignature(); + + $this->assertEquals( + C::SIG_RSA_PSS_SHA256, + $signature + ->getSignedInfo() + ->getSignatureMethod() + ->getAlgorithm() + ->getValue(), + ); + + $certificate = new X509Certificate($this->certificate); + + $factory = new SignatureAlgorithmFactory(); + $verifier = $factory->getAlgorithm( + C::SIG_RSA_PSS_SHA256, + $certificate->getPublicKey(), + ); + + $verified = $customSignable->verify($verifier); + + $this->assertInstanceOf(CustomSignable::class, $verified); + } + + + /** + * Negative: verify a RSA-PSS signature using PKCS#1 v1.5 + */ + public function testPssSignatureCannotBeVerifiedUsingPkcs1(): void + { + // sign using RSAPSS_SHA256 + $customSignable = $this->sign(C::SIG_RSA_PSS_SHA256); + + $certificate = new X509Certificate($this->certificate); + + $factory = new SignatureAlgorithmFactory(); + $verifier = $factory->getAlgorithm( + C::SIG_RSA_SHA256, + $certificate->getPublicKey(), + ); + + $this->expectException(SignatureVerificationFailedException::class); + + $verified = $customSignable->verify($verifier); + } + + + /** + * Negative: verify a PKCS#1 v1.5 signature using RSA-PSS + */ + public function testPkcs1SignatureCannotBeVerifiedUsingPss(): void + { + // sign using RSA_SHA256 + $customSignable = $this->sign(C::SIG_RSA_SHA256); + $certificate = new X509Certificate($this->certificate); + + $factory = new SignatureAlgorithmFactory(); + $verifier = $factory->getAlgorithm( + C::SIG_RSA_PSS_SHA256, + $certificate->getPublicKey(), + ); + + $this->expectException(SignatureVerificationFailedException::class); + $result = $customSignable->verify($verifier); + } + + + /** + * Helper method to create a signed CustomSignable + */ + private function sign(string $algorithm): CustomSignable + { + $xml = DOMDocumentFactory::fromString( + '' + . 'Some', + ); + + $customSignable = CustomSignable::fromXML($xml->documentElement); + + $private = PEMCertificatesMock::getPrivateKey( + PEMCertificatesMock::SELFSIGNED_PRIVATE_KEY, + ); + + $factory = new SignatureAlgorithmFactory(); + $signer = $factory->getAlgorithm( + $algorithm, + $private, + ); + + $keyInfo = new KeyInfo([ + new X509Data([ + new X509( + Base64BinaryValue::fromString( + trim(chunk_split(base64_encode($this->certificate->data()), 64, "\n")), + ), + ), + ]), + ]); + + $customSignable->sign( + $signer, + C::C14N_EXCLUSIVE_WITHOUT_COMMENTS, + $keyInfo, + ); + + return $customSignable; + } }