From b399d40d2af03e7224d3b83f86d66d2a782bec98 Mon Sep 17 00:00:00 2001 From: Ricardo Garim Date: Thu, 11 Sep 2025 21:14:44 -0300 Subject: [PATCH 1/3] feat: adds canAccessMedia middleware --- .../src/api/_matrix/media.ts | 7 ++- .../federation-matrix/src/api/middlewares.ts | 47 +++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 ee/packages/federation-matrix/src/api/middlewares.ts diff --git a/ee/packages/federation-matrix/src/api/_matrix/media.ts b/ee/packages/federation-matrix/src/api/_matrix/media.ts index c6f3b8a4f0d0c..23cb0d464a4c0 100644 --- a/ee/packages/federation-matrix/src/api/_matrix/media.ts +++ b/ee/packages/federation-matrix/src/api/_matrix/media.ts @@ -7,6 +7,7 @@ import { Logger } from '@rocket.chat/logger'; import { ajv } from '@rocket.chat/rest-typings/dist/v1/Ajv'; import { MatrixMediaService } from '../../services/MatrixMediaService'; +import { canAccessMedia } from '../middlewares'; const logger = new Logger('federation-matrix:media'); @@ -76,7 +77,7 @@ async function getMediaFile(mediaId: string, serverName: string): Promise<{ file } export const getMatrixMediaRoutes = (homeserverServices: HomeserverServices) => { - const { config } = homeserverServices; + const { config, federationAuth } = homeserverServices; const router = new Router('/federation'); router.get( @@ -86,12 +87,14 @@ export const getMatrixMediaRoutes = (homeserverServices: HomeserverServices) => response: { 200: isBufferResponseProps, 401: isErrorResponseProps, + 403: isErrorResponseProps, 404: isErrorResponseProps, 429: isErrorResponseProps, 500: isErrorResponseProps, }, tags: ['Federation', 'Media'], }, + canAccessMedia(federationAuth), async (c) => { try { const { mediaId } = c.req.param(); @@ -138,10 +141,12 @@ export const getMatrixMediaRoutes = (homeserverServices: HomeserverServices) => response: { 200: isBufferResponseProps, 401: isErrorResponseProps, + 403: isErrorResponseProps, 404: isErrorResponseProps, }, tags: ['Federation', 'Media'], }, + canAccessMedia(federationAuth), async (context: any) => { try { const { mediaId } = context.req.param(); diff --git a/ee/packages/federation-matrix/src/api/middlewares.ts b/ee/packages/federation-matrix/src/api/middlewares.ts new file mode 100644 index 0000000000000..fcd9f631032d0 --- /dev/null +++ b/ee/packages/federation-matrix/src/api/middlewares.ts @@ -0,0 +1,47 @@ +import type { EventAuthorizationService } from '@hs/federation-sdk'; +import type { Context, Next } from 'hono'; +import type { ContentfulStatusCode } from 'hono/utils/http-status'; + +const errCodes: Record = { + M_UNAUTHORIZED: { + errcode: 'M_UNAUTHORIZED', + error: 'Invalid or missing signature', + status: 401, + }, + M_FORBIDDEN: { + errcode: 'M_FORBIDDEN', + error: 'Access denied', + status: 403, + }, + M_UNKNOWN: { + errcode: 'M_UNKNOWN', + error: 'Internal server error while processing request', + status: 500, + }, +}; + +export const canAccessMedia = (federationAuth: EventAuthorizationService) => async (c: Context, next: Next) => { + try { + const verificationResult = await federationAuth.canAccessMediaFromAuthorizationHeader( + c.req.param('mediaId'), + c.req.header('Authorization') || '', + c.req.method, + c.req.path, + await c.req.json(), + ); + + if (!verificationResult.authorized) { + return c.json( + { + errcode: errCodes[verificationResult.errorCode].errcode, + error: errCodes[verificationResult.errorCode].error, + }, + errCodes[verificationResult.errorCode].status, + ); + } + + return next(); + } catch (error) { + return c.json(errCodes.M_UNKNOWN, 500); + } +}; From 5343239bf84882a487affde655b2ec95c7251cb9 Mon Sep 17 00:00:00 2001 From: Ricardo Garim Date: Fri, 12 Sep 2025 00:35:19 -0300 Subject: [PATCH 2/3] refactor: rewrites the way in which signature is validated --- .../src/api/_matrix/media.ts | 1 - .../federation-matrix/src/api/middlewares.ts | 59 ++++++++++++------- 2 files changed, 38 insertions(+), 22 deletions(-) diff --git a/ee/packages/federation-matrix/src/api/_matrix/media.ts b/ee/packages/federation-matrix/src/api/_matrix/media.ts index 23cb0d464a4c0..7ab093f9034a8 100644 --- a/ee/packages/federation-matrix/src/api/_matrix/media.ts +++ b/ee/packages/federation-matrix/src/api/_matrix/media.ts @@ -125,7 +125,6 @@ export const getMatrixMediaRoutes = (homeserverServices: HomeserverServices) => body: multipartResponse.body, }; } catch (error) { - logger.error('Federation media download error:', error); return { statusCode: 500, body: { errcode: 'M_UNKNOWN', error: 'Internal server error' }, diff --git a/ee/packages/federation-matrix/src/api/middlewares.ts b/ee/packages/federation-matrix/src/api/middlewares.ts index fcd9f631032d0..8e1b0701c86f8 100644 --- a/ee/packages/federation-matrix/src/api/middlewares.ts +++ b/ee/packages/federation-matrix/src/api/middlewares.ts @@ -20,28 +20,45 @@ const errCodes: Record async (c: Context, next: Next) => { - try { - const verificationResult = await federationAuth.canAccessMediaFromAuthorizationHeader( - c.req.param('mediaId'), - c.req.header('Authorization') || '', - c.req.method, - c.req.path, - await c.req.json(), - ); +export const canAccessMedia = (federationAuth: EventAuthorizationService) => { + return async (c: Context, next: Next) => { + const mediaId = c.req.param('mediaId'); + const authHeader = c.req.header('Authorization') || ''; + const { method } = c.req; + const { path } = c.req; + const query = c.req.query(); - if (!verificationResult.authorized) { - return c.json( - { - errcode: errCodes[verificationResult.errorCode].errcode, - error: errCodes[verificationResult.errorCode].error, - }, - errCodes[verificationResult.errorCode].status, - ); + let uriForSignature = path; + const queryString = new URLSearchParams(query).toString(); + if (queryString) { + uriForSignature = `${path}?${queryString}`; } - return next(); - } catch (error) { - return c.json(errCodes.M_UNKNOWN, 500); - } + try { + const body = method === 'GET' ? undefined : await c.req.json(); + + const verificationResult = await federationAuth.canAccessMediaFromAuthorizationHeader( + mediaId, + authHeader, + method, + uriForSignature, // use URI with query params for signature verification + body, + ); + + if (!verificationResult.authorized) { + const errorResponse = errCodes[verificationResult.errorCode]; + return c.json( + { + errcode: errorResponse.errcode, + error: errorResponse.error, + }, + errorResponse.status, + ); + } + + return next(); + } catch (error) { + return c.json(errCodes.M_UNKNOWN, 500); + } + }; }; From 9186a1e917f9e4693cf3e3b17b078de9ef772804 Mon Sep 17 00:00:00 2001 From: Ricardo Garim Date: Sat, 13 Sep 2025 13:56:33 -0300 Subject: [PATCH 3/3] chore: adds empty response check into downloadFromRemoteServer --- .../federation-matrix/src/services/MatrixMediaService.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ee/packages/federation-matrix/src/services/MatrixMediaService.ts b/ee/packages/federation-matrix/src/services/MatrixMediaService.ts index c218885dbaaf7..e25a4a97377f3 100644 --- a/ee/packages/federation-matrix/src/services/MatrixMediaService.ts +++ b/ee/packages/federation-matrix/src/services/MatrixMediaService.ts @@ -112,6 +112,9 @@ export class MatrixMediaService { } const buffer = await this.homeserverServices.media.downloadFromRemoteServer(parts.serverName, parts.mediaId); + if (!buffer) { + throw new Error('Download from remote server returned null content.'); + } const uploadedFile = await Upload.uploadFile({ userId: metadata.userId || 'federation',