All: Fixes #2591: Handle invalid UTF-8 data when encrypting

CliClient/tests/encryption.js

@@ -190,6 +190,24 @@ describe('Encryption', function() {
 		expect(fileContentEqual(sourcePath, decryptedPath)).toBe(true);
 	}));
 
+	it('should encrypt invalid UTF-8 data', asyncTest(async () => {
+		let masterKey = await service.generateMasterKey('123456');
+		masterKey = await MasterKey.save(masterKey);
+
+		await service.loadMasterKey_(masterKey, '123456', true);
+
+		// First check that we can replicate the error with the old encryption method
+		service.defaultEncryptionMethod_ = EncryptionService.METHOD_SJCL;
+		const hasThrown = await checkThrowAsync(async () => await service.encryptString('🐶🐶🐶'.substr(0,5)));
+		expect(hasThrown).toBe(true);
+
+		// Now check that the new one fixes the problem
+		service.defaultEncryptionMethod_ = EncryptionService.METHOD_SJCL_1A;
+		const cipherText = await service.encryptString('🐶🐶🐶'.substr(0,5));
+		const plainText = await service.decryptString(cipherText);
+		expect(plainText).toBe('🐶🐶🐶'.substr(0,5));
+	}));
+
 	// it('should upgrade master key encryption mode', asyncTest(async () => {
 	// 	let masterKey = await service.generateMasterKey('123456', {
 	// 		encryptionMethod: EncryptionService.METHOD_SJCL_2,

ReactNativeClient/lib/models/BaseItem.js

@@ -348,7 +348,9 @@ class BaseItem extends BaseModel {
 		} catch (error) {
 			const msg = [`Could not encrypt item ${item.id}`];
 			if (error && error.message) msg.push(error.message);
-			throw new Error(msg.join(': '));
+			const newError = new Error(msg.join(': '));
+			newError.stack = error.stack;
+			throw newError;
 		}
 
 		// List of keys that won't be encrypted - mostly foreign keys required to link items

ReactNativeClient/lib/services/EncryptionService.js

@@ -28,7 +28,7 @@ class EncryptionService {
 		this.chunkSize_ = 5000;
 		this.loadedMasterKeys_ = {};
 		this.activeMasterKeyId_ = null;
-		this.defaultEncryptionMethod_ = EncryptionService.METHOD_SJCL;
+		this.defaultEncryptionMethod_ = EncryptionService.METHOD_SJCL_1A;
 		this.defaultMasterKeyEncryptionMethod_ = EncryptionService.METHOD_SJCL_2;
 		this.logger_ = new Logger();
 
@@ -274,6 +274,12 @@ class EncryptionService {
 		return true;
 	}
 
+	wrapSjclError(sjclError) {
+		const error = new Error(sjclError.message);
+		error.stack = sjclError.stack;
+		return error;
+	}
+
 	async encrypt(method, key, plainText) {
 		if (!method) throw new Error('Encryption method is required');
 		if (!key) throw new Error('Encryption key is required');
@@ -294,8 +300,28 @@ class EncryptionService {
 					cipher: 'aes',
 				});
 			} catch (error) {
-				// SJCL returns a string as error which means stack trace is missing so convert to an error object here
-				throw new Error(`SJCL error: ${error.message}`);
+				throw this.wrapSjclError(error);
+			}
+		}
+
+		// 2020-03-06: Added method to fix https://github.com/laurent22/joplin/issues/2591
+		//             Also took the opportunity to change number of key derivations, per Isaac Potoczny's suggestion
+		if (method === EncryptionService.METHOD_SJCL_1A) {
+			try {
+				// We need to escape the data because SJCL uses encodeURIComponent to process the data and it only
+				// accepts UTF-8 data, or else it throws an error. And the notes might occasionally contain
+				// invalid UTF-8 data. Fixes https://github.com/laurent22/joplin/issues/2591
+				return sjcl.json.encrypt(key, escape(plainText), {
+					v: 1, // version
+					iter: 101, // Since the master key already uses key derivations and is secure, additional iteration here aren't necessary, which will make decryption faster. SJCL enforces an iter strictly greater than 100
+					ks: 128, // Key size - "128 bits should be secure enough"
+					ts: 64, // ???
+					mode: 'ocb2', //  The cipher mode is a standard for how to use AES and other algorithms to encrypt and authenticate your message. OCB2 mode is slightly faster and has more features, but CCM mode has wider support because it is not patented.
+					// "adata":"", // Associated Data - not needed?
+					cipher: 'aes',
+				});
+			} catch (error) {
+				throw this.wrapSjclError(error);
 			}
 		}
 
@@ -312,8 +338,7 @@ class EncryptionService {
 					cipher: 'aes',
 				});
 			} catch (error) {
-				// SJCL returns a string as error which means stack trace is missing so convert to an error object here
-				throw new Error(`SJCL error: ${error.message}`);
+				throw this.wrapSjclError(error);
 			}
 		}
 
@@ -330,8 +355,7 @@ class EncryptionService {
 					cipher: 'aes',
 				});
 			} catch (error) {
-				// SJCL returns a string as error which means stack trace is missing so convert to an error object here
-				throw new Error(`SJCL error: ${error.message}`);
+				throw this.wrapSjclError(error);
 			}
 		}
 
@@ -347,8 +371,7 @@ class EncryptionService {
 					cipher: 'aes',
 				});
 			} catch (error) {
-				// SJCL returns a string as error which means stack trace is missing so convert to an error object here
-				throw new Error(`SJCL error: ${error.message}`);
+				throw this.wrapSjclError(error);
 			}
 		}
 
@@ -363,7 +386,13 @@ class EncryptionService {
 		if (!this.isValidEncryptionMethod(method)) throw new Error(`Unknown decryption method: ${method}`);
 
 		try {
-			return sjcl.json.decrypt(key, cipherText);
+			const output = sjcl.json.decrypt(key, cipherText);
+
+			if (method === EncryptionService.METHOD_SJCL_1A) {
+				return unescape(output);
+			} else {
+				return output;
+			}
 		} catch (error) {
 			// SJCL returns a string as error which means stack trace is missing so convert to an error object here
 			throw new Error(error.message);
@@ -618,7 +647,7 @@ class EncryptionService {
 	}
 
 	isValidEncryptionMethod(method) {
-		return [EncryptionService.METHOD_SJCL, EncryptionService.METHOD_SJCL_2, EncryptionService.METHOD_SJCL_3, EncryptionService.METHOD_SJCL_4].indexOf(method) >= 0;
+		return [EncryptionService.METHOD_SJCL, EncryptionService.METHOD_SJCL_1A, EncryptionService.METHOD_SJCL_2, EncryptionService.METHOD_SJCL_3, EncryptionService.METHOD_SJCL_4].indexOf(method) >= 0;
 	}
 
 	async itemIsEncrypted(item) {
@@ -640,6 +669,7 @@ EncryptionService.METHOD_SJCL = 1;
 EncryptionService.METHOD_SJCL_2 = 2;
 EncryptionService.METHOD_SJCL_3 = 3;
 EncryptionService.METHOD_SJCL_4 = 4;
+EncryptionService.METHOD_SJCL_1A = 5;
 
 EncryptionService.fsDriver_ = null;