fix(ocm): simpler code

Signed-off-by: Maxence Lange <[email protected]>
nextcloud · Nov 21, 2024 · 7fbd5be · 7fbd5be
1 parent 413466f
commit 7fbd5be
Show file tree

Hide file tree

Showing 18 changed files with 535 additions and 532 deletions.
diff --git a/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php b/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php
@@ -336,8 +336,11 @@ private function mapUid($uid) {
 	 */
 	private function getSignedRequest(): ?IIncomingSignedRequest {
 		try {
-			return $this->signatureManager->getIncomingSignedRequest($this->signatoryManager);
+			$signedRequest = $this->signatureManager->getIncomingSignedRequest($this->signatoryManager);
+			$this->logger->debug('signed request available', ['signedRequest' => $signedRequest]);
+			return $signedRequest;
 		} catch (SignatureNotFoundException|SignatoryNotFoundException $e) {
+			$this->logger->debug('remote does not support signed request', ['exception' => $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)
@@ -346,7 +349,7 @@ private function getSignedRequest(): ?IIncomingSignedRequest {
 				throw new IncomingRequestException('Unsigned request');
 			}
 		} catch (SignatureException $e) {
-			$this->logger->notice('wrongly signed request', ['exception' => $e]);
+			$this->logger->warning('wrongly signed request', ['exception' => $e]);
 			throw new IncomingRequestException('Invalid signature');
 		}
 		return null;
@@ -406,10 +409,17 @@ private function confirmShareOrigin(?IIncomingSignedRequest $signedRequest, stri
 		$share = $provider->getShareByToken($token);
 		try {
 			$this->confirmShareEntry($signedRequest, $share->getSharedWith());
-		} catch (IncomingRequestException) {
+		} catch (IncomingRequestException $e) {
 			// 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());
+			$this->logger->debug('could not confirm origin on sharedWith (' . $share->getSharedWIth() . '); going with shareOwner (' . $share->getShareOwner() . ')', ['exception' => $e]);
+			try {
+				$this->confirmShareEntry($signedRequest, $share->getShareOwner());
+			} catch (IncomingRequestException $f) {
+				// if both entry are failing, we log first exception as warning and second exception
+				// will be logged as warning by the controller
+				$this->logger->warning('could not confirm origin on sharedWith (' . $share->getSharedWIth() . '); going with shareOwner (' . $share->getShareOwner() . ')', ['exception' => $e]);
+				throw $f;
+			}
 		}
 	}
 

diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
@@ -12,6 +12,25 @@
     'NCU\\Config\\Exceptions\\UnknownKeyException' => $baseDir . '/lib/unstable/Config/Exceptions/UnknownKeyException.php',
     'NCU\\Config\\IUserConfig' => $baseDir . '/lib/unstable/Config/IUserConfig.php',
     'NCU\\Config\\ValueType' => $baseDir . '/lib/unstable/Config/ValueType.php',
+    'NCU\\Security\\Signature\\Exceptions\\IdentityNotFoundException' => $baseDir . '/lib/unstable/Security/Signature/Exceptions/IdentityNotFoundException.php',
+    'NCU\\Security\\Signature\\Exceptions\\IncomingRequestException' => $baseDir . '/lib/unstable/Security/Signature/Exceptions/IncomingRequestException.php',
+    'NCU\\Security\\Signature\\Exceptions\\InvalidKeyOriginException' => $baseDir . '/lib/unstable/Security/Signature/Exceptions/InvalidKeyOriginException.php',
+    'NCU\\Security\\Signature\\Exceptions\\InvalidSignatureException' => $baseDir . '/lib/unstable/Security/Signature/Exceptions/InvalidSignatureException.php',
+    'NCU\\Security\\Signature\\Exceptions\\SignatoryConflictException' => $baseDir . '/lib/unstable/Security/Signature/Exceptions/SignatoryConflictException.php',
+    'NCU\\Security\\Signature\\Exceptions\\SignatoryException' => $baseDir . '/lib/unstable/Security/Signature/Exceptions/SignatoryException.php',
+    'NCU\\Security\\Signature\\Exceptions\\SignatoryNotFoundException' => $baseDir . '/lib/unstable/Security/Signature/Exceptions/SignatoryNotFoundException.php',
+    'NCU\\Security\\Signature\\Exceptions\\SignatureElementNotFoundException' => $baseDir . '/lib/unstable/Security/Signature/Exceptions/SignatureElementNotFoundException.php',
+    'NCU\\Security\\Signature\\Exceptions\\SignatureException' => $baseDir . '/lib/unstable/Security/Signature/Exceptions/SignatureException.php',
+    'NCU\\Security\\Signature\\Exceptions\\SignatureNotFoundException' => $baseDir . '/lib/unstable/Security/Signature/Exceptions/SignatureNotFoundException.php',
+    'NCU\\Security\\Signature\\ISignatoryManager' => $baseDir . '/lib/unstable/Security/Signature/ISignatoryManager.php',
+    'NCU\\Security\\Signature\\ISignatureManager' => $baseDir . '/lib/unstable/Security/Signature/ISignatureManager.php',
+    'NCU\\Security\\Signature\\Model\\IIncomingSignedRequest' => $baseDir . '/lib/unstable/Security/Signature/Model/IIncomingSignedRequest.php',
+    'NCU\\Security\\Signature\\Model\\IOutgoingSignedRequest' => $baseDir . '/lib/unstable/Security/Signature/Model/IOutgoingSignedRequest.php',
+    'NCU\\Security\\Signature\\Model\\ISignatory' => $baseDir . '/lib/unstable/Security/Signature/Model/ISignatory.php',
+    'NCU\\Security\\Signature\\Model\\ISignedRequest' => $baseDir . '/lib/unstable/Security/Signature/Model/ISignedRequest.php',
+    'NCU\\Security\\Signature\\Model\\SignatoryStatus' => $baseDir . '/lib/unstable/Security/Signature/Model/SignatoryStatus.php',
+    'NCU\\Security\\Signature\\Model\\SignatoryType' => $baseDir . '/lib/unstable/Security/Signature/Model/SignatoryType.php',
+    'NCU\\Security\\Signature\\SignatureAlgorithm' => $baseDir . '/lib/unstable/Security/Signature/SignatureAlgorithm.php',
     'OCP\\Accounts\\IAccount' => $baseDir . '/lib/public/Accounts/IAccount.php',
     'OCP\\Accounts\\IAccountManager' => $baseDir . '/lib/public/Accounts/IAccountManager.php',
     'OCP\\Accounts\\IAccountProperty' => $baseDir . '/lib/public/Accounts/IAccountProperty.php',
@@ -1392,6 +1411,8 @@
     'OC\\Core\\Migrations\\Version30000Date20240814180800' => $baseDir . '/core/Migrations/Version30000Date20240814180800.php',
     'OC\\Core\\Migrations\\Version30000Date20240815080800' => $baseDir . '/core/Migrations/Version30000Date20240815080800.php',
     'OC\\Core\\Migrations\\Version30000Date20240906095113' => $baseDir . '/core/Migrations/Version30000Date20240906095113.php',
+    'OC\\Core\\Migrations\\Version31000Date20240101084401' => $baseDir . '/core/Migrations/Version31000Date20240101084401.php',
+    'OC\\Core\\Migrations\\Version31000Date20240814184402' => $baseDir . '/core/Migrations/Version31000Date20240814184402.php',
     'OC\\Core\\Migrations\\Version31000Date20241018063111' => $baseDir . '/core/Migrations/Version31000Date20241018063111.php',
     'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php',
     'OC\\Core\\ResponseDefinitions' => $baseDir . '/core/ResponseDefinitions.php',

diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php
@@ -53,6 +53,25 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'NCU\\Config\\Exceptions\\UnknownKeyException' => __DIR__ . '/../../..' . '/lib/unstable/Config/Exceptions/UnknownKeyException.php',
         'NCU\\Config\\IUserConfig' => __DIR__ . '/../../..' . '/lib/unstable/Config/IUserConfig.php',
         'NCU\\Config\\ValueType' => __DIR__ . '/../../..' . '/lib/unstable/Config/ValueType.php',
+        'NCU\\Security\\Signature\\Exceptions\\IdentityNotFoundException' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/Exceptions/IdentityNotFoundException.php',
+        'NCU\\Security\\Signature\\Exceptions\\IncomingRequestException' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/Exceptions/IncomingRequestException.php',
+        'NCU\\Security\\Signature\\Exceptions\\InvalidKeyOriginException' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/Exceptions/InvalidKeyOriginException.php',
+        'NCU\\Security\\Signature\\Exceptions\\InvalidSignatureException' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/Exceptions/InvalidSignatureException.php',
+        'NCU\\Security\\Signature\\Exceptions\\SignatoryConflictException' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/Exceptions/SignatoryConflictException.php',
+        'NCU\\Security\\Signature\\Exceptions\\SignatoryException' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/Exceptions/SignatoryException.php',
+        'NCU\\Security\\Signature\\Exceptions\\SignatoryNotFoundException' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/Exceptions/SignatoryNotFoundException.php',
+        'NCU\\Security\\Signature\\Exceptions\\SignatureElementNotFoundException' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/Exceptions/SignatureElementNotFoundException.php',
+        'NCU\\Security\\Signature\\Exceptions\\SignatureException' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/Exceptions/SignatureException.php',
+        'NCU\\Security\\Signature\\Exceptions\\SignatureNotFoundException' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/Exceptions/SignatureNotFoundException.php',
+        'NCU\\Security\\Signature\\ISignatoryManager' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/ISignatoryManager.php',
+        'NCU\\Security\\Signature\\ISignatureManager' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/ISignatureManager.php',
+        'NCU\\Security\\Signature\\Model\\IIncomingSignedRequest' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/Model/IIncomingSignedRequest.php',
+        'NCU\\Security\\Signature\\Model\\IOutgoingSignedRequest' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/Model/IOutgoingSignedRequest.php',
+        'NCU\\Security\\Signature\\Model\\ISignatory' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/Model/ISignatory.php',
+        'NCU\\Security\\Signature\\Model\\ISignedRequest' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/Model/ISignedRequest.php',
+        'NCU\\Security\\Signature\\Model\\SignatoryStatus' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/Model/SignatoryStatus.php',
+        'NCU\\Security\\Signature\\Model\\SignatoryType' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/Model/SignatoryType.php',
+        'NCU\\Security\\Signature\\SignatureAlgorithm' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/SignatureAlgorithm.php',
         'OCP\\Accounts\\IAccount' => __DIR__ . '/../../..' . '/lib/public/Accounts/IAccount.php',
         'OCP\\Accounts\\IAccountManager' => __DIR__ . '/../../..' . '/lib/public/Accounts/IAccountManager.php',
         'OCP\\Accounts\\IAccountProperty' => __DIR__ . '/../../..' . '/lib/public/Accounts/IAccountProperty.php',
@@ -1433,6 +1452,8 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\Core\\Migrations\\Version30000Date20240814180800' => __DIR__ . '/../../..' . '/core/Migrations/Version30000Date20240814180800.php',
         'OC\\Core\\Migrations\\Version30000Date20240815080800' => __DIR__ . '/../../..' . '/core/Migrations/Version30000Date20240815080800.php',
         'OC\\Core\\Migrations\\Version30000Date20240906095113' => __DIR__ . '/../../..' . '/core/Migrations/Version30000Date20240906095113.php',
+        'OC\\Core\\Migrations\\Version31000Date20240101084401' => __DIR__ . '/../../..' . '/core/Migrations/Version31000Date20240101084401.php',
+        'OC\\Core\\Migrations\\Version31000Date20240814184402' => __DIR__ . '/../../..' . '/core/Migrations/Version31000Date20240814184402.php',
         'OC\\Core\\Migrations\\Version31000Date20241018063111' => __DIR__ . '/../../..' . '/core/Migrations/Version31000Date20241018063111.php',
         'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php',
         'OC\\Core\\ResponseDefinitions' => __DIR__ . '/../../..' . '/core/ResponseDefinitions.php',

diff --git a/lib/private/Federation/CloudFederationProviderManager.php b/lib/private/Federation/CloudFederationProviderManager.php
@@ -226,6 +226,12 @@ private function postOcmPayload(string $cloudId, string $uri, string $payload, ?
 	 */
 	private function prepareOcmPayload(string $uri, string $payload): array {
 		$payload = array_merge($this->getDefaultRequestOptions(), ['body' => $payload]);
+
+		if ($this->appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_ENFORCED, lazy: true) &&
+			$this->signatoryManager->getRemoteSignatory($this->signatureManager->extractIdentityFromUri($uri)) === null) {
+			return $payload;
+		}
+
 		if (!$this->appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_DISABLED, lazy: true)) {
 			$signedPayload = $this->signatureManager->signOutgoingRequestIClientPayload(
 				$this->signatoryManager,

diff --git a/lib/private/OCM/OCMSignatoryManager.php b/lib/private/OCM/OCMSignatoryManager.php
@@ -6,19 +6,20 @@
  * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
+
 namespace OC\OCM;
 
 use NCU\Security\Signature\Exceptions\IdentityNotFoundException;
 use NCU\Security\Signature\ISignatoryManager;
 use NCU\Security\Signature\ISignatureManager;
-use NCU\Security\Signature\Model\IIncomingSignedRequest;
 use NCU\Security\Signature\Model\ISignatory;
 use NCU\Security\Signature\Model\SignatoryType;
 use OC\Security\IdentityProof\Manager;
 use OC\Security\Signature\Model\Signatory;
 use OCP\IAppConfig;
 use OCP\IURLGenerator;
 use OCP\OCM\Exceptions\OCMProviderException;
+use Psr\Log\LoggerInterface;
 
 /**
  * @inheritDoc
@@ -40,14 +41,15 @@ public function __construct(
 		private readonly IURLGenerator $urlGenerator,
 		private readonly Manager $identityProofManager,
 		private readonly OCMDiscoveryService $ocmDiscoveryService,
+		private readonly LoggerInterface $logger,
 	) {
 	}
 
 	/**
 	 * @inheritDoc
 	 *
-	 * @since 31.0.0
 	 * @return string
+	 * @since 31.0.0
 	 */
 	public function getProviderId(): string {
 		return self::PROVIDER_ID;
@@ -56,8 +58,8 @@ public function getProviderId(): string {
 	/**
 	 * @inheritDoc
 	 *
-	 * @since 31.0.0
 	 * @return array
+	 * @since 31.0.0
 	 */
 	public function getOptions(): array {
 		return [];
@@ -121,14 +123,18 @@ private function generateKeyId(): string {
 	/**
 	 * @inheritDoc
 	 *
-	 * @param IIncomingSignedRequest $signedRequest
+	 * @param string $remote
 	 *
 	 * @return ISignatory|null must be NULL if no signatory is found
-	 * @throws OCMProviderException on fail to discover ocm services
 	 * @since 31.0.0
 	 */
-	public function getRemoteSignatory(IIncomingSignedRequest $signedRequest): ?ISignatory {
-		return $this->getRemoteSignatoryFromHost($signedRequest->getOrigin());
+	public function getRemoteSignatory(string $remote): ?ISignatory {
+		try {
+			return $this->getRemoteSignatoryFromHost($remote);
+		} catch (OCMProviderException $e) {
+			$this->logger->warning('fail to get remote signatory', ['exception' => $e, 'remote' => $remote]);
+			return null;
+		}
 	}
 
 	/**