From c3d4bf3529ba9530e709735df6af0bfa89a19a46 Mon Sep 17 00:00:00 2001 From: Wiktor Kwapisiewicz Date: Thu, 12 Jul 2018 14:04:07 +0200 Subject: [PATCH] Fix Key#getPrimaryUser on keys without valid UIDs During tests of weird keys [0] it was found out that OpenPGP.js does not handle keys without valid UIDs well. There are two issues that this change corrects, first one is adding filtering of user IDs in `getPrimaryUser` such as only users with self certifications are considered. Without that change using `getPrimaryUser` on a key without valid UIDs would throw an exception (`Cannot read property 'verified' of undefined` [1]). Second issue is correcting condition whether to throw if no UID was found. Previous condition checked for `userId` variable, but this is initialized by default and as such is almost always set. This causes `key.getPrimaryUser()` (without condition) to throw `Could not find user that matches that user ID`. The condition has been modified to match the filter condition (checking if any property of `userId` has been initialized). [0]: https://gitlab.com/sequoia-pgp/weird-keys/tree/master/openpgpjs [1]: https://gitlab.com/sequoia-pgp/weird-keys/blob/576ed08a54c4878147bb993900f39005078d2da0/openpgpjs/results/no-bound-uid.pgp.txt --- src/key.js | 7 ++++--- test/general/key.js | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/key.js b/src/key.js index 5e9ae18d..ada60088 100644 --- a/src/key.js +++ b/src/key.js @@ -494,15 +494,16 @@ 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 }) => { - return user.userId && ( + }).filter(({ user, selfCertification }) => { + return user.userId && selfCertification && ( (userId.name === undefined || user.userId.name === userId.name) && (userId.email === undefined || user.userId.email === userId.email) && (userId.comment === undefined || user.userId.comment === userId.comment) ); }); if (!users.length) { - if (userId) { + if (userId.name !== undefined || userId.email !== undefined || + userId.comment !== undefined) { throw new Error('Could not find user that matches that user ID'); } return null; diff --git a/test/general/key.js b/test/general/key.js index 7e53d1af..4762b5de 100644 --- a/test/general/key.js +++ b/test/general/key.js @@ -1303,6 +1303,25 @@ p92yZgB3r2+f6/GIe2+7 expect(primUser.selfCertification).to.be.an.instanceof(openpgp.packet.Signature); }); + it('getPrimaryUser() should return null if no UIDs are bound', async function() { + const keyWithoundBoundUid = `-----BEGIN PGP PRIVATE KEY BLOCK----- + +xVgEWxpFkRYJKwYBBAHaRw8BAQdAYjjZLkp4qG7KAqJeVQlxQT6uCPq6rylV02nC +c6D/a8YAAP0YtS4UeLzeJGSgjGTlvSd3TWEsjxdGyzwfHCOejXMRbg2+zSFVbmJv +dW5kIFVJRCA8dW5ib3VuZEBleGFtcGxlLm9yZz7HXQRbGkWREgorBgEEAZdVAQUB +AQdAfxbFuoNlm5KfoqaWICETfMljzVTDAtiSM0TYSHzGAz8DAQoJAAD/cuu7bxUE +goBAhIyVH+pmvWpuDJbfu1Vaj5KiQxsKxJgP/MJ+BBgWCgAwApsMBYJbGkWRBYkB +3+IAFqEE30YL4fxJBMTm8ONLPOiTkVxEIS8JkDzok5FcRCEvAABb+gEAnQb/rMLO +Vz/bMCJoAShgybW1r6kRWejybzIjFSLnx/YA/iLZeo5UNdlXRJco+15RbFiNSAbw +VYGdb3eNlV8CfoEC +=FYbP +-----END PGP PRIVATE KEY BLOCK----- +`; + const key = openpgp.key.readArmored(keyWithoundBoundUid).keys[0]; + const primUser = await key.getPrimaryUser(); + expect(primUser).to.be.null; + }); + it('Generated key is not unlocked by default', function() { const opt = {numBits: 512, userIds: 'test ', passphrase: '123'}; if (openpgp.util.getWebCryptoAll()) { opt.numBits = 2048; } // webkit webcrypto accepts minimum 2048 bit keys