Skip to content

Commit ff02ec6

Browse files
#4529 Show correct message for sender and signer email mismatch (#6180)
* feat: show correct message for sender and signer email mismatch * fix: test * fix: pr review
1 parent 0e911c9 commit ff02ec6

File tree

3 files changed

+81
-9
lines changed

3 files changed

+81
-9
lines changed

extension/js/common/message-renderer.ts

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ export class MessageRenderer {
154154
private static async renderPgpSignatureCheckResult(
155155
renderModule: RenderInterface,
156156
verifyRes: VerifyRes | undefined,
157-
wasSignerEmailSupplied: boolean,
157+
senderEmail: string | undefined,
158158
retryVerification?: () => Promise<VerifyRes | undefined>
159159
) {
160160
if (verifyRes?.error) {
@@ -188,25 +188,39 @@ export class MessageRenderer {
188188
retryVerificationAgain = retryVerification;
189189
}
190190
}
191-
await MessageRenderer.renderPgpSignatureCheckResult(renderModule, verifyRes, wasSignerEmailSupplied, retryVerificationAgain);
191+
await MessageRenderer.renderPgpSignatureCheckResult(renderModule, verifyRes, senderEmail, retryVerificationAgain);
192192
return;
193-
} else if (!wasSignerEmailSupplied) {
193+
} else if (!senderEmail) {
194194
// todo: unit-test this case?
195195
renderModule.renderSignatureStatus('could not verify signature: missing pubkey, missing sender info');
196196
} else {
197-
MessageRenderer.renderMissingPubkeyOrBadSignature(renderModule, verifyRes);
197+
await MessageRenderer.renderMissingPubkeyOrBadSignature(renderModule, verifyRes, senderEmail);
198198
}
199199
}
200200

