diff --git a/src/key.js b/src/key.js index f4b2b78e..b53a6600 100644 --- a/src/key.js +++ b/src/key.js @@ -268,19 +268,21 @@ function isValidSigningKeyPacket(keyPacket, signature, date=new Date()) { */ Key.prototype.getSigningKeyPacket = async function (keyId=null, date=new Date()) { const primaryKey = this.primaryKey; - const primaryUser = await this.getPrimaryUser(date); - if (primaryUser && (!keyId || primaryKey.getKeyId().equals(keyId)) && - isValidSigningKeyPacket(primaryKey, primaryUser.selfCertification, date) && - await this.verifyPrimaryKey(date)) { - return primaryKey; - } - for (let i = 0; i < this.subKeys.length; i++) { - if (!keyId || this.subKeys[i].subKey.getKeyId().equals(keyId)) { - // eslint-disable-next-line no-await-in-loop - await this.subKeys[i].verify(primaryKey, date); - for (let j = 0; j < this.subKeys[i].bindingSignatures.length; j++) { - if (isValidSigningKeyPacket(this.subKeys[i].subKey, this.subKeys[i].bindingSignatures[j], date)) { - return this.subKeys[i].subKey; + if (await this.verifyPrimaryKey(date) === enums.keyStatus.valid) { + const primaryUser = await this.getPrimaryUser(date); + if (primaryUser && (!keyId || primaryKey.getKeyId().equals(keyId)) && + isValidSigningKeyPacket(primaryKey, primaryUser.selfCertification, date)) { + return primaryKey; + } + for (let i = 0; i < this.subKeys.length; i++) { + if (!keyId || this.subKeys[i].subKey.getKeyId().equals(keyId)) { + // eslint-disable-next-line no-await-in-loop + if (await this.subKeys[i].verify(primaryKey, date) === enums.keyStatus.valid) { + for (let j = 0; j < this.subKeys[i].bindingSignatures.length; j++) { + if (isValidSigningKeyPacket(this.subKeys[i].subKey, this.subKeys[i].bindingSignatures[j], date)) { + return this.subKeys[i].subKey; + } + } } } } @@ -313,25 +315,27 @@ function isValidEncryptionKeyPacket(keyPacket, signature, date=new Date()) { */ Key.prototype.getEncryptionKeyPacket = async function(keyId, date=new Date()) { const primaryKey = this.primaryKey; - // V4: by convention subkeys are preferred for encryption service - // V3: keys MUST NOT have subkeys - for (let i = 0; i < this.subKeys.length; i++) { - if (!keyId || this.subKeys[i].subKey.getKeyId().equals(keyId)) { - // eslint-disable-next-line no-await-in-loop - await this.subKeys[i].verify(primaryKey, date); - for (let j = 0; j < this.subKeys[i].bindingSignatures.length; j++) { - if (isValidEncryptionKeyPacket(this.subKeys[i].subKey, this.subKeys[i].bindingSignatures[j], date)) { - return this.subKeys[i].subKey; + if (await this.verifyPrimaryKey(date) === enums.keyStatus.valid) { + // V4: by convention subkeys are preferred for encryption service + // V3: keys MUST NOT have subkeys + for (let i = 0; i < this.subKeys.length; i++) { + if (!keyId || this.subKeys[i].subKey.getKeyId().equals(keyId)) { + // eslint-disable-next-line no-await-in-loop + if (await this.subKeys[i].verify(primaryKey, date) === enums.keyStatus.valid) { + for (let j = 0; j < this.subKeys[i].bindingSignatures.length; j++) { + if (isValidEncryptionKeyPacket(this.subKeys[i].subKey, this.subKeys[i].bindingSignatures[j], date)) { + return this.subKeys[i].subKey; + } + } } } } - } - // if no valid subkey for encryption, evaluate primary key - const primaryUser = await this.getPrimaryUser(date); - if (primaryUser && (!keyId || primaryKey.getKeyId().equals(keyId)) && - isValidEncryptionKeyPacket(primaryKey, primaryUser.selfCertification, date) && - await this.verifyPrimaryKey(date)) { - return primaryKey; + // if no valid subkey for encryption, evaluate primary key + const primaryUser = await this.getPrimaryUser(date); + if (primaryUser && (!keyId || primaryKey.getKeyId().equals(keyId)) && + isValidEncryptionKeyPacket(primaryKey, primaryUser.selfCertification, date)) { + return primaryKey; + } } return null; }; @@ -391,33 +395,6 @@ Key.prototype.isRevoked = async function(signature, key, date=new Date()) { ); }; -/** - * Returns a packetlist containing all verified public or private key packets matching keyId. - * If keyId is not present, returns all verified key packets starting with the primary key. - * Verification is in the context of given date. - * @param {type/keyid} keyId - * @param {Date} date Use the given date instead of the current time - * @returns {Promise} - * @async - */ -Key.prototype.verifyKeyPackets = async function(keyId=null, date=new Date()) { - const packets = new packet.List(); - const { primaryKey } = this; - if (await this.verifyPrimaryKey(date)) { - if (!keyId || primaryKey.getKeyId().equals(keyId)) { - packets.push(primaryKey); - } - } - await Promise.all(this.subKeys.map(async subKey => { - if (!keyId || subKey.subKey.getKeyId().equals(keyId)) { - if (await subKey.verify(primaryKey, date)) { - packets.push(subKey.subKey); - } - } - })); - return packets; -}; - /** * Verify primary key. Checks for revocation signatures, expiration time * and valid self signature diff --git a/test/general/key.js b/test/general/key.js index 4e09926d..71282267 100644 --- a/test/general/key.js +++ b/test/general/key.js @@ -56,7 +56,7 @@ describe('Key', function() { '=w6wd', '-----END PGP PUBLIC KEY BLOCK-----'].join("\n"); - const pub_revoked = + const pub_revoked_subkeys = ['-----BEGIN PGP PUBLIC KEY BLOCK-----', 'Version: GnuPG v2.0.19 (GNU/Linux)', '', @@ -108,6 +108,46 @@ describe('Key', function() { '=ok+o', '-----END PGP PUBLIC KEY BLOCK-----'].join('\n'); + const pub_revoked_with_cert = + ['-----BEGIN PGP PUBLIC KEY BLOCK-----', + 'Comment: GPGTools - https://gpgtools.org', + '', + 'mQENBFqm7EoBCAC9MNVwQqdpz9bQK9fpKGLSPk20ckRvvTwq7oDUM1IMZGb4bd/A', + '3KDi9SoBU4oEFRbCAk3rxj8iGL9pxvsX3szyCEXuWfzilAQ/1amRe2woMst+YkTt', + 'x9rzkRiQta4T1fNlqQsJyIIpHrKAFGPp0UjBTr6Vs+Ti6JkF5su4ea+yEiuBHtY4', + 'NjFb1xuV7OyAsZToh0B5fah4/WZ5Joyt9h8gp4WGSvdhLbdsoo8Tjveh2G+Uy2qC', + 'mM8h5v9qGBBRyGM9QmAlhn9XtvEODGbSPYVijEUu8QmbUwHqONAN4fCKAM+jHPuA', + 'rFG+si6QNEk3SOhXX3nvu9ThXg/gKZmmX5ABABEBAAGJATYEIAEKACAWIQQuQVvj', + 'r/N2jaYeuSDeEOPy1UVxbwUCWqbs0wIdAQAKCRDeEOPy1UVxb8CyCACyEagyyvmg', + 'kmS8pEI+iJQU/LsfnNPHwYrDOm0NodGw8HYkil2kfWJim60vFPC3jozzFmvlfy5z', + 'VAge9sVUl3sk7HxnYdPmK767h5Skp6dQSBeeh5WfH4ZK+hhJt9vJTstzaAhVNkX1', + '5OPBfkpy9pbYblQj56g0ECF4UhUxGFVZfycy+i6jvTpk+ABHWDKdqoKj9pTOzwDV', + 'JVa6Y0UT76PMIDjkeDKUYTU6MHexN1oyC07IYh+HsZtlsPTs/zo1JsrO+D6aEkEg', + 'yoLStyg0uemr6LRQ5YuhpG7OMeGRgJstCSo22JHJEtpSUR688aHKN35KNmxjkJDi', + 'fL7cRKHLlqqKtBlTdW5ueSA8c3VubnlAc3Vubnkuc3Vubnk+iQFUBBMBCgA+FiEE', + 'LkFb46/zdo2mHrkg3hDj8tVFcW8FAlqm7EoCGwMFCQeGH4AFCwkIBwMFFQoJCAsF', + 'FgIDAQACHgECF4AACgkQ3hDj8tVFcW83pgf6Auezf0G4MR8/jfHTshYRO8uGdTVR', + 'PjSmczyk4UAk3xy2dZuVc4CathVs/ID3QhycurL33fiZntx+p3JKUrypnp2Y+ZXW', + 'q4xjL05yirDFq4WGgksovmP4q1NfNB3YIsNulHMJ/qCOHl6d+oIDIKF/udwr0+qf', + 'rhd1rMFqO5lAF5/kSBbRdCCLpvMIWKxvDkbZrsqvWcchP5nuymhVHn9cCVGdxsZ8', + 'a/1iODFsBTDF4LISX2Tk1AW5thT96erbvq9XOluDFNjZY9dc6/JWmyWBvLTNguGV', + 'rx0bydeGaddfZc+3XkpImKrpckz5gwYvkgu6bm7GroERjEeYzQDLsg2L07kBDQRa', + 'puxKAQgApxDXRk9YUQ2Ba9QVe8WW/NSmyYQEvtSuvG86nZn5aMiZkEuDpVcmePOS', + '1u6Pz0RB9k1WzAi6Az2l/fS7xSbzjDPV+VXV704t9r0M3Zr5RMzIRjbGoxaZp7Tv', + 'Da3QGN4VIZN6o4oAJM7G2FeZMstnCDxrT3wyKXaEdOn5Uc6hxl2Bhx2gTCpsTFn8', + 'AaBnSY6+kge6rCkeufndXQUhTVy8dYsaSqGwpQHVtk1X4nDoZlCC929F9d3I2/WV', + 'OGlfHqbpbO+8tprvQS0JSzsa9w7xaGJcYUA2tOWV8ZZgC8/1MHMsj5HhHKmmWPsS', + 's6k6RLg3nP+CV9zkKn4sI+l/erOEaQARAQABiQE8BBgBCgAmFiEELkFb46/zdo2m', + 'Hrkg3hDj8tVFcW8FAlqm7EoCGwwFCQeGH4AACgkQ3hDj8tVFcW/lmwgAs3o/b24U', + 't2jioTzjZNFMrqjc99PpURJ9BhKPqa9RF7HrpM4x2mJEFw0fUZGQpQmL5NP0ZazE', + 'N47YHwZH1O5i4t5HtFmjtmMkJUFPlwy0MrClW+OVu6Wl7rtYuXIBqFouUx1YBZtQ', + 'isAmwBeB6DS8Oc39raZpoHh9lGPN1Kmp6iLX3xq+6IqUEV567vSAAR6N2m18GH79', + '365Dq88eZKS/NtlzFnEzoThYlIVt/dknuQsUSdZHtuBbNXaJ816byVZQQWdqnXd5', + 'BIDZSFjrJY/gm2kgQX2Pn9hGqDdGhxiALjxhA0+OJQNw4v11y0zVGdofh0IHjkcZ', + 'onCOcv4DKguN2w==', + '=OqO3', + '-----END PGP PUBLIC KEY BLOCK----'].join('\n'); + const pub_v3 = ['-----BEGIN PGP PUBLIC KEY BLOCK-----', 'Version: SKS 1.1.3', @@ -771,7 +811,6 @@ describe('Key', function() { // remove subkeys pubKey.subKeys = []; // primary key has only key flags for signing - await pubKey.verifyKeyPackets(); const keyPacket = await pubKey.getEncryptionKeyPacket(); expect(keyPacket).to.not.exist; }); @@ -800,8 +839,8 @@ describe('Key', function() { }); it('update() - merge revocation signatures', function(done) { - const source = openpgp.key.readArmored(pub_revoked).keys[0]; - const dest = openpgp.key.readArmored(pub_revoked).keys[0]; + const source = openpgp.key.readArmored(pub_revoked_subkeys).keys[0]; + const dest = openpgp.key.readArmored(pub_revoked_subkeys).keys[0]; expect(source.revocationSignatures).to.exist; dest.revocationSignatures = []; dest.update(source).then(() => { @@ -1261,12 +1300,20 @@ describe('Key', function() { it('Find a valid subkey binding signature among many invalid ones', async function() { const key = openpgp.key.readArmored(valid_binding_sig_among_many_expired_sigs_pub).keys[0]; - await key.verifyKeyPackets(); expect(await key.getEncryptionKeyPacket()).to.not.be.null; }); it('Reject encryption with revoked subkey', function() { - const key = openpgp.key.readArmored(pub_revoked).keys[0]; + const key = openpgp.key.readArmored(pub_revoked_subkeys).keys[0]; + return openpgp.encrypt({publicKeys: [key], data: 'random data'}).then(() => { + throw new Error('encryptSessionKey should not encrypt with revoked public key'); + }).catch(function(error) { + expect(error.message).to.equal('Error encrypting message: Could not find valid key packet for encryption in key ' + key.primaryKey.getKeyId().toHex()); + }); + }); + + it('Reject encryption with key revoked with appended revocation cert', function() { + const key = openpgp.key.readArmored(pub_revoked_with_cert).keys[0]; return openpgp.encrypt({publicKeys: [key], data: 'random data'}).then(() => { throw new Error('encryptSessionKey should not encrypt with revoked public key'); }).catch(function(error) { @@ -1274,4 +1321,3 @@ describe('Key', function() { }); }); }); -