Merge pull request #644 from openpgpjs/cleartext_hash

Cleartext hashing fixes

src/cleartext.js

@@ -29,6 +29,8 @@ import armor from './encoding/armor';
 import enums from './enums';
 import packet from './packet';
 import { Signature } from './signature';
+import { createVerificationObjects, createSignaturePackets } from './message';
+import { getPreferredHashAlgo } from './key';
 
 /**
  * @class
@@ -78,33 +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.hashAlgorithm = config.prefer_hash_algorithm;
-    signaturePacket.publicKeyAlgorithm = signingKeyPacket.algorithm;
-    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));
 };
 
 /**
@@ -126,28 +105,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) {
+  return createVerificationObjects(signatureList, [literalDataPacket], keys);
-    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;
-  }));
 };
 
 /**
@@ -164,8 +122,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 +195,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');
   }
 }

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();
@@ -417,25 +416,7 @@ Message.prototype.sign = async function(privateKeys=[], signature=null) {
   });
 
   packetlist.push(literalDataPacket);
-
+  packetlist.concat(await createSignaturePackets(literalDataPacket, privateKeys, signature));
-  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);
-  }
 
   return new Message(packetlist);
 };
@@ -467,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<module:key~Key>}           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);
@@ -498,10 +495,8 @@ Message.prototype.signDetached = async function(privateKeys=[], signature=null)
     const existingSigPacketlist = signature.packets.filterByTag(enums.packet.signature);
     packetlist.concat(existingSigPacketlist);
   }
-
+  return packetlist;
-  return new Signature(packetlist);
+}
-};
-
 
 /**
  * Verify message signatures
@@ -541,7 +536,7 @@ Message.prototype.verifyDetached = function(signature, keys) {
  * @param {Array<module:key~Key>} 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) {

test/general/openpgp.js

@@ -1270,6 +1270,32 @@ 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[0], privKeyDE.toPublic()]
+            };
+            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);
+              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);
+            });
+          });
+
           it('should sign and verify cleartext data with detached signatures', function () {
             const signOpt = {
               data: plaintext,