Introduce Protocol Cache for Client Verifier token reuse check

simplesamlphp · Sep 18, 2024 · 4d1d474 · 4d1d474
1 parent 3777422
commit 4d1d474
Show file tree

Hide file tree

Showing 9 changed files with 113 additions and 21 deletions.
diff --git a/routing/services/services.yml b/routing/services/services.yml
@@ -90,6 +90,8 @@ services:
     factory: ['@SimpleSAML\Module\oidc\Factories\ClaimTranslatorExtractorFactory', 'build']
   SimpleSAML\Module\oidc\Utils\FederationCache:
     factory: ['@SimpleSAML\Module\oidc\Factories\CacheFactory', 'forFederation'] # Can return null
+  SimpleSAML\Module\oidc\Utils\ProtocolCache:
+    factory: ['@SimpleSAML\Module\oidc\Factories\CacheFactory', 'forProtocol'] # Can return null
 
   # Use (already available) Laminas\Diactoros package as PSR HTTP Factories.
   Laminas\Diactoros\ServerRequestFactory: ~

diff --git a/src/Factories/CacheFactory.php b/src/Factories/CacheFactory.php
@@ -9,6 +9,7 @@
 use SimpleSAML\Module\oidc\Services\LoggerService;
 use SimpleSAML\Module\oidc\Utils\ClassInstanceBuilder;
 use SimpleSAML\Module\oidc\Utils\FederationCache;
+use SimpleSAML\Module\oidc\Utils\ProtocolCache;
 use Symfony\Component\Cache\Adapter\AdapterInterface;
 use Symfony\Component\Cache\Psr16Cache;
 
@@ -24,31 +25,62 @@ public function __construct(
     /**
      * @throws \SimpleSAML\Module\oidc\OidcException
      */
-    public function forFederation(): ?FederationCache
-    {
-        $class = $this->moduleConfig->getFederationCacheAdapterClass();
-
-        if (is_null($class)) {
-            return null;
-        }
-
+    protected function buildAdapterInstance(
+        string $class,
+        array $args = [],
+    ): AdapterInterface {
         try {
-            $instance = $this->classInstanceBuilder->build(
-                $class,
-                $this->moduleConfig->getFederationCacheAdapterArguments(),
-            );
+            $instance = $this->classInstanceBuilder->build($class, $args);
         } catch (\Throwable $exception) {
-            $message = "Error building federation cache instance: " . $exception->getMessage();
+            $message = "Error building cache adapter instance: " . $exception->getMessage();
             $this->loggerService->error($message);
             throw new OidcException($message);
         }
 
         if (!is_a($instance, AdapterInterface::class)) {
-            $message = "Unexpected federation cache adapter class: $class. Expected type: " . AdapterInterface::class;
+            $message = "Unexpected cache adapter class: $class. Expected type: " . AdapterInterface::class;
             $this->loggerService->error($message);
             throw new OidcException($message);
         }
 
-        return new FederationCache(new Psr16Cache($instance));
+        return $instance;
+    }
+
+    /**
+     * @throws \SimpleSAML\Module\oidc\OidcException
+     */
+    public function forFederation(): ?FederationCache
+    {
+        $class = $this->moduleConfig->getFederationCacheAdapterClass();
+
+        if (is_null($class)) {
+            return null;
+        }
+
+        $adapter = $this->buildAdapterInstance(
+            $class,
+            $this->moduleConfig->getFederationCacheAdapterArguments(),
+        );
+
+        return new FederationCache(new Psr16Cache($adapter));
+    }
+
+    /**
+     * @throws \SimpleSAML\Module\oidc\OidcException
+     */
+    public function forProtocol(): ?ProtocolCache
+    {
+        $class = $this->moduleConfig->getProtocolCacheAdapterClass();
+
+        if (is_null($class)) {
+            return null;
+        }
+
+        $adapter = $this->buildAdapterInstance(
+            $class,
+            $this->moduleConfig->getProtocolCacheAdapterArguments(),
+        );
+
+        return new ProtocolCache(new Psr16Cache($adapter));
     }
 }
diff --git a/src/Factories/RequestRulesManagerFactory.php b/src/Factories/RequestRulesManagerFactory.php
@@ -36,6 +36,7 @@
 use SimpleSAML\Module\oidc\Utils\ClaimTranslatorExtractor;
 use SimpleSAML\Module\oidc\Utils\FederationCache;
 use SimpleSAML\Module\oidc\Utils\JwksResolver;
+use SimpleSAML\Module\oidc\Utils\ProtocolCache;
 use SimpleSAML\Module\oidc\Utils\RequestParamsResolver;
 use SimpleSAML\OpenID\Federation;
 
@@ -57,6 +58,7 @@ public function __construct(
         private readonly Helpers $helpers,
         private readonly JwksResolver $jwksResolver,
         private readonly ?FederationCache $federationCache = null,
+        private readonly ?ProtocolCache $protocolCache = null,
     ) {
     }
 
@@ -104,7 +106,13 @@ private function getDefaultRules(): array
             new UiLocalesRule($this->requestParamsResolver),
             new AcrValuesRule($this->requestParamsResolver),
             new ScopeOfflineAccessRule($this->requestParamsResolver),
-            new ClientAuthenticationRule($this->requestParamsResolver, $this->moduleConfig, $this->jwksResolver),
+            new ClientAuthenticationRule(
+                $this->requestParamsResolver,
+                $this->moduleConfig,
+                $this->jwksResolver,
+                $this->helpers,
+                $this->protocolCache,
+            ),
             new CodeVerifierRule($this->requestParamsResolver),
         ];
     }

