Skip to content

Commit c21e98e

Browse files
ArtificialOwlArtificialOwl
committed
fix(ocm): signatory mapper
Signed-off-by: Maxence Lange <[email protected]>
1 parent e17819a commit c21e98e

File tree

12 files changed

+174
-76
lines changed

12 files changed

+174
-76
lines changed

‎lib/composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
'NCU\\Config\\Exceptions\\UnknownKeyException' => $baseDir . '/lib/unstable/Config/Exceptions/UnknownKeyException.php',
1313
'NCU\\Config\\IUserConfig' => $baseDir . '/lib/unstable/Config/IUserConfig.php',
1414
'NCU\\Config\\ValueType' => $baseDir . '/lib/unstable/Config/ValueType.php',
15+
'NCU\\Security\\Signature\\Enum\\DigestAlgorithm' => $baseDir . '/lib/unstable/Security/Signature/Enum/DigestAlgorithm.php',
1516
'NCU\\Security\\Signature\\Enum\\SignatoryStatus' => $baseDir . '/lib/unstable/Security/Signature/Enum/SignatoryStatus.php',
1617
'NCU\\Security\\Signature\\Enum\\SignatoryType' => $baseDir . '/lib/unstable/Security/Signature/Enum/SignatoryType.php',
1718
'NCU\\Security\\Signature\\Enum\\SignatureAlgorithm' => $baseDir . '/lib/unstable/Security/Signature/Enum/SignatureAlgorithm.php',

‎lib/composer/composer/autoload_static.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
5353
'NCU\\Config\\Exceptions\\UnknownKeyException' => __DIR__ . '/../../..' . '/lib/unstable/Config/Exceptions/UnknownKeyException.php',
5454
'NCU\\Config\\IUserConfig' => __DIR__ . '/../../..' . '/lib/unstable/Config/IUserConfig.php',
5555
'NCU\\Config\\ValueType' => __DIR__ . '/../../..' . '/lib/unstable/Config/ValueType.php',
56+
'NCU\\Security\\Signature\\Enum\\DigestAlgorithm' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/Enum/DigestAlgorithm.php',
5657
'NCU\\Security\\Signature\\Enum\\SignatoryStatus' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/Enum/SignatoryStatus.php',
5758
'NCU\\Security\\Signature\\Enum\\SignatoryType' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/Enum/SignatoryType.php',
5859
'NCU\\Security\\Signature\\Enum\\SignatureAlgorithm' => __DIR__ . '/../../..' . '/lib/unstable/Security/Signature/Enum/SignatureAlgorithm.php',

