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/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..31373c70b47f3 100644 --- a/apps/cloud_federation_api/lib/Controller/TokenController.php +++ b/apps/cloud_federation_api/lib/Controller/TokenController.php @@ -18,12 +18,14 @@ 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; 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 +53,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() ]); @@ -109,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 * @@ -126,7 +174,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/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/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/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/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/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..dbe741da55e7f 100644 --- a/lib/private/OCM/OCMDiscoveryService.php +++ b/lib/private/OCM/OCMDiscoveryService.php @@ -209,7 +209,13 @@ public function getLocalOCMProvider(bool $fullDetails = true): IOCMProvider { $provider->setCapabilities(['notifications', 'shares', 'exchange-token']); $provider->setTokenEndPoint($tokenUrl); if ($signingEnabled) { - $provider->setCapabilities(['http-sig']); + try { + // http-sig advertisement requires a jwksUri + $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(); @@ -253,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) { @@ -277,7 +283,7 @@ public function getIncomingSignedRequest(): ?IIncomingSignedRequest { /** * @inheritDoc * - * @since 34.0.0 + * @since 35.0.0 */ #[\Override] public function confirmRequestOrigin(?string $signedOrigin, string $ocmAddress): void { @@ -304,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/OCMJwksHandler.php b/lib/private/OCM/OCMJwksHandler.php deleted file mode 100644 index 0013b38b1b400..0000000000000 --- a/lib/private/OCM/OCMJwksHandler.php +++ /dev/null @@ -1,49 +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 8320671456a12..433dac293e0a4 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,36 @@ private function signatoryFromPool(int $poolId): ?Signatory { return $signatory; } + /** Absolute URL of the local JWK Set, advertised as `jwksUri`. */ + public function getLocalJwksUri(): string { + return $this->urlGenerator->linkToRouteAbsolute('cloud_federation_api.Token.jwks'); + } + /** * @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); + } + + /** + * 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 + */ + 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); - } 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) : ''; - - return 'https://' . $identity . $sub . '/ocm#' . $fragment; + return $this->urlGenerator->getAbsoluteURL($path); } /** @@ -476,10 +483,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 +521,35 @@ private function fetchJwks(string $origin): ?array { return array_values(array_filter($decoded['keys'], 'is_array')); } + /** + * 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 { + $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; + } + $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; + } + /** * @param list>|null $keys */ @@ -519,8 +561,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..89649d3bbe3a1 100644 --- a/lib/private/Security/Signature/Model/Rfc9421IncomingSignedRequest.php +++ b/lib/private/Security/Signature/Model/Rfc9421IncomingSignedRequest.php @@ -23,39 +23,42 @@ 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 - * 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`. */ - private const DEFAULT_REQUIRED_COMPONENTS = [ + /** + * 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. + */ + public const 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 +91,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(); @@ -118,13 +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 may follow the OCM convention `#`; the OCM layer - // derives origin from the message body in that case. - $this->origin = ''; - } + // keyid is opaque; the signer origin is set by the caller via setOrigin(). $paramsLine = SignatureBase::serializeSignatureParams($this->components, $this->signatureParams); $this->signatureBaseString = SignatureBase::build( @@ -136,7 +149,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'] : '', @@ -161,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'); @@ -222,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( @@ -320,7 +342,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..812d24213dff7 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,6 @@ class Rfc9421OutgoingSignedRequest extends SignedRequest implements IOutgoingSignedRequest, JsonSerializable { - private const DEFAULT_COMPONENTS = ['@method', '@target-uri', 'content-digest', 'content-length', 'date']; private string $host = ''; private array $headers = []; @@ -64,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); @@ -84,6 +84,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..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,17 +146,25 @@ 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()); 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/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 674c907bb1587..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 @@ -76,10 +79,20 @@ 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; + /** + * 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/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/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/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 d218b2dec93bb..e1d41e565fb43 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; @@ -130,12 +131,22 @@ 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 { + // scheme follows the instance base URL + $local = $this->discoveryService->getLocalOCMProvider(); + $jwksUri = $local->getJwksUri(); + $baseUrl = Server::get(IURLGenerator::class)->getBaseUrl(); + $expectedScheme = str_starts_with($baseUrl, 'http://') ? 'http://' : 'https://'; + $this->assertStringStartsWith($expectedScheme, $jwksUri); + $this->assertStringContainsString('cloud_federation_api/api/v1/jwks', $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/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/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..3377c4f7784a2 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,27 @@ 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'], + 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); + } + public function testGetRemoteKeyFetchesAndMatchesByKid(): void { + $this->primeDiscovery(); $kid = 'sender.example.org#key1'; $jwks = [ 'keys' => [ @@ -85,17 +115,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 +137,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 +163,94 @@ 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 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')); + $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 +267,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 +279,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)) @@ -168,6 +293,51 @@ 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 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/index.php/apps/cloud_federation_api/api/v1/jwks', + $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('')); diff --git a/tests/lib/Security/Signature/Model/Rfc9421RoundTripTest.php b/tests/lib/Security/Signature/Model/Rfc9421RoundTripTest.php index e7d42460987f0..4639c1145b8d3 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,39 @@ public function testSignatureNotCoveringRequiredComponentsRejected(): void { new Rfc9421IncomingSignedRequest($body, $req); } + 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); + + $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); + // 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(); + $this->addToAssertionCount(1); + } + private function skipUnlessSodium(): void { if (!extension_loaded('sodium')) { $this->markTestSkipped('ext-sodium is not loaded'); @@ -323,9 +449,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 +479,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..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); } @@ -123,6 +124,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, 'sender.example.org'); + } + private function rsaSignatoryManager(): ISignatoryManager { $key = openssl_pkey_new(['private_key_type' => OPENSSL_KEYTYPE_RSA, 'private_key_bits' => 2048]); $priv = '';