diff --git a/src/ModuleConfig.php b/src/ModuleConfig.php
@@ -606,4 +606,14 @@ public function getTrustAnchorJwks(string $trustAnchorId): ?array
             sprintf('Unexpected JWKS format for Trust Anchor %s: %s', $trustAnchorId, var_export($jwks, true)),
         );
     }
+
+    public function getProtocolCacheAdapterClass(): ?string
+    {
+        return $this->config()->getOptionalString(self::OPTION_PROTOCOL_CACHE_ADAPTER, null);
+    }
+
+    public function getProtocolCacheAdapterArguments(): array
+    {
+        return $this->config()->getOptionalArray(self::OPTION_PROTOCOL_CACHE_ADAPTER_ARGUMENTS, []);
+    }
 }
diff --git a/src/Server/RequestRules/Rules/ClientAuthenticationRule.php b/src/Server/RequestRules/Rules/ClientAuthenticationRule.php
@@ -6,24 +6,30 @@
 
 use Psr\Http\Message\ServerRequestInterface;
 use SimpleSAML\Module\oidc\Codebooks\RoutesEnum;
+use SimpleSAML\Module\oidc\Helpers;
 use SimpleSAML\Module\oidc\ModuleConfig;
 use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException;
 use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultBagInterface;
 use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultInterface;
 use SimpleSAML\Module\oidc\Server\RequestRules\Result;
 use SimpleSAML\Module\oidc\Services\LoggerService;
 use SimpleSAML\Module\oidc\Utils\JwksResolver;
