Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Setting for enabling files encryption and fix whitelist media types stopping E2EE uploads #33003

Merged
merged 17 commits into from
Aug 23, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/stupid-fishes-relate.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@rocket.chat/core-typings': minor
'@rocket.chat/i18n': minor
'@rocket.chat/meteor': minor
---

Added a new setting to to enable/disable file encryption in an end to end encrypted room.
yash-rajpal marked this conversation as resolved.
Show resolved Hide resolved
15 changes: 15 additions & 0 deletions .changeset/violet-radios-begin.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
'@rocket.chat/core-typings': minor
'@rocket.chat/i18n': minor
'@rocket.chat/meteor': minor
---

Fixed a bug related to uploading end to end encrypted file.

E2EE files and uploads are uploaded as files of mime type `application/octet-stream` as we can't reveal the mime type of actual content since it is encrypted and has to be kept confidential.

The server resolves the mime type of encrypted file as `application/octet-stream` but it wasn't playing nicely with existing settings related to whitelisted and blacklisted media types.

E2EE files upload was getting blocked if `application/octet-stream` is not a whitelisted media type.

Now this PR solves this issue by always accepting E2EE uploads even if `application/octet-stream` is not whitelisted but it will block the upload if `application/octet-stream` is black listed.
10 changes: 8 additions & 2 deletions apps/meteor/app/file-upload/server/lib/FileUpload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import URL from 'url';
import { hashLoginToken } from '@rocket.chat/account-utils';
import { Apps, AppEvents } from '@rocket.chat/apps';
import { AppsEngineException } from '@rocket.chat/apps-engine/definition/exceptions';
import type { IUpload } from '@rocket.chat/core-typings';
import { isE2EEUpload, type IUpload } from '@rocket.chat/core-typings';
import { Users, Avatars, UserDataFiles, Uploads, Settings, Subscriptions, Messages, Rooms } from '@rocket.chat/models';
import type { NextFunction } from 'connect';
import filesize from 'filesize';
Expand Down Expand Up @@ -170,7 +170,13 @@ export const FileUpload = {
throw new Meteor.Error('error-file-too-large', reason);
}

if (!fileUploadIsValidContentType(file?.type)) {
if (!settings.get('E2E_Enable_Encrypt_Files') && isE2EEUpload(file)) {
const reason = i18n.t('Encrypted_file_not_allowed', { lng: language });
throw new Meteor.Error('error-invalid-file-type', reason);
}

// E2EE files are of type - application/octet-stream, application/octet-stream is whitelisted for E2EE files.
if (!fileUploadIsValidContentType(file?.type, isE2EEUpload(file) ? 'application/octet-stream' : undefined)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is breaking uploads on mobile

const reason = i18n.t('File_type_is_not_accepted', { lng: language });
throw new Meteor.Error('error-invalid-file-type', reason);
}
Expand Down
6 changes: 6 additions & 0 deletions apps/meteor/client/lib/chats/flows/uploadFiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type { IMessage, FileAttachmentProps, IE2EEMessage, IUpload } from '@rock
import { isRoomFederated } from '@rocket.chat/core-typings';

import { e2e } from '../../../../app/e2e/client';
import { settings } from '../../../../app/settings/client';
import { fileUploadIsValidContentType } from '../../../../app/utils/client';
import { getFileExtension } from '../../../../lib/utils/getFileExtension';
import FileUploadModal from '../../../views/room/modals/FileUploadModal';
Expand Down Expand Up @@ -83,6 +84,11 @@ export const uploadFiles = async (chat: ChatAPI, files: readonly File[], resetFi
return;
}

if (!settings.get('E2E_Enable_Encrypt_Files')) {
uploadFile(file, { description });
return;
}

const shouldConvertSentMessages = await e2eRoom.shouldConvertSentMessages({ msg });

if (!shouldConvertSentMessages) {
Expand Down
8 changes: 8 additions & 0 deletions apps/meteor/server/settings/e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,14 @@ export const createE2ESettings = () =>
enableQuery: { _id: 'E2E_Enable', value: true },
});

await this.add('E2E_Enable_Encrypt_Files', true, {
type: 'boolean',
i18nLabel: 'E2E_Enable_Encrypt_Files',
i18nDescription: 'E2E_Enable_Encrypt_Files_Description',
yash-rajpal marked this conversation as resolved.
Show resolved Hide resolved
public: true,
enableQuery: { _id: 'E2E_Enable', value: true },
});

