From 685f541bb4aa70f1de66edb8ce1e890846188478 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Mon, 4 Dec 2017 19:01:56 +0000 Subject: [PATCH] All: Fixed issue with local resource needlessly marked as encrypted --- CliClient/tests/synchronizer.js | 15 +++++++++++++ ReactNativeClient/lib/models/Resource.js | 14 ++++++------ .../lib/services/DecryptionWorker.js | 2 +- .../lib/services/EncryptionService.js | 22 +++++++++++++++++++ 4 files changed, 45 insertions(+), 8 deletions(-) diff --git a/CliClient/tests/synchronizer.js b/CliClient/tests/synchronizer.js index dd939a775..99fee8a3e 100644 --- a/CliClient/tests/synchronizer.js +++ b/CliClient/tests/synchronizer.js @@ -948,4 +948,19 @@ describe('Synchronizer', function() { expect(await allSyncTargetItemsEncrypted()).toBe(true); })); + it('should upload encrypted resource, but it should not mark the blob as encrypted locally', asyncTest(async () => { + while (insideBeforeEach) await time.msleep(100); + + let folder1 = await Folder.save({ title: "folder1" }); + let note1 = await Note.save({ title: 'ma note', parent_id: folder1.id }); + await shim.attachFileToNote(note1, __dirname + '/../tests/support/photo.jpg'); + const masterKey = await loadEncryptionMasterKey(); + await encryptionService().enableEncryption(masterKey, '123456'); + await encryptionService().loadMasterKeysFromSettings(); + await synchronizer().start(); + + let resource1 = (await Resource.all())[0]; + expect(resource1.encryption_blob_encrypted).toBe(0); + })); + }); \ No newline at end of file diff --git a/ReactNativeClient/lib/models/Resource.js b/ReactNativeClient/lib/models/Resource.js index fb233b2c1..7e6057258 100644 --- a/ReactNativeClient/lib/models/Resource.js +++ b/ReactNativeClient/lib/models/Resource.js @@ -76,10 +76,11 @@ class Resource extends BaseItem { return super.save(decryptedItem, { autoTimestamp: false }); } - // Prepare the resource by encrypting it if needed. - // The call returns the path to the physical file AND the resource object - // which may have been modified. So the caller should update their copy with this. + // The call returns the path to the physical file AND a representation of the resource object + // as it should be uploaded to the sync target. Note that this may be different from what is stored + // in the database. In particular, the flag encryption_blob_encrypted might be 1 on the sync target + // if the resource is encrypted, but will be 0 locally because the device has the decrypted resource. static async fullPathForSyncUpload(resource) { const plainTextPath = this.fullPath(resource); @@ -93,10 +94,9 @@ class Resource extends BaseItem { if (resource.encryption_blob_encrypted) return { path: encryptedPath, resource: resource }; await this.encryptionService().encryptFile(plainTextPath, encryptedPath); - resource.encryption_blob_encrypted = 1; - await Resource.save(resource, { autoTimestamp: false }); - - return { path: encryptedPath, resource: resource }; + const resourceCopy = Object.assign({}, resource); + resourceCopy.encryption_blob_encrypted = 1; + return { path: encryptedPath, resource: resourceCopy }; } static markdownTag(resource) { diff --git a/ReactNativeClient/lib/services/DecryptionWorker.js b/ReactNativeClient/lib/services/DecryptionWorker.js index 8b66e37d9..a275d90e9 100644 --- a/ReactNativeClient/lib/services/DecryptionWorker.js +++ b/ReactNativeClient/lib/services/DecryptionWorker.js @@ -70,8 +70,8 @@ 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); + this.logger().debug('DecryptionWorker: decrypting: ' + item.id + ' (' + ItemClass.tableName() + ')'); try { await ItemClass.decrypt(item); } catch (error) { diff --git a/ReactNativeClient/lib/services/EncryptionService.js b/ReactNativeClient/lib/services/EncryptionService.js index dea3ddee7..7600c394e 100644 --- a/ReactNativeClient/lib/services/EncryptionService.js +++ b/ReactNativeClient/lib/services/EncryptionService.js @@ -322,7 +322,10 @@ class EncryptionService { async decryptAbstract_(source, destination) { const identifier = await source.read(5); + if (!this.isValidHeaderIdentifier(identifier)) throw new Error('Invalid encryption identifier. Data is not actually encrypted? ID was: ' + identifier); const mdSizeHex = await source.read(6); + const mdSize = parseInt(mdSizeHex, 16); + if (isNaN(mdSize) || !mdSize) throw new Error('Invalid header metadata size: ' + mdSizeHex); const md = await source.read(parseInt(mdSizeHex, 16)); const header = this.decodeHeader_(identifier + mdSizeHex + md); const masterKeyPlainText = this.loadedMasterKey(header.masterKeyId); @@ -503,6 +506,25 @@ class EncryptionService { return output; } + isValidHeaderIdentifier(id, ignoreTooLongLength = false) { + if (!ignoreTooLongLength && !id || id.length !== 5) return false; + return /JED\d\d/.test(id); + } + + async itemIsEncrypted(item) { + if (!item) throw new Error('No item'); + const ItemClass = BaseItem.itemClass(item); + if (!ItemClass.encryptionSupported()) return false; + return item.encryption_applied && this.isValidHeaderIdentifier(item.encryption_cipher_text); + } + + async fileIsEncrypted(path) { + const handle = await this.fsDriver().open(path, 'r'); + const headerIdentifier = await this.fsDriver().readFileChunk(handle, 5, 'ascii'); + await this.fsDriver().close(handle); + return this.isValidHeaderIdentifier(headerIdentifier); + } + } EncryptionService.METHOD_SJCL = 1;