From 08d2655f1330869bcc0e7d570d7c0bc048af77fa Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Tue, 26 Dec 2017 11:38:53 +0100 Subject: [PATCH] All: Various improvements to E2EE --- CliClient/app/base-command.js | 4 + CliClient/app/command-attach.js | 1 + CliClient/app/command-done.js | 5 +- CliClient/app/command-e2ee.js | 74 +++++++++++++++++++ CliClient/app/command-edit.js | 2 + CliClient/app/command-encrypt-config.js | 47 ------------ CliClient/app/command-ren.js | 1 + CliClient/app/command-set.js | 2 + CliClient/app/command-todo.js | 2 + CliClient/app/command-undone.js | 2 +- CliClient/app/gui/FolderListWidget.js | 4 +- CliClient/app/gui/NoteListWidget.js | 2 +- CliClient/app/gui/NoteWidget.js | 7 +- README_spec.md | 3 + .../lib/services/DecryptionWorker.js | 25 +++++-- .../lib/services/EncryptionService.js | 7 ++ 16 files changed, 129 insertions(+), 59 deletions(-) create mode 100644 CliClient/app/command-e2ee.js delete mode 100644 CliClient/app/command-encrypt-config.js diff --git a/CliClient/app/base-command.js b/CliClient/app/base-command.js index eb2cc3461..748b034f9 100644 --- a/CliClient/app/base-command.js +++ b/CliClient/app/base-command.js @@ -12,6 +12,10 @@ class BaseCommand { throw new Error('Usage not defined'); } + encryptionCheck(item) { + if (item && item.encryption_applied) throw new Error(_('Cannot change encrypted item')); + } + description() { throw new Error('Description not defined'); } diff --git a/CliClient/app/command-attach.js b/CliClient/app/command-attach.js index 02367e1c7..0b22ef4cd 100644 --- a/CliClient/app/command-attach.js +++ b/CliClient/app/command-attach.js @@ -19,6 +19,7 @@ class Command extends BaseCommand { let title = args['note']; let note = await app().loadItem(BaseModel.TYPE_NOTE, title, { parent: app().currentFolder() }); + this.encryptionCheck(note); if (!note) throw new Error(_('Cannot find "%s".', title)); const localFilePath = args['file']; diff --git a/CliClient/app/command-done.js b/CliClient/app/command-done.js index 435662e76..b05f3f55b 100644 --- a/CliClient/app/command-done.js +++ b/CliClient/app/command-done.js @@ -16,8 +16,9 @@ class Command extends BaseCommand { return _('Marks a to-do as done.'); } - static async handleAction(args, isCompleted) { + static async handleAction(commandInstance, args, isCompleted) { const note = await app().loadItem(BaseModel.TYPE_NOTE, args.note); + commandInstance.encryptionCheck(note); if (!note) throw new Error(_('Cannot find "%s".', args.note)); if (!note.is_todo) throw new Error(_('Note is not a to-do: "%s"', args.note)); @@ -32,7 +33,7 @@ class Command extends BaseCommand { } async action(args) { - await Command.handleAction(args, true); + await Command.handleAction(this, args, true); } } diff --git a/CliClient/app/command-e2ee.js b/CliClient/app/command-e2ee.js new file mode 100644 index 000000000..1aa15b6b1 --- /dev/null +++ b/CliClient/app/command-e2ee.js @@ -0,0 +1,74 @@ +const { BaseCommand } = require('./base-command.js'); +const { _ } = require('lib/locale.js'); +const { cliUtils } = require('./cli-utils.js'); +const EncryptionService = require('lib/services/EncryptionService'); +const DecryptionWorker = require('lib/services/DecryptionWorker'); +const MasterKey = require('lib/models/MasterKey'); +const Setting = require('lib/models/Setting.js'); + +class Command extends BaseCommand { + + usage() { + return 'e2ee '; + } + + description() { + return _('Manages E2EE configuration. Commands are `enable`, `disable` and `decrypt`.'); + } + + options() { + return [ + // This is here mostly for testing - shouldn't be used + ['-p, --password ', 'Use this password as master password (For security reasons, it is not recommended to use this option).'], + ]; + } + + async action(args) { + // change-password + + const options = args.options; + + if (args.command === 'enable') { + const password = options.password ? options.password.toString() : await this.prompt(_('Enter master password:'), { type: 'string', secure: true }); + if (!password) { + this.stdout(_('Operation cancelled')); + return; + } + + await EncryptionService.instance().generateMasterKeyAndEnableEncryption(password); + return; + } + + if (args.command === 'disable') { + await EncryptionService.instance().disableEncryption(); + return; + } + + if (args.command === 'decrypt') { + while (true) { + try { + await DecryptionWorker.instance().start(); + break; + } catch (error) { + if (error.code === 'masterKeyNotLoaded') { + const masterKeyId = error.masterKeyId; + const password = await this.prompt(_('Enter master password:'), { type: 'string', secure: true }); + if (!password) { + this.stdout(_('Operation cancelled')); + return; + } + Setting.setObjectKey('encryption.passwordCache', masterKeyId, password); + await EncryptionService.instance().loadMasterKeysFromSettings(); + continue; + } + + throw error; + } + } + return; + } + } + +} + +module.exports = Command; \ No newline at end of file diff --git a/CliClient/app/command-edit.js b/CliClient/app/command-edit.js index 0971dbb4b..daac43d89 100644 --- a/CliClient/app/command-edit.js +++ b/CliClient/app/command-edit.js @@ -44,6 +44,8 @@ class Command extends BaseCommand { if (!app().currentFolder()) throw new Error(_('No active notebook.')); let note = await app().loadItem(BaseModel.TYPE_NOTE, title); + this.encryptionCheck(note); + if (!note) { const ok = await this.prompt(_('Note does not exist: "%s". Create it?', title)); if (!ok) return; diff --git a/CliClient/app/command-encrypt-config.js b/CliClient/app/command-encrypt-config.js deleted file mode 100644 index 48cd1989c..000000000 --- a/CliClient/app/command-encrypt-config.js +++ /dev/null @@ -1,47 +0,0 @@ -const { BaseCommand } = require('./base-command.js'); -const { _ } = require('lib/locale.js'); -const { cliUtils } = require('./cli-utils.js'); -const EncryptionService = require('lib/services/EncryptionService'); -const MasterKey = require('lib/models/MasterKey'); -const Setting = require('lib/models/Setting.js'); - -class Command extends BaseCommand { - - usage() { - return 'encrypt-config '; - } - - description() { - return _('Manages encryption configuration.'); - } - - options() { - return [ - // This is here mostly for testing - shouldn't be used - ['-p, --password ', 'Use this password as master password (For security reasons, it is not recommended to use this option).'], - ]; - } - - async action(args) { - // init - // change-password - - const options = args.options; - - if (args.command === 'init') { - const password = options.password ? options.password.toString() : await this.prompt(_('Enter master password:'), { type: 'string', secure: true }); - if (!password) { - this.stdout(_('Operation cancelled')); - return; - } - - const service = new EncryptionService(); - let masterKey = await service.generateMasterKey(password); - masterKey = await MasterKey.save(masterKey); - await service.enableEncryption(masterKey, password); - } - } - -} - -module.exports = Command; \ No newline at end of file diff --git a/CliClient/app/command-ren.js b/CliClient/app/command-ren.js index f8b4a5dd2..45b60671c 100644 --- a/CliClient/app/command-ren.js +++ b/CliClient/app/command-ren.js @@ -20,6 +20,7 @@ class Command extends BaseCommand { const name = args['name']; const item = await app().loadItem('folderOrNote', pattern); + this.encryptionCheck(item); if (!item) throw new Error(_('Cannot find "%s".', pattern)); const newItem = { diff --git a/CliClient/app/command-set.js b/CliClient/app/command-set.js index d6762353b..3a01ae5b4 100644 --- a/CliClient/app/command-set.js +++ b/CliClient/app/command-set.js @@ -35,6 +35,8 @@ class Command extends BaseCommand { if (!notes.length) throw new Error(_('Cannot find "%s".', title)); for (let i = 0; i < notes.length; i++) { + this.encryptionCheck(notes[i]); + let newNote = { id: notes[i].id, type_: notes[i].type_, diff --git a/CliClient/app/command-todo.js b/CliClient/app/command-todo.js index 601b92d2d..1f83e2cda 100644 --- a/CliClient/app/command-todo.js +++ b/CliClient/app/command-todo.js @@ -25,6 +25,8 @@ class Command extends BaseCommand { for (let i = 0; i < notes.length; i++) { const note = notes[i]; + this.encryptionCheck(note); + let toSave = { id: note.id, }; diff --git a/CliClient/app/command-undone.js b/CliClient/app/command-undone.js index 51d2fe77e..3373c0ee3 100644 --- a/CliClient/app/command-undone.js +++ b/CliClient/app/command-undone.js @@ -19,7 +19,7 @@ class Command extends BaseCommand { } async action(args) { - await CommandDone.handleAction(args, false); + await CommandDone.handleAction(this, args, false); } } diff --git a/CliClient/app/gui/FolderListWidget.js b/CliClient/app/gui/FolderListWidget.js index b030eee2c..4532d1a7c 100644 --- a/CliClient/app/gui/FolderListWidget.js +++ b/CliClient/app/gui/FolderListWidget.js @@ -24,9 +24,9 @@ class FolderListWidget extends ListWidget { if (item === '-') { output.push('-'.repeat(this.innerWidth)); } else if (item.type_ === Folder.modelType()) { - output.push(item.title); + output.push(Folder.displayTitle(item)); } else if (item.type_ === Tag.modelType()) { - output.push('[' + item.title + ']'); + output.push('[' + Folder.displayTitle(item) + ']'); } else if (item.type_ === BaseModel.TYPE_SEARCH) { output.push(_('Search:')); output.push(item.title); diff --git a/CliClient/app/gui/NoteListWidget.js b/CliClient/app/gui/NoteListWidget.js index aaa46be69..30d44df44 100644 --- a/CliClient/app/gui/NoteListWidget.js +++ b/CliClient/app/gui/NoteListWidget.js @@ -10,7 +10,7 @@ class NoteListWidget extends ListWidget { this.updateIndexFromSelectedNoteId_ = false; this.itemRenderer = (note) => { - let label = note.title; // + ' ' + note.id; + let label = Note.displayTitle(note); // + ' ' + note.id; if (note.is_todo) { label = '[' + (note.todo_completed ? 'X' : ' ') + '] ' + label; } diff --git a/CliClient/app/gui/NoteWidget.js b/CliClient/app/gui/NoteWidget.js index 890b0318e..93e9ef7db 100644 --- a/CliClient/app/gui/NoteWidget.js +++ b/CliClient/app/gui/NoteWidget.js @@ -44,7 +44,12 @@ class NoteWidget extends TextWidget { } else if (this.noteId_) { this.doAsync('loadNote', async () => { this.note_ = await Note.load(this.noteId_); - this.text = this.note_ ? this.note_.title + "\n\n" + this.note_.body : ''; + if (this.note_.encryption_applied) { + this.text = _('One or more items are currently encrypted and you may need to supply a master password. To do so please type `e2ee decrypt`. If you have already supplied the password, the encrypted items are being decrypted in the background and will be available soon.'); + } else { + this.text = this.note_ ? this.note_.title + "\n\n" + this.note_.body : ''; + } + if (this.lastLoadedNoteId_ !== this.noteId_) this.scrollTop = 0; this.lastLoadedNoteId_ = this.noteId_; }); diff --git a/README_spec.md b/README_spec.md index 54d64d131..d1d8637a1 100644 --- a/README_spec.md +++ b/README_spec.md @@ -57,6 +57,9 @@ The apps handle displaying both decrypted and encrypted items, so that user is a Enabling/disabling E2EE while two clients are in sync might have an unintuitive behaviour (although that behaviour might be correct), so below some scenarios are explained: - If client 1 enables E2EE, all items will be synced to target and will appear encrypted on target. Although all items have been re-uploaded to the target, their timestamps did *not* change (because the item data itself has not changed, only its representation). Because of this, client 2 will not be re-download the items - it does not need to do so anyway since it has already the item data. + - When a client sync and download a master key for the first time, encryption will be automatically enabled (user will need to supply the master key password). In that case, all items that are not encrypted will be re-synced. Uploading only non-encrypted items is an optimisation since if an item is already encrypted locally it means it's encrytped on target too. + - If both clients are in sync with E2EE enabled: if client 1 disable E2EE, it's going to re-upload all the items unencrypted. Client 2 again will not re-download the items for the same reason as above (data did not change, only representation). Note that user *must* manually disable E2EE on all clients otherwise some will continue to upload encrypted items. Since synchronisation is stateless, clients do not know whether other clients use E2EE or not so this step has to be manual. + - Although messy, Joplin supports having some clients send encrypted and others unencrypted ones. The situation gets resolved once all the clients have the same E2EE settings. \ No newline at end of file diff --git a/ReactNativeClient/lib/services/DecryptionWorker.js b/ReactNativeClient/lib/services/DecryptionWorker.js index 95b0ab013..cb9cb39af 100644 --- a/ReactNativeClient/lib/services/DecryptionWorker.js +++ b/ReactNativeClient/lib/services/DecryptionWorker.js @@ -40,12 +40,22 @@ class DecryptionWorker { this.scheduleId_ = setTimeout(() => { this.scheduleId_ = null; - this.start(); + this.start({ + materKeyNotLoadedHandler: 'dispatch', + }); }, 1000); } - async start() { - if (this.state_ !== 'idle') return; + async start(options = null) { + if (options === null) options = {}; + if (!('materKeyNotLoadedHandler' in options)) options.materKeyNotLoadedHandler = 'throw'; + + if (this.state_ !== 'idle') { + this.logger().info('DecryptionWorker: cannot start because state is "' + this.state_ + '"'); + return; + } + + this.logger().info('DecryptionWorker: starting decryption...'); this.state_ = 'started'; @@ -58,11 +68,12 @@ class DecryptionWorker { for (let i = 0; i < items.length; i++) { const item = items[i]; + this.logger().debug('DecryptionWorker: decrypting: ' + item.id); const ItemClass = BaseItem.itemClass(item); try { await ItemClass.decrypt(item); } catch (error) { - if (error.code === 'masterKeyNotLoaded') { + if (error.code === 'masterKeyNotLoaded' && options.materKeyNotLoadedHandler === 'dispatch') { excludedIds.push(item.id); this.dispatch({ type: 'MASTERKEY_ADD_NOT_LOADED', @@ -77,9 +88,13 @@ class DecryptionWorker { if (!result.hasMore) break; } } catch (error) { - this.logger().error('DecryptionWorker::start:', error); + this.logger().error('DecryptionWorker:', error); + this.state_ = 'idle'; + throw error; } + this.logger().info('DecryptionWorker: completed decryption.'); + this.state_ = 'idle'; } diff --git a/ReactNativeClient/lib/services/EncryptionService.js b/ReactNativeClient/lib/services/EncryptionService.js index 3c13101d8..4ab4d2c8c 100644 --- a/ReactNativeClient/lib/services/EncryptionService.js +++ b/ReactNativeClient/lib/services/EncryptionService.js @@ -48,6 +48,7 @@ class EncryptionService { masterKey = await MasterKey.save(masterKey); await this.enableEncryption(masterKey, password); await this.loadMasterKeysFromSettings(); + return masterKey; } async enableEncryption(masterKey, password = null) { @@ -220,6 +221,9 @@ class EncryptionService { } async encrypt(method, key, plainText) { + if (!method) throw new Error('Encryption method is required'); + if (!key) throw new Error('Encryption key is required'); + const sjcl = shim.sjclModule; if (method === EncryptionService.METHOD_SJCL) { @@ -261,6 +265,9 @@ class EncryptionService { } async decrypt(method, key, cipherText) { + if (!method) throw new Error('Encryption method is required'); + if (!key) throw new Error('Encryption key is required'); + const sjcl = shim.sjclModule; if (method === EncryptionService.METHOD_SJCL || method === EncryptionService.METHOD_SJCL_2) {