Merge pull request #47340 from nextcloud/fix/federation-certificate-store

fix(federation): Do not overwrite certificate bundle

3 files changed, 46 insertions, 43 deletions

diff --git a/apps/files_sharing/lib/External/Storage.php b/apps/files_sharing/lib/External/Storage.php
index bfaf9a99442..ba237f6c5ef 100644
--- a/apps/files_sharing/lib/External/Storage.php
+++ b/apps/files_sharing/lib/External/Storage.php
@@ -259,19 +259,12 @@ class Storage extends DAV implements ISharedStorage, IDisableEncryptionStorage,
 
 		$client = $this->httpClient->newClient();
 		try {
-			$result = $client->get($url, [
-				'timeout' => 10,
-				'connect_timeout' => 10,
-				'verify' => !$this->config->getSystemValueBool('sharing.federation.allowSelfSignedCertificates', false),
-			])->getBody();
+			$result = $client->get($url, $this->getDefaultRequestOptions())->getBody();
 			$data = json_decode($result);
 			$returnValue = (is_object($data) && !empty($data->version));
-		} catch (ConnectException $e) {
-			$returnValue = false;
-		} catch (ClientException $e) {
-			$returnValue = false;
-		} catch (RequestException $e) {
+		} catch (ConnectException|ClientException|RequestException $e) {
 			$returnValue = false;
+			$this->logger->warning('Failed to test remote URL', ['exception' => $e]);
 		}
 
 		$cache->set($url, $returnValue, 60 * 60 * 24);
@@ -319,12 +312,11 @@ class Storage extends DAV implements ISharedStorage, IDisableEncryptionStorage,
 		// TODO: DI
 		$client = \OC::$server->getHTTPClientService()->newClient();
 		try {
-			$response = $client->post($url, [
+			$response = $client->post($url, array_merge($this->getDefaultRequestOptions(), [
 				'body' => ['password' => $password, 'depth' => $depth],
-				'timeout' => 10,
-				'connect_timeout' => 10,
-			]);
+			]));
 		} catch (\GuzzleHttp\Exception\RequestException $e) {
+			$this->logger->warning('Failed to fetch share info', ['exception' => $e]);
 			if ($e->getCode() === Http::STATUS_UNAUTHORIZED || $e->getCode() === Http::STATUS_FORBIDDEN) {
 				throw new ForbiddenException();
 			}
@@ -422,4 +414,15 @@ class Storage extends DAV implements ISharedStorage, IDisableEncryptionStorage,
 	public function free_space($path) {
 		return parent::free_space('');
 	}
+
+	private function getDefaultRequestOptions(): array {
+		$options = [
+			'timeout' => 10,
+			'connect_timeout' => 10,
+		];
+		if ($this->config->getSystemValueBool('sharing.federation.allowSelfSignedCertificates')) {
+			$options['verify'] = false;
+		}
+		return $options;
+	}
 }
