sync

packages/lib/Synchronizer.ts

@@ -22,7 +22,7 @@ import TaskQueue from './TaskQueue';
 import ItemUploader from './services/synchronizer/ItemUploader';
 import { FileApi, RemoteItem } from './file-api';
 import JoplinDatabase from './JoplinDatabase';
-import { fetchSyncInfo, getActiveMasterKey, localSyncInfo, mergeSyncInfos, saveLocalSyncInfo, SyncInfo, syncInfoEquals, uploadSyncInfo } from './services/synchronizer/syncInfoUtils';
+import { fetchSyncInfo, getActiveMasterKey, localSyncInfo, mergeSyncInfos, saveLocalSyncInfo, setMasterKeyHasBeenUsed, SyncInfo, syncInfoEquals, uploadSyncInfo } from './services/synchronizer/syncInfoUtils';
 import { getMasterPassword, setupAndDisableEncryption, setupAndEnableEncryption } from './services/e2ee/utils';
 import { generateKeyPair } from './services/e2ee/ppk';
 import syncDebugLog from './services/synchronizer/syncDebugLog';
@@ -439,10 +439,13 @@ export default class Synchronizer {
 				let remoteInfo = await fetchSyncInfo(this.api());
 				logger.info('Sync target remote info:', remoteInfo);
 
+				let syncTargetIsNew = false;
+
 				if (!remoteInfo.version) {
 					logger.info('Sync target is new - setting it up...');
 					await this.migrationHandler().upgrade(Setting.value('syncVersion'));
 					remoteInfo = await fetchSyncInfo(this.api());
+					syncTargetIsNew = true;
 				}
 
 				logger.info('Sync target is already setup - checking it...');
@@ -455,11 +458,16 @@ export default class Synchronizer {
 
 				localInfo = await this.setPpkIfNotExist(localInfo, remoteInfo);
 
+				if (syncTargetIsNew && localInfo.activeMasterKeyId) {
+					localInfo = setMasterKeyHasBeenUsed(localInfo, localInfo.activeMasterKeyId);
+				}
+
 				// console.info('LOCAL', localInfo);
 				// console.info('REMOTE', remoteInfo);
 
 				if (!syncInfoEquals(localInfo, remoteInfo)) {
-					const newInfo = mergeSyncInfos(localInfo, remoteInfo);
+					let newInfo = mergeSyncInfos(localInfo, remoteInfo);
+					if (newInfo.activeMasterKeyId) newInfo = setMasterKeyHasBeenUsed(newInfo, newInfo.activeMasterKeyId);
 					const previousE2EE = localInfo.e2ee;
 					logger.info('Sync target info differs between local and remote - merging infos: ', newInfo.toObject());
 

packages/lib/services/synchronizer/Synchronizer.e2ee.test.ts

@@ -9,7 +9,7 @@ import ResourceFetcher from '../../services/ResourceFetcher';
 import MasterKey from '../../models/MasterKey';
 import BaseItem from '../../models/BaseItem';
 import Synchronizer from '../../Synchronizer';
-import { getEncryptionEnabled, setEncryptionEnabled } from '../synchronizer/syncInfoUtils';
+import { fetchSyncInfo, getEncryptionEnabled, localSyncInfo, setEncryptionEnabled } from '../synchronizer/syncInfoUtils';
 import { loadMasterKeysFromSettings, setupAndDisableEncryption, setupAndEnableEncryption } from '../e2ee/utils';
 
 let insideBeforeEach = false;
@@ -73,6 +73,32 @@ describe('Synchronizer.e2ee', function() {
 		expect(!folder1_2.encryption_cipher_text).toBe(true);
 	}));
 
+	it('should mark the key has having been used when synchronising the first time', (async () => {
+		setEncryptionEnabled(true);
+		await loadEncryptionMasterKey();
+		await Folder.save({ title: 'folder1' });
+		await synchronizerStart();
+
+		const localInfo = localSyncInfo();
+		const remoteInfo = await fetchSyncInfo(fileApi());
+		expect(localInfo.masterKeys[0].hasBeenUsed).toBe(true);
+		expect(remoteInfo.masterKeys[0].hasBeenUsed).toBe(true);
+	}));
+
+	it('should mark the key has having been used when synchronising after enabling encryption', (async () => {
+		await Folder.save({ title: 'folder1' });
+		await synchronizerStart();
+
+		setEncryptionEnabled(true);
+		await loadEncryptionMasterKey();
+		await synchronizerStart();
+
+		const localInfo = localSyncInfo();
+		const remoteInfo = await fetchSyncInfo(fileApi());
+		expect(localInfo.masterKeys[0].hasBeenUsed).toBe(true);
+		expect(remoteInfo.masterKeys[0].hasBeenUsed).toBe(true);
+	}));
+
 	it('should enable encryption automatically when downloading new master key (and none was previously available)',(async () => {
 		// Enable encryption on client 1 and sync an item
 		setEncryptionEnabled(true);

packages/lib/services/synchronizer/syncInfoUtils.ts

@@ -90,6 +90,31 @@ export function localSyncInfoFromState(state: State): SyncInfo {
 	return new SyncInfo(state.settings['syncInfoCache']);
 }
 
+// When deciding which master key should be active we should take into account
+// whether it's been used or not. If it's been used before it should most likely
+// remain the active one, regardless of timestamps. This is because the extra
+// key was most likely created by mistake by the user, in particular in this
+// kind of scenario:
+//
+// - Client 1 setup sync with sync target
+// - Client 1 enable encryption
+// - Client 1 sync
+//
+// Then user 2 does the same:
+//
+// - Client 2 setup sync with sync target
+// - Client 2 enable encryption
+// - Client 2 sync
+//
+// The problem is that enabling encryption was not needed since it was already
+// done (and recorded in info.json) on the sync target. As a result an extra key
+// has been created and it has been set as the active one, but we shouldn't use
+// it. Instead the key created by client 1 should be used and made active again.
+//
+// And we can do this using the "hasBeenUsed" field which tells us which keys
+// has already been used to encrypt data. In this case, at the moment we compare
+// local and remote sync info (before synchronising the data), key1.hasBeenUsed
+// is true, but key2.hasBeenUsed is false.
 const mergeActiveMasterKeys = (s1: SyncInfo, s2: SyncInfo, output: SyncInfo) => {
 	const activeMasterKey1 = getActiveMasterKey(s1);
 	const activeMasterKey2 = getActiveMasterKey(s2);
@@ -119,6 +144,8 @@ export function mergeSyncInfos(s1: SyncInfo, s2: SyncInfo): SyncInfo {
 	output.setWithTimestamp(s1.keyTimestamp('ppk') > s2.keyTimestamp('ppk') ? s1 : s2, 'ppk');
 	output.version = s1.version > s2.version ? s1.version : s2.version;
 
+	mergeActiveMasterKeys(s1, s2, output);
+
 	output.masterKeys = s1.masterKeys.slice();
 
 	for (const mk of s2.masterKeys) {
@@ -131,8 +158,6 @@ export function mergeSyncInfos(s1: SyncInfo, s2: SyncInfo): SyncInfo {
 		}
 	}
 
-	mergeActiveMasterKeys(s1, s2, output);
-
 	return output;
 }
 
@@ -306,6 +331,21 @@ export function setMasterKeyEnabled(mkId: string, enabled: boolean = true) {
 	saveLocalSyncInfo(s);
 }
 
+export const setMasterKeyHasBeenUsed = (s: SyncInfo, mkId: string) => {
+	const idx = s.masterKeys.findIndex(mk => mk.id === mkId);
+	if (idx < 0) throw new Error(`No such master key: ${mkId}`);
+
+	s.masterKeys[idx] = {
+		...s.masterKeys[idx],
+		hasBeenUsed: true,
+		updated_time: Date.now(),
+	};
+
+	saveLocalSyncInfo(s);
+
+	return s;
+};
+
 export function masterKeyEnabled(mk: MasterKeyEntity): boolean {
 	if ('enabled' in mk) return !!mk.enabled;
 	return true;