fix: federation valid request being reject due to wrong schemas

.changeset/heavy-lions-attend.md

@@ -0,0 +1,15 @@
+---
+'@rocket.chat/federation-matrix': patch
+'@rocket.chat/meteor': patch
+---
+
+Fixes federation endpoints rejecting requests that are valid per the Matrix specification:
+
+- `publicRooms` (GET and POST) required params/fields the spec marks optional
+- `query/profile` rejected spec-valid profile fields such as `m.tz`
+- `get_missing_events` required the optional `limit` field and bounded it
+- `make_join` returned 500 instead of 400 `M_INCOMPATIBLE_ROOM_VERSION` for unsupported room versions
+- `backfill` rejected spec-valid `limit` values
+- `send` rejected an entire transaction when a single PDU didn't match a fixed event shape, instead of reporting failures per PDU
+
+Also links every federation endpoint to its definition in the Matrix specification.

ee/packages/federation-matrix/src/api/.well-known/server.ts

@@ -1,8 +1,8 @@
-import { Router } from '@rocket.chat/http-router';
-import { ajv } from '@rocket.chat/rest-typings/dist/v1/Ajv';
 import { createHash } from 'node:crypto';
 
 import { federationSDK } from '@rocket.chat/federation-sdk';
+import { Router } from '@rocket.chat/http-router';
+import { ajv } from '@rocket.chat/rest-typings/dist/v1/Ajv';
 
 const WellKnownServerResponseSchema = {
 	type: 'object',
@@ -20,6 +20,8 @@ const isWellKnownServerResponseProps = ajv.compile(WellKnownServerResponseSchema
 // TODO: After changing the domain setting this route is still reporting the old domain until the server is restarted
 // TODO: this is wrong, is siteurl !== domain this path should return 404. this path is to discover the final address, domain being the "proxy" and siteurl the final destination, if domain is different, well-known should be served there, not here.
 export const getWellKnownRoutes = () => {
+	// GET /.well-known/matrix/server
+	// https://spec.matrix.org/v1.19/server-server-api/#getwell-knownmatrixserver
 	return new Router('/matrix').get(
 		'/server',
 		{

ee/packages/federation-matrix/src/api/_matrix/invite.spec.ts

@@ -0,0 +1,144 @@
+import { FederationMatrix } from '@rocket.chat/core-services';
+import { federationSDK } from '@rocket.chat/federation-sdk';
+import { Users } from '@rocket.chat/models';
+
+import { getMatrixInviteRoutes } from './invite';
+
+jest.mock('@rocket.chat/core-services', () => ({
+	FederationMatrix: {
+		canUserAccessFederation: jest.fn(),
+	},
+}));
+
+jest.mock('@rocket.chat/models', () => ({
+	Users: {
+		findOneByUsername: jest.fn(),
+	},
+}));
+
+jest.mock('@rocket.chat/federation-sdk', () => ({
+	federationSDK: {
+		verifyRequestSignature: jest.fn(),
+		getConfig: jest.fn(),
+		processInvite: jest.fn(),
+	},
+	NotAllowedError: class NotAllowedError extends Error {},
+	errCodes: {
+		M_UNAUTHORIZED: { errcode: 'M_UNAUTHORIZED', error: 'Unauthorized', status: 401 },
+		M_UNKNOWN: { errcode: 'M_UNKNOWN', error: 'Unknown error' },
+	},
+}));
+
+const mockVerifyRequestSignature = federationSDK.verifyRequestSignature as jest.MockedFunction<typeof federationSDK.verifyRequestSignature>;
+const mockGetConfig = federationSDK.getConfig as jest.MockedFunction<typeof federationSDK.getConfig>;
+const mockProcessInvite = federationSDK.processInvite as jest.MockedFunction<typeof federationSDK.processInvite>;
+const mockFindOneByUsername = Users.findOneByUsername as jest.MockedFunction<typeof Users.findOneByUsername>;
+const mockCanUserAccessFederation = FederationMatrix.canUserAccessFederation as jest.MockedFunction<
+	typeof FederationMatrix.canUserAccessFederation
+>;
+
+const OUR_SERVER_NAME = 'rocketchat.local';
+
+const buildInviteEvent = (stateKey: string) => ({
+	type: 'm.room.member',
+	state_key: stateKey,
+	sender: '@attacker:attacker.com',
+	room_id: '!room:attacker.com',
+	origin_server_ts: 1600000000000,
+	depth: 1,
+	prev_events: [],
+	auth_events: [],
+	content: { membership: 'invite' },
+});
+
+const sendInvite = async (event: unknown) =>
+	getMatrixInviteRoutes()
+		.getHonoRouter()
+		.request('/v2/invite/!room:attacker.com/$event', {
+			method: 'PUT',
+			headers: {
+				'Authorization': 'X-Matrix origin="attacker.com"',
+				'Content-Type': 'application/json',
+			},
+			body: JSON.stringify({
+				room_version: '10',
+				event,
+				invite_room_state: [{ type: 'm.room.create', state_key: '', content: { creator: '@attacker:attacker.com' } }],
+			}),
+		});
+
+describe('PUT /_matrix/federation/v2/invite/:roomId/:eventId', () => {
+	beforeEach(() => {
+		jest.clearAllMocks();
+
+		mockVerifyRequestSignature.mockResolvedValue({ origin: 'attacker.com' } as any);
+		mockGetConfig.mockImplementation((key) => (key === 'serverName' ? OUR_SERVER_NAME : undefined) as any);
+		mockFindOneByUsername.mockResolvedValue({ _id: 'victimId', username: 'victim' } as any);
+		mockCanUserAccessFederation.mockResolvedValue(true);
+		mockProcessInvite.mockImplementation(async (event: any) => ({ event }) as any);
+	});
+
+	it('should reject an invite whose state_key belongs to another homeserver', async () => {
+		const response = await sendInvite(buildInviteEvent(`@victim:attacker.com`));
+
+		expect(response.status).toBe(400);
+		expect(await response.json()).toEqual({
+			errcode: 'M_UNKNOWN',
+			error: 'The invite event must be for a user of this server',
+		});
+		expect(mockFindOneByUsername).not.toHaveBeenCalled();
+		expect(mockProcessInvite).not.toHaveBeenCalled();
+	});
+
+	it('should reject an invite whose state_key is not a valid user ID', async () => {
+		const response = await sendInvite(buildInviteEvent('victim'));
+
+		expect(response.status).toBe(400);
+		expect(await response.json()).toEqual({
+			errcode: 'M_UNKNOWN',
+			error: 'The invite event state_key is not a valid user ID',
+		});
+		expect(mockFindOneByUsername).not.toHaveBeenCalled();
+		expect(mockProcessInvite).not.toHaveBeenCalled();
+	});
+
+	it('should reject an invite whose state_key has an empty localpart', async () => {
+		const response = await sendInvite(buildInviteEvent(`@:${OUR_SERVER_NAME}`));
+
+		expect(response.status).toBe(400);
+		expect(await response.json()).toEqual({
+			errcode: 'M_UNKNOWN',
+			error: 'The invite event state_key is not a valid user ID',
+		});
+		expect(mockFindOneByUsername).not.toHaveBeenCalled();
+		expect(mockProcessInvite).not.toHaveBeenCalled();
+	});
+
+	it('should reject an invite addressed to a user that does not exist on this server', async () => {
+		mockFindOneByUsername.mockResolvedValue(null);
+
+		const response = await sendInvite(buildInviteEvent(`@ghost:${OUR_SERVER_NAME}`));
+
+		expect(response.status).toBe(403);
+		expect(await response.json()).toEqual({
+			errcode: 'M_FORBIDDEN',
+			error: 'User does not have permission to access federation',
+		});
+		expect(mockProcessInvite).not.toHaveBeenCalled();
+	});
+
+	it('should accept a state_key whose localpart contains characters the spec allows', async () => {
+		const response = await sendInvite(buildInviteEvent(`@victim+1/2:${OUR_SERVER_NAME}`));
+
+		expect(response.status).toBe(200);
+		expect(mockFindOneByUsername).toHaveBeenCalledWith('victim+1/2');
+	});
+
+	it('should process an invite addressed to a user of this server', async () => {
+		const response = await sendInvite(buildInviteEvent(`@victim:${OUR_SERVER_NAME}`));
+
+		expect(response.status).toBe(200);
+		expect(mockFindOneByUsername).toHaveBeenCalledWith('victim');
+		expect(mockProcessInvite).toHaveBeenCalledTimes(1);
+	});
+});

ee/packages/federation-matrix/src/api/_matrix/invite.ts

@@ -4,6 +4,7 @@ import { Router } from '@rocket.chat/http-router';
 import { Users } from '@rocket.chat/models';
 import { ajv } from '@rocket.chat/rest-typings/dist/v1/Ajv';
 
+import { getUsernameServername } from '../../helpers/getUsernameServername';
 import { logger } from '../logger';
 import { isAuthenticatedMiddleware } from '../middlewares/isAuthenticated';
 
@@ -130,10 +131,26 @@ const ProcessInviteResponseSchema = {
 const isProcessInviteResponseProps = ajv.compile(ProcessInviteResponseSchema);
 
 export const getMatrixInviteRoutes = () => {
+	// PUT /_matrix/federation/v2/invite/{roomId}/{eventId}
+	// https://spec.matrix.org/v1.19/server-server-api/#put_matrixfederationv2inviteroomideventid
 	return new Router('/federation').put(
 		'/v2/invite/:roomId/:eventId',
 		{
-			body: ajv.compile({ type: 'object' }), // TODO: add schema from room package.
+			// TODO: add schema from room package. `event` is a PDU whose format varies by room
+			// version, so it stays unconstrained here; room_version and event are required per spec.
+			body: ajv.compile({
+				type: 'object',
+				properties: {
+					room_version: { type: 'string' },
+					event: { type: 'object' },
+					invite_room_state: {
+						type: 'array',
+						items: { type: 'object' },
+						nullable: true,
+					},
+				},
+				required: ['room_version', 'event'],
+			}),
 			params: isProcessInviteParamsProps,
 			response: {
 				200: isProcessInviteResponseProps,
@@ -146,30 +163,72 @@ export const getMatrixInviteRoutes = () => {
 			const { roomId, eventId } = c.req.param();
 			const { event, room_version: roomVersion, invite_room_state: strippedStateEvents } = await c.req.json();
 
-			const userToCheck = event.state_key as string;
+			const userToCheck = event.state_key;
 
-			if (!userToCheck) {
-				throw new Error('join event has missing state key, unable to determine user to join');
+			// matches Synapse: the PDU itself stays unvalidated, but an event that is not an invite
+			// membership event cannot be processed, so reject it instead of failing later
+			if (typeof userToCheck !== 'string' || !userToCheck || event.type !== 'm.room.member' || event.content?.membership !== 'invite') {
+				return {
+					body: {
+						errcode: 'M_UNKNOWN',
+						error: 'The event was not an m.room.member invite event',
+					},
+					statusCode: 400,
+				};
 			}
 
+			// spec: servers SHOULD return M_INVALID_PARAM if m.room.create is missing from invite_room_state
 			if (!strippedStateEvents?.some((e: any) => e.type === 'm.room.create')) {
 				return {
 					body: {
-						errcode: 'M_MISSING_PARAM',
+						errcode: 'M_INVALID_PARAM',
 						error: 'Missing invite_room_state: m.room.create event is required',
 					},
 					statusCode: 400,
 				};
 			}
 
-			const [username /* domain */] = userToCheck.split(':');
+			// spec grammar is `@localpart:server_name`, where localpart is non-empty. deliberately
+			// not `validateFederatedUsername`, which is stricter than the spec and would reject
+			// legal localparts containing `/` or `+`
+			if (!/^@[A-Za-z0-9_=/.+-]+:.+$/.test(userToCheck)) {
+				return {
+					body: {
+						errcode: 'M_UNKNOWN',
+						error: 'The invite event state_key is not a valid user ID',
+					},
+					statusCode: 400,
+				};
+			}
 
-			// TODO: check domain
+			// an invite addressed to a user of another homeserver would create a local subscription
+			// that can never be accepted, since the remote server never invited our copy of that user
+			const [username, , isLocalUser] = getUsernameServername(userToCheck, federationSDK.getConfig('serverName'));
 
-			const ourUser = await Users.findOneByUsername(username.slice(1));
+			if (!isLocalUser) {
+				return {
+					body: {
+						errcode: 'M_UNKNOWN',
+						error: 'The invite event must be for a user of this server',
+					},
+					statusCode: 400,
+				};
+			}
 
+			const ourUser = await Users.findOneByUsername(username);
+
+			// same response as the federation permission check below, so an unauthorized remote
+			// server cannot use the invite endpoint to probe which local users exist
 			if (!ourUser) {
-				throw new Error('user not found not processing invite');
+				logger.info({ msg: 'Invite for unknown local user, rejecting invite to room', userId: userToCheck, roomId });
+
+				return {
+					body: {
+						errcode: 'M_FORBIDDEN',
+						error: 'User does not have permission to access federation',
+					},
+					statusCode: 403,
+				};
 			}
 
 			// check federation permission before processing the invite

ee/packages/federation-matrix/src/api/_matrix/key/server.ts

@@ -33,6 +33,8 @@ const ServerKeyResponseSchema = {
 const isServerKeyResponseProps = ajv.compile(ServerKeyResponseSchema);
 
 export const getKeyServerRoutes = () => {
+	// GET /_matrix/key/v2/server
+	// https://spec.matrix.org/v1.19/server-server-api/#get_matrixkeyv2server
 	return new Router('/key').get(
 		'/v2/server',
 		{

ee/packages/federation-matrix/src/api/_matrix/make-leave.ts

@@ -56,6 +56,8 @@ const isMakeLeaveErrorResponseProps = ajv.compile({
 });
 
 export const getMatrixMakeLeaveRoutes = () => {
+	// GET /_matrix/federation/v1/make_leave/{roomId}/{userId}
+	// https://spec.matrix.org/v1.19/server-server-api/#get_matrixfederationv1make_leaveroomiduserid
 	return new Router('/federation').get(
 		'/v1/make_leave/:roomId/:userId',
 		{