Skip to content

Commit 413466f

Browse files
ArtificialOwlArtificialOwl
committed
fix(ocm): switching to IdentityProof
Signed-off-by: Maxence Lange <[email protected]>
1 parent 4b9f22a commit 413466f

File tree

24 files changed

+171
-647
lines changed

24 files changed

+171
-647
lines changed

‎apps/cloud_federation_api/lib/Capabilities.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*/
99
namespace OCA\CloudFederationAPI;
1010

11-
use NCU\Security\PublicPrivateKeyPairs\Exceptions\KeyPairException;
11+
use NCU\Security\Signature\Exceptions\IdentityNotFoundException;
1212
use NCU\Security\Signature\Exceptions\SignatoryException;
1313
use OC\OCM\OCMSignatoryManager;
1414
use OCP\Capabilities\ICapability;
@@ -79,7 +79,7 @@ public function getCapabilities() {
7979
} else {
8080
$this->logger->debug('ocm public key feature disabled');
8181
}
82-
} catch (SignatoryException|KeyPairException $e) {
82+
} catch (SignatoryException|IdentityNotFoundException $e) {
8383
$this->logger->warning('cannot generate local signatory', ['exception' => $e]);
8484
}
8585

‎apps/cloud_federation_api/lib/Controller/RequestHandlerController.php

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
*/
66
namespace OCA\CloudFederationAPI\Controller;
77

