fix(ocm): treat RFC 9421 keyid as opaque, verify by sender origin

RFC 7517 §4.5 leaves the keyid structure unspecified, so the receiver
must not parse it. The signer origin now comes from the trusted OCM
share/sender identity; the JWK Set is resolved against that origin and
the keyid is matched opaquely to the JWKS kid. Aligns with the OCM
verification procedure and fixes the origin-mismatch / flaky-kid
failures in the two-port integration rig.

Reverts the per-request host-based kid back to a stable persisted kid,
drops keyid->host parsing, threads a sender-origin parameter through
verification, and resolves that origin in every OCM inbound entry point
(notifications, shares, token exchange, OCM requests) and the federation
rate limiter. Cavage is unchanged.

Assisted-by: ClaudeCode:glm-5.2

Signed-off-by: Micke Nordin <kano@sunet.se>

Author: mickenordin

apps/cloud_federation_api/lib/Controller/OCMRequestController.php

@@ -9,7 +9,6 @@
 
 namespace OCA\CloudFederationAPI\Controller;
 
-use JsonException;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\Attribute\BruteForceProtection;
@@ -18,6 +17,7 @@
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\AppFramework\Http\Response;
 use OCP\EventDispatcher\IEventDispatcher;
+use OCP\Federation\ICloudFederationProviderManager;
 use OCP\IRequest;
 use OCP\OCM\Events\OCMEndpointRequestEvent;
 use OCP\OCM\Exceptions\OCMArgumentException;
