From aa0d961438b3b64d7d66690b2122c626f40c7866 Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Tue, 11 May 2021 10:35:08 -0400 Subject: [PATCH 1/7] Revocation handling #3579 --- extension/js/background_page/migrations.ts | 2 +- .../js/common/platform/store/contact-store.ts | 129 ++++++++++++++---- .../browser-unit-tests/unit-ContactStore.js | 109 +++++++++++++++ test/source/tests/tooling/consts.ts | 33 +++++ 4 files changed, 246 insertions(+), 27 deletions(-) diff --git a/extension/js/background_page/migrations.ts b/extension/js/background_page/migrations.ts index f4fd7a77227..3a70348a791 100644 --- a/extension/js/background_page/migrations.ts +++ b/extension/js/background_page/migrations.ts @@ -160,7 +160,7 @@ const moveContactsBatchToEmailsAndPubkeys = async (db: IDBDatabase, count?: numb }; })); { - const tx = db.transaction(['contacts', 'emails', 'pubkeys'], 'readwrite'); + const tx = db.transaction(['contacts', 'emails', 'pubkeys', 'revocations'], 'readwrite'); await new Promise((resolve, reject) => { ContactStore.setTxHandlers(tx, resolve, reject); for (const item of converted) { diff --git a/extension/js/common/platform/store/contact-store.ts b/extension/js/common/platform/store/contact-store.ts index 0146a673008..3b8a0653f97 100644 --- a/extension/js/common/platform/store/contact-store.ts +++ b/extension/js/common/platform/store/contact-store.ts @@ -33,6 +33,10 @@ export type Pubkey = { expiresOn: number | null; }; +type Revocation = { + fingerprint: string; +}; + type PubkeyAttributes = { fingerprint: string | null; expiresOn: number | null; @@ -40,7 +44,8 @@ type PubkeyAttributes = { export type ContactV4 = { info: Email, - pubkeys: Pubkey[] + pubkeys: Pubkey[], + revocations: Revocation[] } export type ContactPreview = { @@ -59,6 +64,8 @@ export type ContactUpdate = { type DbContactFilter = { hasPgp?: boolean, substring?: string, limit?: number }; +const x509postfix = "-X509"; + /** * Store of contacts and their public keys * This includes an index of email and name substrings for easier search when user is typing @@ -72,7 +79,7 @@ export class ContactStore extends AbstractStore { public static dbOpen = async (): Promise => { return await new Promise((resolve, reject) => { - const openDbReq = indexedDB.open('cryptup', 4); + const openDbReq = indexedDB.open('cryptup', 5); openDbReq.onupgradeneeded = (event) => { const db = openDbReq.result; if (event.oldVersion < 4) { @@ -82,6 +89,9 @@ export class ContactStore extends AbstractStore { emails.createIndex('index_fingerprints', 'fingerprints', { multiEntry: true }); // fingerprints of all connected pubkeys pubkeys.createIndex('index_longids', 'longids', { multiEntry: true }); // longids of all public key packets in armored pubkey } + if (event.oldVersion < 5) { + db.createObjectStore('revocations', { keyPath: 'fingerprint' }); + } if (db.objectStoreNames.contains('contacts')) { const countRequest = openDbReq.transaction!.objectStore('contacts').count(); ContactStore.setReqPipe(countRequest, (count: number) => { @@ -199,7 +209,7 @@ export class ContactStore extends AbstractStore { Catch.report(`Wrongly updating prv ${update.pubkey.id} as contact - converting to pubkey`); update.pubkey = await KeyUtil.asPublicKey(update.pubkey); } - const tx = db.transaction(['emails', 'pubkeys'], 'readwrite'); + const tx = db.transaction(['emails', 'pubkeys', 'revocations'], 'readwrite'); await new Promise((resolve, reject) => { ContactStore.setTxHandlers(tx, resolve, reject); ContactStore.updateTx(tx, validEmail, update); @@ -233,8 +243,9 @@ export class ContactStore extends AbstractStore { } public static getOneWithAllPubkeys = async (db: IDBDatabase, email: string): Promise => { - const tx = db.transaction(['emails', 'pubkeys'], 'readonly'); + const tx = db.transaction(['emails', 'pubkeys', 'revocations'], 'readonly'); const pubkeys: Pubkey[] = []; + const revocations: Revocation[] = []; const emailEntity: Email | undefined = await new Promise((resolve, reject) => { const req = tx.objectStore('emails').get(email); ContactStore.setReqPipe(req, @@ -247,7 +258,10 @@ export class ContactStore extends AbstractStore { resolve(email); return; } - let countdown = email.fingerprints.length; + const uniqueAndStrippedFingerprints = email.fingerprints. + map(ContactStore.stripFingerprint). + filter((value, index, self) => !self.slice(0, index).find((el) => el === value)); + let countdown = email.fingerprints.length + uniqueAndStrippedFingerprints.length; // request all pubkeys by fingerprints for (const fp of email.fingerprints) { const req2 = tx.objectStore('pubkeys').get(fp); @@ -262,18 +276,36 @@ export class ContactStore extends AbstractStore { }, reject); } + for (const fp of uniqueAndStrippedFingerprints) { + const range = ContactStore.createFingerprintRange(fp); + const req3 = tx.objectStore('revocations').getAll(range); + ContactStore.setReqPipe(req3, + (revocation: Revocation[]) => { + revocations.push(...revocation); + if (!--countdown) { + resolve(email); + } + }, + reject); + } }, reject); }); - return emailEntity ? { info: emailEntity, pubkeys } : undefined; + return emailEntity ? { info: emailEntity, pubkeys, revocations } : undefined; } public static updateTx = (tx: IDBTransaction, email: string, update: ContactUpdate) => { if (update.pubkey && !update.pubkeyLastCheck) { const req = tx.objectStore('pubkeys').get(ContactStore.getPubkeyId(update.pubkey)); - ContactStore.setReqPipe(req, (pubkey: Pubkey) => ContactStore.updateTxPhase2(tx, email, update, pubkey)); + ContactStore.setReqPipe(req, (pubkey: Pubkey) => { + const range = ContactStore.createFingerprintRange(update.pubkey!.id); + const req2 = tx.objectStore('revocations').getAll(range); + ContactStore.setReqPipe(req2, (revocations: Revocation[]) => { + ContactStore.updateTxPhase2(tx, email, update, pubkey, revocations); + }); + }); } else { - ContactStore.updateTxPhase2(tx, email, update, undefined); + ContactStore.updateTxPhase2(tx, email, update, undefined, []); } } @@ -306,13 +338,39 @@ export class ContactStore extends AbstractStore { } private static getPubkeyId = (pubkey: Key): string => { - return (pubkey.type === 'x509') ? (pubkey.id + '-X509') : pubkey.id; + return (pubkey.type === 'x509') ? (pubkey.id + x509postfix) : pubkey.id; + } + + private static stripFingerprint = (fp: string): string => { + return fp.endsWith(x509postfix) ? fp.slice(0, -x509postfix.length) : fp; + } + + private static equalFingerprints = (fp1: string, fp2: string): boolean => { + return (fp1.endsWith(x509postfix) ? fp1 : (fp1 + x509postfix)) + === (fp2.endsWith(x509postfix) ? fp2 : (fp2 + x509postfix)); } - private static updateTxPhase2 = (tx: IDBTransaction, email: string, update: ContactUpdate, existingPubkey: Pubkey | undefined) => { + private static createFingerprintRange = (fp: string): IDBKeyRange => { + const strippedFp = ContactStore.stripFingerprint(fp); + return IDBKeyRange.bound(strippedFp, strippedFp + x509postfix, false, false); + } + + private static updateTxPhase2 = (tx: IDBTransaction, email: string, update: ContactUpdate, + existingPubkey: Pubkey | undefined, revocations: Revocation[]) => { let pubkeyEntity: Pubkey | undefined; if (update.pubkey) { - pubkeyEntity = ContactStore.pubkeyObj(update.pubkey, update.pubkeyLastCheck ?? existingPubkey?.lastCheck); + const internalFingerprint = ContactStore.getPubkeyId(update.pubkey!); + if (update.pubkey.type === 'openpgp' && !update.pubkey.revoked && revocations.some(r => r.fingerprint === internalFingerprint)) { + // we have this fingerprint revoked but the supplied key isn't + // so let's not save it + // pubkeyEntity = undefined + } else { + pubkeyEntity = ContactStore.pubkeyObj(update.pubkey, update.pubkeyLastCheck ?? existingPubkey?.lastCheck); + } + if (update.pubkey.revoked && !revocations.some(r => r.fingerprint === internalFingerprint)) { + tx.objectStore('revocations').put({ fingerprint: internalFingerprint, armoredKey: KeyUtil.armor(update.pubkey) }); + // todo: we can add a timestamp here and/or some other info + } // todo: will we benefit anything when not saving pubkey if it isn't modified? } else if (update.pubkeyLastCheck) { Catch.report(`Wrongly updating pubkeyLastCheck without specifying pubkey for ${email} - ignoring`); @@ -467,22 +525,30 @@ export class ContactStore extends AbstractStore { if (!contactWithAllPubkeys) { return contactWithAllPubkeys; } - if (!contactWithAllPubkeys.pubkeys.length) { - return await ContactStore.toContact(contactWithAllPubkeys.info, undefined); - } // parse the keys - const parsed = await Promise.all(contactWithAllPubkeys.pubkeys.map(async (pubkey) => { return { lastCheck: pubkey.lastCheck, pubkey: await KeyUtil.parse(pubkey.armoredKey) }; })); - // sort non-expired first, pick first usableForEncryption - const sorted = parsed.sort((a, b) => (typeof b.pubkey.expiration === 'undefined') ? Infinity : b.pubkey.expiration! - - ((typeof a.pubkey.expiration === 'undefined') ? Infinity : a.pubkey.expiration!)); - let selected = sorted.find(entry => entry.pubkey.usableForEncryption); + const parsed = await Promise.all(contactWithAllPubkeys.pubkeys.map(async (pubkey) => { + const pk = await KeyUtil.parse(pubkey.armoredKey); + const revoked = pk.revoked || contactWithAllPubkeys.revocations.some(r => ContactStore.equalFingerprints(pk.id, r.fingerprint)); + const expirationSortValue = (typeof pk.expiration === 'undefined') ? Infinity : pk.expiration!; + return { + lastCheck: pubkey.lastCheck, + pubkey: pk, + revoked, + // sort non-revoked first, then non-expired + sortValue: revoked ? -Infinity : expirationSortValue + } + })); + const sorted = parsed.sort((a, b) => b.sortValue - a.sortValue); + // pick first usableForEncryption + let selected = sorted.find(entry => !entry.revoked && entry.pubkey.usableForEncryption); if (!selected) { - selected = sorted.find(entry => entry.pubkey.usableForEncryptionButExpired); + selected = sorted.find(entry => !entry.revoked && entry.pubkey.usableForEncryptionButExpired); } if (!selected) { selected = sorted[0]; } - return ContactStore.toContactFromKey(contactWithAllPubkeys.info, selected.pubkey, selected.lastCheck); + const safeKey = (selected.revoked && !selected.pubkey.revoked) ? undefined : selected; + return ContactStore.toContactFromKey(contactWithAllPubkeys.info, safeKey?.pubkey, safeKey?.lastCheck); } // search all longids const tx = db.transaction(['emails', 'pubkeys'], 'readonly'); @@ -500,7 +566,7 @@ export class ContactStore extends AbstractStore { if (!email) { resolve(undefined); } else { - resolve(ContactStore.toContact(email, pubkey)); + resolve(ContactStore.toContact(db, email, pubkey)); } }, reject); @@ -513,15 +579,26 @@ export class ContactStore extends AbstractStore { return { fingerprint: key?.id ?? null, expiresOn: DateUtility.asNumber(key?.expiration) }; } - private static toContact = async (email: Email, pubkey: Pubkey | undefined): Promise => { + private static toContact = async (db: IDBDatabase, email: Email, pubkey: Pubkey | undefined): Promise => { if (!email) { return; } - const parsed = pubkey ? await KeyUtil.parse(pubkey.armoredKey) : undefined; + let parsed = pubkey ? await KeyUtil.parse(pubkey.armoredKey) : undefined; + if (parsed && !parsed.revoked) { + const revocations: Revocation[] = await new Promise((resolve, reject) => { + const tx = db.transaction(['revocations'], 'readonly'); + const range = ContactStore.createFingerprintRange(parsed!.id); + const req = tx.objectStore('revocations').getAll(range); + ContactStore.setReqPipe(req, resolve, reject); + }); + if (revocations.length) { + parsed = undefined; + } + } return ContactStore.toContactFromKey(email, parsed, parsed ? pubkey!.lastCheck : null); } - private static toContactFromKey = (email: Email, key: Key | undefined, lastCheck: number | null): Contact | undefined => { + private static toContactFromKey = (email: Email, key: Key | undefined, lastCheck: number | undefined | null): Contact | undefined => { if (!email) { return; } @@ -531,7 +608,7 @@ export class ContactStore extends AbstractStore { pubkey: key, hasPgp: key ? 1 : 0, lastUse: email.lastUse, - pubkeyLastCheck: lastCheck, + pubkeyLastCheck: lastCheck ?? null, ...ContactStore.getKeyAttributes(key) }; } diff --git a/test/source/tests/browser-unit-tests/unit-ContactStore.js b/test/source/tests/browser-unit-tests/unit-ContactStore.js index 1bed5ce13f6..24295420bfd 100644 --- a/test/source/tests/browser-unit-tests/unit-ContactStore.js +++ b/test/source/tests/browser-unit-tests/unit-ContactStore.js @@ -422,3 +422,112 @@ BROWSER_UNIT_TEST_NAME(`ContactStore searches S/MIME Certificate by PKCS#7 messa } return 'pass'; })(); + +BROWSER_UNIT_TEST_NAME(`ContactStore: X-509 revocation affects OpenPGP key`); +(async () => { + const db = await ContactStore.dbOpen(); + const opgpKeyOldAndValid = await KeyUtil.parse(testConstants.somerevokedValid); + const fingerprint = 'D6662C5FB9BDE9DA01F3994AAA1EF832D8CCA4F2'; + if (opgpKeyOldAndValid.id !== fingerprint) { + throw new Error(`Valid OpenPGP Key is expected to have fingerprint ${fingerprint} but actually is ${opgpKeyOldAndValid.id}`); + } + await ContactStore.update(db, 'some.revoked@localhost.com', { pubkey: opgpKeyOldAndValid }); + const [loadedOpgpKey1] = await ContactStore.get(db, [`some.revoked@localhost.com`]); + if (loadedOpgpKey1.pubkey.revoked) { + throw new Error(`The loaded OpenPGP Key (1) was expected to be valid but it is revoked.`); + } + const [loadedOpgpKey2] = await ContactStore.get(db, [`AA1EF832D8CCA4F2`]); + if (loadedOpgpKey2.pubkey.revoked) { + throw new Error(`The loaded OpenPGP Key (2) was expected to be valid but it is revoked.`); + } + // emulate X-509 revocation + await new Promise((resolve, reject) => { + const tx = db.transaction(['revocations'], 'readwrite'); + ContactStore.setTxHandlers(tx, resolve, reject); + tx.objectStore('revocations').put({ fingerprint: fingerprint + "-X509"}); + }); + // original key should be either revoked or missing + const [loadedOpgpKey3] = await ContactStore.get(db, [`some.revoked@localhost.com`]); + if (loadedOpgpKey3.pubkey && !loadedOpgpKey3.pubkey.revoked) { + throw new Error(`The loaded OpenPGP Key (3) was expected to be revoked but it is not.`); + } + const [loadedOpgpKey4] = await ContactStore.get(db, [`AA1EF832D8CCA4F2`]); + if (loadedOpgpKey4.pubkey && !loadedOpgpKey4.pubkey.revoked) { + throw new Error(`The loaded OpenPGP Key (4) was expected to be revoked but it is not.`); + } + return 'pass'; +})(); + +BROWSER_UNIT_TEST_NAME(`ContactStore: OpenPGP revocation affects X.509 certificate`); +(async () => { + const db = await ContactStore.dbOpen(); + const smimeKey = await KeyUtil.parse(testConstants.smimeCert); + await ContactStore.update(db, 'actalis@meta.33mail.com', { pubkey: smimeKey }); + const [loadedCert1] = await ContactStore.get(db, [`actalis@meta.33mail.com`]); + const longid = KeyUtil.getPrimaryLongid(smimeKey); + if (loadedCert1.pubkey.revoked) { + throw new Error(`The loaded X.509 certificate (1) was expected to be valid but it is revoked.`); + } + const [loadedCert2] = await ContactStore.get(db, [longid]); + if (loadedCert2.pubkey.revoked) { + throw new Error(`The loaded X.509 certificate (2) was expected to be valid but it is revoked.`); + } + // emulate openPGP revocation + await new Promise((resolve, reject) => { + const tx = db.transaction(['revocations'], 'readwrite'); + ContactStore.setTxHandlers(tx, resolve, reject); + tx.objectStore('revocations').put({ fingerprint: ContactStore.stripFingerprint(smimeKey.id)}); + }); + // original key should be either revoked or missing + const [loadedCert3] = await ContactStore.get(db, [`actalis@meta.33mail.com`]); + if (loadedCert3.pubkey && !loadedCert3.pubkey.revoked) { + throw new Error(`The loaded X.509 certificate (3) was expected to be revoked but it is not.`); + } + const [loadedCert4] = await ContactStore.get(db, [longid]); + if (loadedCert4.pubkey && !loadedCert4.pubkey.revoked) { + throw new Error(`The loaded X.509 certificate (4) was expected to be revoked but it is not.`); + } + return 'pass'; +})(); + +BROWSER_UNIT_TEST_NAME(`ContactStore doesn't replace revoked key with older version`); +(async () => { + const db = await ContactStore.dbOpen(); + const opgpKeyOldAndValid = await KeyUtil.parse(testConstants.somerevokedValid); + const fingerprint = 'D6662C5FB9BDE9DA01F3994AAA1EF832D8CCA4F2'; + if (opgpKeyOldAndValid.id !== fingerprint) { + throw new Error(`Valid OpenPGP Key is expected to have fingerprint ${fingerprint} but actually is ${opgpKeyOldAndValid.id}`); + } + const opgpKeyRevoked = await KeyUtil.parse(testConstants.somerevokedValidNowRevoked); + if (opgpKeyRevoked.id !== fingerprint) { + throw new Error(`RevokedOpenPGP Key is expected to have fingerprint ${fingerprint} but actually is ${opgpKeyRevoked.id}`); + } + await ContactStore.update(db, 'some.revoked@localhost.com', { pubkey: opgpKeyOldAndValid }); + const [loadedOpgpKey1] = await ContactStore.get(db, [`some.revoked@localhost.com`]); + if (loadedOpgpKey1.pubkey.revoked) { + throw new Error(`The loaded OpenPGP Key (1) was expected to be valid but it is revoked.`); + } + const [loadedOpgpKey2] = await ContactStore.get(db, [`AA1EF832D8CCA4F2`]); + if (loadedOpgpKey2.pubkey.revoked) { + throw new Error(`The loaded OpenPGP Key (2) was expected to be valid but it is revoked.`); + } + await ContactStore.update(db, 'some.revoked@localhost.com', { pubkey: opgpKeyRevoked }); + const [loadedOpgpKey3] = await ContactStore.get(db, [`some.revoked@localhost.com`]); + if (loadedOpgpKey3.pubkey && !loadedOpgpKey3.pubkey.revoked) { + throw new Error(`The loaded OpenPGP Key (3) was expected to be revoked but it is not.`); + } + const [loadedOpgpKey4] = await ContactStore.get(db, [`AA1EF832D8CCA4F2`]); + if (loadedOpgpKey4.pubkey && !loadedOpgpKey4.pubkey.revoked) { + throw new Error(`The loaded OpenPGP Key (4) was expected to be revoked but it is not.`); + } + await ContactStore.update(db, 'some.revoked@localhost.com', { pubkey: opgpKeyOldAndValid }); + const [loadedOpgpKey5] = await ContactStore.get(db, [`some.revoked@localhost.com`]); + if (loadedOpgpKey5.pubkey && !loadedOpgpKey5.pubkey.revoked) { + throw new Error(`The loaded OpenPGP Key (5) was expected to be revoked but it is not.`); + } + const [loadedOpgpKey6] = await ContactStore.get(db, [`AA1EF832D8CCA4F2`]); + if (loadedOpgpKey6.pubkey && !loadedOpgpKey6.pubkey.revoked) { + throw new Error(`The loaded OpenPGP Key (6) was expected to be revoked but it is not.`); + } + return 'pass'; +})(); diff --git a/test/source/tests/tooling/consts.ts b/test/source/tests/tooling/consts.ts index e7d3fb14cfc..51087d7f341 100644 --- a/test/source/tests/tooling/consts.ts +++ b/test/source/tests/tooling/consts.ts @@ -703,6 +703,39 @@ wPK57RZ8W/IQ7x76k7S44m634e6usKnD+reitX1QWi3vel8HC4qxviu/xLbIJyjMR1IgPsUWaMAe DC024L0txF5zDnbODx9X1LM+/8D1pVizUjOwt1liPq0hh2JKU8iLqzdSkv0dte0UbEUPMyCVp8h6 scbnq9KEwLGCMJ0IkCSUNA== =iXGJ +-----END PGP PUBLIC KEY BLOCK-----`, + somerevokedValidNowRevoked: // some.revoked@localhost + // valid key with fingerprint D666 2C5F B9BD E9DA 01F3 994A AA1E F832 D8CC A4F2 + `-----BEGIN PGP PUBLIC KEY BLOCK----- + +xsBNBGAeYQ0BCADHMOjbN/X/TH4JpTz7Sj1VTGIeXzWUVZIbsjLgp8U0dFo2zWXMsgLsnNAZuL43 +pUAnIqw+wvUcSpndEO79upVvUzc1qgvp2DTJuDrVGAPx1cqKOi3A/XPO0uIxTyCChcQBQ+YUvwc6 +7ZU69irRC320AQC5aFrL+yP7RmlWQgslJ0qJXPa3On6Cp71GL26iADPXnQOqZtmhv87nYlHhimOv +bKLtC/YMTqGk0h7HqNQPcP8B6bylofS/7Rgy+JKsqWmlng+U/0uQWsnfIua0BPkrZYwJdaF77cs1 +7A2LV2glUiG7XzPkHPTMtG3xV7ZbiAsLSwWN7x1mG3uvpppeXkd1ABEBAAHCwHYEIAEIACAWIQTW +Zixfub3p2gHzmUqqHvgy2Myk8gUCYJjo5gIdAAAKCRCqHvgy2Myk8jKYB/9MOZCet/lgGb9NiXO9 +BaN/c1hBjnzIDokZ2vUN7ZOUm7FdRtXmtlm8H69YV0+nIOw0tjPk9YavFyd1tIvARMwZnL9huirE +PrSnDT1k8E0Hrs170pY/yOWTP76xwZsihA8Qjf+SaCigKM3mjIRIC41eF1IWrisrPT05Tir7QfOO +6dsoDyVbfe2a5FsH0DWZkvwHlY3nUmN5mMWqbnO1mmCDAfRDOgedqNEAvewARG0vKbsgzLRkEHCt +mMqK+eWkdlgKVAmP1geJoBY4XgJOR+G+ANYCIrftotnTx0tCGlti+MtjzPSv6Kwj1k+wXFnmUz4f +DqmVsfctsNoCu11tkKPdzRZzb21lLnJldm9rZWRAbG9jYWxob3N0wsCJBBMBCAAzFiEE1mYsX7m9 +6doB85lKqh74MtjMpPIFAmAeYRUCGwMFCwkIBwIGFQgJCgsCBRYCAwEAAAoJEKoe+DLYzKTyOyUH +/A58OF2S4EfJWVdEuHvSYRv3EKoSM1ylQBXDBRD1lRB31GEwQg5kz6YuJva5eT/U+9E14zG6W+gc +77k0vjmR38mo/GAd/QuRSrahHpqDMjvlTube2yOI1DBt1LtpLsIDeiNoPI2HgOkmHKFDejRyKUE5 +EtgVtiCAlqlIdPRH3Fb3UG2SmzC2qEC3yb083oB+yz7jcbL37oVT7S/VsziIEhSg5n0J6R5biUSn +AISvmmobVHrXbAXpgaz+yWzSK3iG+vqoZl+KufLhlVBa7nEo0cBaqpcUqOt83tY7Ms6Tr33c7Ggz +DhtciF9vHKnzmSaQ37eZ0I58540kydyOrRh7S6rOwE0EYB5hFQEIAKzecZMc/NAm35vVDk91OcD5 +GMp/ywO1AH3HapPq4C8vt3RiLZ1LNi6mvO3202DH9mY9nxZ7iCdvQ3D8UQRfbPBW3i8dOMJs78OU +0RPACr4OMBhZVpvqUzropOtOvZP9Pa4Am+Xzn8RB+jSgKUJXWmRi1pvE4TV+o799D8CIwfzcHwmh +roOnoS8lxlUibj4T+aGFlK/3fqaWOXDbjmxPN2QHDhGIHVxcxEc089uQ5zt/L4RpZAWWZAWhqSzX +yG0+4PVK+AFZDMyXjiyEbXexw41k/JsMNw1pclG3ToUKBRG0BNqFZmAQWF2DMkucLUU7n6XDWE/l +AvNVvJ7F96w9id8AEQEAAcLAdgQYAQgAIBYhBNZmLF+5venaAfOZSqoe+DLYzKTyBQJgHmEaAhsM +AAoJEKoe+DLYzKTyaeUIAKWfgZvL/xIsISAZ78gLF5VXjJyo51/KYyaLmLSMJODKK8mekYrP1Q9F +hhuZ5oA+cfQm1jS4fqVI+CONbsjCG7fvqucZXOqsOBrGVVJHEyiAESqT1b8ZHGR5QW7ZDY/lEt9Z +b5AZSz6CMp9P0wd2sPUsbvg2NPrTkRjSOlZADsDyue0WfFvyEO8e+pO0uOJut+HurrCpw/q3orV9 +UFot73pfBwuKsb4rv8S2yCcozEdSID7FFmjAHgwtNuC9LcRecw52zg8fV9SzPv/A9aVYs1IzsLdZ +Yj6tIYdiSlPIi6s3UpL9HbXtFGxFDzMglafIerHG56vShMCxgjCdCJAklDQ= +=Khq/ -----END PGP PUBLIC KEY BLOCK-----`, somerevokedRevoked2: // some.revoked@localhost // revoked key with fingerprint 3930 7525 56D5 7C46 A1C5 6B63 DE85 38DD A164 8C76 From dc9bd956f6147bbc3bf0cbe3cecd76482a65e85c Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Tue, 11 May 2021 10:47:56 -0400 Subject: [PATCH 2/7] tslint fix --- extension/js/common/platform/store/contact-store.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extension/js/common/platform/store/contact-store.ts b/extension/js/common/platform/store/contact-store.ts index 3b8a0653f97..0dff3fa25d3 100644 --- a/extension/js/common/platform/store/contact-store.ts +++ b/extension/js/common/platform/store/contact-store.ts @@ -536,7 +536,7 @@ export class ContactStore extends AbstractStore { revoked, // sort non-revoked first, then non-expired sortValue: revoked ? -Infinity : expirationSortValue - } + }; })); const sorted = parsed.sort((a, b) => b.sortValue - a.sortValue); // pick first usableForEncryption From f612a491c3389901c2eded266ea2a671470606c1 Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Fri, 14 May 2021 06:07:57 -0400 Subject: [PATCH 3/7] fix --- extension/js/common/platform/store/contact-store.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extension/js/common/platform/store/contact-store.ts b/extension/js/common/platform/store/contact-store.ts index 0dff3fa25d3..59537caca93 100644 --- a/extension/js/common/platform/store/contact-store.ts +++ b/extension/js/common/platform/store/contact-store.ts @@ -547,7 +547,7 @@ export class ContactStore extends AbstractStore { if (!selected) { selected = sorted[0]; } - const safeKey = (selected.revoked && !selected.pubkey.revoked) ? undefined : selected; + const safeKey = (selected?.revoked && !selected.pubkey.revoked) ? undefined : selected; return ContactStore.toContactFromKey(contactWithAllPubkeys.info, safeKey?.pubkey, safeKey?.lastCheck); } // search all longids From 6f7763283301f309da4b6444b83f1814f0a88c9a Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Fri, 14 May 2021 14:17:14 -0400 Subject: [PATCH 4/7] Added migration to add revocation entries for already revoked keys --- .../js/background_page/background_page.ts | 3 ++- extension/js/background_page/migrations.ts | 26 +++++++++++++++++++ .../js/common/platform/store/contact-store.ts | 8 ++++-- .../js/common/platform/store/global-store.ts | 3 ++- 4 files changed, 36 insertions(+), 4 deletions(-) diff --git a/extension/js/background_page/background_page.ts b/extension/js/background_page/background_page.ts index 4aeac512da8..cf80c493f29 100644 --- a/extension/js/background_page/background_page.ts +++ b/extension/js/background_page/background_page.ts @@ -9,7 +9,7 @@ import { Catch } from '../common/platform/catch.js'; import { GoogleAuth } from '../common/api/email-provider/gmail/google-auth.js'; import { VERSION } from '../common/core/const.js'; import { injectFcIntoWebmail } from './inject.js'; -import { migrateGlobal, moveContactsToEmailsAndPubkeys, updateX509FingerprintsAndLongids } from './migrations.js'; +import { migrateGlobal, moveContactsToEmailsAndPubkeys, updateOpgpRevocations, updateX509FingerprintsAndLongids } from './migrations.js'; import { opgp } from '../common/core/crypto/pgp/openpgpjs-custom.js'; import { GlobalStoreDict, GlobalStore } from '../common/platform/store/global-store.js'; import { ContactStore } from '../common/platform/store/contact-store.js'; @@ -41,6 +41,7 @@ opgp.initWorker({ path: '/lib/openpgp.worker.js' }); try { db = await ContactStore.dbOpen(); // takes 4-10 ms first time + await updateOpgpRevocations(db); await updateX509FingerprintsAndLongids(db); await moveContactsToEmailsAndPubkeys(db); } catch (e) { diff --git a/extension/js/background_page/migrations.ts b/extension/js/background_page/migrations.ts index 3a70348a791..7a72ba1391b 100644 --- a/extension/js/background_page/migrations.ts +++ b/extension/js/background_page/migrations.ts @@ -110,6 +110,32 @@ export const updateX509FingerprintsAndLongids = async (db: IDBDatabase): Promise console.info('done updating'); }; +export const updateOpgpRevocations = async (db: IDBDatabase): Promise => { + const globalStore = await GlobalStore.get(['contact_store_opgp_revoked_flags_updated']); + if (globalStore.contact_store_opgp_revoked_flags_updated) { + return; + } + console.info('updating ContactStorage to revoked flags of OpenPGP keys...'); + const tx = db.transaction(['pubkeys'], 'readonly'); + const pubkeys: Pubkey[] = await new Promise((resolve, reject) => { + const search = tx.objectStore('pubkeys').getAll(); + ContactStore.setReqPipe(search, resolve, reject); + }); + const revokedKeys = (await Promise.all(pubkeys.filter(entity => KeyUtil.getKeyType(entity.armoredKey) === 'openpgp'). + map(async (entity) => await KeyUtil.parse(entity.armoredKey)))). + filter(k => k.revoked); + const txUpdate = db.transaction(['revocations'], 'readwrite'); + await new Promise((resolve, reject) => { + ContactStore.setTxHandlers(txUpdate, resolve, reject); + const revocationsStore = txUpdate.objectStore('revocations'); + for (const revokedKey of revokedKeys) { + revocationsStore.put(ContactStore.revocationObj(revokedKey)); + } + }); + await GlobalStore.set({ contact_store_opgp_revoked_flags_updated: true }); + console.info('done updating'); +}; + export const moveContactsToEmailsAndPubkeys = async (db: IDBDatabase): Promise => { if (!db.objectStoreNames.contains('contacts')) { return; diff --git a/extension/js/common/platform/store/contact-store.ts b/extension/js/common/platform/store/contact-store.ts index 59537caca93..d46b72c9e5b 100644 --- a/extension/js/common/platform/store/contact-store.ts +++ b/extension/js/common/platform/store/contact-store.ts @@ -337,6 +337,11 @@ export class ContactStore extends AbstractStore { }; } + public static revocationObj = (pubkey: Key): { fingerprint: string, armoredKey: string } => { + return { fingerprint: ContactStore.getPubkeyId(pubkey), armoredKey: KeyUtil.armor(pubkey) }; + // todo: we can add a timestamp here and/or some other info + } + private static getPubkeyId = (pubkey: Key): string => { return (pubkey.type === 'x509') ? (pubkey.id + x509postfix) : pubkey.id; } @@ -368,8 +373,7 @@ export class ContactStore extends AbstractStore { pubkeyEntity = ContactStore.pubkeyObj(update.pubkey, update.pubkeyLastCheck ?? existingPubkey?.lastCheck); } if (update.pubkey.revoked && !revocations.some(r => r.fingerprint === internalFingerprint)) { - tx.objectStore('revocations').put({ fingerprint: internalFingerprint, armoredKey: KeyUtil.armor(update.pubkey) }); - // todo: we can add a timestamp here and/or some other info + tx.objectStore('revocations').put(ContactStore.revocationObj(update.pubkey)); } // todo: will we benefit anything when not saving pubkey if it isn't modified? } else if (update.pubkeyLastCheck) { diff --git a/extension/js/common/platform/store/global-store.ts b/extension/js/common/platform/store/global-store.ts index 8a3e447b342..77920488299 100644 --- a/extension/js/common/platform/store/global-store.ts +++ b/extension/js/common/platform/store/global-store.ts @@ -19,11 +19,12 @@ export type GlobalStoreDict = { install_mobile_app_notification_dismissed?: boolean; key_info_store_fingerprints_added?: boolean; contact_store_x509_fingerprints_and_longids_updated?: boolean; + contact_store_opgp_revoked_flags_updated?: boolean; }; export type GlobalIndex = 'version' | 'account_emails' | 'settings_seen' | 'hide_pass_phrases' | 'dev_outlook_allow' | 'admin_codes' | 'install_mobile_app_notification_dismissed' | 'key_info_store_fingerprints_added' | - 'contact_store_x509_fingerprints_and_longids_updated'; + 'contact_store_x509_fingerprints_and_longids_updated' | 'contact_store_opgp_revoked_flags_updated'; /** * Locally stored data that is not associated with any email account From ca7003e40407f8d375968eec00cd8393cadc58e7 Mon Sep 17 00:00:00 2001 From: Tom J Date: Sat, 15 May 2021 17:44:09 +0000 Subject: [PATCH 5/7] updated mock tests to stop using real backend --- test/source/mock/backend/backend-endpoints.ts | 40 +++++-------------- test/source/test.ts | 8 +++- test/source/tests/flaky.ts | 11 +---- 3 files changed, 17 insertions(+), 42 deletions(-) diff --git a/test/source/mock/backend/backend-endpoints.ts b/test/source/mock/backend/backend-endpoints.ts index 9fedfdb9007..809079e6a12 100644 --- a/test/source/mock/backend/backend-endpoints.ts +++ b/test/source/mock/backend/backend-endpoints.ts @@ -1,7 +1,5 @@ /* ©️ 2016 - present FlowCrypt a.s. Limitations apply. Contact human@flowcrypt.com */ -import * as request from 'fc-node-requests'; - import { HttpAuthErr, HttpClientErr } from '../lib/api'; import { BackendData } from './backend-data'; @@ -14,30 +12,6 @@ import { expect } from 'chai'; export const mockBackendData = new BackendData(oauth); -const fwdToRealBackend = async (parsed: any, req: IncomingMessage): Promise => { - // we are forwarding this request to backend, but we are not properly authenticated with real backend - // better remove authentication: requests that we currently forward during tests don't actually require it - delete req.headers.host; - delete req.headers['content-length']; - const forwarding: any = { headers: req.headers, url: `https://flowcrypt.com${req.url}` }; - if (req.url!.includes('message/upload')) { - // Removing mock auth when forwarding request to real backend at ${req.url} - // here a bit more difficult, because the request was already encoded as form-data - parsed.body = (parsed.body as string).replace(/(-----END PGP MESSAGE-----\r\n\r\n------[A-Za-z0-9]+)(.|\r\n)*$/gm, (_, found) => `${found}--\r\n`); - forwarding.body = parsed.body; // FORM-DATA - const r = await request.post(forwarding); - return r.body; // already json-stringified for this call, maybe because backend doesn't return proper content-type - } - if (parsed.body && typeof parsed.body === 'object' && parsed.body.account && parsed.body.uuid) { - // Removing mock auth when forwarding request to real backend at ${req.url} - delete parsed.body.account; - delete parsed.body.uuid; - } - forwarding.json = parsed.body; // JSON - const r = await request.post(forwarding); - return JSON.stringify(r.body); -}; - export const mockBackendEndpoints: HandlersDefinition = { '/api/account/login': async ({ body }, req) => { const parsed = throwIfNotPostWithAuth(body, req); @@ -78,11 +52,15 @@ export const mockBackendEndpoints: HandlersDefinition = { expect((body as any).email).to.equal('flowcrypt.compatibility@gmail.com'); return { sent: true, text: 'Feedback sent' }; }, - '/api/message/presign_files': fwdToRealBackend, - '/api/message/confirm_files': fwdToRealBackend, - '/api/message/upload': fwdToRealBackend, - '/api/link/message': fwdToRealBackend, - '/api/link/me': fwdToRealBackend, + '/api/message/upload': async ({ }) => { + return { short: '0000000000', url: 'https://example.com/msg-123', admin_code: 'mocked-admin-code' }; + }, + '/api/link/message': async ({ }) => { + return { "url": "https://example.com/msg-123", "repliable": "False", "expire": "2100-05-18 16:31:28", "expired": "False", "deleted": "False" }; + }, + '/api/link/me': async ({ }, req) => { + throw new Error(`${req.url} mock not implemented`); + }, }; const throwIfNotPostWithAuth = (body: unknown, req: IncomingMessage) => { diff --git a/test/source/test.ts b/test/source/test.ts index a9646687ccb..a1986effdc1 100644 --- a/test/source/test.ts +++ b/test/source/test.ts @@ -24,6 +24,7 @@ import { TestUrls } from './browser/test-urls'; export const { testVariant, testGroup, oneIfNotPooled, buildDir, isMock } = getParsedCliParams(); export const internalTestState = { expectIntentionalErrReport: false }; // updated when a particular test that causes an error is run const DEBUG_BROWSER_LOG = false; // set to true to print / export information from browser +const DEBUG_MOCK_LOG = false; // se to true to print mock server logs process.setMaxListeners(60); @@ -52,7 +53,12 @@ ava.before('set config and mock api', async t => { Config.extensionId = await browserPool.getExtensionId(t); console.info(`Extension url: chrome-extension://${Config.extensionId}`); if (isMock) { - const mockApi = await mock(line => mockApiLogs.push(line)); + const mockApi = await mock(line => { + if (DEBUG_MOCK_LOG) { + console.log(line); + } + mockApiLogs.push(line); + }); closeMockApi = mockApi.close; } t.pass(); diff --git a/test/source/tests/flaky.ts b/test/source/tests/flaky.ts index d04494a4c02..4d907ac8886 100644 --- a/test/source/tests/flaky.ts +++ b/test/source/tests/flaky.ts @@ -108,16 +108,7 @@ export const defineFlakyTests = (testVariant: TestVariant, testWithBrowser: Test await ComposePageRecipe.sendAndClose(composePage, { password: msgPwd }); const msg = (await GoogleData.withInitializedData('flowcrypt.compatibility@gmail.com')).getMessageBySubject(subject)!; const webDecryptUrl = msg.payload!.body!.data!.replace(///g, '/').match(/https:\/\/flowcrypt.com\/[a-z0-9A-Z]+/g)![0]; - // while this test runs on a mock, it forwards the message/upload call to real backend - see `fwdToRealBackend` - // that's why we are able to test the message on real flowcrypt.com/api and web. - const webDecryptPage = await browser.newPage(t, webDecryptUrl); - await webDecryptPage.waitAndType('@input-msg-pwd', msgPwd); - await webDecryptPage.waitAndClick('@action-decrypt'); - await webDecryptPage.waitForContent('@container-pgp-decrypted-content', subject); - await webDecryptPage.waitForContent('@container-pgp-decrypted-content', 'flowcrypt.compatibility test footer with an img'); - await webDecryptPage.waitAll('@container-att-name(small.txt)'); - const fileText = await webDecryptPage.awaitDownloadTriggeredByClicking('@container-att-name(small.txt)'); - expect(fileText.toString()).to.equal(`small text file\nnot much here\nthis worked\n`); + expect(webDecryptUrl).to.not.be.empty('missing webDecryptUrl'); })); ava.default(`[unit][Stream.readToEnd] efficiently handles multiple chunks`, async t => { From d3c21a8991898431634a9d6b912f3aeef8811ffa Mon Sep 17 00:00:00 2001 From: Tom J Date: Sat, 15 May 2021 17:54:56 +0000 Subject: [PATCH 6/7] rm test - already tested by mock --- test/source/tests/flaky.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/source/tests/flaky.ts b/test/source/tests/flaky.ts index 4d907ac8886..67c8366069a 100644 --- a/test/source/tests/flaky.ts +++ b/test/source/tests/flaky.ts @@ -106,9 +106,6 @@ export const defineFlakyTests = (testVariant: TestVariant, testWithBrowser: Test const fileInput = await composePage.target.$('input[type=file]'); await fileInput!.uploadFile('test/samples/small.txt'); await ComposePageRecipe.sendAndClose(composePage, { password: msgPwd }); - const msg = (await GoogleData.withInitializedData('flowcrypt.compatibility@gmail.com')).getMessageBySubject(subject)!; - const webDecryptUrl = msg.payload!.body!.data!.replace(///g, '/').match(/https:\/\/flowcrypt.com\/[a-z0-9A-Z]+/g)![0]; - expect(webDecryptUrl).to.not.be.empty('missing webDecryptUrl'); })); ava.default(`[unit][Stream.readToEnd] efficiently handles multiple chunks`, async t => { From b13c12fdbec6b4c66cc82c2bc9659ca4230d6793 Mon Sep 17 00:00:00 2001 From: Tom J <6306961+tomholub@users.noreply.github.com> Date: Sat, 15 May 2021 18:09:54 +0000 Subject: [PATCH 7/7] rm unused import --- test/source/tests/flaky.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/source/tests/flaky.ts b/test/source/tests/flaky.ts index 67c8366069a..ff0d8cf27d4 100644 --- a/test/source/tests/flaky.ts +++ b/test/source/tests/flaky.ts @@ -11,7 +11,6 @@ import { PageRecipe } from './page-recipe/abstract-page-recipe'; import { SettingsPageRecipe } from './page-recipe/settings-page-recipe'; import { SetupPageRecipe } from './page-recipe/setup-page-recipe'; import { TestWithBrowser } from './../test'; -import { GoogleData } from './../mock/google/google-data'; import { Stream } from '../core/stream'; // tslint:disable:no-blank-lines-func