Skip to content

Commit 9ef0e6d

Browse files
authored
Merge pull request #1333 from strobelpierre/fix/823/filter-unsupported-jwks-keys
fix(jwks): filter unsupported key types in fixJwksAlg
2 parents f81b4aa + f3601c1 commit 9ef0e6d

File tree

2 files changed

+114
-6
lines changed

2 files changed

+114
-6
lines changed

lib/Service/DiscoveryService.php

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -196,17 +196,19 @@ public function fixJwksAlg(array $jwks, string $jwt): array {
196196
throw new \RuntimeException('Invalid JWKS: missing "keys" array');
197197
}
198198

199+
// Filter out keys with incompatible key types to prevent
200+
// Firebase JWT from failing on unsupported curves (e.g. P-521, Ed448).
201+
// See https://github.com/firebase/php-jwt/issues/561
202+
$jwks['keys'] = array_values(array_filter($jwks['keys'], function ($key) use ($expectedKty) {
203+
return ($key['kty'] ?? null) === $expectedKty;
204+
}));
205+
$keys = $jwks['keys'];
206+
199207
$matchingIndex = null;
200208

201209
foreach ($keys as $index => $key) {
202-
$keyKty = $key['kty'] ?? null;
203210
$keyUse = $key['use'] ?? null;
204211

205-
// Skip keys with incompatible type
206-
if ($keyKty !== $expectedKty) {
207-
continue;
208-
}
209-
210212
// Skip keys not intended for signature
211213
if ($keyUse !== null && $keyUse !== 'sig') {
212214
continue;

tests/unit/Service/DiscoveryServiceTest.php

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,112 @@ public function setUp(): void {
4343
$this->discoveryService = new DiscoveryService($this->logger, $this->clientHelper, $this->providerService, $this->config, $this->cacheFactory);
4444
}
4545

46+
/**
47+
* Test that fixJwksAlg filters out keys with unsupported key types.
48+
* This prevents Firebase JWT from crashing on P-521 or OKP keys.
49+
* See https://github.com/firebase/php-jwt/issues/561
50+
*/
51+
public function testFixJwksAlgFiltersUnsupportedKeyTypes() {
52+
// Build a fake JWT with RS256 alg and a known kid
53+
$header = json_encode(['alg' => 'RS256', 'kid' => 'rsa-key-1', 'typ' => 'JWT']);
54+
$payload = json_encode(['sub' => '1234']);
55+
$fakeJwt = rtrim(strtr(base64_encode($header), '+/', '-_'), '=')
56+
. '.' . rtrim(strtr(base64_encode($payload), '+/', '-_'), '=')
57+
. '.fake-signature';
58+
59+
// JWKS with mixed key types: RSA (matching), EC P-521, and OKP
60+
$jwks = [
61+
'keys' => [
62+
[
63+
'kty' => 'EC',
64+
'crv' => 'P-521',
65+
'kid' => 'ec-p521-key',
66+
'use' => 'sig',
67+
'x' => 'AekpBQ8ST8a8VcfVOTNl353vSrDCLL-Jmn1TZOFz5EhU',
68+
'y' => 'ADSmRA43Z1DSNx_RvcLI87cdL07l6jQyyBXMoxVg_l2T',
69+
],
70+
[
71+
'kty' => 'RSA',
72+
'kid' => 'rsa-key-1',
73+
'use' => 'sig',
74+
'n' => str_repeat('A', 342), // Fake 2048-bit modulus (256 bytes base64)
75+
'e' => 'AQAB',
76+
],
77+
[
78+
'kty' => 'OKP',
79+
'crv' => 'Ed25519',
80+
'kid' => 'okp-key',
81+
'use' => 'sig',
82+
'x' => 'some-value',
83+
],
84+
],
85+
];
86+
87+
// Mock config to disable key strength validation (we use fake key material)
88+
$this->config->method('getSystemValue')
89+
->with('user_oidc', [])
90+
->willReturn(['validate_jwk_strength' => false]);
91+
92+
$result = $this->discoveryService->fixJwksAlg($jwks, $fakeJwt);
93+
94+
// Only the RSA key should remain
95+
Assert::assertCount(1, $result['keys']);
96+
Assert::assertEquals('RSA', $result['keys'][0]['kty']);
97+
Assert::assertEquals('rsa-key-1', $result['keys'][0]['kid']);
98+
Assert::assertEquals('RS256', $result['keys'][0]['alg']);
99+
}
100+
101+
/**
102+
* Test that fixJwksAlg works with EC keys when JWT uses ES256.
103+
*/
104+
public function testFixJwksAlgKeepsCompatibleEcKeys() {
105+
$header = json_encode(['alg' => 'ES256', 'kid' => 'ec-key-1', 'typ' => 'JWT']);
106+
$payload = json_encode(['sub' => '1234']);
107+
$fakeJwt = rtrim(strtr(base64_encode($header), '+/', '-_'), '=')
108+
. '.' . rtrim(strtr(base64_encode($payload), '+/', '-_'), '=')
109+
. '.fake-signature';
110+
111+
$jwks = [
112+
'keys' => [
113+
[
114+
'kty' => 'RSA',
115+
'kid' => 'rsa-key-1',
116+
'use' => 'sig',
117+
'n' => str_repeat('A', 342),
118+
'e' => 'AQAB',
119+
],
120+
[
121+
'kty' => 'EC',
122+
'crv' => 'P-256',
123+
'kid' => 'ec-key-1',
124+
'use' => 'sig',
125+
'x' => 'AekpBQ8ST8a8VcfVOTNl353vSrDCLL-Jmn1TZOFz5EhU',
126+
'y' => 'ADSmRA43Z1DSNx_RvcLI87cdL07l6jQyyBXMoxVg_l2T',
127+
],
128+
[
129+
'kty' => 'EC',
130+
'crv' => 'P-521',
131+
'kid' => 'ec-p521-key',
132+
'use' => 'sig',
133+
'x' => 'AekpBQ8ST8a8VcfVOTNl353vSrDCLL-Jmn1TZOFz5EhU',
134+
'y' => 'ADSmRA43Z1DSNx_RvcLI87cdL07l6jQyyBXMoxVg_l2T',
135+
],
136+
],
137+
];
138+
139+
$this->config->method('getSystemValue')
140+
->with('user_oidc', [])
141+
->willReturn(['validate_jwk_strength' => false]);
142+
143+
$result = $this->discoveryService->fixJwksAlg($jwks, $fakeJwt);
144+
145+
// Both EC keys should remain (same kty), RSA filtered out
146+
Assert::assertCount(2, $result['keys']);
147+
Assert::assertEquals('EC', $result['keys'][0]['kty']);
148+
Assert::assertEquals('ec-key-1', $result['keys'][0]['kid']);
149+
Assert::assertEquals('EC', $result['keys'][1]['kty']);
150+
}
151+
46152
public function testBuildAuthorizationUrl() {
47153
$xss1 = '\'"http-equiv=><svg/onload=alert(1)>';
48154
$cleanedXss1 = '&#039;&quot;http-equiv=&gt;&lt;svg/onload=alert(1)&gt;';

0 commit comments

Comments
 (0)