Merge pull request #62543 from nextcloud/kano-update-httpsig

http-sig: Update http message signatures for OCM after spec review

Author: CarlSchwan

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',

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);

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<string, mixed> $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 '';
 	}
 }

apps/cloud_federation_api/lib/Controller/TokenController.php

@@ -18,20 +18,24 @@
 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;
 use OCP\Security\Signature\Exceptions\SignatureNotFoundException;
 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<Http::STATUS_OK, array{keys: list<array<string, string>>}, 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

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"
         }
     ]
 }

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);

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);

lib/composer/composer/autoload_classmap.php

@@ -2061,7 +2061,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',

lib/composer/composer/autoload_static.php

@@ -2102,7 +2102,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',

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;
 		}
 	}