feat(email-recovery): storage + smtp_request setup flow#3880
Open
sea-snake wants to merge 23 commits into
Open
feat(email-recovery): storage + smtp_request setup flow#3880sea-snake wants to merge 23 commits into
sea-snake wants to merge 23 commits into
Conversation
This was referenced May 12, 2026
Merged
c14b0c9 to
5d5f686
Compare
f04ea77 to
f6d88aa
Compare
5d5f686 to
3a8e9a4
Compare
f6d88aa to
128fdf8
Compare
3a8e9a4 to
33b1582
Compare
128fdf8 to
9f2b62d
Compare
sea-snake-translation-bot
pushed a commit
to sea-snake-translation-bot/internet-identity
that referenced
this pull request
May 12, 2026
…ck) (dfinity#3877) ## Summary PR 2 of the email-recovery stack (`docs/ongoing/email-recovery.md` §10 Phase 0). Stacks on top of PR 3838 (DNSSEC verifier). Lands a hand-rolled RFC 6376 DKIM verifier that consumes a parsed `SmtpRequest` plus an already-trusted DKIM TXT record and returns a per-step `EmailVerificationStatus`. **Note:** This PR targets `main` but includes PR 3838's commits (DNSSEC verifier) as its base. Review the DKIM-specific changes by looking at commits after `9bbd8717` (the last PR 3838 commit). Once PR 3838 merges, this PR's diff will shrink to just the DKIM additions. ## Why hand-rolled The design originally specified `mail-auth` (Stalwart's well-tested DKIM library), but mail-auth pulls a non-optional `hickory-resolver` dep that fails to compile for `wasm32-unknown-unknown` (transitive: tokio + mio). Forking + patching mail-auth would be possible but creates perpetual rebase burden. We hand-roll instead — "the right way, no shortcuts" was the explicit guidance. ## What's in this PR ### `src/internet_identity_interface/src/internet_identity/types/smtp.rs` Brings forward the SMTP gateway protocol types from PoC PR 3760: `SmtpRequest`/`SmtpResponse`/`SmtpHeader`/`SmtpMessage`/`SmtpAddress`/`SmtpEnvelope`, the size bounds, and the input-bound validation (`format_address` lowercases both halves; `truncate_at_char_boundary` clamps to the previous UTF-8 boundary so a multi-byte subject can't trap the canister). Drops postbox-specific bits (PostboxEmail, ValidatedSmtpRequest, anchor-number parser). ### `src/internet_identity/src/dkim/` - **`types.rs`** — Algorithm (RsaSha256, Ed25519Sha256), HeaderCanon/BodyCanon (Relaxed, Simple), DkimCheck/DkimCheckName/DkimCheckStatus per-step diagnostics, EmailVerificationStatus / VerificationFailReason result shape. - **`parse.rs`** (RFC 6376 §3.5) — DKIM-Signature header tag-list parser. Splits structurally on `;` first then on the *first* `=` per element, so a literal `b=` substring inside another tag's base64 doesn't get misread as a new tag start (the bug class the PoC PR review specifically flagged). Folded whitespace inside base64 values is stripped before decoding. Tag names case-insensitive; duplicates rejected. - **`canonicalize.rs`** (§3.4.2 / §3.4.4) — relaxed header canon (lowercase name, unfold continuations, collapse WSP+ to single SP, strip trailing WSP, strip WSP around colon) and relaxed body canon (per-line WSP cleanup, drop trailing empty lines, ensure non-empty output ends in exactly one CRLF). - **`dns_record.rs`** (§3.6.2) — DKIM TXT record parser. Tag names case-insensitive (`P=` vs `p=` was a PoC bug), whitespace inside `p=` tolerated (multi-chunk DNS TXT records), `t=y`/`t=s` flags honoured, unknown tags ignored. - **`signature.rs`** — RSA-SHA256 (RFC 5702 / RFC 8301) and Ed25519-SHA256 (RFC 8463) signature verification on top of `rsa`+`sha2`+`ed25519-dalek` from PR 1's deps. Enforces 1024-bit RSA minimum per design §5.6. Ed25519 path wraps in SHA-256 per RFC 8463. Plus `body_hash_sha256` with optional `l=` truncation per §3.4.5. - **`verify.rs`** — orchestration. Multi-signature loop per §5.5 (accept on first pass), tag enforcement per design §5.4 (c=relaxed/* only, x= expiration, i= alignment with d=, k= match, t=y testing-mode), bottom-up header selection per §5.4 when h= lists a name multiple times, b=value blanking that's structural-position-aware so it doesn't mis-target an internal substring. - **`test_vectors.rs`** — `#[cfg(test)]` .eml loader + 8 end-to-end tests against committed fixtures. ### `test_vectors/dkim/` - 3 synthetic .eml files generated offline with dkimpy + a 2048-bit RSA key (`relaxed/relaxed`, `relaxed/simple`, `simple/simple`). - The matching DKIM TXT record (public key only). - README documenting provenance — the throwaway private key is **not** committed. ## Test plan - [x] `cargo check -p internet_identity --target wasm32-unknown-unknown` — clean. - [x] `cargo test -p internet_identity --bin internet_identity dkim` — 75 tests pass (parse 14, canonicalize 18, dns_record 16, signature 7, verify 12, end-to-end 8). - [x] `cargo test -p internet_identity --bin internet_identity` — 313 tests pass total (was 238 before this PR; +75 DKIM, plus a few in smtp types). - [x] `cargo test -p internet_identity_interface --lib` — 52 tests pass (was 42; +10 SMTP type tests). - [x] `cargo clippy -p internet_identity --bin internet_identity --tests -- -D warnings` — clean. - [x] `cargo fmt --check` — clean (modulo pre-existing diffs unrelated to this PR). ## Stack This is PR 2 of a 12-PR series. Includes PR 3838's commits as its base; once PR 3838 merges, the diff shrinks to just the DKIM additions. Subsequent PRs: - **PR 3** — DMARC alignment. - **PR 4** — DoH outcall fallback for unsigned domains (Gmail / Outlook / iCloud — see the design doc §7.6 and the team Slack writeup). - **PRs 5–9** — storage + Candid + behavior for email recovery. - **PRs 10–12** — frontend. ## PR Stack | # | PR | Description | Status | |---|---|---|---| | 0 | [dfinity#3836](dfinity#3836) | Design doc | Open | | 1 | [dfinity#3838](dfinity#3838) | DNSSEC verifier scaffold | Open | | 2 | [dfinity#3877](dfinity#3877) | DKIM verifier (RFC 6376) | Open | | 3 | [dfinity#3878](dfinity#3878) | DMARC alignment (RFC 7489) | Open | | 4 | [dfinity#3879](dfinity#3879) | DoH fallback | Open | | 5+6 | [dfinity#3880](dfinity#3880) | Setup flow (storage + smtp_request) | Open | | 7 | [dfinity#3881](dfinity#3881) | Recovery flow (delegation) | Open | | 8 | [dfinity#3882](dfinity#3882) | Frontend + feature flag | Open | | 9 | [dfinity#3883](dfinity#3883) | Deploy/upgrade scripts: dnssec_config + doh_config | Open | | 10 | [dfinity#3884](dfinity#3884) | Email-recovery UX overhaul | Open | --------- Co-authored-by: Arshavir Ter-Gabrielyan <arshavir.ter.gabrielyan@dfinity.org> Co-authored-by: Claude <noreply@anthropic.com>
Lays the storage foundation for the email-recovery flow (PRs 5+6 of the email-recovery stack — see docs/ongoing/email-recovery.md): - New module `internet_identity_interface::email_recovery` with the Candid types: EmailRecoveryCredential, EmailRecoveryChallenge, EmailRecoveryDnsInput (DohAllowlist + Dnssec stub), EmailRecoveryError, EmailRecoveryStatus, EmailRecoveryGetDelegationArgs. - New `StorableEmailRecoveryCredential` mirrors the on-disk shape via `minicbor-derive`. Lives inline on `StorableAnchor` as `#[n(5)] email_recovery: Option<...>`. minicbor's `#[cbor(map)]` is forward-compatible across optional-field additions, so old anchors decoded under the previous schema deserialize cleanly with `None`. - `Anchor` (in-memory) gains `email_recovery: Option<EmailRecoveryCredential>`, threaded through the three Storable<->Anchor conversion sites and the storage write-merge in `Storage::write_anchor` (the new field needs no merge — it's an overwrite-on-write single-slot credential). This commit only adds the data structure. Behaviour (canister methods, nonce PRNG, reverse address index, pending-challenge map, smtp_request) lands in subsequent commits on this branch. 414 unit tests pass; wasm32 build clean.
Continues the email-recovery setup flow (PRs 5+6 of the email-recovery
stack). Adds three building blocks under a new `email_recovery` module:
`email_recovery/rng.rs`
- A heap-resident ChaCha20Rng PRNG, seeded once per canister lifetime
via `random_salt()` (which calls `raw_rand` under the hood). The
first prepare call after canister start triggers the async fetch;
every subsequent draw is a single PRNG step.
- `format_nonce` produces `II-Recovery-{16-hex-chars}` — the verbose
prefix is deliberate (every legitimate UI surface starts with it,
so a phisher's alternative format is an obvious tell — see design
§3.2).
`email_recovery/pending.rs`
- Heap-only `HashMap<String, PendingChallenge>` with 30-min TTL and
oldest-first eviction at the bounded capacity (10 000).
- Heap rather than stable: pending entries expire in 30 minutes and
losing them on canister upgrade just costs an in-flight wizard one
retry, not worth the stable-memory churn.
- Random-nonce-only key gives the §8.8 untargetability property —
attackers can fill the map but can't pick whose entry gets evicted.
- Lazy TTL eviction on `status_of` reads + sweep on `insert`, so
polling sees `Expired` at the right moment without paying O(n) on
every read.
`email_recovery/prepare.rs`
- `prepare_add(anchor, dns_input, now_secs)` — async, validates
input, draws a fresh nonce, parks a `Pending` challenge.
- Variant dispatch: only `DohAllowlist` is wired up;
`EmailRecoveryDnsInput::Dnssec` returns
`DnssecPathNotYetSupported`. The DNSSEC path lands in a follow-up.
- DoH allowlist gate happens up front so a non-supported domain
fails at prepare with a clear error, not after the user has
composed an email.
- Address normalisation is intentionally minimal: lowercase + reject
obvious-garbage shapes. Strict RFC 5322 validation lives in the
verifier (PRs 2-3).
- Nonce-collision retry loop guards against the vanishingly small
birthday-collision case (8 attempts, then bail).
17 new unit tests (rng + pending + prepare); full unit suite green.
The `prepare_add` canister method is wired up in a subsequent commit
on this branch.
…methods Adds the canister-method surface for the setup-half of email recovery: - `email_recovery_credential_prepare_add(identity_number, dns_input)` — authenticated update. Validates that the caller owns the anchor, delegates to `email_recovery::prepare_add` for input validation + nonce issuance, returns the user-visible `EmailRecoveryChallenge`. - `email_recovery_status(nonce)` — anonymous query the FE polls while the user composes the email. Returns `Pending` → `RegistrationSucceeded` / `Failed` / `Expired`. - `email_recovery_credential_remove(identity_number, address)` — authenticated update. Detaches the recovery email from the anchor via the new `email_recovery::remove_credential` helper. Inlined authz (rather than using `anchor_operation_with_authz_check`) to avoid an awkward orphan-rule From<IdentityUpdateError> impl on an interface-crate error type. `.did` file: adds `EmailRecoveryCredential`, `EmailRecoveryChallenge`, `EmailRecoveryDnsInput`, `EmailRecoveryError`, `EmailRecoveryStatus`, `DohAllowlistDnsInput` types and the three new methods. `email_recovery::remove_credential` lives in a new `remove.rs` submodule with five unit tests covering the happy path, case- insensitive match, unbound anchor, wrong address (no partial mutation), and double-remove idempotency. `pending_status` is exposed as a thin `mod.rs` wrapper so `main.rs` doesn't need to know which submodule the heap state lives in. The `smtp_request` path (where the gateway forwards an inbound email and the canister verifies + binds) is the next commit on this branch. 436 unit tests pass (up from 414); wasm32 build clean; .did matches.
Two related changes:
1. **smtp_request handler.** New `email_recovery::smtp` submodule
that accepts an inbound email from the gateway and runs the full
verification pipeline:
- Bound-checks the SmtpRequest payload.
- Dispatches by recipient: `register@id.ai` for setup; other
recipients are silently dropped (no per-recipient signal back
to the gateway, so it can't probe the canister for which
mailboxes exist).
- Extracts the canister-issued nonce from the `Subject:` header
(case-insensitive prefix match, hex-suffix length-checked).
- Looks up the pending challenge, snapshotting it under a brief
borrow so the async DoH fetches don't hold a `RefCell` across
awaits.
- Resolves the DKIM key via `crate::doh::fetch_txt(<selector>.
_domainkey.<domain>, domain)` — heap-cached after the first
lookup per provider per TTL window.
- Optionally fetches `_dmarc.<domain>`; absence forces strict
alignment in the verifier.
- Runs `crate::dmarc::verify_email` end-to-end (full DKIM +
DMARC check + the `Subject in h=` enforcement added during
the PR 5 review).
- On success, writes the credential to the anchor and flips
the pending challenge to `Succeeded`. Failures flip it to
`Failed(reason)` for the FE poll. The gateway always sees
`SmtpResponse::Ok`.
Wired up as `smtp_request` in main.rs's email_recovery_api
submodule and added to the .did.
2. **Path-agnostic prepare API.** Per review feedback ("the FE
shouldn't need to know about the DoH allowlist"), drop the
`EmailRecoveryDnsInput::DohAllowlist | Dnssec` variant and
replace it with a flat `record { address; selector }`. The FE
just submits these; the canister picks the path:
- Future DNSSEC follow-up adds an optional `dns_proof` field
(Candid-forward-compatible). When supplied, the canister
validates the chain. When absent, falls through to the DoH
allowlist check.
- Currently only the DoH branch is wired up; non-allowlisted
domains return `DomainNotSupported` (renamed from the now-
removed `DomainNotAllowlisted`).
Drops the `DnssecPathNotYetSupported` error variant — the FE
never sees a DNSSEC-specific rejection, so there's no need to
carry the variant.
`.did` updates: `EmailRecoveryDnsInput` is now a record, the SMTP
gateway types (SmtpRequest/SmtpResponse and friends) are declared,
and the `smtp_request` method is exposed.
441 unit tests pass (up from 436, +5 smtp parser/extract tests);
wasm32 build clean; clippy `-D warnings` clean.
Adds an integration-test module exercising the canister-method surface end-to-end via PocketIC. Covers: - Nonce shape from `email_recovery_credential_prepare_add` (prefix + length + mailbox + expires_at). - Allowlist gate (non-allowlisted domain → `DomainNotSupported`). - Authorization (a non-owner principal cannot prepare for an anchor). - Status query semantics: `Expired` for unknown nonces, `Pending` after prepare_add, `Expired` after TTL via `env.advance_time`. - `smtp_request` silent-drop paths: missing nonce in Subject, unknown nonce, non-`register@` recipient. - `email_recovery_credential_remove` rejection when nothing bound. - **Full end-to-end binding flow**: prepare_add → in-test DKIM signer → `smtp_request` → mocked DoH outcalls → status flips to `RegistrationSucceeded` → `credential_remove` succeeds (proves the binding actually persisted to the anchor). The DKIM signer is a small in-test module (~200 LOC) that mirrors the verifier's relaxed canonicalization rules, generates a fresh 2048-bit RSA keypair per test, and signs against `<selector>._domainkey.<domain>`. The matching `v=DKIM1; k=rsa; p=…` TXT record is wrapped in a wire-format DNS response and served back via `env.mock_canister_http_response` for each of the 5 DoH provider outcalls. DMARC outcalls are answered with 404 (no record), which forces strict alignment — and our test setup satisfies that (DKIM `d=` equals From: domain). Adds matching helpers to `canister_tests::api::internet_identity`: - `email_recovery_credential_prepare_add` - `email_recovery_status` - `email_recovery_credential_remove` - `smtp_request` The integration test crate's existing build setup (II_WASM env var, PocketIC binary on PATH) is reused unchanged.
CI failure (`expected RegistrationSucceeded, got Failed(...DnsRecordMalformed("RSA SPKI decode...")...)`):
- The in-test DKIM signer was encoding the public key as PKCS#1
RsaPublicKey, but the canister verifier expects an X.509
SubjectPublicKeyInfo (SPKI) per RFC 5280 §4.1 — which is also what
real DKIM records publish in the `p=` tag. Switch to
`to_public_key_der()`.
Copilot review comments addressed:
- `EmailRecoveryChallenge.expires_at` now in **nanoseconds** since
epoch (matches `Timestamp` semantics — was off by a factor of 1e9).
- Non-allowlisted domain at prepare time now returns
`DomainNotAllowlisted(domain)` instead of `DomainNotSupported`.
`DomainNotSupported` is reserved for genuinely unsupported domain
shapes (and for the future DNSSEC-attempted-but-failed case).
- `smtp_request` recipient dispatch now matches on the **full**
recipient address (user + domain), case-insensitive on both
halves. New `SETUP_RECIPIENT_DOMAIN = "id.ai"` constant. A direct
caller can no longer set `to.user="register"` with an arbitrary
domain to bypass dispatch.
- `smtp_request` now maps `DohError` variants to typed
`EmailRecoveryError`s: `NotConfigured` / `DomainNotAllowed` →
`DomainNotAllowlisted` (so the FE shows "operator hasn't enabled
this domain"); `AllProvidersFailed` / `QuorumFailed` /
`ResponseMalformed` → `DohFetchFailed` (transient-error UX);
`InvalidName` / `NameOutsideRegisteredDomain` →
`InternalCanisterError` (caller-bug variants `prepare_add` should
have caught).
- Doc fixes:
- Interface module-level: `5-of-3 quorum` → `3-of-5 quorum`.
- `EmailRecoveryCredential` and `StorableEmailRecoveryCredential`:
`created_at` / `last_used` units corrected from "Unix-seconds"
to "nanoseconds since epoch".
- `EmailRecoveryChallenge.nonce` doc: "~16 base32 characters" →
"16 lowercase hex characters (8 random bytes)".
- `EmailRecoveryChallenge.expires_at` doc: "Unix seconds" →
"nanoseconds since epoch".
- `email_recovery::remove`: doc no longer claims to be wrapped in
`anchor_operation_with_authz_check` — that helper isn't used at
the canister-method layer (orphan-rule constraint on the
interface-crate error type, see `main.rs::email_recovery_api`).
Tests updated for the new units; 441 unit tests still pass.
Adds the DNSSEC verification path to the setup flow. The FE can now
optionally pass a `DnsProofBundle` alongside the address+selector;
when supplied, the canister:
1. Validates the chain synchronously against its configured
`DnssecConfig.root_anchors` at prepare time.
2. Confirms the bundle's leaf is a TXT record at the expected
`<selector>._domainkey.<registered_domain>` name (otherwise an
attacker who got a chain validated for *some* TXT record could
bind that record's content as a DKIM key).
3. Caches the verified TXT bytes on the pending challenge.
`smtp_request` then uses the cached TXT directly — no DoH outcall
fan-out at email-arrival time. When `dns_proof` is absent, falls
through to the existing DoH allowlist path unchanged.
Wire-level changes:
- `internet_identity_interface::types::dnssec`: adds Candid-friendly
mirror types for `DnsProofBundle`, `SignedRRset`, `Rrsig`,
`DelegationLink`. Wire-format owner names are `blob`s; the
canister-side verifier types stay as-is and are bridged via
`From<>` impls in `dnssec::types::interface_conversions`.
- `EmailRecoveryDnsInput` gains an optional `dns_proof:
Option<DnsProofBundle>` field. Forward-compatible Candid extension.
- `.did` declares the new types.
- `dnssec::types`: adds `TYPE_TXT` / `TYPE_DS` / `TYPE_DNSKEY`
constants and re-exports them from `dnssec::mod`.
- `dnssec::mod`: makes the `types` submodule `pub(crate)` so the
email-recovery module can reach the internal `DnsProofBundle`
via `From` conversions.
- `PendingChallenge` gains `cached_dkim_txt: Option<Vec<u8>>` so
`smtp_request` knows whether to skip the DoH path.
Two new PocketIC integration tests cover the plumbing end-to-end:
- `dnssec_path_rejects_when_no_trust_anchors_configured` — supplies
a structurally-valid bundle without anchor config; expects
`DomainNotSupported("…trust anchors not configured…")`.
- `dnssec_path_takes_precedence_over_doh_allowlist` — supplies a
malformed bundle for a domain that *is* on the DoH allowlist;
expects DNSSEC failure rather than fall-through to DoH.
A happy-path DNSSEC test (with a real `<selector>._domainkey.<domain>`
chain) is a follow-up — generating a DNSSEC chain at test time would
be a multi-hundred-line in-test signer; the existing
`crate::dnssec::test_vectors` cloudflare.com fixture's leaf is a
plain `cloudflare.com` TXT, not a `_domainkey` TXT.
Also addresses the earlier Copilot review feedback on PR #3842 (in a
separate commit on this branch): expires_at units, error variant
mapping, recipient-dispatch full-address match, DoH error mapping.
…ted split The Copilot-feedback fix introduced a dedicated `DomainNotAllowlisted` variant for the DoH allowlist-miss case (previously collapsed into `DomainNotSupported`), but missed updating this integration test. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…path test The DNSSEC path used to short-circuit only the DKIM DoH outcall — DMARC still fanned out at email-arrival time, defeating the goal of "DNSSEC on, DoH off." Generalise `DnsProofBundle` to carry one *or more* leaves under the same chain walk; `prepare_add` extracts DKIM (required) and DMARC (optional) by name, caches both on the pending challenge, and `smtp_request` skips the DoH fan-out entirely on this path. Also fix `status_flips_to_expired_after_ttl`: PocketIC's query path doesn't see `advance_time` without a `tick()` to roll the round over. Adds `full_setup_flow_via_dnssec_path` — the missing happy-path integration test, with an in-test Ed25519 DNSSEC signer that builds a fresh root → leaf-zone chain and asserts zero DoH outcalls observed. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ng dot The root terminator (`\x00`) was treated as an extra empty label and got its own '.' appended, on top of the trailing dot already added after the final real label. So `\x05test1\x0a_domainkey…\x00` decoded to `test1._domainkey.…com..` — never equal to the FQDN we compare it to. Existing tests masked this: the only callers reached this branch on the trust-anchors-empty short-circuit, never the name compare. Surfaced now by full_setup_flow_via_dnssec_path. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…are equal e2e flows
The module-level docstring presented DoH-with-mocked-outcalls as the
canonical e2e setup, and the DoH happy-path test was titled "Full
end-to-end test" with a comment that read "this one does the real
work" — implicitly demoting the DNSSEC happy-path test to second-tier.
Both are full e2e flows; reframe the docs to make that explicit and
add parallel section headers ("DoH path: full end-to-end setup flow"
vs "DNSSEC path: full end-to-end setup flow + protocol-shape checks").
Comment-only change.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`normalize_address` accepted any non-empty, no-whitespace, has-`@` string. With the 2 MiB ingress limit a caller could submit a multi-KB address and inflate the heap-resident pending-challenge map. Apply the standard caps at every address-handling boundary: - `MAX_ADDRESS = 254` (path limit minus the `<>` framing — RFC 5321 §4.5.3.1.3 with the corrected interpretation, errata to RFC 3696). - `MAX_LOCAL_PART = 64` (§4.5.3.1.1). - `MAX_DOMAIN = 255` (§4.5.3.1.2). Enforced at prepare time (so an invalid address never gets a nonce) and at `smtp_request` From-extraction time (defense in depth — the gateway shouldn't pass through oversized addresses but we don't rely on it). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Several module-level and inline comments still claimed DoH was the only path, that DNSSEC was deferred, that callers hadn't landed, or referenced specific PR numbers in the now-collapsed stack. Now that PR #3842 landed both paths, the on-anchor credential, and the DKIM / DMARC / DoH / DNSSEC consumers, the comments needed to catch up. Concrete updates: - main.rs: prepare_add doc no longer says "DoH path only in this PR"; describes the actual path picker. - email_recovery.rs interface module + EmailRecoveryDnsInput doc: drops "DNSSEC deferred to a follow-up PR" / "DoH only in initial cut" framing; now describes both paths as live with the canister picking per call. - dkim/mod.rs, dmarc/mod.rs, dnssec/mod.rs: dead-code-allow rationale rewritten to point at the actual in-canister consumer (`crate::email_recovery::smtp::verify_setup_email`) instead of promising future PRs. - dkim/verify.rs, dmarc/verify.rs: data-flow doc no longer lists inputs as coming from "PR 1 / PR 4"; describes them as the cached DNSSEC-verified bytes or a DoH fetch. - dmarc/test_vectors.rs: drops "PR 2" references in favor of the module path the fixtures live at. - email_recovery/smtp.rs: pipeline summary mentions both DKIM-source paths (cached DNSSEC vs DoH); drops a stale "added during the PR 5 review" historical note. - email_recovery/remove.rs: TODO rephrased to be self-contained. Comment-only. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The DNSSEC path stores the DKIM TXT (and optionally DMARC TXT) bytes on each pending entry so `smtp_request` can skip the DoH outcall. Those bytes come straight from the verified RDATA — `verify_dnssec_chain` authenticates them but doesn't bound their size. An attacker who controls a signed zone could publish a multi-KB TXT record, push it through prepare_add, and inflate every pending entry; at the 10 000-slot map cap that compounds. Add pragmatic caps on the cached bytes: - `MAX_DKIM_TXT_BYTES = 4096` — ~10× a real RSA-4096 DKIM record. - `MAX_DMARC_TXT_BYTES = 1024` — ~2× the largest realistic DMARC. Enforced in `verify_dnssec_chain` after the chain validates and before the bytes land on the pending challenge. Returns `EmailVerificationFailed` with a clear message naming the leaf and the cap. The DoH path is already bounded — `MAX_DOH_RESPONSE_BYTES = 4096` caps the per-outcall response, and the DoH-fetched bytes aren't stored on a pending entry anyway (they're consumed during `smtp_request` only). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The current canister API still caps a recovery email at one entry per anchor; this only changes the data model shape so multi-credential support can land later without another schema bump. - `Anchor.email_recovery: Option<EmailRecoveryCredential>` → `Vec<EmailRecoveryCredential>`. Empty vec replaces \`None\`. - `StorableAnchor.email_recovery: Option<StorableEmailRecoveryCredential>` → `Option<Vec<StorableEmailRecoveryCredential>>` — same wrapping as `passkey_credentials` / `recovery_keys` so anchors written under the prior schema decode cleanly (`Vec<T>` alone would `MissingValue` on decode; verified empirically against this minicbor-derive version). - Bind / remove logic and storage round-trip tests updated for the new shape.
Two-phase email-recovery only ever needs one leaf per call (the
optional DMARC TXT at prepare time, the DKIM TXT at submit_dkim_leaf
time), so a `Vec<SignedRRset>` was always-overkill — and a smaller
candid argument is a meaningful ingress saving for FE callers. Narrow
`DnsProofBundle.leaves: Vec<SignedRRset>` to
`DnsProofBundle.leaf: Option<SignedRRset>` across:
- `internet_identity_interface::types::DnsProofBundle` (candid + Rust)
- `crate::dnssec::types::DnsProofBundle` (verifier-internal)
- `interface_conversions` From<> impl
- `verify_with_clock` now returns `Result<VerifiedRecord, _>`
(single record); `bundle.leaf = None` triggers Malformed
- The `verify` wall-clock wrapper follows the same return shape
- The `WireBundle` JSON loader and the `cloudflare-com-2026-05.json`
test vector switch from `"leaves": [{...}]` to `"leaf": {...}`
- The `capture-dnssec-chain.py` capture script emits the new shape
- The DNSSEC-path happy-path integration test composes a
single-leaf bundle inline (the helper now exposes `skeleton`,
`dkim_leaf`, and optional `dmarc_leaf` separately so two-phase
callers can construct the bundle they need)
- `internet_identity.did`: `leaves : vec SignedRRset` →
`leaf : opt SignedRRset`
`prepare.rs::verify_dnssec_chain` is the only consumer that
previously expected DKIM + DMARC in one bundle. It now handles the
single-leaf shape — picking the leaf by owner-name match — but
otherwise keeps the legacy "DKIM at prepare time" pipeline since
the two-phase rewrite ships in the next commit. The integration
test still exercises that legacy pipeline.
All 13 dnssec unit tests still pass. The legacy DNSSEC happy-path
integration test was updated to construct its bundle from the new
`ChainOut` components.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Drops the FE selector-probing problem entirely: at prepare time the
FE submits a DNSSEC *skeleton* (chain only, optionally with the
DMARC leaf), and the canister caches the validated zone DNSKEY. When
the email arrives the canister parses `s=` from the DKIM-Signature
header — that's the authoritative selector — flips polled status to
`NeedDkimLeaf { selector }`, and the FE walks DNSSEC for that one
leaf and submits it via the new `email_recovery_submit_dkim_leaf`
update. See `docs/ongoing/email-recovery.md` §8.4 / §8.5.
What landed:
- **Interface types** (`internet_identity_interface`):
- `EmailRecoveryDnsInput.selector` removed.
- `EmailRecoveryStatus::NeedDkimLeaf { selector }` added (mid-flow).
- `EmailRecoveryError::SelectorMismatch` removed; replaced by
`DkimLeafMismatch` and `NoDkimLeafExpected`.
- `EmailRecoverySubmitDkimLeafArg { nonce, dkim_leaf }` added.
- **Pending state** (`email_recovery::pending`):
- `PendingChallenge.selector` and `cached_dkim_txt` removed.
- `registered_domain`, `cached_zone_dnskey: Option<SignedRRset>`,
and `partial_verification: Option<PartialVerification>` added.
- `PartialVerification { headers_digest 32 B + signature blob +
selector + signing_domain + algorithm + from_address_lc +
subject_signed }` (~500 B per pending entry).
- `PendingStatus::NeedDkimLeaf { selector }` added.
- **prepare.rs**:
- Drops selector input.
- Validates skeleton chain via `verify_chain_with_clock`, caching
the deepest-zone DNSKEY.
- Optionally validates a DMARC leaf at `_dmarc.<domain>`; rejects
any other leaf (including DKIM, which now belongs in the
submit-leaf call).
- **smtp.rs** (largely rewritten):
- On the DNSSEC path: parses `DKIM-Signature`, verifies `bh=`,
builds the canonical signed-headers input, computes its
SHA-256, and stashes `PartialVerification`. Body is dropped
once `bh=` validates. Pins From↔claimed_address and
`Subject` in `h=` early. Flips status to
`NeedDkimLeaf { selector }`.
- On the DoH path: behaves as before — fetch DKIM TXT, run
`dmarc::verify_email`, bind, finish synchronously.
- Idempotent: redelivered emails are silent no-ops once the
challenge has advanced past `Pending`.
- `bind_credential_to_anchor` → `bind_credential` taking
primitives so submit-leaf can reuse it.
- **submit_leaf.rs** (new module):
- Validates the FE-supplied `SignedRRset` against the cached
zone DNSKEY (no chain re-walk).
- Confirms the leaf's owner name is exactly
`<expected_selector>._domainkey.<registered_domain>.`.
- Parses the DKIM TXT, runs the cryptographic signature check
using a new prehash-aware helper:
- RSA-SHA256: `RsaVerifyingKey::verify_prehash` (PKCS#1 v1.5).
- Ed25519-SHA256: RFC 8463 — Ed25519 verify directly over
the cached SHA-256 digest.
- Verifies DMARC alignment against the cached DMARC bytes (or
falls back to strict `d=` ↔ From: equality when no DMARC
was cached).
- Binds the credential and flips status to `Succeeded`.
- **dnssec verifier**: chain/leaf split landed in a prior commit; this
commit consumes both halves.
- **DKIM/DMARC**: a few helpers raised to `pub(crate)` so smtp.rs and
submit_leaf.rs can compose the parts they need without re-running
the full single-pass verifier on a body we've already dropped.
- **main.rs**: `email_recovery_submit_dkim_leaf(arg)` update
endpoint added — anonymous (the nonce is the only authentication;
it's a 64-bit canister-issued secret bound to one pending entry).
- **.did** + regenerated FE bindings.
All 441 canister unit tests pass. The integration-test happy-path
fixture compiles but its runtime expectation (single-shot DKIM-leaf
bundle to prepare) is no longer correct under the two-phase flow —
the test needs a rewrite to exercise prepare → smtp_request →
submit_dkim_leaf in sequence. That rewrite ships with the FE work
that exercises the same endpoints.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Multiple lint failures across the stack were masked because earlier CI jobs (clippy) short-circuited before the later jobs (fmt, frontend-checks) got to run. Fix them all in one pass: - **clippy `doc_lazy_continuation`**: the `EmailRecoverySubmitDkimLeafArg` doc comment had a line starting with `+ signature blob)` — markdown saw the `+ ` as a list bullet and complained that the next lines weren't indented as continuation. Reflow the prose to drop the line break. - **clippy `unnecessary closure on Option::None`**: collapse a `ok_or_else(|| ...)` to `ok_or(...)` in submit_leaf.rs. - **fmt drift on existing files**: `dnssec/mod.rs`, `prepare.rs`, `submit_leaf.rs`, `email_recovery.rs`, plus a few outside the email-recovery surface (`openid/generic.rs`, `attributes.rs`, `oidc_configs.rs`) that previously slipped past CI. `cargo fmt` noop, just whitespace reflow. - **frontend-checks tsc error**: the mock `InternetIdentityInit` literal in `iiConnection.test.ts` and the actor-construction call in `vc-flow/index/+page.svelte` were missing the `doh_config` and `dnssec_config` fields that landed in earlier PRs in this stack. Add them as `[]` (the `opt opt T` shape's empty form). All 441 unit tests still pass; FE `npm run lint` is clean (0 errors); `svelte-check` reports 0 errors; `cargo fmt --check` clean; `cargo clippy --workspace --all-targets -- -D warnings` clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The integration test \`full_setup_flow_via_dnssec_path\` was still
sending a DKIM-leaf bundle to \`prepare_add\`, which the canister
now rejects (skeleton bundles only carry DMARC at most; the DKIM
leaf belongs in \`submit_dkim_leaf\` post-email). Six shards of
canister-tests-run failed on it.
Rewrite the test to drive the full two-phase flow:
1. prepare_add with skeleton + DMARC leaf
2. smtp_request — body validates, status flips to NeedDkimLeaf
3. assert NeedDkimLeaf{selector} matches the test selector
4. submit_dkim_leaf with the DKIM leaf bundle → returns
RegistrationSucceeded
5. polled status confirms; remove path works
Also adds the matching \`email_recovery_submit_dkim_leaf\` helper
to canister_tests/src/api/internet_identity.rs.
The mod.rs preamble said the recovery half lives in a follow-up PR because it needs a reverse address index. That PR landed as the flow branch (#3843) and is part of the same stack now, so the note read as out-of-date in the cumulative diff. Trim it to point at where the recovery half actually lives. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The off-chain SMTP gateway calls a query method at RCPT TO time, before it pulls the message body from the sending MTA, to decide whether to accept the connection at all. The PoC defined this as \`smtp_request_validate\` and gated acceptance on the user-part parsing as a numeric anchor number — so on the email-recovery deploy, the gateway rejects \`register@id.ai\` and \`recover@id.ai\` outright, and \`smtp_request\` is never called. Bring the query back, with the email-recovery-specific shape: - \`register@id.ai\` (case-insensitive) → Ok. - \`recover@id.ai\` (case-insensitive) → Ok. Recognised at the validate query even on this PR (where the actual handler lives in the recovery follow-up #3843), since a "yes accept" here with no handler is harmless and avoids a deploy-step ordering constraint between this PR and the recovery one. - Everything else → 550 (mailbox unavailable). Numeric postbox- style addresses are no longer handled; the gateway should bounce them. The query is open (anyone can call it) but has no side effects and leaks nothing beyond the two recipient labels themselves, both already documented in the design doc. Six new unit tests cover register / recover / case-insensitivity, unknown user, known user with wrong domain, missing envelope. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The off-chain SMTP gateway routes mail at a domain that varies per deploy: id.ai on prod, beta.id.ai on beta. The canister was hardcoding `id.ai` everywhere — recipient dispatch, the validate query, and the user-facing label returned from prepare — so on the beta canister mail to `register@beta.id.ai` reached the canister but failed the recipient match and was silently dropped. Drop the hardcoded constant. Derive the accepted mailbox domains from `related_origins`, which is already a per-deploy arg the deploy scripts wire through (and the same one used for security headers + the FE's `getPrimaryOrigin`). All entries are treated as equal aliases — recipient dispatch and the `smtp_request_validate` query accept `register@<host>` / `recover@<host>` for any host listed in `related_origins`. So a prod deploy with `id.ai` + the `*.icp0.io` aliases accepts mail at all of them; a beta deploy with `beta.id.ai` accepts that one. Drop the `mailbox` field from `EmailRecoveryChallenge` too. The FE already knows which origin the user is on (`window.location.hostname`), so it pairs that with `register` / `recover` to render the label — each tab automatically shows the alias matching the origin the user is on, and the canister never has to single one out as canonical. Empty / unset `related_origins` → no domains accepted; the canister drops every inbound recipient. Real deploys always configure this. Tests: extended `email_recovery::smtp::tests` with `set_related_origins` helper + 15 cases (single host, multi-alias prod, beta-only, unknown user, wrong domain, no-origins-configured); all pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The bundle's `chain: Vec<DelegationLink>` / `leaf: Option<SignedRRset>`
got replaced by `chains: Vec<DelegationChain>` / `hops: Vec<SignedRRset>`
when the multi-zone + CNAME-aware verifier landed in PR 1. Bring the
storage-and-smtp consumers in line:
- internet_identity_interface::types::dnssec: new bundle shape +
the new `DelegationChain { links: Vec<DelegationLink> }` wrapper,
CandidType-derived for the FE.
- internet_identity.did: matching candid type definitions.
- dnssec::types::interface_conversions: From<> bridge for the new
shape (CandidType-derived i_types → verifier-internal types).
- email_recovery::prepare::verify_dnssec_skeleton: was using the
old `verify_chain_with_clock(bundle, anchors, now)` (returned the
zone DNSKEY directly) plus `verify_leaf_against_dnskey(...)` for
the optional DMARC. Now uses the new entry points:
1) verify_root_dnskey_with_clock for the anchors check,
2) verify_extra_chains_with_clock to walk the chains into a
ZoneKeysMap,
3) verify_hops_with_clock for the optional DMARC hop.
Caches the single skeleton-zone DNSKEY for the submit phase via
ZoneKeysMap::iter() (added in PR 1).
- email_recovery::submit_leaf::run_submit: was calling
verify_leaf_against_dnskey directly against the cached zone
DNSKEY. Now wraps the cached DNSKEY in a 1-entry ZoneKeysMap and
the DKIM leaf in a 1-hop slice, calls verify_hops_with_clock.
- integration tests: update the three places that construct a
`DnsProofBundle` literal in tests (allowlist-precedence test,
happy-path-via-dnssec helper, in-test signer's `skeleton` field).
460 tests pass; cargo clippy clean with -D warnings.
33b1582 to
071f85c
Compare
9f2b62d to
3884a47
Compare
aterga
added a commit
that referenced
this pull request
May 13, 2026
…of email-recovery stack) (#3878) ## Summary PR 3 of the email-recovery stack (`docs/ongoing/email-recovery.md` §6). Stacks on top of #3877 (DKIM verifier). Lands a hand-rolled DMARC alignment check and reshapes the verifier API: `dkim::verify_dkim` becomes a DKIM-only primitive, and the new `dmarc::verify_email` is the public top-level entry point that produces the combined `EmailVerificationStatus`. **Note:** This PR targets `main` but includes PRs 1+2's commits as its base. Review the DMARC-specific changes by looking at commits on top of `ec371aae3` (PR 2's tip). Once PRs 1+2 merge, this PR's diff shrinks to just the DMARC additions. ## What's in this PR ### `src/internet_identity/src/dmarc/` - **`types.rs`** — `DmarcOutcome` (Aligned / Misaligned / NoRecord / Malformed), `DmarcPolicy` (None / Quarantine / Reject), `AlignmentMode` (Strict / Relaxed), `DmarcRecord`, plus the combined `EmailVerificationStatus` that carries both DKIM diagnostics and the DMARC outcome on success. - **`parse.rs`** (RFC 7489 §6.3) — DMARC TXT record parser. Enforces `v=DMARC1` must be first, `p=` must be one of {none, quarantine, reject}, `pct=` 0..=100, rejects duplicate tags, ignores unknown / reporting tags. 12 unit tests. - **`from_header.rs`** (RFC 5322 / RFC 7489 §3.1.1) — single-mailbox From-header parser. Accepts bare addr-spec, name-addr, and quoted-display-name forms; rejects zero/multiple From: headers, address-lists, group syntax. Tolerates comma/colon inside quoted display names. 16 unit tests. - **`alignment.rs`** — strict (exact match) + relaxed (exact match OR label-aligned subdomain in either direction). Stricter than RFC-compliant relaxed alignment because we deliberately don't consult the PSL — design doc §6.4 documents the trust + asymmetric-failure-mode reasoning. The dot anchor on the subdomain check prevents `evilexample.com` from aliasing `example.com`. 8 unit tests. - **`verify.rs`** — orchestration. DKIM first; on failure, surface the DKIM reason verbatim. On DKIM pass, parse From and check DMARC alignment. Accepted iff Aligned, OR NoRecord with `dkim_domain == from_domain`. 8 unit tests. - **`test_vectors.rs`** — 5 end-to-end tests reusing PR 2's synthetic .eml fixtures. ### `src/internet_identity/src/dkim/types.rs` (rename + new variants) - Renamed `EmailVerificationStatus` → `DkimVerifyResult` (DKIM-only). The combined verdict moved to `dmarc::EmailVerificationStatus` so it can carry the `DmarcOutcome`. - Added `MalformedFromHeader(String)`, `DmarcMalformed(String)`, `DmarcMisaligned` to `VerificationFailReason`. ### `src/internet_identity/src/dkim/mod.rs` - Re-exports `verify` as `verify_dkim` so downstream callers (the dmarc layer) don't have to deal with both a `dkim::verify` and `dmarc::verify` in scope at the same time. ## Test plan - [x] `cargo check -p internet_identity --target wasm32-unknown-unknown` — clean. - [x] `cargo test -p internet_identity --bin internet_identity dmarc` — 49 tests pass (12 parse + 16 from_header + 8 alignment + 8 verify + 5 e2e). - [x] `cargo test -p internet_identity --bin internet_identity` — 365 tests pass total (was 313 with PR 2; +49 dmarc + 3 small reshape adjustments). - [x] `cargo clippy -p internet_identity --bin internet_identity --tests -- -D warnings` — clean. - [x] `cargo fmt --check` — clean (modulo pre-existing unrelated diffs). ## PR Stack | # | PR | Description | Status | |---|---|---|---| | 0 | [#3836](#3836) | Design doc | Open | | 1 | [#3838](#3838) | DNSSEC verifier scaffold | Open | | 2 | [#3877](#3877) | DKIM verifier (RFC 6376) | Open | | 3 | [#3878](#3878) | DMARC alignment (RFC 7489) | Open | | 4 | [#3879](#3879) | DoH fallback | Open | | 5+6 | [#3880](#3880) | Setup flow (storage + smtp_request) | Open | | 7 | [#3881](#3881) | Recovery flow (delegation) | Open | | 8 | [#3882](#3882) | Frontend + feature flag | Open | | 9 | [#3883](#3883) | Deploy/upgrade scripts: dnssec_config + doh_config | Open | | 10 | [#3884](#3884) | Email-recovery UX overhaul | Open | --------- Co-authored-by: Arshavir Ter-Gabrielyan <arshavir.ter.gabrielyan@dfinity.org> Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR 5+6 of the email-recovery stack — the storage layer plus the canister-side surface for the setup (binding) flow. Stacks on #3879 (DoH fallback).
The recovery half (
prepare_delegation,recover@id.aidispatch, delegation issuance) is deferred to a follow-up PR — it needs a stable reverseaddress → AnchorNumberindex for the verified-From:lookup, which is best landed as its own focused change.What this PR adds
Storage:
EmailRecoveryCredentiallives directly on the anchor struct asOption<EmailRecoveryCredential>via a new#[n(5)]field onStorableAnchor. minicbor is forward-compatible across optional-field additions, so old anchors deserialize cleanly withNone— no schema migration.raw_rand. Per-call cost is one PRNG draw, not a management-canister round trip.Canister surface (Candid):
email_recovery_credential_prepare_add(IdentityNumber, EmailRecoveryDnsInput) -> Result<EmailRecoveryChallenge, EmailRecoveryError>— authenticated update.email_recovery_status(text) -> EmailRecoveryStatus— anonymous query (FE polls).email_recovery_credential_remove(IdentityNumber, text) -> Result<(), EmailRecoveryError>— authenticated update.smtp_request(SmtpRequest) -> SmtpResponse— open update; the gateway entry point.Two verification paths (FE is path-agnostic):
The FE just submits
{ address, selector, dns_proof: Option<DnsProofBundle> }. The canister picks the path:dns_proofisSomedns_proofisNonecrate::doh::fetch_txt(3-of-5 quorum, heap-cached)DomainNotSupported/DomainNotAllowlistedThe DNSSEC path validates the chain synchronously against
DnssecConfig.root_anchorsand confirms the bundle's leaf is at<selector>._domainkey.<domain>. Candid-friendly mirror types (DnsProofBundle,SignedRRset,Rrsig,DelegationLink) are defined in the interface crate withFrom<>conversions to the canister-internal verifier types.Verification pipeline (smtp_request):
register@id.ai) — defence-in-depth against direct callers spoofing just the user-part.Subject:header (case-insensitive prefix match, hex-suffix length-checked)._dmarc.<domain>; absence forces strict alignment in the verifier.crate::dmarc::verify_emailend-to-end (full DKIM + DMARC +Subjectinh=).Succeeded. Failures flip toFailed(reason). The gateway always seesOk.What this PR does NOT add (intentional, deferred to follow-ups)
prepare_delegation,recover@id.aidispatch, delegation issuance. Needs a reverse address index in stable storage; cleaner as its own PR.Test plan
cargo check --target wasm32-unknown-unknownclean.cargo clippy --tests -- -D warningsclean.cargo fmt --checkclean..didinterface stays in sync (the existingcheck_candid_interface_compatibilitytest enforces this)._domainkeychain).🤖 Generated with Claude Code
PR Stack
Changes during review
email_recoveryfield is now aVec(Option<Vec<...>>on the storable form, matching the existingpasskey_credentials/recovery_keyspattern). The canister API still enforces ≤1 entry — this is a data-model widening so future multi-credential support doesn't need another schema bump.Option<Vec<T>>means anchors written under the previous schema decode cleanly withemail_recovery: None(verified empirically against the in-tree minicbor-derive version).