Repository navigation
Signer-domain fallback, c=simple headers (+ field-span soundness fix), readable quoted-printable with correct masking - #23
Conversation
…signature verifyDKIMSignature(email) only looks for a signature from the From: domain; ESP/subdomain-signed mail failed although a valid signature exists (17/132 local test emails; 10 then verify). CLI and site now try From:, then each DKIM-Signature d=. New test (synthetic email, stubbed DoH) fails on the #21 CLI and passes here. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…pan gap zkemail.nr (vendored) patches: - header field names match case-insensitively (c=simple keeps 'DKIM-Signature'), - a CR inside a field is only allowed as a fold (CRLF + SP/HTAB), - bh= also accepted after ';' and folded ';\r\n\t' / ';\r\n ', - field length is now asserted <= MAX_DKIM_HEADER_FIELD_LENGTH (300 -> 1024). Previously only the first 300 bytes were scanned for line breaks and length was unchecked, so a claimed DKIM field could run into a later header line whose bh= the prover picks (test: accepted by the #21 circuit, rejected now). Site + CLI locate the (last) DKIM-Signature field themselves; zkemail-nr's lookup is case-sensitive and stops at the first line break. Artifacts are rebuilt in a later commit (with QP decoding). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sk mapping; drop in-circuit QP Reverts the in-circuit QP decode variant (b9791be): ~55 gates/byte, QP large needed 2^23 and OOM'd compiling at 20.3 GB, and it added no soundness (decoding is a public function of the proven raw bytes). Instead: - site + CLI show QP bodies decoded (inspect, preview, verify page, CLI verify), - decoded-text masks map to the exact raw bytes (viewMaskToRawMask), fixing a web privacy bug: highlighted QP text ('Café', text across soft breaks) was searched for in the encoded bytes, not found, and published unmasked (regression test), - per-character masks now convert to UTF-8 byte masks (offsets drifted after the first non-ASCII character), - large tier body cap 49,152 -> 48,000 to stay within 2^22 after the c=simple hardening. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…redc range check, large=48,000) Gates (2048-bit): small 558,745 (2^20), mid 889,679 (2^20), large 4,066,238 (96.9% of 2^22). nargo 1.0.0-rc.3, bb 5.0.0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ndexedDB value limit) bb.js caches the uncompressed CRS as one IndexedDB value; 2^21 points = 134 MB > ~127 MB, so every browser proof threw. Browser tiers are <= 2^20. Verified on 3 real quoted-printable emails in Chromium: proofs valid in 15-19 s, the hidden phrase is absent from the output (the old search-based mapping would not have hidden it), and the body is shown decoded. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (32)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe changes add quoted-printable decoding for email body display and mask mapping, DKIM signer-domain fallback, and expanded DKIM header handling. They also update large-tier body limits, circuit references, browser proving settings, tests, and maintenance documentation. ChangesEmail proof and DKIM compatibility
Priority: ⬆️ High Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant VerifyPage
participant Lib as src/lib.ts
participant BodyView as bodyViewFor
participant MaskMap as viewMaskToRawMask
participant Circuit
VerifyPage->>Lib: submit email and decoded-text mask
Lib->>BodyView: build body view
Lib->>MaskMap: map decoded mask to raw body bytes
Lib->>Circuit: generate proof from raw signed bytes
Circuit-->>Lib: return proof outputs
Lib-->>VerifyPage: return masked body and decoding status
Merge Risk: ⚪ Minimal · up to The decoded-body masking paths align selections with signed bytes, and no actionable merge blocker was established. Merge after normal build and test checks. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to Signer fallback accepts valid signatures from domains other than the displayed sender, but the website does not explain that distinction when reporting authenticity. The redaction changes improve normal quoted-printable handling, while incomplete-mask and asynchronous publication behavior retain uncertainty. No signature-forgery bypass was established. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 63.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 30 functions across 11 files. (9 skipped: 9 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
What was asked
Aayush ran the Noir circuits over the 132 test emails in
private-emlsand asked for:d=domain;c=simpleheader canonicalization;The overall goal is that every proof can be generated on people's own devices. Noir only; no Circom.
Stacked on #21 (base
stack/02-long-emails-noir-rc3).What changes
1. Signer-domain fallback (site + CLI).
verifyDKIMSignature(email)only looks for a signature from theFrom:domain. ESP-signed mail (ccsend.com, schoolmessenger.com) and subdomain-signed mail (email.airbnb.com) failed although a valid signature exists: 17 of the 132 emails, 10 of which then verify. The CLI and site now tryFrom:first, then eachDKIM-Signatured=. The CLI'sverifyalready printsd=and warns when it isn't the From domain. Test: a synthetic ESP-signed email with stubbed DNS-over-HTTPS fails on #21's CLI and passes here.4.
c=simpleheaders + a field-span soundness fix (vendored zkemail.nr patches, documented inVENDORED.md):DKIM-Signature);bh=is also accepted after;and the folded;\r\n\t/;\r\n.bh=the prover picks. The limit is now 1,024 bytes and asserted. Test: this attack is accepted by Prove 48 KB emails + security upgrade to Noir 1.0.0-rc.3 / zkemail.nr v2 (redc binding) #21's circuit and rejected here. The same gap exists upstream in zkemail.nr.DKIM-Signaturefield themselves; zkemail-nr's lookup is case-sensitive and stops at the first line break.2. Quoted-printable, in the display layer. I first built an in-circuit decoding variant (
b9791be). It cost about 55 gates per byte, and the large variant needed 2²³ gates and OOM'd compiling at 20.3 GB, which works against on-device proving. It also adds no soundness: decoding is a fixed public function of the proven raw bytes, so a verifier can repeat it. So:Café, text across soft line breaks) inside the encoded bytes, didn't find it, and published it unmasked. Masks now map from decoded characters to the exact raw bytes (viewMaskToRawMask). A regression test shows the old mapping leaks, and a Chromium test on 3 real QP emails confirms the hidden phrase is absent from the proof and that the old mapping would not have hidden it.--hide/--revealalready matched QP-encoded copies;inspect, the preview andverifynow show decoded text.Circuits rebuilt (nargo 1.0.0-rc.3, bb 5.0.0). 2048-bit gates:
The large body cap is now 48,000 bytes (was 49,152), to stay within 2²² after the
c=simplehardening. The browser prover's CRS stays at 2²⁰: 2²¹ points is 134 MB, over Chromium's ~127 MB IndexedDB per-value limit, which I hit and fixed.ProveKit (evaluated, not adopted)
Measured against bb 5.0.0 on this machine (details in
docs/provekit-eval.mdon branchstack/06-provekit-eval):v1branch lacks the multi-scalar-multiplication black box our Pedersen nullifier needs.Revisit if its witness solver gets parallelized, or if we target phones, where its file-backed memory mode is the one real advantage.
Tests
yarn test: 19/19 pass. New in this PR: signer fallback,c=simpleproves, two field-span attacks, QP decode parity, QP mask mapping plus the old-mapping regression, and display decoding.nargo test: 49/49.bh=value folded mid-value, which is still unsupported (follow-up).Note: this PR's base isn't
main, so CI won't run until it's retargeted. All suites were run locally.🤖 Generated with Claude Code
Summary by CodeRabbit
From:address, simple header canonicalization, and more folded or longer signature fields.