From e3c60c4f765f0f14039da0115924ba3b27949c24 Mon Sep 17 00:00:00 2001 From: Micke Nordin Date: Sun, 26 Jul 2026 14:22:00 +0200 Subject: [PATCH 1/6] feat(ocm): align RFC 9421 http-sig with updated OCM spec Implements the changes from cs3org/OCM-API#391 and closes the remaining gaps with the HTTP Message Signatures section: - discover peer JWK Sets via the new `jwksUri` discovery field instead of the fixed /.well-known/jwks.json path; advertise our own jwksUri and require it (https) from peers advertising the `http-sig` capability - stop covering the Date header; freshness is anchored on the `created` signature parameter - mark and select the OCM signature by its integrity-protected tag="ocm" parameter instead of the dictionary label - require the JWK `alg` parameter and derive the signature algorithm from it, accepting fully-specified RFC 9864 names - reject signed requests whose verification key cannot be resolved instead of treating them as unsigned - derive the signer origin from the spec-canonical fqdn#id keyid form Signed-off-by: Micke Nordin --- lib/private/OCM/Model/OCMProvider.php | 28 +++ lib/private/OCM/OCMDiscoveryService.php | 9 +- lib/private/OCM/OCMJwksHandler.php | 6 +- lib/private/OCM/OCMSignatoryManager.php | 94 +++++++++- .../Model/Rfc9421IncomingSignedRequest.php | 87 ++++++--- .../Model/Rfc9421OutgoingSignedRequest.php | 13 +- .../Security/Signature/Rfc9421/Algorithm.php | 9 +- .../Security/Signature/SignatureManager.php | 4 +- lib/public/OCM/IOCMProvider.php | 24 ++- tests/lib/OCM/DiscoveryServiceTest.php | 13 +- tests/lib/OCM/OCMProviderTest.php | 26 +++ tests/lib/OCM/OCMSignatoryManagerJwksTest.php | 107 ++++++++++- .../Signature/Model/Rfc9421RoundTripTest.php | 172 ++++++++++++++++-- .../Signature/Rfc9421/AlgorithmTest.php | 6 +- .../SignatureManagerDispatchTest.php | 23 +++ 15 files changed, 559 insertions(+), 62 deletions(-) diff --git a/lib/private/OCM/Model/OCMProvider.php b/lib/private/OCM/Model/OCMProvider.php index 76530353613c4..16ec74890e80f 100644 --- a/lib/private/OCM/Model/OCMProvider.php +++ b/lib/private/OCM/Model/OCMProvider.php @@ -26,6 +26,7 @@ class OCMProvider implements IOCMProvider { private array $capabilities = []; private string $endPoint = ''; private string $tokenEndPoint = ''; + private string $jwksUri = ''; /** @var IOCMResource[] */ private array $resourceTypes = []; private ?Signatory $signatory = null; @@ -144,6 +145,26 @@ public function getTokenEndPoint(): string { return ''; } + /** + * @param string $jwksUri + * + * @return $this + */ + #[\Override] + public function setJwksUri(string $jwksUri): static { + $this->jwksUri = $jwksUri; + + return $this; + } + + /** + * @return string + */ + #[\Override] + public function getJwksUri(): string { + return $this->jwksUri; + } + /** * @return string */ @@ -311,6 +332,9 @@ public function import(array $data): static { if (isset($data['tokenEndPoint'])) { $this->setTokenEndPoint($data['tokenEndPoint']); } + if (is_string($data['jwksUri'] ?? null)) { + $this->setJwksUri($data['jwksUri']); + } if (!$this->looksValid()) { throw new OCMProviderException('remote provider does not look valid'); @@ -357,6 +381,10 @@ public function jsonSerialize(): array { if ($inviteAcceptDialog !== '') { $response['inviteAcceptDialog'] = $inviteAcceptDialog; } + $jwksUri = $this->getJwksUri(); + if ($jwksUri !== '') { + $response['jwksUri'] = $jwksUri; + } return $response; } } diff --git a/lib/private/OCM/OCMDiscoveryService.php b/lib/private/OCM/OCMDiscoveryService.php index 9975038f33321..8ab06155ac9a6 100644 --- a/lib/private/OCM/OCMDiscoveryService.php +++ b/lib/private/OCM/OCMDiscoveryService.php @@ -209,7 +209,14 @@ public function getLocalOCMProvider(bool $fullDetails = true): IOCMProvider { $provider->setCapabilities(['notifications', 'shares', 'exchange-token']); $provider->setTokenEndPoint($tokenUrl); if ($signingEnabled) { - $provider->setCapabilities(['http-sig']); + try { + // advertising `http-sig` requires publishing the location of + // the local JWK Set in `jwksUri` (and it must be https) + $provider->setJwksUri($this->signatoryManager->getLocalJwksUri()); + $provider->setCapabilities(['http-sig']); + } catch (IdentityNotFoundException $e) { + $this->logger->warning('cannot build local jwksUri, http-sig capability not advertised', ['exception' => $e]); + } } $resource = $provider->createNewResourceType(); diff --git a/lib/private/OCM/OCMJwksHandler.php b/lib/private/OCM/OCMJwksHandler.php index 0013b38b1b400..92dca5985b38f 100644 --- a/lib/private/OCM/OCMJwksHandler.php +++ b/lib/private/OCM/OCMJwksHandler.php @@ -18,7 +18,11 @@ use Psr\Log\LoggerInterface; use Throwable; -/** Serves `/.well-known/jwks.json` (RFC 7517) with the OCM signing keys. */ +/** + * Serves the local JWK Set (RFC 7517) with the OCM signing keys at + * `/.well-known/jwks.json`, the URL advertised to peers through the + * `jwksUri` field of the OCM discovery response. + */ class OCMJwksHandler implements IHandler { public function __construct( private readonly IAppConfig $appConfig, diff --git a/lib/private/OCM/OCMSignatoryManager.php b/lib/private/OCM/OCMSignatoryManager.php index 8320671456a12..f8a7dcf8e6a38 100644 --- a/lib/private/OCM/OCMSignatoryManager.php +++ b/lib/private/OCM/OCMSignatoryManager.php @@ -23,10 +23,12 @@ use OCP\IConfig; use OCP\IURLGenerator; use OCP\OCM\Exceptions\OCMProviderException; +use OCP\OCM\IOCMDiscoveryService; use OCP\Security\Signature\Enum\DigestAlgorithm; use OCP\Security\Signature\Enum\SignatoryType; use OCP\Security\Signature\Enum\SignatureAlgorithm; use OCP\Security\Signature\Exceptions\IdentityNotFoundException; +use OCP\Security\Signature\Exceptions\SignatureException; use OCP\Security\Signature\ISignatureManager; use OCP\Security\Signature\Model\Signatory; use OCP\Server; @@ -373,31 +375,54 @@ private function signatoryFromPool(int $poolId): ?Signatory { return $signatory; } + /** + * Absolute https URL of the local JWK Set document, advertised as + * `jwksUri` in the OCM discovery response. The spec mandates https and + * requires the field whenever the `http-sig` capability is exposed. + * + * @throws IdentityNotFoundException + */ + public function getLocalJwksUri(): string { + return $this->buildLocalUrl('/.well-known/jwks.json'); + } + /** * @param string $fragment URL fragment (e.g. 'signature' for cavage, 'ecdsa-p256-sha256' for the JWKS-published key) * @return string * @throws IdentityNotFoundException */ private function buildLocalKeyId(string $fragment): string { + return $this->buildLocalUrl('/ocm#' . $fragment); + } + + /** + * Prefix $path with 'https://' and the signing identity of this instance, + * including a possible subfolder. + * + * @param string $path absolute path, starting with a slash + * @return string + * @throws IdentityNotFoundException + */ + private function buildLocalUrl(string $path): string { if ($this->appConfig->hasKey('core', self::APPCONFIG_SIGN_IDENTITY_EXTERNAL, true)) { $identity = $this->appConfig->getValueString('core', self::APPCONFIG_SIGN_IDENTITY_EXTERNAL, lazy: true); - return 'https://' . $identity . '/ocm#' . $fragment; + return 'https://' . $identity . $path; } try { - return $this->signatureManager->generateKeyIdFromConfig('/ocm#' . $fragment); + return $this->signatureManager->generateKeyIdFromConfig($path); } catch (IdentityNotFoundException) { } $url = $this->urlGenerator->linkToRouteAbsolute('cloud_federation_api.requesthandlercontroller.addShare'); $identity = $this->signatureManager->extractIdentityFromUri($url); - // catching possible subfolder to create a keyId like 'https://hostname/subfolder/ocm#' - $path = parse_url($url, PHP_URL_PATH); - $pos = strpos($path, '/ocm/shares'); - $sub = ($pos) ? substr($path, 0, $pos) : ''; + // catching possible subfolder to create a URL like 'https://hostname/subfolder/ocm#' + $routePath = parse_url($url, PHP_URL_PATH); + $pos = strpos($routePath, '/ocm/shares'); + $sub = ($pos) ? substr($routePath, 0, $pos) : ''; - return 'https://' . $identity . $sub . '/ocm#' . $fragment; + return 'https://' . $identity . $sub . $path; } /** @@ -476,10 +501,16 @@ private function readCachedJwks(string $origin): ?array { } /** + * Fetch the peer's JWK Set from the URL advertised in the `jwksUri` + * field of its discovery response. + * * @return list>|null */ private function fetchJwks(string $origin): ?array { - $url = 'https://' . $origin . '/.well-known/jwks.json'; + $url = $this->resolveJwksUri($origin); + if ($url === null) { + return null; + } $options = [ 'timeout' => 10, 'connect_timeout' => 10, @@ -508,6 +539,34 @@ private function fetchJwks(string $origin): ?array { return array_values(array_filter($decoded['keys'], 'is_array')); } + /** + * Location of the peer's JWK Set, read from the `jwksUri` field of its + * discovery response. A peer that advertises `http-sig` without a https + * `jwksUri` is non-conformant: no keys can be obtained from it, so its + * signed requests will fail verification. + */ + private function resolveJwksUri(string $origin): ?string { + try { + $provider = Server::get(IOCMDiscoveryService::class)->discover($origin); + } catch (NotFoundExceptionInterface|ContainerExceptionInterface|OCMProviderException $e) { + $this->logger->warning('cannot discover remote OCM provider for JWKS', ['exception' => $e, 'origin' => $origin]); + return null; + } + + $jwksUri = $provider->getJwksUri(); + if ($jwksUri === '') { + if ($provider->hasCapability('http-sig')) { + $this->logger->warning('remote advertises http-sig but no jwksUri; non-conformant peer', ['origin' => $origin]); + } + return null; + } + if (!str_starts_with($jwksUri, 'https://')) { + $this->logger->warning('remote jwksUri does not use https, ignoring', ['origin' => $origin, 'jwksUri' => $jwksUri]); + return null; + } + return $jwksUri; + } + /** * @param list>|null $keys */ @@ -519,8 +578,25 @@ private function findKid(?array $keys, string $keyId): ?Key { if (($entry['kid'] ?? null) !== $keyId) { continue; } + // every published JWK must carry an `alg` parameter naming an + // acceptable asymmetric signature algorithm; keys without one + // are rejected as non-conformant + $alg = $entry['alg'] ?? null; + if (!is_string($alg) || $alg === '') { + $this->logger->warning('remote JWK carries no alg parameter', ['kid' => $keyId]); + return null; + } try { - return JWK::parseKey($entry, Algorithm::deriveJoseAlgFromJwk($entry)); + $native = Algorithm::normalize($alg); + $derived = Algorithm::deriveJoseAlgFromJwk($entry); + if ($derived !== null && Algorithm::normalize($derived) !== $native) { + $this->logger->warning('remote JWK alg does not match its key type', ['kid' => $keyId, 'alg' => $alg]); + return null; + } + return JWK::parseKey($entry); + } catch (SignatureException $e) { + $this->logger->warning('remote JWK alg is not acceptable', ['exception' => $e, 'kid' => $keyId, 'alg' => $alg]); + return null; } catch (Throwable $e) { $this->logger->warning('failed to parse remote JWK', ['exception' => $e, 'kid' => $keyId]); return null; diff --git a/lib/private/Security/Signature/Model/Rfc9421IncomingSignedRequest.php b/lib/private/Security/Signature/Model/Rfc9421IncomingSignedRequest.php index 3697c156ec82b..00aa3f04e5ace 100644 --- a/lib/private/Security/Signature/Model/Rfc9421IncomingSignedRequest.php +++ b/lib/private/Security/Signature/Model/Rfc9421IncomingSignedRequest.php @@ -34,28 +34,33 @@ /** * RFC 9421 implementation of {@see IIncomingSignedRequest}. Parses the - * inbound Signature-Input / Signature dictionaries, picks the OCM-labeled - * entry (RFC 9421 §3.2 lets verifiers scope by policy), and rebuilds the - * signature base per RFC 9421 §2.5. Crypto is deferred to {@see verify()}, - * which needs a {@see Key} attached via {@see setKey()}. Body integrity - * (RFC 9530 content-digest) is checked before verify() if covered. + * inbound Signature-Input / Signature dictionaries, picks the single entry + * carrying the `tag="ocm"` signature parameter (disregarding dictionary + * labels, as mandated by the OCM spec), and rebuilds the signature base per + * RFC 9421 §2.5. Crypto is deferred to {@see verify()}, which needs a + * {@see Key} attached via {@see setKey()}. Body integrity (RFC 9530 + * content-digest) is checked before verify() if covered. */ class Rfc9421IncomingSignedRequest extends SignedRequest implements IIncomingSignedRequest, JsonSerializable { - /** Baseline cover for OCM. Override via `rfc9421.requiredComponents`. */ + /** + * Baseline cover for OCM. Override via `rfc9421.requiredComponents`. + * The `Date` header is deliberately not part of the required set: + * freshness is anchored on the `created` signature parameter. + */ private const DEFAULT_REQUIRED_COMPONENTS = [ '@method', '@target-uri', 'content-digest', 'content-length', - 'date', ]; /** Max clock skew (seconds) for `created`. Override via `rfc9421.maxClockSkew`. */ private const DEFAULT_MAX_FUTURE_SKEW = 60; private string $origin = ''; + private string $label; /** @var list */ private array $components; /** @var array */ @@ -88,26 +93,42 @@ public function __construct( $inputs = self::parseSignatureInput($signatureInputHeader); $signatures = self::parseSignature($signatureHeader); - // OCM policy (stricter than RFC 8941 §4.2 last-wins): a duplicate - // `ocm` entry is ambiguous; the entire request MUST be rejected. - if (self::countLabel($signatureInputHeader, 'ocm') > 1 - || self::countLabel($signatureHeader, 'ocm') > 1) { + // The OCM signature is identified by its integrity-protected + // `tag="ocm"` parameter, disregarding dictionary labels. A message + // carrying more than one such signature MUST be rejected; one + // without any is unsigned as far as OCM is concerned. + $tagged = []; + foreach ($inputs as $label => $entry) { + if (($entry['params']['tag'] ?? null) === 'ocm') { + $tagged[] = $label; + } + } + if (count($tagged) > 1) { + throw new IncomingRequestException('multiple signatures carrying tag="ocm" in Signature-Input'); + } + if ($tagged === []) { + throw new SignatureNotFoundException('no signature carrying tag="ocm" in Signature-Input'); + } + $this->label = $tagged[0]; + + // A duplicated dictionary label is collapsed to its last entry by + // RFC 8941 §4.2 parsing; that ambiguity on the OCM entry is + // rejected outright. + if (self::countLabel($signatureInputHeader, $this->label) > 1 + || self::countLabel($signatureHeader, $this->label) > 1) { throw new IncomingRequestException( - 'multiple "' . 'ocm' . '" entries in signature headers' + 'multiple "' . $this->label . '" entries in signature headers' ); } - if (!isset($inputs['ocm'])) { - throw new SignatureNotFoundException('missing "' . 'ocm' . '" entry in Signature-Input'); - } - if (!isset($signatures['ocm'])) { - throw new SignatureNotFoundException('missing "' . 'ocm' . '" entry in Signature'); + if (!isset($signatures[$this->label])) { + throw new IncomingRequestException('missing "' . $this->label . '" entry in Signature'); } - $entry = $inputs['ocm']; + $entry = $inputs[$this->label]; $this->components = $entry['components']; $this->signatureParams = $entry['params']; - $this->rawSignature = $signatures['ocm']; + $this->rawSignature = $signatures[$this->label]; $this->verifyRequiredComponents(); $this->verifyTimestamps(); @@ -121,9 +142,11 @@ public function __construct( try { $this->origin = Signatory::extractIdentityFromUri($keyId); } catch (IdentityNotFoundException) { - // keyid may follow the OCM convention `#`; the OCM layer - // derives origin from the message body in that case. - $this->origin = ''; + // keyid is not a URL; the OCM convention (and the examples in + // the spec) use `[:port]#`, in which case the origin + // is the host part before the '#'. If neither form applies the + // origin stays empty and getOrigin() rejects the request. + $this->origin = self::extractHostFromKeyId($keyId); } $paramsLine = SignatureBase::serializeSignatureParams($this->components, $this->signatureParams); @@ -136,7 +159,7 @@ public function __construct( ); $this->setSigningElements([ - 'label' => 'ocm', + 'label' => $this->label, 'keyId' => $keyId, 'algorithm' => isset($this->signatureParams['alg']) ? (string)$this->signatureParams['alg'] : '', 'created' => isset($this->signatureParams['created']) ? (string)$this->signatureParams['created'] : '', @@ -289,6 +312,22 @@ private function reconstructTargetUri(): string { return $scheme . '://' . $host . $path; } + /** + * Derive the signer's origin from the OCM `[:port]#` keyid + * convention, e.g. `sender.example.org#key1`, by parsing the keyid as a + * scheme-less authority. Returns '' when no host can be extracted. + */ + private static function extractHostFromKeyId(string $keyId): string { + if (!str_contains($keyId, '#')) { + return ''; + } + try { + return Signatory::extractIdentityFromUri('https://' . $keyId); + } catch (IdentityNotFoundException) { + return ''; + } + } + /** * Collect the HTTP request fields covered by the signature, keyed by their * lowercased name. Derived components (`@*`) are produced inside @@ -320,7 +359,7 @@ public function jsonSerialize(): array { parent::jsonSerialize(), [ 'origin' => $this->origin, - 'label' => 'ocm', + 'label' => $this->label, 'components' => $this->components, 'signatureParams' => $this->signatureParams, 'signatureBase' => $this->signatureBaseString, diff --git a/lib/private/Security/Signature/Model/Rfc9421OutgoingSignedRequest.php b/lib/private/Security/Signature/Model/Rfc9421OutgoingSignedRequest.php index d2fa2a4ae86a3..02f2c3082a020 100644 --- a/lib/private/Security/Signature/Model/Rfc9421OutgoingSignedRequest.php +++ b/lib/private/Security/Signature/Model/Rfc9421OutgoingSignedRequest.php @@ -25,7 +25,8 @@ * RFC 9421 implementation of {@see IOutgoingSignedRequest}, sibling to the * draft-cavage {@see OutgoingSignedRequest}. Default ECDSA P-256 (`ES256`) * with the `alg` parameter omitted (RFC 9421 §3.3.7); verifier resolves it - * from the JWK. + * from the JWK. The signature carries the `tag="ocm"` parameter mandated by + * the OCM spec; the `ocm` dictionary label is cosmetic. * * Options from {@see ISignatoryManager::getOptions()}: `rfc9421.signingAlgorithm`, * `rfc9421.coveredComponents`, `rfc9421.contentDigestAlgorithm`, @@ -34,7 +35,12 @@ class Rfc9421OutgoingSignedRequest extends SignedRequest implements IOutgoingSignedRequest, JsonSerializable { - private const DEFAULT_COMPONENTS = ['@method', '@target-uri', 'content-digest', 'content-length', 'date']; + /** + * Covered components mandated by the OCM spec. The `Date` header is + * deliberately not covered: intermediaries may rewrite it, and freshness + * is anchored on the `created` signature parameter. + */ + private const DEFAULT_COMPONENTS = ['@method', '@target-uri', 'content-digest', 'content-length']; private string $host = ''; private array $headers = []; @@ -84,6 +90,9 @@ public function __construct( // Off by default per RFC 9421 §3.3.7 (verifier resolves alg from JWK). $this->signatureParams['alg'] = $this->signingAlgorithm; } + // integrity-protected marker (RFC 9421 §2.3) identifying this + // signature as the OCM one; the dictionary label is not significant + $this->signatureParams['tag'] = 'ocm'; $this->signatureBaseString = SignatureBase::build( $this->method, diff --git a/lib/private/Security/Signature/Rfc9421/Algorithm.php b/lib/private/Security/Signature/Rfc9421/Algorithm.php index 4fd7569a1ff12..3c79cd6721d75 100644 --- a/lib/private/Security/Signature/Rfc9421/Algorithm.php +++ b/lib/private/Security/Signature/Rfc9421/Algorithm.php @@ -110,7 +110,8 @@ public static function verify(string $signatureBase, string $signature, Key $key } /** - * Map a JOSE alg (RFC 7518/8037) to the RFC 9421 native identifier. + * Map a JOSE alg (RFC 7518/8037, including fully-specified RFC 9864 + * names such as `Ed25519`) to the RFC 9421 native identifier. * Pass-through if already native. * * @throws SignatureException @@ -132,9 +133,9 @@ public static function normalize(string $algorithm): string { } /** - * Default JOSE alg for {@see \Firebase\JWT\JWK::parseKey} when the JWK has - * no `alg` (RFC 7517 leaves it optional). Null if kty/crv don't pin one - * down (e.g. RSA, where the hash isn't determined). + * JOSE alg implied by a JWK's kty/crv, used to cross-check the JWK's + * mandatory `alg` member against its key material. Null if kty/crv + * don't pin one down (e.g. RSA, where the hash isn't determined). * * @param array $jwk */ diff --git a/lib/private/Security/Signature/SignatureManager.php b/lib/private/Security/Signature/SignatureManager.php index 555ff28e1a337..07d855fe01d15 100644 --- a/lib/private/Security/Signature/SignatureManager.php +++ b/lib/private/Security/Signature/SignatureManager.php @@ -150,7 +150,9 @@ private function getRfc9421IncomingSignedRequest( try { $key = $signatoryManager->getRemoteKey($signedRequest->getOrigin(), $signedRequest->getKeyId()); if ($key === null) { - throw new SignatoryNotFoundException('no JWK resolved for keyid ' . $signedRequest->getKeyId()); + // a present signature MUST be verified; an unresolvable key + // is a verification failure, not an unsigned request + throw new IncomingRequestException('no JWK resolved for keyid ' . $signedRequest->getKeyId()); } $signedRequest->setKey($key); $signedRequest->verify(); diff --git a/lib/public/OCM/IOCMProvider.php b/lib/public/OCM/IOCMProvider.php index bcda666578438..0655e6ad2ad79 100644 --- a/lib/public/OCM/IOCMProvider.php +++ b/lib/public/OCM/IOCMProvider.php @@ -160,6 +160,27 @@ public function setCapabilities(array $capabilities): static; */ public function setInviteAcceptDialog(string $inviteAcceptDialog): static; + /** + * get the URL of the JWK Set document (RFC 7517) containing the public + * keys this OCM provider uses for HTTP Message Signatures (RFC 9421) + * + * @return string empty string if not advertised + * @since 35.0.0 + */ + public function getJwksUri(): string; + + /** + * set the URL of the JWK Set document (RFC 7517) containing the public + * keys this OCM provider uses for HTTP Message Signatures (RFC 9421). + * MUST use https when the `http-sig` capability is advertised. + * + * @param string $jwksUri + * + * @return $this + * @since 35.0.0 + */ + public function setJwksUri(string $jwksUri): static; + /** * get the token endpoint URL * @@ -230,7 +251,8 @@ public function import(array $data): static; * shareTypes: list, * protocols: array * }>, - * version: string + * version: string, + * jwksUri?: string * } * @since 28.0.0 */ diff --git a/tests/lib/OCM/DiscoveryServiceTest.php b/tests/lib/OCM/DiscoveryServiceTest.php index d218b2dec93bb..f158036c48dcc 100644 --- a/tests/lib/OCM/DiscoveryServiceTest.php +++ b/tests/lib/OCM/DiscoveryServiceTest.php @@ -130,12 +130,21 @@ public function testLocalBaseCapability(): void { public function testLocalCapabilitiesAdvertiseHttpSigByDefault(): void { // `http-sig` is the OCM-spec flag signalling RFC 9421 support backed - // by /.well-known/jwks.json. Advertised whenever signing is not - // disabled outright. + // by the JWK Set published at the URL in `jwksUri`. Advertised + // whenever signing is not disabled outright. $local = $this->discoveryService->getLocalOCMProvider(); $this->assertTrue($local->hasCapability('http-sig')); } + public function testLocalDiscoveryAdvertisesJwksUri(): void { + // implementations advertising `http-sig` MUST provide a https + // `jwksUri` as well + $local = $this->discoveryService->getLocalOCMProvider(); + $jwksUri = $local->getJwksUri(); + $this->assertStringStartsWith('https://', $jwksUri); + $this->assertStringEndsWith('/.well-known/jwks.json', $jwksUri); + } + public function testLocalAddedCapability(): void { $this->context->for('ocm-capability-app')->registerEventListener(LocalOCMDiscoveryEvent::class, LocalOCMDiscoveryTestEvent::class); $this->context->delegateEventListenerRegistrations($this->dispatcher); diff --git a/tests/lib/OCM/OCMProviderTest.php b/tests/lib/OCM/OCMProviderTest.php index bae2abef9a8b4..dce9cc1c2567f 100644 --- a/tests/lib/OCM/OCMProviderTest.php +++ b/tests/lib/OCM/OCMProviderTest.php @@ -69,4 +69,30 @@ public function testAddResourceTypeMergeOverwritesSameProtocol(): void { $this->provider->getResourceTypes()[0]->getProtocols(), ); } + + public function testJwksUriImportedFromDiscoveryData(): void { + $this->provider->import([ + 'enabled' => true, + 'apiVersion' => '1.1.0', + 'endPoint' => 'https://cloud.example.org/ocm', + 'capabilities' => ['http-sig'], + 'jwksUri' => 'https://cloud.example.org/ocm/jwks', + ]); + + $this->assertSame('https://cloud.example.org/ocm/jwks', $this->provider->getJwksUri()); + } + + public function testJwksUriSerializedOnlyWhenSet(): void { + $this->provider->setEnabled(true) + ->setApiVersion('1.1.0') + ->setEndPoint('https://cloud.example.org/ocm'); + + $this->assertArrayNotHasKey('jwksUri', $this->provider->jsonSerialize()); + + $this->provider->setJwksUri('https://cloud.example.org/ocm/jwks'); + $this->assertSame( + 'https://cloud.example.org/ocm/jwks', + $this->provider->jsonSerialize()['jwksUri'], + ); + } } diff --git a/tests/lib/OCM/OCMSignatoryManagerJwksTest.php b/tests/lib/OCM/OCMSignatoryManagerJwksTest.php index ee13ee352c194..3e5c4cf0a8974 100644 --- a/tests/lib/OCM/OCMSignatoryManagerJwksTest.php +++ b/tests/lib/OCM/OCMSignatoryManagerJwksTest.php @@ -10,6 +10,7 @@ namespace Test\OCM; use OC\Memcache\ArrayCache; +use OC\OCM\Model\OCMProvider; use OC\OCM\OCMSignatoryManager; use OC\Security\IdentityProof\Manager as IdentityProofManager; use OCP\Http\Client\IClient; @@ -19,6 +20,8 @@ use OCP\ICacheFactory; use OCP\IConfig; use OCP\IURLGenerator; +use OCP\OCM\Exceptions\OCMProviderException; +use OCP\OCM\IOCMDiscoveryService; use OCP\Security\Signature\ISignatureManager; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; @@ -28,6 +31,10 @@ class OCMSignatoryManagerJwksTest extends TestCase { /** RFC 7517 §A.1 test vector for an EC P-256 public key. */ private const TEST_X = 'f83OJ3D2xF1Bg8vub9tLe1gHMzV76e8Tus9uPHvRVEU'; private const TEST_Y = 'x_FEzRu9m36HLN_tue659LNpXW6pCyStikYjKIWI5a0'; + /** RFC 8037 §A.2 test vector for an Ed25519 public key. */ + private const TEST_OKP_X = '11qYAYKxCrfVS_7TyWQHOg7hcvPapiMlrwIaaPcHURo'; + + private const JWKS_URI = 'https://sender.example.org/ocm/jwks'; private IAppConfig&MockObject $appConfig; private ISignatureManager&MockObject $signatureManager; @@ -37,6 +44,7 @@ class OCMSignatoryManagerJwksTest extends TestCase { private IConfig&MockObject $config; private LoggerInterface&MockObject $logger; private IClient&MockObject $client; + private IOCMDiscoveryService&MockObject $discoveryService; private OCMSignatoryManager $signatoryManager; #[\Override] @@ -51,8 +59,10 @@ protected function setUp(): void { $this->config = $this->createMock(IConfig::class); $this->logger = $this->createMock(LoggerInterface::class); $this->client = $this->createMock(IClient::class); + $this->discoveryService = $this->createMock(IOCMDiscoveryService::class); $this->clientService->method('newClient')->willReturn($this->client); + $this->overwriteService(IOCMDiscoveryService::class, $this->discoveryService); $cacheFactory = $this->createMock(ICacheFactory::class); $cacheFactory->method('createDistributed')->willReturn(new ArrayCache('')); @@ -69,7 +79,22 @@ protected function setUp(): void { ); } + #[\Override] + protected function tearDown(): void { + $this->restoreService(IOCMDiscoveryService::class); + parent::tearDown(); + } + + /** Remote discovery response advertising http-sig and $jwksUri. */ + private function primeDiscovery(string $jwksUri = self::JWKS_URI, array $capabilities = ['http-sig']): void { + $provider = new OCMProvider(); + $provider->setCapabilities($capabilities); + $provider->setJwksUri($jwksUri); + $this->discoveryService->method('discover')->willReturn($provider); + } + public function testGetRemoteKeyFetchesAndMatchesByKid(): void { + $this->primeDiscovery(); $kid = 'sender.example.org#key1'; $jwks = [ 'keys' => [ @@ -85,17 +110,20 @@ public function testGetRemoteKeyFetchesAndMatchesByKid(): void { } public function testGetRemoteKeyReturnsNullWhenKidMissing(): void { + $this->primeDiscovery(); $this->respondWith(['keys' => [$this->ecJwk('unrelated')]]); $this->assertNull($this->signatoryManager->getRemoteKey('sender.example.org', 'other-kid')); } public function testGetRemoteKeyReturnsNullOnHttpError(): void { + $this->primeDiscovery(); $this->client->method('get')->willThrowException(new \RuntimeException('boom')); $this->logger->expects($this->once())->method('warning'); $this->assertNull($this->signatoryManager->getRemoteKey('sender.example.org', 'kid')); } public function testGetRemoteKeyReturnsNullOnInvalidJson(): void { + $this->primeDiscovery(); $response = $this->createMock(IResponse::class); $response->method('getBody')->willReturn('not json'); $this->client->method('get')->willReturn($response); @@ -104,22 +132,25 @@ public function testGetRemoteKeyReturnsNullOnInvalidJson(): void { } public function testGetRemoteKeyReturnsNullWhenKeysMissing(): void { + $this->primeDiscovery(); $this->respondWith(['no-keys-here' => []]); $this->assertNull($this->signatoryManager->getRemoteKey('sender.example.org', 'kid')); } public function testGetRemoteKeyReturnsNullOnUnparseableJwk(): void { + $this->primeDiscovery(); // JWK with kty=EC but no crv: parseKey rejects. - $this->respondWith(['keys' => [['kty' => 'EC', 'kid' => 'kid', 'x' => self::TEST_X, 'y' => self::TEST_Y]]]); + $this->respondWith(['keys' => [['kty' => 'EC', 'kid' => 'kid', 'alg' => 'ES256', 'x' => self::TEST_X, 'y' => self::TEST_Y]]]); $this->logger->expects($this->once())->method('warning'); $this->assertNull($this->signatoryManager->getRemoteKey('sender.example.org', 'kid')); } - public function testGetRemoteKeyUsesWellKnownPath(): void { + public function testGetRemoteKeyFetchesFromAdvertisedJwksUri(): void { + $this->primeDiscovery(); $this->client->expects($this->once()) ->method('get') ->with( - $this->equalTo('https://sender.example.org/.well-known/jwks.json'), + $this->equalTo(self::JWKS_URI), $this->isType('array'), ) ->willReturn($this->jsonResponse(['keys' => []])); @@ -127,7 +158,75 @@ public function testGetRemoteKeyUsesWellKnownPath(): void { $this->signatoryManager->getRemoteKey('sender.example.org', 'kid'); } + public function testGetRemoteKeyRejectsMissingJwksUriWhenHttpSigAdvertised(): void { + // a peer advertising http-sig without a jwksUri is non-conformant + $this->primeDiscovery(jwksUri: ''); + $this->client->expects($this->never())->method('get'); + $this->logger->expects($this->once())->method('warning'); + $this->assertNull($this->signatoryManager->getRemoteKey('sender.example.org', 'kid')); + } + + public function testGetRemoteKeyRejectsNonHttpsJwksUri(): void { + $this->primeDiscovery(jwksUri: 'http://sender.example.org/ocm/jwks'); + $this->client->expects($this->never())->method('get'); + $this->logger->expects($this->once())->method('warning'); + $this->assertNull($this->signatoryManager->getRemoteKey('sender.example.org', 'kid')); + } + + public function testGetRemoteKeyReturnsNullWhenDiscoveryFails(): void { + $this->discoveryService->method('discover') + ->willThrowException(new OCMProviderException('no discovery')); + $this->client->expects($this->never())->method('get'); + $this->logger->expects($this->once())->method('warning'); + $this->assertNull($this->signatoryManager->getRemoteKey('sender.example.org', 'kid')); + } + + public function testGetRemoteKeyRejectsJwkWithoutAlg(): void { + $this->primeDiscovery(); + $jwk = $this->ecJwk('kid'); + unset($jwk['alg']); + $this->respondWith(['keys' => [$jwk]]); + $this->logger->expects($this->once())->method('warning'); + $this->assertNull($this->signatoryManager->getRemoteKey('sender.example.org', 'kid')); + } + + public function testGetRemoteKeyRejectsJwkWithSymmetricAlg(): void { + $this->primeDiscovery(); + $jwk = $this->ecJwk('kid'); + $jwk['alg'] = 'HS256'; + $this->respondWith(['keys' => [$jwk]]); + $this->logger->expects($this->once())->method('warning'); + $this->assertNull($this->signatoryManager->getRemoteKey('sender.example.org', 'kid')); + } + + public function testGetRemoteKeyRejectsJwkAlgMismatchingKeyType(): void { + $this->primeDiscovery(); + // EC P-256 key claiming an Ed25519 algorithm + $jwk = $this->ecJwk('kid'); + $jwk['alg'] = 'Ed25519'; + $this->respondWith(['keys' => [$jwk]]); + $this->logger->expects($this->once())->method('warning'); + $this->assertNull($this->signatoryManager->getRemoteKey('sender.example.org', 'kid')); + } + + public function testGetRemoteKeyAcceptsFullySpecifiedEd25519Alg(): void { + $this->primeDiscovery(); + $this->respondWith(['keys' => [[ + 'kty' => 'OKP', + 'crv' => 'Ed25519', + 'kid' => 'kid', + 'alg' => 'Ed25519', + 'use' => 'sig', + 'x' => self::TEST_OKP_X, + ]]]); + + $key = $this->signatoryManager->getRemoteKey('sender.example.org', 'kid'); + $this->assertNotNull($key); + $this->assertSame('Ed25519', $key->getAlgorithm()); + } + public function testGetRemoteKeyPassesSelfSignedFlagThrough(): void { + $this->primeDiscovery(); $this->config->method('getSystemValueBool') ->with('sharing.federation.allowSelfSignedCertificates') ->willReturn(true); @@ -144,6 +243,7 @@ public function testGetRemoteKeyPassesSelfSignedFlagThrough(): void { } public function testJwksCachedAcrossCallsToTheSameOrigin(): void { + $this->primeDiscovery(); $kid = 'sender.example.org#key1'; $jwks = ['keys' => [$this->ecJwk($kid)]]; $this->client->expects($this->once()) @@ -155,6 +255,7 @@ public function testJwksCachedAcrossCallsToTheSameOrigin(): void { } public function testCacheMissOnNewKidTriggersRefetchOnce(): void { + $this->primeDiscovery(); $first = ['keys' => [$this->ecJwk('old')]]; $second = ['keys' => [$this->ecJwk('new')]]; $this->client->expects($this->exactly(2)) diff --git a/tests/lib/Security/Signature/Model/Rfc9421RoundTripTest.php b/tests/lib/Security/Signature/Model/Rfc9421RoundTripTest.php index e7d42460987f0..7c32c7dce3684 100644 --- a/tests/lib/Security/Signature/Model/Rfc9421RoundTripTest.php +++ b/tests/lib/Security/Signature/Model/Rfc9421RoundTripTest.php @@ -12,6 +12,8 @@ use Firebase\JWT\JWK; use OC\Security\Signature\Model\Rfc9421IncomingSignedRequest; use OC\Security\Signature\Model\Rfc9421OutgoingSignedRequest; +use OC\Security\Signature\Rfc9421\Algorithm; +use OC\Security\Signature\Rfc9421\ContentDigest; use OCP\IRequest; use OCP\Security\Signature\Enum\DigestAlgorithm; use OCP\Security\Signature\Enum\SignatureAlgorithm; @@ -39,6 +41,11 @@ public function testEcdsaP256RoundTripVerifies(): void { $in->setKey($jwk); $this->assertSame($out->getSignatureBaseString(), $in->getSignatureBaseString()); + // the Date header is deliberately not covered by the signature + $this->assertSame( + ['@method', '@target-uri', 'content-digest', 'content-length'], + $in->getCoveredComponents(), + ); $in->verify(); // throws on failure $this->addToAssertionCount(1); } @@ -50,11 +57,18 @@ public function testEd25519VerifyAcceptedWhenSodiumLoaded(): void { $body = '{"hello":"world"}'; $out = new Rfc9421OutgoingSignedRequest($body, $signatoryManager, 'receiver.example.org', 'POST', 'https://receiver.example.org/ocm/shares'); - // Ed25519 sign() throws via Algorithm::sign; produce the signature directly. - $rawSig = sodium_crypto_sign_detached($out->getSignatureBaseString(), $signatory->getPrivateKey()); - $out->setSignature(base64_encode($rawSig)); + // Ed25519 sign() throws via Algorithm::sign; produce the signature directly + // over a manually reconstructed signature base. $headers = $out->getHeaders(); - $paramsLine = '("@method" "@target-uri" "content-digest" "content-length" "date");created=' . time() . ';keyid="' . $signatory->getKeyId() . '"'; + $paramsLine = '("@method" "@target-uri" "content-digest" "content-length");created=' . time() . ';keyid="' . $signatory->getKeyId() . '";tag="ocm"'; + $base = implode("\n", [ + '"@method": POST', + '"@target-uri": https://receiver.example.org/ocm/shares', + '"content-digest": ' . $headers['Content-Digest'], + '"content-length": ' . $headers['Content-Length'], + '"@signature-params": ' . $paramsLine, + ]); + $rawSig = sodium_crypto_sign_detached($base, $signatory->getPrivateKey()); $headers['Signature-Input'] = 'ocm=' . $paramsLine; $headers['Signature'] = 'ocm=:' . base64_encode($rawSig) . ':'; @@ -98,7 +112,7 @@ public function testTamperedSignatureRejected(): void { $in->verify(); } - public function testOutgoingUsesOcmLabel(): void { + public function testOutgoingCarriesOcmTag(): void { [$signatory] = $this->ecdsaP256Material('https://sender.example.org/ocm#ecdsa-p256-sha256'); $signatoryManager = $this->makeSignatoryManager($signatory); @@ -106,31 +120,72 @@ public function testOutgoingUsesOcmLabel(): void { $out->sign(); $headers = $out->getHeaders(); + // the label is cosmetic; the integrity-protected tag parameter is + // what marks the signature as the OCM one $this->assertStringStartsWith('ocm=(', (string)$headers['Signature-Input']); + $this->assertStringContainsString(';tag="ocm"', (string)$headers['Signature-Input']); $this->assertStringStartsWith('ocm=:', (string)$headers['Signature']); } - public function testRequestWithoutOcmLabelRejected(): void { - [$signatory] = $this->ecdsaP256Material('https://sender.example.org/ocm#ecdsa-p256-sha256'); + public function testArbitraryLabelWithOcmTagVerifies(): void { + [$signatory, $jwk] = $this->ecdsaP256Material('https://sender.example.org/ocm#ecdsa-p256-sha256'); $signatoryManager = $this->makeSignatoryManager($signatory); $out = new Rfc9421OutgoingSignedRequest('msg', $signatoryManager, 'receiver.example.org', 'POST', 'https://receiver.example.org/ocm/shares'); $out->sign(); - // Rename the OCM label to something else; verifier MUST reject. + // Rename the dictionary label; the verifier MUST select by the + // tag="ocm" parameter and disregard labels. $headers = $out->getHeaders(); $headers['Signature-Input'] = preg_replace('/^ocm=/', 'sig1=', (string)$headers['Signature-Input']); $headers['Signature'] = preg_replace('/^ocm=/', 'sig1=', (string)$headers['Signature']); + $req = $this->mockRequest($headers, 'POST', '/ocm/shares', 'receiver.example.org'); + $in = new Rfc9421IncomingSignedRequest('msg', $req); + $in->setKey($jwk); + $in->verify(); + $this->addToAssertionCount(1); + } + + public function testRequestWithoutOcmTagTreatedAsUnsigned(): void { + [$signatory] = $this->ecdsaP256Material('https://sender.example.org/ocm#ecdsa-p256-sha256'); + $signatoryManager = $this->makeSignatoryManager($signatory); + + $out = new Rfc9421OutgoingSignedRequest('msg', $signatoryManager, 'receiver.example.org', 'POST', 'https://receiver.example.org/ocm/shares'); + $out->sign(); + + // Strip the tag parameter; without tag="ocm" the request carries no + // OCM signature and is handled as unsigned. + $headers = $out->getHeaders(); + $headers['Signature-Input'] = str_replace(';tag="ocm"', '', (string)$headers['Signature-Input']); + $req = $this->mockRequest($headers, 'POST', '/ocm/shares', 'receiver.example.org'); $this->expectException(SignatureNotFoundException::class); new Rfc9421IncomingSignedRequest('msg', $req); } + public function testTwoSignaturesCarryingOcmTagRejected(): void { + [$signatory] = $this->ecdsaP256Material('https://sender.example.org/ocm#ecdsa-p256-sha256'); + $signatoryManager = $this->makeSignatoryManager($signatory); + + $out = new Rfc9421OutgoingSignedRequest('msg', $signatoryManager, 'receiver.example.org', 'POST', 'https://receiver.example.org/ocm/shares'); + $out->sign(); + + // A second, differently-labeled signature also carrying tag="ocm": + // the entire message MUST be rejected. + $headers = $out->getHeaders(); + $headers['Signature-Input'] = (string)$headers['Signature-Input'] . ', ' . preg_replace('/^ocm=/', 'sig2=', (string)$headers['Signature-Input']); + $headers['Signature'] = (string)$headers['Signature'] . ', ' . preg_replace('/^ocm=/', 'sig2=', (string)$headers['Signature']); + + $req = $this->mockRequest($headers, 'POST', '/ocm/shares', 'receiver.example.org'); + $this->expectException(IncomingRequestException::class); + new Rfc9421IncomingSignedRequest('msg', $req); + } + public function testDuplicateOcmLabelRejected(): void { - // RFC 8941 §4.2 last-wins on duplicate dictionary keys, but OCM - // mandates that duplicate `ocm` entries cause the request to be - // rejected outright. The model layer enforces that. + // RFC 8941 §4.2 last-wins on duplicate dictionary keys, which would + // silently hide one of two identically-labeled OCM signatures; that + // ambiguity on the selected entry causes outright rejection. [$signatory] = $this->ecdsaP256Material('https://sender.example.org/ocm#ecdsa-p256-sha256'); $signatoryManager = $this->makeSignatoryManager($signatory); @@ -223,6 +278,44 @@ public function testMissingCreatedRejected(): void { new Rfc9421IncomingSignedRequest($body, $req); } + public function testMissingKeyidRejected(): void { + [$signatory] = $this->ecdsaP256Material('https://sender.example.org/ocm#ecdsa-p256-sha256'); + $signatoryManager = $this->makeSignatoryManager($signatory); + + $body = 'msg'; + $out = new Rfc9421OutgoingSignedRequest($body, $signatoryManager, 'receiver.example.org', 'POST', 'https://receiver.example.org/ocm/shares'); + $out->sign(); + + // Strip the `;keyid="..."` parameter; verifiers MUST reject + // signatures without it. + $headers = $out->getHeaders(); + $headers['Signature-Input'] = preg_replace('/;keyid="[^"]*"/', '', (string)$headers['Signature-Input']); + + $req = $this->mockRequest($headers, 'POST', '/ocm/shares', 'receiver.example.org'); + $this->expectException(IncomingRequestException::class); + new Rfc9421IncomingSignedRequest($body, $req); + } + + public function testExtraCoveredDateStillVerifies(): void { + // covering more than the mandatory components (here: `date`) is + // allowed; only the four baseline components are required + [$signatory, $jwk] = $this->ecdsaP256Material('https://sender.example.org/ocm#ecdsa-p256-sha256'); + $signatoryManager = $this->makeSignatoryManagerWithComponents( + $signatory, + ['@method', '@target-uri', 'content-digest', 'content-length', 'date'], + ); + + $body = 'msg'; + $out = new Rfc9421OutgoingSignedRequest($body, $signatoryManager, 'receiver.example.org', 'POST', 'https://receiver.example.org/ocm/shares'); + $out->sign(); + + $req = $this->mockRequestFromOutgoing($out, 'POST', '/ocm/shares', 'receiver.example.org'); + $in = new Rfc9421IncomingSignedRequest($body, $req); + $in->setKey($jwk); + $in->verify(); + $this->addToAssertionCount(1); + } + public function testSignatureNotCoveringRequiredComponentsRejected(): void { // A peer that signs only `@method` and `@target-uri`: the body and // freshness window aren't bound. Even with a valid signature we @@ -242,6 +335,40 @@ public function testSignatureNotCoveringRequiredComponentsRejected(): void { new Rfc9421IncomingSignedRequest($body, $req); } + public function testHostFragmentKeyIdYieldsOrigin(): void { + // the spec's canonical keyid form is `#`, not a URL; the + // origin used for JWKS resolution is the host before the '#'. + // Nextcloud's own Signatory model rejects such kids, so build the + // peer's request manually. + $kid = 'sender.example.org#key1'; + [$privatePem, $jwk] = $this->ecdsaP256Jwk($kid); + + $body = 'msg'; + $digest = ContentDigest::compute($body, ContentDigest::ALGO_SHA256); + $paramsLine = '("@method" "@target-uri" "content-digest" "content-length");created=' . time() . ';keyid="' . $kid . '";tag="ocm"'; + $base = implode("\n", [ + '"@method": POST', + '"@target-uri": https://receiver.example.org/ocm/shares', + '"content-digest": ' . $digest, + '"content-length": ' . strlen($body), + '"@signature-params": ' . $paramsLine, + ]); + $rawSig = Algorithm::sign($base, $privatePem, 'ecdsa-p256-sha256'); + $headers = [ + 'Content-Digest' => $digest, + 'Content-Length' => (string)strlen($body), + 'Signature-Input' => 'sig1=' . $paramsLine, + 'Signature' => 'sig1=:' . base64_encode($rawSig) . ':', + ]; + + $req = $this->mockRequest($headers, 'POST', '/ocm/shares', 'receiver.example.org'); + $in = new Rfc9421IncomingSignedRequest($body, $req); + $this->assertSame('sender.example.org', $in->getOrigin()); + $in->setKey($jwk); + $in->verify(); + $this->addToAssertionCount(1); + } + private function skipUnlessSodium(): void { if (!extension_loaded('sodium')) { $this->markTestSkipped('ext-sodium is not loaded'); @@ -323,9 +450,29 @@ private function ecdsaP256Material(string $kid): array { $signatory->setPublicKey($publicPem); $signatory->setPrivateKey($privatePem); + $key = self::jwkFromEcDetails($details, $kid); + return [$signatory, $key]; + } + + /** + * Key material for a peer whose kid is not a URL; Nextcloud's Signatory + * model cannot represent those. + * + * @return array{0: string, 1: \Firebase\JWT\Key} [private key PEM, verification key] + */ + private function ecdsaP256Jwk(string $kid): array { + $pkey = openssl_pkey_new(['private_key_type' => OPENSSL_KEYTYPE_EC, 'curve_name' => 'prime256v1']); + $privatePem = ''; + openssl_pkey_export($pkey, $privatePem); + $details = openssl_pkey_get_details($pkey); + + return [$privatePem, self::jwkFromEcDetails($details, $kid)]; + } + + private static function jwkFromEcDetails(array $details, string $kid): \Firebase\JWT\Key { $x = str_pad($details['ec']['x'], 32, "\x00", STR_PAD_LEFT); $y = str_pad($details['ec']['y'], 32, "\x00", STR_PAD_LEFT); - $key = JWK::parseKey([ + return JWK::parseKey([ 'kty' => 'EC', 'crv' => 'P-256', 'kid' => $kid, @@ -333,7 +480,6 @@ private function ecdsaP256Material(string $kid): array { 'x' => self::b64url($x), 'y' => self::b64url($y), ], 'ES256'); - return [$signatory, $key]; } /** diff --git a/tests/lib/Security/Signature/Rfc9421/AlgorithmTest.php b/tests/lib/Security/Signature/Rfc9421/AlgorithmTest.php index abcf200e76048..50610285c3b99 100644 --- a/tests/lib/Security/Signature/Rfc9421/AlgorithmTest.php +++ b/tests/lib/Security/Signature/Rfc9421/AlgorithmTest.php @@ -27,6 +27,8 @@ public function testNormalizeNativeIsPassThrough(): void { public function testNormalizeJoseAliases(): void { $this->assertSame('ed25519', Algorithm::normalize('EdDSA')); + // fully-specified RFC 9864 name, recommended by the OCM spec + $this->assertSame('ed25519', Algorithm::normalize('Ed25519')); $this->assertSame('ecdsa-p256-sha256', Algorithm::normalize('ES256')); $this->assertSame('ecdsa-p384-sha384', Algorithm::normalize('ES384')); $this->assertSame('rsa-v1_5-sha256', Algorithm::normalize('RS256')); @@ -117,7 +119,9 @@ public function testAlgHintConflictsWithJwkAlgRejected(): void { public function testParseKeyRejectsContradictoryAlg(): void { $this->markTestSkipped( 'firebase/php-jwt JWK::parseKey does not validate kty/crv/alg coherence; ' - . 'the alg mismatch is caught at verify() time instead — see testVerifyEd25519KeyAgainstES256Alg.' + . 'OCMSignatoryManager::findKid() rejects such keys before parsing ' + . '(see OCMSignatoryManagerJwksTest::testGetRemoteKeyRejectsJwkAlgMismatchingKeyType) ' + . 'and a remaining mismatch is caught at verify() time.' ); } diff --git a/tests/lib/Security/Signature/SignatureManagerDispatchTest.php b/tests/lib/Security/Signature/SignatureManagerDispatchTest.php index 698ea59d6188e..cbe776933c3ad 100644 --- a/tests/lib/Security/Signature/SignatureManagerDispatchTest.php +++ b/tests/lib/Security/Signature/SignatureManagerDispatchTest.php @@ -123,6 +123,29 @@ public function testInboundRejectsRfc9421WhenSignatoryManagerCannotResolve(): vo $this->signatureManager->getIncomingSignedRequest($signatoryManager, $body); } + public function testInboundRejectsRfc9421WhenNoKeyResolvedForKeyid(): void { + [$signatoryManager, $jwk] = $this->ecdsaP256SignatoryManager(rfc9421Format: true); + + $body = '{"hello":"world"}'; + $out = new Rfc9421OutgoingSignedRequest( + $body, + $signatoryManager, + 'receiver.example.org', + 'POST', + 'https://receiver.example.org/ocm/shares', + ); + $out->sign(); + $this->primeRequest($out->getHeaders(), 'POST', '/ocm/shares', 'receiver.example.org'); + + // resolver knows a different kid only: a present signature whose key + // cannot be resolved is a verification failure, not an unsigned + // request + $resolver = $this->makeKeyResolver($signatoryManager, $jwk, 'https://other.example.org/ocm#nomatch'); + + $this->expectException(IncomingRequestException::class); + $this->signatureManager->getIncomingSignedRequest($resolver, $body); + } + private function rsaSignatoryManager(): ISignatoryManager { $key = openssl_pkey_new(['private_key_type' => OPENSSL_KEYTYPE_RSA, 'private_key_bits' => 2048]); $priv = ''; From 953085af4fceab7654732ec6f70c0300f8407969 Mon Sep 17 00:00:00 2001 From: Micke Nordin Date: Sun, 26 Jul 2026 14:30:08 +0200 Subject: [PATCH 2/6] fix(ocm): correct confirmRequestOrigin @since to 35.0.0 The annotation was added as 34.0.0 in #60136, but stable34 branched before the PR merged, so the API first ships in 35. Signed-off-by: Micke Nordin --- lib/private/OCM/OCMDiscoveryService.php | 2 +- lib/public/OCM/IOCMDiscoveryService.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/private/OCM/OCMDiscoveryService.php b/lib/private/OCM/OCMDiscoveryService.php index 8ab06155ac9a6..c072b797b689a 100644 --- a/lib/private/OCM/OCMDiscoveryService.php +++ b/lib/private/OCM/OCMDiscoveryService.php @@ -284,7 +284,7 @@ public function getIncomingSignedRequest(): ?IIncomingSignedRequest { /** * @inheritDoc * - * @since 34.0.0 + * @since 35.0.0 */ #[\Override] public function confirmRequestOrigin(?string $signedOrigin, string $ocmAddress): void { diff --git a/lib/public/OCM/IOCMDiscoveryService.php b/lib/public/OCM/IOCMDiscoveryService.php index 674c907bb1587..982dc0abc91b8 100644 --- a/lib/public/OCM/IOCMDiscoveryService.php +++ b/lib/public/OCM/IOCMDiscoveryService.php @@ -76,7 +76,7 @@ public function getIncomingSignedRequest(): ?IIncomingSignedRequest; * @param string $ocmAddress in `user@host` or `user@https://host` form * * @throws IncomingRequestException on mismatch or malformed address - * @since 34.0.0 + * @since 35.0.0 */ public function confirmRequestOrigin(?string $signedOrigin, string $ocmAddress): void; From be608c41c11ea599c42f2ba92f3d267e18fbd82b Mon Sep 17 00:00:00 2001 From: Micke Nordin Date: Tue, 28 Jul 2026 16:40:07 +0200 Subject: [PATCH 3/6] fix(ocm): allow plain-http jwksUri for http-only peers The spec mandates https for jwksUri but allows an HTTP fallback in testing setups. Advertise the scheme the instance actually serves, and accept an http jwksUri only from a peer whose own endpoint is http: integration tests and http intranets work without configuration, while an https peer advertising an http jwksUri is still rejected, since that downgrade would let a path attacker swap the key material. An https jwksUri is always accepted, also from http-only peers, and it may live on a different host than the peer. Signed-off-by: Micke Nordin --- lib/private/OCM/OCMDiscoveryService.php | 3 +- lib/private/OCM/OCMSignatoryManager.php | 40 ++++----- tests/lib/OCM/DiscoveryServiceTest.php | 8 +- tests/lib/OCM/OCMSignatoryManagerJwksTest.php | 84 ++++++++++++++++++- .../OCM/OCMSignatoryManagerRotationTest.php | 15 ++-- 5 files changed, 109 insertions(+), 41 deletions(-) diff --git a/lib/private/OCM/OCMDiscoveryService.php b/lib/private/OCM/OCMDiscoveryService.php index c072b797b689a..f6c0a55109dcb 100644 --- a/lib/private/OCM/OCMDiscoveryService.php +++ b/lib/private/OCM/OCMDiscoveryService.php @@ -210,8 +210,7 @@ public function getLocalOCMProvider(bool $fullDetails = true): IOCMProvider { $provider->setTokenEndPoint($tokenUrl); if ($signingEnabled) { try { - // advertising `http-sig` requires publishing the location of - // the local JWK Set in `jwksUri` (and it must be https) + // http-sig advertisement requires a jwksUri $provider->setJwksUri($this->signatoryManager->getLocalJwksUri()); $provider->setCapabilities(['http-sig']); } catch (IdentityNotFoundException $e) { diff --git a/lib/private/OCM/OCMSignatoryManager.php b/lib/private/OCM/OCMSignatoryManager.php index f8a7dcf8e6a38..47a7df1e554b0 100644 --- a/lib/private/OCM/OCMSignatoryManager.php +++ b/lib/private/OCM/OCMSignatoryManager.php @@ -376,9 +376,8 @@ private function signatoryFromPool(int $poolId): ?Signatory { } /** - * Absolute https URL of the local JWK Set document, advertised as - * `jwksUri` in the OCM discovery response. The spec mandates https and - * requires the field whenever the `http-sig` capability is exposed. + * Absolute URL of the local JWK Set, advertised as `jwksUri` in the + * discovery response. * * @throws IdentityNotFoundException */ @@ -396,12 +395,13 @@ private function buildLocalKeyId(string $fragment): string { } /** - * Prefix $path with 'https://' and the signing identity of this instance, - * including a possible subfolder. + * Absolute local URL for a signing path (keyId fragment or jwksUri), + * built via {@see IURLGenerator::getAbsoluteURL()} so the advertised + * signing origin matches the instance URL used for federated shares. + * keyId callers re-canonicalize to https through {@see Signatory::setKeyId}. * * @param string $path absolute path, starting with a slash * @return string - * @throws IdentityNotFoundException */ private function buildLocalUrl(string $path): string { if ($this->appConfig->hasKey('core', self::APPCONFIG_SIGN_IDENTITY_EXTERNAL, true)) { @@ -409,20 +409,7 @@ private function buildLocalUrl(string $path): string { return 'https://' . $identity . $path; } - try { - return $this->signatureManager->generateKeyIdFromConfig($path); - } catch (IdentityNotFoundException) { - } - - $url = $this->urlGenerator->linkToRouteAbsolute('cloud_federation_api.requesthandlercontroller.addShare'); - $identity = $this->signatureManager->extractIdentityFromUri($url); - - // catching possible subfolder to create a URL like 'https://hostname/subfolder/ocm#' - $routePath = parse_url($url, PHP_URL_PATH); - $pos = strpos($routePath, '/ocm/shares'); - $sub = ($pos) ? substr($routePath, 0, $pos) : ''; - - return 'https://' . $identity . $sub . $path; + return $this->urlGenerator->getAbsoluteURL($path); } /** @@ -540,10 +527,9 @@ private function fetchJwks(string $origin): ?array { } /** - * Location of the peer's JWK Set, read from the `jwksUri` field of its - * discovery response. A peer that advertises `http-sig` without a https - * `jwksUri` is non-conformant: no keys can be obtained from it, so its - * signed requests will fail verification. + * The peer's `jwksUri` from its discovery response. Must be https, or + * http from an http-only peer (the spec's testing fallback): an https + * peer pointing at an http jwksUri would downgrade the key fetch. */ private function resolveJwksUri(string $origin): ?string { try { @@ -560,8 +546,10 @@ private function resolveJwksUri(string $origin): ?string { } return null; } - if (!str_starts_with($jwksUri, 'https://')) { - $this->logger->warning('remote jwksUri does not use https, ignoring', ['origin' => $origin, 'jwksUri' => $jwksUri]); + $httpFromHttpPeer = str_starts_with($jwksUri, 'http://') + && str_starts_with($provider->getEndPoint(), 'http://'); + if (!str_starts_with($jwksUri, 'https://') && !$httpFromHttpPeer) { + $this->logger->warning('refusing jwksUri: https is required unless the peer itself is http-only', ['origin' => $origin, 'jwksUri' => $jwksUri]); return null; } return $jwksUri; diff --git a/tests/lib/OCM/DiscoveryServiceTest.php b/tests/lib/OCM/DiscoveryServiceTest.php index f158036c48dcc..128e3f36af381 100644 --- a/tests/lib/OCM/DiscoveryServiceTest.php +++ b/tests/lib/OCM/DiscoveryServiceTest.php @@ -14,6 +14,7 @@ use OCA\CloudFederationAPI\Controller\OCMRequestController; use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; +use OCP\IURLGenerator; use OCP\OCM\Events\LocalOCMDiscoveryEvent; use OCP\OCM\Events\OCMEndpointRequestEvent; use OCP\Server; @@ -137,11 +138,12 @@ public function testLocalCapabilitiesAdvertiseHttpSigByDefault(): void { } public function testLocalDiscoveryAdvertisesJwksUri(): void { - // implementations advertising `http-sig` MUST provide a https - // `jwksUri` as well + // scheme follows the instance base URL $local = $this->discoveryService->getLocalOCMProvider(); $jwksUri = $local->getJwksUri(); - $this->assertStringStartsWith('https://', $jwksUri); + $baseUrl = Server::get(IURLGenerator::class)->getBaseUrl(); + $expectedScheme = str_starts_with($baseUrl, 'http://') ? 'http://' : 'https://'; + $this->assertStringStartsWith($expectedScheme, $jwksUri); $this->assertStringEndsWith('/.well-known/jwks.json', $jwksUri); } diff --git a/tests/lib/OCM/OCMSignatoryManagerJwksTest.php b/tests/lib/OCM/OCMSignatoryManagerJwksTest.php index 3e5c4cf0a8974..66f36f2099f92 100644 --- a/tests/lib/OCM/OCMSignatoryManagerJwksTest.php +++ b/tests/lib/OCM/OCMSignatoryManagerJwksTest.php @@ -86,10 +86,15 @@ protected function tearDown(): void { } /** Remote discovery response advertising http-sig and $jwksUri. */ - private function primeDiscovery(string $jwksUri = self::JWKS_URI, array $capabilities = ['http-sig']): void { + private function primeDiscovery( + string $jwksUri = self::JWKS_URI, + array $capabilities = ['http-sig'], + string $endPoint = 'https://sender.example.org/ocm', + ): void { $provider = new OCMProvider(); $provider->setCapabilities($capabilities); $provider->setJwksUri($jwksUri); + $provider->setEndPoint($endPoint); $this->discoveryService->method('discover')->willReturn($provider); } @@ -166,13 +171,32 @@ public function testGetRemoteKeyRejectsMissingJwksUriWhenHttpSigAdvertised(): vo $this->assertNull($this->signatoryManager->getRemoteKey('sender.example.org', 'kid')); } - public function testGetRemoteKeyRejectsNonHttpsJwksUri(): void { + public function testGetRemoteKeyRejectsHttpJwksUriFromHttpsPeer(): void { + // downgrade guard: http jwksUri from an https peer $this->primeDiscovery(jwksUri: 'http://sender.example.org/ocm/jwks'); $this->client->expects($this->never())->method('get'); $this->logger->expects($this->once())->method('warning'); $this->assertNull($this->signatoryManager->getRemoteKey('sender.example.org', 'kid')); } + public function testGetRemoteKeyAcceptsHttpJwksUriFromHttpPeer(): void { + // the spec's http fallback for testing setups + $this->primeDiscovery( + jwksUri: 'http://sender.example.org/ocm/jwks', + endPoint: 'http://sender.example.org/ocm', + ); + $kid = 'sender.example.org#key1'; + $this->client->expects($this->once()) + ->method('get') + ->with( + $this->equalTo('http://sender.example.org/ocm/jwks'), + $this->isType('array'), + ) + ->willReturn($this->jsonResponse(['keys' => [$this->ecJwk($kid)]])); + + $this->assertNotNull($this->signatoryManager->getRemoteKey('sender.example.org', $kid)); + } + public function testGetRemoteKeyReturnsNullWhenDiscoveryFails(): void { $this->discoveryService->method('discover') ->willThrowException(new OCMProviderException('no discovery')); @@ -269,6 +293,62 @@ public function testCacheMissOnNewKidTriggersRefetchOnce(): void { $this->assertNotNull($this->signatoryManager->getRemoteKey('sender.example.org', 'new')); } + public function testGetRemoteKeyAcceptsHttpsJwksUriFromHttpPeer(): void { + // upgrade from an http-only peer is fine + $this->primeDiscovery( + endPoint: 'http://sender.example.org/ocm', + ); + $kid = 'sender.example.org#key1'; + $this->client->expects($this->once()) + ->method('get') + ->with( + $this->equalTo(self::JWKS_URI), + $this->isType('array'), + ) + ->willReturn($this->jsonResponse(['keys' => [$this->ecJwk($kid)]])); + + $this->assertNotNull($this->signatoryManager->getRemoteKey('sender.example.org', $kid)); + } + + public function testGetRemoteKeyAcceptsJwksUriOnDifferentHost(): void { + // the JWK Set may live on a different host than the peer + $this->primeDiscovery( + jwksUri: 'https://keys.example.net/ocm/jwks', + endPoint: 'http://sender.example.org/ocm', + ); + $kid = 'sender.example.org#key1'; + $this->client->expects($this->once()) + ->method('get') + ->with( + $this->equalTo('https://keys.example.net/ocm/jwks'), + $this->isType('array'), + ) + ->willReturn($this->jsonResponse(['keys' => [$this->ecJwk($kid)]])); + + $this->assertNotNull($this->signatoryManager->getRemoteKey('sender.example.org', $kid)); + } + + public function testGetLocalJwksUriUsesHttpsByDefault(): void { + $this->urlGenerator->method('getAbsoluteURL') + ->willReturnCallback(fn (string $path) => 'https://sender.example.org' . $path); + + $this->assertSame( + 'https://sender.example.org/.well-known/jwks.json', + $this->signatoryManager->getLocalJwksUri(), + ); + } + + public function testGetLocalJwksUriFollowsHttpInstanceScheme(): void { + // http-only deployments must advertise a fetchable jwksUri + $this->urlGenerator->method('getAbsoluteURL') + ->willReturnCallback(fn (string $path) => 'http://localhost:8180' . $path); + + $this->assertSame( + 'http://localhost:8180/.well-known/jwks.json', + $this->signatoryManager->getLocalJwksUri(), + ); + } + private function respondWith(array $body): void { $this->client->method('get')->willReturn($this->jsonResponse($body)); } diff --git a/tests/lib/OCM/OCMSignatoryManagerRotationTest.php b/tests/lib/OCM/OCMSignatoryManagerRotationTest.php index 3c1d7038ee78c..bc3a7f2530c54 100644 --- a/tests/lib/OCM/OCMSignatoryManagerRotationTest.php +++ b/tests/lib/OCM/OCMSignatoryManagerRotationTest.php @@ -19,7 +19,6 @@ use OCP\ICacheFactory; use OCP\IConfig; use OCP\IURLGenerator; -use OCP\Security\Signature\Exceptions\IdentityNotFoundException; use OCP\Security\Signature\ISignatureManager; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; @@ -47,8 +46,9 @@ protected function setUp(): void { $this->wireIdentityProofManager(); $signatureManager = $this->createMock(ISignatureManager::class); - $signatureManager->method('generateKeyIdFromConfig') - ->willReturnCallback(static fn (string $suffix): string => 'https://alice.example/' . ltrim($suffix, '/')); + $urlGenerator = $this->createMock(IURLGenerator::class); + $urlGenerator->method('getAbsoluteURL') + ->willReturnCallback(static fn (string $path): string => 'https://alice.example' . $path); $cacheFactory = $this->createMock(ICacheFactory::class); $cacheFactory->method('createDistributed')->willReturn(new ArrayCache('')); @@ -56,7 +56,7 @@ protected function setUp(): void { $this->signatoryManager = new OCMSignatoryManager( $this->appConfig, $signatureManager, - $this->createMock(IURLGenerator::class), + $urlGenerator, $this->identityProofManager, $this->stubClientService(), $this->createMock(IConfig::class), @@ -188,11 +188,10 @@ public function testSignerReturnsNullWhenIdentityCannotBeDerived(): void { // identity at all; provisioning the first key should fail loudly so // the admin gets a clear message instead of a corrupt half-state. $signatureManager = $this->createMock(ISignatureManager::class); - $signatureManager->method('generateKeyIdFromConfig') - ->willThrowException(new IdentityNotFoundException('no identity')); $urlGenerator = $this->createMock(IURLGenerator::class); - $urlGenerator->method('linkToRouteAbsolute') - ->willThrowException(new IdentityNotFoundException('no url either')); + // getAbsoluteURL() yields no host, so the kid's identity cannot be + // resolved; provisioning must fail loudly rather than corrupt state. + $urlGenerator->method('getAbsoluteURL')->willReturn(''); $cacheFactory = $this->createMock(ICacheFactory::class); $cacheFactory->method('createDistributed')->willReturn(new ArrayCache('')); From 9a4e9cbfb20f052f76d24b5cf136dcdab56d85be Mon Sep 17 00:00:00 2001 From: Micke Nordin Date: Tue, 28 Jul 2026 19:50:08 +0200 Subject: [PATCH 4/6] 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-) 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 --- lib/private/OCM/OCMSignatoryManager.php | 88 +++++-------------- .../OCM/OCMSignatoryManagerRotationTest.php | 4 +- 2 files changed, 25 insertions(+), 67 deletions(-) diff --git a/lib/private/OCM/OCMSignatoryManager.php b/lib/private/OCM/OCMSignatoryManager.php index 47a7df1e554b0..63b48eb7847ae 100644 --- a/lib/private/OCM/OCMSignatoryManager.php +++ b/lib/private/OCM/OCMSignatoryManager.php @@ -57,8 +57,6 @@ class OCMSignatoryManager implements IJwkResolvingSignatoryManager { private const APPKEY_JWKS_POOL_PREFIX = 'ocm_jwks_pool_'; private const APPCONFIG_JWKS_POOL_COUNTER = 'ocm_jwks_pool_counter'; private const APPCONFIG_JWKS_POOL_KID_PREFIX = 'ocm_jwks_pool_kid_'; - /** Stable kid identity portion, reused across rotations so kids stay on one hostname. */ - private const APPCONFIG_JWKS_KID_BASE = 'ocm_jwks_kid_base'; public const SLOT_ACTIVE = 'active'; public const SLOT_PENDING = 'pending'; public const SLOT_RETIRING = 'retiring'; @@ -262,9 +260,7 @@ public function listJwksKeys(): array { } $entries[] = [ 'poolId' => $id, - 'kid' => $this->canonicalKid( - $this->appConfig->getValueString('core', self::APPCONFIG_JWKS_POOL_KID_PREFIX . $id, ''), - ), + 'kid' => $this->appConfig->getValueString('core', self::APPCONFIG_JWKS_POOL_KID_PREFIX . $id, ''), 'slot' => $bySlot[$id] ?? null, ]; } @@ -272,72 +268,24 @@ public function listJwksKeys(): array { } /** - * Generate keypair into a new pool. Kid is canonicalised through - * {@see Signatory::setKeyId} so admin output and wire form agree. + * Generate keypair into a new pool. Only the opaque key id is persisted; + * the host is derived fresh per request at use time, so a key provisioned + * under one URL still verifies when the instance is addressed by another + * (e.g. the two-port integration rig, or a port change behind a proxy). */ - private function generatePool(string $kid): int { + private function generatePool(string $opaqueKeyId): int { $poolId = $this->appConfig->getValueInt('core', self::APPCONFIG_JWKS_POOL_COUNTER, 0) + 1; $this->appConfig->setValueInt('core', self::APPCONFIG_JWKS_POOL_COUNTER, $poolId); $this->identityProofManager->generateEcdsaP256AppKey('core', self::APPKEY_JWKS_POOL_PREFIX . $poolId); - $this->appConfig->setValueString('core', self::APPCONFIG_JWKS_POOL_KID_PREFIX . $poolId, $this->canonicalKid($kid)); + $this->appConfig->setValueString('core', self::APPCONFIG_JWKS_POOL_KID_PREFIX . $poolId, $opaqueKeyId); return $poolId; } - /** Canonical wire-form via a transient {@see Signatory::setKeyId} round-trip. */ - private function canonicalKid(string $kid): string { - $probe = new Signatory(true); - $probe->setKeyId($kid); - return $probe->getKeyId(); - } - - /** - * Build the next kid. Identity portion is derived once and persisted so - * CLI-triggered rotations stay on the same hostname. - * - * @throws \RuntimeException if no instance identity can be derived - */ + /** Next opaque key id (`-`); the host is added per request. */ private function nextPoolKid(): string { - $base = $this->resolveKidBase(); $next = $this->appConfig->getValueInt('core', self::APPCONFIG_JWKS_POOL_COUNTER, 0) + 1; - return $base . '-' . $next; - } - - /** - * Stable identity portion (before the `-N` suffix). Resolution order: - * stored APPCONFIG_JWKS_KID_BASE > active pool's kid sans suffix > - * fresh from {@see buildLocalKeyId}. Persisted so CLI rotations stay - * on one hostname. - * - * @throws \RuntimeException if no instance identity can be derived - */ - private function resolveKidBase(): string { - $base = $this->appConfig->getValueString('core', self::APPCONFIG_JWKS_KID_BASE, ''); - if ($base !== '') { - return $base; - } - - $activePool = $this->getSlotPool(self::SLOT_ACTIVE); - if ($activePool !== null) { - $kid = $this->canonicalKid( - $this->appConfig->getValueString('core', self::APPCONFIG_JWKS_POOL_KID_PREFIX . $activePool, ''), - ); - $pos = strrpos($kid, '-'); - if ($pos !== false) { - $base = substr($kid, 0, $pos); - } - } - - if ($base === '') { - try { - $base = $this->canonicalKid($this->buildLocalKeyId(self::KEYID_FRAGMENT_JWKS)); - } catch (IdentityNotFoundException $e) { - throw new \RuntimeException('cannot derive instance identity for JWKS kid', 0, $e); - } - } - - $this->appConfig->setValueString('core', self::APPCONFIG_JWKS_KID_BASE, $base); - return $base; + return self::KEYID_FRAGMENT_JWKS . '-' . $next; } private function getSlotPool(string $slot): ?int { @@ -357,19 +305,29 @@ private function clearSlot(string $slot): void { $this->appConfig->deleteKey('core', 'ocm_jwks_slot_' . $slot); } - /** Returns null if the underlying appkey was manually deleted. */ + /** + * Returns null if the underlying appkey was manually deleted. The keyId + * is rebuilt per call from the opaque id and the current request URL, so + * it tracks the host the instance is actually addressed as. + * + * @throws \RuntimeException if no instance identity can be derived + */ private function signatoryFromPool(int $poolId): ?Signatory { $appKey = self::APPKEY_JWKS_POOL_PREFIX . $poolId; if (!$this->identityProofManager->hasAppKey('core', $appKey)) { return null; } - $kid = $this->appConfig->getValueString('core', self::APPCONFIG_JWKS_POOL_KID_PREFIX . $poolId, ''); - if ($kid === '') { + $opaqueKeyId = $this->appConfig->getValueString('core', self::APPCONFIG_JWKS_POOL_KID_PREFIX . $poolId, ''); + if ($opaqueKeyId === '') { return null; } $keyPair = $this->identityProofManager->getAppKey('core', $appKey); $signatory = new Signatory(true); - $signatory->setKeyId($kid); + try { + $signatory->setKeyId($this->buildLocalKeyId($opaqueKeyId)); + } catch (IdentityNotFoundException $e) { + throw new \RuntimeException('cannot derive instance identity for JWKS kid', 0, $e); + } $signatory->setPublicKey($keyPair->getPublic()); $signatory->setPrivateKey($keyPair->getPrivate()); return $signatory; diff --git a/tests/lib/OCM/OCMSignatoryManagerRotationTest.php b/tests/lib/OCM/OCMSignatoryManagerRotationTest.php index bc3a7f2530c54..a4a1d073d302e 100644 --- a/tests/lib/OCM/OCMSignatoryManagerRotationTest.php +++ b/tests/lib/OCM/OCMSignatoryManagerRotationTest.php @@ -86,7 +86,7 @@ public function testFirstCallProvisionsActiveKey(): void { $this->assertSame($signatory->getKeyId(), $jwks[0]['kid']); $listed = $this->signatoryManager->listJwksKeys(); - $this->assertSame([['poolId' => 1, 'kid' => $signatory->getKeyId(), 'slot' => 'active']], $listed); + $this->assertSame([['poolId' => 1, 'kid' => 'ecdsa-p256-sha256-1', 'slot' => 'active']], $listed); } public function testStageDoesNotChangeActiveSignerButPublishesNewJwk(): void { @@ -155,7 +155,7 @@ public function testRetireRemovesRetiringKeyFromJwks(): void { // listJwksKeys also drops the retired pool. $listed = $this->signatoryManager->listJwksKeys(); $this->assertCount(1, $listed); - $this->assertSame($staged->getKeyId(), $listed[0]['kid']); + $this->assertSame('ecdsa-p256-sha256-2', $listed[0]['kid']); $this->assertNotContains($first->getKeyId(), array_column($listed, 'kid')); } From 84d0656b8e0356081e992bc47fcd0d824c6cfe6e Mon Sep 17 00:00:00 2001 From: Micke Nordin Date: Tue, 28 Jul 2026 22:21:05 +0200 Subject: [PATCH 5/6] fix(ocm): treat RFC 9421 keyid as opaque, verify by sender origin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RFC 7517 §4.5 leaves the keyid structure unspecified, so the receiver must not parse it. The signer origin now comes from the trusted OCM share/sender identity; the JWK Set is resolved against that origin and the keyid is matched opaquely to the JWKS kid. Aligns with the OCM verification procedure and fixes the origin-mismatch / flaky-kid failures in the two-port integration rig. Reverts the per-request host-based kid back to a stable persisted kid, drops keyid->host parsing, threads a sender-origin parameter through verification, and resolves that origin in every OCM inbound entry point (notifications, shares, token exchange, OCM requests) and the federation rate limiter. Cavage is unchanged. Assisted-by: ClaudeCode:glm-5.2 Signed-off-by: Micke Nordin --- .../lib/Controller/OCMRequestController.php | 30 ++++--- .../Controller/RequestHandlerController.php | 38 ++++---- .../lib/Controller/TokenController.php | 32 ++++++- .../tests/Controller/TokenControllerTest.php | 4 + .../Http/Attributes/FederationRateLimit.php | 17 +++- .../CloudFederationProviderManager.php | 34 +++++++ lib/private/OCM/OCMDiscoveryService.php | 11 ++- lib/private/OCM/OCMSignatoryManager.php | 88 ++++++++++++++----- .../Model/Rfc9421IncomingSignedRequest.php | 37 +++----- .../Security/Signature/SignatureManager.php | 14 ++- .../ICloudFederationProviderManager.php | 9 ++ lib/public/OCM/IOCMDiscoveryService.php | 15 +++- .../Security/Signature/ISignatureManager.php | 8 +- .../OCM/OCMSignatoryManagerRotationTest.php | 4 +- .../Signature/Model/Rfc9421RoundTripTest.php | 9 +- .../SignatureManagerDispatchTest.php | 5 +- 16 files changed, 251 insertions(+), 104 deletions(-) diff --git a/apps/cloud_federation_api/lib/Controller/OCMRequestController.php b/apps/cloud_federation_api/lib/Controller/OCMRequestController.php index 0907756602d42..6615e0256e6c3 100644 --- a/apps/cloud_federation_api/lib/Controller/OCMRequestController.php +++ b/apps/cloud_federation_api/lib/Controller/OCMRequestController.php @@ -9,7 +9,6 @@ namespace OCA\CloudFederationAPI\Controller; -use JsonException; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\BruteForceProtection; @@ -18,6 +17,7 @@ use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\Response; use OCP\EventDispatcher\IEventDispatcher; +use OCP\Federation\ICloudFederationProviderManager; use OCP\IRequest; use OCP\OCM\Events\OCMEndpointRequestEvent; use OCP\OCM\Exceptions\OCMArgumentException; @@ -31,6 +31,7 @@ public function __construct( IRequest $request, private readonly IEventDispatcher $eventDispatcher, private readonly IOCMDiscoveryService $ocmDiscoveryService, + private readonly ICloudFederationProviderManager $cloudFederationProviderManager, private readonly LoggerInterface $logger, ) { parent::__construct($appName, $request); @@ -56,10 +57,22 @@ public function manageOCMRequests(string $ocmPath): Response { throw new OCMArgumentException('path is not UTF-8'); } + // Resolve the signer origin from the payload before verification. + $payload = $this->request->getParams(); + $origin = null; + if ($payload !== []) { + $identity = $this->cloudFederationProviderManager->resolveSenderIdentity($payload); + if ($identity !== null) { + try { + $origin = $this->ocmDiscoveryService->getHostFromOcmAddress($identity); + } catch (IncomingRequestException) { + // unresolvable origin; verification will fail without one + } + } + } + try { - // if request is signed and well signed, no exceptions are thrown - // if request is not signed and host is known for not supporting signed request, no exceptions are thrown - $signedRequest = $this->ocmDiscoveryService->getIncomingSignedRequest(); + $signedRequest = $this->ocmDiscoveryService->getIncomingSignedRequest($origin); } catch (IncomingRequestException $e) { $this->logger->warning('incoming ocm request exception', ['exception' => $e]); $response = new JSONResponse(['message' => $e->getMessage(), 'validationErrors' => []], Http::STATUS_BAD_REQUEST); @@ -67,15 +80,6 @@ public function manageOCMRequests(string $ocmPath): Response { return $response; } - // assuming that ocm request contains a json array - $payload = $signedRequest?->getBody() ?? file_get_contents('php://input'); - try { - $payload = ($payload) ? json_decode($payload, true, 512, JSON_THROW_ON_ERROR) : null; - } catch (JsonException $e) { - $this->logger->debug('json decode error', ['exception' => $e]); - $payload = null; - } - $event = new OCMEndpointRequestEvent( $this->request->getMethod(), preg_replace('@/+@', '/', $ocmPath), diff --git a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php index a855bb96206ca..f7a3a2c7e6d94 100644 --- a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php +++ b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php @@ -110,7 +110,8 @@ public function addShare($shareWith, $name, $description, $providerId, $owner, $ try { // if request is signed and well signed, no exceptions are thrown // if request is not signed and host is known for not supporting signed request, no exception are thrown - $signedRequest = $this->ocmDiscoveryService->getIncomingSignedRequest(); + $origin = $this->ocmDiscoveryService->getHostFromOcmAddress($owner); + $signedRequest = $this->ocmDiscoveryService->getIncomingSignedRequest($origin); $this->confirmSignedOrigin($signedRequest, 'owner', $owner); } catch (IncomingRequestException $e) { $this->logger->warning('incoming request exception', ['exception' => $e]); @@ -307,10 +308,15 @@ public function receiveNotification($notificationType, $resourceType, $providerI if (!$this->appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_DISABLED, lazy: true)) { try { - // if request is signed and well signed, no exception are thrown - // if request is not signed and host is known for not supporting signed request, no exception are thrown - $signedRequest = $this->ocmDiscoveryService->getIncomingSignedRequest(); - $this->confirmNotificationIdentity($signedRequest, $resourceType, $notification); + $identity = $this->resolveNotificationIdentity($resourceType, $notification); + $origin = null; + if ($identity !== '') { + $origin = $this->ocmDiscoveryService->getHostFromOcmAddress($identity); + } + $signedRequest = $this->ocmDiscoveryService->getIncomingSignedRequest($origin); + if ($identity !== '') { + $this->ocmDiscoveryService->confirmRequestOrigin($signedRequest?->getOrigin(), $identity); + } } catch (IncomingRequestException $e) { $this->logger->warning('incoming request exception', ['exception' => $e]); return new JSONResponse(['message' => $e->getMessage(), 'validationErrors' => []], Http::STATUS_BAD_REQUEST); @@ -450,22 +456,16 @@ private function confirmSignedOrigin(?IIncomingSignedRequest $signedRequest, str } /** - * confirm identity of the remote instance on notification, based on the share token. + * Resolve the sender identity from a notification's sharedSecret. + * Returns '' when the provider does not implement signed federation. * - * If request is not signed, we still verify that the hostname from the extracted value does, - * actually, not support signed request - * - * @param IIncomingSignedRequest|null $signedRequest * @param string $resourceType + * @param array $notification * * @throws IncomingRequestException * @throws BadRequestException */ - private function confirmNotificationIdentity( - ?IIncomingSignedRequest $signedRequest, - string $resourceType, - array $notification, - ): void { + private function resolveNotificationIdentity(string $resourceType, array $notification): string { $sharedSecret = $notification['sharedSecret'] ?? ''; if ($sharedSecret === '') { throw new BadRequestException(['sharedSecret']); @@ -481,14 +481,12 @@ private function confirmNotificationIdentity( $mapping = Server::get(OcmTokenMapMapper::class)->getByAccessTokenId($accessTokenDb->getId()); $identity = $provider->getFederationIdFromSharedSecret($mapping->getRefreshToken(), $notification); } - } else { - $this->logger->debug('cloud federation provider {provider} does not implements ISignedCloudFederationProvider', ['provider' => $provider::class]); - return; + return $identity; } + $this->logger->debug('cloud federation provider {provider} does not implement ISignedCloudFederationProvider', ['provider' => $provider::class]); } catch (\Exception $e) { throw new IncomingRequestException($e->getMessage(), previous: $e); } - - $this->ocmDiscoveryService->confirmRequestOrigin($signedRequest?->getOrigin(), $identity); + return ''; } } diff --git a/apps/cloud_federation_api/lib/Controller/TokenController.php b/apps/cloud_federation_api/lib/Controller/TokenController.php index d62281f898a1b..5ca49d96185a2 100644 --- a/apps/cloud_federation_api/lib/Controller/TokenController.php +++ b/apps/cloud_federation_api/lib/Controller/TokenController.php @@ -24,6 +24,7 @@ use OCP\Authentication\Token\IToken; use OCP\IAppConfig; use OCP\IRequest; +use OCP\OCM\IOCMDiscoveryService; use OCP\Security\ISecureRandom; use OCP\Security\Signature\Exceptions\IncomingRequestException; use OCP\Security\Signature\Exceptions\SignatoryNotFoundException; @@ -51,19 +52,44 @@ public function __construct( private readonly IAppConfig $appConfig, private readonly OcmTokenMapMapper $ocmTokenMapMapper, private readonly IShareManager $shareManager, + private readonly IOCMDiscoveryService $ocmDiscoveryService, ) { parent::__construct('cloud_federation_api', $request); } + /** + * Resolve the signer origin from the refresh token's share, or null. + * + * @param string $code refresh token + * @return string|null signer origin, or null if it cannot be determined + */ + private function resolveOriginFromRefreshToken(string $code): ?string { + if ($code === '') { + return null; + } + try { + $share = $this->shareManager->getShareByToken($code); + $sharedWith = $share->getSharedWith(); + if ($sharedWith === null || $sharedWith === '') { + return null; + } + return $this->ocmDiscoveryService->getHostFromOcmAddress($sharedWith); + } catch (\Throwable) { + return null; + } + } + /** * Verify the signature of incoming request if available * + * @param string|null $origin the origin of the request, or null if unknown + * * @return IIncomingSignedRequest|null null if remote does not support signed requests * @throws IncomingRequestException if signature is required but invalid */ - private function verifySignedRequest(): ?IIncomingSignedRequest { + private function verifySignedRequest(?string $origin): ?IIncomingSignedRequest { try { - $signedRequest = $this->signatureManager->getIncomingSignedRequest($this->signatoryManager); + $signedRequest = $this->signatureManager->getIncomingSignedRequest($this->signatoryManager, null, $origin); $this->logger->debug('Token request signature verified', [ 'origin' => $signedRequest->getOrigin() ]); @@ -126,7 +152,7 @@ private function resolveJwtSigningKey(string $privateKeyPem): array { #[FrontpageRoute(verb: 'POST', url: '/api/v1/access-token')] public function accessToken(string $grant_type = '', string $code = ''): DataResponse { try { - $signedRequest = $this->verifySignedRequest(); + $signedRequest = $this->verifySignedRequest($this->resolveOriginFromRefreshToken($code)); } catch (IncomingRequestException $e) { $this->logger->warning('Token request signature verification failed', [ 'exception' => $e diff --git a/apps/cloud_federation_api/tests/Controller/TokenControllerTest.php b/apps/cloud_federation_api/tests/Controller/TokenControllerTest.php index 2506c673882d2..0fe6572f7a2ba 100644 --- a/apps/cloud_federation_api/tests/Controller/TokenControllerTest.php +++ b/apps/cloud_federation_api/tests/Controller/TokenControllerTest.php @@ -23,6 +23,7 @@ use OCP\Authentication\Token\IToken; use OCP\IAppConfig; use OCP\IRequest; +use OCP\OCM\IOCMDiscoveryService; use OCP\Security\ISecureRandom; use OCP\Security\Signature\Exceptions\SignatoryNotFoundException; use OCP\Security\Signature\Exceptions\SignatureException; @@ -47,6 +48,7 @@ class TokenControllerTest extends TestCase { private IAppConfig&MockObject $appConfig; private OcmTokenMapMapper&MockObject $ocmTokenMapMapper; private IShareManager&MockObject $shareManager; + private IOCMDiscoveryService&MockObject $ocmDiscoveryService; private TokenController $controller; @@ -67,6 +69,7 @@ protected function setUp(): void { $this->appConfig = $this->createMock(IAppConfig::class); $this->ocmTokenMapMapper = $this->createMock(OcmTokenMapMapper::class); $this->shareManager = $this->createMock(IShareManager::class); + $this->ocmDiscoveryService = $this->createMock(IOCMDiscoveryService::class); $this->controller = new TokenController( $this->request, @@ -79,6 +82,7 @@ protected function setUp(): void { $this->appConfig, $this->ocmTokenMapMapper, $this->shareManager, + $this->ocmDiscoveryService, ); } diff --git a/lib/private/AppFramework/Http/Attributes/FederationRateLimit.php b/lib/private/AppFramework/Http/Attributes/FederationRateLimit.php index 98930adceca48..412d951b3abfa 100644 --- a/lib/private/AppFramework/Http/Attributes/FederationRateLimit.php +++ b/lib/private/AppFramework/Http/Attributes/FederationRateLimit.php @@ -13,6 +13,7 @@ use OC\OCM\OCMDiscoveryService; use OCA\Federation\TrustedServers; use OCP\AppFramework\Http\Attribute\AnonRateLimit; +use OCP\Federation\ICloudFederationProviderManager; use OCP\IRequest; use OCP\Server; @@ -24,12 +25,14 @@ */ #[Attribute(Attribute::TARGET_METHOD)] class FederationRateLimit extends AnonRateLimit { + private readonly ICloudFederationProviderManager $federationProviderManager; private readonly OCMDiscoveryService $discoveryService; private readonly ?TrustedServers $trustedServers; public function __construct(int $limit, int $period) { parent::__construct($limit, $period); + $this->federationProviderManager = Server::get(ICloudFederationProviderManager::class); $this->discoveryService = Server::get(OCMDiscoveryService::class); $this->trustedServers = Server::get(TrustedServers::class); } @@ -41,14 +44,22 @@ public function shouldApply(IRequest $request): bool { } try { - $signedRequest = $this->discoveryService->getIncomingSignedRequest(); + // Resolve the signer origin from the payload so trusted servers + // can be exempted. + $parsed = $request->getParams(); + $identity = $this->federationProviderManager->resolveSenderIdentity($parsed); + $origin = null; + if ($identity !== null) { + $origin = $this->discoveryService->getHostFromOcmAddress($identity); + } + + $signedRequest = $this->discoveryService->getIncomingSignedRequest($origin); if (!$signedRequest) { return true; } - $signedRequest->verify(); return !$this->trustedServers->isTrustedServer($signedRequest->getOrigin()); } catch (\Exception) { - // no or invalid signature + // no or invalid signature, or unresolvable origin return true; } } diff --git a/lib/private/Federation/CloudFederationProviderManager.php b/lib/private/Federation/CloudFederationProviderManager.php index a0bafa84c997f..3db68b9c16a9f 100644 --- a/lib/private/Federation/CloudFederationProviderManager.php +++ b/lib/private/Federation/CloudFederationProviderManager.php @@ -18,6 +18,7 @@ use OCP\Federation\ICloudFederationProviderManager; use OCP\Federation\ICloudFederationShare; use OCP\Federation\ICloudIdManager; +use OCP\Federation\ISignedCloudFederationProvider; use OCP\Http\Client\IClient; use OCP\Http\Client\IClientService; use OCP\Http\Client\IResponse; @@ -105,6 +106,39 @@ public function getCloudFederationProvider($resourceType) { } } + /** + * @inheritDoc + * + * Notifications resolve via sharedSecret; shares via owner/sender. + */ + #[\Override] + public function resolveSenderIdentity(array $body): ?string { + $resourceType = $body['resourceType'] ?? ''; + if ($resourceType !== '') { + $notification = $body['notification'] ?? null; + $sharedSecret = is_array($notification) ? ($notification['sharedSecret'] ?? '') : ''; + if ($sharedSecret !== '') { + try { + $provider = $this->getCloudFederationProvider($resourceType); + if ($provider instanceof ISignedCloudFederationProvider || $provider instanceof \NCU\Federation\ISignedCloudFederationProvider) { + $identity = $provider->getFederationIdFromSharedSecret($sharedSecret, is_array($notification) ? $notification : []); + if ($identity !== '') { + return $identity; + } + } + } catch (\Exception) { + // unresolved; fall through to share-style fields + } + } + } + foreach (['owner', 'sender', 'sharedBy'] as $field) { + if (isset($body[$field]) && is_string($body[$field]) && $body[$field] !== '') { + return $body[$field]; + } + } + return null; + } + /** * @deprecated 29.0.0 - Use {@see sendCloudShare()} instead and handle errors manually */ diff --git a/lib/private/OCM/OCMDiscoveryService.php b/lib/private/OCM/OCMDiscoveryService.php index f6c0a55109dcb..dbe741da55e7f 100644 --- a/lib/private/OCM/OCMDiscoveryService.php +++ b/lib/private/OCM/OCMDiscoveryService.php @@ -259,9 +259,9 @@ public function getLocalOCMProvider(bool $fullDetails = true): IOCMProvider { * @since 33.0.0 */ #[\Override] - public function getIncomingSignedRequest(): ?IIncomingSignedRequest { + public function getIncomingSignedRequest(?string $origin = null): ?IIncomingSignedRequest { try { - $signedRequest = $this->signatureManager->getIncomingSignedRequest($this->signatoryManager); + $signedRequest = $this->signatureManager->getIncomingSignedRequest($this->signatoryManager, null, $origin); $this->logger->debug('signed request available', ['signedRequest' => $signedRequest]); return $signedRequest; } catch (SignatureNotFoundException|SignatoryNotFoundException $e) { @@ -310,9 +310,14 @@ public function confirmRequestOrigin(?string $signedOrigin, string $ocmAddress): } /** + * Extract the signer origin (host) from an OCM address (`user@host`). + * + * @param string $entry OCM address in `user@host` or `user@https://host` form + * @return string the host (with port) of the OCM address * @throws IncomingRequestException on malformed address or unresolvable host */ - private function getHostFromOcmAddress(string $entry): string { + #[\Override] + public function getHostFromOcmAddress(string $entry): string { try { $cloudId = $this->cloudIdManager->resolveCloudId(trim($entry, '@')); return $this->signatureManager->extractIdentityFromUri($cloudId->getRemote()); diff --git a/lib/private/OCM/OCMSignatoryManager.php b/lib/private/OCM/OCMSignatoryManager.php index 63b48eb7847ae..47a7df1e554b0 100644 --- a/lib/private/OCM/OCMSignatoryManager.php +++ b/lib/private/OCM/OCMSignatoryManager.php @@ -57,6 +57,8 @@ class OCMSignatoryManager implements IJwkResolvingSignatoryManager { private const APPKEY_JWKS_POOL_PREFIX = 'ocm_jwks_pool_'; private const APPCONFIG_JWKS_POOL_COUNTER = 'ocm_jwks_pool_counter'; private const APPCONFIG_JWKS_POOL_KID_PREFIX = 'ocm_jwks_pool_kid_'; + /** Stable kid identity portion, reused across rotations so kids stay on one hostname. */ + private const APPCONFIG_JWKS_KID_BASE = 'ocm_jwks_kid_base'; public const SLOT_ACTIVE = 'active'; public const SLOT_PENDING = 'pending'; public const SLOT_RETIRING = 'retiring'; @@ -260,7 +262,9 @@ public function listJwksKeys(): array { } $entries[] = [ 'poolId' => $id, - 'kid' => $this->appConfig->getValueString('core', self::APPCONFIG_JWKS_POOL_KID_PREFIX . $id, ''), + 'kid' => $this->canonicalKid( + $this->appConfig->getValueString('core', self::APPCONFIG_JWKS_POOL_KID_PREFIX . $id, ''), + ), 'slot' => $bySlot[$id] ?? null, ]; } @@ -268,24 +272,72 @@ public function listJwksKeys(): array { } /** - * Generate keypair into a new pool. Only the opaque key id is persisted; - * the host is derived fresh per request at use time, so a key provisioned - * under one URL still verifies when the instance is addressed by another - * (e.g. the two-port integration rig, or a port change behind a proxy). + * Generate keypair into a new pool. Kid is canonicalised through + * {@see Signatory::setKeyId} so admin output and wire form agree. */ - private function generatePool(string $opaqueKeyId): int { + private function generatePool(string $kid): int { $poolId = $this->appConfig->getValueInt('core', self::APPCONFIG_JWKS_POOL_COUNTER, 0) + 1; $this->appConfig->setValueInt('core', self::APPCONFIG_JWKS_POOL_COUNTER, $poolId); $this->identityProofManager->generateEcdsaP256AppKey('core', self::APPKEY_JWKS_POOL_PREFIX . $poolId); - $this->appConfig->setValueString('core', self::APPCONFIG_JWKS_POOL_KID_PREFIX . $poolId, $opaqueKeyId); + $this->appConfig->setValueString('core', self::APPCONFIG_JWKS_POOL_KID_PREFIX . $poolId, $this->canonicalKid($kid)); return $poolId; } - /** Next opaque key id (`-`); the host is added per request. */ + /** Canonical wire-form via a transient {@see Signatory::setKeyId} round-trip. */ + private function canonicalKid(string $kid): string { + $probe = new Signatory(true); + $probe->setKeyId($kid); + return $probe->getKeyId(); + } + + /** + * Build the next kid. Identity portion is derived once and persisted so + * CLI-triggered rotations stay on the same hostname. + * + * @throws \RuntimeException if no instance identity can be derived + */ private function nextPoolKid(): string { + $base = $this->resolveKidBase(); $next = $this->appConfig->getValueInt('core', self::APPCONFIG_JWKS_POOL_COUNTER, 0) + 1; - return self::KEYID_FRAGMENT_JWKS . '-' . $next; + return $base . '-' . $next; + } + + /** + * Stable identity portion (before the `-N` suffix). Resolution order: + * stored APPCONFIG_JWKS_KID_BASE > active pool's kid sans suffix > + * fresh from {@see buildLocalKeyId}. Persisted so CLI rotations stay + * on one hostname. + * + * @throws \RuntimeException if no instance identity can be derived + */ + private function resolveKidBase(): string { + $base = $this->appConfig->getValueString('core', self::APPCONFIG_JWKS_KID_BASE, ''); + if ($base !== '') { + return $base; + } + + $activePool = $this->getSlotPool(self::SLOT_ACTIVE); + if ($activePool !== null) { + $kid = $this->canonicalKid( + $this->appConfig->getValueString('core', self::APPCONFIG_JWKS_POOL_KID_PREFIX . $activePool, ''), + ); + $pos = strrpos($kid, '-'); + if ($pos !== false) { + $base = substr($kid, 0, $pos); + } + } + + if ($base === '') { + try { + $base = $this->canonicalKid($this->buildLocalKeyId(self::KEYID_FRAGMENT_JWKS)); + } catch (IdentityNotFoundException $e) { + throw new \RuntimeException('cannot derive instance identity for JWKS kid', 0, $e); + } + } + + $this->appConfig->setValueString('core', self::APPCONFIG_JWKS_KID_BASE, $base); + return $base; } private function getSlotPool(string $slot): ?int { @@ -305,29 +357,19 @@ private function clearSlot(string $slot): void { $this->appConfig->deleteKey('core', 'ocm_jwks_slot_' . $slot); } - /** - * Returns null if the underlying appkey was manually deleted. The keyId - * is rebuilt per call from the opaque id and the current request URL, so - * it tracks the host the instance is actually addressed as. - * - * @throws \RuntimeException if no instance identity can be derived - */ + /** Returns null if the underlying appkey was manually deleted. */ private function signatoryFromPool(int $poolId): ?Signatory { $appKey = self::APPKEY_JWKS_POOL_PREFIX . $poolId; if (!$this->identityProofManager->hasAppKey('core', $appKey)) { return null; } - $opaqueKeyId = $this->appConfig->getValueString('core', self::APPCONFIG_JWKS_POOL_KID_PREFIX . $poolId, ''); - if ($opaqueKeyId === '') { + $kid = $this->appConfig->getValueString('core', self::APPCONFIG_JWKS_POOL_KID_PREFIX . $poolId, ''); + if ($kid === '') { return null; } $keyPair = $this->identityProofManager->getAppKey('core', $appKey); $signatory = new Signatory(true); - try { - $signatory->setKeyId($this->buildLocalKeyId($opaqueKeyId)); - } catch (IdentityNotFoundException $e) { - throw new \RuntimeException('cannot derive instance identity for JWKS kid', 0, $e); - } + $signatory->setKeyId($kid); $signatory->setPublicKey($keyPair->getPublic()); $signatory->setPrivateKey($keyPair->getPrivate()); return $signatory; diff --git a/lib/private/Security/Signature/Model/Rfc9421IncomingSignedRequest.php b/lib/private/Security/Signature/Model/Rfc9421IncomingSignedRequest.php index 00aa3f04e5ace..509abc4157298 100644 --- a/lib/private/Security/Signature/Model/Rfc9421IncomingSignedRequest.php +++ b/lib/private/Security/Signature/Model/Rfc9421IncomingSignedRequest.php @@ -23,14 +23,12 @@ use OC\Security\Signature\Rfc9421\SignatureBase; use OC\Security\Signature\SignatureManager; use OCP\IRequest; -use OCP\Security\Signature\Exceptions\IdentityNotFoundException; use OCP\Security\Signature\Exceptions\IncomingRequestException; use OCP\Security\Signature\Exceptions\InvalidSignatureException; use OCP\Security\Signature\Exceptions\SignatoryNotFoundException; use OCP\Security\Signature\Exceptions\SignatureException; use OCP\Security\Signature\Exceptions\SignatureNotFoundException; use OCP\Security\Signature\IIncomingSignedRequest; -use OCP\Security\Signature\Model\Signatory; /** * RFC 9421 implementation of {@see IIncomingSignedRequest}. Parses the @@ -139,15 +137,7 @@ public function __construct( if (!is_string($keyId) || $keyId === '') { throw new IncomingRequestException('missing keyid in Signature-Input'); } - try { - $this->origin = Signatory::extractIdentityFromUri($keyId); - } catch (IdentityNotFoundException) { - // keyid is not a URL; the OCM convention (and the examples in - // the spec) use `[:port]#`, in which case the origin - // is the host part before the '#'. If neither form applies the - // origin stays empty and getOrigin() rejects the request. - $this->origin = self::extractHostFromKeyId($keyId); - } + // keyid is opaque; the signer origin is set by the caller via setOrigin(). $paramsLine = SignatureBase::serializeSignatureParams($this->components, $this->signatureParams); $this->signatureBaseString = SignatureBase::build( @@ -184,6 +174,15 @@ public function getOrigin(): string { return $this->origin; } + /** + * Signer origin, established by the caller from the share/sender identity. + * + * @param string $origin + */ + public function setOrigin(string $origin): void { + $this->origin = $origin; + } + #[\Override] public function getKeyId(): string { return $this->getSigningElement('keyId'); @@ -312,22 +311,6 @@ private function reconstructTargetUri(): string { return $scheme . '://' . $host . $path; } - /** - * Derive the signer's origin from the OCM `[:port]#` keyid - * convention, e.g. `sender.example.org#key1`, by parsing the keyid as a - * scheme-less authority. Returns '' when no host can be extracted. - */ - private static function extractHostFromKeyId(string $keyId): string { - if (!str_contains($keyId, '#')) { - return ''; - } - try { - return Signatory::extractIdentityFromUri('https://' . $keyId); - } catch (IdentityNotFoundException) { - return ''; - } - } - /** * Collect the HTTP request fields covered by the signature, keyed by their * lowercased name. Derived components (`@*`) are produced inside diff --git a/lib/private/Security/Signature/SignatureManager.php b/lib/private/Security/Signature/SignatureManager.php index 07d855fe01d15..efdf30e7b186f 100644 --- a/lib/private/Security/Signature/SignatureManager.php +++ b/lib/private/Security/Signature/SignatureManager.php @@ -97,6 +97,7 @@ public function __construct( public function getIncomingSignedRequest( ISignatoryManager $signatoryManager, ?string $body = null, + ?string $origin = null, ): IIncomingSignedRequest { $body = $body ?? file_get_contents('php://input'); $options = $signatoryManager->getOptions(); @@ -106,7 +107,7 @@ public function getIncomingSignedRequest( // `Signature-Input` is unique to RFC 9421; cavage uses `Signature` only. if ($this->request->getHeader('Signature-Input') !== '') { - return $this->getRfc9421IncomingSignedRequest($signatoryManager, $body, $options); + return $this->getRfc9421IncomingSignedRequest($signatoryManager, $body, $options, $origin); } // generate IncomingSignedRequest based on body and request @@ -132,6 +133,11 @@ public function getIncomingSignedRequest( /** * RFC 9421 inbound path. Requires {@see IJwkResolvingSignatoryManager}. * + * @param ISignatoryManager $signatoryManager + * @param string $body request body + * @param array $options signatory manager options + * @param string|null $origin signer origin from the caller (the keyid is opaque) + * * @throws IncomingRequestException * @throws SignatureException * @throws SignatureNotFoundException @@ -140,12 +146,18 @@ private function getRfc9421IncomingSignedRequest( ISignatoryManager $signatoryManager, string $body, array $options, + ?string $origin, ): IIncomingSignedRequest { if (!($signatoryManager instanceof IJwkResolvingSignatoryManager)) { throw new IncomingRequestException('RFC 9421 inbound is not supported by ' . get_class($signatoryManager)); } + if ($origin === null || $origin === '') { + // The keyid is opaque; the caller must supply the signer origin. + throw new IncomingRequestException('RFC 9421 verification requires the sender origin'); + } $signedRequest = new Rfc9421IncomingSignedRequest($body, $this->request, $options); + $signedRequest->setOrigin($origin); try { $key = $signatoryManager->getRemoteKey($signedRequest->getOrigin(), $signedRequest->getKeyId()); diff --git a/lib/public/Federation/ICloudFederationProviderManager.php b/lib/public/Federation/ICloudFederationProviderManager.php index 815808ffa4189..2bf2ba1ac738a 100644 --- a/lib/public/Federation/ICloudFederationProviderManager.php +++ b/lib/public/Federation/ICloudFederationProviderManager.php @@ -60,6 +60,15 @@ public function getAllCloudFederationProviders(); */ public function getCloudFederationProvider($resourceType); + /** + * Resolve the sender's federated identity from an OCM request body. + * + * @param array $body decoded OCM request body + * @return string|null federated cloud id (`user@host`), or null + * @since 35.0.0 + */ + public function resolveSenderIdentity(array $body): ?string; + /** * send federated share * diff --git a/lib/public/OCM/IOCMDiscoveryService.php b/lib/public/OCM/IOCMDiscoveryService.php index 982dc0abc91b8..68855c59b0652 100644 --- a/lib/public/OCM/IOCMDiscoveryService.php +++ b/lib/public/OCM/IOCMDiscoveryService.php @@ -59,11 +59,14 @@ public function getLocalOCMProvider(bool $fullDetails = true): IOCMProvider; * - if request is signed, but wrongly signed * - if request is not signed but instance is configured to only accept signed ocm request * + * @param string|null $origin for RFC 9421, the signer origin from the caller + * (the keyid is opaque) + * * @return IIncomingSignedRequest|null null if remote does not (and never did) support signed request * @throws IncomingRequestException * @since 33.0.0 */ - public function getIncomingSignedRequest(): ?IIncomingSignedRequest; + public function getIncomingSignedRequest(?string $origin = null): ?IIncomingSignedRequest; /** * Confirm that the host portion of $ocmAddress matches $signedOrigin @@ -80,6 +83,16 @@ public function getIncomingSignedRequest(): ?IIncomingSignedRequest; */ public function confirmRequestOrigin(?string $signedOrigin, string $ocmAddress): void; + /** + * Extract the signer origin (host) from an OCM address (`user@host`). + * + * @param string $entry OCM address in `user@host` or `user@https://host` form + * @return string the host (with port) of the OCM address + * @throws IncomingRequestException on malformed address or unresolvable host + * @since 35.0.0 + */ + public function getHostFromOcmAddress(string $entry): string; + /** * Request a remote OCM endpoint. * diff --git a/lib/public/Security/Signature/ISignatureManager.php b/lib/public/Security/Signature/ISignatureManager.php index e4246d2f82089..ddc7be4778795 100644 --- a/lib/public/Security/Signature/ISignatureManager.php +++ b/lib/public/Security/Signature/ISignatureManager.php @@ -66,6 +66,8 @@ interface ISignatureManager { * * @param ISignatoryManager $signatoryManager used to get details about remote instance * @param string|null $body if NULL, body will be extracted from php://input + * @param string|null $origin for RFC 9421, the signer origin from the caller + * (the keyid is opaque) * * @return IIncomingSignedRequest * @throws IncomingRequestException if anything looks wrong with the incoming request @@ -73,7 +75,11 @@ interface ISignatureManager { * @throws SignatureException if signature could not be confirmed * @since 33.0.0 */ - public function getIncomingSignedRequest(ISignatoryManager $signatoryManager, ?string $body = null): IIncomingSignedRequest; + public function getIncomingSignedRequest( + ISignatoryManager $signatoryManager, + ?string $body = null, + ?string $origin = null, + ): IIncomingSignedRequest; /** * Preparing signature (and headers) to sign an outgoing request. diff --git a/tests/lib/OCM/OCMSignatoryManagerRotationTest.php b/tests/lib/OCM/OCMSignatoryManagerRotationTest.php index a4a1d073d302e..bc3a7f2530c54 100644 --- a/tests/lib/OCM/OCMSignatoryManagerRotationTest.php +++ b/tests/lib/OCM/OCMSignatoryManagerRotationTest.php @@ -86,7 +86,7 @@ public function testFirstCallProvisionsActiveKey(): void { $this->assertSame($signatory->getKeyId(), $jwks[0]['kid']); $listed = $this->signatoryManager->listJwksKeys(); - $this->assertSame([['poolId' => 1, 'kid' => 'ecdsa-p256-sha256-1', 'slot' => 'active']], $listed); + $this->assertSame([['poolId' => 1, 'kid' => $signatory->getKeyId(), 'slot' => 'active']], $listed); } public function testStageDoesNotChangeActiveSignerButPublishesNewJwk(): void { @@ -155,7 +155,7 @@ public function testRetireRemovesRetiringKeyFromJwks(): void { // listJwksKeys also drops the retired pool. $listed = $this->signatoryManager->listJwksKeys(); $this->assertCount(1, $listed); - $this->assertSame('ecdsa-p256-sha256-2', $listed[0]['kid']); + $this->assertSame($staged->getKeyId(), $listed[0]['kid']); $this->assertNotContains($first->getKeyId(), array_column($listed, 'kid')); } diff --git a/tests/lib/Security/Signature/Model/Rfc9421RoundTripTest.php b/tests/lib/Security/Signature/Model/Rfc9421RoundTripTest.php index 7c32c7dce3684..4639c1145b8d3 100644 --- a/tests/lib/Security/Signature/Model/Rfc9421RoundTripTest.php +++ b/tests/lib/Security/Signature/Model/Rfc9421RoundTripTest.php @@ -335,11 +335,8 @@ public function testSignatureNotCoveringRequiredComponentsRejected(): void { new Rfc9421IncomingSignedRequest($body, $req); } - public function testHostFragmentKeyIdYieldsOrigin(): void { - // the spec's canonical keyid form is `#`, not a URL; the - // origin used for JWKS resolution is the host before the '#'. - // Nextcloud's own Signatory model rejects such kids, so build the - // peer's request manually. + public function testKeyIdIsOpaqueAndOriginIsExternal(): void { + // keyid is opaque; the origin is supplied by the caller via setOrigin(). $kid = 'sender.example.org#key1'; [$privatePem, $jwk] = $this->ecdsaP256Jwk($kid); @@ -363,6 +360,8 @@ public function testHostFragmentKeyIdYieldsOrigin(): void { $req = $this->mockRequest($headers, 'POST', '/ocm/shares', 'receiver.example.org'); $in = new Rfc9421IncomingSignedRequest($body, $req); + // The keyid is not parsed; the origin comes from the caller. + $in->setOrigin('sender.example.org'); $this->assertSame('sender.example.org', $in->getOrigin()); $in->setKey($jwk); $in->verify(); diff --git a/tests/lib/Security/Signature/SignatureManagerDispatchTest.php b/tests/lib/Security/Signature/SignatureManagerDispatchTest.php index cbe776933c3ad..cc079272ccda4 100644 --- a/tests/lib/Security/Signature/SignatureManagerDispatchTest.php +++ b/tests/lib/Security/Signature/SignatureManagerDispatchTest.php @@ -100,7 +100,8 @@ public function testInboundDispatchesToRfc9421WhenSignatureInputPresent(): void $resolver = $this->makeKeyResolver($signatoryManager, $jwk, 'https://sender.example.org/ocm#ecdsa-p256-sha256'); - $signed = $this->signatureManager->getIncomingSignedRequest($resolver, $body); + // RFC 9421 verification needs the sender origin from the caller. + $signed = $this->signatureManager->getIncomingSignedRequest($resolver, $body, 'sender.example.org'); $this->assertInstanceOf(Rfc9421IncomingSignedRequest::class, $signed); } @@ -143,7 +144,7 @@ public function testInboundRejectsRfc9421WhenNoKeyResolvedForKeyid(): void { $resolver = $this->makeKeyResolver($signatoryManager, $jwk, 'https://other.example.org/ocm#nomatch'); $this->expectException(IncomingRequestException::class); - $this->signatureManager->getIncomingSignedRequest($resolver, $body); + $this->signatureManager->getIncomingSignedRequest($resolver, $body, 'sender.example.org'); } private function rsaSignatoryManager(): ISignatoryManager { From 3ae67bd1a4bd20ad8c922af2265b49e1e4444926 Mon Sep 17 00:00:00 2001 From: Micke Nordin Date: Wed, 29 Jul 2026 00:54:46 +0200 Subject: [PATCH 6/6] chore(cloud_federation_api): move jwks Move from .well-known/jwks.json to the TokenController Signed-off-by: Micke Nordin --- apps/cloud_federation_api/appinfo/routes.php | 5 + .../lib/Controller/TokenController.php | 22 ++++ apps/cloud_federation_api/openapi.json | 8 +- core/AppInfo/Application.php | 2 - lib/composer/composer/autoload_classmap.php | 1 - lib/composer/composer/autoload_static.php | 1 - lib/private/OCM/OCMJwksHandler.php | 53 -------- lib/private/OCM/OCMSignatoryManager.php | 9 +- .../Model/Rfc9421IncomingSignedRequest.php | 4 +- .../Model/Rfc9421OutgoingSignedRequest.php | 8 +- openapi.json | 8 +- tests/lib/OCM/DiscoveryServiceTest.php | 2 +- tests/lib/OCM/OCMJwksHandlerTest.php | 118 ------------------ tests/lib/OCM/OCMSignatoryManagerJwksTest.php | 19 +-- 14 files changed, 45 insertions(+), 215 deletions(-) delete mode 100644 lib/private/OCM/OCMJwksHandler.php delete mode 100644 tests/lib/OCM/OCMJwksHandlerTest.php diff --git a/apps/cloud_federation_api/appinfo/routes.php b/apps/cloud_federation_api/appinfo/routes.php index 9dcffd0aa3489..189eb8c86c465 100644 --- a/apps/cloud_federation_api/appinfo/routes.php +++ b/apps/cloud_federation_api/appinfo/routes.php @@ -8,6 +8,11 @@ */ return [ 'routes' => [ + [ + 'name' => 'Token#jwks', + 'url' => '/api/v1/jwks', + 'verb' => 'GET', + ], [ 'name' => 'RequestHandler#addShare', 'url' => '/shares', diff --git a/apps/cloud_federation_api/lib/Controller/TokenController.php b/apps/cloud_federation_api/lib/Controller/TokenController.php index 5ca49d96185a2..31373c70b47f3 100644 --- a/apps/cloud_federation_api/lib/Controller/TokenController.php +++ b/apps/cloud_federation_api/lib/Controller/TokenController.php @@ -18,6 +18,7 @@ use OCP\AppFramework\Http\Attribute\NoCSRFRequired; use OCP\AppFramework\Http\Attribute\PublicPage; use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Authentication\Exceptions\ExpiredTokenException; use OCP\Authentication\Exceptions\InvalidTokenException; @@ -135,6 +136,27 @@ private function resolveJwtSigningKey(string $privateKeyPem): array { throw new \RuntimeException('Unsupported signatory key type for JWT access token'); } + /** + * Serve the local JWK Set + * + * @return JSONResponse>}, array{}> + * + * 200: JWK Set returned + */ + #[PublicPage] + #[NoCSRFRequired] + public function jwks(): JSONResponse { + $keys = []; + try { + foreach ($this->signatoryManager->getLocalJwks() as $jwk) { + $keys[] = $jwk; + } + } catch (\Throwable $e) { + $this->logger->warning('failed to build local JWKs', ['exception' => $e]); + } + return new JSONResponse(['keys' => $keys]); + } + /** * Exchange a refresh token for a short-lived access token * diff --git a/apps/cloud_federation_api/openapi.json b/apps/cloud_federation_api/openapi.json index 1f4a4e3a050c4..fffbefe54162b 100644 --- a/apps/cloud_federation_api/openapi.json +++ b/apps/cloud_federation_api/openapi.json @@ -323,13 +323,13 @@ } }, "tags": [ - { - "name": "request_handler", - "description": "Open-Cloud-Mesh-API" - }, { "name": "token", "description": "Controller for the /token endpoint Exchanges long-lived refresh tokens for short-lived access tokens" + }, + { + "name": "request_handler", + "description": "Open-Cloud-Mesh-API" } ] } diff --git a/core/AppInfo/Application.php b/core/AppInfo/Application.php index 51425946a2536..52986a21f6471 100644 --- a/core/AppInfo/Application.php +++ b/core/AppInfo/Application.php @@ -38,7 +38,6 @@ use OC\Core\Sharing\Recipient\TokenShareRecipientType; use OC\Core\Sharing\Recipient\UserShareRecipientType; use OC\OCM\OCMDiscoveryHandler; -use OC\OCM\OCMJwksHandler; use OC\TagManager; use OCP\AppFramework\App; use OCP\AppFramework\Bootstrap\IBootContext; @@ -110,7 +109,6 @@ public function register(IRegistrationContext $context): void { $context->registerConfigLexicon(ConfigLexicon::class); $context->registerWellKnownHandler(OCMDiscoveryHandler::class); - $context->registerWellKnownHandler(OCMJwksHandler::class); $context->registerCapability(Capabilities::class); $context->registerEventListener(RestrictInteractionEvent::class, RestrictInteractionListener::class); diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index beafb4700c69d..4f4de0ee036da 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -2047,7 +2047,6 @@ 'OC\\OCM\\Model\\OCMResource' => $baseDir . '/lib/private/OCM/Model/OCMResource.php', 'OC\\OCM\\OCMDiscoveryHandler' => $baseDir . '/lib/private/OCM/OCMDiscoveryHandler.php', 'OC\\OCM\\OCMDiscoveryService' => $baseDir . '/lib/private/OCM/OCMDiscoveryService.php', - 'OC\\OCM\\OCMJwksHandler' => $baseDir . '/lib/private/OCM/OCMJwksHandler.php', 'OC\\OCM\\OCMSignatoryManager' => $baseDir . '/lib/private/OCM/OCMSignatoryManager.php', 'OC\\OCM\\Rfc9421SignatoryManager' => $baseDir . '/lib/private/OCM/Rfc9421SignatoryManager.php', 'OC\\OCS\\ApiHelper' => $baseDir . '/lib/private/OCS/ApiHelper.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index e024c1fb44b9f..0c02270a66a13 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -2088,7 +2088,6 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\OCM\\Model\\OCMResource' => __DIR__ . '/../../..' . '/lib/private/OCM/Model/OCMResource.php', 'OC\\OCM\\OCMDiscoveryHandler' => __DIR__ . '/../../..' . '/lib/private/OCM/OCMDiscoveryHandler.php', 'OC\\OCM\\OCMDiscoveryService' => __DIR__ . '/../../..' . '/lib/private/OCM/OCMDiscoveryService.php', - 'OC\\OCM\\OCMJwksHandler' => __DIR__ . '/../../..' . '/lib/private/OCM/OCMJwksHandler.php', 'OC\\OCM\\OCMSignatoryManager' => __DIR__ . '/../../..' . '/lib/private/OCM/OCMSignatoryManager.php', 'OC\\OCM\\Rfc9421SignatoryManager' => __DIR__ . '/../../..' . '/lib/private/OCM/Rfc9421SignatoryManager.php', 'OC\\OCS\\ApiHelper' => __DIR__ . '/../../..' . '/lib/private/OCS/ApiHelper.php', diff --git a/lib/private/OCM/OCMJwksHandler.php b/lib/private/OCM/OCMJwksHandler.php deleted file mode 100644 index 92dca5985b38f..0000000000000 --- a/lib/private/OCM/OCMJwksHandler.php +++ /dev/null @@ -1,53 +0,0 @@ -appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_DISABLED, lazy: true)) { - try { - foreach ($this->signatoryManager->getLocalJwks() as $jwk) { - $keys[] = $jwk; - } - } catch (Throwable $e) { - $this->logger->warning('failed to build local JWKs', ['exception' => $e]); - } - } - - return new GenericResponse(new JSONResponse(['keys' => $keys])); - } -} diff --git a/lib/private/OCM/OCMSignatoryManager.php b/lib/private/OCM/OCMSignatoryManager.php index 47a7df1e554b0..433dac293e0a4 100644 --- a/lib/private/OCM/OCMSignatoryManager.php +++ b/lib/private/OCM/OCMSignatoryManager.php @@ -375,14 +375,9 @@ private function signatoryFromPool(int $poolId): ?Signatory { return $signatory; } - /** - * Absolute URL of the local JWK Set, advertised as `jwksUri` in the - * discovery response. - * - * @throws IdentityNotFoundException - */ + /** Absolute URL of the local JWK Set, advertised as `jwksUri`. */ public function getLocalJwksUri(): string { - return $this->buildLocalUrl('/.well-known/jwks.json'); + return $this->urlGenerator->linkToRouteAbsolute('cloud_federation_api.Token.jwks'); } /** diff --git a/lib/private/Security/Signature/Model/Rfc9421IncomingSignedRequest.php b/lib/private/Security/Signature/Model/Rfc9421IncomingSignedRequest.php index 509abc4157298..89649d3bbe3a1 100644 --- a/lib/private/Security/Signature/Model/Rfc9421IncomingSignedRequest.php +++ b/lib/private/Security/Signature/Model/Rfc9421IncomingSignedRequest.php @@ -47,7 +47,7 @@ class Rfc9421IncomingSignedRequest extends SignedRequest implements * The `Date` header is deliberately not part of the required set: * freshness is anchored on the `created` signature parameter. */ - private const DEFAULT_REQUIRED_COMPONENTS = [ + public const REQUIRED_COMPONENTS = [ '@method', '@target-uri', 'content-digest', @@ -244,7 +244,7 @@ public function verify(): void { /** @throws IncomingRequestException if the signature doesn't cover the OCM-required components */ private function verifyRequiredComponents(): void { /** @var list $required */ - $required = $this->options['rfc9421.requiredComponents'] ?? self::DEFAULT_REQUIRED_COMPONENTS; + $required = $this->options['rfc9421.requiredComponents'] ?? self::REQUIRED_COMPONENTS; $missing = array_values(array_diff($required, $this->components)); if ($missing !== []) { throw new IncomingRequestException( diff --git a/lib/private/Security/Signature/Model/Rfc9421OutgoingSignedRequest.php b/lib/private/Security/Signature/Model/Rfc9421OutgoingSignedRequest.php index 02f2c3082a020..812d24213dff7 100644 --- a/lib/private/Security/Signature/Model/Rfc9421OutgoingSignedRequest.php +++ b/lib/private/Security/Signature/Model/Rfc9421OutgoingSignedRequest.php @@ -35,12 +35,6 @@ class Rfc9421OutgoingSignedRequest extends SignedRequest implements IOutgoingSignedRequest, JsonSerializable { - /** - * Covered components mandated by the OCM spec. The `Date` header is - * deliberately not covered: intermediaries may rewrite it, and freshness - * is anchored on the `created` signature parameter. - */ - private const DEFAULT_COMPONENTS = ['@method', '@target-uri', 'content-digest', 'content-length']; private string $host = ''; private array $headers = []; @@ -70,7 +64,7 @@ public function __construct( $this->signingAlgorithm = (string)($options['rfc9421.signingAlgorithm'] ?? 'ecdsa-p256-sha256'); $contentDigestAlgorithm = (string)($options['rfc9421.contentDigestAlgorithm'] ?? ContentDigest::ALGO_SHA256); /** @var list $components */ - $components = $options['rfc9421.coveredComponents'] ?? self::DEFAULT_COMPONENTS; + $components = $options['rfc9421.coveredComponents'] ?? Rfc9421IncomingSignedRequest::REQUIRED_COMPONENTS; $includeAlg = (bool)($options['rfc9421.includeAlgParameter'] ?? false); $dateHeaderFormat = (string)($options['dateHeader'] ?? SignatureManager::DATE_HEADER); diff --git a/openapi.json b/openapi.json index 25ef87591e40b..48cfc9a7925b6 100644 --- a/openapi.json +++ b/openapi.json @@ -37,14 +37,14 @@ "name": "core/open_metrics", "description": "OpenMetrics controller Gather and display metrics" }, - { - "name": "cloud_federation_api/request_handler", - "description": "Open-Cloud-Mesh-API" - }, { "name": "cloud_federation_api/token", "description": "Controller for the /token endpoint Exchanges long-lived refresh tokens for short-lived access tokens" }, + { + "name": "cloud_federation_api/request_handler", + "description": "Open-Cloud-Mesh-API" + }, { "name": "federatedfilesharing/mount_public_link", "description": "Class MountPublicLinkController convert public links to federated shares" diff --git a/tests/lib/OCM/DiscoveryServiceTest.php b/tests/lib/OCM/DiscoveryServiceTest.php index 128e3f36af381..e1d41e565fb43 100644 --- a/tests/lib/OCM/DiscoveryServiceTest.php +++ b/tests/lib/OCM/DiscoveryServiceTest.php @@ -144,7 +144,7 @@ public function testLocalDiscoveryAdvertisesJwksUri(): void { $baseUrl = Server::get(IURLGenerator::class)->getBaseUrl(); $expectedScheme = str_starts_with($baseUrl, 'http://') ? 'http://' : 'https://'; $this->assertStringStartsWith($expectedScheme, $jwksUri); - $this->assertStringEndsWith('/.well-known/jwks.json', $jwksUri); + $this->assertStringContainsString('cloud_federation_api/api/v1/jwks', $jwksUri); } public function testLocalAddedCapability(): void { diff --git a/tests/lib/OCM/OCMJwksHandlerTest.php b/tests/lib/OCM/OCMJwksHandlerTest.php deleted file mode 100644 index f7270298dee09..0000000000000 --- a/tests/lib/OCM/OCMJwksHandlerTest.php +++ /dev/null @@ -1,118 +0,0 @@ -appConfig = $this->createMock(IAppConfig::class); - $this->signatoryManager = $this->createMock(OCMSignatoryManager::class); - $this->logger = $this->createMock(LoggerInterface::class); - $this->context = $this->createMock(IRequestContext::class); - - $this->handler = new OCMJwksHandler( - $this->appConfig, - $this->signatoryManager, - $this->logger, - ); - } - - public function testIgnoresUnrelatedService(): void { - $previous = new JrdResponse('foo'); - $result = $this->handler->handle('webfinger', $this->context, $previous); - $this->assertSame($previous, $result); - } - - public function testEmptyKeySetWhenSigningDisabled(): void { - $this->appConfig->method('getValueBool') - ->with('core', OCMSignatoryManager::APPCONFIG_SIGN_DISABLED, false, true) - ->willReturn(true); - $this->signatoryManager->expects($this->never())->method('getLocalJwks'); - - $body = $this->jsonBody($this->handler->handle('jwks.json', $this->context, null)); - $this->assertSame(['keys' => []], $body); - } - - public function testPublishesJwksWhenAvailable(): void { - $this->appConfig->method('getValueBool')->willReturn(false); - $jwk = [ - 'kty' => 'EC', - 'crv' => 'P-256', - 'kid' => 'https://example.org/ocm#ecdsa-p256-sha256', - 'alg' => 'ES256', - 'use' => 'sig', - 'x' => 'AAAA', - 'y' => 'BBBB', - ]; - $this->signatoryManager->method('getLocalJwks')->willReturn([$jwk]); - - $body = $this->jsonBody($this->handler->handle('jwks.json', $this->context, null)); - $this->assertSame(['keys' => [$jwk]], $body); - } - - public function testPublishesAllSlotsAdvertisedDuringRotation(): void { - $this->appConfig->method('getValueBool')->willReturn(false); - $active = [ - 'kty' => 'EC', 'crv' => 'P-256', 'kid' => 'kid-1', 'alg' => 'ES256', 'use' => 'sig', 'x' => 'AAAA', 'y' => 'BBBB', - ]; - $pending = [ - 'kty' => 'EC', 'crv' => 'P-256', 'kid' => 'kid-2', 'alg' => 'ES256', 'use' => 'sig', 'x' => 'CCCC', 'y' => 'DDDD', - ]; - $this->signatoryManager->method('getLocalJwks')->willReturn([$active, $pending]); - - $body = $this->jsonBody($this->handler->handle('jwks.json', $this->context, null)); - $this->assertSame(['keys' => [$active, $pending]], $body); - } - - public function testEmptyKeySetWhenSignatoryUnavailable(): void { - $this->appConfig->method('getValueBool')->willReturn(false); - $this->signatoryManager->method('getLocalJwks')->willReturn([]); - - $body = $this->jsonBody($this->handler->handle('jwks.json', $this->context, null)); - $this->assertSame(['keys' => []], $body); - } - - public function testFailingJwkBuildIsLoggedAndYieldsEmptyKeySet(): void { - $this->appConfig->method('getValueBool')->willReturn(false); - $this->signatoryManager->method('getLocalJwks') - ->willThrowException(new \RuntimeException('boom')); - $this->logger->expects($this->once())->method('warning'); - - $body = $this->jsonBody($this->handler->handle('jwks.json', $this->context, null)); - $this->assertSame(['keys' => []], $body); - } - - private function jsonBody(?IResponse $response): array { - $this->assertInstanceOf(GenericResponse::class, $response); - $http = $response->toHttpResponse(); - $this->assertInstanceOf(JSONResponse::class, $http); - return $http->getData(); - } -} diff --git a/tests/lib/OCM/OCMSignatoryManagerJwksTest.php b/tests/lib/OCM/OCMSignatoryManagerJwksTest.php index 66f36f2099f92..3377c4f7784a2 100644 --- a/tests/lib/OCM/OCMSignatoryManagerJwksTest.php +++ b/tests/lib/OCM/OCMSignatoryManagerJwksTest.php @@ -328,23 +328,12 @@ public function testGetRemoteKeyAcceptsJwksUriOnDifferentHost(): void { $this->assertNotNull($this->signatoryManager->getRemoteKey('sender.example.org', $kid)); } - public function testGetLocalJwksUriUsesHttpsByDefault(): void { - $this->urlGenerator->method('getAbsoluteURL') - ->willReturnCallback(fn (string $path) => 'https://sender.example.org' . $path); + public function testGetLocalJwksUriPointsAtAppRoute(): void { + $this->urlGenerator->method('linkToRouteAbsolute') + ->willReturn('https://sender.example.org/index.php/apps/cloud_federation_api/api/v1/jwks'); $this->assertSame( - 'https://sender.example.org/.well-known/jwks.json', - $this->signatoryManager->getLocalJwksUri(), - ); - } - - public function testGetLocalJwksUriFollowsHttpInstanceScheme(): void { - // http-only deployments must advertise a fetchable jwksUri - $this->urlGenerator->method('getAbsoluteURL') - ->willReturnCallback(fn (string $path) => 'http://localhost:8180' . $path); - - $this->assertSame( - 'http://localhost:8180/.well-known/jwks.json', + 'https://sender.example.org/index.php/apps/cloud_federation_api/api/v1/jwks', $this->signatoryManager->getLocalJwksUri(), ); }