diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index b2db72ff..bacafd93 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -414,7 +414,7 @@ public function code(string $state = '', string $code = '', string $scope = '', // TODO: proper error handling $idTokenRaw = $data['id_token']; - $jwks = $this->discoveryService->obtainJWK($provider); + $jwks = $this->discoveryService->obtainJWK($provider, $idTokenRaw); JWT::$leeway = 60; $idTokenPayload = JWT::decode($idTokenRaw, $jwks); @@ -615,7 +615,7 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok } // decrypt the logout token - $jwks = $this->discoveryService->obtainJWK($provider); + $jwks = $this->discoveryService->obtainJWK($provider, $logout_token); JWT::$leeway = 60; $logoutTokenPayload = JWT::decode($logout_token, $jwks); diff --git a/lib/Service/DiscoveryService.php b/lib/Service/DiscoveryService.php index 1350c32c..49e1e62e 100644 --- a/lib/Service/DiscoveryService.php +++ b/lib/Service/DiscoveryService.php @@ -27,6 +27,7 @@ use OCA\UserOIDC\Db\Provider; use OCA\UserOIDC\Vendor\Firebase\JWT\JWK; +use OCA\UserOIDC\Vendor\Firebase\JWT\JWT; use OCP\Http\Client\IClientService; use OCP\ICache; use OCP\ICacheFactory; @@ -36,6 +37,20 @@ class DiscoveryService { public const INVALIDATE_DISCOVERY_CACHE_AFTER_SECONDS = 3600; public const INVALIDATE_JWKS_CACHE_AFTER_SECONDS = 3600; + /** + * + * See https://www.imsglobal.org/spec/security/v1p1#approved-jwt-signing-algorithms. + * @var string[] + */ + private const SUPPORTED_JWK_ALGS = [ + 'RS256' => 'RSA', + 'RS384' => 'RSA', + 'RS512' => 'RSA', + 'ES256' => 'EC', + 'ES384' => 'EC', + 'ES512' => 'EC' + ]; + /** @var LoggerInterface */ private $logger; @@ -71,23 +86,29 @@ public function obtainDiscovery(Provider $provider): array { return json_decode($cachedDiscovery, true, 512, JSON_THROW_ON_ERROR); } - public function obtainJWK(Provider $provider): array { + /** + * @param Provider $provider + * @param string $tokenToDecode This is used to potentially fix the missing alg in + * @return array + * @throws \JsonException + */ + public function obtainJWK(Provider $provider, string $tokenToDecode): array { $lastJwksRefresh = $this->providerService->getSetting($provider->getId(), ProviderService::SETTING_JWKS_CACHE_TIMESTAMP); if ($lastJwksRefresh !== '' && (int) $lastJwksRefresh > time() - self::INVALIDATE_JWKS_CACHE_AFTER_SECONDS) { $rawJwks = $this->providerService->getSetting($provider->getId(), ProviderService::SETTING_JWKS_CACHE); $rawJwks = json_decode($rawJwks, true); - $jwks = JWK::parseKeySet($rawJwks); } else { $discovery = $this->obtainDiscovery($provider); $client = $this->clientService->newClient(); $responseBody = $client->get($discovery['jwks_uri'])->getBody(); - $result = json_decode($responseBody, true); - $jwks = JWK::parseKeySet($result); + $rawJwks = json_decode($responseBody, true); // cache jwks $this->providerService->setSetting($provider->getId(), ProviderService::SETTING_JWKS_CACHE, $responseBody); $this->providerService->setSetting($provider->getId(), ProviderService::SETTING_JWKS_CACHE_TIMESTAMP, strval(time())); } + $fixedJwks = $this->fixJwksAlg($rawJwks, $tokenToDecode); + $jwks = JWK::parseKeySet($fixedJwks, 'RS256'); $this->logger->debug('Parsed the jwks'); return $jwks; } @@ -117,4 +138,41 @@ public function buildAuthorizationUrl(string $authorizationEndpoint, array $extr return htmlentities(filter_var($urlWithoutParams, FILTER_SANITIZE_URL), ENT_QUOTES) . (empty($queryParams) ? '' : '?' . http_build_query($queryParams)); } + + /** + * Inspired by https://github.com/snake/moodle/compare/880462a1685...MDL-77077-master + * + * @param array $jwks + * @param string $jwt + * @return array + * @throws \Exception + */ + private function fixJwksAlg(array $jwks, string $jwt): array { + $jwtParts = explode('.', $jwt); + $jwtHeader = json_decode(JWT::urlsafeB64Decode($jwtParts[0]), true); + if (!isset($jwtHeader['kid'])) { + throw new \Exception('Error: kid must be provided in JWT header.'); + } + + foreach ($jwks['keys'] as $index => $key) { + // Only fix the key being referred to in the JWT. + if ($jwtHeader['kid'] != $key['kid']) { + continue; + } + + // Only fix the key if the alg is missing. + if (!empty($key['alg'])) { + continue; + } + + // The header alg must match the key type (family) specified in the JWK's kty. + if (!isset(self::SUPPORTED_JWK_ALGS[$jwtHeader['alg']]) || self::SUPPORTED_JWK_ALGS[$jwtHeader['alg']] !== $key['kty']) { + throw new \Exception('Error: Alg specified in the JWT header is incompatible with the JWK key type'); + } + + $jwks['keys'][$index]['alg'] = $jwtHeader['alg']; + } + + return $jwks; + } } diff --git a/lib/User/Provisioning/SelfEncodedTokenProvisioning.php b/lib/User/Provisioning/SelfEncodedTokenProvisioning.php index 6aa7324b..d45c1fab 100644 --- a/lib/User/Provisioning/SelfEncodedTokenProvisioning.php +++ b/lib/User/Provisioning/SelfEncodedTokenProvisioning.php @@ -30,7 +30,8 @@ public function __construct(ProvisioningService $provisioningService, DiscoveryS public function provisionUser(Provider $provider, string $tokenUserId, string $bearerToken): ?IUser { JWT::$leeway = 60; try { - $payload = JWT::decode($bearerToken, $this->discoveryService->obtainJWK($provider)); + $jwks = $this->discoveryService->obtainJWK($provider, $bearerToken); + $payload = JWT::decode($bearerToken, $jwks); } catch (Throwable $e) { $this->logger->error('Impossible to decode OIDC token:' . $e->getMessage()); return null; diff --git a/lib/User/Validator/SelfEncodedValidator.php b/lib/User/Validator/SelfEncodedValidator.php index 5c02ace1..7c151934 100644 --- a/lib/User/Validator/SelfEncodedValidator.php +++ b/lib/User/Validator/SelfEncodedValidator.php @@ -57,7 +57,8 @@ public function isValidBearerToken(Provider $provider, string $bearerToken): ?st // try to decode the bearer token JWT::$leeway = 60; try { - $payload = JWT::decode($bearerToken, $this->discoveryService->obtainJWK($provider)); + $jwks = $this->discoveryService->obtainJWK($provider, $bearerToken); + $payload = JWT::decode($bearerToken, $jwks); } catch (Throwable $e) { $this->logger->error('Impossible to decode OIDC token:' . $e->getMessage()); return null;