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..9549881d2c1cd 100644 --- a/apps/cloud_federation_api/lib/Controller/OCMRequestController.php +++ b/apps/cloud_federation_api/lib/Controller/OCMRequestController.php @@ -56,10 +56,17 @@ public function manageOCMRequests(string $ocmPath): Response { throw new OCMArgumentException('path is not UTF-8'); } + $ocmAddress = null; + $params = $this->request->getParams(); + foreach (['owner', 'sender', 'sharedBy'] as $field) { + if (is_string($params[$field] ?? null) && $params[$field] !== '') { + $ocmAddress = $params[$field]; + break; + } + } + 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($ocmAddress); } catch (IncomingRequestException $e) { $this->logger->warning('incoming ocm request exception', ['exception' => $e]); $response = new JSONResponse(['message' => $e->getMessage(), 'validationErrors' => []], Http::STATUS_BAD_REQUEST); diff --git a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php index a855bb96206ca..010a9a23f4391 100644 --- a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php +++ b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php @@ -110,7 +110,7 @@ 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(); + $signedRequest = $this->ocmDiscoveryService->getIncomingSignedRequest($owner); $this->confirmSignedOrigin($signedRequest, 'owner', $owner); } catch (IncomingRequestException $e) { $this->logger->warning('incoming request exception', ['exception' => $e]); @@ -307,10 +307,11 @@ 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); + $signedRequest = $this->ocmDiscoveryService->getIncomingSignedRequest($identity !== '' ? $identity : null); + 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 +451,16 @@ private function confirmSignedOrigin(?IIncomingSignedRequest $signedRequest, str } /** - * confirm identity of the remote instance on notification, based on the share token. - * - * If request is not signed, we still verify that the hostname from the extracted value does, - * actually, not support signed request + * Resolve the sender identity from a notification's sharedSecret. + * Returns '' when the provider does not implement signed federation. * - * @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 +476,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..2d8cb18e22613 100644 --- a/apps/cloud_federation_api/lib/Controller/TokenController.php +++ b/apps/cloud_federation_api/lib/Controller/TokenController.php @@ -18,13 +18,16 @@ 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\Federation\ICloudIdManager; use OCP\IAppConfig; use OCP\IRequest; use OCP\Security\ISecureRandom; +use OCP\Security\Signature\Exceptions\IdentityNotFoundException; use OCP\Security\Signature\Exceptions\IncomingRequestException; use OCP\Security\Signature\Exceptions\SignatoryNotFoundException; use OCP\Security\Signature\Exceptions\SignatureException; @@ -32,6 +35,7 @@ use OCP\Security\Signature\IIncomingSignedRequest; use OCP\Security\Signature\ISignatureManager; use OCP\Security\Signature\Model\Signatory; +use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IManager as IShareManager; use Psr\Log\LoggerInterface; @@ -51,19 +55,45 @@ public function __construct( private readonly IAppConfig $appConfig, private readonly OcmTokenMapMapper $ocmTokenMapMapper, private readonly IShareManager $shareManager, + private readonly ICloudIdManager $cloudIdManager, ) { 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; + } + $remote = $this->cloudIdManager->resolveCloudId($sharedWith)->getRemote(); + return $this->signatureManager->extractIdentityFromUri($remote); + } catch (ShareNotFound|IdentityNotFoundException|\InvalidArgumentException) { + return null; + } + } + /** * Verify the signature of incoming request if available * + * @param string|null $origin sender origin, 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 +139,25 @@ 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 { + $keys = $this->signatoryManager->getLocalJwks(); + } 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 +175,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..6185a7ca8471c 100644 --- a/apps/cloud_federation_api/tests/Controller/TokenControllerTest.php +++ b/apps/cloud_federation_api/tests/Controller/TokenControllerTest.php @@ -21,6 +21,8 @@ use OCP\Authentication\Exceptions\ExpiredTokenException; use OCP\Authentication\Exceptions\InvalidTokenException; use OCP\Authentication\Token\IToken; +use OCP\Federation\ICloudId; +use OCP\Federation\ICloudIdManager; use OCP\IAppConfig; use OCP\IRequest; use OCP\Security\ISecureRandom; @@ -47,6 +49,7 @@ class TokenControllerTest extends TestCase { private IAppConfig&MockObject $appConfig; private OcmTokenMapMapper&MockObject $ocmTokenMapMapper; private IShareManager&MockObject $shareManager; + private ICloudIdManager&MockObject $cloudIdManager; private TokenController $controller; @@ -63,10 +66,13 @@ protected function setUp(): void { $this->timeFactory = $this->createMock(ITimeFactory::class); $this->logger = $this->createMock(LoggerInterface::class); $this->signatureManager = $this->createMock(ISignatureManager::class); + $this->signatureManager->method('extractIdentityFromUri') + ->willReturnCallback(static fn (string $uri): string => (string)parse_url($uri, PHP_URL_HOST)); $this->signatoryManager = $this->createMock(OCMSignatoryManager::class); $this->appConfig = $this->createMock(IAppConfig::class); $this->ocmTokenMapMapper = $this->createMock(OcmTokenMapMapper::class); $this->shareManager = $this->createMock(IShareManager::class); + $this->cloudIdManager = $this->createMock(ICloudIdManager::class); $this->controller = new TokenController( $this->request, @@ -79,6 +85,7 @@ protected function setUp(): void { $this->appConfig, $this->ocmTokenMapMapper, $this->shareManager, + $this->cloudIdManager, ); } @@ -129,6 +136,11 @@ private function configureHappyPath( $this->shareManager->method('getShareByToken') ->with($refreshToken) ->willReturn($share); + $cloudId = $this->createMock(ICloudId::class); + $cloudId->method('getRemote')->willReturn('https://remote.example.com'); + $this->cloudIdManager->method('resolveCloudId') + ->with($sharedWith) + ->willReturn($cloudId); $signatory = new Signatory(); $signatory->setKeyId('https://local.example.com/index.php/ocm#signature'); @@ -149,10 +161,10 @@ public function testAccessTokenSuccess(): void { $signedRequest = $this->createMock(IIncomingSignedRequest::class); $signedRequest->method('getOrigin')->willReturn('remote.example.com'); $this->signatureManager->method('getIncomingSignedRequest') - ->with($this->signatoryManager) + ->with($this->signatoryManager, null, 'remote.example.com') ->willReturn($signedRequest); - $this->configureHappyPath('valid-refresh-token', 123, 'testuser', 'owner', 'sharee@remote.example.com', 'fixedjtivalue00'); + $this->configureHappyPath('valid-refresh-token', 123, 'testuser', 'owner', 'sharee@department@remote.example.com', 'fixedjtivalue00'); $this->ocmTokenMapMapper->expects($this->once()) ->method('insert') @@ -177,7 +189,7 @@ public function testAccessTokenSuccess(): void { $decoded = JWT::decode($data['access_token'], new Key($this->publicKeyPem, 'RS256')); $this->assertSame('https://local.example.com', $decoded->iss); $this->assertSame('owner', $decoded->sub); - $this->assertSame('sharee@remote.example.com', $decoded->aud); + $this->assertSame('sharee@department@remote.example.com', $decoded->aud); $this->assertSame('789', $decoded->client_id); $this->assertSame('fixedjtivalue00', $decoded->jti); $this->assertSame(1000000, $decoded->iat); diff --git a/core/AppInfo/Application.php b/core/AppInfo/Application.php index 6d14ac467b9b6..42a3a09f2625a 100644 --- a/core/AppInfo/Application.php +++ b/core/AppInfo/Application.php @@ -41,7 +41,6 @@ use OC\DirectEditing\Listeners\UserDeletedTokenCleanupListener as UserDeletedDirectEditingTokenCleanupListener; use OC\DirectEditing\Listeners\UserDisabledTokenCleanupListener as UserDisabledDirectEditingTokenCleanupListener; use OC\OCM\OCMDiscoveryHandler; -use OC\OCM\OCMJwksHandler; use OC\TagManager; use OCP\AppFramework\App; use OCP\AppFramework\Bootstrap\IBootContext; @@ -113,7 +112,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 6472ccd5e2050..9cac26bb28843 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -2057,7 +2057,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 eb51034b2dff1..91fce24eb69c5 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -2098,7 +2098,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..19cd38bd99c48 100644 --- a/lib/private/AppFramework/Http/Attributes/FederationRateLimit.php +++ b/lib/private/AppFramework/Http/Attributes/FederationRateLimit.php @@ -41,14 +41,14 @@ public function shouldApply(IRequest $request): bool { } try { - $signedRequest = $this->discoveryService->getIncomingSignedRequest(); + $owner = $request->getParam('owner'); + $signedRequest = $this->discoveryService->getIncomingSignedRequest(is_string($owner) ? $owner : null); 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/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..58db2a1d0ed53 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,10 @@ public function getLocalOCMProvider(bool $fullDetails = true): IOCMProvider { * @since 33.0.0 */ #[\Override] - public function getIncomingSignedRequest(): ?IIncomingSignedRequest { + public function getIncomingSignedRequest(?string $ocmAddress = null): ?IIncomingSignedRequest { + $origin = $ocmAddress !== null ? $this->getHostFromOcmAddress($ocmAddress) : null; 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 +284,7 @@ public function getIncomingSignedRequest(): ?IIncomingSignedRequest { /** * @inheritDoc * - * @since 34.0.0 + * @since 35.0.0 */ #[\Override] public function confirmRequestOrigin(?string $signedOrigin, string $ocmAddress): void { @@ -304,6 +311,10 @@ 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 { 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..250d5b8df0341 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,41 @@ 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); + 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) : ''; - - return 'https://' . $identity . $sub . '/ocm#' . $fragment; + return $this->urlGenerator->getAbsoluteURL($path); } /** @@ -476,10 +488,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 +526,46 @@ private function fetchJwks(string $origin): ?array { return array_values(array_filter($decoded['keys'], 'is_array')); } + /** + * Resolve the peer's `jwksUri`, preferring HTTPS discovery. HTTP is accepted + * only when HTTPS discovery fails and the discovery document is fetched over + * HTTP as well. + */ + private function resolveJwksUri(string $origin): ?string { + try { + $discoveryService = Server::get(IOCMDiscoveryService::class); + try { + $provider = $discoveryService->discover('https://' . $origin); + $discoveryScheme = 'https'; + } catch (OCMProviderException) { + $provider = $discoveryService->discover('http://' . $origin); + $discoveryScheme = 'http'; + } + } 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; + } + $secureJwks = str_starts_with($jwksUri, 'https://'); + $httpJwksFromHttpDiscovery = $discoveryScheme === 'http' && str_starts_with($jwksUri, 'http://'); + if (!$secureJwks && !$httpJwksFromHttpDiscovery) { + $this->logger->warning('refusing jwksUri for the OCM discovery transport', [ + 'origin' => $origin, + 'jwksUri' => $jwksUri, + 'discoveryScheme' => $discoveryScheme, + ]); + return null; + } + return $jwksUri; + } + /** * @param list>|null $keys */ @@ -519,8 +577,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/OCM/IOCMDiscoveryService.php b/lib/public/OCM/IOCMDiscoveryService.php index 674c907bb1587..92c705be41d2f 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 $ocmAddress for RFC 9421, the sender's OCM address + * (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 $ocmAddress = null): ?IIncomingSignedRequest; /** * Confirm that the host portion of $ocmAddress matches $signedOrigin @@ -76,7 +79,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; 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..8b0b1aa32cae7 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,32 @@ 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'], + bool $httpOnly = false, + ): void { + $provider = new OCMProvider(); + $provider->setCapabilities($capabilities); + $provider->setJwksUri($jwksUri); + $this->discoveryService->method('discover') + ->willReturnCallback(static function (string $remote) use ($httpOnly, $provider): OCMProvider { + if ($httpOnly && str_starts_with($remote, 'https://')) { + throw new OCMProviderException('HTTPS unavailable'); + } + return $provider; + }); + } + public function testGetRemoteKeyFetchesAndMatchesByKid(): void { + $this->primeDiscovery(); $kid = 'sender.example.org#key1'; $jwks = [ 'keys' => [ @@ -85,17 +120,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 +142,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 +168,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', + httpOnly: true, + ); + $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 +272,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 +284,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 +298,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( + httpOnly: true, + ); + $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', + httpOnly: true, + ); + $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 = '';