Merge pull request #797 from twiss/security-fixes

Security fixes

src/config/config.js

@@ -82,7 +82,7 @@ export default {
    * @memberof module:config
    * @property {Integer} s2k_iteration_count_byte
    */
-  s2k_iteration_count_byte: 96,
+  s2k_iteration_count_byte: 224,
   /** Use integrity protection for symmetric encryption
    * @memberof module:config
    * @property {Boolean} integrity_protect

src/key.js

@@ -301,7 +301,12 @@ Key.prototype.getSigningKey = async function (keyId=null, date=new Date(), userI
         if (await subKeys[i].verify(primaryKey, date) === enums.keyStatus.valid) {
           const dataToVerify = { key: primaryKey, bind: subKeys[i].keyPacket };
           const bindingSignature = await getLatestValidSignature(subKeys[i].bindingSignatures, primaryKey, dataToVerify, date);
-          if (bindingSignature && isValidSigningKeyPacket(subKeys[i].keyPacket, bindingSignature)) {
+          if (
+            bindingSignature &&
+            bindingSignature.embeddedSignature &&
+            isValidSigningKeyPacket(subKeys[i].keyPacket, bindingSignature) &&
+            await getLatestValidSignature([bindingSignature.embeddedSignature], subKeys[i].keyPacket, dataToVerify, date)
+          ) {
             return subKeys[i];
           }
         }
@@ -1235,8 +1240,7 @@ export async function read(data) {
     if (packetlist.filterByTag(enums.packet.signature).some(
       signature => signature.revocationKeyClass !== null
     )) {
-      // Indicate an error, but still parse the key.
+      throw new Error('This key is intended to be revoked with an authorized key, which OpenPGP.js does not support.');
-      err.push(new Error('This key is intended to be revoked with an authorized key, which OpenPGP.js does not support.'));
     }
     const keyIndex = packetlist.indexOfTag(enums.packet.publicKey, enums.packet.secretKey);
     if (keyIndex.length === 0) {
@@ -1515,7 +1519,14 @@ async function wrapKeyObject(secretKeyPacket, secretSubkeyPackets, options) {
     subkeySignaturePacket.signatureType = enums.signature.subkey_binding;
     subkeySignaturePacket.publicKeyAlgorithm = secretKeyPacket.algorithm;
     subkeySignaturePacket.hashAlgorithm = await getPreferredHashAlgo(null, secretSubkeyPacket);
-    subkeySignaturePacket.keyFlags = subkeyOptions.sign ? enums.keyFlags.sign_data : [enums.keyFlags.encrypt_communication | enums.keyFlags.encrypt_storage];
+    if (subkeyOptions.sign) {
+      subkeySignaturePacket.keyFlags = [enums.keyFlags.sign_data];
+      subkeySignaturePacket.embeddedSignature = await createSignaturePacket(dataToSign, null, secretSubkeyPacket, {
+        signatureType: enums.signature.key_binding
+      }, subkeyOptions.date);
+    } else {
+      subkeySignaturePacket.keyFlags = [enums.keyFlags.encrypt_communication | enums.keyFlags.encrypt_storage];
+    }
     if (subkeyOptions.keyExpirationTime > 0) {
       subkeySignaturePacket.keyExpirationTime = subkeyOptions.keyExpirationTime;
       subkeySignaturePacket.keyNeverExpires = false;

src/message.js

@@ -656,7 +656,9 @@ async function createVerificationObject(signature, literalDataList, keys, date=n
  * @async
  */
 export async function createVerificationObjects(signatureList, literalDataList, keys, date=new Date()) {
-  return Promise.all(signatureList.map(async function(signature) {
+  return Promise.all(signatureList.filter(function(signature) {
+    return ['text', 'binary'].includes(enums.read(enums.signature, signature.signatureType));
+  }).map(async function(signature) {
     return createVerificationObject(signature, literalDataList, keys, date);
   }));
 }

src/packet/signature.js

@@ -52,7 +52,7 @@ function Signature(date=new Date()) {
   this.publicKeyAlgorithm = null;
 
   this.signatureData = null;
-  this.unhashedSubpackets = null;
+  this.unhashedSubpackets = [];
   this.signedHashValue = null;
 
   this.created = util.normalizeDate(date);
@@ -109,31 +109,12 @@ Signature.prototype.read = function (bytes) {
     throw new Error('Version ' + this.version + ' of the signature is unsupported.');
   }
 
-  const subpackets = bytes => {
-    // Two-octet scalar octet count for following subpacket data.
-    const subpacket_length = util.readNumber(bytes.subarray(0, 2));
-
-    let i = 2;
-
-    // subpacket data set (zero or more subpackets)
-    while (i < 2 + subpacket_length) {
-      const len = packet.readSimpleLength(bytes.subarray(i, bytes.length));
-      i += len.offset;
-
-      this.read_sub_packet(bytes.subarray(i, i + len.len));
-
-      i += len.len;
-    }
-
-    return i;
-  };
-
   this.signatureType = bytes[i++];
   this.publicKeyAlgorithm = bytes[i++];
   this.hashAlgorithm = bytes[i++];
 
   // hashed subpackets
-  i += subpackets(bytes.subarray(i, bytes.length), true);
+  i += this.read_sub_packets(bytes.subarray(i, bytes.length), true);
 
   // A V4 signature hashes the packet body
   // starting from its first field, the version number, through the end
@@ -142,11 +123,9 @@ Signature.prototype.read = function (bytes) {
   // hash algorithm, the hashed subpacket length, and the hashed
   // subpacket body.
   this.signatureData = bytes.subarray(0, i);
-  const sigDataLength = i;
 
   // unhashed subpackets
-  i += subpackets(bytes.subarray(i, bytes.length), false);
+  i += this.read_sub_packets(bytes.subarray(i, bytes.length), false);
-  this.unhashedSubpackets = bytes.subarray(sigDataLength, i);
 
   // Two-octet field holding left 16 bits of signed hash value.
   this.signedHashValue = bytes.subarray(i, i + 2);
@@ -158,7 +137,7 @@ Signature.prototype.read = function (bytes) {
 Signature.prototype.write = function () {
   const arr = [];
   arr.push(this.signatureData);
-  arr.push(this.unhashedSubpackets ? this.unhashedSubpackets : util.writeNumber(0, 2));
+  arr.push(this.write_unhashed_sub_packets());
   arr.push(this.signedHashValue);
   arr.push(stream.clone(this.signature));
   return util.concat(arr);
@@ -188,7 +167,7 @@ Signature.prototype.sign = async function (key, data) {
   this.issuerKeyId = key.getKeyId();
 
   // Add hashed subpackets
-  arr.push(this.write_all_sub_packets());
+  arr.push(this.write_hashed_sub_packets());
 
   this.signatureData = util.concat(arr);
 
@@ -211,10 +190,10 @@ Signature.prototype.sign = async function (key, data) {
 };
 
 /**
- * Creates string of bytes with all subpacket data
+ * Creates Uint8Array of bytes of all subpacket data except Issuer and Embedded Signature subpackets
- * @returns {String} a string-representation of a all subpacket data
+ * @returns {Uint8Array} subpacket data
  */
-Signature.prototype.write_all_sub_packets = function () {
+Signature.prototype.write_hashed_sub_packets = function () {
   const sub = enums.signatureSubpacket;
   const arr = [];
   let bytes;
@@ -249,11 +228,6 @@ Signature.prototype.write_all_sub_packets = function () {
     bytes = util.concat([bytes, this.revocationKeyFingerprint]);
     arr.push(write_sub_packet(sub.revocation_key, bytes));
   }
-  if (!this.issuerKeyId.isNull() && this.issuerKeyVersion !== 5) {
-    // If the version of [the] key is greater than 4, this subpacket
-    // MUST NOT be included in the signature.
-    arr.push(write_sub_packet(sub.issuer, this.issuerKeyId.write()));
-  }
   if (this.notation !== null) {
     Object.entries(this.notation).forEach(([name, value]) => {
       bytes = [new Uint8Array([0x80, 0, 0, 0])];
@@ -308,6 +282,30 @@ Signature.prototype.write_all_sub_packets = function () {
     bytes = util.concat(bytes);
     arr.push(write_sub_packet(sub.signature_target, bytes));
   }
+  if (this.preferredAeadAlgorithms !== null) {
+    bytes = util.str_to_Uint8Array(util.Uint8Array_to_str(this.preferredAeadAlgorithms));
+    arr.push(write_sub_packet(sub.preferred_aead_algorithms, bytes));
+  }
+
+  const result = util.concat(arr);
+  const length = util.writeNumber(result.length, 2);
+
+  return util.concat([length, result]);
+};
+
+/**
+ * Creates Uint8Array of bytes of Issuer and Embedded Signature subpackets
+ * @returns {Uint8Array} subpacket data
+ */
+Signature.prototype.write_unhashed_sub_packets = function() {
+  const sub = enums.signatureSubpacket;
+  const arr = [];
+  let bytes;
+  if (!this.issuerKeyId.isNull() && this.issuerKeyVersion !== 5) {
+    // If the version of [the] key is greater than 4, this subpacket
+    // MUST NOT be included in the signature.
+    arr.push(write_sub_packet(sub.issuer, this.issuerKeyId.write()));
+  }
   if (this.embeddedSignature !== null) {
     arr.push(write_sub_packet(sub.embedded_signature, this.embeddedSignature.write()));
   }
@@ -316,10 +314,10 @@ Signature.prototype.write_all_sub_packets = function () {
     bytes = util.concat(bytes);
     arr.push(write_sub_packet(sub.issuer_fingerprint, bytes));
   }
-  if (this.preferredAeadAlgorithms !== null) {
+  this.unhashedSubpackets.forEach(data => {
-    bytes = util.str_to_Uint8Array(util.Uint8Array_to_str(this.preferredAeadAlgorithms));
+    arr.push(packet.writeSimpleLength(data.length));
-    arr.push(write_sub_packet(sub.preferred_aead_algorithms, bytes));
+    arr.push(data);
-  }
+  });
 
   const result = util.concat(arr);
   const length = util.writeNumber(result.length, 2);
@@ -346,7 +344,7 @@ function write_sub_packet(type, data) {
 
 // V4 signature sub packets
 
-Signature.prototype.read_sub_packet = function (bytes) {
+Signature.prototype.read_sub_packet = function (bytes, trusted=true) {
   let mypos = 0;
 
   const read_array = (prop, bytes) => {
@@ -357,9 +355,23 @@ Signature.prototype.read_sub_packet = function (bytes) {
     }
   };
 
-  // The leftwost bit denotes a "critical" packet, but we ignore it.
+  // The leftmost bit denotes a "critical" packet
-  const type = bytes[mypos++] & 0x7F;
+  const critical = bytes[mypos] & 0x80;
-  let seconds;
+  const type = bytes[mypos] & 0x7F;
+
+  // GPG puts the Issuer and Signature subpackets in the unhashed area.
+  // Tampering with those invalidates the signature, so we can trust them.
+  // Ignore all other unhashed subpackets.
+  if (!trusted && ![
+    enums.signatureSubpacket.issuer,
+    enums.signatureSubpacket.issuer_fingerprint,
+    enums.signatureSubpacket.embedded_signature
+  ].includes(type)) {
+    this.unhashedSubpackets.push(bytes.subarray(mypos, bytes.length));
+    return;
+  }
+
+  mypos++;
 
   // subpacket type
   switch (type) {
@@ -367,14 +379,15 @@ Signature.prototype.read_sub_packet = function (bytes) {
       // Signature Creation Time
       this.created = util.readDate(bytes.subarray(mypos, bytes.length));
       break;
-    case 3:
+    case 3: {
       // Signature Expiration Time in seconds
-      seconds = util.readNumber(bytes.subarray(mypos, bytes.length));
+      const seconds = util.readNumber(bytes.subarray(mypos, bytes.length));
 
       this.signatureNeverExpires = seconds === 0;
       this.signatureExpirationTime = seconds;
 
       break;
+    }
     case 4:
       // Exportable Certification
       this.exportable = bytes[mypos++] === 1;
@@ -392,14 +405,15 @@ Signature.prototype.read_sub_packet = function (bytes) {
       // Revocable
       this.revocable = bytes[mypos++] === 1;
       break;
-    case 9:
+    case 9: {
       // Key Expiration Time in seconds
-      seconds = util.readNumber(bytes.subarray(mypos, bytes.length));
+      const seconds = util.readNumber(bytes.subarray(mypos, bytes.length));
 
       this.keyExpirationTime = seconds;
       this.keyNeverExpires = seconds === 0;
 
       break;
+    }
     case 11:
       // Preferred Symmetric Algorithms
       read_array('preferredSymmetricAlgorithms', bytes.subarray(mypos, bytes.length));
@@ -510,9 +524,34 @@ Signature.prototype.read_sub_packet = function (bytes) {
       // Preferred AEAD Algorithms
       read_array.call(this, 'preferredAeadAlgorithms', bytes.subarray(mypos, bytes.length));
       break;
-    default:
+    default: {
-      util.print_debug("Unknown signature subpacket type " + type + " @:" + mypos);
+      const err = new Error("Unknown signature subpacket type " + type + " @:" + mypos);
+      if (critical) {
+        throw err;
+      } else {
+        util.print_debug(err);
       }
+    }
+  }
+};
+
+Signature.prototype.read_sub_packets = function(bytes, trusted=true) {
+  // Two-octet scalar octet count for following subpacket data.
+  const subpacket_length = util.readNumber(bytes.subarray(0, 2));
+
+  let i = 2;
+
+  // subpacket data set (zero or more subpackets)
+  while (i < 2 + subpacket_length) {
+    const len = packet.readSimpleLength(bytes.subarray(i, bytes.length));
+    i += len.offset;
+
+    this.read_sub_packet(bytes.subarray(i, i + len.len), trusted);
+
+    i += len.len;
+  }
+
+  return i;
 };
 
 // Produces data to produce signature on

test/general/armor.js

@@ -392,9 +392,10 @@ NJCB6+LWtabSoVIjNVgKwyKqyTLaESNwC2ogZwkdE8qPGiDFEHo4Gg9zuRof
 -----END PGP PUBLIC KEY BLOCK-----
 `;
 
-    const {keys: [key]} = await openpgp.key.readArmored(pubKey);
+    const { type, data } = await openpgp.armor.decode(pubKey);
+    const armor = await openpgp.stream.readToEnd(openpgp.armor.encode(type, data));
     expect(
-      key.armor()
+      armor
         .replace(/^(Version|Comment): .*$\r\n/mg, '')
     ).to.equal(
       pubKey

test/general/key.js

@@ -1798,8 +1798,7 @@ describe('Key', function() {
     expect(pubKeys).to.exist;
     expect(pubKeys.err).to.exist.and.have.length(1);
     expect(pubKeys.err[0].message).to.equal('This key is intended to be revoked with an authorized key, which OpenPGP.js does not support.');
-    expect(pubKeys.keys).to.have.length(1);
+    expect(pubKeys.keys).to.have.length(0);
-    expect(pubKeys.keys[0].getKeyId().toHex()).to.equal('5880a2fd178372b9');
   });
 
   it('Parsing V5 public key packet', async function() {

test/security/index.js

@@ -0,0 +1,5 @@
+describe('Security', function () {
+  require('./message_signature_bypass');
+  require('./unsigned_subpackets');
+  require('./subkey_trust');
+});

test/security/message_signature_bypass.js

@@ -0,0 +1,112 @@
+const openpgp = typeof window !== 'undefined' && window.openpgp ? window.openpgp : require('../../dist/openpgp');
+
+const { key, cleartext, util, packet: { Signature } } = openpgp;
+
+const chai = require('chai');
+chai.use(require('chai-as-promised'));
+
+const expect = chai.expect;
+
+/**
+* public key of another user.
+*/
+const OTHERPUBKEY = `
+-----BEGIN PGP PUBLIC KEY BLOCK-----
+Version: OpenPGP.js VERSION
+Comment: https://openpgpjs.org
+
+xsBNBFuqNY0BCADFUCnl03vimEQRs7mtDIp0g6tItuguhJJu1/QjXwTXUHZg
+pZosOPkGOR1EubydjYz4kvAnZ5r9cWA4xQ96rBdvj/kIaP+oJKLB1jXwh4Ft
++8YT4mVU2yWLu7U2p4tSyRoM5VCDEqG64OcbZMwEdDKf8t6JTjYTtEfPfW5R
+4hy8NjPYOx0Jw8MG+U0aP4WA1xsMXFP/VWF1IseEcVIWKs/VroJc5Xe80QDN
+hRtKTRVJV/wTnkao2MLcq/hgOfhO28NjnxVlX06O/XTWdElA7CCi1Zg1/BZ+
+r2XuuE1J2DjERfTokFzkKnMlGK9zXn0LxPnAJAIfu33/SFuAZcVu4UEJABEB
+AAHNHVRlc3RpIFRlc3QgPHRlc3RAZXhhbXBsZS5jb20+wsB1BBABCAApBQJb
+qjWcBgsJBwgDAgkQVSCLLRis484EFQgKAgMWAgECGQECGwMCHgEAAGNXB/4g
+DX082p83RfMmBv8hRN1V9ruPAvlxDWNBHb5dc1Y67yrBXOLMtaSauSZKrbf1
+moPDHT2eoLl7cV3BQbXWp+hiMZ4W53ZFJt26Kwwwf1yVRAZME7VRNwqW0aJv
+FKgCq7XTgJ61UYNhc31bLH0eVcfCkAExfwqZlwTWRzRSCqr0NL0XZVakJE6F
+al1Y+uN7CEr0/vbc6uSuo0hyZwxAw+Iynd5cO9PRXSssAm4IaulSnYUd96r2
+l8jsa+p6ooBYPotnLQ9fdd457JMoc8jDHf4m+P9/ZiWpycCB0DgUtNw1wH2T
+DHYf+2lfGGoA3osuHeJJfZfJujbKW5L7ZMNJ23tSzsBNBFuqNY0BB/9XKYzS
+PdHC/dXoBC9un3YLCcUX6LMNnaQMryVONYKFE1Rt0/si9XtnIDqyBrTr3LRi
+D+GIR+b7zCXOGkvmjztblD2P3SweCudPIbVLxePI+SfyjRs9EsMOrEPymN7U
+u/CU7jefvNBKvvMHi1m1Ibqg/A+ZheqJ+xBjSQM88dWsY/XB/jh7PGAM0QEu
+ezafNwlUUNnXyYRuC3P4h66OIJcDPcfaao3uAuJ/C81E8ttuws3c08kudd/A
+szIGpPtxAakimiWVHa0ceKi3exXXjRDrufroPcV3+Gbn4J8NqcUPRhB3L3CD
+rCivRme8qGEYh+ADPLy88SytdtCr+6W4hiQVABEBAAHCwF8EGAEIABMFAluq
+NZ0JEFUgiy0YrOPOAhsMAABebggAxANqkwS/Ag3NQLUu/wNZMMifZAxpFIWo
+CQQrCOU94OSsUKz8Q11yoOvsQN3T4CSL8dG5DbIucnHsx39jVeTniG6P3p9f
+NE/lq7RtLnXjVgGYpPNNUbLcOfXaCDhmS4GEunwTsVlsmEqyfLniKLG8to5Q
+6f/wGPJRvYB8rgLfVGV3DCvILg/CMzkceM9ia6jDQeHHwnoFVXnlsRAgQefJ
+rT5hVim4Wzg/5Lxt9Efry0k1ZhT0kondF1qNMv0wKxIJ+/gDNT2ZP4RIr1Kl
+eu6a8CH841yfF0+r5RV9xOky0jxwGgcxT29c8DBoawjXu6TtJ/SP8UrscttA
+bVLYdBmWLw==
+=nMyV
+-----END PGP PUBLIC KEY BLOCK-----`;
+
+/**
+* an original unmodified message as a template.
+*/
+const ORIGINAL = `
+-----BEGIN PGP SIGNED MESSAGE-----
+Hash: SHA256
+
+You owe me € 10
+-----BEGIN PGP SIGNATURE-----
+Version: OpenPGP.js VERSION
+Comment: https://openpgpjs.org
+
+wsBcBAEBCAAQBQJbq0iICRBVIIstGKzjzgAAeV0H/3ZxWuEV+2PNXHR+PdxX
+WRxjk6Zu+jjpb/iRS8IynRoe3iDaai3+iiAHM1GsHvOIBVJU6Bjx1ZyyEI0a
+dDg/yj3LBqBW9U3AiGpsXPfuyLKYIHfPbrygEleRIQKh7+iwNmn9ScVvzJrl
+hUurlZxx1mWbERAchwsrcZpwFCdfjJ/C9sblTxgnsm1YlYZNkf95DFtRnVO5
+prUuOjqJ0bA7bxg5GA4FQskRPIQ0ioZ6DyDi2IU3rdVEOs2Pc8S0EsD9K7af
+vO5oXKiJsyUN5EXEI8kYRulP1l0kvEWVTlnY2ek1qS637RkBI+DHLcXV5Hcu
+fhGyl7nA7UCwgsqf7ZPBhRg=
+=nbjQ
+-----END PGP SIGNATURE-----`;
+async function getOtherPubKey() {
+  return (await key.readArmored(OTHERPUBKEY)).keys[0];
+}
+
+/**
+* The "standalone" signature signed by the victim.
+*/
+const STANDALONE_PKT = util.b64_to_Uint8Array(`
+BAIBCAAQBQJbq3MKCRBVIIstGKzjzgAAWdoIALgj7OuhuuAWr6WEvGfvkx3e
+Fn/mg76lh2Hawxq6ryI6+kzUH+YJsG94CfLgGuh5LghZFBnlkdZS11gK87fN
++ifmPdSDj8fsKqSFdX1sHGwzvzBcuPt+qhtHrACCWwiiBgajIOmIczKUlX4D
+ASBkthx0o9Qb/r3dT91zmrniIK5I0yqe34/1rsHhOAf8ds2EubupFJJqFOb1
+qssMWE+jBrTREoD/EH5q7un2jEGccITcVQSZCqfjHT4EL6dF/bmuggf7wV/E
+QLXfFIJS6cZczK86XW1pGaXBKRLvQXYa/eRWHKcGlrujdFKzJYRoT6LVDk8T
+jhAfE9q2ElqlaAvZZYw=`);
+async function fakeSignature() {
+  // read the template and modify the text to
+  // invalidate the signature.
+  let fake = await cleartext.readArmored(
+    ORIGINAL.replace(
+      'You owe me',
+      'I owe you'));
+  // read the standalone signature packet
+  const tmp = new Signature();
+  await tmp.read(STANDALONE_PKT);
+
+  // replace the "text" signature with the
+  // "standalone" signature
+  fake.signature.packets[0] = tmp;
+  const faked_armored = await fake.armor();
+  // re-read the message to eliminate any
+  // behaviour due to cached values.
+  fake = await cleartext.readArmored(faked_armored);
+  // faked message now verifies correctly
+  const res = await openpgp.verify({
+    message: fake,
+    publicKeys: await getOtherPubKey(),
+    streaming: false
+  });
+  const { signatures } = res;
+  expect(signatures).to.have.length(0);
+}
+
+it('Does not accept non-binary/text signatures', fakeSignature);

test/security/subkey_trust.js

@@ -0,0 +1,76 @@
+const openpgp = typeof window !== 'undefined' && window.openpgp ? window.openpgp : require('../../dist/openpgp');
+
+const { key, cleartext, enums, packet: { List, Signature } } = openpgp;
+
+const chai = require('chai');
+chai.use(require('chai-as-promised'));
+
+const expect = chai.expect;
+
+async function generateTestData() {
+  const victimPrivKey = await key.generate({
+    userIds: ['Victim <victim@example.com>'],
+    numBits: 1024,
+    subkeys: [{
+      sign: true
+    }]
+  });
+  victimPrivKey.revocationSignatures = [];
+
+  const attackerPrivKey = await key.generate({
+    userIds: ['Attacker <attacker@example.com>'],
+    numBits: 1024,
+    subkeys: [],
+    sign: false
+  });
+  attackerPrivKey.revocationSignatures = [];
+  const signed = await openpgp.sign({
+    message: await cleartext.fromText('I am batman'),
+    privateKeys: victimPrivKey,
+    streaming: false,
+    armor: true
+  });
+  return {
+    victimPubKey: victimPrivKey.toPublic(),
+    attackerPrivKey,
+    signed
+  };
+}
+
+async function testSubkeyTrust() {
+  // attacker only has his own private key,
+  // the victim's public key and a signed message
+  const { victimPubKey, attackerPrivKey, signed } = await generateTestData();
+
+  const pktPubVictim = victimPubKey.toPacketlist();
+  const pktPrivAttacker = attackerPrivKey.toPacketlist();
+  const dataToSign = {
+    key: attackerPrivKey.toPublic().keyPacket,
+    bind: pktPubVictim[3] // victim subkey
+  };
+  const fakeBindingSignature = new Signature();
+  fakeBindingSignature.signatureType = enums.signature.subkey_binding;
+  fakeBindingSignature.publicKeyAlgorithm = attackerPrivKey.keyPacket.algorithm;
+  fakeBindingSignature.hashAlgorithm = enums.hash.sha256;
+  fakeBindingSignature.keyFlags = [enums.keyFlags.sign_data];
+  await fakeBindingSignature.sign(attackerPrivKey.keyPacket, dataToSign);
+  const newList = new List();
+  newList.concat([
+    pktPrivAttacker[0], // attacker private key
+    pktPrivAttacker[1], // attacker user
+    pktPrivAttacker[2], // attacker self signature
+    pktPubVictim[3], // victim subkey
+    fakeBindingSignature // faked key binding
+  ]);
+  let fakeKey = new key.Key(newList);
+  fakeKey = (await key.readArmored(await fakeKey.toPublic().armor())).keys[0];
+  const verifyAttackerIsBatman = await openpgp.verify({
+    message: (await cleartext.readArmored(signed.data)),
+    publicKeys: fakeKey,
+    streaming: false
+  });
+  expect(verifyAttackerIsBatman.signatures[0].keyid.equals(victimPubKey.subKeys[0].getKeyId())).to.be.true;
+  expect(verifyAttackerIsBatman.signatures[0].valid).to.be.null;
+}
+
+it('Does not trust subkeys without Primary Key Binding Signature', testSubkeyTrust);

test/security/unsigned_subpackets.js

@@ -0,0 +1,95 @@
+const openpgp = typeof window !== 'undefined' && window.openpgp ? window.openpgp : require('../../dist/openpgp');
+
+const { key, message, enums, packet: { List, Signature } } = openpgp;
+
+const chai = require('chai');
+chai.use(require('chai-as-promised'));
+
+const expect = chai.expect;
+
+/*
+* This key is long expired and cannot be used for encryption.
+*/
+const INVALID_KEY = `
+-----BEGIN PGP PRIVATE KEY BLOCK-----
+Version: OpenPGP.js VERSION
+Comment: https://openpgpjs.org
+
+xcMGBDhtQ4ABB/9uAfnjiE8HLfFrk4AzYIoxISvIbqDlItn3Mk2RK4iGTaAL
+h+hN8BrqOopgdHj5c3pTo6VDvJLieHwymdZ3d296L55zt2ichhVIgRxh20Tv
+j0dYLKGIEWDMBKvQNoDi83eGrIeHGNjRDOipr/PD251LzwaeiNVyw8ce2Fpd
+1ORbC2MJU57C2appZqeMJsWPCnsHNkhxPyRGdp+vifgizi/lt2DcQ6C6EiJx
+HV0jFDPJnb69LxKLUelRH+l/b2ZHTONu2pZwUXcFpjA5yTrSzO/kaUtGu/Cz
+3euQ3scEtvMXgO2R9H7halxYwyXL/PPLmgaUt1RNXGC7BZjkUW4n8qd/ABEB
+AAH+CQMITYNkFGQHMiJgt2s69CHTfwUUZg1Yfcq8alY7GpqeH4CayWCMPI+v
+l7kIJdl2b9N/xGnpaUMmaXJts6AtlIBLwzxg0syIfgRv4/wfrVeruJ9TfCFC
+NbKP3lk3FZCGF0I4T1FSNvyPJ//ee1cX7U/gM7A2g5xyBFnH5d8LTUDlQjXb
+a+BwYN2TZaFrvlWwMIU+NQa+EOiyAwXsgyQbVn2d7JsUUs/lyEG2xkWNTeqe
+FWKJJvyDwixsxd7oobBSM6Krt2TreuelPBFQxKyaYyv81gASga9wxyfbIuTG
+7wAKW9i4pFMgrrIABcnNKOyeAgMDcAYXAW3eNbYDCIDL9/AuOUotPR+0pEun
+WssAZGBM78ZlJZ1Qnbg9nT0rn4pHrFQHnBxlWyPEqj1mZ0Svc0vXHVH+8JgN
+pwOGxo7DiF5lL/bphdFVMF2e+UPoc1efO4cpW+ZH/BOug14dJROfkrPhrUTp
+nYu6VF9N723YVT9PDTg79E4kIzjMDvhV1odHSaxfl4VtgueYv+Bt3n2nXdME
+XZVBXbp7jO7pTS5HsOBcModos8ZYS5RcaHPJ6H8807hFyva4GThZ744ryV8b
+XnROoC+d/xR4ShA4f/f9QszMXZ+Xlh4IU3Ccz5PF5UiZ/nC5ho5KzJphBB53
+c78gjRIXeUK1Rgj2AquF3KDOjCm60oazKzXv8316ZODNJr+HVvGSKeq85z9Z
+z/BfXUtn+PrmzHxegusZfFCpB6YAJCILsHgJ2gT8v26QF+1CJ3ngHVnSkghR
+z64zJexeqA8ChTZnhPbHVhh5qx2hlNTofBV29LJGa/EpMykO5pZiuaSEkmZx
+RpU+iKNYKa3U516O8f9yj+UZ5/t2SJRpT+9fro3RB4lUnt/RdkY8q2R+3owo
+xr4sYaInfvrs3eCsmh5UtygUVARKrK84zR1UZXN0aSBUZXN0IDx0ZXN0QGV4
+YW1wbGUuY29tPsLAewQQAQgALwUCOG1DgAUJAAAACgYLCQcIAwIJEMwSTBo3
+j0N5BBUICgIDFgIBAhkBAhsDAh4BAAD2TQf+KQbrX2zO9SL5ffCK5qu2VigM
+0E3uF763II9vRYfXHdZtXY/8K/uBLbu2rdZHwwb/jAHEe60Qf5VjcbIMtCfA
+khPB5JuCvW+JEsYhXplNxYka27svfWI75/cYVc/0OharKEaaPOv2F8C1k2jL
+Sk7Az01IAJkdwmBkG6fUwupevuvpO+kUQjsHg35q8Lm7G8roCYiK7K7/JQi3
+K+e0ovVFvunFSORaG8jR37uT7X7KA0LHD3S7XYO0o2OJi0QKB1wN3H3FEll0
+bFznfdIzKKIDzGwC/zhpUMGMwsqVLb8sw/H9cr82yPoM6pXVUqnstKDlLce8
+Dc2vwS83Aja9iWrIEg==
+=dvRO
+-----END PGP PRIVATE KEY BLOCK-----`;
+
+async function getInvalidKey() {
+  return (await key.readArmored(INVALID_KEY)).keys[0];
+}
+async function makeKeyValid() {
+  /**
+  * Checks if a key can be used for encryption.
+  */
+  async function encryptFails(k) {
+    try {
+      await openpgp.encrypt({
+        message: message.fromText('Hello', 'hello.txt'),
+        publicKeys: k
+      });
+      return false;
+    } catch (e) {
+      return true;
+    }
+  }
+  const invalidkey = await getInvalidKey();
+  // deconstruct invalid key
+  const [pubkey, puser, pusersig] = invalidkey.toPacketlist().map(i => i);
+  // create a fake signature
+  const fake = new Signature();
+  Object.assign(fake, pusersig);
+  // extend expiration times
+  fake.keyExpirationTime = 0x7FFFFFFF;
+  fake.signatureExpirationTime = 0x7FFFFFFF;
+  // add key capability
+  fake.keyFlags[0] |= enums.keyFlags.encrypt_communication;
+  // create modified subpacket data
+  pusersig.read_sub_packets(fake.write_hashed_sub_packets(), false);
+  // reconstruct the modified key
+  const newlist = new List();
+  newlist.concat([pubkey, puser, pusersig]);
+  let modifiedkey = new key.Key(newlist);
+  // re-read the message to eliminate any
+  // behaviour due to cached values.
+  modifiedkey = (await key.readArmored(
+    await modifiedkey.armor())).keys[0];
+
+  expect(await encryptFails(invalidkey)).to.be.true;
+  expect(await encryptFails(modifiedkey)).to.be.true;
+}
+
+it('Does not accept unsigned subpackets', makeKeyValid);

test/unittests.js

@@ -54,4 +54,5 @@ describe('Unit Tests', function () {
   require('./crypto');
   require('./general');
   require('./worker');
+  require('./security');
 });