fix(ocm): signatory mapper

Signed-off-by: Maxence Lange <[email protected]>
nextcloud · Dec 2, 2024 · 613dd0a · 613dd0a
1 parent e17819a
commit 613dd0a
Show file tree

Hide file tree

Showing 10 changed files with 172 additions and 78 deletions.
diff --git a/lib/private/OCM/Model/OCMProvider.php b/lib/private/OCM/Model/OCMProvider.php
@@ -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);
 		}

diff --git a/lib/private/OCM/OCMSignatoryManager.php b/lib/private/OCM/OCMSignatoryManager.php
@@ -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;
@@ -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,
+		];
 	}
 
 	/**
@@ -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;
+
 	}
 
 	/**

diff --git a/lib/private/Security/Signature/Model/IncomingSignedRequest.php b/lib/private/Security/Signature/Model/IncomingSignedRequest.php
@@ -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();
 	}
 
 	/**
@@ -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 === '') {
@@ -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, '=')) {
@@ -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
 	 *
@@ -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()),

diff --git a/lib/private/Security/Signature/Model/OutgoingSignedRequest.php b/lib/private/Security/Signature/Model/OutgoingSignedRequest.php
@@ -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;
@@ -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,

diff --git a/lib/private/Security/Signature/Model/SignedRequest.php b/lib/private/Security/Signature/Model/SignedRequest.php
@@ -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;
@@ -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 = '';
@@ -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));
 	}
 
 	/**
@@ -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;
 	}
 
@@ -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,
 		];
 	}

diff --git a/lib/private/Security/Signature/SignatureManager.php b/lib/private/Security/Signature/SignatureManager.php
@@ -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(
@@ -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
@@ -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);
 	}
 

diff --git a/lib/unstable/Security/Signature/Enum/DigestAlgorithm.php b/lib/unstable/Security/Signature/Enum/DigestAlgorithm.php
@@ -0,0 +1,29 @@
+<?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
+ * @since 31.0.0
+ */
+enum DigestAlgorithm: string {
+	/** @since 31.0.0 */
+	case SHA256 = 'SHA-256';
+	/** @since 31.0.0 */
+	case SHA512 = 'SHA-512';
+
+	public function getHashingAlgorithm(): string {
+		return match($this) {
+			self::SHA256 => 'sha256',
+			self::SHA512 => 'sha512',
+		};
+	}
+}
diff --git a/lib/unstable/Security/Signature/Enum/SignatureAlgorithm.php b/lib/unstable/Security/Signature/Enum/SignatureAlgorithm.php
@@ -16,7 +16,7 @@
  */
 enum SignatureAlgorithm: string {
 	/** @since 31.0.0 */
-	case SHA256 = 'sha256';
+	case RSA_SHA256 = 'rsa-sha256';
 	/** @since 31.0.0 */
-	case SHA512 = 'sha512';
+	case RSA_SHA512 = 'rsa-sha512';
 }
diff --git a/lib/unstable/Security/Signature/ISignedRequest.php b/lib/unstable/Security/Signature/ISignedRequest.php
@@ -8,6 +8,7 @@
  */
 namespace NCU\Security\Signature;
 
+use NCU\Security\Signature\Enum\DigestAlgorithm;
 use NCU\Security\Signature\Exceptions\SignatoryNotFoundException;
 use NCU\Security\Signature\Exceptions\SignatureElementNotFoundException;
 use NCU\Security\Signature\Model\Signatory;
@@ -30,6 +31,24 @@ interface ISignedRequest {
 	 */
 	public function getBody(): string;
 
+	/**
+	 * set algorithm used to generate digest
+	 *
+	 * @param DigestAlgorithm $algorithm
+	 *
+	 * @return self
+	 * @since 31.0.0
+	 */
+	public function setDigestAlgorithm(DigestAlgorithm $algorithm): self;
+
+	/**
+	 * get algorithm used to generate digest
+	 *
+	 * @return DigestAlgorithm
+	 * @since 31.0.0
+	 */
+	public function getDigestAlgorithm(): DigestAlgorithm;
+
 	/**
 	 * checksum of the payload of the request
 	 *