Skip to content

Commit 824b62e

Browse files
committed
feat(ocm): align RFC 9421 http-sig with updated OCM spec
Implements the changes from cs3org/OCM-API#391 and closes the remaining gaps with the HTTP Message Signatures section: - discover peer JWK Sets via the new `jwksUri` discovery field instead of the fixed /.well-known/jwks.json path; advertise our own jwksUri and require it (https) from peers advertising the `http-sig` capability - stop covering the Date header; freshness is anchored on the `created` signature parameter - mark and select the OCM signature by its integrity-protected tag="ocm" parameter instead of the dictionary label - require the JWK `alg` parameter and derive the signature algorithm from it, accepting fully-specified RFC 9864 names - reject signed requests whose verification key cannot be resolved instead of treating them as unsigned - derive the signer origin from the spec-canonical fqdn#id keyid form Signed-off-by: Micke Nordin <kano@sunet.se>
1 parent 9abf97b commit 824b62e

15 files changed

Lines changed: 559 additions & 62 deletions

lib/private/OCM/Model/OCMProvider.php

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ class OCMProvider implements IOCMProvider {
2626
private array $capabilities = [];
2727
private string $endPoint = '';
2828
private string $tokenEndPoint = '';
29+
private string $jwksUri = '';
2930
/** @var IOCMResource[] */
3031
private array $resourceTypes = [];
3132
private ?Signatory $signatory = null;
@@ -144,6 +145,26 @@ public function getTokenEndPoint(): string {
144145
return '';
145146
}
146147

148+
/**
149+
* @param string $jwksUri
150+
*
151+
* @return $this
152+
*/
153+
#[\Override]
154+
public function setJwksUri(string $jwksUri): static {
155+
$this->jwksUri = $jwksUri;
156+
157+
return $this;
158+
}
159+
160+
/**
161+
* @return string
162+
*/
163+
#[\Override]
164+
public function getJwksUri(): string {
165+
return $this->jwksUri;
166+
}
167+
147168
/**
148169
* @return string
149170
*/
@@ -311,6 +332,9 @@ public function import(array $data): static {
311332
if (isset($data['tokenEndPoint'])) {
312333
$this->setTokenEndPoint($data['tokenEndPoint']);
313334
}
335+
if (is_string($data['jwksUri'] ?? null)) {
336+
$this->setJwksUri($data['jwksUri']);
337+
}
314338

315339
if (!$this->looksValid()) {
316340
throw new OCMProviderException('remote provider does not look valid');
@@ -357,6 +381,10 @@ public function jsonSerialize(): array {
357381
if ($inviteAcceptDialog !== '') {
358382
$response['inviteAcceptDialog'] = $inviteAcceptDialog;
359383
}
384+
$jwksUri = $this->getJwksUri();
385+
if ($jwksUri !== '') {
386+
$response['jwksUri'] = $jwksUri;
387+
}
360388
return $response;
361389
}
362390
}

lib/private/OCM/OCMDiscoveryService.php

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,14 @@ public function getLocalOCMProvider(bool $fullDetails = true): IOCMProvider {
209209
$provider->setCapabilities(['notifications', 'shares', 'exchange-token']);
210210
$provider->setTokenEndPoint($tokenUrl);
211211
if ($signingEnabled) {
212-
$provider->setCapabilities(['http-sig']);
212+
try {
213+
// advertising `http-sig` requires publishing the location of
214+
// the local JWK Set in `jwksUri` (and it must be https)
215+
$provider->setJwksUri($this->signatoryManager->getLocalJwksUri());
216+
$provider->setCapabilities(['http-sig']);
217+
} catch (IdentityNotFoundException $e) {
218+
$this->logger->warning('cannot build local jwksUri, http-sig capability not advertised', ['exception' => $e]);
219+
}
213220
}
214221

215222
$resource = $provider->createNewResourceType();

lib/private/OCM/OCMJwksHandler.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,11 @@
1818
use Psr\Log\LoggerInterface;
1919
use Throwable;
2020

21-
/** Serves `/.well-known/jwks.json` (RFC 7517) with the OCM signing keys. */
21+
/**
22+
* Serves the local JWK Set (RFC 7517) with the OCM signing keys at
23+
* `/.well-known/jwks.json`, the URL advertised to peers through the
24+
* `jwksUri` field of the OCM discovery response.
25+
*/
2226
class OCMJwksHandler implements IHandler {
2327
public function __construct(
2428
private readonly IAppConfig $appConfig,

lib/private/OCM/OCMSignatoryManager.php

Lines changed: 85 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,12 @@
2323
use OCP\IConfig;
2424
use OCP\IURLGenerator;
2525
use OCP\OCM\Exceptions\OCMProviderException;
26+
use OCP\OCM\IOCMDiscoveryService;
2627
use OCP\Security\Signature\Enum\DigestAlgorithm;
2728
use OCP\Security\Signature\Enum\SignatoryType;
2829
use OCP\Security\Signature\Enum\SignatureAlgorithm;
2930
use OCP\Security\Signature\Exceptions\IdentityNotFoundException;
31+
use OCP\Security\Signature\Exceptions\SignatureException;
3032
use OCP\Security\Signature\ISignatureManager;
3133
use OCP\Security\Signature\Model\Signatory;
3234
use OCP\Server;
@@ -373,31 +375,54 @@ private function signatoryFromPool(int $poolId): ?Signatory {
373375
return $signatory;
374376
}
375377

378+
/**
379+
* Absolute https URL of the local JWK Set document, advertised as
380+
* `jwksUri` in the OCM discovery response. The spec mandates https and
381+
* requires the field whenever the `http-sig` capability is exposed.
382+
*
383+
* @throws IdentityNotFoundException
384+
*/
385+
public function getLocalJwksUri(): string {
386+
return $this->buildLocalUrl('/.well-known/jwks.json');
387+
}
388+
376389
/**
377390
* @param string $fragment URL fragment (e.g. 'signature' for cavage, 'ecdsa-p256-sha256' for the JWKS-published key)
378391
* @return string
379392
* @throws IdentityNotFoundException
380393
*/
381394
private function buildLocalKeyId(string $fragment): string {
395+
return $this->buildLocalUrl('/ocm#' . $fragment);
396+
}
397+
398+
/**
399+
* Prefix $path with 'https://' and the signing identity of this instance,
400+
* including a possible subfolder.
401+
*
402+
* @param string $path absolute path, starting with a slash
403+
* @return string
404+
* @throws IdentityNotFoundException
405+
*/
406+
private function buildLocalUrl(string $path): string {
382407
if ($this->appConfig->hasKey('core', self::APPCONFIG_SIGN_IDENTITY_EXTERNAL, true)) {
383408
$identity = $this->appConfig->getValueString('core', self::APPCONFIG_SIGN_IDENTITY_EXTERNAL, lazy: true);
384-
return 'https://' . $identity . '/ocm#' . $fragment;
409+
return 'https://' . $identity . $path;
385410
}
386411

387412
try {
388-
return $this->signatureManager->generateKeyIdFromConfig('/ocm#' . $fragment);
413+
return $this->signatureManager->generateKeyIdFromConfig($path);
389414
} catch (IdentityNotFoundException) {
390415
}
391416

392417
$url = $this->urlGenerator->linkToRouteAbsolute('cloud_federation_api.requesthandlercontroller.addShare');
393418
$identity = $this->signatureManager->extractIdentityFromUri($url);
394419

395-
// catching possible subfolder to create a keyId like 'https://hostname/subfolder/ocm#<fragment>'
396-
$path = parse_url($url, PHP_URL_PATH);
397-
$pos = strpos($path, '/ocm/shares');
398-
$sub = ($pos) ? substr($path, 0, $pos) : '';
420+
// catching possible subfolder to create a URL like 'https://hostname/subfolder/ocm#<fragment>'
421+
$routePath = parse_url($url, PHP_URL_PATH);
422+
$pos = strpos($routePath, '/ocm/shares');
423+
$sub = ($pos) ? substr($routePath, 0, $pos) : '';
399424

400-
return 'https://' . $identity . $sub . '/ocm#' . $fragment;
425+
return 'https://' . $identity . $sub . $path;
401426
}
402427

403428
/**
@@ -476,10 +501,16 @@ private function readCachedJwks(string $origin): ?array {
476501
}
477502

478503
/**
504+
* Fetch the peer's JWK Set from the URL advertised in the `jwksUri`
505+
* field of its discovery response.
506+
*
479507
* @return list<array<string, mixed>>|null
480508
*/
481509
private function fetchJwks(string $origin): ?array {
482-
$url = 'https://' . $origin . '/.well-known/jwks.json';
510+
$url = $this->resolveJwksUri($origin);
511+
if ($url === null) {
512+
return null;
513+
}
483514
$options = [
484515
'timeout' => 10,
485516
'connect_timeout' => 10,
@@ -508,6 +539,34 @@ private function fetchJwks(string $origin): ?array {
508539
return array_values(array_filter($decoded['keys'], 'is_array'));
509540
}
510541

542+
/**
543+
* Location of the peer's JWK Set, read from the `jwksUri` field of its
544+
* discovery response. A peer that advertises `http-sig` without a https
545+
* `jwksUri` is non-conformant: no keys can be obtained from it, so its
546+
* signed requests will fail verification.
547+
*/
548+
private function resolveJwksUri(string $origin): ?string {
549+
try {
550+
$provider = Server::get(IOCMDiscoveryService::class)->discover($origin);
551+
} catch (NotFoundExceptionInterface|ContainerExceptionInterface|OCMProviderException $e) {
552+
$this->logger->warning('cannot discover remote OCM provider for JWKS', ['exception' => $e, 'origin' => $origin]);
553+
return null;
554+
}
555+
556+
$jwksUri = $provider->getJwksUri();
557+
if ($jwksUri === '') {
558+
if ($provider->hasCapability('http-sig')) {
559+
$this->logger->warning('remote advertises http-sig but no jwksUri; non-conformant peer', ['origin' => $origin]);
560+
}
561+
return null;
562+
}
563+
if (!str_starts_with($jwksUri, 'https://')) {
564+
$this->logger->warning('remote jwksUri does not use https, ignoring', ['origin' => $origin, 'jwksUri' => $jwksUri]);
565+
return null;
566+
}
567+
return $jwksUri;
568+
}
569+
511570
/**
512571
* @param list<array<string, mixed>>|null $keys
513572
*/
@@ -519,8 +578,25 @@ private function findKid(?array $keys, string $keyId): ?Key {
519578
if (($entry['kid'] ?? null) !== $keyId) {
520579
continue;
521580
}
581+
// every published JWK must carry an `alg` parameter naming an
582+
// acceptable asymmetric signature algorithm; keys without one
583+
// are rejected as non-conformant
584+
$alg = $entry['alg'] ?? null;
585+
if (!is_string($alg) || $alg === '') {
586+
$this->logger->warning('remote JWK carries no alg parameter', ['kid' => $keyId]);
587+
return null;
588+
}
522589
try {
523-
return JWK::parseKey($entry, Algorithm::deriveJoseAlgFromJwk($entry));
590+
$native = Algorithm::normalize($alg);
591+
$derived = Algorithm::deriveJoseAlgFromJwk($entry);
592+
if ($derived !== null && Algorithm::normalize($derived) !== $native) {
593+
$this->logger->warning('remote JWK alg does not match its key type', ['kid' => $keyId, 'alg' => $alg]);
594+
return null;
595+
}
596+
return JWK::parseKey($entry);
597+
} catch (SignatureException $e) {
598+
$this->logger->warning('remote JWK alg is not acceptable', ['exception' => $e, 'kid' => $keyId, 'alg' => $alg]);
599+
return null;
524600
} catch (Throwable $e) {
525601
$this->logger->warning('failed to parse remote JWK', ['exception' => $e, 'kid' => $keyId]);
526602
return null;

lib/private/Security/Signature/Model/Rfc9421IncomingSignedRequest.php

Lines changed: 63 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -34,28 +34,33 @@
3434

3535
/**
3636
* RFC 9421 implementation of {@see IIncomingSignedRequest}. Parses the
37-
* inbound Signature-Input / Signature dictionaries, picks the OCM-labeled
38-
* entry (RFC 9421 §3.2 lets verifiers scope by policy), and rebuilds the
39-
* signature base per RFC 9421 §2.5. Crypto is deferred to {@see verify()},
40-
* which needs a {@see Key} attached via {@see setKey()}. Body integrity
41-
* (RFC 9530 content-digest) is checked before verify() if covered.
37+
* inbound Signature-Input / Signature dictionaries, picks the single entry
38+
* carrying the `tag="ocm"` signature parameter (disregarding dictionary
39+
* labels, as mandated by the OCM spec), and rebuilds the signature base per
40+
* RFC 9421 §2.5. Crypto is deferred to {@see verify()}, which needs a
41+
* {@see Key} attached via {@see setKey()}. Body integrity (RFC 9530
42+
* content-digest) is checked before verify() if covered.
4243
*/
4344
class Rfc9421IncomingSignedRequest extends SignedRequest implements
4445
IIncomingSignedRequest,
4546
JsonSerializable {
46-
/** Baseline cover for OCM. Override via `rfc9421.requiredComponents`. */
47+
/**
48+
* Baseline cover for OCM. Override via `rfc9421.requiredComponents`.
49+
* The `Date` header is deliberately not part of the required set:
50+
* freshness is anchored on the `created` signature parameter.
51+
*/
4752
private const DEFAULT_REQUIRED_COMPONENTS = [
4853
'@method',
4954
'@target-uri',
5055
'content-digest',
5156
'content-length',
52-
'date',
5357
];
5458

5559
/** Max clock skew (seconds) for `created`. Override via `rfc9421.maxClockSkew`. */
5660
private const DEFAULT_MAX_FUTURE_SKEW = 60;
5761

5862
private string $origin = '';
63+
private string $label;
5964
/** @var list<string> */
6065
private array $components;
6166
/** @var array<string, scalar> */
@@ -88,26 +93,42 @@ public function __construct(
8893
$inputs = self::parseSignatureInput($signatureInputHeader);
8994
$signatures = self::parseSignature($signatureHeader);
9095

91-
// OCM policy (stricter than RFC 8941 §4.2 last-wins): a duplicate
92-
// `ocm` entry is ambiguous; the entire request MUST be rejected.
93-
if (self::countLabel($signatureInputHeader, 'ocm') > 1
94-
|| self::countLabel($signatureHeader, 'ocm') > 1) {
96+
// The OCM signature is identified by its integrity-protected
97+
// `tag="ocm"` parameter, disregarding dictionary labels. A message
98+
// carrying more than one such signature MUST be rejected; one
99+
// without any is unsigned as far as OCM is concerned.
100+
$tagged = [];
101+
foreach ($inputs as $label => $entry) {
102+
if (($entry['params']['tag'] ?? null) === 'ocm') {
103+
$tagged[] = $label;
104+
}
105+
}
106+
if (count($tagged) > 1) {
107+
throw new IncomingRequestException('multiple signatures carrying tag="ocm" in Signature-Input');
108+
}
109+
if ($tagged === []) {
110+
throw new SignatureNotFoundException('no signature carrying tag="ocm" in Signature-Input');
111+
}
112+
$this->label = $tagged[0];
113+
114+
// A duplicated dictionary label is collapsed to its last entry by
115+
// RFC 8941 §4.2 parsing; that ambiguity on the OCM entry is
116+
// rejected outright.
117+
if (self::countLabel($signatureInputHeader, $this->label) > 1
118+
|| self::countLabel($signatureHeader, $this->label) > 1) {
95119
throw new IncomingRequestException(
96-
'multiple "' . 'ocm' . '" entries in signature headers'
120+
'multiple "' . $this->label . '" entries in signature headers'
97121
);
98122
}
99123

100-
if (!isset($inputs['ocm'])) {
101-
throw new SignatureNotFoundException('missing "' . 'ocm' . '" entry in Signature-Input');
102-
}
103-
if (!isset($signatures['ocm'])) {
104-
throw new SignatureNotFoundException('missing "' . 'ocm' . '" entry in Signature');
124+
if (!isset($signatures[$this->label])) {
125+
throw new IncomingRequestException('missing "' . $this->label . '" entry in Signature');
105126
}
106127

107-
$entry = $inputs['ocm'];
128+
$entry = $inputs[$this->label];
108129
$this->components = $entry['components'];
109130
$this->signatureParams = $entry['params'];
110-
$this->rawSignature = $signatures['ocm'];
131+
$this->rawSignature = $signatures[$this->label];
111132

112133
$this->verifyRequiredComponents();
113134
$this->verifyTimestamps();
@@ -121,9 +142,11 @@ public function __construct(
121142
try {
122143
$this->origin = Signatory::extractIdentityFromUri($keyId);
123144
} catch (IdentityNotFoundException) {
124-
// keyid may follow the OCM convention `<fqdn>#<id>`; the OCM layer
125-
// derives origin from the message body in that case.
126-
$this->origin = '';
145+
// keyid is not a URL; the OCM convention (and the examples in
146+
// the spec) use `<fqdn>[:port]#<id>`, in which case the origin
147+
// is the host part before the '#'. If neither form applies the
148+
// origin stays empty and getOrigin() rejects the request.
149+
$this->origin = self::extractHostFromKeyId($keyId);
127150
}
128151

129152
$paramsLine = SignatureBase::serializeSignatureParams($this->components, $this->signatureParams);
@@ -136,7 +159,7 @@ public function __construct(
136159
);
137160

138161
$this->setSigningElements([
139-
'label' => 'ocm',
162+
'label' => $this->label,
140163
'keyId' => $keyId,
141164
'algorithm' => isset($this->signatureParams['alg']) ? (string)$this->signatureParams['alg'] : '',
142165
'created' => isset($this->signatureParams['created']) ? (string)$this->signatureParams['created'] : '',
@@ -289,6 +312,22 @@ private function reconstructTargetUri(): string {
289312
return $scheme . '://' . $host . $path;
290313
}
291314

315+
/**
316+
* Derive the signer's origin from the OCM `<fqdn>[:port]#<id>` keyid
317+
* convention, e.g. `sender.example.org#key1`, by parsing the keyid as a
318+
* scheme-less authority. Returns '' when no host can be extracted.
319+
*/
320+
private static function extractHostFromKeyId(string $keyId): string {
321+
if (!str_contains($keyId, '#')) {
322+
return '';
323+
}
324+
try {
325+
return Signatory::extractIdentityFromUri('https://' . $keyId);
326+
} catch (IdentityNotFoundException) {
327+
return '';
328+
}
329+
}
330+
292331
/**
293332
* Collect the HTTP request fields covered by the signature, keyed by their
294333
* lowercased name. Derived components (`@*`) are produced inside
@@ -320,7 +359,7 @@ public function jsonSerialize(): array {
320359
parent::jsonSerialize(),
321360
[
322361
'origin' => $this->origin,
323-
'label' => 'ocm',
362+
'label' => $this->label,
324363
'components' => $this->components,
325364
'signatureParams' => $this->signatureParams,
326365
'signatureBase' => $this->signatureBaseString,

0 commit comments

Comments
 (0)