-
Notifications
You must be signed in to change notification settings - Fork 83
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
Add DKIM tests #174
Add DKIM tests #174
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
export async function revertCommonARCModifications( | ||
email: string | ||
): Promise<string> { | ||
if (!email.includes("ARC-Authentication-Results")) { | ||
return email; | ||
} | ||
|
||
let modified = revertGoogleModifications(email); | ||
|
||
if (modified === email) { | ||
console.log("ARC Revert: No known ARC modifications found"); | ||
} | ||
|
||
return modified; | ||
} | ||
|
||
function revertGoogleModifications(email: string): string { | ||
// Google sets their own Message-ID and put the original one | ||
// in X-Google-Original-Message-ID when forwarding | ||
const googleReplacedMessageId = getHeaderValue( | ||
email, | ||
"X-Google-Original-Message-ID" | ||
); | ||
|
||
if (googleReplacedMessageId) { | ||
email = setHeaderValue(email, "Message-ID", googleReplacedMessageId); | ||
|
||
console.info( | ||
"ARC Revert: Setting X-Google-Original-Message-ID to Message-ID header..." | ||
); | ||
} | ||
|
||
return email; | ||
} | ||
|
||
function getHeaderValue(email: string, header: string) { | ||
const headerStartIndex = email.indexOf(`${header}: `) + header.length + 2; | ||
const headerEndIndex = email.indexOf("\n", headerStartIndex); | ||
const headerValue = email.substring(headerStartIndex, headerEndIndex); | ||
|
||
return headerValue; | ||
} | ||
|
||
function setHeaderValue(email: string, header: string, value: string) { | ||
return email.replace(getHeaderValue(email, header), value); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,12 @@ | ||
import { pki } from "node-forge"; | ||
import { DkimVerifier } from "./dkim-verifier"; | ||
import { getSigningHeaderLines, parseDkimHeaders, parseHeaders, writeToStream } from "./tools"; | ||
import { | ||
getSigningHeaderLines, | ||
parseDkimHeaders, | ||
parseHeaders, | ||
writeToStream, | ||
} from "./tools"; | ||
import { revertCommonARCModifications } from "./arc"; | ||
|
||
export interface DKIMVerificationResult { | ||
publicKey: bigint; | ||
|
@@ -22,20 +28,28 @@ export async function verifyDKIMSignature( | |
): Promise<DKIMVerificationResult> { | ||
let dkimResult = await tryVerifyDKIM(email, domain); | ||
|
||
if (dkimResult.status.result !== "pass" && tryRevertARCChanges) { | ||
console.info("DKIM verification failed. Trying to verify after reverting forwarder changes..."); | ||
|
||
const modified = await revertForwarderChanges(email.toString()); | ||
// If DKIM verification fails, revert common modifications made by ARC and try again. | ||
if (dkimResult.status.comment === "bad signature" && tryRevertARCChanges) { | ||
const modified = await revertCommonARCModifications(email.toString()); | ||
dkimResult = await tryVerifyDKIM(modified, domain); | ||
} | ||
|
||
if (dkimResult.status.result !== "pass") { | ||
const { | ||
status: { result, comment }, | ||
signingDomain, | ||
publicKey, | ||
signature, | ||
status, | ||
body, | ||
bodyHash, | ||
} = dkimResult; | ||
|
||
if (result !== "pass") { | ||
throw new Error( | ||
`DKIM signature verification failed for domain ${dkimResult.signingDomain}` | ||
`DKIM signature verification failed for domain ${signingDomain}. Reason: ${comment}` | ||
); | ||
} | ||
|
||
const { publicKey, signature, status, body, bodyHash } = dkimResult; | ||
const pubKeyData = pki.publicKeyFromPem(publicKey.toString()); | ||
|
||
return { | ||
|
@@ -72,7 +86,9 @@ async function tryVerifyDKIM(email: Buffer | string, domain: string = "") { | |
); | ||
|
||
if (!dkimResult) { | ||
throw new Error(`DKIM signature not found for domain ${domainToVerifyDKIM}`); | ||
throw new Error( | ||
`DKIM signature not found for domain ${domainToVerifyDKIM}` | ||
); | ||
} | ||
|
||
if (dkimVerifier.headers) { | ||
|
@@ -87,39 +103,15 @@ async function tryVerifyDKIM(email: Buffer | string, domain: string = "") { | |
return dkimResult; | ||
} | ||
|
||
function getHeaderValue(email: string, header: string) { | ||
const headerStartIndex = email.indexOf(`${header}: `) + header.length + 2; | ||
const headerEndIndex = email.indexOf('\n', headerStartIndex); | ||
const headerValue = email.substring(headerStartIndex, headerEndIndex); | ||
|
||
return headerValue; | ||
} | ||
|
||
function setHeaderValue(email: string, header: string, value: string) { | ||
return email.replace(getHeaderValue(email, header), value); | ||
} | ||
|
||
async function revertForwarderChanges(email: string) { | ||
// Google sets their own Message-ID and put the original one in X-Google-Original-Message-ID when forwarding | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see your new logic for this; do we have a test though? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately I dont have a non-sensitive email that contains ARC changes which can be pushed to git. I used the one I have with existing tests and they are passing (after successful modifications) |
||
const googleReplacedMessageId = getHeaderValue(email, "X-Google-Original-Message-ID"); | ||
if (googleReplacedMessageId) { | ||
console.info("Setting X-Google-Original-Message-ID to Message-ID header..."); | ||
email = setHeaderValue(email, "Message-ID", googleReplacedMessageId); | ||
} | ||
|
||
return email; | ||
} | ||
|
||
|
||
export type SignatureType = 'DKIM' | 'ARC' | 'AS'; | ||
export type SignatureType = "DKIM" | "ARC" | "AS"; | ||
|
||
export type ParsedHeaders = ReturnType<typeof parseHeaders>; | ||
|
||
export type Parsed = ParsedHeaders['parsed'][0]; | ||
export type Parsed = ParsedHeaders["parsed"][0]; | ||
|
||
export type ParseDkimHeaders = ReturnType<typeof parseDkimHeaders> | ||
export type ParseDkimHeaders = ReturnType<typeof parseDkimHeaders>; | ||
|
||
export type SigningHeaderLines = ReturnType<typeof getSigningHeaderLines> | ||
export type SigningHeaderLines = ReturnType<typeof getSigningHeaderLines>; | ||
|
||
export interface Options { | ||
signatureHeaderLine: string; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
import { verifyDKIMSignature } from "../src/dkim"; | ||
import fs from "fs"; | ||
import path from "path"; | ||
|
||
jest.setTimeout(10000); | ||
|
||
describe("DKIM signature verification", () => { | ||
it("should pass for valid email", async () => { | ||
const email = fs.readFileSync( | ||
path.join(__dirname, `test-data/email-good.eml`) | ||
); | ||
|
||
const result = await verifyDKIMSignature(email); | ||
|
||
expect(result.signingDomain).toBe("icloud.com"); | ||
}); | ||
|
||
it("should fail for invalid selector", async () => { | ||
const email = fs.readFileSync( | ||
path.join(__dirname, `test-data/email-invalid-selector.eml`) | ||
); | ||
|
||
expect.assertions(1); | ||
|
||
try { | ||
await verifyDKIMSignature(email); | ||
} catch (e) { | ||
expect(e.message).toBe( | ||
"DKIM signature verification failed for domain icloud.com. Reason: no key" | ||
); | ||
} | ||
}); | ||
|
||
it("should fail for tampered body", async () => { | ||
const email = fs.readFileSync( | ||
path.join(__dirname, `test-data/email-body-tampered.eml`) | ||
); | ||
|
||
expect.assertions(1); | ||
|
||
try { | ||
await verifyDKIMSignature(email); | ||
} catch (e) { | ||
expect(e.message).toBe( | ||
"DKIM signature verification failed for domain icloud.com. Reason: body hash did not verify" | ||
); | ||
} | ||
}); | ||
|
||
it("should fail for when DKIM signature is not present for domain", async () => { | ||
// In this email From address is [email protected], but the DKIM signature is only for icloud.com | ||
const email = fs.readFileSync( | ||
path.join(__dirname, `test-data/email-invalid-domain.eml`) | ||
); | ||
|
||
expect.assertions(1); | ||
|
||
try { | ||
await verifyDKIMSignature(email); | ||
} catch (e) { | ||
expect(e.message).toBe( | ||
"DKIM signature not found for domain gmail.com" | ||
); | ||
} | ||
}); | ||
|
||
it("should be able to override domain", async () => { | ||
// From address domain is icloud.com | ||
const email = fs.readFileSync( | ||
path.join(__dirname, `test-data/email-different-domain.eml`) | ||
); | ||
|
||
// Should pass with default domain | ||
await verifyDKIMSignature(email); | ||
|
||
// Should fail because the email wont have a DKIM signature with the overridden domain | ||
// Can be replaced with a better test email where signer is actually | ||
// different from From domain and the below check pass. | ||
expect.assertions(1); | ||
try { | ||
await verifyDKIMSignature(email, "domain.com"); | ||
} catch (e) { | ||
expect(e.message).toBe( | ||
"DKIM signature not found for domain domain.com" | ||
); | ||
} | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=icloud.com; s=1a1hai; t=1693038337; bh=7xQMDuoVVU4m0W0WRVSrVXMeGSIASsnucK9dJsrc+vU=; h=from:Content-Type:Mime-Version:Subject:Message-Id:Date:to; b=EhLyVPpKD7d2/+h1nrnu+iEEBDfh6UWiAf9Y5UK+aPNLt3fAyEKw6Ic46v32NOcZD | ||
M/zhXWucN0FXNiS0pz/QVIEy8Bcdy7eBZA0QA1fp8x5x5SugDELSRobQNbkOjBg7Mx | ||
VXy7h4pKZMm/hKyhvMZXK4AX9fSoXZt4VGlAFymFNavfdAeKgg/SHXLds4lOPJV1wR | ||
2E21g853iz5m/INq3uK6SQKzTnz/wDkdyiq90gC0tHQe8HpDRhPIqgL5KSEpuvUYmJ | ||
wjEOwwHqP6L3JfEeROOt6wyuB1ah7wgRvoABOJ81+qLYRn3bxF+y1BC+PwFd5yFWH5 | ||
Ry43lwp1/3+sA== | ||
from: [email protected] | ||
Content-Type: text/plain; charset=us-ascii | ||
Content-Transfer-Encoding: 7bit | ||
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.500.231\)) | ||
Subject: Hello | ||
Message-Id: <[email protected]> | ||
Date: Sat, 26 Aug 2023 12:25:22 +0400 | ||
to: [email protected] | ||
|
||
Hello, | ||
|
||
bla bla bla |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=icloud.com; s=1a1hai; t=1693038337; bh=7xQMDuoVVU4m0W0WRVSrVXMeGSIASsnucK9dJsrc+vU=; h=from:Content-Type:Mime-Version:Subject:Message-Id:Date:to; b=EhLyVPpKD7d2/+h1nrnu+iEEBDfh6UWiAf9Y5UK+aPNLt3fAyEKw6Ic46v32NOcZD | ||
M/zhXWucN0FXNiS0pz/QVIEy8Bcdy7eBZA0QA1fp8x5x5SugDELSRobQNbkOjBg7Mx | ||
VXy7h4pKZMm/hKyhvMZXK4AX9fSoXZt4VGlAFymFNavfdAeKgg/SHXLds4lOPJV1wR | ||
2E21g853iz5m/INq3uK6SQKzTnz/wDkdyiq90gC0tHQe8HpDRhPIqgL5KSEpuvUYmJ | ||
wjEOwwHqP6L3JfEeROOt6wyuB1ah7wgRvoABOJ81+qLYRn3bxF+y1BC+PwFd5yFWH5 | ||
Ry43lwp1/3+sA== | ||
from: [email protected] | ||
Content-Type: text/plain; charset=us-ascii | ||
Content-Transfer-Encoding: 7bit | ||
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.500.231\)) | ||
Subject: Hello | ||
Message-Id: <[email protected]> | ||
Date: Sat, 26 Aug 2023 12:25:22 +0400 | ||
to: [email protected] | ||
|
||
Hello, | ||
|
||
How are you? |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=icloud.com; s=1a1hai; t=1693038337; bh=7xQMDuoVVU4m0W0WRVSrVXMeGSIASsnucK9dJsrc+vU=; h=from:Content-Type:Mime-Version:Subject:Message-Id:Date:to; b=EhLyVPpKD7d2/+h1nrnu+iEEBDfh6UWiAf9Y5UK+aPNLt3fAyEKw6Ic46v32NOcZD | ||
M/zhXWucN0FXNiS0pz/QVIEy8Bcdy7eBZA0QA1fp8x5x5SugDELSRobQNbkOjBg7Mx | ||
VXy7h4pKZMm/hKyhvMZXK4AX9fSoXZt4VGlAFymFNavfdAeKgg/SHXLds4lOPJV1wR | ||
2E21g853iz5m/INq3uK6SQKzTnz/wDkdyiq90gC0tHQe8HpDRhPIqgL5KSEpuvUYmJ | ||
wjEOwwHqP6L3JfEeROOt6wyuB1ah7wgRvoABOJ81+qLYRn3bxF+y1BC+PwFd5yFWH5 | ||
Ry43lwp1/3+sA== | ||
from: [email protected] | ||
Content-Type: text/plain; charset=us-ascii | ||
Content-Transfer-Encoding: 7bit | ||
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.500.231\)) | ||
Subject: Hello | ||
Message-Id: <[email protected]> | ||
Date: Sat, 26 Aug 2023 12:25:22 +0400 | ||
to: [email protected] | ||
|
||
Hello, | ||
|
||
How are you? |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=icloud.com; s=1a1hai; t=1693038337; bh=7xQMDuoVVU4m0W0WRVSrVXMeGSIASsnucK9dJsrc+vU=; h=from:Content-Type:Mime-Version:Subject:Message-Id:Date:to; b=EhLyVPpKD7d2/+h1nrnu+iEEBDfh6UWiAf9Y5UK+aPNLt3fAyEKw6Ic46v32NOcZD | ||
M/zhXWucN0FXNiS0pz/QVIEy8Bcdy7eBZA0QA1fp8x5x5SugDELSRobQNbkOjBg7Mx | ||
VXy7h4pKZMm/hKyhvMZXK4AX9fSoXZt4VGlAFymFNavfdAeKgg/SHXLds4lOPJV1wR | ||
2E21g853iz5m/INq3uK6SQKzTnz/wDkdyiq90gC0tHQe8HpDRhPIqgL5KSEpuvUYmJ | ||
wjEOwwHqP6L3JfEeROOt6wyuB1ah7wgRvoABOJ81+qLYRn3bxF+y1BC+PwFd5yFWH5 | ||
Ry43lwp1/3+sA== | ||
from: [email protected] | ||
Content-Type: text/plain; charset=us-ascii | ||
Content-Transfer-Encoding: 7bit | ||
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.500.231\)) | ||
Subject: Hello | ||
Message-Id: <[email protected]> | ||
Date: Sat, 26 Aug 2023 12:25:22 +0400 | ||
to: [email protected] | ||
|
||
Hello, | ||
|
||
How are you? |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=icloud.com; s=2a1hai; t=1693038337; bh=7xQMDuoVVU4m0W0WRVSrVXMeGSIASsnucK9dJsrc+vU=; h=from:Content-Type:Mime-Version:Subject:Message-Id:Date:to; b=EhLyVPpKD7d2/+h1nrnu+iEEBDfh6UWiAf9Y5UK+aPNLt3fAyEKw6Ic46v32NOcZD | ||
M/zhXWucN0FXNiS0pz/QVIEy8Bcdy7eBZA0QA1fp8x5x5SugDELSRobQNbkOjBg7Mx | ||
VXy7h4pKZMm/hKyhvMZXK4AX9fSoXZt4VGlAFymFNavfdAeKgg/SHXLds4lOPJV1wR | ||
2E21g853iz5m/INq3uK6SQKzTnz/wDkdyiq90gC0tHQe8HpDRhPIqgL5KSEpuvUYmJ | ||
wjEOwwHqP6L3JfEeROOt6wyuB1ah7wgRvoABOJ81+qLYRn3bxF+y1BC+PwFd5yFWH5 | ||
Ry43lwp1/3+sA== | ||
from: [email protected] | ||
Content-Type: text/plain; charset=us-ascii | ||
Content-Transfer-Encoding: 7bit | ||
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.500.231\)) | ||
Subject: Hello | ||
Message-Id: <[email protected]> | ||
Date: Sat, 26 Aug 2023 12:25:22 +0400 | ||
to: [email protected] | ||
|
||
Hello, | ||
|
||
How are you? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
before we do this, we should add the zkp2p code where they try to process the email via doing the tabs -> spaces replacement as well as removing a [LABEL] tag from an email. i think we shuold try as hard as possible to get a dkim before moving to arc, and output that dkim failed and we are moving to arc before doing modifications and claiming dkim passes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know where this is implemented? I think those changes are also part of ARC? We should definitely be adding other ARC changes.
The
revertCommonARCModifications
will print a log when modifications are done, or if no changes are done.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check out the zkp2p fixes -- basically like if you revert those changes, then you can get DKIM to pass, not just ARC. I think they noticed this on some Venmo emails and diffed them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are doing the same approach - right now only Message-Id is reverted, but can add removing labels.
https://github.com/zkp2p/zk-p2p/blob/develop/client/src/components/ProofGen/validation/hdfc.tsx#L85 seems to be very specific to hdfc pasted emails.
I think we can merge this and address 162, 165 and other sanitizations in another PR.