From 9789af1a0d38bfe4d7677c5079420d9f1259b96a Mon Sep 17 00:00:00 2001 From: Micke Nordin Date: Sun, 26 Jul 2026 14:22:00 +0200 Subject: [PATCH 1/2] 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 97e3ba54c4f0916d6075920da2dd4418867fb9f5 Mon Sep 17 00:00:00 2001 From: Micke Nordin Date: Sun, 26 Jul 2026 14:30:08 +0200 Subject: [PATCH 2/2] 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;