{_('Your master password is needed to decrypt some of your data.')} {_('Please click on "%s" to proceed', buttonTitle)}
+
{_('Your password is needed to decrypt some of your data.')} {_('Please click on "%s" to proceed, or set the passwords in the "%s" list below.', buttonTitle, _('Encryption keys'))}
);
return (
diff --git a/packages/app-desktop/gui/EncryptionConfigScreen/style.scss b/packages/app-desktop/gui/EncryptionConfigScreen/style.scss
index b1a7536f0..75392d089 100644
--- a/packages/app-desktop/gui/EncryptionConfigScreen/style.scss
+++ b/packages/app-desktop/gui/EncryptionConfigScreen/style.scss
@@ -5,6 +5,7 @@
.manage-password-section > .status {
display: flex;
flex-direction: row;
+ align-items: center;
}
.manage-password-section > .needpassword {
diff --git a/packages/app-desktop/gui/MasterPasswordDialog/Dialog.tsx b/packages/app-desktop/gui/MasterPasswordDialog/Dialog.tsx
index ee12fe20a..7fd376c2b 100644
--- a/packages/app-desktop/gui/MasterPasswordDialog/Dialog.tsx
+++ b/packages/app-desktop/gui/MasterPasswordDialog/Dialog.tsx
@@ -34,6 +34,12 @@ export default function(props: Props) {
const [updatingPassword, setUpdatingPassword] = useState(false);
const [mode, setMode] = useState(Mode.Set);
+ const showCurrentPassword = useMemo(() => {
+ if ([MasterPasswordStatus.NotSet, MasterPasswordStatus.Invalid].includes(status)) return false;
+ if (mode === Mode.Reset) return false;
+ return true;
+ }, [status]);
+
const onClose = useCallback(() => {
props.dispatch({
type: 'DIALOG_CLOSE',
@@ -63,7 +69,7 @@ export default function(props: Props) {
setUpdatingPassword(true);
try {
if (mode === Mode.Set) {
- await updateMasterPassword(currentPassword, password1);
+ await updateMasterPassword(showCurrentPassword ? currentPassword : null, password1);
} else if (mode === Mode.Reset) {
await resetMasterPassword(EncryptionService.instance(), KvStore.instance(), ShareService.instance(), password1);
} else {
@@ -115,7 +121,7 @@ export default function(props: Props) {
}, [password1, password2, updatingPassword, needToRepeatPassword]);
useEffect(() => {
- setShowPasswordForm(status === MasterPasswordStatus.NotSet);
+ setShowPasswordForm([MasterPasswordStatus.NotSet, MasterPasswordStatus.Invalid].includes(status));
}, [status]);
useAsyncEffect(async (event: AsyncEffectEvent) => {
@@ -131,8 +137,7 @@ export default function(props: Props) {
function renderPasswordForm() {
const renderCurrentPassword = () => {
- if (status === MasterPasswordStatus.NotSet) return null;
- if (mode === Mode.Reset) return null;
+ if (!showCurrentPassword) return null;
// If the master password is in the keychain we preload it into the
// field and allow displaying it. That way if the user has forgotten
diff --git a/packages/app-desktop/main.scss b/packages/app-desktop/main.scss
index 9b3794345..3c199ab58 100644
--- a/packages/app-desktop/main.scss
+++ b/packages/app-desktop/main.scss
@@ -275,4 +275,5 @@ Component-specific classes
.master-password-dialog .fa-times {
color: var(--joplin-color-error);
+ margin-left: 5px;
}
diff --git a/packages/lib/Synchronizer.ts b/packages/lib/Synchronizer.ts
index f04d0e7b7..4d35d466e 100644
--- a/packages/lib/Synchronizer.ts
+++ b/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());
diff --git a/packages/lib/services/e2ee/EncryptionService.ts b/packages/lib/services/e2ee/EncryptionService.ts
index db9a52585..97cff6846 100644
--- a/packages/lib/services/e2ee/EncryptionService.ts
+++ b/packages/lib/services/e2ee/EncryptionService.ts
@@ -254,6 +254,7 @@ export default class EncryptionService {
model.created_time = now;
model.updated_time = now;
model.source_application = Setting.value('appId');
+ model.hasBeenUsed = false;
return model;
}
diff --git a/packages/lib/services/e2ee/types.ts b/packages/lib/services/e2ee/types.ts
index 564877acc..7899d1556 100644
--- a/packages/lib/services/e2ee/types.ts
+++ b/packages/lib/services/e2ee/types.ts
@@ -8,6 +8,7 @@ export interface MasterKeyEntity {
content?: string;
type_?: number;
enabled?: number;
+ hasBeenUsed?: boolean;
}
export type RSAKeyPair = any; // Depends on implementation
diff --git a/packages/lib/services/synchronizer/Synchronizer.e2ee.test.ts b/packages/lib/services/synchronizer/Synchronizer.e2ee.test.ts
index 003fac1f4..4456ede9d 100644
--- a/packages/lib/services/synchronizer/Synchronizer.e2ee.test.ts
+++ b/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);
diff --git a/packages/lib/services/synchronizer/syncInfoUtils.test.ts b/packages/lib/services/synchronizer/syncInfoUtils.test.ts
index bbdb299e0..a0f813333 100644
--- a/packages/lib/services/synchronizer/syncInfoUtils.test.ts
+++ b/packages/lib/services/synchronizer/syncInfoUtils.test.ts
@@ -1,6 +1,6 @@
-import { afterAllCleanUp, setupDatabaseAndSynchronizer, switchClient, encryptionService } from '../../testing/test-utils';
+import { afterAllCleanUp, setupDatabaseAndSynchronizer, switchClient, encryptionService, msleep } from '../../testing/test-utils';
import MasterKey from '../../models/MasterKey';
-import { masterKeyEnabled, setMasterKeyEnabled, SyncInfo, syncInfoEquals } from './syncInfoUtils';
+import { masterKeyEnabled, mergeSyncInfos, setMasterKeyEnabled, SyncInfo, syncInfoEquals } from './syncInfoUtils';
describe('syncInfoUtils', function() {
@@ -92,4 +92,33 @@ describe('syncInfoUtils', function() {
}
});
+ it('should merge sync target info and takes into account usage of master key - 1', async () => {
+ const syncInfo1 = new SyncInfo();
+ syncInfo1.masterKeys = [{
+ id: '1',
+ content: 'content1',
+ hasBeenUsed: true,
+ }];
+ syncInfo1.activeMasterKeyId = '1';
+
+ await msleep(1);
+
+ const syncInfo2 = new SyncInfo();
+ syncInfo2.masterKeys = [{
+ id: '2',
+ content: 'content2',
+ hasBeenUsed: false,
+ }];
+ syncInfo2.activeMasterKeyId = '2';
+
+ // If one master key has been used and the other not, it should select
+ // the one that's been used regardless of timestamps.
+ expect(mergeSyncInfos(syncInfo1, syncInfo2).activeMasterKeyId).toBe('1');
+
+ // If both master keys have been used it should rely on timestamp
+ // (latest modified is picked).
+ syncInfo2.masterKeys[0].hasBeenUsed = true;
+ expect(mergeSyncInfos(syncInfo1, syncInfo2).activeMasterKeyId).toBe('2');
+ });
+
});
diff --git a/packages/lib/services/synchronizer/syncInfoUtils.ts b/packages/lib/services/synchronizer/syncInfoUtils.ts
index 38a9ccc45..64779de4e 100644
--- a/packages/lib/services/synchronizer/syncInfoUtils.ts
+++ b/packages/lib/services/synchronizer/syncInfoUtils.ts
@@ -90,14 +90,62 @@ 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);
+ let doDefaultAction = false;
+
+ if (activeMasterKey1 && activeMasterKey2) {
+ if (activeMasterKey1.hasBeenUsed && !activeMasterKey2.hasBeenUsed) {
+ output.setWithTimestamp(s1, 'activeMasterKeyId');
+ } else if (!activeMasterKey1.hasBeenUsed && activeMasterKey2.hasBeenUsed) {
+ output.setWithTimestamp(s2, 'activeMasterKeyId');
+ } else {
+ doDefaultAction = true;
+ }
+ } else {
+ doDefaultAction = true;
+ }
+
+ if (doDefaultAction) {
+ output.setWithTimestamp(s1.keyTimestamp('activeMasterKeyId') > s2.keyTimestamp('activeMasterKeyId') ? s1 : s2, 'activeMasterKeyId');
+ }
+};
+
export function mergeSyncInfos(s1: SyncInfo, s2: SyncInfo): SyncInfo {
const output: SyncInfo = new SyncInfo();
output.setWithTimestamp(s1.keyTimestamp('e2ee') > s2.keyTimestamp('e2ee') ? s1 : s2, 'e2ee');
- output.setWithTimestamp(s1.keyTimestamp('activeMasterKeyId') > s2.keyTimestamp('activeMasterKeyId') ? s1 : s2, 'activeMasterKeyId');
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) {
@@ -154,6 +202,14 @@ export class SyncInfo {
this.activeMasterKeyId_ = 'activeMasterKeyId' in s ? s.activeMasterKeyId : { value: '', updatedTime: 0 };
this.masterKeys_ = 'masterKeys' in s ? s.masterKeys : [];
this.ppk_ = 'ppk' in s ? s.ppk : { value: null, updatedTime: 0 };
+
+ // Migration for master keys that didn't have "hasBeenUsed" property -
+ // in that case we assume they've been used at least once.
+ for (const mk of this.masterKeys_) {
+ if (!('hasBeenUsed' in mk) || mk.hasBeenUsed === undefined) {
+ mk.hasBeenUsed = true;
+ }
+ }
}
public setWithTimestamp(fromSyncInfo: SyncInfo, propName: string) {
@@ -275,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;