‎lib/private/OCM/Model/OCMProvider.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,9 @@ public function import(array $data): static {
183183
$this->setResourceTypes($resources);
184184

185185
// import details about the remote request signing public key, if available
186-
$signatory = new Signatory($data['publicKey']['keyId'] ?? '', $data['publicKey']['publicKeyPem'] ?? '');
186+
$signatory = new Signatory();
187+
$signatory->setKeyId($data['publicKey']['keyId'] ?? '');
188+
$signatory->setPublicKey($data['publicKey']['publicKeyPem'] ?? '');
187189
if ($signatory->getKeyId() !== '' && $signatory->getPublicKey() !== '') {
188190
$this->setSignatory($signatory);
189191
}

‎lib/private/OCM/OCMSignatoryManager.php‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@
99

1010
namespace OC\OCM;
1111

12+
use NCU\Security\Signature\Enum\DigestAlgorithm;
1213
use NCU\Security\Signature\Enum\SignatoryType;
14+
use NCU\Security\Signature\Enum\SignatureAlgorithm;
1315
use NCU\Security\Signature\Exceptions\IdentityNotFoundException;
1416
use NCU\Security\Signature\ISignatoryManager;
1517
use NCU\Security\Signature\ISignatureManager;
@@ -61,7 +63,15 @@ public function getProviderId(): string {
6163
* @since 31.0.0
6264
*/
6365
public function getOptions(): array {
64-
return [];
66+
return [
67+
'algorithm' => SignatureAlgorithm::RSA_SHA512,
68+
'digestAlgorithm' => DigestAlgorithm::SHA512,
69+
'extraSignatureHeaders' => [],
70+
'ttl' => 300,
71+
'dateHeader' => 'D, d M Y H:i:s T',
72+
'ttlSignatory' => 86400 * 3,
73+
'bodyMaxSize' => 50000,
74+
];
6575
}
6676

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

95-
return new Signatory($keyId, $keyPair->getPublic(), $keyPair->getPrivate(), local: true);
105+
$signatory = new Signatory(true);
106+
$signatory->setKeyId($keyId);
107+
$signatory->setPublicKey($keyPair->getPublic());
108+
$signatory->setPrivateKey($keyPair->getPrivate());
109+
return $signatory;
110+
96111
}
97112

98113
/**
@@ -148,7 +163,7 @@ public function getRemoteSignatory(string $remote): ?Signatory {
148163
public function getRemoteSignatoryFromHost(string $host): ?Signatory {
149164
$ocmProvider = $this->ocmDiscoveryService->discover($host, true);
150165
$signatory = $ocmProvider->getSignatory();
151-
$signatory?->setType(SignatoryType::TRUSTED);
166+
$signatory?->setSignatoryType(SignatoryType::TRUSTED);
152167
return $signatory;
153168
}
154169
}

‎lib/private/Security/Signature/Model/IncomingSignedRequest.php‎

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,17 +36,23 @@ class IncomingSignedRequest extends SignedRequest implements
3636
private string $origin = '';
3737

3838
/**
39+
* @param string $body
40+
* @param IRequest $request
41+
* @param array $options
42+
*
3943
* @throws IncomingRequestException if incoming request is wrongly signed
40-
* @throws SignatureNotFoundException if signature is not fully implemented
44+
* @throws SignatureException if signature is faulty
45+
* @throws SignatureNotFoundException if signature is not implemented
4146
*/
4247
public function __construct(
4348
string $body,
4449
private readonly IRequest $request,
4550
private readonly array $options = [],
4651
) {
4752
parent::__construct($body);
48-
$this->verifyHeadersFromRequest();
49-
$this->extractSignatureHeaderFromRequest();
53+
$this->verifyHeaders();
54+
$this->extractSignatureHeader();
55+
$this->reconstructSignatureData();
5056
}
5157

5258
/**
@@ -59,7 +65,7 @@ public function __construct(
5965
* @throws IncomingRequestException
6066
* @throws SignatureNotFoundException
6167
*/
62-
private function verifyHeadersFromRequest(): void {
68+
private function verifyHeaders(): void {
6369
// confirm presence of date, content-length, digest and Signature
6470
$date = $this->getRequest()->getHeader('date');
6571
if ($date === '') {
@@ -105,7 +111,7 @@ private function verifyHeadersFromRequest(): void {
105111
*
106112
* @throws IncomingRequestException
107113
*/
108-
private function extractSignatureHeaderFromRequest(): void {
114+
private function extractSignatureHeader(): void {
109115
$details = [];
110116
foreach (explode(',', $this->getRequest()->getHeader('Signature')) as $entry) {
111117
if ($entry === '' || !strpos($entry, '=')) {
@@ -132,6 +138,36 @@ private function extractSignatureHeaderFromRequest(): void {
132138
}
133139
}
134140

141+
/**
142+
* @throws SignatureException
143+
* @throws SignatureElementNotFoundException
144+
*/
145+
private function reconstructSignatureData(): void {
146+
$usedHeaders = explode(' ', $this->getSigningElement('headers'));
147+
$neededHeaders = array_merge(['date', 'host', 'content-length', 'digest'],
148+
array_keys($this->options['extraSignatureHeaders'] ?? []));
149+
150+
$missingHeaders = array_diff($neededHeaders, $usedHeaders);
151+
if ($missingHeaders !== []) {
152+
throw new SignatureException('missing entries in Signature.headers: ' . json_encode($missingHeaders));
153+
}
154+
155+
$estimated = ['(request-target): ' . strtolower($this->request->getMethod()) . ' ' . $this->request->getRequestUri()];
156+
foreach ($usedHeaders as $key) {
157+
if ($key === '(request-target)') {
158+
continue;
159+
}
160+
$value = (strtolower($key) === 'host') ? $this->request->getServerHost() : $this->request->getHeader($key);
161+
if ($value === '') {
162+
throw new SignatureException('missing header ' . $key . ' in request');
163+
}
164+
165+
$estimated[] = $key . ': ' . $value;
166+
}
167+
168+
$this->setSignatureData($estimated);
169+
}
170+
135171
/**
136172
* @inheritDoc
137173
*
@@ -214,7 +250,7 @@ public function verify(): void {
214250
throw new SignatoryNotFoundException('empty public key');
215251
}
216252

217-
$algorithm = SignatureAlgorithm::tryFrom($this->getSigningElement('algorithm')) ?? SignatureAlgorithm::SHA256;
253+
$algorithm = SignatureAlgorithm::tryFrom($this->getSigningElement('algorithm')) ?? SignatureAlgorithm::RSA_SHA256;
218254
if (openssl_verify(
219255
implode("\n", $this->getSignatureData()),
220256
base64_decode($this->getSignature()),

‎lib/private/Security/Signature/Model/OutgoingSignedRequest.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
namespace OC\Security\Signature\Model;
1010

1111
use JsonSerializable;
12+
use NCU\Security\Signature\Enum\DigestAlgorithm;
1213
use NCU\Security\Signature\Enum\SignatureAlgorithm;
1314
use NCU\Security\Signature\Exceptions\SignatoryException;
1415
use NCU\Security\Signature\Exceptions\SignatoryNotFoundException;
@@ -42,8 +43,9 @@ public function __construct(
4243

4344
$options = $signatoryManager->getOptions();
4445
$this->setHost($identity)
45-
->setAlgorithm(SignatureAlgorithm::from($options['algorithm'] ?? 'sha256'))
46-
->setSignatory($signatoryManager->getLocalSignatory());
46+
->setAlgorithm($options['algorithm'] ?? SignatureAlgorithm::RSA_SHA256)
47+
->setSignatory($signatoryManager->getLocalSignatory())
48+
->setDigestAlgorithm($options['digestAlgorithm'] ?? DigestAlgorithm::SHA256);
4749

4850
$headers = array_merge([
4951
'(request-target)' => strtolower($method) . ' ' . $path,

‎lib/private/Security/Signature/Model/SignedRequest.php‎

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
namespace OC\Security\Signature\Model;
1010

1111
use JsonSerializable;
12+
use NCU\Security\Signature\Enum\DigestAlgorithm;
1213
use NCU\Security\Signature\Exceptions\SignatoryNotFoundException;
1314
use NCU\Security\Signature\Exceptions\SignatureElementNotFoundException;
1415
use NCU\Security\Signature\ISignedRequest;
@@ -20,7 +21,8 @@
2021
* @since 31.0.0
2122
*/
2223
class SignedRequest implements ISignedRequest, JsonSerializable {
23-
private string $digest;
24+
private string $digest = '';
25+
private DigestAlgorithm $digestAlgorithm = DigestAlgorithm::SHA256;
2426
private array $signingElements = [];
2527
private array $signatureData = [];
2628
private string $signature = '';
@@ -29,8 +31,6 @@ class SignedRequest implements ISignedRequest, JsonSerializable {
2931
public function __construct(
3032
private readonly string $body,
3133
) {
32-
// digest is created on the fly using $body
33-
$this->digest = 'SHA-256=' . base64_encode(hash('sha256', mb_convert_encoding($body, 'UTF-8', mb_detect_encoding($body)), true));
3434
}
3535

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

46+
/**
47+
* @inheritDoc
48+
*
49+
* @param DigestAlgorithm $algorithm
50+
*
51+
* @return self
52+
* @since 31.0.0
53+
*/
54+
public function setDigestAlgorithm(DigestAlgorithm $algorithm): self {
55+
return $this;
56+
}
57+
58+
/**
59+
* @inheritDoc
60+
*
61+
* @return DigestAlgorithm
62+
* @since 31.0.0
63+
*/
64+
public function getDigestAlgorithm(): DigestAlgorithm {
65+
return $this->digestAlgorithm;
66+
}
67+
4668
/**
4769
* @inheritDoc
4870
*
4971
* @return string
5072
* @since 31.0.0
5173
*/
5274
public function getDigest(): string {
75+
if ($this->digest === '') {
76+
$this->digest = $this->digestAlgorithm->value . '=' .
77+
base64_encode(hash($this->digestAlgorithm->getHashingAlgorithm(), $this->body, true));
78+
}
5379
return $this->digest;
5480
}
5581

@@ -178,10 +204,11 @@ public function hasSignatory(): bool {
178204
public function jsonSerialize(): array {
179205
return [
180206
'body' => $this->body,
181-
'digest' => $this->digest,
182-
'signatureElements' => $this->signingElements,
183-
'clearSignature' => $this->signatureData,
184-
'signedSignature' => $this->signature,
207+
'digest' => $this->getDigest(),
208+
'digestAlgorithm' => $this->getDigestAlgorithm()->value,
209+
'signingElements' => $this->signingElements,
210+
'signatureData' => $this->signatureData,
211+
'signature' => $this->signature,
185212
'signatory' => $this->signatory ?? false,
186213
];
187214
}

‎lib/private/Security/Signature/SignatureManager.php‎

Lines changed: 3 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,6 @@ public function getIncomingSignedRequest(
111111

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

130-
/**
131-
* generating the expected signature (clear version) sent by the remote instance
132-
* based on the data available in the Signature header.
133-
*
134-
* @param IIncomingSignedRequest $signedRequest
135-
* @param array $extraSignatureHeaders
136-
*
137-
* @throws SignatureException
138-
*/
139-
private function generateExpectedClearSignatureFromRequest(
140-
IIncomingSignedRequest $signedRequest,
141-
array $extraSignatureHeaders = [],
142-
): void {
143-
$request = $signedRequest->getRequest();
144-
$usedHeaders = explode(' ', $signedRequest->getSigningElement('headers'));
145-
$neededHeaders = array_merge(['date', 'host', 'content-length', 'digest'], array_keys($extraSignatureHeaders));
146-
147-
$missingHeaders = array_diff($neededHeaders, $usedHeaders);
148-
if ($missingHeaders !== []) {
149-
throw new SignatureException('missing entries in Signature.headers: ' . json_encode($missingHeaders));
150-
}
151-
152-
$estimated = ['(request-target): ' . strtolower($request->getMethod()) . ' ' . $request->getRequestUri()];
153-
foreach ($usedHeaders as $key) {
154-
if ($key === '(request-target)') {
155-
continue;
156-
}
157-
$value = (strtolower($key) === 'host') ? $request->getServerHost() : $request->getHeader($key);
158-
if ($value === '') {
159-
throw new SignatureException('missing header ' . $key . ' in request');
160-
}
161-
162-
$estimated[] = $key . ': ' . $value;
163-
}
164-
165-
$signedRequest->setSignatureData($estimated);
166-
}
167-
168129
/**
169130
* confirm that the Signature is signed using the correct private key, using
170131
* clear version of the Signature and the public key linked to the keyId
@@ -403,9 +364,11 @@ private function storeSignatory(Signatory $signatory): void {
403364

404365
/**
405366
* @param Signatory $signatory
406-
* @throws DBException
407367
*/
408368
private function insertSignatory(Signatory $signatory): void {
369+
$time = time();
370+
$signatory->setCreation($time);
371+
$signatory->setLastUpdated($time);
409372
$this->mapper->insert($signatory);
410373
}
411374

‎lib/unstable/Security/Signature/Enum/DigestAlgorithm.php‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2018 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
namespace NCU\Security\Signature\Enum;
10+
11+
/**
12+
* list of available algorithm when generating digest from body
13+
*
14+
* @experimental 31.0.0
15+
* @since 31.0.0
16+
*/
17+
enum DigestAlgorithm: string {
18+
/** @since 31.0.0 */
19+
case SHA256 = 'SHA-256';
20+
/** @since 31.0.0 */
21+
case SHA512 = 'SHA-512';
22+
23+
public function getHashingAlgorithm(): string {
24+
return match($this) {
25+
self::SHA256 => 'sha256',
26+
self::SHA512 => 'sha512',
27+
};
28+
}
29+
}

‎lib/unstable/Security/Signature/Enum/SignatureAlgorithm.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
*/
1717
enum SignatureAlgorithm: string {
1818
/** @since 31.0.0 */
19-
case SHA256 = 'sha256';
19+
case RSA_SHA256 = 'rsa-sha256';
2020
/** @since 31.0.0 */
21-
case SHA512 = 'sha512';
21+
case RSA_SHA512 = 'rsa-sha512';
2222
}

0 commit comments

Comments
 (0)