From 43590bc9614c1df5bd7b1639088f7d904842892b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaime=20Pe=CC=81rez=20Crespo?= Date: Thu, 12 Jul 2018 11:56:19 +0200 Subject: [PATCH] Change default signature algorithm to RSA-SHA256. This doesn't mean that RSA-SHA1 is no longer supported, but keys must now be created with RSA-SHA256 by default. If another algorithm needs to be used, the key will be cast appropriately. --- src/SAML2/Assertion.php | 2 +- src/SAML2/HTTPRedirect.php | 2 +- src/SAML2/Signature/AbstractChainedValidator.php | 2 +- src/SAML2/SignedElementHelper.php | 2 +- src/SAML2/Utils.php | 2 +- tests/SAML2/AssertionTest.php | 10 +++++----- tests/SAML2/CertificatesMock.php | 12 +++++++----- tests/SAML2/HTTPRedirectTest.php | 4 ++-- 8 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/SAML2/Assertion.php b/src/SAML2/Assertion.php index 9fadbb573..3cba24805 100644 --- a/src/SAML2/Assertion.php +++ b/src/SAML2/Assertion.php @@ -642,7 +642,7 @@ private function parseSignature(\DOMElement $xml) */ public function validate(XMLSecurityKey $key) { - assert($key->type === \RobRichards\XMLSecLibs\XMLSecurityKey::RSA_SHA1); + assert($key->type === \RobRichards\XMLSecLibs\XMLSecurityKey::RSA_SHA256); if ($this->signatureData === null) { return false; diff --git a/src/SAML2/HTTPRedirect.php b/src/SAML2/HTTPRedirect.php index 501a32058..63cfdfd5d 100644 --- a/src/SAML2/HTTPRedirect.php +++ b/src/SAML2/HTTPRedirect.php @@ -219,7 +219,7 @@ public static function validateSignature(array $data, XMLSecurityKey $key) $signature = base64_decode($signature); - if ($key->type !== XMLSecurityKey::RSA_SHA1) { + if ($key->type !== XMLSecurityKey::RSA_SHA256) { throw new \Exception('Invalid key type for validating signature on query string.'); } if ($key->type !== $sigAlg) { diff --git a/src/SAML2/Signature/AbstractChainedValidator.php b/src/SAML2/Signature/AbstractChainedValidator.php index 1be459e20..4fed97f28 100644 --- a/src/SAML2/Signature/AbstractChainedValidator.php +++ b/src/SAML2/Signature/AbstractChainedValidator.php @@ -32,7 +32,7 @@ protected function validateElementWithKeys(SignedElement $element, $pemCandidate { $lastException = null; foreach ($pemCandidates as $index => $candidateKey) { - $key = new XMLSecurityKey(XMLSecurityKey::RSA_SHA1, array('type' => 'public')); + $key = new XMLSecurityKey(XMLSecurityKey::RSA_SHA256, array('type' => 'public')); $key->loadKey($candidateKey->getCertificate()); try { diff --git a/src/SAML2/SignedElementHelper.php b/src/SAML2/SignedElementHelper.php index c502e038c..76c77cd7a 100644 --- a/src/SAML2/SignedElementHelper.php +++ b/src/SAML2/SignedElementHelper.php @@ -181,7 +181,7 @@ public function getValidatingCertificates() "-----END CERTIFICATE-----\n"; /* Extract the public key from the certificate for validation. */ - $key = new XMLSecurityKey(XMLSecurityKey::RSA_SHA1, array('type'=>'public')); + $key = new XMLSecurityKey(XMLSecurityKey::RSA_SHA256, array('type'=>'public')); $key->loadKey($pemCert); try { diff --git a/src/SAML2/Utils.php b/src/SAML2/Utils.php index 06d45dade..5a4a9b447 100644 --- a/src/SAML2/Utils.php +++ b/src/SAML2/Utils.php @@ -170,7 +170,7 @@ public static function validateSignature(array $info, XMLSecurityKey $key) } $algo = $sigMethod->getAttribute('Algorithm'); - if ($key->type === XMLSecurityKey::RSA_SHA1 && $algo !== $key->type) { + if ($key->type === XMLSecurityKey::RSA_SHA256 && $algo !== $key->type) { $key = self::castKey($key, $algo); } diff --git a/tests/SAML2/AssertionTest.php b/tests/SAML2/AssertionTest.php index 52ec5e071..102ce9610 100644 --- a/tests/SAML2/AssertionTest.php +++ b/tests/SAML2/AssertionTest.php @@ -943,7 +943,7 @@ public function testVerifySignedAssertion() $doc = new \DOMDocument(); $doc->load(__DIR__ . '/signedassertion.xml'); - $publicKey = CertificatesMock::getPublicKeySha1(); + $publicKey = CertificatesMock::getPublicKeySha256(); $assertion = new Assertion($doc->firstChild); $result = $assertion->validate($publicKey); @@ -972,7 +972,7 @@ public function testCommentsInSignedAssertion() $doc = new \DOMDocument(); $doc->load(__DIR__ . '/signedassertion_with_comments.xml'); - $publicKey = CertificatesMock::getPublicKeySha1(); + $publicKey = CertificatesMock::getPublicKeySha256(); $assertion = new Assertion($doc->firstChild); $result = $assertion->validate($publicKey); @@ -991,7 +991,7 @@ public function testVerifySignedAssertionChangedBody() $doc = new \DOMDocument(); $doc->load(__DIR__ . '/signedassertion_tampered.xml'); - $publicKey = CertificatesMock::getPublicKeySha1(); + $publicKey = CertificatesMock::getPublicKeySha256(); $this->setExpectedException('Exception', 'Reference validation failed'); $assertion = new Assertion($doc->firstChild); @@ -1006,7 +1006,7 @@ public function testVerifySignedAssertionWrongKey() $doc = new \DOMDocument(); $doc->load(__DIR__ . '/signedassertion.xml'); - $publicKey = CertificatesMock::getPublicKey2Sha1(); + $publicKey = CertificatesMock::getPublicKey2Sha256(); $assertion = new Assertion($doc->firstChild); $this->setExpectedException('Exception', 'Unable to validate Signature'); @@ -1063,7 +1063,7 @@ public function testVerifyUnsignedAssertion() // Was not signed $this->assertFalse($assertion->getWasSignedAtConstruction()); - $publicKey = CertificatesMock::getPublicKeySha1(); + $publicKey = CertificatesMock::getPublicKeySha256(); $result = $assertion->validate($publicKey); $this->assertFalse($result); } diff --git a/tests/SAML2/CertificatesMock.php b/tests/SAML2/CertificatesMock.php index 72d77df6a..5e7341b00 100644 --- a/tests/SAML2/CertificatesMock.php +++ b/tests/SAML2/CertificatesMock.php @@ -138,30 +138,32 @@ public static function getPublicKey3() /** * @return XMLSecurityKey */ - public static function getPublicKeySha1() + public static function getPublicKeySha256() { - $publicKey = new XMLSecurityKey(XMLSecurityKey::RSA_SHA1, array('type'=>'public')); + $publicKey = new XMLSecurityKey(XMLSecurityKey::RSA_SHA256, array('type'=>'public')); $publicKey->loadKey(self::PUBLIC_KEY_PEM); return $publicKey; } + /** * @return XMLSecurityKey */ - public static function getPublicKey2Sha1() + public static function getPublicKey2Sha256() { - $publicKey = new XMLSecurityKey(XMLSecurityKey::RSA_SHA1, array('type'=>'public')); + $publicKey = new XMLSecurityKey(XMLSecurityKey::RSA_SHA256, array('type'=>'public')); $publicKey->loadKey(self::PUBLIC_KEY_2_PEM); return $publicKey; } + /** * Load a X.509 certificate with a DSA public key as RSA key * @return XMLSecurityKey */ public static function getPublicKeyDSAasRSA() { - $publicKey = new XMLSecurityKey(XMLSecurityKey::RSA_SHA1, array('type'=>'public')); + $publicKey = new XMLSecurityKey(XMLSecurityKey::RSA_SHA256, array('type'=>'public')); $publicKey->loadKey(self::PUBLIC_KEY_DSA_PEM); return $publicKey; } diff --git a/tests/SAML2/HTTPRedirectTest.php b/tests/SAML2/HTTPRedirectTest.php index c1ecc0b1c..f7ae2b152 100644 --- a/tests/SAML2/HTTPRedirectTest.php +++ b/tests/SAML2/HTTPRedirectTest.php @@ -82,12 +82,12 @@ public function testSignedRequestValidation() $request = $hr->receive(); // validate with the correct certificate, should verify - $result = $request->validate(CertificatesMock::getPublicKey2Sha1()); + $result = $request->validate(CertificatesMock::getPublicKey2Sha256()); $this->assertTrue($result); // validate with another cert, should fail $this->setExpectedException('Exception', 'Unable to validate signature'); - $result = $request->validate(CertificatesMock::getPublicKeySha1()); + $result = $request->validate(CertificatesMock::getPublicKeySha256()); } /**