[DO NOT MERGE] feat: email reception with Postbox UI for identity owners#3760
[DO NOT MERGE] feat: email reception with Postbox UI for identity owners#3760
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a proof-of-concept SMTP Gateway Protocol surface to the Internet Identity canister, including Candid types, request validation, and stable-memory storage of received emails.
Changes:
- Introduces new SMTP Candid types plus
smtp_request(update) andsmtp_request_validate(query) canister methods. - Implements SMTP request validation (domain/user whitelist, size/header bounds, SMTP-style error responses).
- Adds stable-memory “postbox” storage keyed by recipient address with per-user pruning.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/internet_identity/src/storage/storable/smtp.rs |
Adds stable-structures Storable* wrappers for SMTP postbox keys/values with bounded sizes. |
src/internet_identity/src/storage/storable.rs |
Exposes the new storable::smtp module. |
src/internet_identity/src/storage.rs |
Allocates a new stable memory region and adds store_email() with max-10 retention. |
src/internet_identity/src/smtp.rs |
Adds canister-side handlers for storing vs. validating SMTP requests. |
src/internet_identity/src/main.rs |
Exposes new canister endpoints under mod smtp_gateway. |
src/internet_identity/internet_identity.did |
Adds SMTP types and service methods to the public Candid interface. |
src/internet_identity_interface/src/internet_identity/types/smtp.rs |
Defines interface types plus validation and TryFrom to a validated internal representation. |
src/internet_identity_interface/src/internet_identity/types.rs |
Re-exports the new smtp module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Canon::Relaxed => canonicalize_body_relaxed(raw_body), | ||
| }; | ||
|
|
||
| let body_to_hash = match sig.body_length { |
There was a problem hiding this comment.
If we intend to handle the body length in DKIM per spec, we should make sure to only retain that "trusted" part of the body in future processing.
There was a problem hiding this comment.
Good catch. Today we only honor l= when computing the body hash — we still retain the full untrusted body in the postbox. Fixing this properly means either truncating the stored body to the signed prefix, or carrying the trust boundary through storage so the UI can render only the verified portion. I'm leaving this as a follow-up in the DKIM hardening track rather than addressing it in this PoC PR; noted in the commit message under 'known remaining DKIM gaps'.
|
|
||
| // Find "b=" that is not "bh=" — it follows a semicolon or starts the string | ||
| loop { | ||
| if let Some(pos) = remaining.find("b=") { |
There was a problem hiding this comment.
DKIM headers:
- Allow folding (line breaks + whitespace)
- Allow arbitrary whitespace around = and ;
- Are case-insensitive
- Can contain multiple b=-like substrings inside values
DKIM headers should not be parsed manually like this.
Instead:
Parse into structured tags first (b, bh, h, etc.)
Reconstruct the header canonically with b= empty
or:
Reuse a well-tested DKIM parsing library
There was a problem hiding this comment.
Agreed. Full DKIM-Signature tag parsing (folding, whitespace tolerance, multiple b=-like substrings inside values) isn't there yet. Our current parser is a simple split-on-; and split-on-= pass that handles the shapes real senders emit today but would misread a hand-crafted malicious header. Replacing it with either a dedicated DKIM parser crate or a small state machine is tracked for the DKIM hardening follow-up — not fixed in this PoC.
| used_indices[idx] = true; | ||
| let line = match sig.header_canon { | ||
| Canon::Relaxed => canonicalize_header_relaxed(&header.name, &header.value), | ||
| Canon::Simple => format!("{}: {}", header.name, header.value), |
There was a problem hiding this comment.
This violates RFC 6376, simple canonicalization requires exact original bytes, extra whitespace is added here after :.
| Canon::Simple => format!("{}: {}", header.name, header.value), | |
| Canon::Simple => format!("{}:{}", header.name, header.value), |
There was a problem hiding this comment.
To summarize the issue with current headers:
DKIM verification must operate on the exact original header bytes as received on the wire, but your implementation reconstructs headers from parsed
nameandvaluefields using formatting (and partial normalization). This loses critical details like original whitespace, folding (CRLFcontinuations), and exact spacing, which are part of the signed data even in “simple” canonicalization. As a result, your reconstructed headers can differ from what was actually signed, causing valid DKIM signatures to fail verification or behave inconsistently with real-world mail servers.
There was a problem hiding this comment.
Partially addressed. The concrete bug — the extra space after the colon under simple header canonicalization — is fixed: we now build "{name}:{value}" without any whitespace insertion. The broader point (full byte-exactness over the original wire bytes including folding) isn't addressed in this PR, because our SMTP gateway hands us pre-parsed (name, value) pairs rather than the raw header bytes — we'd need a gateway contract change to thread the originals through. Leaving the thread open to track that larger item.
| pub body_length: Option<usize>, | ||
| } | ||
|
|
||
| pub fn parse_dkim_signature(value: &str) -> Result<DkimSignature, String> { |
There was a problem hiding this comment.
We need some more checks:
The implementation ignores several important DKIM signature and DNS policy tags beyond the cryptographic fields, such as
i=(identity alignment),t=(signature timestamp),x=(expiration time), and DNS-side constraints likek=andv=. These fields are not just metadata—they define whether a signature is valid and acceptable under DKIM policy rules, not just whether it can be cryptographically verified. By ignoring them, your verifier may incorrectly accept replayed, expired, misaligned, or malformed signatures, meaning it performs only raw signature validation rather than full DKIM authentication semantics.
There was a problem hiding this comment.
Partially addressed in this round:
v=— already enforced inparse_dkim_signature(rejects anything other than1).x=(expiration) — now parsed and enforced: an expired signature fails theSignatureValidcheck withSignature expired (x=..., now=...)as the detail. Check is againstic_cdk::api::time() / 1e9.t=(timestamp) — parsed for completeness but not enforced (a future-dated signature is still accepted today).i=(identity alignment) and DNS-sidek=— not implemented yet.
Leaving the thread open to track i=, k=, and the t= future-dated check.
|
Another note, we also need to check this on top of DKIM:
|
c97ff7f to
c6fbb57
Compare
…cations
Adds an in-canister postbox for emails addressed to an identity's
anchor-number@<accepted-domain> mailbox. Emails arrive via the
`smtp_request` canister method (called by an external SMTP gateway),
are stored per-anchor (cap 10), and surfaced through a new postbox UI
in the manage area. Each message carries an async DKIM verification
result (RSA-SHA256 over the signed headers + body hash), fetched via
DoH to dns.google.
Browsers that support Web Push can opt in to notifications from the
postbox page; the canister dispatches VAPID-signed pushes to each
registered endpoint when a verified email arrives. The service worker
opens the postbox and scrolls to the email on click.
The postbox page polls the canister every 5 seconds so new emails
appear without re-authentication, pausing while the tab is hidden.
Review feedback addressed in this rebase:
- CodeQL: service worker `message` handler now rejects messages whose
origin is not ours; same-origin check added on the page side too.
- Safe `subject`/`body` truncation: `String::truncate` can panic on a
non-char-boundary byte; now clamps to the previous char boundary so
crafted multi-byte input can't trap. Body passes through
`from_utf8_lossy` (U+FFFD = 3 bytes) and is re-truncated to keep the
storage bound intact.
- Mailbox key canonicalization: envelope comparisons are
case-insensitive but the stored key wasn't — `format_address` now
lowercases both user and domain so the same logical mailbox can't
blow through per-user pruning by varying case.
- Removed duplicate smtp type import inside `mod smtp_gateway`; the
outer import in `main.rs` is the one needed by
`candid::export_service!()` at expansion time.
- DKIM:
- Simple header canonicalization no longer inserts whitespace after
the colon (RFC 6376 §3.4.2 — simple must be byte-exact).
- DNS TXT parsing: `p=` tag matched case-insensitively per RFC 6376.
- Reject RSA public keys smaller than 1024 bits (RFC 8301 guidance).
- Support multiple DKIM-Signature headers (common with mailing-list
forwarding); email is verified if any one signature verifies.
- Enforce signature expiration (`x=` tag) against canister time.
Known remaining DKIM gaps (tracked for follow-up):
- Header canonicalization still reconstructs from the
gateway-parsed `(name, value)` fields — byte-exact `simple` mode
requires raw header bytes, which the gateway doesn't currently
forward to us.
- `i=` (identity alignment) and DNS-side `k=`/`v=` not checked yet.
Push notification payload encryption (RFC 8291) is stubbed: the
postbox page posts the newest email's sender/subject to the service
worker via `postMessage`, and the SW uses that cache when a push
arrives. Falls back to a generic notification when the worker was
cold-started. Real payload encryption replaces this later.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
c6fbb57 to
e9d98e4
Compare
The 'Stop dev server' step did a bare `kill $pid` which exits non-zero if the process already terminated (e.g. when the previous `icp network stop` step caused a cascade shutdown). Under `-e` that fails the whole job even though all Playwright tests passed — most recently on desktop/3_6 and mobile/3_6. Append `|| true` so a missing process is tolerated. The `if: always()` on the previous step still runs `icp network stop`, and we still upload the dev server logs in the next step regardless of kill exit code. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
<anchor_number>@id.aiand view them in a new Postbox tab in the management dashboard.Changes
Backend
smtp_request(update) andsmtp_request_validate(query) canister endpoints implementing the SMTP Gateway Protocol.get_postbox(query) endpoint returning stored emails for an anchor number (newest first).smtp_postboxstable BTreeMap (memory ID 23).<valid_u64>@id.ai, body ≤ 5 KB, ≤ 10 headers.Frontend
identity_infoin the authenticated layout..didfile.🤖 Generated with Claude Code