All: Fixes #6092: Shared resource was not encrypted with correct encryption key

.eslintignore

@@ -1150,6 +1150,9 @@ packages/lib/models/NoteTag.js.map
 packages/lib/models/Resource.d.ts
 packages/lib/models/Resource.js
 packages/lib/models/Resource.js.map
+packages/lib/models/Resource.test.d.ts
+packages/lib/models/Resource.test.js
+packages/lib/models/Resource.test.js.map
 packages/lib/models/ResourceLocalState.d.ts
 packages/lib/models/ResourceLocalState.js
 packages/lib/models/ResourceLocalState.js.map

.gitignore

@@ -1140,6 +1140,9 @@ packages/lib/models/NoteTag.js.map
 packages/lib/models/Resource.d.ts
 packages/lib/models/Resource.js
 packages/lib/models/Resource.js.map
+packages/lib/models/Resource.test.d.ts
+packages/lib/models/Resource.test.js
+packages/lib/models/Resource.test.js.map
 packages/lib/models/ResourceLocalState.d.ts
 packages/lib/models/ResourceLocalState.js
 packages/lib/models/ResourceLocalState.js.map

packages/app-mobile/root.tsx

@@ -550,6 +550,7 @@ async function initialize(dispatch: Function) {
 		// eslint-disable-next-line require-atomic-updates
 		BaseItem.encryptionService_ = EncryptionService.instance();
 		BaseItem.shareService_ = ShareService.instance();
+		Resource.shareService_ = ShareService.instance();
 		DecryptionWorker.instance().dispatch = dispatch;
 		DecryptionWorker.instance().setLogger(mainLogger);
 		DecryptionWorker.instance().setKvStore(KvStore.instance());

packages/lib/BaseApplication.ts

@@ -54,6 +54,7 @@ import { loadMasterKeysFromSettings, migrateMasterPassword } from './services/e2
 import SyncTargetNone from './SyncTargetNone';
 import { setRSA } from './services/e2ee/ppk';
 import RSA from './services/e2ee/RSA.node';
+import Resource from './models/Resource';
 
 const appLogger: LoggerWrapper = Logger.create('App');
 
@@ -855,6 +856,7 @@ export default class BaseApplication {
 
 		BaseItem.encryptionService_ = EncryptionService.instance();
 		BaseItem.shareService_ = ShareService.instance();
+		Resource.shareService_ = ShareService.instance();
 		DecryptionWorker.instance().setLogger(globalLogger);
 		DecryptionWorker.instance().setEncryptionService(EncryptionService.instance());
 		DecryptionWorker.instance().setKvStore(KvStore.instance());

packages/lib/models/Resource.test.ts

@@ -1,8 +1,8 @@
-const { supportDir, setupDatabaseAndSynchronizer, switchClient } = require('../testing/test-utils.js');
+import { supportDir, setupDatabaseAndSynchronizer, switchClient } from '../testing/test-utils';
-const Folder = require('../models/Folder').default;
+import Folder from '../models/Folder';
-const Note = require('../models/Note').default;
+import Note from '../models/Note';
-const Resource = require('../models/Resource').default;
+import Resource from '../models/Resource';
-const shim = require('../shim').default;
+import shim from '../shim';
 
 const testImagePath = `${supportDir}/photo.jpg`;
 
@@ -77,4 +77,24 @@ describe('models/Resource', function() {
 		expect(originalStat.size).toBe(newStat.size);
 	}));
 
+	// it('should encrypt a shared resource using the correct encryption key', (async () => {
+	// 	const folder1 = await Folder.save({ title: 'folder1' });
+	// 	const note1 = await Note.save({ title: 'ma note', parent_id: folder1.id });
+	// 	await shim.attachFileToNote(note1, testImagePath);
+
+	// 	Resource.shareService_ = {
+	// 		shareById: () => {
+	// 			return {
+	// 				master_key_id: '',
+	// 			};
+	// 		},
+	// 	} as any;
+
+	// 	try {
+
+	// 	} finally {
+	// 		Resource.shareService_ = null;
+	// 	}
+	// }));
+
 });

packages/lib/models/Resource.ts

