Rationalize algorithm blacklist code

simplesamlphp · Jul 29, 2024 · eec06ec · eec06ec
1 parent ad6282d
commit eec06ec
Show file tree

Hide file tree

Showing 10 changed files with 33 additions and 60 deletions.
diff --git a/src/Alg/Encryption/EncryptionAlgorithmFactory.php b/src/Alg/Encryption/EncryptionAlgorithmFactory.php
@@ -60,16 +60,16 @@ final class EncryptionAlgorithmFactory
     /**
      * Build a factory that creates algorithms.
      *
-     * @param string[]|null $blacklist A list of algorithms forbidden for their use.
+     * @param string[] $blacklist A list of algorithms forbidden for their use.
      */
     public function __construct(
-        protected ?array $blacklist = self::DEFAULT_BLACKLIST,
+        protected array $blacklist = self::DEFAULT_BLACKLIST,
     ) {
         // initialize the cache for supported algorithms per known implementation
-        if (!self::$initialized && $blacklist !== null) {
+        if (!self::$initialized) {
             foreach (self::SUPPORTED_DEFAULTS as $algorithm) {
                 foreach ($algorithm::getSupportedAlgorithms() as $algId) {
-                    if (array_key_exists($algId, self::$cache)) {
+                    if (array_key_exists($algId, self::$cache) && !array_key_exists($algId, $this->blacklist)) {
                         /*
                          * If the key existed before initialization, that means someone registered a handler for this
                          * algorithm, so we should respect that and skip registering the default here.
@@ -101,8 +101,9 @@ public function getAlgorithm(
         #[\SensitiveParameter]
         KeyInterface $key,
     ): EncryptionAlgorithmInterface {
-        Assert::false(
-            ($this->blacklist !== null) && in_array($algId, $this->blacklist, true),
+        Assert::notInArray(
+            $algId,
+            $this->blacklist,
             sprintf('Blacklisted algorithm: \'%s\'.', $algId),
             BlacklistedAlgorithmException::class,
         );

diff --git a/src/Alg/KeyTransport/KeyTransportAlgorithmFactory.php b/src/Alg/KeyTransport/KeyTransportAlgorithmFactory.php
@@ -58,13 +58,13 @@ class KeyTransportAlgorithmFactory
     /**
      * Build a factory that creates algorithms.
      *
-     * @param string[]|null $blacklist A list of algorithms forbidden for their use.
+     * @param string[] $blacklist A list of algorithms forbidden for their use.
      */
     public function __construct(
-        protected ?array $blacklist = self::DEFAULT_BLACKLIST,
+        protected array $blacklist = self::DEFAULT_BLACKLIST,
     ) {
         // initialize the cache for supported algorithms per known implementation
-        if (!self::$initialized && $blacklist !== null) {
+        if (!self::$initialized) {
             foreach (self::SUPPORTED_DEFAULTS as $algorithm) {
                 foreach ($algorithm::getSupportedAlgorithms() as $algId) {
                     if (array_key_exists($algId, self::$cache)) {
@@ -99,8 +99,9 @@ public function getAlgorithm(
         #[\SensitiveParameter]
         KeyInterface $key,
     ): KeyTransportAlgorithmInterface {
-        Assert::false(
-            ($this->blacklist !== null) && in_array($algId, $this->blacklist, true),
+        Assert::notInArray(
+            $algId,
+            $this->blacklist,
             sprintf('Blacklisted algorithm: \'%s\'.', $algId),
             BlacklistedAlgorithmException::class,
         );

diff --git a/src/Alg/Signature/SignatureAlgorithmFactory.php b/src/Alg/Signature/SignatureAlgorithmFactory.php
@@ -65,10 +65,10 @@ final class SignatureAlgorithmFactory
      * @param string[]|null $blacklist A list of algorithms forbidden for their use.
      */
     public function __construct(
-        protected ?array $blacklist = self::DEFAULT_BLACKLIST,
+        protected array $blacklist = self::DEFAULT_BLACKLIST,
     ) {
         // initialize the cache for supported algorithms per known implementation
-        if (!self::$initialized && $blacklist !== null) {
+        if (!self::$initialized) {
             foreach (self::SUPPORTED_DEFAULTS as $algorithm) {
                 foreach ($algorithm::getSupportedAlgorithms() as $algId) {
                     if (array_key_exists($algId, self::$cache)) {
@@ -103,8 +103,9 @@ public function getAlgorithm(
         #[\SensitiveParameter]
         KeyInterface $key,
     ): SignatureAlgorithmInterface {
-        Assert::false(
-            ($this->blacklist !== null) && in_array($algId, $this->blacklist, true),
+        Assert::notInArray(
+            $algId,
+            $this->blacklist,
             sprintf('Blacklisted algorithm: \'%s\'.', $algId),
             BlacklistedAlgorithmException::class,
         );

diff --git a/src/XML/EncryptableElementTrait.php b/src/XML/EncryptableElementTrait.php
@@ -99,10 +99,9 @@ abstract public function getEncryptionBackend(): ?EncryptionBackend;
     /**
      * Get the list of algorithms that are blacklisted for any encryption operation.
      *
-     * @return string[]|null An array with all algorithm identifiers that are blacklisted, or null if we want to use the
-     * defaults.
+     * @return string[] An array with all algorithm identifiers that are blacklisted.
      */
-    abstract public function getBlacklistedAlgorithms(): ?array;
+    abstract public function getBlacklistedAlgorithms(): array;
 
 
     /**

diff --git a/src/XML/EncryptedElementTrait.php b/src/XML/EncryptedElementTrait.php
@@ -209,8 +209,7 @@ abstract public function getEncryptionBackend(): ?EncryptionBackend;
     /**
      * Get the list of algorithms that are blacklisted for any encryption operation.
      *
-     * @return string[]|null An array with all algorithm identifiers that are blacklisted, or null if we want to use the
-     * defaults.
+     * @return string[] An array with all algorithm identifiers that are blacklisted.
      */
-    abstract public function getBlacklistedAlgorithms(): ?array;
+    abstract public function getBlacklistedAlgorithms(): array;
 }
diff --git a/src/XML/SignableElementTrait.php b/src/XML/SignableElementTrait.php
@@ -189,8 +189,7 @@ protected function doSign(DOMElement $xml): DOMElement
     /**
      * Get the list of algorithms that are blacklisted for any signing operation.
      *
-     * @return string[]|null An array with all algorithm identifiers that are blacklisted, or null if we want to use the
-     * defaults.
+     * @return string[] An array with all algorithm identifiers that are blacklisted.
      */
-    abstract public function getBlacklistedAlgorithms(): ?array;
+    abstract public function getBlacklistedAlgorithms(): array;
 }
diff --git a/src/XML/SignedElementTrait.php b/src/XML/SignedElementTrait.php
@@ -310,8 +310,7 @@ abstract public function getId(): ?string;
     /**
      * Get the list of algorithms that are blacklisted for any signing operation.
      *
-     * @return string[]|null An array with all algorithm identifiers that are blacklisted, or null if we want to use the
-     * defaults.
+     * @return string[] An array with all algorithm identifiers that are blacklisted.
      */
-    abstract public function getBlacklistedAlgorithms(): ?array;
+    abstract public function getBlacklistedAlgorithms(): array;
 }
diff --git a/tests/Alg/Encryption/EncryptionAlgorithmFactoryTest.php b/tests/Alg/Encryption/EncryptionAlgorithmFactoryTest.php
@@ -35,7 +35,7 @@ public static function setUpBeforeClass(): void
      */
     public function testGetUnknownAlgorithm(): void
     {
-        $factory = new EncryptionAlgorithmFactory([]);
+        $factory = new EncryptionAlgorithmFactory();
         $this->expectException(UnsupportedAlgorithmException::class);
         $factory->getAlgorithm('Unsupported algorithm identifier', self::$skey);
     }
@@ -47,6 +47,7 @@ public function testGetUnknownAlgorithm(): void
     public function testDefaultBlacklistedAlgorithms(): void
     {
         $factory = new EncryptionAlgorithmFactory();
+
         $algorithm = $factory->getAlgorithm(C::BLOCK_ENC_AES128, self::$skey);
         $this->assertInstanceOf(AES::class, $algorithm);
         $this->assertEquals(C::BLOCK_ENC_AES128, $algorithm->getAlgorithmId());
@@ -83,6 +84,7 @@ public function testDefaultBlacklistedAlgorithms(): void
     public function testBlacklistedAlgorithm(): void
     {
         $factory = new EncryptionAlgorithmFactory([C::BLOCK_ENC_AES256_GCM]);
+
         $algorithm = $factory->getAlgorithm(C::BLOCK_ENC_3DES, self::$skey);
         $this->assertInstanceOf(TripleDES::class, $algorithm);
         $this->assertEquals(C::BLOCK_ENC_3DES, $algorithm->getAlgorithmId());

diff --git a/tests/XML/CustomSignable.php b/tests/XML/CustomSignable.php
@@ -143,28 +143,14 @@ public function setEncryptionBackend(?EncryptionBackend $backend): void
      * Implement a method like this if your encrypted object needs to instantiate a new decryptor, for example, to
      * decrypt a session key. This method is required by \SimpleSAML\XMLSecurity\XML\EncryptedElementTrait.
      *
-     * @return string[]|null An array with all algorithm identifiers that we want to blacklist, or null if we want to
-     * use the defaults.
+     * @return string[] An array with all algorithm identifiers that we want to blacklist.
      */
-    public function getBlacklistedAlgorithms(): ?array
+    public function getBlacklistedAlgorithms(): array
     {
         return $this->blacklistedAlgs;
     }
 
 
-    /**
-     * Implement a method like this if your encrypted object needs to instantiate a new decryptor, for example, to
-     * decrypt a session key. This method is required by \SimpleSAML\XMLSecurity\XML\EncryptedElementTrait.
-     *
-     * @param string[]|null $algIds An array with the identifiers of the algorithms we want to blacklist, or null if we
-     * want to use the defaults.
-     */
-    public function setBlacklistedAlgorithms(?array $algIds): void
-    {
-        $this->blacklistedAlgs = $algIds;
-    }
-
-
     /**
      * Convert XML into a CustomSignable
      *

diff --git a/tests/XML/EncryptedCustom.php b/tests/XML/EncryptedCustom.php
@@ -91,28 +91,14 @@ public function setEncryptionBackend(?EncryptionBackend $backend): void
      * Implement a method like this if your encrypted object needs to instantiate a new decryptor, for example, to
      * decrypt a session key. This method is required by \SimpleSAML\XMLSecurity\XML\EncryptedElementTrait.
      *
-     * @return string[]|null An array with all algorithm identifiers that we want to blacklist, or null if we want to
-     * use the defaults.
+     * @return string[] An array with all algorithm identifiers that we want to blacklist.
      */
-    public function getBlacklistedAlgorithms(): ?array
+    public function getBlacklistedAlgorithms(): array
     {
         return $this->blacklistedAlgs;
     }
 
 
-    /**
-     * Implement a method like this if your encrypted object needs to instantiate a new decryptor, for example, to
-     * decrypt a session key. This method is required by \SimpleSAML\XMLSecurity\XML\EncryptedElementTrait.
-     *
-     * @param string[]|null $algIds An array with the identifiers of the algorithms we want to blacklist, or null if we
-     * want to use the defaults.
-     */
-    public function setBlacklistedAlgorithms(?array $algIds): void
-    {
-        $this->blacklistedAlgs = $algIds;
-    }
-
-
     /**
      * Decrypt this encrypted element.
      *