await this.add('E2E_Enabled_Default_DirectRooms', false, {
type: 'boolean',
public: true,
Expand Down
141 changes: 141 additions & 0 deletions apps/meteor/tests/e2e/e2e-encryption.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,147 @@ test.describe.serial('e2e-encryption', () => {
await expect(poHomeChannel.content.nthMessage(0).locator('.rcx-icon--name-key')).toBeVisible();
});

test.describe('File Encryption', async () => {
test.afterAll(async ({ api }) => {
expect((await api.post('/settings/FileUpload_MediaTypeWhiteList', { value: '' })).status()).toBe(200);
expect((await api.post('/settings/FileUpload_MediaTypeBlackList', { value: 'image/svg+xml' })).status()).toBe(200);
});

test('File and description encryption', async ({ page }) => {
await test.step('create an encrypted channel', async () => {
const channelName = faker.string.uuid();

await poHomeChannel.sidenav.openNewByLabel('Channel');
await poHomeChannel.sidenav.inputChannelName.fill(channelName);
await poHomeChannel.sidenav.advancedSettingsAccordion.click();
await poHomeChannel.sidenav.checkboxEncryption.click();
await poHomeChannel.sidenav.btnCreate.click();

await expect(page).toHaveURL(`/group/${channelName}`);

await poHomeChannel.dismissToast();

await expect(poHomeChannel.content.encryptedRoomHeaderIcon).toBeVisible();
});

await test.step('send a file in channel', async () => {
await poHomeChannel.content.dragAndDropTxtFile();
await poHomeChannel.content.descriptionInput.fill('any_description');
await poHomeChannel.content.fileNameInput.fill('any_file1.txt');
await poHomeChannel.content.btnModalConfirm.click();

await expect(poHomeChannel.content.lastUserMessage.locator('.rcx-icon--name-key')).toBeVisible();
await expect(poHomeChannel.content.getFileDescription).toHaveText('any_description');
await expect(poHomeChannel.content.lastMessageFileName).toContainText('any_file1.txt');
});
});

test('File encryption with whitelisted and blacklisted media types', async ({ page, api }) => {
await test.step('create an encrypted channel', async () => {
yash-rajpal marked this conversation as resolved.
Show resolved Hide resolved
const channelName = faker.string.uuid();

await poHomeChannel.sidenav.openNewByLabel('Channel');
await poHomeChannel.sidenav.inputChannelName.fill(channelName);
await poHomeChannel.sidenav.advancedSettingsAccordion.click();
await poHomeChannel.sidenav.checkboxEncryption.click();
await poHomeChannel.sidenav.btnCreate.click();

await expect(page).toHaveURL(`/group/${channelName}`);

await poHomeChannel.dismissToast();

await expect(poHomeChannel.content.encryptedRoomHeaderIcon).toBeVisible();
});

await test.step('send a text file in channel', async () => {
await poHomeChannel.content.dragAndDropTxtFile();
await poHomeChannel.content.descriptionInput.fill('message 1');
await poHomeChannel.content.fileNameInput.fill('any_file1.txt');
await poHomeChannel.content.btnModalConfirm.click();

await expect(poHomeChannel.content.lastUserMessage.locator('.rcx-icon--name-key')).toBeVisible();
await expect(poHomeChannel.content.getFileDescription).toHaveText('message 1');
await expect(poHomeChannel.content.lastMessageFileName).toContainText('any_file1.txt');
});

await test.step('set whitelisted media type setting', async () => {
expect((await api.post('/settings/FileUpload_MediaTypeWhiteList', { value: 'text/plain' })).status()).toBe(200);
});

await test.step('send text file again with whitelist setting set', async () => {
await poHomeChannel.content.dragAndDropTxtFile();
await poHomeChannel.content.descriptionInput.fill('message 2');
await poHomeChannel.content.fileNameInput.fill('any_file1.txt');
await poHomeChannel.content.btnModalConfirm.click();

await expect(poHomeChannel.content.lastUserMessage.locator('.rcx-icon--name-key')).toBeVisible();
await expect(poHomeChannel.content.getFileDescription).toHaveText('message 2');
await expect(poHomeChannel.content.lastMessageFileName).toContainText('any_file1.txt');
});

await test.step('set blacklisted media type setting to not accept application/octet-stream media type', async () => {
expect((await api.post('/settings/FileUpload_MediaTypeBlackList', { value: 'application/octet-stream' })).status()).toBe(200);
});

await test.step('send text file again with blacklisted setting set, file upload should fail', async () => {
await poHomeChannel.content.dragAndDropTxtFile();
await poHomeChannel.content.descriptionInput.fill('message 3');
await poHomeChannel.content.fileNameInput.fill('any_file1.txt');
yash-rajpal marked this conversation as resolved.
Show resolved Hide resolved
await poHomeChannel.content.btnModalConfirm.click();

await expect(poHomeChannel.content.lastUserMessage.locator('.rcx-icon--name-key')).toBeVisible();
await expect(poHomeChannel.content.getFileDescription).toHaveText('message 2');
await expect(poHomeChannel.content.lastMessageFileName).toContainText('any_file1.txt');
});
});

test.describe('File encryption setting disabled', async () => {
test.beforeAll(async ({ api }) => {
expect((await api.post('/settings/E2E_Enable_Encrypt_Files', { value: false })).status()).toBe(200);
});

test.afterAll(async ({ api }) => {
expect((await api.post('/settings/E2E_Enable_Encrypt_Files', { value: true })).status()).toBe(200);
});

test('Upload file without encryption in e2ee room', async ({ page }) => {
await test.step('create an encrypted channel', async () => {
const channelName = faker.string.uuid();

await poHomeChannel.sidenav.openNewByLabel('Channel');
await poHomeChannel.sidenav.inputChannelName.fill(channelName);
await poHomeChannel.sidenav.advancedSettingsAccordion.click();
await poHomeChannel.sidenav.checkboxEncryption.click();
await poHomeChannel.sidenav.btnCreate.click();

await expect(page).toHaveURL(`/group/${channelName}`);

await poHomeChannel.dismissToast();

await expect(poHomeChannel.content.encryptedRoomHeaderIcon).toBeVisible();
});

await test.step('send a test encrypted message to check e2ee is working', async () => {
await poHomeChannel.content.sendMessage('This is an encrypted message.');

await expect(poHomeChannel.content.lastUserMessageBody).toHaveText('This is an encrypted message.');
await expect(poHomeChannel.content.lastUserMessage.locator('.rcx-icon--name-key')).toBeVisible();
});

await test.step('send a text file in channel, file should not be encryption', async () => {
await poHomeChannel.content.dragAndDropTxtFile();
yash-rajpal marked this conversation as resolved.
Show resolved Hide resolved
await poHomeChannel.content.descriptionInput.fill('any_description');
await poHomeChannel.content.fileNameInput.fill('any_file1.txt');
await poHomeChannel.content.btnModalConfirm.click();

await expect(poHomeChannel.content.lastUserMessage.locator('.rcx-icon--name-key')).not.toBeVisible();
await expect(poHomeChannel.content.getFileDescription).toHaveText('any_description');
await expect(poHomeChannel.content.lastMessageFileName).toContainText('any_file1.txt');
});
});
});
});

test('expect slash commands to be enabled in an e2ee room', async ({ page }) => {
const channelName = faker.string.uuid();

Expand Down
72 changes: 72 additions & 0 deletions apps/meteor/tests/end-to-end/api/rooms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,78 @@ describe('[Rooms]', () => {
expect(res.body.message.attachments[0]).to.have.property('description', 'some_file_description');
});
});

it('should correctly save encrypted file', async () => {
let fileId;

await request
.post(api(`rooms.media/${testChannel._id}`))
.set(credentials)
.attach('file', fs.createReadStream(path.join(__dirname, '../../mocks/files/diagram.drawio')), {
contentType: 'application/octet-stream',
})
.field({ content: JSON.stringify({ algorithm: 'rc.v1.aes-sha2', ciphertext: 'something' }) })
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('file');
expect(res.body.file).to.have.property('_id');
expect(res.body.file).to.have.property('url');

fileId = res.body.file._id;
});

await request
.post(api(`rooms.mediaConfirm/${testChannel._id}/${fileId}`))
.set(credentials)
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('message');
expect(res.body.message).to.have.property('files');
expect(res.body.message.files).to.be.an('array').of.length(1);
expect(res.body.message.files[0]).to.have.property('type', 'application/octet-stream');
expect(res.body.message.files[0]).to.have.property('name', 'diagram.drawio');
});
});

