diff --git a/CliClient/app/command-encrypt-config.js b/CliClient/app/command-encrypt-config.js index 3dcb3e19e..75000dd34 100644 --- a/CliClient/app/command-encrypt-config.js +++ b/CliClient/app/command-encrypt-config.js @@ -29,7 +29,7 @@ class Command extends BaseCommand { const service = new EncryptionService(); let masterKey = await service.generateMasterKey(password); masterKey = await MasterKey.save(masterKey); - await service.initializeEncryption(masterKey, password); + await service.enableEncryption(masterKey, password); } } diff --git a/CliClient/tests/synchronizer.js b/CliClient/tests/synchronizer.js index 072302a0d..2e6b85db3 100644 --- a/CliClient/tests/synchronizer.js +++ b/CliClient/tests/synchronizer.js @@ -1,7 +1,7 @@ require('app-module-path').addPath(__dirname); const { time } = require('lib/time-utils.js'); -const { setupDatabase, setupDatabaseAndSynchronizer, db, synchronizer, fileApi, sleep, clearDatabase, switchClient, syncTargetId, encryptionService, loadEncryptionMasterKey, fileContentEqual, decryptionWorker } = require('test-utils.js'); +const { setupDatabase, setupDatabaseAndSynchronizer, db, synchronizer, fileApi, sleep, clearDatabase, switchClient, syncTargetId, encryptionService, loadEncryptionMasterKey, fileContentEqual, decryptionWorker, checkThrowAsync } = require('test-utils.js'); const { shim } = require('lib/shim.js'); const Folder = require('lib/models/Folder.js'); const Note = require('lib/models/Note.js'); @@ -26,6 +26,24 @@ async function allItems() { return folders.concat(notes); } +async function allSyncTargetItemsEncrypted() { + const list = await fileApi().list(); + const files = list.items; + + let output = false; + for (let i = 0; i < files.length; i++) { + const file = files[i]; + const remoteContentString = await fileApi().get(file.path); + const remoteContent = await BaseItem.unserialize(remoteContentString); + const ItemClass = BaseItem.itemClass(remoteContent); + + if (!ItemClass.encryptionSupported()) continue; + if (!!remoteContent.encryption_applied) output = true; + } + + return output; +} + async function localItemsSameAsRemote(locals, expect) { try { let files = await fileApi().list(); @@ -56,13 +74,19 @@ async function localItemsSameAsRemote(locals, expect) { } } +let insideBeforeEach = false; + describe('Synchronizer', function() { - beforeEach( async (done) => { + beforeEach(async (done) => { + insideBeforeEach = true; + await setupDatabaseAndSynchronizer(1); await setupDatabaseAndSynchronizer(2); await switchClient(1); done(); + + insideBeforeEach = false; }); it('should create remote items', async (done) => { @@ -605,7 +629,10 @@ describe('Synchronizer', function() { await switchClient(2); await synchronizer().start(); - if (withEncryption) await loadEncryptionMasterKey(null, true); + if (withEncryption) { + await loadEncryptionMasterKey(null, true); + await decryptionWorker().start(); + } let note2 = await Note.load(note1.id); note2.todo_completed = time.unixMs()-1; await Note.save(note2); @@ -654,6 +681,12 @@ describe('Synchronizer', function() { done(); }); + it('should always handle conflict if local or remote are encrypted', async (done) => { + await ignorableNoteConflictTest(true); + + done(); + }); + it('items should be downloaded again when user cancels in the middle of delta operation', async (done) => { let folder1 = await Folder.save({ title: "folder1" }); let note1 = await Note.save({ title: "un", is_todo: 1, parent_id: folder1.id }); @@ -746,12 +779,6 @@ describe('Synchronizer', function() { done(); }); - it('should always handle conflict if local or remote are encrypted', async (done) => { - await ignorableNoteConflictTest(true); - - done(); - }); - it('should enable encryption automatically when downloading new master key (and none was previously available)', async (done) => { // Enable encryption on client 1 and sync an item Setting.setValue('encryption.enabled', true); @@ -776,7 +803,7 @@ describe('Synchronizer', function() { // If we sync now, nothing should be sent to target since we don't have a password. // Technically it's incorrect to set the property of an encrypted variable but it allows confirming // that encryption doesn't work if user hasn't supplied a password. - let folder1_2 = await Folder.save({ id: folder1.id, title: "change test" }); + await BaseItem.forceSync(folder1.id); await synchronizer().start(); await switchClient(1); @@ -814,7 +841,6 @@ describe('Synchronizer', function() { it('should encrypt existing notes too when enabling E2EE', async (done) => { // First create a folder, without encryption enabled, and sync it - const service = encryptionService(); let folder1 = await Folder.save({ title: "folder1" }); await synchronizer().start(); let files = await fileApi().list() @@ -822,10 +848,10 @@ describe('Synchronizer', function() { expect(content.indexOf('folder1') >= 0).toBe(true) // Then enable encryption and sync again - let masterKey = await service.generateMasterKey('123456'); + let masterKey = await encryptionService().generateMasterKey('123456'); masterKey = await MasterKey.save(masterKey); - await service.initializeEncryption(masterKey, '123456'); - await service.loadMasterKeysFromSettings(); + await encryptionService().enableEncryption(masterKey, '123456'); + await encryptionService().loadMasterKeysFromSettings(); await synchronizer().start(); // Even though the folder has not been changed it should have been synced again so that @@ -849,11 +875,14 @@ describe('Synchronizer', function() { let resource1 = (await Resource.all())[0]; let resourcePath1 = Resource.fullPath(resource1); await synchronizer().start(); + expect((await fileApi().list()).items.length).toBe(3); await switchClient(2); await synchronizer().start(); - let resource1_2 = (await Resource.all())[0]; + let allResources = await Resource.all(); + expect(allResources.length).toBe(1); + let resource1_2 = allResources[0]; let resourcePath1_2 = Resource.fullPath(resource1_2); expect(resource1_2.id).toBe(resource1.id); @@ -888,4 +917,62 @@ describe('Synchronizer', function() { done(); }); + it('should upload decrypted items to sync target after encryption disabled', async (done) => { + Setting.setValue('encryption.enabled', true); + const masterKey = await loadEncryptionMasterKey(); + + let folder1 = await Folder.save({ title: "folder1" }); + await synchronizer().start(); + + let allEncrypted = await allSyncTargetItemsEncrypted(); + expect(allEncrypted).toBe(true); + + await encryptionService().disableEncryption(); + + await synchronizer().start(); + allEncrypted = await allSyncTargetItemsEncrypted(); + expect(allEncrypted).toBe(false); + + done(); + }); + + it('should not upload any item if encryption was enabled, and items have not been decrypted, and then encryption disabled', async (done) => { + // For some reason I can't explain, this test is sometimes executed before beforeEach is finished + // which means it's going to fail in unexpected way. So the loop below wait for beforeEach to be done. + while (insideBeforeEach) await time.msleep(100); + + Setting.setValue('encryption.enabled', true); + const masterKey = await loadEncryptionMasterKey(); + + let folder1 = await Folder.save({ title: "folder1" }); + await synchronizer().start(); + + await switchClient(2); + + await synchronizer().start(); + expect(Setting.value('encryption.enabled')).toBe(true); + + // If we try to disable encryption now, it should throw an error because some items are + // currently encrypted. They must be decrypted first so that they can be sent as + // plain text to the sync target. + let hasThrown = await checkThrowAsync(async () => await encryptionService().disableEncryption()); + expect(hasThrown).toBe(true); + + // Now supply the password, and decrypt the items + Setting.setObjectKey('encryption.passwordCache', masterKey.id, '123456'); + await encryptionService().loadMasterKeysFromSettings(); + await decryptionWorker().start(); + + // Try to disable encryption again + hasThrown = await checkThrowAsync(async () => await encryptionService().disableEncryption()); + expect(hasThrown).toBe(false); + + // If we sync now the target should receive the decrypted items + await synchronizer().start(); + allEncrypted = await allSyncTargetItemsEncrypted(); + expect(allEncrypted).toBe(false); + + done(); + }); + }); \ No newline at end of file diff --git a/CliClient/tests/test-utils.js b/CliClient/tests/test-utils.js index 50dc05fff..523f9f726 100644 --- a/CliClient/tests/test-utils.js +++ b/CliClient/tests/test-utils.js @@ -98,7 +98,7 @@ async function switchClient(id) { return Setting.load(); } -function clearDatabase(id = null) { +async function clearDatabase(id = null) { if (id === null) id = currentClient_; let queries = [ @@ -114,31 +114,53 @@ function clearDatabase(id = null) { 'DELETE FROM sync_items', ]; - return databases_[id].transactionExecBatch(queries); + await databases_[id].transactionExecBatch(queries); } -function setupDatabase(id = null) { +async function setupDatabase(id = null) { if (id === null) id = currentClient_; + Setting.cancelScheduleSave(); + Setting.cache_ = null; + if (databases_[id]) { - return clearDatabase(id).then(() => { - return Setting.load(); - }); + await clearDatabase(id); + await Setting.load(); + return; } const filePath = __dirname + '/data/test-' + id + '.sqlite'; - // Setting.setConstant('resourceDir', RNFetchBlob.fs.dirs.DocumentDir); - return fs.unlink(filePath).catch(() => { + + try { + await fs.unlink(filePath); + } catch (error) { // Don't care if the file doesn't exist - }).then(() => { - databases_[id] = new JoplinDatabase(new DatabaseDriverNode()); - // databases_[id].setLogger(logger); - // console.info(filePath); - return databases_[id].open({ name: filePath }).then(() => { - BaseModel.db_ = databases_[id]; - return setupDatabase(id); - }); - }); + }; + + databases_[id] = new JoplinDatabase(new DatabaseDriverNode()); + await databases_[id].open({ name: filePath }); + + BaseModel.db_ = databases_[id]; + await Setting.load(); + //return setupDatabase(id); + + + + // return databases_[id].open({ name: filePath }).then(() => { + // BaseModel.db_ = databases_[id]; + // return setupDatabase(id); + // }); + + + // return fs.unlink(filePath).catch(() => { + // // Don't care if the file doesn't exist + // }).then(() => { + // databases_[id] = new JoplinDatabase(new DatabaseDriverNode()); + // return databases_[id].open({ name: filePath }).then(() => { + // BaseModel.db_ = databases_[id]; + // return setupDatabase(id); + // }); + // }); } function resourceDir(id = null) { @@ -151,6 +173,9 @@ async function setupDatabaseAndSynchronizer(id = null) { await setupDatabase(id); + EncryptionService.instance_ = null; + DecryptionWorker.instance_ = null; + await fs.remove(resourceDir(id)); await fs.mkdirp(resourceDir(id), 0o755); diff --git a/ReactNativeClient/lib/BaseModel.js b/ReactNativeClient/lib/BaseModel.js index 927b3c6e2..9a1386a87 100644 --- a/ReactNativeClient/lib/BaseModel.js +++ b/ReactNativeClient/lib/BaseModel.js @@ -96,8 +96,11 @@ class BaseModel { return options; } - static count() { - return this.db().selectOne('SELECT count(*) as total FROM `' + this.tableName() + '`').then((r) => { + static count(options = null) { + if (!options) options = {}; + let sql = 'SELECT count(*) as total FROM `' + this.tableName() + '`'; + if (options.where) sql += ' WHERE ' + options.where; + return this.db().selectOne(sql).then((r) => { return r ? r['total'] : 0; }); } diff --git a/ReactNativeClient/lib/database.js b/ReactNativeClient/lib/database.js index b2357680c..a601a032a 100644 --- a/ReactNativeClient/lib/database.js +++ b/ReactNativeClient/lib/database.js @@ -104,29 +104,38 @@ class Database { return this.tryCall('exec', sql, params); } - transactionExecBatch(queries) { - if (queries.length <= 0) return Promise.resolve(); + async transactionExecBatch(queries) { + if (queries.length <= 0) return; if (queries.length == 1) { let q = this.wrapQuery(queries[0]); - return this.exec(q.sql, q.params); + await this.exec(q.sql, q.params); + return; } // There can be only one transaction running at a time so queue // any new transaction here. if (this.inTransaction_) { - return new Promise((resolve, reject) => { - let iid = setInterval(() => { - if (!this.inTransaction_) { - clearInterval(iid); - this.transactionExecBatch(queries).then(() => { - resolve(); - }).catch((error) => { - reject(error); - }); - } - }, 100); - }); + while (true) { + await time.msleep(100); + if (!this.inTransaction_) { + this.inTransaction_ = true; + break; + } + } + + // return new Promise((resolve, reject) => { + // let iid = setInterval(() => { + // if (!this.inTransaction_) { + // clearInterval(iid); + // this.transactionExecBatch(queries).then(() => { + // resolve(); + // }).catch((error) => { + // reject(error); + // }); + // } + // }, 100); + // }); } this.inTransaction_ = true; @@ -134,17 +143,62 @@ class Database { queries.splice(0, 0, 'BEGIN TRANSACTION'); queries.push('COMMIT'); // Note: ROLLBACK is currently not supported - let chain = []; for (let i = 0; i < queries.length; i++) { let query = this.wrapQuery(queries[i]); - chain.push(() => { - return this.exec(query.sql, query.params); - }); + await this.exec(query.sql, query.params); } - return promiseChain(chain).then(() => { - this.inTransaction_ = false; - }); + this.inTransaction_ = false; + + // return promiseChain(chain).then(() => { + // this.inTransaction_ = false; + // }); + + + + + + + // if (queries.length <= 0) return Promise.resolve(); + + // if (queries.length == 1) { + // let q = this.wrapQuery(queries[0]); + // return this.exec(q.sql, q.params); + // } + + // // There can be only one transaction running at a time so queue + // // any new transaction here. + // if (this.inTransaction_) { + // return new Promise((resolve, reject) => { + // let iid = setInterval(() => { + // if (!this.inTransaction_) { + // clearInterval(iid); + // this.transactionExecBatch(queries).then(() => { + // resolve(); + // }).catch((error) => { + // reject(error); + // }); + // } + // }, 100); + // }); + // } + + // this.inTransaction_ = true; + + // queries.splice(0, 0, 'BEGIN TRANSACTION'); + // queries.push('COMMIT'); // Note: ROLLBACK is currently not supported + + // let chain = []; + // for (let i = 0; i < queries.length; i++) { + // let query = this.wrapQuery(queries[i]); + // chain.push(() => { + // return this.exec(query.sql, query.params); + // }); + // } + + // return promiseChain(chain).then(() => { + // this.inTransaction_ = false; + // }); } static enumId(type, s) { diff --git a/ReactNativeClient/lib/models/BaseItem.js b/ReactNativeClient/lib/models/BaseItem.js index 404f9afee..1b8e5307d 100644 --- a/ReactNativeClient/lib/models/BaseItem.js +++ b/ReactNativeClient/lib/models/BaseItem.js @@ -258,7 +258,13 @@ class BaseItem extends BaseModel { static async serializeForSync(item) { const ItemClass = this.itemClass(item); let serialized = await ItemClass.serialize(item); - if (!Setting.value('encryption.enabled') || !ItemClass.encryptionSupported()) return serialized; + if (!Setting.value('encryption.enabled') || !ItemClass.encryptionSupported()) { + // Sanity check - normally not possible + if (!!item.encryption_applied) throw new Error('Item is encrypted but encryption is currently disabled'); + return serialized; + } + + if (!!item.encryption_applied) { const e = new Error('Trying to encrypt item that is already encrypted'); e.code = 'cannotEncryptEncrypted'; throw e; } const cipherText = await this.encryptionService().encryptString(serialized); @@ -343,6 +349,20 @@ class BaseItem extends BaseModel { return output; } + static async hasEncryptedItems() { + const classNames = this.encryptableItemClassNames(); + + for (let i = 0; i < classNames.length; i++) { + const className = classNames[i]; + const ItemClass = this.getClass(className); + + const count = await ItemClass.count({ where: 'encryption_applied = 1' }); + if (count) return true; + } + + return false; + } + static async itemsThatNeedDecryption(exclusions = [], limit = 100) { const classNames = this.encryptableItemClassNames(); @@ -568,6 +588,14 @@ class BaseItem extends BaseModel { } } + static async forceSync(itemId) { + await this.db().exec('UPDATE sync_items SET force_sync = 1 WHERE item_id = ?', [itemId]); + } + + static async forceSyncAll() { + await this.db().exec('UPDATE sync_items SET force_sync = 1'); + } + static async save(o, options = null) { if (!options) options = {}; diff --git a/ReactNativeClient/lib/models/Resource.js b/ReactNativeClient/lib/models/Resource.js index 778752f39..fe717188f 100644 --- a/ReactNativeClient/lib/models/Resource.js +++ b/ReactNativeClient/lib/models/Resource.js @@ -81,10 +81,14 @@ class Resource extends BaseItem { const plainTextPath = this.fullPath(resource); if (!Setting.value('encryption.enabled')) { - if (resource.encryption_blob_encrypted) { - resource.encryption_blob_encrypted = 0; - await Resource.save(resource, { autoTimestamp: false }); - } + // Sanity check - normally not possible + if (!!resource.encryption_blob_encrypted) throw new Error('Trying to access encrypted resource but encryption is currently disabled'); + + // // TODO: why is it set to 0 without decrypting first? + // if (resource.encryption_blob_encrypted) { + // resource.encryption_blob_encrypted = 0; + // await Resource.save(resource, { autoTimestamp: false }); + // } return { path: plainTextPath, resource: resource }; } diff --git a/ReactNativeClient/lib/services/EncryptionService.js b/ReactNativeClient/lib/services/EncryptionService.js index 3b02ff390..5bfeccd5f 100644 --- a/ReactNativeClient/lib/services/EncryptionService.js +++ b/ReactNativeClient/lib/services/EncryptionService.js @@ -3,6 +3,7 @@ const { shim } = require('lib/shim.js'); const Setting = require('lib/models/Setting.js'); const MasterKey = require('lib/models/MasterKey'); const BaseItem = require('lib/models/BaseItem'); +const { _ } = require('lib/locale.js'); function hexPad(s, length) { return padLeft(s, length, '0'); @@ -33,7 +34,7 @@ class EncryptionService { return this.logger_; } - async initializeEncryption(masterKey, password = null) { + async enableEncryption(masterKey, password = null) { Setting.setValue('encryption.enabled', true); Setting.setValue('encryption.activeMasterKeyId', masterKey.id); @@ -43,9 +44,21 @@ class EncryptionService { Setting.setValue('encryption.passwordCache', passwordCache); } + // Mark only the non-encrypted ones for sync since, if there are encrypted ones, + // it means they come from the sync target and are already encrypted over there. await BaseItem.markAllNonEncryptedForSync(); } + async disableEncryption() { + const hasEncryptedItems = await BaseItem.hasEncryptedItems(); + if (hasEncryptedItems) throw new Error(_('Encryption cannot currently be disabled because some items are still encrypted. Please wait for all the items to be decrypted and try again.')); + + Setting.setValue('encryption.enabled', false); + // The only way to make sure everything gets decrypted on the sync target is + // to re-sync everything. + await BaseItem.forceSyncAll(); + } + async loadMasterKeysFromSettings() { if (!Setting.value('encryption.enabled')) { this.unloadAllMasterKeys(); diff --git a/ReactNativeClient/lib/synchronizer.js b/ReactNativeClient/lib/synchronizer.js index 6f746342b..9dd6af6c3 100644 --- a/ReactNativeClient/lib/synchronizer.js +++ b/ReactNativeClient/lib/synchronizer.js @@ -217,7 +217,6 @@ class Synchronizer { if (donePaths.indexOf(path) > 0) throw new Error(sprintf('Processing a path that has already been done: %s. sync_time was not updated?', path)); let remote = await this.api().stat(path); - //let content = await ItemClass.serializeForSync(local); let action = null; let updateSyncTimeOnly = true; let reason = ''; @@ -561,9 +560,9 @@ class Synchronizer { await BaseItem.deleteOrphanSyncItems(); } } catch (error) { - if (error && error.code === 'noActiveMasterKey') { - // Don't log an error for this as this is a common - // condition that the UI should report anyway. + if (error && ['cannotEncryptEncrypted', 'noActiveMasterKey'].indexOf(error.code) >= 0) { + // Only log an info statement for this since this is a common condition that is reported + // in the application, and needs to be resolved by the user this.logger().info(error.message); } else { this.logger().error(error); @@ -583,7 +582,7 @@ class Synchronizer { const mk = await MasterKey.latest(); if (mk) { this.logger().info('Using master key: ', mk); - await this.encryptionService().initializeEncryption(mk); + await this.encryptionService().enableEncryption(mk); await this.encryptionService().loadMasterKeysFromSettings(); this.logger().info('Encryption has been enabled with downloaded master key as active key. However, note that no password was initially supplied. It will need to be provided by user.'); }