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

Mobile,Desktop: Resolves #10856: Allow disabling master keys to improve startup performance #10943

45 changes: 31 additions & 14 deletions packages/app-mobile/components/screens/encryption-config.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ const { dialogs } = require('../../utils/dialogs.js');
import EncryptionService from '@joplin/lib/services/e2ee/EncryptionService';
import { _ } from '@joplin/lib/locale';
import time from '@joplin/lib/time';
import { decryptedStatText, enableEncryptionConfirmationMessages, onSavePasswordClick, useInputMasterPassword, useInputPasswords, usePasswordChecker, useStats } from '@joplin/lib/components/EncryptionConfigScreen/utils';
import { decryptedStatText, enableEncryptionConfirmationMessages, onSavePasswordClick, onToggleEnabledClick, useInputMasterPassword, useInputPasswords, usePasswordChecker, useStats } from '@joplin/lib/components/EncryptionConfigScreen/utils';
import { MasterKeyEntity } from '@joplin/lib/services/e2ee/types';
import { State } from '@joplin/lib/reducer';
import { SyncInfo } from '@joplin/lib/services/synchronizer/syncInfoUtils';
import { masterKeyEnabled, SyncInfo } from '@joplin/lib/services/synchronizer/syncInfoUtils';
import { getDefaultMasterKey, setupAndDisableEncryption, toggleAndSetupEncryption } from '@joplin/lib/services/e2ee/utils';
import { useMemo, useRef, useState } from 'react';

Expand Down Expand Up @@ -53,6 +53,11 @@ const EncryptionConfigScreen = (props: Props) => {
}, [theme]);

