Show warnings on unusable keys

extension/chrome/elements/compose-modules/compose-recipients-module.ts

@@ -828,7 +828,7 @@ export class ComposeRecipientsModule extends ViewModule<ComposeView> {
  this.view.errModule.debug(`renderPubkeyResult: Setting email to wrong / misspelled in harsh mode: ${recipient.email}`);
  $(el).attr('title', 'This email address looks misspelled. Please try again.');
  $(el).addClass("wrong");
- } else if (contact.pubkey && ((contact.expiresOn || Infinity) <= Date.now() || contact.pubkey.usableButExpired)) {
+ } else if (contact.pubkey && ((contact.expiresOn || Infinity) <= Date.now() || contact.pubkey.usableForEncryptionButExpired)) {
  recipient.status = RecipientStatus.EXPIRED;
  $(el).addClass("expired");
  Xss.sanitizePrepend(el, '<img src="/img/svgs/expired-timer.svg" class="expired-time">');

extension/chrome/elements/compose-modules/formatters/encrypted-mail-msg-formatter.ts

@@ -152,7 +152,7 @@ export class EncryptedMsgMailFormatter extends BaseMailFormatter {
  return undefined;
  }
  for (const myKey of pubs.filter(ap => ap.isMine)) {
- if (myKey.pubkey.usableButExpired) {
+ if (myKey.pubkey.usableForEncryptionButExpired) {
  const path = Url.create(chrome.runtime.getURL('chrome/settings/index.htm'), {
  acctEmail: myKey.email,
  page: '/chrome/settings/modules/my_key_update.htm',

extension/chrome/elements/pgp_pubkey.ts

@@ -65,8 +65,8 @@ View.run(class PgpPubkeyView extends View {
  $('.line.fingerprints').css({ display: 'none' });
  }
  if (this.primaryPubKey) {
-const isUsableButExpired = this.primaryPubKey.usableButExpired;
-if (!isUsableButExpired && await !this.primaryPubKey.usableForEncryption && !this.primaryPubKey.usableForSigning) {
+if (!this.primaryPubKey.usableForEncryptionButExpired && !this.primaryPubKey.usableForSigningButExpired
+ && !this.primaryPubKey.usableForEncryption && !this.primaryPubKey.usableForSigning) {
  this.showKeyNotUsableError();
  } else {
  if (this.compact) {

extension/chrome/settings/modules/my_key_update.ts

@@ -85,7 +85,7 @@ View.run(class MyKeyUpdateView extends View {
  return;
  }
  // cannot get a valid encryption key packet
- if (await KeyUtil.isWithoutSelfCertifications(updatedKey) || updatedKey.usableButExpired) { // known issues - key can be fixed
+ if (await KeyUtil.isWithoutSelfCertifications(updatedKey) || updatedKey.usableForEncryptionButExpired) { // known issues - key can be fixed
  const fixedEncryptedPrv = await Settings.renderPrvCompatFixUiAndWaitTilSubmittedByUser(
  this.acctEmail, '.compatibility_fix_container', updatedKeyEncrypted, uddatedKeyPassphrase, this.showKeyUrl
  );

extension/js/common/core/crypto/key.ts

@@ -27,7 +27,10 @@ export interface Key {
  expiration: number | undefined; // number of millis of expiration or undefined if never expires
  usableForEncryption: boolean;
  usableForSigning: boolean;
- usableButExpired: boolean;
+ usableForEncryptionButExpired: boolean;
+ usableForSigningButExpired: boolean;
+ missingPrivateKeyForSigning: boolean;
+ missingPrivateKeyForDecryption: boolean;
  emails: string[];
  identities: string[];
  fullyDecrypted: boolean;
@@ -159,7 +162,8 @@ export class KeyUtil {
  }
  result.set(`expiration`, KeyUtil.formatResult(key.expiration));
  result.set(`internal dateBeforeExpiration`, await KeyUtil.formatResultAsync(async () => KeyUtil.dateBeforeExpirationIfAlreadyExpired(key)));
- result.set(`internal usableButExpired`, KeyUtil.formatResult(key.usableButExpired));
+ result.set(`internal usableForEncryptionButExpired`, KeyUtil.formatResult(key.usableForEncryptionButExpired));
+ result.set(`internal usableForSigningButExpired`, KeyUtil.formatResult(key.usableForSigningButExpired));
  return result;
  }
 

extension/js/common/core/crypto/pgp/openpgp-key.ts

@@ -13,6 +13,9 @@ export class OpenPGPKey {
 
  private static readonly encryptionText = 'This is the text we are encrypting!';
 
+ // mapping of algo names to required param count, lazy initialized
+ private static paramCountByAlgo: { [key: string]: number };
+
  public static parse = async (text: string): Promise<Key> => {
  // TODO: Should we throw if more keys are in the armor?
  return (await OpenPGPKey.parseMany(text))[0];
@@ -192,13 +195,26 @@ export class OpenPGPKey {
  }
  const algoInfo = opgpKey.primaryKey.getAlgorithmInfo();
  const key = keyToUpdate || {} as Key; // if no key to update, use empty object, will get props assigned below
+ const encryptionKey = await Catch.undefinedOnException(opgpKey.getEncryptionKey());
+ const getEncryptionKey = (keyid?: OpenPGP.Keyid | null, date?: Date, userId?: OpenPGP.UserId | null) =>
+ opgpKey.getEncryptionKey(keyid, date, userId);
+ const encryptionKeyIgnoringExpiration = encryptionKey ? encryptionKey : await OpenPGPKey.getKeyIgnoringExpiration(getEncryptionKey, exp, expired);
+ const signingKey = await Catch.undefinedOnException(opgpKey.getSigningKey());
+ const getSigningKey = (keyid?: OpenPGP.Keyid | null, date?: Date, userId?: OpenPGP.UserId | null) =>
+ opgpKey.getSigningKey(keyid, date, userId);
+ const signingKeyIgnoringExpiration = signingKey ? signingKey : await OpenPGPKey.getKeyIgnoringExpiration(getSigningKey, exp, expired);
+ const missingPrivateKeyForSigning = signingKeyIgnoringExpiration?.keyPacket ? OpenPGPKey.arePrivateParamsMissing(signingKeyIgnoringExpiration.keyPacket) : false;
+ const missingPrivateKeyForDecryption = encryptionKeyIgnoringExpiration?.keyPacket ? OpenPGPKey.arePrivateParamsMissing(encryptionKeyIgnoringExpiration.keyPacket) : false;
  Object.assign(key, {
  type: 'openpgp',
  id: fingerprint.toUpperCase(),
  allIds: opgpKey.getKeys().map(k => k.getFingerprint().toUpperCase()),
- usableForEncryption: ! await Catch.doesReject(opgpKey.getEncryptionKey()),
- usableButExpired: await OpenPGPKey.usableButExpired(opgpKey, exp, expired),
- usableForSigning: ! await Catch.doesReject(opgpKey.getSigningKey()),
+ usableForEncryption: encryptionKey ? true : false,
+ usableForEncryptionButExpired: !encryptionKey && !!encryptionKeyIgnoringExpiration,
+ usableForSigning: signingKey ? true : false,
+ usableForSigningButExpired: !signingKey && !!signingKeyIgnoringExpiration,
+ missingPrivateKeyForSigning,
+ missingPrivateKeyForDecryption,
  // valid emails extracted from uids
  emails,
  // full uids that have valid emails in them
@@ -421,27 +437,43 @@ export class OpenPGPKey {
  return raw;
  }
 
- private static usableButExpired = async (key: OpenPGP.key.Key, exp: Date | number | null, expired: () => boolean): Promise<boolean> => {
- if (!key) {
- return false;
- }
- if (! await Catch.doesReject(key.getEncryptionKey())) {
- return false;
+ private static getKeyIgnoringExpiration = async (
+ getter: (keyid?: OpenPGP.Keyid | null, date?: Date, userId?: OpenPGP.UserId | null) => Promise<OpenPGP.key.Key | OpenPGP.key.SubKey | null>,
+ exp: Date | number | null,
+ expired: () => boolean): Promise<OpenPGP.key.Key | OpenPGP.key.SubKey | null> => {
+ const firstTry = await Catch.undefinedOnException(getter());
+ if (firstTry) {
+ return firstTry;
  }
  if (exp === null || typeof exp === 'number') {
  // If key does not expire (exp == Infinity) the encryption key should be available.
- return false;
+ return null; // tslint:disable-line:no-null-keyword
  }
  const oneSecondBeforeExpiration = exp && expired() ? new Date(exp.getTime() - 1000) : undefined;
  if (typeof oneSecondBeforeExpiration === 'undefined') {
- return false;
- }
- try {
- await key.getEncryptionKey(undefined, oneSecondBeforeExpiration);
- return true;
- } catch (e) {
- return false;
+ return null; // tslint:disable-line:no-null-keyword
  }
+ const secondTry = await Catch.undefinedOnException(getter(undefined, oneSecondBeforeExpiration));
+ return secondTry ? secondTry : null; // tslint:disable-line:no-null-keyword
+ }
+
+ private static arePrivateParamsMissing = (packet: OpenPGP.packet.BaseKeyPacket): boolean => {
+ // detection of missing private params to solve #2887
+ if (!OpenPGPKey.paramCountByAlgo) {
+ OpenPGPKey.paramCountByAlgo = {
+ [opgp.enums.read(opgp.enums.publicKey, opgp.enums.publicKey.rsa_encrypt)]: 6,
+ [opgp.enums.read(opgp.enums.publicKey, opgp.enums.publicKey.rsa_encrypt_sign)]: 6,
+ [opgp.enums.read(opgp.enums.publicKey, opgp.enums.publicKey.rsa_sign)]: 6,
+ [opgp.enums.read(opgp.enums.publicKey, opgp.enums.publicKey.dsa)]: 5,
+ [opgp.enums.read(opgp.enums.publicKey, opgp.enums.publicKey.elgamal)]: 4,
+ [opgp.enums.read(opgp.enums.publicKey, opgp.enums.publicKey.ecdsa)]: 2,
+ [opgp.enums.read(opgp.enums.publicKey, opgp.enums.publicKey.ecdh)]: 3,
+ [opgp.enums.read(opgp.enums.publicKey, opgp.enums.publicKey.eddsa)]: 3,
+ };
+ }
+ return packet.algorithm
+ && !packet.isEncrypted // isDecrypted() returns false when isEncrypted is null
+ && OpenPGPKey.paramCountByAlgo[packet.algorithm] > packet.params?.length;
  }
 
  private static testEncryptDecrypt = async (key: OpenPGP.key.Key): Promise<string[]> => {

extension/js/common/core/crypto/smime/smime-key.ts

@@ -19,7 +19,8 @@ export class SmimeKey {
  allIds: [certificate.serialNumber.toUpperCase()],
  usableForEncryption: SmimeKey.isEmailCertificate(certificate),
  usableForSigning: SmimeKey.isEmailCertificate(certificate),
- usableButExpired: false,
+ usableForEncryptionButExpired: false,
+ usableForSigningButExpired: false,
  emails: [normalizedEmail],
  identities: [normalizedEmail],
  created: SmimeKey.dateToNumber(certificate.validity.notBefore),

extension/js/common/core/types/openpgp.d.ts

@@ -125,6 +125,9 @@ declare namespace OpenPGP {
  public getFingerprintBytes(): Uint8Array | null;
  public getCreationTime(): Date;
  public getKeyId(): Keyid;
+ public params: object[];
+ public isDecrypted(): boolean;
+ public isEncrypted: boolean; // may be null, false or true
  }
 
  class BasePrimaryKeyPacket extends BaseKeyPacket {
@@ -156,7 +159,6 @@ declare namespace OpenPGP {
 
  export class PublicKey extends BasePrimaryKeyPacket {
  public tag: enums.packet.publicKey;
- public isDecrypted(): null;
  }
 
  export class SymmetricallyEncrypted extends BasePacket {
@@ -169,7 +171,6 @@ declare namespace OpenPGP {
 
  export class PublicSubkey extends BaseKeyPacket {
  public tag: enums.packet.publicSubkey;
- public isDecrypted(): null;
  }
 
  export class UserAttribute extends BasePacket {
@@ -185,7 +186,6 @@ declare namespace OpenPGP {
  public tag: enums.packet.secretKey;
  // encrypted: null | unknown[]; // Encrypted secret-key data, not meant for public use
  public s2k: { type: string } | null;
- public isDecrypted(): boolean;
  public encrypt(passphrase: string): Promise<boolean>;
  public decrypt(passphrase: string): Promise<true>;
  }
@@ -199,7 +199,6 @@ declare namespace OpenPGP {
  public tag: enums.packet.secretSubkey;
  // encrypted: null | unknown[]; // Encrypted secret-key data, not meant for public use
  public s2k: { type: string } | null;
- public isDecrypted(): boolean;
  public encrypt(passphrase: string): Promise<boolean>;
  public decrypt(passphrase: string): Promise<true>;
  }
@@ -836,10 +835,10 @@ declare namespace OpenPGP {
  public isRevoked(): Promise<boolean>;
  public revoke(reason: { flag?: enums.reasonForRevocation; string?: string; }, date?: Date): Promise<Key>;
  public getRevocationCertificate(): Promise<Stream<string> | string | undefined>;
- public getEncryptionKey(keyid?: Keyid | null, date?: Date, userId?: UserId | null): Promise<packet.PublicSubkey | packet.SecretSubkey | packet.SecretKey | packet.PublicKey | null>;
- public getSigningKey(): Promise<packet.PublicSubkey | packet.SecretSubkey | packet.SecretKey | packet.PublicKey | null>;
+ public getEncryptionKey(keyid?: Keyid | null, date?: Date, userId?: UserId | null): Promise<key.Key | key.SubKey | null>;
+ public getSigningKey(keyid?: Keyid | null, date?: Date, userId?: UserId | null): Promise<key.Key | key.SubKey | null>;
  public getKeys(keyId?: Keyid): (Key | SubKey)[];
-// isDecrypted(): boolean;
+public isDecrypted(): boolean;
  public isFullyEncrypted(): boolean;
  public isFullyDecrypted(): boolean;
  public isPacketDecrypted(keyId: Keyid): boolean;

extension/js/common/platform/catch.ts

@@ -231,6 +231,14 @@ export class Catch {
  }
  }
 
+ public static undefinedOnException = async <T>(p: Promise<T>): Promise<T | undefined> => {
+ try {
+ return await p;
+ } catch (e) {
+ return undefined;
+ }
+ }
+
  private static formatExceptionForReport = (thrown: any, line?: number, col?: number): ErrorReport => {
  if (!line || !col) {
  const { line: parsedLine, col: parsedCol } = Catch.getErrorLineAndCol(thrown);

extension/js/common/ui/key-import-ui.ts

@@ -139,6 +139,19 @@ export class KeyImportUi {
  if (encrypted.identities.length === 0) {
  throw new KeyCanBeFixed(encrypted);
  }
+ // mandatory checks have passed, now display warnings
+ if (decrypted.missingPrivateKeyForDecryption || decrypted.missingPrivateKeyForSigning) {
+ const missing: string[] = [];
+ if (decrypted.missingPrivateKeyForSigning) {
+ missing.push('signing');
+ }
+ if (decrypted.missingPrivateKeyForDecryption) {
+ missing.push('decryption');
+ }
+ await Ui.modal.warning('Looks like this key was exported with --export-secret-subkeys option and missing private key parameters.\n\n' +
+ 'Please export the key with --export-secret-key option if you plan to use it for ' +
+ missing.join(' and ') + '.');
+ }
  return { normalized, passphrase, fingerprint: decrypted.id, decrypted, encrypted }; // will have fp if had longid
  }
 
@@ -268,10 +281,13 @@ export class KeyImportUi {
  }
 
  private checkEncryptionPrvIfSelected = async (k: Key, encrypted: Key) => {
- if (this.checkEncryption && !k.usableForEncryption) {
- if (await KeyUtil.isWithoutSelfCertifications(k)) {
+ if (this.checkEncryption && (!k.usableForEncryption || k.missingPrivateKeyForDecryption)) {
+ if (k.missingPrivateKeyForDecryption) {
+ throw new UserAlert('Looks like this key was exported with --export-secret-subkeys option and missing private key parameters.\n\n' +
+ 'Please export the key with --export-secret-key option.');
+ } else if (await KeyUtil.isWithoutSelfCertifications(k)) {
  throw new KeyCanBeFixed(encrypted);
- } else if (k.usableButExpired) {
+ } else if (k.usableForEncryptionButExpired) {
  // Currently have 2 options: import or skip. Would be better to give user 3 choices:
  // 1) Confirm importing expired key
  // 2) Extend validity of expired key + import
@@ -302,8 +318,13 @@ export class KeyImportUi {
  }
 
  private checkSigningIfSelected = async (k: Key) => {
- if (this.checkSigning && !k.usableForSigning) {
- throw new UserAlert('This looks like a valid key but it cannot be used for signing. Please write at human@flowcrypt.com to see why is that.');
+ if (this.checkSigning && (!k.usableForSigning || k.missingPrivateKeyForSigning)) {
+ if (k.missingPrivateKeyForSigning && !k.usableForSigningButExpired) {
+ throw new UserAlert('Looks like this key was exported with --export-secret-subkeys option and missing private key parameters.\n\n' +
+ 'Please export the key with --export-secret-key option.');
+ } else {
+ throw new UserAlert('This looks like a valid key but it cannot be used for signing. Please write at human@flowcrypt.com to see why is that.');
+ }
  }
  }
 

test/source/platform/catch.ts

@@ -36,6 +36,14 @@ export class Catch {
  }
  }
 
+ public static undefinedOnException = async <T>(p: Promise<T>): Promise<T | undefined> => {
+ try {
+ return await p;
+ } catch (e) {
+ return undefined;
+ }
+ }
+
  public static version = () => {
  return Catch.RUNTIME_VERSION;
  }