fix(ocm): switching to IdentityProof

Signed-off-by: Maxence Lange <[email protected]>

apps/cloud_federation_api/lib/Capabilities.php

@@ -8,7 +8,7 @@
  */
 namespace OCA\CloudFederationAPI;
 
-use NCU\Security\PublicPrivateKeyPairs\Exceptions\KeyPairException;
+use NCU\Security\Signature\Exceptions\IdentityNotFoundException;
 use NCU\Security\Signature\Exceptions\SignatoryException;
 use OC\OCM\OCMSignatoryManager;
 use OCP\Capabilities\ICapability;
@@ -79,7 +79,7 @@ public function getCapabilities() {
 			} else {
 				$this->logger->debug('ocm public key feature disabled');
 			}
-		} catch (SignatoryException|KeyPairException $e) {
+		} catch (SignatoryException|IdentityNotFoundException $e) {
 			$this->logger->warning('cannot generate local signatory', ['exception' => $e]);
 		}
 

apps/cloud_federation_api/lib/Controller/RequestHandlerController.php

@@ -5,6 +5,7 @@
  */
 namespace OCA\CloudFederationAPI\Controller;
 
+use NCU\Security\Signature\Exceptions\IdentityNotFoundException;
 use NCU\Security\Signature\Exceptions\IncomingRequestException;
 use NCU\Security\Signature\Exceptions\SignatoryNotFoundException;
 use NCU\Security\Signature\Exceptions\SignatureException;
@@ -14,6 +15,7 @@
 use OC\OCM\OCMSignatoryManager;
 use OCA\CloudFederationAPI\Config;
 use OCA\CloudFederationAPI\ResponseDefinitions;
