Skip to content

Commit

Permalink
feat(ocm): signing share requests
Browse files Browse the repository at this point in the history
Signed-off-by: Maxence Lange <[email protected]>
  • Loading branch information
ArtificialOwl committed Oct 30, 2024
1 parent 91fd814 commit 9d5986b
Show file tree
Hide file tree
Showing 43 changed files with 3,185 additions and 73 deletions.
14 changes: 13 additions & 1 deletion apps/cloud_federation_api/lib/Capabilities.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,22 @@

namespace OCA\CloudFederationAPI;

use OC\OCM\OCMSignatoryManager;
use OCP\Capabilities\ICapability;
use OCP\IURLGenerator;
use OCP\OCM\Exceptions\OCMArgumentException;
use OCP\OCM\IOCMProvider;
use OCP\Security\Signature\Exceptions\SignatoryException;
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 IOCMProvider $provider,
private readonly OCMSignatoryManager $ocmSignatoryManager,
private readonly LoggerInterface $logger,
) {
}

Expand Down Expand Up @@ -60,6 +65,13 @@ public function getCapabilities() {

$this->provider->addResourceType($resource);

// Adding a public key to the ocm discovery
try {
$this->provider->setSignatory($this->ocmSignatoryManager->getLocalSignatory());
} catch (SignatoryException $e) {
$this->logger->warning('cannot generate local signatory', ['exception' => $e]);
}

return ['ocm' => $this->provider->jsonSerialize()];
}
}
147 changes: 147 additions & 0 deletions apps/cloud_federation_api/lib/Controller/RequestHandlerController.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/
namespace OCA\CloudFederationAPI\Controller;

use OC\OCM\OCMSignatoryManager;
use OCA\CloudFederationAPI\Config;
use OCA\CloudFederationAPI\ResponseDefinitions;
use OCP\AppFramework\Controller;
Expand All @@ -22,11 +23,20 @@
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\Security\Signature\Exceptions\IncomingRequestException;
use OCP\Security\Signature\Exceptions\SignatoryNotFoundException;
use OCP\Security\Signature\Exceptions\SignatureException;
use OCP\Security\Signature\Exceptions\SignatureNotFoundException;
use OCP\Security\Signature\ISignatureManager;
use OCP\Security\Signature\Model\IIncomingSignedRequest;
use OCP\Share\Exceptions\ShareNotFound;
use OCP\Share\IProviderFactory;
use OCP\Share\IShare;
use OCP\Util;
use Psr\Log\LoggerInterface;

Expand All @@ -41,6 +51,8 @@
*/
#[OpenAPI(scope: OpenAPI::SCOPE_FEDERATION)]
class RequestHandlerController extends Controller {
const APPCONFIG_SIGN_ENFORCED = 'enforce_signed_ocm_request';

public function __construct(
string $appName,
IRequest $request,
Expand All @@ -50,8 +62,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);
}
Expand Down Expand Up @@ -81,6 +97,17 @@ 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 ||

Check failure on line 112 in apps/cloud_federation_api/lib/Controller/RequestHandlerController.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

TypeDoesNotContainNull

apps/cloud_federation_api/lib/Controller/RequestHandlerController.php:112:7: TypeDoesNotContainNull: Type string for $owner is never null (see https://psalm.dev/090)
$name === null ||
Expand Down Expand Up @@ -208,6 +235,17 @@ 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 ||
Expand Down Expand Up @@ -286,4 +324,113 @@ 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', self::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 @');
}
[, $instance] = explode('@', $entry, 2);
return $instance;
}
}
1 change: 1 addition & 0 deletions apps/files_sharing/lib/External/Storage.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand Down
11 changes: 6 additions & 5 deletions core/Controller/OCMController.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use OCP\AppFramework\Http\Attribute\PublicPage;
use OCP\AppFramework\Http\DataResponse;
use OCP\Capabilities\ICapability;
use OCP\IAppConfig;
use OCP\IConfig;
use OCP\IRequest;
use OCP\Server;
Expand All @@ -31,7 +32,7 @@
class OCMController extends Controller {
public function __construct(
IRequest $request,
private IConfig $config,
private readonly IAppConfig $appConfig,
private LoggerInterface $logger,
) {
parent::__construct('core', $request);
Expand All @@ -54,10 +55,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
)
);

Expand Down
114 changes: 114 additions & 0 deletions core/Migrations/Version30000Date20240101084401.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OC\Core\Migrations;

use Closure;
use OCP\DB\ISchemaWrapper;
use OCP\DB\Types;
use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;


/**
* @since 30.0.0
*/
class Version30000Date20240101084401 extends SimpleMigrationStep {
public function description(): string {
return "creating new table 'sec_signatory' to store remote signatories";
}

public function name(): string {
return 'create sec_signatory';
}

/**
* @param IOutput $output
* @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
* @param array $options
* @return null|ISchemaWrapper
*/
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();

if (!$schema->hasTable('sec_signatory')) {
$table = $schema->createTable('sec_signatory');
$table->addColumn('id', Types::BIGINT, [
'notnull' => true,
'length' => 64,
'autoincrement' => true,
'unsigned' => true,
]);
// key_id_sum will store a hash version of the key_id, more appropriate for search/index
$table->addColumn('key_id_sum', Types::STRING, [
'notnull' => true,
'length' => 127,
]);
$table->addColumn('key_id', Types::STRING, [
'notnull' => true,
'length' => 512
]);
// host/provider_id/account will help generate a unique entry, not based on key_id
// this way, a spoofed instance cannot publish a new key_id for same host+provider_id
// account will be used only to stored multiple keys for the same provider_id/host
$table->addColumn('host', Types::STRING, [
'notnull' => true,
'length' => 512
]);
$table->addColumn('provider_id', Types::STRING, [
'notnull' => true,
'length' => 31,
]);
$table->addColumn('account', Types::STRING, [
'notnull' => false,
'length' => 127,
'default' => ''
]);
$table->addColumn('public_key', Types::TEXT, [
'notnull' => true,
'default' => ''
]);
$table->addColumn('metadata', Types::TEXT, [
'notnull' => true,
'default' => '[]'
]);
// type+status are informative about the trustability of remote instance and status of the signatory
$table->addColumn('type', Types::SMALLINT, [
'notnull' => true,
'length' => 2,
'default' => 9
]);
$table->addColumn('status', Types::SMALLINT, [
'notnull' => true,
'length' => 2,
'default' => 0,
]);
$table->addColumn('creation', Types::INTEGER, [
'notnull' => false,
'length' => 4,
'default' => 0,
'unsigned' => true,
]);
$table->addColumn('last_updated', Types::INTEGER, [
'notnull' => false,
'length' => 4,
'default' => 0,
'unsigned' => true,
]);

$table->setPrimaryKey(['id'], 'sec_sig_id');
$table->addUniqueIndex(['provider_id', 'host', 'account'], 'sec_sig_unic');
$table->addIndex(['key_id_sum', 'provider_id'], 'sec_sig_key');

return $schema;
}

return null;
}
}
Loading

0 comments on commit 9d5986b

Please sign in to comment.