Skip to content

Commit 51c9537

Browse files
committed
fix(ocm): derive JWKS signing keyId per request, not from a frozen host
The JWKS signing keyId was persisted as a full URL, frozen at whichever host provisioned the key first. An instance addressed by more than one host (the two-port integration rig, or a port change behind a proxy) then signed with a keyId whose host no longer matched its federated share identity, so receivers rejected the request with an origin mismatch. Persist only a stable opaque key id (ecdsa-p256-sha256-<n>) and rebuild the full keyId per request from the current URL, in both the signing signatory and the published JWK Set. The signature keyid and the JWKS kid still match, since both are built from the same opaque id and the current request host. Rotation still works: slots differ by the counter, and the host is just fresh context. Drops the resolveKidBase/canonicalKid hostname machinery and the ocm_jwks_kid_base appconfig, which are no longer needed. Assisted-by: ClaudeCode:glm-5.2 Signed-off-by: Micke Nordin <kano@sunet.se>
1 parent fa1dbcb commit 51c9537

2 files changed

Lines changed: 25 additions & 67 deletions

File tree

lib/private/OCM/OCMSignatoryManager.php

Lines changed: 23 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,6 @@ class OCMSignatoryManager implements IJwkResolvingSignatoryManager {
5757
private const APPKEY_JWKS_POOL_PREFIX = 'ocm_jwks_pool_';
5858
private const APPCONFIG_JWKS_POOL_COUNTER = 'ocm_jwks_pool_counter';
5959
private const APPCONFIG_JWKS_POOL_KID_PREFIX = 'ocm_jwks_pool_kid_';
60-
/** Stable kid identity portion, reused across rotations so kids stay on one hostname. */
61-
private const APPCONFIG_JWKS_KID_BASE = 'ocm_jwks_kid_base';
6260
public const SLOT_ACTIVE = 'active';
6361
public const SLOT_PENDING = 'pending';
6462
public const SLOT_RETIRING = 'retiring';
@@ -262,82 +260,32 @@ public function listJwksKeys(): array {
262260
}
263261
$entries[] = [
264262
'poolId' => $id,
265-
'kid' => $this->canonicalKid(
266-
$this->appConfig->getValueString('core', self::APPCONFIG_JWKS_POOL_KID_PREFIX . $id, ''),
267-
),
263+
'kid' => $this->appConfig->getValueString('core', self::APPCONFIG_JWKS_POOL_KID_PREFIX . $id, ''),
268264
'slot' => $bySlot[$id] ?? null,
269265
];
270266
}
271267
return $entries;
272268
}
273269