diff --git a/lib/private/Federation/CloudFederationProviderManager.php b/lib/private/Federation/CloudFederationProviderManager.php
index be9e66fa9ec..bf7648d472b 100644
--- a/lib/private/Federation/CloudFederationProviderManager.php
+++ b/lib/private/Federation/CloudFederationProviderManager.php
@@ -106,13 +106,9 @@ class CloudFederationProviderManager implements ICloudFederationProviderManager
 
 		$client = $this->httpClientService->newClient();
 		try {
-			$response = $client->post($ocmProvider->getEndPoint() . '/shares', [
+			$response = $client->post($ocmProvider->getEndPoint() . '/shares', array_merge($this->getDefaultRequestOptions(), [
 				'body' => json_encode($share->getShare()),
-				'headers' => ['content-type' => 'application/json'],
-				'verify' => !$this->config->getSystemValueBool('sharing.federation.allowSelfSignedCertificates', false),
-				'timeout' => 10,
-				'connect_timeout' => 10,
-			]);
+			]));
 
 			if ($response->getStatusCode() === Http::STATUS_CREATED) {
 				$result = json_decode($response->getBody(), true);
@@ -143,13 +139,9 @@ class CloudFederationProviderManager implements ICloudFederationProviderManager
 
 		$client = $this->httpClientService->newClient();
 		try {
-			return $client->post($ocmProvider->getEndPoint() . '/shares', [
+			return $client->post($ocmProvider->getEndPoint() . '/shares', array_merge($this->getDefaultRequestOptions(), [
 				'body' => json_encode($share->getShare()),
-				'headers' => ['content-type' => 'application/json'],
-				'verify' => !$this->config->getSystemValueBool('sharing.federation.allowSelfSignedCertificates', false),
-				'timeout' => 10,
-				'connect_timeout' => 10,
-			]);
+			]));
 		} catch (\Throwable $e) {
 			$this->logger->error('Error while sending share to federation server: ' . $e->getMessage(), ['exception' => $e]);
 			try {
@@ -175,13 +167,9 @@ class CloudFederationProviderManager implements ICloudFederationProviderManager
 
 		$client = $this->httpClientService->newClient();
 		try {
-			$response = $client->post($ocmProvider->getEndPoint() . '/notifications', [
+			$response = $client->post($ocmProvider->getEndPoint() . '/notifications', array_merge($this->getDefaultRequestOptions(), [
 				'body' => json_encode($notification->getMessage()),
-				'headers' => ['content-type' => 'application/json'],
-				'verify' => !$this->config->getSystemValueBool('sharing.federation.allowSelfSignedCertificates', false),
-				'timeout' => 10,
-				'connect_timeout' => 10,
-			]);
+			]));
 			if ($response->getStatusCode() === Http::STATUS_CREATED) {
 				$result = json_decode($response->getBody(), true);
 				return (is_array($result)) ? $result : [];
@@ -205,13 +193,9 @@ class CloudFederationProviderManager implements ICloudFederationProviderManager
 
 		$client = $this->httpClientService->newClient();
 		try {
-			return $client->post($ocmProvider->getEndPoint() . '/notifications', [
+			return $client->post($ocmProvider->getEndPoint() . '/notifications', array_merge($this->getDefaultRequestOptions(), [
 				'body' => json_encode($notification->getMessage()),
-				'headers' => ['content-type' => 'application/json'],
-				'verify' => !$this->config->getSystemValueBool('sharing.federation.allowSelfSignedCertificates', false),
-				'timeout' => 10,
-				'connect_timeout' => 10,
-			]);
+			]));
 		} catch (\Throwable $e) {
 			$this->logger->error('Error while sending notification to federation server: ' . $e->getMessage(), ['exception' => $e]);
 			try {
@@ -230,4 +214,17 @@ class CloudFederationProviderManager implements ICloudFederationProviderManager
 	public function isReady() {
 		return $this->appManager->isEnabledForUser('cloud_federation_api');
 	}
+
+	private function getDefaultRequestOptions(): array {
+		$options = [
+			'headers' => ['content-type' => 'application/json'],
+			'timeout' => 10,
+			'connect_timeout' => 10,
+		];
+
+		if ($this->config->getSystemValueBool('sharing.federation.allowSelfSignedCertificates')) {
+			$options['verify'] = false;
+		}
+		return $options;
+	}
 }
diff --git a/lib/private/OCM/OCMDiscoveryService.php b/lib/private/OCM/OCMDiscoveryService.php
index 62313a9af80..279162c76f2 100644
--- a/lib/private/OCM/OCMDiscoveryService.php
+++ b/lib/private/OCM/OCMDiscoveryService.php
@@ -66,13 +66,16 @@ class OCMDiscoveryService implements IOCMDiscoveryService {
 
 		$client = $this->clientService->newClient();
 		try {
+			$options = [
+				'timeout' => 10,
+				'connect_timeout' => 10,
+			];
+			if ($this->config->getSystemValueBool('sharing.federation.allowSelfSignedCertificates') === true) {
+				$options['verify'] = false;
+			}
 			$response = $client->get(
 				$remote . '/ocm-provider/',
-				[
-					'timeout' => 10,
-					'verify' => !$this->config->getSystemValueBool('sharing.federation.allowSelfSignedCertificates'),
-					'connect_timeout' => 10,
-				]
+				$options,
 			);
 
 			if ($response->getStatusCode() === Http::STATUS_OK) {