Skip to content

Commit

Permalink
fix(signed-request): removing unstable from public
Browse files Browse the repository at this point in the history
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
  • Loading branch information
ArtificialOwl committed Dec 3, 2024
1 parent 0a819f5 commit fb608c4
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 75 deletions.
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

0 comments on commit fb608c4

Please sign in to comment.