it('should fail encrypted file upload when files encryption is disabled', async () => {
await updateSetting('E2E_Enable_Encrypt_Files', false);
yash-rajpal marked this conversation as resolved.
Show resolved Hide resolved

await request
.post(api(`rooms.media/${testChannel._id}`))
.set(credentials)
.attach('file', fs.createReadStream(path.join(__dirname, '../../mocks/files/diagram.drawio')), {
contentType: 'application/octet-stream',
})
.field({ content: JSON.stringify({ algorithm: 'rc.v1.aes-sha2', ciphertext: 'something' }) })
.expect('Content-Type', 'application/json')
.expect(400)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('errorType', 'error-invalid-file-type');
});
});

it('should fail encrypted file upload on blacklisted application/octet-stream media type', async () => {
await updateSetting('FileUpload_MediaTypeBlackList', 'application/octet-stream');

await request
.post(api(`rooms.media/${testChannel._id}`))
.set(credentials)
.attach('file', fs.createReadStream(path.join(__dirname, '../../mocks/files/diagram.drawio')), {
contentType: 'application/octet-stream',
})
.field({ content: JSON.stringify({ algorithm: 'rc.v1.aes-sha2', ciphertext: 'something' }) })
.expect('Content-Type', 'application/json')
.expect(400)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('errorType', 'error-invalid-file-type');
});
});
});

