Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix(signed-request): removing unstable from public
Signed-off-by: Maxence Lange <[email protected]>
  • Loading branch information
ArtificialOwl committed Dec 4, 2024
commit 4df315552391af1c89516fa2f2c1796666f086be
4 changes: 4 additions & 0 deletions apps/cloud_federation_api/lib/Capabilities.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ public function getCapabilities() {
// Adding a public key to the ocm discovery
try {
if (!$this->appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_DISABLED, lazy: true)) {
/**
* @experimental 31.0.0
* @psalm-suppress UndefinedInterfaceMethod
*/
$this->provider->setSignatory($this->ocmSignatoryManager->getLocalSignatory());
} else {
$this->logger->debug('ocm public key feature disabled');
Expand Down
7 changes: 5 additions & 2 deletions lib/private/OCM/Model/OCMProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,10 @@ private function looksValid(): bool {
* enabled: bool,
* apiVersion: '1.0-proposal1',
* endPoint: string,
* publicKey: Signatory|null,
* publicKey: array{
* keyId: string,
* publicKeyPem: string
* },
* resourceTypes: list<array{
* name: string,
* shareTypes: list<string>,
Expand All @@ -230,7 +233,7 @@ public function jsonSerialize(): array {
'apiVersion' => '1.0-proposal1', // deprecated, but keep it to stay compatible with old version
'version' => $this->getApiVersion(), // informative but real version
'endPoint' => $this->getEndPoint(),
'publicKey' => $this->getSignatory(),
'publicKey' => $this->getSignatory()->jsonSerialize(),
'resourceTypes' => $resourceTypes
];
}
Expand Down
25 changes: 8 additions & 17 deletions lib/private/OCM/OCMSignatoryManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -144,26 +144,17 @@ private function generateKeyId(): string {
*/
public function getRemoteSignatory(string $remote): ?Signatory {
try {
return $this->getRemoteSignatoryFromHost($remote);
$ocmProvider = $this->ocmDiscoveryService->discover($remote, true);
/**
* @experimental 31.0.0
* @psalm-suppress UndefinedInterfaceMethod
*/
$signatory = $ocmProvider->getSignatory();
$signatory?->setSignatoryType(SignatoryType::TRUSTED);
return $signatory;
} catch (OCMProviderException $e) {
$this->logger->warning('fail to get remote signatory', ['exception' => $e, 'remote' => $remote]);
return null;
}
}

/**
* As host is enough to generate signatory using OCMDiscoveryService
*
* @param string $host
*
* @return Signatory|null
* @throws OCMProviderException on fail to discover ocm services
* @since 31.0.0
*/
public function getRemoteSignatoryFromHost(string $host): ?Signatory {
$ocmProvider = $this->ocmDiscoveryService->discover($host, true);
$signatory = $ocmProvider->getSignatory();
$signatory?->setSignatoryType(SignatoryType::TRUSTED);
return $signatory;
}
}
52 changes: 20 additions & 32 deletions lib/private/Security/Signature/Model/IncomingSignedRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
use NCU\Security\Signature\Exceptions\IdentityNotFoundException;
use NCU\Security\Signature\Exceptions\IncomingRequestException;
use NCU\Security\Signature\Exceptions\InvalidSignatureException;
use NCU\Security\Signature\Exceptions\SignatoryException;
use NCU\Security\Signature\Exceptions\SignatoryNotFoundException;
use NCU\Security\Signature\Exceptions\SignatureElementNotFoundException;
use NCU\Security\Signature\Exceptions\SignatureException;
Expand Down Expand Up @@ -53,6 +52,13 @@ public function __construct(
$this->verifyHeaders();
$this->extractSignatureHeader();
$this->reconstructSignatureData();

try {
// we set origin based on the keyId defined in the Signature header of the request
$this->setOrigin(Signatory::extractIdentityFromUri($this->getSigningElement('keyId')));
} catch (IdentityNotFoundException $e) {
throw new IncomingRequestException($e->getMessage());
}
}

/**
Expand All @@ -66,21 +72,22 @@ public function __construct(
* @throws SignatureNotFoundException
*/
private function verifyHeaders(): void {
if ($this->request->getHeader('Signature') === '') {
throw new SignatureNotFoundException('missing Signature in header');
}

// confirm presence of date, content-length, digest and Signature
$date = $this->getRequest()->getHeader('date');
$date = $this->request->getHeader('date');
if ($date === '') {
throw new SignatureNotFoundException('missing date in header');
throw new IncomingRequestException('missing date in header');
}
$contentLength = $this->getRequest()->getHeader('content-length');
$contentLength = $this->request->getHeader('content-length');
if ($contentLength === '') {
throw new SignatureNotFoundException('missing content-length in header');
throw new IncomingRequestException('missing content-length in header');
}
$digest = $this->getRequest()->getHeader('digest');
$digest = $this->request->getHeader('digest');
if ($digest === '') {
throw new SignatureNotFoundException('missing digest in header');
}
if ($this->getRequest()->getHeader('Signature') === '') {
throw new SignatureNotFoundException('missing Signature in header');
throw new IncomingRequestException('missing digest in header');
}

// confirm date
Expand Down Expand Up @@ -113,7 +120,7 @@ private function verifyHeaders(): void {
*/
private function extractSignatureHeader(): void {
$details = [];
foreach (explode(',', $this->getRequest()->getHeader('Signature')) as $entry) {
foreach (explode(',', $this->request->getHeader('Signature')) as $entry) {
if ($entry === '' || !strpos($entry, '=')) {
continue;
}
Expand All @@ -139,6 +146,8 @@ private function extractSignatureHeader(): void {
}

/**
* reconstruct signature data based on signature's metadata stored in the 'Signature' header
*
* @throws SignatureException
* @throws SignatureElementNotFoundException
*/
Expand Down Expand Up @@ -178,27 +187,6 @@ public function getRequest(): IRequest {
return $this->request;
}

/**
* @inheritDoc
*
* @param Signatory $signatory
*
* @return $this
* @throws IdentityNotFoundException
* @throws IncomingRequestException
* @throws SignatoryException
* @since 31.0.0
*/
public function setSignatory(Signatory $signatory): self {
$identity = \OCP\Server::get(ISignatureManager::class)->extractIdentityFromUri($signatory->getKeyId());
if ($identity !== $this->getOrigin()) {
throw new SignatoryException('keyId from provider is different from the one from signed request');
}

parent::setSignatory($signatory);
return $this;
}

/**
* @inheritDoc
*
Expand Down
6 changes: 0 additions & 6 deletions lib/private/Security/Signature/SignatureManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,6 @@ public function getIncomingSignedRequest(

// generate IncomingSignedRequest based on body and request
$signedRequest = new IncomingSignedRequest($body, $this->request, $options);
try {
// we set origin based on the keyId defined in the Signature header of the request
$signedRequest->setOrigin($this->extractIdentityFromUri($signedRequest->getSigningElement('keyId')));
} catch (IdentityNotFoundException $e) {
throw new IncomingRequestException($e->getMessage());
}

try {
// confirm the validity of content and identity of the incoming request
Expand Down
36 changes: 19 additions & 17 deletions lib/public/OCM/IOCMProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
namespace OCP\OCM;

use JsonSerializable;
use NCU\Security\Signature\Model\Signatory;
use OCP\OCM\Exceptions\OCMArgumentException;
use OCP\OCM\Exceptions\OCMProviderException;

Expand Down Expand Up @@ -120,21 +119,21 @@ public function getResourceTypes(): array;
*/
public function extractProtocolEntry(string $resourceName, string $protocol): string;

/**
* store signatory (public/private key pair) to sign outgoing/incoming request
*
* @param Signatory $signatory
* @since 31.0.0
*/
public function setSignatory(Signatory $signatory): void;

/**
* signatory (public/private key pair) used to sign outgoing/incoming request
*
* @return Signatory|null returns null if no Signatory available
* @since 31.0.0
*/
public function getSignatory(): ?Signatory;
// /**
// * store signatory (public/private key pair) to sign outgoing/incoming request
// *
// * @param Signatory $signatory
// * @experimental 31.0.0
// */
// public function setSignatory(Signatory $signatory): void;

// /**
// * signatory (public/private key pair) used to sign outgoing/incoming request
// *
// * @return Signatory|null returns null if no Signatory available
// * @experimental 31.0.0
// */
// public function getSignatory(): ?Signatory;

/**
* import data from an array
Expand All @@ -152,7 +151,10 @@ public function import(array $data): static;
* enabled: bool,
* apiVersion: '1.0-proposal1',
* endPoint: string,
* publicKey: Signatory|null,
* publicKey: array{
* keyId: string,
* publicKeyPem: string
* },
* resourceTypes: list<array{
* name: string,
* shareTypes: list<string>,
Expand Down
3 changes: 2 additions & 1 deletion lib/unstable/Security/Signature/Model/Signatory.php
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ public function getSignatoryStatus(): SignatoryStatus {
*/
public function setMetaValue(string $key, string|int|float|bool|array $value): void {
$this->metadata[$key] = $value;
$this->setter('metadata', [$this->metadata]);
}

/**
Expand All @@ -174,7 +175,7 @@ public function jsonSerialize(): array {
*
* @return string
* @throws IdentityNotFoundException if identity cannot be extracted
* @since 31.0.0
* @experimental 31.0.0
*/
public static function extractIdentityFromUri(string $uri): string {
$identity = parse_url($uri, PHP_URL_HOST);
Expand Down