diff --git a/src/key.js b/src/key.js index 7e98f33b..ac6d2f4f 100644 --- a/src/key.js +++ b/src/key.js @@ -261,30 +261,37 @@ Key.prototype.armor = function() { }; /** - * Returns the signature that has the latest creation date, while ignoring signatures created in the future. + * Returns the valid and non-expired signature that has the latest creation date, while ignoring signatures created in the future. * @param {Array} signatures List of signatures * @param {Date} date Use the given date instead of the current time - * @returns {module:packet.Signature} The latest signature + * @returns {Promise} The latest valid signature + * @async */ -function getLatestSignature(signatures, date=new Date()) { - let signature = signatures[0]; - for (let i = 1; i < signatures.length; i++) { - if (signatures[i].created >= signature.created && - (signatures[i].created <= date || date === null)) { +async function getLatestValidSignature(signatures, primaryKey, dataToVerify, date=new Date()) { + let signature; + for (let i = signatures.length - 1; i >= 0; i--) { + if ( + (!signature || signatures[i].created >= signature.created) && + // check binding signature is not expired (ie, check for V4 expiration time) + !signatures[i].isExpired(date) && + // check binding signature is verified + (signatures[i].verified || await signatures[i].verify(primaryKey, dataToVerify)) + ) { signature = signatures[i]; } } return signature; } -function isValidSigningKeyPacket(keyPacket, signature, date=new Date()) { +function isValidSigningKeyPacket(keyPacket, signature) { + if (!signature.verified || signature.revoked !== false) { // Sanity check + throw new Error('Signature not verified'); + } return keyPacket.algorithm !== enums.read(enums.publicKey, enums.publicKey.rsa_encrypt) && keyPacket.algorithm !== enums.read(enums.publicKey, enums.publicKey.elgamal) && keyPacket.algorithm !== enums.read(enums.publicKey, enums.publicKey.ecdh) && (!signature.keyFlags || - (signature.keyFlags[0] & enums.keyFlags.sign_data) !== 0) && - signature.verified && !signature.revoked && !signature.isExpired(date) && - !isDataExpired(keyPacket, signature, date); + (signature.keyFlags[0] & enums.keyFlags.sign_data) !== 0); } /** @@ -302,8 +309,9 @@ Key.prototype.getSigningKey = async function (keyId=null, date=new Date(), userI for (let i = 0; i < subKeys.length; i++) { if (!keyId || subKeys[i].getKeyId().equals(keyId)) { if (await subKeys[i].verify(primaryKey, date) === enums.keyStatus.valid) { - const bindingSignature = getLatestSignature(subKeys[i].bindingSignatures, date); - if (isValidSigningKeyPacket(subKeys[i].keyPacket, bindingSignature, date)) { + 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)) { return subKeys[i]; } } @@ -311,23 +319,24 @@ Key.prototype.getSigningKey = async function (keyId=null, date=new Date(), userI } const primaryUser = await this.getPrimaryUser(date, userId); if (primaryUser && (!keyId || primaryKey.getKeyId().equals(keyId)) && - isValidSigningKeyPacket(primaryKey, primaryUser.selfCertification, date)) { + isValidSigningKeyPacket(primaryKey, primaryUser.selfCertification)) { return this; } } return null; }; -function isValidEncryptionKeyPacket(keyPacket, signature, date=new Date()) { +function isValidEncryptionKeyPacket(keyPacket, signature) { + if (!signature.verified || signature.revoked !== false) { // Sanity check + throw new Error('Signature not verified'); + } return keyPacket.algorithm !== enums.read(enums.publicKey, enums.publicKey.dsa) && keyPacket.algorithm !== enums.read(enums.publicKey, enums.publicKey.rsa_sign) && keyPacket.algorithm !== enums.read(enums.publicKey, enums.publicKey.ecdsa) && keyPacket.algorithm !== enums.read(enums.publicKey, enums.publicKey.eddsa) && (!signature.keyFlags || (signature.keyFlags[0] & enums.keyFlags.encrypt_communication) !== 0 || - (signature.keyFlags[0] & enums.keyFlags.encrypt_storage) !== 0) && - signature.verified && !signature.revoked && !signature.isExpired(date) && - !isDataExpired(keyPacket, signature, date); + (signature.keyFlags[0] & enums.keyFlags.encrypt_storage) !== 0); } /** @@ -346,8 +355,9 @@ Key.prototype.getEncryptionKey = async function(keyId, date=new Date(), userId={ for (let i = 0; i < subKeys.length; i++) { if (!keyId || subKeys[i].getKeyId().equals(keyId)) { if (await subKeys[i].verify(primaryKey, date) === enums.keyStatus.valid) { - const bindingSignature = getLatestSignature(subKeys[i].bindingSignatures, date); - if (isValidEncryptionKeyPacket(subKeys[i].keyPacket, bindingSignature, date)) { + const dataToVerify = { key: primaryKey, bind: subKeys[i].keyPacket }; + const bindingSignature = await getLatestValidSignature(subKeys[i].bindingSignatures, primaryKey, dataToVerify, date); + if (bindingSignature && isValidEncryptionKeyPacket(subKeys[i].keyPacket, bindingSignature)) { return subKeys[i]; } } @@ -356,7 +366,7 @@ Key.prototype.getEncryptionKey = async function(keyId, date=new Date(), userId={ // if no valid subkey for encryption, evaluate primary key const primaryUser = await this.getPrimaryUser(date, userId); if (primaryUser && (!keyId || primaryKey.getKeyId().equals(keyId)) && - isValidEncryptionKeyPacket(primaryKey, primaryUser.selfCertification, date)) { + isValidEncryptionKeyPacket(primaryKey, primaryUser.selfCertification)) { return this; } } @@ -492,13 +502,13 @@ Key.prototype.getExpirationTime = async function(capabilities, keyId, userId) { if (capabilities === 'encrypt' || capabilities === 'encrypt_sign') { const encryptKey = await this.getEncryptionKey(keyId, null, userId); if (!encryptKey) return null; - const encryptExpiry = encryptKey.getExpirationTime(); + const encryptExpiry = await encryptKey.getExpirationTime(this.keyPacket); if (encryptExpiry < expiry) expiry = encryptExpiry; } if (capabilities === 'sign' || capabilities === 'encrypt_sign') { const signKey = await this.getSigningKey(keyId, null, userId); if (!signKey) return null; - const signExpiry = signKey.getExpirationTime(); + const signExpiry = await signKey.getExpirationTime(this.keyPacket); if (signExpiry < expiry) expiry = signExpiry; } return expiry; @@ -515,16 +525,20 @@ Key.prototype.getExpirationTime = async function(capabilities, keyId, userId) { * @async */ Key.prototype.getPrimaryUser = async function(date=new Date(), userId={}) { - const users = this.users.map(function(user, index) { - const selfCertification = getLatestSignature(user.selfCertifications, date); - return { index, user, selfCertification }; - }).filter(({ user, selfCertification }) => { - return user.userId && selfCertification && ( + const primaryKey = this.keyPacket; + const users = []; + for (let i = 0; i < this.users.length; i++) { + const user = this.users[i]; + if (!user.userId || !( (userId.name === undefined || user.userId.name === userId.name) && (userId.email === undefined || user.userId.email === userId.email) && (userId.comment === undefined || user.userId.comment === userId.comment) - ); - }); + )) continue; + const dataToVerify = { userId: user.userId, key: primaryKey }; + const selfCertification = await getLatestValidSignature(user.selfCertifications, primaryKey, dataToVerify, date); + if (!selfCertification) continue; + users.push({ index: i, user, selfCertification }); + } if (!users.length) { if (userId.name !== undefined || userId.email !== undefined || userId.comment !== undefined) { @@ -539,18 +553,9 @@ Key.prototype.getPrimaryUser = async function(date=new Date(), userId={}) { return A.isPrimaryUserID - B.isPrimaryUserID || A.created - B.created; }).pop(); const { user, selfCertification: cert } = primaryUser; - const primaryKey = this.keyPacket; - const dataToVerify = { userId: user.userId, key: primaryKey }; - // skip if certificates is invalid, revoked, or expired - if (!(cert.verified || await cert.verify(primaryKey, dataToVerify))) { - return null; - } if (cert.revoked || await user.isRevoked(primaryKey, cert, null, date)) { return null; } - if (cert.isExpired(date)) { - return null; - } return primaryUser; }; @@ -678,12 +683,15 @@ Key.prototype.revoke = async function({ /** * Get revocation certificate from a revoked key. * (To get a revocation certificate for an unrevoked key, call revoke() first.) - * @returns {String} armored revocation certificate + * @returns {Promise} armored revocation certificate + * @async */ -Key.prototype.getRevocationCertificate = function() { - if (this.revocationSignatures.length) { +Key.prototype.getRevocationCertificate = async function() { + const dataToVerify = { key: this.keyPacket }; + const revocationSignature = await getLatestValidSignature(this.revocationSignatures, this.keyPacket, dataToVerify); + if (revocationSignature) { const packetlist = new packet.List(); - packetlist.push(getLatestSignature(this.revocationSignatures)); + packetlist.push(revocationSignature); return armor.encode(enums.armor.public_key, packetlist.write(), null, null, 'This is a revocation certificate'); } }; @@ -1090,17 +1098,17 @@ SubKey.prototype.verify = async function(primaryKey, date=new Date()) { const that = this; const dataToVerify = { key: primaryKey, bind: this.keyPacket }; // check subkey binding signatures - const bindingSignature = getLatestSignature(this.bindingSignatures, date); + const bindingSignature = await getLatestValidSignature(this.bindingSignatures, primaryKey, dataToVerify, date); // check binding signature is verified - if (!(bindingSignature.verified || await bindingSignature.verify(primaryKey, dataToVerify))) { + if (!bindingSignature) { return enums.keyStatus.invalid; } // check binding signature is not revoked if (bindingSignature.revoked || await that.isRevoked(primaryKey, bindingSignature, null, date)) { return enums.keyStatus.revoked; } - // check binding signature is not expired (ie, check for V4 expiration time) - if (bindingSignature.isExpired(date)) { + // check for expiration time + if (isDataExpired(this.keyPacket, bindingSignature, date)) { return enums.keyStatus.expired; } return enums.keyStatus.valid; // binding signature passed all checks @@ -1108,11 +1116,16 @@ SubKey.prototype.verify = async function(primaryKey, date=new Date()) { /** * Returns the expiration time of the subkey or Infinity if key does not expire + * @param {module:packet.SecretKey| + * module:packet.PublicKey} primaryKey The primary key packet * @param {Date} date Use the given date instead of the current time - * @returns {Date} + * @returns {Promise} + * @async */ -SubKey.prototype.getExpirationTime = function(date=new Date()) { - const bindingSignature = getLatestSignature(this.bindingSignatures, date); +SubKey.prototype.getExpirationTime = async function(primaryKey, date=new Date()) { + const dataToVerify = { key: primaryKey, bind: this.keyPacket }; + const bindingSignature = await getLatestValidSignature(this.bindingSignatures, primaryKey, dataToVerify, date); + if (!bindingSignature) return null; const keyExpiry = getExpirationTime(this.keyPacket, bindingSignature); const sigExpiry = bindingSignature.getExpirationTime(); return keyExpiry < sigExpiry ? keyExpiry : sigExpiry; @@ -1556,7 +1569,7 @@ async function isDataRevoked(primaryKey, dataToVerify, revocations, signature, k // TODO further verify that this is the signature that should be revoked if (signature) { signature.revoked = revocationKeyIds.some(keyId => keyId.equals(signature.issuerKeyId)) ? true : - signature.revoked; + signature.revoked || false; return signature.revoked; } return revocationKeyIds.length > 0; diff --git a/src/openpgp.js b/src/openpgp.js index e9540a55..389416b3 100644 --- a/src/openpgp.js +++ b/src/openpgp.js @@ -124,7 +124,7 @@ export function generateKey({ userIds=[], passphrase="", numBits=2048, keyExpira } return generate(options).then(async key => { - const revocationCertificate = key.getRevocationCertificate(); + const revocationCertificate = await key.getRevocationCertificate(); key.revocationSignatures = []; return convertStreams({ @@ -159,8 +159,8 @@ export function reformatKey({privateKey, userIds=[], passphrase="", keyExpiratio options.revoked = options.revocationCertificate; - return reformat(options).then(key => { - const revocationCertificate = key.getRevocationCertificate(); + return reformat(options).then(async key => { + const revocationCertificate = await key.getRevocationCertificate(); key.revocationSignatures = []; return { diff --git a/src/packet/signature.js b/src/packet/signature.js index 0121cfd0..25464690 100644 --- a/src/packet/signature.js +++ b/src/packet/signature.js @@ -201,6 +201,12 @@ Signature.prototype.sign = async function (key, data) { this.signature = stream.fromAsync(async () => crypto.signature.sign( publicKeyAlgorithm, hashAlgorithm, params, toHash, await stream.readToEnd(hash) )); + + // Store the fact that this signature is valid, e.g. for when we call `await + // getLatestValidSignature(this.revocationSignatures, key, data)` later. Note + // that this only holds up if the key and data passed to verify are the same + // as the ones passed to sign. + this.verified = true; return true; }; diff --git a/test/general/key.js b/test/general/key.js index 1275f2f3..9de32570 100644 --- a/test/general/key.js +++ b/test/general/key.js @@ -1389,7 +1389,7 @@ function versionSpecificTests() { const actual_delta = (new Date(expiration) - new Date()) / 1000; expect(Math.abs(actual_delta - expect_delta)).to.be.below(60); - const subKeyExpiration = await key.subKeys[0].getExpirationTime(); + const subKeyExpiration = await key.subKeys[0].getExpirationTime(key.primaryKey); expect(subKeyExpiration).to.exist; const actual_subKeyDelta = (new Date(subKeyExpiration) - new Date()) / 1000; @@ -1799,7 +1799,7 @@ describe('Key', function() { const pubKey = (await openpgp.key.readArmored(twoKeys)).keys[1]; expect(pubKey).to.exist; expect(pubKey).to.be.an.instanceof(openpgp.key.Key); - const expirationTime = await pubKey.subKeys[0].getExpirationTime(); + const expirationTime = await pubKey.subKeys[0].getExpirationTime(pubKey.primaryKey); expect(expirationTime.toISOString()).to.be.equal('2018-11-26T10:58:29.000Z'); }); @@ -1990,7 +1990,7 @@ describe('Key', function() { it('getRevocationCertificate() should produce the same revocation certificate as GnuPG', async function() { const revKey = (await openpgp.key.readArmored(revoked_key_arm4)).keys[0]; - const revocationCertificate = revKey.getRevocationCertificate(); + const revocationCertificate = await revKey.getRevocationCertificate(); const input = await openpgp.armor.decode(revocation_certificate_arm4); const packetlist = new openpgp.packet.List(); @@ -2002,7 +2002,7 @@ describe('Key', function() { it('getRevocationCertificate() should have an appropriate comment', async function() { const revKey = (await openpgp.key.readArmored(revoked_key_arm4)).keys[0]; - const revocationCertificate = revKey.getRevocationCertificate(); + const revocationCertificate = await revKey.getRevocationCertificate(); expect(revocationCertificate).to.match(/Comment: This is a revocation certificate/); expect(revKey.armor()).not.to.match(/Comment: This is a revocation certificate/); @@ -2170,7 +2170,27 @@ VYGdb3eNlV8CfoEC it('Selects the most recent subkey binding signature', async function() { const key = (await openpgp.key.readArmored(multipleBindingSignatures)).keys[0]; - expect(key.subKeys[0].getExpirationTime().toISOString()).to.equal('2015-10-18T07:41:30.000Z'); + expect((await key.subKeys[0].getExpirationTime(key.primaryKey)).toISOString()).to.equal('2015-10-18T07:41:30.000Z'); + }); + + it('Selects the most recent non-expired subkey binding signature', async function() { + const key = (await openpgp.key.readArmored(multipleBindingSignatures)).keys[0]; + key.subKeys[0].bindingSignatures[1].signatureNeverExpires = false; + key.subKeys[0].bindingSignatures[1].signatureExpirationTime = 0; + expect((await key.subKeys[0].getExpirationTime(key.primaryKey)).toISOString()).to.equal('2018-09-07T06:03:37.000Z'); + }); + + it('Selects the most recent valid subkey binding signature', async function() { + const key = (await openpgp.key.readArmored(multipleBindingSignatures)).keys[0]; + key.subKeys[0].bindingSignatures[1].signatureData[0]++; + expect((await key.subKeys[0].getExpirationTime(key.primaryKey)).toISOString()).to.equal('2018-09-07T06:03:37.000Z'); + }); + + it('Handles a key with no valid subkey binding signatures gracefully', async function() { + const key = (await openpgp.key.readArmored(multipleBindingSignatures)).keys[0]; + key.subKeys[0].bindingSignatures[0].signatureData[0]++; + key.subKeys[0].bindingSignatures[1].signatureData[0]++; + expect(await key.subKeys[0].getExpirationTime(key.primaryKey)).to.be.null; }); it('Reject encryption with revoked subkey', async function() {