diff --git a/extension/js/common/core/common.ts b/extension/js/common/core/common.ts index ef98bcc5aa2..7724e604e45 100644 --- a/extension/js/common/core/common.ts +++ b/extension/js/common/core/common.ts @@ -87,8 +87,9 @@ export class Str { if (email.indexOf(' ') !== -1) { return false; } - email = email.replace(/\:8001$/, ''); // for MOCK tests until https://github.com/FlowCrypt/flowcrypt-browser/issues/4631 - return /^(([^<>()\[\]\\.,;:\s@"]+(\.[^<>()\[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/i.test( + email = email.replace(/\:8001$/, ''); // for MOCK tests, todo: remove from production + // `localhost` is a valid top-level domain for an email address, otherwise we require a second-level domain to be present + return /^(([^<>()\[\]\\.,;:\s@"]+(\.[^<>()\[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}])|localhost|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/i.test( email ); }; diff --git a/extension/js/common/core/crypto/pgp/openpgp-key.ts b/extension/js/common/core/crypto/pgp/openpgp-key.ts index b4c2b1560dc..a95ae1b5702 100644 --- a/extension/js/common/core/crypto/pgp/openpgp-key.ts +++ b/extension/js/common/core/crypto/pgp/openpgp-key.ts @@ -305,9 +305,10 @@ export class OpenPGPKey { const key = await OpenPGPKey.extractExternalLibraryObjFromKey(pubkey); const result = new Map(); result.set(`Is Private?`, KeyUtil.formatResult(key.isPrivate())); - for (let i = 0; i < key.users.length; i++) { + const users = await OpenPGPKey.verifyAllUsers(key); + for (let i = 0; i < users.length; i++) { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - result.set(`User id ${i}`, key.users[i].userID!.userID); + result.set(`User id ${i}`, (users[i].valid ? '' : '* REVOKED, INVALID OR MISSING SIGNATURE * ') + users[i].userID); } const user = await key.getPrimaryUser(); result.set(`Primary User`, user?.user?.userID?.userID || 'No primary user'); @@ -470,17 +471,6 @@ export class OpenPGPKey { return keyPacket.isDecrypted() === true; }; - public static getPrimaryUserId = async (pubs: OpenPGP.Key[], keyid: OpenPGP.KeyID): Promise => { - for (const opgpkey of pubs) { - const matchingKeys = opgpkey.getKeys(keyid); - if (matchingKeys.length > 0) { - const primaryUser = await opgpkey.getPrimaryUser(); - return primaryUser?.user?.userID?.userID; - } - } - return undefined; - }; - public static verify = async (msg: OpenpgpMsgOrCleartext, pubs: PubkeyInfo[]): Promise => { // todo: double-check if S/MIME ever gets here const validKeys = pubs.filter(x => !x.revoked && x.pubkey.family === 'openpgp').map(x => x.pubkey); @@ -546,39 +536,35 @@ export class OpenPGPKey { private static getExpirationAsDateOrUndefined = (expirationTime: Date | typeof Infinity | null) => { return expirationTime instanceof Date ? expirationTime : undefined; // we don't differ between Infinity and null }; - private static getUsersAndSelfCertifications = async (key: OpenPGP.Key) => { - const data = ( - await Promise.all( - key.users - .filter(user => user?.userID) - .map(async user => { - const dataToVerify = { userID: user.userID, key: key.keyPacket }; - const selfCertification = await OpenPGPKey.getLatestValidSignature( - user.selfCertifications, - key.keyPacket, - opgp.enums.signature.certGeneric, - dataToVerify - ); - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return { userid: user.userID!.userID, email: user.userID!.email, selfCertification }; - }) - ) - ).filter(x => x.selfCertification); - // sort the same way as OpenPGP.js does - data.sort((a, b) => { - const A = a.selfCertification!; // eslint-disable-line @typescript-eslint/no-non-null-assertion - const B = b.selfCertification!; // eslint-disable-line @typescript-eslint/no-non-null-assertion - return Number(A.revoked) - Number(B.revoked) || Number(B.isPrimaryUserID) - Number(A.isPrimaryUserID) || Number(B.created) - Number(A.created); - }); - return data; - }; - private static getSortedUserids = async (key: OpenPGP.Key): Promise<{ identities: string[]; emails: string[] }> => { - const data = await OpenPGPKey.getUsersAndSelfCertifications(key); - return { - identities: data.map(x => x.userid).filter(Boolean), - emails: data.map(x => x.email).filter(Boolean), // todo: toLowerCase()? - }; + // returns all the `key.users` preserving order with `valid` property + private static verifyAllUsers = async (key: OpenPGP.Key) => { + return await ( + key as unknown as { + // a type patch until https://github.com/openpgpjs/openpgpjs/pull/1594 is resolved + // eslint-disable-next-line no-null/no-null + verifyAllUsers(): Promise<{ userID: string; keyID: OpenPGP.KeyID; valid: boolean | null }[]>; + } + ).verifyAllUsers(); + }; + + private static getSortedUserids = async (key: OpenPGP.Key) => { + const primaryUser = await Catch.undefinedOnException(key.getPrimaryUser()); + // if there is no good enough user id to serve as primary identity, we assume other user ids are even worse + if (primaryUser?.user?.userID?.userID) { + const primaryUserId = primaryUser.user.userID.userID; + const identities = [ + primaryUserId, // put the "primary" identity first + // other identities go in indeterministic order + ...Value.arr.unique((await OpenPGPKey.verifyAllUsers(key)).filter(x => x.valid && x.userID !== primaryUserId).map(x => x.userID)), + ]; + const emails = identities.map(userid => Str.parseEmail(userid).email).filter(Boolean); + if (emails.length === identities.length) { + // OpenPGP.js uses RFC 5322 `email-addresses` parser, so we expect all identities to contain a valid e-mail address + return { emails, identities }; + } + } + return { emails: [], identities: [] }; }; // mimicks OpenPGP.helper.getLatestValidSignature @@ -669,7 +655,10 @@ export class OpenPGPKey { }; private static getPrimaryKeyFlags = async (key: OpenPGP.Key): Promise => { - const selfCertification = (await OpenPGPKey.getUsersAndSelfCertifications(key)).map(x => x.selfCertification).find(Boolean); + // Note: The selected selfCertification (and hence the flags) will differ based on the current date + const primaryUser = await Catch.undefinedOnException(key.getPrimaryUser()); + // a type patch until https://github.com/openpgpjs/openpgpjs/pull/1594 is resolved + const selfCertification = (primaryUser as { selfCertification: OpenPGP.SignaturePacket } | undefined)?.selfCertification; if (!selfCertification) { return 0; } diff --git a/extension/js/common/ui/key-import-ui.ts b/extension/js/common/ui/key-import-ui.ts index 9f8f4c1b39f..df473db28e3 100644 --- a/extension/js/common/ui/key-import-ui.ts +++ b/extension/js/common/ui/key-import-ui.ts @@ -112,19 +112,15 @@ export class KeyImportUi { Ui.event.handle(async target => { $('.action_add_private_key').addClass('btn_disabled').attr('disabled'); $('.input_email_alias').prop('checked', false); - const prv = await Catch.undefinedOnException(opgp.readKey({ armoredKey: String($(target).val()) })); + const prv = await Catch.undefinedOnException(KeyUtil.parse(String($(target).val()))); if (prv !== undefined) { $('.action_add_private_key').removeClass('btn_disabled').removeAttr('disabled'); if (submitKeyForAddrs !== undefined) { - const users = prv.users; - for (const user of users) { - const userId = user.userID; - if (userId) { - for (const inputCheckboxesWithEmail of $('.input_email_alias')) { - if (String($(inputCheckboxesWithEmail).data('email')) === userId.email) { - KeyImportUi.addAliasForSubmission(userId.email, submitKeyForAddrs); - $(inputCheckboxesWithEmail).prop('checked', true); - } + for (const email of prv.emails) { + for (const inputCheckboxesWithEmail of $('.input_email_alias')) { + if (String($(inputCheckboxesWithEmail).data('email')) === email) { + KeyImportUi.addAliasForSubmission(email, submitKeyForAddrs); + $(inputCheckboxesWithEmail).prop('checked', true); } } } diff --git a/test/source/tests/unit-node.ts b/test/source/tests/unit-node.ts index da4d1cb6be9..70c8abbd019 100644 --- a/test/source/tests/unit-node.ts +++ b/test/source/tests/unit-node.ts @@ -1391,6 +1391,42 @@ jSB6A93JmnQGIkAem/kzGkKclmfAdGfc4FS+3Cn+6Q==Xmrz t.pass(); }); + test('[KeyUtil.diagnose] correctly displays revoked userid', async t => { + const key = await KeyUtil.parse(`-----BEGIN PGP PRIVATE KEY BLOCK----- +Version: FlowCrypt Testing Only unspecified + +lFgEX6UIExYJKwYBBAHaRw8BAQdAMfHf64wPQ2LC9In5AKYU/KT1qWvI7e7aXr+L +WeQGUKIAAQCcB3zZlHfepQT26LIwbTDn4lvQ9LuD1fk2hK6i9FXFxxO7tBI8dXNl +ckBleGFtcGxlLmNvbT6IjwQQFgoAIAUCX6UIEwYLCQcIAwIEFQgKAgQWAgEAAhkB +AhsDAh4BACEJEEoCtcZ3snFuFiEENY1GQZqrKQqgUAXASgK1xneycW6P6AEA5iXF +K+fWpj0vn3xpKEuFRqvytPKFzhwd4wEvL+IGSPEBALE/pZdMzsDoKPENiLFpboDV +NVJScwFXIleKmtNaRycFiIwEExYIAD4FAmLqt7IJEEoCtcZ3snFuFiEENY1GQZqr +KQqgUAXASgK1xneycW4CngECmwMEFgIBAAYLCQcIAwIEFQgKAgAA7VwA/3x+J0i5 +DPaKtiosXHEV3LnOjaDGJgQlj7bR1BD4P62RAP0To1EcOvYk3qdgwda00oDkvYon +aAtVAK9dqadkbOI4D4h7BDAWCAAtBQJi6reyCRBKArXGd7JxbhYhBDWNRkGaqykK +oFAFwEoCtcZ3snFuAocAAh0gAABfXQEAvxCRqQz9r7iyrPyo4R/xF1BajPxoHd0Q +y4GYx/aIq5UA/19k0C/X7tH+fPJEd3Z2QjlrvyTbymUa+z4YGK1rh/YHtA9maXJz +dEBtb2NrLnRlc3SIjwQTFggAQQUCYuq3sgkQSgK1xneycW4WIQQ1jUZBmqspCqBQ +BcBKArXGd7JxbgKeAQKbAwQWAgEABgsJBwgDAgQVCAoCApkBAACNnQEA8tTL+tGS +wC9u4ECmo2Y8AUa0nvvv9+JmiMQphqldxD0A/jkDmtuj+KX8zxArkwC4IKCAFd2G +cdgj1z2/dAKVWmICnF0EX6UIExIKKwYBBAGXVQEFAQEHQBDdeawWVNqYkP8c/ihL +EUlVpn8cQw7rmRc/sIhdAXhfAwEIBwAA/0Jy7IelcHDjxE3OzagEzSxNrCVw8uPH +NRl8s6iP+CQYEfGIeAQYFggACQUCX6UIEwIbDAAhCRBKArXGd7JxbhYhBDWNRkGa +qykKoFAFwEoCtcZ3snFuWp8BAIzRBYJSfZzlvlyyPhrbXJoYSICGNy/5x7noXjp/ +ByeOAQDnTbQi4XwXJrU4A8Nl9eyz16ZWUzEPwfWgahIG1eQDDA== +=eyAR +-----END PGP PRIVATE KEY BLOCK-----`); + expect(key.identities).to.have.length(1); + expect(key.identities).to.eql(['first@mock.test']); + expect(key.emails).to.have.length(1); + expect(key.emails).to.eql(['first@mock.test']); + const result = await KeyUtil.diagnose(key, ''); + expect(result.get('Primary User')).to.equal('first@mock.test'); + expect(result.get('User id 0')).to.equal('* REVOKED, INVALID OR MISSING SIGNATURE * '); + expect(result.get('User id 1')).to.equal('first@mock.test'); + t.pass(); + }); + test('[KeyUtil.diagnose] displays PK and SK usage', async t => { const usageRegex = /\[\-\] \[(.*)\]/; /* eslint-disable @typescript-eslint/no-non-null-assertion */