make sure that both server don't try to exchange the shared secret in parallel

3 files changed, 79 insertions, 32 deletions

diff --git a/apps/federation/api/ocsauthapi.php b/apps/federation/api/ocsauthapi.php
index 7965e6f2140..3279ae1dd7b 100644
--- a/apps/federation/api/ocsauthapi.php
+++ b/apps/federation/api/ocsauthapi.php
@@ -91,6 +91,13 @@ class OCSAuthAPI {
 			return new \OC_OCS_Result(null, HTTP::STATUS_FORBIDDEN);
 		}
 
+		// if both server initiated the exchange of the shared secret the greater
+		// token wins
+		$localToken = $this->dbHandler->getToken($url);
+		if (strcmp($localToken, $token) > 0) {
+			return new \OC_OCS_Result(null, HTTP::STATUS_FORBIDDEN);
+		}
+
 		$this->jobList->add(
 			'OCA\Federation\BackgroundJob\GetSharedSecret',
 			[
@@ -123,6 +130,8 @@ class OCSAuthAPI {
 		$sharedSecret = $this->secureRandom->getMediumStrengthGenerator()->generate(32);
 
 		$this->trustedServers->addSharedSecret($url, $sharedSecret);
+		// reset token after the exchange of the shared secret was successful
+		$this->dbHandler->addToken($url, '');
 
 		return new \OC_OCS_Result(['sharedSecret' => $sharedSecret], Http::STATUS_OK);
 
diff --git a/apps/federation/backgroundjob/getsharedsecret.php b/apps/federation/backgroundjob/getsharedsecret.php
index 3e8597cd78b..dd095ed117a 100644
--- a/apps/federation/backgroundjob/getsharedsecret.php
+++ b/apps/federation/backgroundjob/getsharedsecret.php
@@ -22,6 +22,7 @@
 
 namespace OCA\Federation\BackgroundJob;
 
+use GuzzleHttp\Exception\ClientException;
 use OC\BackgroundJob\QueuedJob;
 use OCA\Federation\DbHandler;
 use OCA\Federation\TrustedServers;
@@ -52,6 +53,9 @@ class GetSharedSecret extends QueuedJob{
 	/** @var TrustedServers  */
 	private $trustedServers;
 
+	/** @var DbHandler */
+	private $dbHandler;
+
 	/** @var ILogger */
 	private $logger;
 
@@ -65,23 +69,26 @@ class GetSharedSecret extends QueuedJob{
 	 * @param IJobList $jobList
 	 * @param TrustedServers $trustedServers
 	 * @param ILogger $logger
+	 * @param DbHandler $dbHandler
 	 */
 	public function __construct(
 		IClient $httpClient = null,
 		IURLGenerator $urlGenerator = null,
 		IJobList $jobList = null,
 		TrustedServers $trustedServers = null,
-		ILogger $logger = null
+		ILogger $logger = null,
+		dbHandler $dbHandler = null
 	) {
 		$this->logger = $logger ? $logger : \OC::$server->getLogger();
 		$this->httpClient = $httpClient ? $httpClient : \OC::$server->getHTTPClientService()->newClient();
 		$this->jobList = $jobList ? $jobList : \OC::$server->getJobList();
 		$this->urlGenerator = $urlGenerator ? $urlGenerator : \OC::$server->getURLGenerator();
+		$this->dbHandler = $dbHandler ? $dbHandler : new DbHandler(\OC::$server->getDatabaseConnection(), \OC::$server->getL10N('federation'));
 		if ($trustedServers) {
 			$this->trustedServers = $trustedServers;
 		} else {
 			$this->trustedServers = new TrustedServers(
-					new DbHandler(\OC::$server->getDatabaseConnection(), \OC::$server->getL10N('federation')),
+					$this->dbHandler,
 					\OC::$server->getHTTPClientService(),
 					\OC::$server->getLogger(),
 					$this->jobList,
@@ -112,20 +119,25 @@ class GetSharedSecret extends QueuedJob{
 		$source = rtrim($source, '/');
 		$token = $argument['token'];
 
-		$result = $this->httpClient->get(
-			$target . $this->endPoint,
-			[
-				'query' =>
-					[
-						'url' => $source,
-						'token' => $token
-					],
-				'timeout' => 3,
-				'connect_timeout' => 3,
-			]
-		);
-
-		$status = $result->getStatusCode();
+		try {
+			$result = $this->httpClient->get(
+				$target . $this->endPoint,
+				[
+					'query' =>
+						[
+							'url' => $source,
+							'token' => $token
+						],
+					'timeout' => 3,
+					'connect_timeout' => 3,
+				]
+			);
+
+			$status = $result->getStatusCode();
+
+		} catch (ClientException $e) {
+			$status = $e->getCode();
+		}
 
 		// if we received a unexpected response we try again later
 		if (
@@ -133,10 +145,15 @@ class GetSharedSecret extends QueuedJob{
 			&& $status !== Http::STATUS_FORBIDDEN
 		) {
 			$this->jobList->add(
-				'OCA\Federation\Backgroundjob\RequestSharedSecret',
+				'OCA\Federation\Backgroundjob\GetSharedSecret',
 				$argument
 			);
-		} elseif ($status === Http::STATUS_OK) {
+		}  else {
+			// reset token if we received a valid response
+			$this->dbHandler->addToken($target, '');
+		}
+
+		if ($status === Http::STATUS_OK) {
 			$body = $result->getBody();
 			$result = json_decode($body, true);
 			if (isset($result['ocs']['data']['sharedSecret'])) {
@@ -152,5 +169,6 @@ class GetSharedSecret extends QueuedJob{
 				$this->trustedServers->setServerStatus($target, TrustedServers::STATUS_FAILURE);
 			}
 		}
+
 	}
 }
diff --git a/apps/federation/backgroundjob/requestsharedsecret.php b/apps/federation/backgroundjob/requestsharedsecret.php
index b73b620b4b7..c945757818d 100644
--- a/apps/federation/backgroundjob/requestsharedsecret.php
+++ b/apps/federation/backgroundjob/requestsharedsecret.php
@@ -23,6 +23,7 @@
 namespace OCA\Federation\BackgroundJob;
 
 
+use GuzzleHttp\Exception\ClientException;
 use OC\BackgroundJob\QueuedJob;
 use OCA\Federation\DbHandler;
 use OCA\Federation\TrustedServers;
@@ -50,6 +51,12 @@ class RequestSharedSecret extends QueuedJob {
 	/** @var IURLGenerator */
 	private $urlGenerator;
 
+	/** @var DbHandler */
+	private $dbHandler;
+
+	/** @var TrustedServers */
+	private $trustedServers;
+
 	private $endPoint = '/ocs/v2.php/apps/federation/api/v1/request-shared-secret?format=json';
 
 	/**
@@ -59,21 +66,24 @@ class RequestSharedSecret extends QueuedJob {
 	 * @param IURLGenerator $urlGenerator
 	 * @param IJobList $jobList
 	 * @param TrustedServers $trustedServers
+	 * @param DbHandler $dbHandler
 	 */
 	public function __construct(
 		IClient $httpClient = null,
 		IURLGenerator $urlGenerator = null,
 		IJobList $jobList = null,
-		TrustedServers $trustedServers = null
+		TrustedServers $trustedServers = null,
+		dbHandler $dbHandler = null
 	) {
 		$this->httpClient = $httpClient ? $httpClient : \OC::$server->getHTTPClientService()->newClient();
 		$this->jobList = $jobList ? $jobList : \OC::$server->getJobList();
 		$this->urlGenerator = $urlGenerator ? $urlGenerator : \OC::$server->getURLGenerator();
+		$this->dbHandler = $dbHandler ? $dbHandler : new DbHandler(\OC::$server->getDatabaseConnection(), \OC::$server->getL10N('federation'));
 		if ($trustedServers) {
 			$this->trustedServers = $trustedServers;
 		} else {
 			$this->trustedServers = new TrustedServers(
-				new DbHandler(\OC::$server->getDatabaseConnection(), \OC::$server->getL10N('federation')),
+				$this->dbHandler,
 				\OC::$server->getHTTPClientService(),
 				\OC::$server->getLogger(),
 				$this->jobList,
@@ -106,19 +116,24 @@ class RequestSharedSecret extends QueuedJob {
 		$source = rtrim($source, '/');
 		$token = $argument['token'];
 
-		$result = $this->httpClient->post(
-			$target . $this->endPoint,
-			[
-				'body' => [
-					'url' => $source,
-					'token' => $token,
-				],
-				'timeout' => 3,
-				'connect_timeout' => 3,
-			]
-		);
+		try {
+			$result = $this->httpClient->post(
+				$target . $this->endPoint,
+				[
+					'body' => [
+						'url' => $source,
+						'token' => $token,
+					],
+					'timeout' => 3,
+					'connect_timeout' => 3,
+				]
+			);
 
-		$status = $result->getStatusCode();
+			$status = $result->getStatusCode();
+
+		} catch (ClientException $e) {
+			$status = $e->getCode();
+		}
 
 		// if we received a unexpected response we try again later
 		if (
@@ -131,5 +146,10 @@ class RequestSharedSecret extends QueuedJob {
 			);
 		}
 
+		if ($status === Http::STATUS_FORBIDDEN) {
+			// clear token if remote server refuses to ask for shared secret
+			$this->dbHandler->addToken($target, '');
+		}
+
 	}
 }