From 94b12e566b4bb2f7e044bf287dd7b1a5b0614626 Mon Sep 17 00:00:00 2001 From: Sanjana Rajan Date: Wed, 14 Feb 2018 17:07:39 +0100 Subject: [PATCH 1/4] correctly handle cleartext headers with no hash specified --- src/cleartext.js | 11 ++++++++--- src/message.js | 1 - 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/cleartext.js b/src/cleartext.js index 9d827e0a..da64440c 100644 --- a/src/cleartext.js +++ b/src/cleartext.js @@ -29,6 +29,7 @@ import armor from './encoding/armor'; import enums from './enums'; import packet from './packet'; import { Signature } from './signature'; +import { getPreferredHashAlgo } from './key'; /** * @class @@ -93,8 +94,8 @@ CleartextMessage.prototype.signDetached = async function(privateKeys) { } const signaturePacket = new packet.Signature(); signaturePacket.signatureType = enums.signature.text; - signaturePacket.hashAlgorithm = config.prefer_hash_algorithm; signaturePacket.publicKeyAlgorithm = signingKeyPacket.algorithm; + signaturePacket.hashAlgorithm = getPreferredHashAlgo(privateKey); if (!signingKeyPacket.isDecrypted) { throw new Error('Private key is not decrypted.'); } @@ -164,8 +165,12 @@ CleartextMessage.prototype.getText = function() { * @return {String} ASCII armor */ CleartextMessage.prototype.armor = function() { + let hashes = this.signature.packets.map(function(packet) { + return enums.read(enums.hash, packet.hashAlgorithm).toUpperCase(); + }); + hashes = hashes.filter(function(item, i, ar) { return ar.indexOf(item) === i; }); const body = { - hash: enums.read(enums.hash, config.prefer_hash_algorithm).toUpperCase(), + hash: hashes.join(), text: this.text, data: this.signature.packets.write() }; @@ -233,7 +238,7 @@ function verifyHeaders(headers, packetlist) { if (!hashAlgos.length && !checkHashAlgos([enums.hash.md5])) { throw new Error('If no "Hash" header in cleartext signed message, then only MD5 signatures allowed'); - } else if (!checkHashAlgos(hashAlgos)) { + } else if (hashAlgos.length && !checkHashAlgos(hashAlgos)) { throw new Error('Hash algorithm mismatch in armor header and signature'); } } diff --git a/src/message.js b/src/message.js index e127cd22..9f00e584 100644 --- a/src/message.js +++ b/src/message.js @@ -404,7 +404,6 @@ Message.prototype.sign = async function(privateKeys=[], signature=null) { } const onePassSig = new packet.OnePassSignature(); onePassSig.type = signatureType; - //TODO get preferred hash algo from key signature onePassSig.hashAlgorithm = getPreferredHashAlgo(privateKey); onePassSig.publicKeyAlgorithm = signingKeyPacket.algorithm; onePassSig.signingKeyId = signingKeyPacket.getKeyId(); From 2ffd81553d5a790c85652411a7beb772492310a6 Mon Sep 17 00:00:00 2001 From: Sanjana Rajan Date: Wed, 14 Feb 2018 17:30:35 +0100 Subject: [PATCH 2/4] test multiple private key signing cleartext --- test/general/openpgp.js | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/general/openpgp.js b/test/general/openpgp.js index fb3f6118..476f0c11 100644 --- a/test/general/openpgp.js +++ b/test/general/openpgp.js @@ -1270,6 +1270,29 @@ describe('OpenPGP.js public api tests', function() { }); }); + it('should sign and verify cleartext data with multiple private keys', function () { + const privKeyDE = openpgp.key.readArmored(priv_key_de).keys[0]; + privKeyDE.decrypt(passphrase); + + const signOpt = { + data: plaintext, + privateKeys: [privateKey.keys[0], privKeyDE] + }; + const verifyOpt = { + publicKeys: publicKey.keys + }; + return openpgp.sign(signOpt).then(function (signed) { + expect(signed.data).to.match(/-----BEGIN PGP SIGNED MESSAGE-----/); + verifyOpt.message = openpgp.cleartext.readArmored(signed.data); + return openpgp.verify(verifyOpt); + }).then(function (verified) { + expect(verified.data).to.equal(plaintext); + expect(verified.signatures[0].valid).to.be.true; + expect(verified.signatures[0].keyid.toHex()).to.equal(privateKey.keys[0].getSigningKeyPacket().getKeyId().toHex()); + expect(verified.signatures[0].signature.packets.length).to.equal(1); + }); + }); + it('should sign and verify cleartext data with detached signatures', function () { const signOpt = { data: plaintext, From 38a11d7aaf537e4646e92b7e6a907c51b31e2efa Mon Sep 17 00:00:00 2001 From: Sanjana Rajan Date: Wed, 14 Feb 2018 17:55:54 +0100 Subject: [PATCH 3/4] reuse createVerificationObjects for cleartext --- src/cleartext.js | 25 +++---------------------- src/message.js | 2 +- 2 files changed, 4 insertions(+), 23 deletions(-) diff --git a/src/cleartext.js b/src/cleartext.js index da64440c..84252017 100644 --- a/src/cleartext.js +++ b/src/cleartext.js @@ -29,6 +29,7 @@ import armor from './encoding/armor'; import enums from './enums'; import packet from './packet'; import { Signature } from './signature'; +import { createVerificationObjects } from './message'; import { getPreferredHashAlgo } from './key'; /** @@ -82,6 +83,7 @@ CleartextMessage.prototype.signDetached = async function(privateKeys) { const packetlist = new packet.List(); const literalDataPacket = new packet.Literal(); literalDataPacket.setText(this.text); + await Promise.all(privateKeys.map(async function(privateKey) { if (privateKey.isPublic()) { throw new Error('Need private key for signing'); @@ -127,28 +129,7 @@ CleartextMessage.prototype.verifyDetached = function(signature, keys) { const literalDataPacket = new packet.Literal(); // we assume that cleartext signature is generated based on UTF8 cleartext literalDataPacket.setText(this.text); - return Promise.all(signatureList.map(async function(signature) { - let keyPacket = null; - await Promise.all(keys.map(async function(key) { - await key.verifyPrimaryUser(); - // Look for the unique key packet that matches issuerKeyId of signature - const result = key.getSigningKeyPacket(signature.issuerKeyId, config.verify_expired_keys); - if (result) { - keyPacket = result; - } - })); - - const verifiedSig = { - keyid: signature.issuerKeyId, - valid: keyPacket ? await signature.verify(keyPacket, literalDataPacket) : null - }; - - const packetlist = new packet.List(); - packetlist.push(signature); - verifiedSig.signature = new Signature(packetlist); - - return verifiedSig; - })); + return createVerificationObjects(signatureList, [literalDataPacket], keys); }; /** diff --git a/src/message.js b/src/message.js index 9f00e584..1fc0c96c 100644 --- a/src/message.js +++ b/src/message.js @@ -540,7 +540,7 @@ Message.prototype.verifyDetached = function(signature, keys) { * @param {Array} keys array of keys to verify signatures * @return {Array<({keyid: module:type/keyid, valid: Boolean})>} list of signer's keyid and validity of signature */ -async function createVerificationObjects(signatureList, literalDataList, keys) { +export async function createVerificationObjects(signatureList, literalDataList, keys) { return Promise.all(signatureList.map(async function(signature) { let keyPacket = null; await Promise.all(keys.map(async function(key) { From b5d19b6f8dfd06965edbc4c33eef00c6de51b458 Mon Sep 17 00:00:00 2001 From: Sanjana Rajan Date: Wed, 14 Feb 2018 18:23:50 +0100 Subject: [PATCH 4/4] pull out common signature code --- src/cleartext.js | 28 ++---------------------- src/message.js | 48 +++++++++++++++++++---------------------- test/general/openpgp.js | 5 ++++- 3 files changed, 28 insertions(+), 53 deletions(-) diff --git a/src/cleartext.js b/src/cleartext.js index 84252017..a3ce9895 100644 --- a/src/cleartext.js +++ b/src/cleartext.js @@ -29,7 +29,7 @@ import armor from './encoding/armor'; import enums from './enums'; import packet from './packet'; import { Signature } from './signature'; -import { createVerificationObjects } from './message'; +import { createVerificationObjects, createSignaturePackets } from './message'; import { getPreferredHashAlgo } from './key'; /** @@ -80,34 +80,10 @@ CleartextMessage.prototype.sign = async function(privateKeys) { * @return {module:signature~Signature} new detached signature of message content */ CleartextMessage.prototype.signDetached = async function(privateKeys) { - const packetlist = new packet.List(); const literalDataPacket = new packet.Literal(); literalDataPacket.setText(this.text); - await Promise.all(privateKeys.map(async function(privateKey) { - if (privateKey.isPublic()) { - throw new Error('Need private key for signing'); - } - await privateKey.verifyPrimaryUser(); - const signingKeyPacket = privateKey.getSigningKeyPacket(); - if (!signingKeyPacket) { - throw new Error('Could not find valid key packet for signing in key ' + - privateKey.primaryKey.getKeyId().toHex()); - } - const signaturePacket = new packet.Signature(); - signaturePacket.signatureType = enums.signature.text; - signaturePacket.publicKeyAlgorithm = signingKeyPacket.algorithm; - signaturePacket.hashAlgorithm = getPreferredHashAlgo(privateKey); - if (!signingKeyPacket.isDecrypted) { - throw new Error('Private key is not decrypted.'); - } - await signaturePacket.sign(signingKeyPacket, literalDataPacket); - return signaturePacket; - })).then(signatureList => { - signatureList.forEach(signaturePacket => packetlist.push(signaturePacket)); - }); - - return new Signature(packetlist); + return new Signature(await createSignaturePackets(literalDataPacket, privateKeys)); }; /** diff --git a/src/message.js b/src/message.js index 1fc0c96c..01838921 100644 --- a/src/message.js +++ b/src/message.js @@ -416,25 +416,7 @@ Message.prototype.sign = async function(privateKeys=[], signature=null) { }); packetlist.push(literalDataPacket); - - await Promise.all(privateKeys.map(async function(privateKey) { - const signaturePacket = new packet.Signature(); - const signingKeyPacket = privateKey.getSigningKeyPacket(); - if (!signingKeyPacket.isDecrypted) { - throw new Error('Private key is not decrypted.'); - } - signaturePacket.signatureType = signatureType; - signaturePacket.hashAlgorithm = getPreferredHashAlgo(privateKey); - signaturePacket.publicKeyAlgorithm = signingKeyPacket.algorithm; - await signaturePacket.sign(signingKeyPacket, literalDataPacket); - return signaturePacket; - })).then(signatureList => { - signatureList.forEach(signaturePacket => packetlist.push(signaturePacket)); - }); - - if (signature) { - packetlist.concat(existingSigPacketlist); - } + packetlist.concat(await createSignaturePackets(literalDataPacket, privateKeys, signature)); return new Message(packetlist); }; @@ -466,24 +448,40 @@ Message.prototype.compress = function(compression) { * @return {module:signature~Signature} new detached signature of message content */ Message.prototype.signDetached = async function(privateKeys=[], signature=null) { - const packetlist = new packet.List(); - const literalDataPacket = this.packets.findPacket(enums.packet.literal); if (!literalDataPacket) { throw new Error('No literal data packet to sign.'); } + return new Signature(await createSignaturePackets(literalDataPacket, privateKeys, signature)); +}; + +/** + * Create signature packets for the message + * @param {module:packet/literal} the literal data packet to sign + * @param {Array} privateKey private keys with decrypted secret key data for signing + * @param {Signature} signature (optional) any existing detached signature to append + * @return {module:packet/packetlist} list of signature packets + */ +export async function createSignaturePackets(literalDataPacket, privateKeys, signature=null) { + const packetlist = new packet.List(); const literalFormat = enums.write(enums.literal, literalDataPacket.format); const signatureType = literalFormat === enums.literal.binary ? enums.signature.binary : enums.signature.text; await Promise.all(privateKeys.map(async function(privateKey) { - const signaturePacket = new packet.Signature(); + if (privateKey.isPublic()) { + throw new Error('Need private key for signing'); + } await privateKey.verifyPrimaryUser(); const signingKeyPacket = privateKey.getSigningKeyPacket(); + if (!signingKeyPacket) { + throw new Error('Could not find valid key packet for signing in key ' + privateKey.primaryKey.getKeyId().toHex()); + } if (!signingKeyPacket.isDecrypted) { throw new Error('Private key is not decrypted.'); } + const signaturePacket = new packet.Signature(); signaturePacket.signatureType = signatureType; signaturePacket.publicKeyAlgorithm = signingKeyPacket.algorithm; signaturePacket.hashAlgorithm = getPreferredHashAlgo(privateKey); @@ -497,10 +495,8 @@ Message.prototype.signDetached = async function(privateKeys=[], signature=null) const existingSigPacketlist = signature.packets.filterByTag(enums.packet.signature); packetlist.concat(existingSigPacketlist); } - - return new Signature(packetlist); -}; - + return packetlist; +} /** * Verify message signatures diff --git a/test/general/openpgp.js b/test/general/openpgp.js index 476f0c11..e45f6c01 100644 --- a/test/general/openpgp.js +++ b/test/general/openpgp.js @@ -1279,7 +1279,7 @@ describe('OpenPGP.js public api tests', function() { privateKeys: [privateKey.keys[0], privKeyDE] }; const verifyOpt = { - publicKeys: publicKey.keys + publicKeys: [publicKey.keys[0], privKeyDE.toPublic()] }; return openpgp.sign(signOpt).then(function (signed) { expect(signed.data).to.match(/-----BEGIN PGP SIGNED MESSAGE-----/); @@ -1290,6 +1290,9 @@ describe('OpenPGP.js public api tests', function() { expect(verified.signatures[0].valid).to.be.true; expect(verified.signatures[0].keyid.toHex()).to.equal(privateKey.keys[0].getSigningKeyPacket().getKeyId().toHex()); expect(verified.signatures[0].signature.packets.length).to.equal(1); + expect(verified.signatures[1].valid).to.be.true; + expect(verified.signatures[1].keyid.toHex()).to.equal(privKeyDE.getSigningKeyPacket().getKeyId().toHex()); + expect(verified.signatures[1].signature.packets.length).to.equal(1); }); });