201-
private static renderMissingPubkeyOrBadSignature(renderModule: RenderInterfaceBase, verifyRes: VerifyRes): void {
201+
private static async renderMissingPubkeyOrBadSignature(renderModule: RenderInterfaceBase, verifyRes: VerifyRes, senderEmail: string): Promise<void> {
202202
// eslint-disable-next-line no-null/no-null
203203
if (verifyRes.match === null || !Value.arr.hasIntersection(verifyRes.signerLongids, verifyRes.suppliedLongids)) {
204-
MessageRenderer.renderMissingPubkey(renderModule, verifyRes.signerLongids[0]);
204+
const signerLongid = verifyRes.signerLongids[0];
205+
const signerEmails = await ContactStore.getEmailsByLongid(undefined, signerLongid);
206+
if (signerEmails.includes(senderEmail)) {
207+
// signer key is associated with the sender — not a mismatch, but pubkey wasn't supplied for verification
208+
MessageRenderer.renderMissingPubkey(renderModule, signerLongid);
209+
} else if (signerEmails.length > 0) {
210+
MessageRenderer.renderSignerSenderMismatch(renderModule, senderEmail, signerEmails[0]);
211+
} else {
212+
MessageRenderer.renderMissingPubkey(renderModule, signerLongid);
213+
}
205214
} else {
206215
MessageRenderer.renderBadSignature(renderModule);
207216
}
208217
}
209218

219+
private static renderSignerSenderMismatch(renderModule: RenderInterfaceBase, senderEmail: string, signerEmail: string) {
220+
renderModule.renderSignatureStatus(`could not verify signature: signed by ${signerEmail} but message is from ${senderEmail}`);
221+
renderModule.setFrameColor('red');
222+
}
223+
210224
private static renderMissingPubkey(renderModule: RenderInterfaceBase, signerLongid: string) {
211225
renderModule.renderSignatureStatus(`could not verify signature: missing pubkey ${signerLongid}`);
212226
}
@@ -615,7 +629,7 @@ export class MessageRenderer {
615629
}
616630
decryptedContent = this.clipMessageIfLimitExceeds(decryptedContent);
617631
renderModule.separateQuotedContentAndRenderText(decryptedContent, isHtml, isChecksumInvalid);
618-
await MessageRenderer.renderPgpSignatureCheckResult(renderModule, sigResult, Boolean(signerEmail), retryVerification);
632+
await MessageRenderer.renderPgpSignatureCheckResult(renderModule, sigResult, signerEmail, retryVerification);
619633
if (renderableAttachments.length) {
620634
renderModule.renderInnerAttachments(renderableAttachments, isEncrypted);
621635
}

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -546,6 +546,39 @@ export class ContactStore extends AbstractStore {
546546
return (await ContactStore.extractPubkeys(db, fingerprints)).map(pubkey => pubkey.armoredKey).filter(Boolean);
547547
}
548548

549+
public static async getEmailsByLongid(db: IDBDatabase | undefined, longid: string): Promise<string[]> {
550+
if (!db) {
551+
return (await BrowserMsg.retryOnBgNotReadyErr(() => BrowserMsg.send.bg.await.db({ f: 'getEmailsByLongid', args: [longid] }))) as string[];
552+
}
553+
const tx = db.transaction(['pubkeys', 'emails'], 'readonly');
554+
const emails: string[] = [];
555+
await new Promise<void>((resolve, reject) => {
556+
const req = tx.objectStore('pubkeys').index('index_longids').getAll(longid);
557+
ContactStore.setReqPipe(
558+
req,
559+
(pubkeys: Pubkey[]) => {
560+
if (!pubkeys.length) {
561+
resolve();
562+
return;
563+
}
564+
ContactStore.setTxHandlers(tx, () => resolve(), reject);
565+
for (const pubkey of pubkeys) {
566+
const req2 = tx.objectStore('emails').index('index_fingerprints').getAll(pubkey.fingerprint);
567+
ContactStore.setReqPipe(req2, (emailEntities: Email[]) => {
568+
for (const entity of emailEntities) {
569+
if (!emails.includes(entity.email)) {
570+
emails.push(entity.email);
571+
}
572+
}
573+
});
574+
}
575+
},
576+
reject
577+
);
578+
});
579+
return emails;
580+
}
581+
549582
public static async getOneWithAllPubkeys(db: IDBDatabase | undefined, email: string): Promise<ContactInfoWithSortedPubkeys | undefined> {
550583
if (!db) {
551584
// relay op through background process

test/source/tests/decrypt.ts

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ export const defineDecryptTests = (testVariant: TestVariant, testWithBrowser: Te
177177
const { acctEmail, authHdr } = await BrowserRecipe.setupCommonAcctWithAttester(t, browser, 'compatibility');
178178
const expectedMessage = {
179179
encryption: 'not encrypted',
180-
signature: 'could not verify signature: missing pubkey ADAC279C95093207',
180+
signature: 'could not verify signature: signed by flowcrypt.compatibility@gmail.com but message is from sender@domain.com',
181181
content: ['flowcrypt-browser issue #5029 test email'],
182182
};
183183
const inboxPage = await browser.newExtensionPage(t, `chrome/settings/inbox/inbox.htm?acctEmail=${acctEmail}&threadId=${threadId}`);
@@ -542,7 +542,7 @@ export const defineDecryptTests = (testVariant: TestVariant, testWithBrowser: Te
542542
{
543543
content: ['this was encrypted with gpg', 'gpg --sign --armor -r flowcrypt.compatibility@gmail.com ./text.txt'],
544544
encryption: 'not encrypted',
545-
signature: 'could not verify signature: missing pubkey 7FDE685548AEA788',
545+
signature: 'could not verify signature: signed by flowcrypt.compatibility@gmail.com but message is from sender@domain.com',
546546
quoted: false,
547547
},
548548
authHdr
@@ -1535,6 +1535,31 @@ XZ8r4OC6sguP/yozWlkG+7dDxsgKQVBENeG6Lw==
15351535
})
15361536
);
15371537

1538+
test(
1539+
'signature - shows sender/signer mismatch when signer key belongs to a different email',
1540+
testWithBrowser(async (t, browser) => {
1541+
const threadId = '1766644f13510f58';
1542+
const { authHdr } = await BrowserRecipe.setupCommonAcctWithAttester(t, browser, 'ci.tests.gmail');
1543+
const dbPage = await browser.newExtensionPage(t, 'chrome/dev/ci_unit_test.htm');
1544+
await dbPage.page.evaluate(async (pubkey: string) => {
1545+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
1546+
const key = await (window as any).KeyUtil.parse(pubkey);
1547+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
1548+
await (window as any).ContactStore.update(undefined, 'actual.signer@example.com', { pubkey: key });
1549+
}, testConstants.pubkey2864E326A5BE488A);
1550+
await dbPage.close();
1551+
const gmailPage = await browser.newPage(t, `${t.context.urls?.mockGmailUrl()}/${threadId}`, undefined, authHdr);
1552+
await gmailPage.waitAll('iframe', { timeout: 2 });
1553+
const pgpBlock = await gmailPage.getFrame(['/chrome/elements/pgp_block.htm'], { sleep: 10, timeout: 20 });
1554+
const expectedMessage = {
1555+
content: ['How is my message signed?'],
1556+
encryption: 'not encrypted',
1557+
signature: 'could not verify signature: signed by actual.signer@example.com but message is from sender@example.com',
1558+
};
1559+
await BrowserRecipe.pgpBlockCheck(t, pgpBlock, expectedMessage);
1560+
})
1561+
);
1562+
15381563
test(
15391564
'decrypt - protonmail - PGP/inline signed and encrypted message with pubkey - pubkey signature is ignored',
15401565
testWithBrowser(async (t, browser) => {

0 commit comments

Comments
 (0)