From 9f0f00e0878be24869e924c8fda7c6faa4c8c6e9 Mon Sep 17 00:00:00 2001 From: Daniel Huigens Date: Mon, 25 Jun 2018 20:33:38 +0200 Subject: [PATCH] Make signature.verified a Promise instead of result.signatures Also, fix verifying detached signatures --- src/message.js | 47 +++++++++++++++++++++----------- src/openpgp.js | 24 ++++++++-------- src/packet/clone.js | 30 ++++++++++---------- src/packet/one_pass_signature.js | 6 ++++ test/general/openpgp.js | 27 ++++++++---------- test/general/signature.js | 33 ++++++++++------------ test/general/streaming.js | 14 ++++++---- 7 files changed, 100 insertions(+), 81 deletions(-) diff --git a/src/message.js b/src/message.js index 124cedd5..3e13d94b 100644 --- a/src/message.js +++ b/src/message.js @@ -542,23 +542,36 @@ Message.prototype.verify = async function(keys, date=new Date(), asStream) { if (literalDataList.length !== 1) { throw new Error('Can only verify message with one literal data packet.'); } - if (msg.packets.stream) { - const onePassSigList = msg.packets.filterByTag(enums.packet.onePassSignature); + const onePassSigList = msg.packets.filterByTag(enums.packet.onePassSignature); + const signatureList = msg.packets.filterByTag(enums.packet.signature); + if (onePassSigList.length && !signatureList.length && msg.packets.stream) { onePassSigList.forEach(onePassSig => { - onePassSig.signatureData = stream.fromAsync(() => new Promise(resolve => { - onePassSig.signatureDataResolve = resolve; - })); + onePassSig.correspondingSig = new Promise(resolve => { + onePassSig.correspondingSigResolve = resolve; + }); + onePassSig.signatureData = stream.fromAsync(async () => (await onePassSig.correspondingSig).signatureData); onePassSig.hashed = onePassSig.hash(literalDataList[0], undefined, asStream); }); - return stream.transform(msg.packets.stream, signature => { - const onePassSig = onePassSigList.pop(); - onePassSig.signatureDataResolve(signature.signatureData); - signature.hashed = onePassSig.hashed; - signature.hashedData = onePassSig.hashedData; - return createVerificationObject(signature, literalDataList, keys, date); + const verificationObjects = await createVerificationObjects(onePassSigList, literalDataList, keys, date); + msg.packets.stream = stream.transformPair(msg.packets.stream, async (readable, writable) => { + const writer = stream.getWriter(writable); + try { + await stream.readToEnd(stream.transform(readable, signature => { + onePassSigList.pop().correspondingSigResolve(signature); + })); + await writer.ready; + await writer.close(); + } catch(e) { + onePassSigList.forEach(onePassSig => { + onePassSig.correspondingSigResolve({ + verify: () => undefined + }); + }); + await writer.abort(e); + } }); + return verificationObjects.reverse(); } - const signatureList = msg.packets.filterByTag(enums.packet.signature); return createVerificationObjects(signatureList, literalDataList, keys, date); }; @@ -603,12 +616,14 @@ async function createVerificationObject(signature, literalDataList, keys, date=n const verifiedSig = { keyid: signature.issuerKeyId, - valid: keyPacket ? await signature.verify(keyPacket, literalDataList[0]) : null + verified: keyPacket ? signature.verify(keyPacket, literalDataList[0]) : Promise.resolve(null) }; - const packetlist = new packet.List(); - packetlist.push(signature); - verifiedSig.signature = new Signature(packetlist); + verifiedSig.signature = Promise.resolve(signature.correspondingSig || signature).then(signature => { + const packetlist = new packet.List(); + packetlist.push(signature); + return new Signature(packetlist); + }); return verifiedSig; } diff --git a/src/openpgp.js b/src/openpgp.js index 1c89b616..7f876c70 100644 --- a/src/openpgp.js +++ b/src/openpgp.js @@ -364,26 +364,27 @@ export function decrypt({ message, privateKeys, passwords, sessionKeys, publicKe } const result = {}; - let signatures = signature ? decrypted.verifyDetached(signature, publicKeys, date) : decrypted.verify(publicKeys, date, asStream); + result.signatures = signature ? await decrypted.verifyDetached(signature, publicKeys, date) : await decrypted.verify(publicKeys, date, asStream); result.data = format === 'binary' ? decrypted.getLiteralData() : decrypted.getText(); result.data = await convertStream(result.data, asStream); - signatures = signatures.then(signatures => stream.readToEnd(signatures, arr => arr)); if (asStream) { - result.signatures = signatures.catch(() => []); result.data = stream.transformPair(message.packets.stream, async (readable, writable) => { await stream.pipe(result.data, writable, { preventClose: true }); const writer = stream.getWriter(writable); try { - await signatures; + await stream.readToEnd(decrypted.packets.stream, arr => arr); await writer.close(); } catch(e) { await writer.abort(e); } }); } else { - result.signatures = await signatures; + await Promise.all(result.signatures.map(async signature => { + signature.signature = await signature.signature; + signature.valid = await signature.verified; + })); } result.filename = decrypted.getFilename(); return result; @@ -466,26 +467,27 @@ export function verify({ message, publicKeys, asStream=message.fromStream, signa return Promise.resolve().then(async function() { const result = {}; - const signatures = signature ? await message.verifyDetached(signature, publicKeys, date) : await message.verify(publicKeys, date, asStream); + result.signatures = signature ? await message.verifyDetached(signature, publicKeys, date) : await message.verify(publicKeys, date, asStream); result.data = message instanceof CleartextMessage ? message.getText() : message.getLiteralData(); result.data = await convertStream(result.data, asStream); if (asStream) { - result.data = stream.transformPair(signatures, async (readable, writable) => { - const signatures = stream.readToEnd(readable, arr => arr); - result.signatures = signatures.catch(() => []); + result.data = stream.transformPair(message.packets.stream, async (readable, writable) => { await stream.pipe(result.data, writable, { preventClose: true }); const writer = stream.getWriter(writable); try { - await signatures; + await stream.readToEnd(readable, arr => arr); await writer.close(); } catch(e) { await writer.abort(e); } }); } else { - result.signatures = await stream.readToEnd(signatures, arr => arr); + await Promise.all(result.signatures.map(async signature => { + signature.signature = await signature.signature; + signature.valid = await signature.verified; + })); } return result; }).catch(onError.bind(null, 'Error verifying cleartext signed message')); diff --git a/src/packet/clone.js b/src/packet/clone.js index dad2136e..c768189e 100644 --- a/src/packet/clone.js +++ b/src/packet/clone.js @@ -69,18 +69,20 @@ export function clonePackets(options) { options.signature = options.signature.packets; } if (options.signatures) { - if (options.signatures instanceof Promise) { - const signatures = options.signatures; - options.signatures = stream.fromAsync(async () => (await signatures).map(verificationObjectToClone)); - } else { - options.signatures.forEach(verificationObjectToClone); - } + options.signatures.forEach(verificationObjectToClone); } return options; } function verificationObjectToClone(verObject) { - verObject.signature = verObject.signature.packets; + if (verObject.signature instanceof Promise) { + const signature = verObject.signature; + verObject.signature = stream.fromAsync(async () => (await signature).packets); + } else { + verObject.signature = verObject.signature.packets; + } + const verified = verObject.verified; + verObject.verified = stream.fromAsync(() => verified); return verObject; } @@ -116,11 +118,7 @@ export function parseClonedPackets(options) { options.message = packetlistCloneToMessage(options.message); } if (options.signatures) { - if (util.isStream(options.signatures)) { - options.signatures = stream.readToEnd(options.signatures, arr => arr).then(signatures => signatures.map(packetlistCloneToSignatures)); - } else { - options.signatures = options.signatures.map(packetlistCloneToSignatures); - } + options.signatures = options.signatures.map(packetlistCloneToSignatures); } if (options.signature) { options.signature = packetlistCloneToSignature(options.signature); @@ -146,8 +144,12 @@ function packetlistCloneToCleartextMessage(clone) { //verification objects function packetlistCloneToSignatures(clone) { clone.keyid = type_keyid.fromClone(clone.keyid); - const packetlist = List.fromStructuredClone(clone.signature); - clone.signature = new Signature(packetlist); + if (util.isStream(clone.signature)) { + clone.signature = stream.readToEnd(clone.signature, ([signature]) => new Signature(List.fromStructuredClone(signature))); + } else { + clone.signature = new Signature(List.fromStructuredClone(clone.signature)); + } + clone.verified = stream.readToEnd(clone.verified, ([verified]) => verified); return clone; } diff --git a/src/packet/one_pass_signature.js b/src/packet/one_pass_signature.js index 92a3cf9f..c72df02b 100644 --- a/src/packet/one_pass_signature.js +++ b/src/packet/one_pass_signature.js @@ -140,4 +140,10 @@ OnePassSignature.prototype.toHash = Signature.prototype.toHash; OnePassSignature.prototype.toSign = Signature.prototype.toSign; OnePassSignature.prototype.calculateTrailer = Signature.prototype.calculateTrailer; +OnePassSignature.prototype.verify = async function() { + const correspondingSig = await this.correspondingSig; + correspondingSig.hashed = this.hashed; + return correspondingSig.verify.apply(correspondingSig, arguments); +}; + export default OnePassSignature; diff --git a/test/general/openpgp.js b/test/general/openpgp.js index caa4ed58..28c81987 100644 --- a/test/general/openpgp.js +++ b/test/general/openpgp.js @@ -1827,14 +1827,13 @@ describe('OpenPGP.js public api tests', function() { const literals = packets.packets.filterByTag(openpgp.enums.packet.literal); expect(literals.length).to.equal(1); expect(+literals[0].date).to.equal(+past); - let signatures = await packets.verify(encryptOpt.publicKeys, past); + const signatures = await packets.verify(encryptOpt.publicKeys, past); expect(await openpgp.stream.readToEnd(packets.getText())).to.equal(plaintext); - signatures = await openpgp.stream.readToEnd(signatures, arr => arr); - expect(+signatures[0].signature.packets[0].created).to.equal(+past); - expect(signatures[0].valid).to.be.true; + expect(+(await signatures[0].signature).packets[0].created).to.equal(+past); + expect(await signatures[0].verified).to.be.true; expect(encryptOpt.privateKeys[0].getSigningKey(signatures[0].keyid, past)) .to.be.not.null; - expect(signatures[0].signature.packets.length).to.equal(1); + expect((await signatures[0].signature).packets.length).to.equal(1); }); }); @@ -1856,14 +1855,13 @@ describe('OpenPGP.js public api tests', function() { expect(literals.length).to.equal(1); expect(literals[0].format).to.equal('binary'); expect(+literals[0].date).to.equal(+future); - let signatures = await packets.verify(encryptOpt.publicKeys, future); + const signatures = await packets.verify(encryptOpt.publicKeys, future); expect(await openpgp.stream.readToEnd(packets.getLiteralData())).to.deep.equal(data); - signatures = await openpgp.stream.readToEnd(signatures, arr => arr); - expect(+signatures[0].signature.packets[0].created).to.equal(+future); - expect(signatures[0].valid).to.be.true; + expect(+(await signatures[0].signature).packets[0].created).to.equal(+future); + expect(await signatures[0].verified).to.be.true; expect(encryptOpt.privateKeys[0].getSigningKey(signatures[0].keyid, future)) .to.be.not.null; - expect(signatures[0].signature.packets.length).to.equal(1); + expect((await signatures[0].signature).packets.length).to.equal(1); }); }); @@ -1886,14 +1884,13 @@ describe('OpenPGP.js public api tests', function() { expect(literals.length).to.equal(1); expect(literals[0].format).to.equal('mime'); expect(+literals[0].date).to.equal(+future); - let signatures = await packets.verify(encryptOpt.publicKeys, future); + const signatures = await packets.verify(encryptOpt.publicKeys, future); expect(await openpgp.stream.readToEnd(packets.getLiteralData())).to.deep.equal(data); - signatures = await openpgp.stream.readToEnd(signatures, arr => arr); - expect(+signatures[0].signature.packets[0].created).to.equal(+future); - expect(signatures[0].valid).to.be.true; + expect(+(await signatures[0].signature).packets[0].created).to.equal(+future); + expect(await signatures[0].verified).to.be.true; expect(encryptOpt.privateKeys[0].getSigningKey(signatures[0].keyid, future)) .to.be.not.null; - expect(signatures[0].signature.packets.length).to.equal(1); + expect((await signatures[0].signature).packets.length).to.equal(1); }); }); diff --git a/test/general/signature.js b/test/general/signature.js index 9513ec8a..ac5b00b3 100644 --- a/test/general/signature.js +++ b/test/general/signature.js @@ -344,11 +344,10 @@ describe("Signature", function() { return msg.decrypt([priv_key_gnupg_ext]).then(function(msg) { return msg.verify([pub_key]).then(async verified => { openpgp.stream.pipe(msg.getLiteralData(), new WritableStream()); - verified = await openpgp.stream.readToEnd(verified, arr => arr); expect(verified).to.exist; expect(verified).to.have.length(1); - expect(verified[0].valid).to.be.true; - expect(verified[0].signature.packets.length).to.equal(1); + expect(await verified[0].verified).to.be.true; + expect((await verified[0].signature).packets.length).to.equal(1); }); }); }); @@ -370,11 +369,10 @@ describe("Signature", function() { const pub_key = (await openpgp.key.readArmored(pub_key_arm2)).keys[0]; return sMsg.verify([pub_key]).then(async verified => { openpgp.stream.pipe(sMsg.getLiteralData(), new WritableStream()); - verified = await openpgp.stream.readToEnd(verified, arr => arr); expect(verified).to.exist; expect(verified).to.have.length(1); - expect(verified[0].valid).to.be.true; - expect(verified[0].signature.packets.length).to.equal(1); + expect(await verified[0].verified).to.be.true; + expect((await verified[0].signature).packets.length).to.equal(1); }); }); @@ -442,13 +440,12 @@ describe("Signature", function() { return sMsg.verify([pubKey2, pubKey3]).then(async verifiedSig => { expect(await openpgp.stream.readToEnd(sMsg.getText())).to.equal(plaintext); - verifiedSig = await openpgp.stream.readToEnd(verifiedSig, arr => arr); expect(verifiedSig).to.exist; expect(verifiedSig).to.have.length(2); - expect(verifiedSig[0].valid).to.be.true; - expect(verifiedSig[1].valid).to.be.true; - expect(verifiedSig[0].signature.packets.length).to.equal(1); - expect(verifiedSig[1].signature.packets.length).to.equal(1); + expect(await verifiedSig[0].verified).to.be.true; + expect(await verifiedSig[1].verified).to.be.true; + expect((await verifiedSig[0].signature).packets.length).to.equal(1); + expect((await verifiedSig[1].signature).packets.length).to.equal(1); }); }); @@ -597,10 +594,9 @@ yYDnCgA= return openpgp.verify({ publicKeys:[pubKey], message:sMsg }).then(async function(cleartextSig) { expect(cleartextSig).to.exist; expect(openpgp.util.nativeEOL(openpgp.util.Uint8Array_to_str(await openpgp.stream.readToEnd(cleartextSig.data)))).to.equal(plaintext); - cleartextSig.signatures = await openpgp.stream.readToEnd(cleartextSig.signatures, arr => arr); expect(cleartextSig.signatures).to.have.length(1); - expect(cleartextSig.signatures[0].valid).to.be.true; - expect(cleartextSig.signatures[0].signature.packets.length).to.equal(1); + expect(await cleartextSig.signatures[0].verified).to.be.true; + expect((await cleartextSig.signatures[0].signature).packets.length).to.equal(1); }); }); @@ -886,8 +882,7 @@ yYDnCgA= await msg.appendSignature(detachedSig); return msg.verify(publicKeys).then(async result => { openpgp.stream.pipe(msg.getLiteralData(), new WritableStream()); - result = await openpgp.stream.readToEnd(result, arr => arr); - expect(result[0].valid).to.be.true; + expect(await result[0].verified).to.be.true; }); }); @@ -902,9 +897,9 @@ yYDnCgA= return openpgp.generateKey(opt).then(function(gen) { const generatedKey = gen.key; return msg.signDetached([generatedKey, privKey2]).then(detachedSig => { - return msg.verifyDetached(detachedSig, [generatedKey.toPublic(), pubKey2]).then(result => { - expect(result[0].valid).to.be.true; - expect(result[1].valid).to.be.true; + return msg.verifyDetached(detachedSig, [generatedKey.toPublic(), pubKey2]).then(async result => { + expect(await result[0].verified).to.be.true; + expect(await result[1].verified).to.be.true; }); }); }); diff --git a/test/general/streaming.js b/test/general/streaming.js index 5e3e0685..c7a9703d 100644 --- a/test/general/streaming.js +++ b/test/general/streaming.js @@ -260,7 +260,7 @@ describe('Streaming', function() { expect(await openpgp.stream.getReader(openpgp.stream.clone(decrypted.data)).readBytes(1024)).to.deep.equal(plaintext[0]); if (i > 10) throw new Error('Data did not arrive early.'); expect(await openpgp.stream.readToEnd(decrypted.data)).to.deep.equal(util.concatUint8Array(plaintext)); - expect(await decrypted.signatures).to.exist.and.have.length(0); + expect(decrypted.signatures).to.exist.and.have.length(0); } finally { openpgp.config.unsafe_stream = unsafe_streamValue; } @@ -349,7 +349,7 @@ describe('Streaming', function() { expect(await openpgp.stream.getReader(openpgp.stream.clone(decrypted.data)).readBytes(1024)).not.to.deep.equal(plaintext[0]); if (i > 10) throw new Error('Data did not arrive early.'); await expect(openpgp.stream.readToEnd(decrypted.data)).to.be.rejectedWith('Modification detected.'); - await decrypted.signatures; + expect(decrypted.signatures).to.exist.and.have.length(0); } finally { openpgp.config.unsafe_stream = unsafe_streamValue; } @@ -398,7 +398,7 @@ describe('Streaming', function() { expect(await openpgp.stream.getReader(openpgp.stream.clone(decrypted.data)).readBytes(10)).not.to.deep.equal(plaintext[0]); if (i > 10) throw new Error('Data did not arrive early.'); await expect(openpgp.stream.readToEnd(decrypted.data)).to.be.rejectedWith('Ascii armor integrity check on message failed'); - expect(await decrypted.signatures).to.exist.and.have.length(0); + expect(decrypted.signatures).to.exist.and.have.length(1); } finally { openpgp.config.unsafe_stream = unsafe_streamValue; } @@ -446,7 +446,8 @@ describe('Streaming', function() { expect(await openpgp.stream.getReader(openpgp.stream.clone(decrypted.data)).readBytes(10)).not.to.deep.equal(plaintext[0]); if (i > 10) throw new Error('Data did not arrive early.'); await expect(openpgp.stream.readToEnd(decrypted.data)).to.be.rejectedWith('Ascii armor integrity check on message failed'); - expect(await decrypted.signatures).to.exist.and.have.length(0); + expect(decrypted.signatures).to.exist.and.have.length(1); + expect(await decrypted.signatures[0].verified).to.be.null; } finally { openpgp.config.unsafe_stream = unsafe_streamValue; } @@ -492,7 +493,7 @@ describe('Streaming', function() { expect(await openpgp.stream.getReader(openpgp.stream.clone(decrypted.data)).readBytes(10)).not.to.deep.equal(plaintext[0]); if (i > 10) throw new Error('Data did not arrive early.'); await expect(openpgp.stream.readToEnd(decrypted.data)).to.be.rejectedWith('Ascii armor integrity check on message failed'); - expect(await decrypted.signatures).to.exist.and.have.length(0); + expect(decrypted.signatures).to.exist.and.have.length(1); } finally { openpgp.config.unsafe_stream = unsafe_streamValue; } @@ -677,7 +678,8 @@ describe('Streaming', function() { reader.releaseLock(); await openpgp.stream.cancel(decrypted.data, new Error('canceled by test')); expect(canceled).to.be.true; - expect(await decrypted.signatures).to.exist.and.have.length(0); + expect(decrypted.signatures).to.exist.and.have.length(1); + expect(await decrypted.signatures[0].verified).to.be.undefined; } finally { openpgp.config.aead_protect = aead_protectValue; openpgp.config.aead_chunk_size_byte = aead_chunk_size_byteValue;