From c0ceffe998f1b2723459ad46ec018b855b1981a6 Mon Sep 17 00:00:00 2001 From: Sanjana Rajan Date: Sat, 17 Mar 2018 04:59:58 -0700 Subject: [PATCH 1/2] some refactoring, calculate exp time of expired keys --- src/key.js | 68 ++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 46 insertions(+), 22 deletions(-) diff --git a/src/key.js b/src/key.js index 83e5f9e3..93dbb556 100644 --- a/src/key.js +++ b/src/key.js @@ -437,11 +437,18 @@ Key.prototype.getExpirationTime = async function() { return getExpirationTime(this.primaryKey); } if (this.primaryKey.version === 4) { - const primaryUser = await this.getPrimaryUser(); - if (!primaryUser) { + let validUsers = await this.getValidUsers(new Date(), true); + if (!validUsers.length) { return null; } - return getExpirationTime(this.primaryKey, primaryUser.selfCertification); + validUsers = validUsers.sort(function(a, b) { + const A = a.selfCertification; + const B = b.selfCertification; + const expTimeA = !A.signatureNeverExpires ? A.created.getTime() + A.signatureExpirationTime*1000 : Infinity; + const expTimeB = !B.signatureNeverExpires ? B.created.getTime() + B.signatureExpirationTime*1000 : Infinity; + return expTimeA - expTimeB; + }); + return getExpirationTime(this.primaryKey, validUsers.pop().selfCertification); } }; @@ -450,13 +457,35 @@ Key.prototype.getExpirationTime = async function() { * - if multiple primary users exist, returns the one with the latest self signature * - otherwise, returns the user with the latest self signature * @param {Date} date use the given date for verification instead of the current time - * @returns {Promise<{user: Array, - * selfCertification: Array}>} The primary user and the self signature + * @returns {Promise<{user: module:key.User, + * selfCertification: module:packet.Signature}>} The primary user and the self signature * @async */ Key.prototype.getPrimaryUser = async function(date=new Date()) { + let validUsers = await this.getValidUsers(date); + if (!validUsers.length) { + return null; + } + // sort by primary user flag and signature creation time + validUsers = validUsers.sort(function(a, b) { + const A = a.selfCertification; + const B = b.selfCertification; + return (A.isPrimaryUserID - B.isPrimaryUserID) || (A.created - B.created); + }); + return validUsers.pop(); +}; + +/** + * Returns an array containing all valid users for a key + * @param {Date} date use the given date for verification instead of the current time + * @param {bool} whether to allow expired self certifications + * @returns {Promise>} The valid user array + * @async + */ +Key.prototype.getValidUsers = async function(date=new Date(), allowExpired=false) { const { primaryKey } = this; - let primaryUsers = []; + const validUsers = []; let lastCreated = null; let lastPrimaryUserID = null; // TODO replace when Promise.forEach is implemented @@ -482,23 +511,16 @@ Key.prototype.getPrimaryUser = async function(date=new Date()) { if (cert.revoked || await user.isRevoked(primaryKey, cert, null, date)) { continue; } - if (cert.isExpired(date)) { + if (!allowExpired && cert.isExpired(date)) { continue; } lastPrimaryUserID = cert.isPrimaryUserID; lastCreated = cert.created; - primaryUsers.push({ index: i, user: user, selfCertification: cert }); + validUsers.push({ index: i, user: user, selfCertification: cert }); } } - // sort by primary user flag and signature creation time - primaryUsers = primaryUsers.sort(function(a, b) { - const A = a.selfCertification; - const B = b.selfCertification; - return (A.isPrimaryUserID - B.isPrimaryUserID) || (A.created - B.created); - }); - return primaryUsers.pop(); + return validUsers; }; - /** * Update key with new components from specified key with same key ID: * users, subkeys, certificates are merged into the destination key, @@ -964,8 +986,8 @@ SubKey.prototype.getExpirationTime = function() { let highest; for (let i = 0; i < this.bindingSignatures.length; i++) { const current = getExpirationTime(this.subKey, this.bindingSignatures[i]); - if (current === null) { - return null; + if (current === Infinity) { + return Infinity; } if (!highest || current > highest) { highest = current; @@ -1337,14 +1359,13 @@ function isDataExpired(keyPacket, signature, date=new Date()) { const normDate = util.normalizeDate(date); if (normDate !== null) { const expirationTime = getExpirationTime(keyPacket, signature); - return !(keyPacket.created <= normDate && normDate < expirationTime) || - (signature && signature.isExpired(date)); + return !(keyPacket.created <= normDate && normDate < expirationTime); } return false; } function getExpirationTime(keyPacket, signature) { - let expirationTime; + let expirationTime = Infinity; // check V3 expiration time if (keyPacket.version === 3 && keyPacket.expirationTimeV3 !== 0) { expirationTime = keyPacket.created.getTime() + keyPacket.expirationTimeV3*24*3600*1000; @@ -1353,7 +1374,10 @@ function getExpirationTime(keyPacket, signature) { if (keyPacket.version === 4 && signature.keyNeverExpires === false) { expirationTime = keyPacket.created.getTime() + signature.keyExpirationTime*1000; } - return expirationTime ? new Date(expirationTime) : Infinity; + if (keyPacket.version === 4 && signature.signatureNeverExpires === false) { + expirationTime = Math.min(expirationTime, keyPacket.created.getTime() + signature.signatureExpirationTime*1000); + } + return expirationTime !== Infinity ? new Date(expirationTime) : Infinity; } /** From 75cd4e5e6f693de0cc31b7ab573388fc57ba7c9b Mon Sep 17 00:00:00 2001 From: Sanjana Rajan Date: Sat, 17 Mar 2018 05:00:45 -0700 Subject: [PATCH 2/2] some fixes, add expired key test --- src/key.js | 43 ++++++++++++++++++----------------------- src/packet/signature.js | 10 +++++++++- test/general/key.js | 23 ++++++++++++++++++++++ 3 files changed, 51 insertions(+), 25 deletions(-) diff --git a/src/key.js b/src/key.js index 93dbb556..49418e60 100644 --- a/src/key.js +++ b/src/key.js @@ -437,18 +437,17 @@ Key.prototype.getExpirationTime = async function() { return getExpirationTime(this.primaryKey); } if (this.primaryKey.version === 4) { - let validUsers = await this.getValidUsers(new Date(), true); - if (!validUsers.length) { - return null; + const validUsers = await this.getValidUsers(null, true); + let highest = null; + for (let i = 0; i < validUsers.length; i++) { + const selfCert = validUsers[i].selfCertification; + const current = Math.min(+getExpirationTime(this.primaryKey, selfCert), +selfCert.getExpirationTime()); + if (current === Infinity) { + return Infinity; + } + highest = current > highest ? current : highest; } - validUsers = validUsers.sort(function(a, b) { - const A = a.selfCertification; - const B = b.selfCertification; - const expTimeA = !A.signatureNeverExpires ? A.created.getTime() + A.signatureExpirationTime*1000 : Infinity; - const expTimeB = !B.signatureNeverExpires ? B.created.getTime() + B.signatureExpirationTime*1000 : Infinity; - return expTimeA - expTimeB; - }); - return getExpirationTime(this.primaryKey, validUsers.pop().selfCertification); + return util.normalizeDate(highest); } }; @@ -478,7 +477,7 @@ Key.prototype.getPrimaryUser = async function(date=new Date()) { /** * Returns an array containing all valid users for a key * @param {Date} date use the given date for verification instead of the current time - * @param {bool} whether to allow expired self certifications + * @param {bool} include users with expired certifications * @returns {Promise>} The valid user array * @async @@ -983,17 +982,15 @@ SubKey.prototype.verify = async function(primaryKey, date=new Date()) { * @returns {Date} */ SubKey.prototype.getExpirationTime = function() { - let highest; + let highest = null; for (let i = 0; i < this.bindingSignatures.length; i++) { - const current = getExpirationTime(this.subKey, this.bindingSignatures[i]); + const current = Math.min(+getExpirationTime(this.subKey, this.bindingSignatures[i]), +this.bindingSignatures[i].getExpirationTime()); if (current === Infinity) { return Infinity; } - if (!highest || current > highest) { - highest = current; - } + highest = current > highest ? current : highest; } - return highest; + return util.normalizeDate(highest); }; /** @@ -1359,13 +1356,14 @@ function isDataExpired(keyPacket, signature, date=new Date()) { const normDate = util.normalizeDate(date); if (normDate !== null) { const expirationTime = getExpirationTime(keyPacket, signature); - return !(keyPacket.created <= normDate && normDate < expirationTime); + return !(keyPacket.created <= normDate && normDate < expirationTime) || + (signature && signature.isExpired(date)); } return false; } function getExpirationTime(keyPacket, signature) { - let expirationTime = Infinity; + let expirationTime; // check V3 expiration time if (keyPacket.version === 3 && keyPacket.expirationTimeV3 !== 0) { expirationTime = keyPacket.created.getTime() + keyPacket.expirationTimeV3*24*3600*1000; @@ -1374,10 +1372,7 @@ function getExpirationTime(keyPacket, signature) { if (keyPacket.version === 4 && signature.keyNeverExpires === false) { expirationTime = keyPacket.created.getTime() + signature.keyExpirationTime*1000; } - if (keyPacket.version === 4 && signature.signatureNeverExpires === false) { - expirationTime = Math.min(expirationTime, keyPacket.created.getTime() + signature.signatureExpirationTime*1000); - } - return expirationTime !== Infinity ? new Date(expirationTime) : Infinity; + return expirationTime ? new Date(expirationTime) : Infinity; } /** diff --git a/src/packet/signature.js b/src/packet/signature.js index 0b7acbcd..7131770a 100644 --- a/src/packet/signature.js +++ b/src/packet/signature.js @@ -673,12 +673,20 @@ Signature.prototype.verify = async function (key, data) { Signature.prototype.isExpired = function (date=new Date()) { const normDate = util.normalizeDate(date); if (normDate !== null) { - const expirationTime = !this.signatureNeverExpires ? this.created.getTime() + this.signatureExpirationTime*1000 : Infinity; + const expirationTime = this.getExpirationTime(); return !(this.created <= normDate && normDate < expirationTime); } return false; }; +/** + * Returns the expiration time of the signature or Infinity if signature does not expire + * @returns {Date} expiration time + */ +Signature.prototype.getExpirationTime = function () { + return !this.signatureNeverExpires ? new Date(this.created.getTime() + this.signatureExpirationTime*1000) : Infinity; +}; + /** * Fix custom types after cloning */ diff --git a/test/general/key.js b/test/general/key.js index 71282267..1349b65e 100644 --- a/test/general/key.js +++ b/test/general/key.js @@ -721,6 +721,21 @@ describe('Key', function() { '=6XMW', '-----END PGP PUBLIC KEY BLOCK-----'].join('\n'); + const expiredKey = + `-----BEGIN PGP PRIVATE KEY BLOCK----- + + xcA4BAAAAAEBAgCgONc0J8rfO6cJw5YTP38x1ze2tAYIO7EcmRCNYwMkXngb + 0Qdzg34Q5RW0rNiR56VB6KElPUhePRPVklLFiIvHABEBAAEAAf9qabYMzsz/ + /LeRVZSsTgTljmJTdzd2ambUbpi+vt8MXJsbaWh71vjoLMWSXajaKSPDjVU5 + waFNt9kLqwGGGLqpAQD5ZdMH2XzTq6GU9Ka69iZs6Pbnzwdz59Vc3i8hXlUj + zQEApHargCTsrtvSrm+hK/pN51/BHAy9lxCAw9f2etx+AeMA/RGrijkFZtYt + jeWdv/usXL3mgHvEcJv63N5zcEvDX5X4W1bND3Rlc3QxIDxhQGIuY29tPsJ7 + BBABCAAvBQIAAAABBQMAAAU5BgsJBwgDAgkQzcF99nGrkAkEFQgKAgMWAgEC + GQECGwMCHgEAABAlAfwPehmLZs+gOhOTTaSslqQ50bl/REjmv42Nyr1ZBlQS + DECl1Qu4QyeXin29uEXWiekMpNlZVsEuc8icCw6ABhIZ + =/7PI + -----END PGP PRIVATE KEY BLOCK-----`; + it('Parsing armored text with two keys', function(done) { const pubKeys = openpgp.key.readArmored(twoKeys); expect(pubKeys).to.exist; @@ -823,6 +838,14 @@ describe('Key', function() { expect(expirationTime.toISOString()).to.be.equal('2018-11-26T10:58:29.000Z'); }); + it('Method getExpirationTime expired V4 Key', async function() { + const pubKey = openpgp.key.readArmored(expiredKey).keys[0]; + expect(pubKey).to.exist; + expect(pubKey).to.be.an.instanceof(openpgp.key.Key); + const expirationTime = await pubKey.getExpirationTime(); + expect(expirationTime.toISOString()).to.be.equal('1970-01-01T00:22:18.000Z'); + }); + it('Method getExpirationTime V4 SubKey', async function() { const pubKey = openpgp.key.readArmored(twoKeys).keys[1]; expect(pubKey).to.exist;