From e785df4c8f97a1d6a0f7d4d6391458cbd22ed322 Mon Sep 17 00:00:00 2001 From: larabr Date: Tue, 15 Jun 2021 17:21:18 +0200 Subject: [PATCH] Require keys in `openpgp.sign` and make all top-level functions fully async (#1318) - `openpgp.sign` throws if no signing keys are given, instead of returning a non-signed literal packet. - Any top-level function error will cause Promise rejection, and can thus be handled with `.catch()`. --- src/key/helper.js | 2 +- src/message.js | 4 +- src/openpgp.js | 183 ++++++++++++++++++++-------------------- test/general/openpgp.js | 50 +++++------ 4 files changed, 117 insertions(+), 122 deletions(-) diff --git a/src/key/helper.js b/src/key/helper.js index f6f59e2a..da816112 100644 --- a/src/key/helper.js +++ b/src/key/helper.js @@ -201,7 +201,7 @@ export async function createSignaturePacket(dataToSign, privateKey, signingKeyPa throw new Error('Cannot sign with a gnu-dummy key.'); } if (!signingKeyPacket.isDecrypted()) { - throw new Error('Private key is not decrypted.'); + throw new Error('Signing key is not decrypted.'); } const signaturePacket = new SignaturePacket(); Object.assign(signaturePacket, signatureProperties); diff --git a/src/message.js b/src/message.js index 8620c945..cc92cc95 100644 --- a/src/message.js +++ b/src/message.js @@ -658,7 +658,7 @@ export class Message { /** * Create signature packets for the message * @param {LiteralDataPacket} literalDataPacket - the literal data packet to sign - * @param {Array} signingKeys - private keys with decrypted secret key data for signing + * @param {Array} [signingKeys] - private keys with decrypted secret key data for signing * @param {Signature} [signature] - Any existing detached signature to append * @param {Array} [signingKeyIDs] - Array of key IDs to use for signing. Each signingKeyIDs[i] corresponds to signingKeys[i] * @param {Date} [date] - Override the creationtime of the signature @@ -684,7 +684,7 @@ export async function createSignaturePackets(literalDataPacket, signingKeys, sig const signingKey = await primaryKey.getSigningKey(signingKeyIDs[i], date, userID, config); return createSignaturePacket(literalDataPacket, primaryKey, signingKey.keyPacket, { signatureType }, date, userID, detached, config); })).then(signatureList => { - signatureList.forEach(signaturePacket => packetlist.push(signaturePacket)); + packetlist.push(...signatureList); }); if (signature) { diff --git a/src/openpgp.js b/src/openpgp.js index 6304b138..8f76ffd3 100644 --- a/src/openpgp.js +++ b/src/openpgp.js @@ -50,7 +50,7 @@ import util from './util'; * @async * @static */ -export function generateKey({ userIDs = [], passphrase = "", type = "ecc", rsaBits = 4096, curve = "curve25519", keyExpirationTime = 0, date = new Date(), subkeys = [{}], config }) { +export async function generateKey({ userIDs = [], passphrase = "", type = "ecc", rsaBits = 4096, curve = "curve25519", keyExpirationTime = 0, date = new Date(), subkeys = [{}], config }) { config = { ...defaultConfig, ...config }; userIDs = toArray(userIDs); if (userIDs.length === 0) { @@ -61,19 +61,20 @@ export function generateKey({ userIDs = [], passphrase = "", type = "ecc", rsaBi } const options = { userIDs, passphrase, type, rsaBits, curve, keyExpirationTime, date, subkeys }; - return generate(options, config).then(async key => { + try { + const key = await generate(options, config); const revocationCertificate = await key.getRevocationCertificate(date, config); key.revocationSignatures = []; return { - - key: key, + key, privateKeyArmored: key.armor(config), publicKeyArmored: key.toPublic().armor(config), revocationCertificate: revocationCertificate - }; - }).catch(onError.bind(null, 'Error generating keypair')); + } catch (err) { + throw util.wrapError('Error generating keypair', err); + } } /** @@ -90,7 +91,7 @@ export function generateKey({ userIDs = [], passphrase = "", type = "ecc", rsaBi * @async * @static */ -export function reformatKey({ privateKey, userIDs = [], passphrase = "", keyExpirationTime = 0, date, config }) { +export async function reformatKey({ privateKey, userIDs = [], passphrase = "", keyExpirationTime = 0, date, config }) { config = { ...defaultConfig, ...config }; userIDs = toArray(userIDs); if (userIDs.length === 0) { @@ -98,19 +99,20 @@ export function reformatKey({ privateKey, userIDs = [], passphrase = "", keyExpi } const options = { privateKey, userIDs, passphrase, keyExpirationTime, date }; - return reformat(options, config).then(async key => { - const revocationCertificate = await key.getRevocationCertificate(date, config); - key.revocationSignatures = []; + try { + const reformattedKey = await reformat(options, config); + const revocationCertificate = await reformattedKey.getRevocationCertificate(date, config); + reformattedKey.revocationSignatures = []; return { - - key: key, - privateKeyArmored: key.armor(config), - publicKeyArmored: key.toPublic().armor(config), + key: reformattedKey, + privateKeyArmored: reformattedKey.armor(config), + publicKeyArmored: reformattedKey.toPublic().armor(config), revocationCertificate: revocationCertificate - }; - }).catch(onError.bind(null, 'Error reformatting keypair')); + } catch (err) { + throw util.wrapError('Error reformatting keypair', err); + } } /** @@ -130,29 +132,30 @@ export function reformatKey({ privateKey, userIDs = [], passphrase = "", keyExpi * @async * @static */ -export function revokeKey({ key, revocationCertificate, reasonForRevocation, date = new Date(), config }) { +export async function revokeKey({ key, revocationCertificate, reasonForRevocation, date = new Date(), config }) { config = { ...defaultConfig, ...config }; - return Promise.resolve().then(() => { - if (revocationCertificate) { - return key.applyRevocationCertificate(revocationCertificate, date, config); - } else { - return key.revoke(reasonForRevocation, date, config); - } - }).then(async key => { - if (key.isPrivate()) { - const publicKey = key.toPublic(); + try { + const revokedKey = revocationCertificate ? + await key.applyRevocationCertificate(revocationCertificate, date, config) : + await key.revoke(reasonForRevocation, date, config); + + if (revokedKey.isPrivate()) { + const publicKey = revokedKey.toPublic(); return { - privateKey: key, - privateKeyArmored: key.armor(config), + privateKey: revokedKey, + privateKeyArmored: revokedKey.armor(config), publicKey: publicKey, publicKeyArmored: publicKey.armor(config) }; } + return { - publicKey: key, - publicKeyArmored: key.armor(config) + publicKey: revokedKey, + publicKeyArmored: revokedKey.armor(config) }; - }).catch(onError.bind(null, 'Error revoking key')); + } catch (err) { + throw util.wrapError('Error revoking key', err); + } } /** @@ -171,9 +174,9 @@ export async function decryptKey({ privateKey, passphrase, config }) { throw new Error("Cannot decrypt a public key"); } const clonedPrivateKey = privateKey.clone(true); + const passphrases = util.isArray(passphrase) ? passphrase : [passphrase]; try { - const passphrases = util.isArray(passphrase) ? passphrase : [passphrase]; await Promise.all(clonedPrivateKey.getKeys().map(key => ( // try to decrypt each key with any of the given passphrases util.anyPromise(passphrases.map(passphrase => key.keyPacket.decrypt(passphrase))) @@ -183,7 +186,7 @@ export async function decryptKey({ privateKey, passphrase, config }) { return clonedPrivateKey; } catch (err) { clonedPrivateKey.clearPrivateParams(); - return onError('Error decrypting private key', err); + throw util.wrapError('Error decrypting private key', err); } } @@ -204,13 +207,13 @@ export async function encryptKey({ privateKey, passphrase, config }) { } const clonedPrivateKey = privateKey.clone(true); - try { - const keys = clonedPrivateKey.getKeys(); - const passphrases = util.isArray(passphrase) ? passphrase : new Array(keys.length).fill(passphrase); - if (passphrases.length !== keys.length) { - throw new Error("Invalid number of passphrases for key"); - } + const keys = clonedPrivateKey.getKeys(); + const passphrases = util.isArray(passphrase) ? passphrase : new Array(keys.length).fill(passphrase); + if (passphrases.length !== keys.length) { + throw new Error("Invalid number of passphrases given for key encryption"); + } + try { await Promise.all(keys.map(async (key, i) => { const { keyPacket } = key; await keyPacket.encrypt(passphrases[i], config); @@ -219,7 +222,7 @@ export async function encryptKey({ privateKey, passphrase, config }) { return clonedPrivateKey; } catch (err) { clonedPrivateKey.clearPrivateParams(); - return onError('Error encrypting private key', err); + throw util.wrapError('Error encrypting private key', err); } } @@ -253,7 +256,7 @@ export async function encryptKey({ privateKey, passphrase, config }) { * @async * @static */ -export function encrypt({ message, encryptionKeys, signingKeys, passwords, sessionKey, armor = true, signature = null, wildcard = false, signingKeyIDs = [], encryptionKeyIDs = [], date = new Date(), signingUserIDs = [], encryptionUserIDs = [], config, ...rest }) { +export async function encrypt({ message, encryptionKeys, signingKeys, passwords, sessionKey, armor = true, signature = null, wildcard = false, signingKeyIDs = [], encryptionKeyIDs = [], date = new Date(), signingUserIDs = [], encryptionUserIDs = [], config, ...rest }) { config = { ...defaultConfig, ...config }; checkMessage(message); encryptionKeys = toArray(encryptionKeys); signingKeys = toArray(signingKeys); passwords = toArray(passwords); signingUserIDs = toArray(signingUserIDs); encryptionUserIDs = toArray(encryptionUserIDs); if (rest.detached) { @@ -262,11 +265,12 @@ export function encrypt({ message, encryptionKeys, signingKeys, passwords, sessi if (rest.publicKeys) throw new Error("The `publicKeys` option has been removed from openpgp.encrypt, pass `encryptionKeys` instead"); if (rest.privateKeys) throw new Error("The `privateKeys` option has been removed from openpgp.encrypt, pass `signingKeys` instead"); - return Promise.resolve().then(async function() { - const streaming = message.fromStream; - if (!signingKeys) { - signingKeys = []; - } + if (!signingKeys) { + signingKeys = []; + } + + const streaming = message.fromStream; + try { if (signingKeys.length || signature) { // sign the message only if signing keys or signature is specified message = await message.sign(signingKeys, signature, signingKeyIDs, date, signingUserIDs, config); } @@ -277,7 +281,9 @@ export function encrypt({ message, encryptionKeys, signingKeys, passwords, sessi message = await message.encrypt(encryptionKeys, passwords, sessionKey, wildcard, encryptionKeyIDs, date, encryptionUserIDs, config); const data = armor ? message.armor(config) : message.write(); return convertStream(data, streaming, armor ? 'utf8' : 'binary'); - }).catch(onError.bind(null, 'Error encrypting message')); + } catch (err) { + throw util.wrapError('Error encrypting message', err); + } } /** @@ -311,13 +317,14 @@ export function encrypt({ message, encryptionKeys, signingKeys, passwords, sessi * @async * @static */ -export function decrypt({ message, decryptionKeys, passwords, sessionKeys, verificationKeys, expectSigned = false, format = 'utf8', signature = null, date = new Date(), config, ...rest }) { +export async function decrypt({ message, decryptionKeys, passwords, sessionKeys, verificationKeys, expectSigned = false, format = 'utf8', signature = null, date = new Date(), config, ...rest }) { config = { ...defaultConfig, ...config }; checkMessage(message); verificationKeys = toArray(verificationKeys); decryptionKeys = toArray(decryptionKeys); passwords = toArray(passwords); sessionKeys = toArray(sessionKeys); if (rest.privateKeys) throw new Error("The `privateKeys` option has been removed from openpgp.decrypt, pass `decryptionKeys` instead"); if (rest.publicKeys) throw new Error("The `publicKeys` option has been removed from openpgp.decrypt, pass `verificationKeys` instead"); - return message.decrypt(decryptionKeys, passwords, sessionKeys, date, config).then(async function(decrypted) { + try { + const decrypted = await message.decrypt(decryptionKeys, passwords, sessionKeys, date, config); if (!verificationKeys) { verificationKeys = []; } @@ -344,7 +351,9 @@ export function decrypt({ message, decryptionKeys, passwords, sessionKeys, verif result.data = await convertStream(result.data, message.fromStream, format); if (!message.fromStream) await prepareSignatures(result.signatures); return result; - }).catch(onError.bind(null, 'Error decrypting message')); + } catch (err) { + throw util.wrapError('Error decrypting message', err); + } } @@ -370,7 +379,7 @@ export function decrypt({ message, decryptionKeys, passwords, sessionKeys, verif * @async * @static */ -export function sign({ message, signingKeys, armor = true, detached = false, signingKeyIDs = [], date = new Date(), signingUserIDs = [], config, ...rest }) { +export async function sign({ message, signingKeys, armor = true, detached = false, signingKeyIDs = [], date = new Date(), signingUserIDs = [], config, ...rest }) { config = { ...defaultConfig, ...config }; checkCleartextOrMessage(message); if (rest.privateKeys) throw new Error("The `privateKeys` option has been removed from openpgp.sign, pass `signingKeys` instead"); @@ -378,8 +387,11 @@ export function sign({ message, signingKeys, armor = true, detached = false, sig if (message instanceof CleartextMessage && detached) throw new Error("Can't detach-sign a cleartext message"); signingKeys = toArray(signingKeys); signingUserIDs = toArray(signingUserIDs); + if (!signingKeys || signingKeys.length === 0) { + throw new Error('No signing keys provided'); + } - return Promise.resolve().then(async function() { + try { let signature; if (detached) { signature = await message.signDetached(signingKeys, undefined, signingKeyIDs, date, signingUserIDs, config); @@ -396,7 +408,9 @@ export function sign({ message, signingKeys, armor = true, detached = false, sig }); } return convertStream(signature, message.fromStream, armor ? 'utf8' : 'binary'); - }).catch(onError.bind(null, 'Error signing message')); + } catch (err) { + throw util.wrapError('Error signing message', err); + } } /** @@ -425,7 +439,7 @@ export function sign({ message, signingKeys, armor = true, detached = false, sig * @async * @static */ -export function verify({ message, verificationKeys, expectSigned = false, format = 'utf8', signature = null, date = new Date(), config, ...rest }) { +export async function verify({ message, verificationKeys, expectSigned = false, format = 'utf8', signature = null, date = new Date(), config, ...rest }) { config = { ...defaultConfig, ...config }; checkCleartextOrMessage(message); if (rest.publicKeys) throw new Error("The `publicKeys` option has been removed from openpgp.verify, pass `verificationKeys` instead"); @@ -434,7 +448,7 @@ export function verify({ message, verificationKeys, expectSigned = false, format verificationKeys = toArray(verificationKeys); - return Promise.resolve().then(async function() { + try { const result = {}; if (signature) { result.signatures = await message.verifyDetached(signature, verificationKeys, date, config); @@ -457,7 +471,9 @@ export function verify({ message, verificationKeys, expectSigned = false, format result.data = await convertStream(result.data, message.fromStream, format); if (!message.fromStream) await prepareSignatures(result.signatures); return result; - }).catch(onError.bind(null, 'Error verifying signed message')); + } catch (err) { + throw util.wrapError('Error verifying signed message', err); + } } @@ -478,16 +494,17 @@ export function verify({ message, verificationKeys, expectSigned = false, format * @async * @static */ -export function generateSessionKey({ encryptionKeys, date = new Date(), encryptionUserIDs = [], config, ...rest }) { +export async function generateSessionKey({ encryptionKeys, date = new Date(), encryptionUserIDs = [], config, ...rest }) { config = { ...defaultConfig, ...config }; encryptionKeys = toArray(encryptionKeys); encryptionUserIDs = toArray(encryptionUserIDs); if (rest.publicKeys) throw new Error("The `publicKeys` option has been removed from openpgp.generateSessionKey, pass `encryptionKeys` instead"); - return Promise.resolve().then(async function() { - - return Message.generateSessionKey(encryptionKeys, date, encryptionUserIDs, config); - - }).catch(onError.bind(null, 'Error generating session key')); + try { + const sessionKeys = await Message.generateSessionKey(encryptionKeys, date, encryptionUserIDs, config); + return sessionKeys; + } catch (err) { + throw util.wrapError('Error generating session key', err); + } } /** @@ -509,17 +526,17 @@ export function generateSessionKey({ encryptionKeys, date = new Date(), encrypti * @async * @static */ -export function encryptSessionKey({ data, algorithm, aeadAlgorithm, encryptionKeys, passwords, armor = true, wildcard = false, encryptionKeyIDs = [], date = new Date(), encryptionUserIDs = [], config, ...rest }) { +export async function encryptSessionKey({ data, algorithm, aeadAlgorithm, encryptionKeys, passwords, armor = true, wildcard = false, encryptionKeyIDs = [], date = new Date(), encryptionUserIDs = [], config, ...rest }) { config = { ...defaultConfig, ...config }; checkBinary(data); checkString(algorithm, 'algorithm'); encryptionKeys = toArray(encryptionKeys); passwords = toArray(passwords); encryptionUserIDs = toArray(encryptionUserIDs); if (rest.publicKeys) throw new Error("The `publicKeys` option has been removed from openpgp.encryptSessionKey, pass `encryptionKeys` instead"); - return Promise.resolve().then(async function() { - + try { const message = await Message.encryptSessionKey(data, algorithm, aeadAlgorithm, encryptionKeys, passwords, wildcard, encryptionKeyIDs, date, encryptionUserIDs, config); return armor ? message.armor(config) : message.write(); - - }).catch(onError.bind(null, 'Error encrypting session key')); + } catch (err) { + throw util.wrapError('Error encrypting session key', err); + } } /** @@ -537,16 +554,17 @@ export function encryptSessionKey({ data, algorithm, aeadAlgorithm, encryptionKe * @async * @static */ -export function decryptSessionKeys({ message, decryptionKeys, passwords, date = new Date(), config, ...rest }) { +export async function decryptSessionKeys({ message, decryptionKeys, passwords, date = new Date(), config, ...rest }) { config = { ...defaultConfig, ...config }; checkMessage(message); decryptionKeys = toArray(decryptionKeys); passwords = toArray(passwords); if (rest.privateKeys) throw new Error("The `privateKeys` option has been removed from openpgp.decryptSessionKeys, pass `decryptionKeys` instead"); - return Promise.resolve().then(async function() { - - return message.decryptSessionKeys(decryptionKeys, passwords, date, config); - - }).catch(onError.bind(null, 'Error decrypting session keys')); + try { + const sessionKeys = await message.decryptSessionKeys(decryptionKeys, passwords, date, config); + return sessionKeys; + } catch (err) { + throw util.wrapError('Error decrypting session keys', err); + } } @@ -662,22 +680,3 @@ async function prepareSignatures(signatures) { } })); } - - -/** - * Global error handler that logs the stack trace and rethrows a high lvl error message. - * @param {String} message - A human readable high level error Message - * @param {Error} error - The internal error that caused the failure - * @private - */ -function onError(message, error) { - // log the stack trace - util.printDebugError(error); - - // update error message - try { - error.message = message + ': ' + error.message; - } catch (e) {} - - throw error; -} diff --git a/test/general/openpgp.js b/test/general/openpgp.js index cd156a96..afb406ab 100644 --- a/test/general/openpgp.js +++ b/test/general/openpgp.js @@ -952,7 +952,7 @@ module.exports = () => describe('OpenPGP.js public api tests', function() { }); it('should throw if missing userIDs', async function() { - expect(() => openpgp.generateKey({})).to.throw(/UserIDs are required/); + await expect(openpgp.generateKey({})).to.be.rejectedWith(/UserIDs are required/); }); }); @@ -1126,6 +1126,22 @@ module.exports = () => describe('OpenPGP.js public api tests', function() { openpgp.config.minRSABits = minRSABitsVal; }); + it('Calling decrypt with encrypted key leads to exception', async function() { + const publicKey = await openpgp.readKey({ armoredKey: pub_key }); + const privateKey = await openpgp.readKey({ armoredKey: priv_key }); + + const encOpt = { + message: await openpgp.createMessage({ text: plaintext }), + encryptionKeys: publicKey + }; + const decOpt = { + decryptionKeys: privateKey + }; + const encrypted = await openpgp.encrypt(encOpt); + decOpt.message = await openpgp.readMessage({ armoredMessage: encrypted }); + await expect(openpgp.decrypt(decOpt)).to.be.rejectedWith('Error decrypting message: Decryption key is not decrypted.'); + }); + it('decrypt/verify should succeed with valid signature (expectSigned=true)', async function () { const publicKey = await openpgp.readKey({ armoredKey: pub_key }); const privateKey = await openpgp.decryptKey({ @@ -1517,6 +1533,12 @@ aOU= }); expect(sig).to.match(/-----END PGP MESSAGE-----\n$/); }); + + it('Calling sign with no signing key leads to exception', async function() { + await expect(openpgp.sign({ + message: await openpgp.createMessage({ text: plaintext }) + })).to.be.rejectedWith(/No signing keys provided/); + }); }); describe('encrypt, decrypt, sign, verify - integration tests', function() { @@ -1588,32 +1610,6 @@ aOU= } }); - it('Decrypting key with wrong passphrase should be rejected', async function () { - await expect(openpgp.decryptKey({ - privateKey: await openpgp.readKey({ armoredKey: priv_key }), - passphrase: 'wrong passphrase' - })).to.eventually.be.rejectedWith('Incorrect key passphrase'); - }); - - it('Can decrypt key with correct passphrase', async function () { - expect(privateKey.isDecrypted()).to.be.false; - const decryptedKey = await openpgp.decryptKey({ privateKey, passphrase }); - expect(decryptedKey.isDecrypted()).to.be.true; - }); - - it('Calling decrypt with not decrypted key leads to exception', async function() { - const encOpt = { - message: await openpgp.createMessage({ text: plaintext }), - encryptionKeys: publicKey - }; - const decOpt = { - decryptionKeys: privateKey - }; - const encrypted = await openpgp.encrypt(encOpt); - decOpt.message = await openpgp.readMessage({ armoredMessage: encrypted }); - await expect(openpgp.decrypt(decOpt)).to.be.rejectedWith('Error decrypting message: Decryption key is not decrypted.'); - }); - tryTests('CFB mode (asm.js)', tests, { if: true, beforeEach: function() {