feat(ocm): signing ocm requests

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

apps/cloud_federation_api/lib/Capabilities.php

@@ -6,20 +6,27 @@
  * SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
-
 namespace OCA\CloudFederationAPI;
 
+use NCU\Security\PublicPrivateKeyPairs\Exceptions\KeyPairException;
+use NCU\Security\Signature\Exceptions\SignatoryException;
+use OC\OCM\OCMSignatoryManager;
 use OCP\Capabilities\ICapability;
+use OCP\IAppConfig;
 use OCP\IURLGenerator;
 use OCP\OCM\Exceptions\OCMArgumentException;
 use OCP\OCM\IOCMProvider;
+use Psr\Log\LoggerInterface;
 
 class Capabilities implements ICapability {
-	public const API_VERSION = '1.0-proposal1';
+	public const API_VERSION = '1.1'; // informative, real version.
 
 	public function __construct(
 		private IURLGenerator $urlGenerator,
+		private IAppConfig $appConfig,
 		private IOCMProvider $provider,
+		private readonly OCMSignatoryManager $ocmSignatoryManager,
+		private readonly LoggerInterface $logger,
 	) {
 	}
 
@@ -28,15 +35,20 @@ public function __construct(
 	 *
 	 * @return array{
 	 *     ocm: array{
+	 *     	   apiVersion: '1.0-proposal1',
 	 *         enabled: bool,
-	 *         apiVersion: string,
 	 *         endPoint: string,
+	 *         publicKey: array{
+	 *             keyId: string,
+	 *             publicKeyPem: string,
+	 *         },
 	 *         resourceTypes: list<array{
 	 *             name: string,
 	 *             shareTypes: list<string>,
 	 *             protocols: array<string, string>
-	 *           }>,
-	 *       },
+	 *         }>,
+	 *         version: string
+	 *     }
 	 * }
 	 * @throws OCMArgumentException
 	 */
@@ -60,6 +72,17 @@ public function getCapabilities() {
 
 		$this->provider->addResourceType($resource);
 
-		return ['ocm' => $this->provider->jsonSerialize()];
+		// Adding a public key to the ocm discovery
+		try {
+			if (!$this->appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_DISABLED, lazy: true)) {
+				$this->provider->setSignatory($this->ocmSignatoryManager->getLocalSignatory());
+			} else {
+				$this->logger->debug('ocm public key feature disabled');
+			}
+		} catch (SignatoryException|KeyPairException $e) {
+			$this->logger->warning('cannot generate local signatory', ['exception' => $e]);
+		}
+
+		return ['ocm' => json_decode(json_encode($this->provider->jsonSerialize()), true)];
 	}
 }

apps/cloud_federation_api/lib/Controller/RequestHandlerController.php

@@ -5,6 +5,13 @@
  */
 namespace OCA\CloudFederationAPI\Controller;
 
+use NCU\Security\Signature\Exceptions\IncomingRequestException;
+use NCU\Security\Signature\Exceptions\SignatoryNotFoundException;
+use NCU\Security\Signature\Exceptions\SignatureException;
+use NCU\Security\Signature\Exceptions\SignatureNotFoundException;
+use NCU\Security\Signature\ISignatureManager;
+use NCU\Security\Signature\Model\IIncomingSignedRequest;
+use OC\OCM\OCMSignatoryManager;
 use OCA\CloudFederationAPI\Config;
 use OCA\CloudFederationAPI\ResponseDefinitions;
 use OCP\AppFramework\Controller;
@@ -22,11 +29,14 @@
 use OCP\Federation\ICloudFederationFactory;
 use OCP\Federation\ICloudFederationProviderManager;
 use OCP\Federation\ICloudIdManager;
+use OCP\IAppConfig;
 use OCP\IGroupManager;
 use OCP\IRequest;
 use OCP\IURLGenerator;
 use OCP\IUserManager;
 use OCP\Share\Exceptions\ShareNotFound;
+use OCP\Share\IProviderFactory;
+use OCP\Share\IShare;
 use OCP\Util;
 use Psr\Log\LoggerInterface;
 
@@ -50,8 +60,12 @@ public function __construct(
 		private IURLGenerator $urlGenerator,
 		private ICloudFederationProviderManager $cloudFederationProviderManager,
 		private Config $config,
+		private readonly IAppConfig $appConfig,
 		private ICloudFederationFactory $factory,
 		private ICloudIdManager $cloudIdManager,
+		private readonly ISignatureManager $signatureManager,
+		private readonly OCMSignatoryManager $signatoryManager,
+		private readonly IProviderFactory $shareProviderFactory,
 	) {
 		parent::__construct($appName, $request);
 	}
