Removing keys if absent on EKM

extension/chrome/settings/modules/add_key.ts

@@ -13,13 +13,11 @@ import { Ui } from '../../../js/common/browser/ui.js';
 import { View } from '../../../js/common/view.js';
 import { Xss } from '../../../js/common/platform/xss.js';
 import { initPassphraseToggle } from '../../../js/common/ui/passphrase-ui.js';
-import { PassphraseStore } from '../../../js/common/platform/store/passphrase-store.js';
-import { KeyStore } from '../../../js/common/platform/store/key-store.js';
-import { KeyUtil, UnexpectedKeyTypeError } from '../../../js/common/core/crypto/key.js';
+import { UnexpectedKeyTypeError } from '../../../js/common/core/crypto/key.js';
 import { ClientConfiguration } from '../../../js/common/client-configuration.js';
-import { StorageType } from '../../../js/common/platform/store/abstract-store.js';
 import { Lang } from '../../../js/common/lang.js';
 import { AcctStore } from '../../../js/common/platform/store/acct-store.js';
+import { saveKeysAndPassPhrase, setPassphraseForPrvs } from '../../../js/common/helpers.js';
 
 View.run(class AddKeyView extends View {
 
@@ -100,9 +98,13 @@ View.run(class AddKeyView extends View {
     try {
       const checked = await this.keyImportUi.checkPrv(this.acctEmail, String($('.input_private_key').val()), String($('.input_passphrase').val()));
       if (checked) {
-        await KeyStore.add(this.acctEmail, checked.encrypted); // resulting new_key checked above
-        const storageType: StorageType = ($('.input_passphrase_save').prop('checked') && !this.clientConfiguration.forbidStoringPassPhrase()) ? 'local' : 'session';
-        await PassphraseStore.set(storageType, this.acctEmail, { longid: KeyUtil.getPrimaryLongid(checked.encrypted) }, checked.passphrase);
+        await saveKeysAndPassPhrase(this.acctEmail, [checked.encrypted]); // resulting new_key checked above
+        await setPassphraseForPrvs(
+          this.clientConfiguration,
+          this.acctEmail,
+          [checked.encrypted],
+          { passphrase: checked.passphrase, passphrase_save: !!$('.input_passphrase_save').prop('checked') }
+        );
         BrowserMsg.send.reload(this.parentTabId, { advanced: true });
       }
     } catch (e) {

extension/chrome/settings/setup.ts

@@ -48,7 +48,7 @@ export class SetupView extends View {
 
   public readonly acctEmail: string;
   public readonly parentTabId: string | undefined;
-  public readonly action: 'add_key' | undefined;
+  public readonly action: 'add_key' | 'update_from_ekm' | undefined;
   public readonly idToken: string | undefined; // only needed for initial setup, not for add_key
 
   public readonly keyImportUi = new KeyImportUi({ checkEncryption: true });
@@ -77,7 +77,7 @@ export class SetupView extends View {
     super();
     const uncheckedUrlParams = Url.parse(['acctEmail', 'action', 'idToken', 'parentTabId']);
     this.acctEmail = Assert.urlParamRequire.string(uncheckedUrlParams, 'acctEmail');
-    this.action = Assert.urlParamRequire.oneof(uncheckedUrlParams, 'action', ['add_key', undefined]) as 'add_key' | undefined;
+    this.action = Assert.urlParamRequire.oneof(uncheckedUrlParams, 'action', ['add_key', 'update_from_ekm', undefined]) as 'add_key' | 'update_from_ekm' | undefined;
     if (this.action === 'add_key') {
       this.parentTabId = Assert.urlParamRequire.string(uncheckedUrlParams, 'parentTabId');
     } else {

extension/chrome/settings/setup/setup-key-manager-autogen.ts

@@ -68,7 +68,7 @@ export class SetupWithEmailKeyManagerModule {
         // generate keys on client and store them on key manager
         await this.autoGenerateKeyAndStoreBothLocallyAndToEkm(setupOptions);
       } else {
-        await Ui.modal.error(`Keys for your account were not set up yet - please ask your systems administrator.`);
+        await Ui.modal.error(Lang.setup.noKeys);
         window.location.href = Url.create('index.htm', { acctEmail: this.view.acctEmail });
         return;
       }

extension/chrome/settings/setup/setup-render.ts

@@ -32,7 +32,7 @@ export class SetupRenderModule {
         return await Settings.promptToRetry(e, Lang.setup.failedToLoadEmailAliases, () => this.renderInitial(), Lang.general.contactIfNeedAssistance(this.view.isFesUsed()));
       }
     }
-    if (this.view.storage!.setup_done) {
+    if (this.view.storage!.setup_done && this.view.action !== 'update_from_ekm') {
       if (this.view.action !== 'add_key') {
         await this.renderSetupDone();
       } else if (this.view.clientConfiguration.mustAutoImportOrAutogenPrvWithKeyManager()) {
@@ -66,8 +66,13 @@ export class SetupRenderModule {
       $('.private_key_count').text(storedKeys.length);
       $('.backups_count').text(this.view.fetchedKeyBackupsUniqueLongids.length);
     } else { // successful and complete setup
-      this.displayBlock(this.view.action !== 'add_key' ? 'step_4_done' : 'step_4_close');
-      $('h1').text(this.view.action !== 'add_key' ? 'You\'re all set!' : 'Recovered all keys!');
+      if (this.view.action === 'add_key') {
+        this.displayBlock('step_4_close');
+        $('h1').text('Recovered all keys!');
+      } else {
+        this.displayBlock('step_4_done');
+        $('h1').text('You\'re all set!');
+      }
       $('.email').text(this.view.acctEmail);
     }
   };

extension/js/common/browser/browser-msg.ts

@@ -84,7 +84,7 @@ export namespace Bm {
     export type AjaxGmailAttachmentGetChunk = { chunk: Buf };
     export type _tab_ = { tabId: string | null | undefined };
     export type SaveFetchedPubkeys = boolean;
-    export type ProcessAndStoreKeysFromEkmLocally = { needPassphrase: boolean, updateCount: number };
+    export type ProcessAndStoreKeysFromEkmLocally = { needPassphrase?: boolean, updateCount?: number, noKeysSetup?: boolean };
     export type Db = any; // not included in Any below
     export type Ajax = any; // not included in Any below
 

extension/js/common/helpers.ts

@@ -11,21 +11,41 @@ import { ContactStore } from './platform/store/contact-store.js';
 import { KeyStore } from './platform/store/key-store.js';
 import { PassphraseStore } from './platform/store/passphrase-store.js';
 import { Bm } from './browser/browser-msg.js';
+import { PgpPwd } from './core/crypto/pgp/pgp-password.js';
 
 export const isFesUsed = async (acctEmail: string) => {
   const { fesUrl } = await AcctStore.get(acctEmail, ['fesUrl']);
   return Boolean(fesUrl);
 };
 
-export const saveKeysAndPassPhrase = async (acctEmail: string, prvs: Key[], ppOptions?: PassphraseOptions) => {
-  const clientConfiguration = await ClientConfiguration.newInstance(acctEmail);
+export const setPassphraseForPrvs = async (clientConfiguration: ClientConfiguration, acctEmail: string, prvs: Key[], ppOptions: PassphraseOptions) => {
+  const storageType = (ppOptions.passphrase_save && !clientConfiguration.forbidStoringPassPhrase()) ? 'local' : 'session';
   for (const prv of prvs) {
-    await KeyStore.add(acctEmail, prv);
-    if (ppOptions !== undefined) {
-      await PassphraseStore.set((ppOptions.passphrase_save && !clientConfiguration.forbidStoringPassPhrase()) ? 'local' : 'session',
-        acctEmail, { longid: KeyUtil.getPrimaryLongid(prv) }, ppOptions.passphrase);
+    await PassphraseStore.set(storageType, acctEmail, { longid: KeyUtil.getPrimaryLongid(prv) }, ppOptions.passphrase);
+  }
+};
+
+// note: for `replaceKeys = true` need to make sure that `prvs` don't have duplicate identities,
+// they is currently guaranteed by filterKeysToSave()
+// todo: perhaps split into two different functions for add or replace as part of #4545?
+const addOrReplaceKeysAndPassPhrase = async (acctEmail: string, prvs: Key[], ppOptions?: PassphraseOptions, replaceKeys: boolean = false) => {
+  if (replaceKeys) {
+    // track longids to remove related passhprases
+    const existingKeys = await KeyStore.get(acctEmail);
+    const deletedKeys = existingKeys.filter(old => !prvs.some(prvIdentity => KeyUtil.identityEquals(prvIdentity, old)));
+    // set actually replaces the set of keys in storage with the new set
+    await KeyStore.set(acctEmail, await Promise.all(prvs.map(KeyUtil.keyInfoObj)));
+    await PassphraseStore.removeMany(acctEmail, deletedKeys);
+  } else {
+    for (const prv of prvs) {
+      await KeyStore.add(acctEmail, prv);
     }
   }
+  if (ppOptions !== undefined) {
+    // todo: it would be good to check that the passphrase isn't present in the other storage type
+    //    though this situation is not possible with current use cases
+    await setPassphraseForPrvs(await ClientConfiguration.newInstance(acctEmail), acctEmail, prvs, ppOptions);
+  }
   const { sendAs, full_name: name } = await AcctStore.get(acctEmail, ['sendAs', 'full_name']);
   const myOwnEmailsAddrs: string[] = [acctEmail].concat(Object.keys(sendAs!));
   for (const email of myOwnEmailsAddrs) {
@@ -39,6 +59,8 @@ export const saveKeysAndPassPhrase = async (acctEmail: string, prvs: Key[], ppOp
   }
 };
 
+export const saveKeysAndPassPhrase: (acctEmail: string, prvs: Key[], ppOptions?: PassphraseOptions) => Promise<void> = addOrReplaceKeysAndPassPhrase;
+
 const parseAndCheckPrivateKeys = async (decryptedPrivateKeys: string[]) => {
   const unencryptedPrvs: Key[] = [];
   // parse and check that all the keys are valid
@@ -64,58 +86,81 @@ const parseAndCheckPrivateKeys = async (decryptedPrivateKeys: string[]) => {
 };
 
 const filterKeysToSave = async (candidateKeys: Key[], existingKeys: KeyInfoWithIdentity[]) => {
+  // todo: check for uniqueness of candidateKeys identities here?
   if (!existingKeys.length) {
-    return candidateKeys;
+    return { keysToRetain: [], newUnencryptedKeysToSave: candidateKeys };
   }
-  const result: Key[] = [];
-  for (const candidate of candidateKeys) {
-    const longid = KeyUtil.getPrimaryLongid(candidate);
-    const keyToUpdate = existingKeys.filter(ki => ki.longid === longid && ki.family === candidate.family);
-    if (keyToUpdate.length === 1) {
-      const oldKey = await KeyUtil.parse(keyToUpdate[0].private);
-      if (!candidate.lastModified || (oldKey.lastModified && oldKey.lastModified >= candidate.lastModified)) {
+  const keysToRetain: Key[] = [];
+  const newUnencryptedKeysToSave: Key[] = [];
+  for (const candidateKey of candidateKeys) {
+    const existingKey = existingKeys.find(ki => KeyUtil.identityEquals(ki, candidateKey));
+    if (existingKey) {
+      const parsedExistingKey = await KeyUtil.parse(existingKey.private);
+      if (!candidateKey.lastModified || (parsedExistingKey.lastModified && parsedExistingKey.lastModified >= candidateKey.lastModified)) {
+        keysToRetain.push(parsedExistingKey);
         continue;
       }
-    } else if (keyToUpdate.length > 1) {
-      throw new Error(`Unexpected error: key search by longid=${longid} yielded ${keyToUpdate.length} results`);
     }
-    result.push(candidate);
+    newUnencryptedKeysToSave.push(candidateKey);
   }
-  return result;
+  return { keysToRetain, newUnencryptedKeysToSave };
 };
 
 export const processAndStoreKeysFromEkmLocally = async (
-  { acctEmail, decryptedPrivateKeys, ppOptions }: { acctEmail: string, decryptedPrivateKeys: string[], ppOptions?: PassphraseOptions }
+  { acctEmail, decryptedPrivateKeys, ppOptions: originalOptions }: Bm.ProcessAndStoreKeysFromEkmLocally & { ppOptions?: PassphraseOptions }
 ): Promise<Bm.Res.ProcessAndStoreKeysFromEkmLocally> => {
   const { unencryptedPrvs } = await parseAndCheckPrivateKeys(decryptedPrivateKeys);
   const existingKeys = await KeyStore.get(acctEmail);
+  let { keysToRetain, newUnencryptedKeysToSave } = await filterKeysToSave(unencryptedPrvs, existingKeys);
+  if (!newUnencryptedKeysToSave.length && keysToRetain.length === existingKeys.length) {
+    // nothing to update
+    return { needPassphrase: false, noKeysSetup: !existingKeys.length };
+  }
+  let ppOptions: PassphraseOptions | undefined; // the options to pass to saveKeysAndPassPhrase
+  if (!originalOptions?.passphrase && (await ClientConfiguration.newInstance(acctEmail)).mustAutogenPassPhraseQuietly()) {
+    ppOptions = { passphrase: PgpPwd.random(), passphrase_save: true };
+  } else {
+    ppOptions = originalOptions;
+  }
   let passphrase = ppOptions?.passphrase;
+  let passphraseInLocalStorage = !!ppOptions?.passphrase_save;
   if (passphrase === undefined && !existingKeys.length) {
-    return { needPassphrase: false, updateCount: 0 }; // return success as we can't possibly validate a passphrase
-    // this can only happen on misconfiguration
-    // todo: or should we throw?
+    return { needPassphrase: true, noKeysSetup: true };
   }
-  let unencryptedKeysToSave = await filterKeysToSave(unencryptedPrvs, existingKeys);
-  let encryptedKeys: Key[] = [];
-  if (unencryptedKeysToSave.length) {
+  let encryptedKeys: { passphrase: string, keys: Key[] } | undefined;
+  if (newUnencryptedKeysToSave.length) {
     if (passphrase === undefined) {
       // trying to find a passphrase that unlocks at least one key
       const passphrases = await PassphraseStore.getMany(acctEmail, existingKeys);
-      passphrase = passphrases.find(pp => pp !== undefined);
+      const foundPassphrase = passphrases.find(pp => pp !== undefined);
+      if (foundPassphrase) {
+        passphrase = foundPassphrase.value;
+        passphraseInLocalStorage = foundPassphrase.source === 'local';
+      }
     }
     if (passphrase !== undefined) {
-      const pp = passphrase;
-      // todo: some more fancy conversion, preserving a passphrase for a particual longid?
-      await Promise.all(unencryptedKeysToSave.map(prv => KeyUtil.encrypt(prv, pp)));
-      encryptedKeys = unencryptedKeysToSave;
-      unencryptedKeysToSave = [];
+      const pp = passphrase; // explicitly defined constant string for the mapping function
+      await Promise.all(newUnencryptedKeysToSave.map(prv => KeyUtil.encrypt(prv, pp)));
+      encryptedKeys = { keys: newUnencryptedKeysToSave, passphrase };
+      newUnencryptedKeysToSave = [];
     }
   }
-  if (encryptedKeys.length) {
-    // also updates `name`, todo: refactor in #4545
-    await saveKeysAndPassPhrase(acctEmail, encryptedKeys, ppOptions);
-    return { needPassphrase: false, updateCount: encryptedKeys.length };
-  } else {
-    return { needPassphrase: unencryptedKeysToSave.length > 0, updateCount: 0 };
+  if (newUnencryptedKeysToSave.length > 0) {
+    return { needPassphrase: true };
+  }
+  // stage 1. Clear all existingKeys, except for keysToRetain
+  if (existingKeys.length !== keysToRetain.length) {
+    await addOrReplaceKeysAndPassPhrase(acctEmail, keysToRetain, undefined, true);
+  }
+  // stage 2. Adding new keys
+  if (encryptedKeys?.keys.length) {
+    // new keys are about to be added, they must be accompanied with the passphrase setting
+    const effectivePpOptions = { passphrase: encryptedKeys.passphrase, passphrase_save: passphraseInLocalStorage };
+    // ppOptions have special meaning in saveKeysAndPassPhrase(), they trigger `name` updates, todo: refactor in #4545
+    await saveKeysAndPassPhrase(acctEmail, encryptedKeys.keys, ppOptions ? effectivePpOptions : undefined);
+    if (!ppOptions) {
+      await setPassphraseForPrvs(await ClientConfiguration.newInstance(acctEmail), acctEmail, encryptedKeys.keys, effectivePpOptions);
+    }
   }
+  return { updateCount: encryptedKeys?.keys.length ?? 0 + (existingKeys.length - keysToRetain.length), noKeysSetup: !(encryptedKeys?.keys.length || keysToRetain.length) };
 };

extension/js/common/lang.ts

@@ -35,6 +35,7 @@ export const Lang = { // tslint:disable-line:variable-name
     keyBackupsNotAllowed: 'Key backups are not allowed on this domain.',
     prvHasFixableCompatIssue: 'This key has minor usability issues that can be fixed. This commonly happens when importing keys from Symantec™ PGP Desktop or other legacy software. It may be missing User IDs, or it may be missing a self-signature. It is also possible that the key is simply expired.',
     ppMatchAllSet: 'Your pass phrase matches. Good job! You\'re all set.',
+    noKeys: 'Keys for your account were not set up yet - please ask your systems administrator.',
   },
   account: {
     googleAcctDisabledOrPolicy: `Your Google Account or Google Email seems to be disabled, or access to this app is disabled by your organisation admin policy. Contact your email administrator.`,

extension/js/common/platform/store/in-memory-store.ts

@@ -4,7 +4,7 @@ import { AbstractStore } from './abstract-store.js';
 import { BrowserMsg } from '../../browser/browser-msg.js';
 
 /**
- * Temporrary In-Memory store for sensitive values, expiring after clientConfiguration.in_memory_pass_phrase_session_length (or default 4 hours)
+ * Temporary In-Memory store for sensitive values, expiring after clientConfiguration.in_memory_pass_phrase_session_length (or default 4 hours)
  * see background_page.ts for the other end, also ExpirationCache class
  */
 export class InMemoryStore extends AbstractStore {

extension/js/common/platform/store/key-store.ts

@@ -41,7 +41,7 @@ export class KeyStore extends AbstractStore {
       throw new Error('Cannot import plain, unprotected key.');
     }
     for (const i in keyinfos) {
-      if (prv.id === keyinfos[i].fingerprints[0]) { // replacing a key
+      if (KeyUtil.identityEquals(prv, keyinfos[i])) { // replacing a key
         keyinfos[i] = await KeyUtil.keyInfoObj(prv);
         updated = true;
       }