Skip to content

Commit a80eecc

Browse files
cicnavicicnavi
committed
WIP
1 parent fa92f99 commit a80eecc

13 files changed

Lines changed: 403 additions & 400 deletions

File tree

‎config/module_oidc.php.dist‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,10 @@ $config = [
127127
ModuleConfig::OPTION_PAR_REQUEST_URI_TTL => 'PT10M', // PAR request URI expiration TTL (default: 10 minutes)
128128
ModuleConfig::OPTION_REQUIRE_PUSHED_AUTHORIZATION_REQUESTS => false, // Require PAR globally (default: false)
129129
ModuleConfig::OPTION_REQUIRE_SIGNED_REQUEST_OBJECT => false, // Reject unsigned request objects globally (default: false)
130+
// Whether to support passing the Request Object by reference using the https request_uri parameter (JAR /
131+
// OpenID Federation by reference). Set to false to mitigate DoS / SSRF by disabling outbound fetches. Note
132+
// that this does not affect Pushed Authorization Request URIs (urn form), which are always supported.
133+
ModuleConfig::OPTION_REQUEST_URI_PARAMETER_SUPPORTED => true, // Support https request_uri (default: true)
130134
ModuleConfig::OPTION_REQUEST_URI_TIMEOUT => 5, // Timeout for fetching request_uri (default: 5 seconds)
131135
ModuleConfig::OPTION_REQUEST_URI_MAX_SIZE_BYTES => 102400, // Maximum allowed response size for request_uri in bytes (default: 100KB)
132136

‎src/Factories/RequestRulesManagerFactory.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,6 @@ private function getDefaultRules(): array
126126
$this->requestParamsResolver,
127127
$this->helpers,
128128
$this->pushedAuthorizationRequestRepository,
129-
$this->jwksResolver,
130129
$this->moduleConfig,
131130
),
132131
new ResponseModeRule(

‎src/ModuleConfig.php‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ class ModuleConfig
126126
final public const string OPTION_PAR_REQUEST_URI_TTL = 'parRequestUriDuration';
127127
final public const string OPTION_REQUIRE_PUSHED_AUTHORIZATION_REQUESTS = 'requirePushedAuthorizationRequests';
128128
final public const string OPTION_REQUIRE_SIGNED_REQUEST_OBJECT = 'requireSignedRequestObject';
129+
final public const string OPTION_REQUEST_URI_PARAMETER_SUPPORTED = 'requestUriParameterSupported';
129130
final public const string OPTION_REQUEST_URI_TIMEOUT = 'requestUriTimeout';
130131
final public const string OPTION_REQUEST_URI_MAX_SIZE_BYTES = 'requestUriMaxSizeBytes';
131132

@@ -346,6 +347,16 @@ public function getRequireSignedRequestObject(): bool
346347
return $this->config()->getOptionalBoolean(self::OPTION_REQUIRE_SIGNED_REQUEST_OBJECT, false);
347348
}
348349

