Skip to content

Commit

Permalink
fix(ocm): signatory mapper
Browse files Browse the repository at this point in the history
Signed-off-by: Maxence Lange <[email protected]>
  • Loading branch information
ArtificialOwl committed Dec 2, 2024
1 parent e17819a commit 8807d38
Show file tree
Hide file tree
Showing 28 changed files with 233 additions and 151 deletions.
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
'NCU\\Config\\Exceptions\\UnknownKeyException' => $baseDir . '/lib/unstable/Config/Exceptions/UnknownKeyException.php',
'NCU\\Config\\IUserConfig' => $baseDir . '/lib/unstable/Config/IUserConfig.php',
'NCU\\Config\\ValueType' => $baseDir . '/lib/unstable/Config/ValueType.php',
'NCU\\Security\\Signature\\Enum\\DigestAlgorithm' => $baseDir . '/lib/unstable/Security/Signature/Enum/DigestAlgorithm.php',
'NCU\\Security\\Signature\\Enum\\SignatoryStatus' => $baseDir . '/lib/unstable/Security/Signature/Enum/SignatoryStatus.php',
'NCU\\Security\\Signature\\Enum\\SignatoryType' => $baseDir . '/lib/unstable/Security/Signature/Enum/SignatoryType.php',
'NCU\\Security\\Signature\\Enum\\SignatureAlgorithm' => $baseDir . '/lib/unstable/Security/Signature/Enum/SignatureAlgorithm.php',
Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
'NCU\\Config\\Exceptions\\UnknownKeyException' => __DIR__ . '/../../..' . '/lib/unstable/Config/Exceptions/UnknownKeyException.php',
'NCU\\Config\\IUserConfig' => __DIR__ . '/../../..' . '/lib/unstable/Config/IUserConfig.php',
'NCU\\Config\\ValueType' => __DIR__ . '/../../..' . '/lib/unstable/Config/ValueType.php',
'NCU\\Security\\Signature\\Enum\\DigestAlgorithm' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/Enum/DigestAlgorithm.php',
'NCU\\Security\\Signature\\Enum\\SignatoryStatus' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/Enum/SignatoryStatus.php',
'NCU\\Security\\Signature\\Enum\\SignatoryType' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/Enum/SignatoryType.php',
'NCU\\Security\\Signature\\Enum\\SignatureAlgorithm' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/Enum/SignatureAlgorithm.php',
Expand Down
4 changes: 3 additions & 1 deletion lib/private/OCM/Model/OCMProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,9 @@ public function import(array $data): static {
$this->setResourceTypes($resources);

// import details about the remote request signing public key, if available
$signatory = new Signatory($data['publicKey']['keyId'] ?? '', $data['publicKey']['publicKeyPem'] ?? '');
$signatory = new Signatory();
$signatory->setKeyId($data['publicKey']['keyId'] ?? '');
$signatory->setPublicKey($data['publicKey']['publicKeyPem'] ?? '');
if ($signatory->getKeyId() !== '' && $signatory->getPublicKey() !== '') {
$this->setSignatory($signatory);
}
Expand Down
21 changes: 18 additions & 3 deletions lib/private/OCM/OCMSignatoryManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@

namespace OC\OCM;

use NCU\Security\Signature\Enum\DigestAlgorithm;
use NCU\Security\Signature\Enum\SignatoryType;
use NCU\Security\Signature\Enum\SignatureAlgorithm;
use NCU\Security\Signature\Exceptions\IdentityNotFoundException;
use NCU\Security\Signature\ISignatoryManager;
use NCU\Security\Signature\ISignatureManager;
Expand Down Expand Up @@ -61,7 +63,15 @@ public function getProviderId(): string {
* @since 31.0.0
*/
public function getOptions(): array {
return [];
return [
'algorithm' => SignatureAlgorithm::RSA_SHA512,
'digestAlgorithm' => DigestAlgorithm::SHA512,
'extraSignatureHeaders' => [],
'ttl' => 300,
'dateHeader' => 'D, d M Y H:i:s T',
'ttlSignatory' => 86400 * 3,
'bodyMaxSize' => 50000,
];
}

/**
Expand Down Expand Up @@ -92,7 +102,12 @@ public function getLocalSignatory(): Signatory {
}
$keyPair = $this->identityProofManager->getAppKey('core', 'ocm_external');

return new Signatory($keyId, $keyPair->getPublic(), $keyPair->getPrivate(), local: true);
$signatory = new Signatory(true);
$signatory->setKeyId($keyId);
$signatory->setPublicKey($keyPair->getPublic());
$signatory->setPrivateKey($keyPair->getPrivate());
return $signatory;

}

/**
Expand Down Expand Up @@ -148,7 +163,7 @@ public function getRemoteSignatory(string $remote): ?Signatory {
public function getRemoteSignatoryFromHost(string $host): ?Signatory {
$ocmProvider = $this->ocmDiscoveryService->discover($host, true);
$signatory = $ocmProvider->getSignatory();
$signatory?->setType(SignatoryType::TRUSTED);
$signatory?->setSignatoryType(SignatoryType::TRUSTED);
return $signatory;
}
}
48 changes: 42 additions & 6 deletions lib/private/Security/Signature/Model/IncomingSignedRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,23 @@ class IncomingSignedRequest extends SignedRequest implements
private string $origin = '';

/**
* @param string $body
* @param IRequest $request
* @param array $options
*
* @throws IncomingRequestException if incoming request is wrongly signed
* @throws SignatureNotFoundException if signature is not fully implemented
* @throws SignatureException if signature is faulty
* @throws SignatureNotFoundException if signature is not implemented
*/
public function __construct(
string $body,
private readonly IRequest $request,
private readonly array $options = [],
) {
parent::__construct($body);
$this->verifyHeadersFromRequest();
$this->extractSignatureHeaderFromRequest();
$this->verifyHeaders();
$this->extractSignatureHeader();
$this->reconstructSignatureData();
}

/**
Expand All @@ -59,7 +65,7 @@ public function __construct(
* @throws IncomingRequestException
* @throws SignatureNotFoundException
*/
private function verifyHeadersFromRequest(): void {
private function verifyHeaders(): void {
// confirm presence of date, content-length, digest and Signature
$date = $this->getRequest()->getHeader('date');
if ($date === '') {
Expand Down Expand Up @@ -105,7 +111,7 @@ private function verifyHeadersFromRequest(): void {
*
* @throws IncomingRequestException
*/
private function extractSignatureHeaderFromRequest(): void {
private function extractSignatureHeader(): void {
$details = [];
foreach (explode(',', $this->getRequest()->getHeader('Signature')) as $entry) {
if ($entry === '' || !strpos($entry, '=')) {
Expand All @@ -132,6 +138,36 @@ private function extractSignatureHeaderFromRequest(): void {
}
}

/**
* @throws SignatureException
* @throws SignatureElementNotFoundException
*/
private function reconstructSignatureData(): void {
$usedHeaders = explode(' ', $this->getSigningElement('headers'));
$neededHeaders = array_merge(['date', 'host', 'content-length', 'digest'],
array_keys($this->options['extraSignatureHeaders'] ?? []));

$missingHeaders = array_diff($neededHeaders, $usedHeaders);
if ($missingHeaders !== []) {
throw new SignatureException('missing entries in Signature.headers: ' . json_encode($missingHeaders));
}

$estimated = ['(request-target): ' . strtolower($this->request->getMethod()) . ' ' . $this->request->getRequestUri()];
foreach ($usedHeaders as $key) {
if ($key === '(request-target)') {
continue;
}
$value = (strtolower($key) === 'host') ? $this->request->getServerHost() : $this->request->getHeader($key);
if ($value === '') {
throw new SignatureException('missing header ' . $key . ' in request');
}

$estimated[] = $key . ': ' . $value;
}

$this->setSignatureData($estimated);
}

/**
* @inheritDoc
*
Expand Down Expand Up @@ -214,7 +250,7 @@ public function verify(): void {
throw new SignatoryNotFoundException('empty public key');
}

$algorithm = SignatureAlgorithm::tryFrom($this->getSigningElement('algorithm')) ?? SignatureAlgorithm::SHA256;
$algorithm = SignatureAlgorithm::tryFrom($this->getSigningElement('algorithm')) ?? SignatureAlgorithm::RSA_SHA256;
if (openssl_verify(
implode("\n", $this->getSignatureData()),
base64_decode($this->getSignature()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
namespace OC\Security\Signature\Model;

use JsonSerializable;
use NCU\Security\Signature\Enum\DigestAlgorithm;
use NCU\Security\Signature\Enum\SignatureAlgorithm;
use NCU\Security\Signature\Exceptions\SignatoryException;
use NCU\Security\Signature\Exceptions\SignatoryNotFoundException;
Expand Down Expand Up @@ -42,8 +43,9 @@ public function __construct(

$options = $signatoryManager->getOptions();
$this->setHost($identity)
->setAlgorithm(SignatureAlgorithm::from($options['algorithm'] ?? 'sha256'))
->setSignatory($signatoryManager->getLocalSignatory());
->setAlgorithm($options['algorithm'] ?? SignatureAlgorithm::RSA_SHA256)
->setSignatory($signatoryManager->getLocalSignatory())
->setDigestAlgorithm($options['digestAlgorithm'] ?? DigestAlgorithm::SHA256);

$headers = array_merge([
'(request-target)' => strtolower($method) . ' ' . $path,
Expand Down
41 changes: 34 additions & 7 deletions lib/private/Security/Signature/Model/SignedRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
namespace OC\Security\Signature\Model;

use JsonSerializable;
use NCU\Security\Signature\Enum\DigestAlgorithm;
use NCU\Security\Signature\Exceptions\SignatoryNotFoundException;
use NCU\Security\Signature\Exceptions\SignatureElementNotFoundException;
use NCU\Security\Signature\ISignedRequest;
Expand All @@ -20,7 +21,8 @@
* @since 31.0.0
*/
class SignedRequest implements ISignedRequest, JsonSerializable {
private string $digest;
private string $digest = '';
private DigestAlgorithm $digestAlgorithm = DigestAlgorithm::SHA256;
private array $signingElements = [];
private array $signatureData = [];
private string $signature = '';
Expand All @@ -29,8 +31,6 @@ class SignedRequest implements ISignedRequest, JsonSerializable {
public function __construct(
private readonly string $body,
) {
// digest is created on the fly using $body
$this->digest = 'SHA-256=' . base64_encode(hash('sha256', mb_convert_encoding($body, 'UTF-8', mb_detect_encoding($body)), true));
}

/**
Expand All @@ -43,13 +43,39 @@ public function getBody(): string {
return $this->body;
}

/**
* @inheritDoc
*
* @param DigestAlgorithm $algorithm
*
* @return self
* @since 31.0.0
*/
public function setDigestAlgorithm(DigestAlgorithm $algorithm): self {
return $this;
}

/**
* @inheritDoc
*
* @return DigestAlgorithm
* @since 31.0.0
*/
public function getDigestAlgorithm(): DigestAlgorithm {
return $this->digestAlgorithm;
}

/**
* @inheritDoc
*
* @return string
* @since 31.0.0
*/
public function getDigest(): string {
if ($this->digest === '') {
$this->digest = $this->digestAlgorithm->value . '=' .
base64_encode(hash($this->digestAlgorithm->getHashingAlgorithm(), $this->body, true));
}
return $this->digest;
}

Expand Down Expand Up @@ -178,10 +204,11 @@ public function hasSignatory(): bool {
public function jsonSerialize(): array {
return [
'body' => $this->body,
'digest' => $this->digest,
'signatureElements' => $this->signingElements,
'clearSignature' => $this->signatureData,
'signedSignature' => $this->signature,
'digest' => $this->getDigest(),
'digestAlgorithm' => $this->getDigestAlgorithm()->value,
'signingElements' => $this->signingElements,
'signatureData' => $this->signatureData,
'signature' => $this->signature,
'signatory' => $this->signatory ?? false,
];
}
Expand Down
43 changes: 3 additions & 40 deletions lib/private/Security/Signature/SignatureManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ public function getIncomingSignedRequest(

try {
// confirm the validity of content and identity of the incoming request
$this->generateExpectedClearSignatureFromRequest($signedRequest, $options['extraSignatureHeaders'] ?? []);
$this->confirmIncomingRequestSignature($signedRequest, $signatoryManager, $options['ttlSignatory'] ?? self::SIGNATORY_TTL);
} catch (SignatureException $e) {
$this->logger->warning(
Expand All @@ -127,44 +126,6 @@ public function getIncomingSignedRequest(
return $signedRequest;
}

/**
* generating the expected signature (clear version) sent by the remote instance
* based on the data available in the Signature header.
*
* @param IIncomingSignedRequest $signedRequest
* @param array $extraSignatureHeaders
*
* @throws SignatureException
*/
private function generateExpectedClearSignatureFromRequest(
IIncomingSignedRequest $signedRequest,
array $extraSignatureHeaders = [],
): void {
$request = $signedRequest->getRequest();
$usedHeaders = explode(' ', $signedRequest->getSigningElement('headers'));
$neededHeaders = array_merge(['date', 'host', 'content-length', 'digest'], array_keys($extraSignatureHeaders));

$missingHeaders = array_diff($neededHeaders, $usedHeaders);
if ($missingHeaders !== []) {
throw new SignatureException('missing entries in Signature.headers: ' . json_encode($missingHeaders));
}

$estimated = ['(request-target): ' . strtolower($request->getMethod()) . ' ' . $request->getRequestUri()];
foreach ($usedHeaders as $key) {
if ($key === '(request-target)') {
continue;
}
$value = (strtolower($key) === 'host') ? $request->getServerHost() : $request->getHeader($key);
if ($value === '') {
throw new SignatureException('missing header ' . $key . ' in request');
}

$estimated[] = $key . ': ' . $value;
}

$signedRequest->setSignatureData($estimated);
}

/**
* confirm that the Signature is signed using the correct private key, using
* clear version of the Signature and the public key linked to the keyId
Expand Down Expand Up @@ -403,9 +364,11 @@ private function storeSignatory(Signatory $signatory): void {

/**
* @param Signatory $signatory
* @throws DBException
*/
private function insertSignatory(Signatory $signatory): void {
$time = time();
$signatory->setCreation($time);
$signatory->setLastUpdated($time);
$this->mapper->insert($signatory);
}

Expand Down
34 changes: 34 additions & 0 deletions lib/unstable/Security/Signature/Enum/DigestAlgorithm.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2018 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace NCU\Security\Signature\Enum;

/**
* list of available algorithm when generating digest from body
*
* @experimental 31.0.0
*/
enum DigestAlgorithm: string {
/** @experimental 31.0.0 */
case SHA256 = 'SHA-256';
/** @experimental 31.0.0 */
case SHA512 = 'SHA-512';

/**
* returns hashing algorithm to be used when generating digest
*
* @return string
* @experimental 31.0.0
*/
public function getHashingAlgorithm(): string {
return match($this) {
self::SHA256 => 'sha256',
self::SHA512 => 'sha512',
};
}
}
5 changes: 2 additions & 3 deletions lib/unstable/Security/Signature/Enum/SignatoryStatus.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,10 @@
* - BROKEN = the remote instance does not use the same key pairs than previously
*
* @experimental 31.0.0
* @since 31.0.0
*/
enum SignatoryStatus: int {
/** @since 31.0.0 */
/** @experimental 31.0.0 */
case SYNCED = 1;
/** @since 31.0.0 */
/** @experimental 31.0.0 */
case BROKEN = 9;
}
Loading

0 comments on commit 8807d38

Please sign in to comment.