Add RelayState validation

Author: tvdijen

src/Assert/Assert.php

@@ -12,16 +12,20 @@
  * @package simplesamlphp/saml2
  *
  * @method static void validDateTime(mixed $value, string $message = '', string $exception = '')
- * @method static void validURI(mixed $value, string $message = '', string $exception = '')
  * @method static void validEntityID(mixed $value, string $message = '', string $exception = '')
+ * @method static void validURI(mixed $value, string $message = '', string $exception = '')
+ * @method static void validRelayState(mixed $value, string $message = '', string $exception = '')
  * @method static void nullOrValidDateTime(mixed $value, string $message = '', string $exception = '')
- * @method static void nullOrValidURI(mixed $value, string $message = '', string $exception = '')
  * @method static void nullOrValidEntityID(mixed $value, string $message = '', string $exception = '')
+ * @method static void nullOrValidRelayState(mixed $value, string $message = '', string $exception = '')
+ * @method static void nullOrValidURI(mixed $value, string $message = '', string $exception = '')
  * @method static void allValidDateTime(mixed $value, string $message = '', string $exception = '')
- * @method static void allValidURI(mixed $value, string $message = '', string $exception = '')
  * @method static void allValidEntityID(mixed $value, string $message = '', string $exception = '')
+ * @method static void allValidRelayState(mixed $value, string $message = '', string $exception = '')
+ * @method static void allValidURI(mixed $value, string $message = '', string $exception = '')
  */
 class Assert extends BaseAssert
 {
     use CustomAssertionTrait;
+    use RelayStateTrait;
 }

src/Assert/RelayStateTrait.php

@@ -0,0 +1,38 @@
+<?php
+
+declare(strict_types=1);
+
+namespace SimpleSAML\SAML2\Assert;
+
+use SimpleSAML\Assert\AssertionFailedException;
+use SimpleSAML\SAML2\Constants as C;
+use SimpleSAML\SAML2\Exception\ProtocolViolationException;
+
+/**
+ * @package simplesamlphp/saml2
+ */
+trait RelayStateTrait
+{
+    /**
+     * @param string $value
+     * @param string $message
+     */
+    protected static function validRelayState(string $value, string $message = ''): void
+    {
+        parent::notWhitespaceOnly($value, $message); // Not protocol-defined, but makes zero sense
+        try {
+            /**
+             * 3.4.3 RelayState
+             *
+             * The value MUST NOT exceed 80 bytes in length [..]
+             */
+            parent::maxLength(
+                $value,
+                C::MAX_RELAY_STATE_LENGTH,
+                $message ?: '%s is not a SAML2.0-compliant RelayState',
+            );
+        } catch (AssertionFailedException $e) {
+            throw new ProtocolViolationException($e->getMessage());
+        }
+    }
+}

src/Binding.php

@@ -4,8 +4,8 @@
 
 namespace SimpleSAML\SAML2;
 
-use Psr\Http\Message\ResponseInterface;
-use Psr\Http\Message\ServerRequestInterface;
+use Psr\Http\Message\{ResponseInterface, ServerRequestInterface};
+use SimpleSAML\SAML2\Assert\Assert;
 use SimpleSAML\SAML2\Binding\{HTTPArtifact, HTTPPost, HTTPRedirect, SOAP};
 use SimpleSAML\SAML2\Constants as C;
 use SimpleSAML\SAML2\Exception\Protocol\UnsupportedBindingException;
@@ -163,6 +163,7 @@ public function getDestination(): ?string
      */
     public function setRelayState(?string $relayState = null): void
     {
+        Assert::nullOrValidRelayState($relayState);
         $this->relayState = $relayState;
     }
 

src/Constants.php

@@ -462,6 +462,11 @@ class Constants extends \SimpleSAML\XMLSecurity\Constants
      */
     public const ENTITYID_MAX_LENGTH = 1024;
 
+    /**
+     * The maximum size in bytes for any RelayState as per specification
+     */
+    public const MAX_RELAY_STATE_LENGTH = 80;
+
     /**
      * The maximum size for any entityid as per SAML2INT-specification
      */

tests/SAML2/Assert/RelayStateTest.php