describe('/rooms.favorite', () => {
Expand Down
13 changes: 13 additions & 0 deletions apps/meteor/tests/mocks/files/diagram.drawio
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<mxfile host="app.diagrams.net" modified="2024-05-21T16:10:09.295Z" agent="Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/124.0.0.0 Safari/537.36" etag="ZxZRCTHi-kxhlzk7b9_Z" version="24.4.4" type="device">
<diagram name="Página-1" id="9eBILa8281JaQ4yUkDbp">
<mxGraphModel dx="1434" dy="786" grid="1" gridSize="10" guides="1" tooltips="1" connect="1" arrows="1" fold="1" page="1" pageScale="1" pageWidth="827" pageHeight="1169" math="0" shadow="0">
<root>
<mxCell id="0" />
<mxCell id="1" parent="0" />
<mxCell id="dopCU4gkJe7Sfp6IDYO1-1" value="&lt;b&gt;&lt;font style=&quot;font-size: 30px;&quot;&gt;Rocket.Chat&lt;/font&gt;&lt;/b&gt;" style="text;html=1;align=center;verticalAlign=middle;resizable=0;points=[];autosize=1;strokeColor=none;fillColor=none;" vertex="1" parent="1">
<mxGeometry x="314" y="350" width="200" height="50" as="geometry" />
</mxCell>
</root>
</mxGraphModel>
</diagram>
</mxfile>
9 changes: 9 additions & 0 deletions packages/core-typings/src/IUpload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,12 @@ export interface IUpload {
}

export type IUploadWithUser = IUpload & { user?: Pick<IUser, '_id' | 'name' | 'username'> };

export type IE2EEUpload = IUpload & {
content: {
algorithm: string; // 'rc.v1.aes-sha2'
ciphertext: string; // Encrypted subset JSON of IUpload
};
};

export const isE2EEUpload = (upload: IUpload): upload is IE2EEUpload => Boolean(upload?.content?.ciphertext);
yash-rajpal marked this conversation as resolved.
Show resolved Hide resolved
3 changes: 3 additions & 0 deletions packages/i18n/src/locales/en.i18n.json
Original file line number Diff line number Diff line change
Expand Up @@ -1798,6 +1798,8 @@
"E2E_Enabled": "E2E Enabled",
"E2E_Enabled_Default_DirectRooms": "Enable encryption for Direct Rooms by default",
"E2E_Enabled_Default_PrivateRooms": "Enable encryption for Private Rooms by default",
"E2E_Enable_Encrypt_Files": "Encrypt files",
"E2E_Enable_Encrypt_Files_Description": "Encrypt files sent inside encrypted rooms. Check for possible conflicts in [file upload settings](admin/settings/FileUpload).",
"E2E_Encryption_Password_Change": "Change Encryption Password",
"E2E_Encryption_Password_Explanation": "You can now create encrypted private groups and direct messages. You may also change existing private groups or DMs to encrypted.<br/><br/>This is end-to-end encryption so the key to encode/decode your messages will not be saved on the server. For that reason you need to store your password somewhere safe. You will be required to enter it on other devices you wish to use e2e encryption on.",
"E2E_key_reset_email": "E2E Key Reset Notification",
Expand Down Expand Up @@ -1915,6 +1917,7 @@
"Email_verified": "Email verified",
"Enterprise_Only": "Enterprise only",
"Encrypted_field_hint": "Messages are end-to-end encrypted, search will not work and notifications may not show message content",
"Encrypted_file_not_allowed": "Encrypted file not allowed",
"Email_sent": "Email sent",
"Emoji": "Emoji",
"Emoji_picker": "Emoji picker",
Expand Down
Loading