From 210ec26ed37c64461457c907acaf13f2c68bee76 Mon Sep 17 00:00:00 2001 From: Bart Butler Date: Tue, 6 Feb 2018 16:43:51 -0800 Subject: [PATCH] fix sporadic two password decryption failure --- npm-shrinkwrap.json | 3 +- package.json | 2 +- src/config/config.js | 5 +-- src/message.js | 75 ++++++++++++++++++++++++++++++----------- test/general/openpgp.js | 37 ++++++++++++++++---- 5 files changed, 92 insertions(+), 30 deletions(-) diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index bc5a6b08..b97f5114 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -842,7 +842,8 @@ "babel-plugin-transform-remove-strict-mode": { "version": "0.0.2", "resolved": "https://registry.npmjs.org/babel-plugin-transform-remove-strict-mode/-/babel-plugin-transform-remove-strict-mode-0.0.2.tgz", - "integrity": "sha1-kTaFqrlUOfOg7YjliPvV6ZeJBXk=" + "integrity": "sha1-kTaFqrlUOfOg7YjliPvV6ZeJBXk=", + "dev": true }, "babel-plugin-transform-runtime": { "version": "6.23.0", diff --git a/package.json b/package.json index 18f496b1..b5ae7875 100644 --- a/package.json +++ b/package.json @@ -36,6 +36,7 @@ "babel-plugin-syntax-async-functions": "^6.13.0", "babel-plugin-transform-async-to-generator": "^6.24.1", "babel-plugin-transform-regenerator": "^6.26.0", + "babel-plugin-transform-remove-strict-mode": "0.0.2", "babel-plugin-transform-runtime": "^6.23.0", "babel-polyfill": "^6.26.0", "babel-preset-env": "^1.6.1", @@ -76,7 +77,6 @@ "dependencies": { "asmcrypto-lite": "git+https://github.com/openpgpjs/asmcrypto-lite.git", "asn1.js": "^5.0.0", - "babel-plugin-transform-remove-strict-mode": "0.0.2", "bn.js": "^4.11.8", "buffer": "^5.0.8", "elliptic": "git+https://github.com/openpgpjs/elliptic.git", diff --git a/src/config/config.js b/src/config/config.js index 0e9b2287..d0ef9424 100644 --- a/src/config/config.js +++ b/src/config/config.js @@ -37,7 +37,7 @@ export default { prefer_hash_algorithm: enums.hash.sha256, encryption_cipher: enums.symmetric.aes256, compression: enums.compression.zip, - aead_protect: false, // use Authenticated Encryption with Additional Data (AEAD) protection for symmetric encryption + aead_protect: false, // use Authenticated Encryption with Additional Data (AEAD) protection for symmetric encryption (NOT INTEROPERABLE WITH OTHER OPENPGP IMPLEMENTATIONS) integrity_protect: true, // use integrity protection for symmetric encryption ignore_mdc_error: false, // fail on decrypt if message is not integrity protected checksum_required: false, // do not throw error when armor is missing a checksum @@ -46,7 +46,8 @@ export default { use_native: true, // use native node.js crypto and Web Crypto apis (if available) zero_copy: false, // use transferable objects between the Web Worker and main thread debug: false, - tolerant: true, // ignore unsupported/unrecognizable packets instead of throwing an error + tolerant: true, // ignore unsupported/unrecognizable packets instead of throwing an error, + password_collision_check: false, // work-around for rare GPG decryption bug with encrypting with multiple passwords show_version: true, show_comment: true, versionstring: "OpenPGP.js VERSION", diff --git a/src/message.js b/src/message.js index f39a29a2..a92bd372 100644 --- a/src/message.js +++ b/src/message.js @@ -97,9 +97,9 @@ Message.prototype.getSigningKeyIds = function() { * @return {Message} new message with decrypted content */ Message.prototype.decrypt = async function(privateKey, sessionKey, password) { - const keyObj = sessionKey || await this.decryptSessionKey(privateKey, password); - if (!keyObj || !util.isUint8Array(keyObj.data) || !util.isString(keyObj.algorithm)) { - throw new Error('Invalid session key for decryption.'); + let keyObjs = sessionKey || await this.decryptSessionKey(privateKey, password); + if (!util.isArray(keyObjs)) { + keyObjs = [keyObjs]; } const symEncryptedPacketlist = this.packets.filterByTag( @@ -113,7 +113,26 @@ Message.prototype.decrypt = async function(privateKey, sessionKey, password) { } const symEncryptedPacket = symEncryptedPacketlist[0]; - await symEncryptedPacket.decrypt(keyObj.algorithm, keyObj.data); + let exception = null; + for (let i = 0; i < keyObjs.length; i++) { + if (!keyObjs[i] || !util.isUint8Array(keyObjs[i].data) || !util.isString(keyObjs[i].algorithm)) { + throw new Error('Invalid session key for decryption.'); + } + + try { + // eslint-disable-next-line no-await-in-loop + await symEncryptedPacket.decrypt(keyObjs[i].algorithm, keyObjs[i].data); + break; + } + catch(e) { + exception = e; + } + } + + if (!symEncryptedPacket.packets || !symEncryptedPacket.packets.length) { + throw exception ? exception : new Error('Decryption failed.'); + } + const resultMsg = new Message(symEncryptedPacket.packets); symEncryptedPacket.packets = new packet.List(); // remove packets after decryption @@ -124,23 +143,21 @@ Message.prototype.decrypt = async function(privateKey, sessionKey, password) { * Decrypt an encrypted session key either with a private key or a password. * @param {Key} privateKey (optional) private key with decrypted secret data * @param {String} password (optional) password used to decrypt - * @return {Object} object with sessionKey, algorithm in the form: + * @return {Array} array of object with potential sessionKey, algorithm pairs in the form: * { data:Uint8Array, algorithm:String } */ Message.prototype.decryptSessionKey = function(privateKey, password) { - var keyPacket; + var keyPackets = []; return Promise.resolve().then(async () => { if (password) { var symESKeyPacketlist = this.packets.filterByTag(enums.packet.symEncryptedSessionKey); if (!symESKeyPacketlist) { throw new Error('No symmetrically encrypted session key packet found.'); } - // TODO replace when Promise.some or Promise.any are implemented - await symESKeyPacketlist.some(async function(packet) { + await symESKeyPacketlist.map(async function(packet) { try { await packet.decrypt(password); - keyPacket = packet; - return true; + keyPackets.push(packet); } catch (err) {} }); @@ -159,8 +176,7 @@ Message.prototype.decryptSessionKey = function(privateKey, password) { if (packet.publicKeyId.equals(privateKeyPacket.getKeyId())) { try { await packet.decrypt(privateKeyPacket); - keyPacket = packet; - return true; + keyPackets.push(packet); } catch (err) {} } }); @@ -168,11 +184,8 @@ Message.prototype.decryptSessionKey = function(privateKey, password) { throw new Error('No key or password specified.'); } }).then(() => { - if (keyPacket) { - return { - data: keyPacket.sessionKey, - algorithm: keyPacket.sessionKeyAlgorithm - }; + if (keyPackets.length) { + return keyPackets.map(packet => ({ data: packet.sessionKey, algorithm: packet.sessionKeyAlgorithm })); } else { throw new Error('Session key decryption failed.'); } @@ -296,14 +309,38 @@ export function encryptSessionKey(sessionKey, symAlgo, publicKeys, passwords) { } if (passwords) { - results = await Promise.all(passwords.map(async function(password) { + + const testDecrypt = async function(keyPacket, password) { + try { + await keyPacket.decrypt(password); + return 1; + } + catch (e) { + return 0; + } + }; + + const sum = (accumulator, currentValue) => accumulator + currentValue; + + const encryptPassword = async function(sessionKey, symAlgo, password) { + var symEncryptedSessionKeyPacket = new packet.SymEncryptedSessionKey(); symEncryptedSessionKeyPacket.sessionKey = sessionKey; symEncryptedSessionKeyPacket.sessionKeyAlgorithm = symAlgo; await symEncryptedSessionKeyPacket.encrypt(password); + + if (config.password_collision_check) { + var results = await Promise.all(passwords.map(pwd => testDecrypt(symEncryptedSessionKeyPacket, pwd))); + if (results.reduce(sum) !== 1) { + return encryptPassword(sessionKey, symAlgo, password); + } + } + delete symEncryptedSessionKeyPacket.sessionKey; // delete plaintext session key after encryption return symEncryptedSessionKeyPacket; - })); + }; + + results = await Promise.all(passwords.map(pwd => encryptPassword(sessionKey, symAlgo, pwd))); packetlist.concat(results); } }).then(() => { diff --git a/test/general/openpgp.js b/test/general/openpgp.js index 4991b0e2..cebaf23f 100644 --- a/test/general/openpgp.js +++ b/test/general/openpgp.js @@ -164,6 +164,18 @@ var plaintext = 'short message\nnext line\n한국어/조선말'; var password1 = 'I am a password'; var password2 = 'I am another password'; +var twoPasswordGPGFail = ['-----BEGIN PGP MESSAGE-----', +'Version: OpenPGP.js v3.0.0', +'Comment: https://openpgpjs.org', +'', +'wy4ECQMIWjj3WEfWxGpgrfb3vXu0TS9L8UNTBvNZFIjltGjMVkLFD+/afgs5', +'aXt0wy4ECQMIrFo3TFN5xqtgtB+AaAjBcWJrA4bvIPBpJ38PbMWeF0JQgrqg', +'j3uehxXy0mUB5i7B61g0ho+YplyFGM0s9XayJCnu40tWmr5LqqsRxuwrhJKR', +'migslOF/l6Y9F0F9xGIZWGhxp3ugQPjVKjj8fOH7ap14mLm60C8q8AOxiSmL', +'ubsd/hL7FPZatUYAAZVA0a6hmQ==', +'=cHCV', +'-----END PGP MESSAGE-----'].join('\n'); + describe('OpenPGP.js public api tests', function() { describe('initWorker, getWorker, destroyWorker - unit tests', function() { @@ -532,7 +544,7 @@ describe('OpenPGP.js public api tests', function() { privateKey: privateKey.keys[0] }); }).then(function(decrypted) { - expect(decrypted.data).to.deep.equal(sk); + expect(decrypted[0].data).to.deep.equal(sk); }); }); @@ -547,7 +559,7 @@ describe('OpenPGP.js public api tests', function() { password: password1 }); }).then(function(decrypted) { - expect(decrypted.data).to.deep.equal(sk); + expect(decrypted[0].data).to.deep.equal(sk); }); }); @@ -563,9 +575,9 @@ describe('OpenPGP.js public api tests', function() { privateKey: privateKey.keys[0] }); - }).then(function(decryptedSessionKey) { + }).then(function(decryptedSessionKeys) { return openpgp.decrypt({ - sessionKey: decryptedSessionKey, + sessionKey: decryptedSessionKeys[0], message: openpgp.message.readArmored(msgAsciiArmored) }); @@ -586,9 +598,9 @@ describe('OpenPGP.js public api tests', function() { password: password1 }); - }).then(function(decryptedSessionKey) { + }).then(function(decryptedSessionKeys) { return openpgp.decrypt({ - sessionKey: decryptedSessionKey, + sessionKey: decryptedSessionKeys[0], message: openpgp.message.readArmored(msgAsciiArmored) }); @@ -1224,7 +1236,6 @@ describe('OpenPGP.js public api tests', function() { }); }); - // FIXME this test sporadically fails it('should encrypt and decrypt with two passwords', function() { var encOpt = { data: plaintext, @@ -1242,6 +1253,18 @@ describe('OpenPGP.js public api tests', function() { }); }); + it('should decrypt with two passwords message which GPG fails on', function() { + + var decOpt = { + message: openpgp.message.readArmored(twoPasswordGPGFail), + password: password2 + }; + return openpgp.decrypt(decOpt).then(function(decrypted) { + expect(decrypted.data).to.equal(plaintext); + expect(decrypted.signatures.length).to.equal(0); + }); + }); + it('should encrypt and decrypt with password and not ascii armor', function() { var encOpt = { data: plaintext,