@@ -0,0 +1,54 @@
+<?php
+
+declare(strict_types=1);
+
+namespace SimpleSAML\Test\SAML2\Assert;
+
+use PHPUnit\Framework\Attributes\{CoversClass, DataProvider, Group};
+use PHPUnit\Framework\TestCase;
+use SimpleSAML\Assert\AssertionFailedException;
+use SimpleSAML\SAML2\Assert\Assert;
+use SimpleSAML\SAML2\Constants as C;
+use SimpleSAML\SAML2\Exception\ProtocolViolationException;
+
+use function str_pad;
+
+/**
+ * Class \SimpleSAML\SAML2\Assert\RelayStateTest
+ *
+ * @package simplesamlphp/saml2
+ */
+#[Group('assert')]
+#[CoversClass(Assert::class)]
+final class RelayStateTest extends TestCase
+{
+    /**
+     * @param boolean $shouldPass
+     * @param string $str
+     */
+    #[DataProvider('provideRelayState')]
+    public function testValidRelayState(bool $shouldPass, string $str): void
+    {
+        try {
+            Assert::validRelayState($str);
+            $this->assertTrue($shouldPass);
+        } catch (ProtocolViolationException | AssertionFailedException $e) {
+            $this->assertFalse($shouldPass);
+        }
+    }
+
+
+    /**
+     * @return array<string, array{0: bool, 1: string}>
+     */
+    public static function provideRelayState(): array
+    {
+        return [
+            'POST Binding' => [true, 'https://target.local'],
+            'Redirect Binding' => [true, 'fdcoi3Xgoj32M94ejVh11LtQTMZjNE'],
+            'spaces' => [true, 'this is silly'],
+            'empty' => [false, ''],
+            'too_long' => [false, str_pad('', C::MAX_RELAY_STATE_LENGTH + 1, 'a')],
+        ];
+    }
+}

tests/SAML2/Binding/HTTPRedirectTest.php

@@ -193,7 +193,7 @@ public function testNoSigAlgSpecified(): void
     {
         $q = [
             'SAMLRequest' => 'nVLBauMwEP0Vo7sjW7FpKpJA2rBsoNuGOruHXhZFHm8EsuRqxtv27yvbWWgvYelFgjfvzbx5zBJVazu56enkHuG5B6TktbUO5VhYsT446RUalE61gJK0rDY/7qSYZbILnrz2ln2QXFYoRAhkvGPJbrtiv7VoygJEoTJ9LOusXDSFuJ4vdH6cxwoIEGUjsrqoFUt+QcCoXLHYKMoRe9g5JOUoQlleprlI8/yQz6W4ksXiiSXbuI1xikbViahDyfkRSM2wD40DmjnL0bSdhcE6Hx7BTd3xqnqoIPw1GmbdqWPJNx80jCGtGIUeWLL5t8mtd9i3EM78n493/zWr9XVvx+58mj39IlUaR/QmKOPq4Dtkyf4c9E1EjPtzOePjREL5/XDYp/uH6sDWy6G3HDML66+5ayO7VlHx2dySf2y9nM7pPprabffeGv02ZNcquux5QEydNiNVUlAODTiKMVvrX24DKIJz8nw9jfx8tOt3',
-            'RelayState' => 'https://beta.surfnet.nl/simplesaml/module.php/core/authenticate.php?as=Braindrops',
+            'RelayState' => 'https://profile.surfconext.nl/',
             'Signature' => 'b+qe/XGgICOrEL1v9dwuoy0RJtJ/GNAr7gJGYSJzLG0riPKwo7v5CH8GPC2P9IRikaeaNeQrnhBAaf8FCWrO0cLFw4qR6msK9bxRBGk+hIaTUYCh54ETrVCyGlmBneMgC5/iCRvtEW3ESPXCCqt8Ncu98yZmv9LIVyHSl67Se+fbB9sDw3/fzwYIHRMqK2aS8jnsnqlgnBGGOXqIqN3+d/2dwtCfz14s/9odoYzSUv32qfNPiPez6PSNqwhwH7dWE3TlO/jZmz0DnOeQ2ft6qdZEi5ZN5KCV6VmNKpkrLMq6DDPnuwPm/8oCAoT88R2jG7uf9QZB+ArWJKMEhDLsCA==',
         ];
         $request = new ServerRequest('GET', 'http://tnyholm.se');