+use SimpleSAML\Module\oidc\Utils\ProtocolCache;
 use SimpleSAML\Module\oidc\Utils\RequestParamsResolver;
 use SimpleSAML\OpenID\Codebooks\ClientAssertionTypesEnum;
 use SimpleSAML\OpenID\Codebooks\HttpMethodsEnum;
 use SimpleSAML\OpenID\Codebooks\ParamsEnum;
 
 class ClientAuthenticationRule extends AbstractRule
 {
+    protected const KEY_CLIENT_ASSERTION_JTI = 'client_assertion_jti';
+
     public function __construct(
         RequestParamsResolver $requestParamsResolver,
         protected ModuleConfig $moduleConfig,
         protected JwksResolver $jwksResolver,
+        protected Helpers $helpers,
+        protected ?ProtocolCache $protocolCache,
     ) {
         parent::__construct($requestParamsResolver);
     }
@@ -87,8 +93,14 @@ public function checkRule(
 
         $clientAssertion = $this->requestParamsResolver->parseClientAssertionToken($clientAssertionParam);
 
-        /** @var \SimpleSAML\Module\oidc\Entities\Interfaces\ClientEntityInterface $client */
-        $client = $currentResultBag->getOrFail(ClientIdRule::class)->getValue();
+        // Check if the Client Assertion token has already been used. Only applicable if we have cache available.
+        if ($this->protocolCache) {
+            ($this->protocolCache->has(self::KEY_CLIENT_ASSERTION_JTI, $clientAssertion->getJwtId()) === false)
+            || throw OidcServerException::invalidRequest(
+                ParamsEnum::ClientAssertion->value,
+                'Client Assertion reused.',
+            );
+        }
 
         ($jwks = $this->jwksResolver->forClient($client)) || throw OidcServerException::accessDenied(
             'Can not validate Client Assertion, client JWKS not available.',
@@ -122,6 +134,14 @@ public function checkRule(
         (!empty(array_intersect($expectedAudience, $clientAssertion->getAudience()))) ||
         throw OidcServerException::accessDenied('Invalid Client Assertion Audience claim.');
 
+        // Everything seems ok. Save it in cache so we can check for reuse.
+        $this->protocolCache?->set(
+            $clientAssertion->getJwtId(),
+            $this->helpers->dateTime()->getSecondsToExpirationTime($clientAssertion->getExpirationTime()),
+            self::KEY_CLIENT_ASSERTION_JTI,
+            $clientAssertion->getJwtId(),
+        );
+
         return new Result($this->getKey(), ParamsEnum::ClientAssertion->value);
     }
 }
diff --git a/src/Server/RequestRules/Rules/ClientIdRule.php b/src/Server/RequestRules/Rules/ClientIdRule.php
@@ -109,7 +109,7 @@ public function checkRule(
         // Check for reuse of the Request Object. Request Object MUST only be used once (by OpenID Federation spec).
         if ($this->federationCache) {
             ($this->federationCache->has(self::KEY_REQUEST_OBJECT_JTI, $requestObject->getJwtId()) === false)
-            || throw OidcServerException::invalidRequest(ParamsEnum::Request->value, 'Request object reused.');
+            || throw OidcServerException::invalidRequest(ParamsEnum::Request->value, 'Request Object reused.');
         }
 
         $clientEntityId = $requestObject->getIssuer();

diff --git a/src/Services/Container.php b/src/Services/Container.php
@@ -96,6 +96,7 @@
 use SimpleSAML\Module\oidc\Utils\ClassInstanceBuilder;
 use SimpleSAML\Module\oidc\Utils\FederationCache;
 use SimpleSAML\Module\oidc\Utils\JwksResolver;
+use SimpleSAML\Module\oidc\Utils\ProtocolCache;
 use SimpleSAML\Module\oidc\Utils\RequestParamsResolver;
 use SimpleSAML\OpenID\Core;
 use SimpleSAML\OpenID\Federation;
@@ -202,6 +203,8 @@ public function __construct()
         $this->services[CacheFactory::class] = $cacheFactory;
         $federationCache = $cacheFactory->forFederation();
         $this->services[FederationCache::class] = $federationCache;
+        $protocolCache = $cacheFactory->forProtocol();
+        $this->services[ProtocolCache::class] = $protocolCache;
         $federationFactory = new FederationFactory($moduleConfig, $loggerService, $federationCache);
         $this->services[FederationFactory::class] = $federationFactory;
         $federation = $federationFactory->build();
@@ -279,7 +282,13 @@ public function __construct()
             new UiLocalesRule($requestParamsResolver),
             new AcrValuesRule($requestParamsResolver),
             new ScopeOfflineAccessRule($requestParamsResolver),
-            new ClientAuthenticationRule($requestParamsResolver, $moduleConfig, $jwksResolver),
+            new ClientAuthenticationRule(
+                $requestParamsResolver,
+                $moduleConfig,
+                $jwksResolver,
+                $helpers,
+                $protocolCache,
+            ),
             new CodeVerifierRule($requestParamsResolver),
         ];
         $requestRuleManager = new RequestRulesManager($requestRules, $loggerService);

diff --git a/src/Utils/ProtocolCache.php b/src/Utils/ProtocolCache.php
@@ -0,0 +1,11 @@
+<?php
+
+declare(strict_types=1);
+
+namespace SimpleSAML\Module\oidc\Utils;
+
+use SimpleSAML\OpenID\Decorators\CacheDecorator;
+
+class ProtocolCache extends CacheDecorator
+{
+}
diff --git a/...tRules/Rules/RequestParameterRuleTest.php → ...uestRules/Rules/RequestObjectRuleTest.php b/...tRules/Rules/RequestParameterRuleTest.php → ...uestRules/Rules/RequestObjectRuleTest.php
@@ -22,7 +22,7 @@
 use SimpleSAML\OpenID\Core\RequestObject;
 
 #[CoversClass(RequestObjectRule::class)]
-class RequestParameterRuleTest extends TestCase
+class RequestObjectRuleTest extends TestCase
 {
     protected Stub $clientStub;
     protected Stub $resultBagStub;