From f590142333410fad9e7eb893a273b490012c8100 Mon Sep 17 00:00:00 2001
From: Tim van Dijen <tvdijen@gmail.com>
Date: Fri, 10 May 2024 11:09:45 +0200
Subject: [PATCH] Remove unnecessary utility-class

---
 src/Backend/HMAC.php             |  4 +-
 src/Utils/Security.php           | 64 --------------------------------
 src/XML/SignableElementTrait.php |  5 ++-
 src/XML/SignedElementTrait.php   | 14 +++++--
 tests/Utils/SecurityTest.php     | 31 ----------------
 5 files changed, 16 insertions(+), 102 deletions(-)
 delete mode 100644 src/Utils/Security.php
 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
deleted file mode 100644
index 5860a341..00000000
--- a/src/Utils/Security.php
+++ /dev/null
@@ -1,64 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace SimpleSAML\XMLSecurity\Utils;
-
-use SimpleSAML\Assert\Assert;
-use SimpleSAML\XMLSecurity\Constants as C;
-use SimpleSAML\XMLSecurity\Exception\InvalidArgumentException;
-
-use function hash_equals;
-
-/**
- * A collection of security-related functions.
- *
- * @package simplesamlphp/xml-security
- */
-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.
-     *
-     * @param string $alg The identifier of the algorithm to use.
-     * @param string $data The data to digest.
-     * @param bool $encode Whether to bas64-encode the result or not. Defaults to true.
-     *
-     * @return string The (binary or base64-encoded) digest corresponding to the given data.
-     *
-     * @throws \SimpleSAML\XMLSecurity\Exception\InvalidArgumentException If $alg is not a valid
-     *   identifier of a supported digest algorithm.
-     */
-    public static function hash(string $alg, string $data, bool $encode = true): string
-    {
-        Assert::keyExists(
-            C::$DIGEST_ALGORITHMS,
-            $alg,
-            'Unsupported digest method "' . $alg . '"',
-            InvalidArgumentException::class,
-        );
-
-        $digest = hash(C::$DIGEST_ALGORITHMS[$alg], $data, true);
-        if ($encode) {
-            $digest = base64_encode($digest);
-        }
-        return $digest;
-    }
-}
diff --git a/src/XML/SignableElementTrait.php b/src/XML/SignableElementTrait.php
index eef4e6c8..9da5c1d6 100644
--- a/src/XML/SignableElementTrait.php
+++ b/src/XML/SignableElementTrait.php
@@ -11,7 +11,6 @@
 use SimpleSAML\XMLSecurity\Constants as C;
 use SimpleSAML\XMLSecurity\Exception\RuntimeException;
 use SimpleSAML\XMLSecurity\Exception\UnsupportedAlgorithmException;
-use SimpleSAML\XMLSecurity\Utils\Security;
 use SimpleSAML\XMLSecurity\Utils\XML;
 use SimpleSAML\XMLSecurity\XML\ds\CanonicalizationMethod;
 use SimpleSAML\XMLSecurity\XML\ds\DigestMethod;
@@ -25,6 +24,8 @@
 use SimpleSAML\XMLSecurity\XML\ds\Transform;
 use SimpleSAML\XMLSecurity\XML\ds\Transforms;
 
+use function base64_encode;
+use function hash;
 use function in_array;
 
 /**
@@ -129,7 +130,7 @@ private function getReference(
 
         return new Reference(
             new DigestMethod($digestAlg),
-            new DigestValue(Security::hash($digestAlg, $canonicalDocument)),
+            new DigestValue(base64_encode(hash(C::$DIGEST_ALGORITHMS[$digestAlg], $canonicalDocument, true))),
             $transforms,
             null,
             null,
diff --git a/src/XML/SignedElementTrait.php b/src/XML/SignedElementTrait.php
index ca59d29e..d3dc9baa 100644
--- a/src/XML/SignedElementTrait.php
+++ b/src/XML/SignedElementTrait.php
@@ -19,7 +19,6 @@
 use SimpleSAML\XMLSecurity\Exception\SignatureVerificationFailedException;
 use SimpleSAML\XMLSecurity\Key;
 use SimpleSAML\XMLSecurity\Key\KeyInterface;
-use SimpleSAML\XMLSecurity\Utils\Security;
 use SimpleSAML\XMLSecurity\Utils\XML;
 use SimpleSAML\XMLSecurity\Utils\XPath;
 use SimpleSAML\XMLSecurity\XML\ds\Reference;
@@ -30,6 +29,8 @@
 
 use function array_pop;
 use function base64_decode;
+use function hash;
+use function hash_equals;
 use function in_array;
 
 /**
@@ -149,9 +150,16 @@ private function validateReference(SignedInfo $signedInfo): SignedElementInterfa
         $xml->removeChild($sigNode[0]);
 
         $data = XML::processTransforms($reference->getTransforms(), $xml);
-        $digest = Security::hash($reference->getDigestMethod()->getAlgorithm(), $data, false);
+        $algo = $reference->getDigestMethod()->getAlgorithm();
+        Assert::keyExists(
+            C::$DIGEST_ALGORITHMS,
+            $algo,
+            'Unsupported digest method "' . $algo . '"',
+            InvalidArgumentException::class,
+        );
 
-        if (Security::compareStrings($digest, base64_decode($reference->getDigestValue()->getRawContent(), true)) !== true) {
+        $digest = hash(C::$DIGEST_ALGORITHMS[$algo], $data, 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 @@
-<?php
-
-declare(strict_types=1);
-
-namespace SimpleSAML\XMLSecurity\Test\Utils;
-
-use PHPUnit\Framework\TestCase;
-use SimpleSAML\XMLSecurity\Utils\Security;
-
-/**
- * A class to test SimpleSAML\XMLSecurity\Utils\Security.
- *
- * @package SimpleSAML\XMLSecurity\Utils
- */
-final class SecurityTest extends TestCase
-{
-    /**
-     * Test the constant-time comparison function.
-     */
-    public function testCompareStrings(): void
-    {
-        // test that two equal strings compare successfully
-        $this->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      '));
-    }
-}