diff --git a/src/config/config.js b/src/config/config.js index 8fb39185..a59218ea 100644 --- a/src/config/config.js +++ b/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 diff --git a/src/key.js b/src/key.js index a66d92f6..5d4fd15b 100644 --- a/src/key.js +++ b/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. - err.push(new Error('This key is intended to be revoked with an authorized key, which OpenPGP.js does not support.')); + throw 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; diff --git a/src/message.js b/src/message.js index 87e1f8d0..65597e60 100644 --- a/src/message.js +++ b/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); })); } diff --git a/src/packet/signature.js b/src/packet/signature.js index a3ab97f0..292ba128 100644 --- a/src/packet/signature.js +++ b/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); - this.unhashedSubpackets = bytes.subarray(sigDataLength, i); + i += this.read_sub_packets(bytes.subarray(i, bytes.length), false); // 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 - * @returns {String} a string-representation of a all subpacket data + * Creates Uint8Array of bytes of all subpacket data except Issuer and Embedded Signature subpackets + * @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) { - bytes = util.str_to_Uint8Array(util.Uint8Array_to_str(this.preferredAeadAlgorithms)); - arr.push(write_sub_packet(sub.preferred_aead_algorithms, bytes)); - } + this.unhashedSubpackets.forEach(data => { + arr.push(packet.writeSimpleLength(data.length)); + 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. - const type = bytes[mypos++] & 0x7F; - let seconds; + // The leftmost bit denotes a "critical" packet + const critical = bytes[mypos] & 0x80; + 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,11 +524,36 @@ Signature.prototype.read_sub_packet = function (bytes) { // Preferred AEAD Algorithms read_array.call(this, 'preferredAeadAlgorithms', bytes.subarray(mypos, bytes.length)); break; - default: - util.print_debug("Unknown signature subpacket type " + type + " @:" + mypos); + default: { + 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 Signature.prototype.toSign = function (type, data) { const t = enums.signature; diff --git a/test/general/armor.js b/test/general/armor.js index cafc8287..d91768be 100644 --- a/test/general/armor.js +++ b/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 diff --git a/test/general/key.js b/test/general/key.js index eb21125f..a2ce51a3 100644 --- a/test/general/key.js +++ b/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[0].getKeyId().toHex()).to.equal('5880a2fd178372b9'); + expect(pubKeys.keys).to.have.length(0); }); it('Parsing V5 public key packet', async function() { diff --git a/test/security/index.js b/test/security/index.js new file mode 100644 index 00000000..5ca983e1 --- /dev/null +++ b/test/security/index.js @@ -0,0 +1,5 @@ +describe('Security', function () { + require('./message_signature_bypass'); + require('./unsigned_subpackets'); + require('./subkey_trust'); +}); diff --git a/test/security/message_signature_bypass.js b/test/security/message_signature_bypass.js new file mode 100644 index 00000000..8cc8c345 --- /dev/null +++ b/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); diff --git a/test/security/subkey_trust.js b/test/security/subkey_trust.js new file mode 100644 index 00000000..f0e8db52 --- /dev/null +++ b/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 '], + numBits: 1024, + subkeys: [{ + sign: true + }] + }); + victimPrivKey.revocationSignatures = []; + + const attackerPrivKey = await key.generate({ + userIds: ['Attacker '], + 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); diff --git a/test/security/unsigned_subpackets.js b/test/security/unsigned_subpackets.js new file mode 100644 index 00000000..9ea55056 --- /dev/null +++ b/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); diff --git a/test/unittests.js b/test/unittests.js index bb2f53d9..807cdbfc 100644 --- a/test/unittests.js +++ b/test/unittests.js @@ -54,4 +54,5 @@ describe('Unit Tests', function () { require('./crypto'); require('./general'); require('./worker'); + require('./security'); });