@@ -81,11 +95,20 @@ public function __construct(
 	#[NoCSRFRequired]
 	#[BruteForceProtection(action: 'receiveFederatedShare')]
 	public function addShare($shareWith, $name, $description, $providerId, $owner, $ownerDisplayName, $sharedBy, $sharedByDisplayName, $protocol, $shareType, $resourceType) {
+		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->getSignedRequest();
+			$this->confirmSignedOrigin($signedRequest, 'owner', $owner);
+		} catch (IncomingRequestException $e) {
+			$this->logger->warning('incoming request exception', ['exception' => $e]);
+			return new JSONResponse(['message' => $e->getMessage(), 'validationErrors' => []], Http::STATUS_BAD_REQUEST);
+		}
+
 		// check if all required parameters are set
 		if ($shareWith === null ||
 			$name === null ||
 			$providerId === null ||
-			$owner === null ||
 			$resourceType === null ||
 			$shareType === null ||
 			!is_array($protocol) ||
@@ -208,6 +231,16 @@ public function addShare($shareWith, $name, $description, $providerId, $owner, $
 	#[PublicPage]
 	#[BruteForceProtection(action: 'receiveFederatedShareNotification')]
 	public function receiveNotification($notificationType, $resourceType, $providerId, ?array $notification) {
+		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->getSignedRequest();
+			$this->confirmShareOrigin($signedRequest, $notification['sharedSecret'] ?? '');
+		} catch (IncomingRequestException $e) {
+			$this->logger->warning('incoming request exception', ['exception' => $e]);
+			return new JSONResponse(['message' => $e->getMessage(), 'validationErrors' => []], Http::STATUS_BAD_REQUEST);
+		}
+
 		// check if all required parameters are set
 		if ($notificationType === null ||
 			$resourceType === null ||
@@ -286,4 +319,124 @@ private function mapUid($uid) {
 
 		return $uid;
 	}
+
+
+	/**
+	 * returns signed request if available.
+	 * throw an exception:
+	 * - if request is signed, but wrongly signed
+	 * - if request is not signed but instance is configured to only accept signed ocm request
+	 *
+	 * @return IIncomingSignedRequest|null null if remote does not (and never did) support signed request
+	 * @throws IncomingRequestException
+	 */
+	private function getSignedRequest(): ?IIncomingSignedRequest {
+		try {
+			return $this->signatureManager->getIncomingSignedRequest($this->signatoryManager);
+		} catch (SignatureNotFoundException|SignatoryNotFoundException $e) {
+			// remote does not support signed request.
+			// currently we still accept unsigned request until lazy appconfig
+			// core.enforce_signed_ocm_request is set to true (default: false)
+			if ($this->appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_ENFORCED, lazy: true)) {
+				$this->logger->notice('ignored unsigned request', ['exception' => $e]);
+				throw new IncomingRequestException('Unsigned request');
+			}
+		} catch (SignatureException $e) {
+			$this->logger->notice('wrongly signed request', ['exception' => $e]);
+			throw new IncomingRequestException('Invalid signature');
+		}
+		return null;
+	}
+
+
+	/**
+	 * confirm that the value related to $key entry from the payload is in format userid@hostname
+	 * and compare hostname with the origin of the signed request.
+	 *
+	 * If request is not signed, we still verify that the hostname from the extracted value does,
+	 * actually, not support signed request
+	 *
+	 * @param IIncomingSignedRequest|null $signedRequest
+	 * @param string $key entry from data available in data
+	 * @param string $value value itself used in case request is not signed
+	 *
+	 * @throws IncomingRequestException
+	 */
+	private function confirmSignedOrigin(?IIncomingSignedRequest $signedRequest, string $key, string $value): void {
+		if ($signedRequest === null) {
+			$instance = $this->getHostFromFederationId($value);
+			try {
+				$this->signatureManager->searchSignatory($instance);
+				throw new IncomingRequestException('instance is supposed to sign its request');
+			} catch (SignatoryNotFoundException) {
+				return;
+			}
+		}
+
+		$body = json_decode($signedRequest->getBody(), true) ?? [];
+		$entry = trim($body[$key] ?? '', '@');
+		if ($this->getHostFromFederationId($entry) !== $signedRequest->getOrigin()) {
+			throw new IncomingRequestException('share initiation from different instance');
+		}
+	}
+
+
+	/**
+	 *  confirm that the value related to share token is in format userid@hostname
+	 *  and compare hostname with the origin of the signed request.
+	 *
+	 *  If request is not signed, we still verify that the hostname from the extracted value does,
+	 *  actually, not support signed request
+	 *
+	 * @param IIncomingSignedRequest|null $signedRequest
+	 * @param string $token
+	 *
+	 * @return void
+	 * @throws IncomingRequestException
+	 */
+	private function confirmShareOrigin(?IIncomingSignedRequest $signedRequest, string $token): void {
+		if ($token === '') {
+			throw new BadRequestException(['sharedSecret']);
+		}
+
+		$provider = $this->shareProviderFactory->getProviderForType(IShare::TYPE_REMOTE);
+		$share = $provider->getShareByToken($token);
+		$entry = $share->getSharedWith();
+
+		$instance = $this->getHostFromFederationId($entry);
+		if ($signedRequest === null) {
+			try {
+				$this->signatureManager->searchSignatory($instance);
+				throw new IncomingRequestException('instance is supposed to sign its request');
+			} catch (SignatoryNotFoundException) {
+				return;
+			}
+		} elseif ($instance !== $signedRequest->getOrigin()) {
+			throw new IncomingRequestException('token sharedWith from different instance');
+		}
+	}
+
+	/**
+	 * @param string $entry
+	 * @return string
+	 * @throws IncomingRequestException
+	 */
+	private function getHostFromFederationId(string $entry): string {
+		if (!str_contains($entry, '@')) {
+			throw new IncomingRequestException('entry does not contains @');
+		}
+		[, $rightPart] = explode('@', $entry, 2);
+
+		$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;
+		}
+
+		throw new IncomingRequestException('host is empty');
+	}
 }

apps/cloud_federation_api/openapi.json

@@ -43,21 +43,41 @@
                     "ocm": {
                         "type": "object",
                         "required": [
-                            "enabled",
                             "apiVersion",
+                            "enabled",
                             "endPoint",
-                            "resourceTypes"
+                            "publicKey",
+                            "resourceTypes",
+                            "version"
                         ],
                         "properties": {
+                            "apiVersion": {
+                                "type": "string",
+                                "enum": [
+                                    "1.0-proposal1"
+                                ]
+                            },
                             "enabled": {
                                 "type": "boolean"
                             },
-                            "apiVersion": {
-                                "type": "string"
-                            },
                             "endPoint": {
                                 "type": "string"
                             },
+                            "publicKey": {
+                                "type": "object",
+                                "required": [
+                                    "keyId",
+                                    "publicKeyPem"
+                                ],
+                                "properties": {
+                                    "keyId": {
+                                        "type": "string"
+                                    },
+                                    "publicKeyPem": {
+                                        "type": "string"
+                                    }
+                                }
+                            },
                             "resourceTypes": {
                                 "type": "array",
                                 "items": {
@@ -85,6 +105,9 @@
                                         }
                                     }
                                 }
+                            },
+                            "version": {
+                                "type": "string"
                             }
                         }
                     }

