From fc965635b618b8b4e2599940be086d3072642800 Mon Sep 17 00:00:00 2001 From: 0xSachinK <0xsachink@gmail.com> Date: Sat, 3 Feb 2024 14:52:57 +0530 Subject: [PATCH 1/8] Fix: Assert padding is all zeroes --- packages/circuits/email-verifier.circom | 10 +++++ .../circuits/tests/email-verifier.test.ts | 44 +++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/packages/circuits/email-verifier.circom b/packages/circuits/email-verifier.circom index bd4be9090..3cbcf7f1b 100644 --- a/packages/circuits/email-verifier.circom +++ b/packages/circuits/email-verifier.circom @@ -30,6 +30,11 @@ template EmailVerifier(max_header_bytes, max_body_bytes, n, k, ignore_body_hash_ // Base 64 body hash variables var LEN_SHA_B64 = 44; // ceil(32 / 3) * 4, due to base64 encoding. + // Assert padding is all zeroes + for (var i = in_len_padded_bytes; i < max_header_bytes; i++) { + in_padded[i] === 0; + } + // SHA HEADER: 506,670 constraints // This calculates the SHA256 hash of the header, which is the "base_msg" that is RSA signed. // The header signs the fields in the "h=Date:From:To:Subject:MIME-Version:Content-Type:Message-ID;" @@ -67,6 +72,11 @@ template EmailVerifier(max_header_bytes, max_body_bytes, n, k, ignore_body_hash_ if (ignore_body_hash_check != 1) { signal input body_hash_idx; + // Assert padding is all zeroes + for (var i = in_body_len_padded_bytes; i < max_body_bytes; i++) { + in_body_padded[i] === 0; + } + // BODY HASH REGEX: 617,597 constraints // This extracts the body hash from the header (i.e. the part after bh= within the DKIM-signature section) // which is used to verify the body text matches this signed hash + the signature verifies this hash is legit diff --git a/packages/circuits/tests/email-verifier.test.ts b/packages/circuits/tests/email-verifier.test.ts index 100cc92cd..ef4fe48ca 100644 --- a/packages/circuits/tests/email-verifier.test.ts +++ b/packages/circuits/tests/email-verifier.test.ts @@ -134,6 +134,28 @@ describe("EmailVerifier", () => { } }); + it("should fail if message padding is tampered", async function () { + const emailVerifierInputs = generateCircuitInputs({ + rsaSignature: dkimResult.signature, + rsaPublicKey: dkimResult.publicKey, + body: dkimResult.body, + bodyHash: dkimResult.bodyHash, + message: dkimResult.message, + shaPrecomputeSelector: "How are", + maxMessageLength: 640, + maxBodyLength: 768, + }); + emailVerifierInputs.in_padded[640 - 1] = "1"; + + expect.assertions(1); + try { + const witness = await circuit.calculateWitness(emailVerifierInputs); + await circuit.checkConstraints(witness); + } catch (error) { + expect((error as Error).message).toMatch("Assert Failed"); + } + }); + it("should fail if body is tampered", async function () { const invalidBody = Buffer.from(dkimResult.body); invalidBody[invalidBody.length - 1] = 1; @@ -158,6 +180,28 @@ describe("EmailVerifier", () => { } }); + it("should fail if body padding is tampered", async function () { + const emailVerifierInputs = generateCircuitInputs({ + rsaSignature: dkimResult.signature, + rsaPublicKey: dkimResult.publicKey, + body: dkimResult.body, + bodyHash: dkimResult.bodyHash, + message: dkimResult.message, + shaPrecomputeSelector: "How are", + maxMessageLength: 640, + maxBodyLength: 768, + }); + emailVerifierInputs.in_body_padded[768 - 1] = "1"; + + expect.assertions(1); + try { + const witness = await circuit.calculateWitness(emailVerifierInputs); + await circuit.checkConstraints(witness); + } catch (error) { + expect((error as Error).message).toMatch("Assert Failed"); + } + }); + it("should fail if body hash is tampered", async function () { const invalidBodyHash = dkimResult.bodyHash + "a"; From 2d0cb00e57aa47f622b1d9460d0b5ff0c4715478 Mon Sep 17 00:00:00 2001 From: 0xSachinK <0xsachink@gmail.com> Date: Sat, 3 Feb 2024 14:59:33 +0530 Subject: [PATCH 2/8] Fix compilation bug --- packages/circuits/email-verifier.circom | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/circuits/email-verifier.circom b/packages/circuits/email-verifier.circom index 3cbcf7f1b..e8372600e 100644 --- a/packages/circuits/email-verifier.circom +++ b/packages/circuits/email-verifier.circom @@ -72,11 +72,6 @@ template EmailVerifier(max_header_bytes, max_body_bytes, n, k, ignore_body_hash_ if (ignore_body_hash_check != 1) { signal input body_hash_idx; - // Assert padding is all zeroes - for (var i = in_body_len_padded_bytes; i < max_body_bytes; i++) { - in_body_padded[i] === 0; - } - // BODY HASH REGEX: 617,597 constraints // This extracts the body hash from the header (i.e. the part after bh= within the DKIM-signature section) // which is used to verify the body text matches this signed hash + the signature verifies this hash is legit @@ -99,6 +94,11 @@ template EmailVerifier(max_header_bytes, max_body_bytes, n, k, ignore_body_hash_ signal input in_body_padded[max_body_bytes]; signal input in_body_len_padded_bytes; + // Assert padding is all zeroes + for (var i = in_body_len_padded_bytes; i < max_body_bytes; i++) { + in_body_padded[i] === 0; + } + // This verifies that the hash of the body, when calculated from the precomputed part forwards, // actually matches the hash in the header signal sha_body_out[256] <== Sha256BytesPartial(max_body_bytes)(in_body_padded, in_body_len_padded_bytes, precomputed_sha); From 709ddad5e0295edd2182a587957401ab288ef91c Mon Sep 17 00:00:00 2001 From: 0xSachinK <0xsachink@gmail.com> Date: Sat, 3 Feb 2024 15:55:01 +0530 Subject: [PATCH 3/8] Add AssertZero util and use it to check padding in EmailVerifier --- packages/circuits/email-verifier.circom | 8 ++---- packages/circuits/helpers/extract.circom | 14 ---------- packages/circuits/helpers/utils.circom | 35 ++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 20 deletions(-) diff --git a/packages/circuits/email-verifier.circom b/packages/circuits/email-verifier.circom index e8372600e..1f094304c 100644 --- a/packages/circuits/email-verifier.circom +++ b/packages/circuits/email-verifier.circom @@ -31,9 +31,7 @@ template EmailVerifier(max_header_bytes, max_body_bytes, n, k, ignore_body_hash_ var LEN_SHA_B64 = 44; // ceil(32 / 3) * 4, due to base64 encoding. // Assert padding is all zeroes - for (var i = in_len_padded_bytes; i < max_header_bytes; i++) { - in_padded[i] === 0; - } + AssertZeroes(in_len_padded_bytes)(in_padded, in_len_padded_bytes + 1); // SHA HEADER: 506,670 constraints // This calculates the SHA256 hash of the header, which is the "base_msg" that is RSA signed. @@ -95,9 +93,7 @@ template EmailVerifier(max_header_bytes, max_body_bytes, n, k, ignore_body_hash_ signal input in_body_len_padded_bytes; // Assert padding is all zeroes - for (var i = in_body_len_padded_bytes; i < max_body_bytes; i++) { - in_body_padded[i] === 0; - } + AssertZeroes(in_body_len_padded_bytes)(in_body_padded, in_body_len_padded_bytes + 1); // This verifies that the hash of the body, when calculated from the precomputed part forwards, // actually matches the hash in the header diff --git a/packages/circuits/helpers/extract.circom b/packages/circuits/helpers/extract.circom index 6cc9199db..c02055c0d 100644 --- a/packages/circuits/helpers/extract.circom +++ b/packages/circuits/helpers/extract.circom @@ -4,20 +4,6 @@ include "./utils.circom"; // A set of utils for shifting and packing signal arrays // Performs extraction of reveal signals and packed signals -// From https://github.com/iden3/circomlib/blob/master/circuits/multiplexer.circom -function log2(a) { - if (a == 0) { - return 0; - } - var n = 1; - var r = 1; - while (n Date: Sat, 3 Feb 2024 15:57:22 +0530 Subject: [PATCH 4/8] Fix compilation bug --- packages/circuits/email-verifier.circom | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/circuits/email-verifier.circom b/packages/circuits/email-verifier.circom index 1f094304c..882e4e83f 100644 --- a/packages/circuits/email-verifier.circom +++ b/packages/circuits/email-verifier.circom @@ -31,7 +31,9 @@ template EmailVerifier(max_header_bytes, max_body_bytes, n, k, ignore_body_hash_ var LEN_SHA_B64 = 44; // ceil(32 / 3) * 4, due to base64 encoding. // Assert padding is all zeroes - AssertZeroes(in_len_padded_bytes)(in_padded, in_len_padded_bytes + 1); + component assert_header_padding = AssertZeroes(in_len_padded_bytes); + assert_header_padding.in <== in_padded; + assert_header_padding.start_index <== in_len_padded_bytes + 1; // SHA HEADER: 506,670 constraints // This calculates the SHA256 hash of the header, which is the "base_msg" that is RSA signed. @@ -93,7 +95,9 @@ template EmailVerifier(max_header_bytes, max_body_bytes, n, k, ignore_body_hash_ signal input in_body_len_padded_bytes; // Assert padding is all zeroes - AssertZeroes(in_body_len_padded_bytes)(in_body_padded, in_body_len_padded_bytes + 1); + component assert_body_padding = AssertZeroes(in_body_len_padded_bytes); + assert_body_padding.in <== in_body_padded; + assert_body_padding.start_index <== in_body_len_padded_bytes + 1; // This verifies that the hash of the body, when calculated from the precomputed part forwards, // actually matches the hash in the header From 5a1c9d0b21a1ac0e4bcab845296420fa0beb7dac Mon Sep 17 00:00:00 2001 From: 0xSachinK <0xsachink@gmail.com> Date: Sat, 3 Feb 2024 15:58:22 +0530 Subject: [PATCH 5/8] fix import bug --- packages/circuits/email-verifier.circom | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/circuits/email-verifier.circom b/packages/circuits/email-verifier.circom index 882e4e83f..9e76bf1f3 100644 --- a/packages/circuits/email-verifier.circom +++ b/packages/circuits/email-verifier.circom @@ -6,6 +6,7 @@ include "./helpers/sha.circom"; include "./helpers/rsa.circom"; include "./helpers/base64.circom"; include "./helpers/extract.circom"; +include "./helpers/utils.circom"; // include "./regexes/body_hash_regex.circom"; include "@zk-email/zk-regex-circom/circuits/common/body_hash_regex.circom"; From a2ad0be9ee7c8ef244b10f5bef7ddd79aa414824 Mon Sep 17 00:00:00 2001 From: 0xSachinK <0xsachink@gmail.com> Date: Sat, 3 Feb 2024 15:59:44 +0530 Subject: [PATCH 6/8] Fix compilation bug --- packages/circuits/email-verifier.circom | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/circuits/email-verifier.circom b/packages/circuits/email-verifier.circom index 9e76bf1f3..7a60ed976 100644 --- a/packages/circuits/email-verifier.circom +++ b/packages/circuits/email-verifier.circom @@ -32,7 +32,7 @@ template EmailVerifier(max_header_bytes, max_body_bytes, n, k, ignore_body_hash_ var LEN_SHA_B64 = 44; // ceil(32 / 3) * 4, due to base64 encoding. // Assert padding is all zeroes - component assert_header_padding = AssertZeroes(in_len_padded_bytes); + component assert_header_padding = AssertZeroes(max_header_bytes); assert_header_padding.in <== in_padded; assert_header_padding.start_index <== in_len_padded_bytes + 1; @@ -96,7 +96,7 @@ template EmailVerifier(max_header_bytes, max_body_bytes, n, k, ignore_body_hash_ signal input in_body_len_padded_bytes; // Assert padding is all zeroes - component assert_body_padding = AssertZeroes(in_body_len_padded_bytes); + component assert_body_padding = AssertZeroes(max_body_bytes); assert_body_padding.in <== in_body_padded; assert_body_padding.start_index <== in_body_len_padded_bytes + 1; From 11b07345a4adf1395b3b9592b67b4e53fc40e796 Mon Sep 17 00:00:00 2001 From: 0xSachinK <0xsachink@gmail.com> Date: Sat, 3 Feb 2024 16:00:44 +0530 Subject: [PATCH 7/8] Use udpated syntax --- packages/circuits/email-verifier.circom | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/packages/circuits/email-verifier.circom b/packages/circuits/email-verifier.circom index 7a60ed976..ba4319226 100644 --- a/packages/circuits/email-verifier.circom +++ b/packages/circuits/email-verifier.circom @@ -32,10 +32,8 @@ template EmailVerifier(max_header_bytes, max_body_bytes, n, k, ignore_body_hash_ var LEN_SHA_B64 = 44; // ceil(32 / 3) * 4, due to base64 encoding. // Assert padding is all zeroes - component assert_header_padding = AssertZeroes(max_header_bytes); - assert_header_padding.in <== in_padded; - assert_header_padding.start_index <== in_len_padded_bytes + 1; - + AssertZeroes(max_header_bytes)(in_padded, in_len_padded_bytes + 1); + // SHA HEADER: 506,670 constraints // This calculates the SHA256 hash of the header, which is the "base_msg" that is RSA signed. // The header signs the fields in the "h=Date:From:To:Subject:MIME-Version:Content-Type:Message-ID;" @@ -96,10 +94,8 @@ template EmailVerifier(max_header_bytes, max_body_bytes, n, k, ignore_body_hash_ signal input in_body_len_padded_bytes; // Assert padding is all zeroes - component assert_body_padding = AssertZeroes(max_body_bytes); - assert_body_padding.in <== in_body_padded; - assert_body_padding.start_index <== in_body_len_padded_bytes + 1; - + AssertZeroes(max_body_bytes)(in_body_padded, in_body_len_padded_bytes + 1); + // This verifies that the hash of the body, when calculated from the precomputed part forwards, // actually matches the hash in the header signal sha_body_out[256] <== Sha256BytesPartial(max_body_bytes)(in_body_padded, in_body_len_padded_bytes, precomputed_sha); From ca783205e63d19c64b4f6b136a52777fe86d3275 Mon Sep 17 00:00:00 2001 From: Elo <104064497+Metachaser24@users.noreply.github.com> Date: Mon, 5 Feb 2024 12:50:21 -0600 Subject: [PATCH 8/8] Create pull_request_template.md Added pull request template --- .github/pull_request_template.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 .github/pull_request_template.md diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 000000000..2aefe7916 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,21 @@ +## Description + +Please include a summary of the changes and the related issue. Please also include relevant motivation and context. +Mention the issue number that your changes are addressing. Use the format "Fixes #" to automatically link your pull request to the relevant issue. + +## Type of Change + +Please delete options that are not relevant. + +- [ ] Bug fix (non-breaking change which fixes an issue) +- [ ] New feature (non-breaking change which adds functionality) +- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) +- [ ] This change requires a documentation update + +## Checklist: + +- [ ] I have discussed with the team prior to submitting this PR +- [ ] I have performed a self-review of my code +- [ ] I have commented my code, particularly in hard-to-understand areas +- [ ] My changes generate no new warnings +- [ ] New and existing unit tests pass locally with my changes