@@ -31,6 +31,7 @@ public function __construct(
 		IRequest $request,
 		private readonly IEventDispatcher $eventDispatcher,
 		private readonly IOCMDiscoveryService $ocmDiscoveryService,
+		private readonly ICloudFederationProviderManager $cloudFederationProviderManager,
 		private readonly LoggerInterface $logger,
 	) {
 		parent::__construct($appName, $request);
@@ -56,26 +57,29 @@ public function manageOCMRequests(string $ocmPath): Response {
 			throw new OCMArgumentException('path is not UTF-8');
 		}
 
+		// Resolve the signer origin from the payload before verification.
+		$payload = $this->request->getParams();
+		$origin = null;
+		if ($payload !== []) {
+			$identity = $this->cloudFederationProviderManager->resolveSenderIdentity($payload);
+			if ($identity !== null) {
+				try {
+					$origin = $this->ocmDiscoveryService->getHostFromOcmAddress($identity);
+				} catch (IncomingRequestException) {
+					// unresolvable origin; verification will fail without one
+				}
+			}
+		}
+
 		try {
-			// if request is signed and well signed, no exceptions are thrown
-			// if request is not signed and host is known for not supporting signed request, no exceptions are thrown
-			$signedRequest = $this->ocmDiscoveryService->getIncomingSignedRequest();
+			$signedRequest = $this->ocmDiscoveryService->getIncomingSignedRequest($origin);
 		} catch (IncomingRequestException $e) {
 			$this->logger->warning('incoming ocm request exception', ['exception' => $e]);
 			$response = new JSONResponse(['message' => $e->getMessage(), 'validationErrors' => []], Http::STATUS_BAD_REQUEST);
 			$response->throttle();
 			return $response;
 		}
 
-		// assuming that ocm request contains a json array
-		$payload = $signedRequest?->getBody() ?? file_get_contents('php://input');
-		try {
-			$payload = ($payload) ? json_decode($payload, true, 512, JSON_THROW_ON_ERROR) : null;
-		} catch (JsonException $e) {
-			$this->logger->debug('json decode error', ['exception' => $e]);
-			$payload = null;
-		}
-
 		$event = new OCMEndpointRequestEvent(
 			$this->request->getMethod(),
 			preg_replace('@/+@', '/', $ocmPath),

apps/cloud_federation_api/lib/Controller/RequestHandlerController.php

@@ -110,7 +110,8 @@ public function addShare($shareWith, $name, $description, $providerId, $owner, $
 			try {
 				// if request is signed and well signed, no exceptions are thrown
 				// if request is not signed and host is known for not supporting signed request, no exception are thrown
-				$signedRequest = $this->ocmDiscoveryService->getIncomingSignedRequest();
+				$origin = $this->ocmDiscoveryService->getHostFromOcmAddress($owner);
+				$signedRequest = $this->ocmDiscoveryService->getIncomingSignedRequest($origin);
 				$this->confirmSignedOrigin($signedRequest, 'owner', $owner);
 			} catch (IncomingRequestException $e) {
 				$this->logger->warning('incoming request exception', ['exception' => $e]);
@@ -307,10 +308,15 @@ public function receiveNotification($notificationType, $resourceType, $providerI
 
 		if (!$this->appConfig->getValueBool('core', OCMSignatoryManager::APPCONFIG_SIGN_DISABLED, lazy: true)) {
 			try {
-				// if request is signed and well signed, no exception are thrown
-				// if request is not signed and host is known for not supporting signed request, no exception are thrown
-				$signedRequest = $this->ocmDiscoveryService->getIncomingSignedRequest();
-				$this->confirmNotificationIdentity($signedRequest, $resourceType, $notification);
+				$identity = $this->resolveNotificationIdentity($resourceType, $notification);
+				$origin = null;
+				if ($identity !== '') {
+					$origin = $this->ocmDiscoveryService->getHostFromOcmAddress($identity);
+				}
+				$signedRequest = $this->ocmDiscoveryService->getIncomingSignedRequest($origin);
+				if ($identity !== '') {
+					$this->ocmDiscoveryService->confirmRequestOrigin($signedRequest?->getOrigin(), $identity);
+				}
 			} catch (IncomingRequestException $e) {
 				$this->logger->warning('incoming request exception', ['exception' => $e]);
 				return new JSONResponse(['message' => $e->getMessage(), 'validationErrors' => []], Http::STATUS_BAD_REQUEST);
@@ -450,22 +456,16 @@ private function confirmSignedOrigin(?IIncomingSignedRequest $signedRequest, str
 	}
 
 	/**
-	 *  confirm identity of the remote instance on notification, based on the share token.
+	 * Resolve the sender identity from a notification's sharedSecret.
+	 * Returns '' when the provider does not implement signed federation.
 	 *
-	 *  If request is not signed, we still verify that the hostname from the extracted value does,
-	 *  actually, not support signed request
-	 *
-	 * @param IIncomingSignedRequest|null $signedRequest
 	 * @param string $resourceType
+	 * @param array<string, mixed> $notification
 	 *
 	 * @throws IncomingRequestException
 	 * @throws BadRequestException
 	 */
-	private function confirmNotificationIdentity(
-		?IIncomingSignedRequest $signedRequest,
-		string $resourceType,
-		array $notification,
-	): void {
+	private function resolveNotificationIdentity(string $resourceType, array $notification): string {
 		$sharedSecret = $notification['sharedSecret'] ?? '';
 		if ($sharedSecret === '') {
 			throw new BadRequestException(['sharedSecret']);
@@ -481,14 +481,12 @@ private function confirmNotificationIdentity(
 					$mapping = Server::get(OcmTokenMapMapper::class)->getByAccessTokenId($accessTokenDb->getId());
 					$identity = $provider->getFederationIdFromSharedSecret($mapping->getRefreshToken(), $notification);
 				}
-			} else {
-				$this->logger->debug('cloud federation provider {provider} does not implements ISignedCloudFederationProvider', ['provider' => $provider::class]);
-				return;
+				return $identity;
 			}
+			$this->logger->debug('cloud federation provider {provider} does not implement ISignedCloudFederationProvider', ['provider' => $provider::class]);
 		} catch (\Exception $e) {
 			throw new IncomingRequestException($e->getMessage(), previous: $e);
 		}
-
-		$this->ocmDiscoveryService->confirmRequestOrigin($signedRequest?->getOrigin(), $identity);
+		return '';
 	}
 }

apps/cloud_federation_api/lib/Controller/TokenController.php

@@ -24,6 +24,7 @@
 use OCP\Authentication\Token\IToken;
 use OCP\IAppConfig;
 use OCP\IRequest;
+use OCP\OCM\IOCMDiscoveryService;
 use OCP\Security\ISecureRandom;
 use OCP\Security\Signature\Exceptions\IncomingRequestException;
 use OCP\Security\Signature\Exceptions\SignatoryNotFoundException;
@@ -51,19 +52,39 @@ public function __construct(
 		private readonly IAppConfig $appConfig,
 		private readonly OcmTokenMapMapper $ocmTokenMapMapper,
 		private readonly IShareManager $shareManager,
+		private readonly IOCMDiscoveryService $ocmDiscoveryService,
 	) {
 		parent::__construct('cloud_federation_api', $request);
 	}
 
+	/**
+	 * Resolve the signer origin from the refresh token's share, or null.
+	 */
+	private function resolveOriginFromRefreshToken(string $code): ?string {
+		if ($code === '') {
+			return null;
+		}
+		try {
+			$share = $this->shareManager->getShareByToken($code);
+			$sharedWith = $share->getSharedWith();
+			if ($sharedWith === null || $sharedWith === '') {
+				return null;
+			}
+			return $this->ocmDiscoveryService->getHostFromOcmAddress($sharedWith);
+		} catch (\Throwable) {
+			return null;
+		}
+	}
+
 	/**
 	 * Verify the signature of incoming request if available
 	 *
 	 * @return IIncomingSignedRequest|null null if remote does not support signed requests
 	 * @throws IncomingRequestException if signature is required but invalid
 	 */
-	private function verifySignedRequest(): ?IIncomingSignedRequest {
+	private function verifySignedRequest(?string $origin): ?IIncomingSignedRequest {
 		try {
-			$signedRequest = $this->signatureManager->getIncomingSignedRequest($this->signatoryManager);
+			$signedRequest = $this->signatureManager->getIncomingSignedRequest($this->signatoryManager, null, $origin);
 			$this->logger->debug('Token request signature verified', [
 				'origin' => $signedRequest->getOrigin()
 			]);
@@ -126,7 +147,7 @@ private function resolveJwtSigningKey(string $privateKeyPem): array {
 	#[FrontpageRoute(verb: 'POST', url: '/api/v1/access-token')]
 	public function accessToken(string $grant_type = '', string $code = ''): DataResponse {
 		try {
-			$signedRequest = $this->verifySignedRequest();
+			$signedRequest = $this->verifySignedRequest($this->resolveOriginFromRefreshToken($code));
 		} catch (IncomingRequestException $e) {
 			$this->logger->warning('Token request signature verification failed', [
 				'exception' => $e

apps/cloud_federation_api/tests/Controller/TokenControllerTest.php

@@ -23,6 +23,7 @@
 use OCP\Authentication\Token\IToken;
 use OCP\IAppConfig;
 use OCP\IRequest;
+use OCP\OCM\IOCMDiscoveryService;
 use OCP\Security\ISecureRandom;
 use OCP\Security\Signature\Exceptions\SignatoryNotFoundException;
 use OCP\Security\Signature\Exceptions\SignatureException;
@@ -47,6 +48,7 @@ class TokenControllerTest extends TestCase {
 	private IAppConfig&MockObject $appConfig;
 	private OcmTokenMapMapper&MockObject $ocmTokenMapMapper;
 	private IShareManager&MockObject $shareManager;
+	private IOCMDiscoveryService&MockObject $ocmDiscoveryService;
 
 	private TokenController $controller;
 
@@ -67,6 +69,7 @@ protected function setUp(): void {
 		$this->appConfig = $this->createMock(IAppConfig::class);
 		$this->ocmTokenMapMapper = $this->createMock(OcmTokenMapMapper::class);
 		$this->shareManager = $this->createMock(IShareManager::class);
+		$this->ocmDiscoveryService = $this->createMock(IOCMDiscoveryService::class);
 
 		$this->controller = new TokenController(
 			$this->request,
@@ -79,6 +82,7 @@ protected function setUp(): void {
 			$this->appConfig,
 			$this->ocmTokenMapMapper,
 			$this->shareManager,
+			$this->ocmDiscoveryService,
 		);
 	}
 

lib/private/AppFramework/Http/Attributes/FederationRateLimit.php

@@ -13,6 +13,7 @@
 use OC\OCM\OCMDiscoveryService;
 use OCA\Federation\TrustedServers;
 use OCP\AppFramework\Http\Attribute\AnonRateLimit;
+use OCP\Federation\ICloudFederationProviderManager;
 use OCP\IRequest;
 use OCP\Server;
 
@@ -24,12 +25,14 @@
  */
 #[Attribute(Attribute::TARGET_METHOD)]
 class FederationRateLimit extends AnonRateLimit {
+	private readonly ICloudFederationProviderManager $federationProviderManager;
 	private readonly OCMDiscoveryService $discoveryService;
 	private readonly ?TrustedServers $trustedServers;
 
 	public function __construct(int $limit, int $period) {
 		parent::__construct($limit, $period);
 
+		$this->federationProviderManager = Server::get(ICloudFederationProviderManager::class);
 		$this->discoveryService = Server::get(OCMDiscoveryService::class);
 		$this->trustedServers = Server::get(TrustedServers::class);
 	}
@@ -41,14 +44,22 @@ public function shouldApply(IRequest $request): bool {
 		}
 
 		try {
-			$signedRequest = $this->discoveryService->getIncomingSignedRequest();
+			// Resolve the signer origin from the payload so trusted servers
+			// can be exempted.
+			$parsed = $request->getParams();
+			$identity = $this->federationProviderManager->resolveSenderIdentity($parsed);
+			$origin = null;
+			if ($identity !== null) {
+				$origin = $this->discoveryService->getHostFromOcmAddress($identity);
+			}
+
+			$signedRequest = $this->discoveryService->getIncomingSignedRequest($origin);
 			if (!$signedRequest) {
 				return true;
 			}
-			$signedRequest->verify();
 			return !$this->trustedServers->isTrustedServer($signedRequest->getOrigin());
 		} catch (\Exception) {
-			// no or invalid signature
+			// no or invalid signature, or unresolvable origin
 			return true;
 		}
 	}

lib/private/Federation/CloudFederationProviderManager.php

@@ -18,6 +18,7 @@
 use OCP\Federation\ICloudFederationProviderManager;
 use OCP\Federation\ICloudFederationShare;
 use OCP\Federation\ICloudIdManager;
+use OCP\Federation\ISignedCloudFederationProvider;
 use OCP\Http\Client\IClient;
 use OCP\Http\Client\IClientService;
 use OCP\Http\Client\IResponse;
@@ -105,6 +106,40 @@ public function getCloudFederationProvider($resourceType) {
 		}
 	}
 
+	/**
+	 * @inheritDoc
+	 *
+	 * Notifications resolve via sharedSecret; shares via owner/sender.
+	 * No access-token exchange here (app-layer); callers keep their own.
+	 */
+	#[\Override]
+	public function resolveSenderIdentity(array $body): ?string {
+		$resourceType = $body['resourceType'] ?? '';
+		if ($resourceType !== '') {
+			$notification = $body['notification'] ?? null;
+			$sharedSecret = is_array($notification) ? ($notification['sharedSecret'] ?? '') : '';
+			if ($sharedSecret !== '') {
+				try {
+					$provider = $this->getCloudFederationProvider($resourceType);
+					if ($provider instanceof ISignedCloudFederationProvider || $provider instanceof \NCU\Federation\ISignedCloudFederationProvider) {
+						$identity = $provider->getFederationIdFromSharedSecret($sharedSecret, is_array($notification) ? $notification : []);
+						if ($identity !== '') {
+							return $identity;
+						}
+					}
+				} catch (\Exception) {
+					// unresolved; fall through to share-style fields
+				}
+			}
+		}
+		foreach (['owner', 'sender', 'sharedBy'] as $field) {
+			if (isset($body[$field]) && is_string($body[$field]) && $body[$field] !== '') {
+				return $body[$field];
+			}
+		}
+		return null;
+	}
+
 	/**
 	 * @deprecated 29.0.0 - Use {@see sendCloudShare()} instead and handle errors manually
 	 */

lib/private/OCM/OCMDiscoveryService.php

@@ -259,9 +259,9 @@ public function getLocalOCMProvider(bool $fullDetails = true): IOCMProvider {
 	 * @since 33.0.0
 	 */
 	#[\Override]
-	public function getIncomingSignedRequest(): ?IIncomingSignedRequest {
+	public function getIncomingSignedRequest(?string $origin = null): ?IIncomingSignedRequest {
 		try {
-			$signedRequest = $this->signatureManager->getIncomingSignedRequest($this->signatoryManager);
+			$signedRequest = $this->signatureManager->getIncomingSignedRequest($this->signatoryManager, null, $origin);
 			$this->logger->debug('signed request available', ['signedRequest' => $signedRequest]);
 			return $signedRequest;
 		} catch (SignatureNotFoundException|SignatoryNotFoundException $e) {
@@ -310,9 +310,12 @@ public function confirmRequestOrigin(?string $signedOrigin, string $ocmAddress):
 	}
 
 	/**
+	 * Extract the signer origin (host) from an OCM address (`user@host`).
+	 *
 	 * @throws IncomingRequestException on malformed address or unresolvable host
 	 */
-	private function getHostFromOcmAddress(string $entry): string {
+	#[\Override]
+	public function getHostFromOcmAddress(string $entry): string {
 		try {
 			$cloudId = $this->cloudIdManager->resolveCloudId(trim($entry, '@'));
 			return $this->signatureManager->extractIdentityFromUri($cloudId->getRemote());