8+
use NCU\Security\Signature\Exceptions\IdentityNotFoundException;
89
use NCU\Security\Signature\Exceptions\IncomingRequestException;
910
use NCU\Security\Signature\Exceptions\SignatoryNotFoundException;
1011
use NCU\Security\Signature\Exceptions\SignatureException;
@@ -14,6 +15,7 @@
1415
use OC\OCM\OCMSignatoryManager;
1516
use OCA\CloudFederationAPI\Config;
1617
use OCA\CloudFederationAPI\ResponseDefinitions;
18+
use OCA\FederatedFileSharing\AddressHandler;
1719
use OCP\AppFramework\Controller;
1820
use OCP\AppFramework\Http;
1921
use OCP\AppFramework\Http\Attribute\BruteForceProtection;
@@ -60,6 +62,7 @@ public function __construct(
6062
private IURLGenerator $urlGenerator,
6163
private ICloudFederationProviderManager $cloudFederationProviderManager,
6264
private Config $config,
65+
private readonly AddressHandler $addressHandler,
6366
private readonly IAppConfig $appConfig,
6467
private ICloudFederationFactory $factory,
6568
private ICloudIdManager $cloudIdManager,
@@ -289,6 +292,7 @@ public function receiveNotification($notificationType, $resourceType, $providerI
289292
$response->throttle();
290293
return $response;
291294
} catch (\Exception $e) {
295+
$this->logger->warning('incoming notification exception', ['exception' => $e]);
292296
return new JSONResponse(
293297
[
294298
'message' => 'Internal error at ' . $this->urlGenerator->getBaseUrl(),
@@ -376,7 +380,7 @@ private function confirmSignedOrigin(?IIncomingSignedRequest $signedRequest, str
376380
$body = json_decode($signedRequest->getBody(), true) ?? [];
377381
$entry = trim($body[$key] ?? '', '@');
378382
if ($this->getHostFromFederationId($entry) !== $signedRequest->getOrigin()) {
379-
throw new IncomingRequestException('share initiation from different instance');
383+
throw new IncomingRequestException('share initiation (' . $signedRequest->getOrigin() . ') from different instance (' . $entry . ') [key=' . $key . ']');
380384
}
381385
}
382386

@@ -391,7 +395,6 @@ private function confirmSignedOrigin(?IIncomingSignedRequest $signedRequest, str
391395
* @param IIncomingSignedRequest|null $signedRequest
392396
* @param string $token
393397
*
394-
* @return void
395398
* @throws IncomingRequestException
396399
*/
397400
private function confirmShareOrigin(?IIncomingSignedRequest $signedRequest, string $token): void {
@@ -401,8 +404,23 @@ private function confirmShareOrigin(?IIncomingSignedRequest $signedRequest, stri
401404

402405
$provider = $this->shareProviderFactory->getProviderForType(IShare::TYPE_REMOTE);
403406
$share = $provider->getShareByToken($token);
404-
$entry = $share->getSharedWith();
407+
try {
408+
$this->confirmShareEntry($signedRequest, $share->getSharedWith());
409+
} catch (IncomingRequestException) {
410+
// notification might come from the instance that owns the share
411+
$this->logger->debug('could not confirm origin on sharedWith (' . $share->getSharedWIth() . '); going with shareOwner (' . $share->getShareOwner() . ')');
412+
$this->confirmShareEntry($signedRequest, $share->getShareOwner());
413+
}
414+
}
405415

416+
/**
417+
* @param IIncomingSignedRequest|null $signedRequest
418+
* @param string $entry
419+
*
420+
* @return void
421+
* @throws IncomingRequestException
422+
*/
423+
private function confirmShareEntry(?IIncomingSignedRequest $signedRequest, string $entry): void {
406424
$instance = $this->getHostFromFederationId($entry);
407425
if ($signedRequest === null) {
408426
try {
@@ -412,7 +430,7 @@ private function confirmShareOrigin(?IIncomingSignedRequest $signedRequest, stri
412430
return;
413431
}
414432
} elseif ($instance !== $signedRequest->getOrigin()) {
415-
throw new IncomingRequestException('token sharedWith from different instance');
433+
throw new IncomingRequestException('token sharedWith (' . $instance . ') not linked to origin (' . $signedRequest->getOrigin() . ')');
416434
}
417435
}
418436

@@ -423,20 +441,16 @@ private function confirmShareOrigin(?IIncomingSignedRequest $signedRequest, stri
423441
*/
424442
private function getHostFromFederationId(string $entry): string {
425443
if (!str_contains($entry, '@')) {
426-
throw new IncomingRequestException('entry does not contains @');
444+
throw new IncomingRequestException('entry ' . $entry . ' does not contains @');
427445
}
428-
[, $rightPart] = explode('@', $entry, 2);
446+
$rightPart = substr($entry, strrpos($entry, '@') + 1);
429447

430-
$host = parse_url($rightPart, PHP_URL_HOST);
431-
$port = parse_url($rightPart, PHP_URL_PORT);
432-
if ($port !== null && $port !== false) {
433-
$host .= ':' . $port;
434-
}
435-
436-
if (is_string($host) && $host !== '') {
437-
return $host;
448+
// in case the full scheme is sent; getting rid of it
449+
$rightPart = $this->addressHandler->removeProtocolFromUrl($rightPart);
450+
try {
451+
return $this->signatureManager->extractIdentityFromUri('https://' . $rightPart);
452+
} catch (IdentityNotFoundException) {
453+
throw new IncomingRequestException('invalid host within federation id: ' . $entry);
438454
}
439-
440-
throw new IncomingRequestException('host is empty');
441455
}
442456
}

‎apps/federatedfilesharing/lib/FederatedShareProvider.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,8 @@ protected function askOwnerToReShare($shareWith, IShare $share, $shareId) {
250250
$remote,
251251
$shareWith,
252252
$share->getPermissions(),
253-
$share->getNode()->getName()
253+
$share->getNode()->getName(),
254+
$share->getShareType(),
254255
);
255256

256257
return [$token, $remoteId];

‎apps/federatedfilesharing/lib/Notifications.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,12 +108,13 @@ public function sendRemoteShare($token, $shareWith, $name, $remoteId, $owner, $o
108108
* @throws HintException
109109
* @throws \OC\ServerNotAvailableException
110110
*/
111-
public function requestReShare($token, $id, $shareId, $remote, $shareWith, $permission, $filename) {
111+
public function requestReShare($token, $id, $shareId, $remote, $shareWith, $permission, $filename, $shareType) {
112112
$fields = [
113113
'shareWith' => $shareWith,
114114
'token' => $token,
115115
'permission' => $permission,
116116
'remoteId' => $shareId,
117+
'shareType' => $shareType,
117118
];
118119

119120
$ocmFields = $fields;

‎build/integration/features/bootstrap/FederationContext.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public function startFederatedServer() {
3838

3939
$port = getenv('PORT_FED');
4040

41-
self::$phpFederatedServerPid = exec('php -S localhost:' . $port . ' -t ../../ >/dev/null & echo $!');
41+
self::$phpFederatedServerPid = exec('PHP_CLI_SERVER_WORKERS=2 php -S localhost:' . $port . ' -t ../../ >/dev/null & echo $!');
4242
}
4343

4444
/**

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

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,27 @@ Feature: cleanup-remote-storage
44
Background:
55
Given using api version "1"
66

7+
Scenario: cleanup remote storage with no storage
8+
Given Using server "LOCAL"
9+
And user "user0" exists
10+
Given Using server "REMOTE"
11+
And user "user1" exists
12+
# Rename file so it has a unique name in the target server (as the target
13+
# server may have its own /textfile0.txt" file)
14+
And User "user1" copies file "/textfile0.txt" to "/remote-share.txt"
15+
And User "user1" from server "REMOTE" shares "/remote-share.txt" with user "user0" from server "LOCAL"
16+
And As an "user1"
17+
And Deleting last share
18+
And the OCS status code should be "100"
19+
And the HTTP status code should be "200"
20+
And Deleting last share
21+
And Using server "LOCAL"
22+
When invoking occ with "sharing:cleanup-remote-storage"
23+
Then the command was successful
24+
And the command output contains the text "0 remote storage(s) need(s) to be checked"
25+
And the command output contains the text "0 remote share(s) exist"
26+
And the command output contains the text "no storages deleted"
27+
728
Scenario: cleanup remote storage with active storages
829
Given Using server "LOCAL"
930
And user "user0" exists
@@ -35,9 +56,6 @@ Feature: cleanup-remote-storage
3556
# server may have its own /textfile0.txt" file)
3657
And User "user1" copies file "/textfile0.txt" to "/remote-share.txt"
3758
And User "user1" from server "REMOTE" shares "/remote-share.txt" with user "user0" from server "LOCAL"
38-
And As an "user1"
39-
And sending "GET" to "/apps/files_sharing/api/v1/shares"
40-
And the list of returned shares has 1 shares
4159
And Using server "LOCAL"
4260
# Accept and download the file to ensure that a storage is created for the
4361
# federated share

‎lib/composer/composer/autoload_classmap.php

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1901,8 +1901,6 @@
19011901
'OC\\Security\\Ip\\Range' => $baseDir . '/lib/private/Security/Ip/Range.php',
19021902
'OC\\Security\\Ip\\RemoteAddress' => $baseDir . '/lib/private/Security/Ip/RemoteAddress.php',
19031903
'OC\\Security\\Normalizer\\IpAddress' => $baseDir . '/lib/private/Security/Normalizer/IpAddress.php',
1904-
'OC\\Security\\PublicPrivateKeyPairs\\KeyPairManager' => $baseDir . '/lib/private/Security/PublicPrivateKeyPairs/KeyPairManager.php',
1905-
'OC\\Security\\PublicPrivateKeyPairs\\Model\\KeyPair' => $baseDir . '/lib/private/Security/PublicPrivateKeyPairs/Model/KeyPair.php',
19061904
'OC\\Security\\RateLimiting\\Backend\\DatabaseBackend' => $baseDir . '/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php',
19071905
'OC\\Security\\RateLimiting\\Backend\\IBackend' => $baseDir . '/lib/private/Security/RateLimiting/Backend/IBackend.php',
19081906
'OC\\Security\\RateLimiting\\Backend\\MemoryCacheBackend' => $baseDir . '/lib/private/Security/RateLimiting/Backend/MemoryCacheBackend.php',

‎lib/composer/composer/autoload_static.php

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1942,8 +1942,6 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
19421942
'OC\\Security\\Ip\\Range' => __DIR__ . '/../../..' . '/lib/private/Security/Ip/Range.php',
19431943
'OC\\Security\\Ip\\RemoteAddress' => __DIR__ . '/../../..' . '/lib/private/Security/Ip/RemoteAddress.php',
19441944
'OC\\Security\\Normalizer\\IpAddress' => __DIR__ . '/../../..' . '/lib/private/Security/Normalizer/IpAddress.php',
1945-
'OC\\Security\\PublicPrivateKeyPairs\\KeyPairManager' => __DIR__ . '/../../..' . '/lib/private/Security/PublicPrivateKeyPairs/KeyPairManager.php',
1946-
'OC\\Security\\PublicPrivateKeyPairs\\Model\\KeyPair' => __DIR__ . '/../../..' . '/lib/private/Security/PublicPrivateKeyPairs/Model/KeyPair.php',
19471945
'OC\\Security\\RateLimiting\\Backend\\DatabaseBackend' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php',
19481946
'OC\\Security\\RateLimiting\\Backend\\IBackend' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Backend/IBackend.php',
19491947
'OC\\Security\\RateLimiting\\Backend\\MemoryCacheBackend' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Backend/MemoryCacheBackend.php',

‎lib/private/Federation/CloudFederationProviderManager.php

Lines changed: 45 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use OCP\Federation\ICloudFederationProviderManager;
1919
use OCP\Federation\ICloudFederationShare;
2020
use OCP\Federation\ICloudIdManager;
21+
use OCP\Http\Client\IClient;
2122
use OCP\Http\Client\IClientService;
2223
use OCP\Http\Client\IResponse;
2324
use OCP\IAppConfig;
@@ -105,25 +106,11 @@ public function getCloudFederationProvider($resourceType) {
105106
public function sendShare(ICloudFederationShare $share) {
106107
$cloudID = $this->cloudIdManager->resolveCloudId($share->getShareWith());
107108
try {
108-
$ocmProvider = $this->discoveryService->discover($cloudID->getRemote());
109-
} catch (OCMProviderException $e) {
110-
return false;
111-
}
112-
113-
$client = $this->httpClientService->newClient();
114-
try {
115-
// signing the payload using OCMSignatoryManager before initializing the request
116-
$uri = $ocmProvider->getEndPoint() . '/shares';
117-
$payload = array_merge($this->getDefaultRequestOptions(), ['body' => json_encode($share->getShare())]);
118-
if (!$this->appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_DISABLED, lazy: true)) {
119-
$signedPayload = $this->signatureManager->signOutgoingRequestIClientPayload(
120-
$this->signatoryManager,
121-
$payload,
122-
'post', $uri
123-
);
109+
try {
110+
$response = $this->postOcmPayload($cloudID->getRemote(), '/shares', json_encode($share->getShare()));
111+
} catch (OCMProviderException) {
112+
return false;
124113
}
125-
$response = $client->post($uri, $signedPayload ?? $payload);
126-
127114
if ($response->getStatusCode() === Http::STATUS_CREATED) {
128115
$result = json_decode($response->getBody(), true);
129116
return (is_array($result)) ? $result : [];
@@ -149,22 +136,9 @@ public function sendShare(ICloudFederationShare $share) {
149136
*/
150137
public function sendCloudShare(ICloudFederationShare $share): IResponse {
151138
$cloudID = $this->cloudIdManager->resolveCloudId($share->getShareWith());
152-
$ocmProvider = $this->discoveryService->discover($cloudID->getRemote());
153-
154139
$client = $this->httpClientService->newClient();
155140
try {
156-
// signing the payload using OCMSignatoryManager before initializing the request
157-
$uri = $ocmProvider->getEndPoint() . '/shares';
158-
$payload = array_merge($this->getDefaultRequestOptions(), ['body' => json_encode($share->getShare())]);
159-
if (!$this->appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_DISABLED, lazy: true)) {
160-
$signedPayload = $this->signatureManager->signOutgoingRequestIClientPayload(
161-
$this->signatoryManager,
162-
$payload,
163-
'post', $uri
164-
);
165-
}
166-
167-
return $client->post($uri, $signedPayload ?? $payload);
141+
return $this->postOcmPayload($cloudID->getRemote(), '/shares', json_encode($share->getShare()), $client);
168142
} catch (\Throwable $e) {
169143
$this->logger->error('Error while sending share to federation server: ' . $e->getMessage(), ['exception' => $e]);
170144
try {
@@ -183,26 +157,11 @@ public function sendCloudShare(ICloudFederationShare $share): IResponse {
183157
*/
184158
public function sendNotification($url, ICloudFederationNotification $notification) {
185159
try {
186-
$ocmProvider = $this->discoveryService->discover($url);
187-
} catch (OCMProviderException $e) {
188-
return false;
189-
}
190-
191-
$client = $this->httpClientService->newClient();
192-
try {
193-
194-
// signing the payload using OCMSignatoryManager before initializing the request
195-
$uri = $ocmProvider->getEndPoint() . '/notifications';
196-
$payload = array_merge($this->getDefaultRequestOptions(), ['body' => json_encode($notification->getMessage())]);
197-
if (!$this->appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_DISABLED, lazy: true)) {
198-
$signedPayload = $this->signatureManager->signOutgoingRequestIClientPayload(
199-
$this->signatoryManager,
200-
$payload,
201-
'post', $uri
202-
);
160+
try {
161+
$response = $this->postOcmPayload($url, '/notifications', json_encode($notification->getMessage()));
162+
} catch (OCMProviderException) {
163+
return false;
203164
}
204-
$response = $client->post($uri, $signedPayload ?? $payload);
205-
206165
if ($response->getStatusCode() === Http::STATUS_CREATED) {
207166
$result = json_decode($response->getBody(), true);
208167
return (is_array($result)) ? $result : [];
@@ -222,21 +181,9 @@ public function sendNotification($url, ICloudFederationNotification $notificatio
222181
* @throws OCMProviderException
223182
*/
224183
public function sendCloudNotification(string $url, ICloudFederationNotification $notification): IResponse {
225-
$ocmProvider = $this->discoveryService->discover($url);
226-
227184
$client = $this->httpClientService->newClient();
228185
try {
229-
// signing the payload using OCMSignatoryManager before initializing the request
230-
$uri = $ocmProvider->getEndPoint() . '/notifications';
231-
$payload = array_merge($this->getDefaultRequestOptions(), ['body' => json_encode($notification->getMessage())]);
232-
if (!$this->appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_DISABLED, lazy: true)) {
233-
$signedPayload = $this->signatureManager->signOutgoingRequestIClientPayload(
234-
$this->signatoryManager,
235-
$payload,
236-
'post', $uri
237-
);
238-
}
239-
return $client->post($uri, $signedPayload ?? $payload);
186+
return $this->postOcmPayload($url, '/notifications', json_encode($notification->getMessage()), $client);
240187
} catch (\Throwable $e) {
241188
$this->logger->error('Error while sending notification to federation server: ' . $e->getMessage(), ['exception' => $e]);
242189
try {
@@ -256,6 +203,40 @@ public function isReady() {
256203
return $this->appManager->isEnabledForUser('cloud_federation_api');
257204
}
258205

206+
/**
207+
* @param string $cloudId
208+
* @param string $uri
209+
* @param string $payload
210+
*
211+
* @return IResponse
212+
* @throws OCMProviderException
213+
*/
214+
private function postOcmPayload(string $cloudId, string $uri, string $payload, ?IClient $client = null): IResponse {
215+
$ocmProvider = $this->discoveryService->discover($cloudId);
216+
$uri = $ocmProvider->getEndPoint() . '/' . ltrim($uri, '/');
217+
$client = $client ?? $this->httpClientService->newClient();
218+
return $client->post($uri, $this->prepareOcmPayload($uri, $payload));
219+
}
220+
221+
/**
222+
* @param string $uri
223+
* @param string $payload
224+
*
225+
* @return array
226+
*/
227+
private function prepareOcmPayload(string $uri, string $payload): array {
228+
$payload = array_merge($this->getDefaultRequestOptions(), ['body' => $payload]);
229+
if (!$this->appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_DISABLED, lazy: true)) {
230+
$signedPayload = $this->signatureManager->signOutgoingRequestIClientPayload(
231+
$this->signatoryManager,
232+
$payload,
233+
'post', $uri
234+
);
235+
}
236+
237+
return $signedPayload ?? $payload;
238+
}
239+
259240
private function getDefaultRequestOptions(): array {
260241
return [
261242
'headers' => ['content-type' => 'application/json'],

0 commit comments

Comments
 (0)