@@ -14,6 +14,7 @@ const { FsDriverDummy } = require('../fs-driver-dummy.js');
 import JoplinError from '../JoplinError';
 import itemCanBeEncrypted from './utils/itemCanBeEncrypted';
 import { getEncryptionEnabled } from '../services/synchronizer/syncInfoUtils';
+import ShareService from '../services/share/ShareService';
 
 export default class Resource extends BaseItem {
 
@@ -24,6 +25,8 @@ export default class Resource extends BaseItem {
 	public static FETCH_STATUS_DONE = 2;
 	public static FETCH_STATUS_ERROR = 3;
 
+	public static shareService_: ShareService = null;
+
 	public static fsDriver_: any;
 
 	static tableName() {
@@ -39,6 +42,11 @@ export default class Resource extends BaseItem {
 		return this.encryptionService_;
 	}
 
+	protected static shareService() {
+		if (!this.shareService_) throw new Error('Resource.shareService_ is not set!!');
+		return this.shareService_;
+	}
+
 	static isSupportedImageMimeType(type: string) {
 		const imageMimeTypes = ['image/jpg', 'image/jpeg', 'image/png', 'image/gif', 'image/svg+xml', 'image/webp'];
 		return imageMimeTypes.indexOf(type.toLowerCase()) >= 0;
@@ -197,6 +205,8 @@ export default class Resource extends BaseItem {
 	public static async fullPathForSyncUpload(resource: ResourceEntity) {
 		const plainTextPath = this.fullPath(resource);
 
+		const share = resource.share_id ? await this.shareService().shareById(resource.share_id) : null;
+
 		if (!getEncryptionEnabled() || !itemCanBeEncrypted(resource as any)) {
 			// Normally not possible since itemsThatNeedSync should only return decrypted items
 			if (resource.encryption_blob_encrypted) throw new Error('Trying to access encrypted resource but encryption is currently disabled');
@@ -207,7 +217,9 @@ export default class Resource extends BaseItem {
 		if (resource.encryption_blob_encrypted) return { path: encryptedPath, resource: resource };
 
 		try {
-			await this.encryptionService().encryptFile(plainTextPath, encryptedPath);
+			await this.encryptionService().encryptFile(plainTextPath, encryptedPath, {
+				masterKeyId: share && share.master_key_id ? share.master_key_id : '',
+			});
 		} catch (error) {
 			if (error.code === 'ENOENT') throw new JoplinError(`File not found:${error.toString()}`, 'fileNotFound');
 			throw error;

packages/lib/services/share/ShareService.test.ts

@@ -1,16 +1,26 @@
 import Note from '../../models/Note';
-import { encryptionService, msleep, setupDatabaseAndSynchronizer, switchClient } from '../../testing/test-utils';
+import { encryptionService, loadEncryptionMasterKey, msleep, resourceService, setupDatabaseAndSynchronizer, supportDir, switchClient } from '../../testing/test-utils';
 import ShareService from './ShareService';
 import reducer, { defaultState } from '../../reducer';
 import { createStore } from 'redux';
-import { FolderEntity, NoteEntity } from '../database/types';
+import { NoteEntity } from '../database/types';
 import Folder from '../../models/Folder';
 import { setEncryptionEnabled, setPpk } from '../synchronizer/syncInfoUtils';
 import { generateKeyPair } from '../e2ee/ppk';
 import MasterKey from '../../models/MasterKey';
 import { MasterKeyEntity } from '../e2ee/types';
-import { updateMasterPassword } from '../e2ee/utils';
+import { loadMasterKeysFromSettings, setupAndEnableEncryption, updateMasterPassword } from '../e2ee/utils';
 import Logger, { LogLevel } from '../../Logger';
+import shim from '../../shim';
+import Resource from '../../models/Resource';
+import { readFile } from 'fs-extra';
+import BaseItem from '../../models/BaseItem';
+
+interface TestShareFolderServiceOptions {
+	master_key_id?: string;
+}
+
+const testImagePath = `${supportDir}/photo.jpg`;
 
 const testReducer = (state: any = defaultState, action: any) => {
 	return reducer(state, action);
@@ -70,11 +80,22 @@ describe('ShareService', function() {
 		}
 	});
 
-	function testShareFolderService(extraExecHandlers: Record<string, Function> = {}) {
+	function testShareFolderService(extraExecHandlers: Record<string, Function> = {}, options: TestShareFolderServiceOptions = {}) {
 		return mockService({
 			exec: async (method: string, path: string, query: Record<string, any>, body: any) => {
 				if (extraExecHandlers[`${method} ${path}`]) return extraExecHandlers[`${method} ${path}`](query, body);
 
+				if (method === 'GET' && path === 'api/shares') {
+					return {
+						items: [
+							{
+								id: 'share_1',
+								master_key_id: options.master_key_id,
+							},
+						],
+					};
+				}
+
 				if (method === 'POST' && path === 'api/shares') {
 					return {
 						id: 'share_1',
@@ -88,14 +109,20 @@ describe('ShareService', function() {
 
 	async function testShareFolder(service: ShareService) {
 		const folder = await Folder.save({});
-		const note = await Note.save({ parent_id: folder.id });
+		let note = await Note.save({ parent_id: folder.id });
+		note = await shim.attachFileToNote(note, testImagePath);
+		const resourceId = (await Note.linkedResourceIds(note.body))[0];
+		const resource = await Resource.load(resourceId);
+
+		await resourceService().indexNoteResources();
 
 		const share = await service.shareFolder(folder.id);
 		expect(share.id).toBe('share_1');
 		expect((await Folder.load(folder.id)).share_id).toBe('share_1');
 		expect((await Note.load(note.id)).share_id).toBe('share_1');
+		expect((await Resource.load(resource.id)).share_id).toBe('share_1');
 
-		return share;
+		return { share, folder, note, resource };
 	}
 
 	it('should share a folder', async () => {
@@ -103,18 +130,54 @@ describe('ShareService', function() {
 	});
 
 	it('should share a folder - E2EE', async () => {
-		setEncryptionEnabled(true);
+		const masterKey = await loadEncryptionMasterKey();
-		await updateMasterPassword('', '111111');
+		await setupAndEnableEncryption(encryptionService(), masterKey, '111111');
 		const ppk = await generateKeyPair(encryptionService(), '111111');
 		setPpk(ppk);
 
-		await testShareFolder(testShareFolderService());
+		let shareService = testShareFolderService();
 
-		expect((await MasterKey.all()).length).toBe(1);
+		expect(await MasterKey.count()).toBe(1);
 
-		const mk = (await MasterKey.all())[0];
+		let { folder, note, resource } = await testShareFolder(shareService);
-		const folder: FolderEntity = (await Folder.all())[0];
+
-		expect(folder.master_key_id).toBe(mk.id);
+		// The share service should automatically create a new encryption key
+		// specifically for that shared folder
+		expect(await MasterKey.count()).toBe(2);
+
+		folder = await Folder.load(folder.id);
+		note = await Note.load(note.id);
+		resource = await Resource.load(resource.id);
+
+		// The key that is not the master key is the folder key
+		const folderKey = (await MasterKey.all()).find(mk => mk.id !== masterKey.id);
+
+		// Double-check that it's going to encrypt the folder using the shared
+		// key (and not the user's own master key)
+		expect(folderKey.id).not.toBe(masterKey.id);
+		expect(folder.master_key_id).toBe(folderKey.id);
+
+		await loadMasterKeysFromSettings(encryptionService());
+
+		// Reload the service so that the mocked calls use the newly created key
+		shareService = testShareFolderService({}, { master_key_id: folderKey.id });
+
+		BaseItem.shareService_ = shareService;
+		Resource.shareService_ = shareService;
+
+		try {
+			const serializedNote = await Note.serializeForSync(note);
+			expect(serializedNote).toContain(folderKey.id);
+
+			// The resource should be encrypted using the above key (if it is,
+			// the key ID will be in the header).
+			const result = await Resource.fullPathForSyncUpload(resource);
+			const content = await readFile(result.path, 'utf8');
+			expect(content).toContain(folderKey.id);
+		} finally {
+			BaseItem.shareService_ = shareService;
+			Resource.shareService_ = null;
+		}
 	});
 
 	it('should add a recipient', async () => {
@@ -144,7 +207,7 @@ describe('ShareService', function() {
 			},
 		});
 
-		const share = await testShareFolder(service);
+		const { share } = await testShareFolder(service);
 
 		await service.addShareRecipient(share.id, share.master_key_id, 'toto@example.com');