feat: e2ee security hardening (RocketChat#36942)

Author: cardoso

apps/meteor/app/lib/server/functions/notifications/desktop.ts

@@ -57,6 +57,9 @@ export async function notifyDesktopUser({
 				...('t' in message && {
 					t: message.t,
 				}),
+				...('content' in message && {
+					content: message.content,
+				}),
 			},
 			name,
 			audioNotificationValue,

apps/meteor/app/message-pin/server/pinMessage.ts

@@ -119,6 +119,7 @@ export async function pinMessage(message: IMessage, userId: string, pinnedAt?: D
 				text: originalMessage.msg,
 				author_name: originalMessage.u.username,
 				author_icon: getUserAvatarURL(originalMessage.u.username),
+				content: originalMessage.content,
 				ts: originalMessage.ts,
 				attachments: attachments.map(recursiveRemove),
 			},

apps/meteor/client/hooks/notification/useDesktopNotification.ts

@@ -22,10 +22,14 @@ export const useDesktopNotification = () => {
 			return;
 		}
 
-		if (notification.payload.message?.t === 'e2e') {
+		const { message } = notification.payload;
+
+		if (message.t === 'e2e' && message.content) {
 			const e2eRoom = await e2e.getInstanceByRoomId(notification.payload.rid);
 			if (e2eRoom) {
-				notification.text = (await e2eRoom.decrypt(notification.payload.message.msg)).text;
+				const decrypted = await e2eRoom.decrypt(message.content);
+				// TODO(@cardoso): review backward compatibility
+				notification.text = decrypted.msg ?? '';
 			}
 		}
 

apps/meteor/client/hooks/roomActions/useE2EERoomAction.spec.ts

@@ -3,7 +3,6 @@ import { useSetting, usePermission, useEndpoint } from '@rocket.chat/ui-contexts
 import { act, renderHook, waitFor } from '@testing-library/react';
 
 import { OtrRoomState } from '../../../app/otr/lib/OtrRoomState';
-import { E2EEState } from '../../lib/e2ee/E2EEState';
 import { e2e } from '../../lib/e2ee/rocketchat.e2e';
 import { useRoom, useRoomSubscription } from '../../views/room/contexts/RoomContext';
 import { useE2EEState } from '../../views/room/hooks/useE2EEState';
@@ -70,7 +69,7 @@ describe('useE2EERoomAction', () => {
 		(useSetting as jest.Mock).mockReturnValue(true);
 		(useRoom as jest.Mock).mockReturnValue(mockRoom);
 		(useRoomSubscription as jest.Mock).mockReturnValue(mockSubscription);
-		(useE2EEState as jest.Mock).mockReturnValue(E2EEState.READY);
+		(useE2EEState as jest.Mock).mockReturnValue('READY');
 		(usePermission as jest.Mock).mockReturnValue(true);
 		(useEndpoint as jest.Mock).mockReturnValue(jest.fn().mockResolvedValue({ success: true }));
 		(e2e.isReady as jest.Mock).mockReturnValue(true);

apps/meteor/client/hooks/roomActions/useE2EERoomAction.ts

@@ -6,8 +6,6 @@ import { useMemo } from 'react';
 import { useTranslation } from 'react-i18next';
 
 import { OtrRoomState } from '../../../app/otr/lib/OtrRoomState';
-import { E2EEState } from '../../lib/e2ee/E2EEState';
-import { E2ERoomState } from '../../lib/e2ee/E2ERoomState';
 import { getRoomTypeTranslation } from '../../lib/getRoomTypeTranslation';
 import { useRoom, useRoomSubscription } from '../../views/room/contexts/RoomContext';
 import type { RoomToolboxActionConfig } from '../../views/room/contexts/RoomToolboxContext';
@@ -23,7 +21,7 @@ export const useE2EERoomAction = () => {
 	const subscription = useRoomSubscription();
 	const e2eeState = useE2EEState();
 	const e2eeRoomState = useE2EERoomState(room._id);
-	const isE2EEReady = e2eeState === E2EEState.READY || e2eeState === E2EEState.SAVE_PASSWORD;
+	const isE2EEReady = e2eeState === 'READY' || e2eeState === 'SAVE_PASSWORD';
 	const readyToEncrypt = isE2EEReady || room.encrypted;
 	const permittedToToggleEncryption = usePermission('toggle-room-e2e-encryption', room._id);
 	const permittedToEditRoom = usePermission('edit-room', room._id);
@@ -34,13 +32,7 @@ export const useE2EERoomAction = () => {
 	const { otrState } = useOTR();
 
 	const isE2EERoomNotReady = () => {
-		if (
-			e2eeRoomState === E2ERoomState.NO_PASSWORD_SET ||
-			e2eeRoomState === E2ERoomState.NOT_STARTED ||
-			e2eeRoomState === E2ERoomState.DISABLED ||
-			e2eeRoomState === E2ERoomState.ERROR ||
-			e2eeRoomState === E2ERoomState.WAITING_KEYS
-		) {
+		if (e2eeRoomState === 'NOT_STARTED' || e2eeRoomState === 'DISABLED' || e2eeRoomState === 'ERROR' || e2eeRoomState === 'WAITING_KEYS') {
 			return true;
 		}
 

apps/meteor/client/lib/e2ee/E2EEState.ts

@@ -1,9 +1 @@
-export enum E2EEState {
-	NOT_STARTED = 'NOT_STARTED',
-	DISABLED = 'DISABLED',
-	LOADING_KEYS = 'LOADING_KEYS',
-	READY = 'READY',
-	SAVE_PASSWORD = 'SAVE_PASSWORD',
-	ENTER_PASSWORD = 'ENTER_PASSWORD',
-	ERROR = 'ERROR',
-}
+export type E2EEState = 'NOT_STARTED' | 'DISABLED' | 'LOADING_KEYS' | 'READY' | 'SAVE_PASSWORD' | 'ENTER_PASSWORD' | 'ERROR';

apps/meteor/client/lib/e2ee/E2ERoomState.ts

@@ -1,12 +1,9 @@
-export enum E2ERoomState {
-	NO_PASSWORD_SET = 'NO_PASSWORD_SET',
-	NOT_STARTED = 'NOT_STARTED',
-	DISABLED = 'DISABLED',
-	HANDSHAKE = 'HANDSHAKE',
-	ESTABLISHING = 'ESTABLISHING',
-	CREATING_KEYS = 'CREATING_KEYS',
-	WAITING_KEYS = 'WAITING_KEYS',
-	KEYS_RECEIVED = 'KEYS_RECEIVED',
-	READY = 'READY',
-	ERROR = 'ERROR',
-}
+export type E2ERoomState =
+	| 'NOT_STARTED'
+	| 'DISABLED'
+	| 'ESTABLISHING'
+	| 'CREATING_KEYS'
+	| 'WAITING_KEYS'
+	| 'KEYS_RECEIVED'
+	| 'READY'
+	| 'ERROR';

apps/meteor/client/lib/e2ee/binary.spec.ts

@@ -0,0 +1,37 @@
+import { Binary } from './binary';
+
+describe('Binary', () => {
+	describe('toString', () => {
+		it('should convert ArrayBuffer to string', () => {
+			const array = new Uint8Array([72, 101, 108, 108, 111]); // "Hello"
+			const result = Binary.encode(array.buffer);
+			expect(result).toBe('Hello');
+		});
+
+		it('should handle empty ArrayBuffer', () => {
+			const buffer = new ArrayBuffer(0);
+			const result = Binary.encode(buffer);
+			expect(result).toBe('');
+		});
+	});
+
+	describe('toArrayBuffer', () => {
+		it('should convert string to ArrayBuffer', () => {
+			const str = 'Hello';
+			const buffer = Binary.decode(str);
+			const uint8 = new Uint8Array(buffer);
+			expect(Array.from(uint8)).toEqual([72, 101, 108, 108, 111]);
+		});
+
+		it('should handle empty string', () => {
+			const str = '';
+			const buffer = Binary.decode(str);
+			expect(buffer.byteLength).toBe(0);
+		});
+
+		it('should throw RangeError for illegal char code', () => {
+			const str = 'Hello\u0100'; // Character with char code 256
+			expect(() => Binary.decode(str)).toThrowErrorMatchingInlineSnapshot(`"illegal char code: 256"`);
+		});
+	});
+});

apps/meteor/client/lib/e2ee/binary.ts

@@ -0,0 +1,32 @@
+import type { ICodec } from './codec';
+
+export const Binary: ICodec<string, ArrayBuffer> = {
+	encode(buffer: ArrayBuffer): string {
+		const uint8 = new Uint8Array(buffer);
+		const CHUNK_SIZE = 8192; // Process in chunks for performance
+		let result = '';
+		for (let i = 0; i < uint8.length; i += CHUNK_SIZE) {
+			const chunk = uint8.subarray(i, i + CHUNK_SIZE);
+			result += String.fromCharCode(...chunk);
+		}
+		return result;
+	},
+	decode(str: string): ArrayBuffer {
+		// Create a Uint8Array of the same length as the string.
+		// This will be a view on the new ArrayBuffer.
+		const buffer = new ArrayBuffer(str.length);
+		const uint8 = new Uint8Array(buffer);
+
+		// Iterate through the string, getting the character code for each
+		// character and setting it as the value for the corresponding byte.
+		for (let i = 0; i < str.length; i++) {
+			const charCode = str.charCodeAt(i);
+			if (charCode > 0xff) {
+				throw new RangeError(`illegal char code: ${charCode}`);
+			}
+			uint8[i] = charCode;
+		}
+
+		return buffer;
+	},
+};

apps/meteor/client/lib/e2ee/codec.ts

@@ -0,0 +1,4 @@
+export interface ICodec<TIn, TOut, TEnc = TIn> {
+	decode: (data: TIn) => TOut;
+	encode: (data: TOut) => TEnc;
+}