apps/files_sharing/lib/External/Storage.php

@@ -88,6 +88,7 @@ public function __construct($options) {
 		parent::__construct(
 			[
 				'secure' => ((parse_url($remote, PHP_URL_SCHEME) ?? 'https') === 'https'),
+				'verify' => !$this->config->getSystemValueBool('sharing.federation.allowSelfSignedCertificates', false),
 				'host' => $host,
 				'root' => $webDavEndpoint,
 				'user' => $options['token'],

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

@@ -35,6 +35,9 @@ 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

core/Controller/OCMController.php

@@ -17,7 +17,7 @@
 use OCP\AppFramework\Http\Attribute\PublicPage;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\Capabilities\ICapability;
-use OCP\IConfig;
+use OCP\IAppConfig;
 use OCP\IRequest;
 use OCP\Server;
 use Psr\Container\ContainerExceptionInterface;
@@ -31,7 +31,7 @@
 class OCMController extends Controller {
 	public function __construct(
 		IRequest $request,
-		private IConfig $config,
+		private readonly IAppConfig $appConfig,
 		private LoggerInterface $logger,
 	) {
 		parent::__construct('core', $request);
@@ -54,10 +54,10 @@ public function __construct(
 	public function discovery(): DataResponse {
 		try {
 			$cap = Server::get(
-				$this->config->getAppValue(
-					'core',
-					'ocm_providers',
-					'\OCA\CloudFederationAPI\Capabilities'
+				$this->appConfig->getValueString(
+					'core', 'ocm_providers',
+					\OCA\CloudFederationAPI\Capabilities::class,
+					lazy: true
 				)
 			);