274270
/**
275-
* Generate keypair into a new pool. Kid is canonicalised through
276-
* {@see Signatory::setKeyId} so admin output and wire form agree.
271+
* Generate keypair into a new pool. Only the opaque key id is persisted;
272+
* the host is derived fresh per request at use time, so a key provisioned
273+
* under one URL still verifies when the instance is addressed by another
274+
* (e.g. the two-port integration rig, or a port change behind a proxy).
277275
*/
278-
private function generatePool(string $kid): int {
276+
private function generatePool(string $opaqueKeyId): int {
279277
$poolId = $this->appConfig->getValueInt('core', self::APPCONFIG_JWKS_POOL_COUNTER, 0) + 1;
280278
$this->appConfig->setValueInt('core', self::APPCONFIG_JWKS_POOL_COUNTER, $poolId);
281279

282280
$this->identityProofManager->generateEcdsaP256AppKey('core', self::APPKEY_JWKS_POOL_PREFIX . $poolId);
283-
$this->appConfig->setValueString('core', self::APPCONFIG_JWKS_POOL_KID_PREFIX . $poolId, $this->canonicalKid($kid));
281+
$this->appConfig->setValueString('core', self::APPCONFIG_JWKS_POOL_KID_PREFIX . $poolId, $opaqueKeyId);
284282
return $poolId;
285283
}
286284

287-
/** Canonical wire-form via a transient {@see Signatory::setKeyId} round-trip. */
288-
private function canonicalKid(string $kid): string {
289-
$probe = new Signatory(true);
290-
$probe->setKeyId($kid);
291-
return $probe->getKeyId();
292-
}
293-
294-
/**
295-
* Build the next kid. Identity portion is derived once and persisted so
296-
* CLI-triggered rotations stay on the same hostname.
297-
*
298-
* @throws \RuntimeException if no instance identity can be derived
299-
*/
285+
/** Next opaque key id (`<fragment>-<n>`); the host is added per request. */
300286
private function nextPoolKid(): string {
301-
$base = $this->resolveKidBase();
302287
$next = $this->appConfig->getValueInt('core', self::APPCONFIG_JWKS_POOL_COUNTER, 0) + 1;
303-
return $base . '-' . $next;
304-
}
305-
306-
/**
307-
* Stable identity portion (before the `-N` suffix). Resolution order:
308-
* stored APPCONFIG_JWKS_KID_BASE > active pool's kid sans suffix >
309-
* fresh from {@see buildLocalKeyId}. Persisted so CLI rotations stay
310-
* on one hostname.
311-
*
312-
* @throws \RuntimeException if no instance identity can be derived
313-
*/
314-
private function resolveKidBase(): string {
315-
$base = $this->appConfig->getValueString('core', self::APPCONFIG_JWKS_KID_BASE, '');
316-
if ($base !== '') {
317-
return $base;
318-
}
319-
320-
$activePool = $this->getSlotPool(self::SLOT_ACTIVE);
321-
if ($activePool !== null) {
322-
$kid = $this->canonicalKid(
323-
$this->appConfig->getValueString('core', self::APPCONFIG_JWKS_POOL_KID_PREFIX . $activePool, ''),
324-
);
325-
$pos = strrpos($kid, '-');
326-
if ($pos !== false) {
327-
$base = substr($kid, 0, $pos);
328-
}
329-
}
330-
331-
if ($base === '') {
332-
try {
333-
$base = $this->canonicalKid($this->buildLocalKeyId(self::KEYID_FRAGMENT_JWKS));
334-
} catch (IdentityNotFoundException $e) {
335-
throw new \RuntimeException('cannot derive instance identity for JWKS kid', 0, $e);
336-
}
337-
}
338-
339-
$this->appConfig->setValueString('core', self::APPCONFIG_JWKS_KID_BASE, $base);
340-
return $base;
288+
return self::KEYID_FRAGMENT_JWKS . '-' . $next;
341289
}
342290

343291
private function getSlotPool(string $slot): ?int {
@@ -357,19 +305,29 @@ private function clearSlot(string $slot): void {
357305
$this->appConfig->deleteKey('core', 'ocm_jwks_slot_' . $slot);
358306
}
359307

360-
/** Returns null if the underlying appkey was manually deleted. */
308+
/**
309+
* Returns null if the underlying appkey was manually deleted. The keyId
310+
* is rebuilt per call from the opaque id and the current request URL, so
311+
* it tracks the host the instance is actually addressed as.
312+
*
313+
* @throws \RuntimeException if no instance identity can be derived
314+
*/
361315
private function signatoryFromPool(int $poolId): ?Signatory {
362316
$appKey = self::APPKEY_JWKS_POOL_PREFIX . $poolId;
363317
if (!$this->identityProofManager->hasAppKey('core', $appKey)) {
364318
return null;
365319
}
366-
$kid = $this->appConfig->getValueString('core', self::APPCONFIG_JWKS_POOL_KID_PREFIX . $poolId, '');
367-
if ($kid === '') {
320+
$opaqueKeyId = $this->appConfig->getValueString('core', self::APPCONFIG_JWKS_POOL_KID_PREFIX . $poolId, '');
321+
if ($opaqueKeyId === '') {
368322
return null;
369323
}
370324
$keyPair = $this->identityProofManager->getAppKey('core', $appKey);
371325
$signatory = new Signatory(true);
372-
$signatory->setKeyId($kid);
326+
try {
327+
$signatory->setKeyId($this->buildLocalKeyId($opaqueKeyId));
328+
} catch (IdentityNotFoundException $e) {
329+
throw new \RuntimeException('cannot derive instance identity for JWKS kid', 0, $e);
330+
}
373331
$signatory->setPublicKey($keyPair->getPublic());
374332
$signatory->setPrivateKey($keyPair->getPrivate());
375333
return $signatory;

tests/lib/OCM/OCMSignatoryManagerRotationTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ public function testFirstCallProvisionsActiveKey(): void {
8686
$this->assertSame($signatory->getKeyId(), $jwks[0]['kid']);
8787

8888
$listed = $this->signatoryManager->listJwksKeys();
89-
$this->assertSame([['poolId' => 1, 'kid' => $signatory->getKeyId(), 'slot' => 'active']], $listed);
89+
$this->assertSame([['poolId' => 1, 'kid' => 'ecdsa-p256-sha256-1', 'slot' => 'active']], $listed);
9090
}
9191

9292
public function testStageDoesNotChangeActiveSignerButPublishesNewJwk(): void {
@@ -155,7 +155,7 @@ public function testRetireRemovesRetiringKeyFromJwks(): void {
155155
// listJwksKeys also drops the retired pool.
156156
$listed = $this->signatoryManager->listJwksKeys();
157157
$this->assertCount(1, $listed);
158-
$this->assertSame($staged->getKeyId(), $listed[0]['kid']);
158+
$this->assertSame('ecdsa-p256-sha256-2', $listed[0]['kid']);
159159
$this->assertNotContains($first->getKeyId(), array_column($listed, 'kid'));
160160
}
161161

0 commit comments

Comments
 (0)