350+
/**
351+
* Whether the OP supports passing the Request Object by reference using the https request_uri parameter
352+
* (JWT-Secured Authorization Request by reference / OpenID Federation Authentication Request by reference).
353+
* Note that this does not affect Pushed Authorization Request URIs (urn form), which are always supported.
354+
*/
355+
public function getRequestUriParameterSupported(): bool
356+
{
357+
return $this->config()->getOptionalBoolean(self::OPTION_REQUEST_URI_PARAMETER_SUPPORTED, true);
358+
}
359+
349360
public function getRequestUriTimeout(): int
350361
{
351362
return $this->config()->getOptionalInteger(self::OPTION_REQUEST_URI_TIMEOUT, 5);

‎src/Server/RequestRules/Rules/ClientRule.php‎

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
use SimpleSAML\OpenID\Codebooks\ParamsEnum;
3030
use SimpleSAML\OpenID\Exceptions\JwsException;
3131
use SimpleSAML\OpenID\Federation;
32+
use SimpleSAML\OpenID\Federation\RequestObject as FederationRequestObject;
3233
use Throwable;
3334

3435
/**
@@ -161,30 +162,26 @@ public function resolveFromFederation(
161162
): ?ClientEntityInterface {
162163
$this->loggerService->debug('ClientRule: Resolving client from federation.');
163164
// Federation is enabled.
164-
// Check if we have a request object available. If not, we don't have anything else to do.
165-
$requestParam = $this->requestParamsResolver->getFromRequestBasedOnAllowedMethods(
166-
ParamsEnum::Request->value,
167-
$request,
168-
$allowedMethods,
169-
);
165+
// Check if we have a Request Object available, either passed by value (request param) or by reference
166+
// (https request_uri param). The RequestParamsResolver does the heavy lifting (parsing / fetching).
167+
// If not available, we don't have anything else to do.
168+
$requestObjectBag = $this->requestParamsResolver->getRequestObjectBag($request, $allowedMethods);
170169

171-
if (is_null($requestParam)) {
172-
$this->loggerService->error('ClientRule: No request param available, nothing to do.');
170+
if ($requestObjectBag === null) {
171+
$this->loggerService->error('ClientRule: No request object available, nothing to do.');
173172
return null;
174173
}
175174

176-
$this->loggerService->debug('ClientRule: Request param available.', ['requestParam' => $requestParam]);
175+
// We must verify that the Request Object is the one compatible with OpenID Federation specification
176+
// (not only Core specification).
177+
$requestObject = $requestObjectBag->get(FederationRequestObject::class);
177178

178-
// We have a request object available. We must verify that it is the one compatible with OpenID Federation
179-
// specification (not only Core specification).
180-
try {
181-
$requestObject = $this->requestParamsResolver->parseFederationRequestObjectToken($requestParam);
182-
} catch (Throwable $exception) {
183-
$this->loggerService->error('ClientRule: Request object error: ' . $exception->getMessage());
179+
if (!$requestObject instanceof FederationRequestObject) {
180+
$this->loggerService->error('ClientRule: Request object is not a Federation Request Object.');
184181
return null;
185182
}
186183

187-
$this->loggerService->debug('ClientRule: Request object parsed successfully.');
184+
$this->loggerService->debug('ClientRule: Federation Request object available.');
188185

189186
// We have a Federation-compatible Request Object.
190187
// The Audience (aud) value MUST be or include the OP's Issuer Identifier URL.

‎src/Server/RequestRules/Rules/RequestObjectRule.php‎

Lines changed: 54 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -50,31 +50,25 @@ public function checkRule(
5050
): ?ResultInterface {
5151
$loggerService->debug('RequestObjectRule::checkRule');
5252

53-
$requestParam = $this->requestParamsResolver->getFromRequestBasedOnAllowedMethods(
54-
ParamsEnum::Request->value,
55-
$request,
56-
$allowedServerRequestMethods,
57-
);
58-
59-
if (is_null($requestParam)) {
53+
// A Request Object can be passed by value (request param) or by reference (https request_uri param).
54+
// Either way, the parsing/fetching is done by the RequestParamsResolver; here we only need to know
55+
// whether such a Request Object is present for this request.
56+
if (!$this->hasRequestObjectSource($request, $allowedServerRequestMethods)) {
6057
return null;
6158
}
6259

63-
// Request param exists. Check if the result bag already has a request
64-
// object resolved. This can happen if the request object was used as
65-
// a way to do automatic client registration in OpenID Federation.
66-
// @see ClientIdRule
60+
// Request object is present. Check if the result bag already has a request object resolved. This can
61+
// happen if the request object was used as a way to do automatic client registration in OpenID
62+
// Federation.
63+
// @see ClientRule::resolveFromFederation()
6764
if ($currentResultBag->has($this->getKey())) {
6865
$loggerService->debug('Request object has already been resolved, skipping rule ' . $this->getKey());
6966
return null;
7067
}
7168

72-
// There is no request object already resolved. We will do it now.
73-
// Parse it using all available Request Object flavors, so we can
74-
// differentiate between OpenID Connect Core Request Objects
75-
// (which can be unsigned) and JAR Request Objects (which must be
76-
// signed).
77-
$requestObjectBag = $this->requestParamsResolver->parseRequestObjectBag($requestParam);
69+
// Parse it using all available Request Object flavors, so we can differentiate between OpenID Connect
70+
// Core Request Objects (which can be unsigned) and JAR Request Objects (which must be signed).
71+
$requestObjectBag = $this->requestParamsResolver->getRequestObjectBag($request, $allowedServerRequestMethods);
7872

7973
/** @var \SimpleSAML\Module\oidc\Entities\Interfaces\ClientEntityInterface $client */
8074
$client = $currentResultBag->getOrFail(ClientRule::class)->getValue();
@@ -83,6 +77,19 @@ public function checkRule(
8377
/** @var ?string $stateValue */
8478
$stateValue = ($currentResultBag->get(StateRule::class))?->getValue();
8579

80+
// The Request Object source is present, but it could not be parsed (by value) or fetched/parsed (by
81+
// reference). Note that for the by-reference case, RequestUriRule would normally reject this earlier.
82+
if ($requestObjectBag === null) {
83+
throw OidcServerException::invalidRequest(
84+
'request',
85+
'Request object could not be parsed or fetched.',
86+
null,
87+
$redirectUri,
88+
$stateValue,
89+
$responseMode,
90+
);
91+
}
92+
8693
if (!$this->isOidcAuthorizationRequest($request, $allowedServerRequestMethods)) {
8794
// This is a plain OAuth 2.0 authorization request, so JAR
8895
// (RFC 9101) rules apply: the Request Object must be a signed JWT
@@ -153,6 +160,36 @@ public function checkRule(
153160
return new Result($this->getKey(), $requestObject->getPayload());
154161
}
155162

163+
/**
164+
* Check whether the request carries a Request Object, either by value (request param) or by reference
165+
* (https request_uri param). Note that a Pushed Authorization Request URI (urn form) is not a Request
166+
* Object source (it carries previously pushed params, handled by RequestUriRule).
167+
*
168+
* @param \SimpleSAML\OpenID\Codebooks\HttpMethodsEnum[] $allowedServerRequestMethods
169+
*/
170+
protected function hasRequestObjectSource(
171+
ServerRequestInterface $request,
172+
array $allowedServerRequestMethods,
173+
): bool {
174+
if (
175+
!is_null($this->requestParamsResolver->getFromRequestBasedOnAllowedMethods(
176+
ParamsEnum::Request->value,
177+
$request,
178+
$allowedServerRequestMethods,
179+
))
180+
) {
181+
return true;
182+
}
183+
184+
$requestUri = $this->requestParamsResolver->getFromRequestBasedOnAllowedMethods(
185+
ParamsEnum::RequestUri->value,
186+
$request,
187+
$allowedServerRequestMethods,
188+
);
189+
190+
return is_string($requestUri) && str_starts_with(strtolower($requestUri), 'https://');
191+
}
192+
156193
/**
157194
* @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException
158195
*/

‎src/Server/RequestRules/Rules/RequestUriRule.php‎

Lines changed: 22 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -26,34 +26,31 @@
2626
use SimpleSAML\Module\oidc\Server\ResponseModes\QueryResponseMode;
2727
use SimpleSAML\Module\oidc\Server\ResponseModes\ResponseModeInterface;
2828
use SimpleSAML\Module\oidc\Services\LoggerService;
29-
use SimpleSAML\Module\oidc\Utils\JwksResolver;
3029
use SimpleSAML\Module\oidc\Utils\RequestParamsResolver;
3130
use SimpleSAML\OpenID\Codebooks\HttpMethodsEnum;
3231
use SimpleSAML\OpenID\Codebooks\ParamsEnum;
33-
use SimpleSAML\OpenID\Core\RequestObject as ConnectRequestObject;
34-
use SimpleSAML\OpenID\Jar\RequestObject as JarRequestObject;
3532

3633
/**
37-
* Handle the request_uri authorization request parameter:
38-
* - Pushed Authorization Request URIs (RFC 9126, urn form): validate existence, expiration, one-time use
39-
* (consume on validation) and client binding,
40-
* - https Request URIs (RFC 9101 / OpenID Connect Core, Request Object by reference): validate that the
41-
* Request URI is registered for the client, and validate the fetched Request Object (signature, client
42-
* binding), differentiating between OpenID Connect and plain OAuth 2.0 (JAR) requests,
43-
* - enforce Pushed Authorization Request usage if required by server or client policy.
44-
*
45-
* Note that the actual resolution of params from the request_uri value is done in RequestParamsResolver, so
46-
* that resolved params are transparently available to all other rules.
34+
* Gatekeeper for the request_uri authorization request parameter. It does not parse, fetch or verify the
35+
* Request Object itself (that is the job of the RequestParamsResolver and the RequestObjectRule); it only
36+
* enforces request_uri usage policy:
37+
* - request and request_uri must not be used together (RFC 9101),
38+
* - client_id is required when using request_uri,
39+
* - Pushed Authorization Request URIs (RFC 9126, urn form): existence, expiration, one-time use (consume on
40+
* validation) and client binding,
41+
* - https Request URIs (Request Object by reference): the OP must support the request_uri parameter, and the
42+
* Request Object must be resolvable (registration / federation policy is enforced in RequestParamsResolver),
43+
* - Pushed Authorization Request usage if required by server or client policy.
4744
*
4845
* @see \SimpleSAML\Module\oidc\Utils\RequestParamsResolver
46+
* @see \SimpleSAML\Module\oidc\Server\RequestRules\Rules\RequestObjectRule
4947
*/
5048
class RequestUriRule extends AbstractRule
5149
{
5250
public function __construct(
5351
RequestParamsResolver $requestParamsResolver,
5452
Helpers $helpers,
5553
protected PushedAuthorizationRequestRepository $pushedAuthorizationRequestRepository,
56-
protected JwksResolver $jwksResolver,
5754
protected ModuleConfig $moduleConfig,
5855
) {
5956
parent::__construct($requestParamsResolver, $helpers);
@@ -76,9 +73,8 @@ public function checkRule(
7673
): ?ResultInterface {
7774
$loggerService->debug('RequestUriRule::checkRule');
7875

79-
// Note: we are intentionally working with raw request params here
80-
// (not the merged view which includes params resolved from the
81-
// request_uri itself).
76+
// Note: we are intentionally working with raw request params here (not the merged view which includes
77+
// params resolved from the request_uri itself).
8278
$requestUri = $this->requestParamsResolver->getFromRequestBasedOnAllowedMethods(
8379
ParamsEnum::RequestUri->value,
8480
$request,
@@ -140,9 +136,7 @@ public function checkRule(
140136
if (str_starts_with(strtolower($requestUri), 'https://')) {
141137
return $this->checkHttpsRequestUri(
142138
$requestUri,
143-
$client,
144139
$request,
145-
$currentResultBag,
146140
$isParRequired,
147141
$allowedServerRequestMethods,
148142
);
@@ -224,9 +218,7 @@ protected function checkPushedAuthorizationRequestUri(
224218
*/
225219
protected function checkHttpsRequestUri(
226220
string $requestUri,
227-
ClientEntityInterface $client,
228221
ServerRequestInterface $request,
229-
ResultBagInterface $currentResultBag,
230222
bool $isParRequired,
231223
array $allowedServerRequestMethods,
232224
): ResultInterface {
@@ -237,98 +229,26 @@ protected function checkHttpsRequestUri(
237229
);
238230
}
239231

240-
if (!in_array($requestUri, $client->getRequestUris(), true)) {
232+
if (!$this->moduleConfig->getRequestUriParameterSupported()) {
241233
throw OidcServerException::invalidRequest(
242234
ParamsEnum::RequestUri->value,
243-
'The request_uri is not registered for this client.',
235+
'Passing the request object by reference (request_uri) is not supported.',
244236
);
245237
}
246238

247-
// Make sure the request_uri resolution ran (it is memoized in
248-
// RequestParamsResolver, so this is inexpensive if other rules already
249-
// triggered it), then grab the resolved Request Object Bag.
250-
$this->requestParamsResolver->getAllBasedOnAllowedMethods($request, $allowedServerRequestMethods);
251-
252-
$requestObjectBag = $this->requestParamsResolver->getResolvedRequestUriBag($requestUri);
239+
// Make sure the Request Object behind the request_uri can actually be resolved (fetched and parsed,
240+
// and allowed by registration / federation policy in RequestParamsResolver). The signature and other
241+
// request object validations are then done by the RequestObjectRule (or by ClientRule for the
242+
// federation case).
243+
$requestObjectBag = $this->requestParamsResolver->getRequestObjectBag($request, $allowedServerRequestMethods);
253244
if ($requestObjectBag === null) {
254245
throw OidcServerException::invalidRequest(
255246
ParamsEnum::RequestUri->value,
256-
'Could not fetch or parse the Request Object from request_uri.',
257-
);
258-
}
259-
260-
if (!$this->isOidcAuthorizationRequest($request, $allowedServerRequestMethods)) {
261-
// This is a plain OAuth 2.0 authorization request, so JAR
262-
// (RFC 9101) rules apply: the Request Object must be a signed
263-
// JWT containing the Client ID claim.
264-
$requestObject = $requestObjectBag->get(JarRequestObject::class);
265-
if (!$requestObject instanceof JarRequestObject) {
266-
throw OidcServerException::invalidRequest(
267-
ParamsEnum::RequestUri->value,
268-
'Request object is not a valid JAR Request Object (note that it must be signed).',
269-
);
270-
}
271-
272-
$this->verifySignature($requestObject, $client);
273-
} else {
274-
// This is an OpenID Connect authorization request, so OpenID Connect Core rules apply: the
275-
// Request Object can be unsigned (unless signature is required by policy).
276-
$requestObject = $requestObjectBag->get(ConnectRequestObject::class);
277-
if (!$requestObject instanceof ConnectRequestObject) {
278-
throw OidcServerException::invalidRequest(
279-
ParamsEnum::RequestUri->value,
280-
'Request object is not a valid Request Object.',
281-
);
282-
}
283-
284-
if ($requestObject->isProtected()) {
285-
$this->verifySignature($requestObject, $client);
286-
} elseif (
287-
$this->moduleConfig->getRequireSignedRequestObject() ||
288-
$client->getRequireSignedRequestObject()
289-
) {
290-
throw OidcServerException::invalidRequest(
291-
ParamsEnum::RequestUri->value,
292-
'Request object must be signed (alg: none is not allowed).',
293-
);
294-
}
295-
}
296-
297-
$payload = $requestObject->getPayload();
298-
299-
/** @psalm-suppress MixedAssignment */
300-
$clientIdClaim = $payload[ParamsEnum::ClientId->value] ?? null;
301-
if ($clientIdClaim !== $client->getIdentifier()) {
302-
throw OidcServerException::invalidRequest(
303-
ParamsEnum::RequestUri->value,
304-
'Client ID claim in request object does not match the client_id parameter.',
247+
'The request_uri could not be resolved (it may not be allowed for this client, or the fetch ' .
248+
'failed).',
305249
);
306250
}
307251

308-
// Mark the Request Object as resolved (and validated), so that RequestObjectRule does not need to
309-
// run again for it.
310-
$currentResultBag->add(new Result(RequestObjectRule::class, $payload));
311-
312252
return new Result($this->getKey(), $requestUri);
313253
}
314-
315-
/**
316-
* @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException
317-
*/
318-
protected function verifySignature(
319-
ConnectRequestObject|JarRequestObject $requestObject,
320-
ClientEntityInterface $client,
321-
): void {
322-
($jwks = $this->jwksResolver->forClient($client)) || throw OidcServerException::accessDenied(
323-
'can not validate request object, client JWKS not available',
324-
);
325-
326-
try {
327-
$requestObject->verifyWithKeySet($jwks);
328-
} catch (\Throwable $exception) {
329-
throw OidcServerException::accessDenied(
330-
'request object validation failed: ' . $exception->getMessage(),
331-
);
332-
}
333-
}
334254
}

‎src/Services/Container.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,6 @@ public function __construct()
485485
$requestParamsResolver,
486486
$helpers,
487487
$pushedAuthorizationRequestRepository,
488-
$jwksResolver,
489488
$moduleConfig,
490489
),
491490
new ResponseModeRule(

0 commit comments

Comments
 (0)