Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
8f250eb
refactor decryption functions to use verificationPubs as parameter
rrrooommmaaa Nov 25, 2021
b8e5ce5
more refactorings and fixes
rrrooommmaaa Nov 28, 2021
e2baa50
Merge remote-tracking branch 'origin/master' into issue-3884-verifica…
rrrooommmaaa Nov 28, 2021
6436edf
fix
rrrooommmaaa Nov 28, 2021
7c9c25c
fix unit tests that used to query by longid
rrrooommmaaa Nov 28, 2021
ba745e8
fixed some tests
rrrooommmaaa Nov 30, 2021
8ae8d04
removed unused browser msg
rrrooommmaaa Nov 30, 2021
b0c43d4
renames
rrrooommmaaa Nov 30, 2021
9287eaa
Merge remote-tracking branch 'origin/master' into issue-3884-verifica…
rrrooommmaaa Nov 30, 2021
482fff0
fixed Message Not Signed and Missing Pubkey
rrrooommmaaa Dec 1, 2021
ba4db34
Merge remote-tracking branch 'origin/master' into issue-3884-verifica…
rrrooommmaaa Dec 1, 2021
d4160b7
fix
rrrooommmaaa Dec 1, 2021
94a6846
added remarks to link to #4158
rrrooommmaaa Dec 1, 2021
7130cf8
remark
rrrooommmaaa Dec 1, 2021
e048d61
refactorings
rrrooommmaaa Dec 2, 2021
2609b7d
re-fetching signed-only message on non-fatal verification error
rrrooommmaaa Dec 5, 2021
4098b19
Merge remote-tracking branch 'origin/master' into issue-3884-verifica…
rrrooommmaaa Dec 5, 2021
7dfdb80
remark
rrrooommmaaa Dec 5, 2021
ee7c40d
Merge remote-tracking branch 'origin/master' into issue-3884-verifica…
rrrooommmaaa Dec 6, 2021
937622c
Merge remote-tracking branch 'origin/master' into issue-3884-verifica…
rrrooommmaaa Dec 6, 2021
99b40fc
differentiate bad signature and missing pubkey, red color any outcome…
rrrooommmaaa Dec 6, 2021
a86ced2
Merge remote-tracking branch 'origin/master' into issue-3884-verifica…
rrrooommmaaa Dec 7, 2021
ca27504
save fetched pubkeys to ContactStore
rrrooommmaaa Dec 7, 2021
48f6bf7
lint fix
rrrooommmaaa Dec 7, 2021
2de48fa
remarks
rrrooommmaaa Dec 7, 2021
00ddd74
Merge remote-tracking branch 'origin/master' into issue-3884-verifica…
rrrooommmaaa Dec 8, 2021
4f009ed
fixes
rrrooommmaaa Dec 8, 2021
12b3f67
lint fix
rrrooommmaaa Dec 8, 2021
df83c88
Merge remote-tracking branch 'origin/master' into issue-3884-verifica…
rrrooommmaaa Dec 10, 2021
a07a895
lint fix
rrrooommmaaa Dec 10, 2021
84d9298
removed capitalization and changed sender's email for one message
rrrooommmaaa Dec 10, 2021
f5c2dc6
Merge remote-tracking branch 'origin/master' into issue-3884-verifica…
rrrooommmaaa Dec 11, 2021
3bbd2ad
common method to save fetched pubkeys to storage
rrrooommmaaa Dec 11, 2021
1456c40
remark regarding cleartext
rrrooommmaaa Dec 11, 2021
a73fc48
VerifyRes.match=false on message digest mismatch
rrrooommmaaa Dec 12, 2021
362b7b5
removed PII
rrrooommmaaa Dec 12, 2021
4d96aa7
Merge remote-tracking branch 'origin/master' into issue-3884-verifica…
rrrooommmaaa Dec 14, 2021
cb682af
moved key sorting to platform-independent module
rrrooommmaaa Dec 15, 2021
e1415f0
niceties
rrrooommmaaa Dec 15, 2021
fe88d62
removed unnecessary optionalPwd parameter
rrrooommmaaa Dec 15, 2021
2df3953
better naming
rrrooommmaaa Dec 15, 2021
761f9a9
don't refetch encrypted message on verification error
rrrooommmaaa Dec 16, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions extension/chrome/elements/attachment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,11 @@ export class AttachmentDownloadView extends View {
private processAsPublicKeyAndHideAttachmentIfAppropriate = async () => {
if (this.attachment.msgId && this.attachment.id && this.attachment.treatAs() === 'publicKey') { // this is encrypted public key - download && decrypt & parse & render
const { data } = await this.gmail.attachmentGet(this.attachment.msgId, this.attachment.id);
const decrRes = await MsgUtil.decryptMessage({ kisWithPp: await KeyStore.getAllWithOptionalPassPhrase(this.acctEmail), encryptedData: data });
const decrRes = await MsgUtil.decryptMessage({
kisWithPp: await KeyStore.getAllWithOptionalPassPhrase(this.acctEmail),
encryptedData: data,
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 });
if (openpgpType && openpgpType.type === 'publicKey' && openpgpType.armored) { // 'openpgpType.armored': could potentially process unarmored pubkey files, maybe later
Expand Down Expand Up @@ -254,7 +258,11 @@ export class AttachmentDownloadView extends View {
};

private decryptAndSaveAttachmentToDownloads = async () => {
const result = await MsgUtil.decryptMessage({ kisWithPp: await KeyStore.getAllWithOptionalPassPhrase(this.acctEmail), encryptedData: this.attachment.getData() });
const result = await MsgUtil.decryptMessage({
kisWithPp: await KeyStore.getAllWithOptionalPassPhrase(this.acctEmail),
encryptedData: this.attachment.getData(),
verificationPubs: [] // todo: #4158 signature verification of attachments
});
Xss.sanitizeRender(this.downloadButton, this.originalButtonHTML || '');
if (result.success) {
if (!result.filename || ['msg.txt', 'null'].includes(result.filename)) {
Expand Down
6 changes: 5 additions & 1 deletion extension/chrome/elements/attachment_preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,11 @@ View.run(class AttachmentPreviewView extends AttachmentDownloadView {
};

private decrypt = async () => {
const result = await MsgUtil.decryptMessage({ kisWithPp: await KeyStore.getAllWithOptionalPassPhrase(this.acctEmail), encryptedData: this.attachment.getData() });
const result = await MsgUtil.decryptMessage({
kisWithPp: await KeyStore.getAllWithOptionalPassPhrase(this.acctEmail),
encryptedData: this.attachment.getData(),
verificationPubs: [] // todo: #4158 signature verification of attachments
});
if ((result as DecryptSuccess).content) {
return result.content;
} else if ((result as DecryptError).error.type === DecryptErrTypes.needPassphrase) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,11 @@ export class ComposeDraftModule extends ViewModule<ComposeView> {
return await this.abortAndRenderReplyMsgComposeTableIfIsReplyBox('!rawBlock');
}
const encryptedData = rawBlock.content instanceof Buf ? rawBlock.content : Buf.fromUtfStr(rawBlock.content);
const decrypted = await MsgUtil.decryptMessage({ kisWithPp: await KeyStore.getAllWithOptionalPassPhrase(this.view.acctEmail), encryptedData });
const decrypted = await MsgUtil.decryptMessage({
kisWithPp: await KeyStore.getAllWithOptionalPassPhrase(this.view.acctEmail),
encryptedData,
verificationPubs: []
});
if (!decrypted.success) {
if (decrypted.error.type === DecryptErrTypes.needPassphrase) {
// "close" button will wipe this frame out, so no need to exit the recursion
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,11 @@ export class ComposeQuoteModule extends ViewModule<ComposeView> {
let attachmentMeta: { content: Buf, filename?: string } | undefined;
if (block.type === 'encryptedAttachment') {
this.setQuoteLoaderProgress('decrypting...');
const result = await MsgUtil.decryptMessage({ kisWithPp: await KeyStore.getAllWithOptionalPassPhrase(this.view.acctEmail), encryptedData: block.attachmentMeta.data });
const result = await MsgUtil.decryptMessage({
kisWithPp: await KeyStore.getAllWithOptionalPassPhrase(this.view.acctEmail),
encryptedData: block.attachmentMeta.data,
verificationPubs: [] // todo: #4158 signature verification of attachments
});
if (result.success) {
attachmentMeta = { content: result.content, filename: result.filename };
}
Expand Down Expand Up @@ -160,7 +164,7 @@ export class ComposeQuoteModule extends ViewModule<ComposeView> {
};

private decryptMessage = async (encryptedData: Buf): Promise<string> => {
const decryptRes = await MsgUtil.decryptMessage({ kisWithPp: await KeyStore.getAllWithOptionalPassPhrase(this.view.acctEmail), encryptedData });
const decryptRes = await MsgUtil.decryptMessage({ kisWithPp: await KeyStore.getAllWithOptionalPassPhrase(this.view.acctEmail), encryptedData, verificationPubs: [] });
if (decryptRes.success) {
return decryptRes.content.toUtfStr();
} else if (decryptRes.error && decryptRes.error.type === DecryptErrTypes.needPassphrase) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
'use strict';

import { ChunkedCb, EmailProviderContact, RecipientType } from '../../../js/common/api/shared/api.js';
import { Contact, KeyUtil } from '../../../js/common/core/crypto/key.js';
import { Contact, KeyUtil, PubkeyInfo } from '../../../js/common/core/crypto/key.js';
import { PUBKEY_LOOKUP_RESULT_FAIL, PUBKEY_LOOKUP_RESULT_WRONG } from './compose-err-module.js';
import { ProviderContactsQuery, Recipients } from '../../../js/common/api/email-provider/email-provider-api.js';
import { RecipientElement, RecipientStatus } from './compose-types.js';
Expand All @@ -20,7 +20,7 @@ import { moveElementInArray } from '../../../js/common/platform/util.js';
import { ViewModule } from '../../../js/common/view-module.js';
import { ComposeView } from '../compose.js';
import { AcctStore } from '../../../js/common/platform/store/acct-store.js';
import { ContactPreview, ContactStore, ContactUpdate, PubkeyInfo } from '../../../js/common/platform/store/contact-store.js';
import { ContactPreview, ContactStore, ContactUpdate } from '../../../js/common/platform/store/contact-store.js';

/**
* todo - this class is getting too big
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
'use strict';

import { BrowserMsg } from '../../../js/common/browser/browser-msg.js';
import { KeyInfo, KeyUtil, Key, PubkeyResult } from '../../../js/common/core/crypto/key.js';
import { KeyInfo, KeyUtil, Key, PubkeyInfo, PubkeyResult } from '../../../js/common/core/crypto/key.js';
import { ApiErr } from '../../../js/common/api/shared/api-error.js';
import { Assert } from '../../../js/common/assert.js';
import { Catch, UnreportableError } from '../../../js/common/platform/catch.js';
Expand All @@ -13,9 +13,10 @@ import { ViewModule } from '../../../js/common/view-module.js';
import { ComposeView } from '../compose.js';
import { KeyStore } from '../../../js/common/platform/store/key-store.js';
import { AcctStore } from '../../../js/common/platform/store/acct-store.js';
import { ContactStore, PubkeyInfo } from '../../../js/common/platform/store/contact-store.js';
import { ContactStore } from '../../../js/common/platform/store/contact-store.js';
import { PassphraseStore } from '../../../js/common/platform/store/passphrase-store.js';
import { Settings } from '../../../js/common/settings.js';
import { compareAndSavePubkeysToStorage } from '../../../js/common/shared.js';

export class ComposeStorageModule extends ViewModule<ComposeView> {
// if `type` is supplied, returns undefined if no keys of this type are found
Expand Down Expand Up @@ -176,15 +177,10 @@ export class ComposeStorageModule extends ViewModule<ComposeView> {
}
try {
const lookupResult = await this.view.pubLookup.lookupEmail(email);
const fetchedPubkeys = await Promise.all(lookupResult.pubkeys.map(KeyUtil.parse));
for (const fetched of fetchedPubkeys) {
const stored = storedPubkeys.find(p => KeyUtil.identityEquals(p.pubkey, fetched))?.pubkey;
if (!stored || KeyUtil.isFetchedNewer({ fetched, stored })) {
await ContactStore.update(undefined, email, { name, pubkey: fetched, pubkeyLastCheck: Date.now() });
await this.view.recipientsModule.reRenderRecipientFor(email);
}
if (await compareAndSavePubkeysToStorage(email, lookupResult.pubkeys, storedPubkeys)) {
await this.view.recipientsModule.reRenderRecipientFor(email);
}
if (!fetchedPubkeys.length && name) { // update just name
if (name) { // update name
await ContactStore.update(undefined, email, { name });
}
} catch (e) {
Expand Down
28 changes: 25 additions & 3 deletions extension/chrome/elements/pgp_block.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import { View } from '../../js/common/view.js';
import { PubLookup } from '../../js/common/api/pub-lookup.js';
import { OrgRules } from '../../js/common/org-rules.js';
import { AcctStore } from '../../js/common/platform/store/acct-store.js';
import { ContactStore } from '../../js/common/platform/store/contact-store.js';
import { KeyUtil } from '../../js/common/core/crypto/key.js';

export class PgpBlockView extends View {

Expand All @@ -28,7 +30,10 @@ export class PgpBlockView extends View {
public readonly senderEmail: string;
public readonly msgId: string | undefined;
public readonly encryptedMsgUrlParam: Buf | undefined;
public signature: string | boolean | undefined; // when supplied with "true", decryptModule will replace this with actual signature data
public readonly signature?: {
// when parsedSignature is undefined, decryptModule will try to fetch the message
parsedSignature?: string
};

public gmail: Gmail;
public orgRules!: OrgRules;
Expand Down Expand Up @@ -56,7 +61,11 @@ export class PgpBlockView extends View {
throw new Error('API path traversal forbidden');
}
this.encryptedMsgUrlParam = uncheckedUrlParams.message ? Buf.fromUtfStr(Assert.urlParamRequire.string(uncheckedUrlParams, 'message')) : undefined;
this.signature = uncheckedUrlParams.signature === true ? true : (uncheckedUrlParams.signature ? String(uncheckedUrlParams.signature) : undefined);
if (uncheckedUrlParams.signature === true) {
this.signature = { parsedSignature: undefined }; // decryptModule will try to fetch the message
} else if (uncheckedUrlParams.signature) {
this.signature = { parsedSignature: String(uncheckedUrlParams.signature) };
}
this.gmail = new Gmail(this.acctEmail);
// modules
this.attachmentsModule = new PgpBlockViewAttachmentsModule(this);
Expand All @@ -67,14 +76,27 @@ export class PgpBlockView extends View {
this.decryptModule = new PgpBlockViewDecryptModule(this);
}

public getExpectedSignerEmail = () => {
// We always attempt to verify all signatures as "signed by sender", with public keys of the sender.
// That way, signature spoofing attacks are prevented: if Joe manages to spoof a sending address
// of Jane (send an email from Jane address), then we expect Jane to be this signer: we look up
// keys recorded for Jane and the signature either succeeds or fails to verify. If it fails (that pubkey
// which Joe used is not recorded for Jane), it will show an error.
return this.senderEmail;
};

public render = async () => {
const storage = await AcctStore.get(this.acctEmail, ['setup_done', 'google_token_scopes']);
this.orgRules = await OrgRules.newInstance(this.acctEmail);
this.pubLookup = new PubLookup(this.orgRules);
const scopes = await AcctStore.getScopes(this.acctEmail);
this.decryptModule.canReadEmails = scopes.read || scopes.modify;
if (storage.setup_done) {
await this.decryptModule.initialize();
const parsedPubs = (await ContactStore.getOneWithAllPubkeys(undefined, this.getExpectedSignerEmail()))?.sortedPubkeys ?? [];
// todo: we don't actually need parsed pubs here because we're going to pass them to the backgorund page
// maybe we can have a method in ContactStore to extract armored keys
const verificationPubs = parsedPubs.map(key => KeyUtil.armor(key.pubkey));
await this.decryptModule.initialize(verificationPubs, false);
} else {
await this.errorModule.renderErr(Lang.pgpBlock.refreshWindow, this.encryptedMsgUrlParam ? this.encryptedMsgUrlParam.toUtfStr() : undefined);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ export class PgpBlockViewAttachmentsModule {

private decryptAndSaveAttachmentToDownloads = async (encrypted: Attachment) => {
const kisWithPp = await KeyStore.getAllWithOptionalPassPhrase(this.view.acctEmail);
const decrypted = await BrowserMsg.send.bg.await.pgpMsgDecrypt({ kisWithPp, encryptedData: encrypted.getData() });
// todo: #4158 signature verification of attachments
const decrypted = await BrowserMsg.send.bg.await.pgpMsgDecrypt({ kisWithPp, encryptedData: encrypted.getData(), verificationPubs: [] });
if (decrypted.success) {
const attachment = new Attachment({ name: encrypted.name.replace(/\.(pgp|gpg)$/, ''), type: encrypted.type, data: decrypted.content });
Browser.saveToDownloads(attachment);
Expand Down
Loading