+use OCA\FederatedFileSharing\AddressHandler;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\Attribute\BruteForceProtection;
@@ -60,6 +62,7 @@ public function __construct(
 		private IURLGenerator $urlGenerator,
 		private ICloudFederationProviderManager $cloudFederationProviderManager,
 		private Config $config,
+		private readonly AddressHandler $addressHandler,
 		private readonly IAppConfig $appConfig,
 		private ICloudFederationFactory $factory,
 		private ICloudIdManager $cloudIdManager,
@@ -289,6 +292,7 @@ public function receiveNotification($notificationType, $resourceType, $providerI
 			$response->throttle();
 			return $response;
 		} catch (\Exception $e) {
+			$this->logger->warning('incoming notification exception', ['exception' => $e]);
 			return new JSONResponse(
 				[
 					'message' => 'Internal error at ' . $this->urlGenerator->getBaseUrl(),
@@ -376,7 +380,7 @@ private function confirmSignedOrigin(?IIncomingSignedRequest $signedRequest, str
 		$body = json_decode($signedRequest->getBody(), true) ?? [];
 		$entry = trim($body[$key] ?? '', '@');
 		if ($this->getHostFromFederationId($entry) !== $signedRequest->getOrigin()) {
-			throw new IncomingRequestException('share initiation from different instance');
+			throw new IncomingRequestException('share initiation (' . $signedRequest->getOrigin() . ') from different instance (' . $entry . ') [key=' . $key . ']');
 		}
 	}
 
@@ -391,7 +395,6 @@ private function confirmSignedOrigin(?IIncomingSignedRequest $signedRequest, str
 	 * @param IIncomingSignedRequest|null $signedRequest
 	 * @param string $token
 	 *
-	 * @return void
 	 * @throws IncomingRequestException
 	 */
 	private function confirmShareOrigin(?IIncomingSignedRequest $signedRequest, string $token): void {
@@ -401,8 +404,23 @@ private function confirmShareOrigin(?IIncomingSignedRequest $signedRequest, stri
 
 		$provider = $this->shareProviderFactory->getProviderForType(IShare::TYPE_REMOTE);
 		$share = $provider->getShareByToken($token);
-		$entry = $share->getSharedWith();
+		try {
+			$this->confirmShareEntry($signedRequest, $share->getSharedWith());
+		} catch (IncomingRequestException) {
+			// notification might come from the instance that owns the share
+			$this->logger->debug('could not confirm origin on sharedWith (' . $share->getSharedWIth() . '); going with shareOwner (' . $share->getShareOwner() . ')');
+			$this->confirmShareEntry($signedRequest, $share->getShareOwner());
+		}
+	}
 
+	/**
+	 * @param IIncomingSignedRequest|null $signedRequest
+	 * @param string $entry
+	 *
+	 * @return void
+	 * @throws IncomingRequestException
+	 */
+	private function confirmShareEntry(?IIncomingSignedRequest $signedRequest, string $entry): void {
 		$instance = $this->getHostFromFederationId($entry);
 		if ($signedRequest === null) {
 			try {
@@ -412,7 +430,7 @@ private function confirmShareOrigin(?IIncomingSignedRequest $signedRequest, stri
 				return;
 			}
 		} elseif ($instance !== $signedRequest->getOrigin()) {
-			throw new IncomingRequestException('token sharedWith from different instance');
+			throw new IncomingRequestException('token sharedWith (' . $instance . ') not linked to origin (' . $signedRequest->getOrigin() . ')');
 		}
 	}
 
@@ -423,20 +441,16 @@ private function confirmShareOrigin(?IIncomingSignedRequest $signedRequest, stri
 	 */
 	private function getHostFromFederationId(string $entry): string {
 		if (!str_contains($entry, '@')) {
-			throw new IncomingRequestException('entry does not contains @');
+			throw new IncomingRequestException('entry ' . $entry . ' does not contains @');
 		}
-		[, $rightPart] = explode('@', $entry, 2);
+		$rightPart = substr($entry, strrpos($entry, '@') + 1);
 
-		$host = parse_url($rightPart, PHP_URL_HOST);
-		$port = parse_url($rightPart, PHP_URL_PORT);
-		if ($port !== null && $port !== false) {
-			$host .= ':' . $port;
-		}
-
-		if (is_string($host) && $host !== '') {
-			return $host;
+		// in case the full scheme is sent; getting rid of it
+		$rightPart = $this->addressHandler->removeProtocolFromUrl($rightPart);
+		try {
+			return $this->signatureManager->extractIdentityFromUri('https://' . $rightPart);
+		} catch (IdentityNotFoundException) {
+			throw new IncomingRequestException('invalid host within federation id: ' . $entry);
 		}
-
-		throw new IncomingRequestException('host is empty');
 	}
 }

apps/federatedfilesharing/lib/FederatedShareProvider.php

@@ -250,7 +250,8 @@ protected function askOwnerToReShare($shareWith, IShare $share, $shareId) {
 			$remote,
 			$shareWith,
 			$share->getPermissions(),
-			$share->getNode()->getName()
+			$share->getNode()->getName(),
+			$share->getShareType(),
 		);
 
 		return [$token, $remoteId];

apps/federatedfilesharing/lib/Notifications.php

@@ -108,12 +108,13 @@ public function sendRemoteShare($token, $shareWith, $name, $remoteId, $owner, $o
 	 * @throws HintException
 	 * @throws \OC\ServerNotAvailableException
 	 */
-	public function requestReShare($token, $id, $shareId, $remote, $shareWith, $permission, $filename) {
+	public function requestReShare($token, $id, $shareId, $remote, $shareWith, $permission, $filename, $shareType) {
 		$fields = [
 			'shareWith' => $shareWith,
 			'token' => $token,
 			'permission' => $permission,
 			'remoteId' => $shareId,
+			'shareType' => $shareType,
 		];
 
 		$ocmFields = $fields;

build/integration/features/bootstrap/FederationContext.php

@@ -38,7 +38,7 @@ public function startFederatedServer() {
 
 		$port = getenv('PORT_FED');
 
-		self::$phpFederatedServerPid = exec('php -S localhost:' . $port . ' -t ../../ >/dev/null & echo $!');
+		self::$phpFederatedServerPid = exec('PHP_CLI_SERVER_WORKERS=2 php -S localhost:' . $port . ' -t ../../ >/dev/null & echo $!');
 	}
 
 	/**

build/integration/federation_features/cleanup-remote-storage.feature

@@ -4,6 +4,27 @@ Feature: cleanup-remote-storage
   Background:
     Given using api version "1"
 
+  Scenario: cleanup remote storage with no storage
+    Given Using server "LOCAL"
+    And user "user0" exists
+    Given Using server "REMOTE"
+    And user "user1" exists
+    # Rename file so it has a unique name in the target server (as the target
+    # server may have its own /textfile0.txt" file)
+    And User "user1" copies file "/textfile0.txt" to "/remote-share.txt"
+    And User "user1" from server "REMOTE" shares "/remote-share.txt" with user "user0" from server "LOCAL"
+    And As an "user1"
+    And Deleting last share
+    And the OCS status code should be "100"
+    And the HTTP status code should be "200"
+    And Deleting last share
+    And Using server "LOCAL"
+    When invoking occ with "sharing:cleanup-remote-storage"
+    Then the command was successful
+    And the command output contains the text "0 remote storage(s) need(s) to be checked"
+    And the command output contains the text "0 remote share(s) exist"
+    And the command output contains the text "no storages deleted"
+
   Scenario: cleanup remote storage with active storages
     Given Using server "LOCAL"
     And user "user0" exists
@@ -35,9 +56,6 @@ Feature: cleanup-remote-storage
     # server may have its own /textfile0.txt" file)
     And User "user1" copies file "/textfile0.txt" to "/remote-share.txt"
     And User "user1" from server "REMOTE" shares "/remote-share.txt" with user "user0" from server "LOCAL"
-    And As an "user1"
-    And sending "GET" to "/apps/files_sharing/api/v1/shares"
-    And the list of returned shares has 1 shares
     And Using server "LOCAL"
     # Accept and download the file to ensure that a storage is created for the
     # federated share

lib/composer/composer/autoload_classmap.php

@@ -1901,8 +1901,6 @@
     'OC\\Security\\Ip\\Range' => $baseDir . '/lib/private/Security/Ip/Range.php',
     'OC\\Security\\Ip\\RemoteAddress' => $baseDir . '/lib/private/Security/Ip/RemoteAddress.php',
     'OC\\Security\\Normalizer\\IpAddress' => $baseDir . '/lib/private/Security/Normalizer/IpAddress.php',
-    'OC\\Security\\PublicPrivateKeyPairs\\KeyPairManager' => $baseDir . '/lib/private/Security/PublicPrivateKeyPairs/KeyPairManager.php',
-    'OC\\Security\\PublicPrivateKeyPairs\\Model\\KeyPair' => $baseDir . '/lib/private/Security/PublicPrivateKeyPairs/Model/KeyPair.php',
     'OC\\Security\\RateLimiting\\Backend\\DatabaseBackend' => $baseDir . '/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php',
     'OC\\Security\\RateLimiting\\Backend\\IBackend' => $baseDir . '/lib/private/Security/RateLimiting/Backend/IBackend.php',
     'OC\\Security\\RateLimiting\\Backend\\MemoryCacheBackend' => $baseDir . '/lib/private/Security/RateLimiting/Backend/MemoryCacheBackend.php',

lib/composer/composer/autoload_static.php

@@ -1942,8 +1942,6 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\Security\\Ip\\Range' => __DIR__ . '/../../..' . '/lib/private/Security/Ip/Range.php',
         'OC\\Security\\Ip\\RemoteAddress' => __DIR__ . '/../../..' . '/lib/private/Security/Ip/RemoteAddress.php',
         'OC\\Security\\Normalizer\\IpAddress' => __DIR__ . '/../../..' . '/lib/private/Security/Normalizer/IpAddress.php',
-        'OC\\Security\\PublicPrivateKeyPairs\\KeyPairManager' => __DIR__ . '/../../..' . '/lib/private/Security/PublicPrivateKeyPairs/KeyPairManager.php',
-        'OC\\Security\\PublicPrivateKeyPairs\\Model\\KeyPair' => __DIR__ . '/../../..' . '/lib/private/Security/PublicPrivateKeyPairs/Model/KeyPair.php',
         'OC\\Security\\RateLimiting\\Backend\\DatabaseBackend' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php',
         'OC\\Security\\RateLimiting\\Backend\\IBackend' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Backend/IBackend.php',
         'OC\\Security\\RateLimiting\\Backend\\MemoryCacheBackend' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Backend/MemoryCacheBackend.php',

lib/private/Federation/CloudFederationProviderManager.php

@@ -18,6 +18,7 @@
 use OCP\Federation\ICloudFederationProviderManager;
 use OCP\Federation\ICloudFederationShare;
 use OCP\Federation\ICloudIdManager;
+use OCP\Http\Client\IClient;
 use OCP\Http\Client\IClientService;
 use OCP\Http\Client\IResponse;
 use OCP\IAppConfig;
@@ -105,25 +106,11 @@ public function getCloudFederationProvider($resourceType) {
 	public function sendShare(ICloudFederationShare $share) {
 		$cloudID = $this->cloudIdManager->resolveCloudId($share->getShareWith());
 		try {
-			$ocmProvider = $this->discoveryService->discover($cloudID->getRemote());
-		} catch (OCMProviderException $e) {
-			return false;
-		}
-
-		$client = $this->httpClientService->newClient();
-		try {
-			// signing the payload using OCMSignatoryManager before initializing the request
-			$uri = $ocmProvider->getEndPoint() . '/shares';
-			$payload = array_merge($this->getDefaultRequestOptions(), ['body' => json_encode($share->getShare())]);
-			if (!$this->appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_DISABLED, lazy: true)) {
-				$signedPayload = $this->signatureManager->signOutgoingRequestIClientPayload(
-					$this->signatoryManager,
-					$payload,
-					'post', $uri
-				);
+			try {
+				$response = $this->postOcmPayload($cloudID->getRemote(), '/shares', json_encode($share->getShare()));
+			} catch (OCMProviderException) {
+				return false;
 			}
-			$response = $client->post($uri, $signedPayload ?? $payload);
-
 			if ($response->getStatusCode() === Http::STATUS_CREATED) {
 				$result = json_decode($response->getBody(), true);
 				return (is_array($result)) ? $result : [];
@@ -149,22 +136,9 @@ public function sendShare(ICloudFederationShare $share) {
 	 */
 	public function sendCloudShare(ICloudFederationShare $share): IResponse {
 		$cloudID = $this->cloudIdManager->resolveCloudId($share->getShareWith());
-		$ocmProvider = $this->discoveryService->discover($cloudID->getRemote());
-
 		$client = $this->httpClientService->newClient();
 		try {
-			// signing the payload using OCMSignatoryManager before initializing the request
-			$uri = $ocmProvider->getEndPoint() . '/shares';
-			$payload = array_merge($this->getDefaultRequestOptions(), ['body' => json_encode($share->getShare())]);
-			if (!$this->appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_DISABLED, lazy: true)) {
-				$signedPayload = $this->signatureManager->signOutgoingRequestIClientPayload(
-					$this->signatoryManager,
-					$payload,
-					'post', $uri
-				);
-			}
-
-			return $client->post($uri, $signedPayload ?? $payload);
+			return $this->postOcmPayload($cloudID->getRemote(), '/shares', json_encode($share->getShare()), $client);
 		} catch (\Throwable $e) {
 			$this->logger->error('Error while sending share to federation server: ' . $e->getMessage(), ['exception' => $e]);
 			try {
@@ -183,26 +157,11 @@ public function sendCloudShare(ICloudFederationShare $share): IResponse {
 	 */
 	public function sendNotification($url, ICloudFederationNotification $notification) {
 		try {
-			$ocmProvider = $this->discoveryService->discover($url);
-		} catch (OCMProviderException $e) {
-			return false;
-		}
-
-		$client = $this->httpClientService->newClient();
-		try {
-
-			// signing the payload using OCMSignatoryManager before initializing the request
-			$uri = $ocmProvider->getEndPoint() . '/notifications';
-			$payload = array_merge($this->getDefaultRequestOptions(), ['body' => json_encode($notification->getMessage())]);
-			if (!$this->appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_DISABLED, lazy: true)) {
-				$signedPayload = $this->signatureManager->signOutgoingRequestIClientPayload(
-					$this->signatoryManager,
-					$payload,
-					'post', $uri
-				);
+			try {
+				$response = $this->postOcmPayload($url, '/notifications', json_encode($notification->getMessage()));
+			} catch (OCMProviderException) {
+				return false;
 			}
-			$response = $client->post($uri, $signedPayload ?? $payload);
-
 			if ($response->getStatusCode() === Http::STATUS_CREATED) {
 				$result = json_decode($response->getBody(), true);
 				return (is_array($result)) ? $result : [];
@@ -222,21 +181,9 @@ public function sendNotification($url, ICloudFederationNotification $notificatio
 	 * @throws OCMProviderException
 	 */
 	public function sendCloudNotification(string $url, ICloudFederationNotification $notification): IResponse {
-		$ocmProvider = $this->discoveryService->discover($url);
-
 		$client = $this->httpClientService->newClient();
 		try {
-			// signing the payload using OCMSignatoryManager before initializing the request
-			$uri = $ocmProvider->getEndPoint() . '/notifications';
-			$payload = array_merge($this->getDefaultRequestOptions(), ['body' => json_encode($notification->getMessage())]);
-			if (!$this->appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_DISABLED, lazy: true)) {
-				$signedPayload = $this->signatureManager->signOutgoingRequestIClientPayload(
-					$this->signatoryManager,
-					$payload,
-					'post', $uri
-				);
-			}
-			return $client->post($uri, $signedPayload ?? $payload);
+			return $this->postOcmPayload($url, '/notifications', json_encode($notification->getMessage()), $client);
 		} catch (\Throwable $e) {
 			$this->logger->error('Error while sending notification to federation server: ' . $e->getMessage(), ['exception' => $e]);
 			try {
@@ -256,6 +203,40 @@ public function isReady() {
 		return $this->appManager->isEnabledForUser('cloud_federation_api');
 	}
 
+	/**
+	 * @param string $cloudId
+	 * @param string $uri
+	 * @param string $payload
+	 *
+	 * @return IResponse
+	 * @throws OCMProviderException
+	 */
+	private function postOcmPayload(string $cloudId, string $uri, string $payload, ?IClient $client = null): IResponse {
+		$ocmProvider = $this->discoveryService->discover($cloudId);
+		$uri = $ocmProvider->getEndPoint() . '/' . ltrim($uri, '/');
+		$client = $client ?? $this->httpClientService->newClient();
+		return $client->post($uri, $this->prepareOcmPayload($uri, $payload));
+	}
+
+	/**
+	 * @param string $uri
+	 * @param string $payload
+	 *
+	 * @return array
+	 */
+	private function prepareOcmPayload(string $uri, string $payload): array {
+		$payload = array_merge($this->getDefaultRequestOptions(), ['body' => $payload]);
+		if (!$this->appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_DISABLED, lazy: true)) {
+			$signedPayload = $this->signatureManager->signOutgoingRequestIClientPayload(
+				$this->signatoryManager,
+				$payload,
+				'post', $uri
+			);
+		}
+
+		return $signedPayload ?? $payload;
+	}
+
 	private function getDefaultRequestOptions(): array {
 		return [
 			'headers' => ['content-type' => 'application/json'],