Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#5868 Display warning for incorrect pgp checksum #5883

Merged
merged 3 commits into from
Dec 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion extension/chrome/elements/pgp_block.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@ export class PgpBlockView extends View {
if (data?.separateQuotedContentAndRenderText) {
this.quoteModule.separateQuotedContentAndRenderText(
data.separateQuotedContentAndRenderText.decryptedContent,
data.separateQuotedContentAndRenderText.isHtml
data.separateQuotedContentAndRenderText.isHtml,
data.separateQuotedContentAndRenderText.isChecksumInvalid
);
}
if (data?.setFrameColor) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { Xss } from '../../../js/common/platform/xss.js';
export class PgpBlockViewQuoteModule {
public constructor(private view: PgpBlockView) {}

public separateQuotedContentAndRenderText = (decryptedContent: string, isHtml: boolean) => {
public separateQuotedContentAndRenderText = (decryptedContent: string, isHtml: boolean, isChecksumInvalid: boolean) => {
if (isHtml) {
const message = $('<div>').html(Xss.htmlSanitizeKeepBasicTags(decryptedContent)); // xss-sanitized
let htmlBlockQuoteExists = false;
Expand All @@ -32,10 +32,10 @@ export class PgpBlockViewQuoteModule {
message[0].removeChild(shouldBeQuoted[i]);
quotedHtml += shouldBeQuoted[i].outerHTML;
}
this.view.renderModule.renderContent(message.html(), false);
this.view.renderModule.renderContent(message.html(), false, isChecksumInvalid);
this.appendCollapsedQuotedContentButton(quotedHtml, true);
} else {
this.view.renderModule.renderContent(decryptedContent, false);
this.view.renderModule.renderContent(decryptedContent, false, isChecksumInvalid);
}
} else {
const lines = decryptedContent.split(/\r?\n/);
Expand All @@ -62,7 +62,7 @@ export class PgpBlockViewQuoteModule {
// only got quoted part, no real text -> show everything as real text, without quoting
lines.push(...linesQuotedPart.splice(0, linesQuotedPart.length));
}
this.view.renderModule.renderContent(Str.escapeTextAsRenderableHtml(lines.join('\n')), false);
this.view.renderModule.renderContent(Str.escapeTextAsRenderableHtml(lines.join('\n')), false, isChecksumInvalid);
if (linesQuotedPart.join('').trim()) {
this.appendCollapsedQuotedContentButton(linesQuotedPart.join('\n'));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,11 @@ export class PgpBlockViewRenderModule {
});
};

public renderContent = (htmlContent: string, isErr: boolean) => {
public renderContent = (htmlContent: string, isErr: boolean, isChecksumInvalid = false) => {
let contentWithLink = linkifyHtml(htmlContent);

if (isChecksumInvalid) {
contentWithLink = `<div class="pgp-invalid-checksum">${Lang.pgpBlock.invalidCheckSum}</div>${contentWithLink}}`;
}
// Temporary workaround for an issue where 'cryptup_reply' divs are not being hidden when replying to all
// messages from the FES. The root cause is that FES currently returns the body of
// password message replies as 'text/plain', which does not hide the divs as intended.
Expand Down
7 changes: 7 additions & 0 deletions extension/css/cryptup.css
Original file line number Diff line number Diff line change
Expand Up @@ -1592,6 +1592,13 @@ td {
align-items: center;
}

.pgp-invalid-checksum {
border-left: 3px solid #d18826;
color: #d18826;
margin: 10px 0;
padding-left: 5px;
}

.pgp_badge {
opacity: 1;
margin-bottom: 1em;
Expand Down
54 changes: 54 additions & 0 deletions extension/js/common/core/crypto/key.ts
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,60 @@ export class KeyUtil {
return keys.map(k => ({ id: k.id, emails: k.emails, armored: KeyUtil.armor(k), family: k.family }));
}

public static validateChecksum = (armoredText: string): boolean => {
ioanmo226 marked this conversation as resolved.
Show resolved Hide resolved
const lines = armoredText.split('\n').map(l => l.trim());

// Filter out known non-data lines
const dataCandidates = lines.filter(line => line.length > 0 && !line.startsWith('-----') && !line.startsWith('Version:') && !line.startsWith('Comment:'));

// Find checksum line
const checksumIndex = dataCandidates.findIndex(line => line.startsWith('='));
if (checksumIndex === -1) return false;

const checksumLine = dataCandidates[checksumIndex].slice(1);
const dataLines = dataCandidates.slice(0, checksumIndex);

// Decode checksum
let providedBytes: string;
try {
providedBytes = atob(checksumLine);
} catch {
return false;
}
if (providedBytes.length !== 3) return false;

const providedCRC = (providedBytes.charCodeAt(0) << 16) | (providedBytes.charCodeAt(1) << 8) | providedBytes.charCodeAt(2);

// Attempt to decode all data lines (some may not be base64)
const decodedChunks: string[] = [];
for (const line of dataLines) {
try {
decodedChunks.push(atob(line));
} catch {
// Not a valid base64 line, skip it
}
}

if (decodedChunks.length === 0) return false;

const rawData = decodedChunks.join('');
const dataBytes = new Uint8Array([...rawData].map(c => c.charCodeAt(0)));

return KeyUtil.crc24(dataBytes) === providedCRC;
};

private static crc24 = (dataBytes: Uint8Array): number => {
let crc = 0xb704ce;
for (const dataByte of dataBytes) {
crc ^= dataByte << 16;
for (let j = 0; j < 8; j++) {
crc <<= 1;
if (crc & 0x1000000) crc ^= 0x1864cfb;
}
}
return crc & 0xffffff;
};

private static getSortValue(pubinfo: PubkeyInfo): number {
const expirationSortValue = typeof pubinfo.pubkey.expiration === 'undefined' ? Infinity : pubinfo.pubkey.expiration;
// sort non-revoked first, then non-expired
Expand Down
2 changes: 2 additions & 0 deletions extension/js/common/lang.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ export const Lang = {
cannotLocate: 'Could not locate this message.',
brokenLink: 'It seems it contains a broken link.',
pwdMsgAskSenderUsePubkey: 'This appears to be a password-protected message. Please ask the sender to encrypt messages for your Public Key instead.',
invalidCheckSum:
'Warning: Checksum mismatch detected.\nThis indicates the message may have been altered or corrupted during transmission.\nDecryption may still succeed, but verify the message source and integrity if possible.',
},
compose: {
abortSending: 'A message is currently being sent. Closing the compose window may abort sending the message.\nAbort sending?',
Expand Down
8 changes: 5 additions & 3 deletions extension/js/common/message-renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,8 @@ export class MessageRenderer {
sigResult: VerifyRes | undefined,
renderModule: RenderInterface,
retryVerification: (() => Promise<VerifyRes | undefined>) | undefined,
plainSubject: string | undefined
plainSubject: string | undefined,
isChecksumInvalid = false
): Promise<{ publicKeys?: string[] }> => {
if (isEncrypted) {
renderModule.renderEncryptionStatus('encrypted');
Expand Down Expand Up @@ -603,7 +604,7 @@ export class MessageRenderer {
);
}
decryptedContent = this.clipMessageIfLimitExceeds(decryptedContent);
renderModule.separateQuotedContentAndRenderText(decryptedContent, isHtml);
renderModule.separateQuotedContentAndRenderText(decryptedContent, isHtml, isChecksumInvalid);
await MessageRenderer.renderPgpSignatureCheckResult(renderModule, sigResult, Boolean(signerEmail), retryVerification);
if (renderableAttachments.length) {
renderModule.renderInnerAttachments(renderableAttachments, isEncrypted);
Expand Down Expand Up @@ -743,7 +744,8 @@ export class MessageRenderer {
result.signature,
renderModule,
this.getRetryVerification(signerEmail, verificationPubs => MessageRenderer.decryptFunctionToVerifyRes(() => decrypt(verificationPubs))),
plainSubject
plainSubject,
!KeyUtil.validateChecksum(encryptedData.toString())
);
} else if (result.error.type === DecryptErrTypes.format) {
if (fallbackToPlainText) {
Expand Down
2 changes: 1 addition & 1 deletion extension/js/common/render-interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export interface RenderInterface extends RenderInterfaceBase {
renderPassphraseNeeded(longids: string[]): void;
renderErr(errBoxContent: string, renderRawMsg: string | undefined, errMsg?: string): void;
renderInnerAttachments(attachments: TransferableAttachment[], isEncrypted: boolean): void;
separateQuotedContentAndRenderText(decryptedContent: string, isHtml: boolean): void;
separateQuotedContentAndRenderText(decryptedContent: string, isHtml: boolean, isChecksumInvalid: boolean): void;
renderVerificationInProgress(): void;
renderSignatureOffline(retry: () => void): void;
}
2 changes: 1 addition & 1 deletion extension/js/common/render-message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export type MessageInfo = {
export type RenderMessage = {
done?: true;
resizePgpBlockFrame?: true;
separateQuotedContentAndRenderText?: { decryptedContent: string; isHtml: boolean };
separateQuotedContentAndRenderText?: { decryptedContent: string; isHtml: boolean; isChecksumInvalid: boolean };
renderText?: string;
progressOperation?: { operationId: string; text: string; perc?: number; init?: boolean };
setFrameColor?: 'green' | 'gray' | 'red';
Expand Down
4 changes: 2 additions & 2 deletions extension/js/common/render-relay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ export class RenderRelay implements RenderInterface {
this.relay({ resizePgpBlockFrame: true });
};

public separateQuotedContentAndRenderText = (decryptedContent: string, isHtml: boolean) => {
this.relay({ separateQuotedContentAndRenderText: { decryptedContent, isHtml } });
public separateQuotedContentAndRenderText = (decryptedContent: string, isHtml: boolean, isChecksumInvalid: boolean) => {
this.relay({ separateQuotedContentAndRenderText: { decryptedContent, isHtml, isChecksumInvalid } });
};

public renderText = (text: string) => {
Expand Down
2 changes: 1 addition & 1 deletion test/source/tests/decrypt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1882,7 +1882,7 @@ XZ8r4OC6sguP/yozWlkG+7dDxsgKQVBENeG6Lw==
browser,
threadId,
{
content: [expectedContent],
content: [expectedContent, 'Warning: Checksum mismatch detected'],
encryption: 'encrypted',
},
authHdr
Expand Down
Loading