#3448 Ignore signature for pubkey in protonmail

extension/chrome/elements/attachment.ts

@@ -4,7 +4,7 @@
 
 import { Bm, BrowserMsg } from '../../js/common/browser/browser-msg.js';
 import { DecryptErrTypes, MsgUtil } from '../../js/common/core/crypto/pgp/msg-util.js';
-import { PromiseCancellation, Url } from '../../js/common/core/common.js';
+import { PromiseCancellation, Str, Url } from '../../js/common/core/common.js';
 import { Api } from '../../js/common/api/shared/api.js';
 import { ApiErr } from '../../js/common/api/shared/api-error.js';
 import { Assert } from '../../js/common/assert.js';
@@ -66,7 +66,7 @@ export class AttachmentDownloadView extends View {
     this.acctEmail = Assert.urlParamRequire.string(uncheckedUrlParams, 'acctEmail');
     this.parentTabId = Assert.urlParamRequire.string(uncheckedUrlParams, 'parentTabId');
     this.frameId = Assert.urlParamRequire.string(uncheckedUrlParams, 'frameId');
-    this.origNameBasedOnFilename = uncheckedUrlParams.name ? String(uncheckedUrlParams.name).replace(/\.(pgp|gpg)$/gi, '') : 'noname';
+    this.origNameBasedOnFilename = uncheckedUrlParams.name ? Str.stripPgpOrGpgExtensionIfPresent(String(uncheckedUrlParams.name)) : 'noname';
     this.isEncrypted = uncheckedUrlParams.isEncrypted === true;
     this.errorDetailsOpened = uncheckedUrlParams.errorDetailsOpened === true;
     this.size = uncheckedUrlParams.size ? parseInt(String(uncheckedUrlParams.size)) : undefined;
@@ -238,16 +238,18 @@ export class AttachmentDownloadView extends View {
   };
 
   private processAsPublicKeyAndHideAttachmentIfAppropriate = async () => {
-    if (this.attachment.msgId && this.attachment.id && this.attachment.treatAs() === 'publicKey') {
+    // todo: we should call this detection in the main `core/Attachment.treatAs` (e.g. in the context of GmailElementReplacer and InboxActiveThreadModule)
+    // should be possible after #4906 is done
+    if (((this.attachment.msgId && this.attachment.id) || this.attachment.url) && this.attachment.isPublicKey()) {
       // this is encrypted public key - download && decrypt & parse & render
-      const { data } = await this.gmail.attachmentGet(this.attachment.msgId, this.attachment.id);
+      await this.downloadDataIfNeeded();
       const decrRes = await MsgUtil.decryptMessage({
         kisWithPp: await KeyStore.getAllWithOptionalPassPhrase(this.acctEmail),
-        encryptedData: data,
+        encryptedData: this.attachment.getData(),
         verificationPubs: [], // no need to worry about the public key signature, as public key exchange is inherently unsafe
       });
       if (decrRes.success && decrRes.content) {
-        const openpgpType = await MsgUtil.type({ data: decrRes.content });
+        const openpgpType = MsgUtil.type({ data: decrRes.content });
         if (openpgpType && openpgpType.type === 'publicKey' && openpgpType.armored) {
           // 'openpgpType.armored': could potentially process unarmored pubkey files, maybe later
           BrowserMsg.send.renderPublicKeys(this.parentTabId, {

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

@@ -450,7 +450,7 @@ export class ComposeRenderModule extends ViewModule<ComposeView> {
 
   private renderReplySuccessAttachments = (attachments: Attachment[], msgId: string, isEncrypted: boolean) => {
     const hideAttachmentTypes = this.view.sendBtnModule.popover.choices.richtext ? ['hidden', 'encryptedMsg', 'signature', 'publicKey'] : ['publicKey'];
-    const renderableAttachments = attachments.filter(attachment => !hideAttachmentTypes.includes(attachment.treatAs()));
+    const renderableAttachments = attachments.filter(attachment => !hideAttachmentTypes.includes(attachment.treatAs(attachments)));
     if (renderableAttachments.length) {
       this.view.S.cached('replied_attachments')
         .html(

extension/chrome/elements/pgp_block_modules/pgp-block-attachmens-module.ts

@@ -11,6 +11,7 @@ import { Ui } from '../../../js/common/browser/ui.js';
 import { Xss } from '../../../js/common/platform/xss.js';
 import { KeyStore } from '../../../js/common/platform/store/key-store.js';
 import { XssSafeFactory } from '../../../js/common/xss-safe-factory.js';
+import { Str } from '../../../js/common/core/common.js';
 
 declare const filesize: { filesize: Function }; // eslint-disable-line @typescript-eslint/ban-types
 
@@ -23,7 +24,7 @@ export class PgpBlockViewAttachmentsModule {
     Xss.sanitizeAppend('#pgp_block', '<div id="attachments"></div>');
     this.includedAttachments = attachments;
     for (const i of attachments.keys()) {
-      const name = (attachments[i].name ? attachments[i].name : 'noname').replace(/\.(pgp|gpg)$/, '');
+      const name = attachments[i].name ? Str.stripPgpOrGpgExtensionIfPresent(attachments[i].name) : 'noname';
       const nameVisible = name.length > 100 ? name.slice(0, 100) + '…' : name;
       const size = filesize.filesize(attachments[i].length);
       const htmlContent = `<b>${Xss.escape(nameVisible)}</b>&nbsp;&nbsp;&nbsp;${size}<span class="progress"><span class="percent"></span></span>`;
@@ -90,7 +91,7 @@ export class PgpBlockViewAttachmentsModule {
     });
     if (decrypted.success) {
       const attachment = new Attachment({
-        name: encrypted.name.replace(/\.(pgp|gpg)$/, ''),
+        name: Str.stripPgpOrGpgExtensionIfPresent(encrypted.name),
         type: encrypted.type,
         data: decrypted.content,
       });

extension/chrome/elements/pgp_block_modules/pgp-block-decrypt-module.ts

@@ -30,16 +30,18 @@ export class PgpBlockViewDecryptModule {
         // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
         const mimeMsg = Buf.fromBase64UrlStr(raw!); // used 'raw' above
         const parsed = await Mime.decode(mimeMsg);
-        if (parsed && typeof parsed.rawSignedContent === 'string' && parsed.signature) {
-          this.view.signature.parsedSignature = parsed.signature;
-          await this.decryptAndRender(Buf.fromUtfStr(parsed.rawSignedContent), verificationPubs);
-        } else {
-          await this.view.errorModule.renderErr(
-            'Error: could not properly parse signed message',
-            parsed.rawSignedContent || parsed.text || parsed.html || mimeMsg.toUtfStr(),
-            'parse error'
-          );
+        if (parsed && typeof parsed.rawSignedContent === 'string') {
+          const signatureAttachment = parsed.attachments.find(a => a.treatAs(parsed.attachments) === 'signature'); // todo: more than one signature candidate?
+          if (signatureAttachment) {
+            this.view.signature.parsedSignature = signatureAttachment.getData().toUtfStr();
+            return await this.decryptAndRender(Buf.fromUtfStr(parsed.rawSignedContent), verificationPubs);
+          }
         }
+        await this.view.errorModule.renderErr(
+          'Error: could not properly parse signed message',
+          parsed.rawSignedContent || parsed.text || parsed.html || mimeMsg.toUtfStr(),
+          'parse error'
+        );
       } else if (this.view.encryptedMsgUrlParam && !forcePullMsgFromApi) {
         // ascii armored message supplied
         this.view.renderModule.renderText(this.view.signature ? 'Verifying...' : 'Decrypting...');

extension/chrome/elements/pgp_block_modules/pgp-block-render-module.ts

@@ -293,7 +293,7 @@ export class PgpBlockViewRenderModule {
         decryptedContent = this.getEncryptedSubjectText(decoded.subject, isHtml) + decryptedContent; // render encrypted subject in message
       }
       for (const attachment of decoded.attachments) {
-        if (attachment.treatAs() !== 'publicKey') {
+        if (attachment.treatAs(decoded.attachments) !== 'publicKey') {
           renderableAttachments.push(attachment);
         } else {
           publicKeys.push(attachment.getData().toUtfStr());

extension/js/common/api/email-provider/gmail/gmail.ts

@@ -325,7 +325,7 @@ export class Gmail extends EmailProviderApi implements EmailProviderInterface {
         return { armored: fromHtmlBody, subject, isPwdMsg };
       }
       for (const attachment of attachments) {
-        if (attachment.treatAs(!!textBody) === 'encryptedMsg') {
+        if (attachment.treatAs(attachments, !!textBody) === 'encryptedMsg') {
           await this.fetchAttachments([attachment], progressCb);
           const armoredMsg = PgpArmor.clip(attachment.getData().toUtfStr());
           if (!armoredMsg) {

extension/js/common/core/attachment.ts

@@ -41,7 +41,7 @@ export class Attachment {
   public contentTransferEncoding?: ContentTransferEncoding;
 
   private bytes: Uint8Array | undefined;
-  private treatAsValue: Attachment$treatAs | undefined;
+  private treatAsValue: Attachment$treatAs | undefined; // this field is to disable on-the-fly detection by this.treatAs()
 
   public constructor({ data, type, name, length, url, inline, id, msgId, treatAs, cid, contentDescription, contentTransferEncoding }: AttachmentMeta) {
     if (typeof data === 'undefined' && typeof url === 'undefined' && typeof id === 'undefined') {
@@ -101,6 +101,18 @@ export class Attachment {
     return `f_${Str.sloppyRandom(30)}@flowcrypt`;
   };
 
+  public isPublicKey = (): boolean => {
+    if (this.treatAsValue) {
+      return this.treatAsValue === 'publicKey';
+    }
+    return (
+      this.type === 'application/pgp-keys' ||
+      /^(0|0x)?[A-F0-9]{8}([A-F0-9]{8})?.*\.asc$/g.test(this.name) || // name starts with a key id
+      (this.name.toLowerCase().includes('public') && /[A-F0-9]{8}.*\.asc$/g.test(this.name)) || // name contains the word "public", any key id and ends with .asc
+      (/\.asc$/.test(this.name) && this.hasData() && Buf.with(this.getData().subarray(0, 100)).toUtfStr().includes('-----BEGIN PGP PUBLIC KEY BLOCK-----'))
+    );
+  };
+
   public hasData = () => {
     return this.bytes instanceof Uint8Array;
   };
@@ -122,13 +134,24 @@ export class Attachment {
     throw new Error('Attachment has no data set');
   };
 
-  public treatAs = (isBodyEmpty = false): Attachment$treatAs => {
+  public treatAs = (attachments: Attachment[], isBodyEmpty = false): Attachment$treatAs => {
     if (this.treatAsValue) {
       // pre-set
       return this.treatAsValue;
     } else if (['PGPexch.htm.pgp', 'PGPMIME version identification', 'Version.txt', 'PGPMIME Versions Identification'].includes(this.name)) {
       return 'hidden'; // PGPexch.htm.pgp is html alternative of textual body content produced by PGP Desktop and GPG4o
-    } else if (this.name === 'signature.asc' || this.type === 'application/pgp-signature') {
+    } else if (this.name === 'signature.asc') {
+      return 'signature';
+    } else if (this.type === 'application/pgp-signature') {
+      // this may be a signature for an attachment following these patterns:
+      // sample.name.sig for sample.name.pgp #3448
+      // or sample.name.sig for sample.name
+      if (attachments.length > 1) {
+        const nameWithoutExtension = Str.getFilenameWithoutExtension(this.name);
+        if (attachments.some(a => a !== this && (a.name === nameWithoutExtension || Str.getFilenameWithoutExtension(a.name) === nameWithoutExtension))) {
+          return 'hidden';
+        }
+      }
       return 'signature';
     } else if (!this.name && !this.type.startsWith('image/')) {
       // this.name may be '' or undefined - catch either
@@ -143,22 +166,11 @@ export class Attachment {
     } else if (this.name.match(/(\.pgp$)|(\.gpg$)|(\.[a-zA-Z0-9]{3,4}\.asc$)/g)) {
       // ends with one of .gpg, .pgp, .???.asc, .????.asc
       return 'encryptedFile';
+      // todo: after #4906 is done we should "decrypt" the encryptedFile here to see if it's a binary 'publicKey' (as in message 1869220e0c8f16dd)
+    } else if (this.isPublicKey()) {
+      return 'publicKey';
     } else if (this.name.match(/(cryptup|flowcrypt)-backup-[a-z0-9]+\.(key|asc)$/g)) {
       return 'privateKey';
-    } else if (this.type === 'application/pgp-keys') {
-      return 'publicKey';
-    } else if (this.name.match(/^(0|0x)?[A-F0-9]{8}([A-F0-9]{8})?.*\.asc$/g)) {
-      // name starts with a key id
-      return 'publicKey';
-    } else if (this.name.toLowerCase().includes('public') && this.name.match(/[A-F0-9]{8}.*\.asc$/g)) {
-      // name contains the word "public", any key id and ends with .asc
-      return 'publicKey';
-    } else if (
-      this.name.match(/\.asc$/) &&
-      this.hasData() &&
-      Buf.with(this.getData().subarray(0, 100)).toUtfStr().includes('-----BEGIN PGP PUBLIC KEY BLOCK-----')
-    ) {
-      return 'publicKey';
     } else if (this.name.match(/\.asc$/) && this.length < 100000 && !this.inline) {
       return 'encryptedMsg';
     } else {

extension/js/common/core/common.ts

@@ -196,6 +196,14 @@ export class Str {
     return rtlCount > lrtCount;
   };
 
+  // the regex has the most votes https://stackoverflow.com/a/4250408
+  public static getFilenameWithoutExtension = (filename: string): string => {
+    return filename.replace(/\.[^/.]+$/, '');
+  };
+
+  public static stripPgpOrGpgExtensionIfPresent = (filename: string) => {
+    return filename.replace(/\.(pgp|gpg)$/i, '');
+  };
   private static formatEmailWithOptionalNameEx = ({ email, name }: EmailParts, forceBrackets?: boolean): string => {
     if (name) {
       return `${Str.rmSpecialCharsKeepUtf(name, 'ALLOW-SOME')} <${email}>`;

extension/js/common/core/mime.ts

@@ -25,7 +25,6 @@ type MimeContentHeader = string | AddressHeader[];
 export type MimeContent = {
   headers: Dict<MimeContentHeader>;
   attachments: Attachment[];
-  signature?: string;
   rawSignedContent?: string;
   subject?: string;
   html?: string;
@@ -73,16 +72,17 @@ export class Mime {
     } else if (decoded.html) {
       blocks.push(MsgBlock.fromContent('plainHtml', decoded.html));
     }
+    const signatureAttachments: Attachment[] = [];
     for (const file of decoded.attachments) {
       const isBodyEmpty = decoded.text === '' || decoded.text === '\n';
-      const treatAs = file.treatAs(isBodyEmpty);
+      const treatAs = file.treatAs(decoded.attachments, isBodyEmpty);
       if (treatAs === 'encryptedMsg') {
         const armored = PgpArmor.clip(file.getData().toUtfStr());
         if (armored) {
           blocks.push(MsgBlock.fromContent('encryptedMsg', armored));
         }
       } else if (treatAs === 'signature') {
-        decoded.signature = decoded.signature || file.getData().toUtfStr();
+        signatureAttachments.push(file);
       } else if (treatAs === 'publicKey') {
         blocks.push(...MsgBlockParser.detectBlocks(file.getData().toUtfStr()).blocks);
       } else if (treatAs === 'privateKey') {
@@ -109,19 +109,21 @@ export class Mime {
         );
       }
     }
-    if (decoded.signature) {
+    if (signatureAttachments.length) {
+      // todo: if multiple signatures, figure out which fits what
+      const signature = signatureAttachments[0].getData().toUtfStr();
       for (const block of blocks) {
         if (block.type === 'plainText') {
           block.type = 'signedText';
-          block.signature = decoded.signature;
+          block.signature = signature;
         } else if (block.type === 'plainHtml') {
           block.type = 'signedHtml';
-          block.signature = decoded.signature;
+          block.signature = signature;
         }
       }
       if (!blocks.find(block => ['plainText', 'plainHtml', 'signedMsg', 'signedHtml', 'signedText'].includes(block.type))) {
         // signed an empty message
-        blocks.push(new MsgBlock('signedMsg', '', true, decoded.signature));
+        blocks.push(new MsgBlock('signedMsg', '', true, signature));
       }
     }
     return {
@@ -182,7 +184,6 @@ export class Mime {
       subject: undefined,
       text: undefined,
       html: undefined,
-      signature: undefined,
       from: undefined,
       to: [],
       cc: [],
@@ -210,9 +211,7 @@ export class Mime {
             }
             for (const node of Object.values(leafNodes)) {
               const nodeType = Mime.getNodeType(node);
-              if (nodeType === 'application/pgp-signature') {
-                mimeContent.signature = node.rawContent;
-              } else if (nodeType === 'text/html' && !Mime.getNodeFilename(node)) {
+              if (nodeType === 'text/html' && !Mime.getNodeFilename(node)) {
                 // html content may be broken up into smaller pieces by attachments in between
                 // AppleMail does this with inline attachments
                 mimeContent.html = (mimeContent.html || '') + Mime.getNodeContentAsUtfStr(node);
@@ -258,9 +257,9 @@ export class Mime {
         contentNode = Mime.newContentNode(MimeBuilder, Object.keys(body)[0], body[Object.keys(body)[0] as 'text/plain' | 'text/html'] || '');
       } else {
         contentNode = new MimeBuilder('multipart/alternative');
-        for (const type of Object.keys(body)) {
+        for (const [type, content] of Object.entries(body)) {
           // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-          contentNode.appendChild(Mime.newContentNode(MimeBuilder, type, body[type]!.toString())); // already present, that's why part of for loop
+          contentNode.appendChild(Mime.newContentNode(MimeBuilder, type, content!.toString())); // already present, that's why part of for loop
         }
       }
       rootNode.appendChild(contentNode);
@@ -306,9 +305,9 @@ export class Mime {
       rootNode.addHeader(key, headers[key]);
     }
     const bodyNodes = new MimeBuilder('multipart/alternative');
-    for (const type of Object.keys(body)) {
+    for (const [type, content] of Object.entries(body)) {
       // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-      bodyNodes.appendChild(Mime.newContentNode(MimeBuilder, type, body[type]!.toString()));
+      bodyNodes.appendChild(Mime.newContentNode(MimeBuilder, type, content!.toString()));
     }
     const signedContentNode = new MimeBuilder('multipart/mixed');
     signedContentNode.appendChild(bodyNodes);

extension/js/common/core/msg-block-parser.ts

@@ -82,7 +82,7 @@ export class MsgBlockParser {
       blocks.push(MsgBlock.fromContent('decryptedHtml', Str.escapeTextAsRenderableHtml(Buf.with(decryptedContent).toUtfStr()))); // escaped mime text as html
     }
     for (const attachment of decoded.attachments) {
-      if (attachment.treatAs() === 'publicKey') {
+      if (attachment.treatAs(decoded.attachments) === 'publicKey') {
         await MsgBlockParser.pushArmoredPubkeysToBlocks([attachment.getData().toUtfStr()], blocks);
       } else {
         blocks.push(

extension/js/content_scripts/webmail/gmail-element-replacer.ts

@@ -432,7 +432,7 @@ export class GmailElementReplacer implements WebmailElementReplacer {
     attachmentsContainerInner.parent().find(this.sel.numberOfAttachments).hide();
     let nRenderedAttachments = attachmentMetas.length;
     for (const a of attachmentMetas) {
-      const treatAs = a.treatAs(isBodyEmpty);
+      const treatAs = a.treatAs(attachmentMetas, isBodyEmpty);
       // todo - [same name + not processed].first() ... What if attachment metas are out of order compared to how gmail shows it? And have the same name?
       const attachmentSel = this.filterAttachments(
         attachmentsContainerInner.children().not('.attachment_processed'),

test/source/browser/browser-handle.ts

@@ -49,8 +49,8 @@ export class BrowserHandle {
     return this.newPage(t, t.urls?.extension(url));
   };
 
-  public newExtensionInboxPage = async (t: AvaContext, acctEmail: string): Promise<ControllablePage> => {
-    return this.newPage(t, t.urls?.extensionInbox(acctEmail));
+  public newExtensionInboxPage = async (t: AvaContext, acctEmail: string, threadId?: string): Promise<ControllablePage> => {
+    return this.newPage(t, t.urls?.extensionInbox(acctEmail, threadId));
   };
 
   public newExtensionSettingsPage = async (t: AvaContext, acctEmail?: string | undefined): Promise<ControllablePage> => {

test/source/browser/test-urls.ts

@@ -21,8 +21,9 @@ export class TestUrls {
     return this.extension(`chrome/settings/index.htm?account_email=${acctEmail || ''}`);
   };
 
-  public extensionInbox = (acctEmail: string) => {
-    return this.extension(`chrome/settings/inbox/inbox.htm?acctEmail=${acctEmail}`);
+  public extensionInbox = (acctEmail: string, threadId?: string) => {
+    const url = this.extension(`chrome/settings/inbox/inbox.htm?acctEmail=${acctEmail}`);
+    return threadId ? url + `&threadId=${threadId}` : url;
   };
 
   public mockGmailUrl = () => `https://gmail.localhost:${this.port}/gmail`;