From 08c3b644e5a3a5e8a2aa7b98ba1bc34324bba894 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Fri, 10 May 2024 11:09:45 +0200 Subject: [PATCH] Remove unnecessary utility-method --- src/Backend/HMAC.php | 4 ++-- src/Utils/Security.php | 17 ----------------- src/XML/SignedElementTrait.php | 2 +- tests/Utils/SecurityTest.php | 31 ------------------------------- 4 files changed, 3 insertions(+), 51 deletions(-) delete mode 100644 tests/Utils/SecurityTest.php diff --git a/src/Backend/HMAC.php b/src/Backend/HMAC.php index 2df3d3bc..bb4f40eb 100644 --- a/src/Backend/HMAC.php +++ b/src/Backend/HMAC.php @@ -8,8 +8,8 @@ use SimpleSAML\XMLSecurity\Constants as C; use SimpleSAML\XMLSecurity\Exception\InvalidArgumentException; use SimpleSAML\XMLSecurity\Key\KeyInterface; -use SimpleSAML\XMLSecurity\Utils\Security; +use function hash_equals; use function hash_hmac; /** @@ -77,6 +77,6 @@ public function sign(KeyInterface $key, string $plaintext): string */ public function verify(KeyInterface $key, string $plaintext, string $signature): bool { - return Security::compareStrings(hash_hmac($this->digest, $plaintext, $key->getMaterial(), true), $signature); + return hash_equals(hash_hmac($this->digest, $plaintext, $key->getMaterial(), true), $signature); } } diff --git a/src/Utils/Security.php b/src/Utils/Security.php index 5860a341..e31c4f85 100644 --- a/src/Utils/Security.php +++ b/src/Utils/Security.php @@ -17,23 +17,6 @@ */ class Security { - /** - * Compare two strings in constant time. - * - * This function allows us to compare two given strings without any timing side channels - * leaking information about them. - * - * @param string $known The reference string. - * @param string $user The user-provided string to test. - * - * @return bool True if both strings are equal, false otherwise. - */ - public static function compareStrings(string $known, string $user): bool - { - return hash_equals($known, $user); - } - - /** * Compute the hash for some data with a given algorithm. * diff --git a/src/XML/SignedElementTrait.php b/src/XML/SignedElementTrait.php index ca59d29e..48453252 100644 --- a/src/XML/SignedElementTrait.php +++ b/src/XML/SignedElementTrait.php @@ -151,7 +151,7 @@ private function validateReference(SignedInfo $signedInfo): SignedElementInterfa $data = XML::processTransforms($reference->getTransforms(), $xml); $digest = Security::hash($reference->getDigestMethod()->getAlgorithm(), $data, false); - if (Security::compareStrings($digest, base64_decode($reference->getDigestValue()->getRawContent(), true)) !== true) { + if (hash_equals($digest, base64_decode($reference->getDigestValue()->getRawContent(), true)) !== true) { throw new SignatureVerificationFailedException('Failed to verify signature.'); } diff --git a/tests/Utils/SecurityTest.php b/tests/Utils/SecurityTest.php deleted file mode 100644 index d32695e1..00000000 --- a/tests/Utils/SecurityTest.php +++ /dev/null @@ -1,31 +0,0 @@ -assertTrue(Security::compareStrings('random string', 'random string')); - - // test that two different, equal-length strings fail to compare - $this->assertFalse(Security::compareStrings('random string', 'string random')); - - // test that two different-length strings fail to compare - $this->assertFalse(Security::compareStrings('one string', 'one string ')); - } -}