const styles = useMemo(() => {
const normalText = {
flex: 1,
fontSize: theme.fontSize,
color: theme.color,
};
const styles = {
titleText: {
flex: 1,
Expand All @@ -65,17 +70,18 @@ const EncryptionConfigScreen = (props: Props) => {
marginBottom: 5,
color: theme.color,
},
normalText: {
flex: 1,
fontSize: theme.fontSize,
color: theme.color,
},
normalText,
normalTextInput: {
margin: 10,
color: theme.color,
borderWidth: 1,
borderColor: theme.dividerColor,
},
fadedText: {
...normalText,
color: theme.colorFaded,
fontStyle: 'italic',
},
container: {
flex: 1,
padding: theme.margin,
Expand All @@ -98,10 +104,12 @@ const EncryptionConfigScreen = (props: Props) => {
inputStyle.borderBottomWidth = 1;
inputStyle.borderBottomColor = theme.dividerColor;

const renderPasswordInput = (masterKeyId: string) => {
const masterKeyId = mk.id;

const renderPasswordInput = () => {
if (masterPasswordKeys[masterKeyId] || !passwordChecks['master']) {
return (
<Text style={{ ...styles.normalText, color: theme.colorFaded, fontStyle: 'italic' }}>({_('Master password')})</Text>
<Text style={styles.fadedText}>({_('Master password')})</Text>
);
} else {
return (
Expand All @@ -114,14 +122,23 @@ const EncryptionConfigScreen = (props: Props) => {
}
};

return (
<View key={mk.id}>
<Text style={styles.titleText}>{_('Master Key %s', mk.id.substr(0, 6))}</Text>
<Text style={styles.normalText}>{_('Created: %s', time.formatMsToLocal(mk.created_time))}</Text>
const renderPasswordRow = () => {
if (!masterKeyEnabled(mk)) return null;

return (
<View style={{ flexDirection: 'row', alignItems: 'center' }}>
<Text style={{ flex: 0, fontSize: theme.fontSize, marginRight: 10, color: theme.color }}>{_('Password:')}</Text>
{renderPasswordInput(mk.id)}
{renderPasswordInput()}
</View>
);
};

return (
<View key={mk.id}>
<Text style={styles.titleText}>{_('Master Key %s', masterKeyId.substring(0, 6))}</Text>
<Text style={styles.normalText}>{_('Created: %s', time.formatMsToLocal(mk.created_time))}</Text>
{renderPasswordRow()}
<Button onPress={() => onToggleEnabledClick(mk)} title={masterKeyEnabled(mk) ? _('Disable') : _('Enable')}/>
</View>
);
};
Expand Down
8 changes: 7 additions & 1 deletion packages/app-mobile/root.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,13 @@ const generalMiddleware = (store: any) => (next: any) => async (action: any) =>
setTimeLocale(Setting.value('locale'));
}

if ((action.type === 'SETTING_UPDATE_ONE' && (action.key.indexOf('encryption.') === 0)) || (action.type === 'SETTING_UPDATE_ALL')) {
// Like the desktop and CLI apps, we run this whenever the sync target properties change.
// Previously, this only ran when encryption was enabled/disabled. However, after fetching
// a new key, this needs to run and so we run it when the sync target info changes.
if (
(action.type === 'SETTING_UPDATE_ONE' && ['encryption.passwordCache', 'encryption.masterPassword', 'syncInfoCache'].includes(action.key))
|| action.type === 'SETTING_UPDATE_ALL'
) {
await loadMasterKeysFromSettings(EncryptionService.instance());
void DecryptionWorker.instance().scheduleStart();
const loadedMasterKeyIds = EncryptionService.instance().loadedMasterKeyIds();
Expand Down
4 changes: 2 additions & 2 deletions packages/lib/models/BaseItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { LoadOptions, SaveOptions } from './utils/types';
import { State as ShareState } from '../services/share/reducer';
import { checkIfItemCanBeAddedToFolder, checkIfItemCanBeChanged, checkIfItemsCanBeChanged, needsShareReadOnlyChecks } from './utils/readOnly';
import { checkObjectHasProperties } from '@joplin/utils/object';
import EncryptionService from '../services/e2ee/EncryptionService';

const { sprintf } = require('sprintf-js');
const moment = require('moment');
Expand Down Expand Up @@ -47,8 +48,7 @@ export interface EncryptedItemsStats {

export default class BaseItem extends BaseModel {

// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
public static encryptionService_: any = null;
public static encryptionService_: EncryptionService = null;
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
public static revisionService_: any = null;
public static shareService_: ShareService = null;
Expand Down
12 changes: 12 additions & 0 deletions packages/lib/services/e2ee/EncryptionService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ export default class EncryptionService {
// changed easily since the chunk size is incorporated into the encrypted data.
private chunkSize_ = 5000;
private decryptedMasterKeys_: Record<string, DecryptedMasterKey> = {};
private disabledMasterKeyIds_: Set<string> = new Set();
public defaultEncryptionMethod_ = EncryptionMethod.SJCL1a; // public because used in tests
private defaultMasterKeyEncryptionMethod_ = EncryptionMethod.SJCL4;

Expand Down Expand Up @@ -122,6 +123,11 @@ export default class EncryptionService {
return id;
}

public disableMasterKey(masterKey: MasterKeyEntity) {
this.unloadMasterKey(masterKey);
this.disabledMasterKeyIds_.add(masterKey.id);
}

public isMasterKeyLoaded(masterKey: MasterKeyEntity) {
const d = this.decryptedMasterKeys_[masterKey.id];
if (!d) return false;
Expand All @@ -138,6 +144,7 @@ export default class EncryptionService {
updatedTime: model.updated_time,
};

this.disabledMasterKeyIds_.delete(model.id);
if (makeActive) this.setActiveMasterKeyId(model.id);
}

Expand All @@ -146,6 +153,11 @@ export default class EncryptionService {
}

public loadedMasterKey(id: string) {
if (this.disabledMasterKeyIds_.has(id)) {
const error = new JoplinError(`Master key is disabled: ${id}`, 'masterKeyDisabled');
throw error;
}

if (!this.decryptedMasterKeys_[id]) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
const error: any = new Error(`Master key is not loaded: ${id}`);
Expand Down
14 changes: 13 additions & 1 deletion packages/lib/services/e2ee/utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { afterAllCleanUp, setupDatabaseAndSynchronizer, switchClient, encryptionService, expectNotThrow, expectThrow, kvStore, msleep } from '../../testing/test-utils';
import MasterKey from '../../models/MasterKey';
import { activeMasterKeySanityCheck, migrateMasterPassword, resetMasterPassword, showMissingMasterKeyMessage, updateMasterPassword } from './utils';
import { activeMasterKeySanityCheck, loadMasterKeysFromSettings, migrateMasterPassword, resetMasterPassword, showMissingMasterKeyMessage, updateMasterPassword } from './utils';
import { localSyncInfo, masterKeyById, masterKeyEnabled, setActiveMasterKeyId, setMasterKeyEnabled, setPpk } from '../synchronizer/syncInfoUtils';
import Setting from '../../models/Setting';
import { generateKeyPair, ppkPasswordIsValid } from './ppk';
Expand Down Expand Up @@ -184,4 +184,16 @@ describe('e2ee/utils', () => {
expect(syncInfo.activeMasterKeyId).toBe(mk1.id);
});

test('should only load enabled master keys', async () => {
const mk1 = await MasterKey.save({ ...await encryptionService().generateMasterKey('test'), enabled: 0 });
const mk2 = await MasterKey.save(await encryptionService().generateMasterKey('test'));

Setting.setValue('encryption.masterPassword', 'test');
await loadMasterKeysFromSettings(encryptionService());

await expect(
() => encryptionService().encryptString('test', { masterKeyId: mk1.id }),
).rejects.toThrow(/disabled/i);
expect(await encryptionService().encryptString('test', { masterKeyId: mk2.id })).toBeTruthy();
});
});
16 changes: 10 additions & 6 deletions packages/lib/services/e2ee/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,13 +142,17 @@ export async function loadMasterKeysFromSettings(service: EncryptionService) {
const mk = masterKeys[i];
if (service.isMasterKeyLoaded(mk)) continue;

const password = await findMasterKeyPassword(service, mk);
if (!password) continue;
if (masterKeyEnabled(mk)) {
const password = await findMasterKeyPassword(service, mk);
if (!password) continue;

try {
await service.loadMasterKey(mk, password, activeMasterKeyId === mk.id);
} catch (error) {
logger.warn(`Cannot load master key ${mk.id}. Invalid password?`, error);
try {
await service.loadMasterKey(mk, password, activeMasterKeyId === mk.id);
} catch (error) {
logger.warn(`Cannot load master key ${mk.id}. Invalid password?`, error);
}
} else {
await service.disableMasterKey(mk);
}
}

Expand Down
Loading